netdev - Re: [PATCH net 3/4] ice: fix truesize operations for PAGE

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: <91b5e7da-ec4e-40f8-908f-faf7082b2f63@intel.com>
Date: Tue, 20 Aug 2024 15:12:20 -0700
From: Jacob Keller <jacob.e.keller@...el.com>
To: Tony Nguyen <anthony.l.nguyen@...el.com>, <davem@...emloft.net>,
	<kuba@...nel.org>, <pabeni@...hat.com>, <edumazet@...gle.com>,
	<netdev@...r.kernel.org>
CC: Maciej Fijalkowski <maciej.fijalkowski@...el.com>,
	<magnus.karlsson@...el.com>, <ast@...nel.org>, <daniel@...earbox.net>,
	<hawk@...nel.org>, <john.fastabend@...il.com>, <bpf@...r.kernel.org>, "Luiz
 Capitulino" <luizcap@...hat.com>, Chandan Kumar Rout
	<chandanx.rout@...el.com>
Subject: Re: [PATCH net 3/4] ice: fix truesize operations for PAGE_SIZE >=
 8192



On 8/20/2024 2:56 PM, Tony Nguyen wrote:
> From: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
> 
> When working on multi-buffer packet on arch that has PAGE_SIZE >= 8192,
> truesize is calculated and stored in xdp_buff::frame_sz per each
> processed Rx buffer. This means that frame_sz will contain the truesize
> based on last received buffer, but commit 1dc1a7e7f410 ("ice:
> Centrallize Rx buffer recycling") assumed this value will be constant
> for each buffer, which breaks the page recycling scheme and mess up the
> way we update the page::page_offset.
> 
> To fix this, let us work on constant truesize when PAGE_SIZE >= 8192
> instead of basing this on size of a packet read from Rx descriptor. This
> way we can simplify the code and avoid calculating truesize per each
> received frame and on top of that when using
> xdp_update_skb_shared_info(), current formula for truesize update will
> be valid.
> 
> This means ice_rx_frame_truesize() can be removed altogether.
> Furthermore, first call to it within ice_clean_rx_irq() for 4k PAGE_SIZE
> was redundant as xdp_buff::frame_sz is initialized via xdp_init_buff()
> in ice_vsi_cfg_rxq(). This should have been removed at the point where
> xdp_buff struct started to be a member of ice_rx_ring and it was no
> longer a stack based variable.
> 
> There are two fixes tags as my understanding is that the first one
> exposed us to broken truesize and page_offset handling and then second
> introduced broken skb_shared_info update in ice_{construct,build}_skb().
> 
> Reported-and-tested-by: Luiz Capitulino <luizcap@...hat.com>
> Closes: https://lore.kernel.org/netdev/8f9e2a5c-fd30-4206-9311-946a06d031bb@redhat.com/
> Fixes: 1dc1a7e7f410 ("ice: Centrallize Rx buffer recycling")
> Fixes: 2fba7dc5157b ("ice: Add support for XDP multi-buffer on Rx side")
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
> Tested-by: Chandan Kumar Rout <chandanx.rout@...el.com> (A Contingent Worker at Intel)
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@...el.com>
> ---

Much simpler too!

Reviewed-by: Jacob Keller <jacob.e.keller@...el.com>