Skip to contentSkip to conversation

bnxt_en: Bug fixes.

PATCH In review Michael Chan 2 participants 10 messages Mar 10, 2018 – Mar 12, 2018

Subsystems (3)
Networking Network drivers Broadcom bnxt_en 50 gigabit Ethernet driver
lore
Share & tools
lore

Michael Chan

author maintainer
#0 lore raw

cover bnxt_en: Bug fixes.

There are 3 bug fixes in this series to fix regressions recently introduced when adding the new ring reservations scheme. 2 minor fixes in the TC Flower code to return standard errno values and to elide some unnecessary warning dmesg. One Fixes the VLAN TCI value passed to the stack by including the entire 16-bit VLAN TCI, and the last fix is to check for valid VNIC ID before setting up or shutting down LRO/GRO.

Eddie Wai (1):
bnxt_en: Fix vnic accounting in the bnxt_check_rings() path.

Michael Chan (4):
bnxt_en: Refactor the functions to reserve hardware rings.
bnxt_en: Pass complete VLAN TCI to the stack.
bnxt_en: Fix regressions when setting up MQPRIO TX rings.
bnxt_en: Check valid VNIC ID in bnxt_hwrm_vnic_set_tpa().

Sriharsha Basavapatna (1):
bnxt_en: Remove unwanted ovs-offload messages in some conditions

Venkat Duvvuru (2):
bnxt_en: Return standard Linux error codes for hwrm flow cmds.
bnxt_en: close & open NIC, only when the interface is in running
state.

 drivers/net/ethernet/broadcom/bnxt/bnxt.c    | 180 ++++++++++++++-------------
 drivers/net/ethernet/broadcom/bnxt/bnxt.h    |   1 +
 drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c |  33 +++--
 3 files changed, 118 insertions(+), 96 deletions(-)
Signature · 1 line
1.8.3.1
author maintainer
#1 lore raw

PATCH 1/8 bnxt_en: Refactor the functions to reserve hardware rings.

The bnxt_hwrm_reserve_{pf|vf}_rings() functions are very similar to the bnxt_hwrm_check_{pf|vf}_rings() functions. Refactor the former so that the latter can make use of common code in the next patch.

Signed-off-by Michael Chan <[email protected]>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 85 +++++++++++++++++++------------
 1 file changed, 53 insertions(+), 32 deletions(-)
drivers/net/ethernet/broadcom/bnxt/bnxt.c +53 -32
index 1500243..377fceb 100644
@@ -4558,18 +4558,17 @@ int __bnxt_hwrm_get_tx_rings(struct bnxt *bp, u16 fid, int *tx_rings)
 	return rc;
 }
 
