From owner-svn-src-head@FreeBSD.ORG Mon Feb 4 11:06:11 2013 Return-Path: Delivered-To: svn-src-head@FreeBSD.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 64C30700; Mon, 4 Feb 2013 11:06:11 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from citadel.icyb.net.ua (citadel.icyb.net.ua [212.40.38.140]) by mx1.freebsd.org (Postfix) with ESMTP id 1820BC3B; Mon, 4 Feb 2013 11:06:09 +0000 (UTC) Received: from odyssey.starpoint.kiev.ua (alpha-e.starpoint.kiev.ua [212.40.38.101]) by citadel.icyb.net.ua (8.8.8p3/ICyb-2.3exp) with ESMTP id NAA18861; Mon, 04 Feb 2013 13:06:07 +0200 (EET) (envelope-from avg@FreeBSD.org) Message-ID: <510F961F.1070104@FreeBSD.org> Date: Mon, 04 Feb 2013 13:06:07 +0200 From: Andriy Gapon User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:17.0) Gecko/20130113 Thunderbird/17.0.2 MIME-Version: 1.0 To: Marko Zec Subject: Re: svn commit: r246245 - head/sys/netgraph References: <201302021154.r12Bs0tp030831@svn.freebsd.org> <201302040927.43559.zec@fer.hr> <510F8287.7030708@FreeBSD.org> <201302041058.45725.zec@fer.hr> In-Reply-To: <201302041058.45725.zec@fer.hr> X-Enigmail-Version: 1.4.6 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, Mark Johnston , src-committers@FreeBSD.org X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 04 Feb 2013 11:06:11 -0000 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