[dpdk-dev] [PATCH v2] net/tap: add CRC stripping capability

[dpdk-dev] [PATCH v2] net/tap: add CRC stripping capability

[Ophir Munk]

CRC stripping is executed in the kernel outside of TAP PMD scope.
There is no prevention that the TAP PMD will report on Rx CRC
stripping capability.
In the corrupted code, TAP PMD did not report on this capability.
The fix enables TAP PMD to report that Rx CRC stripping is supported.

Fixes: 02f96a0a82d1 ("net/tap: add TUN/TAP device PMD")
Cc: stable@dpdk.org

Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
---
v2: updated commit message

 drivers/net/tap/rte_eth_tap.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 9d39384..f09db0e 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -261,7 +261,8 @@ tap_rx_offload_get_port_capa(void)
 	return DEV_RX_OFFLOAD_SCATTER |
 	       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;
 }
 
 static uint64_t
@@ -270,7 +271,8 @@ tap_rx_offload_get_queue_capa(void)
 	return DEV_RX_OFFLOAD_SCATTER |
 	       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;
 }
 
 static bool
-- 
2.7.4

Re: [dpdk-dev] [PATCH v2] net/tap: add CRC stripping capability

[Thomas Monjalon]

13/02/2018 09:14, Ophir Munk:
> CRC stripping is executed in the kernel outside of TAP PMD scope.
> There is no prevention that the TAP PMD will report on Rx CRC
> stripping capability.
> In the corrupted code, TAP PMD did not report on this capability.
> The fix enables TAP PMD to report that Rx CRC stripping is supported.
> 
> Fixes: 02f96a0a82d1 ("net/tap: add TUN/TAP device PMD")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ophir Munk <ophirmu@mellanox.com>

Applied, thanks

Re: [dpdk-dev] [PATCH v2] net/tap: add CRC stripping capability

[Stephen Hemminger]

On Tue, 13 Feb 2018 17:35:20 +0100
Thomas Monjalon <thomas@monjalon.net> wrote:

> 13/02/2018 09:14, Ophir Munk:
> > CRC stripping is executed in the kernel outside of TAP PMD scope.
> > There is no prevention that the TAP PMD will report on Rx CRC
> > stripping capability.
> > In the corrupted code, TAP PMD did not report on this capability.
> > The fix enables TAP PMD to report that Rx CRC stripping is supported.
> > 
> > Fixes: 02f96a0a82d1 ("net/tap: add TUN/TAP device PMD")
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Ophir Munk <ophirmu@mellanox.com>  
> 
> Applied, thanks
> 

The whole CRC strip flag notion is backwards. It really should of been
a bit set if driver allows preserving CRC.

Since changing the ABI is not possible right now;
the ethdev core ought to log a warning whenever driver is registered
without CRC_STRIP flag.

Or is lack of CRC_STRIP in offload flags implying that driver can
do strip and not stripping?

Re: [dpdk-dev] [PATCH v2] net/tap: add CRC stripping capability

[Thomas Monjalon]

15/02/2018 22:55, Stephen Hemminger:
> On Tue, 13 Feb 2018 17:35:20 +0100
> Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> > 13/02/2018 09:14, Ophir Munk:
> > > CRC stripping is executed in the kernel outside of TAP PMD scope.
> > > There is no prevention that the TAP PMD will report on Rx CRC
> > > stripping capability.
> > > In the corrupted code, TAP PMD did not report on this capability.
> > > The fix enables TAP PMD to report that Rx CRC stripping is supported.
> > > 
> > > Fixes: 02f96a0a82d1 ("net/tap: add TUN/TAP device PMD")
> > > Cc: stable@dpdk.org
> > > 
> > > Signed-off-by: Ophir Munk <ophirmu@mellanox.com>  
> > 
> > Applied, thanks
> > 
> 
> The whole CRC strip flag notion is backwards. It really should of been
> a bit set if driver allows preserving CRC.
> 
> Since changing the ABI is not possible right now;
> the ethdev core ought to log a warning whenever driver is registered
> without CRC_STRIP flag.
> 
> Or is lack of CRC_STRIP in offload flags implying that driver can
> do strip and not stripping?

I agree we should change the API.
Let's open a new thread to discuss it with a wider audience.