-static int
-bnxt_hwrm_reserve_pf_rings(struct bnxt *bp, int tx_rings, int rx_rings,
-			   int ring_grps, int cp_rings, int vnics)
+static void
+__bnxt_hwrm_reserve_pf_rings(struct bnxt *bp, struct hwrm_func_cfg_input *req,
+			     int tx_rings, int rx_rings, int ring_grps,
+			     int cp_rings, int vnics)
 {
-	struct hwrm_func_cfg_input req = {0};
 	u32 enables = 0;
-	int rc;
 
-	bnxt_hwrm_cmd_hdr_init(bp, &req, HWRM_FUNC_CFG, -1, -1);
-	req.fid = cpu_to_le16(0xffff);
+	bnxt_hwrm_cmd_hdr_init(bp, req, HWRM_FUNC_CFG, -1, -1);
+	req->fid = cpu_to_le16(0xffff);
 	enables |= tx_rings ? FUNC_CFG_REQ_ENABLES_NUM_TX_RINGS : 0;
-	req.num_tx_rings = cpu_to_le16(tx_rings);
+	req->num_tx_rings = cpu_to_le16(tx_rings);
 	if (bp->flags & BNXT_FLAG_NEW_RM) {
 		enables |= rx_rings ? FUNC_CFG_REQ_ENABLES_NUM_RX_RINGS : 0;
 		enables |= cp_rings ? FUNC_CFG_REQ_ENABLES_NUM_CMPL_RINGS |

@@ -4578,16 +4577,53 @@ int __bnxt_hwrm_get_tx_rings(struct bnxt *bp, u16 fid, int *tx_rings)
 			   FUNC_CFG_REQ_ENABLES_NUM_HW_RING_GRPS : 0;
 		enables |= vnics ? FUNC_VF_CFG_REQ_ENABLES_NUM_VNICS : 0;
 
-		req.num_rx_rings = cpu_to_le16(rx_rings);
-		req.num_hw_ring_grps = cpu_to_le16(ring_grps);
-		req.num_cmpl_rings = cpu_to_le16(cp_rings);
-		req.num_stat_ctxs = req.num_cmpl_rings;
-		req.num_vnics = cpu_to_le16(vnics);
+		req->num_rx_rings = cpu_to_le16(rx_rings);
+		req->num_hw_ring_grps = cpu_to_le16(ring_grps);
+		req->num_cmpl_rings = cpu_to_le16(cp_rings);
+		req->num_stat_ctxs = req->num_cmpl_rings;
+		req->num_vnics = cpu_to_le16(vnics);
 	}
-	if (!enables)
+	req->enables = cpu_to_le32(enables);
+}
+
+static void
+__bnxt_hwrm_reserve_vf_rings(struct bnxt *bp,
+			     struct hwrm_func_vf_cfg_input *req, int tx_rings,
+			     int rx_rings, int ring_grps, int cp_rings,
+			     int vnics)
+{
+	u32 enables = 0;
+
+	bnxt_hwrm_cmd_hdr_init(bp, req, HWRM_FUNC_VF_CFG, -1, -1);
+	enables |= tx_rings ? FUNC_VF_CFG_REQ_ENABLES_NUM_TX_RINGS : 0;
+	enables |= rx_rings ? FUNC_VF_CFG_REQ_ENABLES_NUM_RX_RINGS : 0;
+	enables |= cp_rings ? FUNC_VF_CFG_REQ_ENABLES_NUM_CMPL_RINGS |
+			      FUNC_VF_CFG_REQ_ENABLES_NUM_STAT_CTXS : 0;
+	enables |= ring_grps ? FUNC_VF_CFG_REQ_ENABLES_NUM_HW_RING_GRPS : 0;
+	enables |= vnics ? FUNC_VF_CFG_REQ_ENABLES_NUM_VNICS : 0;
+
+	req->num_tx_rings = cpu_to_le16(tx_rings);
+	req->num_rx_rings = cpu_to_le16(rx_rings);
+	req->num_hw_ring_grps = cpu_to_le16(ring_grps);
+	req->num_cmpl_rings = cpu_to_le16(cp_rings);
+	req->num_stat_ctxs = req->num_cmpl_rings;
+	req->num_vnics = cpu_to_le16(vnics);
+
+	req->enables = cpu_to_le32(enables);
+}
+
+static int
+bnxt_hwrm_reserve_pf_rings(struct bnxt *bp, int tx_rings, int rx_rings,
+			   int ring_grps, int cp_rings, int vnics)
+{
+	struct hwrm_func_cfg_input req = {0};
+	int rc;
+
+	__bnxt_hwrm_reserve_pf_rings(bp, &req, tx_rings, rx_rings, ring_grps,
+				     cp_rings, vnics);
+	if (!req.enables)
 		return 0;
 
-	req.enables = cpu_to_le32(enables);
 	rc = hwrm_send_message(bp, &req, sizeof(req), HWRM_CMD_TIMEOUT);
 	if (rc)
 		return -ENOMEM;

@@ -4604,7 +4640,6 @@ int __bnxt_hwrm_get_tx_rings(struct bnxt *bp, u16 fid, int *tx_rings)
 			   int ring_grps, int cp_rings, int vnics)
 {
 	struct hwrm_func_vf_cfg_input req = {0};
-	u32 enables = 0;
 	int rc;
 
 	if (!(bp->flags & BNXT_FLAG_NEW_RM)) {

@@ -4612,22 +4647,8 @@ int __bnxt_hwrm_get_tx_rings(struct bnxt *bp, u16 fid, int *tx_rings)
 		return 0;
 	}
 
-	bnxt_hwrm_cmd_hdr_init(bp, &req, HWRM_FUNC_VF_CFG, -1, -1);
-	enables |= tx_rings ? FUNC_VF_CFG_REQ_ENABLES_NUM_TX_RINGS : 0;
-	enables |= rx_rings ? FUNC_VF_CFG_REQ_ENABLES_NUM_RX_RINGS : 0;
-	enables |= cp_rings ? FUNC_VF_CFG_REQ_ENABLES_NUM_CMPL_RINGS |
-			      FUNC_VF_CFG_REQ_ENABLES_NUM_STAT_CTXS : 0;
-	enables |= ring_grps ? FUNC_VF_CFG_REQ_ENABLES_NUM_HW_RING_GRPS : 0;
-	enables |= vnics ? FUNC_VF_CFG_REQ_ENABLES_NUM_VNICS : 0;
-
-	req.num_tx_rings = cpu_to_le16(tx_rings);
-	req.num_rx_rings = cpu_to_le16(rx_rings);
-	req.num_hw_ring_grps = cpu_to_le16(ring_grps);
-	req.num_cmpl_rings = cpu_to_le16(cp_rings);
-	req.num_stat_ctxs = req.num_cmpl_rings;
-	req.num_vnics = cpu_to_le16(vnics);
-
-	req.enables = cpu_to_le32(enables);
+	__bnxt_hwrm_reserve_vf_rings(bp, &req, tx_rings, rx_rings, ring_grps,
+				     cp_rings, vnics);
 	rc = hwrm_send_message(bp, &req, sizeof(req), HWRM_CMD_TIMEOUT);
 	if (rc)
 		return -ENOMEM;
Signature · 1 line
1.8.3.1
author maintainer
#2 lore raw

PATCH 2/8 bnxt_en: Fix vnic accounting in the bnxt_check_rings() path.

In reply to Michael Chan

From: Eddie Wai <[email protected]>

The number of vnics to check must be determined ahead of time because only standard RX rings require vnics to support RFS. The logic is similar to the ring reservation logic and we can now use the refactored common functions to do most of the work in setting up the firmware message.

Fixes 8f23d638b36b ("bnxt_en: Expand bnxt_check_rings() to check all resources.")
Signed-off-by Eddie Wai <[email protected]>
Signed-off-by Michael Chan <[email protected]>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 64 ++++++++++---------------------
 1 file changed, 20 insertions(+), 44 deletions(-)
drivers/net/ethernet/broadcom/bnxt/bnxt.c +20 -44
index 377fceb..b847d54 100644
@@ -4764,39 +4764,25 @@ static bool bnxt_need_reserve_rings(struct bnxt *bp)
 }
 
 static int bnxt_hwrm_check_vf_rings(struct bnxt *bp, int tx_rings, int rx_rings,
-				    int ring_grps, int cp_rings)
+				    int ring_grps, int cp_rings, int vnics)
 {
 	struct hwrm_func_vf_cfg_input req = {0};
-	u32 flags, enables;
+	u32 flags;
 	int rc;
 
 	if (!(bp->flags & BNXT_FLAG_NEW_RM))
 		return 0;
 
-	bnxt_hwrm_cmd_hdr_init(bp, &req, HWRM_FUNC_VF_CFG, -1, -1);
+	__bnxt_hwrm_reserve_vf_rings(bp, &req, tx_rings, rx_rings, ring_grps,
+				     cp_rings, vnics);
 	flags = FUNC_VF_CFG_REQ_FLAGS_TX_ASSETS_TEST |
 		FUNC_VF_CFG_REQ_FLAGS_RX_ASSETS_TEST |
 		FUNC_VF_CFG_REQ_FLAGS_CMPL_ASSETS_TEST |
 		FUNC_VF_CFG_REQ_FLAGS_RING_GRP_ASSETS_TEST |
 		FUNC_VF_CFG_REQ_FLAGS_STAT_CTX_ASSETS_TEST |
 		FUNC_VF_CFG_REQ_FLAGS_VNIC_ASSETS_TEST;
-	enables = FUNC_VF_CFG_REQ_ENABLES_NUM_TX_RINGS |
-		  FUNC_VF_CFG_REQ_ENABLES_NUM_RX_RINGS |
-		  FUNC_VF_CFG_REQ_ENABLES_NUM_CMPL_RINGS |
-		  FUNC_VF_CFG_REQ_ENABLES_NUM_HW_RING_GRPS |
-		  FUNC_VF_CFG_REQ_ENABLES_NUM_STAT_CTXS |
-		  FUNC_VF_CFG_REQ_ENABLES_NUM_VNICS;
 
 	req.flags = cpu_to_le32(flags);
-	req.enables = cpu_to_le32(enables);
-	req.num_tx_rings = cpu_to_le16(tx_rings);
-	req.num_rx_rings = cpu_to_le16(rx_rings);
-	req.num_cmpl_rings = cpu_to_le16(cp_rings);
-	req.num_hw_ring_grps = cpu_to_le16(ring_grps);
-	req.num_stat_ctxs = cpu_to_le16(cp_rings);
-	req.num_vnics = cpu_to_le16(1);
-	if (bp->flags & BNXT_FLAG_RFS)
-		req.num_vnics = cpu_to_le16(rx_rings + 1);
 	rc = hwrm_send_message_silent(bp, &req, sizeof(req), HWRM_CMD_TIMEOUT);
 	if (rc)
 		return -ENOMEM;

@@ -4804,38 +4790,23 @@ static int bnxt_hwrm_check_vf_rings(struct bnxt *bp, int tx_rings, int rx_rings,
 }
 
 static int bnxt_hwrm_check_pf_rings(struct bnxt *bp, int tx_rings, int rx_rings,
-				    int ring_grps, int cp_rings)
+				    int ring_grps, int cp_rings, int vnics)
 {
 	struct hwrm_func_cfg_input req = {0};
-	u32 flags, enables;
+	u32 flags;
 	int rc;
 
-	bnxt_hwrm_cmd_hdr_init(bp, &req, HWRM_FUNC_CFG, -1, -1);
-	req.fid = cpu_to_le16(0xffff);
+	__bnxt_hwrm_reserve_pf_rings(bp, &req, tx_rings, rx_rings, ring_grps,
+				     cp_rings, vnics);
 	flags = FUNC_CFG_REQ_FLAGS_TX_ASSETS_TEST;
-	enables = FUNC_CFG_REQ_ENABLES_NUM_TX_RINGS;
-	req.num_tx_rings = cpu_to_le16(tx_rings);
-	if (bp->flags & BNXT_FLAG_NEW_RM) {
+	if (bp->flags & BNXT_FLAG_NEW_RM)
 		flags |= FUNC_CFG_REQ_FLAGS_RX_ASSETS_TEST |
 			 FUNC_CFG_REQ_FLAGS_CMPL_ASSETS_TEST |
 			 FUNC_CFG_REQ_FLAGS_RING_GRP_ASSETS_TEST |
 			 FUNC_CFG_REQ_FLAGS_STAT_CTX_ASSETS_TEST |
 			 FUNC_CFG_REQ_FLAGS_VNIC_ASSETS_TEST;
-		enables |= FUNC_CFG_REQ_ENABLES_NUM_RX_RINGS |
-			   FUNC_CFG_REQ_ENABLES_NUM_CMPL_RINGS |
-			   FUNC_CFG_REQ_ENABLES_NUM_HW_RING_GRPS |
-			   FUNC_CFG_REQ_ENABLES_NUM_STAT_CTXS |
-			   FUNC_CFG_REQ_ENABLES_NUM_VNICS;
-		req.num_rx_rings = cpu_to_le16(rx_rings);
-		req.num_cmpl_rings = cpu_to_le16(cp_rings);
-		req.num_hw_ring_grps = cpu_to_le16(ring_grps);
-		req.num_stat_ctxs = cpu_to_le16(cp_rings);
-		req.num_vnics = cpu_to_le16(1);
-		if (bp->flags & BNXT_FLAG_RFS)
-			req.num_vnics = cpu_to_le16(rx_rings + 1);
-	}
+
 	req.flags = cpu_to_le32(flags);
-	req.enables = cpu_to_le32(enables);
 	rc = hwrm_send_message_silent(bp, &req, sizeof(req), HWRM_CMD_TIMEOUT);
 	if (rc)
 		return -ENOMEM;

@@ -4843,17 +4814,17 @@ static int bnxt_hwrm_check_pf_rings(struct bnxt *bp, int tx_rings, int rx_rings,
 }
 
 static int bnxt_hwrm_check_rings(struct bnxt *bp, int tx_rings, int rx_rings,
-				 int ring_grps, int cp_rings)
+				 int ring_grps, int cp_rings, int vnics)
 {
 	if (bp->hwrm_spec_code < 0x10801)
 		return 0;
 
 	if (BNXT_PF(bp))
 		return bnxt_hwrm_check_pf_rings(bp, tx_rings, rx_rings,
-						ring_grps, cp_rings);
+						ring_grps, cp_rings, vnics);
 
 	return bnxt_hwrm_check_vf_rings(bp, tx_rings, rx_rings, ring_grps,
-					cp_rings);
+					cp_rings, vnics);
 }
 
 static void bnxt_hwrm_set_coal_params(struct bnxt_coal *hw_coal,

@@ -7552,7 +7523,7 @@ int bnxt_check_rings(struct bnxt *bp, int tx, int rx, bool sh, int tcs,
 	int max_rx, max_tx, tx_sets = 1;
 	int tx_rings_needed;
 	int rx_rings = rx;
-	int cp, rc;
+	int cp, vnics, rc;
 
 	if (tcs)
 		tx_sets = tcs;

@@ -7568,10 +7539,15 @@ int bnxt_check_rings(struct bnxt *bp, int tx, int rx, bool sh, int tcs,
 	if (max_tx < tx_rings_needed)
 		return -ENOMEM;
 
+	vnics = 1;
+	if (bp->flags & BNXT_FLAG_RFS)
+		vnics += rx_rings;
+
 	if (bp->flags & BNXT_FLAG_AGG_RINGS)
 		rx_rings <<= 1;
 	cp = sh ? max_t(int, tx_rings_needed, rx) : tx_rings_needed + rx;
-	return bnxt_hwrm_check_rings(bp, tx_rings_needed, rx_rings, rx, cp);
+	return bnxt_hwrm_check_rings(bp, tx_rings_needed, rx_rings, rx, cp,
+				     vnics);
 }
 
 static void bnxt_unmap_bars(struct bnxt *bp, struct pci_dev *pdev)
Signature · 1 line
1.8.3.1
author maintainer
#3 lore raw

PATCH 3/8 bnxt_en: Remove unwanted ovs-offload messages in some conditions

In reply to Michael Chan

From: Sriharsha Basavapatna <[email protected]>

In some conditions when the driver fails to add a flow in HW and returns an error back to the stack, the stack continues to invoke get_flow_stats() and/or del_flow() on it. The driver fails these APIs with an error message "no flow_node for cookie". The message gets logged repeatedly as long as the stack keeps invoking these functions.

Fix this by removing the corresponding netdev_info() calls from these functions.

Fixes d7bc73053024 ("bnxt_en: add code to query TC flower offload stats")
Signed-off-by Sriharsha Basavapatna <[email protected]>
Signed-off-by Michael Chan <[email protected]>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)
drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c +2 -8
index fbe6e20..2049d43 100644
@@ -1269,11 +1269,8 @@ static int bnxt_tc_del_flow(struct bnxt *bp,
 	flow_node = rhashtable_lookup_fast(&tc_info->flow_table,
 					   &tc_flow_cmd->cookie,
 					   tc_info->flow_ht_params);
-	if (!flow_node) {
-		netdev_info(bp->dev, "ERROR: no flow_node for cookie %lx",
-			    tc_flow_cmd->cookie);
+	if (!flow_node)
 		return -EINVAL;
-	}
 
 	return __bnxt_tc_del_flow(bp, flow_node);
 }

@@ -1290,11 +1287,8 @@ static int bnxt_tc_get_flow_stats(struct bnxt *bp,
 	flow_node = rhashtable_lookup_fast(&tc_info->flow_table,
 					   &tc_flow_cmd->cookie,
 					   tc_info->flow_ht_params);
-	if (!flow_node) {
-		netdev_info(bp->dev, "Error: no flow_node for cookie %lx",
-			    tc_flow_cmd->cookie);
+	if (!flow_node)
 		return -1;
-	}
 
 	flow = &flow_node->flow;
 	curr_stats = &flow->stats;
Signature · 1 line
1.8.3.1
author maintainer
#4 lore raw

PATCH 4/8 bnxt_en: Pass complete VLAN TCI to the stack.

In reply to Michael Chan

When receiving a packet with VLAN tag, pass the entire 16-bit TCI to the stack when calling __vlan_hwaccel_put_tag(). The current code is only passing the 12-bit tag and it is missing the priority bits.

Signed-off-by Michael Chan <[email protected]>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 4 ++--
 drivers/net/ethernet/broadcom/bnxt/bnxt.h | 1 +
 2 files changed, 3 insertions(+), 2 deletions(-)
drivers/net/ethernet/broadcom/bnxt/bnxt.c +2 -2
index b847d54..35099c8 100644
@@ -1439,7 +1439,7 @@ static inline struct sk_buff *bnxt_tpa_end(struct bnxt *bp,
 	    (skb->dev->features & NETIF_F_HW_VLAN_CTAG_RX)) {
 		u16 vlan_proto = tpa_info->metadata >>
 			RX_CMP_FLAGS2_METADATA_TPID_SFT;
-		u16 vtag = tpa_info->metadata & RX_CMP_FLAGS2_METADATA_VID_MASK;
+		u16 vtag = tpa_info->metadata & RX_CMP_FLAGS2_METADATA_TCI_MASK;
 
 		__vlan_hwaccel_put_tag(skb, htons(vlan_proto), vtag);
 	}

@@ -1623,7 +1623,7 @@ static int bnxt_rx_pkt(struct bnxt *bp, struct bnxt_napi *bnapi, u32 *raw_cons,
 	     cpu_to_le32(RX_CMP_FLAGS2_META_FORMAT_VLAN)) &&
 	    (skb->dev->features & NETIF_F_HW_VLAN_CTAG_RX)) {
 		u32 meta_data = le32_to_cpu(rxcmp1->rx_cmp_meta_data);
-		u16 vtag = meta_data & RX_CMP_FLAGS2_METADATA_VID_MASK;
+		u16 vtag = meta_data & RX_CMP_FLAGS2_METADATA_TCI_MASK;
 		u16 vlan_proto = meta_data >> RX_CMP_FLAGS2_METADATA_TPID_SFT;
 
 		__vlan_hwaccel_put_tag(skb, htons(vlan_proto), vtag);
drivers/net/ethernet/broadcom/bnxt/bnxt.h +1 -0
index 1989c47..5e3d621 100644
@@ -189,6 +189,7 @@ struct rx_cmp_ext {
 	#define RX_CMP_FLAGS2_T_L4_CS_CALC			(0x1 << 3)
 	#define RX_CMP_FLAGS2_META_FORMAT_VLAN			(0x1 << 4)
 	__le32 rx_cmp_meta_data;
+	#define RX_CMP_FLAGS2_METADATA_TCI_MASK			0xffff
 	#define RX_CMP_FLAGS2_METADATA_VID_MASK			0xfff
 	#define RX_CMP_FLAGS2_METADATA_TPID_MASK		0xffff0000
 	 #define RX_CMP_FLAGS2_METADATA_TPID_SFT		 16
Signature · 1 line
1.8.3.1
author maintainer
#5 lore raw

PATCH 5/8 bnxt_en: Fix regressions when setting up MQPRIO TX rings.

In reply to Michael Chan

Recent changes added the bnxt_init_int_mode() call in the driver's open path whenever ring reservations are changed. This call was previously only called in the probe path. In the open path, if MQPRIO TC has been setup, the bnxt_init_int_mode() call would reset and mess up the MQPRIO per TC rings.

Fix it by not re-initilizing bp->tx_nr_rings_per_tc in bnxt_init_int_mode(). Instead, initialize it in the probe path only after the bnxt_init_int_mode() call.

Fixes 674f50a5b026 ("bnxt_en: Implement new method to reserve rings.")
Signed-off-by Michael Chan <[email protected]>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
drivers/net/ethernet/broadcom/bnxt/bnxt.c +5 -2
index 35099c8..cbdb54f 100644
@@ -5857,7 +5857,6 @@ static int bnxt_init_msix(struct bnxt *bp)
 		if (rc)
 			goto msix_setup_exit;
 
-		bp->tx_nr_rings_per_tc = bp->tx_nr_rings;
 		bp->cp_nr_rings = (min == 1) ?
 				  max_t(int, bp->tx_nr_rings, bp->rx_nr_rings) :
 				  bp->tx_nr_rings + bp->rx_nr_rings;

@@ -5889,7 +5888,6 @@ static int bnxt_init_inta(struct bnxt *bp)
 	bp->rx_nr_rings = 1;
 	bp->tx_nr_rings = 1;
 	bp->cp_nr_rings = 1;
-	bp->tx_nr_rings_per_tc = bp->tx_nr_rings;
 	bp->flags |= BNXT_FLAG_SHARED_RINGS;
 	bp->irq_tbl[0].vector = bp->pdev->irq;
 	return 0;

@@ -8661,6 +8659,11 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (rc)
 		goto init_err_pci_clean;
 
+	/* No TC has been set yet and rings may have been trimmed due to
+	 * limited MSIX, so we re-initialize the TX rings per TC.
+	 */
+	bp->tx_nr_rings_per_tc = bp->tx_nr_rings;
+
 	bnxt_get_wol_settings(bp);
 	if (bp->flags & BNXT_FLAG_WOL_CAP)
 		device_set_wakeup_enable(&pdev->dev, bp->wol);
Signature · 1 line
1.8.3.1
author maintainer
#6 lore raw

PATCH 6/8 bnxt_en: Return standard Linux error codes for hwrm flow cmds.

In reply to Michael Chan

From: Venkat Duvvuru <[email protected]>

Currently, internal error value is returned by the driver, when hwrm_cfa_flow_alloc() fails due lack of resources. We should be returning Linux errno value -ENOSPC instead.

This patch also converts other similar command errors to standard Linux errno code (-EIO) in bnxt_tc.c

Fixes db1d36a27324 ("bnxt_en: add TC flower offload flow_alloc/free FW cmds")
Signed-off-by Venkat Duvvuru <[email protected]>
Signed-off-by Michael Chan <[email protected]>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)
drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c +20 -3
index 2049d43..65c2cee 100644
@@ -349,6 +349,9 @@ static int bnxt_hwrm_cfa_flow_free(struct bnxt *bp, __le16 flow_handle)
 	if (rc)
 		netdev_info(bp->dev, "Error: %s: flow_handle=0x%x rc=%d",
 			    __func__, flow_handle, rc);
+
+	if (rc)
+		rc = -EIO;
 	return rc;
 }
 

@@ -484,13 +487,15 @@ static int bnxt_hwrm_cfa_flow_alloc(struct bnxt *bp, struct bnxt_tc_flow *flow,
 	req.action_flags = cpu_to_le16(action_flags);
 
 	mutex_lock(&bp->hwrm_cmd_lock);
-
 	rc = _hwrm_send_message(bp, &req, sizeof(req), HWRM_CMD_TIMEOUT);
 	if (!rc)
 		*flow_handle = resp->flow_handle;
-
 	mutex_unlock(&bp->hwrm_cmd_lock);
 
+	if (rc == HWRM_ERR_CODE_RESOURCE_ALLOC_ERROR)
+		rc = -ENOSPC;
+	else if (rc)
+		rc = -EIO;
 	return rc;
 }
 

@@ -561,6 +566,8 @@ static int hwrm_cfa_decap_filter_alloc(struct bnxt *bp,
 		netdev_info(bp->dev, "%s: Error rc=%d", __func__, rc);
 	mutex_unlock(&bp->hwrm_cmd_lock);
 
+	if (rc)
+		rc = -EIO;
 	return rc;
 }
 

@@ -576,6 +583,9 @@ static int hwrm_cfa_decap_filter_free(struct bnxt *bp,
 	rc = hwrm_send_message(bp, &req, sizeof(req), HWRM_CMD_TIMEOUT);
 	if (rc)
 		netdev_info(bp->dev, "%s: Error rc=%d", __func__, rc);
+
+	if (rc)
+		rc = -EIO;
 	return rc;
 }
 

@@ -624,6 +634,8 @@ static int hwrm_cfa_encap_record_alloc(struct bnxt *bp,
 		netdev_info(bp->dev, "%s: Error rc=%d", __func__, rc);
 	mutex_unlock(&bp->hwrm_cmd_lock);
 
+	if (rc)
+		rc = -EIO;
 	return rc;
 }
 

@@ -639,6 +651,9 @@ static int hwrm_cfa_encap_record_free(struct bnxt *bp,
 	rc = hwrm_send_message(bp, &req, sizeof(req), HWRM_CMD_TIMEOUT);
 	if (rc)
 		netdev_info(bp->dev, "%s: Error rc=%d", __func__, rc);
+
+	if (rc)
+		rc = -EIO;
 	return rc;
 }
 

@@ -1338,8 +1353,10 @@ static int bnxt_tc_get_flow_stats(struct bnxt *bp,
 	} else {
 		netdev_info(bp->dev, "error rc=%d", rc);
 	}
-
 	mutex_unlock(&bp->hwrm_cmd_lock);
+
+	if (rc)
+		rc = -EIO;
 	return rc;
 }
