On Wed, May 27, 2020 at 9:31 PM Eli Britstein <el...@mellanox.com> wrote: > > 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 <el...@mellanox.com> > Reviewed-by: Roni Bar Yanai <ron...@mellanox.com> > --- > lib/netdev-offload-dpdk.c | 122 > ++++++++++++++++------------------------------ > 1 file changed, 43 insertions(+), 79 deletions(-) Acked-by: sriharsha.basavapa...@broadcom.com
Thanks, -Harsha > > diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c > index f8c46bbaa..2f1b42bf7 100644 > --- a/lib/netdev-offload-dpdk.c > +++ b/lib/netdev-offload-dpdk.c > @@ -559,11 +559,21 @@ 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; > > + consumed_masks = &match->wc.masks; > + > + memset(&consumed_masks->in_port, 0, sizeof consumed_masks->in_port); > + if (match->flow.recirc_id != 0) { > + return -1; > + } > + consumed_masks->recirc_id = 0; > + consumed_masks->packet_type = 0; > + > /* Eth */ > if (!eth_addr_is_zero(match->wc.masks.dl_src) || > !eth_addr_is_zero(match->wc.masks.dl_dst)) { > @@ -580,6 +590,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 +604,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 +621,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 +642,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 +655,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 && > @@ -665,6 +688,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 +710,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 +731,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 +752,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,6 +765,9 @@ parse_flow_match(struct flow_patterns *patterns, > > add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_END, NULL, NULL); > > + if (!is_all_zeros(consumed_masks, sizeof *consumed_masks)) { > + return -1; > + } > return 0; > } > > @@ -1039,7 +1078,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 +1091,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 +1120,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 +1163,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 > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev