Convert caddr_t to char * for pointer arithmetic. (@glebius)

@glebius this revision is an extention of GRAND which only affects anycast and proxy ipv6 addresses.
Could you please review it c-wise?

@bz I would really appreciate it if you could review this when you have some free time.

Thank you, I'll review this revision after you address Gleb comments.

Yes, I made a stupid mistake. Thank you!

In D55599#1272617, @glebius wrote:

A general rule of thumb: you ever want to use IS_DEFAULT_VNET(curvnet) in some stub network module - you are doing something wrong :)

Since @markj just announce that he's AFK, could you take a look at this please? @glebius

In D53484#1272251, @zlei wrote:

1. People should not use RIP anymore. I think that is not correct. I still see low end routers / l3 switches ship basic implementation of RIP / RIPng. That is an easy protocol to setup small network, without manipulating dozens of static routing entries across multiple routers / l3 switches. So why people want more complexed IGP protocols or to buy expensive routers / l3 switches those support more EGP / IGP protocols ?

In D55602#1272170, @bms wrote:

Any plans to add stateless offload support as has been done by @np for vxlan(4) ?

FYI, Steps to reproduce:

# vmstat -m | grep ifaddr
ifaddr   38 16352   50 16,32,128,256,384,1024,2048,4096
# jail -c persist name=vnet1 mount.devfs devfs_ruleset=5 vnet
path=/
# vmstat -m | grep ifaddr
ifaddr   43 23808   55 16,32,128,256,384,1024,2048,4096
# jexec -l 4 ip6addrctl add 2a01:e140::/32 60 20
# vmstat -m | grep ifaddr
ifaddr   44 23936   56 16,32,128,256,384,1024,2048,4096
# jail -r 4
# vmstat -m | grep ifaddr
ifaddr   40 16736   56 16,32,128,256,384,1024,2048,4096

128B memory leak for each undeleted ip6ctl policy in vnets. + lock leaks

In D55593#1271953, @zlei wrote:

@pouria

MFC to: stable/15

I have not tested yet, but I believe stable/14 and stable/13 are also affected.

In D55593#1271927, @zlei wrote:

I'd rather prefer this, to ease my brain ;)

