On Sun, Jul 5, 2020 at 5:34 AM William Tu <[email protected]> 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@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@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 <[email protected]>
>
> ---
> 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.

>      if ((chain || recirc_act) && !info->recirc_id_shared_with_tc) {
>          VLOG_ERR_RL(&error_rl, "flow_put: recirc_id sharing not supported");
>          return EOPNOTSUPP;
> --
> 2.7.4
>


-- 
Best regards, Tonghao
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to