Signature · 1 line
1.8.3.1
author maintainer
#7 lore raw

PATCH 7/8 bnxt_en: close & open NIC, only when the interface is in running state.

In reply to Michael Chan

From: Venkat Duvvuru <[email protected]>

bnxt_restore_pf_fw_resources routine frees PF resources by calling close_nic and allocates the resources back, by doing open_nic. However, this is not needed, if the PF is already in closed state.

This bug causes the driver to call open the device and call request_irq() when it is not needed. Ultimately, pci_disable_msix() will crash when bnxt_en is unloaded.

This patch fixes the problem by skipping __bnxt_close_nic and __bnxt_open_nic inside bnxt_restore_pf_fw_resources routine, if the interface is not running.

Fixes 80fcaf46c092 ("bnxt_en: Restore MSIX after disabling SRIOV.")
Signed-off-by Venkat Duvvuru <[email protected]>
Signed-off-by Michael Chan <[email protected]>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)
drivers/net/ethernet/broadcom/bnxt/bnxt.c +12 -5
index cbdb54f..cc9569f 100644
@@ -8432,13 +8432,20 @@ int bnxt_restore_pf_fw_resources(struct bnxt *bp)
 		return 0;
 
 	bnxt_hwrm_func_qcaps(bp);
-	__bnxt_close_nic(bp, true, false);
+
+	if (netif_running(bp->dev))
+		__bnxt_close_nic(bp, true, false);
+
 	bnxt_clear_int_mode(bp);
 	rc = bnxt_init_int_mode(bp);
