net: remove gfp_mask from napi_alloc_skb()

__napi_alloc_skb() is napi_alloc_skb() with the added flexibility
of choosing gfp_mask. This is a NAPI function, so GFP_ATOMIC is
implied. The only practical choice the caller has is whether to
set __GFP_NOWARN. But that's a false choice, too, allocation failures
in atomic context will happen, and printing warnings in logs,
effectively for a packet drop, is both too much and very likely
non-actionable.

This leads me to a conclusion that most uses of napi_alloc_skb()
are simply misguided, and should use __GFP_NOWARN in the first
place. We also have a "standard" way of reporting allocation
failures via the queue stat API (qstats::rx-alloc-fail).

The direct motivation for this patch is that one of the drivers
used at Meta calls napi_alloc_skb() (so prior to this patch without
__GFP_NOWARN), and the resulting OOM warning is the top networking
warning in our fleet.

Reviewed-by: Alexander Lobakin <aleksander.lobakin@intel.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Link: https://lore.kernel.org/r/20240327040213.3153864-1-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/Documentation/mm/page_frags.rst b/Documentation/mm/page_frags.rst
index a81617e688a84fa063ffec59308a3c1a221112a2..503ca6cdb80401cc96cdc0f3f28078fac2158d06 100644 (file)
--- a/Documentation/mm/page_frags.rst
+++ b/Documentation/mm/page_frags.rst
@@ -25,7 +25,7 @@ to be disabled when executing the fragment allocation.
 The network stack uses two separate caches per CPU to handle fragment
 allocation.  The netdev_alloc_cache is used by callers making use of the
 netdev_alloc_frag and __netdev_alloc_skb calls.  The napi_alloc_cache is
-used by callers of the __napi_alloc_frag and __napi_alloc_skb calls.  The
+used by callers of the __napi_alloc_frag and napi_alloc_skb calls.  The
 main difference between these two calls is the context in which they may be
 called.  The "netdev" prefixed functions are usable in any context as these
 functions will disable interrupts, while the "napi" prefixed functions are

diff --git a/Documentation/translations/zh_CN/mm/page_frags.rst b/Documentation/translations/zh_CN/mm/page_frags.rst
index 20bd3fafdc8c993b196666feb0fae9a93bec4d81..a5b22486a91345fb3b0c6aff7491b0a5640be2b7 100644 (file)
--- a/Documentation/translations/zh_CN/mm/page_frags.rst
+++ b/Documentation/translations/zh_CN/mm/page_frags.rst
@@ -25,7 +25,7 @@ sk_buff->head使用，或者用于skb_shared_info的 “frags” 部分。
 
 网络堆栈在每个CPU使用两个独立的缓存来处理碎片分配。netdev_alloc_cache被使用
 netdev_alloc_frag和__netdev_alloc_skb调用的调用者使用。napi_alloc_cache
