In D56288#1287849, @pouria wrote:

This LGTM, but here's an idea: why not do it the other way around?

This is part of the cleanup.

Landed D55875 after running extensive tests.

Rebased on latest main.

Looks good to me.

In D55875#1278350, @pouria wrote:

I'm not sure how these assertions will be beneficial if we have to add them to every function of the ifp [un]linking.
IMHO, KASSERT/MPASS are existing to verify programmatic assumptions.

Looks sane to me.

Looks good to me.

PS: I bought a USB adapter ( AX88179A ) but were not able to repeat ( with stress tests for about two days ).

Looks good to me.

I have not looked through the related RFC. So this quick review is not completed.

Reasonable !

Looks good to me.

I do not have hardware to test this change, but the code looks good to me.

Tested with a copper interface and SFP+ with a DAC.
media: Ethernet autoselect (10Gbase-LR <full-duplex>)

Looks good to me.

In D55832#1278488, @zlei wrote:

Superseded by D55880.

In D55887#1278756, @pouria wrote:

In D55887#1278653, @zlei wrote:

I have not looked into the tests. Since the Jumbogram feature has been removed in the inet6 stack, shall the relevant tests be updated to reflect the removal but not removing the tests ?

Could you please explain more?
Is there any other jumbogram tests that I missed or you mean renumbering the fragment tests?
If it's the latter, I don't like to mix that one with this change because it would affect the git blame result of all the other fragment tests.
I'll do that in a separate revision and then I can add that commit into .git-blame-ignore-revs to avoid its side-effects.

In D55885#1278789, @pouria wrote:

In D55885#1278785, @pouria wrote:

In D55885#1278782, @zlei wrote:

The link-local check here is also redundant. You have already checked the scope of the address in nd6_grand_start(). So if the address is global one, it must not be a link-local one.

nd6_grand_start(struct ifaddr *ifa, uint32_t flags)
{
...
        /* Check if new address is global */
        if ((flags & ND6_QUEUE_FLAG_NEWGUA) != 0 &&
            in6_addrscope(IFA_IN6(ifa)) != IPV6_ADDR_SCOPE_GLOBAL)
                return;
...
}

You're right! what a brain glitch!
Thank you!

Oh no, I reacted too fast, this shows I don't trust my own memory :)
This check only applies for new address event, not link-layer address change.

Removed the changes to comments.

The link-local check here is also redundant. You have already checked the scope of the address in nd6_grand_start(). So if the address is global one, it must not be a link-local one.

In D55884#1278698, @glebius wrote:

In D55884#1278650, @zlei wrote:

The ROUTE_MPATH still has subtle performance impact and the kernel built with this option has larger text size. I'd expect some people want to keep this option so they can disable it.

What is the performance impact? AFAIU, it is just array of fibs that by default has only one element.

I have not looked into the tests. Since the Jumbogram feature has been removed in the inet6 stack, shall the relevant tests be updated to reflect the removal ?

I'm for reduce routing complexity, if it is possible to keep that option.

The ROUTE_MPATH still has subtle performance impact and the kernel built with this option has larger text size. I'd expect some people want to keep this option so they can disable it.

In D55880#1278503, @glebius wrote:

Correct me if I am wrong, but this seems to be two separate changes. Can this be split please?

Superseded by D55880.

In D55682#1278294, @rkitover_gmail.com wrote:

Apologies if I'm being impatient, but when does the patch go into the tree?

In D55828#1277603, @pouria wrote:

In D55828#1277300, @zlei wrote:

Ah, I have WIP to fix the race condition between if_detach() and if_vmove(). See PR 292993 .

Oh, I was working on that problem too.
I thought you had seen D55777 earlier, you're on the reviewers list.
I can release the PR and assign it to you to avoid duplicating effort, if you want.

In D55828#1277278, @glebius wrote:

In D55828#1277277, @zlei wrote:

I though that a bit, and I think no need for the assertions.

So your point is that as we see that ifioctl() already did this check the assertions are not needed, as we already know that they will never fire.

In D55828#1277278, @glebius wrote:

In D55828#1277277, @zlei wrote:

I though that a bit, and I think no need for the assertions.

So your point is that as we see that ifioctl() already did this check the assertions are not needed, as we already know that they will never fire. But that's the point of assertions - to put them so that they never fire :) Assertions also act as documentation.

I'm not insisting on assertions, just suggesting. My longer term plan is to get rid of if_vmove anyway.

The current logic of checking the src / dst vnet is a bit confusing. Refactor them a little. See D55832 .

In D55828#1277259, @glebius wrote:

I'd suggest to add assertions that VNET is not shutting down.

In D55828#1277269, @jamie wrote:

This makes sense in if_vmove_reclaim, where the vnet comes from the held prison.

I think I read the code carefully. Please do not hesitate to correct me if I missed anything important.

I'm almost certainly that leaving start_all_aps() public visible was unintended. Post to review for sure.

The git change made me crazy while I revisit this after one year -:)

In D53484#1272094, @ivy wrote:

In D53484#1270635, @bms wrote:

It's not strictly necessary, but extra karma if you do. It looks like @cy is tracking that as ports/net/freebsd-routed anyway.

well, i don't care about this code or RIP in general and i have no idea who Vernon Schryver is or what sort of diffs we might want to upstream, so i don't really want to be responsible for this. if someone else would like to take on that task, the code will still be available in our git history (and in the port, as you mentioned).

I wouldn't rule out RIP/RIPng entirely, as there may still be "sticking plaster" situations where they are required

people who still require RIP can use a modern implementation, such as in the ports net/bird3 or net/frr10. as far as i'm aware these both work well on FreeBSD and have good implementations of both RIP and RIPng.

i personally don't think anyone should be using RIP anymore, but if they want to, they can continue to do so; we just don't want to support this in src anymore.

@pouria

MFC to: stable/15

Looks good to me.

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