[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