[ovs-dev] [PATCH] netdev-offload-tc: Add drop action support.

[William Tu]

On Thu, Jul 02, 2020 at 07:24:35PM +0800, Tonghao Zhang wrote:
> On Wed, Jul 1, 2020 at 12:30 AM William Tu <u9012063 at gmail.com> wrote:
> >
> > On Tue, Jun 30, 2020 at 9:13 AM Simon Horman <simon.horman at netronome.com> wrote:
> > >
> > > On Tue, Jun 30, 2020 at 09:06:40AM -0700, William Tu wrote:
> > > > On Tue, Jun 30, 2020 at 9:01 AM Simon Horman <simon.horman at netronome.com> wrote:
> > > > >
> > > > > On Tue, Jun 30, 2020 at 08:30:45AM -0700, William Tu wrote:
> > > > > > Currently drop action is not offloaded when using userspace datapath
> > > > > > with tc offload.  The patch programs tc gact (generic action) chain
> > > > > > ID 0 to drop the packet by setting it to TC_ACT_SHOT.
> > > > > >
> > > > > > Example:
> > > > > > $ ovs-appctl dpctl/add-flow netdev at ovs-netdev \
> > > > > >   'recirc_id(0),in_port(2),eth(),eth_type(0x0806),\
> > > > > >   arp(op=2,tha=00:50:56:e1:4b:ab,tip=10.255.1.116)' drop
> > > > > >
> > > > > > $ tc filter show dev ovs-p0 ingress
> > > > > > filter protocol arp pref 2 flower chain 0
> > > > > > filter protocol arp pref 2 flower chain 0 handle 0x1
> > > > > >   eth_type arp
> > > > > >   arp_tip 10.255.1.116
> > > > > >   arp_op reply
> > > > > >   arp_tha 00:50:56:e1:4b:ab
> > > > > >   skip_hw
> > > > > >   not_in_hw
> > > > > >       action order 1: gact action drop
> > > > > >     ...
> > > > > >
> > > > > > Signed-off-by: William Tu <u9012063 at gmail.com>
> > > > >
> > > > > Hi William,
> > > > >
> > > > > this change looks good to me other than that I'm unsure
> > > > > why we need #ifndef __KERNEL__.
> > > > >
> > > > Because for kernel datapath, we don't define
> > > > OVS_ACTION_ATTR_DROP
> > > >
> > > > at datapath/linux/compat/include/linux/openvswitch.h
> > > > ...
> > > > #ifndef __KERNEL__
> > > >     OVS_ACTION_ATTR_TUNNEL_PUSH,   /* struct ovs_action_push_tnl*/
> > > >     OVS_ACTION_ATTR_TUNNEL_POP,    /* u32 port number. */
> > > >     OVS_ACTION_ATTR_DROP,          /* u32 xlate_error. */
> > > >     OVS_ACTION_ATTR_LB_OUTPUT,     /* u32 bond-id. */
> > > > #endif
> > >
> > > I feel that I'm missing something terribly obvious, but it seems to me that
> > > lib/netdev-offload-tc.c is only ever compiled as user-space code (its not
> > > compiled into the kernel datapath) and thus __KERNEL__ would never be set.
> >
> > I see, so probably checking __KERNEL__ here is not necessary. Will remove it.
> Hi William
> It looks good to me.
> BTW, If there is no action, we use the drop action as default.
> $ ovs-appctl dpctl/add-flow
> 'recirc_id(0),in_port(3),eth(),eth_type(0x0806),arp(op=2,tha=00:50:56:e1:4b:ab,tip=10.255.1.116)'

Thanks, I will fix it in v2.
William