Skip to content

Commit

Permalink
openvswitch: Avoid NULL mask check while building mask
Browse files Browse the repository at this point in the history 
OVS does mask validation even if it does not need to convert
netlink mask attributes to mask structure.  ovs_nla_get_match()
caller can pass NULL mask structure pointer if the caller does
not need mask.  Therefore NULL check is required in SW_FLOW_KEY*
macros.  Following patch does not convert mask netlink attributes
if mask pointer is NULL, so we do not need these checks in
SW_FLOW_KEY* macro.

Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Daniele Di Proietto <ddiproietto@vmware.com>
Acked-by: Andy Zhou <azhou@nicira.com>
  • Loading branch information
Pravin B Shelar committed Nov 6, 2014
1 parent 2fdb957 commit a85311b
Showing 1 changed file with 53 additions and 54 deletions.
107 changes: 53 additions & 54 deletions net/openvswitch/flow_netlink.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,21 +50,18 @@

#include "flow_netlink.h"

static void update_range__(struct sw_flow_match *match,
size_t offset, size_t size, bool is_mask)
static void update_range(struct sw_flow_match *match,
size_t offset, size_t size, bool is_mask)
{
struct sw_flow_key_range *range = NULL;
struct sw_flow_key_range *range;
size_t start = rounddown(offset, sizeof(long));
size_t end = roundup(offset + size, sizeof(long));

if (!is_mask)
range = &match->range;
else if (match->mask)
else
range = &match->mask->range;

if (!range)
return;

if (range->start == range->end) {
range->start = start;
range->end = end;
Expand All @@ -80,22 +77,20 @@ static void update_range__(struct sw_flow_match *match,

#define SW_FLOW_KEY_PUT(match, field, value, is_mask) \
do { \
update_range__(match, offsetof(struct sw_flow_key, field), \
sizeof((match)->key->field), is_mask); \
if (is_mask) { \
if ((match)->mask) \
(match)->mask->key.field = value; \
} else { \
update_range(match, offsetof(struct sw_flow_key, field), \
sizeof((match)->key->field), is_mask); \
if (is_mask) \
(match)->mask->key.field = value; \
else \
(match)->key->field = value; \
} \
} while (0)

#define SW_FLOW_KEY_MEMCPY_OFFSET(match, offset, value_p, len, is_mask) \
do { \
update_range__(match, offset, len, is_mask); \
update_range(match, offset, len, is_mask); \
if (is_mask) \
memcpy((u8 *)&(match)->mask->key + offset, value_p, \
len); \
len); \
else \
memcpy((u8 *)(match)->key + offset, value_p, len); \
} while (0)
Expand All @@ -104,18 +99,16 @@ static void update_range__(struct sw_flow_match *match,
SW_FLOW_KEY_MEMCPY_OFFSET(match, offsetof(struct sw_flow_key, field), \
value_p, len, is_mask)

#define SW_FLOW_KEY_MEMSET_FIELD(match, field, value, is_mask) \
do { \
update_range__(match, offsetof(struct sw_flow_key, field), \
sizeof((match)->key->field), is_mask); \
if (is_mask) { \
if ((match)->mask) \
memset((u8 *)&(match)->mask->key.field, value,\
sizeof((match)->mask->key.field)); \
} else { \
#define SW_FLOW_KEY_MEMSET_FIELD(match, field, value, is_mask) \
do { \
update_range(match, offsetof(struct sw_flow_key, field), \
sizeof((match)->key->field), is_mask); \
if (is_mask) \
memset((u8 *)&(match)->mask->key.field, value, \
sizeof((match)->mask->key.field)); \
else \
memset((u8 *)&(match)->key->field, value, \
sizeof((match)->key->field)); \
} \
} while (0)

static bool match_validate(const struct sw_flow_match *match,
Expand Down Expand Up @@ -677,8 +670,7 @@ static int ovs_key_from_nlattrs(struct sw_flow_match *match, u64 attrs,

SW_FLOW_KEY_PUT(match, eth.tci, tci, is_mask);
attrs &= ~(1 << OVS_KEY_ATTR_VLAN);
} else if (!is_mask)
SW_FLOW_KEY_PUT(match, eth.tci, htons(0xffff), true);
}

if (attrs & (1 << OVS_KEY_ATTR_ETHERTYPE)) {
__be16 eth_type;
Expand Down Expand Up @@ -903,8 +895,8 @@ static void mask_set_nlattr(struct nlattr *attr, u8 val)
* attribute specifies the mask field of the wildcarded flow.
*/
int ovs_nla_get_match(struct sw_flow_match *match,
const struct nlattr *key,
const struct nlattr *mask)
const struct nlattr *nla_key,
const struct nlattr *nla_mask)
{
const struct nlattr *a[OVS_KEY_ATTR_MAX + 1];
const struct nlattr *encap;
Expand All @@ -914,7 +906,7 @@ int ovs_nla_get_match(struct sw_flow_match *match,
bool encap_valid = false;
int err;

err = parse_flow_nlattrs(key, a, &key_attrs);
err = parse_flow_nlattrs(nla_key, a, &key_attrs);
if (err)
return err;

Expand Down Expand Up @@ -955,36 +947,43 @@ int ovs_nla_get_match(struct sw_flow_match *match,
if (err)
return err;

if (match->mask && !mask) {
/* Create an exact match mask. We need to set to 0xff all the
* 'match->mask' fields that have been touched in 'match->key'.
* We cannot simply memset 'match->mask', because padding bytes
* and fields not specified in 'match->key' should be left to 0.
* Instead, we use a stream of netlink attributes, copied from
* 'key' and set to 0xff: ovs_key_from_nlattrs() will take care
* of filling 'match->mask' appropriately.
*/
newmask = kmemdup(key, nla_total_size(nla_len(key)),
GFP_KERNEL);
if (!newmask)
return -ENOMEM;
if (match->mask) {
if (!nla_mask) {
/* Create an exact match mask. We need to set to 0xff
* all the 'match->mask' fields that have been touched
* in 'match->key'. We cannot simply memset
* 'match->mask', because padding bytes and fields not
* specified in 'match->key' should be left to 0.
* Instead, we use a stream of netlink attributes,
* copied from 'key' and set to 0xff.
* ovs_key_from_nlattrs() will take care of filling
* 'match->mask' appropriately.
*/
newmask = kmemdup(nla_key,
nla_total_size(nla_len(nla_key)),
GFP_KERNEL);
if (!newmask)
return -ENOMEM;

mask_set_nlattr(newmask, 0xff);
mask_set_nlattr(newmask, 0xff);

/* The userspace does not send tunnel attributes that are 0,
* but we should not wildcard them nonetheless.
*/
if (match->key->tun_key.ipv4_dst)
SW_FLOW_KEY_MEMSET_FIELD(match, tun_key, 0xff, true);
/* The userspace does not send tunnel attributes that
* are 0, but we should not wildcard them nonetheless.
*/
if (match->key->tun_key.ipv4_dst)
SW_FLOW_KEY_MEMSET_FIELD(match, tun_key,
0xff, true);

mask = newmask;
}
nla_mask = newmask;
}

if (mask) {
err = parse_flow_mask_nlattrs(mask, a, &mask_attrs);
err = parse_flow_mask_nlattrs(nla_mask, a, &mask_attrs);
if (err)
goto free_newmask;

/* Always match on tci. */
SW_FLOW_KEY_PUT(match, eth.tci, htons(0xffff), true);

if (mask_attrs & 1 << OVS_KEY_ATTR_ENCAP) {
__be16 eth_type = 0;
__be16 tci = 0;
Expand Down

0 comments on commit a85311b

Please sign in to comment.