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

William Tu u9012063 at gmail.com
Mon Jul 6 16:13:24 UTC 2020


On Sun, Jul 5, 2020 at 9:49 PM Tonghao Zhang <xiangxia.m.yue at gmail.com> wrote:
>
> On Sun, Jul 5, 2020 at 5:34 AM William Tu <u9012063 at gmail.com> 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
> >
> > Or no action also infers drop
> > $ 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)' ''
> >
> > $ 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>
> >
> > ---
> > v2:
> >   travis: https://travis-ci.org/github/williamtu/ovs-travis/builds/704996060
> >   address feedback from Simon and Tonghao
> > ---
> >  lib/netdev-offload-tc.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
> > index 258d31f54b08..ce90425c5f1b 100644
> > --- a/lib/netdev-offload-tc.c
> > +++ b/lib/netdev-offload-tc.c
> > @@ -1788,6 +1788,10 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match,
> >              action->chain = nl_attr_get_u32(nla);
> >              flower.action_count++;
> >              recirc_act = true;
> > +        } else if (nl_attr_type(nla) == OVS_ACTION_ATTR_DROP) {
> > +            action->type = TC_ACT_GOTO;
> > +            action->chain = 0;  /* 0 is reserved and not used by recirc. */
> > +            flower.action_count++;
> >          } else {
> >              VLOG_DBG_RL(&rl, "unsupported put action type: %d",
> >                          nl_attr_type(nla));
> > @@ -1795,6 +1799,13 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match,
> >          }
> >      }
> >
> > +    if (flower.action_count == 0) {
> > +        action = &flower.actions[flower.action_count];
> > +        action->type = TC_ACT_GOTO;
> > +        action->chain = 0;  /* Drop Action. */
> > +        flower.action_count++;
> > +    }
> Sorry for not being clear.
> I mean that we added drop action when there is no action. the call tree is:
> netdev_tc_flow_put
> tc_replace_flower
> nl_msg_put_flower_options
> nl_msg_put_flower_acts:
>     if (!flower->action_count) {
>         nl_msg_put_act_gact(request, 0);
>         ....
>     }
>
> And we can also add the check there as your patch did, for rules when no action.
Do you mean add the check at both functions ? (nl_msg_put_flower_acts
and netdev_tc_flow_put)

I'm confused. What's the difference between adding at drop at
nl_msg_put_flower_acts(), as your above code,
and adding the drop at
netdev_tc_flow_put(), as my patch?

Aren't both handling the case where there is no action, and insert a
drop action?
And we should only need it in one place, either at nl_msg_put_flower_acts or at
netdev_tc_flow_put.

William


More information about the dev mailing list