From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f48.google.com (mail-wm0-f48.google.com [74.125.82.48]) by dpdk.org (Postfix) with ESMTP id B7D659382 for ; Tue, 5 Jan 2016 17:18:55 +0100 (CET) Received: by mail-wm0-f48.google.com with SMTP id f206so29154336wmf.0 for ; Tue, 05 Jan 2016 08:18:55 -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=y4vdUZg0e6iP59VvwTl1zd8/eu0/I//0PTPfEOYwLQo=; b=yEY/XsmSvf/oZ+u1NaZHcVgeHTwcaN1gz4JooFEMZN8Xgl5TtwaLLmZvWCAPD+RkMo 4ZTUKnRxU1ZKUmp7vnEJ8peT8QG3+A/JTmgVWfj9WKFhD6oI86rngzikgXX6SFcNJJxF YFziuTt6AqBVwxWRDn8imBKIBqQBx82hZ0ZcqNhZx8QwHMwS2mP0dI9r8oAkePM5TnEI MZo7MaBlPziKFztNKJ2GxUwTVoU8zykI4Cuwd2X7B906sr0yDFPY2UWNya1cFAzNnRGd MlCpiVkrhc+6IsNHp1QaMb5l8c6j+brValcKL73waRA91RA9HnQBPTzWeQnJmCE4s21+ OQMg== 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=y4vdUZg0e6iP59VvwTl1zd8/eu0/I//0PTPfEOYwLQo=; b=fR/RQD0dLfJlDkvOMmutBDfuLKWog0/8wUvGE7/PSq1PkXXH+d77vJYI851MZgQ9o0 Pliw76YXifO7VReaBXJHRq7sqam5EoNjbJ2Pts1vxTJnoiO7XB4lScBQrb0Vvj8U2jBy tZt9AKst13PJcTGSEuelw/YRrrJMfXWAkEXrqJepnbC035ZfB6iV2j1ZVi2Z2p93MkHW Y+fDFaBv9Z2GGfs94207Ke3WFNBaAbivWcwOP3ko4ELuIIblTTuBXzg8O8Mtm3QAmIQ6 eb137xYjHxj0rK2frSE7yP4O4X3jgTExpywUdfT8MYz1zBeI8ZEdrWUp18iFYM1H1ZhF QHuQ== X-Gm-Message-State: ALoCoQnQT7pPj3b0AkY3vxTxG+UHfhtz5HQyNL01xjGMxdF0byFGtIzME4wSKOH00065vWv/YntDXKT9Rj+8rHhNFWJjGFGUAg== X-Received: by 10.28.4.139 with SMTP id 133mr4733006wme.21.1452010735587; Tue, 05 Jan 2016 08:18:55 -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 z137sm4278510wmc.8.2016.01.05.08.18.53 (version=TLSv1/SSLv3 cipher=OTHER); Tue, 05 Jan 2016 08:18:54 -0800 (PST) Date: Tue, 5 Jan 2016 17:18:36 +0100 From: Adrien Mazarguil To: "Tan, Jianfeng" Message-ID: <20160105161835.GF12095@6wind.com> Mail-Followup-To: "Tan, Jianfeng" , dev@dpdk.org References: <1451544799-70776-1-git-send-email-jianfeng.tan@intel.com> <1451544799-70776-9-git-send-email-jianfeng.tan@intel.com> <20160104111114.GS3806@6wind.com> <568B3394.7020207@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <568B3394.7020207@intel.com> Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH 08/12] pmd/mlx4: add dev_ptype_info_get implementation 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: Tue, 05 Jan 2016 16:18:55 -0000 On Tue, Jan 05, 2016 at 11:08:04AM +0800, Tan, Jianfeng wrote: > > > On 1/4/2016 7:11 PM, Adrien Mazarguil wrote: > >Hi Jianfeng, > > > >I'm only commenting the mlx4/mlx5 bits in this message, see below. > > > >On Thu, Dec 31, 2015 at 02:53:15PM +0800, Jianfeng Tan wrote: > >>Signed-off-by: Jianfeng Tan > >>--- > >> drivers/net/mlx4/mlx4.c | 27 +++++++++++++++++++++++++++ > >> 1 file changed, 27 insertions(+) > >> > >>diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c > >>index 207bfe2..85afa32 100644 > >>--- a/drivers/net/mlx4/mlx4.c > >>+++ b/drivers/net/mlx4/mlx4.c > >>@@ -2836,6 +2836,8 @@ rxq_cleanup(struct rxq *rxq) > >> * @param flags > >> * RX completion flags returned by poll_length_flags(). > >> * > >>+ * @note: fix mlx4_dev_ptype_info_get() if any change here. > >>+ * > >> * @return > >> * Packet type for struct rte_mbuf. > >> */ > >>@@ -4268,6 +4270,30 @@ mlx4_dev_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *info) > >> priv_unlock(priv); > >> } > >>+static int > >>+mlx4_dev_ptype_info_get(struct rte_eth_dev *dev, uint32_t ptype_mask, > >>+ uint32_t ptypes[]) Note this line is not properly indented (uint32_t should be aligned like the rest of the file). > >>+{ > >>+ int num = 0; > >>+ > >>+ if ((dev->rx_pkt_burst == mlx4_rx_burst) > >>+ || (dev->rx_pkt_burst == mlx4_rx_burst_sp)) { I prefer operators/separators at the end of the previous line, indentation should be fixed as well. > >>+ /* refers to rxq_cq_to_pkt_type() */ > >>+ if ((ptype_mask & RTE_PTYPE_L3_MASK) == RTE_PTYPE_L3_MASK) { > >>+ ptypes[num++] = RTE_PTYPE_L3_IPV4; > >>+ ptypes[num++] = RTE_PTYPE_L3_IPV6; > >>+ } > >>+ > >>+ if ((ptype_mask & RTE_PTYPE_INNER_L3_MASK) == RTE_PTYPE_INNER_L3_MASK) { > >>+ ptypes[num++] = RTE_PTYPE_INNER_L3_IPV4; > >>+ ptypes[num++] = RTE_PTYPE_INNER_L3_IPV6; > >>+ } > >>+ } else > >>+ num = -ENOTSUP; > >>+ > >>+ return num; > >>+} > >I think checking for mlx4_rx_burst and mlx4_rx_burst_sp is unnecessary at > >the moment, all RX burst functions do update the packet_type field, no need > >for extra complexity. > > > >Same comment for mlx5. > > Hi Mazarguil, > > My original thought is that rx_pkt_burst could be also set as > removed_rx_burst, which does not make sense indeed > because it's only possible when the device is closed. Yes, indeed. > Another consideration is to keep same style with other devices. Each > kind of device could have several rx burst functions. > So current implementation can keep extensibility to add new rx burst > functions. How do you think of it? OK, that makes sense. Please check my above comments about coding style/indents (I know I'm annoying). > >>+ > >> /** > >> * DPDK callback to get device statistics. > >> * > >>@@ -4989,6 +5015,7 @@ static const struct eth_dev_ops mlx4_dev_ops = { > >> .stats_reset = mlx4_stats_reset, > >> .queue_stats_mapping_set = NULL, > >> .dev_infos_get = mlx4_dev_infos_get, > >>+ .dev_ptypes_info_get = mlx4_dev_ptype_info_get, > >> .vlan_filter_set = mlx4_vlan_filter_set, > >> .vlan_tpid_set = NULL, > >> .vlan_strip_queue_set = NULL, > >>-- > >>2.1.4 > >> > -- Adrien Mazarguil 6WIND