diff --git a/sys/netinet6/nd6_rtr.c b/sys/netinet6/nd6_rtr.c
index ba2f025b7db1..3a6f5f501f8f 100644
--- a/sys/netinet6/nd6_rtr.c
+++ b/sys/netinet6/nd6_rtr.c
@@ -1370,7 +1370,7 @@ nd6_prelist_add(struct nd_prefixctl *pr, struct nd_defrouter *dr,
        new = malloc(sizeof(*new), M_IP6NDP, M_NOWAIT | M_ZERO);
        if (new == NULL)
                return (ENOMEM);
-       refcount_init(&new->ndpr_refcnt, newp != NULL ? 2 : 1);
+       refcount_init(&new->ndpr_refcnt, 1);
        new->ndpr_ifp = pr->ndpr_ifp;
        new->ndpr_prefix = pr->ndpr_prefix;
        new->ndpr_plen = pr->ndpr_plen;
@@ -1410,8 +1410,10 @@ nd6_prelist_add(struct nd_prefixctl *pr, struct nd_defrouter *dr,
 
        if (dr != NULL)
                pfxrtr_add(new, dr);
-       if (newp != NULL)
+       if (newp != NULL) {
+               nd6_prefix_ref(new);
                *newp = new;
+       }
        return (0);
 }

@markj How about the above proposal ?

LGTM, thank you

Address Gleb comment.

Fix multipath expire values too.

Add support for nexthop expire value on multipath routes.
route(8) and netstat(1) will be updated too.

The patch allows interface renaming (by rc.conf(5)) for epair (cloned) interfaces and also for devd(8) configured interfaces.
'ifconfing description' is changed to be always applied (if defined), whether or not the NOAUTO keywork was found.
It also fixes non-functional NOAUTO config keyword for targeted start_interface commands (by devd(8)).
And in addition makes NOAUTO the default for devd(8) configured interfaces if rc.conf(5) lacks a corresponding interface configuration.

In D50968#1270115, @kp wrote:

This is already in the tree, as cd0169c9379c400ec75b77e87ca770e37f964276. I managed to forget to add the 'differential revision' tag, so Phabricator didn't notice.

Could you please update this revision according to D51677.
This change looks good.
I'll review it if you update it again.

@meta
IMHO, You may proceed again with 'Co-Authored-By: full name <email>' tag if and only if D51866 stalled again for another month.
I pinged the original revision to see if author is available to address @zlei and your comments or not.

This is a good work.
Please address comments or concerns on D52594 to make this revision ready for review again.

Adding both singular and plural form is good to me.
But adding another icmp6types is redundant.

I like the initial idea, but this revision requires changes to follow 5ead817c3b7af

In D48646#1109792, @afedorov wrote:

I think the only reliable method is to remove the creation of dynamic sysctl altogether and use ioctl(2) or netlink(4) to get statistics and "routing" tables. Or create them only in the host VNET. But this violates POLA.

As for this review, since it solves at least some of the problems that real users face, then of course it should be commited.

In D53484#1223559, @ivy wrote:

In D53484#1223366, @glebius wrote:

Note that there are tests that use routed :(

oh, i missed that; i'll investigate and update the diff. thanks.

I've been using this along with my other v6 changes on our servers, IXP routers, and on my laptop for about a week now.
I haven't found any issues!

Thank you

In D43213#1269045, @rscheff wrote:

Personally I would like a TCP session NOT fail because it has ECN negotiated over a virtio driver, who then throws an error when a TSO superframe with CWR is handed down...

I agree.
I think we should talk about this on next transport meeting.

Are you waiting for Andrew's feedback from vendor?
Could you please set the planned change state for this revision?

Is this still relevant?
If that's the case, could you please set the planned change state?

Please address Richard's comment.

Are you still working on this? @glebius
if that's the case, Could you please set the planned change state for this revision?

Are you still working on this? @zlei
Could you please set the planned change state for this revision?

I also agree with Randall and Tuexen.
I hate phabricator dashboard since it's filled with many revisions, and I have to rely solely on my emails to see what I need to review.
Which is not fair to older but still relevant revisions that need reviews.
Therefore, could you please abondon this revision if that's the case?

Is this revision still relevant?

This revision is committed. Shouldn't be open.

LGTM, any concern related to this patch is revealing a design issue and IMHO, that's not something that would block this revision.

LGTM

In D55398#1267129, @ae wrote:

When you issue ioctl that does some changes in softc, another thread can do transmission in the same time and use some data from softc.
To avoid access to not yet fully modified or already freed data you need to block transmission.
Or you can allocate new gre_priv structure for each modification and free old one using NET_EPOCH_CALL(). This allows avoid blocking of transmit thread and it uses always stable data while it works in net_epoch.

I understand, I mean, personaly, I'd prefer to block transmission since reconfiguration of gre interface is rare in comparison to transmission.
This is what all other interfaces do. IMHO, it's not worth the extra complexity.

I think that's how it happened historically, for some interfaces we have different files of INET and INET6 implementation.

When I was working on implementing netlink for GRE, I noticed that the GRE implementation has too many layers of indirection due to its historical context.
It's not important and just an idea, but would you like to merge them under sys/net? I can work on it if you want.

In D55316#1265083, @glebius wrote:

Immediately caught in bridge(4):

NET_EPOCH_ENTER(et);

callout_drain(&sc->sc_brcallout);

fix missed skip.

style commented to use #ifdef without functional change.

Some other notes for reviewers: (see description above too please)

It also resolves one of the issues in PR263982.

there is one in sys/net/route/route_helpers.c -> ip6_writemask

LGTM

LGTM

In D54443#1267143, @ngie wrote:

Are you using kyua from ports or base?

I don't understand why we need a separate data structure (priv).
I apologize if this seems like a stupid question, but wouldn't it be better to move ip6_gre.c and ip_gre.c into if_gre.c itself like other interfaces?

In D54443#1267022, @pouria wrote:

However, I see gcc14 complaining too:

/workspace/src/tests/sys/netlink/test_rtnl_gre.c: In function 'atfu_test_rtnl_gre_body':
/workspace/src/tests/sys/netlink/test_rtnl_gre.c:91:28: note: at offset 8 into source object 'src' of size 16
   91 |         struct sockaddr_in src, dst;
      |                            ^~~
In function 'snl_add_msg_attr',
    inlined from 'snl_add_msg_attr_ip' at /workspace/src/sys/netlink/netlink_snl_route.h:153:11,
    inlined from 'atfu_test_rtnl_gre_body' at /workspace/src/tests/sys/netlink/test_rtnl_gre.c:117:2:
/workspace/src/sys/netlink/netlink_snl.h:1166:17: error: 'memcpy' reading 16 bytes from a region of size 8 [-Werror=stringop-overread]
 1166 |                 memcpy((nla + 1), data, attr_len);
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/workspace/src/tests/sys/netlink/test_rtnl_gre.c: In function 'atfu_test_rtnl_gre_body':
/workspace/src/tests/sys/netlink/test_rtnl_gre.c:91:33: note: at offset 8 into source object 'dst' of size 16
   91 |         struct sockaddr_in src, dst;
      |                                 ^~~

working on it.

In D54443#1266960, @siva wrote:

It seems that the test added by this commit is failing in CI: https://ci.freebsd.org/job/FreeBSD-main-amd64-test/27898/testReport/sys.netlink/test_rtnl_gre/test_rtnl_gre/

@pouria can you take a look?

Here is the patch:
https://github.com/spmzt/freebsd-src/commit/1635ba90615a5d9342604d495b71ac5380030b36
I'm not sure why it tells unknown property.
But I'm sure the reason for CI failure is the if_gre is not loaded.
@siva

In D54443#1266960, @siva wrote:

It seems that the test added by this commit is failing in CI: https://ci.freebsd.org/job/FreeBSD-main-amd64-test/27898/testReport/sys.netlink/test_rtnl_gre/test_rtnl_gre/

@pouria can you take a look?