[PATCH net-next 0/2] net-gro: GRO_DROP deprecation

[PATCH net-next 0/2] net-gro: GRO_DROP deprecation

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

GRO_DROP has no practical use and can be removed,
once ice driver is cleaned up.

This removes one useless conditionel test in napi_gro_frags()

Eric Dumazet (2):
  ice: drop dead code in ice_receive_skb()
  net-gro: remove GRO_DROP

 drivers/net/ethernet/intel/ice/ice_txrx_lib.c |  7 +------
 include/linux/netdevice.h                     |  1 -
 net/core/dev.c                                | 11 -----------
 3 files changed, 1 insertion(+), 18 deletions(-)

-- 
2.29.2.729.g45daf8777d-goog

[PATCH net-next 1/2] ice: drop dead code in ice_receive_skb()

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

napi_gro_receive() can never return GRO_DROP

GRO_DROP can only be returned from napi_gro_frags()
which is the other NAPI GRO entry point.

Followup patch will remove GRO_DROP, because drivers
are not supposed to call napi_gro_frags() if prior
napi_get_frags() has failed.

Note that I have left the gro_dropped variable. I leave to ice
maintainers the decision to further remove it from ethtool -S results.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_txrx_lib.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c
index bc2f4390b51dc67b2ef8cf929a6451a0259e3d51..02b12736ea80d4551d3d3460339eb0706941d22f 100644
--- a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c
@@ -191,12 +191,7 @@ ice_receive_skb(struct ice_ring *rx_ring, struct sk_buff *skb, u16 vlan_tag)
 	if ((rx_ring->netdev->features & NETIF_F_HW_VLAN_CTAG_RX) &&
 	    (vlan_tag & VLAN_VID_MASK))
 		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vlan_tag);
-	if (napi_gro_receive(&rx_ring->q_vector->napi, skb) == GRO_DROP) {
-		/* this is tracked separately to help us debug stack drops */
-		rx_ring->rx_stats.gro_dropped++;
-		netdev_dbg(rx_ring->netdev, "Receive Queue %d: Dropped packet from GRO\n",
-			   rx_ring->q_index);
-	}
+	napi_gro_receive(&rx_ring->q_vector->napi, skb);
 }
 
 /**
-- 
2.29.2.729.g45daf8777d-goog

[PATCH net-next 2/2] net-gro: remove GRO_DROP

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

GRO_DROP can only be returned from napi_gro_frags()
if the skb has not been allocated by a prior napi_get_frags()

Since drivers must use napi_get_frags() and test its result
before populating the skb with metadata, we can safely remove
GRO_DROP since it offers no practical use.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
---
 include/linux/netdevice.h |  1 -
 net/core/dev.c            | 11 -----------
 2 files changed, 12 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 1ec3ac5d5bbffe6062216fbd4009e88d8c909fa9..5b949076ed2319fc676a7172350480efea5807d9 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -376,7 +376,6 @@ enum gro_result {
 	GRO_MERGED_FREE,
 	GRO_HELD,
 	GRO_NORMAL,
-	GRO_DROP,
 	GRO_CONSUMED,
 };
 typedef enum gro_result gro_result_t;
diff --git a/net/core/dev.c b/net/core/dev.c
index 7afbb642e203ad1556e96e2fc7595b6289152201..e4d77c8abe761408caf3a0d1880727f33b5134b6 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6070,10 +6070,6 @@ static gro_result_t napi_skb_finish(struct napi_struct *napi,
 		gro_normal_one(napi, skb);
 		break;
 
-	case GRO_DROP:
-		kfree_skb(skb);
-		break;
-
 	case GRO_MERGED_FREE:
 		if (NAPI_GRO_CB(skb)->free == NAPI_GRO_FREE_STOLEN_HEAD)
 			napi_skb_free_stolen_head(skb);
@@ -6158,10 +6154,6 @@ static gro_result_t napi_frags_finish(struct napi_struct *napi,
 			gro_normal_one(napi, skb);
 		break;
 
-	case GRO_DROP:
-		napi_reuse_skb(napi, skb);
-		break;
-
 	case GRO_MERGED_FREE:
 		if (NAPI_GRO_CB(skb)->free == NAPI_GRO_FREE_STOLEN_HEAD)
 			napi_skb_free_stolen_head(skb);
@@ -6223,9 +6215,6 @@ gro_result_t napi_gro_frags(struct napi_struct *napi)
 	gro_result_t ret;
 	struct sk_buff *skb = napi_frags_skb(napi);
 
-	if (!skb)
-		return GRO_DROP;
-
 	trace_napi_gro_frags_entry(skb);
 
 	ret = napi_frags_finish(napi, skb, dev_gro_receive(napi, skb));
-- 
2.29.2.729.g45daf8777d-goog

Re: [PATCH net-next 2/2] net-gro: remove GRO_DROP

[Edward Cree]

On 08/01/2021 11:39, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
> 
> GRO_DROP can only be returned from napi_gro_frags()
> if the skb has not been allocated by a prior napi_get_frags()
> 
> Since drivers must use napi_get_frags() and test its result
> before populating the skb with metadata, we can safely remove
> GRO_DROP since it offers no practical use.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>

Fwiw,
Acked-by: Edward Cree <ecree.xilinx@gmail.com>

Re: [PATCH net-next 1/2] ice: drop dead code in ice_receive_skb()

[Jesse Brandeburg]

Eric Dumazet wrote:

> From: Eric Dumazet <edumazet@google.com>
> 
> napi_gro_receive() can never return GRO_DROP
> 
> GRO_DROP can only be returned from napi_gro_frags()
> which is the other NAPI GRO entry point.
> 
> Followup patch will remove GRO_DROP, because drivers
> are not supposed to call napi_gro_frags() if prior
> napi_get_frags() has failed.
> 
> Note that I have left the gro_dropped variable. I leave to ice
> maintainers the decision to further remove it from ethtool -S results.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>

Acked-by: Jesse Brandeburg <jesse.brandeburg@intel.com>

Jakub or David, you're welcome to apply directly as part of this series.

The original code went into the kernel right as the code to remove
GRO_DROP returns went in just before, but the reviews crossed each
other and no-one (especially me :-( ) caught it.

for reference:
commit 0e00c05fa72554c86d7c7e0f538ec83bfe277c91
Merge: b18e9834f7b2 045790b7bc66
Author: David S. Miller <davem@davemloft.net>
Date:   Thu Jun 25 16:16:21 2020 -0700
Subject: Merge branch 'napi_gro_receive-caller-return-value-cleanups'

Re: [PATCH net-next 0/2] net-gro: GRO_DROP deprecation

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (refs/heads/master):

On Fri,  8 Jan 2021 03:39:01 -0800 you wrote:
> From: Eric Dumazet <edumazet@google.com>
> 
> GRO_DROP has no practical use and can be removed,
> once ice driver is cleaned up.
> 
> This removes one useless conditionel test in napi_gro_frags()
> 
> [...]

Here is the summary with links:
  - [net-next,1/2] ice: drop dead code in ice_receive_skb()
    https://git.kernel.org/netdev/net-next/c/f73fc40327c0
  - [net-next,2/2] net-gro: remove GRO_DROP
    https://git.kernel.org/netdev/net-next/c/1d11fa696733

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html