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
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "[email protected]"
