diff mbox series

[ovs-dev,v2] tc: Changes to netlink message population for OVS-TC Flower Offload

Message ID 20200619184437.20323-1-satish.d@oneconvergence.com
State New
Headers show
Series [ovs-dev,v2] tc: Changes to netlink message population for OVS-TC Flower Offload | expand

Commit Message

Satish June 19, 2020, 6:44 p.m. UTC
From: Satish Dhote <satish.d@oneconvergence.com>

OVS-TC offload sends only fields that are not completely masked out
to the TC kernel module. Some of the hardware we work with requires
all fields extracted from the packet along with the mask to be sent
to the driver. Hardware may optimize tables based on entire fields
of packet and mask of fields, even though certain fields are masked
out.

OVS when offloading to ovs kernel datapath sends the entire key
along with the mask. Proposal requests for similar behaviour in
ovs-vswitchd in case of tc-offload. While constructing netlink
messages, ovs-vswitchd is removing completely masked out fields.
Sending additional masked fields should not impact existing drivers.
TC internally uses dissectors to extract non masked fields, so it
should not impact existing drivers in tc.

Example:
ovs-rule:ovs-ofctl add-flow br0 \
    "table=0, dl_dst=00:a1:45:23:23:11/ff:ff:ff:ff:ff:ff, actions=1"

If we have above rule in ovs and an icmp packet matching destination
MAC 00:a1:45:23:23:11 arrives, ovs prepares the key and mask with all
relevant fields. When sending to TC it removes fields which are
completely masked out. In this case ethertype, and dst_mac fields are
sent as part of netlink message to tc.

OVS patch also requires minor fixes in TC. Patch to TC is submitted
at following link. 1/3 patch has the fix for the same.
http://patchwork.ozlabs.org/project/netdev/list/?series=184493

Co-authored-by: Chandra Kesava <kesavac@gmail.com>
Co-authored-by: Prathibha Nagooru <prathibha.nagooru@oneconvergence.com>
Signed-off-by: Chandra Kesava <kesavac@gmail.com>
Signed-off-by: Prathibha Nagooru <prathibha.nagooru@oneconvergence.com>
Signed-off-by: Satish Dhote <satish.d@oneconvergence.com>
---
v2:
- Corrected From author name.
- Added missing co-author detail.
- Deleted unused variable.
---
 lib/tc.c | 111 +++++++++++++++++++++----------------------------------
 1 file changed, 42 insertions(+), 69 deletions(-)

Comments

Satish June 30, 2020, 4:17 a.m. UTC | #1
Hi Simon,

We decided to withdraw this patch for the time being. We'll resubmit this patch
along with the linux net-next patch(with driver code) in future".

On Sat, Jun 20, 2020 at 12:17 AM dsatish <satish.d@oneconvergence.com> wrote:
>
> From: Satish Dhote <satish.d@oneconvergence.com>
>
> OVS-TC offload sends only fields that are not completely masked out
> to the TC kernel module. Some of the hardware we work with requires
> all fields extracted from the packet along with the mask to be sent
> to the driver. Hardware may optimize tables based on entire fields
> of packet and mask of fields, even though certain fields are masked
> out.
>
> OVS when offloading to ovs kernel datapath sends the entire key
> along with the mask. Proposal requests for similar behaviour in
> ovs-vswitchd in case of tc-offload. While constructing netlink
> messages, ovs-vswitchd is removing completely masked out fields.
> Sending additional masked fields should not impact existing drivers.
> TC internally uses dissectors to extract non masked fields, so it
> should not impact existing drivers in tc.
>
> Example:
> ovs-rule:ovs-ofctl add-flow br0 \
>     "table=0, dl_dst=00:a1:45:23:23:11/ff:ff:ff:ff:ff:ff, actions=1"
>
> If we have above rule in ovs and an icmp packet matching destination
> MAC 00:a1:45:23:23:11 arrives, ovs prepares the key and mask with all
> relevant fields. When sending to TC it removes fields which are
> completely masked out. In this case ethertype, and dst_mac fields are
> sent as part of netlink message to tc.
>
> OVS patch also requires minor fixes in TC. Patch to TC is submitted
> at following link. 1/3 patch has the fix for the same.
> http://patchwork.ozlabs.org/project/netdev/list/?series=184493
>
> Co-authored-by: Chandra Kesava <kesavac@gmail.com>
> Co-authored-by: Prathibha Nagooru <prathibha.nagooru@oneconvergence.com>
> Signed-off-by: Chandra Kesava <kesavac@gmail.com>
> Signed-off-by: Prathibha Nagooru <prathibha.nagooru@oneconvergence.com>
> Signed-off-by: Satish Dhote <satish.d@oneconvergence.com>
> ---
> v2:
> - Corrected From author name.
> - Added missing co-author detail.
> - Deleted unused variable.
> ---
>  lib/tc.c | 111 +++++++++++++++++++++----------------------------------
>  1 file changed, 42 insertions(+), 69 deletions(-)
>
> diff --git a/lib/tc.c b/lib/tc.c
> index c2ab77553..4d0232120 100644
> --- a/lib/tc.c
> +++ b/lib/tc.c
> @@ -2648,9 +2648,6 @@ nl_msg_put_masked_value(struct ofpbuf *request, uint16_t type,
>                          const void *mask_data, size_t len)
>  {
>      if (mask_type != TCA_FLOWER_UNSPEC) {
> -        if (is_all_zeros(mask_data, len)) {
> -            return;
> -        }
>          nl_msg_put_unspec(request, mask_type, mask_data, len);
>      }
>      nl_msg_put_unspec(request, type, data, len);
> @@ -2705,17 +2702,16 @@ nl_msg_put_flower_tunnel(struct ofpbuf *request, struct tc_flower *flower)
>      uint8_t ttl = flower->key.tunnel.ttl;
>      uint8_t tos_mask = flower->mask.tunnel.tos;
>      uint8_t ttl_mask = flower->mask.tunnel.ttl;
> -    ovs_be64 id_mask = flower->mask.tunnel.id;
>
> -    if (ipv4_dst_mask || ipv4_src_mask) {
> +    if (ipv4_dst || ipv4_src) {
>          nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_IPV4_DST_MASK,
>                          ipv4_dst_mask);
>          nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_IPV4_SRC_MASK,
>                          ipv4_src_mask);
>          nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_IPV4_DST, ipv4_dst);
>          nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_IPV4_SRC, ipv4_src);
> -    } else if (ipv6_addr_is_set(ipv6_dst_mask) ||
> -               ipv6_addr_is_set(ipv6_src_mask)) {
> +    } else if (ipv6_addr_is_set(ipv6_dst) ||
> +               ipv6_addr_is_set(ipv6_src)) {
>          nl_msg_put_in6_addr(request, TCA_FLOWER_KEY_ENC_IPV6_DST_MASK,
>                              ipv6_dst_mask);
>          nl_msg_put_in6_addr(request, TCA_FLOWER_KEY_ENC_IPV6_SRC_MASK,
> @@ -2723,20 +2719,18 @@ nl_msg_put_flower_tunnel(struct ofpbuf *request, struct tc_flower *flower)
>          nl_msg_put_in6_addr(request, TCA_FLOWER_KEY_ENC_IPV6_DST, ipv6_dst);
>          nl_msg_put_in6_addr(request, TCA_FLOWER_KEY_ENC_IPV6_SRC, ipv6_src);
>      }
> -    if (tos_mask) {
> -        nl_msg_put_u8(request, TCA_FLOWER_KEY_ENC_IP_TOS, tos);
> -        nl_msg_put_u8(request, TCA_FLOWER_KEY_ENC_IP_TOS_MASK, tos_mask);
> -    }
> -    if (ttl_mask) {
> -        nl_msg_put_u8(request, TCA_FLOWER_KEY_ENC_IP_TTL, ttl);
> -        nl_msg_put_u8(request, TCA_FLOWER_KEY_ENC_IP_TTL_MASK, ttl_mask);
> -    }
> +
> +    nl_msg_put_u8(request, TCA_FLOWER_KEY_ENC_IP_TOS, tos);
> +    nl_msg_put_u8(request, TCA_FLOWER_KEY_ENC_IP_TOS_MASK, tos_mask);
> +
> +    nl_msg_put_u8(request, TCA_FLOWER_KEY_ENC_IP_TTL, ttl);
> +    nl_msg_put_u8(request, TCA_FLOWER_KEY_ENC_IP_TTL_MASK, ttl_mask);
> +
>      if (tp_dst) {
>          nl_msg_put_be16(request, TCA_FLOWER_KEY_ENC_UDP_DST_PORT, tp_dst);
>      }
> -    if (id_mask) {
> -        nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_KEY_ID, id);
> -    }
> +
> +    nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_KEY_ID, id);
>      nl_msg_put_flower_tunnel_opts(request, TCA_FLOWER_KEY_ENC_OPTS,
>                                    flower->key.tunnel.metadata);
>      nl_msg_put_flower_tunnel_opts(request, TCA_FLOWER_KEY_ENC_OPTS_MASK,
> @@ -2796,12 +2790,10 @@ nl_msg_put_flower_options(struct ofpbuf *request, struct tc_flower *flower)
>                            flower->key.ip_proto);
>          }
>
> -        if (flower->mask.flags) {
> -            nl_msg_put_be32(request, TCA_FLOWER_KEY_FLAGS,
> -                           htonl(flower->key.flags));
> -            nl_msg_put_be32(request, TCA_FLOWER_KEY_FLAGS_MASK,
> -                           htonl(flower->mask.flags));
> -        }
> +        nl_msg_put_be32(request, TCA_FLOWER_KEY_FLAGS,
> +                       htonl(flower->key.flags));
> +        nl_msg_put_be32(request, TCA_FLOWER_KEY_FLAGS_MASK,
> +                       htonl(flower->mask.flags));
>
>          if (flower->key.ip_proto == IPPROTO_UDP) {
>              FLOWER_PUT_MASKED_VALUE(udp_src, TCA_FLOWER_KEY_UDP_SRC);
> @@ -2815,10 +2807,12 @@ nl_msg_put_flower_options(struct ofpbuf *request, struct tc_flower *flower)
>              FLOWER_PUT_MASKED_VALUE(sctp_dst, TCA_FLOWER_KEY_SCTP_DST);
>          }
>
> -        FLOWER_PUT_MASKED_VALUE(ct_state, TCA_FLOWER_KEY_CT_STATE);
> -        FLOWER_PUT_MASKED_VALUE(ct_zone, TCA_FLOWER_KEY_CT_ZONE);
> -        FLOWER_PUT_MASKED_VALUE(ct_mark, TCA_FLOWER_KEY_CT_MARK);
> -        FLOWER_PUT_MASKED_VALUE(ct_label, TCA_FLOWER_KEY_CT_LABELS);
> +        if (flower->mask.ct_state) {
> +            FLOWER_PUT_MASKED_VALUE(ct_state, TCA_FLOWER_KEY_CT_STATE);
> +            FLOWER_PUT_MASKED_VALUE(ct_zone, TCA_FLOWER_KEY_CT_ZONE);
> +            FLOWER_PUT_MASKED_VALUE(ct_mark, TCA_FLOWER_KEY_CT_MARK);
> +            FLOWER_PUT_MASKED_VALUE(ct_label, TCA_FLOWER_KEY_CT_LABELS);
> +        }
>      }
>
>      if (host_eth_type == ETH_P_IP) {
> @@ -2832,51 +2826,30 @@ nl_msg_put_flower_options(struct ofpbuf *request, struct tc_flower *flower)
>      nl_msg_put_be16(request, TCA_FLOWER_KEY_ETH_TYPE, flower->key.eth_type);
>
>      if (is_mpls) {
> -        if (mpls_lse_to_ttl(flower->mask.mpls_lse)) {
> -            nl_msg_put_u8(request, TCA_FLOWER_KEY_MPLS_TTL,
> -                          mpls_lse_to_ttl(flower->key.mpls_lse));
> -        }
> -        if (mpls_lse_to_tc(flower->mask.mpls_lse)) {
> -            nl_msg_put_u8(request, TCA_FLOWER_KEY_MPLS_TC,
> -                          mpls_lse_to_tc(flower->key.mpls_lse));
> -        }
> -        if (mpls_lse_to_bos(flower->mask.mpls_lse)) {
> -            nl_msg_put_u8(request, TCA_FLOWER_KEY_MPLS_BOS,
> -                          mpls_lse_to_bos(flower->key.mpls_lse));
> -        }
> -        if (mpls_lse_to_label(flower->mask.mpls_lse)) {
> -            nl_msg_put_u32(request, TCA_FLOWER_KEY_MPLS_LABEL,
> -                           mpls_lse_to_label(flower->key.mpls_lse));
> -        }
> +        nl_msg_put_u8(request, TCA_FLOWER_KEY_MPLS_TTL,
> +                      mpls_lse_to_ttl(flower->key.mpls_lse));
> +        nl_msg_put_u8(request, TCA_FLOWER_KEY_MPLS_TC,
> +                      mpls_lse_to_tc(flower->key.mpls_lse));
> +        nl_msg_put_u8(request, TCA_FLOWER_KEY_MPLS_BOS,
> +                      mpls_lse_to_bos(flower->key.mpls_lse));
> +        nl_msg_put_u32(request, TCA_FLOWER_KEY_MPLS_LABEL,
> +                       mpls_lse_to_label(flower->key.mpls_lse));
>      }
>
>      if (is_vlan) {
> -        if (flower->mask.vlan_id[0]) {
> -            nl_msg_put_u16(request, TCA_FLOWER_KEY_VLAN_ID,
> -                           flower->key.vlan_id[0]);
> -        }
> -        if (flower->mask.vlan_prio[0]) {
> -            nl_msg_put_u8(request, TCA_FLOWER_KEY_VLAN_PRIO,
> -                          flower->key.vlan_prio[0]);
> -        }
> -        if (flower->key.encap_eth_type[0]) {
> -            nl_msg_put_be16(request, TCA_FLOWER_KEY_VLAN_ETH_TYPE,
> -                            flower->key.encap_eth_type[0]);
> -        }
> -
> +        nl_msg_put_u16(request, TCA_FLOWER_KEY_VLAN_ID,
> +                       flower->key.vlan_id[0]);
> +        nl_msg_put_u8(request, TCA_FLOWER_KEY_VLAN_PRIO,
> +                      flower->key.vlan_prio[0]);
> +        nl_msg_put_be16(request, TCA_FLOWER_KEY_VLAN_ETH_TYPE,
> +                        flower->key.encap_eth_type[0]);
>          if (is_qinq) {
> -            if (flower->mask.vlan_id[1]) {
> -                nl_msg_put_u16(request, TCA_FLOWER_KEY_CVLAN_ID,
> -                               flower->key.vlan_id[1]);
> -            }
> -            if (flower->mask.vlan_prio[1]) {
> -                nl_msg_put_u8(request, TCA_FLOWER_KEY_CVLAN_PRIO,
> -                              flower->key.vlan_prio[1]);
> -            }
> -            if (flower->key.encap_eth_type[1]) {
> -                nl_msg_put_be16(request, TCA_FLOWER_KEY_CVLAN_ETH_TYPE,
> -                                flower->key.encap_eth_type[1]);
> -            }
> +            nl_msg_put_u16(request, TCA_FLOWER_KEY_CVLAN_ID,
> +                           flower->key.vlan_id[1]);
> +            nl_msg_put_u8(request, TCA_FLOWER_KEY_CVLAN_PRIO,
> +                          flower->key.vlan_prio[1]);
> +            nl_msg_put_be16(request, TCA_FLOWER_KEY_CVLAN_ETH_TYPE,
> +                            flower->key.encap_eth_type[1]);
>          }
>      }
>
> --
> 2.17.1
>
diff mbox series

Patch

diff --git a/lib/tc.c b/lib/tc.c
index c2ab77553..4d0232120 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -2648,9 +2648,6 @@  nl_msg_put_masked_value(struct ofpbuf *request, uint16_t type,
                         const void *mask_data, size_t len)
 {
     if (mask_type != TCA_FLOWER_UNSPEC) {
-        if (is_all_zeros(mask_data, len)) {
-            return;
-        }
         nl_msg_put_unspec(request, mask_type, mask_data, len);
     }
     nl_msg_put_unspec(request, type, data, len);
@@ -2705,17 +2702,16 @@  nl_msg_put_flower_tunnel(struct ofpbuf *request, struct tc_flower *flower)
     uint8_t ttl = flower->key.tunnel.ttl;
     uint8_t tos_mask = flower->mask.tunnel.tos;
     uint8_t ttl_mask = flower->mask.tunnel.ttl;
-    ovs_be64 id_mask = flower->mask.tunnel.id;
 
-    if (ipv4_dst_mask || ipv4_src_mask) {
+    if (ipv4_dst || ipv4_src) {
         nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_IPV4_DST_MASK,
                         ipv4_dst_mask);
         nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_IPV4_SRC_MASK,
                         ipv4_src_mask);
         nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_IPV4_DST, ipv4_dst);
         nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_IPV4_SRC, ipv4_src);
