[ovs-discuss] [PATCH 00/20] Implement abstract interfaces to network devices
jesse at nicira.com
Tue Jul 28 00:09:06 UTC 2009
Ben Pfaff wrote:
> This series of commits first refactors the network device layer to be
> more amenable to portability, then it (in a big commit toward the end)
> actually abstracts the network device implementation into a class
> structure. This should make it easier to port netdev to other
> systems, or to support more than one kind of network device on a
> single system.
I looked through this little bundle of patches and it seemed generally
good. It doesn't look like we'll have any problems adding a Unix domain
socket netdev, so it seems like a good abstraction. Here are a couple
of things that I noticed:
In ofproto.c/ofproto_create(), it seems weird to be calling
pick_fallback_dpid() twice - once for p->fallback_dpid and once for
p->datapath_id since this will generate two different random ID's.
p->datapath_id gets overwritten later on with the final ID, so it seems
like it is either redundant to generate it earlier (if no one uses it
between those to calls) or potentially bad (if someone uses datapath_id
and assumes that it is the final value).
In bridge.c, I'm not sure that open_iface_netdev is the best name for
the function if it also retrieves other information like carrier status.
The name lxnetdev isn't overly descriptive of the actual functionality.
I'm not sure what's better though.
The fact that an initialization failure in one of the classes of netdevs
blocks all netdevs seems a little fragile to me.
Two comments about comments in netdev-provider.c/netdev_class:
* prefix says that the empty string matches class names that wouldn't
otherwise match. In reality, the empty string only matches names
* get_in6 refers to setting in4
I believe that netdev-linux.c/netdev_tap_class needs to have a prefix of
Having both a Linux netdev and a tap netdev will cause the
initialization to happen twice, overwriting some of the static variables
Do we really want to reinitialize all of the netdev classes every time
someone opens a netdev?
Some of the netdevs may want to use the config file, so it would be
useful to have a reconfigure call in the netdev interface.
More information about the discuss