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).

I only scrolled through and what I saw looked ok to me.

Scrolling through the kernel changes it looks good to me; not done an in-detail review on them.

OK, this seems fine to me.

In D4794#424935, @glebius wrote:

After I spent sometime around this code, I support melifaro@'s suggestion.

The descriptions needs slightly rewording "is requested". I'd be a bit more verbose maybe explaining distinct use of block_size and block_count?

This is missing files I think. The mmcreg.h structure changes are missing from this one. This won't compile.

LGTM

Use the correct field `sim_dev`` not `sim``.
Last minute cleanup changes are never a good idea without compiling/booting them,

@imp if you can confirm that the two changes (usdbdevs.awk -> sdiodevs.awk) and the disclaimers should be added I'll be happy to go ahead.

In D19745#423290, @andrew wrote:

I wonder if there is a way to make this generic as the dprintf or similar macro seems to be common in arm code.

Just adding three comments to point out the not-so-obvious obvious apart from the fact that
(a) unloading is not yet supported.
(b) a man page is missing.

Will be handled as part of kibab's D19674 .

Can we break this up into 2-3 changes please?
(1) Adding block_size and block_count to mmcreg.h and fixing the initialiers with memset or = 0; these changes are by themselves and undisputed and could get out of this.
(2) The change to the #defines in mmcreg.h can go in as well; they are not all strictly related to blk count from what I can see, so keeping them on their own would be nice.

The interrupt issues I am still seeing on CRC DAT errors seem specific to the broadcom chip on the RPi. I found other references. I have not been able to track down a solution yet but I'd suggest we ignore that for the moment.

I've got three proposed changes. The first two hunks remove a timeout interrupt here (unsurprisingly) by using a correct block size.

Seems fine to me. I keep running into that at boot and have yet to figure out why transmit is not possible at that point yet; it kills the first IPv6 RS/IPv4 DHCP packet (but that's for another review).

For your information; sdhci_data_irq() needs more love. Currently it runs into the CRC error case which is wrong when I am trying to blk_count = 511 (yes I am avoiding blk_count = 0 for infinite). I haven't looked into it yet, but it'll be next; here's a short debug trace:

DBG: brcmf_sdiod_ramrw:283: write 32768/600487 bytes at offset 0x00000000 in window  0x00198000
sdhci_bcm0-slot0: Got XPT_MMC_IO
sdhci_bcm0-slot0: CMD53 arg 0x9d0001ff flags 0x35 dlen 32704 dflags 0x9 blksz=64 blkcnt=511
sdhci_bcm0-slot0: SDIO Custom block params: blksz:       0x40, blk cnt:      0x1ff
sdhci_bcm0-slot0: Blk size: 0x00000040 | Blk cnt:  0x000001ff
sdhci_bcm0-slot0: Starting command opcode 0x35 flags 0x3a
sdhci_bcm0-slot0: Interrupt 0x11
sdhci_bcm0-slot0: sdhci_finish_command: called, err 0 flags 0x35
Resp: 0x1000 0000 0000 0000
sdhci_bcm0-slot0: Interrupt 0x208010
sdhci_bcm0-slot0: sdhci_data_irq:2174 set error 0x02 intmask 0x200010
                                                     ^^^^^^^^^^^^^^^^^^

I haven't convinced myself yet that these two extra fields are actually needed but meanwhile this is needed (independently). I'll put it into a proper review later tonight.

In D19622#421202, @glebius wrote:

I think that while in flight problem can be fixed with epoch,

In D15955#420988, @manu wrote:

In D15955#420643, @kibab wrote:

In D15955#357900, @manu wrote:

Quoting myself :

The correct solution is implementing proper card insertion detection. This is, however, out of scope for this change. I'm just replicating the way it's implemented for mmc(4) in this driver, for now.

Yes and that's what I don't like, this is not the correct solution for this problem.

Do you have a quick overview of what all gets completely flushed this way? Hmm IP (reassmbly), frag6, sctp, and tcp is what I can spot, are there others?

While this solves the immediate problem you were experiencing at some collateral damage for all other interfaces, it doesn't solve the underlying issues for all places, like the netisr queue.

In D19488#419791, @bz wrote:

In D19488#419769, @pawel.biernacki-gmail.com wrote:

I think I see how this can happen; I'll send another patch later today for testing.

In D19488#419769, @pawel.biernacki-gmail.com wrote:

In D19488#419762, @pawel.biernacki-gmail.com wrote:

This breaks ifconfig_DEFAULT="DHCP" somehow. It works with either SYNCDHCP or this commit reverted.

Sorry Kyle, that the full discussion on this is now done on this review. Thanks for starting to consolidate all the "random" uses.
Let's take this slowly and try to get it righter and with rough consensus into something which will (a) sort the various individual implementations, and (b) solve problems for our users.

In D19587#419396, @imp wrote:

OUI spaces aren't for randomly assigning address bits that change from boot to boot. They are for more permanent allocations according to the standards that I read a few years ago. The local administrative space is for things like random assignment of MAC addresses.

Die CVS die!

Re-sort the struct fields to not change the expected order and use a spare field in-place. That will help netstat to continue working when updating from 12->13 without shuffling the values around.

Remove the RTSOL interface pseudo-option and run rtsol unconditionally on accept_rtadv (if it is installed).

In D19487#417291, @hrs wrote:

Agreed. I meant logging upon a state toggle by RA, not each time receiving an RA.

In D19488#417164, @hrs wrote:

For (2), I think the old version unconditionally runs rtsol (w/o "d") when rtsold_enable="NO", and does not when rtsold_enable="YES" because rtsold (w/ "d") will be invoked later by rc.d/rtsold should handle it. In short, the condition which determines if rtsol is invoked or not depends only on "accept_rtadv" flag. Isn't it enough?

In D19487#417158, @hrs wrote:

Looks good to me. We might want to put an INFO log line when ND6_IFF_IPV6_ONLY flag is changed by receiving RA or link-down event so that the sysadmin can know what is going on.

In D19487#417165, @hrs wrote:

One more comment: immediate change of the flag just after a link-down event may be problematic when the link is unstable. In such a situation, IPv4 packets can be allowed after a link-up and before another RA arrives even if the all of advertising routers keep sending ipv6only flag. We can assume that the interface should be configured to accept RA when the machine moves from a network to another, so sending a RS after a link-up then clearing ND6_IFF_IPV6_ONLY only when no RA arrives in a few seconds looks a more reasonable behavior. If at least one RA arrives after the RS, the normal handling of ipv6only flag works.

Should add: X-MFC: never.