Re: [ovs-dev] [PATCH branch-2.13] netdev-offload-dpdk: Support vxlan encap offload with load actions

2020-10-20 Thread Sriharsha Basavapatna via dev 
On Tue, Oct 20, 2020 at 8:59 PM Ilya Maximets  wrote:
>
> On 10/20/20 5:04 PM, Eli Britstein wrote:
> >
> > On 10/20/2020 1:12 PM, Sriharsha Basavapatna wrote:
> >> On Tue, Oct 20, 2020 at 2:40 PM Ilya Maximets  wrote:
> >>> On 10/20/20 10:51 AM, Sriharsha Basavapatna wrote:
>  On Mon, Oct 19, 2020 at 7:59 PM Ilya Maximets  wrote:
> > On 10/18/20 9:10 AM, Eli Britstein wrote:
> >> Struct match has the tunnel values/masks in
> >> match->flow.tunnel/match->wc.masks.tunnel.
> >> Load actions such as load:0xa566c10->NXM_NX_TUN_IPV4_DST[],
> >> load:0xbba->NXM_NX_TUN_ID[] are utilizing the tunnel masks fields, but
> >> those should not be used for matching.
> >> Offloading fails if masks is not clear. Fix it by checking if tunnel is
> >> present.
> >>
> >> Signed-off-by: Eli Britstein 
> >> ---
> > Thanks, Eli.
> >
> > Harsha, Emma, could you, please, review/test this version?
> >
> > Best regards, Ilya Maximets.
>  Reviewed this change. Do we even need this backported to 2.13, since
>  we don't support clone/tnl-push actions with offload ?
> >>> I think we could have metadata partially set even if we're not performing
> >>> clone/tnl-push actions.  Maybe something like this:
> >>>
> >>>table=0,in_port=1,ip,actions=load:0xbba->NXM_NX_TUN_ID[],goto_table(1)
> >>>
> >>> table=1,ip,nw_src=192.168.0.1,actions=load:0xa566c10->NXM_NX_TUN_IPV4_DST[], >>>  to tunnel>
> >>>table=1,ip,nw_src=192.168.0.2,actions=drop
> >>>
> >>> In this scenario packet that goes to 'drop' action might have tun_id set
> >>> in the metadata and we will not offload it.  I didn't test that though.
> >>> Does that make sense?
> >> I tested with the above set of rules. It doesn't fail in
> >> validate_flow() even without this fix, since it checks
> >> match_zero_wc.flow.tunnel and not match_zero_wc.masks.tunnel which has
> >> the tun_id (mask) set. So, this fix doesn't make any difference
> >> (validate succeeds with or without it).
> >>
> >> (gdb) p /x match_zero_wc.flow.tunnel.tun_id
> >> $4 = 0x0
> >> (gdb) p /x match_zero_wc.wc.masks.tunnel.tun_id
> >> $5 = 0x
> >
> > Harsha - you are right. Thanks. This patch is not needed there as is. Maybe 
> > it was intentional to check the "flow" and not "masks" for tunnel, so I 
> > think we can abandon it for backport <=2.13.
>
> Good catch. Thanks, Harsha!
>
> Lets abandon this patch in this case.

Thanks Ilya and Eli.
-Harsha

