Date: Mon, 10 Jan 2011 11:08:52 +0000 (UTC) From: "Bjoern A. Zeeb" <bzeeb-lists@lists.zabbadoz.net> To: Jeff Roberson <jroberson@jroberson.net> Cc: freebsd-net@freebsd.org Subject: Re: vlan changes to support ipoib Message-ID: <20110110103329.W14966@maildrop.int.zabbadoz.net> In-Reply-To: <alpine.BSF.2.00.1101091328190.1404@desktop> References: <alpine.BSF.2.00.1101081838330.1404@desktop> <20110109102339.K14966@maildrop.int.zabbadoz.net> <alpine.BSF.2.00.1101091328190.1404@desktop>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, 9 Jan 2011, Jeff Roberson wrote: > On Sun, 9 Jan 2011, Bjoern A. Zeeb wrote: > >> On Sat, 8 Jan 2011, Jeff Roberson wrote: >> >>> Hi Folks, >>> >>> I have made some changes to vlans and I was wondering if anyone would care >>> to review or test. Especially current vlan users. The diff is here: >>> >>> http://people.freebsd.org/~jeff/vlan.diff >>> >>> I can't say that I like adding all of the function pointers but with vlan >>> support available as a loadable module I have little choice. The new >>> functions allow a driver to fetch the virtual interface associated with a >>> tag, cache and fetch a cookie pointer in a virtual interface, and fetch >>> the tag of a virtual interface. >>> >>> Additionally, the driver was changed to make no assumptions about address >>> size unless the type is IFT_ETHER. This means for multicast entries we >>> store a full sockaddr_dl rather than an ethernet address. It actually >>> simplifies the code here. I also simplified the VLAN_ARRAY stuff by >>> moving it into functions that match the hash names so there aren't ifdef's >>> everywhere. I've tested both hash and array. >>> >>> I also changed the global mtx to an sx lock so the vlan_config event >>> handlers can allocate memory. The way ipoib works requires a full new >>> ipoib softc when a vlan is created. I didn't want to duplicate the vlan >>> config logic so I store that softc using the cookie field and fetch it >>> when necessary. As a result ipoib also doesn't need the weird >>> vlan_input() recursion since the packets appear on the correct device as >>> they are received. >>> >>> I am committing this to my infiniband tree immediately but I would like >>> some review before I merge to current. >> >> May I ask you to split the diff up into logical junks for both review >> and commit? Otherwise possible errors not caught with by review will >> just be a lot harder to track down later. > > Most if not all of the chunks are interdependent or useless without each > other. When I commit to current it will probably be a merge from my branch > anyway. > > What would you like to see separated? - the noise (ifp initialization, ..) - the address size changes in preparation for .. - locking can maybe come with the rest of the (*f) for ipoib I think as that's what needs it. You could go by the paragraphs you have above as well. >> >> Some of the stuff like "Initialize from parent" seems to be simple >> enough to be possibly merged just upfront leaving the real beef. > > I notice that I don't have many comments. Perhaps if I documented better > what I have done it would be sufficient? I fear the one "large" commit that in 16 month from now someone will have no idea about. Been there. I know it's additional work but it sometimes really helps. Having debugged too many network stack bugs from corner cases the last 2 years, I'd really appreciate it. /bz -- Bjoern A. Zeeb You have to have visions! <ks> Going to jail sucks -- <bz> All my daemons like it! http://www.freebsd.org/doc/en_US.ISO8859-1/books/handbook/jails.html
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20110110103329.W14966>