From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 5170E8E98 for ; Fri, 15 Jan 2016 16:11:22 +0100 (CET) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga101.jf.intel.com with ESMTP; 15 Jan 2016 07:11:21 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,300,1449561600"; d="scan'208";a="727724830" Received: from irsmsx108.ger.corp.intel.com ([163.33.3.3]) by orsmga003.jf.intel.com with ESMTP; 15 Jan 2016 07:11:20 -0800 Received: from irsmsx155.ger.corp.intel.com (163.33.192.3) by IRSMSX108.ger.corp.intel.com (163.33.3.3) with Microsoft SMTP Server (TLS) id 14.3.248.2; Fri, 15 Jan 2016 15:11:19 +0000 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.104]) by irsmsx155.ger.corp.intel.com ([169.254.14.138]) with mapi id 14.03.0248.002; Fri, 15 Jan 2016 15:11:18 +0000 From: "Ananyev, Konstantin" To: Adrien Mazarguil , "Tan, Jianfeng" Thread-Topic: [dpdk-dev] [PATCH v2 01/12] ethdev: add API to query packet type filling info Thread-Index: AQHRT5LBzIQ8xSOrQkuYMeiXqH5ZgJ78mo4AgAAQd+A= Date: Fri, 15 Jan 2016 15:11:18 +0000 Message-ID: <2601191342CEEE43887BDE71AB97725836AE619F@irsmsx105.ger.corp.intel.com> References: <1451544799-70776-1-git-send-email-jianfeng.tan@intel.com> <1452836759-63540-1-git-send-email-jianfeng.tan@intel.com> <1452836759-63540-2-git-send-email-jianfeng.tan@intel.com> <20160115135846.GO12095@6wind.com> In-Reply-To: <20160115135846.GO12095@6wind.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.182] 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 01/12] ethdev: add API to query packet type filling info 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: Fri, 15 Jan 2016 15:11:23 -0000 Hi lads, > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Adrien Mazarguil > Sent: Friday, January 15, 2016 1:59 PM > To: Tan, Jianfeng > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2 01/12] ethdev: add API to query packet = type filling info >=20 > Hi Jianfeng, a few comments below. >=20 > On Fri, Jan 15, 2016 at 01:45:48PM +0800, Jianfeng Tan wrote: > > Add a new API rte_eth_dev_get_ptype_info to query wether/what packet ty= pe will > > be filled by given pmd rx burst function. > > > > Signed-off-by: Jianfeng Tan > > --- > > lib/librte_ether/rte_ethdev.c | 20 ++++++++++++++++++++ > > lib/librte_ether/rte_ethdev.h | 27 +++++++++++++++++++++++++++ > > lib/librte_mbuf/rte_mbuf.h | 6 ++++++ > > 3 files changed, 53 insertions(+) > > > > diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethde= v.c > > index ed971b4..cd34f46 100644 > > --- a/lib/librte_ether/rte_ethdev.c > > +++ b/lib/librte_ether/rte_ethdev.c > > @@ -1614,6 +1614,26 @@ rte_eth_dev_info_get(uint8_t port_id, struct rte= _eth_dev_info *dev_info) > > dev_info->driver_name =3D dev->data->drv_name; > > } > > > > +int > > +rte_eth_dev_get_ptype_info(uint8_t port_id, uint32_t ptype_mask, > > + uint32_t ptypes[], int num) > > +{ > > + int ret, i, j; > > + struct rte_eth_dev *dev; > > + uint32_t all_ptypes[RTE_PTYPE_MAX_NUM]; > > + > > + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); > > + dev =3D &rte_eth_devices[port_id]; > > + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_ptype_info_get, -ENOTSUP); > > + ret =3D (*dev->dev_ops->dev_ptype_info_get)(dev, all_ptypes); > > + > > + for (i =3D 0, j =3D 0; i < ret && j < num; ++i) > > + if (all_ptypes[i] & ptype_mask) > > + ptypes[j++] =3D all_ptypes[i]; > > + > > + return ret; > > +} >=20 > It's a good thing that the size of ptypes[] can be provided, but I think = num > should be passed to the dev_ptype_info_get() callback as well. >=20 > If you really do not want to pass the size, you have to force the array t= ype > size onto callbacks using a pointer to the array otherwise they look unsa= fe > (and are actually unsafe when not called from the rte_eth_dev wrapper), > something like this: >=20 > int (*dev_ptype_info_get)(uint8_t port_id, uint32_t (*ptypes)[RTE_PTYPE_= MAX_NUM]); >=20 > In which case you might as well drop the num argument from > rte_eth_dev_get_ptype_info() to use the same syntax. That way there is no > need to allocate a ptypes array on the stack twice. >=20 > But since people usually do not like this syntax, I think passing num and > letting callbacks check for overflow themselves on the user-provided ptyp= es > array directly is better. They have to return the total number of packet > types supported even when num is 0 (ptypes may be NULL in that case). >=20 > I understand the result needs to be temporarily stored somewhere for > filtering and for that purpose the entire size must be known in advance, > hence my previous suggestion for rte_eth_dev_get_ptype_info() to return > the total number of ptypes and providing a separate function for filterin= g > a ptypes array for applications that need it: >=20 > /* Return remaining number entries in ptypes[] after filtering it > * according to ptype_mask. */ > int rte_eth_dev_ptypes_filter(uint32_t ptype_mask, uint32_t ptypes[], in= t num); >=20 > Usage would be like: >=20 > int ptypes_num =3D rte_eth_dev_get_ptype_info(42, NULL, 0); >=20 > if (ptypes_num <=3D 0) > goto err; >=20 > uint32_t ptypes[ptypes_num]; >=20 > rte_eth_dev_get_ptype_info(42, ptypes, ptypes_num); > ptypes_num =3D rte_eth_dev_ptypes_filter(RTE_PTYPE_INNER_L4_MASK, ptypes= , ptypes_num); >=20 > if (ptypes_num <=3D 0) > goto err; >=20 > /* process ptypes... */ >=20 > What about this? Actually while thinking about it, we probably can make: const uint32_t * (*dev_ptype_info_get)(uint8_t port_id);=20 So PMD return to ethdev layer a pointer to a constant array of supported pt= ypes, terminated by RTE_PTYPE_UNKNOWN. =20 Then rte_eth_dev_get_ptype_info() will iterate over it, and fill array prov= ided by the user. all_pytpes =3D (*dev->dev_ops->dev_ptype_info_get)(dev); for (i =3D 0; all_ptypes[i] !=3D RTE_PTYPE_UNKNOWN; i++) { if (all_ptypes[i] & ptype_mask) { if (j < num) ptypes[j] =3D all_ptypes[i]; j++; } return j; Konstantin >=20 > > + > > void > > rte_eth_macaddr_get(uint8_t port_id, struct ether_addr *mac_addr) > > { > > diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethde= v.h > > index bada8ad..42f8188 100644 > > --- a/lib/librte_ether/rte_ethdev.h > > +++ b/lib/librte_ether/rte_ethdev.h > > @@ -1021,6 +1021,10 @@ typedef void (*eth_dev_infos_get_t)(struct rte_e= th_dev *dev, > > struct rte_eth_dev_info *dev_info); > > /**< @internal Get specific informations of an Ethernet device. */ > > > > +typedef int (*eth_dev_ptype_info_get_t)(struct rte_eth_dev *dev, > > + uint32_t ptypes[]); > > +/**< @internal Get ptype info of eth_rx_burst_t. */ > > + > > typedef int (*eth_queue_start_t)(struct rte_eth_dev *dev, > > uint16_t queue_id); > > /**< @internal Start rx and tx of a queue of an Ethernet device. */ > > @@ -1347,6 +1351,7 @@ struct eth_dev_ops { > > eth_queue_stats_mapping_set_t queue_stats_mapping_set; > > /**< Configure per queue stat counter mapping. */ > > eth_dev_infos_get_t dev_infos_get; /**< Get device info. */ > > + eth_dev_ptype_info_get_t dev_ptype_info_get; /** Get ptype info */ > > mtu_set_t mtu_set; /**< Set MTU. */ > > vlan_filter_set_t vlan_filter_set; /**< Filter VLAN Setup. = */ > > vlan_tpid_set_t vlan_tpid_set; /**< Outer VLAN TPID S= etup. */ > > @@ -2273,6 +2278,28 @@ extern void rte_eth_dev_info_get(uint8_t port_id= , > > struct rte_eth_dev_info *dev_info); > > > > /** > > + * Retrieve the contextual information of an Ethernet device. > > + * > > + * @param port_id > > + * The port identifier of the Ethernet device. > > + * @param ptype_mask > > + * A hint of what kind of packet type which the caller is interested= in. > > + * @param ptypes > > + * An array of packet types to be filled with. > > + * @param num > > + * The size of ptypes array. > > + * @return > > + * - (>0) Number of ptypes supported. May be greater than param num = and > > + * caller needs to check that. > > + * - (0 or -ENOTSUP) if PMD does not fill the specified ptype. > > + * - (-ENODEV) if *port_id* invalid. > > + */ > > +extern int rte_eth_dev_get_ptype_info(uint8_t port_id, > > + uint32_t ptype_mask, > > + uint32_t ptypes[], > > + int num); > > + > > +/** > > * Retrieve the MTU of an Ethernet device. > > * > > * @param port_id > > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h > > index f234ac9..d116711 100644 > > --- a/lib/librte_mbuf/rte_mbuf.h > > +++ b/lib/librte_mbuf/rte_mbuf.h > > @@ -667,6 +667,12 @@ extern "C" { > > #define RTE_PTYPE_INNER_L4_MASK 0x0f000000 > > > > /** > > + * Total number of all kinds of RTE_PTYPE_*, except from *_MASK, is 3= 7 for now > > + * and reserve some space for new ptypes > > + */ > > +#define RTE_PTYPE_MAX_NUM 64 >=20 > This macro should not be needed if the num argument is kept. Applications > should only rely on returned values. >=20 > > + > > +/** > > * Check if the (outer) L3 header is IPv4. To avoid comparing IPv4 typ= es one by > > * one, bit 4 is selected to be used for IPv4 only. Then checking bit = 4 can > > * determine if it is an IPV4 packet. > > -- > > 2.1.4 > > >=20 > -- > Adrien Mazarguil > 6WIND