[ovs-dev] [PATCH v2 1/5] xlate: fix packets loopback caused by duplicate read of xcfgp.

Ben Pfaff blp at ovn.org
Wed Jan 24 19:40:16 UTC 2018


From: Huanle Han <hanxueluo at gmail.com>

Some functions, such as xlate_normal_mcast_send_mrouters, test xbundle
pointers equality to avoid sending packet back to in bundle. However,
xbundle pointers port from different xcfgp for same port are inequal.
This may lead to the packet loopback.

This commit stores xcfgp on ctx at first and always uses the same xcfgp
during one packet process period.

Signed-off-by: Huanle Han <hanxueluo at gmail.com>
Signed-off-by: Ben Pfaff <blp at ovn.org>
---
 ofproto/ofproto-dpif-xlate.c | 43 ++++++++++++++-----------------------------
 1 file changed, 14 insertions(+), 29 deletions(-)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 40c04cc4fb4a..f767224941cf 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -182,6 +182,7 @@ struct xlate_ctx {
     struct xlate_in *xin;
     struct xlate_out *xout;
 
+    struct xlate_cfg *xcfg;
     const struct xbridge *xbridge;
 
     /* Flow at the last commit. */
@@ -514,7 +515,6 @@ static struct xlate_cfg *new_xcfg = NULL;
 
 typedef void xlate_actions_handler(const struct ofpact *, size_t ofpacts_len,
                                    struct xlate_ctx *, bool);
-
 static bool may_receive(const struct xport *, struct xlate_ctx *);
 static void do_xlate_actions(const struct ofpact *, size_t ofpacts_len,
                              struct xlate_ctx *, bool);
@@ -1965,8 +1965,7 @@ mirror_packet(struct xlate_ctx *ctx, struct xbundle *xbundle,
 
         /* Send the packet to the mirror. */
         if (out) {
-            struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
-            struct xbundle *out_xbundle = xbundle_lookup(xcfg, out);
+            struct xbundle *out_xbundle = xbundle_lookup(ctx->xcfg, out);
             if (out_xbundle) {
                 output_normal(ctx, out_xbundle, &xvlan);
             }
@@ -2234,7 +2233,6 @@ output_normal(struct xlate_ctx *ctx, const struct xbundle *out_xbundle,
         xport = CONTAINER_OF(ovs_list_front(&out_xbundle->xports), struct xport,
                              bundle_node);
     } else {
-        struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
         struct flow_wildcards *wc = ctx->wc;
         struct ofport_dpif *ofport;
 
@@ -2256,7 +2254,7 @@ output_normal(struct xlate_ctx *ctx, const struct xbundle *out_xbundle,
 
         ofport = bond_choose_output_slave(out_xbundle->bond,
                                           &ctx->xin->flow, wc, vid);
-        xport = xport_lookup(xcfg, ofport);
+        xport = xport_lookup(ctx->xcfg, ofport);
 
         if (!xport) {
             /* No slaves enabled, so drop packet. */
@@ -2525,7 +2523,6 @@ update_mcast_snooping_table(const struct xlate_ctx *ctx,
                             const struct dp_packet *packet)
 {
     struct mcast_snooping *ms = ctx->xbridge->ms;
-    struct xlate_cfg *xcfg;
     struct xbundle *mcast_xbundle;
     struct mcast_port_bundle *fport;
 
@@ -2537,9 +2534,8 @@ update_mcast_snooping_table(const struct xlate_ctx *ctx,
     /* Don't learn from flood ports */
     mcast_xbundle = NULL;
     ovs_rwlock_wrlock(&ms->rwlock);
-    xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
     LIST_FOR_EACH(fport, node, &ms->fport_list) {
-        mcast_xbundle = xbundle_lookup(xcfg, fport->port);
+        mcast_xbundle = xbundle_lookup(ctx->xcfg, fport->port);
         if (mcast_xbundle == in_xbundle) {
             break;
         }
@@ -2566,13 +2562,11 @@ xlate_normal_mcast_send_group(struct xlate_ctx *ctx,
                               const struct xvlan *xvlan)
     OVS_REQ_RDLOCK(ms->rwlock)
 {
-    struct xlate_cfg *xcfg;
     struct mcast_group_bundle *b;
     struct xbundle *mcast_xbundle;
 
-    xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
     LIST_FOR_EACH(b, bundle_node, &grp->bundle_lru) {
-        mcast_xbundle = xbundle_lookup(xcfg, b->port);
+        mcast_xbundle = xbundle_lookup(ctx->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);
@@ -2594,13 +2588,11 @@ xlate_normal_mcast_send_mrouters(struct xlate_ctx *ctx,
                                  const struct xvlan *xvlan)
     OVS_REQ_RDLOCK(ms->rwlock)
 {
-    struct xlate_cfg *xcfg;
     struct mcast_mrouter_bundle *mrouter;
     struct xbundle *mcast_xbundle;
 
-    xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
     LIST_FOR_EACH(mrouter, mrouter_node, &ms->mrouter_lru) {
-        mcast_xbundle = xbundle_lookup(xcfg, mrouter->port);
+        mcast_xbundle = xbundle_lookup(ctx->xcfg, mrouter->port);
         if (mcast_xbundle && mcast_xbundle != in_xbundle
             && mrouter->vlan == xvlan->v[0].vid) {
             xlate_report(ctx, OFT_DETAIL, "forwarding to mcast router port");
@@ -2626,13 +2618,11 @@ xlate_normal_mcast_send_fports(struct xlate_ctx *ctx,
                                const struct xvlan *xvlan)
     OVS_REQ_RDLOCK(ms->rwlock)
 {
-    struct xlate_cfg *xcfg;
     struct mcast_port_bundle *fport;
     struct xbundle *mcast_xbundle;
 
-    xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
     LIST_FOR_EACH(fport, node, &ms->fport_list) {
-        mcast_xbundle = xbundle_lookup(xcfg, fport->port);
+        mcast_xbundle = xbundle_lookup(ctx->xcfg, fport->port);
         if (mcast_xbundle && mcast_xbundle != in_xbundle) {
             xlate_report(ctx, OFT_DETAIL, "forwarding to mcast flood port");
             output_normal(ctx, mcast_xbundle, xvlan);
@@ -2654,13 +2644,11 @@ xlate_normal_mcast_send_rports(struct xlate_ctx *ctx,
                                const struct xvlan *xvlan)
     OVS_REQ_RDLOCK(ms->rwlock)
 {
-    struct xlate_cfg *xcfg;
     struct mcast_port_bundle *rport;
     struct xbundle *mcast_xbundle;
 
-    xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
     LIST_FOR_EACH(rport, node, &ms->rport_list) {
-        mcast_xbundle = xbundle_lookup(xcfg, rport->port);
+        mcast_xbundle = xbundle_lookup(ctx->xcfg, rport->port);
         if (mcast_xbundle
             && mcast_xbundle != in_xbundle
             && mcast_xbundle->ofbundle != in_xbundle->ofbundle) {
@@ -2891,8 +2879,7 @@ xlate_normal(struct xlate_ctx *ctx)
         ovs_rwlock_unlock(&ctx->xbridge->ml->rwlock);
 
         if (mac_port) {
-            struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
-            struct xbundle *mac_xbundle = xbundle_lookup(xcfg, mac_port);
+            struct xbundle *mac_xbundle = xbundle_lookup(ctx->xcfg, mac_port);
             if (mac_xbundle
                 && mac_xbundle != in_xbundle
                 && mac_xbundle->ofbundle != in_xbundle->ofbundle) {
@@ -3139,13 +3126,13 @@ process_special(struct xlate_ctx *ctx, const struct xport *xport)
 }
 
 static int
-tnl_route_lookup_flow(const struct flow *oflow,
+tnl_route_lookup_flow(const struct xlate_ctx *ctx,
+                      const struct flow *oflow,
                       struct in6_addr *ip, struct in6_addr *src,
                       struct xport **out_port)
 {
     char out_dev[IFNAMSIZ];
     struct xbridge *xbridge;
-    struct xlate_cfg *xcfg;
     struct in6_addr gw;
     struct in6_addr dst;
 
@@ -3161,10 +3148,7 @@ tnl_route_lookup_flow(const struct flow *oflow,
         *ip = dst;
     }
 
-    xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
-    ovs_assert(xcfg);
-
-    HMAP_FOR_EACH (xbridge, hmap_node, &xcfg->xbridges) {
+    HMAP_FOR_EACH (xbridge, hmap_node, &ctx->xcfg->xbridges) {
         if (!strncmp(xbridge->name, out_dev, IFNAMSIZ)) {
             struct xport *port;
 
@@ -3333,7 +3317,7 @@ native_tunnel_output(struct xlate_ctx *ctx, const struct xport *xport,
     memcpy(&old_base_flow, &ctx->base_flow, sizeof old_base_flow);
     memcpy(&old_flow, &ctx->xin->flow, sizeof old_flow);
 
-    err = tnl_route_lookup_flow(flow, &d_ip6, &s_ip6, &out_dev);
+    err = tnl_route_lookup_flow(ctx, flow, &d_ip6, &s_ip6, &out_dev);
     if (err) {
         xlate_report(ctx, OFT_WARN, "native tunnel routing failed");
         return err;
@@ -6841,6 +6825,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
         .xout = xout,
         .base_flow = *flow,
         .orig_tunnel_ipv6_dst = flow_tnl_dst(&flow->tunnel),
+        .xcfg = xcfg,
         .xbridge = xbridge,
         .stack = OFPBUF_STUB_INITIALIZER(stack_stub),
         .rule = xin->rule,
-- 
2.10.2



More information about the dev mailing list