diff mbox

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

Message ID 1518509665-12171-1-git-send-email-ophirmu@mellanox.com (mailing list archive)
State Accepted, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Ophir Munk Feb. 13, 2018, 8:14 a.m. UTC 
  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(-)

Comments

Thomas Monjalon Feb. 13, 2018, 4:35 p.m. UTC | #1 
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
Stephen Hemminger Feb. 15, 2018, 9:55 p.m. UTC | #2 
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?
Thomas Monjalon Feb. 16, 2018, 1 p.m. UTC | #3 
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.
diff mbox

Patch

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