Skip to content

Commit

Permalink
datapath: fix the incorrect flow action alloc size
Browse files Browse the repository at this point in the history
Upstream commit:
    commit 67c8d22a73128ff910e2287567132530abcf5b71
    Author: zhangliping <zhangliping02@baidu.com>
    Date:   Sat Nov 25 22:02:12 2017 +0800

    openvswitch: fix the incorrect flow action alloc size

    If we want to add a datapath flow, which has more than 500 vxlan outputs'
    action, we will get the following error reports:
      openvswitch: netlink: Flow action size 32832 bytes exceeds max
      openvswitch: netlink: Flow action size 32832 bytes exceeds max
      openvswitch: netlink: Actions may not be safe on all matching packets
      ... ...

    It seems that we can simply enlarge the MAX_ACTIONS_BUFSIZE to fix it, but
    this is not the root cause. For example, for a vxlan output action, we need
    about 60 bytes for the nlattr, but after it is converted to the flow
    action, it only occupies 24 bytes. This means that we can still support
    more than 1000 vxlan output actions for a single datapath flow under the
    the current 32k max limitation.

    So even if the nla_len(attr) is larger than MAX_ACTIONS_BUFSIZE, we
    shouldn't report EINVAL and keep it move on, as the judgement can be
    done by the reserve_sfa_size.

    Signed-off-by: zhangliping <zhangliping02@baidu.com>
    Acked-by: Pravin B Shelar <pshelar@ovn.org>
    Signed-off-by: David S. Miller <davem@davemloft.net>

Cc: zhangliping <zhangliping02@baidu.com>
Signed-off-by: Greg Rose <gvrose8192@gmail.com>
Acked-by: Pravin B Shelar <pshelar@ovn.org>
  • Loading branch information
zhangliping authored and pshelar committed Feb 12, 2018
1 parent f54a7a5 commit 5505308
Showing 1 changed file with 8 additions and 8 deletions.
16 changes: 8 additions & 8 deletions datapath/flow_netlink.c
Original file line number Diff line number Diff line change
Expand Up @@ -2198,14 +2198,11 @@ int ovs_nla_put_mask(const struct sw_flow *flow, struct sk_buff *skb)
#define MAX_ACTIONS_BUFSIZE (32 * 1024)
#endif

static struct sw_flow_actions *nla_alloc_flow_actions(int size, bool log)
static struct sw_flow_actions *nla_alloc_flow_actions(int size)
{
struct sw_flow_actions *sfa;

if (size > MAX_ACTIONS_BUFSIZE) {
OVS_NLERR(log, "Flow action size %u bytes exceeds max", size);
return ERR_PTR(-EINVAL);
}
WARN_ON_ONCE(size > MAX_ACTIONS_BUFSIZE);

sfa = kmalloc(sizeof(*sfa) + size, GFP_KERNEL);
if (!sfa)
Expand Down Expand Up @@ -2278,12 +2275,15 @@ static struct nlattr *reserve_sfa_size(struct sw_flow_actions **sfa,
new_acts_size = ksize(*sfa) * 2;

if (new_acts_size > MAX_ACTIONS_BUFSIZE) {
if ((MAX_ACTIONS_BUFSIZE - next_offset) < req_size)
if ((MAX_ACTIONS_BUFSIZE - next_offset) < req_size) {
OVS_NLERR(log, "Flow action size exceeds max %u",
MAX_ACTIONS_BUFSIZE);
return ERR_PTR(-EMSGSIZE);
}
new_acts_size = MAX_ACTIONS_BUFSIZE;
}

acts = nla_alloc_flow_actions(new_acts_size, log);
acts = nla_alloc_flow_actions(new_acts_size);
if (IS_ERR(acts))
return (void *)acts;

Expand Down Expand Up @@ -3012,7 +3012,7 @@ int ovs_nla_copy_actions(struct net *net, const struct nlattr *attr,
{
int err;

*sfa = nla_alloc_flow_actions(nla_len(attr), log);
*sfa = nla_alloc_flow_actions(min(nla_len(attr), MAX_ACTIONS_BUFSIZE));
if (IS_ERR(*sfa))
return PTR_ERR(*sfa);

Expand Down

0 comments on commit 5505308

Please sign in to comment.