[ovs-dev] [PATCH] dpif-netdev: Initialize upcall->packet when queuing to userspace.

Ben Pfaff blp at nicira.com
Fri Aug 1 17:17:23 UTC 2014


On Wed, Jul 23, 2014 at 09:48:10AM -0700, Pravin Shelar wrote:
> On Tue, Jul 22, 2014 at 10:37 PM, Ben Pfaff <blp at nicira.com> wrote:
> > Pravin, the code in dpif-netdev.c modified by this patch is similar but
> > not identical on branch-2.3.  Do you have an idea whether differences
> > elsewhere in the tree make this safe on branch-2.3 without change?
> >
> I think same bug exist on 2.3. I do not know why it is safe on 2.3.
> Let me check code.

Did you follow up on this?

> > Thanks,
> >
> > Ben.
> >
> > On Tue, Jul 22, 2014 at 10:30:27PM -0700, Ben Pfaff wrote:
> >> Thanks, applied to master.
> >>
> >> On Tue, Jul 22, 2014 at 10:22:30PM -0700, Justin Pettit wrote:
> >> > Acked-by: Justin Pettit <jpettit at nicira.com>
> >> >
> >> >
> >> >
> >> > On July 22, 2014 at 9:13:58 PM, Ben Pfaff (blp at nicira.com) wrote:
> >> > > Commit db73f7166a6 (netdev-dpdk: Fix race condition with DPDK mempools in
> >> > > non pmd threads) switched to a new way of setting up 'upcall->packet', but
> >> > > only initialized two of the fields in the packet. This could cause
> >> > > core dumps and other strange behavior. In particular it caused failures in
> >> > > several unit tests on XenServer.
> >> > >
> >> > > This commit fixes the problem by initializing the entire ofpbuf.
> >> > >
> >> > > Signed-off-by: Ben Pfaff
> >> > > ---
> >> > > lib/dpif-netdev.c | 5 +++--
> >> > > 1 file changed, 3 insertions(+), 2 deletions(-)
> >> > >
> >> > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> >> > > index cfd7539..742b118 100644
> >> > > --- a/lib/dpif-netdev.c
> >> > > +++ b/lib/dpif-netdev.c
> >> > > @@ -2153,6 +2153,7 @@ OVS_REQUIRES(q->mutex)
> >> > > struct ofpbuf *buf = &u->buf;
> >> > > size_t buf_size;
> >> > > struct flow flow;
> >> > > + void *data;
> >> > >
> >> > > upcall->type = type;
> >> > >
> >> > > @@ -2179,8 +2180,8 @@ OVS_REQUIRES(q->mutex)
> >> > > /* We have to perform a copy of the packet, because we cannot send DPDK
> >> > > * mbufs to a non pmd thread. When the upcall processing will be done
> >> > > * in the pmd thread, this copy can be avoided */
> >> > > - ofpbuf_set_data(&upcall->packet, ofpbuf_put(buf, ofpbuf_data(packet),
> >> > > - ofpbuf_size(packet)));
> >> > > + data = ofpbuf_put(buf, ofpbuf_data(packet), ofpbuf_size(packet));
> >> > > + ofpbuf_use_stub(&upcall->packet, data, ofpbuf_size(packet));
> >> > > ofpbuf_set_size(&upcall->packet, ofpbuf_size(packet));
> >> > >
> >> > > seq_change(q->seq);
> >> > > --
> >> > > 1.9.1
> >> > >
> >> > > _______________________________________________
> >> > > dev mailing list
> >> > > dev at openvswitch.org
> >> > > http://openvswitch.org/mailman/listinfo/dev
> >> > >
> >> >



More information about the dev mailing list