[ovs-dev] [tests+nxm-ofctl 01/42] dpif-netdev: Simplify code by using shash for names and dropping indexes.

Ben Pfaff blp at nicira.com
Wed Nov 24 20:36:34 UTC 2010


On Tue, Nov 23, 2010 at 05:33:50PM -0800, Justin Pettit wrote:
> On Nov 23, 2010, at 2:43 PM, Ben Pfaff wrote:
> 
> > /* Datapath based on the network device interface from netdev.h. */
> > struct dp_netdev {
> > -    struct list node;
> > -    int dp_idx;
> > +    char *name;                 /* Owned by the containing shash. */
> 
> Is this ownership comment actually true?  I thought the shash was
> creating its own copy of name.

It was true during some of my work on this commit, but it isn't as I
sent it out.  I removed it.

> > static struct dpif *
> > create_dpif_netdev(struct dp_netdev *dp)
> > {
> >     struct dpif_netdev *dpif;
> > -    char *dpname;
> > 
> >     dp->open_cnt++;
> > 
> > -    dpname = xasprintf("dp%d", dp->dp_idx);
> >     dpif = xmalloc(sizeof *dpif);
> > -    dpif_init(&dpif->dpif, &dpif_netdev_class, dpname, dp->dp_idx, dp->dp_idx);
> > +    dpif_init(&dpif->dpif, &dpif_netdev_class, dp->name, 0, 0);
> 
> The ovs-vswitch.conf.db man page states that the default NetFlow
> engine id is the datapath index, but this code will always set it to
> zero. Do we want to still provide some sort of sane default value?

OK, I changed it to use a hash of the datapath name.  There will be at
least some stability then.

> > @@ -277,6 +225,7 @@ dp_netdev_free(struct dp_netdev *dp)
> > {
> >     int i;
> > 
> > +
> 
> This seems like an unnecessary newline introduction.

Fixed, thanks.




More information about the dev mailing list