| Message ID | 20240813234122.53083-2-brett.creeley@amd.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | ionic: Small fixes | expand |
On Tue, Aug 13, 2024 at 04:41:21PM -0700, Brett Creeley wrote: > The change in the fixes allowed the ionic_tx_cq_service() function > to be called when budget == 0, but no packet completions will > actually be serviced since it returns immediately when budget is > passed in as 0. Fix this by not checking budget before entering > the completion servicing while loop. This will allow a single > cq entry to be processed since ++work_done will always be greater > than work_to_do. > > With this change a simple netconsole test as described in > Documentation/networking/netconsole.txt works as expected. > > Fixes: 2f74258d997c ("ionic: minimal work with 0 budget") > Signed-off-by: Brett Creeley <brett.creeley@amd.com> > --- > drivers/net/ethernet/pensando/ionic/ionic_txrx.c | 3 --- > 1 file changed, 3 deletions(-) I think fixes may need to CC stable, but in either case: Reviewed-by: Joe Damato <jdamato@fastly.com>
On 8/14/2024 4:33 AM, Joe Damato wrote: > Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. > > > On Tue, Aug 13, 2024 at 04:41:21PM -0700, Brett Creeley wrote: >> The change in the fixes allowed the ionic_tx_cq_service() function >> to be called when budget == 0, but no packet completions will >> actually be serviced since it returns immediately when budget is >> passed in as 0. Fix this by not checking budget before entering >> the completion servicing while loop. This will allow a single >> cq entry to be processed since ++work_done will always be greater >> than work_to_do. >> >> With this change a simple netconsole test as described in >> Documentation/networking/netconsole.txt works as expected. >> >> Fixes: 2f74258d997c ("ionic: minimal work with 0 budget") >> Signed-off-by: Brett Creeley <brett.creeley@amd.com> >> --- >> drivers/net/ethernet/pensando/ionic/ionic_txrx.c | 3 --- >> 1 file changed, 3 deletions(-) > > I think fixes may need to CC stable, but in either case: > > Reviewed-by: Joe Damato <jdamato@fastly.com> Thanks for the reminder and review. I will CC stable if I end up needing to re-push a v2. Brett
On Tue, 13 Aug 2024 16:41:21 -0700 Brett Creeley wrote: > The change in the fixes allowed the ionic_tx_cq_service() function > to be called when budget == 0, but no packet completions will > actually be serviced since it returns immediately when budget is > passed in as 0. Fix this by not checking budget before entering > the completion servicing while loop. This will allow a single > cq entry to be processed since ++work_done will always be greater > than work_to_do. > > With this change a simple netconsole test as described in > Documentation/networking/netconsole.txt works as expected. I think I see a call to XDP cleanup as part of Tx processing. XDP can't be handled with budget of 0 :(
On 8/14/2024 5:27 PM, Jakub Kicinski wrote: > Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. > > > On Tue, 13 Aug 2024 16:41:21 -0700 Brett Creeley wrote: >> The change in the fixes allowed the ionic_tx_cq_service() function >> to be called when budget == 0, but no packet completions will >> actually be serviced since it returns immediately when budget is >> passed in as 0. Fix this by not checking budget before entering >> the completion servicing while loop. This will allow a single >> cq entry to be processed since ++work_done will always be greater >> than work_to_do. >> >> With this change a simple netconsole test as described in >> Documentation/networking/netconsole.txt works as expected. > > I think I see a call to XDP cleanup as part of Tx processing. > XDP can't be handled with budget of 0 :( Well that's unfortunate to say the least. I will look at how to fix this as well and send a v2. Thanks, Brett > -- > pw-bot: cr
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c index fc79baad4561..8557d672d269 100644 --- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c +++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c @@ -1261,9 +1261,6 @@ unsigned int ionic_tx_cq_service(struct ionic_cq *cq, unsigned int bytes = 0; unsigned int pkts = 0; - if (work_to_do == 0) - return 0; - while (ionic_tx_service(cq, &pkts, &bytes, in_napi)) { if (cq->tail_idx == cq->num_descs - 1) cq->done_color = !cq->done_color;
The change in the fixes allowed the ionic_tx_cq_service() function to be called when budget == 0, but no packet completions will actually be serviced since it returns immediately when budget is passed in as 0. Fix this by not checking budget before entering the completion servicing while loop. This will allow a single cq entry to be processed since ++work_done will always be greater than work_to_do. With this change a simple netconsole test as described in Documentation/networking/netconsole.txt works as expected. Fixes: 2f74258d997c ("ionic: minimal work with 0 budget") Signed-off-by: Brett Creeley <brett.creeley@amd.com> --- drivers/net/ethernet/pensando/ionic/ionic_txrx.c | 3 --- 1 file changed, 3 deletions(-)