On 04/09/2014 01:38 AM, Jarno Rajahalme wrote:
Masked set actions allow more megaflow wildcarding. All other key
types than the tunnel key that can be set, can now be set with a mask.
It is not clear wether masked set is useful for skb_priority.
However, we already use the LSB of pkt_mark for IPSec in tunnels, so
it might be useful to be able to set individual bits on pkt_mark.
This looks great!
+ if (mask) {
+ ether_addr_copy_masked(eth_hdr(skb)->h_source, key->eth_src,
+ mask->eth_src);
+ ether_addr_copy_masked(eth_hdr(skb)->h_dest, key->eth_dst,
+ mask->eth_dst);
+ } else {
+ ether_addr_copy(eth_hdr(skb)->h_source, key->eth_src);
+ ether_addr_copy(eth_hdr(skb)->h_dest, key->eth_dst);
+ }
Quite a few of these branches are introduced in the fast path. Might
be worth checking 'perf stat' quickly to see if this generates a lot
of branch misses for either the wildcards enabled or the wildcards
disabled case. It might in fact be faster to always assume a mask if
the compiler cannot predict this correctly.
@@ -1281,13 +1294,17 @@ static int validate_set(const struct nlattr *a,
{
const struct nlattr *ovs_key = nla_data(a);
int key_type = nla_type(ovs_key);
+ bool have_mask;
/* There can be only one key in a action */
if (nla_total_size(nla_len(ovs_key)) != nla_len(a))
return -EINVAL;
+ have_mask = (ovs_key_lens[key_type] * 2 == nla_len(ovs_key));
+
key_type is not validated at this point and might be > OVS_KEY_ATTR_MAX
and thus access ovs_key_lens[] out of bound.
if (key_type > OVS_KEY_ATTR_MAX ||
(ovs_key_lens[key_type] != nla_len(ovs_key) &&
+ (!have_mask || !validate_masked(nla_data(ovs_key), nla_len(ovs_key)))
&&
ovs_key_lens[key_type] != -1))
return -EINVAL;
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
