[ovs-dev] netdev-tc-offloads: Fix offloading when tunnel id is 0

From: Paul Blakey <paulb@mellanox.com>

Currently we test the tunnel id value and not the mask so matching on
tunnel id 0 is skipped and we fail to offload the rule with an
unknown attribute error.

Fix this by testing the mask of tunnel id instead.

Fixes: 8f283af89298 ("netdev-tc-offloads: Implement netdev flow put using tc interface")
Signed-off-by: Paul Blakey <paulb@mellanox.com>
Reviewed-by: Roi Dayan <roid@mellanox.com>
---
 lib/netdev-tc-offloads.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

On 29/08/2017 07:29, Roi Dayan wrote:
> From: Paul Blakey <paulb@mellanox.com>
> 
> Currently we test the tunnel id value and not the mask so matching on
> tunnel id 0 is skipped and we fail to offload the rule with an
> unknown attribute error.
> 
> Fix this by testing the mask of tunnel id instead.
> 
> Fixes: 8f283af89298 ("netdev-tc-offloads: Implement netdev flow put using tc interface")
> Signed-off-by: Paul Blakey <paulb@mellanox.com>
> Reviewed-by: Roi Dayan <roid@mellanox.com>
> ---
>   lib/netdev-tc-offloads.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
> index 099c021..ba7a873 100644
> --- a/lib/netdev-tc-offloads.c
> +++ b/lib/netdev-tc-offloads.c
> @@ -678,7 +678,7 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match,
>   
>       memset(&flower, 0, sizeof flower);
>   
> -    if (tnl->tun_id) {
> +    if (mask->tunnel.tun_id) {
>           VLOG_DBG_RL(&rl,
>                       "tunnel: id %#" PRIx64 " src " IP_FMT
>                       " dst " IP_FMT " tp_src %d tp_dst %d",
> 

hi,

just pinging

thanks

On Tue, Aug 29, 2017 at 07:29:57AM +0300, Roi Dayan wrote:
> From: Paul Blakey <paulb@mellanox.com>
> 
> Currently we test the tunnel id value and not the mask so matching on
> tunnel id 0 is skipped and we fail to offload the rule with an
> unknown attribute error.
> 
> Fix this by testing the mask of tunnel id instead.
> 
> Fixes: 8f283af89298 ("netdev-tc-offloads: Implement netdev flow put using tc interface")
> Signed-off-by: Paul Blakey <paulb@mellanox.com>
> Reviewed-by: Roi Dayan <roid@mellanox.com>

Acked-by: Simon Horman <simon.horman@netronome.com>

> ---
>  lib/netdev-tc-offloads.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
> index 099c021..ba7a873 100644
> --- a/lib/netdev-tc-offloads.c
> +++ b/lib/netdev-tc-offloads.c
> @@ -678,7 +678,7 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match,
>  
>      memset(&flower, 0, sizeof flower);
>  
> -    if (tnl->tun_id) {
> +    if (mask->tunnel.tun_id) {
>          VLOG_DBG_RL(&rl,
>                      "tunnel: id %#" PRIx64 " src " IP_FMT
>                      " dst " IP_FMT " tp_src %d tp_dst %d",
> -- 
> 2.7.0
>

On 07/09/2017 19:15, Simon Horman wrote:
> On Tue, Aug 29, 2017 at 07:29:57AM +0300, Roi Dayan wrote:
>> From: Paul Blakey <paulb@mellanox.com>
>>
>> Currently we test the tunnel id value and not the mask so matching on
>> tunnel id 0 is skipped and we fail to offload the rule with an
>> unknown attribute error.
>>
>> Fix this by testing the mask of tunnel id instead.
>>
>> Fixes: 8f283af89298 ("netdev-tc-offloads: Implement netdev flow put using tc interface")
>> Signed-off-by: Paul Blakey <paulb@mellanox.com>
>> Reviewed-by: Roi Dayan <roid@mellanox.com>
> 
> Acked-by: Simon Horman <simon.horman@netronome.com>

Thanks for the Ack.
We found this change by itself is not good enough when working
with vxlan setup and key=flow.
Since this commit is not merged we are doing a new commit instead of 
this one.

Thanks,
Roi

> 
>> ---
>>   lib/netdev-tc-offloads.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
>> index 099c021..ba7a873 100644
>> --- a/lib/netdev-tc-offloads.c
>> +++ b/lib/netdev-tc-offloads.c
>> @@ -678,7 +678,7 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match,
>>   
>>       memset(&flower, 0, sizeof flower);
>>   
>> -    if (tnl->tun_id) {
>> +    if (mask->tunnel.tun_id) {
>>           VLOG_DBG_RL(&rl,
>>                       "tunnel: id %#" PRIx64 " src " IP_FMT
>>                       " dst " IP_FMT " tp_src %d tp_dst %d",
>> -- 
>> 2.7.0
>>