[PATCH 0/2] mbuf: improve Tx offload API doc

[PATCH 0/2] mbuf: improve Tx offload API doc

[Thomas Monjalon]

When reviewing the new tunnel offload flags proposed by Xueming,
I have seen that some old doxygen comments should be improved or fixed.

Thomas Monjalon (2):
  mbuf: fix Tx checksum offload API doc
  mbuf: improve API doc for tunnel Tx offloads

 lib/librte_ether/rte_ethdev.h |  3 +++
 lib/librte_mbuf/rte_mbuf.h    | 24 ++++++++----------------
 2 files changed, 11 insertions(+), 16 deletions(-)

-- 
2.16.2

[PATCH 1/2] mbuf: fix Tx checksum offload API doc

[Thomas Monjalon]

When introducing rte_eth_tx_prepare(), the constraints on checksum
pre-filling for Tx offloads were relaxed because implemented in
the PMDs with rte_net_intel_cksum_flags_prepare() helper.
As a consequence, these old requirements are removed for:
	- PKT_TX_OUTER_IP_CKSUM
	- PKT_TX_IP_CKSUM
	- PKT_TX_[L4]_CKSUM
	- PKT_TX_TCP_SEG

Not sure SCTP offload is properly implemented though.

A reference to rte_eth_tx_prepare() is added in rte_eth_tx_burst() doc.

