vlan changes to support ipoib

Bjoern A. Zeeb bzeeb-lists at lists.zabbadoz.net
Mon Jan 10 11:10:07 UTC 2011


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


More information about the freebsd-net mailing list