[ovs-dev] [PATCH v5 5/6] ofproto-dpif-xlate: refactor compose_output_action__

Zoltán Balogh zoltan.balogh at ericsson.com
Sat May 6 15:50:53 UTC 2017


From: Jan Scheurich <jan.scheurich at ericsson.com>

The very long function compose_output_action__() has been re-factored to make
the different cases for output to patch-port, native tunnel port, kernel tunnel
port, recirculation, or termination of a native tunnel at output to LOCAL port
clearer. Larger, self-contained blocks have been split out into separate
functions.

Signed-off-by: Jan Scheurich <jan.scheurich at ericsson.com>
Co-authored-by: Zoltan Balogh <zoltan.balogh at ericsson.com>
---
 ofproto/ofproto-dpif-xlate.c | 213 ++++++++++++++++++++++++-------------------
 1 file changed, 120 insertions(+), 93 deletions(-)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 0485a4f..4486039 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3269,6 +3269,62 @@ xlate_flow_is_protected(const struct xlate_ctx *ctx, const struct flow *flow, co
             xport_in->xbundle->protected && xport_out->xbundle->protected);
 }
 
+static bool
+check_output_prerequisites(struct xlate_ctx *ctx,
+                           const struct xport *xport,
+                           struct flow *flow,
+                           bool check_stp)
+{
+    struct flow_wildcards *wc = ctx->wc;
+
+    if (!xport) {
+        xlate_report(ctx, OFT_WARN, "Nonexistent output port");
+        return false;
+    } else if (xport->config & OFPUTIL_PC_NO_FWD) {
+        xlate_report(ctx, OFT_DETAIL, "OFPPC_NO_FWD set, skipping output");
+        return false;
+    } else if (ctx->mirror_snaplen != 0 && xport->odp_port == ODPP_NONE) {
+        xlate_report(ctx, OFT_WARN,
+                     "Mirror truncate to ODPP_NONE, skipping output");
+        return false;
+    } else if (xlate_flow_is_protected(ctx, flow, xport)) {
+        xlate_report(ctx, OFT_WARN,
+                     "Flow is between protected ports, skipping output.");
+        return false;
+    } else if (check_stp) {
+        if (is_stp(&ctx->base_flow)) {
+            if (!xport_stp_should_forward_bpdu(xport) &&
+                !xport_rstp_should_manage_bpdu(xport)) {
+                if (ctx->xbridge->stp != NULL) {
+                    xlate_report(ctx, OFT_WARN,
+                                 "STP not in listening state, "
+                                 "skipping bpdu output");
+                } else if (ctx->xbridge->rstp != NULL) {
+                    xlate_report(ctx, OFT_WARN,
+                                 "RSTP not managing BPDU in this state, "
+                                 "skipping bpdu output");
+                }
+                return false;
+            }
+        } else if ((xport->cfm && cfm_should_process_flow(xport->cfm, flow, wc))
+                   || (xport->bfd && bfd_should_process_flow(xport->bfd, flow,
+                                                             wc))) {
+            /* Pass; STP should not block link health detection. */
+        } else if (!xport_stp_forward_state(xport) ||
+                   !xport_rstp_forward_state(xport)) {
+            if (ctx->xbridge->stp != NULL) {
+                xlate_report(ctx, OFT_WARN,
+                             "STP not in forwarding state, skipping output");
+            } else if (ctx->xbridge->rstp != NULL) {
+                xlate_report(ctx, OFT_WARN,
+                             "RSTP not in forwarding state, skipping output");
+            }
+            return false;
+        }
+    }
+    return true;
+}
+
 /* Populate and apply nested actions on 'out_dev'.
  * The nested actions are applied on cloned packets in dp while outputting to
  * either patch or tunnel ports.
@@ -3399,6 +3455,25 @@ apply_nested_clone_actions(struct xlate_ctx *ctx, const struct xport *in_dev,
     }
 }
 
+static inline bool
+terminate_native_tunnel(struct xlate_ctx *ctx,
+                        ofp_port_t ofp_port,
+                        struct flow *flow,
+                        struct flow_wildcards *wc,
+                        odp_port_t *tnl_port)
+{
+    *tnl_port = ODPP_NONE;
+
+    /* XXX: Write better Filter for tunnel port. We can use in_port
+     * in tunnel-port flow to avoid these checks completely. */
+    if (ofp_port == OFPP_LOCAL &&
+        ovs_native_tunneling_is_on(ctx->xbridge->ofproto)) {
+        *tnl_port = tnl_port_map_lookup(flow, wc);
+    }
+
+    return (*tnl_port != ODPP_NONE);
+}
+
 static void
 compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
                         const struct xlate_bond_recirc *xr, bool check_stp)
