On Fri, Nov 20, 2020 at 10:12 PM Jakub Kicinski <k...@kernel.org> wrote:
>
> On Thu, 19 Nov 2020 04:04:04 -0500 Eelco Chaudron wrote:
> > 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.
>
> IOW this change will not break any known user space, correct?
>
> But existing OvS user space already expects it to work like you
> make it work now?
>
> What's the harm in leaving it as is?
>
> > Fixes: 744676e77720 ("openvswitch: add TTL decrement action")
> > Signed-off-by: Eelco Chaudron <echau...@redhat.com>
>
> Can we get a review from OvS folks? Matteo looks good to you (as the
> original author)?
>Hi, I think that the userspace still has to implement the dec_ttl action; by now dec_ttl is implemented with set_ttl(). So there is no breakage yet. Eelco, with this fix we will encode the netlink attribute in the same way for the kernel and netdev datapath? If so, go for it. > > - 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; > > You're not canceling any nests on error, I assume this is normal. > > > + add_nested_action_end(*sfa, action_start); > > add_nested_action_end(*sfa, start); > > return 0; > > } > -- per aspera ad upstream
