[ovs-dev] [PATCH ovs-dev] netdev-offload-tc: Allow installing arp rules to TC dp.

Simon Horman simon.horman at netronome.com
Fri Jun 5 08:12:33 UTC 2020


On Thu, Jun 04, 2020 at 08:31:08PM +0800, xiangxia.m.yue at gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue at gmail.com>
> 
> This patch allows to install arp rules to tc dp.
> In the future, arp will be offloaded to hardware to
> be processed. So OvS enable this now.
> 
> $ 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)' 2
> 
> $ ovs-appctl dpctl/dump-flows
>   ... arp(tip=10.255.1.116,op=2,tha=00:50:56:e1:4b:ab) ...
> 
> $ tc filter show dev <ethx> ingress
>   ...
>   eth_type arp
>   arp_tip 10.255.1.116
>   arp_op reply
>   arp_tha 00:50:56:e1:4b:ab
>   not_in_hw
>     action order 1: mirred (Egress Redirect to device <ethy>) stolen
>     ...
> 
> Cc: Simon Horman <simon.horman at netronome.com>
> Cc: Paul Blakey <paulb at mellanox.com>
> Cc: Roi Dayan <roid at mellanox.com>
> Cc: Ben Pfaff <blp at ovn.org>
> Cc: William Tu <u9012063 at gmail.com>
> Cc: Ilya Maximets <i.maximets at ovn.org>
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue at gmail.com>

Thanks for your patch, mostly it looks good to me.

Travis-CI flagged a minor problem, as described below.
Could you take a look into it and consider posting v2?

...

> diff --git a/lib/odp-util.c b/lib/odp-util.c
> index 72601dc6ba2b..c630b830ecd0 100644
> --- a/lib/odp-util.c
> +++ b/lib/odp-util.c
> @@ -7993,7 +7993,8 @@ get_arp_key(const struct flow *flow, struct ovs_key_arp *arp)
>  
>      arp->arp_sip = flow->nw_src;
>      arp->arp_tip = flow->nw_dst;
> -    arp->arp_op = htons(flow->nw_proto);
> +    arp->arp_op = flow->nw_proto == UINT8_MAX ?
> +                  UINT16_MAX: htons(flow->nw_proto);

This appears to cause a build error.

https://travis-ci.org/github/horms2/ovs/jobs/694632844#L2809

lib/odp-util.c:7997:36: error: restricted ovs_be16 degrades to integer
lib/odp-util.c:7996:17: error: incorrect type in assignment (different base types)
lib/odp-util.c:7996:17:    expected restricted ovs_be16 [usertype] arp_op
lib/odp-util.c:7996:17:    got int

Perhaps using OVS_BE16_MAX instead of UINT16_MAX is a good approach here.

Also, there should be a space both before and after ':'.
E.g.

	arp->arp_op = flow->nw_proto == UINT8_MAX ?
		OVS_BE16_MAX : htons(flow->nw_proto);

>      arp->arp_sha = flow->arp_sha;
>      arp->arp_tha = flow->arp_tha;
>  }

...


More information about the dev mailing list