Move struct ovs_key_ipv4_tunnel out of struct
sw_flow_key and into struct sw_flow. This allows it to
"float" and be used for matching only when needed.
Modify the matching code in ovs_flow_tbl_lookup() to
match on the tunnel header if it's set.
In particular, I'd like review of the method used for adding the matching
logic here. The method I've chosen here adds one additional check during
flow lookup to see if a tunnel key is set, and if so will add one comparison
before matching a flow. If this is deemed suboptimal, I'm open to suggestions
around optimizing this.
Signed-off-by: Kyle Mestery <kmest...@cisco.com>
---
datapath/actions.c | 1 +
datapath/datapath.c | 29 ++++++++++++++----------
datapath/datapath.h | 1 +
datapath/flow.c | 65 +++++++++++++++++++++++++++++++++--------------------
datapath/flow.h | 17 +++++++-------
datapath/tunnel.c | 5 +++--
lib/odp-util.c | 37 ++++++++++++++++++++++++++++++
7 files changed, 109 insertions(+), 46 deletions(-)
diff --git a/datapath/actions.c b/datapath/actions.c
index a70cde8..063d3af 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -288,6 +288,7 @@ static int output_userspace(struct datapath *dp, struct
sk_buff *skb,
upcall.cmd = OVS_PACKET_CMD_ACTION;
upcall.key = &OVS_CB(skb)->flow->key;
+ upcall.tun_key = &OVS_CB(skb)->flow->tun_key;
upcall.userdata = NULL;
upcall.pid = 0;
diff --git a/datapath/datapath.c b/datapath/datapath.c
index d8a198e..845494c 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -313,10 +313,11 @@ void ovs_dp_process_received_packet(struct vport *p,
struct sk_buff *skb)
if (!OVS_CB(skb)->flow) {
struct sw_flow_key key;
+ struct ovs_key_ipv4_tunnel tun_key;
int key_len;
/* Extract flow from 'skb' into 'key'. */
- error = ovs_flow_extract(skb, p->port_no, &key, &key_len);
+ error = ovs_flow_extract(skb, p->port_no, &key, &tun_key,
&key_len);
if (unlikely(error)) {
kfree_skb(skb);
return;
@@ -324,12 +325,13 @@ void ovs_dp_process_received_packet(struct vport *p,
struct sk_buff *skb)
/* Look up flow. */
flow = ovs_flow_tbl_lookup(rcu_dereference(dp->table),
- &key, key_len);
+ &key, &tun_key, key_len);
if (unlikely(!flow)) {
struct dp_upcall_info upcall;
upcall.cmd = OVS_PACKET_CMD_MISS;
upcall.key = &key;
+ upcall.tun_key = &tun_key;
upcall.userdata = NULL;
upcall.pid = p->upcall_pid;
ovs_dp_upcall(dp, skb, &upcall);
@@ -492,7 +494,7 @@ static int queue_userspace_packet(struct net *net, int
dp_ifindex,
upcall->dp_ifindex = dp_ifindex;
nla = nla_nest_start(user_skb, OVS_PACKET_ATTR_KEY);
- ovs_flow_to_nlattrs(upcall_info->key, user_skb);
+ ovs_flow_to_nlattrs(upcall_info->key, upcall_info->tun_key, user_skb);
nla_nest_end(user_skb, nla);
if (upcall_info->userdata)
@@ -787,13 +789,13 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb,
struct genl_info *info)
if (IS_ERR(flow))
goto err_kfree_skb;
- err = ovs_flow_extract(packet, -1, &flow->key, &key_len);
+ err = ovs_flow_extract(packet, -1, &flow->key, &flow->tun_key,
&key_len);
if (err)
goto err_flow_put;
err = ovs_flow_metadata_from_nlattrs(&flow->key.phy.priority,
&flow->key.phy.in_port,
- &flow->key.tun.tun_key,
+ &flow->tun_key,
a[OVS_PACKET_ATTR_KEY]);
if (err)
goto err_flow_put;
@@ -922,7 +924,7 @@ static int ovs_flow_cmd_fill_info(struct sw_flow *flow,
struct datapath *dp,
nla = nla_nest_start(skb, OVS_FLOW_ATTR_KEY);
if (!nla)
goto nla_put_failure;
- err = ovs_flow_to_nlattrs(&flow->key, skb);
+ err = ovs_flow_to_nlattrs(&flow->key, &flow->tun_key, skb);
if (err)
goto error;
nla_nest_end(skb, nla);
@@ -1016,6 +1018,7 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb,
struct genl_info *info)
struct nlattr **a = info->attrs;
struct ovs_header *ovs_header = info->userhdr;
struct sw_flow_key key;
+ struct ovs_key_ipv4_tunnel tun_key;
struct sw_flow *flow;
struct sk_buff *reply;
struct datapath *dp;
@@ -1027,7 +1030,7 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb,
struct genl_info *info)
error = -EINVAL;
if (!a[OVS_FLOW_ATTR_KEY])
goto error;
- error = ovs_flow_from_nlattrs(&key, &key_len, a[OVS_FLOW_ATTR_KEY]);
+ error = ovs_flow_from_nlattrs(&key, &tun_key, &key_len,
a[OVS_FLOW_ATTR_KEY]);
if (error)
goto error;
@@ -1047,7 +1050,7 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb,
struct genl_info *info)
goto error;
table = genl_dereference(dp->table);
- flow = ovs_flow_tbl_lookup(table, &key, key_len);
+ flow = ovs_flow_tbl_lookup(table, &key, &tun_key, key_len);
if (!flow) {
struct sw_flow_actions *acts;
@@ -1157,6 +1160,7 @@ static int ovs_flow_cmd_get(struct sk_buff *skb, struct
genl_info *info)
struct nlattr **a = info->attrs;
struct ovs_header *ovs_header = info->userhdr;
struct sw_flow_key key;
+ struct ovs_key_ipv4_tunnel tun_key;
struct sk_buff *reply;
struct sw_flow *flow;
struct datapath *dp;
@@ -1166,7 +1170,7 @@ static int ovs_flow_cmd_get(struct sk_buff *skb, struct
genl_info *info)
if (!a[OVS_FLOW_ATTR_KEY])
return -EINVAL;
- err = ovs_flow_from_nlattrs(&key, &key_len, a[OVS_FLOW_ATTR_KEY]);
+ err = ovs_flow_from_nlattrs(&key, &tun_key, &key_len,
a[OVS_FLOW_ATTR_KEY]);
if (err)
return err;
@@ -1175,7 +1179,7 @@ static int ovs_flow_cmd_get(struct sk_buff *skb, struct
genl_info *info)
return -ENODEV;
table = genl_dereference(dp->table);
- flow = ovs_flow_tbl_lookup(table, &key, key_len);
+ flow = ovs_flow_tbl_lookup(table, &key, &tun_key, key_len);
if (!flow)
return -ENOENT;
@@ -1192,6 +1196,7 @@ static int ovs_flow_cmd_del(struct sk_buff *skb, struct
genl_info *info)
struct nlattr **a = info->attrs;
struct ovs_header *ovs_header = info->userhdr;
struct sw_flow_key key;
+ struct ovs_key_ipv4_tunnel tun_key;
struct sk_buff *reply;
struct sw_flow *flow;
struct datapath *dp;
@@ -1206,12 +1211,12 @@ static int ovs_flow_cmd_del(struct sk_buff *skb, struct
genl_info *info)
if (!a[OVS_FLOW_ATTR_KEY])
return flush_flows(dp);
- err = ovs_flow_from_nlattrs(&key, &key_len, a[OVS_FLOW_ATTR_KEY]);
+ err = ovs_flow_from_nlattrs(&key, &tun_key, &key_len,
a[OVS_FLOW_ATTR_KEY]);
if (err)
return err;
table = genl_dereference(dp->table);
- flow = ovs_flow_tbl_lookup(table, &key, key_len);
+ flow = ovs_flow_tbl_lookup(table, &key, &tun_key, key_len);
if (!flow)
return -ENOENT;
diff --git a/datapath/datapath.h b/datapath/datapath.h
index c5df12d..074aa85 100644
--- a/datapath/datapath.h
+++ b/datapath/datapath.h
@@ -132,6 +132,7 @@ struct ovs_skb_cb {
struct dp_upcall_info {
u8 cmd;
const struct sw_flow_key *key;
+ const struct ovs_key_ipv4_tunnel *tun_key;
const struct nlattr *userdata;
u32 pid;
};
diff --git a/datapath/flow.c b/datapath/flow.c
index 5be492e..f6f5b94 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -601,6 +601,7 @@ out:
* Ethernet header
* @in_port: port number on which @skb was received.
* @key: output flow key
+ * @tun_key: output tunnel key
* @key_lenp: length of output flow key
*
* The caller must ensure that skb->len >= ETH_HLEN.
@@ -620,17 +621,18 @@ out:
* For other key->dl_type values it is left untouched.
*/
int ovs_flow_extract(struct sk_buff *skb, u16 in_port, struct sw_flow_key *key,
- int *key_lenp)
+ struct ovs_key_ipv4_tunnel *tun_key, int *key_lenp)
{
int error = 0;
int key_len = SW_FLOW_KEY_OFFSET(eth);
struct ethhdr *eth;
memset(key, 0, sizeof(*key));
+ memset(tun_key, 0, sizeof(*tun_key));
key->phy.priority = skb->priority;
if (OVS_CB(skb)->tun_key)
- key->tun.tun_key = *OVS_CB(skb)->tun_key;
+ *tun_key = *OVS_CB(skb)->tun_key;
key->phy.in_port = in_port;
skb_reset_mac_header(skb);
@@ -793,7 +795,9 @@ u32 ovs_flow_hash(const struct sw_flow_key *key, int
key_len)
}
struct sw_flow *ovs_flow_tbl_lookup(struct flow_table *table,
- struct sw_flow_key *key, int key_len)
+ struct sw_flow_key *key,
+ struct ovs_key_ipv4_tunnel *tun_key,
+ int key_len)
{
struct sw_flow *flow;
struct hlist_node *n;
@@ -807,7 +811,10 @@ struct sw_flow *ovs_flow_tbl_lookup(struct flow_table
*table,
if (flow->hash == hash &&
!memcmp(&flow->key, key, key_len)) {
- return flow;
+ if (!flow->tun_key.ipv4_dst ||
+ (flow->tun_key.ipv4_dst &&
+ !memcmp(&flow->tun_key, tun_key, sizeof(*tun_key))))
+ return flow;
}
}
return NULL;
@@ -993,7 +1000,8 @@ static int parse_flow_nlattrs(const struct nlattr *attr,
* @attr: Netlink attribute holding nested %OVS_KEY_ATTR_* Netlink attribute
* sequence.
*/
-int ovs_flow_from_nlattrs(struct sw_flow_key *swkey, int *key_lenp,
+int ovs_flow_from_nlattrs(struct sw_flow_key *swkey,
+ struct ovs_key_ipv4_tunnel *tun_key, int *key_lenp,
const struct nlattr *attr)
{
const struct nlattr *a[OVS_KEY_ATTR_MAX + 1];
@@ -1004,6 +1012,7 @@ int ovs_flow_from_nlattrs(struct sw_flow_key *swkey, int
*key_lenp,
int err;
memset(swkey, 0, sizeof(struct sw_flow_key));
+ memset(tun_key, 0, sizeof(struct ovs_key_ipv4_tunnel));
key_len = SW_FLOW_KEY_OFFSET(eth);
err = parse_flow_nlattrs(attr, a, &attrs);
@@ -1025,25 +1034,25 @@ int ovs_flow_from_nlattrs(struct sw_flow_key *swkey,
int *key_lenp,
swkey->phy.in_port = DP_MAX_PORTS;
}
- /* OVS_KEY_ATTR_TUN_ID and OVS_KEY_ATTR_IPV4_TUNEL must both arrive
+ /* OVS_KEY_ATTR_TUN_ID and OVS_KEY_ATTR_IPV4_TUNNEL must both arrive
* together.
*/
- if (attrs & (1ULL << OVS_KEY_ATTR_TUN_ID) &&
- attrs & (1ULL << OVS_KEY_ATTR_IPV4_TUNNEL)) {
- struct ovs_key_ipv4_tunnel *tun_key;
- tun_key = nla_data(a[OVS_KEY_ATTR_IPV4_TUNNEL]);
+ if ((attrs & (1ULL << OVS_KEY_ATTR_TUN_ID)) &&
+ (attrs & (1ULL << OVS_KEY_ATTR_IPV4_TUNNEL))) {
+ struct ovs_key_ipv4_tunnel *tunnel_key;
+ tunnel_key = nla_data(a[OVS_KEY_ATTR_IPV4_TUNNEL]);
tun_id = nla_get_be64(a[OVS_KEY_ATTR_TUN_ID]);
- if (tun_id != tun_key->tun_id)
+ if (tun_id != tunnel_key->tun_id)
return -EINVAL;
- swkey->tun.tun_key = *tun_key;
+ *tun_key = *tunnel_key;
attrs &= ~(1ULL << OVS_KEY_ATTR_TUN_ID);
attrs &= ~(1ULL << OVS_KEY_ATTR_IPV4_TUNNEL);
- } else if ((attrs & (1ULL << OVS_KEY_ATTR_TUN_ID) &&
- !attrs & (1ULL << OVS_KEY_ATTR_IPV4_TUNNEL)) ||
- (attrs & (1ULL << OVS_KEY_ATTR_IPV4_TUNNEL) &&
- !attrs & (1ULL << OVS_KEY_ATTR_TUN_ID)))
+ } else if (((attrs & (1ULL << OVS_KEY_ATTR_TUN_ID)) &&
+ (!(attrs & (1ULL << OVS_KEY_ATTR_IPV4_TUNNEL)))) ||
+ ((attrs & (1ULL << OVS_KEY_ATTR_IPV4_TUNNEL)) &&
+ (!(attrs & (1ULL << OVS_KEY_ATTR_TUN_ID)))))
return -EINVAL;
/* Data attributes. */
@@ -1234,7 +1243,8 @@ int ovs_flow_metadata_from_nlattrs(u32 *priority, u16
*in_port,
return 0;
}
-int ovs_flow_to_nlattrs(const struct sw_flow_key *swkey, struct sk_buff *skb)
+int ovs_flow_to_nlattrs(const struct sw_flow_key *swkey,
+ const struct ovs_key_ipv4_tunnel *tun_key, struct
sk_buff *skb)
{
struct ovs_key_ethernet *eth_key;
struct nlattr *nla, *encap;
@@ -1243,18 +1253,25 @@ int ovs_flow_to_nlattrs(const struct sw_flow_key
*swkey, struct sk_buff *skb)
nla_put_u32(skb, OVS_KEY_ATTR_PRIORITY, swkey->phy.priority))
goto nla_put_failure;
- if (swkey->tun.tun_key.tun_id != cpu_to_be64(0) &&
- nla_put_be64(skb, OVS_KEY_ATTR_TUN_ID, swkey->tun.tun_key.tun_id))
+ if (tun_key->tun_id != cpu_to_be64(0) &&
+ nla_put_be64(skb, OVS_KEY_ATTR_TUN_ID, tun_key->tun_id))
goto nla_put_failure;
- if (swkey->tun.tun_key.ipv4_dst) {
- struct ovs_key_ipv4_tunnel *tun_key;
+ if (tun_key->ipv4_dst && tun_key->tun_id != cpu_to_be64(0)) {
+ struct ovs_key_ipv4_tunnel *tunnel_key;
nla = nla_reserve(skb, OVS_KEY_ATTR_IPV4_TUNNEL,
- sizeof(*tun_key));
+ sizeof(*tunnel_key));
if (!nla)
goto nla_put_failure;
- tun_key = nla_data(nla);
- *tun_key = swkey->tun.tun_key;
+ if (!nla_put_be64(skb, OVS_KEY_ATTR_TUN_ID, tun_key->tun_id))
+ goto nla_put_failure;
+ tunnel_key = nla_data(nla);
+ tunnel_key->tun_id = tun_key->tun_id;
+ tunnel_key->tun_flags = tun_key->tun_flags;
+ tunnel_key->ipv4_src = tun_key->ipv4_src;
+ tunnel_key->ipv4_dst = tun_key->ipv4_dst;
+ tunnel_key->ipv4_tos = tun_key->ipv4_tos;
+ tunnel_key->ipv4_ttl = tun_key->ipv4_ttl;
}
if (swkey->phy.in_port != DP_MAX_PORTS &&
diff --git a/datapath/flow.h b/datapath/flow.h
index 5d4fcde..030cdb0 100644
--- a/datapath/flow.h
+++ b/datapath/flow.h
@@ -91,9 +91,6 @@ struct sw_flow_key {
} nd;
} ipv6;
};
- struct {
- struct ovs_key_ipv4_tunnel tun_key; /* Encapsulating tunnel
key. */
- } tun;
};
struct sw_flow {
@@ -102,6 +99,7 @@ struct sw_flow {
u32 hash;
struct sw_flow_key key;
+ struct ovs_key_ipv4_tunnel tun_key; /* Encapsulating tunnel key. */
struct sw_flow_actions __rcu *sf_acts;
atomic_t refcnt;
@@ -141,7 +139,7 @@ void ovs_flow_hold(struct sw_flow *);
void ovs_flow_put(struct sw_flow *);
int ovs_flow_extract(struct sk_buff *, u16 in_port, struct sw_flow_key *,
- int *key_lenp);
+ struct ovs_key_ipv4_tunnel *tun_key, int *key_lenp);
void ovs_flow_used(struct sw_flow *, struct sk_buff *);
u64 ovs_flow_used_time(unsigned long flow_jiffies);
@@ -167,9 +165,10 @@ u64 ovs_flow_used_time(unsigned long flow_jiffies);
*/
#define FLOW_BUFSIZE 184
-int ovs_flow_to_nlattrs(const struct sw_flow_key *, struct sk_buff *);
-int ovs_flow_from_nlattrs(struct sw_flow_key *swkey, int *key_lenp,
- const struct nlattr *);
+int ovs_flow_to_nlattrs(const struct sw_flow_key *,
+ const struct ovs_key_ipv4_tunnel *tun_key, struct
sk_buff *);
+int ovs_flow_from_nlattrs(struct sw_flow_key *swkey, struct
ovs_key_ipv4_tunnel *tun_key,
+ int *key_lenp, const struct nlattr *);
int ovs_flow_metadata_from_nlattrs(u32 *priority, u16 *in_port,
struct ovs_key_ipv4_tunnel *tun_key,
const struct nlattr *);
@@ -197,7 +196,9 @@ static inline int ovs_flow_tbl_need_to_expand(struct
flow_table *table)
}
struct sw_flow *ovs_flow_tbl_lookup(struct flow_table *table,
- struct sw_flow_key *key, int len);
+ struct sw_flow_key *key,
+ struct ovs_key_ipv4_tunnel *tun_key,
+ int len);
void ovs_flow_tbl_destroy(struct flow_table *table);
void ovs_flow_tbl_deferred_destroy(struct flow_table *table);
struct flow_table *ovs_flow_tbl_alloc(int new_size);
diff --git a/datapath/tunnel.c b/datapath/tunnel.c
index b62c469..e0d0bf9 100644
--- a/datapath/tunnel.c
+++ b/datapath/tunnel.c
@@ -945,6 +945,7 @@ static struct tnl_cache *build_cache(struct vport *vport,
if (ovs_is_internal_dev(rt_dst(rt).dev)) {
struct sw_flow_key flow_key;
+ struct ovs_key_ipv4_tunnel tun_key;
struct vport *dst_vport;
struct sk_buff *skb;
int err;
@@ -963,14 +964,14 @@ static struct tnl_cache *build_cache(struct vport *vport,
memcpy(skb->data, get_cached_header(cache), cache->len);
err = ovs_flow_extract(skb, dst_vport->port_no, &flow_key,
- &flow_key_len);
+ &tun_key, &flow_key_len);
consume_skb(skb);
if (err)
goto done;
flow =
ovs_flow_tbl_lookup(rcu_dereference(dst_vport->dp->table),
- &flow_key, flow_key_len);
+ &flow_key, &tun_key, flow_key_len);
if (flow) {
cache->flow = flow;
ovs_flow_hold(flow);
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 433255e..22e5ef7 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -918,8 +918,13 @@ parse_odp_key_attr(const char *s, const struct simap
*port_names,
if (sscanf(s, "tun_id(%31[x0123456789abcdefABCDEF])%n",
tun_id_s, &n) > 0 && n > 0) {
+ struct ovs_key_ipv4_tunnel tun_key;
uint64_t tun_id = strtoull(tun_id_s, NULL, 0);
+ memset(&tun_key, 0, sizeof(tun_key));
+ tun_key.tun_id = tun_id;
nl_msg_put_be64(key, OVS_KEY_ATTR_TUN_ID, htonll(tun_id));
+ nl_msg_put_unspec(key, OVS_KEY_ATTR_IPV4_TUNNEL,
+ &tun_key, sizeof tun_key);
return n;
}
}
@@ -1274,7 +1279,14 @@ odp_flow_key_from_flow(struct ofpbuf *buf, const struct
flow *flow)
}
if (flow->tun_id != htonll(0)) {
+ struct ovs_key_ipv4_tunnel *tun_key;
+
nl_msg_put_be64(buf, OVS_KEY_ATTR_TUN_ID, flow->tun_id);
+
+ tun_key = nl_msg_put_unspec_uninit(buf, OVS_KEY_ATTR_IPV4_TUNNEL,
+ sizeof *tun_key);
+ memset(tun_key, 0, sizeof *tun_key);
+ tun_key->tun_id = flow->tun_id;
}
if (flow->in_port != OFPP_NONE && flow->in_port != OFPP_CONTROLLER) {
@@ -1770,6 +1782,13 @@ 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)) {
+ struct ovs_key_ipv4_tunnel *tun_key;
+
+ tun_key = nl_attr_get(attrs[OVS_KEY_ATTR_IPV4_TUNNEL]);
+ expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_IPV4_TUNNEL;
+ }
+
if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_IN_PORT)) {
uint32_t in_port = nl_attr_get_u32(attrs[OVS_KEY_ATTR_IN_PORT]);
if (in_port >= UINT16_MAX || in_port >= OFPP_MAX) {
@@ -1871,6 +1890,23 @@ commit_set_tun_id_action(const struct flow *flow, struct
flow *base,
}
static void
+commit_set_tun_key_action(const struct flow *flow, struct flow *base,
+ struct ofpbuf *odp_actions)
+{
+ struct ovs_key_ipv4_tunnel tun_key;
+
+ memset(&tun_key, 0, sizeof tun_key);
+
+ if (base->tun_id == flow->tun_id) {
+ return;
+ }
+ tun_key.tun_id = flow->tun_id;
+
+ commit_set_action(odp_actions, OVS_KEY_ATTR_IPV4_TUNNEL,
+ &tun_key, sizeof tun_key);
+}
+
+static void
commit_set_ether_addr_action(const struct flow *flow, struct flow *base,
struct ofpbuf *odp_actions)
{
@@ -2039,6 +2075,7 @@ 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_tun_key_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);
--
1.7.11.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
