[ovs-dev] [PATCH] datapath: Don't hold dp_mutex when setting internal devs MTU.

[Jesse Gross]

On Fri, Apr 30, 2010 at 5:14 PM, Ben Pfaff <blp at nicira.com> wrote:

> On Fri, Apr 30, 2010 at 05:09:24PM -0700, Jesse Gross wrote:
> > On Fri, Apr 30, 2010 at 5:01 PM, Ben Pfaff <blp at nicira.com> wrote:
> >
> > > On Mon, Apr 26, 2010 at 06:30:03PM -0700, Jesse Gross wrote:
> > > > We currently acquire dp_mutex when we are notified that the MTU
> > > > of a device attached to the datapath has changed so that we can
> > > > set the internal devices to the minimum MTU.  However, it is not
> > > > required to hold dp_mutex because we already have RTNL lock and it
> > > > causes a deadlock, so don't do it.
> > >
> > > I don't see a problem with this patch.
> > >
> > > But I don't see the deadlock, either.  dp->mutex nests inside
> > > rtnl_lock, and I don't any inversion of that order being fixed here.
> > >
> > The issue is that DP mutex is acquired twice: once in dp_device_event()
> > before calling set_internal_devs_mtu() and then again in
> > internal_dev_change_mtu() when it is actually being changed (since the
> MTU
> > can also be set directly).  Since it's not a recursive mutex, deadlock.
>
> Thanks.
>
> > > I don't know why the (!(port.flags & ODP_PORT_INTERNAL)) test was
> > > dropped in attach_port().
> >
> > It's to take into account the MTU of any devices that have already been
> > added to the datapath before the internal port.
>
> Oh, I see, it's to set the *new* internal port's MTU.  For some reason
> I imagined that this was taken care of in the vport_internal_dev
> "create" function, but it seems not.
>

Yeah, we can't do it during creation because at that point we aren't yet
attached to a specific datapath, so we do it at attach time.


> Thanks, this is good.  (Perhaps you could make the commit message more
> explicit so I don't have to think hard in the future.)
>

Thanks, I described the problem a little bit more and pushed.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://openvswitch.org/pipermail/dev/attachments/20100430/960a5da8/attachment.htm>