* [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).