[ovs-dev] [netlink v4+2 2/2] datapath: Change dp_idx to dp_ifindex, the ifindex of the local port.

[Jesse Gross]

On Fri, Jan 14, 2011 at 9:53 AM, Ben Pfaff <blp at nicira.com> wrote:
> diff --git a/datapath/datapath.c b/datapath/datapath.c
> index c73968c..fd19ef6 100644
> --- a/datapath/datapath.c
> +++ b/datapath/datapath.c
>  static int odpdc_dump(struct sk_buff *skb, struct netlink_callback *cb)
>  {
> -       u32 dp_idx;
> +       struct datapath *dp;
> +       int skip = cb->args[0];
> +       int i = 0;
>
>        mutex_lock(&dp_mutex);
> -       for (dp_idx = cb->args[0]; dp_idx < ARRAY_SIZE(dps); dp_idx++) {
> -               struct datapath *dp = get_dp(dp_idx);
> -               if (!dp)
> +       list_for_each_entry_rcu (dp, &dps, list_node) {

Is this actually protected by RCU?  It looks like it's using dp_mutex.

> diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
> index b097731..9ad20e1 100644
> --- a/lib/dpif-linux.c
> +++ b/lib/dpif-linux.c
>  static int
>  dpif_linux_get_all_names(const struct dpif *dpif_, struct svec *all_names)
>  {
> -    struct dpif_linux *dpif = dpif_linux_cast(dpif_);
> -
> -    svec_add_nocopy(all_names, xasprintf("dp%d", dpif->dp_idx));
> -    svec_add(all_names, dpif->local_ifname);
> +    svec_add(all_names, dpif_base_name(dpif_));
>     return 0;
>  }

I think we could just make this a NULL pointer, as the implementation
in dpif.c does this by default.  The comment above
dpif_get_all_names() also refers to the Linux dpif using dpX, which is
no longer true.

Do we actually even need dpif_get_all_names() anymore?  I don't
believe that anything is currently using and I'm not sure whether
other dpif implementations would be likely to do so.

> diff --git a/lib/dpif.man b/lib/dpif.man
> index 775ec58..9597ca1 100644
> --- a/lib/dpif.man
> +++ b/lib/dpif.man
> @@ -1,11 +1,5 @@
>  .RS
>  .TP
> -[\fItype\fB@\fR]\fBdp\fIN\fR
> -Datapath number \fIN\fR, where \fIN\fR is a number between 0 and 255,
> -inclusive.  If \fItype\fR is given, it specifies the datapath provider of
> -\fBdp\fIN\fR, otherwise the default provider \fBsystem\fR is assumed.
> -.
> -.TP
>  [\fItype\fB@\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,

This still refers to datapath number.

The text above this in the ovs-dpctl man page refers to multiple forms
of input but there is only a single one now.