I left you a sample change which should be good enough for the IPv4 frag reassembly to no longer trip over VNET shutdown problems and is independent on when things will happen and independent on any SI_SUB_* ordering.

I am sorry, was I not clear enough in email that we actually need none of this changed (neither the previous commits nor any of this) in order to solve your original problem? Can we please try not to further touch what was a working system (or complicate it more).

In general I like the change very much. See the minor comment for sanity checks in the internal implementation.

If you are happy, just go ahead and commit it. I didn't boot it.

Okay. I see why the vnet0 state is not updated. I'll take that as a different problem as it requires a bit of trickyness as it goes through the code module loading also uses.

In D19622#439705, @jtl wrote:

In the interests of avoiding discussion fragmentation, I am adding the feedback from the transport working group meeting today.
@hselasky raised this on the working group call and solicited feedback. The strong (seemingly-unanimous) feedback on the call was that we should not discard fragments when the associated interface goes away. Some points that were raised include:

• Systems which have multiple interfaces can receive different fragments over different interfaces. Therefore, the other fragments may still arrive over other interfaces.
• Systems with flapping interfaces may receive fragments after the interface recovers.
• It is not unheard of for systems to queue fragments for multiple seconds. Therefore, it is possible that the fragments will arrive once connectivity is restored and/or rerouted.

It was the strong feeling of the participants on the call that this problem should be solved in a way which preserves the fragments on the queue. Some suggestions included:

• At the time of enqueueing, copying the information from the ifnet pointer which is necessary to process the packet. Save this information, rather than the ifnet pointer.
• When an interface goes away, clear the ifnet pointer from the fragments on the queue. Skip processing steps which depend on this information (which @hselasky described as incrementing interface statistics and sending an error response when a fragment is dropped).

In general this seems good.

In D19111#476113, @glebius wrote:

I'd like to go forward with this change. It will bring a number of new epoch recursions, that don't exist today, but I want to resolve them separately, one per subsystem, otherwise the changeset will get bigger and more messy. I need your review, guys. :)

This may sound like a pain but as you already say in your message, this is two changes .. First .. Second ..
Can you split them up into such? First should be really easy to review and second should then be straight forward as well by itself.

Adding TCP folks explicitly as reviewers.

There's a couple of public IP(v6) addresses in the test scripts. We'd prefer not to have accidents with people. Can you please change them?

In addition to the other comments I still nowhere see a loop that walks the entire extension header chain. If you don't do that you cannot fulfil the requirement you are trying to solve. What am I missing?

I changed my mind...

Just as a comment: I finally figured out what's so confusing when reading this these days. The ip6q[] is the buckets and not of struct ip6q anymore; sadly struct ip6q is a public interface used inside the MAC framework so don't want to break that by renaming it.

Grat work on the cleanup; I think apart from the license there is very few minor nits left which are acceptable.

In D20967#456233, @shivank wrote:

I have a few doubts:

• I'm not clear about the license, should the TrustedBSD be included? if yes, then how? Also, I have copied the sysctl_rules from mac_portacl, Is it infringing any copyright as of now? I've read BSD license is very open, can I mention the mac_portacl?

Hey, thank you for all the updates. There are a few more. Please let me know when you think you are done with all of them and I'll have a full look again.

I think sys/security/mac_ipacl/design_notes.txt and sys/security/mac_ipacl/notes.txt can be removed. Should be turned into a man page really!?

Thanks, was still on my TODO. Always happy if someone does the work for me :-)

I have a kind-of stupid question: where in all this do you actually walk an entire header chain to the end rather than just checking the next one? Are there no cases when this might be needed?

In D17512#445767, @bdragon wrote:

Over in powerpc land, we've been chasing crashes with dpcpu and vnet as well. Our current prospective fix is https://reviews.freebsd.org/D20461 but we don't understand why it seems to fix it.

Does this patch fix your issues?
https://svnweb.freebsd.org/base?view=revision&revision=336909 has a brief explanation of the problem; can you look at the code and see if you get a similar problem, just to confirm (and for you to understand as-to why that is?

I'd like to commit this the next days. Can someone please review? If not it'll go in by Sat 8 June.

I'd like to commit this the next days. Can someone please review? If not it'll go in by Sat 8 June.

Warner if you have any further comments .. I'd like to commit this the next days. If not it'll go in by Sat 8 June.

I'd like to commit this the next days. Can someone please review? If not it'll go in by Sat 8 June.

I'd like to commit this the next days. Can someone please review? If not it'll go in by Sat 8 June.

https://svnweb.freebsd.org/changeset/base/348494 missed the Diff-Rev: tag.

I just only scrolled through this and there's a few things. I am not going into detail at the moment as I'd love to give this a full review once my brain capacity for yet-another-module-thing-complication is there. It'll be helpful to later decompose this into "whitespace change" and functional change (as a lot of renaming, etc. makes this hard to read).

In D19622#441636, @tuexen wrote:

In D19622#440454, @kbowling wrote:

I'll register strong desire to discard fragments immediately on interface vanishing. I don't expect that to come with any significant weight, my rational is whatever makes state management easier within current codebase and usage patterns of fbsd.

Why do you want to discard a packet if the interface it was received from is going away?
The packet is there, it doesn't matter if the interface goes away after it was received. And we don't have to wait for the interface to come back before continuing processing. We should be able to just process the packet even without that interface it was received on still being present when the processing happens.

In D17512#439294, @jhb wrote:

I still don't understand why i386 is special in this regard. Surely i386 isn't the only architecture to ever use this type of relocation?

so this happened as well with ipsec.ko not just carp.ko so we need a solution to the problem which scales. We discussed it here a bit in Waterloo and I think the only question is as to whether to use the BYTE(1) option and just put a padding byte at the end, or as to whether put a larger magic number there and actually check that the module might be compiled with the right options?

Did you also want something for ipcomp?

Thank you!!!! And for the .Dd changes discussed in person :-)

LGTM

I kept thinking which states we can really observe and when:

Handle reviewer comments:

• add SDIO disclaimer
• fix typos
• split up CMD53 function
• add one device per function (and with that re-adjust to use the parent device as we no longer cache that, add ivar support rather than having hand-rolled accessor functions)
• Use CMD52 to set blocksize
• use a sdio specific awk to generate device and vendor defines
• remove the DEFAULT implementations from the sdio_if.m file as they were only providing the general default
• rework some error handling
• rename field "smb" into more descriptive
• Use dprintfs instead of printfs in the middle of a parsing API

I think a dependency was rejected. This was Cambridge Teaching material code. I would assume this is overcome by time and changes a lot since. I have no more access to these trees where this came from to see. I am abandoning it to clean up.

I've fixed all requests in my internal tree but the CMD53 request (which I left a comment for what I'll do).
I'll upload a new diff the next 48 hours (need to also adjust the driver to deal with multiple devices).

The ip6_fragment() change seems fine if you want to go ahead and factor that out.