[ovs-dev] [PATCH] ofproto-dpif: Fix vlan-splinter megaflow bug

Andy Zhou azhou at nicira.com
Tue Jan 7 09:11:18 UTC 2014


When vlan-splinter is enabled, ovs receives non-vlan flows from the
kernel vlan ports, vlan tag is then added to the incoming flow before
xlating, so that they look like those received from a trunk port.

In case megaflow is enabled, xlating may set vlan masks during rule
processing as usual. If those vlan masks were serialized and downloaded
to the kernel (this bug), those mega flows will be rejected due to
unexpected vlan mask encapsulation, since the original kernel flows do
not have vlan tags. This bug does not break connectivity, but impacts
performance since all traffic received on vlan splinter ports will now
be handled by vswitchd, as no datapath flows can be successfully
installed.

This fix is to make sure no vlan mask encapsulation is generated for
the datapath flow if its in_port was re-written by vlan-splinter
receiving logic.

Bug fixed: 22567

Signed-off-by: Andy Zhou <azhou at nicira.com>
---
 ofproto/ofproto-dpif-upcall.c |   45 ++++++++++++++++++++++++-----------------
 ofproto/ofproto-dpif-xlate.c  |    3 ---
 2 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index b57afdc..ab68f5d 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -1076,9 +1076,7 @@ handle_upcalls(struct handler *handler, struct list *upcalls)
     LIST_FOR_EACH (upcall, list_node, upcalls) {
         struct flow_miss *miss = upcall->flow_miss;
         struct ofpbuf *packet = &upcall->dpif_upcall.packet;
-        struct ofpbuf mask;
         struct dpif_op *op;
-        bool megaflow;
 
         if (miss->xout.slow) {
             struct xlate_in xin;
@@ -1087,14 +1085,35 @@ handle_upcalls(struct handler *handler, struct list *upcalls)
             xlate_actions_for_side_effects(&xin);
         }
 
-        atomic_read(&enable_megaflows, &megaflow);
-        ofpbuf_use_stack(&mask, &miss->mask_buf, sizeof miss->mask_buf);
-        if (megaflow) {
-            odp_flow_key_from_mask(&mask, &miss->xout.wc.masks, &miss->flow,
-                                   UINT32_MAX);
+        if (miss->flow.in_port.ofp_port
+            != vsp_realdev_to_vlandev(miss->ofproto,
+                                      miss->flow.in_port.ofp_port,
+                                      miss->flow.vlan_tci)) {
+            /* This packet was received on a VLAN splinter port.  We
+             * added a VLAN to the packet to make the packet resemble
+             * the flow, but the actions were composed assuming that
+             * the packet contained no VLAN.  So, we must remove the
+             * VLAN header from the packet before trying to execute the
+             * actions. */
+            eth_pop_vlan(packet);
+
+            /* Remove the flow vlan tags inserted by vlan splinter logic
+	     * to ensure megaflow masks generated will match the original
+	     * incoming flow. */
+            miss->flow.vlan_tci = 0;
         }
 
         if (may_put) {
+            struct ofpbuf mask;
+            bool megaflow;
+
+            atomic_read(&enable_megaflows, &megaflow);
+            ofpbuf_use_stack(&mask, &miss->mask_buf, sizeof miss->mask_buf);
+            if (megaflow) {
+                odp_flow_key_from_mask(&mask, &miss->xout.wc.masks, &miss->flow,
+                                       UINT32_MAX);
+            }
+
             op = &ops[n_ops++];
             op->type = DPIF_OP_FLOW_PUT;
             op->u.flow_put.flags = DPIF_FP_CREATE | DPIF_FP_MODIFY;
@@ -1119,18 +1138,6 @@ handle_upcalls(struct handler *handler, struct list *upcalls)
         }
 
         if (miss->xout.odp_actions.size) {
-            if (miss->flow.in_port.ofp_port
-                != vsp_realdev_to_vlandev(miss->ofproto,
-                                          miss->flow.in_port.ofp_port,
-                                          miss->flow.vlan_tci)) {
-                /* This packet was received on a VLAN splinter port.  We
-                 * added a VLAN to the packet to make the packet resemble
-                 * the flow, but the actions were composed assuming that
-                 * the packet contained no VLAN.  So, we must remove the
-                 * VLAN header from the packet before trying to execute the
-                 * actions. */
-                eth_pop_vlan(packet);
-            }
 
             op = &ops[n_ops++];
             op->type = DPIF_OP_EXECUTE;
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 13a5d07..160ca4b 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -1793,9 +1793,6 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
         ofp_port_t vlandev_port;
 
         odp_port = xport->odp_port;
-        if (ofproto_has_vlan_splinters(ctx->xbridge->ofproto)) {
-            wc->masks.vlan_tci |= htons(VLAN_VID_MASK | VLAN_CFI);
-        }
         vlandev_port = vsp_realdev_to_vlandev(ctx->xbridge->ofproto, ofp_port,
                                               flow->vlan_tci);
         if (vlandev_port == ofp_port) {
-- 
1.7.9.5




More information about the dev mailing list