[net PATCH v2 0/7] bnx2x: bug fixes for net

[net PATCH v2 0/7] bnx2x: bug fixes for net

[Yuval Mintz]

Hi Dave,

This patch series contains various bug fixes for the bnx2x driver -
about half of them are link related, while the other either add
code missing in previous git commits or fix erroneous flows.

Changes from v1:
----------------
  -  Patch 06/07 - update returned version to 1 (from 0), when changing
     the registered dumped. Corrected patch description.

Please consider applying this patch series to 'net'.

Thanks,
Yuval Mintz

[net PATCH v2 1/7] bnx2x: Avoid sending multiple statistics queries

[Yuval Mintz]

From: Dmitry Kravkov <dmitry@broadcom.com>

During traffic when DCB is enabled, it is possible for multiple instances
of statistics queries to be sent to the chip - this may cause the FW to assert.

This patch prevents the sending of an additional instance of statistics query
while the previous query hasn't completed.

Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
Signed-off-by: Yuval Mintz <yuvalmin@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c
index 332db64..d848dc9 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c
@@ -1151,9 +1151,11 @@ static void bnx2x_stats_update(struct bnx2x *bp)
 	if (bp->port.pmf)
 		bnx2x_hw_stats_update(bp);
 
-	if (bnx2x_storm_stats_update(bp) && (bp->stats_pending++ == 3)) {
-		BNX2X_ERR("storm stats were not updated for 3 times\n");
-		bnx2x_panic();
+	if (bnx2x_storm_stats_update(bp)) {
+		if (bp->stats_pending++ == 3) {
+			BNX2X_ERR("storm stats were not updated for 3 times\n");
+			bnx2x_panic();
+		}
 		return;
 	}
 
-- 
1.7.9.rc2

[net PATCH v2 2/7] bnx2x: fix stats copying logic

[Yuval Mintz]

FW needs the driver statistics for management. Current logic is broken
in that the function that gathers the port statistics does not copy
its own statistics to a place where the FW can use it.
This patch causes every function that can pass statistics to the FW to
do so.

Signed-off-by: Yuval Mintz <yuvalmin@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c
index d848dc9..a1d0446 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c
@@ -101,6 +101,11 @@ static void bnx2x_hw_stats_post(struct bnx2x *bp)
 	if (CHIP_REV_IS_SLOW(bp))
 		return;
 
+	/* Update MCP's statistics if possible */
+	if (bp->func_stx)
+		memcpy(bnx2x_sp(bp, func_stats), &bp->func_stats,
+		       sizeof(bp->func_stats));
+
 	/* loader */
 	if (bp->executer_idx) {
 		int loader_idx = PMF_DMAE_C(bp);
@@ -128,8 +133,6 @@ static void bnx2x_hw_stats_post(struct bnx2x *bp)
 
 	} else if (bp->func_stx) {
 		*stats_comp = 0;
-		memcpy(bnx2x_sp(bp, func_stats), &bp->func_stats,
-		       sizeof(bp->func_stats));
 		bnx2x_post_dmae(bp, dmae, INIT_DMAE_C(bp));
 	}
 }
-- 
1.7.9.rc2

[net PATCH v2 3/7] bnx2x: prevent timeouts when using PFC

[Yuval Mintz]

From: Yaniv Rosner <yaniv.rosner@broadcom.com>

Prevent updating the xmac PFC configuration when using a link speed
slower than 10G -the umac block is responsible for 1G or slower connections,
therefore it is possible the xmac block is reset when connection is slower.

