lib: Fix MPLS masking.

Previously we masked labels not present in the incoming packet.

Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>

diff --git a/lib/flow.c b/lib/flow.c
index df1996f..7fb53de 100644 (file)
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -1334,9 +1334,7 @@ flow_set_vlan_pcp(struct flow *flow, uint8_t pcp)
 int
 flow_count_mpls_labels(const struct flow *flow, struct flow_wildcards *wc)
 {
-    if (wc) {
-        wc->masks.dl_type = OVS_BE16_MAX;
-    }
+    /* dl_type is always masked. */
     if (eth_type_mpls(flow->dl_type)) {
         int i;
         int len = FLOW_MAX_MPLS_LABELS;
@@ -1405,7 +1403,7 @@ flow_count_common_mpls_labels(const struct flow *a, int an,
  *
  *     - BoS: 1.
  *
- * If the new label is the second or label MPLS label in 'flow', it is
+ * If the new label is the second or later label MPLS label in 'flow', it is
  * generated as;
  *
  *     - label: Copied from outer label.
@@ -1426,15 +1424,16 @@ flow_push_mpls(struct flow *flow, int n, ovs_be16 mpls_eth_type,
     ovs_assert(eth_type_mpls(mpls_eth_type));
     ovs_assert(n < FLOW_MAX_MPLS_LABELS);
 
-    memset(wc->masks.mpls_lse, 0xff, sizeof wc->masks.mpls_lse);
     if (n) {
         int i;
 
+        if (wc) {
+            memset(&wc->masks.mpls_lse, 0xff, sizeof *wc->masks.mpls_lse * n);
+        }
         for (i = n; i >= 1; i--) {
             flow->mpls_lse[i] = flow->mpls_lse[i - 1];
         }
-        flow->mpls_lse[0] = (flow->mpls_lse[1]
-                             & htonl(~MPLS_BOS_MASK));
+        flow->mpls_lse[0] = (flow->mpls_lse[1] & htonl(~MPLS_BOS_MASK));
     } else {
         int label = 0;          /* IPv4 Explicit Null. */
         int tc = 0;
@@ -1446,12 +1445,14 @@ flow_push_mpls(struct flow *flow, int n, ovs_be16 mpls_eth_type,
 
         if (is_ip_any(flow)) {
             tc = (flow->nw_tos & IP_DSCP_MASK) >> 2;
-            wc->masks.nw_tos |= IP_DSCP_MASK;
+            if (wc) {
+                wc->masks.nw_tos |= IP_DSCP_MASK;
+                wc->masks.nw_ttl = 0xff;
+            }
 
             if (flow->nw_ttl) {
                 ttl = flow->nw_ttl;
             }
-            wc->masks.nw_ttl = 0xff;
         }
 
         flow->mpls_lse[0] = set_mpls_lse_values(ttl, tc, 1, htonl(label));
@@ -1478,13 +1479,20 @@ flow_pop_mpls(struct flow *flow, int n, ovs_be16 eth_type,
     if (n == 0) {
         /* Nothing to pop. */
         return false;
-    } else if (n == FLOW_MAX_MPLS_LABELS
-               && !(flow->mpls_lse[n - 1] & htonl(MPLS_BOS_MASK))) {
-        /* Can't pop because we don't know what to fill in mpls_lse[n - 1]. */
-        return false;
+    } else if (n == FLOW_MAX_MPLS_LABELS) {
+        if (wc) {
+            wc->masks.mpls_lse[n - 1] |= htonl(MPLS_BOS_MASK);
+        }
+        if (!(flow->mpls_lse[n - 1] & htonl(MPLS_BOS_MASK))) {
+            /* Can't pop because don't know what to fill in mpls_lse[n - 1]. */
+            return false;
+        }
     }
 
-    memset(wc->masks.mpls_lse, 0xff, sizeof wc->masks.mpls_lse);
+    if (wc) {
+        memset(&wc->masks.mpls_lse[1], 0xff,
+               sizeof *wc->masks.mpls_lse * (n - 1));
+    }
     for (i = 1; i < n; i++) {
         flow->mpls_lse[i - 1] = flow->mpls_lse[i];
     }

diff --git a/lib/odp-util.c b/lib/odp-util.c
index b9e1e21..8f5ed08 100644 (file)
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -3687,14 +3687,15 @@ commit_vlan_action(ovs_be16 vlan_tci, struct flow *base,
     base->vlan_tci = vlan_tci;
 }
 
+/* Wildcarding already done at action translation time. */
 static void
 commit_mpls_action(const struct flow *flow, struct flow *base,
-                   struct ofpbuf *odp_actions, struct flow_wildcards *wc)
+                   struct ofpbuf *odp_actions)
 {
-    int base_n = flow_count_mpls_labels(base, wc);
-    int flow_n = flow_count_mpls_labels(flow, wc);
+    int base_n = flow_count_mpls_labels(base, NULL);
+    int flow_n = flow_count_mpls_labels(flow, NULL);
     int common_n = flow_count_common_mpls_labels(flow, flow_n, base, base_n,
-                                                 wc);
+                                                 NULL);
 
     while (base_n > common_n) {
         if (base_n - 1 == common_n && flow_n > common_n) {
@@ -3731,7 +3732,7 @@ commit_mpls_action(const struct flow *flow, struct flow *base,
                 dl_type = flow->dl_type;
             }
             nl_msg_put_be16(odp_actions, OVS_ACTION_ATTR_POP_MPLS, dl_type);
-            popped = flow_pop_mpls(base, base_n, flow->dl_type, wc);
+            popped = flow_pop_mpls(base, base_n, flow->dl_type, NULL);
             ovs_assert(popped);
             base_n--;
         }
@@ -3747,7 +3748,7 @@ commit_mpls_action(const struct flow *flow, struct flow *base,
                                       sizeof *mpls);
         mpls->mpls_ethertype = flow->dl_type;
         mpls->mpls_lse = flow->mpls_lse[flow_n - base_n - 1];
-        flow_push_mpls(base, base_n, mpls->mpls_ethertype, wc);
+        flow_push_mpls(base, base_n, mpls->mpls_ethertype, NULL);
         flow_set_mpls_lse(base, 0, mpls->mpls_lse);
         base_n++;
     }
@@ -4032,7 +4033,7 @@ commit_odp_actions(const struct flow *flow, struct flow *base,
     commit_set_ether_addr_action(flow, base, odp_actions, wc, use_masked);
     slow = commit_set_nw_action(flow, base, odp_actions, wc, use_masked);
     commit_set_port_action(flow, base, odp_actions, wc, use_masked);
-    commit_mpls_action(flow, base, odp_actions, wc);
+    commit_mpls_action(flow, base, odp_actions);
     commit_vlan_action(flow->vlan_tci, base, odp_actions, wc);
     commit_set_priority_action(flow, base, odp_actions, wc, use_masked);
     commit_set_pkt_mark_action(flow, base, odp_actions, wc, use_masked);

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 1d46456..851b946 100644 (file)
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3242,24 +3242,23 @@ static bool
 compose_dec_mpls_ttl_action(struct xlate_ctx *ctx)
 {
     struct flow *flow = &ctx->xin->flow;
-    uint8_t ttl = mpls_lse_to_ttl(flow->mpls_lse[0]);
     struct flow_wildcards *wc = &ctx->xout->wc;
 
-    memset(&wc->masks.mpls_lse, 0xff, sizeof wc->masks.mpls_lse);
     if (eth_type_mpls(flow->dl_type)) {
+        uint8_t ttl = mpls_lse_to_ttl(flow->mpls_lse[0]);
+
+        wc->masks.mpls_lse[0] |= htonl(MPLS_TTL_MASK);
         if (ttl > 1) {
             ttl--;
             set_mpls_lse_ttl(&flow->mpls_lse[0], ttl);
             return false;
         } else {
             execute_controller_action(ctx, UINT16_MAX, OFPR_INVALID_TTL, 0);
-
-            /* Stop processing for current table. */
-            return true;
         }
-    } else {
-        return true;
     }
+
+    /* Stop processing for current table. */
+    return true;
 }
 
 static void

diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index dad9795..dd966cd 100644 (file)
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -4674,11 +4674,10 @@ for dl_src in 00 01; do
     AT_CHECK([ovs-appctl netdev-dummy/receive p1 "505400000007 6066666666$dl_src 8847 00014020 00014120 45 00 00 2c 00 00 00 00 40 06 3b 78 c0 a8 00 01 c0 a8 00 02 00 50 00 00 00 00 00 2a 00 00 00 2a 50 00 27 10 77 44 00 00 48 4f 47 45"])
 done
 sleep 1  # wait for the datapath flow installed
-for dl_src in 00 01; do
-    AT_CHECK_UNQUOTED([cat ovs-vswitchd.log | FILTER_FLOW_INSTALL | grep "$dl_src," | STRIP_USED], [0], [dnl
-recirc_id=0,mpls,in_port=1,dl_src=60:66:66:66:66:$dl_src,mpls_label=20,mpls_tc=0,mpls_ttl=32,mpls_bos=0,mpls_lse1=82208,mpls_lse2=0, actions:userspace(pid=0,slow_path(controller))
+AT_CHECK_UNQUOTED([cat ovs-vswitchd.log | FILTER_FLOW_INSTALL | STRIP_USED], [0], [dnl
+recirc_id=0,mpls,in_port=1,dl_src=60:66:66:66:66:00,mpls_label=20,mpls_tc=0,mpls_ttl=32,mpls_bos=0,mpls_lse1=82208, actions:userspace(pid=0,slow_path(controller))
+recirc_id=0,mpls,in_port=1,dl_src=60:66:66:66:66:01,mpls_bos=0,mpls_lse1=82208, actions:userspace(pid=0,slow_path(controller))
 ])
-done
 
 OVS_VSWITCHD_STOP
 AT_CLEANUP
@@ -4714,11 +4713,10 @@ for dl_src in 00 01; do
     AT_CHECK([ovs-appctl netdev-dummy/receive p1 "505400000007 6066666666$dl_src 8847 00014020 00014120 45 00 00 2c 00 00 00 00 40 06 3b 78 c0 a8 00 01 c0 a8 00 02 00 50 00 00 00 00 00 2a 00 00 00 2a 50 00 27 10 77 44 00 00 48 4f 47 45"])
 done
 sleep 1  # wait for the datapath flow installed
-for dl_src in 00 01; do
-    AT_CHECK_UNQUOTED([cat ovs-vswitchd.log | FILTER_FLOW_INSTALL | grep "$dl_src," | STRIP_USED], [0], [dnl
-recirc_id=0,mpls,in_port=1,dl_src=60:66:66:66:66:$dl_src,mpls_label=20,mpls_tc=0,mpls_ttl=32,mpls_bos=0,mpls_lse1=82208,mpls_lse2=0, actions:userspace(pid=0,slow_path(controller))
+AT_CHECK_UNQUOTED([cat ovs-vswitchd.log | FILTER_FLOW_INSTALL | STRIP_USED], [0], [dnl
+recirc_id=0,mpls,in_port=1,dl_src=60:66:66:66:66:00,mpls_label=20,mpls_tc=0,mpls_ttl=32,mpls_bos=0,mpls_lse1=82208, actions:userspace(pid=0,slow_path(controller))
+recirc_id=0,mpls,in_port=1,dl_src=60:66:66:66:66:01,mpls_bos=0,mpls_lse1=82208, actions:userspace(pid=0,slow_path(controller))
 ])
-done
 
 OVS_VSWITCHD_STOP
 AT_CLEANUP
@@ -4936,8 +4934,8 @@ AT_CHECK([ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00:00:00:
 AT_CHECK([ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0a),eth_type(0x8847),mpls(label=11,tc=3,ttl=64,bos=1)'])
 sleep 1
 AT_CHECK([cat ovs-vswitchd.log | FILTER_FLOW_INSTALL | STRIP_XOUT], [0], [dnl
-recirc_id=0,mpls,in_port=1,dl_src=50:54:00:00:00:09,mpls_label=11,mpls_tc=3,mpls_ttl=64,mpls_bos=1,mpls_lse1=0,mpls_lse2=0, actions: <del>
-recirc_id=0,mpls,in_port=1,dl_src=50:54:00:00:00:0b,mpls_label=11,mpls_tc=3,mpls_ttl=64,mpls_bos=1,mpls_lse1=0,mpls_lse2=0, actions: <del>
+recirc_id=0,mpls,in_port=1,dl_src=50:54:00:00:00:09,mpls_label=11,mpls_tc=3,mpls_ttl=64,mpls_bos=1, actions: <del>
+recirc_id=0,mpls,in_port=1,dl_src=50:54:00:00:00:0b,mpls_bos=1, actions: <del>
 ])
 OVS_VSWITCHD_STOP
 AT_CLEANUP