[PATCH net 0/2] ionic: Small fixes

[PATCH net 0/2] ionic: Small fixes

[Brett Creeley]

Fixes for the ionic driver related to the case where napi budget
is 0 and also a fix for possible tx_timeout.

Brett Creeley (2):
  ionic: Fix napi case where budget == 0
  ionic: Prevent tx_timeout due to frequent doorbell ringing

 drivers/net/ethernet/pensando/ionic/ionic_dev.h  | 2 +-
 drivers/net/ethernet/pensando/ionic/ionic_lif.c  | 2 +-
 drivers/net/ethernet/pensando/ionic/ionic_txrx.c | 3 ---
 3 files changed, 2 insertions(+), 5 deletions(-)

-- 
2.17.1

[PATCH net 1/2] ionic: Fix napi case where budget == 0

[Brett Creeley]

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(-)

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;
-- 
2.17.1

Re: [PATCH net 1/2] ionic: Fix napi case where budget == 0

[Joe Damato]

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>

Re: [PATCH net 1/2] ionic: Fix napi case where budget == 0

[Brett Creeley]

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

Re: [PATCH net 1/2] ionic: Fix napi case where budget == 0

[Jakub Kicinski]

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 :(
-- 
pw-bot: cr

Re: [PATCH net 1/2] ionic: Fix napi case where budget == 0

[Brett Creeley]

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

[PATCH net 2/2] ionic: Prevent tx_timeout due to frequent doorbell ringing

[Brett Creeley]

With recent work to the doorbell workaround code a small hole
was introduced that could cause a tx_timeout. This happens if
the rx dbell_deadline goes beyond the netdev watchdog timeout
set by the driver (i.e. 2 seconds). Fix this by changing the
netdev watchdog timeout to 5 seconds and reduce the max rx
dbell_deadline to 4 seconds.

The test that can reproduce the issue being fixed is a
multi-queue send test via pktgen with the "burst" setting to 1.
This causes the queue's doorbell to be rung on every packet
sent to the driver, which may result in the device missing
doorbells due to the high doorbell rate.

Fixes: 4ded136c78f8 ("ionic: add work item for missed-doorbell check")
Signed-off-by: Brett Creeley <brett.creeley@amd.com>
---
 drivers/net/ethernet/pensando/ionic/ionic_dev.h | 2 +-
 drivers/net/ethernet/pensando/ionic/ionic_lif.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.h b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
index c647033f3ad2..f2f07bf88545 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_dev.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
@@ -32,7 +32,7 @@
 #define IONIC_ADMIN_DOORBELL_DEADLINE	(HZ / 2)	/* 500ms */
 #define IONIC_TX_DOORBELL_DEADLINE	(HZ / 100)	/* 10ms */
 #define IONIC_RX_MIN_DOORBELL_DEADLINE	(HZ / 100)	/* 10ms */
-#define IONIC_RX_MAX_DOORBELL_DEADLINE	(HZ * 5)	/* 5s */
+#define IONIC_RX_MAX_DOORBELL_DEADLINE	(HZ * 4)	/* 4s */
 
 struct ionic_dev_bar {
 	void __iomem *vaddr;
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index aa0cc31dfe6e..86774d9922d8 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -3220,7 +3220,7 @@ int ionic_lif_alloc(struct ionic *ionic)
 	netdev->netdev_ops = &ionic_netdev_ops;
 	ionic_ethtool_set_ops(netdev);
 
-	netdev->watchdog_timeo = 2 * HZ;
+	netdev->watchdog_timeo = 5 * HZ;
 	netif_carrier_off(netdev);
 
 	lif->identity = lid;
-- 
2.17.1