In D50019#1141170, @ivy wrote:

here's the thing though, you can already accept TCP connections to an anycast address by simply not marking the address as IFF_ANYCAST, and this is what everyone does today. if you force users to set a sysctl to do this, they will just not bother marking addresses as IFF_ANYCAST and will be more likely to run into problems from e.g. accidentally originating outgoing connections from an anycast address.

or in other words, the choice here isn't "should we let users accept TCP connections to anycast addresses?" because it's impossible to prevent that, it's "should we allow users to correctly mark their existing anycast addresses as anycast addresses?". i don't see any advantage to forcing users to set a sysctl in order to configure networking more correctly.

more sysctls is more confusing and creates more pain for users, removing special cases and unnecessary code is more beautiful. :-)

however, i would be willing to add some text to ifconfig.8 to mention the potential issues with TCP anycast and refer them to some appropriate documentation.

How about a compromise here... I do think Michael has a valid point...

Only question I have is why the change from hlen -> sizeof(struct ip6_hdr)

To me this is marginally less readable in the changed form. Both are much better than the original but considering your comments about what the compiler actually does I suggest we leave it the way it is!

Note I do agree with Gleb here, these macros are just nasty and not very clear. However I think that making such a change should be a separate commit from this one!

I agree with Peter :)

old code :)

Are you sure this condition cannot be reached?

In D47925#1095113, @glebius wrote:

Later I'd probably bring the input and output to same KBI, so that both return same signedness error for unlocked return. But this requires a sweeping change over stacks, so later.

Ok after a detailed analysis:

I agree with Michael here, macros badly hide things.. which is their intention :)

Fix what I thought I had fixed i.e. Unused_71 .. but for some reason I missed it ⭕

go through all the enum's and mark UNUSED those that are not used.

I am fine with this Drew, just went through and verified the departure code running earlier should have no impact...

This changes the enum, as discussed on a previous conf call to be "UNUSED" still taking the slot. I don't necessarily
agree with that but if everyone else wants it I am fine with it :)

Take out inadvertent space adds.

This gets rid of the defines mentioned by Michael in comments and as prescribed by Gleb on the last TCP conf call i.e.
we comment them out like in other code.

rcv-ack-finwait-1-ipv6 PASSED
rcv-fin-finwait-1-ipv4 PASSED
rcv-fin-finwait-1-ipv6 PASSED
rcv-fin-finwait-2-ipv4 PASSED
rcv-fin-finwait-2-ipv6 PASSED
rcv-ack-closing-ipv4 PASSED
rcv-ack-closing-ipv6 PASSED
rcv-ack-last-ack-ipv4 PASSED
rcv-ack-last-ack-ipv6 PASSED
rcv-ack-timewait-ipv4 PASSED
rcv-ack-timewait-ipv6 PASSED
rcv-fin-timewait-ipv4 PASSED
rcv-fin-timewait-ipv6 PASSED
rcv-syn-timewait-ipv4 PASSED

rcv-syn-timewait-ipv6 PASSED

Summary: Number of tests run: 881

Number of tests passed:                 849
Number of tests failed (expected):       32
Number of tests failed (unexpected):      0
Number of tests timed out (expected):     0
Number of tests timed out (unexpected):   0
Number of tests skipped:                  0

Committed it: fce03f85c5bf

Address all of Richards comments including the one I had to think about it was correct :)

One more update, need to use the tcp function call not t_maxseg - 12 to get the maxseg for filtering purposes

Add detailed comments so it is more clear what is going on inside sack_filter.h

One minor update my user space test did not show a new unused var that my kernel compile shows up. Fix that and
update the test information since I now validated the kernel version as well.

Updated the diff to fix some corner cases and to better handle compile and testing in user space.