diff mbox series

[ovs-dev] netdev-offload-tc: Fix requesting match on wildcarded vlan tpid.

Message ID 20190619091642.30070-1-i.maximets@samsung.com
State Accepted
Headers show
Series [ovs-dev] netdev-offload-tc: Fix requesting match on wildcarded vlan tpid. | expand

Commit Message

Ilya Maximets June 19, 2019, 9:16 a.m. UTC
'mask' must be checked first before configuring key in flower.

CC: Eli Britstein <elibr@mellanox.com>
Fixes: 0b0a84783cd6 ("netdev-tc-offloads: Support match on priority tags")
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
---
 lib/netdev-offload-tc.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Roi Dayan June 19, 2019, 5:34 p.m. UTC | #1
On 2019-06-19 12:16 PM, Ilya Maximets wrote:
> 'mask' must be checked first before configuring key in flower.
> 
> CC: Eli Britstein <elibr@mellanox.com>
> Fixes: 0b0a84783cd6 ("netdev-tc-offloads: Support match on priority tags")
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> ---
>  lib/netdev-offload-tc.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
> index 902ec9543..4cc044b4b 100644
> --- a/lib/netdev-offload-tc.c
> +++ b/lib/netdev-offload-tc.c
> @@ -1146,9 +1146,11 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match,
>      }
>      mask->mpls_lse[0] = 0;
>  
> -    if (eth_type_vlan(key->vlans[0].tpid)) {
> +    if (mask->vlans[0].tpid && eth_type_vlan(key->vlans[0].tpid)) {
>          flower.key.encap_eth_type[0] = flower.key.eth_type;
> +        flower.mask.encap_eth_type[0] = flower.mask.eth_type;
>          flower.key.eth_type = key->vlans[0].tpid;
> +        flower.mask.eth_type = mask->vlans[0].tpid;
>      }
>      if (mask->vlans[0].tci) {
>          ovs_be16 vid_mask = mask->vlans[0].tci & htons(VLAN_VID_MASK);
> @@ -1179,9 +1181,11 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match,
>          }
>      }
>  
> -    if (eth_type_vlan(key->vlans[1].tpid)) {
> +    if (mask->vlans[1].tpid && eth_type_vlan(key->vlans[1].tpid)) {
>          flower.key.encap_eth_type[1] = flower.key.encap_eth_type[0];
> +        flower.mask.encap_eth_type[1] = flower.mask.encap_eth_type[0];
>          flower.key.encap_eth_type[0] = key->vlans[1].tpid;
> +        flower.mask.encap_eth_type[0] = mask->vlans[1].tpid;
>      }
>      if (mask->vlans[1].tci) {
>          ovs_be16 vid_mask = mask->vlans[1].tci & htons(VLAN_VID_MASK);
> 

Acked-by: Roi Dayan <roid@mellanox.com>
Ilya Maximets July 1, 2019, 11:21 a.m. UTC | #2
Hi, Eli.
Did you have a chance to test this?

Best regards, Ilya Maximets.

On 19.06.2019 12:16, Ilya Maximets wrote:
> 'mask' must be checked first before configuring key in flower.
> 
> CC: Eli Britstein <elibr@mellanox.com>
> Fixes: 0b0a84783cd6 ("netdev-tc-offloads: Support match on priority tags")
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> ---
>  lib/netdev-offload-tc.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
> index 902ec9543..4cc044b4b 100644
> --- a/lib/netdev-offload-tc.c
> +++ b/lib/netdev-offload-tc.c
> @@ -1146,9 +1146,11 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match,
>      }
>      mask->mpls_lse[0] = 0;
>  
> -    if (eth_type_vlan(key->vlans[0].tpid)) {
> +    if (mask->vlans[0].tpid && eth_type_vlan(key->vlans[0].tpid)) {
>          flower.key.encap_eth_type[0] = flower.key.eth_type;
> +        flower.mask.encap_eth_type[0] = flower.mask.eth_type;
>          flower.key.eth_type = key->vlans[0].tpid;
> +        flower.mask.eth_type = mask->vlans[0].tpid;
>      }
>      if (mask->vlans[0].tci) {
>          ovs_be16 vid_mask = mask->vlans[0].tci & htons(VLAN_VID_MASK);
> @@ -1179,9 +1181,11 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match,
>          }
>      }
>  
> -    if (eth_type_vlan(key->vlans[1].tpid)) {
> +    if (mask->vlans[1].tpid && eth_type_vlan(key->vlans[1].tpid)) {
>          flower.key.encap_eth_type[1] = flower.key.encap_eth_type[0];
> +        flower.mask.encap_eth_type[1] = flower.mask.encap_eth_type[0];
>          flower.key.encap_eth_type[0] = key->vlans[1].tpid;
> +        flower.mask.encap_eth_type[0] = mask->vlans[1].tpid;
>      }
>      if (mask->vlans[1].tci) {
>          ovs_be16 vid_mask = mask->vlans[1].tci & htons(VLAN_VID_MASK);
>
Eli Britstein July 1, 2019, 6:53 p.m. UTC | #3
On 7/1/2019 2:21 PM, Ilya Maximets wrote:
> Hi, Eli.
> Did you have a chance to test this?

Yes, sorry for the delay. It works fine (though didn't test QinQ. only 
native/single-tagged).

Reviewed-by: Eli Britstein <elibr@mellanox.com>


> Best regards, Ilya Maximets.
>
> On 19.06.2019 12:16, Ilya Maximets wrote:
>> 'mask' must be checked first before configuring key in flower.
>>
>> CC: Eli Britstein <elibr@mellanox.com>
>> Fixes: 0b0a84783cd6 ("netdev-tc-offloads: Support match on priority tags")
>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
>> ---
>>   lib/netdev-offload-tc.c | 8 ++++++--
>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
>> index 902ec9543..4cc044b4b 100644
>> --- a/lib/netdev-offload-tc.c
>> +++ b/lib/netdev-offload-tc.c
>> @@ -1146,9 +1146,11 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match,
>>       }
>>       mask->mpls_lse[0] = 0;
>>   
>> -    if (eth_type_vlan(key->vlans[0].tpid)) {
>> +    if (mask->vlans[0].tpid && eth_type_vlan(key->vlans[0].tpid)) {
>>           flower.key.encap_eth_type[0] = flower.key.eth_type;
>> +        flower.mask.encap_eth_type[0] = flower.mask.eth_type;
>>           flower.key.eth_type = key->vlans[0].tpid;
>> +        flower.mask.eth_type = mask->vlans[0].tpid;
>>       }
>>       if (mask->vlans[0].tci) {
>>           ovs_be16 vid_mask = mask->vlans[0].tci & htons(VLAN_VID_MASK);
>> @@ -1179,9 +1181,11 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match,
>>           }
>>       }
>>   
>> -    if (eth_type_vlan(key->vlans[1].tpid)) {
>> +    if (mask->vlans[1].tpid && eth_type_vlan(key->vlans[1].tpid)) {
>>           flower.key.encap_eth_type[1] = flower.key.encap_eth_type[0];
>> +        flower.mask.encap_eth_type[1] = flower.mask.encap_eth_type[0];
>>           flower.key.encap_eth_type[0] = key->vlans[1].tpid;
>> +        flower.mask.encap_eth_type[0] = mask->vlans[1].tpid;
>>       }
>>       if (mask->vlans[1].tci) {
>>           ovs_be16 vid_mask = mask->vlans[1].tci & htons(VLAN_VID_MASK);
>>
Ilya Maximets July 2, 2019, 9:23 a.m. UTC | #4
On 19.06.2019 12:16, Ilya Maximets wrote:
> 'mask' must be checked first before configuring key in flower.
> 
> CC: Eli Britstein <elibr@mellanox.com>
> Fixes: 0b0a84783cd6 ("netdev-tc-offloads: Support match on priority tags")
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> ---
>  lib/netdev-offload-tc.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)

Thanks, Roi and Eli! Applied to master.

Best regards, Ilya Maximets.
diff mbox series

Patch

diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index 902ec9543..4cc044b4b 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -1146,9 +1146,11 @@  netdev_tc_flow_put(struct netdev *netdev, struct match *match,
     }
     mask->mpls_lse[0] = 0;
 
-    if (eth_type_vlan(key->vlans[0].tpid)) {
+    if (mask->vlans[0].tpid && eth_type_vlan(key->vlans[0].tpid)) {
         flower.key.encap_eth_type[0] = flower.key.eth_type;
+        flower.mask.encap_eth_type[0] = flower.mask.eth_type;
         flower.key.eth_type = key->vlans[0].tpid;
+        flower.mask.eth_type = mask->vlans[0].tpid;
     }
     if (mask->vlans[0].tci) {
         ovs_be16 vid_mask = mask->vlans[0].tci & htons(VLAN_VID_MASK);
@@ -1179,9 +1181,11 @@  netdev_tc_flow_put(struct netdev *netdev, struct match *match,
         }
     }
 
-    if (eth_type_vlan(key->vlans[1].tpid)) {
+    if (mask->vlans[1].tpid && eth_type_vlan(key->vlans[1].tpid)) {
         flower.key.encap_eth_type[1] = flower.key.encap_eth_type[0];
+        flower.mask.encap_eth_type[1] = flower.mask.encap_eth_type[0];
         flower.key.encap_eth_type[0] = key->vlans[1].tpid;
+        flower.mask.encap_eth_type[0] = mask->vlans[1].tpid;
     }
     if (mask->vlans[1].tci) {
         ovs_be16 vid_mask = mask->vlans[1].tci & htons(VLAN_VID_MASK);