patches for DPDK stable branches
 help / color / mirror / Atom feed
* [dpdk-stable] [19.11 1/2] net/bnxt: fix checking VNIC in shutdown path
@ 2020-10-29  8:44 Somnath Kotur
  2020-10-29  8:44 ` [dpdk-stable] [19.11 2/2] net/bnxt: add separate mutex for FW health check Somnath Kotur
  2020-10-29 10:36 ` [dpdk-stable] [19.11 1/2] net/bnxt: fix checking VNIC in shutdown path Luca Boccassi
  0 siblings, 2 replies; 4+ messages in thread
From: Somnath Kotur @ 2020-10-29  8:44 UTC (permalink / raw)
  To: stable; +Cc: luca.boccassi, Somnath Kotur, Ajit Khaparde

[ backported from upstream commit 4b029f02de3a0ce9cdd9a3475b84ca2e42d74281 ]

Add a couple of NULL pointer checks in bnxt_free_all_filters()
and bnxt_free_vnics() respectively to guard against certain error
injection/recovery scenarios where it was found that the application
was crashing with the bp->vnic_info pointer being NULL.

Fixes: 51fafb89a9a0 ("net/bnxt: get rid of ff pools and use VNIC info array")
Cc: stable@dpdk.org

Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_filter.c | 15 +++++++++------
 drivers/net/bnxt/bnxt_vnic.c   |  3 +++
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_filter.c b/drivers/net/bnxt/bnxt_filter.c
index 622a9bb41..f4b18d5b8 100644
--- a/drivers/net/bnxt/bnxt_filter.c
+++ b/drivers/net/bnxt/bnxt_filter.c
@@ -81,6 +81,15 @@ void bnxt_free_all_filters(struct bnxt *bp)
 	struct bnxt_filter_info *filter, *temp_filter;
 	unsigned int i;
 
+	for (i = 0; i < bp->pf.max_vfs; i++) {
+		STAILQ_FOREACH(filter, &bp->pf.vf_info[i].filter, next) {
+			bnxt_hwrm_clear_l2_filter(bp, filter);
+		}
+	}
+
+	if (bp->vnic_info == NULL)
+		return;
+
 	for (i = 0; i < bp->nr_vnics; i++) {
 		vnic = &bp->vnic_info[i];
 		filter = STAILQ_FIRST(&vnic->filter);
@@ -94,12 +103,6 @@ void bnxt_free_all_filters(struct bnxt *bp)
 		}
 		STAILQ_INIT(&vnic->filter);
 	}
-
-	for (i = 0; i < bp->pf.max_vfs; i++) {
-		STAILQ_FOREACH(filter, &bp->pf.vf_info[i].filter, next) {
-			bnxt_hwrm_clear_l2_filter(bp, filter);
-		}
-	}
 }
 
 void bnxt_free_filter_mem(struct bnxt *bp)
diff --git a/drivers/net/bnxt/bnxt_vnic.c b/drivers/net/bnxt/bnxt_vnic.c
index bc054a8e0..ef0772114 100644
--- a/drivers/net/bnxt/bnxt_vnic.c
+++ b/drivers/net/bnxt/bnxt_vnic.c
@@ -78,6 +78,9 @@ void bnxt_free_all_vnics(struct bnxt *bp)
 	struct bnxt_vnic_info *vnic;
 	unsigned int i;
 
