* [PATCH net 0/2] qede: fix ethernet self adapter and skb failure issue
@ 2021-03-16 18:34 Bhaskar Upadhaya
2021-03-16 18:34 ` [PATCH net 1/2] qede: fix to disable start_xmit functionality during self adapter test Bhaskar Upadhaya
2021-03-16 18:34 ` [PATCH net 2/2] qede: fix memory allocation failures under heavy load Bhaskar Upadhaya
0 siblings, 2 replies; 6+ messages in thread
From: Bhaskar Upadhaya @ 2021-03-16 18:34 UTC (permalink / raw)
To: netdev, kuba, aelior, irusskikh; +Cc: davem, bupadhaya
Patch 1: Fix ethernet self adapter test issue by preventing start_xmit
function to be called.start_xmit function should not be called
during the execution of self adapter test, netif_tx_disable()
ensures this.
Patch 2: Fix to return proper error code when sdk buffer allocation fails.
Bhaskar Upadhaya (2):
qede: fix to disable start_xmit functionality during self adapter test
qede: fix memory allocation failures under heavy load
.../net/ethernet/qlogic/qede/qede_ethtool.c | 4 +++-
drivers/net/ethernet/qlogic/qede/qede_fp.c | 19 +++++++++++++------
2 files changed, 16 insertions(+), 7 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net 1/2] qede: fix to disable start_xmit functionality during self adapter test
2021-03-16 18:34 [PATCH net 0/2] qede: fix ethernet self adapter and skb failure issue Bhaskar Upadhaya
@ 2021-03-16 18:34 ` Bhaskar Upadhaya
2021-03-16 21:59 ` Jakub Kicinski
2021-03-16 18:34 ` [PATCH net 2/2] qede: fix memory allocation failures under heavy load Bhaskar Upadhaya
1 sibling, 1 reply; 6+ messages in thread
From: Bhaskar Upadhaya @ 2021-03-16 18:34 UTC (permalink / raw)
To: netdev, kuba, aelior, irusskikh; +Cc: davem, bupadhaya
start_xmit function should not be called during the execution of self
adapter test, netif_tx_disable() gives this guarantee, since it takes
the transmit queue lock while marking the queue stopped. This will
wait for the transmit function to complete before returning.
Fixes: 16f46bf054f8 ("qede: add implementation for internal loopback test.")
Signed-off-by: Bhaskar Upadhaya <bupadhaya@marvell.com>
Signed-off-by: Igor Russkikh <irusskikh@marvell.com>
Signed-off-by: Ariel Elior <aelior@marvell.com>
---
drivers/net/ethernet/qlogic/qede/qede_ethtool.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
index 1560ad3d9290..f9702cc7bc55 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
@@ -1611,7 +1611,7 @@ static int qede_selftest_run_loopback(struct qede_dev *edev, u32 loopback_mode)
return -EINVAL;
}
- qede_netif_stop(edev);
+ netif_tx_disable(edev->ndev);
/* Bring up the link in Loopback mode */
memset(&link_params, 0, sizeof(link_params));
@@ -1623,6 +1623,8 @@ static int qede_selftest_run_loopback(struct qede_dev *edev, u32 loopback_mode)
/* Wait for loopback configuration to apply */
msleep_interruptible(500);
+ qede_netif_stop(edev);
+
/* Setting max packet size to 1.5K to avoid data being split over
* multiple BDs in cases where MTU > PAGE_SIZE.
*/
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net 2/2] qede: fix memory allocation failures under heavy load
2021-03-16 18:34 [PATCH net 0/2] qede: fix ethernet self adapter and skb failure issue Bhaskar Upadhaya
2021-03-16 18:34 ` [PATCH net 1/2] qede: fix to disable start_xmit functionality during self adapter test Bhaskar Upadhaya
@ 2021-03-16 18:34 ` Bhaskar Upadhaya
1 sibling, 0 replies; 6+ messages in thread
From: Bhaskar Upadhaya @ 2021-03-16 18:34 UTC (permalink / raw)
To: netdev, kuba, aelior, irusskikh; +Cc: davem, bupadhaya
when the system is heavily stressed, kernel get lots of memory
allocation failures which causes kernel panic, so enable proper
error handling during skb allocation
Fixes: 8a8633978b84 ("qede: Add build_skb() support.")
Signed-off-by: Bhaskar Upadhaya <bupadhaya@marvell.com>
Signed-off-by: Igor Russkikh <irusskikh@marvell.com>
Signed-off-by: Ariel Elior <aelior@marvell.com>
---
drivers/net/ethernet/qlogic/qede/qede_fp.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qede/qede_fp.c b/drivers/net/ethernet/qlogic/qede/qede_fp.c
index 8c47a9d2a965..f2d74b53e421 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_fp.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_fp.c
@@ -751,6 +751,8 @@ qede_build_skb(struct qede_rx_queue *rxq,
buf = page_address(bd->data) + bd->page_offset;
skb = build_skb(buf, rxq->rx_buf_seg_size);
+ if (unlikely(!skb))
+ return NULL;
skb_reserve(skb, pad);
skb_put(skb, len);
@@ -767,6 +769,9 @@ qede_tpa_rx_build_skb(struct qede_dev *edev,
struct sk_buff *skb;
skb = qede_build_skb(rxq, bd, len, pad);
+ if (unlikely(!skb))
+ return NULL;
+
bd->page_offset += rxq->rx_buf_seg_size;
if (bd->page_offset == PAGE_SIZE) {
@@ -814,6 +819,8 @@ qede_rx_build_skb(struct qede_dev *edev,
}
skb = qede_build_skb(rxq, bd, len, pad);
+ if (unlikely(!skb))
+ return NULL;
if (unlikely(qede_realloc_rx_buffer(rxq, bd))) {
/* Incr page ref count to reuse on allocation failure so
@@ -851,11 +858,16 @@ static void qede_tpa_start(struct qede_dev *edev,
if (unlikely(!tpa_info->skb)) {
DP_NOTICE(edev, "Failed to allocate SKB for gro\n");
+ /* Re-use the buffer instantly instead doing it at tpa_end
+ * as we are already going to throw away this aggregated packet
+ * (i.e CQEs till tpa_end) and then going to update the
+ * producer, so it's safe to pin the buffer here only.
+ */
+ qede_reuse_page(rxq, sw_rx_data_cons);
/* Consume from ring but do not produce since
* this might be used by FW still, it will be re-used
* at TPA end.
*/
- tpa_info->tpa_start_fail = true;
qede_rx_bd_ring_consume(rxq);
tpa_info->state = QEDE_AGG_STATE_ERROR;
goto cons_buf;
@@ -1025,11 +1037,6 @@ static int qede_tpa_end(struct qede_dev *edev,
err:
tpa_info->state = QEDE_AGG_STATE_NONE;
- if (tpa_info->tpa_start_fail) {
- qede_reuse_page(rxq, &tpa_info->buffer);
- tpa_info->tpa_start_fail = false;
- }
-
dev_kfree_skb_any(tpa_info->skb);
tpa_info->skb = NULL;
return 0;
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net 1/2] qede: fix to disable start_xmit functionality during self adapter test
2021-03-16 18:34 ` [PATCH net 1/2] qede: fix to disable start_xmit functionality during self adapter test Bhaskar Upadhaya
@ 2021-03-16 21:59 ` Jakub Kicinski
2021-03-17 6:33 ` [EXT] " Bhaskar Upadhaya
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2021-03-16 21:59 UTC (permalink / raw)
To: Bhaskar Upadhaya; +Cc: netdev, aelior, irusskikh, davem
On Tue, 16 Mar 2021 11:34:09 -0700 Bhaskar Upadhaya wrote:
> start_xmit function should not be called during the execution of self
> adapter test, netif_tx_disable() gives this guarantee, since it takes
> the transmit queue lock while marking the queue stopped. This will
> wait for the transmit function to complete before returning.
>
> Fixes: 16f46bf054f8 ("qede: add implementation for internal loopback test.")
> Signed-off-by: Bhaskar Upadhaya <bupadhaya@marvell.com>
> Signed-off-by: Igor Russkikh <irusskikh@marvell.com>
> Signed-off-by: Ariel Elior <aelior@marvell.com>
> ---
> drivers/net/ethernet/qlogic/qede/qede_ethtool.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
> index 1560ad3d9290..f9702cc7bc55 100644
> --- a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
> +++ b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
> @@ -1611,7 +1611,7 @@ static int qede_selftest_run_loopback(struct qede_dev *edev, u32 loopback_mode)
> return -EINVAL;
> }
>
> - qede_netif_stop(edev);
> + netif_tx_disable(edev->ndev);
But an interrupt can come in after and enable Tx again.
I think you should keep the qede_netif_stop() here instead of moving
it down, no?
> /* Bring up the link in Loopback mode */
> memset(&link_params, 0, sizeof(link_params));
> @@ -1623,6 +1623,8 @@ static int qede_selftest_run_loopback(struct qede_dev *edev, u32 loopback_mode)
> /* Wait for loopback configuration to apply */
> msleep_interruptible(500);
>
> + qede_netif_stop(edev);
> +
> /* Setting max packet size to 1.5K to avoid data being split over
> * multiple BDs in cases where MTU > PAGE_SIZE.
> */
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [EXT] Re: [PATCH net 1/2] qede: fix to disable start_xmit functionality during self adapter test
2021-03-16 21:59 ` Jakub Kicinski
@ 2021-03-17 6:33 ` Bhaskar Upadhaya
2021-03-17 18:40 ` Jakub Kicinski
0 siblings, 1 reply; 6+ messages in thread
From: Bhaskar Upadhaya @ 2021-03-17 6:33 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, Ariel Elior, Igor Russkikh, davem
> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Wednesday, March 17, 2021 3:30 AM
> To: Bhaskar Upadhaya <bupadhaya@marvell.com>
> Cc: netdev@vger.kernel.org; Ariel Elior <aelior@marvell.com>; Igor Russkikh
> <irusskikh@marvell.com>; davem@davemloft.net
> Subject: [EXT] Re: [PATCH net 1/2] qede: fix to disable start_xmit functionality
> during self adapter test
>
> External Email
>
> ----------------------------------------------------------------------
> On Tue, 16 Mar 2021 11:34:09 -0700 Bhaskar Upadhaya wrote:
> > start_xmit function should not be called during the execution of self
> > adapter test, netif_tx_disable() gives this guarantee, since it takes
> > the transmit queue lock while marking the queue stopped. This will
> > wait for the transmit function to complete before returning.
> >
> > Fixes: 16f46bf054f8 ("qede: add implementation for internal loopback
> > test.")
> > Signed-off-by: Bhaskar Upadhaya <bupadhaya@marvell.com>
> > Signed-off-by: Igor Russkikh <irusskikh@marvell.com>
> > Signed-off-by: Ariel Elior <aelior@marvell.com>
> > ---
> > drivers/net/ethernet/qlogic/qede/qede_ethtool.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
> > b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
> > index 1560ad3d9290..f9702cc7bc55 100644
> > --- a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
> > +++ b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
> > @@ -1611,7 +1611,7 @@ static int qede_selftest_run_loopback(struct
> qede_dev *edev, u32 loopback_mode)
> > return -EINVAL;
> > }
> >
> > - qede_netif_stop(edev);
> > + netif_tx_disable(edev->ndev);
>
> But an interrupt can come in after and enable Tx again.
> I think you should keep the qede_netif_stop() here instead of moving it
> down, no?
Hi Jakub,
Normal Traffic flow is enabled by qede_netif_start(edev) and which is placed at the end of this qede_selftest_run_loopback()
qede_netif_stop(edev) is called prior to the call to qede_netif_start(edev), so unless qede_netif_start(edev) is called Normal traffic flow will not
be operational.
>
> > /* Bring up the link in Loopback mode */
> > memset(&link_params, 0, sizeof(link_params)); @@ -1623,6 +1623,8
> @@
> > static int qede_selftest_run_loopback(struct qede_dev *edev, u32
> loopback_mode)
> > /* Wait for loopback configuration to apply */
> > msleep_interruptible(500);
> >
> > + qede_netif_stop(edev);
> > +
> > /* Setting max packet size to 1.5K to avoid data being split over
> > * multiple BDs in cases where MTU > PAGE_SIZE.
> > */
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [EXT] Re: [PATCH net 1/2] qede: fix to disable start_xmit functionality during self adapter test
2021-03-17 6:33 ` [EXT] " Bhaskar Upadhaya
@ 2021-03-17 18:40 ` Jakub Kicinski
0 siblings, 0 replies; 6+ messages in thread
From: Jakub Kicinski @ 2021-03-17 18:40 UTC (permalink / raw)
To: Bhaskar Upadhaya; +Cc: netdev, Ariel Elior, Igor Russkikh, davem
On Wed, 17 Mar 2021 06:33:37 +0000 Bhaskar Upadhaya wrote:
> > But an interrupt can come in after and enable Tx again.
> > I think you should keep the qede_netif_stop() here instead of moving it
> > down, no?
>
> Hi Jakub,
> Normal Traffic flow is enabled by qede_netif_start(edev) and which is placed at the end of this qede_selftest_run_loopback()
> qede_netif_stop(edev) is called prior to the call to qede_netif_start(edev), so unless qede_netif_start(edev) is called Normal traffic flow will not
> be operational.
I'm not talking about submitting more traffic.
Consider the following order of events
normal traffic test
xmit()
netif_tx_disable()
IRQ
NAPI
netif_tx_wake_queue()
<--- traffic running again --->
qede_netif_stop()
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-03-17 18:41 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-16 18:34 [PATCH net 0/2] qede: fix ethernet self adapter and skb failure issue Bhaskar Upadhaya
2021-03-16 18:34 ` [PATCH net 1/2] qede: fix to disable start_xmit functionality during self adapter test Bhaskar Upadhaya
2021-03-16 21:59 ` Jakub Kicinski
2021-03-17 6:33 ` [EXT] " Bhaskar Upadhaya
2021-03-17 18:40 ` Jakub Kicinski
2021-03-16 18:34 ` [PATCH net 2/2] qede: fix memory allocation failures under heavy load Bhaskar Upadhaya
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.