From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <thomas.monjalon@6wind.com>
Received: from mail-lf0-f41.google.com (mail-lf0-f41.google.com
 [209.85.215.41]) by dpdk.org (Postfix) with ESMTP id B9D9A2B9A
 for <dev@dpdk.org>; Thu, 13 Oct 2016 09:08:55 +0200 (CEST)
Received: by mail-lf0-f41.google.com with SMTP id x79so123774256lff.0
 for <dev@dpdk.org>; Thu, 13 Oct 2016 00:08:55 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=6wind-com.20150623.gappssmtp.com; s=20150623;
 h=from:to:cc:subject:date:message-id:user-agent:in-reply-to
 :references:mime-version:content-transfer-encoding;
 bh=QnWPk4QyLp9FnDG4vfKptol9cTSFyP7rgAlcBcTcZ1g=;
 b=bNMKHywbkd2GN0e8stj7QCkRrb/nk6acBa+Ha35dPk8VihuOqPgiEJLj9P4sgL6OAS
 J0KJWJ3ExIgKdOBNApngaPGd7MqDbOMptqjE/PjOReCmVgZ3ZzX6e2sPVh4+MvIrpIYc
 y7RwiFwhq4UMio4Af4A4aqvOIy3SjyPJGgVvQWzpWGTyNOeytkN3VG4+s6Cr2PA45HKI
 viNeT2/bWXhWNauLu4s3wmb89pjzGLaCo2KjqfZJDBthzq1zH6upzxGpX4iGaUCnh5LM
 uwdhxRuUxYIUTLGOIicQhA/jriJDwTjKVY3HnKglf6h7NeuotFiz+K87AROYduxtc4AO
 jiTg==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20130820;
 h=x-gm-message-state:from:to:cc:subject:date:message-id:user-agent
 :in-reply-to:references:mime-version:content-transfer-encoding;
 bh=QnWPk4QyLp9FnDG4vfKptol9cTSFyP7rgAlcBcTcZ1g=;
 b=eEz2180YalOzxowTqkCmNBUMYeNWxP22vHJ5qkefGH/dTYbIX2QvHZq5EfHp9AWxPI
 cTdw5czJO9xAl9m1TrPZTZGQ34uJmDkv+NdMslaKF063v9VG2s5UMAsMP4v9XGI3IdBE
 NoGeR9KwtGGiIDkP4vqgMj/TaTGFysLwL7yzlVvgElvGM6eq9whCb16MB+mpF4LCrmW2
 KRvn+TrlEAzSNesLsWXuNPaYhWZ34EC4yL9Lqj5GM4Pa1E0lEUi94uvbc426OLaezXHf
 CPew1/hFXE4moXxcUADnBpgKslw9/FO1s48JRfv6C1IWMMi7FEeCarwNVrYSWDJwP2+e
 BfaA==
X-Gm-Message-State: AA6/9Rm/2/XvVfQV6TfJZgTsUoPdEoX+WFtsT+42E75DSueqjsRnVrKbfFdaa2tf7b7AjXb5
X-Received: by 10.25.211.142 with SMTP id k136mr6902984lfg.45.1476342535198;
 Thu, 13 Oct 2016 00:08:55 -0700 (PDT)
Received: from xps13.localnet (184.203.134.77.rev.sfr.net. [77.134.203.184])
 by smtp.gmail.com with ESMTPSA id z84sm1813862lfa.23.2016.10.13.00.08.54
 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);
 Thu, 13 Oct 2016 00:08:54 -0700 (PDT)
From: Thomas Monjalon <thomas.monjalon@6wind.com>
To: Tomasz Kulasek <tomaszx.kulasek@intel.com>
Cc: dev@dpdk.org, konstantin.ananyev@intel.com
Date: Thu, 13 Oct 2016 09:08:52 +0200
Message-ID: <1506813.gKiMgNqmq7@xps13>
User-Agent: KMail/4.14.10 (Linux/4.5.4-1-ARCH; KDE/4.14.11; x86_64; ; )
In-Reply-To: <1995417.Vz0qMdTBBe@xps13>
References: <20160928111052.9968-1-tomaszx.kulasek@intel.com>
 <20160930090039.10164-2-tomaszx.kulasek@intel.com> <1995417.Vz0qMdTBBe@xps13>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
Subject: Re: [dpdk-dev] [PATCH v4 1/6] ethdev: add Tx preparation
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Thu, 13 Oct 2016 07:08:56 -0000

Hi Tomasz,

Any news?
Sorry to speed up, we are very very late for RC1.

2016-10-10 16:08, Thomas Monjalon:
> Hi,
> 
> Now that the feature seems to meet a consensus, I've looked at it more
> closely before integrating. Sorry if it appears like a late review.
> 
> 2016-09-30 11:00, Tomasz Kulasek:
> > Added API for `rte_eth_tx_prep`
> 
> I would love to read the usability and performance considerations here.
> No need for something as long as the cover letter. Just few lines
> about why it is needed and why it is a good choice for performance.
> 
> > uint16_t rte_eth_tx_prep(uint8_t port_id, uint16_t queue_id,
> > 	struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
> > 
> > Added fields to the `struct rte_eth_desc_lim`:
> > 
> > 	uint16_t nb_seg_max;
> > 		/**< Max number of segments per whole packet. */
> > 
> > 	uint16_t nb_mtu_seg_max;
> > 		/**< Max number of segments per one MTU */
> [...]
> > +#else
> > +
> > +static inline uint16_t
> > +rte_eth_tx_prep(uint8_t port_id __rte_unused, uint16_t queue_id __rte_unused,
> > +		struct rte_mbuf **tx_pkts __rte_unused, uint16_t nb_pkts)
> 
> Doxygen is failing here.
> Have you tried to move __rte_unused before the identifier?
> 
> [...]
> > +#define PKT_TX_OFFLOAD_MASK (    \
> > +		PKT_TX_IP_CKSUM |        \
> > +		PKT_TX_L4_MASK |         \
> > +		PKT_TX_OUTER_IP_CKSUM |  \
> > +		PKT_TX_TCP_SEG |         \
> > +		PKT_TX_QINQ_PKT |        \
> > +		PKT_TX_VLAN_PKT)
> 
> We should really stop adding some public constants without the proper
> RTE prefix.
> And by the way, should not we move such flags into rte_net?

Do not worry with this comment. It was just a thought which could be
addressed in a separate patch by someone else.

> [...]
> > -SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include := rte_ip.h rte_tcp.h rte_udp.h rte_sctp.h rte_icmp.h rte_arp.h
> > +SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include := rte_ip.h rte_tcp.h rte_udp.h rte_sctp.h rte_icmp.h rte_arp.h rte_pkt.h
> 
> You can use the += operator on a new line for free :)
> 
> No more comments, the rest seems OK. Thanks