diff mbox series

[ovs-dev,ovs] netdev-offload-tc: Use accurately function to check dl_type.

Message ID 1586512967-18878-1-git-send-email-xiangxia.m.yue@gmail.com
State Rejected
Headers show
Series [ovs-dev,ovs] netdev-offload-tc: Use accurately function to check dl_type. | expand

Commit Message

Tonghao Zhang April 10, 2020, 10:02 a.m. UTC
From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

When offloading the flows which matching the vlan feild,
there is an error[1]. Then we can't offload them, though
TC flower supports this. To fix it, we should use the
*dl_type_is_ip_any* instead of *is_ip_any*. Use ovs-appctl
command to reproduce it.

$ ovs-appctl dpctl/add-flow "recirc_id(0),in_port(5),eth(dst=00:11:22:33:44:66),
	eth_type(0x8100),vlan(vid=100,pcp=0),encap(eth_type(0x0800),ipv4(dst=1.1.1.2))"
	"pop_vlan,3"

[1] - "offloading isn't supported, unknown attribute"

Cc: Roi Dayan <roid@mellanox.com>
Cc: Paul Blakey <paulb@mellanox.com>
Cc: Ben Pfaff <blp@ovn.org>
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
 lib/netdev-offload-tc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

William Tu April 11, 2020, 3:48 p.m. UTC | #1
On Fri, Apr 10, 2020 at 06:02:47PM +0800, xiangxia.m.yue@gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> 
> When offloading the flows which matching the vlan feild,
> there is an error[1]. Then we can't offload them, though
> TC flower supports this. To fix it, we should use the
> *dl_type_is_ip_any* instead of *is_ip_any*. Use ovs-appctl
> command to reproduce it.
> 
> $ ovs-appctl dpctl/add-flow "recirc_id(0),in_port(5),eth(dst=00:11:22:33:44:66),
> 	eth_type(0x8100),vlan(vid=100,pcp=0),encap(eth_type(0x0800),ipv4(dst=1.1.1.2))"
> 	"pop_vlan,3"
> 
> [1] - "offloading isn't supported, unknown attribute"
> 
> Cc: Roi Dayan <roid@mellanox.com>
> Cc: Paul Blakey <paulb@mellanox.com>
> Cc: Ben Pfaff <blp@ovn.org>
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> ---
Hi Tonghao,

Do I need special HW to reproduce it?
I tried on my VM:

 $ ovs-vsctl set Open_vSwitch . other_config:hw-offload=true
 $ ovs-vsctl set Open_vSwitch . other_config:tc-policy=skip_hw
 $ ovs-appctl dpctl/add-flow "recirc_id(0),in_port(5),eth(dst=00:11:22:33:44:66),\
     eth_type(0x8100),vlan(vid=100,pcp=0),encap(eth_type(0x0800),ipv4(dst=1.1.1.2))" \
     "pop_vlan,3"

Seems to work ok.
William
 
>  lib/netdev-offload-tc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
> index 875ebef71941..3c52db3e90cd 100644
> --- a/lib/netdev-offload-tc.c
> +++ b/lib/netdev-offload-tc.c
> @@ -1500,7 +1500,7 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match,
>      mask->dl_type = 0;
>      mask->in_port.odp_port = 0;
>  
> -    if (is_ip_any(key)) {
> +    if (dl_type_is_ip_any(key->dl_type)) {
>          flower.key.ip_proto = key->nw_proto;
>          flower.mask.ip_proto = mask->nw_proto;
>          mask->nw_proto = 0;
> -- 
> 1.8.3.1
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Tonghao Zhang April 12, 2020, 5:19 a.m. UTC | #2
On Sat, Apr 11, 2020 at 11:48 PM William Tu <u9012063@gmail.com> wrote:
>
> On Fri, Apr 10, 2020 at 06:02:47PM +0800, xiangxia.m.yue@gmail.com wrote:
> > From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> >
> > When offloading the flows which matching the vlan feild,
> > there is an error[1]. Then we can't offload them, though
> > TC flower supports this. To fix it, we should use the
> > *dl_type_is_ip_any* instead of *is_ip_any*. Use ovs-appctl
> > command to reproduce it.
> >
> > $ ovs-appctl dpctl/add-flow "recirc_id(0),in_port(5),eth(dst=00:11:22:33:44:66),
> >       eth_type(0x8100),vlan(vid=100,pcp=0),encap(eth_type(0x0800),ipv4(dst=1.1.1.2))"
> >       "pop_vlan,3"
> >
> > [1] - "offloading isn't supported, unknown attribute"
> >
> > Cc: Roi Dayan <roid@mellanox.com>
> > Cc: Paul Blakey <paulb@mellanox.com>
> > Cc: Ben Pfaff <blp@ovn.org>
> > Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> > ---
> Hi Tonghao,
>
> Do I need special HW to reproduce it?
> I tried on my VM:
>
>  $ ovs-vsctl set Open_vSwitch . other_config:hw-offload=true
>  $ ovs-vsctl set Open_vSwitch . other_config:tc-policy=skip_hw
>  $ ovs-appctl dpctl/add-flow "recirc_id(0),in_port(5),eth(dst=00:11:22:33:44:66),\
>      eth_type(0x8100),vlan(vid=100,pcp=0),encap(eth_type(0x0800),ipv4(dst=1.1.1.2))" \
>      "pop_vlan,3"
>
> Seems to work ok.
Hi William
Thanks for this, in my ovs branch, the ufid will be created when using
the ovs-appctl dpctl command,
so the parse_flow_put(tc offload) will be invoked in
try_send_to_netdev. I guess I should send a patch
firstly to support TC offload via dpctl command.

> William
>
> >  lib/netdev-offload-tc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
> > index 875ebef71941..3c52db3e90cd 100644
> > --- a/lib/netdev-offload-tc.c
> > +++ b/lib/netdev-offload-tc.c
> > @@ -1500,7 +1500,7 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match,
> >      mask->dl_type = 0;
> >      mask->in_port.odp_port = 0;
> >
> > -    if (is_ip_any(key)) {
> > +    if (dl_type_is_ip_any(key->dl_type)) {
> >          flower.key.ip_proto = key->nw_proto;
> >          flower.mask.ip_proto = mask->nw_proto;
> >          mask->nw_proto = 0;
> > --
> > 1.8.3.1
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff mbox series

Patch

diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index 875ebef71941..3c52db3e90cd 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -1500,7 +1500,7 @@  netdev_tc_flow_put(struct netdev *netdev, struct match *match,
     mask->dl_type = 0;
     mask->in_port.odp_port = 0;
 
-    if (is_ip_any(key)) {
+    if (dl_type_is_ip_any(key->dl_type)) {
         flower.key.ip_proto = key->nw_proto;
         flower.mask.ip_proto = mask->nw_proto;
         mask->nw_proto = 0;