[ovs-dev] [PATCH 1/2] dpif: Update dpif interface to match netdev.

Ben Pfaff blp at nicira.com
Sat Jan 23 04:22:03 UTC 2010


On Fri, Jan 22, 2010 at 02:52:41PM -0500, Jesse Gross wrote:
> This brings over some features that were added to the netdev interface,
> most notably the separation between the name and the type.  In addition

...to...

> being cleaner, this also avoids problems where it is expected that the
> local port has the same name as the datapth.

"datapath"

>  int
> -dp_enumerate(struct svec *all_dps)
> +dp_enumerate(struct svec *names, struct svec *types)

This function has a really weird interface now.  A cleaner alternative
might be to create a dp_enumerate_types() to enumerate all the known
types, and then a dp_enumerate_names() to enumerate the names of all the
dps with a given type.

But it seems unlikely that this function will ever be used enough to be
annoying, so that might be overkill.

> +    if (!type || !strlen(type)) {

This use of strlen() always bothers me--please write 
        if (!type || *type == '\0')
instead.

> +        type = "system";
>      }
>  

> diff --git a/lib/dpif.man b/lib/dpif.man
> index 3a58caf..31f126a 100644
> --- a/lib/dpif.man
> +++ b/lib/dpif.man
> @@ -1,11 +1,13 @@
>  .RS
>  .TP
> -\fBdp\fIN\fR
> +[\fItype\fR:]\fBdp\fIN\fR

I think that should be
        [\fItype\fB:\fR]\fBdp\fIN\fR
(i.e. change \fR: to \fB:\fR) because ":" is a literal character.

But the whole contents of this file are specific to dpif-linux.  In
particular I wouldn't expect every dpif implementation to do this
name-to-number translation dance.  Should we generalize the file?  Or
should we make it specific to what actually gets built?

>  Datapath number \fIN\fR, where \fIN\fR is a number between 0 and 255,
> -inclusive.
> +inclusive.  If \fItype\fR is given, it specifies the datapath provider of
> +\fBdp\fIN\fR, otherwise the default provider \fBsystem\fR is assumed.
>  .TP
> -\fIname\fR
> +[\fItype\fR:]\fIname\fR
>  The name of the network device associated with the datapath's local
>  port.  (\fB\*(PN\fR internally converts this into a datapath number,
> -as above.)
> +as above.)  If \fItype\fR is given, it specifies the datapath provider of
> +\fBname\fR, otherwise the default provider \fBsystem\fR is assumed.

It looks like the changes to ovs-ofctl remove the ability to specify a
vconn by giving the name of a datapath.  Is that correct?  It's too
bad--that feature was really useful sometimes.  Can it be restored?




More information about the dev mailing list