Signed-off-by: Yaniv Rosner <yaniv.rosner@broadcom.com>
Signed-off-by: Yuval Mintz <yuvalmin@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c
index f4beb46..40a7b8d 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c
@@ -2667,9 +2667,11 @@ int bnx2x_update_pfc(struct link_params *params,
 		return bnx2x_status;
 
 	DP(NETIF_MSG_LINK, "About to update PFC in BMAC\n");
-	if (CHIP_IS_E3(bp))
-		bnx2x_update_pfc_xmac(params, vars, 0);
-	else {
+
+	if (CHIP_IS_E3(bp)) {
+		if (vars->mac_type == MAC_TYPE_XMAC)
+			bnx2x_update_pfc_xmac(params, vars, 0);
+	} else {
 		val = REG_RD(bp, MISC_REG_RESET_REG_2);
 		if ((val &
 		     (MISC_REGISTERS_RESET_REG_2_RST_BMAC0 << params->port))
-- 
1.7.9.rc2

[net PATCH v2 4/7] bnx2x: display the correct duplex value

[Yuval Mintz]

From: Yaniv Rosner <yaniv.rosner@broadcom.com>

Prior to this fix, the driver reported the chip's active duplex state
is always 'full', even if using half-duplex mode.

Signed-off-by: Yaniv Rosner <yaniv.rosner@broadcom.com>
Signed-off-by: Yuval Mintz <yuvalmin@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c
index 40a7b8d..b046beb 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c
@@ -5434,7 +5434,7 @@ static int bnx2x_get_link_speed_duplex(struct bnx2x_phy *phy,
 		switch (speed_mask) {
 		case GP_STATUS_10M:
 			vars->line_speed = SPEED_10;
-			if (vars->duplex == DUPLEX_FULL)
+			if (is_duplex == DUPLEX_FULL)
 				vars->link_status |= LINK_10TFD;
 			else
 				vars->link_status |= LINK_10THD;
@@ -5442,7 +5442,7 @@ static int bnx2x_get_link_speed_duplex(struct bnx2x_phy *phy,
 
 		case GP_STATUS_100M:
 			vars->line_speed = SPEED_100;
-			if (vars->duplex == DUPLEX_FULL)
+			if (is_duplex == DUPLEX_FULL)
 				vars->link_status |= LINK_100TXFD;
 			else
 				vars->link_status |= LINK_100TXHD;
@@ -5451,7 +5451,7 @@ static int bnx2x_get_link_speed_duplex(struct bnx2x_phy *phy,
 		case GP_STATUS_1G:
 		case GP_STATUS_1G_KX:
 			vars->line_speed = SPEED_1000;
-			if (vars->duplex == DUPLEX_FULL)
+			if (is_duplex == DUPLEX_FULL)
 				vars->link_status |= LINK_1000TFD;
 			else
 				vars->link_status |= LINK_1000THD;
@@ -5459,7 +5459,7 @@ static int bnx2x_get_link_speed_duplex(struct bnx2x_phy *phy,
 
 		case GP_STATUS_2_5G:
 			vars->line_speed = SPEED_2500;
-			if (vars->duplex == DUPLEX_FULL)
+			if (is_duplex == DUPLEX_FULL)
 				vars->link_status |= LINK_2500TFD;
 			else
 				vars->link_status |= LINK_2500THD;
@@ -5533,6 +5533,7 @@ static int bnx2x_link_settings_status(struct bnx2x_phy *phy,
 
 	if (gp_status & MDIO_GP_STATUS_TOP_AN_STATUS1_LINK_STATUS) {
 		if (SINGLE_MEDIA_DIRECT(params)) {
+			vars->duplex = duplex;
 			bnx2x_flow_ctrl_resolve(phy, params, vars, gp_status);
 			if (phy->req_line_speed == SPEED_AUTO_NEG)
 				bnx2x_xgxs_an_resolve(phy, params, vars,
@@ -5627,6 +5628,7 @@ static int bnx2x_warpcore_read_status(struct bnx2x_phy *phy,
 					LINK_STATUS_PARALLEL_DETECTION_USED;
 			}
 			bnx2x_ext_phy_resolve_fc(phy, params, vars);
+			vars->duplex = duplex;
 		}
 	}
 
-- 
1.7.9.rc2

[net PATCH v2 5/7] bnx2x: correct advertisement of pause capabilities

[Yuval Mintz]

From: Yaniv Rosner <yaniv.rosner@broadcom.com>

This patch propagates users' requested flow-control into the link layer,
which will later be used to advertise this flow-control for auto-negotiation
(until now these values were ignored).

Signed-off-by: Yaniv Rosner <yaniv.rosner@broadcom.com>
Signed-off-by: Yuval Mintz <yuvalmin@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
 .../net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c    |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
index c37a68d..bbf4cf0 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
@@ -1587,6 +1587,12 @@ static int bnx2x_set_pauseparam(struct net_device *dev,
 			bp->link_params.req_flow_ctrl[cfg_idx] =
 				BNX2X_FLOW_CTRL_AUTO;
 		}
+		bp->link_params.req_fc_auto_adv = BNX2X_FLOW_CTRL_NONE;
+		if (epause->rx_pause)
+			bp->link_params.req_fc_auto_adv |= BNX2X_FLOW_CTRL_RX;
+
+		if (epause->tx_pause)
+			bp->link_params.req_fc_auto_adv |= BNX2X_FLOW_CTRL_TX;
 	}
 
 	DP(BNX2X_MSG_ETHTOOL,
-- 
1.7.9.rc2

[net PATCH v2 6/7] bnx2x: fix registers dumped

[Yuval Mintz]

From: Dmitry Kravkov <dmitry@broadcom.com>

Under traffic, there are several registers that when read (e.g., via
'ethtool -d') may cause the chip to stall.
This patch corrects the registers read in such flows.

Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
Signed-off-by: Yuval Mintz <yuvalmin@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_dump.h   |   25 ++++++-------------
 .../net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c    |    2 +-
 2 files changed, 9 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_dump.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_dump.h
index 3e4cff9..b926f58 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_dump.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_dump.h
@@ -401,11 +401,11 @@ static const struct reg_addr reg_addrs[] = {
 	{ 0x70000, 8, RI_ALL_ONLINE },
 	{ 0x70020, 8184, RI_ALL_OFFLINE },
 	{ 0x78000, 8192, RI_E3E3B0_OFFLINE },
-	{ 0x85000, 3, RI_ALL_ONLINE },
-	{ 0x8501c, 7, RI_ALL_ONLINE },
-	{ 0x85048, 1, RI_ALL_ONLINE },
-	{ 0x85200, 32, RI_ALL_ONLINE },
-	{ 0xb0000, 16384, RI_E1H_ONLINE },
+	{ 0x85000, 3, RI_ALL_OFFLINE },
+	{ 0x8501c, 7, RI_ALL_OFFLINE },
+	{ 0x85048, 1, RI_ALL_OFFLINE },
+	{ 0x85200, 32, RI_ALL_OFFLINE },
+	{ 0xb0000, 16384, RI_E1H_OFFLINE },
 	{ 0xc1000, 7, RI_ALL_ONLINE },
 	{ 0xc103c, 2, RI_E2E3E3B0_ONLINE },
 	{ 0xc1800, 2, RI_ALL_ONLINE },
@@ -581,17 +581,12 @@ static const struct reg_addr reg_addrs[] = {
 	{ 0x140188, 3, RI_E1E1HE2E3_ONLINE },
 	{ 0x140194, 13, RI_ALL_ONLINE },
 	{ 0x140200, 6, RI_E1E1HE2E3_ONLINE },
-	{ 0x140220, 4, RI_E2E3_ONLINE },
-	{ 0x140240, 4, RI_E2E3_ONLINE },
 	{ 0x140260, 4, RI_E2E3_ONLINE },
 	{ 0x140280, 4, RI_E2E3_ONLINE },
-	{ 0x1402a0, 4, RI_E2E3_ONLINE },
-	{ 0x1402c0, 4, RI_E2E3_ONLINE },
 	{ 0x1402e0, 2, RI_E2E3_ONLINE },
 	{ 0x1402e8, 2, RI_E2E3E3B0_ONLINE },
 	{ 0x1402f0, 9, RI_E2E3_ONLINE },
 	{ 0x140314, 44, RI_E3B0_ONLINE },
-	{ 0x1403d0, 70, RI_E3B0_ONLINE },
 	{ 0x144000, 4, RI_E1E1H_ONLINE },
 	{ 0x148000, 4, RI_E1E1H_ONLINE },
 	{ 0x14c000, 4, RI_E1E1H_ONLINE },
@@ -704,7 +699,6 @@ static const struct reg_addr reg_addrs[] = {
 	{ 0x180398, 1, RI_E2E3E3B0_ONLINE },
 	{ 0x1803a0, 5, RI_E2E3E3B0_ONLINE },
 	{ 0x1803b4, 2, RI_E3E3B0_ONLINE },
-	{ 0x180400, 1, RI_ALL_ONLINE },
 	{ 0x180404, 255, RI_E1E1H_OFFLINE },
 	{ 0x181000, 4, RI_ALL_ONLINE },
 	{ 0x181010, 1020, RI_ALL_OFFLINE },
@@ -800,9 +794,9 @@ static const struct reg_addr reg_addrs[] = {
 	{ 0x1b905c, 1, RI_E3E3B0_ONLINE },
 	{ 0x1b9064, 1, RI_E3B0_ONLINE },
 	{ 0x1b9080, 10, RI_E3B0_ONLINE },
-	{ 0x1b9400, 14, RI_E2E3E3B0_ONLINE },
-	{ 0x1b943c, 19, RI_E2E3E3B0_ONLINE },
-	{ 0x1b9490, 10, RI_E2E3E3B0_ONLINE },
+	{ 0x1b9400, 14, RI_E2E3E3B0_OFFLINE },
+	{ 0x1b943c, 19, RI_E2E3E3B0_OFFLINE },
+	{ 0x1b9490, 10, RI_E2E3E3B0_OFFLINE },
 	{ 0x1c0000, 2, RI_ALL_ONLINE },
 	{ 0x200000, 65, RI_ALL_ONLINE },
 	{ 0x20014c, 2, RI_E1HE2E3E3B0_ONLINE },
@@ -814,7 +808,6 @@ static const struct reg_addr reg_addrs[] = {
 	{ 0x200398, 1, RI_E2E3E3B0_ONLINE },
 	{ 0x2003a0, 1, RI_E2E3E3B0_ONLINE },
 	{ 0x2003a8, 2, RI_E2E3E3B0_ONLINE },
-	{ 0x200400, 1, RI_ALL_ONLINE },
 	{ 0x200404, 255, RI_E1E1H_OFFLINE },
 	{ 0x202000, 4, RI_ALL_ONLINE },
 	{ 0x202010, 2044, RI_ALL_OFFLINE },
@@ -921,7 +914,6 @@ static const struct reg_addr reg_addrs[] = {
 	{ 0x280398, 1, RI_E2E3E3B0_ONLINE },
 	{ 0x2803a0, 1, RI_E2E3E3B0_ONLINE },
 	{ 0x2803a8, 2, RI_E2E3E3B0_ONLINE },
-	{ 0x280400, 1, RI_ALL_ONLINE },
 	{ 0x280404, 255, RI_E1E1H_OFFLINE },
 	{ 0x282000, 4, RI_ALL_ONLINE },
 	{ 0x282010, 2044, RI_ALL_OFFLINE },
@@ -1031,7 +1023,6 @@ static const struct reg_addr reg_addrs[] = {
 	{ 0x300398, 1, RI_E2E3E3B0_ONLINE },
 	{ 0x3003a0, 1, RI_E2E3E3B0_ONLINE },
 	{ 0x3003a8, 2, RI_E2E3E3B0_ONLINE },
-	{ 0x300400, 1, RI_ALL_ONLINE },
 	{ 0x300404, 255, RI_E1E1H_OFFLINE },
 	{ 0x302000, 4, RI_ALL_ONLINE },
 	{ 0x302010, 2044, RI_ALL_OFFLINE },
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
index bbf4cf0..ebf40cd 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
@@ -775,7 +775,7 @@ static void bnx2x_get_regs(struct net_device *dev,
 	struct bnx2x *bp = netdev_priv(dev);
 	struct dump_hdr dump_hdr = {0};
 
-	regs->version = 0;
+	regs->version = 1;
 	memset(p, 0, regs->len);
 
 	if (!netif_running(bp->dev))
-- 
1.7.9.rc2

[net PATCH v2 7/7] bnx2x: Add missing afex code

[Yuval Mintz]

Commit a334872224a67b614dc888460377862621f3dac7 added afex support but lacked
several logical changes. This lack can cause afex to crash, and also
have a slight effect on other flows (i.e., driver always assumes the Tx ring
has less available buffers than what it actually has).

This patch adds the missing segments, fixing said issues.

Signed-off-by: Yuval Mintz <yuvalmin@broadcom.com>
Signed-off-by: Barak Witkowski <barak@broadcom.com>
Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h  |   11 +++++------
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c |   21 ++++++++++++++++-----
 2 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h
index 21b5532..dfd86a5 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h
@@ -710,17 +710,15 @@ static inline u16 bnx2x_tx_avail(struct bnx2x *bp,
 	prod = txdata->tx_bd_prod;
 	cons = txdata->tx_bd_cons;
 
-	/* NUM_TX_RINGS = number of "next-page" entries
-	   It will be used as a threshold */
-	used = SUB_S16(prod, cons) + (s16)NUM_TX_RINGS;
+	used = SUB_S16(prod, cons);
 
 #ifdef BNX2X_STOP_ON_ERROR
 	WARN_ON(used < 0);
-	WARN_ON(used > bp->tx_ring_size);
-	WARN_ON((bp->tx_ring_size - used) > MAX_TX_AVAIL);
+	WARN_ON(used > txdata->tx_ring_size);
+	WARN_ON((txdata->tx_ring_size - used) > MAX_TX_AVAIL);
 #endif
 
-	return (s16)(bp->tx_ring_size) - used;
+	return (s16)(txdata->tx_ring_size) - used;
 }
 
 static inline int bnx2x_tx_queue_has_work(struct bnx2x_fp_txdata *txdata)
@@ -1088,6 +1086,7 @@ static inline void bnx2x_init_txdata(struct bnx2x *bp,
 	txdata->txq_index = txq_index;
 	txdata->tx_cons_sb = tx_cons_sb;
 	txdata->parent_fp = fp;
+	txdata->tx_ring_size = IS_FCOE_FP(fp) ? MAX_TX_AVAIL : bp->tx_ring_size;
 
 	DP(NETIF_MSG_IFUP, "created tx data cid %d, txq %d\n",
 	   txdata->cid, txdata->txq_index);
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index 2105498..211753e 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -7561,8 +7561,14 @@ int bnx2x_set_mac_one(struct bnx2x *bp, u8 *mac,
 	}
 
 	rc = bnx2x_config_vlan_mac(bp, &ramrod_param);
-	if (rc < 0)
+
+	if (rc == -EEXIST) {
+		DP(BNX2X_MSG_SP, "Failed to schedule ADD operations: %d\n", rc);
+		/* do not treat adding same MAC as error */
+		rc = 0;
+	} else if (rc < 0)
 		BNX2X_ERR("%s MAC failed\n", (set ? "Set" : "Del"));
+
 	return rc;
 }
 
@@ -10294,13 +10300,11 @@ static void __devinit bnx2x_get_fcoe_info(struct bnx2x *bp)
 				dev_info.port_hw_config[port].
 				 fcoe_wwn_node_name_lower);
 	} else if (!IS_MF_SD(bp)) {
-		u32 cfg = MF_CFG_RD(bp, func_ext_config[func].func_cfg);
-
 		/*
 		 * Read the WWN info only if the FCoE feature is enabled for
 		 * this function.
 		 */
-		if (cfg & MACP_FUNC_CFG_FLAGS_FCOE_OFFLOAD)
+		if (BNX2X_MF_EXT_PROTOCOL_FCOE(bp) && !CHIP_IS_E1x(bp))
 			bnx2x_get_ext_wwn_info(bp, func);
 
 	} else if (IS_MF_FCOE_SD(bp))
@@ -11073,7 +11077,14 @@ static int bnx2x_set_uc_list(struct bnx2x *bp)
 	netdev_for_each_uc_addr(ha, dev) {
 		rc = bnx2x_set_mac_one(bp, bnx2x_uc_addr(ha), mac_obj, true,
 				       BNX2X_UC_LIST_MAC, &ramrod_flags);
-		if (rc < 0) {
+		if (rc == -EEXIST) {
+			DP(BNX2X_MSG_SP,
+			   "Failed to schedule ADD operations: %d\n", rc);
+			/* do not treat adding same MAC as error */
+			rc = 0;
+
+		} else if (rc < 0) {
+
 			BNX2X_ERR("Failed to schedule ADD operations: %d\n",
 				  rc);
 			return rc;
-- 
1.7.9.rc2

Re: [net PATCH v2 1/7] bnx2x: Avoid sending multiple statistics queries

[David Miller]

From: "Yuval Mintz" <yuvalmin@broadcom.com>
Date: Tue, 11 Sep 2012 17:34:08 +0300

> From: Dmitry Kravkov <dmitry@broadcom.com>
> 
> During traffic when DCB is enabled, it is possible for multiple instances
> of statistics queries to be sent to the chip - this may cause the FW to assert.
> 
> This patch prevents the sending of an additional instance of statistics query
> while the previous query hasn't completed.
> 
> Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
> Signed-off-by: Yuval Mintz <yuvalmin@broadcom.com>
> Signed-off-by: Eilon Greenstein <eilong@broadcom.com>

This change results in no change in behavior as far as I can tell.

> -	if (bnx2x_storm_stats_update(bp) && (bp->stats_pending++ == 3)) {
> -		BNX2X_ERR("storm stats were not updated for 3 times\n");
> -		bnx2x_panic();
> +	if (bnx2x_storm_stats_update(bp)) {
> +		if (bp->stats_pending++ == 3) {
> +			BNX2X_ERR("storm stats were not updated for 3 times\n");
> +			bnx2x_panic();
> +		}

There is no difference between:

	if (A && B) {
		C;
	}

and:

	if (A) {
		if (B) {
			C;
		}
	}

Yet that's exactly what is happening in this patch.

And such a do-nothing change is certainly not appropriate this late in
the -rc series.

I'm tossing this entire series, please sort this out and submit
the real actual critical bug fixes.

RE: [net PATCH v2 1/7] bnx2x: Avoid sending multiple statistics queries

[Yuval Mintz]

> > This patch prevents the sending of an additional instance of statistics
> query
> > while the previous query hasn't completed.
> >
> This change results in no change in behavior as far as I can tell.
> 
> > -	if (bnx2x_storm_stats_update(bp) && (bp->stats_pending++ == 3)) {
> > -		BNX2X_ERR("storm stats were not updated for 3 times\n");
> > -		bnx2x_panic();
> > +	if (bnx2x_storm_stats_update(bp)) {
> > +		if (bp->stats_pending++ == 3) {
> > +			BNX2X_ERR("storm stats were not updated for 3
> times\n");
> > +			bnx2x_panic();
> > +		}

But you're missing the 'return;' statement at the end.

> 
> There is no difference between:
> 
> 	if (A && B) {
> 		C;
> 	}
> 
> and:
> 
> 	if (A) {
> 		if (B) {
> 			C;
> 		}
> 	}

But there is a difference between:
If (A && B) {
	C;
	D;
}

And:
if (A) {
	if (B) {
		C;
	}
	D;
}

The point of this patch was not to change the condition of the print & panic but rather to guarantee that
if bnx2x_storm_stats_update failed, no more ramrods will be posted, as the function will return 
(regardless of stats_pending value).

Sorry it wasn't clearer in the patch.

RE: [net PATCH v2 1/7] bnx2x: Avoid sending multiple statistics queries

[Dmitry Kravkov]

> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Thursday, September 13, 2012 11:17 PM
> To: Yuval Mintz
> Cc: netdev@vger.kernel.org; Dmitry Kravkov; Eilon Greenstein
> Subject: Re: [net PATCH v2 1/7] bnx2x: Avoid sending multiple statistics queries
> 
> From: "Yuval Mintz" <yuvalmin@broadcom.com>
> Date: Tue, 11 Sep 2012 17:34:08 +0300
> 
> > From: Dmitry Kravkov <dmitry@broadcom.com>
> >
> > During traffic when DCB is enabled, it is possible for multiple instances
> > of statistics queries to be sent to the chip - this may cause the FW to assert.
> >
> > This patch prevents the sending of an additional instance of statistics query
> > while the previous query hasn't completed.
> >
> > Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
> > Signed-off-by: Yuval Mintz <yuvalmin@broadcom.com>
> > Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
> 
> This change results in no change in behavior as far as I can tell.
> 
> > -	if (bnx2x_storm_stats_update(bp) && (bp->stats_pending++ == 3)) {
> > -		BNX2X_ERR("storm stats were not updated for 3 times\n");
> > -		bnx2x_panic();
> > +	if (bnx2x_storm_stats_update(bp)) {
> > +		if (bp->stats_pending++ == 3) {
> > +			BNX2X_ERR("storm stats were not updated for 3
> times\n");
> > +			bnx2x_panic();
> > +		}
> 
> There is no difference between:
> 
> 	if (A && B) {
> 		C;
> 	}
> 
> and:
> 
> 	if (A) {
> 		if (B) {
> 			C;
> 		}
> 	}
> 
> Yet that's exactly what is happening in this patch.
> 
> And such a do-nothing change is certainly not appropriate this late in
> the -rc series.
> 
> I'm tossing this entire series, please sort this out and submit
> the real actual critical bug fixes.
return statement is not seen in the patch:
Before the change we returned from the function if (A &&B)
Now we return even if (A)

Re: [net PATCH v2 1/7] bnx2x: Avoid sending multiple statistics queries

[David Miller]

From: "Yuval Mintz" <yuvalmin@broadcom.com>
Date: Thu, 13 Sep 2012 20:34:57 +0000

> But there is a difference between:
> If (A && B) {
> 	C;
> 	D;
> }
> 
> And:
> if (A) {
> 	if (B) {
> 		C;
> 	}
> 	D;
> }

Indeed, I missed the return, it make a lot more sense now.

I've applied this series to 'net' and will push it out after
build testing.

Thanks.