Skip to content

Commit

Permalink
datapath: Backport "openvswitch: Zero flows on allocation."
Browse files Browse the repository at this point in the history
Upstream commit:
    openvswitch: Zero flows on allocation.

    When support for megaflows was introduced, OVS needed to start
    installing flows with a mask applied to them. Since masking is an
    expensive operation, OVS also had an optimization that would only
    take the parts of the flow keys that were covered by a non-zero
    mask. The values stored in the remaining pieces should not matter
    because they are masked out.

    While this works fine for the purposes of matching (which must always
    look at the mask), serialization to netlink can be problematic. Since
    the flow and the mask are serialized separately, the uninitialized
    portions of the flow can be encoded with whatever values happen to be
    present.

    In terms of functionality, this has little effect since these fields
    will be masked out by definition. However, it leaks kernel memory to
    userspace, which is a potential security vulnerability. It is also
    possible that other code paths could look at the masked key and get
    uninitialized data, although this does not currently appear to be an
    issue in practice.

    This removes the mask optimization for flows that are being installed.
    This was always intended to be the case as the mask optimizations were
    really targetting per-packet flow operations.

    Fixes: 03f0d916 ("openvswitch: Mega flow implementation")
    Signed-off-by: Jesse Gross <jesse@nicira.com>
    Acked-by: Pravin B Shelar <pshelar@nicira.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

Upstream: ae5f2fb1 ("openvswitch: Zero flows on allocation.")
Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Pravin B Shelar <pshelar@nicira.com>
  • Loading branch information
jessegross committed Sep 24, 2015
1 parent dbdcaf8 commit b19d3dd
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 14 deletions.
4 changes: 2 additions & 2 deletions datapath/datapath.c
Expand Up @@ -917,7 +917,7 @@ static int ovs_flow_cmd_new(struct sk_buff *skb, struct genl_info *info)
if (error)
goto err_kfree_flow;

ovs_flow_mask_key(&new_flow->key, &key, &mask);
ovs_flow_mask_key(&new_flow->key, &key, true, &mask);

/* Extract flow identifier. */
error = ovs_nla_get_identifier(&new_flow->id, a[OVS_FLOW_ATTR_UFID],
Expand Down Expand Up @@ -1044,7 +1044,7 @@ static struct sw_flow_actions *get_flow_actions(const struct nlattr *a,
struct sw_flow_key masked_key;
int error;

ovs_flow_mask_key(&masked_key, key, mask);
ovs_flow_mask_key(&masked_key, key, true, mask);
error = ovs_nla_copy_actions(a, &masked_key, &acts, log);
if (error) {
OVS_NLERR(log,
Expand Down
23 changes: 12 additions & 11 deletions datapath/flow_table.c
Expand Up @@ -63,20 +63,21 @@ static u16 range_n_bytes(const struct sw_flow_key_range *range)
}

void ovs_flow_mask_key(struct sw_flow_key *dst, const struct sw_flow_key *src,
const struct sw_flow_mask *mask)
bool full, const struct sw_flow_mask *mask)
{
const long *m = (const long *)((const u8 *)&mask->key +
mask->range.start);
const long *s = (const long *)((const u8 *)src +
mask->range.start);
long *d = (long *)((u8 *)dst + mask->range.start);
int start = full ? 0 : mask->range.start;
int len = full ? sizeof *dst : range_n_bytes(&mask->range);
const long *m = (const long *)((const u8 *)&mask->key + start);
const long *s = (const long *)((const u8 *)src + start);
long *d = (long *)((u8 *)dst + start);
int i;

/* The memory outside of the 'mask->range' are not set since
* further operations on 'dst' only uses contents within
* 'mask->range'.
/* If 'full' is true then all of 'dst' is fully initialized. Otherwise,
* if 'full' is false the memory outside of the 'mask->range' is left
* uninitialized. This can be used as an optimization when further
* operations on 'dst' only use contents within 'mask->range'.
*/
for (i = 0; i < range_n_bytes(&mask->range); i += sizeof(long))
for (i = 0; i < len; i += sizeof(long))
*d++ = *s++ & *m++;
}

Expand Down Expand Up @@ -554,7 +555,7 @@ static struct sw_flow *masked_flow_lookup(struct table_instance *ti,
u32 hash;
struct sw_flow_key masked_key;

ovs_flow_mask_key(&masked_key, unmasked, mask);
ovs_flow_mask_key(&masked_key, unmasked, false, mask);
hash = flow_hash(&masked_key, &mask->range);
head = find_bucket(ti, hash);
(*n_mask_hit)++;
Expand Down
2 changes: 1 addition & 1 deletion datapath/flow_table.h
Expand Up @@ -99,5 +99,5 @@ struct sw_flow *ovs_flow_tbl_lookup_ufid(struct flow_table *,
bool ovs_flow_cmp(const struct sw_flow *, const struct sw_flow_match *);

void ovs_flow_mask_key(struct sw_flow_key *dst, const struct sw_flow_key *src,
const struct sw_flow_mask *mask);
bool full, const struct sw_flow_mask *mask);
#endif /* flow_table.h */

0 comments on commit b19d3dd

Please sign in to comment.