-	if (rc)
-		dev_close(bp->dev);
-	else
-		rc = bnxt_open_nic(bp, true, false);
+
+	if (netif_running(bp->dev)) {
+		if (rc)
+			dev_close(bp->dev);
+		else
+			rc = bnxt_open_nic(bp, true, false);
+	}
+
 	return rc;
 }
Signature · 1 line
1.8.3.1
author maintainer
#8 lore raw

PATCH 8/8 bnxt_en: Check valid VNIC ID in bnxt_hwrm_vnic_set_tpa().

In reply to Michael Chan

During initialization, if we encounter errors, there is a code path that calls bnxt_hwrm_vnic_set_tpa() with invalid VNIC ID. This may cause a warning in firmware logs.

Fixes c0c050c58d84 ("bnxt_en: New Broadcom ethernet driver.")
Signed-off-by Michael Chan <[email protected]>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/net/ethernet/broadcom/bnxt/bnxt.c +3 -0
index cc9569f..c7e5e6f 100644
@@ -3847,6 +3847,9 @@ static int bnxt_hwrm_vnic_set_tpa(struct bnxt *bp, u16 vnic_id, u32 tpa_flags)
 	struct bnxt_vnic_info *vnic = &bp->vnic_info[vnic_id];
 	struct hwrm_vnic_tpa_cfg_input req = {0};
 
+	if (vnic->fw_vnic_id == INVALID_HW_RING_ID)
+		return 0;
+
 	bnxt_hwrm_cmd_hdr_init(bp, &req, HWRM_VNIC_TPA_CFG, -1, -1);
 
 	if (tpa_flags) {
Signature · 1 line
1.8.3.1
maintainer
#9 lore raw
In reply to Michael Chan

From: Michael Chan <[email protected]>
Date: Fri, 9 Mar 2018 23:46:02 -0500

Quoted 7 lines

There are 3 bug fixes in this series to fix regressions recently introduced when adding the new ring reservations scheme. 2 minor fixes in the TC Flower code to return standard errno values and to elide some unnecessary warning dmesg. One Fixes the VLAN TCI value passed to the stack by including the entire 16-bit VLAN TCI, and the last fix is to check for valid VNIC ID before setting up or shutting down LRO/GRO.

Series applied, thanks Michael.