+	if (bp->vnic_info == NULL)
+		return;
+
 	for (i = 0; i < bp->max_vnics; i++) {
 		vnic = &bp->vnic_info[i];
 		STAILQ_INSERT_TAIL(&bp->free_vnic_list, vnic, next);
-- 
2.28.0.497.g54e85e7


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [dpdk-stable] [19.11 2/2] net/bnxt: add separate mutex for FW health check
  2020-10-29  8:44 [dpdk-stable] [19.11 1/2] net/bnxt: fix checking VNIC in shutdown path Somnath Kotur
@ 2020-10-29  8:44 ` Somnath Kotur
  2020-10-29 10:36 ` [dpdk-stable] [19.11 1/2] net/bnxt: fix checking VNIC in shutdown path Luca Boccassi
  1 sibling, 0 replies; 4+ messages in thread
From: Somnath Kotur @ 2020-10-29  8:44 UTC (permalink / raw)
  To: stable
  Cc: luca.boccassi, Somnath Kotur, Sriharsha Basavapatna, Kalesh AP,
	Ajit Khaparde

[ backported from upstream commit 2993075dc253484ba3c87996981f32468aa86cbd ]

def_cp_lock was added to sync race between dev_configure and
int_handler. It should not be used to synchronize scheduling of FW
health check between dev_start and async event handler as well,
use a separate mutex for the same.

Fixes: a73b8e939f10 ("net/bnxt: fix race between start and interrupt handler")

Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com>
Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt.h        |  1 +
 drivers/net/bnxt/bnxt_ethdev.c | 17 +++++++++++++----
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
index 46cf41864..cdb3e2d3f 100644
--- a/drivers/net/bnxt/bnxt.h
+++ b/drivers/net/bnxt/bnxt.h
@@ -593,6 +593,7 @@ struct bnxt {
 	rte_iova_t			hwrm_short_cmd_req_dma_addr;
 	rte_spinlock_t			hwrm_lock;
 	pthread_mutex_t			def_cp_lock;
+	pthread_mutex_t			health_check_lock;
 	uint16_t			max_req_len;
 	uint16_t			max_resp_len;
 	uint16_t                        hwrm_max_ext_req_len;
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index fe240b6cc..d6afd03e5 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -885,9 +885,8 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev)
 	eth_dev->rx_pkt_burst = bnxt_receive_function(eth_dev);
 	eth_dev->tx_pkt_burst = bnxt_transmit_function(eth_dev);
 
-	pthread_mutex_lock(&bp->def_cp_lock);
 	bnxt_schedule_fw_health_check(bp);
-	pthread_mutex_unlock(&bp->def_cp_lock);
+
 	return 0;
 
 error:
@@ -4205,17 +4204,22 @@ void bnxt_schedule_fw_health_check(struct bnxt *bp)
 {
 	uint32_t polling_freq;
 
+	pthread_mutex_lock(&bp->health_check_lock);
+
 	if (!bnxt_is_recovery_enabled(bp))
-		return;
+		goto done;
 
 	if (bp->flags & BNXT_FLAG_FW_HEALTH_CHECK_SCHEDULED)
-		return;
+		goto done;
 
 	polling_freq = bp->recovery_info->driver_polling_freq;
 
 	rte_eal_alarm_set(US_PER_MS * polling_freq,
 			  bnxt_check_fw_health, (void *)bp);
 	bp->flags |= BNXT_FLAG_FW_HEALTH_CHECK_SCHEDULED;
+
+done:
+	pthread_mutex_unlock(&bp->health_check_lock);
 }
 
 static void bnxt_cancel_fw_health_check(struct bnxt *bp)
@@ -4747,6 +4751,10 @@ bnxt_init_locks(struct bnxt *bp)
 	err = pthread_mutex_init(&bp->def_cp_lock, NULL);
 	if (err)
 		PMD_DRV_LOG(ERR, "Unable to initialize def_cp_lock\n");
+
+	err = pthread_mutex_init(&bp->health_check_lock, NULL);
+	if (err)
+		PMD_DRV_LOG(ERR, "Unable to initialize health_check_lock\n");
 	return err;
 }
 
@@ -4888,6 +4896,7 @@ bnxt_uninit_locks(struct bnxt *bp)
 {
 	pthread_mutex_destroy(&bp->flow_lock);
 	pthread_mutex_destroy(&bp->def_cp_lock);
+	pthread_mutex_destroy(&bp->health_check_lock);
 }
 
 static int
