svn commit: r228071 - head/sys/net

[Bjoern A. Zeeb]

On Tue, 29 Nov 2011, Gleb Smirnoff wrote:

> 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.

I have some fairly intrusive changes to cloners sitting in p4 for the
V_irtualization but it could make my life easier;  I'll be happy to
look at the patch.


> 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().
>
>

-- 
Bjoern A. Zeeb                                 You have to have visions!
          Stop bit received. Insert coin for new address family.