> -----Original Message----- > From: Intel-wired-lan <[email protected]> On Behalf Of > Simon Horman > Sent: Thursday, October 17, 2024 2:04 AM > To: Hay, Joshua A <[email protected]> > Cc: [email protected]; Lobakin, Aleksander > <[email protected]>; Chittim, Madhu > <[email protected]>; [email protected] > Subject: Re: [Intel-wired-lan] [PATCH iwl-net] idpf: set completion tag for > "empty" bufs associated with a packet > > On Mon, Oct 07, 2024 at 01:24:35PM -0700, Joshua Hay wrote: > > Commit d9028db618a6 ("idpf: convert to libeth Tx buffer completion") > > inadvertently removed code that was necessary for the tx buffer cleaning > > routine to iterate over all buffers associated with a packet. > > > > When a frag is too large for a single data descriptor, it will be split > > across multiple data descriptors. This means the frag will span multiple > > buffers in the buffer ring in order to keep the descriptor and buffer > > ring indexes aligned. The buffer entries in the ring are technically > > empty and no cleaning actions need to be performed. These empty buffers > > can precede other frags associated with the same packet. I.e. a single > > packet on the buffer ring can look like: > > > > buf[0]=skb0.frag0 > > buf[1]=skb0.frag1 > > buf[2]=empty > > buf[3]=skb0.frag2 > > > > The cleaning routine iterates through these buffers based on a matching > > completion tag. If the completion tag is not set for buf2, the loop will > > end prematurely. Frag2 will be left uncleaned and next_to_clean will be > > left pointing to the end of packet, which will break the cleaning logic > > for subsequent cleans. This consequently leads to tx timeouts. > > > > Assign the empty bufs the same completion tag for the packet to ensure > > the cleaning routine iterates over all of the buffers associated with > > the packet. > > > > Fixes: d9028db618a6 ("idpf: convert to libeth Tx buffer completion") > > Signed-off-by: Joshua Hay <[email protected]> > > Acked-by: Alexander Lobakin <[email protected]> > > Reviewed-by: Madhu chittim <[email protected]> > > Thanks for the detailed description. > > Reviewed-by: Simon Horman <[email protected]>
Tested-by: Krishneil Singh <[email protected]>
