summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDavid S. Miller <davem@davemloft.net>2020-06-25 16:16:21 -0700
committerDavid S. Miller <davem@davemloft.net>2020-06-25 16:16:21 -0700
commit0e00c05fa72554c86d7c7e0f538ec83bfe277c91 (patch)
tree1b90af4ad0784abbcbb8987925a07f4b421b3eb6
parentb18e9834f7b248b41e8179a039ec80803bb3d67a (diff)
parent045790b7bc66a75070c112a61558c639cef2263e (diff)
Merge branch 'napi_gro_receive-caller-return-value-cleanups'
Jason A. Donenfeld says: ==================== napi_gro_receive caller return value cleanups In 6570bc79c0df ("net: core: use listified Rx for GRO_NORMAL in napi_gro_receive()"), the GRO_NORMAL case stopped calling netif_receive_skb_internal, checking its return value, and returning GRO_DROP in case it failed. Instead, it calls into netif_receive_skb_list_internal (after a bit of indirection), which doesn't return any error. Therefore, napi_gro_receive will never return GRO_DROP, making handling GRO_DROP dead code. I emailed the author of 6570bc79c0df on netdev [1] to see if this change was intentional, but the dlink.ru email address has been disconnected, and looking a bit further myself, it seems somewhat infeasible to start propagating return values backwards from the internal machinations of netif_receive_skb_list_internal. Taking a look at all the callers of napi_gro_receive, it appears that three are checking the return value for the purpose of comparing it to the now never-happening GRO_DROP, and one just casts it to (void), a likely historical leftover. Every other of the 120 callers does not bother checking the return value. And it seems like these remaining 116 callers are doing the right thing: after calling napi_gro_receive, the packet is now in the hands of the upper layers of the newtworking, and the device driver itself has no business now making decisions based on what the upper layers choose to do. Incrementing stats counters on GRO_DROP seems like a mistake, made by these three drivers, but not by the remaining 117. It would seem, therefore, that after rectifying these four callers of napi_gro_receive, that I should go ahead and just remove returning the value from napi_gro_receive all together. However, napi_gro_receive has a function event tracer, and being able to introspect into the networking stack to see how often napi_gro_receive is returning whatever interesting GRO status (aside from _DROP) remains an interesting data point worth keeping for debugging. So, this series simply gets rid of the return value checking for the four useless places where that check never evaluates to anything meaningful. [1] https://lore.kernel.org/netdev/20200624210606.GA1362687@zx2c4.com/ ==================== Acked-by: Edward Cree <ecree@solarflare.com> Signed-off-by: David S. Miller <davem@davemloft.net>
-rw-r--r--drivers/net/ethernet/hisilicon/hns/hns_enet.c2
-rw-r--r--drivers/net/ethernet/socionext/netsec.c5
-rw-r--r--drivers/net/wireguard/receive.c10
-rw-r--r--drivers/net/wireless/ath/wil6210/txrx.c39
4 files changed, 17 insertions, 39 deletions
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_enet.c b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
index c117074c16e3..23f278e46975 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
@@ -699,7 +699,7 @@ static void hns_nic_rx_up_pro(struct hns_nic_ring_data *ring_data,
struct net_device *ndev = ring_data->napi.dev;
skb->protocol = eth_type_trans(skb, ndev);
- (void)napi_gro_receive(&ring_data->napi, skb);
+ napi_gro_receive(&ring_data->napi, skb);
}
static int hns_desc_unused(struct hnae_ring *ring)
diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
index 328bc38848bb..0f366cc50b74 100644
--- a/drivers/net/ethernet/socionext/netsec.c
+++ b/drivers/net/ethernet/socionext/netsec.c
@@ -1044,8 +1044,9 @@ static int netsec_process_rx(struct netsec_priv *priv, int budget)
skb->ip_summed = CHECKSUM_UNNECESSARY;
next:
- if ((skb && napi_gro_receive(&priv->napi, skb) != GRO_DROP) ||
- xdp_result) {
+ if (skb)
+ napi_gro_receive(&priv->napi, skb);
+ if (skb || xdp_result) {
ndev->stats.rx_packets++;
ndev->stats.rx_bytes += xdp.data_end - xdp.data;
}
diff --git a/drivers/net/wireguard/receive.c b/drivers/net/wireguard/receive.c
index 91438144e4f7..9b2ab6fc91cd 100644
--- a/drivers/net/wireguard/receive.c
+++ b/drivers/net/wireguard/receive.c
@@ -414,14 +414,8 @@ static void wg_packet_consume_data_done(struct wg_peer *peer,
if (unlikely(routed_peer != peer))
goto dishonest_packet_peer;
- if (unlikely(napi_gro_receive(&peer->napi, skb) == GRO_DROP)) {
- ++dev->stats.rx_dropped;
- net_dbg_ratelimited("%s: Failed to give packet to userspace from peer %llu (%pISpfsc)\n",
- dev->name, peer->internal_id,
- &peer->endpoint.addr);
- } else {
- update_rx_stats(peer, message_data_len(len_before_trim));
- }
+ napi_gro_receive(&peer->napi, skb);
+ update_rx_stats(peer, message_data_len(len_before_trim));
return;
dishonest_packet_peer:
diff --git a/drivers/net/wireless/ath/wil6210/txrx.c b/drivers/net/wireless/ath/wil6210/txrx.c
index bc8c15fb609d..080e5aa60bea 100644
--- a/drivers/net/wireless/ath/wil6210/txrx.c
+++ b/drivers/net/wireless/ath/wil6210/txrx.c
@@ -897,7 +897,6 @@ static void wil_rx_handle_eapol(struct wil6210_vif *vif, struct sk_buff *skb)
void wil_netif_rx(struct sk_buff *skb, struct net_device *ndev, int cid,
struct wil_net_stats *stats, bool gro)
{
- gro_result_t rc = GRO_NORMAL;
struct wil6210_vif *vif = ndev_to_vif(ndev);
struct wil6210_priv *wil = ndev_to_wil(ndev);
struct wireless_dev *wdev = vif_to_wdev(vif);
@@ -908,22 +907,16 @@ void wil_netif_rx(struct sk_buff *skb, struct net_device *ndev, int cid,
*/
int mcast = is_multicast_ether_addr(da);
struct sk_buff *xmit_skb = NULL;
- static const char * const gro_res_str[] = {
- [GRO_MERGED] = "GRO_MERGED",
- [GRO_MERGED_FREE] = "GRO_MERGED_FREE",
- [GRO_HELD] = "GRO_HELD",
- [GRO_NORMAL] = "GRO_NORMAL",
- [GRO_DROP] = "GRO_DROP",
- [GRO_CONSUMED] = "GRO_CONSUMED",
- };
if (wdev->iftype == NL80211_IFTYPE_STATION) {
sa = wil_skb_get_sa(skb);
if (mcast && ether_addr_equal(sa, ndev->dev_addr)) {
/* mcast packet looped back to us */
- rc = GRO_DROP;
dev_kfree_skb(skb);
- goto stats;
+ ndev->stats.rx_dropped++;
+ stats->rx_dropped++;
+ wil_dbg_txrx(wil, "Rx drop %d bytes\n", len);
+ return;
}
} else if (wdev->iftype == NL80211_IFTYPE_AP && !vif->ap_isolate) {
if (mcast) {
@@ -967,26 +960,16 @@ void wil_netif_rx(struct sk_buff *skb, struct net_device *ndev, int cid,
wil_rx_handle_eapol(vif, skb);
if (gro)
- rc = napi_gro_receive(&wil->napi_rx, skb);
+ napi_gro_receive(&wil->napi_rx, skb);
else
netif_rx_ni(skb);
- wil_dbg_txrx(wil, "Rx complete %d bytes => %s\n",
- len, gro_res_str[rc]);
- }
-stats:
- /* statistics. rc set to GRO_NORMAL for AP bridging */
- if (unlikely(rc == GRO_DROP)) {
- ndev->stats.rx_dropped++;
- stats->rx_dropped++;
- wil_dbg_txrx(wil, "Rx drop %d bytes\n", len);
- } else {
- ndev->stats.rx_packets++;
- stats->rx_packets++;
- ndev->stats.rx_bytes += len;
- stats->rx_bytes += len;
- if (mcast)
- ndev->stats.multicast++;
}
+ ndev->stats.rx_packets++;
+ stats->rx_packets++;
+ ndev->stats.rx_bytes += len;
+ stats->rx_bytes += len;
+ if (mcast)
+ ndev->stats.multicast++;
}
void wil_netif_rx_any(struct sk_buff *skb, struct net_device *ndev)