From mboxrd@z Thu Jan 1 00:00:00 1970 From: Adrien Mazarguil Subject: Re: [PATCH v4] net/mlx4: enhance Rx packet type offloads Date: Tue, 7 Nov 2017 09:43:27 +0100 Message-ID: <20171107084327.GC24849@6wind.com> References: <1509902816-26299-1-git-send-email-motih@mellanox.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: dev@dpdk.org To: Moti Haimovsky Return-path: Received: from mail-wm0-f47.google.com (mail-wm0-f47.google.com [74.125.82.47]) by dpdk.org (Postfix) with ESMTP id 408C81B5F2 for ; Tue, 7 Nov 2017 09:43:40 +0100 (CET) Received: by mail-wm0-f47.google.com with SMTP id r68so1977367wmr.1 for ; Tue, 07 Nov 2017 00:43:40 -0800 (PST) Content-Disposition: inline In-Reply-To: <1509902816-26299-1-git-send-email-motih@mellanox.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Sun, Nov 05, 2017 at 07:26:56PM +0200, Moti Haimovsky wrote: > This patch enhances the Rx packet type offload to also report the L4 > protocol information in the hw ptype filled by the PMD for each received > packet. > > Signed-off-by: Moti Haimovsky Patch looks fine therefore: Acked-by: Adrien Mazarguil Although in the end you didn't initialize mlx4_ptype_table[] using a loop (went all the way back to static array initialization) and I'm still wondering why since you never replied on that topic. Another comment: please remember to send patch updates as replies to their previous iteration. If you search for "enhance Rx packet type offloads" in [1], you notice they're not part of the same thread. While minor this doesn't help the review process. Regarding your previous reply about mlx4_dev_supported_ptypes_get(): [...] > > > + > > > + if (dev->rx_pkt_burst == mlx4_rx_burst) > > > + return ptypes; > > > + return NULL; > > > > How about just returning the array regardless? > > From DPDK documentation (and as done in other drivers): > http://dpdk.org/doc/api/rte__ethdev_8h.html#aa63202d322632467f9cc5fc460e04ea4 > Note > Better to invoke this API after the device is already started or rx burst > function is decided, to obtain correct supported ptypes. > if a given PMD does not report what ptypes it supports, then the supported > ptype count is reported as 0. > > In our case rx_pkt_burst can also point to mlx4_rx_burst_removed Right, and it doesn't hurt. My point was that mlx4_rx_burst_removed() is set as a safety when closing/stopping the device, no more Rx in that case. The mlx4 PMD currently exposes a single Rx function which does support everything, not multiple optimized ones with fewer capabilities, which is the actual reason behind checking the current Rx function. [1] http://dpdk.org/ml/archives/dev/2017-November/ -- Adrien Mazarguil 6WIND