netdev - Re: [PATCH V12 net-next 07/10] net: hibmcge: Implement rx

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2dd71e95-5fb2-42c9-aff0-3189e958730a@redhat.com>
Date: Tue, 15 Oct 2024 12:28:26 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Jijie Shao <shaojijie@...wei.com>, davem@...emloft.net,
 edumazet@...gle.com, kuba@...nel.org
Cc: shenjian15@...wei.com, wangpeiyang1@...wei.com, liuyonglong@...wei.com,
 chenhao418@...wei.com, sudongming1@...wei.com, xujunsheng@...wei.com,
 shiyongbang@...wei.com, libaihan@...wei.com, andrew@...n.ch,
 jdamato@...tly.com, horms@...nel.org, kalesh-anakkur.purayil@...adcom.com,
 christophe.jaillet@...adoo.fr, jonathan.cameron@...wei.com,
 shameerali.kolothum.thodi@...wei.com, salil.mehta@...wei.com,
 netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH V12 net-next 07/10] net: hibmcge: Implement rx_poll
 function to receive packets

On 10/10/24 16:21, Jijie Shao wrote:
> @@ -124,6 +129,20 @@ static void hbg_buffer_free_skb(struct hbg_buffer *buffer)
>   	buffer->skb = NULL;
>   }
>   
> +static int hbg_buffer_alloc_skb(struct hbg_buffer *buffer)
> +{
> +	u32 len = hbg_spec_max_frame_len(buffer->priv, buffer->dir);
> +	struct hbg_priv *priv = buffer->priv;
> +
> +	buffer->skb = netdev_alloc_skb(priv->netdev, len);
> +	if (unlikely(!buffer->skb))
> +		return -ENOMEM;

It looks like I was not clear enough in my previous feedback: allocating 
the sk_buff struct at packet reception time, will be much more 
efficient, because the sk_buff contents will be hot in cache for the RX 
path, while allocating it here, together with the data pointer itself 
will almost ensure 2-4 cache misses per RX packet.

You could allocate here the data buffer i.e. via a page allocator and
at rx processing time use build_skb() on top of such data buffer.

I understand it's probably such refactor would be painful at this point, 
but you should consider it as a follow-up.

Side note: the above always uses the maximum MTU for the packet size, if 
the device supports jumbo frames (8Kb size packets), it will produce 
quite bad layout for the incoming packets... Is the device able to use 
multiple buffers for the incoming packets?

Thanks,

Paolo