This version adds odp-level support for tunnel metadata.

Currently it seems the datapath expects userspace to specify all the
tunnel fields, or none of them.  IMO, it would be better to use of
configured values as defaults, allowing flow entries to provide only
some of the fields (e.g., tun_dst).

This is obviously similar to how any other field changes are viewed
at the OF level, but tunnel metadata is different since packets don't
have the metadata when they arrive on a normal port, but the tunnel
metadata is figured out only when the output port is determined
(which may be taken from a register if I'm not mistaken).

So, is the design intent that the userspace code digs the missing
values from the tunnel configuration data?  Currently, it seems that
data is readily available in the datapath when it is needed, but maybe
doing it "once" on the userspace is better than figuring out which
values to use on the datapath for each packet separately?


Signed-off-by: Jarno Rajahalme <jarno.rajaha...@nsn.com>
---
 include/openflow/nicira-ext.h |    3 ++
 lib/meta-flow.c               |   26 ++++++++++-------
 lib/nx-match.c                |    8 +++++-
 lib/odp-util.c                |   62 +++++++++++++++++++++++++++++++++++++++--
 lib/ofp-util.c                |   21 +++++---------
 tests/ofproto-dpif.at         |   19 +++++++++++++
 6 files changed, 112 insertions(+), 27 deletions(-)

diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
index 11b761d..d250623 100644
--- a/include/openflow/nicira-ext.h
+++ b/include/openflow/nicira-ext.h
@@ -1763,6 +1763,9 @@ OFP_ASSERT(sizeof(struct nx_action_output_reg) == 24);
 #define NXM_NX_COOKIE     NXM_HEADER  (0x0001, 30, 8)
 #define NXM_NX_COOKIE_W   NXM_HEADER_W(0x0001, 30, 8)
 
+#define NXM_NX_TUN_IPV4_SRC   NXM_HEADER  (0x0001, 31, 4)
+#define NXM_NX_TUN_IPV4_DST   NXM_HEADER  (0x0001, 32, 4)
+
 /* ## --------------------- ## */
 /* ## Requests and replies. ## */
 /* ## --------------------- ## */
diff --git a/lib/meta-flow.c b/lib/meta-flow.c
index 749898f..e4aa259 100644
--- a/lib/meta-flow.c
+++ b/lib/meta-flow.c
@@ -58,21 +58,21 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
     }, {
         MFF_TUN_SRC, "tun_src", NULL,
         MF_FIELD_SIZES(be32),
-        MFM_NONE,
+        MFM_FULLY,
         MFS_IPV4,
         MFP_NONE,
-        false,
-        0, NULL,
-        0, NULL,
+        true,
+        NXM_NX_TUN_IPV4_SRC, "NXM_NX_TUN_IPV4_SRC",
+        NXM_NX_TUN_IPV4_SRC, "NXM_NX_TUN_IPV4_SRC",
     }, {
         MFF_TUN_DST, "tun_dst", NULL,
         MF_FIELD_SIZES(be32),
-        MFM_NONE,
+        MFM_FULLY,
         MFS_IPV4,
         MFP_NONE,
-        false,
-        0, NULL,
-        0, NULL,
+        true,
+        NXM_NX_TUN_IPV4_DST, "NXM_NX_TUN_IPV4_DST",
+        NXM_NX_TUN_IPV4_DST, "NXM_NX_TUN_IPV4_DST",
     }, {
         MFF_TUN_FLAGS, "tun_flags", NULL,
         MF_FIELD_SIZES(be16),
@@ -618,9 +618,11 @@ bool
 mf_is_all_wild(const struct mf_field *mf, const struct flow_wildcards *wc)
 {
     switch (mf->id) {
-    case MFF_TUN_ID:
     case MFF_TUN_SRC:
+        return !wc->masks.tunnel.ip_src;
     case MFF_TUN_DST:
+        return !wc->masks.tunnel.ip_dst;
+    case MFF_TUN_ID:
     case MFF_TUN_TOS:
     case MFF_TUN_TTL:
     case MFF_TUN_FLAGS:
@@ -720,9 +722,13 @@ mf_get_mask(const struct mf_field *mf, const struct 
flow_wildcards *wc,
             union mf_value *mask)
 {
     switch (mf->id) {
-    case MFF_TUN_ID:
     case MFF_TUN_SRC:
+        mask->be32 = wc->masks.tunnel.ip_src;
+        break;
     case MFF_TUN_DST:
+        mask->be32 = wc->masks.tunnel.ip_dst;
+        break;
+    case MFF_TUN_ID:
     case MFF_TUN_TOS:
     case MFF_TUN_TTL:
     case MFF_TUN_FLAGS:
diff --git a/lib/nx-match.c b/lib/nx-match.c
index 05f1ce9..7715b91 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -645,7 +645,13 @@ nx_put_raw(struct ofpbuf *b, bool oxm, const struct match 
*match,
 
     /* Tunnel ID. */
     nxm_put_64m(b, oxm ? OXM_OF_TUNNEL_ID : NXM_NX_TUN_ID,
-               flow->tunnel.tun_id, match->wc.masks.tunnel.tun_id);
+                flow->tunnel.tun_id, match->wc.masks.tunnel.tun_id);
+
+    /* Other tunnel metadata. */
+    nxm_put_32m(b, NXM_NX_TUN_IPV4_SRC,
+                flow->tunnel.ip_src, match->wc.masks.tunnel.ip_src);
+    nxm_put_32m(b, NXM_NX_TUN_IPV4_DST,
+                flow->tunnel.ip_dst, match->wc.masks.tunnel.ip_dst);
 
     /* Registers. */
     for (i = 0; i < FLOW_N_REGS; i++) {
diff --git a/lib/odp-util.c b/lib/odp-util.c
index de97fd2..24972d2 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -1361,7 +1361,21 @@ odp_flow_key_from_flow(struct ofpbuf *buf, const struct 
flow *flow,
         nl_msg_put_u32(buf, OVS_KEY_ATTR_PRIORITY, flow->skb_priority);
     }
 
-    if (flow->tunnel.tun_id != htonll(0)) {
+    if (flow->tunnel.ip_dst) {
+        struct ovs_key_ipv4_tunnel *ipv4_tun_key;
+
+        ipv4_tun_key = nl_msg_put_unspec_uninit(buf, OVS_KEY_ATTR_IPV4_TUNNEL,
+                                            sizeof *ipv4_tun_key);
+        /* layouts differ, flags has different size */
+        ipv4_tun_key->tun_id = flow->tunnel.tun_id;
+        ipv4_tun_key->tun_flags = flow->tunnel.flags;
+        ipv4_tun_key->ipv4_src = flow->tunnel.ip_src;
+        ipv4_tun_key->ipv4_dst = flow->tunnel.ip_dst;
+        ipv4_tun_key->ipv4_tos = flow->tunnel.ip_tos;
+        ipv4_tun_key->ipv4_ttl = flow->tunnel.ip_ttl;
+        memset(ipv4_tun_key->pad, 0, sizeof ipv4_tun_key->pad);
+    }
+    else if (flow->tunnel.tun_id != htonll(0)) {
         nl_msg_put_be64(buf, OVS_KEY_ATTR_TUN_ID, flow->tunnel.tun_id);
     }
 
@@ -1871,6 +1885,19 @@ odp_flow_key_to_flow(const struct nlattr *key, size_t 
key_len,
         expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_TUN_ID;
     }
 
+    if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_IPV4_TUNNEL)) {
+        const struct ovs_key_ipv4_tunnel *ipv4_tun_key;
+
+        ipv4_tun_key = nl_attr_get(attrs[OVS_KEY_ATTR_IPV4_TUNNEL]);
+        flow->tunnel.tun_id = ipv4_tun_key->tun_id;
+        flow->tunnel.ip_src = ipv4_tun_key->ipv4_src;
+        flow->tunnel.ip_dst = ipv4_tun_key->ipv4_dst;
+        flow->tunnel.flags = (uint16_t)ipv4_tun_key->tun_flags;
+        flow->tunnel.ip_tos = ipv4_tun_key->ipv4_tos;
+        flow->tunnel.ip_ttl = ipv4_tun_key->ipv4_ttl;
+        expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_IPV4_TUNNEL;
+    }
+
     if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_IN_PORT)) {
         flow->in_port = nl_attr_get_u32(attrs[OVS_KEY_ATTR_IN_PORT]);
         expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_IN_PORT;
@@ -1966,6 +1993,33 @@ commit_set_tun_id_action(const struct flow *flow, struct 
flow *base,
 }
 
 static void
+commit_set_tunnel_action(const struct flow *flow, struct flow *base,
+                         struct ofpbuf *odp_actions)
+{
+    struct ovs_key_ipv4_tunnel ipv4_tun_key;
+
+    if (base->tunnel.tun_id == flow->tunnel.tun_id &&
+        base->tunnel.ip_src == flow->tunnel.ip_src &&
+        base->tunnel.ip_dst == flow->tunnel.ip_dst &&
+        base->tunnel.flags == flow->tunnel.flags &&
+        base->tunnel.ip_tos == flow->tunnel.ip_tos &&
+        base->tunnel.ip_ttl == flow->tunnel.ip_ttl) {
+        return;
+    }
+
+    ipv4_tun_key.tun_id = base->tunnel.tun_id = flow->tunnel.tun_id;
+    ipv4_tun_key.tun_flags = base->tunnel.flags = flow->tunnel.flags;
+    ipv4_tun_key.ipv4_src = base->tunnel.ip_src = flow->tunnel.ip_src;
+    ipv4_tun_key.ipv4_dst = base->tunnel.ip_dst = flow->tunnel.ip_dst;
+    ipv4_tun_key.ipv4_tos = base->tunnel.ip_tos = flow->tunnel.ip_tos;
+    ipv4_tun_key.ipv4_ttl = base->tunnel.ip_ttl = flow->tunnel.ip_ttl;
+    memset(ipv4_tun_key.pad, 0, sizeof ipv4_tun_key.pad);
+
+    commit_set_action(odp_actions, OVS_KEY_ATTR_IPV4_TUNNEL,
+                      &ipv4_tun_key, sizeof(ipv4_tun_key));
+}
+
+static void
 commit_set_ether_addr_action(const struct flow *flow, struct flow *base,
                              struct ofpbuf *odp_actions)
 {
@@ -2140,12 +2194,16 @@ commit_set_skb_mark_action(const struct flow *flow, 
struct flow *base,
 }
 /* If any of the flow key data that ODP actions can modify are different in
  * 'base' and 'flow', appends ODP actions to 'odp_actions' that change the flow
- * key from 'base' into 'flow', and then changes 'base' the same way. */
+ * key from 'base' into 'flow', and then changes 'base' the same way.
+ *
+ * tunnel_action is after tun_id_action so that if the only different
+ * tunnel field is tun_id, only tun_id action is used. */
 void
 commit_odp_actions(const struct flow *flow, struct flow *base,
                    struct ofpbuf *odp_actions)
 {
     commit_set_tun_id_action(flow, base, odp_actions);
+    commit_set_tunnel_action(flow, base, odp_actions);
     commit_set_ether_addr_action(flow, base, odp_actions);
     commit_vlan_action(flow, base, odp_actions);
     commit_set_nw_action(flow, base, odp_actions);
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 11b6192..a11ce66 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1041,16 +1041,6 @@ regs_fully_wildcarded(const struct flow_wildcards *wc)
     return true;
 }
 
-static bool
-tun_parms_fully_wildcarded(const struct flow_wildcards *wc)
-{
-    return (!wc->masks.tunnel.ip_src &&
-            !wc->masks.tunnel.ip_dst &&
-            !wc->masks.tunnel.ip_ttl &&
-            !wc->masks.tunnel.ip_tos &&
-            !wc->masks.tunnel.flags);
-}
-
 /* Returns a bit-mask of ofputil_protocols that can be used for sending 'match'
  * to a switch (e.g. to add or remove a flow).  Only NXM can handle tunnel IDs,
  * registers, or fixing the Ethernet multicast bit.  Otherwise, it's better to
@@ -1062,8 +1052,9 @@ ofputil_usable_protocols(const struct match *match)
 
     BUILD_ASSERT_DECL(FLOW_WC_SEQ == 18);
 
-    /* tunnel params other than tun_id can't be sent in a flow_mod */
-    if (!tun_parms_fully_wildcarded(wc)) {
+    /* These tunnel params can't be sent in a flow_mod */
+    if (wc->masks.tunnel.ip_ttl
+        || wc->masks.tunnel.ip_tos || wc->masks.tunnel.flags) {
         return OFPUTIL_P_NONE;
     }
 
@@ -1104,8 +1095,10 @@ ofputil_usable_protocols(const struct match *match)
             | OFPUTIL_P_OF13_OXM;
     }
 
-    /* NXM and OXM support matching tun_id. */
-    if (wc->masks.tunnel.tun_id != htonll(0)) {
+    /* NXM and OXM support matching tun_id, ip_src, and ip_dst. */
+    if (wc->masks.tunnel.tun_id != htonll(0)
+        || wc->masks.tunnel.ip_src != htonll(0)
+        || wc->masks.tunnel.ip_dst != htonll(0)) {
         return OFPUTIL_P_OF10_NXM_ANY | OFPUTIL_P_OF12_OXM
             | OFPUTIL_P_OF13_OXM;
     }
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 6693f1b..8fd1dfd 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -231,6 +231,25 @@ AT_CHECK([tail -1 stdout], [0],
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([ofproto-dpif - set_field - tun_src/tun_dst/tun_id])
+OVS_VSWITCHD_START
+ADD_OF_PORTS([br0], [1], [2], [3], [4], [5], [90])
+AT_DATA([flows.txt], [dnl
+in_port=90 actions=resubmit:1,resubmit:2,resubmit:3,resubmit:4,resubmit:5
+in_port=1 actions=set_field:1.1.1.1->tun_src,output:1
+in_port=2 actions=set_field:2.2.2.2->tun_dst,output:2
+in_port=3 
actions=set_field:1.1.1.1->tun_src,set_field:2.2.2.2->tun_dst,output:3
+in_port=4 actions=set_field:2.2.2.2->tun_dst,set_field:3->tun_id,output:4
+in_port=5 actions=set_field:5->tun_id
+])
+AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
+AT_CHECK([ovs-appctl ofproto/trace br0 
'tun_id(0x1),in_port(90),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=1,tos=0,ttl=128,frag=no),icmp(type=8,code=0)'],
 [0], [stdout])
+AT_CHECK([tail -1 stdout], [0],
+  [Datapath actions: 
set(tun_id(0x1)),set(ipv4_tunnel(tun_id=0x1,src=1.1.1.1,dst=0.0.0.0,tos=0x0,ttl=0,flags())),1,set(ipv4_tunnel(tun_id=0x1,src=1.1.1.1,dst=2.2.2.2,tos=0x0,ttl=0,flags())),2,3,set(tun_id(0x3)),4
+])
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_SETUP([ofproto-dpif - controller])
 OVS_VSWITCHD_START([dnl
    add-port br0 p1 -- set Interface p1 type=dummy
-- 
1.7.10.4

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to