From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from stargate3.asicdesigners.com (stargate.chelsio.com [12.32.117.8]) by dpdk.org (Postfix) with ESMTP id E8F938E5E for ; Wed, 13 Jan 2016 09:49:50 +0100 (CET) Received: from localhost (scalar.blr.asicdesigners.com [10.193.185.94]) by stargate3.asicdesigners.com (8.13.8/8.13.8) with ESMTP id u0D8nkXQ006267; Wed, 13 Jan 2016 00:49:47 -0800 Date: Wed, 13 Jan 2016 14:19:22 +0530 From: Rahul Lakkireddy To: "Wu, Jingjing" Message-ID: <20160113084920.GA6469@scalar.blr.asicdesigners.com> References: <9BB6961774997848B5B42BEC655768F8D74B6B@SHSMSX104.ccr.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <9BB6961774997848B5B42BEC655768F8D74B6B@SHSMSX104.ccr.corp.intel.com> User-Agent: Mutt/1.5.24 (2015-08-30) Cc: "dev@dpdk.org" , Felix Marti , Nirranjan Kirubaharan , Kumar A S Subject: Re: [dpdk-dev] [RFC v2 1/2] ethdev: add packet filter flow and new behavior switch to fdir X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 13 Jan 2016 08:49:51 -0000 Hi Jingjing, On Tuesday, January 01/12/16, 2016 at 17:12:47 -0800, Wu, Jingjing wrote: > > > > diff --git a/lib/librte_ether/rte_eth_ctrl.h b/lib/librte_ether/rte_eth_ctrl.h > > index ce224ad..5cc22a0 100644 > > --- a/lib/librte_ether/rte_eth_ctrl.h > > +++ b/lib/librte_ether/rte_eth_ctrl.h > > @@ -74,7 +74,11 @@ extern "C" { > > #define RTE_ETH_FLOW_IPV6_EX 15 > > #define RTE_ETH_FLOW_IPV6_TCP_EX 16 > > #define RTE_ETH_FLOW_IPV6_UDP_EX 17 > > -#define RTE_ETH_FLOW_MAX 18 > > +#define RTE_ETH_FLOW_PKT_FILTER_IPV4_TCP 18 #define > > +RTE_ETH_FLOW_PKT_FILTER_IPV4_UDP 19 #define > > +RTE_ETH_FLOW_PKT_FILTER_IPV6_TCP 20 #define > > +RTE_ETH_FLOW_PKT_FILTER_IPV6_UDP 21 > > +#define RTE_ETH_FLOW_MAX 22 > > > How to distinguish RTE_ETH_FLOW_PKT_FILTER_IPV4_XX with RTE_ETH_FLOW_NONFRAG_IPV4_XX, what is the difference? The packet filter flow is basically a superset containing Ethernet, vlan, ipv4/ipv6 and tcp/udp flows whose fields can all be matched at the same time, unlike in case of the current flow director which seems to match only one of the flows at any given time. Additionally, it also allows specifying masks. I separated the two to make this meaning explicit. If this is not necessary, then I will merge them. > > /** > > * Feature filter types > > @@ -407,6 +411,9 @@ struct rte_eth_l2_flow { struct rte_eth_ipv4_flow { > > uint32_t src_ip; /**< IPv4 source address to match. */ > > uint32_t dst_ip; /**< IPv4 destination address to match. */ > > + uint8_t tos; /**< IPV4 type of service to match. */ > > + uint8_t proto; /**< IPV4 proto to match. */ > > + uint8_t ttl; /**< IPV4 time to live to match. */ > > }; > > > > /** > > @@ -443,6 +450,10 @@ struct rte_eth_sctpv4_flow { struct > > rte_eth_ipv6_flow { > > uint32_t src_ip[4]; /**< IPv6 source address to match. */ > > uint32_t dst_ip[4]; /**< IPv6 destination address to match. */ > > + uint8_t tc; /**< IPv6 traffic class to match. */ > > + uint32_t flow_label; /**< IPv6 flow label to match. */ > > + uint8_t next_header; /**< IPv6 next header to match. */ > > + uint8_t hop_limit; /**< IPv6 hop limits to match. */ > > }; > > > There is also a patch http://dpdk.org/dev/patchwork/patch/9661/ which added these fields. Maybe we can merge them together. Yes, we can merge them. Would you like me to merge your patch here? > > +struct rte_eth_pkt_filter_flow { > > + enum rte_eth_pkt_filter_type type; /**< Type of filter */ > > + enum rte_eth_pkt_filter_type prio; > > + /**< Prioritize the filter type when a packet matches several types */ > > + struct rte_eth_pkt_filter pkt; /**< Packet fields to match. */ > > + struct rte_eth_pkt_filter mask; /**< Mask for matched fields. */ > > +}; > > + > > +/** > > * An union contains the inputs for all types of flow > > */ > > union rte_eth_fdir_flow { > > @@ -514,6 +570,7 @@ union rte_eth_fdir_flow { > > struct rte_eth_ipv6_flow ipv6_flow; > > struct rte_eth_mac_vlan_flow mac_vlan_flow; > > struct rte_eth_tunnel_flow tunnel_flow; > > + struct rte_eth_pkt_filter_flow pkt_filter_flow; > > }; > Why not use rte_eth_XX_flow directly but add a new one? Is it because of the mask? If so, how about to add a field in rte_eth_fdir_input like: > struct rte_eth_fdir_input { > uint16_t flow_type; > union rte_eth_fdir_flow flow; > /**< Flow fields to match, dependent on flow_type */ > union rte_eth_fdir_flow flow_mask; > struct rte_eth_fdir_flow_ext flow_ext; > /**< Additional fields to match */ > }; > > Thanks > Jingjing The current rte_eth_XX_flow only allow matching _one_ of the flows because of the union. In contrast, rte_eth_pkt_filter_flow can match several flows at the same time; i.e. it can match ethernet, vlan, ip, and tcp/udp all at the same time. rte_eth_pkt_filter_flow is basically a superset containing several flows that can be matched at the same time. In our Chelsio T5 hardware, it's possible to have several flows that can be matched in a single rule. This is why I've created a superset that can match several flows in the same rule. Thanks, Rahul