-    } else if (ipv6_addr_is_set(ipv6_dst_mask) ||
-               ipv6_addr_is_set(ipv6_src_mask)) {
+    } else if (ipv6_addr_is_set(ipv6_dst) ||
+               ipv6_addr_is_set(ipv6_src)) {
         nl_msg_put_in6_addr(request, TCA_FLOWER_KEY_ENC_IPV6_DST_MASK,
                             ipv6_dst_mask);
         nl_msg_put_in6_addr(request, TCA_FLOWER_KEY_ENC_IPV6_SRC_MASK,
@@ -2723,20 +2719,18 @@  nl_msg_put_flower_tunnel(struct ofpbuf *request, struct tc_flower *flower)
         nl_msg_put_in6_addr(request, TCA_FLOWER_KEY_ENC_IPV6_DST, ipv6_dst);
         nl_msg_put_in6_addr(request, TCA_FLOWER_KEY_ENC_IPV6_SRC, ipv6_src);
     }
-    if (tos_mask) {
-        nl_msg_put_u8(request, TCA_FLOWER_KEY_ENC_IP_TOS, tos);
-        nl_msg_put_u8(request, TCA_FLOWER_KEY_ENC_IP_TOS_MASK, tos_mask);
-    }
-    if (ttl_mask) {
-        nl_msg_put_u8(request, TCA_FLOWER_KEY_ENC_IP_TTL, ttl);
-        nl_msg_put_u8(request, TCA_FLOWER_KEY_ENC_IP_TTL_MASK, ttl_mask);
-    }
+
+    nl_msg_put_u8(request, TCA_FLOWER_KEY_ENC_IP_TOS, tos);
+    nl_msg_put_u8(request, TCA_FLOWER_KEY_ENC_IP_TOS_MASK, tos_mask);
+
+    nl_msg_put_u8(request, TCA_FLOWER_KEY_ENC_IP_TTL, ttl);
+    nl_msg_put_u8(request, TCA_FLOWER_KEY_ENC_IP_TTL_MASK, ttl_mask);
+
     if (tp_dst) {
         nl_msg_put_be16(request, TCA_FLOWER_KEY_ENC_UDP_DST_PORT, tp_dst);
     }
-    if (id_mask) {
-        nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_KEY_ID, id);
-    }
+
+    nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_KEY_ID, id);
     nl_msg_put_flower_tunnel_opts(request, TCA_FLOWER_KEY_ENC_OPTS,
                                   flower->key.tunnel.metadata);
     nl_msg_put_flower_tunnel_opts(request, TCA_FLOWER_KEY_ENC_OPTS_MASK,
@@ -2796,12 +2790,10 @@  nl_msg_put_flower_options(struct ofpbuf *request, struct tc_flower *flower)
                           flower->key.ip_proto);
         }
 