@@ -3409,8 +3484,8 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
     struct flow_tnl flow_tnl;
     union flow_vlan_hdr flow_vlans[FLOW_MAX_VLAN_HEADERS];
     uint8_t flow_nw_tos;
-    odp_port_t out_port, odp_port;
-    bool tnl_push_pop_send = false;
+    odp_port_t out_port, odp_port, odp_tnl_port;
+    bool is_native_tunnel = false;
     uint8_t dscp;
 
     /* If 'struct flow' gets additional metadata, we'll need to zero it out
@@ -3418,52 +3493,18 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
     BUILD_ASSERT_DECL(FLOW_WC_SEQ == 39);
     memset(&flow_tnl, 0, sizeof flow_tnl);
 
-    if (!xport) {
-        xlate_report(ctx, OFT_WARN, "Nonexistent output port");
+    if (!check_output_prerequisites(ctx, xport, flow, check_stp)) {
         return;
-    } else if (xport->config & OFPUTIL_PC_NO_FWD) {
-        xlate_report(ctx, OFT_DETAIL, "OFPPC_NO_FWD set, skipping output");
-        return;
-    } else if (ctx->mirror_snaplen != 0 && xport->odp_port == ODPP_NONE) {
-        xlate_report(ctx, OFT_WARN,
-                     "Mirror truncate to ODPP_NONE, skipping output");
-        return;
-    } else if (xlate_flow_is_protected(ctx, flow, xport)) {
-        xlate_report(ctx, OFT_WARN,
-                     "Flow is between protected ports, skipping output.");
-        return;
-    } else if (check_stp) {
-        if (is_stp(&ctx->base_flow)) {
-            if (!xport_stp_should_forward_bpdu(xport) &&
-                !xport_rstp_should_manage_bpdu(xport)) {
-                if (ctx->xbridge->stp != NULL) {
-                    xlate_report(ctx, OFT_WARN,
-                                 "STP not in listening state, "
-                                 "skipping bpdu output");
-                } else if (ctx->xbridge->rstp != NULL) {
-                    xlate_report(ctx, OFT_WARN,
-                                 "RSTP not managing BPDU in this state, "
-                                 "skipping bpdu output");
-                }
-                return;
-            }
-        } else if ((xport->cfm && cfm_should_process_flow(xport->cfm, flow, wc))
-                   || (xport->bfd && bfd_should_process_flow(xport->bfd, flow,
-                                                             wc))) {
-            /* Pass; STP should not block link health detection. */
-        } else if (!xport_stp_forward_state(xport) ||
-                   !xport_rstp_forward_state(xport)) {
-            if (ctx->xbridge->stp != NULL) {
-                xlate_report(ctx, OFT_WARN,
-                             "STP not in forwarding state, skipping output");
-            } else if (ctx->xbridge->rstp != NULL) {
-                xlate_report(ctx, OFT_WARN,
-                             "RSTP not in forwarding state, skipping output");
-            }
-            return;
-        }
     }
 
