cvs commit: src/sys/sys protosw.h src/sys/kern uipc_domain.cuipc_socket2.c

[Max Laier]

On Tuesday 19 October 2004 17:39, Andre Oppermann wrote:
> Sam Leffler wrote:
> > Andre Oppermann wrote:
> > > andre       2004-10-19 15:13:30 UTC
> > >
> > >   FreeBSD src repository
> > >
> > >   Modified files:
> > >     sys/sys              protosw.h
> > >     sys/kern             uipc_domain.c uipc_socket2.c
> > >   Log:
> > >   Support for dynamically loadable and unloadable protocols within
> > > existing protocol families.
> >
> > I don't recall seeing this posted anywhere for comment.  I have some
> > concerns about this general topic and this code seems incomplete (e.g. I
> > see no locking).
>
> Locking is not needed because there are no dead moments in transitioning
> from unregistered to registered and back.  All calls to any of the protocol
> specific functions will return a valid result (even if it is only
> EOPNOTSUPP). There is no list manipulation going on.
>
> The caller of the function is required to assure that no dangeling sockets,
> references or memory allocations are left behind after unregistering.  It's
> simply impossible to solve otherwise.  For IPDIVERT which I have converted
> this works very well (it will simply refuse to unload if a divert socket is
> open).
>
> What remaining concerns do you have?

I am also a bit worried about this. While it is a cool thing to have something 
like this, but I am afraid that there is code that will trigger problems. For 
example, in ip_icmp.c line 457 ff we have:

               ctlfunc = inetsw[ip_protox[icp->icmp_ip.ip_p]].pr_ctlinput;
               if (ctlfunc)
                        (*ctlfunc)(code, (struct sockaddr *)&icmpsrc,
                                   (void *)&icp->icmp_ip);

This is clearly a problem if we can remove protocols. There might be more 
places where we (temporary) cache values from the protocol array. Another 
problem might be that we check for protocol existence early and assume that 
this remains true ...

I'd suggest, that you remove the possibility to remove protocols completely. 
It is very likely that there are no races with adding protocols - though it 
might take "some time" for the protocol to be fully useable - but the removal 
is critical.

We also have to check that really all code can cope with the addition and 
properly reinitializes it's view of the protocol arrays.

Another point: If you really want to keep the possibility to remove a 
protocol, you have to introduce some busy counter that pervents removal while 
the kernel is inside a protocol function. This has to be handled by the 
protocol itself, but it has to be taken care of somehow.

-- 
/"\  Best regards,                      | mlaier at freebsd.org
\ /  Max Laier                          | ICQ #67774661
 X   http://pf4freebsd.love2party.net/  | mlaier at EFnet
/ \  ASCII Ribbon Campaign              | Against HTML Mail and News
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 187 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/cvs-src/attachments/20041019/d1202fcd/attachment.bin