[dpdk-dev] [PATCH 1/2] bnx2x: fix error handling in bnx2x_loop_obtain_resources()

[dpdk-dev] [PATCH 1/2] bnx2x: fix error handling in bnx2x_loop_obtain_resources()

[Chas Williams]

From: "Charles (Chas) Williams" <ciwillia@brocade.com>

bnx2x_loop_obtain_resources() returns a struct containing the status and
the error message.  If bnx2x_do_req4pf() fails, it shouldn't return both
of these fields set to 0 indicating failure and no error.

Further, bnx2x_do_req4pf() needs to be able fail and return NO_RESOURCES
so that bnx2x_loop_obtain_resources() can negotiate reduced resource
requirments.  This requires additional checking around bnx2x_do_req4pf().

Signed-off-by: Chas Williams <3chas3@gmail.com>
---
 drivers/net/bnx2x/bnx2x_vfpf.c | 75 +++++++++++++++++++-----------------------
 1 file changed, 33 insertions(+), 42 deletions(-)

diff --git a/drivers/net/bnx2x/bnx2x_vfpf.c b/drivers/net/bnx2x/bnx2x_vfpf.c
index 765cc92..34b6360 100644
--- a/drivers/net/bnx2x/bnx2x_vfpf.c
+++ b/drivers/net/bnx2x/bnx2x_vfpf.c
@@ -122,16 +122,10 @@ bnx2x_do_req4pf(struct bnx2x_softc *sc, phys_addr_t phys_addr)
 				break;
 		}
 
