From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f53.google.com (mail-wm0-f53.google.com [74.125.82.53]) by dpdk.org (Postfix) with ESMTP id 6C1D7902 for ; Fri, 15 Jan 2016 14:59:07 +0100 (CET) Received: by mail-wm0-f53.google.com with SMTP id f206so21421747wmf.0 for ; Fri, 15 Jan 2016 05:59:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:mail-followup-to:references :mime-version:content-type:content-disposition:in-reply-to; bh=bAYGvEu4kFqdhsDsZBzf3sdhYa2IXBcfBpeJOyubDSg=; b=hyLJdgRND7VI428cD/2M5NZrUxTmllu4ZG4L2YrB/S0HxKYmbRGgjNUaPVy3rN0Zfw yTEiSsoFee+iTHNVGwXzlNs3PPNOytah/xS7XqYYC90yzNrHTNHyRwKZUJATUWx4oclC wqHpffm5HtknOCYHXAIVsh0wmMhIvyvh+aMwrGLEntwK2kOi4Wj4bK5+opJjb3bJMqZF 7xJ/ucHnt0SjnniJ5pHKHupItHp12VTqjKWPPzrY11WJkM14f5F73KX00V6UfjJ163IC +HSt1kDzUCGsdXuWOcifUnoWdquSsJRbB3zMDBb6l4mVyoqv8Ql+t5ReqqH6oOgRA2zN XqGg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id :mail-followup-to:references:mime-version:content-type :content-disposition:in-reply-to; bh=bAYGvEu4kFqdhsDsZBzf3sdhYa2IXBcfBpeJOyubDSg=; b=RfxGCvzU3kAXUFErRQlWLfxdslwG8PrtM1+wZ3C522PYmHkX/ju+G575Eht3vkCD/W A0BS34+2kJaDd6ucU4cKhLjdYNehPg+4lBkQC31mYKC/uxl43ctrrvkW5ie3+al0j1w8 4WpBU/5L8Y9fO28Ja+m1YiqbxNZzLHKWTf/ATb/jriCFkWPPwiKzoUhF0xqn7ab1g+hy 53Kz4gQeE6NyhBZS0DYDlDmaer8chA/fVa4k2aVWd2cLqHqHKo0VVRCUiXpbzoiHuIL+ 9tqEtHQLR3KBhayP4qyEbvlx+2wvCPbPdVy/qrPWGUVuuFftKJlA9tsanT9Sicy1xCim auhg== X-Gm-Message-State: AG10YOTc3jzCAZieQ3hfR2EJLEsn5PtFftUSyC3T46tn8+E3996Z6K7Rk8qU7RXhlSk6ZbQ/ X-Received: by 10.194.133.10 with SMTP id oy10mr2645539wjb.93.1452866347170; Fri, 15 Jan 2016 05:59:07 -0800 (PST) Received: from 6wind.com (guy78-3-82-239-227-177.fbx.proxad.net. [82.239.227.177]) by smtp.gmail.com with ESMTPSA id bg10sm10721461wjb.46.2016.01.15.05.59.03 (version=TLSv1/SSLv3 cipher=OTHER); Fri, 15 Jan 2016 05:59:04 -0800 (PST) Date: Fri, 15 Jan 2016 14:58:46 +0100 From: Adrien Mazarguil To: Jianfeng Tan Message-ID: <20160115135846.GO12095@6wind.com> Mail-Followup-To: Jianfeng Tan , dev@dpdk.org 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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1452836759-63540-2-git-send-email-jianfeng.tan@intel.com> 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 13:59:07 -0000 Hi Jianfeng, a few comments below. 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 type 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_ethdev.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 = 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 = &rte_eth_devices[port_id]; > + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_ptype_info_get, -ENOTSUP); > + ret = (*dev->dev_ops->dev_ptype_info_get)(dev, all_ptypes); > + > + for (i = 0, j = 0; i < ret && j < num; ++i) > + if (all_ptypes[i] & ptype_mask) > + ptypes[j++] = all_ptypes[i]; > + > + return ret; > +} 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. If you really do not want to pass the size, you have to force the array type size onto callbacks using a pointer to the array otherwise they look unsafe (and are actually unsafe when not called from the rte_eth_dev wrapper), something like this: int (*dev_ptype_info_get)(uint8_t port_id, uint32_t (*ptypes)[RTE_PTYPE_MAX_NUM]); 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. But since people usually do not like this syntax, I think passing num and letting callbacks check for overflow themselves on the user-provided ptypes 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). 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 filtering a ptypes array for applications that need it: /* 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[], int num); Usage would be like: int ptypes_num = rte_eth_dev_get_ptype_info(42, NULL, 0); if (ptypes_num <= 0) goto err; uint32_t ptypes[ptypes_num]; rte_eth_dev_get_ptype_info(42, ptypes, ptypes_num); ptypes_num = rte_eth_dev_ptypes_filter(RTE_PTYPE_INNER_L4_MASK, ptypes, ptypes_num); if (ptypes_num <= 0) goto err; /* process ptypes... */ What about this? > + > 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_ethdev.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_eth_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 Setup. */ > @@ -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 37 for now > + * and reserve some space for new ptypes > + */ > +#define RTE_PTYPE_MAX_NUM 64 This macro should not be needed if the num argument is kept. Applications should only rely on returned values. > + > +/** > * Check if the (outer) L3 header is IPv4. To avoid comparing IPv4 types 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 > -- Adrien Mazarguil 6WIND