>
> >
> > Regarding not supporting clone/tnl-push - it is not related. The issue is 
> > (might have been) with validation of matches. If we checked the masks we 
> > would fail also the partial offload.
> >
> >>
> >> Thanks,
> >> -Harsha
>  Thanks,
>  -Harsha
> >>   lib/netdev-offload-dpdk.c | 3 ++-
> >>   1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> >> index 4538baf5e..c68d539ea 100644
> >> --- a/lib/netdev-offload-dpdk.c
> >> +++ b/lib/netdev-offload-dpdk.c
> >> @@ -1092,7 +1092,8 @@ netdev_offload_dpdk_validate_flow(const struct 
> >> match *match)
> >>   /* Create a wc-zeroed version of flow. */
> >>   match_init(_zero_wc, >flow, >wc);
> >>
> >> -if (!is_all_zeros(_zero_wc.flow.tunnel,
> >> +if (flow_tnl_dst_is_set(>flow.tunnel) &&
> >> +!is_all_zeros(_zero_wc.flow.tunnel,
> >> sizeof match_zero_wc.flow.tunnel)) {
> >>   goto err;
> >>   }
> >>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH branch-2.13] netdev-offload-dpdk: Support vxlan encap offload with load actions

2020-10-20 Thread Ilya Maximets 
On 10/20/20 5:04 PM, Eli Britstein wrote:
> 
> On 10/20/2020 1:12 PM, Sriharsha Basavapatna wrote:
>> On Tue, Oct 20, 2020 at 2:40 PM Ilya Maximets  wrote:
>>> On 10/20/20 10:51 AM, Sriharsha Basavapatna wrote:
 On Mon, Oct 19, 2020 at 7:59 PM Ilya Maximets  wrote:
> On 10/18/20 9:10 AM, Eli Britstein wrote:
>> Struct match has the tunnel values/masks in
>> match->flow.tunnel/match->wc.masks.tunnel.
>> Load actions such as load:0xa566c10->NXM_NX_TUN_IPV4_DST[],
>> load:0xbba->NXM_NX_TUN_ID[] are utilizing the tunnel masks fields, but
>> those should not be used for matching.
>> Offloading fails if masks is not clear. Fix it by checking if tunnel is
>> present.
>>
>> Signed-off-by: Eli Britstein 
>> ---
> Thanks, Eli.
>
> Harsha, Emma, could you, please, review/test this version?
>
> Best regards, Ilya Maximets.
 Reviewed this change. Do we even need this backported to 2.13, since
 we don't support clone/tnl-push actions with offload ?
>>> I think we could have metadata partially set even if we're not performing
>>> clone/tnl-push actions.  Maybe something like this:
>>>
>>>    table=0,in_port=1,ip,actions=load:0xbba->NXM_NX_TUN_ID[],goto_table(1)
>>>    
>>> table=1,ip,nw_src=192.168.0.1,actions=load:0xa566c10->NXM_NX_TUN_IPV4_DST[],>>  to tunnel>
>>>    table=1,ip,nw_src=192.168.0.2,actions=drop
>>>
>>> In this scenario packet that goes to 'drop' action might have tun_id set
>>> in the metadata and we will not offload it.  I didn't test that though.
>>> Does that make sense?
>> I tested with the above set of rules. It doesn't fail in
>> validate_flow() even without this fix, since it checks
>> match_zero_wc.flow.tunnel and not match_zero_wc.masks.tunnel which has
>> the tun_id (mask) set. So, this fix doesn't make any difference
>> (validate succeeds with or without it).
>>
>> (gdb) p /x match_zero_wc.flow.tunnel.tun_id
>> $4 = 0x0
>> (gdb) p /x match_zero_wc.wc.masks.tunnel.tun_id
>> $5 = 0x
> 
> Harsha - you are right. Thanks. This patch is not needed there as is. Maybe 
> it was intentional to check the "flow" and not "masks" for tunnel, so I think 
> we can abandon it for backport <=2.13.

Good catch. Thanks, Harsha!

Lets abandon this patch in this case.

> 
> Regarding not supporting clone/tnl-push - it is not related. The issue is 
> (might have been) with validation of matches. If we checked the masks we 
> would fail also the partial offload.
> 
>>
>> Thanks,
>> -Harsha
 Thanks,
 -Harsha
>>   lib/netdev-offload-dpdk.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
>> index 4538baf5e..c68d539ea 100644
>> --- a/lib/netdev-offload-dpdk.c
>> +++ b/lib/netdev-offload-dpdk.c
>> @@ -1092,7 +1092,8 @@ netdev_offload_dpdk_validate_flow(const struct 
>> match *match)
>>   /* Create a wc-zeroed version of flow. */
>>   match_init(_zero_wc, >flow, >wc);
>>
>> -    if (!is_all_zeros(_zero_wc.flow.tunnel,
>> +    if (flow_tnl_dst_is_set(>flow.tunnel) &&
>> +    !is_all_zeros(_zero_wc.flow.tunnel,
>>     sizeof match_zero_wc.flow.tunnel)) {
>>   goto err;
>>   }
>>

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH branch-2.13] netdev-offload-dpdk: Support vxlan encap offload with load actions

2020-10-20 Thread Eli Britstein 



On 10/20/2020 1:12 PM, Sriharsha Basavapatna wrote:

On Tue, Oct 20, 2020 at 2:40 PM Ilya Maximets  wrote:

On 10/20/20 10:51 AM, Sriharsha Basavapatna wrote:

On Mon, Oct 19, 2020 at 7:59 PM Ilya Maximets  wrote:

On 10/18/20 9:10 AM, Eli Britstein wrote:

Struct match has the tunnel values/masks in
match->flow.tunnel/match->wc.masks.tunnel.
Load actions such as load:0xa566c10->NXM_NX_TUN_IPV4_DST[],
load:0xbba->NXM_NX_TUN_ID[] are utilizing the tunnel masks fields, but
those should not be used for matching.
Offloading fails if masks is not clear. Fix it by checking if tunnel is
present.

Signed-off-by: Eli Britstein 
---

Thanks, Eli.

Harsha, Emma, could you, please, review/test this version?

Best regards, Ilya Maximets.

Reviewed this change. Do we even need this backported to 2.13, since
we don't support clone/tnl-push actions with offload ?

I think we could have metadata partially set even if we're not performing
clone/tnl-push actions.  Maybe something like this:

   table=0,in_port=1,ip,actions=load:0xbba->NXM_NX_TUN_ID[],goto_table(1)
   
table=1,ip,nw_src=192.168.0.1,actions=load:0xa566c10->NXM_NX_TUN_IPV4_DST[],
   table=1,ip,nw_src=192.168.0.2,actions=drop

In this scenario packet that goes to 'drop' action might have tun_id set
in the metadata and we will not offload it.  I didn't test that though.
Does that make sense?

I tested with the above set of rules. It doesn't fail in
validate_flow() even without this fix, since it checks
match_zero_wc.flow.tunnel and not match_zero_wc.masks.tunnel which has
the tun_id (mask) set. So, this fix doesn't make any difference
(validate succeeds with or without it).

(gdb) p /x match_zero_wc.flow.tunnel.tun_id
$4 = 0x0
(gdb) p /x match_zero_wc.wc.masks.tunnel.tun_id
$5 = 0x


Harsha - you are right. Thanks. This patch is not needed there as is. 
Maybe it was intentional to check the "flow" and not "masks" for tunnel, 
so I think we can abandon it for backport <=2.13.


Regarding not supporting clone/tnl-push - it is not related. The issue 
is (might have been) with validation of matches. If we checked the masks 
we would fail also the partial offload.




Thanks,
-Harsha

Thanks,
-Harsha

  lib/netdev-offload-dpdk.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 4538baf5e..c68d539ea 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -1092,7 +1092,8 @@ netdev_offload_dpdk_validate_flow(const struct match 
*match)
  /* Create a wc-zeroed version of flow. */
  match_init(_zero_wc, >flow, >wc);

-if (!is_all_zeros(_zero_wc.flow.tunnel,
+if (flow_tnl_dst_is_set(>flow.tunnel) &&
+!is_all_zeros(_zero_wc.flow.tunnel,
sizeof match_zero_wc.flow.tunnel)) {
  goto err;
  }


___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH branch-2.13] netdev-offload-dpdk: Support vxlan encap offload with load actions

2020-10-20 Thread Sriharsha Basavapatna via dev 
On Tue, Oct 20, 2020 at 2:40 PM Ilya Maximets  wrote:
>
> On 10/20/20 10:51 AM, Sriharsha Basavapatna wrote:
> > On Mon, Oct 19, 2020 at 7:59 PM Ilya Maximets  wrote:
> >>
> >> On 10/18/20 9:10 AM, Eli Britstein wrote:
> >>> Struct match has the tunnel values/masks in
> >>> match->flow.tunnel/match->wc.masks.tunnel.
> >>> Load actions such as load:0xa566c10->NXM_NX_TUN_IPV4_DST[],
> >>> load:0xbba->NXM_NX_TUN_ID[] are utilizing the tunnel masks fields, but
> >>> those should not be used for matching.
> >>> Offloading fails if masks is not clear. Fix it by checking if tunnel is
> >>> present.
> >>>
> >>> Signed-off-by: Eli Britstein 
> >>> ---
> >>
> >> Thanks, Eli.
> >>
> >> Harsha, Emma, could you, please, review/test this version?
> >>
> >> Best regards, Ilya Maximets.
> >
> > Reviewed this change. Do we even need this backported to 2.13, since
> > we don't support clone/tnl-push actions with offload ?
>
> I think we could have metadata partially set even if we're not performing
> clone/tnl-push actions.  Maybe something like this:
>
>   table=0,in_port=1,ip,actions=load:0xbba->NXM_NX_TUN_ID[],goto_table(1)
>   
> table=1,ip,nw_src=192.168.0.1,actions=load:0xa566c10->NXM_NX_TUN_IPV4_DST[],  to tunnel>
>   table=1,ip,nw_src=192.168.0.2,actions=drop
>
> In this scenario packet that goes to 'drop' action might have tun_id set
> in the metadata and we will not offload it.  I didn't test that though.
> Does that make sense?

I tested with the above set of rules. It doesn't fail in
validate_flow() even without this fix, since it checks
match_zero_wc.flow.tunnel and not match_zero_wc.masks.tunnel which has
the tun_id (mask) set. So, this fix doesn't make any difference
(validate succeeds with or without it).

(gdb) p /x match_zero_wc.flow.tunnel.tun_id
$4 = 0x0
(gdb) p /x match_zero_wc.wc.masks.tunnel.tun_id
$5 = 0x

Thanks,
-Harsha
>
> >
> > Thanks,
> > -Harsha
> >>
> >>>  lib/netdev-offload-dpdk.c | 3 ++-
> >>>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> >>> index 4538baf5e..c68d539ea 100644
> >>> --- a/lib/netdev-offload-dpdk.c
> >>> +++ b/lib/netdev-offload-dpdk.c
> >>> @@ -1092,7 +1092,8 @@ netdev_offload_dpdk_validate_flow(const struct 
> >>> match *match)
> >>>  /* Create a wc-zeroed version of flow. */
> >>>  match_init(_zero_wc, >flow, >wc);
> >>>
> >>> -if (!is_all_zeros(_zero_wc.flow.tunnel,
> >>> +if (flow_tnl_dst_is_set(>flow.tunnel) &&
> >>> +!is_all_zeros(_zero_wc.flow.tunnel,
> >>>sizeof match_zero_wc.flow.tunnel)) {
> >>>  goto err;
> >>>  }
> >>>
> >>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH branch-2.13] netdev-offload-dpdk: Support vxlan encap offload with load actions

2020-10-20 Thread Ilya Maximets 
On 10/20/20 10:51 AM, Sriharsha Basavapatna wrote:
> On Mon, Oct 19, 2020 at 7:59 PM Ilya Maximets  wrote:
>>
>> On 10/18/20 9:10 AM, Eli Britstein wrote:
>>> Struct match has the tunnel values/masks in
>>> match->flow.tunnel/match->wc.masks.tunnel.
>>> Load actions such as load:0xa566c10->NXM_NX_TUN_IPV4_DST[],
>>> load:0xbba->NXM_NX_TUN_ID[] are utilizing the tunnel masks fields, but
>>> those should not be used for matching.
>>> Offloading fails if masks is not clear. Fix it by checking if tunnel is
>>> present.
>>>
>>> Signed-off-by: Eli Britstein 
>>> ---
>>
>> Thanks, Eli.
>>
>> Harsha, Emma, could you, please, review/test this version?
>>
>> Best regards, Ilya Maximets.
> 
> Reviewed this change. Do we even need this backported to 2.13, since
> we don't support clone/tnl-push actions with offload ?

I think we could have metadata partially set even if we're not performing
clone/tnl-push actions.  Maybe something like this:

  table=0,in_port=1,ip,actions=load:0xbba->NXM_NX_TUN_ID[],goto_table(1)
  
table=1,ip,nw_src=192.168.0.1,actions=load:0xa566c10->NXM_NX_TUN_IPV4_DST[],
  table=1,ip,nw_src=192.168.0.2,actions=drop

In this scenario packet that goes to 'drop' action might have tun_id set
in the metadata and we will not offload it.  I didn't test that though.
Does that make sense?

> 
> Thanks,
> -Harsha
>>
>>>  lib/netdev-offload-dpdk.c | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
>>> index 4538baf5e..c68d539ea 100644
>>> --- a/lib/netdev-offload-dpdk.c
>>> +++ b/lib/netdev-offload-dpdk.c
>>> @@ -1092,7 +1092,8 @@ netdev_offload_dpdk_validate_flow(const struct match 
>>> *match)
>>>  /* Create a wc-zeroed version of flow. */
>>>  match_init(_zero_wc, >flow, >wc);
>>>
>>> -if (!is_all_zeros(_zero_wc.flow.tunnel,
>>> +if (flow_tnl_dst_is_set(>flow.tunnel) &&
>>> +!is_all_zeros(_zero_wc.flow.tunnel,
>>>sizeof match_zero_wc.flow.tunnel)) {
>>>  goto err;
>>>  }
>>>
>>

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH branch-2.13] netdev-offload-dpdk: Support vxlan encap offload with load actions

2020-10-20 Thread Sriharsha Basavapatna via dev 
On Mon, Oct 19, 2020 at 7:59 PM Ilya Maximets  wrote:
>
> On 10/18/20 9:10 AM, Eli Britstein wrote:
> > Struct match has the tunnel values/masks in
> > match->flow.tunnel/match->wc.masks.tunnel.
> > Load actions such as load:0xa566c10->NXM_NX_TUN_IPV4_DST[],
> > load:0xbba->NXM_NX_TUN_ID[] are utilizing the tunnel masks fields, but
> > those should not be used for matching.
> > Offloading fails if masks is not clear. Fix it by checking if tunnel is
> > present.
> >
> > Signed-off-by: Eli Britstein 
> > ---
>
> Thanks, Eli.
>
> Harsha, Emma, could you, please, review/test this version?
>
> Best regards, Ilya Maximets.

Reviewed this change. Do we even need this backported to 2.13, since
we don't support clone/tnl-push actions with offload ?

Thanks,
-Harsha
>
> >  lib/netdev-offload-dpdk.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> > index 4538baf5e..c68d539ea 100644
> > --- a/lib/netdev-offload-dpdk.c
> > +++ b/lib/netdev-offload-dpdk.c
> > @@ -1092,7 +1092,8 @@ netdev_offload_dpdk_validate_flow(const struct match 
> > *match)
> >  /* Create a wc-zeroed version of flow. */
> >  match_init(_zero_wc, >flow, >wc);
> >
> > -if (!is_all_zeros(_zero_wc.flow.tunnel,
> > +if (flow_tnl_dst_is_set(>flow.tunnel) &&
> > +!is_all_zeros(_zero_wc.flow.tunnel,
> >sizeof match_zero_wc.flow.tunnel)) {
> >  goto err;
> >  }
> >
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH branch-2.13] netdev-offload-dpdk: Support vxlan encap offload with load actions

2020-10-20 Thread Finn, Emma 



> -Original Message-
> From: Sriharsha Basavapatna 
> Sent: Tuesday 20 October 2020 04:17
> To: Ilya Maximets 
> Cc: Eli Britstein ; ovs dev ; Finn,
> Emma ; Stokes, Ian ; Sriharsha
> Basavapatna 
> Subject: Re: [PATCH branch-2.13] netdev-offload-dpdk: Support vxlan encap
> offload with load actions
> 
> On Mon, Oct 19, 2020 at 7:59 PM Ilya Maximets  wrote:
> >
> > On 10/18/20 9:10 AM, Eli Britstein wrote:
> > > Struct match has the tunnel values/masks in
> > > match->flow.tunnel/match->wc.masks.tunnel.
> > > Load actions such as load:0xa566c10->NXM_NX_TUN_IPV4_DST[],
> > > load:0xbba->NXM_NX_TUN_ID[] are utilizing the tunnel masks fields, but
> > > those should not be used for matching.
> > > Offloading fails if masks is not clear. Fix it by checking if tunnel is
> > > present.
> > >
> > > Signed-off-by: Eli Britstein 
> > > ---
> >
> > Thanks, Eli.
> >
> > Harsha, Emma, could you, please, review/test this version?
> >
> > Best regards, Ilya Maximets.
> 
> Ilya, I'll review/test this today.
> Thanks,
> -Harsha

Tested-by: Emma Finn 

> >
> > >  lib/netdev-offload-dpdk.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> > > index 4538baf5e..c68d539ea 100644
> > > --- a/lib/netdev-offload-dpdk.c
> > > +++ b/lib/netdev-offload-dpdk.c
> > > @@ -1092,7 +1092,8 @@ netdev_offload_dpdk_validate_flow(const struct
> match *match)
> > >  /* Create a wc-zeroed version of flow. */
> > >  match_init(_zero_wc, >flow, >wc);
> > >
> > > -if (!is_all_zeros(_zero_wc.flow.tunnel,
> > > +if (flow_tnl_dst_is_set(>flow.tunnel) &&
> > > +!is_all_zeros(_zero_wc.flow.tunnel,
> > >sizeof match_zero_wc.flow.tunnel)) {
> > >  goto err;
> > >  }
> > >
> >
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH branch-2.13] netdev-offload-dpdk: Support vxlan encap offload with load actions

2020-10-19 Thread Sriharsha Basavapatna via dev 
On Mon, Oct 19, 2020 at 7:59 PM Ilya Maximets  wrote:
>
> On 10/18/20 9:10 AM, Eli Britstein wrote:
> > Struct match has the tunnel values/masks in
> > match->flow.tunnel/match->wc.masks.tunnel.
> > Load actions such as load:0xa566c10->NXM_NX_TUN_IPV4_DST[],
> > load:0xbba->NXM_NX_TUN_ID[] are utilizing the tunnel masks fields, but
> > those should not be used for matching.
> > Offloading fails if masks is not clear. Fix it by checking if tunnel is
> > present.
> >
> > Signed-off-by: Eli Britstein 
> > ---
>
> Thanks, Eli.
>
> Harsha, Emma, could you, please, review/test this version?
>
> Best regards, Ilya Maximets.

Ilya, I'll review/test this today.
Thanks,
-Harsha
>
> >  lib/netdev-offload-dpdk.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> > index 4538baf5e..c68d539ea 100644
> > --- a/lib/netdev-offload-dpdk.c
> > +++ b/lib/netdev-offload-dpdk.c
> > @@ -1092,7 +1092,8 @@ netdev_offload_dpdk_validate_flow(const struct match 
> > *match)
> >  /* Create a wc-zeroed version of flow. */
> >  match_init(_zero_wc, >flow, >wc);
> >
> > -if (!is_all_zeros(_zero_wc.flow.tunnel,
> > +if (flow_tnl_dst_is_set(>flow.tunnel) &&
> > +!is_all_zeros(_zero_wc.flow.tunnel,
> >sizeof match_zero_wc.flow.tunnel)) {
> >  goto err;
> >  }
> >
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH branch-2.13] netdev-offload-dpdk: Support vxlan encap offload with load actions

2020-10-19 Thread Ilya Maximets 
On 10/18/20 9:10 AM, Eli Britstein wrote:
> Struct match has the tunnel values/masks in
> match->flow.tunnel/match->wc.masks.tunnel.
> Load actions such as load:0xa566c10->NXM_NX_TUN_IPV4_DST[],
> load:0xbba->NXM_NX_TUN_ID[] are utilizing the tunnel masks fields, but
> those should not be used for matching.
> Offloading fails if masks is not clear. Fix it by checking if tunnel is
> present.
> 
> Signed-off-by: Eli Britstein 
> ---

Thanks, Eli.

Harsha, Emma, could you, please, review/test this version?

Best regards, Ilya Maximets.

>  lib/netdev-offload-dpdk.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> index 4538baf5e..c68d539ea 100644
> --- a/lib/netdev-offload-dpdk.c
> +++ b/lib/netdev-offload-dpdk.c
> @@ -1092,7 +1092,8 @@ netdev_offload_dpdk_validate_flow(const struct match 
> *match)
>  /* Create a wc-zeroed version of flow. */
>  match_init(_zero_wc, >flow, >wc);
>  
> -if (!is_all_zeros(_zero_wc.flow.tunnel,
> +if (flow_tnl_dst_is_set(>flow.tunnel) &&
> +!is_all_zeros(_zero_wc.flow.tunnel,
>sizeof match_zero_wc.flow.tunnel)) {
>  goto err;
>  }
> 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH branch-2.13] netdev-offload-dpdk: Support vxlan encap offload with load actions

2020-10-18 Thread Eli Britstein 
Struct match has the tunnel values/masks in
match->flow.tunnel/match->wc.masks.tunnel.
Load actions such as load:0xa566c10->NXM_NX_TUN_IPV4_DST[],
load:0xbba->NXM_NX_TUN_ID[] are utilizing the tunnel masks fields, but
those should not be used for matching.
Offloading fails if masks is not clear. Fix it by checking if tunnel is
present.

Signed-off-by: Eli Britstein 
---
 lib/netdev-offload-dpdk.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 4538baf5e..c68d539ea 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -1092,7 +1092,8 @@ netdev_offload_dpdk_validate_flow(const struct match 
*match)
 /* Create a wc-zeroed version of flow. */
 match_init(_zero_wc, >flow, >wc);
 
-if (!is_all_zeros(_zero_wc.flow.tunnel,
+if (flow_tnl_dst_is_set(>flow.tunnel) &&
+!is_all_zeros(_zero_wc.flow.tunnel,
   sizeof match_zero_wc.flow.tunnel)) {
 goto err;
 }
-- 
2.26.2.1730.g385c171

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev