[dpdk-dev] [PATCH 0/4] GCC8 fixes for MLX4/5/BNX2X PMD

[dpdk-dev] [PATCH 0/4] GCC8 fixes for MLX4/5/BNX2X PMD

[Andy Green]

The following series fixes build breakage if you
additionally enable

CONFIG_RTE_LIBRTE_MLX4_PMD
CONFIG_RTE_LIBRTE_MLX5_PMD
CONFIG_RTE_LIBRTE_BNX2X_PMD

---

Andy Green (4):
      net/bnx2x: do not cast function pointers as a policy
      net/bnx2x: remove unmeetable comparison
      net/mlx5: solve var may be used uninitialized
      net/bnx2x: solve overruns


 drivers/net/bnx2x/bnx2x.c |    4 -
 drivers/net/bnx2x/elink.c |  298 +++++++++++++++++++++++----------------------
 drivers/net/mlx5/mlx5.c   |    2 
 3 files changed, 152 insertions(+), 152 deletions(-)

--
Signature

[dpdk-dev] [PATCH 1/4] net/bnx2x: do not cast function pointers as a policy

[Andy Green]

This is stopping the compiler telling you when you have
done something stupid... that is something none of us
can afford...

Now gcc 8.x can tell you did something stupid despite
trying to hide the evidence.

Remove all the "black magic" casts.

Fix the actual problems.
---
 drivers/net/bnx2x/elink.c |  294 +++++++++++++++++++++++----------------------
 1 file changed, 148 insertions(+), 146 deletions(-)

diff --git a/drivers/net/bnx2x/elink.c b/drivers/net/bnx2x/elink.c
index 9d0f31364..99684a5f9 100644
--- a/drivers/net/bnx2x/elink.c
+++ b/drivers/net/bnx2x/elink.c
@@ -4143,9 +4143,9 @@ static void elink_sfp_e3_set_transmitter(struct elink_params *params,
 		elink_set_cfg_pin(sc, cfg_pin + 3, tx_en ^ 1);
 }
 
-static void elink_warpcore_config_init(struct elink_phy *phy,
-				       struct elink_params *params,
-				       struct elink_vars *vars)
+static uint8_t elink_warpcore_config_init(struct elink_phy *phy,
+					  struct elink_params *params,
+					  struct elink_vars *vars)
 {
 	struct bnx2x_softc *sc = params->sc;
 	uint32_t serdes_net_if;
@@ -4222,7 +4222,7 @@ static void elink_warpcore_config_init(struct elink_phy *phy,
 		case PORT_HW_CFG_NET_SERDES_IF_DXGXS:
 			if (vars->line_speed != ELINK_SPEED_20000) {
 				PMD_DRV_LOG(DEBUG, "Speed not supported yet");
-				return;
+				return 0;
 			}
 			PMD_DRV_LOG(DEBUG, "Setting 20G DXGXS");
 			elink_warpcore_set_20G_DXGXS(sc, phy, lane);
@@ -4242,13 +4242,15 @@ static void elink_warpcore_config_init(struct elink_phy *phy,
 			PMD_DRV_LOG(DEBUG,
 				    "Unsupported Serdes Net Interface 0x%x",
 				    serdes_net_if);
-			return;
+			return 0;
 		}
 	}
 
 	/* Take lane out of reset after configuration is finished */
 	elink_warpcore_reset_lane(sc, phy, 0);
 	PMD_DRV_LOG(DEBUG, "Exit config init");
+
+	return 0;
 }
 
 static void elink_warpcore_link_reset(struct elink_phy *phy,
@@ -5226,9 +5228,9 @@ static elink_status_t elink_get_link_speed_duplex(struct elink_phy *phy,
 	return ELINK_STATUS_OK;
 }
 
-static elink_status_t elink_link_settings_status(struct elink_phy *phy,
-						 struct elink_params *params,
-						 struct elink_vars *vars)
+static uint8_t elink_link_settings_status(struct elink_phy *phy,
+					  struct elink_params *params,
+					  struct elink_vars *vars)
 {
 	struct bnx2x_softc *sc = params->sc;
 
@@ -5299,9 +5301,9 @@ static elink_status_t elink_link_settings_status(struct elink_phy *phy,
 	return rc;
 }
 
-static elink_status_t elink_warpcore_read_status(struct elink_phy *phy,
-						 struct elink_params *params,
-						 struct elink_vars *vars)
+static uint8_t elink_warpcore_read_status(struct elink_phy *phy,
+					  struct elink_params *params,
+					  struct elink_vars *vars)
 {
 	struct bnx2x_softc *sc = params->sc;
 	uint8_t lane;
@@ -5520,9 +5522,9 @@ static void elink_set_preemphasis(struct elink_phy *phy,
 	}
 }
 
-static void elink_xgxs_config_init(struct elink_phy *phy,
-				   struct elink_params *params,
-				   struct elink_vars *vars)
+static uint8_t elink_xgxs_config_init(struct elink_phy *phy,
+				      struct elink_params *params,
+				      struct elink_vars *vars)
 {
 	uint8_t enable_cl73 = (ELINK_SINGLE_MEDIA_DIRECT(params) ||
 			       (params->loopback_mode == ELINK_LOOPBACK_XGXS));
@@ -5567,6 +5569,8 @@ static void elink_xgxs_config_init(struct elink_phy *phy,
 
 		elink_initialize_sgmii_process(phy, params, vars);
 	}
+
+	return 0;
 }
 
 static elink_status_t elink_prepare_xgxs(struct elink_phy *phy,
@@ -5751,8 +5755,8 @@ static void elink_link_int_ack(struct elink_params *params,
 	}
 }
 
-static elink_status_t elink_format_ver(uint32_t num, uint8_t * str,
-				       uint16_t * len)
+static uint8_t elink_format_ver(uint32_t num, uint8_t * str,
+				uint16_t * len)
 {
 	uint8_t *str_ptr = str;
 	uint32_t mask = 0xf0000000;
@@ -5790,8 +5794,8 @@ static elink_status_t elink_format_ver(uint32_t num, uint8_t * str,
 	return ELINK_STATUS_OK;
 }
 
-static elink_status_t elink_null_format_ver(__rte_unused uint32_t spirom_ver,
-					    uint8_t * str, uint16_t * len)
+static uint8_t elink_null_format_ver(__rte_unused uint32_t spirom_ver,
+				     uint8_t * str, uint16_t * len)
 {
 	str[0] = '\0';
 	(*len)--;
@@ -6802,9 +6806,9 @@ static void elink_8073_specific_func(struct elink_phy *phy,
 	}
 }
 
-static elink_status_t elink_8073_config_init(struct elink_phy *phy,
-					     struct elink_params *params,
-					     struct elink_vars *vars)
+static uint8_t elink_8073_config_init(struct elink_phy *phy,
+				      struct elink_params *params,
+				      struct elink_vars *vars)
 {
 	struct bnx2x_softc *sc = params->sc;
 	uint16_t val = 0, tmp1;
@@ -7097,9 +7101,9 @@ static void elink_8073_link_reset(__rte_unused struct elink_phy *phy,
 /******************************************************************/
 /*			BNX2X8705 PHY SECTION			  */
 /******************************************************************/
-static elink_status_t elink_8705_config_init(struct elink_phy *phy,
-					     struct elink_params *params,
-					     __rte_unused struct elink_vars
+static uint8_t elink_8705_config_init(struct elink_phy *phy,
+				      struct elink_params *params,
+				      __rte_unused struct elink_vars
 					     *vars)
 {
 	struct bnx2x_softc *sc = params->sc;
@@ -8403,9 +8407,9 @@ static uint8_t elink_8706_config_init(struct elink_phy *phy,
 	return ELINK_STATUS_OK;
 }
 
-static elink_status_t elink_8706_read_status(struct elink_phy *phy,
-					     struct elink_params *params,
-					     struct elink_vars *vars)
+static uint8_t elink_8706_read_status(struct elink_phy *phy,
+				      struct elink_params *params,
+				      struct elink_vars *vars)
 {
 	return elink_8706_8726_read_status(phy, params, vars);
 }
@@ -8477,9 +8481,9 @@ static uint8_t elink_8726_read_status(struct elink_phy *phy,
 	return link_up;
 }
 
-static elink_status_t elink_8726_config_init(struct elink_phy *phy,
-					     struct elink_params *params,
-					     struct elink_vars *vars)
+static uint8_t elink_8726_config_init(struct elink_phy *phy,
+				      struct elink_params *params,
+				      struct elink_vars *vars)
 {
 	struct bnx2x_softc *sc = params->sc;
 	PMD_DRV_LOG(DEBUG, "Initializing BNX2X8726");
@@ -8684,9 +8688,9 @@ static void elink_8727_config_speed(struct elink_phy *phy,
 	}
 }
 
-static elink_status_t elink_8727_config_init(struct elink_phy *phy,
-					     struct elink_params *params,
-					     __rte_unused struct elink_vars
+static uint8_t elink_8727_config_init(struct elink_phy *phy,
+				      struct elink_params *params,
+				      __rte_unused struct elink_vars
 					     *vars)
 {
 	uint32_t tx_en_mode;
@@ -9291,7 +9295,7 @@ static elink_status_t elink_848xx_cmn_config_init(struct elink_phy *phy,
 	return ELINK_STATUS_OK;
 }
 
-static elink_status_t elink_8481_config_init(struct elink_phy *phy,
+static uint8_t elink_8481_config_init(struct elink_phy *phy,
 					     struct elink_params *params,
 					     struct elink_vars *vars)
 {
@@ -9442,8 +9446,8 @@ static uint8_t elink_84833_get_reset_gpios(struct bnx2x_softc *sc,
 	return reset_gpios;
 }
 
-static elink_status_t elink_84833_hw_reset_phy(struct elink_phy *phy,
-					       struct elink_params *params)
+static void elink_84833_hw_reset_phy(struct elink_phy *phy,
+					struct elink_params *params)
 {
 	struct bnx2x_softc *sc = params->sc;
 	uint8_t reset_gpios;
@@ -9471,8 +9475,6 @@ static elink_status_t elink_84833_hw_reset_phy(struct elink_phy *phy,
 				 MISC_REGISTERS_GPIO_OUTPUT_LOW);
 	DELAY(10);
 	PMD_DRV_LOG(DEBUG, "84833 hw reset on pin values 0x%x", reset_gpios);
-
-	return ELINK_STATUS_OK;
 }
 
 static elink_status_t elink_8483x_disable_eee(struct elink_phy *phy,
@@ -9513,9 +9515,9 @@ static elink_status_t elink_8483x_enable_eee(struct elink_phy *phy,
 }
 
 #define PHY84833_CONSTANT_LATENCY 1193
-static elink_status_t elink_848x3_config_init(struct elink_phy *phy,
-					      struct elink_params *params,
-					      struct elink_vars *vars)
+static uint8_t elink_848x3_config_init(struct elink_phy *phy,
+				       struct elink_params *params,
+				       struct elink_vars *vars)
 {
 	struct bnx2x_softc *sc = params->sc;
 	uint8_t port, initialize = 1;
@@ -9819,7 +9821,7 @@ static uint8_t elink_848xx_read_status(struct elink_phy *phy,
 	return link_up;
 }
 
-static elink_status_t elink_848xx_format_ver(uint32_t raw_ver, uint8_t * str,
+static uint8_t elink_848xx_format_ver(uint32_t raw_ver, uint8_t * str,
 					     uint16_t * len)
 {
 	elink_status_t status = ELINK_STATUS_OK;
@@ -10146,9 +10148,9 @@ static void elink_54618se_specific_func(struct elink_phy *phy,
 	}
 }
 
-static elink_status_t elink_54618se_config_init(struct elink_phy *phy,
-						struct elink_params *params,
-						struct elink_vars *vars)
+static uint8_t elink_54618se_config_init(struct elink_phy *phy,
+					 struct elink_params *params,
+					 struct elink_vars *vars)
 {
 	struct bnx2x_softc *sc = params->sc;
 	uint8_t port;
@@ -10542,9 +10544,9 @@ static void elink_7101_config_loopback(struct elink_phy *phy,
 			 MDIO_XS_DEVAD, MDIO_XS_SFX7101_XGXS_TEST1, 0x100);
 }
 
-static elink_status_t elink_7101_config_init(struct elink_phy *phy,
-					     struct elink_params *params,
-					     struct elink_vars *vars)
+static uint8_t elink_7101_config_init(struct elink_phy *phy,
+				      struct elink_params *params,
+				      struct elink_vars *vars)
 {
 	uint16_t fw_ver1, fw_ver2, val;
 	struct bnx2x_softc *sc = params->sc;
@@ -10614,8 +10616,8 @@ static uint8_t elink_7101_read_status(struct elink_phy *phy,
 	return link_up;
 }
 
-static elink_status_t elink_7101_format_ver(uint32_t spirom_ver, uint8_t * str,
-					    uint16_t * len)
+static uint8_t elink_7101_format_ver(uint32_t spirom_ver, uint8_t * str,
+				     uint16_t * len)
 {
 	if (*len < 5)
 		return ELINK_STATUS_ERROR;
@@ -10785,14 +10787,14 @@ static const struct elink_phy phy_warpcore = {
 	.speed_cap_mask = 0,
 	/* req_duplex = */ 0,
 	/* rsrv = */ 0,
-	.config_init = (config_init_t) elink_warpcore_config_init,
-	.read_status = (read_status_t) elink_warpcore_read_status,
-	.link_reset = (link_reset_t) elink_warpcore_link_reset,
-	.config_loopback = (config_loopback_t) elink_set_warpcore_loopback,
-	.format_fw_ver = (format_fw_ver_t) NULL,
-	.hw_reset = (hw_reset_t) elink_warpcore_hw_reset,
-	.set_link_led = (set_link_led_t) NULL,
-	.phy_specific_func = (phy_specific_func_t) NULL
+	.config_init = elink_warpcore_config_init,
+	.read_status = elink_warpcore_read_status,
+	.link_reset = elink_warpcore_link_reset,
+	.config_loopback = elink_set_warpcore_loopback,
+	.format_fw_ver = NULL,
+	.hw_reset = elink_warpcore_hw_reset,
+	.set_link_led = NULL,
+	.phy_specific_func = NULL
 };
 
 static const struct elink_phy phy_7101 = {
@@ -10814,14 +10816,14 @@ static const struct elink_phy phy_7101 = {
 	.speed_cap_mask = 0,
 	.req_duplex = 0,
 	.rsrv = 0,
-	.config_init = (config_init_t) elink_7101_config_init,
-	.read_status = (read_status_t) elink_7101_read_status,
-	.link_reset = (link_reset_t) elink_common_ext_link_reset,
-	.config_loopback = (config_loopback_t) elink_7101_config_loopback,
-	.format_fw_ver = (format_fw_ver_t) elink_7101_format_ver,
-	.hw_reset = (hw_reset_t) elink_7101_hw_reset,
-	.set_link_led = (set_link_led_t) elink_7101_set_link_led,
-	.phy_specific_func = (phy_specific_func_t) NULL
+	.config_init = elink_7101_config_init,
+	.read_status = elink_7101_read_status,
+	.link_reset = elink_common_ext_link_reset,
+	.config_loopback = elink_7101_config_loopback,
+	.format_fw_ver = elink_7101_format_ver,
+	.hw_reset = elink_7101_hw_reset,
+	.set_link_led = elink_7101_set_link_led,
+	.phy_specific_func = NULL
 };
 
 static const struct elink_phy phy_8073 = {
@@ -10845,14 +10847,14 @@ static const struct elink_phy phy_8073 = {
 	.speed_cap_mask = 0,
 	.req_duplex = 0,
 	.rsrv = 0,
-	.config_init = (config_init_t) elink_8073_config_init,
-	.read_status = (read_status_t) elink_8073_read_status,
-	.link_reset = (link_reset_t) elink_8073_link_reset,
-	.config_loopback = (config_loopback_t) NULL,
-	.format_fw_ver = (format_fw_ver_t) elink_format_ver,
-	.hw_reset = (hw_reset_t) NULL,
-	.set_link_led = (set_link_led_t) NULL,
-	.phy_specific_func = (phy_specific_func_t) elink_8073_specific_func
+	.config_init = elink_8073_config_init,
+	.read_status = elink_8073_read_status,
+	.link_reset = elink_8073_link_reset,
+	.config_loopback = NULL,
+	.format_fw_ver = elink_format_ver,
+	.hw_reset = NULL,
+	.set_link_led = NULL,
+	.phy_specific_func = elink_8073_specific_func
 };
 
 static const struct elink_phy phy_8705 = {
@@ -10873,14 +10875,14 @@ static const struct elink_phy phy_8705 = {
 	.speed_cap_mask = 0,
 	.req_duplex = 0,
 	.rsrv = 0,
-	.config_init = (config_init_t) elink_8705_config_init,
-	.read_status = (read_status_t) elink_8705_read_status,
-	.link_reset = (link_reset_t) elink_common_ext_link_reset,
-	.config_loopback = (config_loopback_t) NULL,
-	.format_fw_ver = (format_fw_ver_t) elink_null_format_ver,
-	.hw_reset = (hw_reset_t) NULL,
-	.set_link_led = (set_link_led_t) NULL,
-	.phy_specific_func = (phy_specific_func_t) NULL
+	.config_init = elink_8705_config_init,
+	.read_status = elink_8705_read_status,
+	.link_reset = elink_common_ext_link_reset,
+	.config_loopback = NULL,
+	.format_fw_ver = elink_null_format_ver,
+	.hw_reset = NULL,
+	.set_link_led = NULL,
+	.phy_specific_func = NULL
 };
 
 static const struct elink_phy phy_8706 = {
@@ -10902,14 +10904,14 @@ static const struct elink_phy phy_8706 = {
 	.speed_cap_mask = 0,
 	.req_duplex = 0,
 	.rsrv = 0,
-	.config_init = (config_init_t) elink_8706_config_init,
-	.read_status = (read_status_t) elink_8706_read_status,
-	.link_reset = (link_reset_t) elink_common_ext_link_reset,
-	.config_loopback = (config_loopback_t) NULL,
-	.format_fw_ver = (format_fw_ver_t) elink_format_ver,
-	.hw_reset = (hw_reset_t) NULL,
-	.set_link_led = (set_link_led_t) NULL,
-	.phy_specific_func = (phy_specific_func_t) NULL
+	.config_init = elink_8706_config_init,
+	.read_status = elink_8706_read_status,
+	.link_reset = elink_common_ext_link_reset,
+	.config_loopback = NULL,
+	.format_fw_ver = elink_format_ver,
+	.hw_reset = NULL,
+	.set_link_led = NULL,
+	.phy_specific_func = NULL
 };
 
 static const struct elink_phy phy_8726 = {
@@ -10932,14 +10934,14 @@ static const struct elink_phy phy_8726 = {
 	.speed_cap_mask = 0,
 	.req_duplex = 0,
 	.rsrv = 0,
-	.config_init = (config_init_t) elink_8726_config_init,
-	.read_status = (read_status_t) elink_8726_read_status,
-	.link_reset = (link_reset_t) elink_8726_link_reset,
-	.config_loopback = (config_loopback_t) elink_8726_config_loopback,
-	.format_fw_ver = (format_fw_ver_t) elink_format_ver,
-	.hw_reset = (hw_reset_t) NULL,
-	.set_link_led = (set_link_led_t) NULL,
-	.phy_specific_func = (phy_specific_func_t) NULL
+	.config_init = elink_8726_config_init,
+	.read_status = elink_8726_read_status,
+	.link_reset = elink_8726_link_reset,
+	.config_loopback = elink_8726_config_loopback,
+	.format_fw_ver = elink_format_ver,
+	.hw_reset = NULL,
+	.set_link_led = NULL,
+	.phy_specific_func = NULL
 };
 
 static const struct elink_phy phy_8727 = {
@@ -10961,14 +10963,14 @@ static const struct elink_phy phy_8727 = {
 	.speed_cap_mask = 0,
 	.req_duplex = 0,
 	.rsrv = 0,
-	.config_init = (config_init_t) elink_8727_config_init,
-	.read_status = (read_status_t) elink_8727_read_status,
-	.link_reset = (link_reset_t) elink_8727_link_reset,
-	.config_loopback = (config_loopback_t) NULL,
-	.format_fw_ver = (format_fw_ver_t) elink_format_ver,
-	.hw_reset = (hw_reset_t) elink_8727_hw_reset,
-	.set_link_led = (set_link_led_t) elink_8727_set_link_led,
-	.phy_specific_func = (phy_specific_func_t) elink_8727_specific_func
+	.config_init = elink_8727_config_init,
+	.read_status = elink_8727_read_status,
+	.link_reset = elink_8727_link_reset,
+	.config_loopback = NULL,
+	.format_fw_ver = elink_format_ver,
+	.hw_reset = elink_8727_hw_reset,
+	.set_link_led = elink_8727_set_link_led,
+	.phy_specific_func = elink_8727_specific_func
 };
 
 static const struct elink_phy phy_8481 = {
@@ -10996,14 +10998,14 @@ static const struct elink_phy phy_8481 = {
 	.speed_cap_mask = 0,
 	.req_duplex = 0,
 	.rsrv = 0,
-	.config_init = (config_init_t) elink_8481_config_init,
-	.read_status = (read_status_t) elink_848xx_read_status,
-	.link_reset = (link_reset_t) elink_8481_link_reset,
-	.config_loopback = (config_loopback_t) NULL,
-	.format_fw_ver = (format_fw_ver_t) elink_848xx_format_ver,
-	.hw_reset = (hw_reset_t) elink_8481_hw_reset,
-	.set_link_led = (set_link_led_t) elink_848xx_set_link_led,
-	.phy_specific_func = (phy_specific_func_t) NULL
+	.config_init = elink_8481_config_init,
+	.read_status = elink_848xx_read_status,
+	.link_reset = elink_8481_link_reset,
+	.config_loopback = NULL,
+	.format_fw_ver = elink_848xx_format_ver,
+	.hw_reset = elink_8481_hw_reset,
+	.set_link_led = elink_848xx_set_link_led,
+	.phy_specific_func = NULL
 };
 
 static const struct elink_phy phy_84823 = {
@@ -11031,14 +11033,14 @@ static const struct elink_phy phy_84823 = {
 	.speed_cap_mask = 0,
 	.req_duplex = 0,
 	.rsrv = 0,
-	.config_init = (config_init_t) elink_848x3_config_init,
-	.read_status = (read_status_t) elink_848xx_read_status,
-	.link_reset = (link_reset_t) elink_848x3_link_reset,
-	.config_loopback = (config_loopback_t) NULL,
-	.format_fw_ver = (format_fw_ver_t) elink_848xx_format_ver,
-	.hw_reset = (hw_reset_t) NULL,
-	.set_link_led = (set_link_led_t) elink_848xx_set_link_led,
-	.phy_specific_func = (phy_specific_func_t) elink_848xx_specific_func
+	.config_init = elink_848x3_config_init,
+	.read_status = elink_848xx_read_status,
+	.link_reset = elink_848x3_link_reset,
+	.config_loopback = NULL,
+	.format_fw_ver = elink_848xx_format_ver,
+	.hw_reset = NULL,
+	.set_link_led = elink_848xx_set_link_led,
+	.phy_specific_func = elink_848xx_specific_func
 };
 
 static const struct elink_phy phy_84833 = {
@@ -11065,14 +11067,14 @@ static const struct elink_phy phy_84833 = {
 	.speed_cap_mask = 0,
 	.req_duplex = 0,
 	.rsrv = 0,
-	.config_init = (config_init_t) elink_848x3_config_init,
-	.read_status = (read_status_t) elink_848xx_read_status,
-	.link_reset = (link_reset_t) elink_848x3_link_reset,
-	.config_loopback = (config_loopback_t) NULL,
-	.format_fw_ver = (format_fw_ver_t) elink_848xx_format_ver,
-	.hw_reset = (hw_reset_t) elink_84833_hw_reset_phy,
-	.set_link_led = (set_link_led_t) elink_848xx_set_link_led,
-	.phy_specific_func = (phy_specific_func_t) elink_848xx_specific_func
+	.config_init = elink_848x3_config_init,
+	.read_status = elink_848xx_read_status,
+	.link_reset = elink_848x3_link_reset,
+	.config_loopback = NULL,
+	.format_fw_ver = elink_848xx_format_ver,
+	.hw_reset = elink_84833_hw_reset_phy,
+	.set_link_led = elink_848xx_set_link_led,
+	.phy_specific_func = elink_848xx_specific_func
 };
 
 static const struct elink_phy phy_84834 = {
@@ -11098,14 +11100,14 @@ static const struct elink_phy phy_84834 = {
 	.speed_cap_mask = 0,
 	.req_duplex = 0,
 	.rsrv = 0,
-	.config_init = (config_init_t) elink_848x3_config_init,
-	.read_status = (read_status_t) elink_848xx_read_status,
-	.link_reset = (link_reset_t) elink_848x3_link_reset,
-	.config_loopback = (config_loopback_t) NULL,
-	.format_fw_ver = (format_fw_ver_t) elink_848xx_format_ver,
-	.hw_reset = (hw_reset_t) elink_84833_hw_reset_phy,
-	.set_link_led = (set_link_led_t) elink_848xx_set_link_led,
-	.phy_specific_func = (phy_specific_func_t) elink_848xx_specific_func
+	.config_init = elink_848x3_config_init,
+	.read_status = elink_848xx_read_status,
+	.link_reset = elink_848x3_link_reset,
+	.config_loopback = NULL,
+	.format_fw_ver = elink_848xx_format_ver,
+	.hw_reset = elink_84833_hw_reset_phy,
+	.set_link_led = elink_848xx_set_link_led,
+	.phy_specific_func = elink_848xx_specific_func
 };
 
 static const struct elink_phy phy_54618se = {
@@ -11131,14 +11133,14 @@ static const struct elink_phy phy_54618se = {
 	.speed_cap_mask = 0,
 	/* req_duplex = */ 0,
 	/* rsrv = */ 0,
-	.config_init = (config_init_t) elink_54618se_config_init,
-	.read_status = (read_status_t) elink_54618se_read_status,
-	.link_reset = (link_reset_t) elink_54618se_link_reset,
-	.config_loopback = (config_loopback_t) elink_54618se_config_loopback,
-	.format_fw_ver = (format_fw_ver_t) NULL,
-	.hw_reset = (hw_reset_t) NULL,
-	.set_link_led = (set_link_led_t) elink_5461x_set_link_led,
-	.phy_specific_func = (phy_specific_func_t) elink_54618se_specific_func
+	.config_init = elink_54618se_config_init,
+	.read_status = elink_54618se_read_status,
+	.link_reset = elink_54618se_link_reset,
+	.config_loopback = elink_54618se_config_loopback,
+	.format_fw_ver = NULL,
+	.hw_reset = NULL,
+	.set_link_led = elink_5461x_set_link_led,
+	.phy_specific_func = elink_54618se_specific_func
 };
 
 /*****************************************************************/

Re: [dpdk-dev] [PATCH 1/4] net/bnx2x: do not cast function pointers as a policy

[Ferruh Yigit]

On 5/11/2018 2:51 AM, Andy Green wrote:
> This is stopping the compiler telling you when you have
> done something stupid... that is something none of us
> can afford...
> 
> Now gcc 8.x can tell you did something stupid despite
> trying to hide the evidence.
> 
> Remove all the "black magic" casts.
> 
> Fix the actual problems.

Missing sign-off

<...>

> @@ -5226,9 +5228,9 @@ static elink_status_t elink_get_link_speed_duplex(struct elink_phy *phy,
>  	return ELINK_STATUS_OK;
>  }
>  
> -static elink_status_t elink_link_settings_status(struct elink_phy *phy,
> -						 struct elink_params *params,
> -						 struct elink_vars *vars)
> +static uint8_t elink_link_settings_status(struct elink_phy *phy,
> +					  struct elink_params *params,
> +					  struct elink_vars *vars)
>  {

It is possible to remove "read_status_t" casting from phy_serdes and phy_xgxs
after this change.

<...>

> @@ -5520,9 +5522,9 @@ static void elink_set_preemphasis(struct elink_phy *phy,
>  	}
>  }
>  
> -static void elink_xgxs_config_init(struct elink_phy *phy,
> -				   struct elink_params *params,
> -				   struct elink_vars *vars)
> +static uint8_t elink_xgxs_config_init(struct elink_phy *phy,
> +				      struct elink_params *params,
> +				      struct elink_vars *vars)

Same here, this change can eliminate some "config_init_t" casts.

Re: [dpdk-dev] [PATCH 1/4] net/bnx2x: do not cast function pointers as a policy

[Andy Green]

On 05/12/2018 12:03 AM, Ferruh Yigit wrote:
> On 5/11/2018 2:51 AM, Andy Green wrote:
>> This is stopping the compiler telling you when you have
>> done something stupid... that is something none of us
>> can afford...
>>
>> Now gcc 8.x can tell you did something stupid despite
>> trying to hide the evidence.
>>
>> Remove all the "black magic" casts.
>>
>> Fix the actual problems.
> 
> Missing sign-off
> 
> <...>
> 
>> @@ -5226,9 +5228,9 @@ static elink_status_t elink_get_link_speed_duplex(struct elink_phy *phy,
>>   	return ELINK_STATUS_OK;
>>   }
>>   
>> -static elink_status_t elink_link_settings_status(struct elink_phy *phy,
>> -						 struct elink_params *params,
>> -						 struct elink_vars *vars)
>> +static uint8_t elink_link_settings_status(struct elink_phy *phy,
>> +					  struct elink_params *params,
>> +					  struct elink_vars *vars)
>>   {
> 
> It is possible to remove "read_status_t" casting from phy_serdes and phy_xgxs
> after this change.
> 
> <...>
> 
>> @@ -5520,9 +5522,9 @@ static void elink_set_preemphasis(struct elink_phy *phy,
>>   	}
>>   }
>>   
>> -static void elink_xgxs_config_init(struct elink_phy *phy,
>> -				   struct elink_params *params,
>> -				   struct elink_vars *vars)
>> +static uint8_t elink_xgxs_config_init(struct elink_phy *phy,
>> +				      struct elink_params *params,
>> +				      struct elink_vars *vars)
> 
> Same here, this change can eliminate some "config_init_t" casts.
> 

The first errors from the compile came after phy_serdes and phy_xgxs 
definition, so I did not notice they also have "black magic casts" 
(casting NULL amongst others...) ... I removed all the casts on those as 
well and build is still happy.

-Andy

Re: [dpdk-dev] [PATCH 1/4] net/bnx2x: do not cast function pointers as a policy

[De Lara Guarch, Pablo]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Andy Green
> Sent: Friday, May 11, 2018 2:51 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 1/4] net/bnx2x: do not cast function pointers as a
> policy
> 
> This is stopping the compiler telling you when you have done something stupid...
> that is something none of us can afford...
> 
> Now gcc 8.x can tell you did something stupid despite trying to hide the
> evidence.
> 
> Remove all the "black magic" casts.
> 
> Fix the actual problems.
> ---
>  drivers/net/bnx2x/elink.c |  294 +++++++++++++++++++++++----------------------

Missing signed-off-by line, fixes line and Cc stable:

Fixes: b5bf7719221d ("bnx2x: driver support routines")
Cc: stable@dpdk.org

[dpdk-dev] [PATCH 2/4] net/bnx2x: remove unmeetable comparison

[Andy Green]

/home/agreen/projects/dpdk/drivers/net/bnx2x/elink.c:
In function ‘elink_check_kr2_wa’:
/home/agreen/projects/dpdk/drivers/net/bnx2x/elink.c:12922:28:
error: bitwise comparison always evaluates to false
[-Werror=tautological-compare]
        ((next_page & 0xe0) == 0x2))));
---
 drivers/net/bnx2x/elink.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/bnx2x/elink.c b/drivers/net/bnx2x/elink.c
index 99684a5f9..2aef2b6da 100644
--- a/drivers/net/bnx2x/elink.c
+++ b/drivers/net/bnx2x/elink.c
@@ -12919,9 +12919,7 @@ static void elink_check_kr2_wa(struct elink_params *params,
 	 * but only KX is advertised, declare this link partner as non-KR2
 	 * device.
 	 */
-	not_kr2_device = (((base_page & 0x8000) == 0) ||
-			  (((base_page & 0x8000) &&
-			    ((next_page & 0xe0) == 0x2))));
+	not_kr2_device = !(base_page & 0x8000);
 
 	/* In case KR2 is already disabled, check if we need to re-enable it */
 	if (!(vars->link_attr_sync & LINK_ATTR_SYNC_KR2_ENABLE)) {

Re: [dpdk-dev] [PATCH 2/4] net/bnx2x: remove unmeetable comparison

[De Lara Guarch, Pablo]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Andy Green
> Sent: Friday, May 11, 2018 2:51 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 2/4] net/bnx2x: remove unmeetable comparison
> 
> /home/agreen/projects/dpdk/drivers/net/bnx2x/elink.c:
> In function ‘elink_check_kr2_wa’:
> /home/agreen/projects/dpdk/drivers/net/bnx2x/elink.c:12922:28:
> error: bitwise comparison always evaluates to false [-Werror=tautological-
> compare]
>         ((next_page & 0xe0) == 0x2))));
> ---
>  drivers/net/bnx2x/elink.c |    4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/net/bnx2x/elink.c b/drivers/net/bnx2x/elink.c index
> 99684a5f9..2aef2b6da 100644
> --- a/drivers/net/bnx2x/elink.c
> +++ b/drivers/net/bnx2x/elink.c
> @@ -12919,9 +12919,7 @@ static void elink_check_kr2_wa(struct elink_params
> *params,
>  	 * but only KX is advertised, declare this link partner as non-KR2
>  	 * device.
>  	 */
> -	not_kr2_device = (((base_page & 0x8000) == 0) ||
> -			  (((base_page & 0x8000) &&
> -			    ((next_page & 0xe0) == 0x2))));
> +	not_kr2_device = !(base_page & 0x8000);
> 
>  	/* In case KR2 is already disabled, check if we need to re-enable it */
>  	if (!(vars->link_attr_sync & LINK_ATTR_SYNC_KR2_ENABLE)) {

Looks like a good fix, but I wonder what's the actual intention of that conditional.
CC'ing the maintainers and Stephen Hemminger (the author of this code) to figure it out.

Apart from that, missing signed-off-by, fixes line and Cc stable:

Fixes: b5bf7719221d ("bnx2x: driver support routines")
Cc: stable@dpdk.org

Re: [dpdk-dev] [PATCH 2/4] net/bnx2x: remove unmeetable comparison

[Stephen Hemminger]

On Fri, 11 May 2018 16:32:24 +0000
"De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com> wrote:

> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Andy Green
> > Sent: Friday, May 11, 2018 2:51 AM
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH 2/4] net/bnx2x: remove unmeetable comparison
> > 
> > /home/agreen/projects/dpdk/drivers/net/bnx2x/elink.c:
> > In function ‘elink_check_kr2_wa’:
> > /home/agreen/projects/dpdk/drivers/net/bnx2x/elink.c:12922:28:
> > error: bitwise comparison always evaluates to false [-Werror=tautological-
> > compare]
> >         ((next_page & 0xe0) == 0x2))));
> > ---
> >  drivers/net/bnx2x/elink.c |    4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/drivers/net/bnx2x/elink.c b/drivers/net/bnx2x/elink.c index
> > 99684a5f9..2aef2b6da 100644
> > --- a/drivers/net/bnx2x/elink.c
> > +++ b/drivers/net/bnx2x/elink.c
> > @@ -12919,9 +12919,7 @@ static void elink_check_kr2_wa(struct elink_params
> > *params,
> >  	 * but only KX is advertised, declare this link partner as non-KR2
> >  	 * device.
> >  	 */
> > -	not_kr2_device = (((base_page & 0x8000) == 0) ||
> > -			  (((base_page & 0x8000) &&
> > -			    ((next_page & 0xe0) == 0x2))));
> > +	not_kr2_device = !(base_page & 0x8000);
> > 
> >  	/* In case KR2 is already disabled, check if we need to re-enable it */
> >  	if (!(vars->link_attr_sync & LINK_ATTR_SYNC_KR2_ENABLE)) {  
> 
> Looks like a good fix, but I wonder what's the actual intention of that conditional.
> CC'ing the maintainers and Stephen Hemminger (the author of this code) to figure it out.
> 
> Apart from that, missing signed-off-by, fixes line and Cc stable:
> 
> Fixes: b5bf7719221d ("bnx2x: driver support routines")
> Cc: stable@dpdk.org
> 

Actually, I didn't write this. It came from contractors.  Based of FreeBSD
driver so look there.

Re: [dpdk-dev] [PATCH 2/4] net/bnx2x: remove unmeetable comparison

[Andy Green]

On 05/12/2018 02:10 AM, Stephen Hemminger wrote:
> On Fri, 11 May 2018 16:32:24 +0000
> "De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com> wrote:
> 
>>> -----Original Message-----
>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Andy Green
>>> Sent: Friday, May 11, 2018 2:51 AM
>>> To: dev@dpdk.org
>>> Subject: [dpdk-dev] [PATCH 2/4] net/bnx2x: remove unmeetable comparison
>>>
>>> /home/agreen/projects/dpdk/drivers/net/bnx2x/elink.c:
>>> In function ‘elink_check_kr2_wa’:
>>> /home/agreen/projects/dpdk/drivers/net/bnx2x/elink.c:12922:28:
>>> error: bitwise comparison always evaluates to false [-Werror=tautological-
>>> compare]
>>>          ((next_page & 0xe0) == 0x2))));
>>> ---
>>>   drivers/net/bnx2x/elink.c |    4 +---
>>>   1 file changed, 1 insertion(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/net/bnx2x/elink.c b/drivers/net/bnx2x/elink.c index
>>> 99684a5f9..2aef2b6da 100644
>>> --- a/drivers/net/bnx2x/elink.c
>>> +++ b/drivers/net/bnx2x/elink.c
>>> @@ -12919,9 +12919,7 @@ static void elink_check_kr2_wa(struct elink_params
>>> *params,
>>>   	 * but only KX is advertised, declare this link partner as non-KR2
>>>   	 * device.
>>>   	 */
>>> -	not_kr2_device = (((base_page & 0x8000) == 0) ||
>>> -			  (((base_page & 0x8000) &&
>>> -			    ((next_page & 0xe0) == 0x2))));
>>> +	not_kr2_device = !(base_page & 0x8000);
>>>
>>>   	/* In case KR2 is already disabled, check if we need to re-enable it */
>>>   	if (!(vars->link_attr_sync & LINK_ATTR_SYNC_KR2_ENABLE)) {
>>
>> Looks like a good fix, but I wonder what's the actual intention of that conditional.
>> CC'ing the maintainers and Stephen Hemminger (the author of this code) to figure it out.
>>
>> Apart from that, missing signed-off-by, fixes line and Cc stable:
>>
>> Fixes: b5bf7719221d ("bnx2x: driver support routines")
>> Cc: stable@dpdk.org
>>
> 
> Actually, I didn't write this. It came from contractors.  Based of FreeBSD
> driver so look there.

Googling around, the correct comparison is == 0x20, I updated the patch 
accordingly.

-Andy

[dpdk-dev] [PATCH 3/4] net/mlx5: solve var may be used uninitialized

[Andy Green]

---
 drivers/net/mlx5/mlx5.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 8f983061a..4d379fb13 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -624,7 +624,7 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 	int err = 0;
 	struct ibv_context *attr_ctx = NULL;
 	struct ibv_device_attr_ex device_attr;
-	unsigned int vf;
+	unsigned int vf = 0;
 	unsigned int mps;
 	unsigned int cqe_comp;
 	unsigned int tunnel_en = 0;

Re: [dpdk-dev] [PATCH 3/4] net/mlx5: solve var may be used uninitialized

[De Lara Guarch, Pablo]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Andy Green
> Sent: Friday, May 11, 2018 2:51 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 3/4] net/mlx5: solve var may be used uninitialized
> 
> 
> ---

Missing signed-off-by and fixes line:

Fixes: ccdcba53a3f4 ("net/mlx5: use Netlink to add/remove MAC addresses")

Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>

Re: [dpdk-dev] [PATCH 3/4] net/mlx5: solve var may be used uninitialized

[Shahaf Shuler]

Hi Andy

The patch logic is OK but:
1. since it is a fix patch the title should start with "fix ..."
2. add Fixes tag of the relevant commit and Cc stable if needed 
3. add signed-of-by tag
4. explicitly include the PMD maintainers (Cc'ed now). 

Friday, May 11, 2018 4:51 AM, Andy Green
> Subject: [dpdk-dev] [PATCH 3/4] net/mlx5: solve var may be used
> uninitialized
> 
> 
> ---
>  drivers/net/mlx5/mlx5.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c index
> 8f983061a..4d379fb13 100644
> --- a/drivers/net/mlx5/mlx5.c
> +++ b/drivers/net/mlx5/mlx5.c
> @@ -624,7 +624,7 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv
> __rte_unused,
>  	int err = 0;
>  	struct ibv_context *attr_ctx = NULL;
>  	struct ibv_device_attr_ex device_attr;
> -	unsigned int vf;
> +	unsigned int vf = 0;
>  	unsigned int mps;
>  	unsigned int cqe_comp;
>  	unsigned int tunnel_en = 0;

Re: [dpdk-dev] [PATCH 3/4] net/mlx5: solve var may be used uninitialized

[Andy Green]

On 05/13/2018 12:45 PM, Shahaf Shuler wrote:
> Hi Andy
> 
> The patch logic is OK but:
> 1. since it is a fix patch the title should start with "fix ..."
> 2. add Fixes tag of the relevant commit and Cc stable if needed
> 3. add signed-of-by tag
> 4. explicitly include the PMD maintainers (Cc'ed now).

For a patch adding 4 characters, from a drive-by, it's a lot of overhead.

There's already a V2 of this sent yesterday with the signed-off-by (OK, 
that is not really any overhead) and most of the rest of your stuff 
donated by Pablo.

-Andy

> Friday, May 11, 2018 4:51 AM, Andy Green
>> Subject: [dpdk-dev] [PATCH 3/4] net/mlx5: solve var may be used
>> uninitialized
>>
>>
>> ---
>>   drivers/net/mlx5/mlx5.c |    2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c index
>> 8f983061a..4d379fb13 100644
>> --- a/drivers/net/mlx5/mlx5.c
>> +++ b/drivers/net/mlx5/mlx5.c
>> @@ -624,7 +624,7 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv
>> __rte_unused,
>>   	int err = 0;
>>   	struct ibv_context *attr_ctx = NULL;
>>   	struct ibv_device_attr_ex device_attr;
>> -	unsigned int vf;
>> +	unsigned int vf = 0;
>>   	unsigned int mps;
>>   	unsigned int cqe_comp;
>>   	unsigned int tunnel_en = 0;
> 

[dpdk-dev] [PATCH 4/4] net/bnx2x: solve overruns

[Andy Green]

---
 drivers/net/bnx2x/bnx2x.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bnx2x/bnx2x.c b/drivers/net/bnx2x/bnx2x.c
index bfd9cce51..3892934cc 100644
--- a/drivers/net/bnx2x/bnx2x.c
+++ b/drivers/net/bnx2x/bnx2x.c
@@ -170,10 +170,10 @@ bnx2x_dma_alloc(struct bnx2x_softc *sc, size_t size, struct bnx2x_dma *dma,
 
 	dma->sc = sc;
 	if (IS_PF(sc))
-		sprintf(mz_name, "bnx2x%d_%s_%" PRIx64, SC_ABS_FUNC(sc), msg,
+		snprintf(mz_name, sizeof(mz_name), "bnx2x%d_%s_%" PRIx64, SC_ABS_FUNC(sc), msg,
 			rte_get_timer_cycles());
 	else
-		sprintf(mz_name, "bnx2x%d_%s_%" PRIx64, sc->pcie_device, msg,
+		snprintf(mz_name, sizeof(mz_name), "bnx2x%d_%s_%" PRIx64, sc->pcie_device, msg,
 			rte_get_timer_cycles());
 
 	/* Caller must take care that strlen(mz_name) < RTE_MEMZONE_NAMESIZE */

Re: [dpdk-dev] [PATCH 4/4] net/bnx2x: solve overruns

[De Lara Guarch, Pablo]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Andy Green
> Sent: Friday, May 11, 2018 2:52 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 4/4] net/bnx2x: solve overruns
> 
As Ferruh said, Signed-off-by is missing, plus fixes line and Cc stable

Fixes: 540a211084a7 ("bnx2x: driver core")
Cc: stable@dpdk.org

Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>

Re: [dpdk-dev] [PATCH 0/4] GCC8 fixes for MLX4/5/BNX2X PMD

[Ferruh Yigit]

On 5/11/2018 2:51 AM, Andy Green wrote:
> The following series fixes build breakage if you
> additionally enable
> 
> CONFIG_RTE_LIBRTE_MLX4_PMD
> CONFIG_RTE_LIBRTE_MLX5_PMD
> CONFIG_RTE_LIBRTE_BNX2X_PMD
> 
> ---
> 
> Andy Green (4):
>       net/bnx2x: do not cast function pointers as a policy
>       net/bnx2x: remove unmeetable comparison
>       net/mlx5: solve var may be used uninitialized
>       net/bnx2x: solve overruns

Hi Andy,

Thanks for fixes. Patches are missing your sign-off, can you please send a new
version adding it?