diff mbox

[ovs-dev] packet loopback because different xcfgp pointer is used in xlate_normal*

Message ID 20170818174859.GA6175@ovn.org
State Not Applicable
Headers show

Commit Message

Ben Pfaff Aug. 18, 2017, 5:48 p.m. UTC 
On Fri, Aug 18, 2017 at 01:00:57AM +0800, Huanle Han wrote:
> Hi, Ben
>  I find a bug in ovs that mcast packet loops back because a different xcfgp
> pointer is used in xlate_normal*().
>  I find it on v2.5.0 and I haven't verify it on master yet.
> 
> Here is the my investigation:
> In function xlate_normal_mcast_send_group,  the inbundle pointer and
> outbundle pointer may be unequal even they are actually same port. ovs
> sends the packet back, because mcast_xbundle != in_xbundle.  I think it's
> because they are from different xcfgs.
> if mcast_xbun
> ```
> 
>     xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
>     LIST_FOR_EACH(b, bundle_node, &grp->bundle_lru) {
>         mcast_xbundle = xbundle_lookup(xcfg, b->port);
>         if (mcast_xbundle && mcast_xbundle != in_xbundle) {
>             xlate_report(ctx, OFT_DETAIL, "forwarding to mcast group port");
>             output_normal(ctx, mcast_xbundle, xvlan);
>         }
>       ......
>     }
> ```
> 
> Question 1: Does this bug fixed in master? if it is, which commit?

I noticed and fixed one possibly relevant bug recently, see:

From b24fa3f486d4061cb9facd181fd6234f281904df Mon Sep 17 00:00:00 2001
From: Ben Pfaff <blp@ovn.org>
Date: Wed, 2 Aug 2017 08:36:07 -0700
Subject: [PATCH] ofproto-dpif-xlate: Eliminate duplicate read of xcfgp.

This inner 'xcfg' shadowed the outer one and could have read a different
value if 'xcfgp' was changing, so this is possibly a bug fix.

Found by -Wshadow=local in GCC 7.

Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Andy Zhou <azhou@ovn.org>
---
 ofproto/ofproto-dpif-xlate.c | 1 -
 1 file changed, 1 deletion(-)
diff mbox

Patch

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index a2f5dc7df676..023dc82a366b 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -6679,7 +6679,6 @@  xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
 
         /* Set the bridge for post-recirculation processing if needed. */
         if (!uuid_equals(&ctx.xbridge->ofproto->uuid, &state->ofproto_uuid)) {
-            struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
             const struct xbridge *new_bridge
                 = xbridge_lookup_by_uuid(xcfg, &state->ofproto_uuid);