* [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
* 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
* [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
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.