- User Since
- Aug 29 2014, 12:11 PM (223 w, 3 d)
Thu, Dec 6
Mon, Dec 3
Sorry but my understanding is that this could possibly free the softc even before lagg_clone_destroy() has finished, couldn't it?
Thu, Nov 29
Tue, Nov 27
Mon, Nov 26
Sat, Nov 24
@imp could you please comment on your architectural views (or wish-list) while there is still time? Otherwise I might have to sort it out after the facts; I am expecting to be at a point when I need to make a driver talk to some SDIO in about a week and that means I'll work on any middle-glue-code I'll see fit.
Sat, Nov 17
Thu, Nov 15
I'll trust you to get the assertion right; sounds good to me.
Mon, Nov 12
Nov 9 2018
Sorry, getting IPv4 fragments into my head is absolutely not a good idea.
Sorry; getting IPv4 fragments into my head is not a good idea.
Nov 8 2018
Can you please site the Apple CVE and possibly the original writeup instead of a (random) reddit thing?
To me this change seems wrong. The only caller for this function is exactly for the situation when the link-local address is missing.
If we are in the progress of "configuring" the interface and someone is already de-configuring it to me that sounds like a concurrency problem elsewhere.
This entire function seems to be based on the idea that there's a lock held around it and it's the only actor (which might very well still be coming from &Giant days of FreeBSD 4).
Nov 4 2018
Nov 3 2018
Nov 2 2018
See PR 230857 for details.
Nov 1 2018
So is it the last 5 commits on your github branch or is there anything in there from before that? Having this broken up in logical junks for review will make it a lot easier.
Oct 31 2018
Oct 30 2018
Oct 26 2018
Ok, I've tested this (well the shorter constants version mostly) with my two test modules (no longer panics, size of 1 symbol works), which a linker script which had a wrong padding, and with the matching modules. I am aware that 3rd party modules will be unhappy but after spending days and weeks to get to this, no better solution could be found. Does anyone want to review this quickly so it can go into HEAD and go to 12, to prevent panics there?
Update the constants to sizeof linker-script-LONG (32bit) to have a smaller
possibility of accidentally matching.
Go with Alex's suggestion of fixed padding at the end as linkers are not
working reliable enough to simply extend an already existing section (see PR 232291).
This simplfis the linker script logic and adds some extra checkes to link_elf.c
for just i386.
Oct 25 2018
Neither this nor the kernel version (though that one just less likely) is not deadlock safe. It can still happen.
Oct 24 2018
What's the reason we can't just use the IFNET_WLOCK() and be good with it and not rewriting the entire code but going back to what it was?
The original version if_grow() could not fail, ifindex_alloc() had at best one retry, ... I am still not getting why this wasn't good enough?
Oct 23 2018
Anyone? I'd love to get this in ...
Oct 22 2018
Oct 19 2018
Oct 18 2018
Silly question; can you explain that test case? How do 255 multicast addresses assigned to 255 interfaces make you run out of ifindex space?
For as much as my brain still functions this seems ok
It would greatly help if the proposed commit message would also cite the RFC sections not just the RFCs; ideally actually comments in the code would do that as well at the right place so that future reference lookups will be easier.
Oct 17 2018
Scrolling through it looks OK with cross-checking to the RFCs.
Mostly commenting on the fact that we should get this in before BETA as it does change the kernel KPI.
Hope someone will do the full technical review (want to add lstewart as well?)
May I ask if you have a chance to look at this? It's been sitting around here so long that I forgot about it.
Oct 12 2018
Maybe both? We also don't want to not get notified and have an interface hanging around for ever given pf never releases a reference?
In the first block of you patch you might want to switch the order and set the kif to NULL before releasing the reference.
Oct 11 2018
Oct 10 2018
The kmod.mk changes can be reduced to two linker invocations instead of three,
also needing one less intermediate file to cleanup.
Argh the kmod changes are the old ones. I'll update in a second
Still wonder if pf should hold a reference to the interface as well.
That was kind-of the model I had in mind for the other callbacks as well ;-)
OK, this only makes sense after the callback functions are virtualized; before the above comment did make sense but was expecting a different architechtural solution (whether true or not at the time of writing)
OK. I was thinking (when reading the UNINIT change) that that only starts to make sense after virtualising these here; and then I wondered why I'd want to virtualise if these called functions would be perfectly fine and happy to determine if pfsync is enabled (RUNNING) in this vnet or not. That way there'd be less virtualisation.
Actually, why is this needed?
I assume the "other way round" is only true after the function pointers have been virtualised, otherwise not.
And that makes me wonder..
I only scrolled through but it looks good to me.
Oct 8 2018
While you are at it, what about https://reviews.freebsd.org/D13865 ?