From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 49835C130 for ; Thu, 22 Oct 2015 14:57:22 +0200 (CEST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga102.jf.intel.com with ESMTP; 22 Oct 2015 05:57:21 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.20,182,1444719600"; d="scan'208";a="832975899" Received: from bricha3-mobl3.ger.corp.intel.com ([10.237.208.66]) by fmsmga002.fm.intel.com with SMTP; 22 Oct 2015 05:57:19 -0700 Received: by (sSMTP sendmail emulation); Thu, 22 Oct 2015 13:57:18 +0025 Date: Thu, 22 Oct 2015 13:57:18 +0100 From: Bruce Richardson To: Wenzhuo Lu Message-ID: <20151022125718.GB20740@bricha3-MOBL3> References: <1443161125-1035-1-git-send-email-wenzhuo.lu@intel.com> <1445497902-16703-1-git-send-email-wenzhuo.lu@intel.com> <1445497902-16703-2-git-send-email-wenzhuo.lu@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1445497902-16703-2-git-send-email-wenzhuo.lu@intel.com> Organization: Intel Shannon Ltd. User-Agent: Mutt/1.5.23 (2014-03-12) Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH v3 1/7] lib/librte_ether: modify the structures for fdir new modes 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: Thu, 22 Oct 2015 12:57:22 -0000 On Thu, Oct 22, 2015 at 03:11:36PM +0800, Wenzhuo Lu wrote: > Define the new modes and modify the filter and mask structures for > the mac vlan and tunnel modes. > > Signed-off-by: Wenzhuo Lu Hi Wenzhuo, couple of stylistic comments below, which would help with patch review, especially with regards to checking for ABI issues. /Bruce > --- > lib/librte_ether/rte_eth_ctrl.h | 69 ++++++++++++++++++++++++++++++----------- > 1 file changed, 51 insertions(+), 18 deletions(-) > > diff --git a/lib/librte_ether/rte_eth_ctrl.h b/lib/librte_ether/rte_eth_ctrl.h > index 26b7b33..078faf9 100644 > --- a/lib/librte_ether/rte_eth_ctrl.h > +++ b/lib/librte_ether/rte_eth_ctrl.h > @@ -248,6 +248,17 @@ enum rte_eth_tunnel_type { > }; > > /** > + * Flow Director setting modes: none, signature or perfect. > + */ > +enum rte_fdir_mode { > + RTE_FDIR_MODE_NONE = 0, /**< Disable FDIR support. */ > + RTE_FDIR_MODE_SIGNATURE, /**< Enable FDIR signature filter mode. */ > + RTE_FDIR_MODE_PERFECT, /**< Enable FDIR perfect filter mode for IP. */ > + RTE_FDIR_MODE_PERFECT_MAC_VLAN, /**< Enable FDIR filter mode - MAC VLAN. */ > + RTE_FDIR_MODE_PERFECT_TUNNEL, /**< Enable FDIR filter mode - tunnel. */ > +}; > + Why is this structure definition moved in the file, it makes seeing the diff vs the old version difficult. > +/** > * filter type of tunneling packet > */ > #define ETH_TUNNEL_FILTER_OMAC 0x01 /**< filter by outer MAC addr */ > @@ -377,18 +388,46 @@ struct rte_eth_sctpv6_flow { > }; > > /** > + * A structure used to define the input for MAC VLAN flow > + */ > +struct rte_eth_mac_vlan_flow { > + struct ether_addr mac_addr; /**< Mac address to match. */ > +}; > + > +/** > + * Tunnel type for flow director. > + */ > +enum rte_eth_fdir_tunnel_type { > + RTE_FDIR_TUNNEL_TYPE_NVGRE = 0, > + RTE_FDIR_TUNNEL_TYPE_VXLAN, > + RTE_FDIR_TUNNEL_TYPE_UNKNOWN, > +}; > + > +/** > + * A structure used to define the input for tunnel flow, now it's VxLAN or > + * NVGRE > + */ > +struct rte_eth_tunnel_flow { > + enum rte_eth_fdir_tunnel_type tunnel_type; /**< Tunnel type to match. */ > + uint32_t tunnel_id; /**< Tunnel ID to match. TNI, VNI... */ > + struct ether_addr mac_addr; /**< Mac address to match. */ > +}; > + > +/** > * An union contains the inputs for all types of flow > */ > union rte_eth_fdir_flow { > - struct rte_eth_l2_flow l2_flow; > - struct rte_eth_udpv4_flow udp4_flow; > - struct rte_eth_tcpv4_flow tcp4_flow; > - struct rte_eth_sctpv4_flow sctp4_flow; > - struct rte_eth_ipv4_flow ip4_flow; > - struct rte_eth_udpv6_flow udp6_flow; > - struct rte_eth_tcpv6_flow tcp6_flow; > - struct rte_eth_sctpv6_flow sctp6_flow; > - struct rte_eth_ipv6_flow ipv6_flow; > + struct rte_eth_l2_flow l2_flow; > + struct rte_eth_udpv4_flow udp4_flow; > + struct rte_eth_tcpv4_flow tcp4_flow; > + struct rte_eth_sctpv4_flow sctp4_flow; > + struct rte_eth_ipv4_flow ip4_flow; > + struct rte_eth_udpv6_flow udp6_flow; > + struct rte_eth_tcpv6_flow tcp6_flow; > + struct rte_eth_sctpv6_flow sctp6_flow; > + struct rte_eth_ipv6_flow ipv6_flow; > + struct rte_eth_mac_vlan_flow mac_vlan_flow; > + struct rte_eth_tunnel_flow tunnel_flow; Can you please minimize the whitespace changes here. It looks in the diff like you are replacing the entire set of entries, but on closer inspection it looks like you are just adding in two extra lines. > }; > > /** > @@ -465,6 +504,9 @@ struct rte_eth_fdir_masks { > struct rte_eth_ipv6_flow ipv6_mask; > uint16_t src_port_mask; > uint16_t dst_port_mask; > + uint8_t mac_addr_byte_mask; /** Per byte MAC address mask */ > + uint32_t tunnel_id_mask; /** tunnel ID mask */ > + uint8_t tunnel_type_mask; > }; > > /** > @@ -515,15 +557,6 @@ struct rte_eth_fdir_flex_conf { > /**< Flex mask configuration for each flow type */ > }; > > -/** > - * Flow Director setting modes: none, signature or perfect. > - */ > -enum rte_fdir_mode { > - RTE_FDIR_MODE_NONE = 0, /**< Disable FDIR support. */ > - RTE_FDIR_MODE_SIGNATURE, /**< Enable FDIR signature filter mode. */ > - RTE_FDIR_MODE_PERFECT, /**< Enable FDIR perfect filter mode. */ > -}; > - > #define UINT32_BIT (CHAR_BIT * sizeof(uint32_t)) > #define RTE_FLOW_MASK_ARRAY_SIZE \ > (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT32_BIT)/UINT32_BIT) > -- > 1.9.3 >