Date: Tue, 19 Oct 2004 23:29:33 +0200 From: Max Laier <max@love2party.net> To: Andre Oppermann <andre@freebsd.org> Cc: cvs-src@freebsd.org Subject: Re: cvs commit: src/sys/sys protosw.h src/sys/kern uipc_domain.cuipc_socket2.c Message-ID: <200410192329.46723.max@love2party.net> In-Reply-To: <41753522.1E39FEAE@freebsd.org> References: <200410191513.i9JFDUbf072176@repoman.freebsd.org> <417532A2.9000901@errno.com> <41753522.1E39FEAE@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--nextPart2773990.mzSJZD2ttK
Content-Type: text/plain;
charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline
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 protoc=
ol
> 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 socket=
s,
> 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 someth=
ing=20
like this, but I am afraid that there is code that will trigger problems. F=
or=20
example, in ip_icmp.c line 457 ff we have:
ctlfunc =3D 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=20
places where we (temporary) cache values from the protocol array. Another=20
problem might be that we check for protocol existence early and assume that=
=20
this remains true ...
I'd suggest, that you remove the possibility to remove protocols completely=
=2E=20
It is very likely that there are no races with adding protocols - though it=
=20
might take "some time" for the protocol to be fully useable - but the remov=
al=20
is critical.
We also have to check that really all code can cope with the addition and=20
properly reinitializes it's view of the protocol arrays.
Another point: If you really want to keep the possibility to remove a=20
protocol, you have to introduce some busy counter that pervents removal whi=
le=20
the kernel is inside a protocol function. This has to be handled by the=20
protocol itself, but it has to be taken care of somehow.
=2D-=20
/"\ Best regards, | mlaier@freebsd.org
\ / Max Laier | ICQ #67774661
X http://pf4freebsd.love2party.net/ | mlaier@EFnet
/ \ ASCII Ribbon Campaign | Against HTML Mail and News
--nextPart2773990.mzSJZD2ttK
Content-Type: application/pgp-signature
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.6 (FreeBSD)
iD8DBQBBdYdKXyyEoT62BG0RAobtAJ9PrJVaNANAiyB+aWivYsr48MEZswCdG54+
SqrpAqMHgwbJDJxRlXl+mVc=
=W45W
-----END PGP SIGNATURE-----
--nextPart2773990.mzSJZD2ttK--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200410192329.46723.max>