Correctly implement the OpenFlow 1.2+ OXM_OF_IP_DSCP field.

NXM puts the DSCP value in bits 2-7 of NXM_OF_IP_TOS. OXM puts the DSCP value in bits 0-6 of OXM_OF_IP_DSCP. Before this commit, Open vSwitch incorrectly implemented OXM_OF_IP_DSCP with the same format as NXM_OF_IP_TOS. This commit fixes the problem and adds a test (previously missing but I don't know why). Reported-by: Hiroshi Miyata <[email protected]> Tested-by: Hiroshi Miyata <[email protected]> Signed-off-by: Ben Pfaff <[email protected]>
Cartmanfku · Apr 18, 2013 · 1638b90 · 1638b90
1 parent 781d447
commit 1638b90
Show file tree

Hide file tree

Showing 5 changed files with 61 additions and 4 deletions.
diff --git a/AUTHORS b/AUTHORS
@@ -139,6 +139,7 @@ Hassan Khan             [email protected]
 Hector Oron             [email protected]
 Henrik Amren            [email protected]
 Hiroshi Tanaka          [email protected]
+Hiroshi Miyata          [email protected]
 Igor Ganichev           [email protected]
 Jacob Cherkas           [email protected]
 Jad Naous               [email protected]

diff --git a/lib/meta-flow.c b/lib/meta-flow.c
@@ -359,6 +359,15 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
         MFP_IP_ANY,
         true,
         NXM_OF_IP_TOS, "NXM_OF_IP_TOS",
+        NXM_OF_IP_TOS, "NXM_OF_IP_TOS",
+    }, {
+        MFF_IP_DSCP_SHIFTED, "nw_tos_shifted", NULL,
+        MF_FIELD_SIZES(u8),
+        MFM_NONE,
+        MFS_DECIMAL,
+        MFP_IP_ANY,
+        true,
+        OXM_OF_IP_DSCP, "OXM_OF_IP_DSCP",
         OXM_OF_IP_DSCP, "OXM_OF_IP_DSCP",
     }, {
         MFF_IP_ECN, "nw_ecn", NULL,
@@ -733,6 +742,7 @@ mf_is_all_wild(const struct mf_field *mf, const struct flow_wildcards *wc)
     case MFF_IP_PROTO:
         return !wc->masks.nw_proto;
     case MFF_IP_DSCP:
+    case MFF_IP_DSCP_SHIFTED:
         return !(wc->masks.nw_tos & IP_DSCP_MASK);
     case MFF_IP_ECN:
         return !(wc->masks.nw_tos & IP_ECN_MASK);
@@ -916,6 +926,8 @@ mf_is_value_valid(const struct mf_field *mf, const union mf_value *value)
 
     case MFF_IP_DSCP:
         return !(value->u8 & ~IP_DSCP_MASK);
+    case MFF_IP_DSCP_SHIFTED:
+        return !(value->u8 & (~IP_DSCP_MASK >> 2));
     case MFF_IP_ECN:
         return !(value->u8 & ~IP_ECN_MASK);
     case MFF_IP_FRAG:
@@ -1065,6 +1077,10 @@ mf_get_value(const struct mf_field *mf, const struct flow *flow,
         value->u8 = flow->nw_tos & IP_DSCP_MASK;
         break;
 
+    case MFF_IP_DSCP_SHIFTED:
+        value->u8 = flow->nw_tos >> 2;
+        break;
+
     case MFF_IP_ECN:
         value->u8 = flow->nw_tos & IP_ECN_MASK;
         break;
@@ -1244,6 +1260,10 @@ mf_set_value(const struct mf_field *mf,
         match_set_nw_dscp(match, value->u8);
         break;
 
+    case MFF_IP_DSCP_SHIFTED:
+        match_set_nw_dscp(match, value->u8 << 2);
+        break;
+
     case MFF_IP_ECN:
         match_set_nw_ecn(match, value->u8);
         break;
@@ -1424,6 +1444,11 @@ mf_set_flow_value(const struct mf_field *mf,
         flow->nw_tos |= value->u8 & IP_DSCP_MASK;
         break;
 
+    case MFF_IP_DSCP_SHIFTED:
+        flow->nw_tos &= ~IP_DSCP_MASK;
+        flow->nw_tos |= value->u8 << 2;
+        break;
+
     case MFF_IP_ECN:
         flow->nw_tos &= ~IP_ECN_MASK;
         flow->nw_tos |= value->u8 & IP_ECN_MASK;
@@ -1624,6 +1649,7 @@ mf_set_wild(const struct mf_field *mf, struct match *match)
         break;
 
     case MFF_IP_DSCP:
+    case MFF_IP_DSCP_SHIFTED:
         match->wc.masks.nw_tos &= ~IP_DSCP_MASK;
         match->flow.nw_tos &= ~IP_DSCP_MASK;
         break;
@@ -1726,6 +1752,7 @@ mf_set(const struct mf_field *mf,
     case MFF_IP_PROTO:
     case MFF_IP_TTL:
     case MFF_IP_DSCP:
+    case MFF_IP_DSCP_SHIFTED:
     case MFF_IP_ECN:
     case MFF_ARP_OP:
     case MFF_ICMPV4_TYPE:
@@ -1956,6 +1983,10 @@ mf_random_value(const struct mf_field *mf, union mf_value *value)
         value->u8 &= IP_DSCP_MASK;
         break;
 
+    case MFF_IP_DSCP_SHIFTED:
+        value->u8 &= IP_DSCP_MASK >> 2;
+        break;
+
     case MFF_IP_ECN:
         value->u8 &= IP_ECN_MASK;
         break;

diff --git a/lib/meta-flow.h b/lib/meta-flow.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2012 Nicira, Inc.
+ * Copyright (c) 2011, 2012, 2013 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -91,8 +91,18 @@ enum mf_field_id {
     MFF_IPV6_DST,               /* ipv6 */
     MFF_IPV6_LABEL,             /* be32 */
 
+    /* The IPv4/IPv6 DSCP field has two different views:
+     *
+     *   - MFF_IP_DSCP has the DSCP in bits 2-7, their bit positions in the
+     *     IPv4 and IPv6 "traffic class" field, as used in OpenFlow 1.0 and 1.1
+     *     flow format and in NXM's NXM_OF_IP_TOS
+     *
+     *   - MFF_IP_DSCP has the DSCP in bits 0-5, shifted right two bits from
+     *     their positions in the IPv4 and IPv6 "traffic class" field, as used
+     *     in OpenFlow 1.2+ OXM's OXM_OF_IP_DSCP. */
     MFF_IP_PROTO,               /* u8 (used for IPv4 or IPv6) */
     MFF_IP_DSCP,                /* u8 (used for IPv4 or IPv6) */
+    MFF_IP_DSCP_SHIFTED,        /* u8 (used for IPv4 or IPv6) (OF1.2 compat) */
     MFF_IP_ECN,                 /* u8 (used for IPv4 or IPv6) */
     MFF_IP_TTL,                 /* u8 (used for IPv4 or IPv6) */
     MFF_IP_FRAG,                /* u8 (used for IPv4 or IPv6) */

diff --git a/lib/nx-match.c b/lib/nx-match.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2011, 2012 Nicira, Inc.
+ * Copyright (c) 2010, 2011, 2012, 2013 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -517,8 +517,11 @@ nxm_put_ip(struct ofpbuf *b, const struct match *match,
     nxm_put_frag(b, match);
 
     if (match->wc.masks.nw_tos & IP_DSCP_MASK) {
-        nxm_put_8(b, oxm ? OXM_OF_IP_DSCP : NXM_OF_IP_TOS,
-                  flow->nw_tos & IP_DSCP_MASK);
+        if (oxm) {
+            nxm_put_8(b, OXM_OF_IP_DSCP, flow->nw_tos >> 2);
+        } else {
+            nxm_put_8(b, NXM_OF_IP_TOS, flow->nw_tos & IP_DSCP_MASK);
+        }
     }
 
     if (match->wc.masks.nw_tos & IP_ECN_MASK) {

diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
@@ -1466,6 +1466,12 @@ OXM_OF_VLAN_VID_W(1123/1f0f), OXM_OF_VLAN_PCP(01)  # Packets with VID=123 (maske
 OXM_OF_VLAN_VID_W(1000/1000)    # Packets with any VID, any PCP
 OXM_OF_VLAN_VID_W(1000/1000), OXM_OF_VLAN_PCP(01)  # Packets with any VID, PCP=1.
 
+# IP TOS
+OXM_OF_ETH_TYPE(0800) OXM_OF_IP_DSCP(f0)
+OXM_OF_ETH_TYPE(0800) OXM_OF_IP_DSCP(41)
+OXM_OF_ETH_TYPE(0800) OXM_OF_IP_DSCP(3f)
+OXM_OF_IP_DSCP(f0)
+
 # IP ECN
 OXM_OF_ETH_TYPE(0800) OXM_OF_IP_ECN(03)
 OXM_OF_ETH_TYPE(0800) OXM_OF_IP_ECN(06)
@@ -1662,6 +1668,12 @@ OXM_OF_VLAN_VID_W(1103/1f0f), OXM_OF_VLAN_PCP(01)
 OXM_OF_VLAN_VID_W(1000/1000)
 OXM_OF_VLAN_VID_W(1000/1000), OXM_OF_VLAN_PCP(01)
 
+# IP TOS
+nx_pull_match() returned error OFPBMC_BAD_VALUE
+nx_pull_match() returned error OFPBMC_BAD_VALUE
+OXM_OF_ETH_TYPE(0800), OXM_OF_IP_DSCP(3f)
+nx_pull_match() returned error OFPBMC_BAD_PREREQ
+
 # IP ECN
 OXM_OF_ETH_TYPE(0800), OXM_OF_IP_ECN(03)
 nx_pull_match() returned error OFPBMC_BAD_VALUE