From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf0-f42.google.com (mail-lf0-f42.google.com [209.85.215.42]) by dpdk.org (Postfix) with ESMTP id A5D8FFFA for ; Mon, 10 Oct 2016 16:08:20 +0200 (CEST) Received: by mail-lf0-f42.google.com with SMTP id b75so126518429lfg.3 for ; Mon, 10 Oct 2016 07:08:20 -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=jAl7WK3DIimOVE+maxzAtk7FzPdeBdcsSUTcQ9EnKpM=; b=hWIiQbD8uUhPrz53fVWZEsXo7iNd3Cn0BW8fkD06Ev9dGtAum7TNxXLO5SJ+ZDhOGy aAkXfSslWc0kSquayZGHgbDGjrsI+V4J7nmalMTOaOx5SS+OdCcyJmsH0FZfar6slfyW mRJJkdOCjRP/iJuNPUrHtWnqwG3WVy2oRpB+fokcFUv7ci8s1x01Ih0zhhfjShUBYVur osJJX7lsUtrA4A6Bz85d7vq9ypeuTJL+x4PP0jelmYPemCfAx46vQEHMYWetxr3ikcGf IuVsmljDy4HAsn3qhNI33IdaT8KUWiNuMPIH7f/ZFq7G1QgXPrrcSFS03P7y4n7VUrNM yS0A== 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=jAl7WK3DIimOVE+maxzAtk7FzPdeBdcsSUTcQ9EnKpM=; b=IaxYCw51J+WPz9glIMvEiVXDUfZm23ftnpTK6CkC9fbxH8reWfZXsFhGQAqTWVbht0 VENhXZfwGpdPbg2AJ+NzbG+LjUy9QtZiCmlJsCDksfdVkZVIn8GV4BSQUqt8ixJrV3jT tslpfe14vJ/f3jfhdieZeVPw68PNKLwPqQobgQnjKVVmX+HiXsytGxVLj4ce5iknBXUB OTw+M76hyiHaw5cPLrP0Ow8ystoa0C/fdlCvN1F1NG/opkIO1QQS6Xq+ZN2+ErZlDIvM 9gHPvR0I8wWoCdZUAGPC0U1LGHx+TrVXCROofj2Sqvca9Y/jf5bhIyXkBBpvcd1lNlZg 2JPQ== X-Gm-Message-State: AA6/9Rl941ywSbK14CXFGtxNKKfNagGD9zgf+F3kzyxWergDL1iN/OpeVzKX8oCFRxnVZYCP X-Received: by 10.25.17.37 with SMTP id g37mr12619933lfi.43.1476108499530; Mon, 10 Oct 2016 07:08:19 -0700 (PDT) Received: from xps13.localnet (184.203.134.77.rev.sfr.net. [77.134.203.184]) by smtp.gmail.com with ESMTPSA id h7sm5313066ljh.21.2016.10.10.07.08.18 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 10 Oct 2016 07:08:18 -0700 (PDT) From: Thomas Monjalon To: Tomasz Kulasek Cc: dev@dpdk.org, konstantin.ananyev@intel.com Date: Mon, 10 Oct 2016 16:08:17 +0200 Message-ID: <1995417.Vz0qMdTBBe@xps13> User-Agent: KMail/4.14.10 (Linux/4.5.4-1-ARCH; KDE/4.14.11; x86_64; ; ) In-Reply-To: <20160930090039.10164-2-tomaszx.kulasek@intel.com> References: <20160928111052.9968-1-tomaszx.kulasek@intel.com> <20160930090039.10164-1-tomaszx.kulasek@intel.com> <20160930090039.10164-2-tomaszx.kulasek@intel.com> 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 10 Oct 2016 14:08:20 -0000 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? [...] > -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