Fixes: 4fb7e803eb1a ("ethdev: add Tx preparation")
Cc: tomaszx.kulasek@intel.com

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 lib/librte_ether/rte_ethdev.h |  3 +++
 lib/librte_mbuf/rte_mbuf.h    | 16 +++-------------
 2 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 7e4e57b3c..eb090df9a 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -3911,6 +3911,9 @@ static inline int rte_eth_tx_descriptor_status(uint16_t port_id,
  * invoke this function concurrently on the same tx queue without SW lock.
  * @see rte_eth_dev_info_get, struct rte_eth_txconf::txq_flags
  *
+ * @see rte_eth_tx_prepare to perform some prior checks or adjustments
+ * for offloads.
+ *
  * @param port_id
  *   The port identifier of the Ethernet device.
  * @param queue_id
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 06eceba37..7ae3bab14 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -226,12 +226,8 @@ extern "C" {
  *  - set the PKT_TX_TCP_SEG flag in mbuf->ol_flags (this flag implies
  *    PKT_TX_TCP_CKSUM)
  *  - set the flag PKT_TX_IPV4 or PKT_TX_IPV6
- *  - if it's IPv4, set the PKT_TX_IP_CKSUM flag and write the IP checksum
- *    to 0 in the packet
+ *  - if it's IPv4, set the PKT_TX_IP_CKSUM flag
  *  - fill the mbuf offload information: l2_len, l3_len, l4_len, tso_segsz
- *  - calculate the pseudo header checksum without taking ip_len in account,
- *    and set it in the TCP header. Refer to rte_ipv4_phdr_cksum() and
- *    rte_ipv6_phdr_cksum() that can be used as helpers.
  */
 #define PKT_TX_TCP_SEG       (1ULL << 50)
 
@@ -244,9 +240,6 @@ extern "C" {
  *  - fill l2_len and l3_len in mbuf
  *  - set the flags PKT_TX_TCP_CKSUM, PKT_TX_SCTP_CKSUM or PKT_TX_UDP_CKSUM
  *  - set the flag PKT_TX_IPV4 or PKT_TX_IPV6
- *  - calculate the pseudo header checksum and set it in the L4 header (only
- *    for TCP or UDP). See rte_ipv4_phdr_cksum() and rte_ipv6_phdr_cksum().
- *    For SCTP, set the crc field to 0.
  */
 #define PKT_TX_L4_NO_CKSUM   (0ULL << 52) /**< Disable L4 cksum of TX pkt. */
 #define PKT_TX_TCP_CKSUM     (1ULL << 52) /**< TCP cksum of TX pkt. computed by NIC. */
@@ -258,7 +251,6 @@ extern "C" {
  * Offload the IP checksum in the hardware. The flag PKT_TX_IPV4 should
  * also be set by the application, although a PMD will only check
  * PKT_TX_IP_CKSUM.
- *  - set the IP checksum field in the packet to 0
  *  - fill the mbuf offload information: l2_len, l3_len
  */
 #define PKT_TX_IP_CKSUM      (1ULL << 54)
@@ -288,10 +280,8 @@ extern "C" {
 
 /**
  * Offload the IP checksum of an external header in the hardware. The
- * flag PKT_TX_OUTER_IPV4 should also be set by the application, alto ugh
- * a PMD will only check PKT_TX_IP_CKSUM.  The IP checksum field in the
- * packet must be set to 0.
- *  - set the outer IP checksum field in the packet to 0
+ * flag PKT_TX_OUTER_IPV4 should also be set by the application, altough
+ * a PMD will only check PKT_TX_OUTER_IP_CKSUM.
  *  - fill the mbuf offload information: outer_l2_len, outer_l3_len
  */
 #define PKT_TX_OUTER_IP_CKSUM   (1ULL << 58)
-- 
2.16.2

[PATCH 2/2] mbuf: improve API doc for tunnel Tx offloads

[Thomas Monjalon]

Add few details to remind TSO flag, checksum flags and header lengths.

The doxygen syntax for MPLS-in-UDP is fixed.

Fixes: d95188551fa1 ("mbuf: introduce new Tx offload flag for MPLS-in-UDP")
Cc: harish.patil@cavium.com

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 lib/librte_mbuf/rte_mbuf.h | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 7ae3bab14..adf95e3cd 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -201,14 +201,16 @@ extern "C" {
 
 /**
  * Bits 45:48 used for the tunnel type.
- * When doing Tx offload like TSO or checksum, the HW needs to configure the
- * tunnel type into the HW descriptors.
+ * The tunnel type must be specified for TSO or checksum on tunnel packets.
+ * These flags can be used with PKT_TX_TCP_SEG for TSO, or PKT_TX_xxx_CKSUM.
+ * The mbuf fields for inner and outer header lengths may be required:
+ * outer_l2_len, outer_l3_len, l2_len, l3_len, l4_len.
  */
 #define PKT_TX_TUNNEL_VXLAN   (0x1ULL << 45)
 #define PKT_TX_TUNNEL_GRE     (0x2ULL << 45)
 #define PKT_TX_TUNNEL_IPIP    (0x3ULL << 45)
 #define PKT_TX_TUNNEL_GENEVE  (0x4ULL << 45)
-/**< TX packet with MPLS-in-UDP RFC 7510 header. */
+/** TX packet with MPLS-in-UDP RFC 7510 header. */
 #define PKT_TX_TUNNEL_MPLSINUDP (0x5ULL << 45)
 /* add new TX TUNNEL type here */
 #define PKT_TX_TUNNEL_MASK    (0xFULL << 45)
-- 
2.16.2

Re: [PATCH 1/2] mbuf: fix Tx checksum offload API doc

[Olivier Matz]

On Fri, Apr 20, 2018 at 02:13:23AM +0200, Thomas Monjalon wrote:
> When introducing rte_eth_tx_prepare(), the constraints on checksum
> pre-filling for Tx offloads were relaxed because implemented in
> the PMDs with rte_net_intel_cksum_flags_prepare() helper.
> As a consequence, these old requirements are removed for:
> 	- PKT_TX_OUTER_IP_CKSUM
> 	- PKT_TX_IP_CKSUM
> 	- PKT_TX_[L4]_CKSUM
> 	- PKT_TX_TCP_SEG
> 
> Not sure SCTP offload is properly implemented though.
> 
> A reference to rte_eth_tx_prepare() is added in rte_eth_tx_burst() doc.
> 
> Fixes: 4fb7e803eb1a ("ethdev: add Tx preparation")
> Cc: tomaszx.kulasek@intel.com
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>

Acked-by: Olivier Matz <olivier.matz@6wind.com>


> @@ -288,10 +280,8 @@ extern "C" {
>  
>  /**
>   * Offload the IP checksum of an external header in the hardware. The
> - * flag PKT_TX_OUTER_IPV4 should also be set by the application, alto ugh
> - * a PMD will only check PKT_TX_IP_CKSUM.  The IP checksum field in the
> - * packet must be set to 0.
> - *  - set the outer IP checksum field in the packet to 0
> + * flag PKT_TX_OUTER_IPV4 should also be set by the application, altough
> + * a PMD will only check PKT_TX_OUTER_IP_CKSUM.
>   *  - fill the mbuf offload information: outer_l2_len, outer_l3_len
>   */
>  #define PKT_TX_OUTER_IP_CKSUM   (1ULL << 58)

Nice catch (PKT_TX_IP_CKSUM -> PKT_TX_OUTER_IP_CKSUM).

It could eventually deserve also a:
Fixes: 609dd68ef14f ("mbuf: enhance the API documentation of offload flags")


Thanks
Olivier

Re: [PATCH 2/2] mbuf: improve API doc for tunnel Tx offloads

[Olivier Matz]

On Fri, Apr 20, 2018 at 02:13:24AM +0200, Thomas Monjalon wrote:
> Add few details to remind TSO flag, checksum flags and header lengths.
> 
> The doxygen syntax for MPLS-in-UDP is fixed.
> 
> Fixes: d95188551fa1 ("mbuf: introduce new Tx offload flag for MPLS-in-UDP")
> Cc: harish.patil@cavium.com
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
>  lib/librte_mbuf/rte_mbuf.h | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> index 7ae3bab14..adf95e3cd 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -201,14 +201,16 @@ extern "C" {
>  
>  /**
>   * Bits 45:48 used for the tunnel type.
> - * When doing Tx offload like TSO or checksum, the HW needs to configure the
> - * tunnel type into the HW descriptors.
> + * The tunnel type must be specified for TSO or checksum on tunnel packets.

on tunnel packets -> on the inner part of tunnel packets

> + * These flags can be used with PKT_TX_TCP_SEG for TSO, or PKT_TX_xxx_CKSUM.
> + * The mbuf fields for inner and outer header lengths may be required:

may be -> are

> + * outer_l2_len, outer_l3_len, l2_len, l3_len, l4_len.

and tso_segsz for TSO.

>   */
>  #define PKT_TX_TUNNEL_VXLAN   (0x1ULL << 45)
>  #define PKT_TX_TUNNEL_GRE     (0x2ULL << 45)
>  #define PKT_TX_TUNNEL_IPIP    (0x3ULL << 45)
>  #define PKT_TX_TUNNEL_GENEVE  (0x4ULL << 45)
> -/**< TX packet with MPLS-in-UDP RFC 7510 header. */
> +/** TX packet with MPLS-in-UDP RFC 7510 header. */
>  #define PKT_TX_TUNNEL_MPLSINUDP (0x5ULL << 45)
>  /* add new TX TUNNEL type here */
>  #define PKT_TX_TUNNEL_MASK    (0xFULL << 45)
> -- 
> 2.16.2
> 

Re: [PATCH 2/2] mbuf: improve API doc for tunnel Tx offloads

[Thomas Monjalon]

23/04/2018 10:14, Olivier Matz:
> On Fri, Apr 20, 2018 at 02:13:24AM +0200, Thomas Monjalon wrote:
> > + * These flags can be used with PKT_TX_TCP_SEG for TSO, or PKT_TX_xxx_CKSUM.
> > + * The mbuf fields for inner and outer header lengths may be required:
> 
> may be -> are
> 
> > + * outer_l2_len, outer_l3_len, l2_len, l3_len, l4_len.
> 
> and tso_segsz for TSO.

>From a HW point of view, some header lengths may be omitted if they are
guessed by the HW.
But you are right, from an API point of view, we should require them,
no matter what is the underlying hardware.

So the sentence becomes:
 * The mbuf fields for inner and outer header lengths are required:
 * outer_l2_len, outer_l3_len, l2_len, l3_len, l4_len and tso_segsz for TSO.

Re: [PATCH 2/2] mbuf: improve API doc for tunnel Tx offloads

[Olivier Matz]

On Mon, Apr 23, 2018 at 10:53:17AM +0200, Thomas Monjalon wrote:
> 23/04/2018 10:14, Olivier Matz:
> > On Fri, Apr 20, 2018 at 02:13:24AM +0200, Thomas Monjalon wrote:
> > > + * These flags can be used with PKT_TX_TCP_SEG for TSO, or PKT_TX_xxx_CKSUM.
> > > + * The mbuf fields for inner and outer header lengths may be required:
> > 
> > may be -> are
> > 
> > > + * outer_l2_len, outer_l3_len, l2_len, l3_len, l4_len.
> > 
> > and tso_segsz for TSO.
> 
> From a HW point of view, some header lengths may be omitted if they are
> guessed by the HW.
> But you are right, from an API point of view, we should require them,
> no matter what is the underlying hardware.
> 
> So the sentence becomes:
>  * The mbuf fields for inner and outer header lengths are required:
>  * outer_l2_len, outer_l3_len, l2_len, l3_len, l4_len and tso_segsz for TSO.

Sounds good, thanks.

[PATCH v2 0/2] mbuf: improve Tx offload API doc

[Thomas Monjalon]

When reviewing the new tunnel offload flags proposed by Xueming,
I have seen that some old doxygen comments should be improved or fixed.

This v2 is addressing some comments done by Olivier.

Thomas Monjalon (2):
  mbuf: fix Tx checksum offload API doc
  mbuf: improve API doc for tunnel Tx offloads

 lib/librte_ether/rte_ethdev.h |  3 +++
 lib/librte_mbuf/rte_mbuf.h    | 25 +++++++++----------------
 2 files changed, 12 insertions(+), 16 deletions(-)

-- 
2.16.2

[PATCH v2 1/2] mbuf: fix Tx checksum offload API doc

[Thomas Monjalon]

When introducing rte_eth_tx_prepare(), the constraints on checksum
pre-filling for Tx offloads were relaxed because implemented in
the PMDs with rte_net_intel_cksum_flags_prepare() helper.
As a consequence, these old requirements are removed for:
	- PKT_TX_OUTER_IP_CKSUM
	- PKT_TX_IP_CKSUM
	- PKT_TX_[L4]_CKSUM
	- PKT_TX_TCP_SEG

Not sure SCTP offload is properly implemented though.

A reference to rte_eth_tx_prepare() is added in rte_eth_tx_burst() doc.

Fixes: 609dd68ef14f ("mbuf: enhance the API documentation of offload flags")
Fixes: 4fb7e803eb1a ("ethdev: add Tx preparation")
Cc: stable@dpdk.org

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Acked-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_ether/rte_ethdev.h |  3 +++
 lib/librte_mbuf/rte_mbuf.h    | 16 +++-------------
 2 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 7e4e57b3c..eb090df9a 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -3911,6 +3911,9 @@ static inline int rte_eth_tx_descriptor_status(uint16_t port_id,
  * invoke this function concurrently on the same tx queue without SW lock.
  * @see rte_eth_dev_info_get, struct rte_eth_txconf::txq_flags
  *
+ * @see rte_eth_tx_prepare to perform some prior checks or adjustments
+ * for offloads.
+ *
  * @param port_id
  *   The port identifier of the Ethernet device.
  * @param queue_id
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 06eceba37..029822226 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -226,12 +226,8 @@ extern "C" {
  *  - set the PKT_TX_TCP_SEG flag in mbuf->ol_flags (this flag implies
  *    PKT_TX_TCP_CKSUM)
  *  - set the flag PKT_TX_IPV4 or PKT_TX_IPV6
- *  - if it's IPv4, set the PKT_TX_IP_CKSUM flag and write the IP checksum
- *    to 0 in the packet
+ *  - if it's IPv4, set the PKT_TX_IP_CKSUM flag
  *  - fill the mbuf offload information: l2_len, l3_len, l4_len, tso_segsz
- *  - calculate the pseudo header checksum without taking ip_len in account,
- *    and set it in the TCP header. Refer to rte_ipv4_phdr_cksum() and
- *    rte_ipv6_phdr_cksum() that can be used as helpers.
  */
 #define PKT_TX_TCP_SEG       (1ULL << 50)
 
@@ -244,9 +240,6 @@ extern "C" {
  *  - fill l2_len and l3_len in mbuf
  *  - set the flags PKT_TX_TCP_CKSUM, PKT_TX_SCTP_CKSUM or PKT_TX_UDP_CKSUM
  *  - set the flag PKT_TX_IPV4 or PKT_TX_IPV6
- *  - calculate the pseudo header checksum and set it in the L4 header (only
- *    for TCP or UDP). See rte_ipv4_phdr_cksum() and rte_ipv6_phdr_cksum().
- *    For SCTP, set the crc field to 0.
  */
 #define PKT_TX_L4_NO_CKSUM   (0ULL << 52) /**< Disable L4 cksum of TX pkt. */
 #define PKT_TX_TCP_CKSUM     (1ULL << 52) /**< TCP cksum of TX pkt. computed by NIC. */
@@ -258,7 +251,6 @@ extern "C" {
  * Offload the IP checksum in the hardware. The flag PKT_TX_IPV4 should
  * also be set by the application, although a PMD will only check
  * PKT_TX_IP_CKSUM.
- *  - set the IP checksum field in the packet to 0
  *  - fill the mbuf offload information: l2_len, l3_len
  */
 #define PKT_TX_IP_CKSUM      (1ULL << 54)
@@ -288,10 +280,8 @@ extern "C" {
 
 /**
  * Offload the IP checksum of an external header in the hardware. The
- * flag PKT_TX_OUTER_IPV4 should also be set by the application, alto ugh
- * a PMD will only check PKT_TX_IP_CKSUM.  The IP checksum field in the
- * packet must be set to 0.
- *  - set the outer IP checksum field in the packet to 0
+ * flag PKT_TX_OUTER_IPV4 should also be set by the application, although
+ * a PMD will only check PKT_TX_OUTER_IP_CKSUM.
  *  - fill the mbuf offload information: outer_l2_len, outer_l3_len
  */
 #define PKT_TX_OUTER_IP_CKSUM   (1ULL << 58)
-- 
2.16.2

[PATCH v2 2/2] mbuf: improve API doc for tunnel Tx offloads

[Thomas Monjalon]

Add few details to remind TSO flag, checksum flags and header lengths.

The doxygen syntax for MPLS-in-UDP is fixed.

Fixes: d95188551fa1 ("mbuf: introduce new Tx offload flag for MPLS-in-UDP")
Cc: stable@dpdk.org

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 lib/librte_mbuf/rte_mbuf.h | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 029822226..63a03c2f7 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -201,14 +201,17 @@ extern "C" {
 
 /**
  * Bits 45:48 used for the tunnel type.
- * When doing Tx offload like TSO or checksum, the HW needs to configure the
- * tunnel type into the HW descriptors.
+ * The tunnel type must be specified for TSO or checksum on the inner part
+ * of tunnel packets.
+ * These flags can be used with PKT_TX_TCP_SEG for TSO, or PKT_TX_xxx_CKSUM.
+ * The mbuf fields for inner and outer header lengths are required:
+ * outer_l2_len, outer_l3_len, l2_len, l3_len, l4_len and tso_segsz for TSO.
  */
 #define PKT_TX_TUNNEL_VXLAN   (0x1ULL << 45)
 #define PKT_TX_TUNNEL_GRE     (0x2ULL << 45)
 #define PKT_TX_TUNNEL_IPIP    (0x3ULL << 45)
 #define PKT_TX_TUNNEL_GENEVE  (0x4ULL << 45)
-/**< TX packet with MPLS-in-UDP RFC 7510 header. */
+/** TX packet with MPLS-in-UDP RFC 7510 header. */
 #define PKT_TX_TUNNEL_MPLSINUDP (0x5ULL << 45)
 /* add new TX TUNNEL type here */
 #define PKT_TX_TUNNEL_MASK    (0xFULL << 45)
-- 
2.16.2

Re: [PATCH v2 2/2] mbuf: improve API doc for tunnel Tx offloads

[Olivier Matz]

On Mon, Apr 23, 2018 at 12:48:36PM +0200, Thomas Monjalon wrote:
> Add few details to remind TSO flag, checksum flags and header lengths.
> 
> The doxygen syntax for MPLS-in-UDP is fixed.
> 
> Fixes: d95188551fa1 ("mbuf: introduce new Tx offload flag for MPLS-in-UDP")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>

Acked-by: Olivier Matz <olivier.matz@6wind.com>

Thanks

Re: [PATCH v2 2/2] mbuf: improve API doc for tunnel Tx offloads

[Thomas Monjalon]

23/04/2018 13:06, Olivier Matz:
> On Mon, Apr 23, 2018 at 12:48:36PM +0200, Thomas Monjalon wrote:
> > Add few details to remind TSO flag, checksum flags and header lengths.
> > 
> > The doxygen syntax for MPLS-in-UDP is fixed.
> > 
> > Fixes: d95188551fa1 ("mbuf: introduce new Tx offload flag for MPLS-in-UDP")
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> 
> Acked-by: Olivier Matz <olivier.matz@6wind.com>

Series applied