svn commit: r228071 - head/sys/net

[Gleb Smirnoff]

On Mon, Nov 28, 2011 at 07:47:22PM +0000, Bjoern A. Zeeb wrote:
B> On 28. Nov 2011, at 14:44 , Gleb Smirnoff wrote:
B> 
B> > Author: glebius
B> > Date: Mon Nov 28 14:44:59 2011
B> > New Revision: 228071
B> > URL: http://svn.freebsd.org/changeset/base/228071
B> > 
B> > Log:
B> >  - Use generic alloc_unr(9) allocator for if_clone, instead
B> >    of hand-made.
B> >  - When registering new cloner, check whether a cloner with
B> >    same name already exist.
B> >  - When allocating unit, also check with help of ifunit()
B> >    whether such interface already exist or not. [1]
B> 
B> This forces packages to be recompiled;  they might like to have a __FreeBSD_version for that?
B> It's not MFCable, at least I think - don't see a MFC after, just want to be sure.

No plans for MFC.

btw, I don't like the static initializer of cloners, since it require re-compile of
dependencies.

What about making an API change: remove the static initializer and make an initializer
function. Modules should have only a pointer to opaque structure. We will bump
__FreeBSD_version for that. But later with any change to the if_clone struct, we
won't have ABI change. If everyone agrees, I can go for that.

B> See one more comment inline?
B> > +	ifc->ifc_unrhdr = new_unrhdr(0, ifc->ifc_maxunit, &ifc->ifc_mtx);
B> > +	LIST_INIT(&ifc->ifc_iflist);
B> > 
B> > 	IF_CLONERS_LOCK();
B> > +	LIST_FOREACH(ifc1, &V_if_cloners, ifc_list)
B> > +		if (strcmp(ifc->ifc_name, ifc1->ifc_name) == 0) {
B> > +			IF_CLONERS_UNLOCK();
B> > +			IF_CLONE_REMREF(ifc);
B> > +			return (EEXIST);
B> 
B> At this point you may have a problem not freeing the unr?

No, there is no problem. The code path that goes to delete_unrhdr() is the
following:

IF_CLONE_REMREF calls IF_CLONE_REMREF_LOCKED, the latter calls if_clone_free(),
and the last one calls delete_unrhdr().

-- 
Totus tuus, Glebius.