-- 
2.28.0.497.g54e85e7


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-stable] [19.11 1/2] net/bnxt: fix checking VNIC in shutdown path
  2020-10-29  8:44 [dpdk-stable] [19.11 1/2] net/bnxt: fix checking VNIC in shutdown path Somnath Kotur
  2020-10-29  8:44 ` [dpdk-stable] [19.11 2/2] net/bnxt: add separate mutex for FW health check Somnath Kotur
@ 2020-10-29 10:36 ` Luca Boccassi
  2020-10-29 10:44   ` Somnath Kotur
  1 sibling, 1 reply; 4+ messages in thread
From: Luca Boccassi @ 2020-10-29 10:36 UTC (permalink / raw)
  To: Somnath Kotur, stable; +Cc: Ajit Khaparde

On Thu, 2020-10-29 at 14:14 +0530, Somnath Kotur wrote:
> [ backported from upstream commit 4b029f02de3a0ce9cdd9a3475b84ca2e42d74281 ]
> 
> Add a couple of NULL pointer checks in bnxt_free_all_filters()
> and bnxt_free_vnics() respectively to guard against certain error
> injection/recovery scenarios where it was found that the application
> was crashing with the bp->vnic_info pointer being NULL.
> 
> Fixes: 51fafb89a9a0 ("net/bnxt: get rid of ff pools and use VNIC info array")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
> Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> ---
>  drivers/net/bnxt/bnxt_filter.c | 15 +++++++++------
>  drivers/net/bnxt/bnxt_vnic.c   |  3 +++
>  2 files changed, 12 insertions(+), 6 deletions(-)

Hi,

Both of these patches appear to be 100% identical to the ones I pushed
yesterday? Was there something unclear in the generated email that made
you think the rebase was different that we can fix?

https://github.com/bluca/dpdk-stable/commit/77abebfe906e80404ecdfa71c60da318cd0fb563
https://github.com/bluca/dpdk-stable/commit/618bfe2417717a3b446180a10d1a2299d3f626e9

-- 
Kind regards,
Luca Boccassi

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-stable] [19.11 1/2] net/bnxt: fix checking VNIC in shutdown path
  2020-10-29 10:36 ` [dpdk-stable] [19.11 1/2] net/bnxt: fix checking VNIC in shutdown path Luca Boccassi
@ 2020-10-29 10:44   ` Somnath Kotur
  0 siblings, 0 replies; 4+ messages in thread
From: Somnath Kotur @ 2020-10-29 10:44 UTC (permalink / raw)
  To: Luca Boccassi; +Cc: dpdk stable, Ajit Khaparde

Oh ok... Apologies then Luca..We went by the final diff you had between the
upstream commit patch and the actual patch... perhaps that confused us or
threw us ( speaking for kalesh's 2 patches as well) off a bit ...
If they( all 4) are identical to what you already have on your sandbox,
then indeed we are good to go

Thanks a lot
Som




On Thu, 29 Oct 2020, 16:06 Luca Boccassi, <bluca@debian.org> wrote:

> On Thu, 2020-10-29 at 14:14 +0530, Somnath Kotur wrote:
> > [ backported from upstream commit
> 4b029f02de3a0ce9cdd9a3475b84ca2e42d74281 ]
> >
> > Add a couple of NULL pointer checks in bnxt_free_all_filters()
> > and bnxt_free_vnics() respectively to guard against certain error
> > injection/recovery scenarios where it was found that the application
> > was crashing with the bp->vnic_info pointer being NULL.
> >
> > Fixes: 51fafb89a9a0 ("net/bnxt: get rid of ff pools and use VNIC info
> array")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
> > Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> > ---
> >  drivers/net/bnxt/bnxt_filter.c | 15 +++++++++------
> >  drivers/net/bnxt/bnxt_vnic.c   |  3 +++
> >  2 files changed, 12 insertions(+), 6 deletions(-)
>
> Hi,
>
> Both of these patches appear to be 100% identical to the ones I pushed
> yesterday? Was there something unclear in the generated email that made
> you think the rebase was different that we can fix?
>
>
> https://github.com/bluca/dpdk-stable/commit/77abebfe906e80404ecdfa71c60da318cd0fb563
>
> https://github.com/bluca/dpdk-stable/commit/618bfe2417717a3b446180a10d1a2299d3f626e9
>
> --
> Kind regards,
> Luca Boccassi
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-10-29 10:45 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-29  8:44 [dpdk-stable] [19.11 1/2] net/bnxt: fix checking VNIC in shutdown path Somnath Kotur
2020-10-29  8:44 ` [dpdk-stable] [19.11 2/2] net/bnxt: add separate mutex for FW health check Somnath Kotur
2020-10-29 10:36 ` [dpdk-stable] [19.11 1/2] net/bnxt: fix checking VNIC in shutdown path Luca Boccassi
2020-10-29 10:44   ` Somnath Kotur

patches for DPDK stable branches

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/stable/0 stable/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 stable stable/ https://inbox.dpdk.org/stable \
		stable@dpdk.org
	public-inbox-index stable

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.stable


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git