+    if (xport->peer) {
+       apply_nested_clone_actions(ctx, xport, xport->peer);
+       return;
+    }
+
+    memcpy(flow_vlans, flow->vlans, sizeof flow_vlans);
+    flow_nw_tos = flow->nw_tos;
+
     if (flow->packet_type == htonl(PT_ETH) && xport->is_layer3 ) {
         /* Ethernet packet to L3 outport -> pop ethernet header. */
         flow->packet_type = PACKET_TYPE_BE(OFPHTN_ETHERTYPE,
@@ -3476,14 +3517,6 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
         flow->dl_src = eth_addr_zero;
     }
 
-    if (xport->peer) {
-       apply_nested_clone_actions(ctx, xport, xport->peer);
-       return;
-    }
-
-    memcpy(flow_vlans, flow->vlans, sizeof flow_vlans);
-    flow_nw_tos = flow->nw_tos;
-
     if (count_skb_priorities(xport)) {
         memset(&wc->masks.skb_priority, 0xff, sizeof wc->masks.skb_priority);
         if (dscp_from_skb_priority(xport, flow->skb_priority, &dscp)) {
@@ -3522,7 +3555,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
         out_port = odp_port;
         if (ovs_native_tunneling_is_on(ctx->xbridge->ofproto)) {
             xlate_report(ctx, OFT_DETAIL, "output to native tunnel");
-            tnl_push_pop_send = true;
+            is_native_tunnel = true;
         } else {
             xlate_report(ctx, OFT_DETAIL, "output to kernel tunnel");
             commit_odp_tunnel_action(flow, &ctx->base_flow, ctx->odp_actions);
@@ -3534,9 +3567,12 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
     }
 
     if (out_port != ODPP_NONE) {
+
+        /* Commit accumulated flow updates before output. */
         xlate_commit_actions(ctx);
 
         if (xr) {
+            /* Recirculate the packet */
             struct ovs_action_hash *act_hash;
 
             /* Hash action. */
@@ -3549,50 +3585,41 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
             /* Recirc action. */
             nl_msg_put_u32(ctx->odp_actions, OVS_ACTION_ATTR_RECIRC,
                            xr->recirc_id);
-        } else {
 
-            if (tnl_push_pop_send) {
-                build_tunnel_send(ctx, xport, flow, odp_port);
-                flow->tunnel = flow_tnl; /* Restore tunnel metadata */
-            } else {
-                odp_port_t odp_tnl_port = ODPP_NONE;
-
-                /* XXX: Write better Filter for tunnel port. We can use inport
-                * int tunnel-port flow to avoid these checks completely. */
-                if (ofp_port == OFPP_LOCAL &&
-                    ovs_native_tunneling_is_on(ctx->xbridge->ofproto)) {
+        } else if (is_native_tunnel) {
+            /* Output to native tunnel port. */
+            build_tunnel_send(ctx, xport, flow, odp_port);
+            flow->tunnel = flow_tnl; /* Restore tunnel metadata */
 
-                    odp_tnl_port = tnl_port_map_lookup(flow, wc);
-                }
+        } else if (terminate_native_tunnel(ctx, ofp_port, flow, wc,
+                                           &odp_tnl_port)) {
+            /* Intercept packet to be received on native tunnel port. */
+            nl_msg_put_odp_port(ctx->odp_actions,
+                                OVS_ACTION_ATTR_TUNNEL_POP,
+                                odp_tnl_port);
 
-                if (odp_tnl_port != ODPP_NONE) {
-                    nl_msg_put_odp_port(ctx->odp_actions,
-                                        OVS_ACTION_ATTR_TUNNEL_POP,
-                                        odp_tnl_port);
-                } else {
-                    /* Tunnel push-pop action is not compatible with
-                     * IPFIX action. */
-                    compose_ipfix_action(ctx, out_port);
-
-                    /* Handle truncation of the mirrored packet. */
-                    if (ctx->mirror_snaplen > 0 &&
-                        ctx->mirror_snaplen < UINT16_MAX) {
-                        struct ovs_action_trunc *trunc;
-
-                        trunc = nl_msg_put_unspec_uninit(ctx->odp_actions,
-                                                         OVS_ACTION_ATTR_TRUNC,
-                                                         sizeof *trunc);
-                        trunc->max_len = ctx->mirror_snaplen;
-                        if (!ctx->xbridge->support.trunc) {
-                            ctx->xout->slow |= SLOW_ACTION;
-                        }
-                    }
-
-                    nl_msg_put_odp_port(ctx->odp_actions,
-                                        OVS_ACTION_ATTR_OUTPUT,
-                                        out_port);
+        } else {
+            /* Tunnel push-pop action is not compatible with
+             * IPFIX action. */
+            compose_ipfix_action(ctx, out_port);
+
+            /* Handle truncation of the mirrored packet. */
+            if (ctx->mirror_snaplen > 0 &&
+                    ctx->mirror_snaplen < UINT16_MAX) {
+                struct ovs_action_trunc *trunc;
+
+                trunc = nl_msg_put_unspec_uninit(ctx->odp_actions,
+                                                 OVS_ACTION_ATTR_TRUNC,
+                                                 sizeof *trunc);
+                trunc->max_len = ctx->mirror_snaplen;
+                if (!ctx->xbridge->support.trunc) {
+                    ctx->xout->slow |= SLOW_ACTION;
                 }
             }
+
+            nl_msg_put_odp_port(ctx->odp_actions,
+                                OVS_ACTION_ATTR_OUTPUT,
+                                out_port);
         }
 
         ctx->sflow_odp_port = odp_port;
-- 
2.7.4



More information about the dev mailing list