-被调用__napi_alloc_frag和__napi_alloc_skb的调用者使用。这两个调用的主要区别是
+被调用__napi_alloc_frag和napi_alloc_skb的调用者使用。这两个调用的主要区别是
 它们可能被调用的环境。“netdev” 前缀的函数可以在任何上下文中使用，因为这些函数
 将禁用中断，而 ”napi“ 前缀的函数只可以在softirq上下文中使用。
 

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 0d7177083708f29d3b4deba11d00abdcb017f886..ac2fcc5ac595ec7b506496ea95e1827e5aa324f5 100644 (file)
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -2144,9 +2144,7 @@ static struct sk_buff *i40e_construct_skb(struct i40e_ring *rx_ring,
         */
 
        /* allocate a skb to store the frags */
-       skb = __napi_alloc_skb(&rx_ring->q_vector->napi,
-                              I40E_RX_HDR_SIZE,
-                              GFP_ATOMIC | __GFP_NOWARN);
+       skb = napi_alloc_skb(&rx_ring->q_vector->napi, I40E_RX_HDR_SIZE);
        if (unlikely(!skb))
                return NULL;
 

diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
index 11500003af0d47dbfb203ea51914c2f452b42368..a85b425794df2c0ebc2adf286721906228271fc4 100644 (file)
--- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
@@ -301,8 +301,7 @@ static struct sk_buff *i40e_construct_skb_zc(struct i40e_ring *rx_ring,
        net_prefetch(xdp->data_meta);
 
        /* allocate a skb to store the frags */
-       skb = __napi_alloc_skb(&rx_ring->q_vector->napi, totalsize,
-                              GFP_ATOMIC | __GFP_NOWARN);
+       skb = napi_alloc_skb(&rx_ring->q_vector->napi, totalsize);
        if (unlikely(!skb))
                goto out;
 

diff --git a/drivers/net/ethernet/intel/iavf/iavf_txrx.c b/drivers/net/ethernet/intel/iavf/iavf_txrx.c
index b71484c87a84618f5e8722fee0506ba23841e051..32bb604a13825f76192425c9359daf28b557659d 100644 (file)
--- a/drivers/net/ethernet/intel/iavf/iavf_txrx.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_txrx.c
@@ -1334,9 +1334,7 @@ static struct sk_buff *iavf_construct_skb(struct iavf_ring *rx_ring,
        net_prefetch(va);
 
        /* allocate a skb to store the frags */
-       skb = __napi_alloc_skb(&rx_ring->q_vector->napi,
-                              IAVF_RX_HDR_SIZE,
-                              GFP_ATOMIC | __GFP_NOWARN);
+       skb = napi_alloc_skb(&rx_ring->q_vector->napi, IAVF_RX_HDR_SIZE);
        if (unlikely(!skb))
                return NULL;
 

diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c b/drivers/net/ethernet/intel/ice/ice_txrx.c
index 97d41d6ebf1fb69419e2cf13dae17db08fd27910..8bb743f78fcb4771d852293dfcbbd3b28dbf3726 100644 (file)
--- a/drivers/net/ethernet/intel/ice/ice_txrx.c
+++ b/drivers/net/ethernet/intel/ice/ice_txrx.c
@@ -1051,8 +1051,7 @@ ice_construct_skb(struct ice_rx_ring *rx_ring, struct xdp_buff *xdp)
        }
 
        /* allocate a skb to store the frags */
-       skb = __napi_alloc_skb(&rx_ring->q_vector->napi, ICE_RX_HDR_SIZE,
-                              GFP_ATOMIC | __GFP_NOWARN);
+       skb = napi_alloc_skb(&rx_ring->q_vector->napi, ICE_RX_HDR_SIZE);
        if (unlikely(!skb))
                return NULL;
 

diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c b/drivers/net/ethernet/intel/ice/ice_xsk.c
index 1857220d27fee7389e83622164dc71b3d8173f70..aa81d1162b8156a7b61f8fdda2317c91cbf76eff 100644 (file)
--- a/drivers/net/ethernet/intel/ice/ice_xsk.c
+++ b/drivers/net/ethernet/intel/ice/ice_xsk.c
@@ -555,8 +555,7 @@ ice_construct_skb_zc(struct ice_rx_ring *rx_ring, struct xdp_buff *xdp)
        }
        net_prefetch(xdp->data_meta);
 
-       skb = __napi_alloc_skb(&rx_ring->q_vector->napi, totalsize,
-                              GFP_ATOMIC | __GFP_NOWARN);
+       skb = napi_alloc_skb(&rx_ring->q_vector->napi, totalsize);
        if (unlikely(!skb))
                return NULL;
 

diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.c b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
index 6dd7a66bb8979ab16a21b5a4243c03cc2957ff29..f940f650cd788520a27faae9ec0c576b6a7a6c4a 100644 (file)
--- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
@@ -3005,8 +3005,7 @@ struct sk_buff *idpf_rx_construct_skb(struct idpf_queue *rxq,
        /* prefetch first cache line of first page */
        net_prefetch(va);
        /* allocate a skb to store the frags */
-       skb = __napi_alloc_skb(&rxq->q_vector->napi, IDPF_RX_HDR_SIZE,
-                              GFP_ATOMIC);
+       skb = napi_alloc_skb(&rxq->q_vector->napi, IDPF_RX_HDR_SIZE);
        if (unlikely(!skb)) {
                idpf_rx_put_page(rx_buf);
 
@@ -3060,7 +3059,7 @@ static struct sk_buff *idpf_rx_hdr_construct_skb(struct idpf_queue *rxq,
        struct sk_buff *skb;
 
        /* allocate a skb to store the frags */
-       skb = __napi_alloc_skb(&rxq->q_vector->napi, size, GFP_ATOMIC);
+       skb = napi_alloc_skb(&rxq->q_vector->napi, size);
        if (unlikely(!skb))
                return NULL;
 

diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 35ad40a803cb64a66b983b3a2103a8f15b808df5..0cd923c8ac9b217f5073a0f6cc80dac2297c36f4 100644 (file)
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -2712,8 +2712,7 @@ static struct sk_buff *igc_construct_skb_zc(struct igc_ring *ring,
 
        net_prefetch(xdp->data_meta);
 
-       skb = __napi_alloc_skb(&ring->q_vector->napi, totalsize,
-                              GFP_ATOMIC | __GFP_NOWARN);
+       skb = napi_alloc_skb(&ring->q_vector->napi, totalsize);
        if (unlikely(!skb))
                return NULL;
 

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
index d34d715c59ebcd4c0119d63f0122d89308a925aa..397cb773fabbe71c2b87947135a3d68244249ab4 100644 (file)
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
@@ -220,8 +220,7 @@ static struct sk_buff *ixgbe_construct_skb_zc(struct ixgbe_ring *rx_ring,
        net_prefetch(xdp->data_meta);
 
        /* allocate a skb to store the frags */
-       skb = __napi_alloc_skb(&rx_ring->q_vector->napi, totalsize,
-                              GFP_ATOMIC | __GFP_NOWARN);
+       skb = napi_alloc_skb(&rx_ring->q_vector->napi, totalsize);
        if (unlikely(!skb))
                return NULL;
 

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 24cd80490d19cf86c2cd566b81f13b137109d784..bcdde68a099a29d1c242ab2fcbb60703050cb1c9 100644 (file)
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -5109,9 +5109,8 @@ static struct sk_buff *stmmac_construct_skb_zc(struct stmmac_channel *ch,
        unsigned int datasize = xdp->data_end - xdp->data;
        struct sk_buff *skb;
 
-       skb = __napi_alloc_skb(&ch->rxtx_napi,
-                              xdp->data_end - xdp->data_hard_start,
-                              GFP_ATOMIC | __GFP_NOWARN);
+       skb = napi_alloc_skb(&ch->rxtx_napi,
+                            xdp->data_end - xdp->data_hard_start);
        if (unlikely(!skb))
                return NULL;
 

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 517e546a120aec086b45657d73761a5c90f5f475..b7f1ecdaec382e05769dce3bebc49d9f66eb764b 100644 (file)
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3350,13 +3350,7 @@ static inline void *napi_alloc_frag_align(unsigned int fragsz,
        return __napi_alloc_frag_align(fragsz, -align);
 }
 
-struct sk_buff *__napi_alloc_skb(struct napi_struct *napi,
-                                unsigned int length, gfp_t gfp_mask);
-static inline struct sk_buff *napi_alloc_skb(struct napi_struct *napi,
-                                            unsigned int length)
-{
-       return __napi_alloc_skb(napi, length, GFP_ATOMIC);
-}
+struct sk_buff *napi_alloc_skb(struct napi_struct *napi, unsigned int length);
 void napi_consume_skb(struct sk_buff *skb, int budget);
 
 void napi_skb_free_stolen_head(struct sk_buff *skb);

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 17617c29be2df53058163e63b15a71dabf280db3..a1be84be5d35a570a86b6fd36e69e756687f9988 100644 (file)
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -775,10 +775,9 @@ skb_fail:
 EXPORT_SYMBOL(__netdev_alloc_skb);
 
 /**
- *     __napi_alloc_skb - allocate skbuff for rx in a specific NAPI instance
+ *     napi_alloc_skb - allocate skbuff for rx in a specific NAPI instance
  *     @napi: napi instance this buffer was allocated for
  *     @len: length to allocate
- *     @gfp_mask: get_free_pages mask, passed to alloc_skb and alloc_pages
  *
  *     Allocate a new sk_buff for use in NAPI receive.  This buffer will
  *     attempt to allocate the head from a special reserved region used
@@ -787,9 +786,9 @@ EXPORT_SYMBOL(__netdev_alloc_skb);
  *
  *     %NULL is returned if there is no free memory.
  */
-struct sk_buff *__napi_alloc_skb(struct napi_struct *napi, unsigned int len,
-                                gfp_t gfp_mask)
+struct sk_buff *napi_alloc_skb(struct napi_struct *napi, unsigned int len)
 {
+       gfp_t gfp_mask = GFP_ATOMIC | __GFP_NOWARN;
        struct napi_alloc_cache *nc;
        struct sk_buff *skb;
        bool pfmemalloc;
@@ -860,7 +859,7 @@ skb_success:
 skb_fail:
        return skb;
 }
-EXPORT_SYMBOL(__napi_alloc_skb);
+EXPORT_SYMBOL(napi_alloc_skb);
 
 void skb_add_rx_frag_netmem(struct sk_buff *skb, int i, netmem_ref netmem,
                            int off, int size, unsigned int truesize)