[dpdk-dev] [PATCH v3 1/6] ethdev: add Tx preparation

[Ananyev, Konstantin]

Hi Tomasz,

> -----Original Message-----
> From: Kulasek, TomaszX
> Sent: Wednesday, September 28, 2016 12:11 PM
> To: dev at dpdk.org
> Cc: Ananyev, Konstantin <konstantin.ananyev at intel.com>; Kulasek, TomaszX <tomaszx.kulasek at intel.com>
> Subject: [PATCH v3 1/6] ethdev: add Tx preparation
> 
> Added API for `rte_eth_tx_prep`
> 
> 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 */
> 
> Created `rte_pkt.h` header with common used functions:
> 
> int rte_validate_tx_offload(struct rte_mbuf *m)
> 	to validate general requirements for tx offload in packet such a
> 	flag completness. In current implementation this function is called
> 	optionaly when RTE_LIBRTE_ETHDEV_DEBUG is enabled.
> 
> int rte_phdr_cksum_fix(struct rte_mbuf *m)
> 	to fix pseudo header checksum for TSO and non-TSO tcp/udp packets
> 	before hardware tx checksum offload.
> 	 - for non-TSO tcp/udp packets full pseudo-header checksum is
> 	   counted and set.
> 	 - for TSO the IP payload length is not included.
> 
> Signed-off-by: Tomasz Kulasek <tomaszx.kulasek at intel.com>
> ---
>  config/common_base            |    1 +
>  lib/librte_ether/rte_ethdev.h |   85 ++++++++++++++++++++++++++
>  lib/librte_mbuf/rte_mbuf.h    |    8 +++
>  lib/librte_net/Makefile       |    2 +-
>  lib/librte_net/rte_pkt.h      |  133 +++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 228 insertions(+), 1 deletion(-)  create mode 100644 lib/librte_net/rte_pkt.h
> 

....

> diff --git a/lib/librte_net/rte_pkt.h b/lib/librte_net/rte_pkt.h new file mode 100644 index 0000000..72903ac
> --- /dev/null
> +++ b/lib/librte_net/rte_pkt.h
> @@ -0,0 +1,133 @@
> +/*-
> + *   BSD LICENSE
> + *
> + *   Copyright(c) 2016 Intel Corporation. All rights reserved.
> + *   All rights reserved.
> + *
> + *   Redistribution and use in source and binary forms, with or without
> + *   modification, are permitted provided that the following conditions
> + *   are met:
> + *
> + *     * Redistributions of source code must retain the above copyright
> + *       notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above copyright
> + *       notice, this list of conditions and the following disclaimer in
> + *       the documentation and/or other materials provided with the
> + *       distribution.
> + *     * Neither the name of Intel Corporation nor the names of its
> + *       contributors may be used to endorse or promote products derived
> + *       from this software without specific prior written permission.
> + *
> + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#ifndef _RTE_PKT_H_
> +#define _RTE_PKT_H_
> +
> +#include <rte_ip.h>
> +#include <rte_udp.h>
> +#include <rte_tcp.h>
> +#include <rte_sctp.h>
> +
> +/**
> + * Validate general requirements for tx offload in packet.
> + */
> +static inline int
> +rte_validate_tx_offload(struct rte_mbuf *m) {
> +	uint64_t ol_flags = m->ol_flags;
> +
> +	/* Does packet set any of available offloads? */
> +	if (!(ol_flags & PKT_TX_OFFLOAD_MASK))
> +		return 0;
> +
> +	/* IP checksum can be counted only for IPv4 packet */
> +	if ((ol_flags & PKT_TX_IP_CKSUM) && (ol_flags & PKT_TX_IPV6))
> +		return -EINVAL;
> +
> +	if (ol_flags & (PKT_TX_L4_MASK | PKT_TX_TCP_SEG))

Not sure what you are trying to test here?
Is that PKT_TX_TCP_SEG is set?
If so, then the test condition doesn't look correct to me.

> +		/* IP type not set */
> +		if (!(ol_flags & (PKT_TX_IPV4 | PKT_TX_IPV6)))
> +			return -EINVAL;
> +
> +	if (ol_flags & PKT_TX_TCP_SEG)
> +		/* PKT_TX_IP_CKSUM offload not set for IPv4 TSO packet */
> +		if ((m->tso_segsz == 0) ||
> +				((ol_flags & PKT_TX_IPV4) && !(ol_flags & PKT_TX_IP_CKSUM)))
> +			return -EINVAL;
> +

Why not just:
If ((ol_flags & PKT_TX_L4_MASK) == PKT_TX_TCP_SEG) {

         uint64_t f = ol_flags & PKT_TX_L4_MASK;

         if ((f  & (PKT_TX_IPV4 | PKT_TX_IPV6)) == 0 || f == PKT_TX_IPV4 || m->tso_segsz == 0)
		return -EINVAL;
}

Instead of 2 ifs around TCP_SEG above?

Konstantin