Date: Mon, 04 Feb 2013 13:06:07 +0200 From: Andriy Gapon <avg@FreeBSD.org> To: Marko Zec <zec@fer.hr> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, Mark Johnston <markj@FreeBSD.org>, src-committers@FreeBSD.org Subject: Re: svn commit: r246245 - head/sys/netgraph Message-ID: <510F961F.1070104@FreeBSD.org> In-Reply-To: <201302041058.45725.zec@fer.hr> References: <201302021154.r12Bs0tp030831@svn.freebsd.org> <201302040927.43559.zec@fer.hr> <510F8287.7030708@FreeBSD.org> <201302041058.45725.zec@fer.hr>
next in thread | previous in thread | raw e-mail | index | archive | help
on 04/02/2013 11:58 Marko Zec said the following:
> On Monday 04 February 2013 10:42:31 Andriy Gapon wrote:
>> on 04/02/2013 10:27 Marko Zec said the following:
>>> On Monday 04 February 2013 08:41:32 Andriy Gapon wrote:
>>>> + /* Only ethernet interfaces are of interest. */
>>>> + if (ifp->if_type != IFT_ETHER)
>>>> + return;
>>>
>>> And what about IFT_FDDI, IFT_XETHER, IFT_ISO88025, IFT_L2VLAN,
>>> IFT_BRIDGE, IFT_ARCNET, IFT_IEEE8023ADLAG, IFT_IEEE80211?
>>
>> Oh, I didn't realize that many drivers changed if_type after if_alloc.
>> Honestly, the networking code is not my strong skill, I ventured here
>> only because nobody else did...
>>
>> So what do you suggest? if_alloctype or a different approach?
>> I'd like to prevent if_l2com being mis-interpreted as struct arpcom.
>
> We already have this in vnet_ng_ether_init():
>
> 865 TAILQ_FOREACH(ifp, &V_ifnet, if_link) {
> 866 if (ifp->if_type == IFT_ETHER
> 867 || ifp->if_type == IFT_L2VLAN)
> 868 ng_ether_attach(ifp);
> 869 }
>
> So at least in ng_ether_ifnet_arrival_event() we should do a check
> consistent to the above code.
OK, that makes sense.
Although I am not sure if perhaps that check should be extended too.
But that's not something for me to worry about.
> OTOH we don't check for interface types on
> entry into ng_ether_attach(), and perhaps a better strategy would be to
> move your ifp->if_type check there.
Definitely not move, perhaps copy...
OTOH, ng_ether_attach is invoked via a different mechanism (an explicit hook),
directly from ether_ifattach.
And so, as you note, there seems to be an inconsistency between
ether_ifattach->ng_ether_attach and vnet_ng_ether_init. If a bridge is created
after ng_ether is loaded, then there would be ng_ether node for the bridge.
If ng_ether is loaded after a bridge is created, then there would be no ng_ether
node for it. Unless I miss something.
But I don't know if we actually want ng_ether for a bridge (or something else of
the types you listed)...
> Perhaps the check could be #defined as
> a macro to ensure consistency between vnet_ng_ether_init() and
> ng_ether_attach()?
Perhaps...
--
Andriy Gapon
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?510F961F.1070104>