From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 8A0C2595B for ; Thu, 13 Nov 2014 06:36:19 +0100 (CET) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga102.fm.intel.com with ESMTP; 12 Nov 2014 21:46:15 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.97,862,1389772800"; d="scan'208";a="415828346" Received: from kmsmsx153.gar.corp.intel.com ([172.21.73.88]) by FMSMGA003.fm.intel.com with ESMTP; 12 Nov 2014 21:37:15 -0800 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by KMSMSX153.gar.corp.intel.com (172.21.73.88) with Microsoft SMTP Server (TLS) id 14.3.195.1; Thu, 13 Nov 2014 13:44:14 +0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.182]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.110]) with mapi id 14.03.0195.001; Thu, 13 Nov 2014 13:44:13 +0800 From: "Wu, Jingjing" To: Thomas Monjalon Thread-Topic: [dpdk-dev] [PATCH v2 1/3] ethdev: define ctrl_pkt filter type and its structure Thread-Index: AQHP7dEkLbxhXZVk7kqrDLbF9UwYj5xIxSIAgAD9qxD//6lbAIAUtdfA Date: Thu, 13 Nov 2014 05:44:13 +0000 Message-ID: <9BB6961774997848B5B42BEC655768F8B47F69@SHSMSX104.ccr.corp.intel.com> References: <1411628369-29532-1-git-send-email-jingjing.wu@intel.com> <1607705.DDyl0z8Nv8@xps13> <9BB6961774997848B5B42BEC655768F8B26609@SHSMSX104.ccr.corp.intel.com> <4699150.INYaCBBBvY@xps13> In-Reply-To: <4699150.INYaCBBBvY@xps13> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH v2 1/3] ethdev: define ctrl_pkt filter type and its structure 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, 13 Nov 2014 05:36:20 -0000 Hi, Thomas The input set of control packet filter are dst_mac and ethertype in Etherne= t head. To be clear, I think it's better to use the name ethertype filter. While there is already ethertype filter existing in igb and ixgbe driver. I= will rename The patchset to ethertype filter and also integrate igb and ixgbe's etherty= pe filter To the filter_ctrl API. What do you think? =20 Jingjing > -----Original Message----- > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > Sent: Friday, October 31, 2014 4:45 PM > To: Wu, Jingjing > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2 1/3] ethdev: define ctrl_pkt filter typ= e > and its structure >=20 > Hi Jingjing, >=20 > I'm sorry, but your explanations are not sufficient. > Please keep in mind that the user of the API don't know i40e internals. >=20 > 2014-10-31 07:05, Wu, Jingjing: > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > > > 2014-10-22 16:19, Jingjing Wu: > > > > +/** > > > > + * Define all structures for Control Packet Filter type > > > > +corresponding with > > > specific operations. > > > > + */ > > > > > > Please explain what is a control packet. > > > > A control element in Fortville can be used to receive control packets a= nd > control other switching elements. Control packet filter can filter contro= l > packet (such as LLDP) to different queues in receive and identify the swi= tch > element that should process the packets in transmit. > > At the same time, we also can use this filter to route non-control pack= ets to > queue or other engine by filtering mac and ether_type. The name "control > packet filter" comes from Fortville. >=20 > I still don't know what is a control packet. >=20 > > > > +#define RTE_CONTROL_PACKET_FLAGS_IGNORE_MAC 0x0001 > > > > +#define RTE_CONTROL_PACKET_FLAGS_DROP 0x0002 > > > > +#define RTE_CONTROL_PACKET_FLAGS_TO_QUEUE 0x0004 > > > > +#define RTE_CONTROL_PACKET_FLAGS_TX 0x0008 > > > > +#define RTE_CONTROL_PACKET_FLAGS_RX 0x0000 > > > > > > Flag RX is 0? > > > > Yes, RX is default value. Maybe it need to be removed. >=20 > No, it doesn't need to be removed. But a flag must not be 0. > 0 means none. > It's impossible to disable this flag. >=20 > Moreover, you should comment each flag. >=20 > > > > +/** > > > > + * A structure used to define the control packet filter entry > > > > + * to support RTE_ETH_FILTER_CTRL_PKT with RTE_ETH_FILTER_ADD > > > > + * and RTE_ETH_FILTER_DELETE operations. > > > > + */ > > > > +struct rte_ctrl_pkt_filter { > > > > + struct ether_addr mac_addr; /**< mac address to match. */ > > > > + uint16_t ether_type; /**< ether type to match */ > > > > + uint16_t flags; /**< options for filter's behavior*= / > > > > + uint16_t dest_id; /**< destination vsi id or pool id*= / > > > > > > vsi id and pool id cannot be understood in a generic context. > > > Please explain what you mean and why queue is not enough. > > > > If queue is not specified, dest_id defines which element (vsi) will get= the > packet. > > If queue is specified, the queue need belong to the destination element= . >=20 > You really need to define what is a vsi id and pool id. These notions are= not > known in the API layer. >=20 > > > > + uint16_t queue; /**< queue assign to if TO QUEUE fl= ag is set > > > */ > > > > > > TO QUEUE is not defined. Is it really needed? > > > > > TO QUEUE is just the flag RTE_CONTROL_PACKET_FLAGS_TO_QUEUE > above. >=20 > OK, please use the same wording or users will get lost. >=20 > -- > Thomas