[ovs-dev] [PATCH] xlate: fix xport lookup for recirc

Zoltán Balogh zoltan.balogh at ericsson.com
Thu Dec 21 14:22:43 UTC 2017


Xlate_lookup and xlate_lookup_ofproto_() provides in_port and ofproto
based on xport determined using flow, which is extracted from packet.
The lookup can happen due to recirculation as well. It can happen, that
packet_type has been modified during xlate before recirculation is
triggered, so the lookup fails or delivers wrong xport.
This can be worked around by propagating xport to ctx->xin after the very
first lookup and store it in frozen state of the recirculation.
So, when lookup is performed due to recirculation, the xport can be
retrieved from the frozen state.

Signed-off-by: Zoltan Balogh <zoltan.balogh at ericsson.com>
CC: Jan Scheurich <jan.scheurich at ericsson.com>
---
 ofproto/ofproto-dpif-rid.c    |  4 +++-
 ofproto/ofproto-dpif-rid.h    |  1 +
 ofproto/ofproto-dpif-upcall.c |  2 ++
 ofproto/ofproto-dpif-xlate.c  | 30 ++++++++++++++++++++++++++++++
 ofproto/ofproto-dpif-xlate.h  |  3 +++
 5 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/ofproto/ofproto-dpif-rid.c b/ofproto/ofproto-dpif-rid.c
index fc5700489..79de90520 100644
--- a/ofproto/ofproto-dpif-rid.c
+++ b/ofproto/ofproto-dpif-rid.c
@@ -158,7 +158,8 @@ frozen_state_equal(const struct frozen_state *a, const struct frozen_state *b)
             && ofpacts_equal(a->ofpacts, a->ofpacts_len,
                              b->ofpacts, b->ofpacts_len)
             && ofpacts_equal(a->action_set, a->action_set_len,
-                             b->action_set, b->action_set_len));
+                             b->action_set, b->action_set_len)
+            && a->xport_in == b->xport_in);
 }
 
 /* Lockless RCU protected lookup.  If node is needed accross RCU quiescent
@@ -285,6 +286,7 @@ recirc_alloc_id(struct ofproto_dpif *ofproto)
                 .ipv6_dst = in6addr_any,
             },
             .in_port = OFPP_NONE },
+        .xport_in = NULL,
     };
     return recirc_alloc_id__(&state, frozen_state_hash(&state))->id;
 }
diff --git a/ofproto/ofproto-dpif-rid.h b/ofproto/ofproto-dpif-rid.h
index 19fc27c7c..f02221094 100644
--- a/ofproto/ofproto-dpif-rid.h
+++ b/ofproto/ofproto-dpif-rid.h
@@ -143,6 +143,7 @@ struct frozen_state {
     size_t stack_size;
     mirror_mask_t mirrors;        /* Mirrors already output. */
     bool conntracked;             /* Conntrack occurred prior to freeze. */
+    const struct xport *xport_in; /* Port packet was received on. */
 
     /* Actions to be translated when thawing. */
     struct ofpact *ofpacts;
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 02cf5415b..92a92d288 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -236,6 +236,8 @@ struct upcall {
     const struct nlattr *out_tun_key;  /* Datapath output tunnel key. */
 
     uint64_t odp_actions_stub[1024 / 8]; /* Stub for odp_actions. */
+
+    const struct xport *xport_in;   /* Port the packet was received on. */
 };
 
 /* Ukeys must transition through these states using transition_ukey(). */
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 9b3a2f28a..c9fb32338 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -1362,12 +1362,36 @@ xlate_lookup_ofproto_(const struct dpif_backer *backer, const struct flow *flow,
     struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
     const struct xport *xport;
 
+    /* If packet is recirculated, xport can be retrieved from frozen state. */
+    if (flow->recirc_id) {
+        const struct recirc_id_node *recirc_id_node;
+
+        recirc_id_node = recirc_id_node_find(flow->recirc_id);
+
+        if (OVS_UNLIKELY(!recirc_id_node)) {
+            return NULL;
+        }
+
+        /* If recirculation was initiated due to bond (in_port = OFPP_NONE),
+         * xport cannot be retrieved from frozen state. */
+        if (recirc_id_node->state.metadata.in_port != OFPP_NONE) {
+            xport = recirc_id_node->state.xport_in;
+            /* Verify, if xport is valid. */
+            if (xport && hmap_contains(&xcfg->xports, &xport->hmap_node) &&
+                xport->xbridge && xport->xbridge->ofproto) {
+                goto out;
+            }
+        }
+    }
+
     xport = xport_lookup(xcfg, tnl_port_should_receive(flow)
                          ? tnl_port_receive(flow)
                          : odp_port_to_ofport(backer, flow->in_port.odp_port));
     if (OVS_UNLIKELY(!xport)) {
         return NULL;
     }
+
+out:
     *xportp = xport;
     if (ofp_in_port) {
         *ofp_in_port = xport->ofp_port;
@@ -4623,6 +4647,7 @@ finish_freezing__(struct xlate_ctx *ctx, uint8_t table)
         .stack_size = ctx->stack.size,
         .mirrors = ctx->mirrors,
         .conntracked = ctx->conntracked,
+        .xport_in = ctx->xin->xport_in,
         .ofpacts = ctx->frozen_actions.data,
         .ofpacts_len = ctx->frozen_actions.size,
         .action_set = ctx->action_set.data,
@@ -6613,6 +6638,7 @@ xlate_in_init(struct xlate_in *xin, struct ofproto_dpif *ofproto,
     xin->odp_actions = odp_actions;
     xin->in_packet_out = false;
     xin->recirc_queue = NULL;
+    xin->xport_in = NULL;
 
     /* Do recirc lookup. */
     xin->frozen_state = NULL;
@@ -7040,6 +7066,9 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
      * flow->in_port is the ultimate input port of the packet.) */
     struct xport *in_port = get_ofp_port(xbridge,
                                          ctx.base_flow.in_port.ofp_port);
+    if (in_port && !in_port->peer) {
+        ctx.xin->xport_in = in_port;
+    }
 
     if (flow->packet_type != htonl(PT_ETH) && in_port &&
         in_port->pt_mode == NETDEV_PT_LEGACY_L3 && ctx.table_id == 0) {
@@ -7279,6 +7308,7 @@ xlate_resume(struct ofproto_dpif *ofproto,
         .stack_size = pin->stack_size,
         .mirrors = pin->mirrors,
         .conntracked = pin->conntracked,
+        .xport_in = NULL,
 
         /* When there are no actions, xlate_actions() will search the flow
          * table.  We don't want it to do that (we want it to resume), so
diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h
index 39542de2b..932ebc492 100644
--- a/ofproto/ofproto-dpif-xlate.h
+++ b/ofproto/ofproto-dpif-xlate.h
@@ -162,6 +162,9 @@ struct xlate_in {
     /* ofproto/trace maintains this queue to trace flows that require
      * recirculation. */
     struct ovs_list *recirc_queue;
+
+    /* Non-patch port packet was received on.*/
+    const struct xport *xport_in;
 };
 
 void xlate_ofproto_set(struct ofproto_dpif *, const char *name, struct dpif *,
-- 
2.14.1



More information about the dev mailing list