Re: [PATCH net-next] net: ena: reuse the whole RX page on 64kB page kernels

[Arthur Kiyanovski]

On Fri, 06 Nov 2026 03:17:11 +0000, Harry Yoo (Meta) <harry@xxxxxxxxxx> wrote:
> [...]
> ENA_PAGE_SIZE. With this fixed, a comparable workload pins only 497MB.
> 
> When PAGE_SIZE equals ENA_PAGE_SIZE, the room left in the page equals
> ena_buf->len, so nothing changes.
> 
> Signed-off-by: Harry Yoo (Meta) <harry@xxxxxxxxxx>

Hi Harry,
Thank you for the patch. The issue you are fixing is real, and the patch
is very welcome.

I want to suggest a few changes to make sure that the performance of the
datapath of 4KB pages is changed as little as possible, as this will be
the common case.

First:
DRB (Dynamic RX Buffers) - is what we call the reuse of the pages as
long as the remaining part is "large enough" instead of allocating a
new page for every new packet.

The ENA driver supports DRB only in the non XDP case.

Therefore some of the calculations you are doing per call to
ena_try_rx_buf_page_reuse() can be saved as compile time constants.

For that I suggest defining the following in ena_netdev.h:

+ #define ENA_RX_PAGE_ROOM    (PAGE_SIZE - NET_SKB_PAD -          \
+                   SKB_DATA_ALIGN(sizeof(struct skb_shared_info)))
+ #define ENA_RX_MAX_BUF_LEN  (ENA_PAGE_SIZE - NET_SKB_PAD -          \
+                   SKB_DATA_ALIGN(sizeof(struct skb_shared_info)))

Next I would like to use the old calculations for the simpler 4KB pages
case. See these suggestions inline.

Could you fold these into the respin?

Thanks,
Arthur

>
>
> diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> index 7eb6456ed0d5..4a35cb6a7785 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
> +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> @@ -926,19 +926,27 @@ static struct sk_buff *ena_alloc_skb(struct ena_ring *rx_ring, void *first_frag,
>  	return skb;
>  }
>  
> -static bool ena_try_rx_buf_page_reuse(struct ena_rx_buffer *rx_info, u16 buf_len,
> +static bool ena_try_rx_buf_page_reuse(struct ena_ring *rx_ring,

Becomes unused: struct ena_ring *rx_ring parameter and in call sites.

> +				      struct ena_rx_buffer *rx_info, u16 buf_len,
>  				      u16 len, int pkt_offset)
>  {
> +	int tailroom = SKB_DATA_ALIGN(sizeof(struct skb_shared_info));

Becomes unused:
-       int tailroom = SKB_DATA_ALIGN(sizeof(struct skb_shared_info));

>  	struct ena_com_buf *ena_buf = &rx_info->ena_buf;
> +	unsigned long room;

-       unsigned long room;
+       u32 room;

> +
> +	/* Room left up to the end of the page. On 64kB page kernels this
> +	 * can be more than ena_buf->len, which is capped at ENA_PAGE_SIZE.
> +	 */
> +	room = PAGE_SIZE - rx_info->page_offset - rx_ring->rx_headroom - tailroom;
>  

-       room = PAGE_SIZE - rx_info->page_offset - rx_ring->rx_headroom - tailroom;

In runtime there is no branch here because the result is known at
compile time (same with the next suggestion below):

+       if (PAGE_SIZE > ENA_PAGE_SIZE)
+               room = ENA_RX_PAGE_ROOM - rx_info->page_offset;
+       else
+               room = ena_buf->len;

>  	/* More than ENA_MIN_RX_BUF_SIZE left in the reused buffer
>  	 * for data + headroom + tailroom.
>  	 */
> -	if (SKB_DATA_ALIGN(len + pkt_offset) + ENA_MIN_RX_BUF_SIZE <= ena_buf->len) {
> +	if (SKB_DATA_ALIGN(len + pkt_offset) + ENA_MIN_RX_BUF_SIZE <= room) {
>  		page_ref_inc(rx_info->page);
>  		rx_info->page_offset += buf_len;
>  		ena_buf->paddr += buf_len;
> -		ena_buf->len -= buf_len;
> +		ena_buf->len = min(room - buf_len, ENA_PAGE_SIZE);

-               ena_buf->len = min(room - buf_len, ENA_PAGE_SIZE);
+               if (PAGE_SIZE > ENA_PAGE_SIZE)
+                       ena_buf->len = min_t(u32, room - buf_len,
+                                            ENA_RX_MAX_BUF_LEN);
+               else
+                       ena_buf->len -= buf_len;

Use ENA_RX_MAX_BUF_LEN here too, so that headroom + len + tailroom
stays within ENA_PAGE_SIZE as it does elsewhere in the driver.

-- 
Arthur Kiyanovski <akiyano@xxxxxxxxxx>