| Message ID | 160708417520.39389.4157710029285521561.stgit@wsfd-netdev64.ntdv.lab.eng.bos.redhat.com |
|---|---|
| State | Awaiting Upstream |
| Headers | show |
| Series | [ovs-dev,net] net: openvswitch: fix TTL decrement exception action execution | expand |
On Fri, 4 Dec 2020 07:16:23 -0500 Eelco Chaudron wrote: > Currently, the exception actions are not processed correctly as the wrong > dataset is passed. This change fixes this, including the misleading > comment. > > In addition, a check was added to make sure we work on an IPv4 packet, > and not just assume if it's not IPv6 it's IPv4. > > Small cleanup which removes an unsessesaty parameter from the > dec_ttl_exception_handler() function. No cleanups in fixes, please. Especially when we're at -rc6.. You can clean this up in net-next within a week after trees merge. > Fixes: 69929d4c49e1 ("net: openvswitch: fix TTL decrement action netlink message format") :( and please add some info on how these changes are tested. Thanks!
On 5 Dec 2020, at 1:30, Jakub Kicinski wrote: > On Fri, 4 Dec 2020 07:16:23 -0500 Eelco Chaudron wrote: >> Currently, the exception actions are not processed correctly as the >> wrong >> dataset is passed. This change fixes this, including the misleading >> comment. >> >> In addition, a check was added to make sure we work on an IPv4 >> packet, >> and not just assume if it's not IPv6 it's IPv4. >> >> Small cleanup which removes an unsessesaty parameter from the >> dec_ttl_exception_handler() function. > > No cleanups in fixes, please. Especially when we're at -rc6.. > > You can clean this up in net-next within a week after trees merge. Ack, will undo the parameter removal, and sent out a v2. >> Fixes: 69929d4c49e1 ("net: openvswitch: fix TTL decrement action >> netlink message format") > > :( > and please add some info on how these changes are tested. Will add the details to v2.
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c index 5829a020b81c..616fa43ff6df 100644 --- a/net/openvswitch/actions.c +++ b/net/openvswitch/actions.c @@ -954,18 +954,15 @@ static int output_userspace(struct datapath *dp, struct sk_buff *skb, static int dec_ttl_exception_handler(struct datapath *dp, struct sk_buff *skb, struct sw_flow_key *key, - const struct nlattr *attr, bool last) + const struct nlattr *attr) { - /* The first action is always 'OVS_DEC_TTL_ATTR_ARG'. */ - struct nlattr *dec_ttl_arg = nla_data(attr); + /* The first attribute is always 'OVS_DEC_TTL_ATTR_ACTION'. */ + struct nlattr *actions = nla_data(attr); - if (nla_len(dec_ttl_arg)) { - struct nlattr *actions = nla_data(dec_ttl_arg); + if (nla_len(actions)) + return clone_execute(dp, skb, key, 0, nla_data(actions), + nla_len(actions), true, false); - if (actions) - return clone_execute(dp, skb, key, 0, nla_data(actions), - nla_len(actions), last, false); - } consume_skb(skb); return 0; } @@ -1209,7 +1206,7 @@ static int execute_dec_ttl(struct sk_buff *skb, struct sw_flow_key *key) return -EHOSTUNREACH; key->ip.ttl = --nh->hop_limit; - } else { + } else if (skb->protocol == htons(ETH_P_IP)) { struct iphdr *nh; u8 old_ttl; @@ -1418,11 +1415,9 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb, case OVS_ACTION_ATTR_DEC_TTL: err = execute_dec_ttl(skb, key); - if (err == -EHOSTUNREACH) { - err = dec_ttl_exception_handler(dp, skb, key, - a, true); - return err; - } + if (err == -EHOSTUNREACH) + return dec_ttl_exception_handler(dp, skb, + key, a); break; }
Currently, the exception actions are not processed correctly as the wrong dataset is passed. This change fixes this, including the misleading comment. In addition, a check was added to make sure we work on an IPv4 packet, and not just assume if it's not IPv6 it's IPv4. Small cleanup which removes an unsessesaty parameter from the dec_ttl_exception_handler() function. Fixes: 69929d4c49e1 ("net: openvswitch: fix TTL decrement action netlink message format") Signed-off-by: Eelco Chaudron <echaudro@redhat.com> --- net/openvswitch/actions.c | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-)