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