[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