Message ID | 20241007202435.664345-1-joshua.a.hay@intel.com |
---|---|
State | Under Review |
Delegated to: | Anthony Nguyen |
Headers | show |
Series | [iwl-net] idpf: set completion tag for "empty" bufs associated with a packet | expand |
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 <joshua.a.hay@intel.com> > Acked-by: Alexander Lobakin <aleksander.lobakin@intel.com> > Reviewed-by: Madhu chittim <madhu.chittim@intel.com> Thanks for the detailed description. Reviewed-by: Simon Horman <horms@kernel.org>
> -----Original Message----- > From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of > Simon Horman > Sent: Thursday, October 17, 2024 2:04 AM > To: Hay, Joshua A <joshua.a.hay@intel.com> > Cc: intel-wired-lan@lists.osuosl.org; Lobakin, Aleksander > <aleksander.lobakin@intel.com>; Chittim, Madhu > <madhu.chittim@intel.com>; netdev@vger.kernel.org > 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 <joshua.a.hay@intel.com> > > Acked-by: Alexander Lobakin <aleksander.lobakin@intel.com> > > Reviewed-by: Madhu chittim <madhu.chittim@intel.com> > > Thanks for the detailed description. > > Reviewed-by: Simon Horman <horms@kernel.org> Tested-by: Krishneil Singh <krishneil.k.singh@intel.com>
diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.c b/drivers/net/ethernet/intel/idpf/idpf_txrx.c index d4e6f0e10487..60d15b3e6e2f 100644 --- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c +++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c @@ -2448,6 +2448,7 @@ static void idpf_tx_splitq_map(struct idpf_tx_queue *tx_q, * rest of the packet. */ tx_buf->type = LIBETH_SQE_EMPTY; + idpf_tx_buf_compl_tag(tx_buf) = params->compl_tag; /* Adjust the DMA offset and the remaining size of the * fragment. On the first iteration of this loop,