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.

Update based on feedback from marius/imp.

Adding @tuexen as a reviewer. If my memory serves me right he worked on a small part of of tun recently.

LGTM... waiting for more beef :-)

In D19622#430172, @hselasky wrote:

@bz: There is another bug I believe in the IPv6 VNET shutdown code.
Where is the counterpart mtx_destroy() for the following mtx_init():

void
frag6_init(void)
{

struct ip6q *q6;
int i;
V_ip6_maxfragpackets = IP6_MAXFRAGPACKETS;
frag6_set_bucketsize();
for (i = 0; i < IP6REASS_NHASH; i++) {
        q6 = IP6Q_HEAD(i);
        q6->ip6q_next = q6->ip6q_prev = q6;
        mtx_init(&V_ip6q[i].lock, "ip6qlock", NULL, MTX_DEF);
        V_ip6q[i].count = 0;
}

Sorry my bad for missing the VIMAGE state problem in the initial submit.

@emaste update the comment a bit.
@tuexen removed the KASSERT and left the sin = to be removed in a separate commit (as it is noise here)
@karels I think with the change we are fine as splitting up the case was for IP_SENDSRCADDR.

Seems fine to me.

In D19973#429776, @cem wrote:

In D19973#429748, @thj wrote:

I think we can just use zero for the label if enough entropy is not available. A flow label of zero indicates no flow label "treatment" in the network.

That’s easy and fine with me. Is that reasonable, @bz?

Thanks for persevering on this. Let's get this for now. I also think the discussion had in the last days was very fruitful. Either way, there is a way forward now beyond just the IP reassembly queues, whether we start fixing the ifp on the mbuf replacing it or adding more event handlers for other parts of the tree which need clearing.

Is there a notification for when we are fully seeded the first time and entropy is available? FlowIDs are a controversial subject as to what they mean and what they are good to use for (see various RFCs and drafts, especially from Brian Carpenter in the last years). BSD enable random flowids by default but we could equally, if not set by application, leave them 0 for the first few packets.

Again, can you you please mention rip6_input() in the commit message?

Again can you please mention "... in rip6_input() ..." in the commit message.

Can you please improve the description before committing stating that it's related to rip6_output().

In D19965#429167, @tuexen wrote:

This seems like four different issues and it's kind of hard to keep them apart in a single change. For the sake of having a readable history and easily seeing/understanding each problem, can you please split them up?

That said, there are vendors who list it (maybe not on the forwarding plane) but as RFC compliance supporting it:
https://www.juniper.net/documentation/en_US/junos/topics/reference/standards/ipv6.html

In D19960#429033, @tuexen wrote:

Maybe a very short ID to move RFC 2675 to historic... See RFC7805 for a ,much more complex example of such a document.

In D19960#429029, @jtl wrote:

I agree with @bz about ideal process. I also agree with @kristof about the practical implications of this feature. :-)
Might a valid way forward be to write a draft, propose it, and see if anyone complains by the next IETF (July) before purging this?

In D19960#429003, @kristof wrote:

Burn it with fire! This code is not used, and pretty much can't be used, so let's get rid of it.

I'l resign as a reviewer from this as removing valid RFC support seems counter-intuitive to me just because currently to your best knowledge no device supporting it exists. Someone should write the draft first to deprecate this and see if people stand up and then remove it.

In D19622#428071, @glebius wrote:

Sorry, I withdraw my approval. The more I think about this, the more I dislike it. What the patch does - it fixes the panic in a most straightforward way. But let's look at it from standpoint of network engineer, not kernel developer.

I think this looks fine now. I haven't given it a full read-down or test again (just going by the diff of changes and last comments).