-		if (i == BNX2X_VF_CHANNEL_TRIES) {
+		if (!*status) {
 			PMD_DRV_LOG(ERR, "Response from PF timed out");
 			return -EAGAIN;
 		}
-
-		if (BNX2X_VF_STATUS_SUCCESS != *status) {
-			PMD_DRV_LOG(ERR, "Bad reply from PF : %u",
-					*status);
-			return -EINVAL;
-		}
 	} else {
 		PMD_DRV_LOG(ERR, "status should be zero before message"
 				"to pf was sent");
@@ -193,10 +187,10 @@ struct bnx2x_obtain_status bnx2x_loop_obtain_resources(struct bnx2x_softc *sc)
 	do {
 		PMD_DRV_LOG(DEBUG, "trying to get resources");
 
-		if ( bnx2x_do_req4pf(sc, sc->vf2pf_mbox_mapping.paddr) ) {
+		if (bnx2x_do_req4pf(sc, sc->vf2pf_mbox_mapping.paddr)) {
 			/* timeout */
 			status.success = 0;
-			status.err_code = 0;
+			status.err_code = -EAGAIN;
 			return status;
 		}
 
@@ -310,8 +304,8 @@ void
 bnx2x_vf_close(struct bnx2x_softc *sc)
 {
 	struct vf_release_tlv *query;
+	struct vf_common_reply_tlv *reply = &sc->vf2pf_mbox->resp.common_reply;
 	int vf_id = bnx2x_read_vf_id(sc);
-	int ret;
 
 	if (vf_id >= 0) {
 		query = &sc->vf2pf_mbox->query[0].release;
@@ -322,11 +316,9 @@ bnx2x_vf_close(struct bnx2x_softc *sc)
 		BNX2X_TLV_APPEND(query, query->first_tlv.length, BNX2X_VF_TLV_LIST_END,
 				sizeof(struct channel_list_end_tlv));
 
-		ret = bnx2x_do_req4pf(sc, sc->vf2pf_mbox_mapping.paddr);
-
-		if (ret) {
+		bnx2x_do_req4pf(sc, sc->vf2pf_mbox_mapping.paddr);
+		if (reply->status != BNX2X_VF_STATUS_SUCCESS)
 			PMD_DRV_LOG(ERR, "Failed to release VF");
-		}
 	}
 }
 
@@ -335,7 +327,8 @@ int
 bnx2x_vf_init(struct bnx2x_softc *sc)
 {
 	struct vf_init_tlv *query;
-	int i, ret;
+	struct vf_common_reply_tlv *reply = &sc->vf2pf_mbox->resp.common_reply;
+	int i;
 
 	query = &sc->vf2pf_mbox->query[0].init;
 	bnx2x_init_first_tlv(sc, &query->first_tlv, BNX2X_VF_TLV_INIT,
@@ -352,11 +345,10 @@ bnx2x_vf_init(struct bnx2x_softc *sc)
 	BNX2X_TLV_APPEND(query, query->first_tlv.length, BNX2X_VF_TLV_LIST_END,
 			sizeof(struct channel_list_end_tlv));
 
-	ret = bnx2x_do_req4pf(sc, sc->vf2pf_mbox_mapping.paddr);
-
-	if (ret) {
+	bnx2x_do_req4pf(sc, sc->vf2pf_mbox_mapping.paddr);
+	if (reply->status != BNX2X_VF_STATUS_SUCCESS) {
 		PMD_DRV_LOG(ERR, "Failed to init VF");
-		return ret;
+		return -EINVAL;
 	}
 
 	PMD_DRV_LOG(DEBUG, "VF was initialized");
@@ -367,8 +359,9 @@ void
 bnx2x_vf_unload(struct bnx2x_softc *sc)
 {
 	struct vf_close_tlv *query;
+	struct vf_common_reply_tlv *reply = &sc->vf2pf_mbox->resp.common_reply;
 	struct vf_q_op_tlv *query_op;
-	int i, vf_id, ret;
+	int i, vf_id;
 
 	vf_id = bnx2x_read_vf_id(sc);
 	if (vf_id > 0) {
@@ -384,10 +377,10 @@ bnx2x_vf_unload(struct bnx2x_softc *sc)
 					BNX2X_VF_TLV_LIST_END,
 					sizeof(struct channel_list_end_tlv));
 
-			ret = bnx2x_do_req4pf(sc, sc->vf2pf_mbox_mapping.paddr);
-			if (ret)
+			bnx2x_do_req4pf(sc, sc->vf2pf_mbox_mapping.paddr);
+			if (reply->status != BNX2X_VF_STATUS_SUCCESS)
 				PMD_DRV_LOG(ERR,
-						"Bad reply for vf_q %d teardown", i);
+					    "Bad reply for vf_q %d teardown", i);
 		}
 
 		bnx2x_vf_set_mac(sc, false);
@@ -402,11 +395,10 @@ bnx2x_vf_unload(struct bnx2x_softc *sc)
 				BNX2X_VF_TLV_LIST_END,
 				sizeof(struct channel_list_end_tlv));
 
-		ret = bnx2x_do_req4pf(sc, sc->vf2pf_mbox_mapping.paddr);
-
-		if (ret)
+		bnx2x_do_req4pf(sc, sc->vf2pf_mbox_mapping.paddr);
+		if (reply->status != BNX2X_VF_STATUS_SUCCESS)
 			PMD_DRV_LOG(ERR,
-				"Bad reply from PF for close message");
+				    "Bad reply from PF for close message");
 	}
 }
 
@@ -469,8 +461,8 @@ int
 bnx2x_vf_setup_queue(struct bnx2x_softc *sc, struct bnx2x_fastpath *fp, int leading)
 {
 	struct vf_setup_q_tlv *query;
+	struct vf_common_reply_tlv *reply = &sc->vf2pf_mbox->resp.common_reply;
 	uint16_t flags = bnx2x_vf_q_flags(leading);
-	int ret;
 
 	query = &sc->vf2pf_mbox->query[0].setup_q;
 	bnx2x_init_first_tlv(sc, &query->first_tlv, BNX2X_VF_TLV_SETUP_Q,
@@ -485,11 +477,10 @@ bnx2x_vf_setup_queue(struct bnx2x_softc *sc, struct bnx2x_fastpath *fp, int lead
 	BNX2X_TLV_APPEND(query, query->first_tlv.length, BNX2X_VF_TLV_LIST_END,
 			sizeof(struct channel_list_end_tlv));
 
-	ret = bnx2x_do_req4pf(sc, sc->vf2pf_mbox_mapping.paddr);
-
-	if (ret) {
+	bnx2x_do_req4pf(sc, sc->vf2pf_mbox_mapping.paddr);
+	if (reply->status != BNX2X_VF_STATUS_SUCCESS) {
 		PMD_DRV_LOG(ERR, "Failed to setup VF queue[%d]",
-				fp->index);
+				 fp->index);
 		return -EINVAL;
 	}
 
@@ -548,7 +539,7 @@ bnx2x_vf_config_rss(struct bnx2x_softc *sc,
 			  struct ecore_config_rss_params *params)
 {
 	struct vf_rss_tlv *query;
-	int ret;
+	struct vf_common_reply_tlv *reply = &sc->vf2pf_mbox->resp.common_reply;
 
 	query = &sc->vf2pf_mbox->query[0].update_rss;
 
@@ -568,10 +559,10 @@ bnx2x_vf_config_rss(struct bnx2x_softc *sc,
 	query->rss_result_mask = params->rss_result_mask;
 	query->rss_flags = params->rss_flags;
 
-	ret = bnx2x_do_req4pf(sc, sc->vf2pf_mbox_mapping.paddr);
-	if (ret) {
-		PMD_DRV_LOG(ERR, "Failed to send message to PF, rc %d", ret);
-		return ret;
+	bnx2x_do_req4pf(sc, sc->vf2pf_mbox_mapping.paddr);
+	if (reply->status != BNX2X_VF_STATUS_SUCCESS) {
+		PMD_DRV_LOG(ERR, "Failed to configure RSS");
+		return -EINVAL;;
 	}
 
 	return 0;
@@ -581,8 +572,8 @@ int
 bnx2x_vf_set_rx_mode(struct bnx2x_softc *sc)
 {
 	struct vf_set_q_filters_tlv *query;
+	struct vf_common_reply_tlv *reply = &sc->vf2pf_mbox->resp.common_reply;
 	unsigned long tx_mask;
-	int ret;
 
 	query = &sc->vf2pf_mbox->query[0].set_q_filters;
 	bnx2x_init_first_tlv(sc, &query->first_tlv, BNX2X_VF_TLV_SET_Q_FILTERS,
@@ -598,10 +589,10 @@ bnx2x_vf_set_rx_mode(struct bnx2x_softc *sc)
 	BNX2X_TLV_APPEND(query, query->first_tlv.length, BNX2X_VF_TLV_LIST_END,
 			sizeof(struct channel_list_end_tlv));
 
-	ret = bnx2x_do_req4pf(sc, sc->vf2pf_mbox_mapping.paddr);
-	if (ret) {
-		PMD_DRV_LOG(ERR, "Failed to send message to PF, rc %d", ret);
-		return ret;
+	bnx2x_do_req4pf(sc, sc->vf2pf_mbox_mapping.paddr);
+	if (reply->status != BNX2X_VF_STATUS_SUCCESS) {
+		PMD_DRV_LOG(ERR, "Failed to set RX mode");
+		return -EINVAL;
 	}
 
 	return 0;
-- 
2.5.0

[dpdk-dev] [PATCH 2/2] bnx2x: Determine rx/tx queue sizes sooner

[Chas Williams]

From: "Charles (Chas) Williams" <ciwillia@brocade.com>

The VF needs to determine the queues sizes before .dev_infos_get
so that it can hint to the upper layer the proper sizes.  Move
bnx2x_vf_get_resources() to .eth_dev_init and probe with the guesses
from bnx2x_init_rte().

Signed-off-by: Chas Williams <3chas3@gmail.com>
---
 drivers/net/bnx2x/bnx2x.c        |  9 ++++++--
 drivers/net/bnx2x/bnx2x_ethdev.c | 46 ++++++++++++++++++++++------------------
 drivers/net/bnx2x/bnx2x_vfpf.c   |  2 ++
 3 files changed, 34 insertions(+), 23 deletions(-)

diff --git a/drivers/net/bnx2x/bnx2x.c b/drivers/net/bnx2x/bnx2x.c
index 67af5da..f003875 100644
--- a/drivers/net/bnx2x/bnx2x.c
+++ b/drivers/net/bnx2x/bnx2x.c
@@ -9578,8 +9578,13 @@ static int bnx2x_pci_get_caps(struct bnx2x_softc *sc)
 
 static void bnx2x_init_rte(struct bnx2x_softc *sc)
 {
-	sc->max_tx_queues = 128;
-	sc->max_rx_queues = 128;
+	if (IS_VF(sc)) {
+		sc->max_tx_queues = BNX2X_VF_MAX_QUEUES_PER_VF;
+		sc->max_rx_queues = BNX2X_VF_MAX_QUEUES_PER_VF;
+	} else {
+		sc->max_tx_queues = 128;
+		sc->max_rx_queues = 128;
+	}
 }
 
 #define FW_HEADER_LEN 104
diff --git a/drivers/net/bnx2x/bnx2x_ethdev.c b/drivers/net/bnx2x/bnx2x_ethdev.c
index 69df02e..fe8cfd0 100644
--- a/drivers/net/bnx2x/bnx2x_ethdev.c
+++ b/drivers/net/bnx2x/bnx2x_ethdev.c
@@ -87,7 +87,6 @@ bnx2x_dev_configure(struct rte_eth_dev *dev)
 {
 	struct bnx2x_softc *sc = dev->data->dev_private;
 	int mp_ncpus = sysconf(_SC_NPROCESSORS_CONF);
-	int ret;
 
 	PMD_INIT_FUNC_TRACE();
 
@@ -121,25 +120,6 @@ bnx2x_dev_configure(struct rte_eth_dev *dev)
 		return -ENXIO;
 	}
 
-	if (IS_VF(sc)) {
-		if (bnx2x_dma_alloc(sc, sizeof(struct bnx2x_vf_mbx_msg),
-				  &sc->vf2pf_mbox_mapping, "vf2pf_mbox",
-				  RTE_CACHE_LINE_SIZE) != 0)
-			return -ENOMEM;
-
-		sc->vf2pf_mbox = (struct bnx2x_vf_mbx_msg *)sc->vf2pf_mbox_mapping.vaddr;
-		if (bnx2x_dma_alloc(sc, sizeof(struct bnx2x_vf_bulletin),
-				  &sc->pf2vf_bulletin_mapping, "vf2pf_bull",
-				  RTE_CACHE_LINE_SIZE) != 0)
-			return -ENOMEM;
-
-		sc->pf2vf_bulletin = (struct bnx2x_vf_bulletin *)sc->pf2vf_bulletin_mapping.vaddr;
-
-		ret = bnx2x_vf_get_resources(sc, sc->num_queues, sc->num_queues);
-		if (ret)
-			return ret;
-	}
-
 	return 0;
 }
 
@@ -466,6 +446,7 @@ bnx2x_common_dev_init(struct rte_eth_dev *eth_dev, int is_vf)
 	ret = bnx2x_attach(sc);
 	if (ret) {
 		PMD_DRV_LOG(ERR, "bnx2x_attach failed (%d)", ret);
+		return ret;
 	}
 
 	eth_dev->data->mac_addrs = (struct ether_addr *)sc->link_params.mac_addr;
@@ -479,7 +460,30 @@ bnx2x_common_dev_init(struct rte_eth_dev *eth_dev, int is_vf)
 	PMD_DRV_LOG(INFO, "portID=%d vendorID=0x%x deviceID=0x%x",
 			eth_dev->data->port_id, pci_dev->id.vendor_id, pci_dev->id.device_id);
 
-	return ret;
+	if (IS_VF(sc)) {
+		if (bnx2x_dma_alloc(sc, sizeof(struct bnx2x_vf_mbx_msg),
+				    &sc->vf2pf_mbox_mapping, "vf2pf_mbox",
+				    RTE_CACHE_LINE_SIZE) != 0)
+			return -ENOMEM;
+
+		sc->vf2pf_mbox = (struct bnx2x_vf_mbx_msg *)
+					 sc->vf2pf_mbox_mapping.vaddr;
+
+		if (bnx2x_dma_alloc(sc, sizeof(struct bnx2x_vf_bulletin),
+				    &sc->pf2vf_bulletin_mapping, "vf2pf_bull",
+				    RTE_CACHE_LINE_SIZE) != 0)
+			return -ENOMEM;
+
+		sc->pf2vf_bulletin = (struct bnx2x_vf_bulletin *)
+					     sc->pf2vf_bulletin_mapping.vaddr;
+
+		ret = bnx2x_vf_get_resources(sc, sc->max_tx_queues,
+					     sc->max_rx_queues);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
 }
 
 static int
diff --git a/drivers/net/bnx2x/bnx2x_vfpf.c b/drivers/net/bnx2x/bnx2x_vfpf.c
index 34b6360..bce734f 100644
--- a/drivers/net/bnx2x/bnx2x_vfpf.c
+++ b/drivers/net/bnx2x/bnx2x_vfpf.c
@@ -282,6 +282,8 @@ int bnx2x_vf_get_resources(struct bnx2x_softc *sc, uint8_t tx_count, uint8_t rx_
 	sc->igu_sb_cnt  = sc_resp.resc.num_sbs;
 	sc->igu_base_sb = sc_resp.resc.hw_sbs[0] & 0xFF;
 	sc->igu_dsb_id  = -1;
+	sc->max_tx_queues = sc_resp.resc.num_txqs;
+	sc->max_rx_queues = sc_resp.resc.num_rxqs;
 
 	sc->link_params.chip_id = sc->devinfo.chip_id;
 	sc->doorbell_size = sc_resp.db_size;
-- 
2.5.0

Re: [dpdk-dev] [PATCH 2/2] bnx2x: Determine rx/tx queue sizes sooner

[Rasesh Mody]

> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Chas Williams
> Sent: Wednesday, December 30, 2015 4:38 PM
> 
> From: "Charles (Chas) Williams" <ciwillia@brocade.com>
> 
> The VF needs to determine the queues sizes before .dev_infos_get so that it
> can hint to the upper layer the proper sizes.  Move
> bnx2x_vf_get_resources() to .eth_dev_init and probe with the guesses from
> bnx2x_init_rte().
> 
> Signed-off-by: Chas Williams <3chas3@gmail.com>
> ---

Acked-by: Rasesh Mody <rasesh.mody@qlogic.com>

The change looks good.

Thanks!
Rasesh

>  drivers/net/bnx2x/bnx2x.c        |  9 ++++++--
>  drivers/net/bnx2x/bnx2x_ethdev.c | 46 ++++++++++++++++++++++--------
> ----------
>  drivers/net/bnx2x/bnx2x_vfpf.c   |  2 ++
>  3 files changed, 34 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/net/bnx2x/bnx2x.c b/drivers/net/bnx2x/bnx2x.c index
> 67af5da..f003875 100644
> --- a/drivers/net/bnx2x/bnx2x.c
> +++ b/drivers/net/bnx2x/bnx2x.c
> @@ -9578,8 +9578,13 @@ static int bnx2x_pci_get_caps(struct bnx2x_softc
> *sc)
> 
>  static void bnx2x_init_rte(struct bnx2x_softc *sc)  {
> -	sc->max_tx_queues = 128;
> -	sc->max_rx_queues = 128;
> +	if (IS_VF(sc)) {
> +		sc->max_tx_queues = BNX2X_VF_MAX_QUEUES_PER_VF;
> +		sc->max_rx_queues = BNX2X_VF_MAX_QUEUES_PER_VF;
> +	} else {
> +		sc->max_tx_queues = 128;
> +		sc->max_rx_queues = 128;
> +	}
>  }
> 
>  #define FW_HEADER_LEN 104
> diff --git a/drivers/net/bnx2x/bnx2x_ethdev.c
> b/drivers/net/bnx2x/bnx2x_ethdev.c
> index 69df02e..fe8cfd0 100644
> --- a/drivers/net/bnx2x/bnx2x_ethdev.c
> +++ b/drivers/net/bnx2x/bnx2x_ethdev.c
> @@ -87,7 +87,6 @@ bnx2x_dev_configure(struct rte_eth_dev *dev)  {
>  	struct bnx2x_softc *sc = dev->data->dev_private;
>  	int mp_ncpus = sysconf(_SC_NPROCESSORS_CONF);
> -	int ret;
> 
>  	PMD_INIT_FUNC_TRACE();
> 
> @@ -121,25 +120,6 @@ bnx2x_dev_configure(struct rte_eth_dev *dev)
>  		return -ENXIO;
>  	}
> 
> -	if (IS_VF(sc)) {
> -		if (bnx2x_dma_alloc(sc, sizeof(struct bnx2x_vf_mbx_msg),
> -				  &sc->vf2pf_mbox_mapping, "vf2pf_mbox",
> -				  RTE_CACHE_LINE_SIZE) != 0)
> -			return -ENOMEM;
> -
> -		sc->vf2pf_mbox = (struct bnx2x_vf_mbx_msg *)sc-
> >vf2pf_mbox_mapping.vaddr;
> -		if (bnx2x_dma_alloc(sc, sizeof(struct bnx2x_vf_bulletin),
> -				  &sc->pf2vf_bulletin_mapping, "vf2pf_bull",
> -				  RTE_CACHE_LINE_SIZE) != 0)
> -			return -ENOMEM;
> -
> -		sc->pf2vf_bulletin = (struct bnx2x_vf_bulletin *)sc-
> >pf2vf_bulletin_mapping.vaddr;
> -
> -		ret = bnx2x_vf_get_resources(sc, sc->num_queues, sc-
> >num_queues);
> -		if (ret)
> -			return ret;
> -	}
> -
>  	return 0;
>  }
> 
> @@ -466,6 +446,7 @@ bnx2x_common_dev_init(struct rte_eth_dev
> *eth_dev, int is_vf)
>  	ret = bnx2x_attach(sc);
>  	if (ret) {
>  		PMD_DRV_LOG(ERR, "bnx2x_attach failed (%d)", ret);
> +		return ret;
>  	}
> 
>  	eth_dev->data->mac_addrs = (struct ether_addr *)sc-
> >link_params.mac_addr; @@ -479,7 +460,30 @@
> bnx2x_common_dev_init(struct rte_eth_dev *eth_dev, int is_vf)
>  	PMD_DRV_LOG(INFO, "portID=%d vendorID=0x%x deviceID=0x%x",
>  			eth_dev->data->port_id, pci_dev->id.vendor_id,
> pci_dev->id.device_id);
> 
> -	return ret;
> +	if (IS_VF(sc)) {
> +		if (bnx2x_dma_alloc(sc, sizeof(struct bnx2x_vf_mbx_msg),
> +				    &sc->vf2pf_mbox_mapping,
> "vf2pf_mbox",
> +				    RTE_CACHE_LINE_SIZE) != 0)
> +			return -ENOMEM;
> +
> +		sc->vf2pf_mbox = (struct bnx2x_vf_mbx_msg *)
> +					 sc->vf2pf_mbox_mapping.vaddr;
> +
> +		if (bnx2x_dma_alloc(sc, sizeof(struct bnx2x_vf_bulletin),
> +				    &sc->pf2vf_bulletin_mapping,
> "vf2pf_bull",
> +				    RTE_CACHE_LINE_SIZE) != 0)
> +			return -ENOMEM;
> +
> +		sc->pf2vf_bulletin = (struct bnx2x_vf_bulletin *)
> +					     sc->pf2vf_bulletin_mapping.vaddr;
> +
> +		ret = bnx2x_vf_get_resources(sc, sc->max_tx_queues,
> +					     sc->max_rx_queues);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
>  }
> 
>  static int
> diff --git a/drivers/net/bnx2x/bnx2x_vfpf.c
> b/drivers/net/bnx2x/bnx2x_vfpf.c index 34b6360..bce734f 100644
> --- a/drivers/net/bnx2x/bnx2x_vfpf.c
> +++ b/drivers/net/bnx2x/bnx2x_vfpf.c
> @@ -282,6 +282,8 @@ int bnx2x_vf_get_resources(struct bnx2x_softc *sc,
> uint8_t tx_count, uint8_t rx_
>  	sc->igu_sb_cnt  = sc_resp.resc.num_sbs;
>  	sc->igu_base_sb = sc_resp.resc.hw_sbs[0] & 0xFF;
>  	sc->igu_dsb_id  = -1;
> +	sc->max_tx_queues = sc_resp.resc.num_txqs;
> +	sc->max_rx_queues = sc_resp.resc.num_rxqs;
> 
>  	sc->link_params.chip_id = sc->devinfo.chip_id;
>  	sc->doorbell_size = sc_resp.db_size;
> --
> 2.5.0

Re: [dpdk-dev] [PATCH 2/2] bnx2x: Determine rx/tx queue sizes sooner

[Bruce Richardson]

On Fri, Mar 04, 2016 at 10:28:57PM +0000, Rasesh Mody wrote:
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Chas Williams
> > Sent: Wednesday, December 30, 2015 4:38 PM
> > 
> > From: "Charles (Chas) Williams" <ciwillia@brocade.com>
> > 
> > The VF needs to determine the queues sizes before .dev_infos_get so that it
> > can hint to the upper layer the proper sizes.  Move
> > bnx2x_vf_get_resources() to .eth_dev_init and probe with the guesses from
> > bnx2x_init_rte().
> > 
> > Signed-off-by: Chas Williams <3chas3@gmail.com>
> > ---
> 
> Acked-by: Rasesh Mody <rasesh.mody@qlogic.com>
> 
> The change looks good.
> 
> Thanks!
> Rasesh
Applied to dpdk-next-net/rel_16_04

/Bruce

Re: [dpdk-dev] [PATCH 1/2] bnx2x: fix error handling in bnx2x_loop_obtain_resources()

[Bruce Richardson]

On Wed, Dec 30, 2015 at 07:37:50PM -0500, Chas Williams wrote:
> From: "Charles (Chas) Williams" <ciwillia@brocade.com>
> 
> bnx2x_loop_obtain_resources() returns a struct containing the status and
> the error message.  If bnx2x_do_req4pf() fails, it shouldn't return both
> of these fields set to 0 indicating failure and no error.
> 
> Further, bnx2x_do_req4pf() needs to be able fail and return NO_RESOURCES
> so that bnx2x_loop_obtain_resources() can negotiate reduced resource
> requirments.  This requires additional checking around bnx2x_do_req4pf().
> 
> Signed-off-by: Chas Williams <3chas3@gmail.com>
> ---

Hi Chas,

can you please provide a commit reference for a fixes line for this patch. [No
need for a V2 just for that alone, just the reference is enough to save me
diving into the git history. I'll manually add the fixes line when committing
to the next-net tree]

Maintainers, can you please review this patchset and provide feedback or an ACK.

Regards,
/Bruce

Re: [dpdk-dev] [PATCH 1/2] bnx2x: fix error handling in bnx2x_loop_obtain_resources()

[Charles (Chas) Williams]

I am afraid I don't understand what you are asking. This was broken in 
the commit that added bnx2x, 540a211084a7695a1c7bc43068934c140d6989be

On 02/08/2016 05:51 AM, Bruce Richardson wrote:
> On Wed, Dec 30, 2015 at 07:37:50PM -0500, Chas Williams wrote:
>> From: "Charles (Chas) Williams" <ciwillia@brocade.com>
>>
>> bnx2x_loop_obtain_resources() returns a struct containing the status and
>> the error message.  If bnx2x_do_req4pf() fails, it shouldn't return both
>> of these fields set to 0 indicating failure and no error.
>>
>> Further, bnx2x_do_req4pf() needs to be able fail and return NO_RESOURCES
>> so that bnx2x_loop_obtain_resources() can negotiate reduced resource
>> requirments.  This requires additional checking around bnx2x_do_req4pf().
>>
>> Signed-off-by: Chas Williams <3chas3@gmail.com>
>> ---
>
> Hi Chas,
>
> can you please provide a commit reference for a fixes line for this patch. [No
> need for a V2 just for that alone, just the reference is enough to save me
> diving into the git history. I'll manually add the fixes line when committing
> to the next-net tree]
>
> Maintainers, can you please review this patchset and provide feedback or an ACK.
>
> Regards,
> /Bruce
>

Re: [dpdk-dev] [PATCH 1/2] bnx2x: fix error handling in bnx2x_loop_obtain_resources()

[Bruce Richardson]

On Mon, Feb 08, 2016 at 07:53:46AM -0800, Charles (Chas) Williams wrote:
> I am afraid I don't understand what you are asking. This was broken in the
> commit that added bnx2x, 540a211084a7695a1c7bc43068934c140d6989be
> 

Perfect. Generally, when adding a patch which fixes something, we add in a 
"fixes line" into the comment. In this case it would be:

	Fixes: 540a211084a7 ("bnx2x: driver core")

See also: http://dpdk.org/doc/guides/contributing/patches.html#commit-messages-body

/Bruce

> On 02/08/2016 05:51 AM, Bruce Richardson wrote:
> >On Wed, Dec 30, 2015 at 07:37:50PM -0500, Chas Williams wrote:
> >>From: "Charles (Chas) Williams" <ciwillia@brocade.com>
> >>
> >>bnx2x_loop_obtain_resources() returns a struct containing the status and
> >>the error message.  If bnx2x_do_req4pf() fails, it shouldn't return both
> >>of these fields set to 0 indicating failure and no error.
> >>
> >>Further, bnx2x_do_req4pf() needs to be able fail and return NO_RESOURCES
> >>so that bnx2x_loop_obtain_resources() can negotiate reduced resource
> >>requirments.  This requires additional checking around bnx2x_do_req4pf().
> >>
> >>Signed-off-by: Chas Williams <3chas3@gmail.com>
> >>---
> >
> >Hi Chas,
> >
> >can you please provide a commit reference for a fixes line for this patch. [No
> >need for a V2 just for that alone, just the reference is enough to save me
> >diving into the git history. I'll manually add the fixes line when committing
> >to the next-net tree]
> >
> >Maintainers, can you please review this patchset and provide feedback or an ACK.
> >
> >Regards,
> >/Bruce
> >

Re: [dpdk-dev] [PATCH 1/2] bnx2x: fix error handling in bnx2x_loop_obtain_resources()

[Rasesh Mody]

> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Chas Williams
> Sent: Wednesday, December 30, 2015 4:38 PM
> 
> From: "Charles (Chas) Williams" <ciwillia@brocade.com>
> 
> bnx2x_loop_obtain_resources() returns a struct containing the status and
> the error message.  If bnx2x_do_req4pf() fails, it shouldn't return both of
> these fields set to 0 indicating failure and no error.
> 
> Further, bnx2x_do_req4pf() needs to be able fail and return NO_RESOURCES
> so that bnx2x_loop_obtain_resources() can negotiate reduced resource
> requirments.  This requires additional checking around bnx2x_do_req4pf().
> 
> Signed-off-by: Chas Williams <3chas3@gmail.com>
> ---

Acked-by: Rasesh Mody <rasesh.mody@qlogic.com>

The change looks good.

Thanks!
Rasesh

>  drivers/net/bnx2x/bnx2x_vfpf.c | 75 +++++++++++++++++++-----------------
> ------
>  1 file changed, 33 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/net/bnx2x/bnx2x_vfpf.c
> b/drivers/net/bnx2x/bnx2x_vfpf.c index 765cc92..34b6360 100644
> --- a/drivers/net/bnx2x/bnx2x_vfpf.c
> +++ b/drivers/net/bnx2x/bnx2x_vfpf.c
> @@ -122,16 +122,10 @@ bnx2x_do_req4pf(struct bnx2x_softc *sc,
> phys_addr_t phys_addr)
>  				break;
>  		}
> 
> -		if (i == BNX2X_VF_CHANNEL_TRIES) {
> +		if (!*status) {
>  			PMD_DRV_LOG(ERR, "Response from PF timed
> out");
>  			return -EAGAIN;
>  		}
> -
> -		if (BNX2X_VF_STATUS_SUCCESS != *status) {
> -			PMD_DRV_LOG(ERR, "Bad reply from PF : %u",
> -					*status);
> -			return -EINVAL;
> -		}
>  	} else {
>  		PMD_DRV_LOG(ERR, "status should be zero before
> message"
>  				"to pf was sent");
> @@ -193,10 +187,10 @@ struct bnx2x_obtain_status
> bnx2x_loop_obtain_resources(struct bnx2x_softc *sc)
>  	do {
>  		PMD_DRV_LOG(DEBUG, "trying to get resources");
> 
> -		if ( bnx2x_do_req4pf(sc, sc->vf2pf_mbox_mapping.paddr) )
> {
> +		if (bnx2x_do_req4pf(sc, sc->vf2pf_mbox_mapping.paddr)) {
>  			/* timeout */
>  			status.success = 0;
> -			status.err_code = 0;
> +			status.err_code = -EAGAIN;
>  			return status;
>  		}
> 
> @@ -310,8 +304,8 @@ void
>  bnx2x_vf_close(struct bnx2x_softc *sc)
>  {
>  	struct vf_release_tlv *query;
> +	struct vf_common_reply_tlv *reply =
> +&sc->vf2pf_mbox->resp.common_reply;
>  	int vf_id = bnx2x_read_vf_id(sc);
> -	int ret;
> 
>  	if (vf_id >= 0) {
>  		query = &sc->vf2pf_mbox->query[0].release;
> @@ -322,11 +316,9 @@ bnx2x_vf_close(struct bnx2x_softc *sc)
>  		BNX2X_TLV_APPEND(query, query->first_tlv.length,
> BNX2X_VF_TLV_LIST_END,
>  				sizeof(struct channel_list_end_tlv));
> 
> -		ret = bnx2x_do_req4pf(sc, sc-
> >vf2pf_mbox_mapping.paddr);
> -
> -		if (ret) {
> +		bnx2x_do_req4pf(sc, sc->vf2pf_mbox_mapping.paddr);
> +		if (reply->status != BNX2X_VF_STATUS_SUCCESS)
>  			PMD_DRV_LOG(ERR, "Failed to release VF");
> -		}
>  	}
>  }
> 
> @@ -335,7 +327,8 @@ int
>  bnx2x_vf_init(struct bnx2x_softc *sc)
>  {
>  	struct vf_init_tlv *query;
> -	int i, ret;
> +	struct vf_common_reply_tlv *reply = &sc->vf2pf_mbox-
> >resp.common_reply;
> +	int i;
> 
>  	query = &sc->vf2pf_mbox->query[0].init;
>  	bnx2x_init_first_tlv(sc, &query->first_tlv, BNX2X_VF_TLV_INIT, @@
> -352,11 +345,10 @@ bnx2x_vf_init(struct bnx2x_softc *sc)
>  	BNX2X_TLV_APPEND(query, query->first_tlv.length,
> BNX2X_VF_TLV_LIST_END,
>  			sizeof(struct channel_list_end_tlv));
> 
> -	ret = bnx2x_do_req4pf(sc, sc->vf2pf_mbox_mapping.paddr);
> -
> -	if (ret) {
> +	bnx2x_do_req4pf(sc, sc->vf2pf_mbox_mapping.paddr);
> +	if (reply->status != BNX2X_VF_STATUS_SUCCESS) {
>  		PMD_DRV_LOG(ERR, "Failed to init VF");
> -		return ret;
> +		return -EINVAL;
>  	}
> 
>  	PMD_DRV_LOG(DEBUG, "VF was initialized"); @@ -367,8 +359,9 @@
> void  bnx2x_vf_unload(struct bnx2x_softc *sc)  {
>  	struct vf_close_tlv *query;
> +	struct vf_common_reply_tlv *reply =
> +&sc->vf2pf_mbox->resp.common_reply;
>  	struct vf_q_op_tlv *query_op;
> -	int i, vf_id, ret;
> +	int i, vf_id;
> 
>  	vf_id = bnx2x_read_vf_id(sc);
>  	if (vf_id > 0) {
> @@ -384,10 +377,10 @@ bnx2x_vf_unload(struct bnx2x_softc *sc)
>  					BNX2X_VF_TLV_LIST_END,
>  					sizeof(struct channel_list_end_tlv));
> 
> -			ret = bnx2x_do_req4pf(sc, sc-
> >vf2pf_mbox_mapping.paddr);
> -			if (ret)
> +			bnx2x_do_req4pf(sc, sc-
> >vf2pf_mbox_mapping.paddr);
> +			if (reply->status != BNX2X_VF_STATUS_SUCCESS)
>  				PMD_DRV_LOG(ERR,
> -						"Bad reply for vf_q %d
> teardown", i);
> +					    "Bad reply for vf_q %d teardown",
> i);
>  		}
> 
>  		bnx2x_vf_set_mac(sc, false);
> @@ -402,11 +395,10 @@ bnx2x_vf_unload(struct bnx2x_softc *sc)
>  				BNX2X_VF_TLV_LIST_END,
>  				sizeof(struct channel_list_end_tlv));
> 
> -		ret = bnx2x_do_req4pf(sc, sc-
> >vf2pf_mbox_mapping.paddr);
> -
> -		if (ret)
> +		bnx2x_do_req4pf(sc, sc->vf2pf_mbox_mapping.paddr);
> +		if (reply->status != BNX2X_VF_STATUS_SUCCESS)
>  			PMD_DRV_LOG(ERR,
> -				"Bad reply from PF for close message");
> +				    "Bad reply from PF for close message");
>  	}
>  }
> 
> @@ -469,8 +461,8 @@ int
>  bnx2x_vf_setup_queue(struct bnx2x_softc *sc, struct bnx2x_fastpath *fp,
> int leading)  {
>  	struct vf_setup_q_tlv *query;
> +	struct vf_common_reply_tlv *reply =
> +&sc->vf2pf_mbox->resp.common_reply;
>  	uint16_t flags = bnx2x_vf_q_flags(leading);
> -	int ret;
> 
>  	query = &sc->vf2pf_mbox->query[0].setup_q;
>  	bnx2x_init_first_tlv(sc, &query->first_tlv, BNX2X_VF_TLV_SETUP_Q,
> @@ -485,11 +477,10 @@ bnx2x_vf_setup_queue(struct bnx2x_softc *sc,
> struct bnx2x_fastpath *fp, int lead
>  	BNX2X_TLV_APPEND(query, query->first_tlv.length,
> BNX2X_VF_TLV_LIST_END,
>  			sizeof(struct channel_list_end_tlv));
> 
> -	ret = bnx2x_do_req4pf(sc, sc->vf2pf_mbox_mapping.paddr);
> -
> -	if (ret) {
> +	bnx2x_do_req4pf(sc, sc->vf2pf_mbox_mapping.paddr);
> +	if (reply->status != BNX2X_VF_STATUS_SUCCESS) {
>  		PMD_DRV_LOG(ERR, "Failed to setup VF queue[%d]",
> -				fp->index);
> +				 fp->index);
>  		return -EINVAL;
>  	}
> 
> @@ -548,7 +539,7 @@ bnx2x_vf_config_rss(struct bnx2x_softc *sc,
>  			  struct ecore_config_rss_params *params)  {
>  	struct vf_rss_tlv *query;
> -	int ret;
> +	struct vf_common_reply_tlv *reply =
> +&sc->vf2pf_mbox->resp.common_reply;
> 
>  	query = &sc->vf2pf_mbox->query[0].update_rss;
> 
> @@ -568,10 +559,10 @@ bnx2x_vf_config_rss(struct bnx2x_softc *sc,
>  	query->rss_result_mask = params->rss_result_mask;
>  	query->rss_flags = params->rss_flags;
> 
> -	ret = bnx2x_do_req4pf(sc, sc->vf2pf_mbox_mapping.paddr);
> -	if (ret) {
> -		PMD_DRV_LOG(ERR, "Failed to send message to PF, rc %d",
> ret);
> -		return ret;
> +	bnx2x_do_req4pf(sc, sc->vf2pf_mbox_mapping.paddr);
> +	if (reply->status != BNX2X_VF_STATUS_SUCCESS) {
> +		PMD_DRV_LOG(ERR, "Failed to configure RSS");
> +		return -EINVAL;;
>  	}
> 
>  	return 0;
> @@ -581,8 +572,8 @@ int
>  bnx2x_vf_set_rx_mode(struct bnx2x_softc *sc)  {
>  	struct vf_set_q_filters_tlv *query;
> +	struct vf_common_reply_tlv *reply =
> +&sc->vf2pf_mbox->resp.common_reply;
>  	unsigned long tx_mask;
> -	int ret;
> 
>  	query = &sc->vf2pf_mbox->query[0].set_q_filters;
>  	bnx2x_init_first_tlv(sc, &query->first_tlv,
> BNX2X_VF_TLV_SET_Q_FILTERS, @@ -598,10 +589,10 @@
> bnx2x_vf_set_rx_mode(struct bnx2x_softc *sc)
>  	BNX2X_TLV_APPEND(query, query->first_tlv.length,
> BNX2X_VF_TLV_LIST_END,
>  			sizeof(struct channel_list_end_tlv));
> 
> -	ret = bnx2x_do_req4pf(sc, sc->vf2pf_mbox_mapping.paddr);
> -	if (ret) {
> -		PMD_DRV_LOG(ERR, "Failed to send message to PF, rc %d",
> ret);
> -		return ret;
> +	bnx2x_do_req4pf(sc, sc->vf2pf_mbox_mapping.paddr);
> +	if (reply->status != BNX2X_VF_STATUS_SUCCESS) {
> +		PMD_DRV_LOG(ERR, "Failed to set RX mode");
> +		return -EINVAL;
>  	}
> 
>  	return 0;
> --
> 2.5.0

Re: [dpdk-dev] [PATCH 1/2] bnx2x: fix error handling in bnx2x_loop_obtain_resources()

[Bruce Richardson]

On Fri, Mar 04, 2016 at 10:28:44PM +0000, Rasesh Mody wrote:
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Chas Williams
> > Sent: Wednesday, December 30, 2015 4:38 PM
> > 
> > From: "Charles (Chas) Williams" <ciwillia@brocade.com>
> > 
> > bnx2x_loop_obtain_resources() returns a struct containing the status and
> > the error message.  If bnx2x_do_req4pf() fails, it shouldn't return both of
> > these fields set to 0 indicating failure and no error.
> > 
> > Further, bnx2x_do_req4pf() needs to be able fail and return NO_RESOURCES
> > so that bnx2x_loop_obtain_resources() can negotiate reduced resource
> > requirments.  This requires additional checking around bnx2x_do_req4pf().
> > 
> > Signed-off-by: Chas Williams <3chas3@gmail.com>
> > ---
> 
> Acked-by: Rasesh Mody <rasesh.mody@qlogic.com>
> 
> The change looks good.
> 
> Thanks!
> Rasesh
> 

Applied to dpdk-next-net/rel_16_04, with added fixes line in message.

/Bruce