Max Laier wrote:
> On Wednesday 20 October 2004 01:04, Andre Oppermann wrote:
> > Please point them out. We can discuss specifics then instead of creating
> > a clout of FUD.
> Okay, that's the last straw. This is not FUD. We are really concerned that you
> are introduceing something that is not fully though about.
Not exactly. Dynamically registering and unregistering is not the problem.
It works without causing panics or races (except when two protcols want
to register exactly at the same time which is perfectly solved with GIANT).
> We will have problems with this and I really think that it should be backed
> out now and fixed before reconsidered!
What can be problematic is not the unregistering of an protocol but the
subsequent unloading of the module that implemented said protocol. This
is where where all this discussion comes from (?). In the case of IPDIVERT
I made it unloadable and as you correctly say there can be certain race
The solution is not to back out what I committed but to make IPDIVERT to
refuse to unload, *or* to fix IPDIVERT in a way to make it 100% race safe
> Unloading (of divert) has races that can be triggered. Claiming that they will
> not be "most of the time" is not exactly the right approach for serious
For the record; The risk for a panic I introduced here with IPDIVERT is
several squareorders in magnitude lower than the accept() panic fixed a
couple of days ago because here it could only happen if someone was actually
trying to unload IPDIVERT. It would never happen in any other circumstances.
That being said, see above how to fix.
> And for pointing out specifics: ip_encap is going to be a lot of fun. And
> that's just the most obvious, I could find. I will not scan the tree for more
> places, but the ip_icmp.c example shows that you didn't scan the tree
> carefully enough before you committed this work without previous discussion.
> It seems that you just assume that everything is fine. It really is not!
You are on the wrong boat here. ip_encap is an entirely different animal.
ip_encap manages its own private protosw to demux many sub-protocols that
have accumulated over time on the original GRE IP protocol type.
What ip_encap is doing has no relation to the general ip_proto_[un]register
function because it can't and doesn't deal with ip_encap's private protosw.
And to rebut that I have carelessly thrown some untested and immature piece
of code in there; I have in fact looked and thought through this and
studied the code that uses it. It fits nicely into the design and intention
of the protosw structure concept. That's why I have done this and not some
The ip_icmp.c example is the only one in the netinet code. In other places
tests are done to check if the function pointer is != NULL. As Harti said
the compiler will do whatever it wants. This makes unloading almost any
module unsafe, doesn't it?
> I understand that you didn't want to slow things down in a bikeshed, but this
> is not ready, sorry.
No no, things got mixed up wrt. protcol unregistering (which is fine and has
its uses) and unloading a module immediatly after it unregistered its protocol.