From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 98D13C31E57 for ; Mon, 17 Jun 2019 09:19:35 +0000 (UTC) Received: from dpdk.org (dpdk.org [92.243.14.124]) by mail.kernel.org (Postfix) with ESMTP id 0AF7020848 for ; Mon, 17 Jun 2019 09:19:34 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0AF7020848 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=dev-bounces@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 16C881BEA8; Mon, 17 Jun 2019 11:19:28 +0200 (CEST) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id BC1FB1BEA0 for ; Mon, 17 Jun 2019 11:19:25 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 17 Jun 2019 02:19:24 -0700 X-ExtLoop1: 1 Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by orsmga002.jf.intel.com with ESMTP; 17 Jun 2019 02:19:24 -0700 Received: from fmsmsx113.amr.corp.intel.com (10.18.116.7) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 17 Jun 2019 02:19:23 -0700 Received: from shsmsx107.ccr.corp.intel.com (10.239.4.96) by FMSMSX113.amr.corp.intel.com (10.18.116.7) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 17 Jun 2019 02:19:23 -0700 Received: from shsmsx106.ccr.corp.intel.com ([169.254.10.89]) by SHSMSX107.ccr.corp.intel.com ([169.254.9.173]) with mapi id 14.03.0439.000; Mon, 17 Jun 2019 17:19:21 +0800 From: "Wang, Xiao W" To: "Yang, Qiming" , "dev@dpdk.org" CC: "Xing, Beilei" Thread-Topic: [dpdk-dev] [PATCH v2 2/3] net/ice: add generic flow API Thread-Index: AQHVIPPcggUQHfuwx0y78mqMPcdtaqafk9zg Date: Mon, 17 Jun 2019 09:19:20 +0000 Message-ID: References: <1559552722-8970-1-git-send-email-qiming.yang@intel.com> <20190612075029.109914-1-qiming.yang@intel.com> <20190612075029.109914-3-qiming.yang@intel.com> In-Reply-To: <20190612075029.109914-3-qiming.yang@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_NT x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNjQxOWY4NGYtYzBlMS00ODU1LWFmMjEtNmM3Y2YxYjEwZTczIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiTHI3OVZcL0V2UzNWQmRvUzRYNHBRWmtSbzJDR1NOamllNWpaNkFLN1k1a0F3R3RWcFhWNXdnSW1ORFdGajZEakEifQ== dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v2 2/3] net/ice: add generic flow API X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Qiming, > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Qiming Yang > Sent: Wednesday, June 12, 2019 3:50 PM > To: dev@dpdk.org > Cc: Yang, Qiming > Subject: [dpdk-dev] [PATCH v2 2/3] net/ice: add generic flow API >=20 > This patch adds ice_flow_create, ice_flow_destroy, > ice_flow_flush and ice_flow_validate support, > these are going to used to handle all the generic filters. >=20 > Signed-off-by: Qiming Yang > --- > drivers/net/ice/Makefile | 1 + > drivers/net/ice/ice_ethdev.c | 44 +++ > drivers/net/ice/ice_ethdev.h | 7 +- > drivers/net/ice/ice_generic_flow.c | 567 > +++++++++++++++++++++++++++++++++++++ > drivers/net/ice/ice_generic_flow.h | 404 ++++++++++++++++++++++++++ > 5 files changed, 1022 insertions(+), 1 deletion(-) > create mode 100644 drivers/net/ice/ice_generic_flow.c > create mode 100644 drivers/net/ice/ice_generic_flow.h >=20 > diff --git a/drivers/net/ice/Makefile b/drivers/net/ice/Makefile > index b10d826..32abeb6 100644 > --- a/drivers/net/ice/Makefile [...] > bool offset_loaded; > bool adapter_stopped; > + struct ice_flow_list flow_list; > }; >=20 > /** > diff --git a/drivers/net/ice/ice_generic_flow.c > b/drivers/net/ice/ice_generic_flow.c > new file mode 100644 > index 0000000..4fb50b2 > --- /dev/null > +++ b/drivers/net/ice/ice_generic_flow.c > @@ -0,0 +1,567 @@ License header is missing. > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > + > +#include "ice_ethdev.h" > +#include "ice_generic_flow.h" > +#include "ice_switch_filter.h" > + [...] > + if (eth_mask->type =3D=3D RTE_BE16(0xffff)) > + input_set |=3D ICE_INSET_ETHERTYPE; > + } > + break; > + case RTE_FLOW_ITEM_TYPE_IPV4: > + ipv4_spec =3D item->spec; > + ipv4_mask =3D item->mask; > + > + if (!(ipv4_spec && ipv4_mask)) { > + rte_flow_error_set(error, EINVAL, > + RTE_FLOW_ERROR_TYPE_ITEM, > + item, > + "Invalid IPv4 spec or mask."); > + return 0; > + } A lot of this kind of check in this function, could we just check " item->s= pec && item->mask" once before the switch {}? > + > + /* Check IPv4 mask and update input set */ > + if (ipv4_mask->hdr.version_ihl || > + ipv4_mask->hdr.total_length || > + ipv4_mask->hdr.packet_id || > + ipv4_mask->hdr.hdr_checksum) { > + rte_flow_error_set(error, EINVAL, > + RTE_FLOW_ERROR_TYPE_ITEM, > + item, > + "Invalid IPv4 mask."); > + return 0; > + } > + > + if (outer_ip) { > + if (ipv4_mask->hdr.src_addr =3D=3D UINT32_MAX) > + input_set |=3D ICE_INSET_IPV4_SRC; > + if (ipv4_mask->hdr.dst_addr =3D=3D UINT32_MAX) > + input_set |=3D ICE_INSET_IPV4_DST; > + if (ipv4_mask->hdr.type_of_service =3D=3D > UINT8_MAX) > + input_set |=3D ICE_INSET_IPV4_TOS; > + if (ipv4_mask->hdr.time_to_live =3D=3D UINT8_MAX) > + input_set |=3D ICE_INSET_IPV4_TTL; > + if (ipv4_mask->hdr.fragment_offset =3D=3D 0) > + input_set |=3D ICE_INSET_IPV4_PROTO; > + outer_ip =3D false; > + } else { > + if (ipv4_mask->hdr.src_addr =3D=3D UINT32_MAX) > + input_set |=3D > ICE_INSET_TUN_IPV4_SRC; > + if (ipv4_mask->hdr.dst_addr =3D=3D UINT32_MAX) > + input_set |=3D > ICE_INSET_TUN_IPV4_DST; > + if (ipv4_mask->hdr.time_to_live =3D=3D UINT8_MAX) > + input_set |=3D ICE_INSET_TUN_IPV4_TTL; > + if (ipv4_mask->hdr.next_proto_id =3D=3D > UINT8_MAX) > + input_set |=3D > ICE_INSET_TUN_IPV4_PROTO; > + } > + break; > + case RTE_FLOW_ITEM_TYPE_IPV6: > + ipv6_spec =3D item->spec; > + ipv6_mask =3D item->mask; > + > + if (!(ipv6_spec && ipv6_mask)) { > + rte_flow_error_set(error, EINVAL, > + RTE_FLOW_ERROR_TYPE_ITEM, > + item, "Invalid IPv6 spec or mask"); > + return 0; > + } > + > + if (ipv6_mask->hdr.payload_len || > + ipv6_mask->hdr.vtc_flow) { > + rte_flow_error_set(error, EINVAL, > + RTE_FLOW_ERROR_TYPE_ITEM, > + item, > + "Invalid IPv6 mask"); > + return 0; > + } > + > + if (outer_ip) { > + if (!memcmp(ipv6_mask->hdr.src_addr, [...] > + item, > + "Invalid ICMP mask"); > + return 0; > + } > + > + if (icmp_mask->hdr.icmp_type =3D=3D UINT8_MAX) > + input_set |=3D ICE_INSET_ICMP; > + break; > + case RTE_FLOW_ITEM_TYPE_ICMP6: > + icmp6_mask =3D item->mask; > + if (icmp6_mask->code || > + icmp6_mask->checksum) { > + rte_flow_error_set(error, EINVAL, > + > RTE_FLOW_ERROR_TYPE_ITEM, > + item, > + "Invalid ICMP6 mask"); > + return 0; > + } > + > + if (icmp6_mask->type =3D=3D UINT8_MAX) > + input_set |=3D ICE_INSET_ICMP6; Add a '\t' for Indent. > + break; > + default: > + rte_flow_error_set(error, EINVAL, > + RTE_FLOW_ERROR_TYPE_ITEM, > + item, > + "Invalid mask no exist"); > + break; > + } > + } > + return input_set; > +} > + > +static int ice_flow_valid_inset(const struct rte_flow_item pattern[], > + uint64_t inset, struct rte_flow_error *error) > +{ > + uint64_t fields; > + > + /* get valid field */ > + fields =3D ice_get_flow_field(pattern, error); > + if ((!fields) || (fields && (!inset))) { > + rte_flow_error_set(error, EINVAL, > + RTE_FLOW_ERROR_TYPE_ITEM_SPEC, > + pattern, > + "Invalid input set"); > + return -rte_errno; > + } > + > + return 0; > +} > + > +static int ice_flow_valid_action(const struct rte_flow_action *actions, > + struct rte_flow_error *error) > +{ > + switch (actions->type) { > + case RTE_FLOW_ACTION_TYPE_QUEUE: > + break; > + case RTE_FLOW_ACTION_TYPE_DROP: > + break; > + default: > + rte_flow_error_set(error, EINVAL, > + RTE_FLOW_ERROR_TYPE_ACTION, actions, > + "Invalid action."); > + return -rte_errno; > + } > + > + return 0; > +} > + > +static int > +ice_flow_validate(__rte_unused struct rte_eth_dev *dev, > + const struct rte_flow_attr *attr, > + const struct rte_flow_item pattern[], > + const struct rte_flow_action actions[], > + struct rte_flow_error *error) > +{ > + uint64_t inset =3D 0; > + int ret =3D ICE_ERR_NOT_SUPPORTED; > + > + if (!pattern) { > + rte_flow_error_set(error, EINVAL, > RTE_FLOW_ERROR_TYPE_ITEM_NUM, > + NULL, "NULL pattern."); > + return -rte_errno; > + } > + > + if (!actions) { > + rte_flow_error_set(error, EINVAL, > + RTE_FLOW_ERROR_TYPE_ACTION_NUM, > + NULL, "NULL action."); > + return -rte_errno; > + } > + > + if (!attr) { > + rte_flow_error_set(error, EINVAL, > + RTE_FLOW_ERROR_TYPE_ATTR, > + NULL, "NULL attribute."); > + return -rte_errno; > + } > + > + ret =3D ice_flow_valid_attr(attr, error); > + if (!ret) > + return ret; > + > + inset =3D ice_flow_valid_pattern(pattern, error); > + if (!inset) > + return -rte_errno; > + > + ret =3D ice_flow_valid_inset(pattern, inset, error); > + if (ret) > + return ret; > + > + ret =3D ice_flow_valid_action(actions, error); > + if (ret) > + return ret; > + > + return 0; > +} > + > +static struct rte_flow * > +ice_flow_create(struct rte_eth_dev *dev, > + const struct rte_flow_attr *attr, > + const struct rte_flow_item pattern[], > + const struct rte_flow_action actions[], > + struct rte_flow_error *error) > +{ > + struct ice_pf *pf =3D ICE_DEV_PRIVATE_TO_PF(dev->data->dev_private); > + struct rte_flow *flow =3D NULL; > + int ret; > + > + flow =3D rte_zmalloc("ice_flow", sizeof(struct rte_flow), 0); > + if (!flow) { > + rte_flow_error_set(error, ENOMEM, > + RTE_FLOW_ERROR_TYPE_HANDLE, NULL, > + "Failed to allocate memory"); > + return flow; > + } > + > + ret =3D ice_flow_validate(dev, attr, pattern, actions, error); > + if (ret < 0) > + return NULL; > + > + ret =3D ice_create_switch_filter(pf, pattern, actions, flow, error); > + if (ret) > + goto free_flow; > + > + TAILQ_INSERT_TAIL(&pf->flow_list, flow, node); > + return flow; > + > +free_flow: > + rte_flow_error_set(error, -ret, > + RTE_FLOW_ERROR_TYPE_HANDLE, NULL, > + "Failed to create flow."); > + rte_free(flow); > + return NULL; > +} > + > +static int > +ice_flow_destroy(struct rte_eth_dev *dev, > + struct rte_flow *flow, > + struct rte_flow_error *error) > +{ > + struct ice_pf *pf =3D ICE_DEV_PRIVATE_TO_PF(dev->data->dev_private); > + int ret =3D 0; > + > + ret =3D ice_destroy_switch_filter(pf, flow); > + > + if (!ret) { > + TAILQ_REMOVE(&pf->flow_list, flow, node); > + rte_free(flow); > + } else > + rte_flow_error_set(error, -ret, > + RTE_FLOW_ERROR_TYPE_HANDLE, NULL, > + "Failed to destroy flow."); > + > + return ret; > +} > + > +static int > +ice_flow_flush(struct rte_eth_dev *dev, > + struct rte_flow_error *error) > +{ > + struct ice_pf *pf =3D ICE_DEV_PRIVATE_TO_PF(dev->data->dev_private); > + struct rte_flow *p_flow; > + int ret; > + > + TAILQ_FOREACH(p_flow, &pf->flow_list, node) { > + ret =3D ice_flow_destroy(dev, p_flow, error); > + if (ret) { > + rte_flow_error_set(error, -ret, > + RTE_FLOW_ERROR_TYPE_HANDLE, > NULL, > + "Failed to flush SW flows."); > + return -rte_errno; > + } > + } > + > + return ret; > +} > diff --git a/drivers/net/ice/ice_generic_flow.h > b/drivers/net/ice/ice_generic_flow.h > new file mode 100644 > index 0000000..46c3461 > --- /dev/null > +++ b/drivers/net/ice/ice_generic_flow.h > @@ -0,0 +1,404 @@ > +/* SPDX-License-Identifier: BSD-3-Clause > + * Copyright(c) 2018 Intel Corporation s/2018/2019/g > + */ > + > +#ifndef _ICE_GENERIC_FLOW_H_ > +#define _ICE_GENERIC_FLOW_H_ > + > +#include > + > +struct ice_flow_pattern { [...] > + RTE_FLOW_ITEM_TYPE_IPV4, > + RTE_FLOW_ITEM_TYPE_UDP, > + RTE_FLOW_ITEM_TYPE_ETH, > + RTE_FLOW_ITEM_TYPE_IPV6, > + RTE_FLOW_ITEM_TYPE_SCTP, > + RTE_FLOW_ITEM_TYPE_END, > +}; > + > +static enum rte_flow_item_type pattern_ipv4_tunnel_eth_ipv6_icmp[] =3D { > + RTE_FLOW_ITEM_TYPE_ETH, > + RTE_FLOW_ITEM_TYPE_IPV4, > + RTE_FLOW_ITEM_TYPE_UDP, > + RTE_FLOW_ITEM_TYPE_ETH, > + RTE_FLOW_ITEM_TYPE_IPV6, > + RTE_FLOW_ITEM_TYPE_ICMP, > + RTE_FLOW_ITEM_TYPE_END, > +}; > + > +static struct ice_flow_pattern ice_supported_patterns[] =3D { > + {pattern_ethertype, INSET_ETHER}, > + {pattern_ipv4, INSET_MAC_IPV4}, > + {pattern_ipv4_udp, INSET_MAC_IPV4_L4}, > + {pattern_ipv4_sctp, INSET_MAC_IPV4_L4}, > + {pattern_ipv4_tcp, INSET_MAC_IPV4_L4}, > + {pattern_ipv4_icmp, INSET_MAC_IPV4_ICMP}, > + {pattern_ipv6, INSET_MAC_IPV6}, > + {pattern_ipv6_udp, INSET_MAC_IPV6_L4}, > + {pattern_ipv6_sctp, INSET_MAC_IPV6_L4}, > + {pattern_ipv6_tcp, INSET_MAC_IPV6_L4}, > + {pattern_ipv6_icmp6, INSET_MAC_IPV6_ICMP}, > + {pattern_ipv4_tunnel_ipv4, INSET_TUNNEL_IPV4_TYPE1}, > + {pattern_ipv4_tunnel_ipv4_udp, INSET_TUNNEL_IPV4_TYPE2}, > + {pattern_ipv4_tunnel_ipv4_tcp, INSET_TUNNEL_IPV4_TYPE2}, > + {pattern_ipv4_tunnel_ipv4_sctp, INSET_TUNNEL_IPV4_TYPE2}, > + {pattern_ipv4_tunnel_ipv4_icmp, INSET_TUNNEL_IPV4_TYPE3}, > + {pattern_ipv4_tunnel_eth_ipv4, INSET_TUNNEL_IPV4_TYPE1}, > + {pattern_ipv4_tunnel_eth_ipv4_udp, INSET_TUNNEL_IPV4_TYPE2}, > + {pattern_ipv4_tunnel_eth_ipv4_tcp, INSET_TUNNEL_IPV4_TYPE2}, > + {pattern_ipv4_tunnel_eth_ipv4_sctp, INSET_TUNNEL_IPV4_TYPE2}, > + {pattern_ipv4_tunnel_eth_ipv4_icmp, INSET_TUNNEL_IPV4_TYPE3}, > + {pattern_ipv4_tunnel_ipv6, INSET_TUNNEL_IPV6_TYPE1}, > + {pattern_ipv4_tunnel_ipv6_udp, INSET_TUNNEL_IPV6_TYPE2}, > + {pattern_ipv4_tunnel_ipv6_tcp, INSET_TUNNEL_IPV6_TYPE2}, > + {pattern_ipv4_tunnel_ipv6_sctp, INSET_TUNNEL_IPV6_TYPE2}, > + {pattern_ipv4_tunnel_ipv6_icmp, INSET_TUNNEL_IPV6_TYPE3}, > + {pattern_ipv4_tunnel_eth_ipv6, INSET_TUNNEL_IPV6_TYPE1}, > + {pattern_ipv4_tunnel_eth_ipv6_udp, INSET_TUNNEL_IPV6_TYPE2}, > + {pattern_ipv4_tunnel_eth_ipv6_tcp, INSET_TUNNEL_IPV6_TYPE2}, > + {pattern_ipv4_tunnel_eth_ipv6_sctp, INSET_TUNNEL_IPV6_TYPE2}, > + {pattern_ipv4_tunnel_eth_ipv6_icmp, INSET_TUNNEL_IPV6_TYPE3}, > +}; > + > +#endif > -- > 2.9.5 I have the same feeling with Beilei for the duplication, some flow validati= on work is done in this patch, however the switch filter part (1/3) also im= plements some check. BRs, Xiao