[ovs-dev] [PATCH 01/11] netdev-offload-dpdk: Remove pre-validate of patterns function

Eli Britstein elibr at mellanox.com
Mon May 18 15:40:16 UTC 2020


The function of adding patterns by requested matches checks that it
consumed all the required matches, and err if not. This nullify the
purpose of the validation function. Future supported matches will only
change the pattern parsing code.

Signed-off-by: Eli Britstein <elibr at mellanox.com>
Reviewed-by: Roni Bar Yanai <roniba at mellanox.com>
---
 lib/netdev-offload-dpdk.c | 133 ++++++++++++++++++----------------------------
 1 file changed, 51 insertions(+), 82 deletions(-)

diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index f8c46bbaa..84bbe29e7 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -559,10 +559,22 @@ free_flow_actions(struct flow_actions *actions)
 
 static int
 parse_flow_match(struct flow_patterns *patterns,
-                 const struct match *match)
+                 struct match *match)
 {
     uint8_t *next_proto_mask = NULL;
+    struct flow *consumed_masks;
     uint8_t proto = 0;
+    int ret = 0;
+
+    consumed_masks = &match->wc.masks;
+
+    memset(&consumed_masks->in_port, 0, sizeof consumed_masks->in_port);
+    if (match->flow.recirc_id != 0) {
+        ret = -1;
+        goto out;
+    }
+    consumed_masks->recirc_id = 0;
+    consumed_masks->packet_type = 0;
 
     /* Eth */
     if (!eth_addr_is_zero(match->wc.masks.dl_src) ||
@@ -580,6 +592,9 @@ parse_flow_match(struct flow_patterns *patterns,
         memcpy(&mask->src, &match->wc.masks.dl_src, sizeof mask->src);
         mask->type = match->wc.masks.dl_type;
 
+        memset(&consumed_masks->dl_dst, 0, sizeof consumed_masks->dl_dst);
+        memset(&consumed_masks->dl_src, 0, sizeof consumed_masks->dl_src);
+
         add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_ETH, spec, mask);
     } else {
         /*
@@ -591,6 +606,7 @@ parse_flow_match(struct flow_patterns *patterns,
          */
         add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_ETH, NULL, NULL);
     }
+    consumed_masks->dl_type = 0;
 
     /* VLAN */
     if (match->wc.masks.vlans[0].tci && match->flow.vlans[0].tci) {
@@ -607,6 +623,7 @@ parse_flow_match(struct flow_patterns *patterns,
 
         add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_VLAN, spec, mask);
     }
+    memset(&consumed_masks->vlans[0], 0, sizeof consumed_masks->vlans[0]);
 
     /* IP v4 */
     if (match->flow.dl_type == htons(ETH_TYPE_IP)) {
@@ -627,6 +644,12 @@ parse_flow_match(struct flow_patterns *patterns,
         mask->hdr.src_addr        = match->wc.masks.nw_src;
         mask->hdr.dst_addr        = match->wc.masks.nw_dst;
 
+        consumed_masks->nw_tos = 0;
+        consumed_masks->nw_ttl = 0;
+        consumed_masks->nw_proto = 0;
+        consumed_masks->nw_src = 0;
+        consumed_masks->nw_dst = 0;
+
         add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_IPV4, spec, mask);
 
         /* Save proto for L4 protocol setup. */
@@ -634,6 +657,8 @@ parse_flow_match(struct flow_patterns *patterns,
                 mask->hdr.next_proto_id;
         next_proto_mask = &mask->hdr.next_proto_id;
     }
+    /* ignore mask match for now */
+    consumed_masks->nw_frag = 0;
 
     if (proto != IPPROTO_ICMP && proto != IPPROTO_UDP  &&
         proto != IPPROTO_SCTP && proto != IPPROTO_TCP  &&
@@ -641,12 +666,14 @@ parse_flow_match(struct flow_patterns *patterns,
          match->wc.masks.tp_dst ||
          match->wc.masks.tcp_flags)) {
         VLOG_DBG("L4 Protocol (%u) not supported", proto);
-        return -1;
+        ret = -1;
+        goto out;
     }
 
     if ((match->wc.masks.tp_src && match->wc.masks.tp_src != OVS_BE16_MAX) ||
         (match->wc.masks.tp_dst && match->wc.masks.tp_dst != OVS_BE16_MAX)) {
-        return -1;
+        ret = -1;
+        goto out;
     }
 
     if (proto == IPPROTO_TCP) {
@@ -665,6 +692,10 @@ parse_flow_match(struct flow_patterns *patterns,
         mask->hdr.data_off  = ntohs(match->wc.masks.tcp_flags) >> 8;
         mask->hdr.tcp_flags = ntohs(match->wc.masks.tcp_flags) & 0xff;
 
+        consumed_masks->tp_src = 0;
+        consumed_masks->tp_dst = 0;
+        consumed_masks->tcp_flags = 0;
+
         add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_TCP, spec, mask);
 
         /* proto == TCP and ITEM_TYPE_TCP, thus no need for proto match. */
@@ -683,6 +714,9 @@ parse_flow_match(struct flow_patterns *patterns,
         mask->hdr.src_port = match->wc.masks.tp_src;
         mask->hdr.dst_port = match->wc.masks.tp_dst;
 
+        consumed_masks->tp_src = 0;
+        consumed_masks->tp_dst = 0;
+
         add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_UDP, spec, mask);
 
         /* proto == UDP and ITEM_TYPE_UDP, thus no need for proto match. */
@@ -701,6 +735,9 @@ parse_flow_match(struct flow_patterns *patterns,
         mask->hdr.src_port = match->wc.masks.tp_src;
         mask->hdr.dst_port = match->wc.masks.tp_dst;
 
+        consumed_masks->tp_src = 0;
+        consumed_masks->tp_dst = 0;
+
         add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_SCTP, spec, mask);
 
         /* proto == SCTP and ITEM_TYPE_SCTP, thus no need for proto match. */
@@ -719,6 +756,9 @@ parse_flow_match(struct flow_patterns *patterns,
         mask->hdr.icmp_type = (uint8_t) ntohs(match->wc.masks.tp_src);
         mask->hdr.icmp_code = (uint8_t) ntohs(match->wc.masks.tp_dst);
 
+        consumed_masks->tp_src = 0;
+        consumed_masks->tp_dst = 0;
+
         add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_ICMP, spec, mask);
 
         /* proto == ICMP and ITEM_TYPE_ICMP, thus no need for proto match. */
@@ -729,7 +769,11 @@ parse_flow_match(struct flow_patterns *patterns,
 
     add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_END, NULL, NULL);
 
-    return 0;
+    if (!is_all_zeros(consumed_masks, sizeof *consumed_masks)) {
+        ret = -1;
+    }
+out:
+    return ret;
 }
 
 static void
@@ -1039,7 +1083,7 @@ out:
 
 static int
 netdev_offload_dpdk_add_flow(struct netdev *netdev,
-                             const struct match *match,
+                             struct match *match,
                              struct nlattr *nl_actions,
                              size_t actions_len,
                              const ovs_u128 *ufid,
@@ -1052,6 +1096,8 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev,
 
     ret = parse_flow_match(&patterns, match);
     if (ret) {
+        VLOG_DBG_RL(&rl, "%s: matches of ufid "UUID_FMT" are not supported\n",
+                    netdev_get_name(netdev), UUID_ARGS((struct uuid *)ufid));
         goto out;
     }
 
@@ -1079,78 +1125,6 @@ out:
     return ret;
 }
 
-/*
- * Check if any unsupported flow patterns are specified.
- */
-static int
-netdev_offload_dpdk_validate_flow(const struct match *match)
-{
-    struct match match_zero_wc;
-    const struct flow *masks = &match->wc.masks;
-
-    /* Create a wc-zeroed version of flow. */
-    match_init(&match_zero_wc, &match->flow, &match->wc);
-
-    if (!is_all_zeros(&match_zero_wc.flow.tunnel,
-                      sizeof match_zero_wc.flow.tunnel)) {
-        goto err;
-    }
-
-    if (masks->metadata || masks->skb_priority ||
-        masks->pkt_mark || masks->dp_hash) {
-        goto err;
-    }
-
-    /* recirc id must be zero. */
-    if (match_zero_wc.flow.recirc_id) {
-        goto err;
-    }
-
-    if (masks->ct_state || masks->ct_nw_proto ||
-        masks->ct_zone  || masks->ct_mark     ||
-        !ovs_u128_is_zero(masks->ct_label)) {
-        goto err;
-    }
-
-    if (masks->conj_id || masks->actset_output) {
-        goto err;
-    }
-
-    /* Unsupported L2. */
-    if (!is_all_zeros(masks->mpls_lse, sizeof masks->mpls_lse)) {
-        goto err;
-    }
-
-    /* Unsupported L3. */
-    if (masks->ipv6_label || masks->ct_nw_src || masks->ct_nw_dst     ||
-        !is_all_zeros(&masks->ipv6_src,    sizeof masks->ipv6_src)    ||
-        !is_all_zeros(&masks->ipv6_dst,    sizeof masks->ipv6_dst)    ||
-        !is_all_zeros(&masks->ct_ipv6_src, sizeof masks->ct_ipv6_src) ||
-        !is_all_zeros(&masks->ct_ipv6_dst, sizeof masks->ct_ipv6_dst) ||
-        !is_all_zeros(&masks->nd_target,   sizeof masks->nd_target)   ||
-        !is_all_zeros(&masks->nsh,         sizeof masks->nsh)         ||
-        !is_all_zeros(&masks->arp_sha,     sizeof masks->arp_sha)     ||
-        !is_all_zeros(&masks->arp_tha,     sizeof masks->arp_tha)) {
-        goto err;
-    }
-
-    /* If fragmented, then don't HW accelerate - for now. */
-    if (match_zero_wc.flow.nw_frag) {
-        goto err;
-    }
-
-    /* Unsupported L4. */
-    if (masks->igmp_group_ip4 || masks->ct_tp_src || masks->ct_tp_dst) {
-        goto err;
-    }
-
-    return 0;
-
-err:
-    VLOG_ERR("cannot HW accelerate this flow due to unsupported protocols");
-    return -1;
-}
-
 static int
 netdev_offload_dpdk_destroy_flow(struct netdev *netdev,
                                  const ovs_u128 *ufid,
@@ -1194,11 +1168,6 @@ netdev_offload_dpdk_flow_put(struct netdev *netdev, struct match *match,
         }
     }
 
-    ret = netdev_offload_dpdk_validate_flow(match);
-    if (ret < 0) {
-        return ret;
-    }
-
     if (stats) {
         memset(stats, 0, sizeof *stats);
     }
-- 
2.14.5



More information about the dev mailing list