Commit 69929d4c authored by Eelco Chaudron's avatar Eelco Chaudron Committed by Jakub Kicinski

net: openvswitch: fix TTL decrement action netlink message format

Currently, the openvswitch module is not accepting the correctly formated
netlink message for the TTL decrement action. For both setting and getting
the dec_ttl action, the actions should be nested in the
OVS_DEC_TTL_ATTR_ACTION attribute as mentioned in the openvswitch.h uapi.

When the original patch was sent, it was tested with a private OVS userspace
implementation. This implementation was unfortunately not upstreamed and
reviewed, hence an erroneous version of this patch was sent out.

Leaving the patch as-is would cause problems as the kernel module could
interpret additional attributes as actions and vice-versa, due to the
actions not being encapsulated/nested within the actual attribute, but
being concatinated after it.

Fixes: 744676e7 ("openvswitch: add TTL decrement action")
Signed-off-by: default avatarEelco Chaudron <echaudro@redhat.com>
Link: https://lore.kernel.org/r/160622121495.27296.888010441924340582.stgit@wsfd-netdev64.ntdv.lab.eng.bos.redhat.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent cbf3d603
......@@ -1058,4 +1058,6 @@ enum ovs_dec_ttl_attr {
__OVS_DEC_TTL_ATTR_MAX
};
#define OVS_DEC_TTL_ATTR_MAX (__OVS_DEC_TTL_ATTR_MAX - 1)
#endif /* _LINUX_OPENVSWITCH_H */
......@@ -958,14 +958,13 @@ static int dec_ttl_exception_handler(struct datapath *dp, struct sk_buff *skb,
{
/* The first action is always 'OVS_DEC_TTL_ATTR_ARG'. */
struct nlattr *dec_ttl_arg = nla_data(attr);
int rem = nla_len(attr);
if (nla_len(dec_ttl_arg)) {
struct nlattr *actions = nla_next(dec_ttl_arg, &rem);
struct nlattr *actions = nla_data(dec_ttl_arg);
if (actions)
return clone_execute(dp, skb, key, 0, actions, rem,
last, false);
return clone_execute(dp, skb, key, 0, nla_data(actions),
nla_len(actions), last, false);
}
consume_skb(skb);
return 0;
......
......@@ -2503,28 +2503,42 @@ static int validate_and_copy_dec_ttl(struct net *net,
__be16 eth_type, __be16 vlan_tci,
u32 mpls_label_count, bool log)
{
int start, err;
u32 nested = true;
const struct nlattr *attrs[OVS_DEC_TTL_ATTR_MAX + 1];
int start, action_start, err, rem;
const struct nlattr *a, *actions;
memset(attrs, 0, sizeof(attrs));
nla_for_each_nested(a, attr, rem) {
int type = nla_type(a);
if (!nla_len(attr))
return ovs_nla_add_action(sfa, OVS_ACTION_ATTR_DEC_TTL,
NULL, 0, log);
/* Ignore unknown attributes to be future proof. */
if (type > OVS_DEC_TTL_ATTR_MAX)
continue;
if (!type || attrs[type])
return -EINVAL;
attrs[type] = a;
}
actions = attrs[OVS_DEC_TTL_ATTR_ACTION];
if (rem || !actions || (nla_len(actions) && nla_len(actions) < NLA_HDRLEN))
return -EINVAL;
start = add_nested_action_start(sfa, OVS_ACTION_ATTR_DEC_TTL, log);
if (start < 0)
return start;
err = ovs_nla_add_action(sfa, OVS_DEC_TTL_ATTR_ACTION, &nested,
sizeof(nested), log);
if (err)
return err;
action_start = add_nested_action_start(sfa, OVS_DEC_TTL_ATTR_ACTION, log);
if (action_start < 0)
return start;
err = __ovs_nla_copy_actions(net, attr, key, sfa, eth_type,
err = __ovs_nla_copy_actions(net, actions, key, sfa, eth_type,
vlan_tci, mpls_label_count, log);
if (err)
return err;
add_nested_action_end(*sfa, action_start);
add_nested_action_end(*sfa, start);
return 0;
}
......@@ -3487,20 +3501,42 @@ static int check_pkt_len_action_to_attr(const struct nlattr *attr,
static int dec_ttl_action_to_attr(const struct nlattr *attr,
struct sk_buff *skb)
{
int err = 0, rem = nla_len(attr);
struct nlattr *start;
struct nlattr *start, *action_start;
const struct nlattr *a;
int err = 0, rem;
start = nla_nest_start_noflag(skb, OVS_ACTION_ATTR_DEC_TTL);
if (!start)
return -EMSGSIZE;
err = ovs_nla_put_actions(nla_data(attr), rem, skb);
if (err)
nla_nest_cancel(skb, start);
else
nla_nest_end(skb, start);
nla_for_each_attr(a, nla_data(attr), nla_len(attr), rem) {
switch (nla_type(a)) {
case OVS_DEC_TTL_ATTR_ACTION:
action_start = nla_nest_start_noflag(skb, OVS_DEC_TTL_ATTR_ACTION);
if (!action_start) {
err = -EMSGSIZE;
goto out;
}
err = ovs_nla_put_actions(nla_data(a), nla_len(a), skb);
if (err)
goto out;
nla_nest_end(skb, action_start);
break;
default:
/* Ignore all other option to be future compatible */
break;
}
}
nla_nest_end(skb, start);
return 0;
out:
nla_nest_cancel(skb, start);
return err;
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment