[dpdk-dev] [RFC] ethdev: add new offload flag to keep CRC

[dpdk-dev] [RFC] ethdev: add new offload flag to keep CRC

[Ferruh Yigit]

DEV_RX_OFFLOAD_KEEP_CRC offload flag added.

DEV_RX_OFFLOAD_CRC_STRIP flag will remain one more release
default behavior in PMDs is to keep the CRC until this flag removed

Until DEV_RX_OFFLOAD_CRC_STRIP flag is removed:
- Setting both KEEP_CRC & CRC_STRIP is INVALID
- Setting only CRC_STRIP PMD should strip the CRC
- Setting only KEEP_CRC PMD should keep the CRC
- Not setting both PMD should keep the CRC

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 doc/guides/rel_notes/deprecation.rst | 10 ----------
 lib/librte_ethdev/rte_ethdev.h       |  4 ++++
 2 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 1ce692eac..cd128ae23 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -67,16 +67,6 @@ Deprecation Notices
   - removal of ``txq_flags`` field from ``rte_eth_txconf`` struct.
   - removal of the offloads bit-field from ``rte_eth_rxmode`` struct.
 
-* ethdev: A new offloading flag ``DEV_RX_OFFLOAD_KEEP_CRC`` will be added in v18.08,
-  This will replace the usage of not setting ``DEV_RX_OFFLOAD_CRC_STRIP`` flag
-  and will be implemented in PMDs accordingly.
-  In v18.08 both ``DEV_RX_OFFLOAD_KEEP_CRC`` and ``DEV_RX_OFFLOAD_CRC_STRIP`` flags
-  will be available, usage will be:
-  ``CRC_STRIP``: Strip CRC from packet
-  ``KEEP_CRC``: Keep CRC in packet
-  Both ``CRC_STRIP`` & ``KEEP_CRC``: Invalid
-  No flag: Keep CRC in packet
-
 * ethdev: In v18.11 ``DEV_RX_OFFLOAD_CRC_STRIP`` offload flag will be removed, default
   behavior without any flag will be changed to CRC strip.
   To keep CRC ``DEV_RX_OFFLOAD_KEEP_CRC`` flag is required.
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index 36e3984ea..c81af09f8 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -939,6 +939,10 @@ struct rte_eth_conf {
 #define DEV_RX_OFFLOAD_SCATTER		0x00002000
 #define DEV_RX_OFFLOAD_TIMESTAMP	0x00004000
 #define DEV_RX_OFFLOAD_SECURITY         0x00008000
+
+/* Invalid to set both DEV_RX_OFFLOAD_CRC_STRIP and DEV_RX_OFFLOAD_KEEP_CRC
+ * No DEV_RX_OFFLOAD_CRC_STRIP flag means keep CRC */
+#define DEV_RX_OFFLOAD_KEEP_CRC		0x00010000
 #define DEV_RX_OFFLOAD_CHECKSUM (DEV_RX_OFFLOAD_IPV4_CKSUM | \
 				 DEV_RX_OFFLOAD_UDP_CKSUM | \
 				 DEV_RX_OFFLOAD_TCP_CKSUM)
-- 
2.14.4

Re: [dpdk-dev] [RFC] ethdev: add new offload flag to keep CRC

[Andrew Rybchenko]

On 06/09/2018 01:57 AM, Ferruh Yigit wrote:
> DEV_RX_OFFLOAD_KEEP_CRC offload flag added.
>
> DEV_RX_OFFLOAD_CRC_STRIP flag will remain one more release
> default behavior in PMDs is to keep the CRC until this flag removed
>
> Until DEV_RX_OFFLOAD_CRC_STRIP flag is removed:
> - Setting both KEEP_CRC & CRC_STRIP is INVALID

ethdev layer should enforce it.

> - Setting only CRC_STRIP PMD should strip the CRC
> - Setting only KEEP_CRC PMD should keep the CRC
> - Not setting both PMD should keep the CRC
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>

- rte_rx_offload_names should be updated as well
- testpmd should be updated

I'm not sure that I understand the transition plan. I think PMDs which 
support
KEEP_CRC should be updated by the patch to advertise the offload. Otherwise,
generic checks in ethdev will not allow to enable it. Other option is to 
simply
drop it on ethdev layer (since basically it changes nothing for PMD now) and
encourage PMD maintainers to advertise and take it into account if the 
feature
is supported (however, if the bit is dropped on ethdev layer, the code 
would
be unused/dead regardless application request). Too complicated. I guess
there are not so many PMDs which support KEEP_CRC. Better to update them.

The patch should encourage applications which would like to keep CRC to
use the offload since the next release will drop CRC_STRIP and it will
change behaviour (no KEEP_CRC => strip it)

Re: [dpdk-dev] [RFC] ethdev: add new offload flag to keep CRC

[Ferruh Yigit]

On 6/9/2018 11:11 AM, Andrew Rybchenko wrote:
> On 06/09/2018 01:57 AM, Ferruh Yigit wrote:
>> DEV_RX_OFFLOAD_KEEP_CRC offload flag added.
>>
>> DEV_RX_OFFLOAD_CRC_STRIP flag will remain one more release
>> default behavior in PMDs is to keep the CRC until this flag removed
>>
>> Until DEV_RX_OFFLOAD_CRC_STRIP flag is removed:
>> - Setting both KEEP_CRC & CRC_STRIP is INVALID
> 
> ethdev layer should enforce it.

OK, will add this check.

> 
>> - Setting only CRC_STRIP PMD should strip the CRC
>> - Setting only KEEP_CRC PMD should keep the CRC
>> - Not setting both PMD should keep the CRC
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 
> - rte_rx_offload_names should be updated as well
> - testpmd should be updated
> 
> I'm not sure that I understand the transition plan. I think PMDs which support
> KEEP_CRC should be updated by the patch to advertise the offload. Otherwise,
> generic checks in ethdev will not allow to enable it. 

Right, PMDs should be updated to advertise this offload, will do it.

> Other option is to simply
> drop it on ethdev layer (since basically it changes nothing for PMD now) and
> encourage PMD maintainers to advertise and take it into account if the feature
> is supported (however, if the bit is dropped on ethdev layer, the code would
> be unused/dead regardless application request). Too complicated. I guess
> there are not so many PMDs which support KEEP_CRC. Better to update them

> The patch should encourage applications which would like to keep CRC to
> use the offload since the next release will drop CRC_STRIP and it will
> change behaviour (no KEEP_CRC => strip it)

+1, will check apps.

Re: [dpdk-dev] [RFC] ethdev: add new offload flag to keep CRC

[Stephen Hemminger]

On Fri,  8 Jun 2018 23:57:09 +0100
Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> DEV_RX_OFFLOAD_KEEP_CRC offload flag added.
> 
> DEV_RX_OFFLOAD_CRC_STRIP flag will remain one more release
> default behavior in PMDs is to keep the CRC until this flag removed

This won't work for virtual devices that never keep CRC.
Maybe wording should be changed.

Re: [dpdk-dev] [RFC] ethdev: add new offload flag to keep CRC

[Ferruh Yigit]

On 6/11/2018 4:25 PM, Stephen Hemminger wrote:
> On Fri,  8 Jun 2018 23:57:09 +0100
> Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> 
>> DEV_RX_OFFLOAD_KEEP_CRC offload flag added.
>>
>> DEV_RX_OFFLOAD_CRC_STRIP flag will remain one more release
>> default behavior in PMDs is to keep the CRC until this flag removed
> 
> This won't work for virtual devices that never keep CRC.
> Maybe wording should be changed.

Yes, it won't work for virtual devices, but it is already the case, no offload
flag means keep CRC.

This task is to replace that behavior gradually. What will be replaced is:
flag: DEV_RX_OFFLOAD_CRC_STRIP => DEV_RX_OFFLOAD_KEEP_CRC
default, no flag, behavior : KEEP_CRC => STRIP_CRC

But because of gradually switch, this release, 18.08, both flags will exits and
no flag default still will be KEEP_CRC

For virtual devices, for this release, since virtual PMDs don't announce any CRC
related offload capability, applications can't explicitly request to keep the CRC.
But no flag case (default keep CRC) will be wrong and ignored by virtual PMDs.

on 18.11 when DEV_RX_OFFLOAD_CRC_STRIP is removed and default behavior is STRIP
CRC, it will be correct for virtual device.

[dpdk-dev] [PATCH] ethdev: add new offload flag to keep CRC

[Ferruh Yigit]

DEV_RX_OFFLOAD_KEEP_CRC offload flag added. PMDs that supports keeping
CRC should advertise this offload capability.

DEV_RX_OFFLOAD_CRC_STRIP flag will remain one more release
default behavior in PMDs are to keep the CRC until this flag removed

Until DEV_RX_OFFLOAD_CRC_STRIP flag is removed:
- Setting both KEEP_CRC & CRC_STRIP is INVALID
- Setting only CRC_STRIP PMD should strip the CRC
- Setting only KEEP_CRC PMD should keep the CRC
- Not setting both PMD should keep the CRC

A helper function rte_eth_dev_is_keep_crc() has been added to be able to
change the no flag behavior with minimal changes in PMDs.

The PMDs that doesn't report the DEV_RX_OFFLOAD_KEEP_CRC offload can
remove rte_eth_dev_is_keep_crc() checks next release, related code
commented to help the maintenance task.

And DEV_RX_OFFLOAD_CRC_STRIP has been added to virtual drivers since
they don't use CRC at all, when an application requires this offload
virtual PMDs should not return error.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 doc/guides/rel_notes/deprecation.rst      | 10 ---------
 drivers/net/af_packet/rte_eth_af_packet.c |  1 +
 drivers/net/avp/avp_ethdev.c              |  1 +
 drivers/net/axgbe/axgbe_ethdev.c          |  4 +++-
 drivers/net/axgbe/axgbe_rxtx.c            |  6 +++--
 drivers/net/bnxt/bnxt_ethdev.c            |  1 +
 drivers/net/bnxt/bnxt_rxq.c               |  3 +--
 drivers/net/cxgbe/cxgbe_ethdev.c          |  6 ++++-
 drivers/net/e1000/em_rxtx.c               | 20 ++++++++++-------
 drivers/net/e1000/igb_ethdev.c            |  4 ++--
 drivers/net/e1000/igb_rxtx.c              | 27 ++++++++++++++---------
 drivers/net/fm10k/fm10k_ethdev.c          |  6 +++--
 drivers/net/i40e/i40e_ethdev.c            |  1 +
 drivers/net/i40e/i40e_ethdev_vf.c         |  3 ++-
 drivers/net/i40e/i40e_rxtx.c              |  6 +++--
 drivers/net/ixgbe/ixgbe_ethdev.c          |  4 ++--
 drivers/net/ixgbe/ixgbe_ipsec.c           |  2 +-
 drivers/net/ixgbe/ixgbe_rxtx.c            | 25 ++++++++++++---------
 drivers/net/kni/rte_eth_kni.c             |  1 +
 drivers/net/mlx4/mlx4_rxq.c               | 23 ++++++++++---------
 drivers/net/mlx5/mlx5_rxq.c               | 25 ++++++++++++---------
 drivers/net/mvpp2/mrvl_ethdev.c           |  8 ++++---
 drivers/net/nfp/nfp_net.c                 |  9 +++++---
 drivers/net/null/rte_eth_null.c           |  1 +
 drivers/net/octeontx/octeontx_ethdev.c    |  5 ++++-
 drivers/net/pcap/rte_eth_pcap.c           |  1 +
 drivers/net/qede/qede_ethdev.c            |  2 ++
 drivers/net/ring/rte_eth_ring.c           |  1 +
 drivers/net/sfc/sfc_rx.c                  |  5 ++++-
 drivers/net/softnic/rte_eth_softnic.c     |  1 +
 drivers/net/szedata2/rte_eth_szedata2.c   |  3 ++-
 drivers/net/thunderx/nicvf_ethdev.c       |  5 ++++-
 drivers/net/vhost/rte_eth_vhost.c         |  3 ++-
 drivers/net/virtio/virtio_ethdev.c        |  3 ++-
 drivers/net/vmxnet3/vmxnet3_ethdev.c      |  3 ++-
 lib/librte_ethdev/rte_ethdev.c            |  9 ++++++++
 lib/librte_ethdev/rte_ethdev.h            |  5 +++++
 lib/librte_ethdev/rte_ethdev_driver.h     | 20 +++++++++++++++++
 38 files changed, 172 insertions(+), 91 deletions(-)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 1ce692eac..cd128ae23 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -67,16 +67,6 @@ Deprecation Notices
   - removal of ``txq_flags`` field from ``rte_eth_txconf`` struct.
   - removal of the offloads bit-field from ``rte_eth_rxmode`` struct.
 
-* ethdev: A new offloading flag ``DEV_RX_OFFLOAD_KEEP_CRC`` will be added in v18.08,
-  This will replace the usage of not setting ``DEV_RX_OFFLOAD_CRC_STRIP`` flag
-  and will be implemented in PMDs accordingly.
-  In v18.08 both ``DEV_RX_OFFLOAD_KEEP_CRC`` and ``DEV_RX_OFFLOAD_CRC_STRIP`` flags
-  will be available, usage will be:
-  ``CRC_STRIP``: Strip CRC from packet
-  ``KEEP_CRC``: Keep CRC in packet
-  Both ``CRC_STRIP`` & ``KEEP_CRC``: Invalid
-  No flag: Keep CRC in packet
-
 * ethdev: In v18.11 ``DEV_RX_OFFLOAD_CRC_STRIP`` offload flag will be removed, default
   behavior without any flag will be changed to CRC strip.
   To keep CRC ``DEV_RX_OFFLOAD_KEEP_CRC`` flag is required.
diff --git a/drivers/net/af_packet/rte_eth_af_packet.c b/drivers/net/af_packet/rte_eth_af_packet.c
index ea47abbf8..8cfb7ada4 100644
--- a/drivers/net/af_packet/rte_eth_af_packet.c
+++ b/drivers/net/af_packet/rte_eth_af_packet.c
@@ -305,6 +305,7 @@ eth_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	dev_info->max_rx_queues = (uint16_t)internals->nb_queues;
 	dev_info->max_tx_queues = (uint16_t)internals->nb_queues;
 	dev_info->min_rx_bufsize = 0;
+	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_CRC_STRIP;
 }
 
 static int
diff --git a/drivers/net/avp/avp_ethdev.c b/drivers/net/avp/avp_ethdev.c
index dc97e60e6..9cc8fbcdf 100644
--- a/drivers/net/avp/avp_ethdev.c
+++ b/drivers/net/avp/avp_ethdev.c
@@ -2170,6 +2170,7 @@ avp_dev_info_get(struct rte_eth_dev *eth_dev,
 		dev_info->rx_offload_capa = DEV_RX_OFFLOAD_VLAN_STRIP;
 		dev_info->tx_offload_capa = DEV_TX_OFFLOAD_VLAN_INSERT;
 	}
+	dev_info->rx_offload_capa |= DEV_RX_OFFLOAD_CRC_STRIP;
 }
 
 static int
diff --git a/drivers/net/axgbe/axgbe_ethdev.c b/drivers/net/axgbe/axgbe_ethdev.c
index 7a3ba2e7b..e3773f4a2 100644
--- a/drivers/net/axgbe/axgbe_ethdev.c
+++ b/drivers/net/axgbe/axgbe_ethdev.c
@@ -363,7 +363,9 @@ axgbe_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	dev_info->rx_offload_capa =
 		DEV_RX_OFFLOAD_IPV4_CKSUM |
 		DEV_RX_OFFLOAD_UDP_CKSUM  |
-		DEV_RX_OFFLOAD_TCP_CKSUM;
+		DEV_RX_OFFLOAD_TCP_CKSUM  |
+		DEV_RX_OFFLOAD_CRC_STRIP  |
+		DEV_RX_OFFLOAD_KEEP_CRC;
 
 	dev_info->tx_offload_capa =
 		DEV_TX_OFFLOAD_IPV4_CKSUM  |
diff --git a/drivers/net/axgbe/axgbe_rxtx.c b/drivers/net/axgbe/axgbe_rxtx.c
index b302bdd1f..e665ef939 100644
--- a/drivers/net/axgbe/axgbe_rxtx.c
+++ b/drivers/net/axgbe/axgbe_rxtx.c
@@ -74,8 +74,10 @@ int axgbe_dev_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
 		(DMA_CH_INC * rxq->queue_id));
 	rxq->dma_tail_reg = (volatile uint32_t *)((uint8_t *)rxq->dma_regs +
 						  DMA_CH_RDTR_LO);
-	rxq->crc_len = (uint8_t)((dev->data->dev_conf.rxmode.offloads &
-			DEV_RX_OFFLOAD_CRC_STRIP) ? 0 : ETHER_CRC_LEN);
+	if (rte_eth_dev_is_keep_crc(dev->data->dev_conf.rxmode.offloads))
+		rxq->crc_len = ETHER_CRC_LEN;
+	else
+		rxq->crc_len = 0;
 
 	/* CRC strip in AXGBE supports per port not per queue */
 	pdata->crc_strip_enable = (rxq->crc_len == 0) ? 1 : 0;
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 6e56bfd36..f26461c42 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -147,6 +147,7 @@ static const struct rte_pci_id bnxt_pci_id_map[] = {
 				     DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM | \
 				     DEV_RX_OFFLOAD_JUMBO_FRAME | \
 				     DEV_RX_OFFLOAD_CRC_STRIP | \
+				     DEV_RX_OFFLOAD_KEEP_CRC | \
 				     DEV_RX_OFFLOAD_TCP_LRO)
 
 static int bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask);
diff --git a/drivers/net/bnxt/bnxt_rxq.c b/drivers/net/bnxt/bnxt_rxq.c
index c55ddec4b..265901e63 100644
--- a/drivers/net/bnxt/bnxt_rxq.c
+++ b/drivers/net/bnxt/bnxt_rxq.c
@@ -326,8 +326,7 @@ int bnxt_rx_queue_setup_op(struct rte_eth_dev *eth_dev,
 
 	rxq->queue_id = queue_idx;
 	rxq->port_id = eth_dev->data->port_id;
-	rxq->crc_len = rx_offloads & DEV_RX_OFFLOAD_CRC_STRIP ?
-		0 : ETHER_CRC_LEN;
+	rxq->crc_len = rte_eth_dev_is_keep_crc(rx_offloads) ? ETHER_CRC_LEN : 0;
 
 	eth_dev->data->rx_queues[queue_idx] = rxq;
 	/* Allocate RX ring hardware descriptors */
diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c
index 713dc8fae..b0156ec77 100644
--- a/drivers/net/cxgbe/cxgbe_ethdev.c
+++ b/drivers/net/cxgbe/cxgbe_ethdev.c
@@ -354,7 +354,11 @@ int cxgbe_dev_configure(struct rte_eth_dev *eth_dev)
 
 	CXGBE_FUNC_TRACE();
 	configured_offloads = eth_dev->data->dev_conf.rxmode.offloads;
-	if (!(configured_offloads & DEV_RX_OFFLOAD_CRC_STRIP)) {
+
+	/* KEEP_CRC offload flag is not supported by PMD
+	 * can remove the below block when DEV_RX_OFFLOAD_CRC_STRIP removed
+	 */
+	if (rte_eth_dev_is_keep_crc(configured_offloads)) {
 		dev_info(adapter, "can't disable hw crc strip\n");
 		eth_dev->data->dev_conf.rxmode.offloads |=
 			DEV_RX_OFFLOAD_CRC_STRIP;
diff --git a/drivers/net/e1000/em_rxtx.c b/drivers/net/e1000/em_rxtx.c
index a6b3e92a6..944118baf 100644
--- a/drivers/net/e1000/em_rxtx.c
+++ b/drivers/net/e1000/em_rxtx.c
@@ -1364,6 +1364,7 @@ em_get_rx_port_offloads_capa(struct rte_eth_dev *dev)
 		DEV_RX_OFFLOAD_UDP_CKSUM   |
 		DEV_RX_OFFLOAD_TCP_CKSUM   |
 		DEV_RX_OFFLOAD_CRC_STRIP   |
+		DEV_RX_OFFLOAD_KEEP_CRC    |
 		DEV_RX_OFFLOAD_SCATTER;
 	if (max_rx_pktlen > ETHER_MAX_LEN)
 		rx_offload_capa |= DEV_RX_OFFLOAD_JUMBO_FRAME;
@@ -1458,8 +1459,10 @@ eth_em_rx_queue_setup(struct rte_eth_dev *dev,
 	rxq->rx_free_thresh = rx_conf->rx_free_thresh;
 	rxq->queue_id = queue_idx;
 	rxq->port_id = dev->data->port_id;
-	rxq->crc_len = (uint8_t)((dev->data->dev_conf.rxmode.offloads &
-		DEV_RX_OFFLOAD_CRC_STRIP) ? 0 : ETHER_CRC_LEN);
+	if (rte_eth_dev_is_keep_crc(dev->data->dev_conf.rxmode.offloads))
+		rxq->crc_len = ETHER_CRC_LEN;
+	else
+		rxq->crc_len = 0;
 
 	rxq->rdt_reg_addr = E1000_PCI_REG_ADDR(hw, E1000_RDT(queue_idx));
 	rxq->rdh_reg_addr = E1000_PCI_REG_ADDR(hw, E1000_RDH(queue_idx));
@@ -1792,9 +1795,10 @@ eth_em_rx_init(struct rte_eth_dev *dev)
 		 * Reset crc_len in case it was changed after queue setup by a
 		 *  call to configure
 		 */
-		rxq->crc_len =
-			(uint8_t)(dev->data->dev_conf.rxmode.offloads &
-				DEV_RX_OFFLOAD_CRC_STRIP ? 0 : ETHER_CRC_LEN);
+		if (rte_eth_dev_is_keep_crc(dev->data->dev_conf.rxmode.offloads))
+			rxq->crc_len = ETHER_CRC_LEN;
+		else
+			rxq->crc_len = 0;
 
 		bus_addr = rxq->rx_ring_phys_addr;
 		E1000_WRITE_REG(hw, E1000_RDLEN(i),
@@ -1873,10 +1877,10 @@ eth_em_rx_init(struct rte_eth_dev *dev)
 	}
 
 	/* Setup the Receive Control Register. */
-	if (dev->data->dev_conf.rxmode.offloads & DEV_RX_OFFLOAD_CRC_STRIP)
-		rctl |= E1000_RCTL_SECRC; /* Strip Ethernet CRC. */
-	else
+	if (rte_eth_dev_is_keep_crc(dev->data->dev_conf.rxmode.offloads))
 		rctl &= ~E1000_RCTL_SECRC; /* Do not Strip Ethernet CRC. */
+	else
+		rctl |= E1000_RCTL_SECRC; /* Strip Ethernet CRC. */
 
 	rctl &= ~(3 << E1000_RCTL_MO_SHIFT);
 	rctl |= E1000_RCTL_EN | E1000_RCTL_BAM | E1000_RCTL_LBM_NO |
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index edc7be319..5c62445fe 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -3194,12 +3194,12 @@ igbvf_dev_configure(struct rte_eth_dev *dev)
 	 * Keep the persistent behavior the same as Host PF
 	 */
 #ifndef RTE_LIBRTE_E1000_PF_DISABLE_STRIP_CRC
-	if (!(conf->rxmode.offloads & DEV_RX_OFFLOAD_CRC_STRIP)) {
+	if (rte_eth_dev_is_keep_crc(conf->rxmode.offloads)) {
 		PMD_INIT_LOG(NOTICE, "VF can't disable HW CRC Strip");
 		conf->rxmode.offloads |= DEV_RX_OFFLOAD_CRC_STRIP;
 	}
 #else
-	if (conf->rxmode.offloads & DEV_RX_OFFLOAD_CRC_STRIP) {
+	if (!rte_eth_dev_is_keep_crc(conf->rxmode.offloads)) {
 		PMD_INIT_LOG(NOTICE, "VF can't enable HW CRC Strip");
 		conf->rxmode.offloads &= ~DEV_RX_OFFLOAD_CRC_STRIP;
 	}
diff --git a/drivers/net/e1000/igb_rxtx.c b/drivers/net/e1000/igb_rxtx.c
index 5f729f271..db594c1a3 100644
--- a/drivers/net/e1000/igb_rxtx.c
+++ b/drivers/net/e1000/igb_rxtx.c
@@ -1639,6 +1639,7 @@ igb_get_rx_port_offloads_capa(struct rte_eth_dev *dev)
 			  DEV_RX_OFFLOAD_TCP_CKSUM   |
 			  DEV_RX_OFFLOAD_JUMBO_FRAME |
 			  DEV_RX_OFFLOAD_CRC_STRIP   |
+			  DEV_RX_OFFLOAD_KEEP_CRC    |
 			  DEV_RX_OFFLOAD_SCATTER;
 
 	return rx_offload_capa;
@@ -1720,8 +1721,10 @@ eth_igb_rx_queue_setup(struct rte_eth_dev *dev,
 	rxq->reg_idx = (uint16_t)((RTE_ETH_DEV_SRIOV(dev).active == 0) ?
 		queue_idx : RTE_ETH_DEV_SRIOV(dev).def_pool_q_idx + queue_idx);
 	rxq->port_id = dev->data->port_id;
-	rxq->crc_len = (uint8_t)((dev->data->dev_conf.rxmode.offloads &
-			DEV_RX_OFFLOAD_CRC_STRIP) ? 0 : ETHER_CRC_LEN);
+	if (rte_eth_dev_is_keep_crc(dev->data->dev_conf.rxmode.offloads))
+		rxq->crc_len = ETHER_CRC_LEN;
+	else
+		rxq->crc_len = 0;
 
 	/*
 	 *  Allocate RX ring hardware descriptors. A memzone large enough to
@@ -2371,8 +2374,10 @@ eth_igb_rx_init(struct rte_eth_dev *dev)
 		 * Reset crc_len in case it was changed after queue setup by a
 		 *  call to configure
 		 */
-		rxq->crc_len = (uint8_t)(dev->data->dev_conf.rxmode.offloads &
-				DEV_RX_OFFLOAD_CRC_STRIP ? 0 : ETHER_CRC_LEN);
+		if (rte_eth_dev_is_keep_crc(dev->data->dev_conf.rxmode.offloads))
+			rxq->crc_len = ETHER_CRC_LEN;
+		else
+			rxq->crc_len = 0;
 
 		bus_addr = rxq->rx_ring_phys_addr;
 		E1000_WRITE_REG(hw, E1000_RDLEN(rxq->reg_idx),
@@ -2501,10 +2506,10 @@ eth_igb_rx_init(struct rte_eth_dev *dev)
 	E1000_WRITE_REG(hw, E1000_RXCSUM, rxcsum);
 
 	/* Setup the Receive Control Register. */
-	if (dev->data->dev_conf.rxmode.offloads & DEV_RX_OFFLOAD_CRC_STRIP) {
-		rctl |= E1000_RCTL_SECRC; /* Strip Ethernet CRC. */
+	if (rte_eth_dev_is_keep_crc(dev->data->dev_conf.rxmode.offloads)) {
+		rctl &= ~E1000_RCTL_SECRC; /* Do not Strip Ethernet CRC. */
 
-		/* set STRCRC bit in all queues */
+		/* clear STRCRC bit in all queues */
 		if (hw->mac.type == e1000_i350 ||
 		    hw->mac.type == e1000_i210 ||
 		    hw->mac.type == e1000_i211 ||
@@ -2513,14 +2518,14 @@ eth_igb_rx_init(struct rte_eth_dev *dev)
 				rxq = dev->data->rx_queues[i];
 				uint32_t dvmolr = E1000_READ_REG(hw,
 					E1000_DVMOLR(rxq->reg_idx));
-				dvmolr |= E1000_DVMOLR_STRCRC;
+				dvmolr &= ~E1000_DVMOLR_STRCRC;
 				E1000_WRITE_REG(hw, E1000_DVMOLR(rxq->reg_idx), dvmolr);
 			}
 		}
 	} else {
-		rctl &= ~E1000_RCTL_SECRC; /* Do not Strip Ethernet CRC. */
+		rctl |= E1000_RCTL_SECRC; /* Strip Ethernet CRC. */
 
-		/* clear STRCRC bit in all queues */
+		/* set STRCRC bit in all queues */
 		if (hw->mac.type == e1000_i350 ||
 		    hw->mac.type == e1000_i210 ||
 		    hw->mac.type == e1000_i211 ||
@@ -2529,7 +2534,7 @@ eth_igb_rx_init(struct rte_eth_dev *dev)
 				rxq = dev->data->rx_queues[i];
 				uint32_t dvmolr = E1000_READ_REG(hw,
 					E1000_DVMOLR(rxq->reg_idx));
-				dvmolr &= ~E1000_DVMOLR_STRCRC;
+				dvmolr |= E1000_DVMOLR_STRCRC;
 				E1000_WRITE_REG(hw, E1000_DVMOLR(rxq->reg_idx), dvmolr);
 			}
 		}
diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index 3ff1b0e0f..2b354dc15 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -451,8 +451,10 @@ fm10k_dev_configure(struct rte_eth_dev *dev)
 
 	PMD_INIT_FUNC_TRACE();
 
-	if ((dev->data->dev_conf.rxmode.offloads &
-	     DEV_RX_OFFLOAD_CRC_STRIP) == 0)
+	/* KEEP_CRC offload flag is not supported by PMD
+	 * can remove the below block when DEV_RX_OFFLOAD_CRC_STRIP removed
+	 */
+	if (rte_eth_dev_is_keep_crc(dev->data->dev_conf.rxmode.offloads))
 		PMD_INIT_LOG(WARNING, "fm10k always strip CRC");
 
 	/* multipe queue mode checking */
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 13c5d3296..1276bb207 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -3327,6 +3327,7 @@ i40e_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 		DEV_RX_OFFLOAD_TCP_CKSUM |
 		DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM |
 		DEV_RX_OFFLOAD_CRC_STRIP |
+		DEV_RX_OFFLOAD_KEEP_CRC |
 		DEV_RX_OFFLOAD_VLAN_EXTEND |
 		DEV_RX_OFFLOAD_VLAN_FILTER |
 		DEV_RX_OFFLOAD_JUMBO_FRAME;
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 804e44530..520655bbe 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -1536,7 +1536,7 @@ i40evf_dev_configure(struct rte_eth_dev *dev)
 	/* For non-DPDK PF drivers, VF has no ability to disable HW
 	 * CRC strip, and is implicitly enabled by the PF.
 	 */
-	if (!(conf->rxmode.offloads & DEV_RX_OFFLOAD_CRC_STRIP)) {
+	if (rte_eth_dev_is_keep_crc(conf->rxmode.offloads)) {
 		vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
 		if ((vf->version_major == VIRTCHNL_VERSION_MAJOR) &&
 		    (vf->version_minor <= VIRTCHNL_VERSION_MINOR)) {
@@ -2200,6 +2200,7 @@ i40evf_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 		DEV_RX_OFFLOAD_TCP_CKSUM |
 		DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM |
 		DEV_RX_OFFLOAD_CRC_STRIP |
+		DEV_RX_OFFLOAD_KEEP_CRC |
 		DEV_RX_OFFLOAD_SCATTER |
 		DEV_RX_OFFLOAD_JUMBO_FRAME |
 		DEV_RX_OFFLOAD_VLAN_FILTER;
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index 6032d5541..4c3a2924f 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -1829,8 +1829,10 @@ i40e_dev_rx_queue_setup(struct rte_eth_dev *dev,
 	rxq->queue_id = queue_idx;
 	rxq->reg_idx = reg_idx;
 	rxq->port_id = dev->data->port_id;
-	rxq->crc_len = (uint8_t)((dev->data->dev_conf.rxmode.offloads &
-			DEV_RX_OFFLOAD_CRC_STRIP) ? 0 : ETHER_CRC_LEN);
+	if (rte_eth_dev_is_keep_crc(dev->data->dev_conf.rxmode.offloads))
+		rxq->crc_len = ETHER_CRC_LEN;
+	else
+		rxq->crc_len = 0;
 	rxq->drop_en = rx_conf->rx_drop_en;
 	rxq->vsi = vsi;
 	rxq->rx_deferred_start = rx_conf->rx_deferred_start;
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 87d2ad090..60b7f038f 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -4991,12 +4991,12 @@ ixgbevf_dev_configure(struct rte_eth_dev *dev)
 	 * Keep the persistent behavior the same as Host PF
 	 */
 #ifndef RTE_LIBRTE_IXGBE_PF_DISABLE_STRIP_CRC
-	if (!(conf->rxmode.offloads & DEV_RX_OFFLOAD_CRC_STRIP)) {
+	if (rte_eth_dev_is_keep_crc(conf->rxmode.offloads)) {
 		PMD_INIT_LOG(NOTICE, "VF can't disable HW CRC Strip");
 		conf->rxmode.offloads |= DEV_RX_OFFLOAD_CRC_STRIP;
 	}
 #else
-	if (conf->rxmode.offloads & DEV_RX_OFFLOAD_CRC_STRIP) {
+	if (!rte_eth_dev_is_keep_crc(conf->rxmode.offloads)) {
 		PMD_INIT_LOG(NOTICE, "VF can't enable HW CRC Strip");
 		conf->rxmode.offloads &= ~DEV_RX_OFFLOAD_CRC_STRIP;
 	}
diff --git a/drivers/net/ixgbe/ixgbe_ipsec.c b/drivers/net/ixgbe/ixgbe_ipsec.c
index de7ed3676..da861accf 100644
--- a/drivers/net/ixgbe/ixgbe_ipsec.c
+++ b/drivers/net/ixgbe/ixgbe_ipsec.c
@@ -609,7 +609,7 @@ ixgbe_crypto_enable_ipsec(struct rte_eth_dev *dev)
 		PMD_DRV_LOG(ERR, "RSC and IPsec not supported");
 		return -1;
 	}
-	if (!(rx_offloads & DEV_RX_OFFLOAD_CRC_STRIP)) {
+	if (rte_eth_dev_is_keep_crc(rx_offloads)) {
 		PMD_DRV_LOG(ERR, "HW CRC strip needs to be enabled for IPsec");
 		return -1;
 	}
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index 3e13d26ae..b7b1d6be4 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -2849,6 +2849,7 @@ ixgbe_get_rx_port_offloads(struct rte_eth_dev *dev)
 		   DEV_RX_OFFLOAD_UDP_CKSUM   |
 		   DEV_RX_OFFLOAD_TCP_CKSUM   |
 		   DEV_RX_OFFLOAD_CRC_STRIP   |
+		   DEV_RX_OFFLOAD_KEEP_CRC    |
 		   DEV_RX_OFFLOAD_JUMBO_FRAME |
 		   DEV_RX_OFFLOAD_SCATTER;
 
@@ -2935,8 +2936,10 @@ ixgbe_dev_rx_queue_setup(struct rte_eth_dev *dev,
 	rxq->reg_idx = (uint16_t)((RTE_ETH_DEV_SRIOV(dev).active == 0) ?
 		queue_idx : RTE_ETH_DEV_SRIOV(dev).def_pool_q_idx + queue_idx);
 	rxq->port_id = dev->data->port_id;
-	rxq->crc_len = (uint8_t)((dev->data->dev_conf.rxmode.offloads &
-		DEV_RX_OFFLOAD_CRC_STRIP) ? 0 : ETHER_CRC_LEN);
+	if (rte_eth_dev_is_keep_crc(dev->data->dev_conf.rxmode.offloads))
+		rxq->crc_len = ETHER_CRC_LEN;
+	else
+		rxq->crc_len = 0;
 	rxq->drop_en = rx_conf->rx_drop_en;
 	rxq->rx_deferred_start = rx_conf->rx_deferred_start;
 	rxq->offloads = offloads;
@@ -4702,7 +4705,7 @@ ixgbe_set_rsc(struct rte_eth_dev *dev)
 
 	/* RSC global configuration (chapter 4.6.7.2.1 of 82599 Spec) */
 
-	if (!(rx_conf->offloads & DEV_RX_OFFLOAD_CRC_STRIP) &&
+	if (rte_eth_dev_is_keep_crc(rx_conf->offloads) &&
 	     (rx_conf->offloads & DEV_RX_OFFLOAD_TCP_LRO)) {
 		/*
 		 * According to chapter of 4.6.7.2.1 of the Spec Rev.
@@ -4851,10 +4854,10 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
 	 * Configure CRC stripping, if any.
 	 */
 	hlreg0 = IXGBE_READ_REG(hw, IXGBE_HLREG0);
-	if (rx_conf->offloads & DEV_RX_OFFLOAD_CRC_STRIP)
-		hlreg0 |= IXGBE_HLREG0_RXCRCSTRP;
-	else
+	if (rte_eth_dev_is_keep_crc(rx_conf->offloads))
 		hlreg0 &= ~IXGBE_HLREG0_RXCRCSTRP;
+	else
+		hlreg0 |= IXGBE_HLREG0_RXCRCSTRP;
 
 	/*
 	 * Configure jumbo frame support, if any.
@@ -4892,8 +4895,8 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
 		 * Reset crc_len in case it was changed after queue setup by a
 		 * call to configure.
 		 */
-		rxq->crc_len = (rx_conf->offloads & DEV_RX_OFFLOAD_CRC_STRIP) ?
-				0 : ETHER_CRC_LEN;
+		rxq->crc_len = rte_eth_dev_is_keep_crc(rx_conf->offloads) ?
+				ETHER_CRC_LEN : 0;
 
 		/* Setup the Base and Length of the Rx Descriptor Rings */
 		bus_addr = rxq->rx_ring_phys_addr;
@@ -4962,10 +4965,10 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
 	if (hw->mac.type == ixgbe_mac_82599EB ||
 	    hw->mac.type == ixgbe_mac_X540) {
 		rdrxctl = IXGBE_READ_REG(hw, IXGBE_RDRXCTL);
-		if (rx_conf->offloads & DEV_RX_OFFLOAD_CRC_STRIP)
-			rdrxctl |= IXGBE_RDRXCTL_CRCSTRIP;
-		else
+		if (rte_eth_dev_is_keep_crc(rx_conf->offloads))
 			rdrxctl &= ~IXGBE_RDRXCTL_CRCSTRIP;
+		else
+			rdrxctl |= IXGBE_RDRXCTL_CRCSTRIP;
 		rdrxctl &= ~IXGBE_RDRXCTL_RSCFRSTSIZE;
 		IXGBE_WRITE_REG(hw, IXGBE_RDRXCTL, rdrxctl);
 	}
diff --git a/drivers/net/kni/rte_eth_kni.c b/drivers/net/kni/rte_eth_kni.c
index ab63ea427..04cab248e 100644
--- a/drivers/net/kni/rte_eth_kni.c
+++ b/drivers/net/kni/rte_eth_kni.c
@@ -207,6 +207,7 @@ eth_kni_dev_info(struct rte_eth_dev *dev __rte_unused,
 	dev_info->max_rx_queues = KNI_MAX_QUEUE_PER_PORT;
 	dev_info->max_tx_queues = KNI_MAX_QUEUE_PER_PORT;
 	dev_info->min_rx_bufsize = 0;
+	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_CRC_STRIP;
 }
 
 static int
diff --git a/drivers/net/mlx4/mlx4_rxq.c b/drivers/net/mlx4/mlx4_rxq.c
index 87688c1c7..d223e15cb 100644
--- a/drivers/net/mlx4/mlx4_rxq.c
+++ b/drivers/net/mlx4/mlx4_rxq.c
@@ -672,7 +672,8 @@ uint64_t
 mlx4_get_rx_queue_offloads(struct priv *priv)
 {
 	uint64_t offloads = DEV_RX_OFFLOAD_SCATTER |
-			    DEV_RX_OFFLOAD_CRC_STRIP;
+			    DEV_RX_OFFLOAD_CRC_STRIP |
+			    DEV_RX_OFFLOAD_KEEP_CRC;
 
 	if (priv->hw_csum)
 		offloads |= DEV_RX_OFFLOAD_CHECKSUM;
@@ -771,16 +772,16 @@ mlx4_rx_queue_setup(struct rte_eth_dev *dev, uint16_t idx, uint16_t desc,
 		     (void *)dev, idx, desc);
 	}
 	/* By default, FCS (CRC) is stripped by hardware. */
-	if (offloads & DEV_RX_OFFLOAD_CRC_STRIP) {
-		crc_present = 0;
-	} else if (priv->hw_fcs_strip) {
-		crc_present = 1;
-	} else {
-		WARN("%p: CRC stripping has been disabled but will still"
-		     " be performed by hardware, make sure MLNX_OFED and"
-		     " firmware are up to date",
-		     (void *)dev);
-		crc_present = 0;
+	crc_present = 0;
+	if (rte_eth_dev_is_keep_crc(offloads)) {
+		if (priv->hw_fcs_strip) {
+			crc_present = 1;
+		} else {
+			WARN("%p: CRC stripping has been disabled but will still"
+			     " be performed by hardware, make sure MLNX_OFED and"
+			     " firmware are up to date",
+			     (void *)dev);
+		}
 	}
 	DEBUG("%p: CRC stripping is %s, %u bytes will be subtracted from"
 	      " incoming frames to hide it",
diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
index de3f869ed..28cf168aa 100644
--- a/drivers/net/mlx5/mlx5_rxq.c
+++ b/drivers/net/mlx5/mlx5_rxq.c
@@ -388,6 +388,9 @@ mlx5_get_rx_queue_offloads(struct rte_eth_dev *dev)
 
 	if (config->hw_fcs_strip)
 		offloads |= DEV_RX_OFFLOAD_CRC_STRIP;
+	else
+		offloads |= DEV_RX_OFFLOAD_KEEP_CRC;
+
 	if (config->hw_csum)
 		offloads |= (DEV_RX_OFFLOAD_IPV4_CKSUM |
 			     DEV_RX_OFFLOAD_UDP_CKSUM |
@@ -1419,17 +1422,17 @@ mlx5_rxq_new(struct rte_eth_dev *dev, uint16_t idx, uint16_t desc,
 	/* Configure VLAN stripping. */
 	tmpl->rxq.vlan_strip = !!(offloads & DEV_RX_OFFLOAD_VLAN_STRIP);
 	/* By default, FCS (CRC) is stripped by hardware. */
-	if (offloads & DEV_RX_OFFLOAD_CRC_STRIP) {
-		tmpl->rxq.crc_present = 0;
-	} else if (config->hw_fcs_strip) {
-		tmpl->rxq.crc_present = 1;
-	} else {
-		DRV_LOG(WARNING,
-			"port %u CRC stripping has been disabled but will"
-			" still be performed by hardware, make sure MLNX_OFED"
-			" and firmware are up to date",
-			dev->data->port_id);
-		tmpl->rxq.crc_present = 0;
+	tmpl->rxq.crc_present = 0;
+	if (rte_eth_dev_is_keep_crc(offloads)) {
+		if (config->hw_fcs_strip) {
+			tmpl->rxq.crc_present = 1;
+		} else {
+			DRV_LOG(WARNING,
+				"port %u CRC stripping has been disabled but will"
+				" still be performed by hardware, make sure MLNX_OFED"
+				" and firmware are up to date",
+				dev->data->port_id);
+		}
 	}
 	DRV_LOG(DEBUG,
 		"port %u CRC stripping is %s, %u bytes will be subtracted from"
diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c
index d5eb1fe69..74d067ac2 100644
--- a/drivers/net/mvpp2/mrvl_ethdev.c
+++ b/drivers/net/mvpp2/mrvl_ethdev.c
@@ -314,9 +314,11 @@ mrvl_dev_configure(struct rte_eth_dev *dev)
 		return -EINVAL;
 	}
 
-	if (!(dev->data->dev_conf.rxmode.offloads & DEV_RX_OFFLOAD_CRC_STRIP)) {
-		MRVL_LOG(INFO,
-			"L2 CRC stripping is always enabled in hw");
+	/* KEEP_CRC offload flag is not supported by PMD
+	 * can remove the below block when DEV_RX_OFFLOAD_CRC_STRIP removed
+	 */
+	if (rte_eth_dev_is_keep_crc(dev->data->dev_conf.rxmode.offloads)) {
+		MRVL_LOG(INFO, "L2 CRC stripping is always enabled in hw");
 		dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_CRC_STRIP;
 	}
 
diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index 36586969f..f8c65c1c6 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -411,8 +411,10 @@ nfp_net_configure(struct rte_eth_dev *dev)
 		return -EINVAL;
 	}
 
-	/* Checking RX offloads */
-	if (!(rxmode->offloads & DEV_RX_OFFLOAD_CRC_STRIP))
+	/* KEEP_CRC offload flag is not supported by PMD
+	 * can remove the below block when DEV_RX_OFFLOAD_CRC_STRIP removed
+	 */
+	if (rte_eth_dev_is_keep_crc(rxmode->offloads))
 		PMD_INIT_LOG(INFO, "HW does strip CRC. No configurable!");
 
 	return 0;
@@ -1166,7 +1168,8 @@ nfp_net_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 					     DEV_RX_OFFLOAD_UDP_CKSUM |
 					     DEV_RX_OFFLOAD_TCP_CKSUM;
 
-	dev_info->rx_offload_capa |= DEV_RX_OFFLOAD_JUMBO_FRAME;
+	dev_info->rx_offload_capa |= DEV_RX_OFFLOAD_JUMBO_FRAME |
+				     DEV_RX_OFFLOAD_KEEP_CRC;
 
 	if (hw->cap & NFP_NET_CFG_CTRL_TXVLAN)
 		dev_info->tx_offload_capa = DEV_TX_OFFLOAD_VLAN_INSERT;
diff --git a/drivers/net/null/rte_eth_null.c b/drivers/net/null/rte_eth_null.c
index 1d2e6b9e9..38c8c42bc 100644
--- a/drivers/net/null/rte_eth_null.c
+++ b/drivers/net/null/rte_eth_null.c
@@ -305,6 +305,7 @@ eth_dev_info(struct rte_eth_dev *dev,
 	dev_info->min_rx_bufsize = 0;
 	dev_info->reta_size = internals->reta_size;
 	dev_info->flow_type_rss_offloads = internals->flow_type_rss_offloads;
+	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_CRC_STRIP;
 }
 
 static int
diff --git a/drivers/net/octeontx/octeontx_ethdev.c b/drivers/net/octeontx/octeontx_ethdev.c
index 1eb453b21..b0541f115 100644
--- a/drivers/net/octeontx/octeontx_ethdev.c
+++ b/drivers/net/octeontx/octeontx_ethdev.c
@@ -283,7 +283,10 @@ octeontx_dev_configure(struct rte_eth_dev *dev)
 		return -EINVAL;
 	}
 
-	if (!(rxmode->offloads & DEV_RX_OFFLOAD_CRC_STRIP)) {
+	/* KEEP_CRC offload flag is not supported by PMD
+	 * can remove the below block when DEV_RX_OFFLOAD_CRC_STRIP removed
+	 */
+	if (rte_eth_dev_is_keep_crc(rxmode->offloads)) {
 		PMD_INIT_LOG(NOTICE, "can't disable hw crc strip");
 		rxmode->offloads |= DEV_RX_OFFLOAD_CRC_STRIP;
 	}
diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index 6bd4a7d79..626e89833 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -538,6 +538,7 @@ eth_dev_info(struct rte_eth_dev *dev,
 	dev_info->max_rx_queues = dev->data->nb_rx_queues;
 	dev_info->max_tx_queues = dev->data->nb_tx_queues;
 	dev_info->min_rx_bufsize = 0;
+	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_CRC_STRIP;
 }
 
 static int
diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index 7a63d0564..e5e8ea4a3 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -1532,6 +1532,7 @@ qede_dev_info_get(struct rte_eth_dev *eth_dev,
 				     DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM |
 				     DEV_RX_OFFLOAD_TCP_LRO	|
 				     DEV_RX_OFFLOAD_CRC_STRIP	|
+				     DEV_RX_OFFLOAD_KEEP_CRC    |
 				     DEV_RX_OFFLOAD_SCATTER	|
 				     DEV_RX_OFFLOAD_JUMBO_FRAME |
 				     DEV_RX_OFFLOAD_VLAN_FILTER |
@@ -1562,6 +1563,7 @@ qede_dev_info_get(struct rte_eth_dev *eth_dev,
 		.rx_drop_en = 1,
 		/* The below RX offloads are always enabled */
 		.offloads = (DEV_RX_OFFLOAD_CRC_STRIP  |
+			     DEV_RX_OFFLOAD_KEEP_CRC   |
 			     DEV_RX_OFFLOAD_IPV4_CKSUM |
 			     DEV_RX_OFFLOAD_TCP_CKSUM  |
 			     DEV_RX_OFFLOAD_UDP_CKSUM),
diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
index 35b837c3f..5aa48da42 100644
--- a/drivers/net/ring/rte_eth_ring.c
+++ b/drivers/net/ring/rte_eth_ring.c
@@ -164,6 +164,7 @@ eth_dev_info(struct rte_eth_dev *dev,
 	dev_info->max_rx_queues = (uint16_t)internals->max_rx_queues;
 	dev_info->max_tx_queues = (uint16_t)internals->max_tx_queues;
 	dev_info->min_rx_bufsize = 0;
+	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_CRC_STRIP;
 }
 
 static int
diff --git a/drivers/net/sfc/sfc_rx.c b/drivers/net/sfc/sfc_rx.c
index cc76a5b15..cdcc70b26 100644
--- a/drivers/net/sfc/sfc_rx.c
+++ b/drivers/net/sfc/sfc_rx.c
@@ -1443,7 +1443,10 @@ sfc_rx_check_mode(struct sfc_adapter *sa, struct rte_eth_rxmode *rxmode)
 		rc = EINVAL;
 	}
 
-	if (~rxmode->offloads & DEV_RX_OFFLOAD_CRC_STRIP) {
+	/* KEEP_CRC offload flag is not supported by PMD
+	 * can remove the below block when DEV_RX_OFFLOAD_CRC_STRIP removed
+	 */
+	if (rte_eth_dev_is_keep_crc(rxmode->offloads)) {
 		sfc_warn(sa, "FCS stripping cannot be disabled - always on");
 		rxmode->offloads |= DEV_RX_OFFLOAD_CRC_STRIP;
 		rxmode->hw_strip_crc = 1;
diff --git a/drivers/net/softnic/rte_eth_softnic.c b/drivers/net/softnic/rte_eth_softnic.c
index 6b3c13e5c..9360cf6c3 100644
--- a/drivers/net/softnic/rte_eth_softnic.c
+++ b/drivers/net/softnic/rte_eth_softnic.c
@@ -65,6 +65,7 @@ static const struct rte_eth_dev_info pmd_dev_info = {
 		.nb_min = 0,
 		.nb_align = 1,
 	},
+	.rx_offload_capa = DEV_RX_OFFLOAD_CRC_STRIP,
 };
 
 static int pmd_softnic_logtype;
diff --git a/drivers/net/szedata2/rte_eth_szedata2.c b/drivers/net/szedata2/rte_eth_szedata2.c
index 910c64d04..829ad13fa 100644
--- a/drivers/net/szedata2/rte_eth_szedata2.c
+++ b/drivers/net/szedata2/rte_eth_szedata2.c
@@ -1056,7 +1056,8 @@ eth_dev_info(struct rte_eth_dev *dev,
 	dev_info->max_rx_queues = internals->max_rx_queues;
 	dev_info->max_tx_queues = internals->max_tx_queues;
 	dev_info->min_rx_bufsize = 0;
-	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_SCATTER;
+	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_SCATTER |
+				    DEV_RX_OFFLOAD_CRC_STRIP;
 	dev_info->tx_offload_capa = 0;
 	dev_info->rx_queue_offload_capa = 0;
 	dev_info->tx_queue_offload_capa = 0;
diff --git a/drivers/net/thunderx/nicvf_ethdev.c b/drivers/net/thunderx/nicvf_ethdev.c
index 99fcd516b..f92f38f42 100644
--- a/drivers/net/thunderx/nicvf_ethdev.c
+++ b/drivers/net/thunderx/nicvf_ethdev.c
@@ -1887,7 +1887,10 @@ nicvf_dev_configure(struct rte_eth_dev *dev)
 		return -EINVAL;
 	}
 
-	if ((rxmode->offloads & DEV_RX_OFFLOAD_CRC_STRIP) == 0) {
+	/* KEEP_CRC offload flag is not supported by PMD
+	 * can remove the below block when DEV_RX_OFFLOAD_CRC_STRIP removed
+	 */
+	if (rte_eth_dev_is_keep_crc(rxmode->offloads)) {
 		PMD_INIT_LOG(NOTICE, "Can't disable hw crc strip");
 		rxmode->offloads |= DEV_RX_OFFLOAD_CRC_STRIP;
 	}
diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index ba9d768a0..eb1afe691 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -1070,7 +1070,8 @@ eth_dev_info(struct rte_eth_dev *dev,
 
 	dev_info->tx_offload_capa = DEV_TX_OFFLOAD_MULTI_SEGS |
 				DEV_TX_OFFLOAD_VLAN_INSERT;
-	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_VLAN_STRIP;
+	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_VLAN_STRIP |
+				    DEV_RX_OFFLOAD_CRC_STRIP;
 }
 
 static int
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index df50a571a..b629cd45d 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -2127,7 +2127,8 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	};
 
 	host_features = VTPCI_OPS(hw)->get_features(hw);
-	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_VLAN_STRIP;
+	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_VLAN_STRIP |
+				    DEV_RX_OFFLOAD_CRC_STRIP;
 	if (host_features & (1ULL << VIRTIO_NET_F_GUEST_CSUM)) {
 		dev_info->rx_offload_capa |=
 			DEV_RX_OFFLOAD_TCP_CKSUM |
diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
index ba932ff27..95b465952 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
@@ -57,7 +57,8 @@
 	 DEV_RX_OFFLOAD_UDP_CKSUM |	\
 	 DEV_RX_OFFLOAD_TCP_CKSUM |	\
 	 DEV_RX_OFFLOAD_TCP_LRO |	\
-	 DEV_RX_OFFLOAD_JUMBO_FRAME)
+	 DEV_RX_OFFLOAD_JUMBO_FRAME |   \
+	 DEV_RX_OFFLOAD_CRC_STRIP)
 
 static int eth_vmxnet3_dev_init(struct rte_eth_dev *eth_dev);
 static int eth_vmxnet3_dev_uninit(struct rte_eth_dev *eth_dev);
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index a9977df97..5d0132223 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -129,6 +129,7 @@ static const struct {
 	RTE_RX_OFFLOAD_BIT2STR(SCATTER),
 	RTE_RX_OFFLOAD_BIT2STR(TIMESTAMP),
 	RTE_RX_OFFLOAD_BIT2STR(SECURITY),
+	RTE_RX_OFFLOAD_BIT2STR(KEEP_CRC),
 };
 
 #undef RTE_RX_OFFLOAD_BIT2STR
@@ -1185,6 +1186,14 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
 		return -EINVAL;
 	}
 
+	if ((local_conf.rxmode.offloads & DEV_RX_OFFLOAD_CRC_STRIP) &&
+			(local_conf.rxmode.offloads & DEV_RX_OFFLOAD_KEEP_CRC)) {
+		ethdev_log(ERR,
+			"Port id=%u not allowed to set both CRC STRIP and KEEP CRC offload flags\n",
+			port_id);
+		return -EINVAL;
+	}
+
 	/* Check that device supports requested rss hash functions. */
 	if ((dev_info.flow_type_rss_offloads |
 	     dev_conf->rx_adv_conf.rss_conf.rss_hf) !=
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index 36e3984ea..1f5652e85 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -939,6 +939,11 @@ struct rte_eth_conf {
 #define DEV_RX_OFFLOAD_SCATTER		0x00002000
 #define DEV_RX_OFFLOAD_TIMESTAMP	0x00004000
 #define DEV_RX_OFFLOAD_SECURITY         0x00008000
+
+/* Invalid to set both DEV_RX_OFFLOAD_CRC_STRIP and DEV_RX_OFFLOAD_KEEP_CRC
+ * No DEV_RX_OFFLOAD_CRC_STRIP flag means keep CRC
+ */
+#define DEV_RX_OFFLOAD_KEEP_CRC		0x00010000
 #define DEV_RX_OFFLOAD_CHECKSUM (DEV_RX_OFFLOAD_IPV4_CKSUM | \
 				 DEV_RX_OFFLOAD_UDP_CKSUM | \
 				 DEV_RX_OFFLOAD_TCP_CKSUM)
diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h
index c9c825e3f..09a42f8c2 100644
--- a/lib/librte_ethdev/rte_ethdev_driver.h
+++ b/lib/librte_ethdev/rte_ethdev_driver.h
@@ -325,6 +325,26 @@ typedef int (*ethdev_uninit_t)(struct rte_eth_dev *ethdev);
 int __rte_experimental
 rte_eth_dev_destroy(struct rte_eth_dev *ethdev, ethdev_uninit_t ethdev_uninit);
 
+/**
+ * PMD helper function to check if keeping CRC is requested
+ *
+ * @param rx_offloads
+ *   offloads variable
+ *
+ * @return
+ *   Return positive if keeping CRC is requested,
+ *   zero if stripping CRC is requested
+ */
+static inline int
+rte_eth_dev_is_keep_crc(uint64_t rx_offloads)
+{
+	if (rx_offloads & DEV_RX_OFFLOAD_CRC_STRIP)
+		return 0;
+
+	/* no KEEP_CRC or CRC_STRIP offload flags means keep CRC */
+	return 1;
+}
+
 #ifdef __cplusplus
 }
 #endif
-- 
2.17.1

Re: [dpdk-dev] [PATCH] ethdev: add new offload flag to keep CRC

[Andrew Rybchenko]

On 06/19/2018 09:02 PM, Ferruh Yigit wrote:
> DEV_RX_OFFLOAD_KEEP_CRC offload flag added. PMDs that supports keeping
> CRC should advertise this offload capability.
>
> DEV_RX_OFFLOAD_CRC_STRIP flag will remain one more release
> default behavior in PMDs are to keep the CRC until this flag removed
>
> Until DEV_RX_OFFLOAD_CRC_STRIP flag is removed:
> - Setting both KEEP_CRC & CRC_STRIP is INVALID
> - Setting only CRC_STRIP PMD should strip the CRC
> - Setting only KEEP_CRC PMD should keep the CRC
> - Not setting both PMD should keep the CRC
>
> A helper function rte_eth_dev_is_keep_crc() has been added to be able to
> change the no flag behavior with minimal changes in PMDs.
>
> The PMDs that doesn't report the DEV_RX_OFFLOAD_KEEP_CRC offload can
> remove rte_eth_dev_is_keep_crc() checks next release, related code
> commented to help the maintenance task.
>
> And DEV_RX_OFFLOAD_CRC_STRIP has been added to virtual drivers since
> they don't use CRC at all, when an application requires this offload
> virtual PMDs should not return error.
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---

<...>

> diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h
> index c9c825e3f..09a42f8c2 100644
> --- a/lib/librte_ethdev/rte_ethdev_driver.h
> +++ b/lib/librte_ethdev/rte_ethdev_driver.h
> @@ -325,6 +325,26 @@ typedef int (*ethdev_uninit_t)(struct rte_eth_dev *ethdev);
>   int __rte_experimental
>   rte_eth_dev_destroy(struct rte_eth_dev *ethdev, ethdev_uninit_t ethdev_uninit);
>   
> +/**
> + * PMD helper function to check if keeping CRC is requested
> + *
> + * @param rx_offloads
> + *   offloads variable
> + *
> + * @return
> + *   Return positive if keeping CRC is requested,
> + *   zero if stripping CRC is requested
> + */
> +static inline int
> +rte_eth_dev_is_keep_crc(uint64_t rx_offloads)
> +{
> +	if (rx_offloads & DEV_RX_OFFLOAD_CRC_STRIP)
> +		return 0;
> +
> +	/* no KEEP_CRC or CRC_STRIP offload flags means keep CRC */
> +	return 1;
> +}
> +
>   #ifdef __cplusplus
>   }
>   #endif

A couple of control questions about the function:
  - shouldn't __rte_experimental be used?
  - if the function remains in the future, it will be a bit asymmetric 
vs other
    offload flags. Right now it is clear why the function is introduced, but
    it is the question if the function should remain or go away in the 
future
    (as far as I know no other offload flag has similar function to check).

above things are really minor, ethdev and net/sfc
Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>

Re: [dpdk-dev] [PATCH] ethdev: add new offload flag to keep CRC

[Legacy, Allain]

> -----Original Message-----
> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> Sent: Tuesday, June 19, 2018 2:03 PM
<...>
> Subject: [PATCH] ethdev: add new offload flag to keep CRC
> 
> DEV_RX_OFFLOAD_KEEP_CRC offload flag added. PMDs that supports
> keeping CRC should advertise this offload capability.
> 
> DEV_RX_OFFLOAD_CRC_STRIP flag will remain one more release default
> behavior in PMDs are to keep the CRC until this flag removed
> 
> Until DEV_RX_OFFLOAD_CRC_STRIP flag is removed:
> - Setting both KEEP_CRC & CRC_STRIP is INVALID
> - Setting only CRC_STRIP PMD should strip the CRC
> - Setting only KEEP_CRC PMD should keep the CRC
> - Not setting both PMD should keep the CRC
> 
> A helper function rte_eth_dev_is_keep_crc() has been added to be able to
> change the no flag behavior with minimal changes in PMDs.
> 
> The PMDs that doesn't report the DEV_RX_OFFLOAD_KEEP_CRC offload can
> remove rte_eth_dev_is_keep_crc() checks next release, related code
> commented to help the maintenance task.
> 
> And DEV_RX_OFFLOAD_CRC_STRIP has been added to virtual drivers since
> they don't use CRC at all, when an application requires this offload virtual
> PMDs should not return error.
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---

For net/avp:

Acked-by:  Allain Legacy <allain.legacy@windriver.com>

Re: [dpdk-dev] [PATCH] ethdev: add new offload flag to keep CRC

[Shahaf Shuler]

Hi Ferruh,

Tuesday, June 19, 2018 9:03 PM, Ferruh Yigit
> Subject: [dpdk-dev] [PATCH] ethdev: add new offload flag to keep CRC
> 

[...]


> diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
> index de3f869ed..28cf168aa 100644
> --- a/drivers/net/mlx5/mlx5_rxq.c
> +++ b/drivers/net/mlx5/mlx5_rxq.c
> @@ -388,6 +388,9 @@ mlx5_get_rx_queue_offloads(struct rte_eth_dev
> *dev)
> 
>  	if (config->hw_fcs_strip)
>  		offloads |= DEV_RX_OFFLOAD_CRC_STRIP;
> +	else
> +		offloads |= DEV_RX_OFFLOAD_KEEP_CRC;
> +

I think it should be:
if (config->hw_fcs_strip) {
	offloads |= DEV_RX_OFFLOAD_CRC_STRIP;
	offloads |= DEV_RX_OFFLOAD_KEEP_CRC;
}

The hw_fcs_strip is the capability from device which allows the PMD to toggle the CRC stripping.

>  	if (config->hw_csum)
>  		offloads |= (DEV_RX_OFFLOAD_IPV4_CKSUM |
>  			     DEV_RX_OFFLOAD_UDP_CKSUM |
> @@ -1419,17 +1422,17 @@ mlx5_rxq_new(struct rte_eth_dev *dev,
> uint16_t idx, uint16_t desc,
>  	/* Configure VLAN stripping. */
>  	tmpl->rxq.vlan_strip = !!(offloads &
> DEV_RX_OFFLOAD_VLAN_STRIP);
>  	/* By default, FCS (CRC) is stripped by hardware. */
> -	if (offloads & DEV_RX_OFFLOAD_CRC_STRIP) {
> -		tmpl->rxq.crc_present = 0;
> -	} else if (config->hw_fcs_strip) {
> -		tmpl->rxq.crc_present = 1;
> -	} else {
> -		DRV_LOG(WARNING,
> -			"port %u CRC stripping has been disabled but will"
> -			" still be performed by hardware, make sure
> MLNX_OFED"
> -			" and firmware are up to date",
> -			dev->data->port_id);
> -		tmpl->rxq.crc_present = 0;
> +	tmpl->rxq.crc_present = 0;
> +	if (rte_eth_dev_is_keep_crc(offloads)) {
> +		if (config->hw_fcs_strip) {
> +			tmpl->rxq.crc_present = 1;
> +		} else {
> +			DRV_LOG(WARNING,
> +				"port %u CRC stripping has been disabled but
> will"
> +				" still be performed by hardware, make sure
> MLNX_OFED"
> +				" and firmware are up to date",
> +				dev->data->port_id);
> +		}
>  	}
>  	DRV_LOG(DEBUG,
>  		"port %u CRC stripping is %s, %u bytes will be subtracted
> from"

Re: [dpdk-dev] [PATCH] ethdev: add new offload flag to keep CRC

[Ferruh Yigit]

On 6/20/2018 2:44 PM, Shahaf Shuler wrote:
> 
> Hi Ferruh,
> 
> Tuesday, June 19, 2018 9:03 PM, Ferruh Yigit
>> Subject: [dpdk-dev] [PATCH] ethdev: add new offload flag to keep CRC
>>
> 
> [...]
> 
> 
>> diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
>> index de3f869ed..28cf168aa 100644
>> --- a/drivers/net/mlx5/mlx5_rxq.c
>> +++ b/drivers/net/mlx5/mlx5_rxq.c
>> @@ -388,6 +388,9 @@ mlx5_get_rx_queue_offloads(struct rte_eth_dev
>> *dev)
>>
>>  	if (config->hw_fcs_strip)
>>  		offloads |= DEV_RX_OFFLOAD_CRC_STRIP;
>> +	else
>> +		offloads |= DEV_RX_OFFLOAD_KEEP_CRC;
>> +
> 
> I think it should be:
> if (config->hw_fcs_strip) {
> 	offloads |= DEV_RX_OFFLOAD_CRC_STRIP;
> 	offloads |= DEV_RX_OFFLOAD_KEEP_CRC;
> }
> 
> The hw_fcs_strip is the capability from device which allows the PMD to toggle the CRC stripping.

>From below logic, (how "crc_present" set), hw_fcs_strip looks like capability
from device that says keeping CRC is supported. If so it the original code was
not clear to me, why to report CRC stripping only if HW supports keeping CRC?

Following makes more sense to me, based on below code, report CRC stripping
capability by default and report KEEP CRC capability if device supports it:

 offloads |= DEV_RX_OFFLOAD_CRC_STRIP;
 if (config->hw_fcs_strip)
	offloads |= DEV_RX_OFFLOAD_KEEP_CRC;

What do you think?

> 
>>  	if (config->hw_csum)
>>  		offloads |= (DEV_RX_OFFLOAD_IPV4_CKSUM |
>>  			     DEV_RX_OFFLOAD_UDP_CKSUM |
>> @@ -1419,17 +1422,17 @@ mlx5_rxq_new(struct rte_eth_dev *dev,
>> uint16_t idx, uint16_t desc,
>>  	/* Configure VLAN stripping. */
>>  	tmpl->rxq.vlan_strip = !!(offloads &
>> DEV_RX_OFFLOAD_VLAN_STRIP);
>>  	/* By default, FCS (CRC) is stripped by hardware. */
>> -	if (offloads & DEV_RX_OFFLOAD_CRC_STRIP) {
>> -		tmpl->rxq.crc_present = 0;
>> -	} else if (config->hw_fcs_strip) {
>> -		tmpl->rxq.crc_present = 1;
>> -	} else {
>> -		DRV_LOG(WARNING,
>> -			"port %u CRC stripping has been disabled but will"
>> -			" still be performed by hardware, make sure
>> MLNX_OFED"
>> -			" and firmware are up to date",
>> -			dev->data->port_id);
>> -		tmpl->rxq.crc_present = 0;
>> +	tmpl->rxq.crc_present = 0;
>> +	if (rte_eth_dev_is_keep_crc(offloads)) {
>> +		if (config->hw_fcs_strip) {
>> +			tmpl->rxq.crc_present = 1;
>> +		} else {
>> +			DRV_LOG(WARNING,
>> +				"port %u CRC stripping has been disabled but
>> will"
>> +				" still be performed by hardware, make sure
>> MLNX_OFED"
>> +				" and firmware are up to date",
>> +				dev->data->port_id);
>> +		}
>>  	}
>>  	DRV_LOG(DEBUG,
>>  		"port %u CRC stripping is %s, %u bytes will be subtracted
>> from"

Re: [dpdk-dev] [PATCH] ethdev: add new offload flag to keep CRC

[Ferruh Yigit]

On 6/20/2018 8:42 AM, Andrew Rybchenko wrote:
> On 06/19/2018 09:02 PM, Ferruh Yigit wrote:
>> DEV_RX_OFFLOAD_KEEP_CRC offload flag added. PMDs that supports keeping
>> CRC should advertise this offload capability.
>>
>> DEV_RX_OFFLOAD_CRC_STRIP flag will remain one more release
>> default behavior in PMDs are to keep the CRC until this flag removed
>>
>> Until DEV_RX_OFFLOAD_CRC_STRIP flag is removed:
>> - Setting both KEEP_CRC & CRC_STRIP is INVALID
>> - Setting only CRC_STRIP PMD should strip the CRC
>> - Setting only KEEP_CRC PMD should keep the CRC
>> - Not setting both PMD should keep the CRC
>>
>> A helper function rte_eth_dev_is_keep_crc() has been added to be able to
>> change the no flag behavior with minimal changes in PMDs.
>>
>> The PMDs that doesn't report the DEV_RX_OFFLOAD_KEEP_CRC offload can
>> remove rte_eth_dev_is_keep_crc() checks next release, related code
>> commented to help the maintenance task.
>>
>> And DEV_RX_OFFLOAD_CRC_STRIP has been added to virtual drivers since
>> they don't use CRC at all, when an application requires this offload
>> virtual PMDs should not return error.
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> ---
> 
> <...>
> 
>> diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h
>> index c9c825e3f..09a42f8c2 100644
>> --- a/lib/librte_ethdev/rte_ethdev_driver.h
>> +++ b/lib/librte_ethdev/rte_ethdev_driver.h
>> @@ -325,6 +325,26 @@ typedef int (*ethdev_uninit_t)(struct rte_eth_dev *ethdev);
>>  int __rte_experimental
>>  rte_eth_dev_destroy(struct rte_eth_dev *ethdev, ethdev_uninit_t ethdev_uninit);
>>  
>> +/**
>> + * PMD helper function to check if keeping CRC is requested
>> + *
>> + * @param rx_offloads
>> + *   offloads variable
>> + *
>> + * @return
>> + *   Return positive if keeping CRC is requested,
>> + *   zero if stripping CRC is requested
>> + */
>> +static inline int
>> +rte_eth_dev_is_keep_crc(uint64_t rx_offloads)
>> +{
>> +	if (rx_offloads & DEV_RX_OFFLOAD_CRC_STRIP)
>> +		return 0;
>> +
>> +	/* no KEEP_CRC or CRC_STRIP offload flags means keep CRC */
>> +	return 1;
>> +}
>> +
>>  #ifdef __cplusplus
>>  }
>>  #endif
> 
> A couple of control questions about the function:
>  - shouldn't __rte_experimental be used?

This is an internal function, not API, so I think doesn't require to be
experimental.

>  - if the function remains in the future, it will be a bit asymmetric vs other
>    offload flags. Right now it is clear why the function is introduced, but
>    it is the question if the function should remain or go away in the future
>    (as far as I know no other offload flag has similar function to check).

No other offloads don't have similar functions, this is kind special.

There will be more changes related CRC next release, CRC_STRIP will be removed
and no flag will mean strip CRC. So the conditions to is_keep_crc will be changed.
This function is to manage this change easier, localize the information in to
single function to make it easy to update later.

> 
> above things are really minor, ethdev and net/sfc
> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> 

Re: [dpdk-dev] [PATCH] ethdev: add new offload flag to keep CRC

[Andrew Rybchenko]

On 06/20/2018 08:24 PM, Ferruh Yigit wrote:
> On 6/20/2018 8:42 AM, Andrew Rybchenko wrote:
>> On 06/19/2018 09:02 PM, Ferruh Yigit wrote:
>>> DEV_RX_OFFLOAD_KEEP_CRC offload flag added. PMDs that supports keeping
>>> CRC should advertise this offload capability.
>>>
>>> DEV_RX_OFFLOAD_CRC_STRIP flag will remain one more release
>>> default behavior in PMDs are to keep the CRC until this flag removed
>>>
>>> Until DEV_RX_OFFLOAD_CRC_STRIP flag is removed:
>>> - Setting both KEEP_CRC & CRC_STRIP is INVALID
>>> - Setting only CRC_STRIP PMD should strip the CRC
>>> - Setting only KEEP_CRC PMD should keep the CRC
>>> - Not setting both PMD should keep the CRC
>>>
>>> A helper function rte_eth_dev_is_keep_crc() has been added to be able to
>>> change the no flag behavior with minimal changes in PMDs.
>>>
>>> The PMDs that doesn't report the DEV_RX_OFFLOAD_KEEP_CRC offload can
>>> remove rte_eth_dev_is_keep_crc() checks next release, related code
>>> commented to help the maintenance task.
>>>
>>> And DEV_RX_OFFLOAD_CRC_STRIP has been added to virtual drivers since
>>> they don't use CRC at all, when an application requires this offload
>>> virtual PMDs should not return error.
>>>
>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>> ---
>> <...>
>>
>>> diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h
>>> index c9c825e3f..09a42f8c2 100644
>>> --- a/lib/librte_ethdev/rte_ethdev_driver.h
>>> +++ b/lib/librte_ethdev/rte_ethdev_driver.h
>>> @@ -325,6 +325,26 @@ typedef int (*ethdev_uninit_t)(struct rte_eth_dev *ethdev);
>>>   int __rte_experimental
>>>   rte_eth_dev_destroy(struct rte_eth_dev *ethdev, ethdev_uninit_t ethdev_uninit);
>>>   
>>> +/**
>>> + * PMD helper function to check if keeping CRC is requested
>>> + *
>>> + * @param rx_offloads
>>> + *   offloads variable
>>> + *
>>> + * @return
>>> + *   Return positive if keeping CRC is requested,
>>> + *   zero if stripping CRC is requested
>>> + */
>>> +static inline int
>>> +rte_eth_dev_is_keep_crc(uint64_t rx_offloads)
>>> +{
>>> +	if (rx_offloads & DEV_RX_OFFLOAD_CRC_STRIP)
>>> +		return 0;
>>> +
>>> +	/* no KEEP_CRC or CRC_STRIP offload flags means keep CRC */
>>> +	return 1;
>>> +}
>>> +
>>>   #ifdef __cplusplus
>>>   }
>>>   #endif
>> A couple of control questions about the function:
>>   - shouldn't __rte_experimental be used?
> This is an internal function, not API, so I think doesn't require to be
> experimental.

Just to make my thoughts clear: description does not say that it is an 
internal.
So, nothing prevents external entities to use it. Changes will be API 
breakage.

>>   - if the function remains in the future, it will be a bit asymmetric vs other
>>     offload flags. Right now it is clear why the function is introduced, but
>>     it is the question if the function should remain or go away in the future
>>     (as far as I know no other offload flag has similar function to check).
> No other offloads don't have similar functions, this is kind special.
>
> There will be more changes related CRC next release, CRC_STRIP will be removed
> and no flag will mean strip CRC. So the conditions to is_keep_crc will be changed.
> This function is to manage this change easier, localize the information in to
> single function to make it easy to update later.

It is perfectly clear why it is required right now and introduced (as I said
from the very beginning).
Yes, it is will be the history which explains why it is so, but if we make
a step forward and discard the history it will look asymmetric -
it will be a function which checks single bit. It is really minor and
100% up to you.

Many thanks for reply.

Re: [dpdk-dev] [PATCH] ethdev: add new offload flag to keep CRC

[Ferruh Yigit]

On 6/20/2018 6:39 PM, Andrew Rybchenko wrote:
> On 06/20/2018 08:24 PM, Ferruh Yigit wrote:
>> On 6/20/2018 8:42 AM, Andrew Rybchenko wrote:
>>> On 06/19/2018 09:02 PM, Ferruh Yigit wrote:
>>>> DEV_RX_OFFLOAD_KEEP_CRC offload flag added. PMDs that supports keeping
>>>> CRC should advertise this offload capability.
>>>>
>>>> DEV_RX_OFFLOAD_CRC_STRIP flag will remain one more release
>>>> default behavior in PMDs are to keep the CRC until this flag removed
>>>>
>>>> Until DEV_RX_OFFLOAD_CRC_STRIP flag is removed:
>>>> - Setting both KEEP_CRC & CRC_STRIP is INVALID
>>>> - Setting only CRC_STRIP PMD should strip the CRC
>>>> - Setting only KEEP_CRC PMD should keep the CRC
>>>> - Not setting both PMD should keep the CRC
>>>>
>>>> A helper function rte_eth_dev_is_keep_crc() has been added to be able to
>>>> change the no flag behavior with minimal changes in PMDs.
>>>>
>>>> The PMDs that doesn't report the DEV_RX_OFFLOAD_KEEP_CRC offload can
>>>> remove rte_eth_dev_is_keep_crc() checks next release, related code
>>>> commented to help the maintenance task.
>>>>
>>>> And DEV_RX_OFFLOAD_CRC_STRIP has been added to virtual drivers since
>>>> they don't use CRC at all, when an application requires this offload
>>>> virtual PMDs should not return error.
>>>>
>>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>>> ---
>>> <...>
>>>
>>>> diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h
>>>> index c9c825e3f..09a42f8c2 100644
>>>> --- a/lib/librte_ethdev/rte_ethdev_driver.h
>>>> +++ b/lib/librte_ethdev/rte_ethdev_driver.h
>>>> @@ -325,6 +325,26 @@ typedef int (*ethdev_uninit_t)(struct rte_eth_dev *ethdev);
>>>>  int __rte_experimental
>>>>  rte_eth_dev_destroy(struct rte_eth_dev *ethdev, ethdev_uninit_t ethdev_uninit);
>>>>  
>>>> +/**
>>>> + * PMD helper function to check if keeping CRC is requested
>>>> + *
>>>> + * @param rx_offloads
>>>> + *   offloads variable
>>>> + *
>>>> + * @return
>>>> + *   Return positive if keeping CRC is requested,
>>>> + *   zero if stripping CRC is requested
>>>> + */
>>>> +static inline int
>>>> +rte_eth_dev_is_keep_crc(uint64_t rx_offloads)
>>>> +{
>>>> +	if (rx_offloads & DEV_RX_OFFLOAD_CRC_STRIP)
>>>> +		return 0;
>>>> +
>>>> +	/* no KEEP_CRC or CRC_STRIP offload flags means keep CRC */
>>>> +	return 1;
>>>> +}
>>>> +
>>>>  #ifdef __cplusplus
>>>>  }
>>>>  #endif
>>> A couple of control questions about the function:
>>>  - shouldn't __rte_experimental be used?
>> This is an internal function, not API, so I think doesn't require to be
>> experimental.
> 
> Just to make my thoughts clear: description does not say that it is an internal.
> So, nothing prevents external entities to use it. Changes will be API breakage.

rte_ethdev_driver.h is not public header, it is just for PMDs.

> 
>>>  - if the function remains in the future, it will be a bit asymmetric vs other
>>>    offload flags. Right now it is clear why the function is introduced, but
>>>    it is the question if the function should remain or go away in the future
>>>    (as far as I know no other offload flag has similar function to check).
>> No other offloads don't have similar functions, this is kind special.
>>
>> There will be more changes related CRC next release, CRC_STRIP will be removed
>> and no flag will mean strip CRC. So the conditions to is_keep_crc will be changed.
>> This function is to manage this change easier, localize the information in to
>> single function to make it easy to update later.
> 
> It is perfectly clear why it is required right now and introduced (as I said
> from the very beginning).
> Yes, it is will be the history which explains why it is so, but if we make
> a step forward and discard the history it will look asymmetric -
> it will be a function which checks single bit. It is really minor and
> 100% up to you.

I see, right it will be just a wrapper to bit check. In this patch it helps to
revert to logic, from strip_crc to keep_crc. In next release I am OK to remove
function and return back to bit check in PMDs, will this be more reasonable?

Re: [dpdk-dev] [PATCH] ethdev: add new offload flag to keep CRC

[Andrew Rybchenko]

On 20.06.2018 21:12, Ferruh Yigit wrote:
> On 6/20/2018 6:39 PM, Andrew Rybchenko wrote:
>> On 06/20/2018 08:24 PM, Ferruh Yigit wrote:
>>> On 6/20/2018 8:42 AM, Andrew Rybchenko wrote:
>>>> On 06/19/2018 09:02 PM, Ferruh Yigit wrote:
>>>>> DEV_RX_OFFLOAD_KEEP_CRC offload flag added. PMDs that supports keeping
>>>>> CRC should advertise this offload capability.
>>>>>
>>>>> DEV_RX_OFFLOAD_CRC_STRIP flag will remain one more release
>>>>> default behavior in PMDs are to keep the CRC until this flag removed
>>>>>
>>>>> Until DEV_RX_OFFLOAD_CRC_STRIP flag is removed:
>>>>> - Setting both KEEP_CRC & CRC_STRIP is INVALID
>>>>> - Setting only CRC_STRIP PMD should strip the CRC
>>>>> - Setting only KEEP_CRC PMD should keep the CRC
>>>>> - Not setting both PMD should keep the CRC
>>>>>
>>>>> A helper function rte_eth_dev_is_keep_crc() has been added to be able to
>>>>> change the no flag behavior with minimal changes in PMDs.
>>>>>
>>>>> The PMDs that doesn't report the DEV_RX_OFFLOAD_KEEP_CRC offload can
>>>>> remove rte_eth_dev_is_keep_crc() checks next release, related code
>>>>> commented to help the maintenance task.
>>>>>
>>>>> And DEV_RX_OFFLOAD_CRC_STRIP has been added to virtual drivers since
>>>>> they don't use CRC at all, when an application requires this offload
>>>>> virtual PMDs should not return error.
>>>>>
>>>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>>>> ---
>>>> <...>
>>>>
>>>>> diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h
>>>>> index c9c825e3f..09a42f8c2 100644
>>>>> --- a/lib/librte_ethdev/rte_ethdev_driver.h
>>>>> +++ b/lib/librte_ethdev/rte_ethdev_driver.h
>>>>> @@ -325,6 +325,26 @@ typedef int (*ethdev_uninit_t)(struct rte_eth_dev *ethdev);
>>>>>   int __rte_experimental
>>>>>   rte_eth_dev_destroy(struct rte_eth_dev *ethdev, ethdev_uninit_t ethdev_uninit);
>>>>>   
>>>>> +/**
>>>>> + * PMD helper function to check if keeping CRC is requested
>>>>> + *
>>>>> + * @param rx_offloads
>>>>> + *   offloads variable
>>>>> + *
>>>>> + * @return
>>>>> + *   Return positive if keeping CRC is requested,
>>>>> + *   zero if stripping CRC is requested
>>>>> + */
>>>>> +static inline int
>>>>> +rte_eth_dev_is_keep_crc(uint64_t rx_offloads)
>>>>> +{
>>>>> +	if (rx_offloads & DEV_RX_OFFLOAD_CRC_STRIP)
>>>>> +		return 0;
>>>>> +
>>>>> +	/* no KEEP_CRC or CRC_STRIP offload flags means keep CRC */
>>>>> +	return 1;
>>>>> +}
>>>>> +
>>>>>   #ifdef __cplusplus
>>>>>   }
>>>>>   #endif
>>>> A couple of control questions about the function:
>>>>   - shouldn't __rte_experimental be used?
>>> This is an internal function, not API, so I think doesn't require to be
>>> experimental.
>> Just to make my thoughts clear: description does not say that it is an internal.
>> So, nothing prevents external entities to use it. Changes will be API breakage.
> rte_ethdev_driver.h is not public header, it is just for PMDs.

I see. So, it will not be a problem to remove it. OK.

>>>>   - if the function remains in the future, it will be a bit asymmetric vs other
>>>>     offload flags. Right now it is clear why the function is introduced, but
>>>>     it is the question if the function should remain or go away in the future
>>>>     (as far as I know no other offload flag has similar function to check).
>>> No other offloads don't have similar functions, this is kind special.
>>>
>>> There will be more changes related CRC next release, CRC_STRIP will be removed
>>> and no flag will mean strip CRC. So the conditions to is_keep_crc will be changed.
>>> This function is to manage this change easier, localize the information in to
>>> single function to make it easy to update later.
>> It is perfectly clear why it is required right now and introduced (as I said
>> from the very beginning).
>> Yes, it is will be the history which explains why it is so, but if we make
>> a step forward and discard the history it will look asymmetric -
>> it will be a function which checks single bit. It is really minor and
>> 100% up to you.
> I see, right it will be just a wrapper to bit check. In this patch it helps to
> revert to logic, from strip_crc to keep_crc. In next release I am OK to remove
> function and return back to bit check in PMDs, will this be more reasonable?

Just for consistency I'd say yes.

Re: [dpdk-dev] [PATCH] ethdev: add new offload flag to keep CRC

[Shahaf Shuler]

Wednesday, June 20, 2018 7:13 PM. Ferruh Yigit:
> Subject: Re: [dpdk-dev] [PATCH] ethdev: add new offload flag to keep CRC
> 
> On 6/20/2018 2:44 PM, Shahaf Shuler wrote:
> >
> > Hi Ferruh,
> >
> > Tuesday, June 19, 2018 9:03 PM, Ferruh Yigit
> >> Subject: [dpdk-dev] [PATCH] ethdev: add new offload flag to keep CRC
> >>
> >
> > [...]
> >
> >
> >> diff --git a/drivers/net/mlx5/mlx5_rxq.c
> >> b/drivers/net/mlx5/mlx5_rxq.c index de3f869ed..28cf168aa 100644
> >> --- a/drivers/net/mlx5/mlx5_rxq.c
> >> +++ b/drivers/net/mlx5/mlx5_rxq.c
> >> @@ -388,6 +388,9 @@ mlx5_get_rx_queue_offloads(struct rte_eth_dev
> >> *dev)
> >>
> >>  	if (config->hw_fcs_strip)
> >>  		offloads |= DEV_RX_OFFLOAD_CRC_STRIP;
> >> +	else
> >> +		offloads |= DEV_RX_OFFLOAD_KEEP_CRC;
> >> +
> >
> > I think it should be:
> > if (config->hw_fcs_strip) {
> > 	offloads |= DEV_RX_OFFLOAD_CRC_STRIP;
> > 	offloads |= DEV_RX_OFFLOAD_KEEP_CRC;
> > }
> >
> > The hw_fcs_strip is the capability from device which allows the PMD to
> toggle the CRC stripping.
> 
> From below logic, (how "crc_present" set), hw_fcs_strip looks like capability
> from device that says keeping CRC is supported. If so it the original code was
> not clear to me, why to report CRC stripping only if HW supports keeping
> CRC?

What we report is the option to toggle the CRC stripping by the PMD, and this is what the hw_fcs_strip capability is about. 
The default behavior of HW in case this option is not supported is to remove the CRC. 

I> 
> Following makes more sense to me, based on below code, report CRC
> stripping capability by default and report KEEP CRC capability if device
> supports it:
> 
>  offloads |= DEV_RX_OFFLOAD_CRC_STRIP;
>  if (config->hw_fcs_strip)
> 	offloads |= DEV_RX_OFFLOAD_KEEP_CRC;
> 
> What do you think?

Yes it is better. Thanks. 

> 
> >
> >>  	if (config->hw_csum)
> >>  		offloads |= (DEV_RX_OFFLOAD_IPV4_CKSUM |
> >>  			     DEV_RX_OFFLOAD_UDP_CKSUM |
> >> @@ -1419,17 +1422,17 @@ mlx5_rxq_new(struct rte_eth_dev *dev,
> >> uint16_t idx, uint16_t desc,
> >>  	/* Configure VLAN stripping. */
> >>  	tmpl->rxq.vlan_strip = !!(offloads &
> DEV_RX_OFFLOAD_VLAN_STRIP);
> >>  	/* By default, FCS (CRC) is stripped by hardware. */
> >> -	if (offloads & DEV_RX_OFFLOAD_CRC_STRIP) {
> >> -		tmpl->rxq.crc_present = 0;
> >> -	} else if (config->hw_fcs_strip) {
> >> -		tmpl->rxq.crc_present = 1;
> >> -	} else {
> >> -		DRV_LOG(WARNING,
> >> -			"port %u CRC stripping has been disabled but will"
> >> -			" still be performed by hardware, make sure
> >> MLNX_OFED"
> >> -			" and firmware are up to date",
> >> -			dev->data->port_id);
> >> -		tmpl->rxq.crc_present = 0;
> >> +	tmpl->rxq.crc_present = 0;
> >> +	if (rte_eth_dev_is_keep_crc(offloads)) {
> >> +		if (config->hw_fcs_strip) {
> >> +			tmpl->rxq.crc_present = 1;
> >> +		} else {
> >> +			DRV_LOG(WARNING,
> >> +				"port %u CRC stripping has been disabled but
> >> will"
> >> +				" still be performed by hardware, make sure
> >> MLNX_OFED"
> >> +				" and firmware are up to date",
> >> +				dev->data->port_id);
> >> +		}
> >>  	}
> >>  	DRV_LOG(DEBUG,
> >>  		"port %u CRC stripping is %s, %u bytes will be subtracted
> from"

[dpdk-dev] [PATCH v2] ethdev: add new offload flag to keep CRC

[Ferruh Yigit]

DEV_RX_OFFLOAD_KEEP_CRC offload flag is added. PMDs that support
keeping CRC should advertise this offload capability.

DEV_RX_OFFLOAD_CRC_STRIP flag will remain one more release
default behavior in PMDs are to keep the CRC until this flag removed

Until DEV_RX_OFFLOAD_CRC_STRIP flag is removed:
- Setting both KEEP_CRC & CRC_STRIP is INVALID
- Setting only CRC_STRIP PMD should strip the CRC
- Setting only KEEP_CRC PMD should keep the CRC
- Not setting both PMD should keep the CRC

A helper function rte_eth_dev_is_keep_crc() has been added to be able to
change the no flag behavior with minimal changes in PMDs.

The PMDs that doesn't report the DEV_RX_OFFLOAD_KEEP_CRC offload can
remove rte_eth_dev_is_keep_crc() checks next release, related code
commented to help the maintenance task.

And DEV_RX_OFFLOAD_CRC_STRIP has been added to virtual drivers since
they don't use CRC at all, when an application requires this offload
virtual PMDs should not return error.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
Acked-by:  Allain Legacy <allain.legacy@windriver.com>
Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
v2:
* mlx5 PMD updated on how to report KEEP_CRC
---
 doc/guides/rel_notes/deprecation.rst      | 10 ---------
 drivers/net/af_packet/rte_eth_af_packet.c |  1 +
 drivers/net/avp/avp_ethdev.c              |  1 +
 drivers/net/axgbe/axgbe_ethdev.c          |  4 +++-
 drivers/net/axgbe/axgbe_rxtx.c            |  6 +++--
 drivers/net/bnxt/bnxt_ethdev.c            |  1 +
 drivers/net/bnxt/bnxt_rxq.c               |  3 +--
 drivers/net/cxgbe/cxgbe_ethdev.c          |  6 ++++-
 drivers/net/e1000/em_rxtx.c               | 20 ++++++++++-------
 drivers/net/e1000/igb_ethdev.c            |  4 ++--
 drivers/net/e1000/igb_rxtx.c              | 27 ++++++++++++++---------
 drivers/net/fm10k/fm10k_ethdev.c          |  6 +++--
 drivers/net/i40e/i40e_ethdev.c            |  1 +
 drivers/net/i40e/i40e_ethdev_vf.c         |  3 ++-
 drivers/net/i40e/i40e_rxtx.c              |  6 +++--
 drivers/net/ixgbe/ixgbe_ethdev.c          |  4 ++--
 drivers/net/ixgbe/ixgbe_ipsec.c           |  2 +-
 drivers/net/ixgbe/ixgbe_rxtx.c            | 25 ++++++++++++---------
 drivers/net/kni/rte_eth_kni.c             |  1 +
 drivers/net/mlx4/mlx4_rxq.c               | 23 ++++++++++---------
 drivers/net/mlx5/mlx5_rxq.c               | 26 ++++++++++++----------
 drivers/net/mvpp2/mrvl_ethdev.c           |  8 ++++---
 drivers/net/nfp/nfp_net.c                 |  9 +++++---
 drivers/net/null/rte_eth_null.c           |  1 +
 drivers/net/octeontx/octeontx_ethdev.c    |  5 ++++-
 drivers/net/pcap/rte_eth_pcap.c           |  1 +
 drivers/net/qede/qede_ethdev.c            |  2 ++
 drivers/net/ring/rte_eth_ring.c           |  1 +
 drivers/net/sfc/sfc_rx.c                  |  5 ++++-
 drivers/net/softnic/rte_eth_softnic.c     |  1 +
 drivers/net/szedata2/rte_eth_szedata2.c   |  3 ++-
 drivers/net/thunderx/nicvf_ethdev.c       |  5 ++++-
 drivers/net/vhost/rte_eth_vhost.c         |  3 ++-
 drivers/net/virtio/virtio_ethdev.c        |  3 ++-
 drivers/net/vmxnet3/vmxnet3_ethdev.c      |  3 ++-
 lib/librte_ethdev/rte_ethdev.c            |  9 ++++++++
 lib/librte_ethdev/rte_ethdev.h            |  5 +++++
 lib/librte_ethdev/rte_ethdev_driver.h     | 20 +++++++++++++++++
 38 files changed, 172 insertions(+), 92 deletions(-)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 1ce692eac..cd128ae23 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -67,16 +67,6 @@ Deprecation Notices
   - removal of ``txq_flags`` field from ``rte_eth_txconf`` struct.
   - removal of the offloads bit-field from ``rte_eth_rxmode`` struct.
 
-* ethdev: A new offloading flag ``DEV_RX_OFFLOAD_KEEP_CRC`` will be added in v18.08,
-  This will replace the usage of not setting ``DEV_RX_OFFLOAD_CRC_STRIP`` flag
-  and will be implemented in PMDs accordingly.
-  In v18.08 both ``DEV_RX_OFFLOAD_KEEP_CRC`` and ``DEV_RX_OFFLOAD_CRC_STRIP`` flags
-  will be available, usage will be:
-  ``CRC_STRIP``: Strip CRC from packet
-  ``KEEP_CRC``: Keep CRC in packet
-  Both ``CRC_STRIP`` & ``KEEP_CRC``: Invalid
-  No flag: Keep CRC in packet
-
 * ethdev: In v18.11 ``DEV_RX_OFFLOAD_CRC_STRIP`` offload flag will be removed, default
   behavior without any flag will be changed to CRC strip.
   To keep CRC ``DEV_RX_OFFLOAD_KEEP_CRC`` flag is required.
diff --git a/drivers/net/af_packet/rte_eth_af_packet.c b/drivers/net/af_packet/rte_eth_af_packet.c
index ea47abbf8..8cfb7ada4 100644
--- a/drivers/net/af_packet/rte_eth_af_packet.c
+++ b/drivers/net/af_packet/rte_eth_af_packet.c
@@ -305,6 +305,7 @@ eth_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	dev_info->max_rx_queues = (uint16_t)internals->nb_queues;
 	dev_info->max_tx_queues = (uint16_t)internals->nb_queues;
 	dev_info->min_rx_bufsize = 0;
+	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_CRC_STRIP;
 }
 
 static int
diff --git a/drivers/net/avp/avp_ethdev.c b/drivers/net/avp/avp_ethdev.c
index dc97e60e6..9cc8fbcdf 100644
--- a/drivers/net/avp/avp_ethdev.c
+++ b/drivers/net/avp/avp_ethdev.c
@@ -2170,6 +2170,7 @@ avp_dev_info_get(struct rte_eth_dev *eth_dev,
 		dev_info->rx_offload_capa = DEV_RX_OFFLOAD_VLAN_STRIP;
 		dev_info->tx_offload_capa = DEV_TX_OFFLOAD_VLAN_INSERT;
 	}
+	dev_info->rx_offload_capa |= DEV_RX_OFFLOAD_CRC_STRIP;
 }
 
 static int
diff --git a/drivers/net/axgbe/axgbe_ethdev.c b/drivers/net/axgbe/axgbe_ethdev.c
index 7a3ba2e7b..e3773f4a2 100644
--- a/drivers/net/axgbe/axgbe_ethdev.c
+++ b/drivers/net/axgbe/axgbe_ethdev.c
@@ -363,7 +363,9 @@ axgbe_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	dev_info->rx_offload_capa =
 		DEV_RX_OFFLOAD_IPV4_CKSUM |
 		DEV_RX_OFFLOAD_UDP_CKSUM  |
-		DEV_RX_OFFLOAD_TCP_CKSUM;
+		DEV_RX_OFFLOAD_TCP_CKSUM  |
+		DEV_RX_OFFLOAD_CRC_STRIP  |
+		DEV_RX_OFFLOAD_KEEP_CRC;
 
 	dev_info->tx_offload_capa =
 		DEV_TX_OFFLOAD_IPV4_CKSUM  |
diff --git a/drivers/net/axgbe/axgbe_rxtx.c b/drivers/net/axgbe/axgbe_rxtx.c
index b302bdd1f..e665ef939 100644
--- a/drivers/net/axgbe/axgbe_rxtx.c
+++ b/drivers/net/axgbe/axgbe_rxtx.c
@@ -74,8 +74,10 @@ int axgbe_dev_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
 		(DMA_CH_INC * rxq->queue_id));
 	rxq->dma_tail_reg = (volatile uint32_t *)((uint8_t *)rxq->dma_regs +
 						  DMA_CH_RDTR_LO);
-	rxq->crc_len = (uint8_t)((dev->data->dev_conf.rxmode.offloads &
-			DEV_RX_OFFLOAD_CRC_STRIP) ? 0 : ETHER_CRC_LEN);
+	if (rte_eth_dev_is_keep_crc(dev->data->dev_conf.rxmode.offloads))
+		rxq->crc_len = ETHER_CRC_LEN;
+	else
+		rxq->crc_len = 0;
 
 	/* CRC strip in AXGBE supports per port not per queue */
 	pdata->crc_strip_enable = (rxq->crc_len == 0) ? 1 : 0;
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 6e56bfd36..f26461c42 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -147,6 +147,7 @@ static const struct rte_pci_id bnxt_pci_id_map[] = {
 				     DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM | \
 				     DEV_RX_OFFLOAD_JUMBO_FRAME | \
 				     DEV_RX_OFFLOAD_CRC_STRIP | \
+				     DEV_RX_OFFLOAD_KEEP_CRC | \
 				     DEV_RX_OFFLOAD_TCP_LRO)
 
 static int bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask);
diff --git a/drivers/net/bnxt/bnxt_rxq.c b/drivers/net/bnxt/bnxt_rxq.c
index c55ddec4b..265901e63 100644
--- a/drivers/net/bnxt/bnxt_rxq.c
+++ b/drivers/net/bnxt/bnxt_rxq.c
@@ -326,8 +326,7 @@ int bnxt_rx_queue_setup_op(struct rte_eth_dev *eth_dev,
 
 	rxq->queue_id = queue_idx;
 	rxq->port_id = eth_dev->data->port_id;
-	rxq->crc_len = rx_offloads & DEV_RX_OFFLOAD_CRC_STRIP ?
-		0 : ETHER_CRC_LEN;
+	rxq->crc_len = rte_eth_dev_is_keep_crc(rx_offloads) ? ETHER_CRC_LEN : 0;
 
 	eth_dev->data->rx_queues[queue_idx] = rxq;
 	/* Allocate RX ring hardware descriptors */
diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c
index 713dc8fae..b0156ec77 100644
--- a/drivers/net/cxgbe/cxgbe_ethdev.c
+++ b/drivers/net/cxgbe/cxgbe_ethdev.c
@@ -354,7 +354,11 @@ int cxgbe_dev_configure(struct rte_eth_dev *eth_dev)
 
 	CXGBE_FUNC_TRACE();
 	configured_offloads = eth_dev->data->dev_conf.rxmode.offloads;
-	if (!(configured_offloads & DEV_RX_OFFLOAD_CRC_STRIP)) {
+
+	/* KEEP_CRC offload flag is not supported by PMD
+	 * can remove the below block when DEV_RX_OFFLOAD_CRC_STRIP removed
+	 */
+	if (rte_eth_dev_is_keep_crc(configured_offloads)) {
 		dev_info(adapter, "can't disable hw crc strip\n");
 		eth_dev->data->dev_conf.rxmode.offloads |=
 			DEV_RX_OFFLOAD_CRC_STRIP;
diff --git a/drivers/net/e1000/em_rxtx.c b/drivers/net/e1000/em_rxtx.c
index a6b3e92a6..944118baf 100644
--- a/drivers/net/e1000/em_rxtx.c
+++ b/drivers/net/e1000/em_rxtx.c
@@ -1364,6 +1364,7 @@ em_get_rx_port_offloads_capa(struct rte_eth_dev *dev)
 		DEV_RX_OFFLOAD_UDP_CKSUM   |
 		DEV_RX_OFFLOAD_TCP_CKSUM   |
 		DEV_RX_OFFLOAD_CRC_STRIP   |
+		DEV_RX_OFFLOAD_KEEP_CRC    |
 		DEV_RX_OFFLOAD_SCATTER;
 	if (max_rx_pktlen > ETHER_MAX_LEN)
 		rx_offload_capa |= DEV_RX_OFFLOAD_JUMBO_FRAME;
@@ -1458,8 +1459,10 @@ eth_em_rx_queue_setup(struct rte_eth_dev *dev,
 	rxq->rx_free_thresh = rx_conf->rx_free_thresh;
 	rxq->queue_id = queue_idx;
 	rxq->port_id = dev->data->port_id;
-	rxq->crc_len = (uint8_t)((dev->data->dev_conf.rxmode.offloads &
-		DEV_RX_OFFLOAD_CRC_STRIP) ? 0 : ETHER_CRC_LEN);
+	if (rte_eth_dev_is_keep_crc(dev->data->dev_conf.rxmode.offloads))
+		rxq->crc_len = ETHER_CRC_LEN;
+	else
+		rxq->crc_len = 0;
 
 	rxq->rdt_reg_addr = E1000_PCI_REG_ADDR(hw, E1000_RDT(queue_idx));
 	rxq->rdh_reg_addr = E1000_PCI_REG_ADDR(hw, E1000_RDH(queue_idx));
@@ -1792,9 +1795,10 @@ eth_em_rx_init(struct rte_eth_dev *dev)
 		 * Reset crc_len in case it was changed after queue setup by a
 		 *  call to configure
 		 */
-		rxq->crc_len =
-			(uint8_t)(dev->data->dev_conf.rxmode.offloads &
-				DEV_RX_OFFLOAD_CRC_STRIP ? 0 : ETHER_CRC_LEN);
+		if (rte_eth_dev_is_keep_crc(dev->data->dev_conf.rxmode.offloads))
+			rxq->crc_len = ETHER_CRC_LEN;
+		else
+			rxq->crc_len = 0;
 
 		bus_addr = rxq->rx_ring_phys_addr;
 		E1000_WRITE_REG(hw, E1000_RDLEN(i),
@@ -1873,10 +1877,10 @@ eth_em_rx_init(struct rte_eth_dev *dev)
 	}
 
 	/* Setup the Receive Control Register. */
-	if (dev->data->dev_conf.rxmode.offloads & DEV_RX_OFFLOAD_CRC_STRIP)
-		rctl |= E1000_RCTL_SECRC; /* Strip Ethernet CRC. */
-	else
+	if (rte_eth_dev_is_keep_crc(dev->data->dev_conf.rxmode.offloads))
 		rctl &= ~E1000_RCTL_SECRC; /* Do not Strip Ethernet CRC. */
+	else
+		rctl |= E1000_RCTL_SECRC; /* Strip Ethernet CRC. */
 
 	rctl &= ~(3 << E1000_RCTL_MO_SHIFT);
 	rctl |= E1000_RCTL_EN | E1000_RCTL_BAM | E1000_RCTL_LBM_NO |
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index edc7be319..5c62445fe 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -3194,12 +3194,12 @@ igbvf_dev_configure(struct rte_eth_dev *dev)
 	 * Keep the persistent behavior the same as Host PF
 	 */
 #ifndef RTE_LIBRTE_E1000_PF_DISABLE_STRIP_CRC
-	if (!(conf->rxmode.offloads & DEV_RX_OFFLOAD_CRC_STRIP)) {
+	if (rte_eth_dev_is_keep_crc(conf->rxmode.offloads)) {
 		PMD_INIT_LOG(NOTICE, "VF can't disable HW CRC Strip");
 		conf->rxmode.offloads |= DEV_RX_OFFLOAD_CRC_STRIP;
 	}
 #else
-	if (conf->rxmode.offloads & DEV_RX_OFFLOAD_CRC_STRIP) {
+	if (!rte_eth_dev_is_keep_crc(conf->rxmode.offloads)) {
 		PMD_INIT_LOG(NOTICE, "VF can't enable HW CRC Strip");
 		conf->rxmode.offloads &= ~DEV_RX_OFFLOAD_CRC_STRIP;
 	}
diff --git a/drivers/net/e1000/igb_rxtx.c b/drivers/net/e1000/igb_rxtx.c
index 5f729f271..db594c1a3 100644
--- a/drivers/net/e1000/igb_rxtx.c
+++ b/drivers/net/e1000/igb_rxtx.c
@@ -1639,6 +1639,7 @@ igb_get_rx_port_offloads_capa(struct rte_eth_dev *dev)
 			  DEV_RX_OFFLOAD_TCP_CKSUM   |
 			  DEV_RX_OFFLOAD_JUMBO_FRAME |
 			  DEV_RX_OFFLOAD_CRC_STRIP   |
+			  DEV_RX_OFFLOAD_KEEP_CRC    |
 			  DEV_RX_OFFLOAD_SCATTER;
 
 	return rx_offload_capa;
@@ -1720,8 +1721,10 @@ eth_igb_rx_queue_setup(struct rte_eth_dev *dev,
 	rxq->reg_idx = (uint16_t)((RTE_ETH_DEV_SRIOV(dev).active == 0) ?
 		queue_idx : RTE_ETH_DEV_SRIOV(dev).def_pool_q_idx + queue_idx);
 	rxq->port_id = dev->data->port_id;
-	rxq->crc_len = (uint8_t)((dev->data->dev_conf.rxmode.offloads &
-			DEV_RX_OFFLOAD_CRC_STRIP) ? 0 : ETHER_CRC_LEN);
+	if (rte_eth_dev_is_keep_crc(dev->data->dev_conf.rxmode.offloads))
+		rxq->crc_len = ETHER_CRC_LEN;
+	else
+		rxq->crc_len = 0;
 
 	/*
 	 *  Allocate RX ring hardware descriptors. A memzone large enough to
@@ -2371,8 +2374,10 @@ eth_igb_rx_init(struct rte_eth_dev *dev)
 		 * Reset crc_len in case it was changed after queue setup by a
 		 *  call to configure
 		 */
-		rxq->crc_len = (uint8_t)(dev->data->dev_conf.rxmode.offloads &
-				DEV_RX_OFFLOAD_CRC_STRIP ? 0 : ETHER_CRC_LEN);
+		if (rte_eth_dev_is_keep_crc(dev->data->dev_conf.rxmode.offloads))
+			rxq->crc_len = ETHER_CRC_LEN;
+		else
+			rxq->crc_len = 0;
 
 		bus_addr = rxq->rx_ring_phys_addr;
 		E1000_WRITE_REG(hw, E1000_RDLEN(rxq->reg_idx),
@@ -2501,10 +2506,10 @@ eth_igb_rx_init(struct rte_eth_dev *dev)
 	E1000_WRITE_REG(hw, E1000_RXCSUM, rxcsum);
 
 	/* Setup the Receive Control Register. */
-	if (dev->data->dev_conf.rxmode.offloads & DEV_RX_OFFLOAD_CRC_STRIP) {
-		rctl |= E1000_RCTL_SECRC; /* Strip Ethernet CRC. */
+	if (rte_eth_dev_is_keep_crc(dev->data->dev_conf.rxmode.offloads)) {
+		rctl &= ~E1000_RCTL_SECRC; /* Do not Strip Ethernet CRC. */
 
-		/* set STRCRC bit in all queues */
+		/* clear STRCRC bit in all queues */
 		if (hw->mac.type == e1000_i350 ||
 		    hw->mac.type == e1000_i210 ||
 		    hw->mac.type == e1000_i211 ||
@@ -2513,14 +2518,14 @@ eth_igb_rx_init(struct rte_eth_dev *dev)
 				rxq = dev->data->rx_queues[i];
 				uint32_t dvmolr = E1000_READ_REG(hw,
 					E1000_DVMOLR(rxq->reg_idx));
-				dvmolr |= E1000_DVMOLR_STRCRC;
+				dvmolr &= ~E1000_DVMOLR_STRCRC;
 				E1000_WRITE_REG(hw, E1000_DVMOLR(rxq->reg_idx), dvmolr);
 			}
 		}
 	} else {
-		rctl &= ~E1000_RCTL_SECRC; /* Do not Strip Ethernet CRC. */
+		rctl |= E1000_RCTL_SECRC; /* Strip Ethernet CRC. */
 
-		/* clear STRCRC bit in all queues */
+		/* set STRCRC bit in all queues */
 		if (hw->mac.type == e1000_i350 ||
 		    hw->mac.type == e1000_i210 ||
 		    hw->mac.type == e1000_i211 ||
@@ -2529,7 +2534,7 @@ eth_igb_rx_init(struct rte_eth_dev *dev)
 				rxq = dev->data->rx_queues[i];
 				uint32_t dvmolr = E1000_READ_REG(hw,
 					E1000_DVMOLR(rxq->reg_idx));
-				dvmolr &= ~E1000_DVMOLR_STRCRC;
+				dvmolr |= E1000_DVMOLR_STRCRC;
 				E1000_WRITE_REG(hw, E1000_DVMOLR(rxq->reg_idx), dvmolr);
 			}
 		}
diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index 3ff1b0e0f..2b354dc15 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -451,8 +451,10 @@ fm10k_dev_configure(struct rte_eth_dev *dev)
 
 	PMD_INIT_FUNC_TRACE();
 
-	if ((dev->data->dev_conf.rxmode.offloads &
-	     DEV_RX_OFFLOAD_CRC_STRIP) == 0)
+	/* KEEP_CRC offload flag is not supported by PMD
+	 * can remove the below block when DEV_RX_OFFLOAD_CRC_STRIP removed
+	 */
+	if (rte_eth_dev_is_keep_crc(dev->data->dev_conf.rxmode.offloads))
 		PMD_INIT_LOG(WARNING, "fm10k always strip CRC");
 
 	/* multipe queue mode checking */
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 13c5d3296..1276bb207 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -3327,6 +3327,7 @@ i40e_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 		DEV_RX_OFFLOAD_TCP_CKSUM |
 		DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM |
 		DEV_RX_OFFLOAD_CRC_STRIP |
+		DEV_RX_OFFLOAD_KEEP_CRC |
 		DEV_RX_OFFLOAD_VLAN_EXTEND |
 		DEV_RX_OFFLOAD_VLAN_FILTER |
 		DEV_RX_OFFLOAD_JUMBO_FRAME;
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 804e44530..520655bbe 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -1536,7 +1536,7 @@ i40evf_dev_configure(struct rte_eth_dev *dev)
 	/* For non-DPDK PF drivers, VF has no ability to disable HW
 	 * CRC strip, and is implicitly enabled by the PF.
 	 */
-	if (!(conf->rxmode.offloads & DEV_RX_OFFLOAD_CRC_STRIP)) {
+	if (rte_eth_dev_is_keep_crc(conf->rxmode.offloads)) {
 		vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
 		if ((vf->version_major == VIRTCHNL_VERSION_MAJOR) &&
 		    (vf->version_minor <= VIRTCHNL_VERSION_MINOR)) {
@@ -2200,6 +2200,7 @@ i40evf_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 		DEV_RX_OFFLOAD_TCP_CKSUM |
 		DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM |
 		DEV_RX_OFFLOAD_CRC_STRIP |
+		DEV_RX_OFFLOAD_KEEP_CRC |
 		DEV_RX_OFFLOAD_SCATTER |
 		DEV_RX_OFFLOAD_JUMBO_FRAME |
 		DEV_RX_OFFLOAD_VLAN_FILTER;
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index 6032d5541..4c3a2924f 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -1829,8 +1829,10 @@ i40e_dev_rx_queue_setup(struct rte_eth_dev *dev,
 	rxq->queue_id = queue_idx;
 	rxq->reg_idx = reg_idx;
 	rxq->port_id = dev->data->port_id;
-	rxq->crc_len = (uint8_t)((dev->data->dev_conf.rxmode.offloads &
-			DEV_RX_OFFLOAD_CRC_STRIP) ? 0 : ETHER_CRC_LEN);
+	if (rte_eth_dev_is_keep_crc(dev->data->dev_conf.rxmode.offloads))
+		rxq->crc_len = ETHER_CRC_LEN;
+	else
+		rxq->crc_len = 0;
 	rxq->drop_en = rx_conf->rx_drop_en;
 	rxq->vsi = vsi;
 	rxq->rx_deferred_start = rx_conf->rx_deferred_start;
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 87d2ad090..60b7f038f 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -4991,12 +4991,12 @@ ixgbevf_dev_configure(struct rte_eth_dev *dev)
 	 * Keep the persistent behavior the same as Host PF
 	 */
 #ifndef RTE_LIBRTE_IXGBE_PF_DISABLE_STRIP_CRC
-	if (!(conf->rxmode.offloads & DEV_RX_OFFLOAD_CRC_STRIP)) {
+	if (rte_eth_dev_is_keep_crc(conf->rxmode.offloads)) {
 		PMD_INIT_LOG(NOTICE, "VF can't disable HW CRC Strip");
 		conf->rxmode.offloads |= DEV_RX_OFFLOAD_CRC_STRIP;
 	}
 #else
-	if (conf->rxmode.offloads & DEV_RX_OFFLOAD_CRC_STRIP) {
+	if (!rte_eth_dev_is_keep_crc(conf->rxmode.offloads)) {
 		PMD_INIT_LOG(NOTICE, "VF can't enable HW CRC Strip");
 		conf->rxmode.offloads &= ~DEV_RX_OFFLOAD_CRC_STRIP;
 	}
diff --git a/drivers/net/ixgbe/ixgbe_ipsec.c b/drivers/net/ixgbe/ixgbe_ipsec.c
index de7ed3676..da861accf 100644
--- a/drivers/net/ixgbe/ixgbe_ipsec.c
+++ b/drivers/net/ixgbe/ixgbe_ipsec.c
@@ -609,7 +609,7 @@ ixgbe_crypto_enable_ipsec(struct rte_eth_dev *dev)
 		PMD_DRV_LOG(ERR, "RSC and IPsec not supported");
 		return -1;
 	}
-	if (!(rx_offloads & DEV_RX_OFFLOAD_CRC_STRIP)) {
+	if (rte_eth_dev_is_keep_crc(rx_offloads)) {
 		PMD_DRV_LOG(ERR, "HW CRC strip needs to be enabled for IPsec");
 		return -1;
 	}
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index 3e13d26ae..b7b1d6be4 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -2849,6 +2849,7 @@ ixgbe_get_rx_port_offloads(struct rte_eth_dev *dev)
 		   DEV_RX_OFFLOAD_UDP_CKSUM   |
 		   DEV_RX_OFFLOAD_TCP_CKSUM   |
 		   DEV_RX_OFFLOAD_CRC_STRIP   |
+		   DEV_RX_OFFLOAD_KEEP_CRC    |
 		   DEV_RX_OFFLOAD_JUMBO_FRAME |
 		   DEV_RX_OFFLOAD_SCATTER;
 
@@ -2935,8 +2936,10 @@ ixgbe_dev_rx_queue_setup(struct rte_eth_dev *dev,
 	rxq->reg_idx = (uint16_t)((RTE_ETH_DEV_SRIOV(dev).active == 0) ?
 		queue_idx : RTE_ETH_DEV_SRIOV(dev).def_pool_q_idx + queue_idx);
 	rxq->port_id = dev->data->port_id;
-	rxq->crc_len = (uint8_t)((dev->data->dev_conf.rxmode.offloads &
-		DEV_RX_OFFLOAD_CRC_STRIP) ? 0 : ETHER_CRC_LEN);
+	if (rte_eth_dev_is_keep_crc(dev->data->dev_conf.rxmode.offloads))
+		rxq->crc_len = ETHER_CRC_LEN;
+	else
+		rxq->crc_len = 0;
 	rxq->drop_en = rx_conf->rx_drop_en;
 	rxq->rx_deferred_start = rx_conf->rx_deferred_start;
 	rxq->offloads = offloads;
@@ -4702,7 +4705,7 @@ ixgbe_set_rsc(struct rte_eth_dev *dev)
 
 	/* RSC global configuration (chapter 4.6.7.2.1 of 82599 Spec) */
 
-	if (!(rx_conf->offloads & DEV_RX_OFFLOAD_CRC_STRIP) &&
+	if (rte_eth_dev_is_keep_crc(rx_conf->offloads) &&
 	     (rx_conf->offloads & DEV_RX_OFFLOAD_TCP_LRO)) {
 		/*
 		 * According to chapter of 4.6.7.2.1 of the Spec Rev.
@@ -4851,10 +4854,10 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
 	 * Configure CRC stripping, if any.
 	 */
 	hlreg0 = IXGBE_READ_REG(hw, IXGBE_HLREG0);
-	if (rx_conf->offloads & DEV_RX_OFFLOAD_CRC_STRIP)
-		hlreg0 |= IXGBE_HLREG0_RXCRCSTRP;
-	else
+	if (rte_eth_dev_is_keep_crc(rx_conf->offloads))
 		hlreg0 &= ~IXGBE_HLREG0_RXCRCSTRP;
+	else
+		hlreg0 |= IXGBE_HLREG0_RXCRCSTRP;
 
 	/*
 	 * Configure jumbo frame support, if any.
@@ -4892,8 +4895,8 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
 		 * Reset crc_len in case it was changed after queue setup by a
 		 * call to configure.
 		 */
-		rxq->crc_len = (rx_conf->offloads & DEV_RX_OFFLOAD_CRC_STRIP) ?
-				0 : ETHER_CRC_LEN;
+		rxq->crc_len = rte_eth_dev_is_keep_crc(rx_conf->offloads) ?
+				ETHER_CRC_LEN : 0;
 
 		/* Setup the Base and Length of the Rx Descriptor Rings */
 		bus_addr = rxq->rx_ring_phys_addr;
@@ -4962,10 +4965,10 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
 	if (hw->mac.type == ixgbe_mac_82599EB ||
 	    hw->mac.type == ixgbe_mac_X540) {
 		rdrxctl = IXGBE_READ_REG(hw, IXGBE_RDRXCTL);
-		if (rx_conf->offloads & DEV_RX_OFFLOAD_CRC_STRIP)
-			rdrxctl |= IXGBE_RDRXCTL_CRCSTRIP;
-		else
+		if (rte_eth_dev_is_keep_crc(rx_conf->offloads))
 			rdrxctl &= ~IXGBE_RDRXCTL_CRCSTRIP;
+		else
+			rdrxctl |= IXGBE_RDRXCTL_CRCSTRIP;
 		rdrxctl &= ~IXGBE_RDRXCTL_RSCFRSTSIZE;
 		IXGBE_WRITE_REG(hw, IXGBE_RDRXCTL, rdrxctl);
 	}
diff --git a/drivers/net/kni/rte_eth_kni.c b/drivers/net/kni/rte_eth_kni.c
index ab63ea427..04cab248e 100644
--- a/drivers/net/kni/rte_eth_kni.c
+++ b/drivers/net/kni/rte_eth_kni.c
@@ -207,6 +207,7 @@ eth_kni_dev_info(struct rte_eth_dev *dev __rte_unused,
 	dev_info->max_rx_queues = KNI_MAX_QUEUE_PER_PORT;
 	dev_info->max_tx_queues = KNI_MAX_QUEUE_PER_PORT;
 	dev_info->min_rx_bufsize = 0;
+	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_CRC_STRIP;
 }
 
 static int
diff --git a/drivers/net/mlx4/mlx4_rxq.c b/drivers/net/mlx4/mlx4_rxq.c
index 87688c1c7..d223e15cb 100644
--- a/drivers/net/mlx4/mlx4_rxq.c
+++ b/drivers/net/mlx4/mlx4_rxq.c
@@ -672,7 +672,8 @@ uint64_t
 mlx4_get_rx_queue_offloads(struct priv *priv)
 {
 	uint64_t offloads = DEV_RX_OFFLOAD_SCATTER |
-			    DEV_RX_OFFLOAD_CRC_STRIP;
+			    DEV_RX_OFFLOAD_CRC_STRIP |
+			    DEV_RX_OFFLOAD_KEEP_CRC;
 
 	if (priv->hw_csum)
 		offloads |= DEV_RX_OFFLOAD_CHECKSUM;
@@ -771,16 +772,16 @@ mlx4_rx_queue_setup(struct rte_eth_dev *dev, uint16_t idx, uint16_t desc,
 		     (void *)dev, idx, desc);
 	}
 	/* By default, FCS (CRC) is stripped by hardware. */
-	if (offloads & DEV_RX_OFFLOAD_CRC_STRIP) {
-		crc_present = 0;
-	} else if (priv->hw_fcs_strip) {
-		crc_present = 1;
-	} else {
-		WARN("%p: CRC stripping has been disabled but will still"
-		     " be performed by hardware, make sure MLNX_OFED and"
-		     " firmware are up to date",
-		     (void *)dev);
-		crc_present = 0;
+	crc_present = 0;
+	if (rte_eth_dev_is_keep_crc(offloads)) {
+		if (priv->hw_fcs_strip) {
+			crc_present = 1;
+		} else {
+			WARN("%p: CRC stripping has been disabled but will still"
+			     " be performed by hardware, make sure MLNX_OFED and"
+			     " firmware are up to date",
+			     (void *)dev);
+		}
 	}
 	DEBUG("%p: CRC stripping is %s, %u bytes will be subtracted from"
 	      " incoming frames to hide it",
diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
index de3f869ed..6eb2e56ff 100644
--- a/drivers/net/mlx5/mlx5_rxq.c
+++ b/drivers/net/mlx5/mlx5_rxq.c
@@ -386,8 +386,10 @@ mlx5_get_rx_queue_offloads(struct rte_eth_dev *dev)
 			     DEV_RX_OFFLOAD_TIMESTAMP |
 			     DEV_RX_OFFLOAD_JUMBO_FRAME);
 
+	offloads |= DEV_RX_OFFLOAD_CRC_STRIP;
 	if (config->hw_fcs_strip)
-		offloads |= DEV_RX_OFFLOAD_CRC_STRIP;
+		offloads |= DEV_RX_OFFLOAD_KEEP_CRC;
+
 	if (config->hw_csum)
 		offloads |= (DEV_RX_OFFLOAD_IPV4_CKSUM |
 			     DEV_RX_OFFLOAD_UDP_CKSUM |
@@ -1419,17 +1421,17 @@ mlx5_rxq_new(struct rte_eth_dev *dev, uint16_t idx, uint16_t desc,
 	/* Configure VLAN stripping. */
 	tmpl->rxq.vlan_strip = !!(offloads & DEV_RX_OFFLOAD_VLAN_STRIP);
 	/* By default, FCS (CRC) is stripped by hardware. */
-	if (offloads & DEV_RX_OFFLOAD_CRC_STRIP) {
-		tmpl->rxq.crc_present = 0;
-	} else if (config->hw_fcs_strip) {
-		tmpl->rxq.crc_present = 1;
-	} else {
-		DRV_LOG(WARNING,
-			"port %u CRC stripping has been disabled but will"
-			" still be performed by hardware, make sure MLNX_OFED"
-			" and firmware are up to date",
-			dev->data->port_id);
-		tmpl->rxq.crc_present = 0;
+	tmpl->rxq.crc_present = 0;
+	if (rte_eth_dev_is_keep_crc(offloads)) {
+		if (config->hw_fcs_strip) {
+			tmpl->rxq.crc_present = 1;
+		} else {
+			DRV_LOG(WARNING,
+				"port %u CRC stripping has been disabled but will"
+				" still be performed by hardware, make sure MLNX_OFED"
+				" and firmware are up to date",
+				dev->data->port_id);
+		}
 	}
 	DRV_LOG(DEBUG,
 		"port %u CRC stripping is %s, %u bytes will be subtracted from"
diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c
index d5eb1fe69..74d067ac2 100644
--- a/drivers/net/mvpp2/mrvl_ethdev.c
+++ b/drivers/net/mvpp2/mrvl_ethdev.c
@@ -314,9 +314,11 @@ mrvl_dev_configure(struct rte_eth_dev *dev)
 		return -EINVAL;
 	}
 
-	if (!(dev->data->dev_conf.rxmode.offloads & DEV_RX_OFFLOAD_CRC_STRIP)) {
-		MRVL_LOG(INFO,
-			"L2 CRC stripping is always enabled in hw");
+	/* KEEP_CRC offload flag is not supported by PMD
+	 * can remove the below block when DEV_RX_OFFLOAD_CRC_STRIP removed
+	 */
+	if (rte_eth_dev_is_keep_crc(dev->data->dev_conf.rxmode.offloads)) {
+		MRVL_LOG(INFO, "L2 CRC stripping is always enabled in hw");
 		dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_CRC_STRIP;
 	}
 
diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index 36586969f..f8c65c1c6 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -411,8 +411,10 @@ nfp_net_configure(struct rte_eth_dev *dev)
 		return -EINVAL;
 	}
 
-	/* Checking RX offloads */
-	if (!(rxmode->offloads & DEV_RX_OFFLOAD_CRC_STRIP))
+	/* KEEP_CRC offload flag is not supported by PMD
+	 * can remove the below block when DEV_RX_OFFLOAD_CRC_STRIP removed
+	 */
+	if (rte_eth_dev_is_keep_crc(rxmode->offloads))
 		PMD_INIT_LOG(INFO, "HW does strip CRC. No configurable!");
 
 	return 0;
@@ -1166,7 +1168,8 @@ nfp_net_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 					     DEV_RX_OFFLOAD_UDP_CKSUM |
 					     DEV_RX_OFFLOAD_TCP_CKSUM;
 
-	dev_info->rx_offload_capa |= DEV_RX_OFFLOAD_JUMBO_FRAME;
+	dev_info->rx_offload_capa |= DEV_RX_OFFLOAD_JUMBO_FRAME |
+				     DEV_RX_OFFLOAD_KEEP_CRC;
 
 	if (hw->cap & NFP_NET_CFG_CTRL_TXVLAN)
 		dev_info->tx_offload_capa = DEV_TX_OFFLOAD_VLAN_INSERT;
diff --git a/drivers/net/null/rte_eth_null.c b/drivers/net/null/rte_eth_null.c
index 1d2e6b9e9..38c8c42bc 100644
--- a/drivers/net/null/rte_eth_null.c
+++ b/drivers/net/null/rte_eth_null.c
@@ -305,6 +305,7 @@ eth_dev_info(struct rte_eth_dev *dev,
 	dev_info->min_rx_bufsize = 0;
 	dev_info->reta_size = internals->reta_size;
 	dev_info->flow_type_rss_offloads = internals->flow_type_rss_offloads;
+	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_CRC_STRIP;
 }
 
 static int
diff --git a/drivers/net/octeontx/octeontx_ethdev.c b/drivers/net/octeontx/octeontx_ethdev.c
index 1eb453b21..b0541f115 100644
--- a/drivers/net/octeontx/octeontx_ethdev.c
+++ b/drivers/net/octeontx/octeontx_ethdev.c
@@ -283,7 +283,10 @@ octeontx_dev_configure(struct rte_eth_dev *dev)
 		return -EINVAL;
 	}
 
-	if (!(rxmode->offloads & DEV_RX_OFFLOAD_CRC_STRIP)) {
+	/* KEEP_CRC offload flag is not supported by PMD
+	 * can remove the below block when DEV_RX_OFFLOAD_CRC_STRIP removed
+	 */
+	if (rte_eth_dev_is_keep_crc(rxmode->offloads)) {
 		PMD_INIT_LOG(NOTICE, "can't disable hw crc strip");
 		rxmode->offloads |= DEV_RX_OFFLOAD_CRC_STRIP;
 	}
diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index 6bd4a7d79..626e89833 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -538,6 +538,7 @@ eth_dev_info(struct rte_eth_dev *dev,
 	dev_info->max_rx_queues = dev->data->nb_rx_queues;
 	dev_info->max_tx_queues = dev->data->nb_tx_queues;
 	dev_info->min_rx_bufsize = 0;
+	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_CRC_STRIP;
 }
 
 static int
diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index 7a63d0564..e5e8ea4a3 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -1532,6 +1532,7 @@ qede_dev_info_get(struct rte_eth_dev *eth_dev,
 				     DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM |
 				     DEV_RX_OFFLOAD_TCP_LRO	|
 				     DEV_RX_OFFLOAD_CRC_STRIP	|
+				     DEV_RX_OFFLOAD_KEEP_CRC    |
 				     DEV_RX_OFFLOAD_SCATTER	|
 				     DEV_RX_OFFLOAD_JUMBO_FRAME |
 				     DEV_RX_OFFLOAD_VLAN_FILTER |
@@ -1562,6 +1563,7 @@ qede_dev_info_get(struct rte_eth_dev *eth_dev,
 		.rx_drop_en = 1,
 		/* The below RX offloads are always enabled */
 		.offloads = (DEV_RX_OFFLOAD_CRC_STRIP  |
+			     DEV_RX_OFFLOAD_KEEP_CRC   |
 			     DEV_RX_OFFLOAD_IPV4_CKSUM |
 			     DEV_RX_OFFLOAD_TCP_CKSUM  |
 			     DEV_RX_OFFLOAD_UDP_CKSUM),
diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
index 35b837c3f..5aa48da42 100644
--- a/drivers/net/ring/rte_eth_ring.c
+++ b/drivers/net/ring/rte_eth_ring.c
@@ -164,6 +164,7 @@ eth_dev_info(struct rte_eth_dev *dev,
 	dev_info->max_rx_queues = (uint16_t)internals->max_rx_queues;
 	dev_info->max_tx_queues = (uint16_t)internals->max_tx_queues;
 	dev_info->min_rx_bufsize = 0;
+	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_CRC_STRIP;
 }
 
 static int
diff --git a/drivers/net/sfc/sfc_rx.c b/drivers/net/sfc/sfc_rx.c
index cc76a5b15..cdcc70b26 100644
--- a/drivers/net/sfc/sfc_rx.c
+++ b/drivers/net/sfc/sfc_rx.c
@@ -1443,7 +1443,10 @@ sfc_rx_check_mode(struct sfc_adapter *sa, struct rte_eth_rxmode *rxmode)
 		rc = EINVAL;
 	}
 
-	if (~rxmode->offloads & DEV_RX_OFFLOAD_CRC_STRIP) {
+	/* KEEP_CRC offload flag is not supported by PMD
+	 * can remove the below block when DEV_RX_OFFLOAD_CRC_STRIP removed
+	 */
+	if (rte_eth_dev_is_keep_crc(rxmode->offloads)) {
 		sfc_warn(sa, "FCS stripping cannot be disabled - always on");
 		rxmode->offloads |= DEV_RX_OFFLOAD_CRC_STRIP;
 		rxmode->hw_strip_crc = 1;
diff --git a/drivers/net/softnic/rte_eth_softnic.c b/drivers/net/softnic/rte_eth_softnic.c
index 6b3c13e5c..9360cf6c3 100644
--- a/drivers/net/softnic/rte_eth_softnic.c
+++ b/drivers/net/softnic/rte_eth_softnic.c
@@ -65,6 +65,7 @@ static const struct rte_eth_dev_info pmd_dev_info = {
 		.nb_min = 0,
 		.nb_align = 1,
 	},
+	.rx_offload_capa = DEV_RX_OFFLOAD_CRC_STRIP,
 };
 
 static int pmd_softnic_logtype;
diff --git a/drivers/net/szedata2/rte_eth_szedata2.c b/drivers/net/szedata2/rte_eth_szedata2.c
index 910c64d04..829ad13fa 100644
--- a/drivers/net/szedata2/rte_eth_szedata2.c
+++ b/drivers/net/szedata2/rte_eth_szedata2.c
@@ -1056,7 +1056,8 @@ eth_dev_info(struct rte_eth_dev *dev,
 	dev_info->max_rx_queues = internals->max_rx_queues;
 	dev_info->max_tx_queues = internals->max_tx_queues;
 	dev_info->min_rx_bufsize = 0;
-	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_SCATTER;
+	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_SCATTER |
+				    DEV_RX_OFFLOAD_CRC_STRIP;
 	dev_info->tx_offload_capa = 0;
 	dev_info->rx_queue_offload_capa = 0;
 	dev_info->tx_queue_offload_capa = 0;
diff --git a/drivers/net/thunderx/nicvf_ethdev.c b/drivers/net/thunderx/nicvf_ethdev.c
index 99fcd516b..f92f38f42 100644
--- a/drivers/net/thunderx/nicvf_ethdev.c
+++ b/drivers/net/thunderx/nicvf_ethdev.c
@@ -1887,7 +1887,10 @@ nicvf_dev_configure(struct rte_eth_dev *dev)
 		return -EINVAL;
 	}
 
-	if ((rxmode->offloads & DEV_RX_OFFLOAD_CRC_STRIP) == 0) {
+	/* KEEP_CRC offload flag is not supported by PMD
+	 * can remove the below block when DEV_RX_OFFLOAD_CRC_STRIP removed
+	 */
+	if (rte_eth_dev_is_keep_crc(rxmode->offloads)) {
 		PMD_INIT_LOG(NOTICE, "Can't disable hw crc strip");
 		rxmode->offloads |= DEV_RX_OFFLOAD_CRC_STRIP;
 	}
diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index ba9d768a0..eb1afe691 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -1070,7 +1070,8 @@ eth_dev_info(struct rte_eth_dev *dev,
 
 	dev_info->tx_offload_capa = DEV_TX_OFFLOAD_MULTI_SEGS |
 				DEV_TX_OFFLOAD_VLAN_INSERT;
-	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_VLAN_STRIP;
+	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_VLAN_STRIP |
+				    DEV_RX_OFFLOAD_CRC_STRIP;
 }
 
 static int
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index df50a571a..b629cd45d 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -2127,7 +2127,8 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	};
 
 	host_features = VTPCI_OPS(hw)->get_features(hw);
-	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_VLAN_STRIP;
+	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_VLAN_STRIP |
+				    DEV_RX_OFFLOAD_CRC_STRIP;
 	if (host_features & (1ULL << VIRTIO_NET_F_GUEST_CSUM)) {
 		dev_info->rx_offload_capa |=
 			DEV_RX_OFFLOAD_TCP_CKSUM |
diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
index ba932ff27..95b465952 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
@@ -57,7 +57,8 @@
 	 DEV_RX_OFFLOAD_UDP_CKSUM |	\
 	 DEV_RX_OFFLOAD_TCP_CKSUM |	\
 	 DEV_RX_OFFLOAD_TCP_LRO |	\
-	 DEV_RX_OFFLOAD_JUMBO_FRAME)
+	 DEV_RX_OFFLOAD_JUMBO_FRAME |   \
+	 DEV_RX_OFFLOAD_CRC_STRIP)
 
 static int eth_vmxnet3_dev_init(struct rte_eth_dev *eth_dev);
 static int eth_vmxnet3_dev_uninit(struct rte_eth_dev *eth_dev);
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index a9977df97..5d0132223 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -129,6 +129,7 @@ static const struct {
 	RTE_RX_OFFLOAD_BIT2STR(SCATTER),
 	RTE_RX_OFFLOAD_BIT2STR(TIMESTAMP),
 	RTE_RX_OFFLOAD_BIT2STR(SECURITY),
+	RTE_RX_OFFLOAD_BIT2STR(KEEP_CRC),
 };
 
 #undef RTE_RX_OFFLOAD_BIT2STR
@@ -1185,6 +1186,14 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
 		return -EINVAL;
 	}
 
+	if ((local_conf.rxmode.offloads & DEV_RX_OFFLOAD_CRC_STRIP) &&
+			(local_conf.rxmode.offloads & DEV_RX_OFFLOAD_KEEP_CRC)) {
+		ethdev_log(ERR,
+			"Port id=%u not allowed to set both CRC STRIP and KEEP CRC offload flags\n",
+			port_id);
+		return -EINVAL;
+	}
+
 	/* Check that device supports requested rss hash functions. */
 	if ((dev_info.flow_type_rss_offloads |
 	     dev_conf->rx_adv_conf.rss_conf.rss_hf) !=
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index 36e3984ea..1f5652e85 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -939,6 +939,11 @@ struct rte_eth_conf {
 #define DEV_RX_OFFLOAD_SCATTER		0x00002000
 #define DEV_RX_OFFLOAD_TIMESTAMP	0x00004000
 #define DEV_RX_OFFLOAD_SECURITY         0x00008000
+
+/* Invalid to set both DEV_RX_OFFLOAD_CRC_STRIP and DEV_RX_OFFLOAD_KEEP_CRC
+ * No DEV_RX_OFFLOAD_CRC_STRIP flag means keep CRC
+ */
+#define DEV_RX_OFFLOAD_KEEP_CRC		0x00010000
 #define DEV_RX_OFFLOAD_CHECKSUM (DEV_RX_OFFLOAD_IPV4_CKSUM | \
 				 DEV_RX_OFFLOAD_UDP_CKSUM | \
 				 DEV_RX_OFFLOAD_TCP_CKSUM)
diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h
index c9c825e3f..09a42f8c2 100644
--- a/lib/librte_ethdev/rte_ethdev_driver.h
+++ b/lib/librte_ethdev/rte_ethdev_driver.h
@@ -325,6 +325,26 @@ typedef int (*ethdev_uninit_t)(struct rte_eth_dev *ethdev);
 int __rte_experimental
 rte_eth_dev_destroy(struct rte_eth_dev *ethdev, ethdev_uninit_t ethdev_uninit);
 
+/**
+ * PMD helper function to check if keeping CRC is requested
+ *
+ * @param rx_offloads
+ *   offloads variable
+ *
+ * @return
+ *   Return positive if keeping CRC is requested,
+ *   zero if stripping CRC is requested
+ */
+static inline int
+rte_eth_dev_is_keep_crc(uint64_t rx_offloads)
+{
+	if (rx_offloads & DEV_RX_OFFLOAD_CRC_STRIP)
+		return 0;
+
+	/* no KEEP_CRC or CRC_STRIP offload flags means keep CRC */
+	return 1;
+}
+
 #ifdef __cplusplus
 }
 #endif
-- 
2.17.1

Re: [dpdk-dev] [PATCH v2] ethdev: add new offload flag to keep CRC

[Thomas Monjalon]

21/06/2018 15:14, Ferruh Yigit:
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -939,6 +939,11 @@ struct rte_eth_conf {
>  #define DEV_RX_OFFLOAD_SCATTER         0x00002000
>  #define DEV_RX_OFFLOAD_TIMESTAMP       0x00004000
>  #define DEV_RX_OFFLOAD_SECURITY         0x00008000
> +
> +/* Invalid to set both DEV_RX_OFFLOAD_CRC_STRIP and DEV_RX_OFFLOAD_KEEP_CRC
> + * No DEV_RX_OFFLOAD_CRC_STRIP flag means keep CRC
> + */
> +#define DEV_RX_OFFLOAD_KEEP_CRC                0x00010000

Can we convert this comment into a doxygen one?

> --- a/lib/librte_ethdev/rte_ethdev_driver.h
> +++ b/lib/librte_ethdev/rte_ethdev_driver.h
> +/**
> + * PMD helper function to check if keeping CRC is requested
> + *
> + * @param rx_offloads
> + *   offloads variable

Maybe more precise: "offload bits to be applied"

> + *
> + * @return
> + *   Return positive if keeping CRC is requested,
> + *   zero if stripping CRC is requested
> + */
> +static inline int
> +rte_eth_dev_is_keep_crc(uint64_t rx_offloads)

I suggest a different name: rte_eth_dev_must_keep_crc

> +{
> +       if (rx_offloads & DEV_RX_OFFLOAD_CRC_STRIP)
> +               return 0;
> +
> +       /* no KEEP_CRC or CRC_STRIP offload flags means keep CRC */
> +       return 1;
> +}

Maybe add a comment to explain how the function must be replaced
by a check of bit KEEP_CRC in every drivers for 18.11?

Re: [dpdk-dev] [PATCH v3] ethdev: add new offload flag to keep CRC

[Thomas Monjalon]

29/06/2018 14:41, Ferruh Yigit:
> DEV_RX_OFFLOAD_KEEP_CRC offload flag is added. PMDs that support
> keeping CRC should advertise this offload capability.
> 
> DEV_RX_OFFLOAD_CRC_STRIP flag will remain one more release
> default behavior in PMDs are to keep the CRC until this flag removed
> 
> Until DEV_RX_OFFLOAD_CRC_STRIP flag is removed:
> - Setting both KEEP_CRC & CRC_STRIP is INVALID
> - Setting only CRC_STRIP PMD should strip the CRC
> - Setting only KEEP_CRC PMD should keep the CRC
> - Not setting both PMD should keep the CRC
> 
> A helper function rte_eth_dev_is_keep_crc() has been added to be able to
> change the no flag behavior with minimal changes in PMDs.
> 
> The PMDs that doesn't report the DEV_RX_OFFLOAD_KEEP_CRC offload can
> remove rte_eth_dev_is_keep_crc() checks next release, related code
> commented to help the maintenance task.
> 
> And DEV_RX_OFFLOAD_CRC_STRIP has been added to virtual drivers since
> they don't use CRC at all, when an application requires this offload
> virtual PMDs should not return error.
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> Acked-by: Allain Legacy <allain.legacy@windriver.com>
> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>

Acked-by: Thomas Monjalon <thomas@monjalon.net>

[dpdk-dev] [PATCH v3] ethdev: add new offload flag to keep CRC

[Ferruh Yigit]

DEV_RX_OFFLOAD_KEEP_CRC offload flag is added. PMDs that support
keeping CRC should advertise this offload capability.

DEV_RX_OFFLOAD_CRC_STRIP flag will remain one more release
default behavior in PMDs are to keep the CRC until this flag removed

Until DEV_RX_OFFLOAD_CRC_STRIP flag is removed:
- Setting both KEEP_CRC & CRC_STRIP is INVALID
- Setting only CRC_STRIP PMD should strip the CRC
- Setting only KEEP_CRC PMD should keep the CRC
- Not setting both PMD should keep the CRC

A helper function rte_eth_dev_is_keep_crc() has been added to be able to
change the no flag behavior with minimal changes in PMDs.

The PMDs that doesn't report the DEV_RX_OFFLOAD_KEEP_CRC offload can
remove rte_eth_dev_is_keep_crc() checks next release, related code
commented to help the maintenance task.

And DEV_RX_OFFLOAD_CRC_STRIP has been added to virtual drivers since
they don't use CRC at all, when an application requires this offload
virtual PMDs should not return error.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
Acked-by: Allain Legacy <allain.legacy@windriver.com>
Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
v2:
* mlx5 PMD updated on how to report KEEP_CRC

v3:
* rename helper function to rte_eth_dev_must_keep_crc
* Add more comment to helper function and add an note that it will be
  removed later.
* Convert DEV_RX_OFFLOAD_KEEP_CRC flag comment into doxygen comment
---
 doc/guides/rel_notes/deprecation.rst      | 10 ---------
 drivers/net/af_packet/rte_eth_af_packet.c |  1 +
 drivers/net/avp/avp_ethdev.c              |  1 +
 drivers/net/axgbe/axgbe_ethdev.c          |  4 +++-
 drivers/net/axgbe/axgbe_rxtx.c            |  6 +++--
 drivers/net/bnxt/bnxt_ethdev.c            |  1 +
 drivers/net/bnxt/bnxt_rxq.c               |  4 ++--
 drivers/net/cxgbe/cxgbe_ethdev.c          |  6 ++++-
 drivers/net/e1000/em_rxtx.c               | 20 ++++++++++-------
 drivers/net/e1000/igb_ethdev.c            |  4 ++--
 drivers/net/e1000/igb_rxtx.c              | 27 ++++++++++++++---------
 drivers/net/fm10k/fm10k_ethdev.c          |  6 +++--
 drivers/net/i40e/i40e_ethdev.c            |  1 +
 drivers/net/i40e/i40e_ethdev_vf.c         |  3 ++-
 drivers/net/i40e/i40e_rxtx.c              |  6 +++--
 drivers/net/ixgbe/ixgbe_ethdev.c          |  4 ++--
 drivers/net/ixgbe/ixgbe_ipsec.c           |  2 +-
 drivers/net/ixgbe/ixgbe_rxtx.c            | 25 ++++++++++++---------
 drivers/net/kni/rte_eth_kni.c             |  1 +
 drivers/net/mlx4/mlx4_rxq.c               | 23 ++++++++++---------
 drivers/net/mlx5/mlx5_rxq.c               | 26 ++++++++++++----------
 drivers/net/mvpp2/mrvl_ethdev.c           |  8 ++++---
 drivers/net/nfp/nfp_net.c                 |  9 +++++---
 drivers/net/null/rte_eth_null.c           |  1 +
 drivers/net/octeontx/octeontx_ethdev.c    |  5 ++++-
 drivers/net/pcap/rte_eth_pcap.c           |  1 +
 drivers/net/qede/qede_ethdev.c            |  1 +
 drivers/net/ring/rte_eth_ring.c           |  1 +
 drivers/net/sfc/sfc_rx.c                  |  5 ++++-
 drivers/net/softnic/rte_eth_softnic.c     |  1 +
 drivers/net/szedata2/rte_eth_szedata2.c   |  3 ++-
 drivers/net/thunderx/nicvf_ethdev.c       |  5 ++++-
 drivers/net/vhost/rte_eth_vhost.c         |  3 ++-
 drivers/net/virtio/virtio_ethdev.c        |  3 ++-
 drivers/net/vmxnet3/vmxnet3_ethdev.c      |  3 ++-
 lib/librte_ethdev/rte_ethdev.c            |  9 ++++++++
 lib/librte_ethdev/rte_ethdev.h            |  6 +++++
 lib/librte_ethdev/rte_ethdev_driver.h     | 26 ++++++++++++++++++++++
 38 files changed, 179 insertions(+), 92 deletions(-)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 1ce692eac..cd128ae23 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -67,16 +67,6 @@ Deprecation Notices
   - removal of ``txq_flags`` field from ``rte_eth_txconf`` struct.
   - removal of the offloads bit-field from ``rte_eth_rxmode`` struct.
 
-* ethdev: A new offloading flag ``DEV_RX_OFFLOAD_KEEP_CRC`` will be added in v18.08,
-  This will replace the usage of not setting ``DEV_RX_OFFLOAD_CRC_STRIP`` flag
-  and will be implemented in PMDs accordingly.
-  In v18.08 both ``DEV_RX_OFFLOAD_KEEP_CRC`` and ``DEV_RX_OFFLOAD_CRC_STRIP`` flags
-  will be available, usage will be:
-  ``CRC_STRIP``: Strip CRC from packet
-  ``KEEP_CRC``: Keep CRC in packet
-  Both ``CRC_STRIP`` & ``KEEP_CRC``: Invalid
-  No flag: Keep CRC in packet
-
 * ethdev: In v18.11 ``DEV_RX_OFFLOAD_CRC_STRIP`` offload flag will be removed, default
   behavior without any flag will be changed to CRC strip.
   To keep CRC ``DEV_RX_OFFLOAD_KEEP_CRC`` flag is required.
diff --git a/drivers/net/af_packet/rte_eth_af_packet.c b/drivers/net/af_packet/rte_eth_af_packet.c
index ea47abbf8..8cfb7ada4 100644
--- a/drivers/net/af_packet/rte_eth_af_packet.c
+++ b/drivers/net/af_packet/rte_eth_af_packet.c
@@ -305,6 +305,7 @@ eth_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	dev_info->max_rx_queues = (uint16_t)internals->nb_queues;
 	dev_info->max_tx_queues = (uint16_t)internals->nb_queues;
 	dev_info->min_rx_bufsize = 0;
+	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_CRC_STRIP;
 }
 
 static int
diff --git a/drivers/net/avp/avp_ethdev.c b/drivers/net/avp/avp_ethdev.c
index dc97e60e6..9cc8fbcdf 100644
--- a/drivers/net/avp/avp_ethdev.c
+++ b/drivers/net/avp/avp_ethdev.c
@@ -2170,6 +2170,7 @@ avp_dev_info_get(struct rte_eth_dev *eth_dev,
 		dev_info->rx_offload_capa = DEV_RX_OFFLOAD_VLAN_STRIP;
 		dev_info->tx_offload_capa = DEV_TX_OFFLOAD_VLAN_INSERT;
 	}
+	dev_info->rx_offload_capa |= DEV_RX_OFFLOAD_CRC_STRIP;
 }
 
 static int
diff --git a/drivers/net/axgbe/axgbe_ethdev.c b/drivers/net/axgbe/axgbe_ethdev.c
index 7a3ba2e7b..e3773f4a2 100644
--- a/drivers/net/axgbe/axgbe_ethdev.c
+++ b/drivers/net/axgbe/axgbe_ethdev.c
@@ -363,7 +363,9 @@ axgbe_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	dev_info->rx_offload_capa =
 		DEV_RX_OFFLOAD_IPV4_CKSUM |
 		DEV_RX_OFFLOAD_UDP_CKSUM  |
-		DEV_RX_OFFLOAD_TCP_CKSUM;
+		DEV_RX_OFFLOAD_TCP_CKSUM  |
+		DEV_RX_OFFLOAD_CRC_STRIP  |
+		DEV_RX_OFFLOAD_KEEP_CRC;
 
 	dev_info->tx_offload_capa =
 		DEV_TX_OFFLOAD_IPV4_CKSUM  |
diff --git a/drivers/net/axgbe/axgbe_rxtx.c b/drivers/net/axgbe/axgbe_rxtx.c
index b302bdd1f..eae830f2e 100644
--- a/drivers/net/axgbe/axgbe_rxtx.c
+++ b/drivers/net/axgbe/axgbe_rxtx.c
@@ -74,8 +74,10 @@ int axgbe_dev_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
 		(DMA_CH_INC * rxq->queue_id));
 	rxq->dma_tail_reg = (volatile uint32_t *)((uint8_t *)rxq->dma_regs +
 						  DMA_CH_RDTR_LO);
-	rxq->crc_len = (uint8_t)((dev->data->dev_conf.rxmode.offloads &
-			DEV_RX_OFFLOAD_CRC_STRIP) ? 0 : ETHER_CRC_LEN);
+	if (rte_eth_dev_must_keep_crc(dev->data->dev_conf.rxmode.offloads))
+		rxq->crc_len = ETHER_CRC_LEN;
+	else
+		rxq->crc_len = 0;
 
 	/* CRC strip in AXGBE supports per port not per queue */
 	pdata->crc_strip_enable = (rxq->crc_len == 0) ? 1 : 0;
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 6e56bfd36..f26461c42 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -147,6 +147,7 @@ static const struct rte_pci_id bnxt_pci_id_map[] = {
 				     DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM | \
 				     DEV_RX_OFFLOAD_JUMBO_FRAME | \
 				     DEV_RX_OFFLOAD_CRC_STRIP | \
+				     DEV_RX_OFFLOAD_KEEP_CRC | \
 				     DEV_RX_OFFLOAD_TCP_LRO)
 
 static int bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask);
diff --git a/drivers/net/bnxt/bnxt_rxq.c b/drivers/net/bnxt/bnxt_rxq.c
index c55ddec4b..e4e3941cf 100644
--- a/drivers/net/bnxt/bnxt_rxq.c
+++ b/drivers/net/bnxt/bnxt_rxq.c
@@ -326,8 +326,8 @@ int bnxt_rx_queue_setup_op(struct rte_eth_dev *eth_dev,
 
 	rxq->queue_id = queue_idx;
 	rxq->port_id = eth_dev->data->port_id;
-	rxq->crc_len = rx_offloads & DEV_RX_OFFLOAD_CRC_STRIP ?
-		0 : ETHER_CRC_LEN;
+	rxq->crc_len = rte_eth_dev_must_keep_crc(rx_offloads) ?
+		ETHER_CRC_LEN : 0;
 
 	eth_dev->data->rx_queues[queue_idx] = rxq;
 	/* Allocate RX ring hardware descriptors */
diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c
index 713dc8fae..a1dcf5882 100644
--- a/drivers/net/cxgbe/cxgbe_ethdev.c
+++ b/drivers/net/cxgbe/cxgbe_ethdev.c
@@ -354,7 +354,11 @@ int cxgbe_dev_configure(struct rte_eth_dev *eth_dev)
 
 	CXGBE_FUNC_TRACE();
 	configured_offloads = eth_dev->data->dev_conf.rxmode.offloads;
-	if (!(configured_offloads & DEV_RX_OFFLOAD_CRC_STRIP)) {
+
+	/* KEEP_CRC offload flag is not supported by PMD
+	 * can remove the below block when DEV_RX_OFFLOAD_CRC_STRIP removed
+	 */
+	if (rte_eth_dev_must_keep_crc(configured_offloads)) {
 		dev_info(adapter, "can't disable hw crc strip\n");
 		eth_dev->data->dev_conf.rxmode.offloads |=
 			DEV_RX_OFFLOAD_CRC_STRIP;
diff --git a/drivers/net/e1000/em_rxtx.c b/drivers/net/e1000/em_rxtx.c
index a6b3e92a6..7d2ac4eb7 100644
--- a/drivers/net/e1000/em_rxtx.c
+++ b/drivers/net/e1000/em_rxtx.c
@@ -1364,6 +1364,7 @@ em_get_rx_port_offloads_capa(struct rte_eth_dev *dev)
 		DEV_RX_OFFLOAD_UDP_CKSUM   |
 		DEV_RX_OFFLOAD_TCP_CKSUM   |
 		DEV_RX_OFFLOAD_CRC_STRIP   |
+		DEV_RX_OFFLOAD_KEEP_CRC    |
 		DEV_RX_OFFLOAD_SCATTER;
 	if (max_rx_pktlen > ETHER_MAX_LEN)
 		rx_offload_capa |= DEV_RX_OFFLOAD_JUMBO_FRAME;
@@ -1458,8 +1459,10 @@ eth_em_rx_queue_setup(struct rte_eth_dev *dev,
 	rxq->rx_free_thresh = rx_conf->rx_free_thresh;
 	rxq->queue_id = queue_idx;
 	rxq->port_id = dev->data->port_id;
-	rxq->crc_len = (uint8_t)((dev->data->dev_conf.rxmode.offloads &
-		DEV_RX_OFFLOAD_CRC_STRIP) ? 0 : ETHER_CRC_LEN);
+	if (rte_eth_dev_must_keep_crc(dev->data->dev_conf.rxmode.offloads))
+		rxq->crc_len = ETHER_CRC_LEN;
+	else
+		rxq->crc_len = 0;
 
 	rxq->rdt_reg_addr = E1000_PCI_REG_ADDR(hw, E1000_RDT(queue_idx));
 	rxq->rdh_reg_addr = E1000_PCI_REG_ADDR(hw, E1000_RDH(queue_idx));
@@ -1792,9 +1795,10 @@ eth_em_rx_init(struct rte_eth_dev *dev)
 		 * Reset crc_len in case it was changed after queue setup by a
 		 *  call to configure
 		 */
-		rxq->crc_len =
-			(uint8_t)(dev->data->dev_conf.rxmode.offloads &
-				DEV_RX_OFFLOAD_CRC_STRIP ? 0 : ETHER_CRC_LEN);
+		if (rte_eth_dev_must_keep_crc(dev->data->dev_conf.rxmode.offloads))
+			rxq->crc_len = ETHER_CRC_LEN;
+		else
+			rxq->crc_len = 0;
 
 		bus_addr = rxq->rx_ring_phys_addr;
 		E1000_WRITE_REG(hw, E1000_RDLEN(i),
@@ -1873,10 +1877,10 @@ eth_em_rx_init(struct rte_eth_dev *dev)
 	}
 
 	/* Setup the Receive Control Register. */
-	if (dev->data->dev_conf.rxmode.offloads & DEV_RX_OFFLOAD_CRC_STRIP)
-		rctl |= E1000_RCTL_SECRC; /* Strip Ethernet CRC. */
-	else
+	if (rte_eth_dev_must_keep_crc(dev->data->dev_conf.rxmode.offloads))
 		rctl &= ~E1000_RCTL_SECRC; /* Do not Strip Ethernet CRC. */
+	else
+		rctl |= E1000_RCTL_SECRC; /* Strip Ethernet CRC. */
 
 	rctl &= ~(3 << E1000_RCTL_MO_SHIFT);
 	rctl |= E1000_RCTL_EN | E1000_RCTL_BAM | E1000_RCTL_LBM_NO |
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index edc7be319..0209c0988 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -3194,12 +3194,12 @@ igbvf_dev_configure(struct rte_eth_dev *dev)
 	 * Keep the persistent behavior the same as Host PF
 	 */
 #ifndef RTE_LIBRTE_E1000_PF_DISABLE_STRIP_CRC
-	if (!(conf->rxmode.offloads & DEV_RX_OFFLOAD_CRC_STRIP)) {
+	if (rte_eth_dev_must_keep_crc(conf->rxmode.offloads)) {
 		PMD_INIT_LOG(NOTICE, "VF can't disable HW CRC Strip");
 		conf->rxmode.offloads |= DEV_RX_OFFLOAD_CRC_STRIP;
 	}
 #else
-	if (conf->rxmode.offloads & DEV_RX_OFFLOAD_CRC_STRIP) {
+	if (!rte_eth_dev_must_keep_crc(conf->rxmode.offloads)) {
 		PMD_INIT_LOG(NOTICE, "VF can't enable HW CRC Strip");
 		conf->rxmode.offloads &= ~DEV_RX_OFFLOAD_CRC_STRIP;
 	}
diff --git a/drivers/net/e1000/igb_rxtx.c b/drivers/net/e1000/igb_rxtx.c
index 5f729f271..b955068a8 100644
--- a/drivers/net/e1000/igb_rxtx.c
+++ b/drivers/net/e1000/igb_rxtx.c
@@ -1639,6 +1639,7 @@ igb_get_rx_port_offloads_capa(struct rte_eth_dev *dev)
 			  DEV_RX_OFFLOAD_TCP_CKSUM   |
 			  DEV_RX_OFFLOAD_JUMBO_FRAME |
 			  DEV_RX_OFFLOAD_CRC_STRIP   |
+			  DEV_RX_OFFLOAD_KEEP_CRC    |
 			  DEV_RX_OFFLOAD_SCATTER;
 
 	return rx_offload_capa;
@@ -1720,8 +1721,10 @@ eth_igb_rx_queue_setup(struct rte_eth_dev *dev,
 	rxq->reg_idx = (uint16_t)((RTE_ETH_DEV_SRIOV(dev).active == 0) ?
 		queue_idx : RTE_ETH_DEV_SRIOV(dev).def_pool_q_idx + queue_idx);
 	rxq->port_id = dev->data->port_id;
-	rxq->crc_len = (uint8_t)((dev->data->dev_conf.rxmode.offloads &
-			DEV_RX_OFFLOAD_CRC_STRIP) ? 0 : ETHER_CRC_LEN);
+	if (rte_eth_dev_must_keep_crc(dev->data->dev_conf.rxmode.offloads))
+		rxq->crc_len = ETHER_CRC_LEN;
+	else
+		rxq->crc_len = 0;
 
 	/*
 	 *  Allocate RX ring hardware descriptors. A memzone large enough to
@@ -2371,8 +2374,10 @@ eth_igb_rx_init(struct rte_eth_dev *dev)
 		 * Reset crc_len in case it was changed after queue setup by a
 		 *  call to configure
 		 */
-		rxq->crc_len = (uint8_t)(dev->data->dev_conf.rxmode.offloads &
-				DEV_RX_OFFLOAD_CRC_STRIP ? 0 : ETHER_CRC_LEN);
+		if (rte_eth_dev_must_keep_crc(dev->data->dev_conf.rxmode.offloads))
+			rxq->crc_len = ETHER_CRC_LEN;
+		else
+			rxq->crc_len = 0;
 
 		bus_addr = rxq->rx_ring_phys_addr;
 		E1000_WRITE_REG(hw, E1000_RDLEN(rxq->reg_idx),
@@ -2501,10 +2506,10 @@ eth_igb_rx_init(struct rte_eth_dev *dev)
 	E1000_WRITE_REG(hw, E1000_RXCSUM, rxcsum);
 
 	/* Setup the Receive Control Register. */
-	if (dev->data->dev_conf.rxmode.offloads & DEV_RX_OFFLOAD_CRC_STRIP) {
-		rctl |= E1000_RCTL_SECRC; /* Strip Ethernet CRC. */
+	if (rte_eth_dev_must_keep_crc(dev->data->dev_conf.rxmode.offloads)) {
+		rctl &= ~E1000_RCTL_SECRC; /* Do not Strip Ethernet CRC. */
 
-		/* set STRCRC bit in all queues */
+		/* clear STRCRC bit in all queues */
 		if (hw->mac.type == e1000_i350 ||
 		    hw->mac.type == e1000_i210 ||
 		    hw->mac.type == e1000_i211 ||
@@ -2513,14 +2518,14 @@ eth_igb_rx_init(struct rte_eth_dev *dev)
 				rxq = dev->data->rx_queues[i];
 				uint32_t dvmolr = E1000_READ_REG(hw,
 					E1000_DVMOLR(rxq->reg_idx));
-				dvmolr |= E1000_DVMOLR_STRCRC;
+				dvmolr &= ~E1000_DVMOLR_STRCRC;
 				E1000_WRITE_REG(hw, E1000_DVMOLR(rxq->reg_idx), dvmolr);
 			}
 		}
 	} else {
-		rctl &= ~E1000_RCTL_SECRC; /* Do not Strip Ethernet CRC. */
+		rctl |= E1000_RCTL_SECRC; /* Strip Ethernet CRC. */
 
-		/* clear STRCRC bit in all queues */
+		/* set STRCRC bit in all queues */
 		if (hw->mac.type == e1000_i350 ||
 		    hw->mac.type == e1000_i210 ||
 		    hw->mac.type == e1000_i211 ||
@@ -2529,7 +2534,7 @@ eth_igb_rx_init(struct rte_eth_dev *dev)
 				rxq = dev->data->rx_queues[i];
 				uint32_t dvmolr = E1000_READ_REG(hw,
 					E1000_DVMOLR(rxq->reg_idx));
-				dvmolr &= ~E1000_DVMOLR_STRCRC;
+				dvmolr |= E1000_DVMOLR_STRCRC;
 				E1000_WRITE_REG(hw, E1000_DVMOLR(rxq->reg_idx), dvmolr);
 			}
 		}
diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index 3ff1b0e0f..edecc283b 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -451,8 +451,10 @@ fm10k_dev_configure(struct rte_eth_dev *dev)
 
 	PMD_INIT_FUNC_TRACE();
 
-	if ((dev->data->dev_conf.rxmode.offloads &
-	     DEV_RX_OFFLOAD_CRC_STRIP) == 0)
+	/* KEEP_CRC offload flag is not supported by PMD
+	 * can remove the below block when DEV_RX_OFFLOAD_CRC_STRIP removed
+	 */
+	if (rte_eth_dev_must_keep_crc(dev->data->dev_conf.rxmode.offloads))
 		PMD_INIT_LOG(WARNING, "fm10k always strip CRC");
 
 	/* multipe queue mode checking */
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index e06e0a20b..0743eac78 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -3331,6 +3331,7 @@ i40e_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 		DEV_RX_OFFLOAD_TCP_CKSUM |
 		DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM |
 		DEV_RX_OFFLOAD_CRC_STRIP |
+		DEV_RX_OFFLOAD_KEEP_CRC |
 		DEV_RX_OFFLOAD_VLAN_EXTEND |
 		DEV_RX_OFFLOAD_VLAN_FILTER |
 		DEV_RX_OFFLOAD_JUMBO_FRAME;
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 86b38d202..5ee0e3607 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -1536,7 +1536,7 @@ i40evf_dev_configure(struct rte_eth_dev *dev)
 	/* For non-DPDK PF drivers, VF has no ability to disable HW
 	 * CRC strip, and is implicitly enabled by the PF.
 	 */
-	if (!(conf->rxmode.offloads & DEV_RX_OFFLOAD_CRC_STRIP)) {
+	if (rte_eth_dev_must_keep_crc(conf->rxmode.offloads)) {
 		vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
 		if ((vf->version_major == VIRTCHNL_VERSION_MAJOR) &&
 		    (vf->version_minor <= VIRTCHNL_VERSION_MINOR)) {
@@ -2199,6 +2199,7 @@ i40evf_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 		DEV_RX_OFFLOAD_TCP_CKSUM |
 		DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM |
 		DEV_RX_OFFLOAD_CRC_STRIP |
+		DEV_RX_OFFLOAD_KEEP_CRC |
 		DEV_RX_OFFLOAD_SCATTER |
 		DEV_RX_OFFLOAD_JUMBO_FRAME |
 		DEV_RX_OFFLOAD_VLAN_FILTER;
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index e7916f99c..3be87fe6a 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -1836,8 +1836,10 @@ i40e_dev_rx_queue_setup(struct rte_eth_dev *dev,
 	rxq->queue_id = queue_idx;
 	rxq->reg_idx = reg_idx;
 	rxq->port_id = dev->data->port_id;
-	rxq->crc_len = (uint8_t)((dev->data->dev_conf.rxmode.offloads &
-			DEV_RX_OFFLOAD_CRC_STRIP) ? 0 : ETHER_CRC_LEN);
+	if (rte_eth_dev_must_keep_crc(dev->data->dev_conf.rxmode.offloads))
+		rxq->crc_len = ETHER_CRC_LEN;
+	else
+		rxq->crc_len = 0;
 	rxq->drop_en = rx_conf->rx_drop_en;
 	rxq->vsi = vsi;
 	rxq->rx_deferred_start = rx_conf->rx_deferred_start;
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 440724543..997683d3b 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -5008,12 +5008,12 @@ ixgbevf_dev_configure(struct rte_eth_dev *dev)
 	 * Keep the persistent behavior the same as Host PF
 	 */
 #ifndef RTE_LIBRTE_IXGBE_PF_DISABLE_STRIP_CRC
-	if (!(conf->rxmode.offloads & DEV_RX_OFFLOAD_CRC_STRIP)) {
+	if (rte_eth_dev_must_keep_crc(conf->rxmode.offloads)) {
 		PMD_INIT_LOG(NOTICE, "VF can't disable HW CRC Strip");
 		conf->rxmode.offloads |= DEV_RX_OFFLOAD_CRC_STRIP;
 	}
 #else
-	if (conf->rxmode.offloads & DEV_RX_OFFLOAD_CRC_STRIP) {
+	if (!rte_eth_dev_must_keep_crc(conf->rxmode.offloads)) {
 		PMD_INIT_LOG(NOTICE, "VF can't enable HW CRC Strip");
 		conf->rxmode.offloads &= ~DEV_RX_OFFLOAD_CRC_STRIP;
 	}
diff --git a/drivers/net/ixgbe/ixgbe_ipsec.c b/drivers/net/ixgbe/ixgbe_ipsec.c
index de7ed3676..08405f1e3 100644
--- a/drivers/net/ixgbe/ixgbe_ipsec.c
+++ b/drivers/net/ixgbe/ixgbe_ipsec.c
@@ -609,7 +609,7 @@ ixgbe_crypto_enable_ipsec(struct rte_eth_dev *dev)
 		PMD_DRV_LOG(ERR, "RSC and IPsec not supported");
 		return -1;
 	}
-	if (!(rx_offloads & DEV_RX_OFFLOAD_CRC_STRIP)) {
+	if (rte_eth_dev_must_keep_crc(rx_offloads)) {
 		PMD_DRV_LOG(ERR, "HW CRC strip needs to be enabled for IPsec");
 		return -1;
 	}
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index 3e13d26ae..9f2e5f114 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -2849,6 +2849,7 @@ ixgbe_get_rx_port_offloads(struct rte_eth_dev *dev)
 		   DEV_RX_OFFLOAD_UDP_CKSUM   |
 		   DEV_RX_OFFLOAD_TCP_CKSUM   |
 		   DEV_RX_OFFLOAD_CRC_STRIP   |
+		   DEV_RX_OFFLOAD_KEEP_CRC    |
 		   DEV_RX_OFFLOAD_JUMBO_FRAME |
 		   DEV_RX_OFFLOAD_SCATTER;
 
@@ -2935,8 +2936,10 @@ ixgbe_dev_rx_queue_setup(struct rte_eth_dev *dev,
 	rxq->reg_idx = (uint16_t)((RTE_ETH_DEV_SRIOV(dev).active == 0) ?
 		queue_idx : RTE_ETH_DEV_SRIOV(dev).def_pool_q_idx + queue_idx);
 	rxq->port_id = dev->data->port_id;
-	rxq->crc_len = (uint8_t)((dev->data->dev_conf.rxmode.offloads &
-		DEV_RX_OFFLOAD_CRC_STRIP) ? 0 : ETHER_CRC_LEN);
+	if (rte_eth_dev_must_keep_crc(dev->data->dev_conf.rxmode.offloads))
+		rxq->crc_len = ETHER_CRC_LEN;
+	else
+		rxq->crc_len = 0;
 	rxq->drop_en = rx_conf->rx_drop_en;
 	rxq->rx_deferred_start = rx_conf->rx_deferred_start;
 	rxq->offloads = offloads;
@@ -4702,7 +4705,7 @@ ixgbe_set_rsc(struct rte_eth_dev *dev)
 
 	/* RSC global configuration (chapter 4.6.7.2.1 of 82599 Spec) */
 
-	if (!(rx_conf->offloads & DEV_RX_OFFLOAD_CRC_STRIP) &&
+	if (rte_eth_dev_must_keep_crc(rx_conf->offloads) &&
 	     (rx_conf->offloads & DEV_RX_OFFLOAD_TCP_LRO)) {
 		/*
 		 * According to chapter of 4.6.7.2.1 of the Spec Rev.
@@ -4851,10 +4854,10 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
 	 * Configure CRC stripping, if any.
 	 */
 	hlreg0 = IXGBE_READ_REG(hw, IXGBE_HLREG0);
-	if (rx_conf->offloads & DEV_RX_OFFLOAD_CRC_STRIP)
-		hlreg0 |= IXGBE_HLREG0_RXCRCSTRP;
-	else
+	if (rte_eth_dev_must_keep_crc(rx_conf->offloads))
 		hlreg0 &= ~IXGBE_HLREG0_RXCRCSTRP;
+	else
+		hlreg0 |= IXGBE_HLREG0_RXCRCSTRP;
 
 	/*
 	 * Configure jumbo frame support, if any.
@@ -4892,8 +4895,8 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
 		 * Reset crc_len in case it was changed after queue setup by a
 		 * call to configure.
 		 */
-		rxq->crc_len = (rx_conf->offloads & DEV_RX_OFFLOAD_CRC_STRIP) ?
-				0 : ETHER_CRC_LEN;
+		rxq->crc_len = rte_eth_dev_must_keep_crc(rx_conf->offloads) ?
+				ETHER_CRC_LEN : 0;
 
 		/* Setup the Base and Length of the Rx Descriptor Rings */
 		bus_addr = rxq->rx_ring_phys_addr;
@@ -4962,10 +4965,10 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
 	if (hw->mac.type == ixgbe_mac_82599EB ||
 	    hw->mac.type == ixgbe_mac_X540) {
 		rdrxctl = IXGBE_READ_REG(hw, IXGBE_RDRXCTL);
-		if (rx_conf->offloads & DEV_RX_OFFLOAD_CRC_STRIP)
-			rdrxctl |= IXGBE_RDRXCTL_CRCSTRIP;
-		else
+		if (rte_eth_dev_must_keep_crc(rx_conf->offloads))
 			rdrxctl &= ~IXGBE_RDRXCTL_CRCSTRIP;
+		else
+			rdrxctl |= IXGBE_RDRXCTL_CRCSTRIP;
 		rdrxctl &= ~IXGBE_RDRXCTL_RSCFRSTSIZE;
 		IXGBE_WRITE_REG(hw, IXGBE_RDRXCTL, rdrxctl);
 	}
diff --git a/drivers/net/kni/rte_eth_kni.c b/drivers/net/kni/rte_eth_kni.c
index ab63ea427..04cab248e 100644
--- a/drivers/net/kni/rte_eth_kni.c
+++ b/drivers/net/kni/rte_eth_kni.c
@@ -207,6 +207,7 @@ eth_kni_dev_info(struct rte_eth_dev *dev __rte_unused,
 	dev_info->max_rx_queues = KNI_MAX_QUEUE_PER_PORT;
 	dev_info->max_tx_queues = KNI_MAX_QUEUE_PER_PORT;
 	dev_info->min_rx_bufsize = 0;
+	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_CRC_STRIP;
 }
 
 static int
diff --git a/drivers/net/mlx4/mlx4_rxq.c b/drivers/net/mlx4/mlx4_rxq.c
index 87688c1c7..0cd9560fc 100644
--- a/drivers/net/mlx4/mlx4_rxq.c
+++ b/drivers/net/mlx4/mlx4_rxq.c
@@ -672,7 +672,8 @@ uint64_t
 mlx4_get_rx_queue_offloads(struct priv *priv)
 {
 	uint64_t offloads = DEV_RX_OFFLOAD_SCATTER |
-			    DEV_RX_OFFLOAD_CRC_STRIP;
+			    DEV_RX_OFFLOAD_CRC_STRIP |
+			    DEV_RX_OFFLOAD_KEEP_CRC;
 
 	if (priv->hw_csum)
 		offloads |= DEV_RX_OFFLOAD_CHECKSUM;
@@ -771,16 +772,16 @@ mlx4_rx_queue_setup(struct rte_eth_dev *dev, uint16_t idx, uint16_t desc,
 		     (void *)dev, idx, desc);
 	}
 	/* By default, FCS (CRC) is stripped by hardware. */
-	if (offloads & DEV_RX_OFFLOAD_CRC_STRIP) {
-		crc_present = 0;
-	} else if (priv->hw_fcs_strip) {
-		crc_present = 1;
-	} else {
-		WARN("%p: CRC stripping has been disabled but will still"
-		     " be performed by hardware, make sure MLNX_OFED and"
-		     " firmware are up to date",
-		     (void *)dev);
-		crc_present = 0;
+	crc_present = 0;
+	if (rte_eth_dev_must_keep_crc(offloads)) {
+		if (priv->hw_fcs_strip) {
+			crc_present = 1;
+		} else {
+			WARN("%p: CRC stripping has been disabled but will still"
+			     " be performed by hardware, make sure MLNX_OFED and"
+			     " firmware are up to date",
+			     (void *)dev);
+		}
 	}
 	DEBUG("%p: CRC stripping is %s, %u bytes will be subtracted from"
 	      " incoming frames to hide it",
diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
index 08dd5596b..fd0df177e 100644
--- a/drivers/net/mlx5/mlx5_rxq.c
+++ b/drivers/net/mlx5/mlx5_rxq.c
@@ -386,8 +386,10 @@ mlx5_get_rx_queue_offloads(struct rte_eth_dev *dev)
 			     DEV_RX_OFFLOAD_TIMESTAMP |
 			     DEV_RX_OFFLOAD_JUMBO_FRAME);
 
+	offloads |= DEV_RX_OFFLOAD_CRC_STRIP;
 	if (config->hw_fcs_strip)
-		offloads |= DEV_RX_OFFLOAD_CRC_STRIP;
+		offloads |= DEV_RX_OFFLOAD_KEEP_CRC;
+
 	if (config->hw_csum)
 		offloads |= (DEV_RX_OFFLOAD_IPV4_CKSUM |
 			     DEV_RX_OFFLOAD_UDP_CKSUM |
@@ -1417,17 +1419,17 @@ mlx5_rxq_new(struct rte_eth_dev *dev, uint16_t idx, uint16_t desc,
 	/* Configure VLAN stripping. */
 	tmpl->rxq.vlan_strip = !!(offloads & DEV_RX_OFFLOAD_VLAN_STRIP);
 	/* By default, FCS (CRC) is stripped by hardware. */
-	if (offloads & DEV_RX_OFFLOAD_CRC_STRIP) {
-		tmpl->rxq.crc_present = 0;
-	} else if (config->hw_fcs_strip) {
-		tmpl->rxq.crc_present = 1;
-	} else {
-		DRV_LOG(WARNING,
-			"port %u CRC stripping has been disabled but will"
-			" still be performed by hardware, make sure MLNX_OFED"
-			" and firmware are up to date",
-			dev->data->port_id);
-		tmpl->rxq.crc_present = 0;
+	tmpl->rxq.crc_present = 0;
+	if (rte_eth_dev_must_keep_crc(offloads)) {
+		if (config->hw_fcs_strip) {
+			tmpl->rxq.crc_present = 1;
+		} else {
+			DRV_LOG(WARNING,
+				"port %u CRC stripping has been disabled but will"
+				" still be performed by hardware, make sure MLNX_OFED"
+				" and firmware are up to date",
+				dev->data->port_id);
+		}
 	}
 	DRV_LOG(DEBUG,
 		"port %u CRC stripping is %s, %u bytes will be subtracted from"
diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c
index d5eb1fe69..85a04fbb4 100644
--- a/drivers/net/mvpp2/mrvl_ethdev.c
+++ b/drivers/net/mvpp2/mrvl_ethdev.c
@@ -314,9 +314,11 @@ mrvl_dev_configure(struct rte_eth_dev *dev)
 		return -EINVAL;
 	}
 
-	if (!(dev->data->dev_conf.rxmode.offloads & DEV_RX_OFFLOAD_CRC_STRIP)) {
-		MRVL_LOG(INFO,
-			"L2 CRC stripping is always enabled in hw");
+	/* KEEP_CRC offload flag is not supported by PMD
+	 * can remove the below block when DEV_RX_OFFLOAD_CRC_STRIP removed
+	 */
+	if (rte_eth_dev_must_keep_crc(dev->data->dev_conf.rxmode.offloads)) {
+		MRVL_LOG(INFO, "L2 CRC stripping is always enabled in hw");
 		dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_CRC_STRIP;
 	}
 
diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index 1422d0543..5e3533901 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -411,8 +411,10 @@ nfp_net_configure(struct rte_eth_dev *dev)
 		return -EINVAL;
 	}
 
-	/* Checking RX offloads */
-	if (!(rxmode->offloads & DEV_RX_OFFLOAD_CRC_STRIP))
+	/* KEEP_CRC offload flag is not supported by PMD
+	 * can remove the below block when DEV_RX_OFFLOAD_CRC_STRIP removed
+	 */
+	if (rte_eth_dev_must_keep_crc(rxmode->offloads))
 		PMD_INIT_LOG(INFO, "HW does strip CRC. No configurable!");
 
 	return 0;
@@ -1166,7 +1168,8 @@ nfp_net_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 					     DEV_RX_OFFLOAD_UDP_CKSUM |
 					     DEV_RX_OFFLOAD_TCP_CKSUM;
 
-	dev_info->rx_offload_capa |= DEV_RX_OFFLOAD_JUMBO_FRAME;
+	dev_info->rx_offload_capa |= DEV_RX_OFFLOAD_JUMBO_FRAME |
+				     DEV_RX_OFFLOAD_KEEP_CRC;
 
 	if (hw->cap & NFP_NET_CFG_CTRL_TXVLAN)
 		dev_info->tx_offload_capa = DEV_TX_OFFLOAD_VLAN_INSERT;
diff --git a/drivers/net/null/rte_eth_null.c b/drivers/net/null/rte_eth_null.c
index 1d2e6b9e9..38c8c42bc 100644
--- a/drivers/net/null/rte_eth_null.c
+++ b/drivers/net/null/rte_eth_null.c
@@ -305,6 +305,7 @@ eth_dev_info(struct rte_eth_dev *dev,
 	dev_info->min_rx_bufsize = 0;
 	dev_info->reta_size = internals->reta_size;
 	dev_info->flow_type_rss_offloads = internals->flow_type_rss_offloads;
+	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_CRC_STRIP;
 }
 
 static int
diff --git a/drivers/net/octeontx/octeontx_ethdev.c b/drivers/net/octeontx/octeontx_ethdev.c
index 1eb453b21..6eb6836d2 100644
--- a/drivers/net/octeontx/octeontx_ethdev.c
+++ b/drivers/net/octeontx/octeontx_ethdev.c
@@ -283,7 +283,10 @@ octeontx_dev_configure(struct rte_eth_dev *dev)
 		return -EINVAL;
 	}
 
-	if (!(rxmode->offloads & DEV_RX_OFFLOAD_CRC_STRIP)) {
+	/* KEEP_CRC offload flag is not supported by PMD
+	 * can remove the below block when DEV_RX_OFFLOAD_CRC_STRIP removed
+	 */
+	if (rte_eth_dev_must_keep_crc(rxmode->offloads)) {
 		PMD_INIT_LOG(NOTICE, "can't disable hw crc strip");
 		rxmode->offloads |= DEV_RX_OFFLOAD_CRC_STRIP;
 	}
diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index b21930b49..78c54fcca 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -538,6 +538,7 @@ eth_dev_info(struct rte_eth_dev *dev,
 	dev_info->max_rx_queues = dev->data->nb_rx_queues;
 	dev_info->max_tx_queues = dev->data->nb_tx_queues;
 	dev_info->min_rx_bufsize = 0;
+	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_CRC_STRIP;
 }
 
 static int
diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index 876052772..32005e969 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -1554,6 +1554,7 @@ qede_dev_info_get(struct rte_eth_dev *eth_dev,
 				     DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM |
 				     DEV_RX_OFFLOAD_TCP_LRO	|
 				     DEV_RX_OFFLOAD_CRC_STRIP	|
+				     DEV_RX_OFFLOAD_KEEP_CRC    |
 				     DEV_RX_OFFLOAD_SCATTER	|
 				     DEV_RX_OFFLOAD_JUMBO_FRAME |
 				     DEV_RX_OFFLOAD_VLAN_FILTER |
diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
index 35b837c3f..5aa48da42 100644
--- a/drivers/net/ring/rte_eth_ring.c
+++ b/drivers/net/ring/rte_eth_ring.c
@@ -164,6 +164,7 @@ eth_dev_info(struct rte_eth_dev *dev,
 	dev_info->max_rx_queues = (uint16_t)internals->max_rx_queues;
 	dev_info->max_tx_queues = (uint16_t)internals->max_tx_queues;
 	dev_info->min_rx_bufsize = 0;
+	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_CRC_STRIP;
 }
 
 static int
diff --git a/drivers/net/sfc/sfc_rx.c b/drivers/net/sfc/sfc_rx.c
index cc76a5b15..98858d9d9 100644
--- a/drivers/net/sfc/sfc_rx.c
+++ b/drivers/net/sfc/sfc_rx.c
@@ -1443,7 +1443,10 @@ sfc_rx_check_mode(struct sfc_adapter *sa, struct rte_eth_rxmode *rxmode)
 		rc = EINVAL;
 	}
 
-	if (~rxmode->offloads & DEV_RX_OFFLOAD_CRC_STRIP) {
+	/* KEEP_CRC offload flag is not supported by PMD
+	 * can remove the below block when DEV_RX_OFFLOAD_CRC_STRIP removed
+	 */
+	if (rte_eth_dev_must_keep_crc(rxmode->offloads)) {
 		sfc_warn(sa, "FCS stripping cannot be disabled - always on");
 		rxmode->offloads |= DEV_RX_OFFLOAD_CRC_STRIP;
 		rxmode->hw_strip_crc = 1;
diff --git a/drivers/net/softnic/rte_eth_softnic.c b/drivers/net/softnic/rte_eth_softnic.c
index 6b3c13e5c..9360cf6c3 100644
--- a/drivers/net/softnic/rte_eth_softnic.c
+++ b/drivers/net/softnic/rte_eth_softnic.c
@@ -65,6 +65,7 @@ static const struct rte_eth_dev_info pmd_dev_info = {
 		.nb_min = 0,
 		.nb_align = 1,
 	},
+	.rx_offload_capa = DEV_RX_OFFLOAD_CRC_STRIP,
 };
 
 static int pmd_softnic_logtype;
diff --git a/drivers/net/szedata2/rte_eth_szedata2.c b/drivers/net/szedata2/rte_eth_szedata2.c
index 910c64d04..829ad13fa 100644
--- a/drivers/net/szedata2/rte_eth_szedata2.c
+++ b/drivers/net/szedata2/rte_eth_szedata2.c
@@ -1056,7 +1056,8 @@ eth_dev_info(struct rte_eth_dev *dev,
 	dev_info->max_rx_queues = internals->max_rx_queues;
 	dev_info->max_tx_queues = internals->max_tx_queues;
 	dev_info->min_rx_bufsize = 0;
-	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_SCATTER;
+	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_SCATTER |
+				    DEV_RX_OFFLOAD_CRC_STRIP;
 	dev_info->tx_offload_capa = 0;
 	dev_info->rx_queue_offload_capa = 0;
 	dev_info->tx_queue_offload_capa = 0;
diff --git a/drivers/net/thunderx/nicvf_ethdev.c b/drivers/net/thunderx/nicvf_ethdev.c
index 596842137..76fed9f99 100644
--- a/drivers/net/thunderx/nicvf_ethdev.c
+++ b/drivers/net/thunderx/nicvf_ethdev.c
@@ -1905,7 +1905,10 @@ nicvf_dev_configure(struct rte_eth_dev *dev)
 		return -EINVAL;
 	}
 
-	if ((rxmode->offloads & DEV_RX_OFFLOAD_CRC_STRIP) == 0) {
+	/* KEEP_CRC offload flag is not supported by PMD
+	 * can remove the below block when DEV_RX_OFFLOAD_CRC_STRIP removed
+	 */
+	if (rte_eth_dev_must_keep_crc(rxmode->offloads)) {
 		PMD_INIT_LOG(NOTICE, "Can't disable hw crc strip");
 		rxmode->offloads |= DEV_RX_OFFLOAD_CRC_STRIP;
 	}
diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index ba9d768a0..eb1afe691 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -1070,7 +1070,8 @@ eth_dev_info(struct rte_eth_dev *dev,
 
 	dev_info->tx_offload_capa = DEV_TX_OFFLOAD_MULTI_SEGS |
 				DEV_TX_OFFLOAD_VLAN_INSERT;
-	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_VLAN_STRIP;
+	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_VLAN_STRIP |
+				    DEV_RX_OFFLOAD_CRC_STRIP;
 }
 
 static int
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index df50a571a..b629cd45d 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -2127,7 +2127,8 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	};
 
 	host_features = VTPCI_OPS(hw)->get_features(hw);
-	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_VLAN_STRIP;
+	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_VLAN_STRIP |
+				    DEV_RX_OFFLOAD_CRC_STRIP;
 	if (host_features & (1ULL << VIRTIO_NET_F_GUEST_CSUM)) {
 		dev_info->rx_offload_capa |=
 			DEV_RX_OFFLOAD_TCP_CKSUM |
diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
index ba932ff27..95b465952 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
@@ -57,7 +57,8 @@
 	 DEV_RX_OFFLOAD_UDP_CKSUM |	\
 	 DEV_RX_OFFLOAD_TCP_CKSUM |	\
 	 DEV_RX_OFFLOAD_TCP_LRO |	\
-	 DEV_RX_OFFLOAD_JUMBO_FRAME)
+	 DEV_RX_OFFLOAD_JUMBO_FRAME |   \
+	 DEV_RX_OFFLOAD_CRC_STRIP)
 
 static int eth_vmxnet3_dev_init(struct rte_eth_dev *eth_dev);
 static int eth_vmxnet3_dev_uninit(struct rte_eth_dev *eth_dev);
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 97f2c6af7..5aa7a1a7d 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -126,6 +126,7 @@ static const struct {
 	RTE_RX_OFFLOAD_BIT2STR(SCATTER),
 	RTE_RX_OFFLOAD_BIT2STR(TIMESTAMP),
 	RTE_RX_OFFLOAD_BIT2STR(SECURITY),
+	RTE_RX_OFFLOAD_BIT2STR(KEEP_CRC),
 };
 
 #undef RTE_RX_OFFLOAD_BIT2STR
@@ -1184,6 +1185,14 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
 		return -EINVAL;
 	}
 
+	if ((local_conf.rxmode.offloads & DEV_RX_OFFLOAD_CRC_STRIP) &&
+			(local_conf.rxmode.offloads & DEV_RX_OFFLOAD_KEEP_CRC)) {
+		RTE_ETHDEV_LOG(ERR,
+			"Port id=%u not allowed to set both CRC STRIP and KEEP CRC offload flags\n",
+			port_id);
+		return -EINVAL;
+	}
+
 	/* Check that device supports requested rss hash functions. */
 	if ((dev_info.flow_type_rss_offloads |
 	     dev_conf->rx_adv_conf.rss_conf.rss_hf) !=
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index 5760f45d3..aa991da62 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -944,6 +944,12 @@ struct rte_eth_conf {
 #define DEV_RX_OFFLOAD_SCATTER		0x00002000
 #define DEV_RX_OFFLOAD_TIMESTAMP	0x00004000
 #define DEV_RX_OFFLOAD_SECURITY         0x00008000
+
+/**
+ * Invalid to set both DEV_RX_OFFLOAD_CRC_STRIP and DEV_RX_OFFLOAD_KEEP_CRC
+ * No DEV_RX_OFFLOAD_CRC_STRIP flag means keep CRC
+ */
+#define DEV_RX_OFFLOAD_KEEP_CRC		0x00010000
 #define DEV_RX_OFFLOAD_CHECKSUM (DEV_RX_OFFLOAD_IPV4_CKSUM | \
 				 DEV_RX_OFFLOAD_UDP_CKSUM | \
 				 DEV_RX_OFFLOAD_TCP_CKSUM)
diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h
index c9c825e3f..e2426aff8 100644
--- a/lib/librte_ethdev/rte_ethdev_driver.h
+++ b/lib/librte_ethdev/rte_ethdev_driver.h
@@ -325,6 +325,32 @@ typedef int (*ethdev_uninit_t)(struct rte_eth_dev *ethdev);
 int __rte_experimental
 rte_eth_dev_destroy(struct rte_eth_dev *ethdev, ethdev_uninit_t ethdev_uninit);
 
+/**
+ * PMD helper function to check if keeping CRC is requested
+ *
+ * @note
+ * When CRC_STRIP offload flag is removed and default behavior switch to
+ * strip CRC, as planned, this helper function is not that useful and will be
+ * removed. In PMDs this function will be replaced with check:
+ *   if (offloads & DEV_RX_OFFLOAD_KEEP_CRC)
+ *
+ * @param rx_offloads
+ *   offload bits to be applied
+ *
+ * @return
+ *   Return positive if keeping CRC is requested,
+ *   zero if stripping CRC is requested
+ */
+static inline int
+rte_eth_dev_must_keep_crc(uint64_t rx_offloads)
+{
+	if (rx_offloads & DEV_RX_OFFLOAD_CRC_STRIP)
+		return 0;
+
+	/* no KEEP_CRC or CRC_STRIP offload flags means keep CRC */
+	return 1;
+}
+
 #ifdef __cplusplus
 }
 #endif
-- 
2.17.1

Re: [dpdk-dev] [PATCH v3] ethdev: add new offload flag to keep CRC

[Ferruh Yigit]

On 6/29/2018 12:57 PM, Thomas Monjalon wrote:
> 29/06/2018 14:41, Ferruh Yigit:
>> DEV_RX_OFFLOAD_KEEP_CRC offload flag is added. PMDs that support
>> keeping CRC should advertise this offload capability.
>>
>> DEV_RX_OFFLOAD_CRC_STRIP flag will remain one more release
>> default behavior in PMDs are to keep the CRC until this flag removed
>>
>> Until DEV_RX_OFFLOAD_CRC_STRIP flag is removed:
>> - Setting both KEEP_CRC & CRC_STRIP is INVALID
>> - Setting only CRC_STRIP PMD should strip the CRC
>> - Setting only KEEP_CRC PMD should keep the CRC
>> - Not setting both PMD should keep the CRC
>>
>> A helper function rte_eth_dev_is_keep_crc() has been added to be able to
>> change the no flag behavior with minimal changes in PMDs.
>>
>> The PMDs that doesn't report the DEV_RX_OFFLOAD_KEEP_CRC offload can
>> remove rte_eth_dev_is_keep_crc() checks next release, related code
>> commented to help the maintenance task.
>>
>> And DEV_RX_OFFLOAD_CRC_STRIP has been added to virtual drivers since
>> they don't use CRC at all, when an application requires this offload
>> virtual PMDs should not return error.
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> Acked-by: Allain Legacy <allain.legacy@windriver.com>
>> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> 
> Acked-by: Thomas Monjalon <thomas@monjalon.net>

Applied to dpdk-next-net/master, thanks.