-        if (flower->mask.flags) {
-            nl_msg_put_be32(request, TCA_FLOWER_KEY_FLAGS,
-                           htonl(flower->key.flags));
-            nl_msg_put_be32(request, TCA_FLOWER_KEY_FLAGS_MASK,
-                           htonl(flower->mask.flags));
-        }
+        nl_msg_put_be32(request, TCA_FLOWER_KEY_FLAGS,
+                       htonl(flower->key.flags));
+        nl_msg_put_be32(request, TCA_FLOWER_KEY_FLAGS_MASK,
+                       htonl(flower->mask.flags));
 
         if (flower->key.ip_proto == IPPROTO_UDP) {
             FLOWER_PUT_MASKED_VALUE(udp_src, TCA_FLOWER_KEY_UDP_SRC);
@@ -2815,10 +2807,12 @@  nl_msg_put_flower_options(struct ofpbuf *request, struct tc_flower *flower)
             FLOWER_PUT_MASKED_VALUE(sctp_dst, TCA_FLOWER_KEY_SCTP_DST);
         }
 
-        FLOWER_PUT_MASKED_VALUE(ct_state, TCA_FLOWER_KEY_CT_STATE);
-        FLOWER_PUT_MASKED_VALUE(ct_zone, TCA_FLOWER_KEY_CT_ZONE);
-        FLOWER_PUT_MASKED_VALUE(ct_mark, TCA_FLOWER_KEY_CT_MARK);
-        FLOWER_PUT_MASKED_VALUE(ct_label, TCA_FLOWER_KEY_CT_LABELS);
+        if (flower->mask.ct_state) {
+            FLOWER_PUT_MASKED_VALUE(ct_state, TCA_FLOWER_KEY_CT_STATE);
+            FLOWER_PUT_MASKED_VALUE(ct_zone, TCA_FLOWER_KEY_CT_ZONE);
+            FLOWER_PUT_MASKED_VALUE(ct_mark, TCA_FLOWER_KEY_CT_MARK);
+            FLOWER_PUT_MASKED_VALUE(ct_label, TCA_FLOWER_KEY_CT_LABELS);
+        }
     }
 
     if (host_eth_type == ETH_P_IP) {
@@ -2832,51 +2826,30 @@  nl_msg_put_flower_options(struct ofpbuf *request, struct tc_flower *flower)
     nl_msg_put_be16(request, TCA_FLOWER_KEY_ETH_TYPE, flower->key.eth_type);
 
     if (is_mpls) {
-        if (mpls_lse_to_ttl(flower->mask.mpls_lse)) {
-            nl_msg_put_u8(request, TCA_FLOWER_KEY_MPLS_TTL,
-                          mpls_lse_to_ttl(flower->key.mpls_lse));
-        }
-        if (mpls_lse_to_tc(flower->mask.mpls_lse)) {
-            nl_msg_put_u8(request, TCA_FLOWER_KEY_MPLS_TC,
-                          mpls_lse_to_tc(flower->key.mpls_lse));
-        }
-        if (mpls_lse_to_bos(flower->mask.mpls_lse)) {
-            nl_msg_put_u8(request, TCA_FLOWER_KEY_MPLS_BOS,
-                          mpls_lse_to_bos(flower->key.mpls_lse));
-        }
-        if (mpls_lse_to_label(flower->mask.mpls_lse)) {
-            nl_msg_put_u32(request, TCA_FLOWER_KEY_MPLS_LABEL,
-                           mpls_lse_to_label(flower->key.mpls_lse));
-        }
+        nl_msg_put_u8(request, TCA_FLOWER_KEY_MPLS_TTL,
+                      mpls_lse_to_ttl(flower->key.mpls_lse));
+        nl_msg_put_u8(request, TCA_FLOWER_KEY_MPLS_TC,
+                      mpls_lse_to_tc(flower->key.mpls_lse));
+        nl_msg_put_u8(request, TCA_FLOWER_KEY_MPLS_BOS,
+                      mpls_lse_to_bos(flower->key.mpls_lse));
+        nl_msg_put_u32(request, TCA_FLOWER_KEY_MPLS_LABEL,
+                       mpls_lse_to_label(flower->key.mpls_lse));
     }
 
     if (is_vlan) {
-        if (flower->mask.vlan_id[0]) {
-            nl_msg_put_u16(request, TCA_FLOWER_KEY_VLAN_ID,
-                           flower->key.vlan_id[0]);
-        }
-        if (flower->mask.vlan_prio[0]) {
-            nl_msg_put_u8(request, TCA_FLOWER_KEY_VLAN_PRIO,
-                          flower->key.vlan_prio[0]);
-        }
-        if (flower->key.encap_eth_type[0]) {
-            nl_msg_put_be16(request, TCA_FLOWER_KEY_VLAN_ETH_TYPE,
-                            flower->key.encap_eth_type[0]);
-        }
-
+        nl_msg_put_u16(request, TCA_FLOWER_KEY_VLAN_ID,
+                       flower->key.vlan_id[0]);
+        nl_msg_put_u8(request, TCA_FLOWER_KEY_VLAN_PRIO,
+                      flower->key.vlan_prio[0]);
+        nl_msg_put_be16(request, TCA_FLOWER_KEY_VLAN_ETH_TYPE,
+                        flower->key.encap_eth_type[0]);
         if (is_qinq) {
-            if (flower->mask.vlan_id[1]) {
-                nl_msg_put_u16(request, TCA_FLOWER_KEY_CVLAN_ID,
-                               flower->key.vlan_id[1]);
-            }
-            if (flower->mask.vlan_prio[1]) {
-                nl_msg_put_u8(request, TCA_FLOWER_KEY_CVLAN_PRIO,
-                              flower->key.vlan_prio[1]);
-            }
-            if (flower->key.encap_eth_type[1]) {
-                nl_msg_put_be16(request, TCA_FLOWER_KEY_CVLAN_ETH_TYPE,
-                                flower->key.encap_eth_type[1]);
-            }
+            nl_msg_put_u16(request, TCA_FLOWER_KEY_CVLAN_ID,
+                           flower->key.vlan_id[1]);
+            nl_msg_put_u8(request, TCA_FLOWER_KEY_CVLAN_PRIO,
+                          flower->key.vlan_prio[1]);
+            nl_msg_put_be16(request, TCA_FLOWER_KEY_CVLAN_ETH_TYPE,
+                            flower->key.encap_eth_type[1]);
         }
     }