[ovs-dev] [netlink v3 3/5] datapath: Merge "struct dp_port" into "struct vport".

Ben Pfaff blp at nicira.com
Fri Dec 3 21:06:36 UTC 2010


On Thu, Dec 02, 2010 at 06:29:21PM -0800, Jesse Gross wrote:
> On Tue, Nov 16, 2010 at 5:11 PM, Ben Pfaff <blp at nicira.com> wrote:
> > diff --git a/datapath/vport-internal_dev.c b/datapath/vport-internal_dev.c
> > index c74bce8..f6348be 100644
> > --- a/datapath/vport-internal_dev.c
> > +++ b/datapath/vport-internal_dev.c
> > @@ -105,16 +105,13 @@ static void internal_dev_getinfo(struct net_device *netdev,
> >                                 struct ethtool_drvinfo *info)
> >  {
> >        struct vport *vport = internal_dev_get_vport(netdev);
> > -       struct dp_port *dp_port;
> >
> >        strcpy(info->driver, "openvswitch");
> >
> > -       if (!vport)
> > +       if (!vport || !vport->dp)
> >                return;
> 
> I don't think that the check for !vport->dp should be necessary any
> more.  Now that dp is assigned in vport_alloc(), it should always be
> valid.

OK, I reverted this change.

> > @@ -135,14 +132,8 @@ static int internal_dev_change_mtu(struct net_device *netdev, int new_mtu)
> >        if (new_mtu < 68)
> >                return -EINVAL;
> >
> > -       if (vport) {
> > -               struct dp_port *dp_port = vport_get_dp_port(vport);
> > -
> > -               if (dp_port) {
> > -                       if (new_mtu > dp_min_mtu(dp_port->dp))
> > -                               return -EINVAL;
> > -               }
> > -       }
> > +       if (vport && vport->dp && new_mtu > dp_min_mtu(vport->dp))
> > +               return -EINVAL;
> 
> Same thing here with vport->dp.

OK, I dropped that check here too, thanks.

> > diff --git a/datapath/vport.c b/datapath/vport.c
> > index 6b50bd6..b67a34b 100644
> > --- a/datapath/vport.c
> > +++ b/datapath/vport.c
> > @@ -47,13 +47,12 @@ static struct hlist_head *dev_table;
> >  * one of these locks if you don't want the vport to be deleted out from under
> >  * you.
> >  *
> > - * If you get a reference to a vport through a dp_port, it is protected
> > + * If you get a reference to a vport through a datapath, it is protected
> >  * by RCU and you need to hold rcu_read_lock instead when reading.
> >  *
> >  * If multiple locks are taken, the hierarchy is:
> >  * 1. RTNL
> > - * 2. DP
> > - * 3. vport
> > + * 2. vport
> >  */
> 
> DP refers to dp_mutex, which still exists even though dp_port does
> not, so it should be included in the hierarchy.

Good point.  I dropped the change to the hierarchy here.

> Otherwise:
> Acked-by: Jesse Gross <jesse at nicira.com>

Thanks.




More information about the dev mailing list