[dpdk-dev] [PATCH 0/3] add flag to disable CRC checksum offloading

[dpdk-dev] [PATCH 0/3] add flag to disable CRC checksum offloading

[Paul Emmerich]

This patch adds a new tx checksum offloading flag: PKT_TX_NO_CRC_CSUM.
This allows disabling CRC checksum offloading on a per-packet basis.
Doing this can be useful if you want to send out invalid packets on
purpose, e.g. in a packet generator/test framework.


Paul Emmerich (3):
  add tx crc disable flag
  ixgbe: use crc checksum disable flag
  i40e: use crc checksum disable flag

[dpdk-dev] [PATCH 1/3] add tx crc disable flag

[Paul Emmerich]

Signed-off-by: Paul Emmerich <emmericp@net.in.tum.de>
---
 lib/librte_mbuf/rte_mbuf.c | 1 +
 lib/librte_mbuf/rte_mbuf.h | 6 ++++++
 2 files changed, 7 insertions(+)

diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
index f506517..744fb4e 100644
--- a/lib/librte_mbuf/rte_mbuf.c
+++ b/lib/librte_mbuf/rte_mbuf.c
@@ -270,6 +270,7 @@ const char *rte_get_rx_ol_flag_name(uint64_t mask)
 const char *rte_get_tx_ol_flag_name(uint64_t mask)
 {
 	switch (mask) {
+	case PKT_TX_NO_CRC_CSUM: return "PKT_TX_NO_CRC_CSUM";
 	case PKT_TX_VLAN_PKT: return "PKT_TX_VLAN_PKT";
 	case PKT_TX_IP_CKSUM: return "PKT_TX_IP_CKSUM";
 	case PKT_TX_TCP_CKSUM: return "PKT_TX_TCP_CKSUM";
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index ab6de67..096d84a 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -106,6 +106,12 @@ extern "C" {
 /* add new TX flags here */
 
 /**
+ * Disable CRC checksum offload
+ */
+#define PKT_TX_NO_CRC_CSUM   (1ULL << 49)
+
+
+/**
  * TCP segmentation offload. To enable this offload feature for a
  * packet to be transmitted on hardware supporting TSO:
  *  - set the PKT_TX_TCP_SEG flag in mbuf->ol_flags (this flag implies
-- 
1.9.1

Re: [dpdk-dev] [PATCH 1/3] add tx crc disable flag

[Olivier MATZ]

Hi Paul,

On 02/15/2016 05:55 PM, Paul Emmerich wrote:
> Signed-off-by: Paul Emmerich <emmericp@net.in.tum.de>
> ---
>  lib/librte_mbuf/rte_mbuf.c | 1 +
>  lib/librte_mbuf/rte_mbuf.h | 6 ++++++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
> index f506517..744fb4e 100644
> --- a/lib/librte_mbuf/rte_mbuf.c
> +++ b/lib/librte_mbuf/rte_mbuf.c
> @@ -270,6 +270,7 @@ const char *rte_get_rx_ol_flag_name(uint64_t mask)
>  const char *rte_get_tx_ol_flag_name(uint64_t mask)
>  {
>  	switch (mask) {
> +	case PKT_TX_NO_CRC_CSUM: return "PKT_TX_NO_CRC_CSUM";
>  	case PKT_TX_VLAN_PKT: return "PKT_TX_VLAN_PKT";
>  	case PKT_TX_IP_CKSUM: return "PKT_TX_IP_CKSUM";
>  	case PKT_TX_TCP_CKSUM: return "PKT_TX_TCP_CKSUM";
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> index ab6de67..096d84a 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -106,6 +106,12 @@ extern "C" {
>  /* add new TX flags here */
>  
>  /**
> + * Disable CRC checksum offload
> + */
> +#define PKT_TX_NO_CRC_CSUM   (1ULL << 49)
> +
> +
> +/**
>   * TCP segmentation offload. To enable this offload feature for a
>   * packet to be transmitted on hardware supporting TSO:
>   *  - set the PKT_TX_TCP_SEG flag in mbuf->ol_flags (this flag implies
> 

The patch does not apply properly. It seems you've done it on a version
that is more than 6 months old. Could you please rebase it?

My second question concerns the behavior of this flag when it is not
supported by the driver. Shouldn't we have a feature flag saying "this
driver supports CRC stripping"?

Also, I think "PKT_TX_STRIP_CRC" would be a better name.

Regards,
Olivier

[dpdk-dev] [PATCH 2/3] ixgbe: use crc checksum disable flag

[Paul Emmerich]

Signed-off-by: Paul Emmerich <emmericp@net.in.tum.de>
---
 lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
index 57c9430..800e224 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
@@ -730,8 +730,9 @@ ixgbe_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 		 * are only set in the last Data Descriptor:
 		 *   - IXGBE_TXD_CMD_RS
 		 */
-		cmd_type_len = IXGBE_ADVTXD_DTYP_DATA |
-			IXGBE_ADVTXD_DCMD_IFCS | IXGBE_ADVTXD_DCMD_DEXT;
+		cmd_type_len = IXGBE_ADVTXD_DTYP_DATA | IXGBE_ADVTXD_DCMD_DEXT;
+		if (!(ol_flags & PKT_TX_NO_CRC_CSUM))
+			cmd_type_len |= IXGBE_ADVTXD_DCMD_IFCS;
 
 #ifdef RTE_LIBRTE_IEEE1588
 		if (ol_flags & PKT_TX_IEEE1588_TMST)
-- 
1.9.1

[dpdk-dev] [PATCH 3/3] i40e: use crc checksum disable flag

[Paul Emmerich]

Signed-off-by: Paul Emmerich <emmericp@net.in.tum.de>
---
 drivers/net/i40e/i40e_rxtx.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index 40cffc1..52f7955 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -799,6 +799,11 @@ i40e_txd_enable_checksum(uint64_t ol_flags,
 		*td_offset |= (tx_offload.l2_len >> 1)
 			<< I40E_TX_DESC_LENGTH_MACLEN_SHIFT;
 
+	/* Enable L2 checksum offload */
+	if (!(ol_flags & PKT_TX_NO_CRC_CSUM))
+		*td_cmd |= I40E_TX_DESC_CMD_ICRC;
+
+
 	/* Enable L3 checksum offloads */
 	if (ol_flags & PKT_TX_IP_CKSUM) {
 		*td_cmd |= I40E_TX_DESC_CMD_IIPT_IPV4_CSUM;
@@ -1613,9 +1618,6 @@ i40e_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 						I40E_TX_FLAG_L2TAG1_SHIFT;
 		}
 
-		/* Always enable CRC offload insertion */
-		td_cmd |= I40E_TX_DESC_CMD_ICRC;
-
 		/* Enable checksum offloading */
 		cd_tunneling_params = 0;
 		if (ol_flags & I40E_TX_CKSUM_OFFLOAD_MASK) {
-- 
1.9.1

Re: [dpdk-dev] [PATCH 0/3] add flag to disable CRC checksum offloading

[Bruce Richardson]

On Mon, Feb 15, 2016 at 05:55:22PM +0100, Paul Emmerich wrote:
> This patch adds a new tx checksum offloading flag: PKT_TX_NO_CRC_CSUM.
> This allows disabling CRC checksum offloading on a per-packet basis.
> Doing this can be useful if you want to send out invalid packets on
> purpose, e.g. in a packet generator/test framework.
> 
> 
> Paul Emmerich (3):
>   add tx crc disable flag
>   ixgbe: use crc checksum disable flag
>   i40e: use crc checksum disable flag
> 
Maintainers,

Any comments or ack on the patchset. [The first patch is missing the "mbuf"
prefix as it's a patch to that library.]

/Bruce