[dpdk-dev] [PATCH v2 2/2] ethdev: introduce Tx queue offloads API
Andrew Rybchenko
arybchenko at solarflare.com
Mon Sep 11 10:03:18 CEST 2017
On 09/10/2017 03:07 PM, Shahaf Shuler wrote:
> Introduce a new API to configure Tx offloads.
>
> In the new API, offloads are divided into per-port and per-queue
> offloads. The PMD reports capability for each of them.
> Offloads are enabled using the existing DEV_TX_OFFLOAD_* flags.
> To enable per-port offload, the offload should be set on both device
> configuration and queue configuration. To enable per-queue offload, the
> offloads can be set only on queue configuration.
>
> In addition the Tx offloads will be disabled by default and be
> enabled per application needs. This will much simplify PMD management of
> the different offloads.
>
> The new API does not have an equivalent for the below, benchmark
> specific, flags:
>
> - ETH_TXQ_FLAGS_NOREFCOUNT
> - ETH_TXQ_FLAGS_NOMULTMEMP
>
> Applications should set the ETH_TXQ_FLAGS_IGNORE flag on txq_flags
> field in order to move to the new API.
>
> The old Tx offloads API is kept for the meanwhile, in order to enable a
> smooth transition for PMDs and application to the new API.
>
> Signed-off-by: Shahaf Shuler <shahafs at mellanox.com>
> ---
>
> Note:
>
> In the special case were application is using the old API and the PMD
> has allready converted to the new one. If there are Tx offloads which can
> be set only per-port the queue setup may fail.
>
> I choose to treat this case as an exception considering all Tx offloads are
> currently defined to be per-queue. New ones to be added should require from
> the application to move to the new API as well.
>
> ---
> doc/guides/nics/features.rst | 8 ++++++
> lib/librte_ether/rte_ethdev.c | 59 +++++++++++++++++++++++++++++++++++++-
> lib/librte_ether/rte_ethdev.h | 32 ++++++++++++++++++++-
> 3 files changed, 97 insertions(+), 2 deletions(-)
>
> diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
> index f2c8497c2..bb25a1cee 100644
> --- a/doc/guides/nics/features.rst
> +++ b/doc/guides/nics/features.rst
> @@ -131,6 +131,7 @@ Lock-free Tx queue
> If a PMD advertises DEV_TX_OFFLOAD_MT_LOCKFREE capable, multiple threads can
> invoke rte_eth_tx_burst() concurrently on the same Tx queue without SW lock.
>
> +* **[uses] rte_eth_txq_conf**: ``offloads:DEV_TX_OFFLOAD_MT_LOCKFREE``.
It should be rte_eth_txconf here and below since renaming is postponed.
> * **[provides] rte_eth_dev_info**: ``tx_offload_capa:DEV_TX_OFFLOAD_MT_LOCKFREE``.
> * **[related] API**: ``rte_eth_tx_burst()``.
>
> @@ -220,6 +221,7 @@ TSO
>
> Supports TCP Segmentation Offloading.
>
> +* **[uses] rte_eth_txq_conf**: ``offloads:DEV_TX_OFFLOAD_TCP_TSO``.
> * **[uses] rte_eth_desc_lim**: ``nb_seg_max``, ``nb_mtu_seg_max``.
> * **[uses] mbuf**: ``mbuf.ol_flags:PKT_TX_TCP_SEG``.
> * **[uses] mbuf**: ``mbuf.tso_segsz``, ``mbuf.l2_len``, ``mbuf.l3_len``, ``mbuf.l4_len``.
> @@ -510,6 +512,7 @@ VLAN offload
> Supports VLAN offload to hardware.
>
> * **[uses] rte_eth_rxq_conf**: ``offloads:DEV_RX_OFFLOAD_VLAN_STRIP,DEV_RX_OFFLOAD_VLAN_FILTER,DEV_RX_OFFLOAD_VLAN_EXTEND``.
> +* **[uses] rte_eth_txq_conf**: ``offloads:DEV_TX_OFFLOAD_VLAN_INSERT``.
> * **[implements] eth_dev_ops**: ``vlan_offload_set``.
> * **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_VLAN_STRIPPED``, ``mbuf.vlan_tci``.
> * **[provides] rte_eth_dev_info**: ``rx_offload_capa:DEV_RX_OFFLOAD_VLAN_STRIP``,
> @@ -526,6 +529,7 @@ QinQ offload
> Supports QinQ (queue in queue) offload.
>
> * **[uses] rte_eth_rxq_conf**: ``offloads:DEV_RX_OFFLOAD_QINQ_STRIP``.
> +* **[uses] rte_eth_txq_conf**: ``offloads:DEV_TX_OFFLOAD_QINQ_INSERT``.
> * **[uses] mbuf**: ``mbuf.ol_flags:PKT_TX_QINQ_PKT``.
> * **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_QINQ_STRIPPED``, ``mbuf.vlan_tci``,
> ``mbuf.vlan_tci_outer``.
> @@ -541,6 +545,7 @@ L3 checksum offload
> Supports L3 checksum offload.
>
> * **[uses] rte_eth_rxq_conf**: ``offloads:DEV_RX_OFFLOAD_IPV4_CKSUM``.
> +* **[uses] rte_eth_txq_conf**: ``offloads:DEV_TX_OFFLOAD_IPV4_CKSUM``.
> * **[uses] mbuf**: ``mbuf.ol_flags:PKT_TX_IP_CKSUM``,
> ``mbuf.ol_flags:PKT_TX_IPV4`` | ``PKT_TX_IPV6``.
> * **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_IP_CKSUM_UNKNOWN`` |
> @@ -558,6 +563,7 @@ L4 checksum offload
> Supports L4 checksum offload.
>
> * **[uses] rte_eth_rxq_conf**: ``offloads:DEV_RX_OFFLOAD_UDP_CKSUM,DEV_RX_OFFLOAD_TCP_CKSUM``.
> +* **[uses] rte_eth_txq_conf**: ``offloads:DEV_TX_OFFLOAD_UDP_CKSUM,DEV_TX_OFFLOAD_TCP_CKSUM,DEV_TX_OFFLOAD_SCTP_CKSUM``.
> * **[uses] mbuf**: ``mbuf.ol_flags:PKT_TX_IPV4`` | ``PKT_TX_IPV6``,
> ``mbuf.ol_flags:PKT_TX_L4_NO_CKSUM`` | ``PKT_TX_TCP_CKSUM`` |
> ``PKT_TX_SCTP_CKSUM`` | ``PKT_TX_UDP_CKSUM``.
> @@ -576,6 +582,7 @@ MACsec offload
> Supports MACsec.
>
> * **[uses] rte_eth_rxq_conf**: ``offloads:DEV_RX_OFFLOAD_MACSEC_STRIP``.
> +* **[uses] rte_eth_txq_conf**: ``offloads:DEV_TX_OFFLOAD_MACSEC_INSERT``.
> * **[uses] mbuf**: ``mbuf.ol_flags:PKT_TX_MACSEC``.
> * **[provides] rte_eth_dev_info**: ``rx_offload_capa:DEV_RX_OFFLOAD_MACSEC_STRIP``,
> ``tx_offload_capa:DEV_TX_OFFLOAD_MACSEC_INSERT``.
> @@ -589,6 +596,7 @@ Inner L3 checksum
> Supports inner packet L3 checksum.
>
> * **[uses] rte_eth_rxq_conf**: ``offloads:DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM``.
> +* **[uses] rte_eth_txq_conf**: ``offloads:DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM``.
> * **[uses] mbuf**: ``mbuf.ol_flags:PKT_TX_IP_CKSUM``,
> ``mbuf.ol_flags:PKT_TX_IPV4`` | ``PKT_TX_IPV6``,
> ``mbuf.ol_flags:PKT_TX_OUTER_IP_CKSUM``,
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index b3c10701e..cd79cb1c9 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -1186,6 +1186,50 @@ rte_eth_rx_queue_setup(uint8_t port_id, uint16_t rx_queue_id,
> return ret;
> }
>
> +/**
> + * A conversion function from txq_flags API.
> + */
> +static void
> +rte_eth_convert_txq_flags(const uint32_t txq_flags, uint64_t *tx_offloads)
Maybe tx_offlaods should be simply return value of the function instead
of void.
Similar comment is applicable to rte_eth_convert_txq_offloads().
> +{
> + uint64_t offloads = 0;
> +
> + if (!(txq_flags & ETH_TXQ_FLAGS_NOMULTSEGS))
> + offloads |= DEV_TX_OFFLOAD_MULTI_SEGS;
> + if (!(txq_flags & ETH_TXQ_FLAGS_NOVLANOFFL))
> + offloads |= DEV_TX_OFFLOAD_VLAN_INSERT;
> + if (!(txq_flags & ETH_TXQ_FLAGS_NOXSUMSCTP))
> + offloads |= DEV_TX_OFFLOAD_SCTP_CKSUM;
> + if (!(txq_flags & ETH_TXQ_FLAGS_NOXSUMUDP))
> + offloads |= DEV_TX_OFFLOAD_UDP_CKSUM;
> + if (!(txq_flags & ETH_TXQ_FLAGS_NOXSUMTCP))
> + offloads |= DEV_TX_OFFLOAD_TCP_CKSUM;
> +
> + *tx_offloads = offloads;
> +}
> +
> +/**
> + * A conversion function from offloads API.
> + */
> +static void
> +rte_eth_convert_txq_offloads(const uint64_t tx_offloads, uint32_t *txq_flags)
> +{
> + uint32_t flags = 0;
> +
> + if (!(tx_offloads & DEV_TX_OFFLOAD_MULTI_SEGS))
> + flags |= ETH_TXQ_FLAGS_NOMULTSEGS;
> + if (!(tx_offloads & DEV_TX_OFFLOAD_VLAN_INSERT))
> + flags |= ETH_TXQ_FLAGS_NOVLANOFFL;
> + if (!(tx_offloads & DEV_TX_OFFLOAD_SCTP_CKSUM))
> + flags |= ETH_TXQ_FLAGS_NOXSUMSCTP;
> + if (!(tx_offloads & DEV_TX_OFFLOAD_UDP_CKSUM))
> + flags |= ETH_TXQ_FLAGS_NOXSUMUDP;
> + if (!(tx_offloads & DEV_TX_OFFLOAD_TCP_CKSUM))
> + flags |= ETH_TXQ_FLAGS_NOXSUMTCP;
> +
> + *txq_flags = flags;
> +}
> +
> int
> rte_eth_tx_queue_setup(uint8_t port_id, uint16_t tx_queue_id,
> uint16_t nb_tx_desc, unsigned int socket_id,
> @@ -1193,6 +1237,7 @@ rte_eth_tx_queue_setup(uint8_t port_id, uint16_t tx_queue_id,
> {
> struct rte_eth_dev *dev;
> struct rte_eth_dev_info dev_info;
> + struct rte_eth_txconf local_conf;
> void **txq;
>
> RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
> @@ -1237,8 +1282,20 @@ rte_eth_tx_queue_setup(uint8_t port_id, uint16_t tx_queue_id,
> if (tx_conf == NULL)
> tx_conf = &dev_info.default_txconf;
>
> + /*
> + * Convert between the offloads API to enable PMDs to support
> + * only one of them.
> + */
> + local_conf = *tx_conf;
> + if (tx_conf->txq_flags & ETH_TXQ_FLAGS_IGNORE)
> + rte_eth_convert_txq_offloads(tx_conf->offloads,
> + &local_conf.txq_flags);
> + else
> + rte_eth_convert_txq_flags(tx_conf->txq_flags,
> + &local_conf.offloads);
> +
> return (*dev->dev_ops->tx_queue_setup)(dev, tx_queue_id, nb_tx_desc,
> - socket_id, tx_conf);
> + socket_id, &local_conf);
> }
>
> void
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index f424cba04..4ad7dd059 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -692,6 +692,12 @@ struct rte_eth_vmdq_rx_conf {
> */
> struct rte_eth_txmode {
> enum rte_eth_tx_mq_mode mq_mode; /**< TX multi-queues mode. */
> + uint64_t offloads;
> + /**
It should be /**< to say Doxygen that it is a comment for the previous line.
However, I'd prefer to see the comment before uint64_t offloads; (and
keep /** )
Not sure, since it highly depends on what is used in other similar
places in the file.
Similar comments are applicable to a number of lines below.
> + * Per-port Tx offloads to be set using DEV_TX_OFFLOAD_* flags.
> + * Only offloads set on tx_offload_capa field on rte_eth_dev_info
> + * structure are allowed to be set.
> + */
>
> /* For i40e specifically */
> uint16_t pvid;
> @@ -733,6 +739,14 @@ struct rte_eth_rxconf {
> #define ETH_TXQ_FLAGS_NOXSUMS \
> (ETH_TXQ_FLAGS_NOXSUMSCTP | ETH_TXQ_FLAGS_NOXSUMUDP | \
> ETH_TXQ_FLAGS_NOXSUMTCP)
> +#define ETH_TXQ_FLAGS_IGNORE 0x8000
> + /**
> + * When set the txq_flags should be ignored,
> + * instead per-queue Tx offloads will be set on offloads field
> + * located on rte_eth_txq_conf struct.
> + * This flag is temporary till the rte_eth_txq_conf.txq_flags
> + * API will be deprecated.
> + */
>
> /**
> * A structure used to configure a TX ring of an Ethernet port.
> @@ -745,6 +759,12 @@ struct rte_eth_txconf {
>
> uint32_t txq_flags; /**< Set flags for the Tx queue */
> uint8_t tx_deferred_start; /**< Do not start queue with rte_eth_dev_start(). */
> + uint64_t offloads;
> + /**
> + * Per-queue Tx offloads to be set using DEV_TX_OFFLOAD_* flags.
> + * Only offloads set on tx_queue_offload_capa field on rte_eth_dev_info
> + * structure are allowed to be set.
> + */
> };
>
> /**
> @@ -969,6 +989,8 @@ struct rte_eth_conf {
> /**< Multiple threads can invoke rte_eth_tx_burst() concurrently on the same
> * tx queue without SW lock.
> */
> +#define DEV_TX_OFFLOAD_MULTI_SEGS 0x00008000
> +/**< multi segment send is supported. */
The comment should start from capital letter as everywhere else in the
file (as far as I can see).
>
> struct rte_pci_device;
>
> @@ -991,9 +1013,12 @@ struct rte_eth_dev_info {
> uint16_t max_vmdq_pools; /**< Maximum number of VMDq pools. */
> uint64_t rx_offload_capa;
> /**< Device per port RX offload capabilities. */
> - uint32_t tx_offload_capa; /**< Device TX offload capabilities. */
> + uint64_t tx_offload_capa;
> + /**< Device per port TX offload capabilities. */
> uint64_t rx_queue_offload_capa;
> /**< Device per queue RX offload capabilities. */
> + uint64_t tx_queue_offload_capa;
> + /**< Device per queue TX offload capabilities. */
> uint16_t reta_size;
> /**< Device redirection table size, the total number of entries. */
> uint8_t hash_key_size; /**< Hash key size in bytes */
> @@ -2024,6 +2049,11 @@ int rte_eth_rx_queue_setup(uint8_t port_id, uint16_t rx_queue_id,
> * - The *txq_flags* member contains flags to pass to the TX queue setup
> * function to configure the behavior of the TX queue. This should be set
> * to 0 if no special configuration is required.
> + * This API is obsolete and will be deprecated. Applications
> + * should set it to ETH_TXQ_FLAGS_IGNORE and use
> + * the offloads field below.
> + * - The *offloads* member contains Tx offloads to be enabled.
> + * Offloads which are not set cannot be used on the datapath.
> *
> * Note that setting *tx_free_thresh* or *tx_rs_thresh* value to 0 forces
> * the transmit function to use default values.
More information about the dev
mailing list