[dpdk-dev] [PATCH 00/10] net/atlantic: bugfixes and code cleanup

[dpdk-dev] [PATCH 00/10] net/atlantic: bugfixes and code cleanup

[Igor Russkikh]

The patchset mainly fixes some Coverity issues,
couple of minor bugs and overflows.

It also improves eeprom get/set logic with ability of custom
device address and dump length.

Igor Russkikh (7):
  net/atlantic: Fix negative error codes
  net/atlantic: remove unused variable
  net/atlantic: check for error codes
  net/atlantic: fix link configuration
  net/atlantic: eliminate excessive log levels on rx/tx
  net/atlantic: fix missing vlan filter offload
  net/atlantic: fix xstats to return correct number of requested items

Pavel Belous (3):
  net/atlantic: fix buffer overflow
  net/atlantic: use eeprom magic as a device address
  net/atlantic: fix eeprom fetching for small and uneven lengths

 drivers/net/atlantic/atl_ethdev.c             | 71 ++++++++++---------
 drivers/net/atlantic/atl_rxtx.c               | 20 ++----
 drivers/net/atlantic/atl_types.h              |  5 +-
 drivers/net/atlantic/hw_atl/hw_atl_b0.c       |  6 +-
 drivers/net/atlantic/hw_atl/hw_atl_utils.c    |  9 +--
 drivers/net/atlantic/hw_atl/hw_atl_utils.h    | 23 +++---
 .../net/atlantic/hw_atl/hw_atl_utils_fw2x.c   | 59 ++++++++++-----
 7 files changed, 105 insertions(+), 88 deletions(-)

-- 
2.17.1

[dpdk-dev] [PATCH 01/10] net/atlantic: Fix negative error codes

[Igor Russkikh]

Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
---
 drivers/net/atlantic/atl_rxtx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/atlantic/atl_rxtx.c b/drivers/net/atlantic/atl_rxtx.c
index 40c913795..aea58c0d5 100644
--- a/drivers/net/atlantic/atl_rxtx.c
+++ b/drivers/net/atlantic/atl_rxtx.c
@@ -812,12 +812,12 @@ atl_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf **tx_pkts,
 		ol_flags = m->ol_flags;
 
 		if (m->nb_segs > AQ_HW_MAX_SEGS_SIZE) {
-			rte_errno = -EINVAL;
+			rte_errno = EINVAL;
 			return i;
 		}
 
 		if (ol_flags & ATL_TX_OFFLOAD_NOTSUP_MASK) {
-			rte_errno = -ENOTSUP;
+			rte_errno = ENOTSUP;
 			return i;
 		}
 
-- 
2.17.1

[dpdk-dev] [PATCH 02/10] net/atlantic: remove unused variable

[Igor Russkikh]

Found by coverity scan

Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
---
 drivers/net/atlantic/hw_atl/hw_atl_b0.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/net/atlantic/hw_atl/hw_atl_b0.c b/drivers/net/atlantic/hw_atl/hw_atl_b0.c
index 9400e0edb..53fd8e9f0 100644
--- a/drivers/net/atlantic/hw_atl/hw_atl_b0.c
+++ b/drivers/net/atlantic/hw_atl/hw_atl_b0.c
@@ -31,7 +31,6 @@ static int hw_atl_b0_hw_qos_set(struct aq_hw_s *self)
 	u32 tc = 0U;
 	u32 buff_size = 0U;
 	unsigned int i_priority = 0U;
-	bool is_rx_flow_control = false;
 
 	/* TPS Descriptor rate init */
 	hw_atl_tps_tx_pkt_shed_desc_rate_curr_time_res_set(self, 0x0U);
@@ -64,7 +63,6 @@ static int hw_atl_b0_hw_qos_set(struct aq_hw_s *self)
 
 	/* QoS Rx buf size per TC */
 	tc = 0;
-	is_rx_flow_control = 0;
 	buff_size = HW_ATL_B0_RXBUF_MAX;
 
 	hw_atl_rpb_rx_pkt_buff_size_per_tc_set(self, buff_size, tc);
@@ -76,9 +74,7 @@ static int hw_atl_b0_hw_qos_set(struct aq_hw_s *self)
 						   (buff_size *
 						   (1024U / 32U) * 50U) /
 						   100U, tc);
-	hw_atl_rpb_rx_xoff_en_per_tc_set(self,
-					 is_rx_flow_control ? 1U : 0U,
-					 tc);
+	hw_atl_rpb_rx_xoff_en_per_tc_set(self, 0U, tc);
 
 	/* QoS 802.1p priority -> TC mapping */
 	for (i_priority = 8U; i_priority--;)
-- 
2.17.1

Re: [dpdk-dev] [PATCH 02/10] net/atlantic: remove unused variable

[Rami Rosen]

Reviewed-by: Rami Rosen <ramirose@gmail.com>


>

[dpdk-dev] [PATCH 03/10] net/atlantic: check for error codes

[Igor Russkikh]

Found by Coverity

Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
---
 drivers/net/atlantic/hw_atl/hw_atl_utils.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/atlantic/hw_atl/hw_atl_utils.c b/drivers/net/atlantic/hw_atl/hw_atl_utils.c
index f11093a50..13f02b9f9 100644
--- a/drivers/net/atlantic/hw_atl/hw_atl_utils.c
+++ b/drivers/net/atlantic/hw_atl/hw_atl_utils.c
@@ -462,8 +462,6 @@ int hw_atl_utils_fw_rpc_wait(struct aq_hw_s *self,
 				goto err_exit;
 		}
 	} while (sw.tid != fw.tid || 0xFFFFU == fw.len);
-	if (err < 0)
-		goto err_exit;
 
 	if (rpc) {
 		if (fw.len) {
@@ -875,8 +873,7 @@ static int aq_fw1x_set_wol(struct aq_hw_s *self, bool wol_enabled, u8 *mac)
 	}
 
 	err = hw_atl_utils_fw_rpc_call(self, rpc_size);
-	if (err < 0)
-		goto err_exit;
+
 err_exit:
 	return err;
 }
-- 
2.17.1

[dpdk-dev] [PATCH 04/10] net/atlantic: fix buffer overflow

[Igor Russkikh]

From: Pavel Belous <Pavel.Belous@aquantia.com>

Found by Coverity

Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
Signed-off-by: Pavel Belous <Pavel.Belous@aquantia.com>
---
 drivers/net/atlantic/hw_atl/hw_atl_utils_fw2x.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/atlantic/hw_atl/hw_atl_utils_fw2x.c b/drivers/net/atlantic/hw_atl/hw_atl_utils_fw2x.c
index 6841d9bce..a155d4aab 100644
--- a/drivers/net/atlantic/hw_atl/hw_atl_utils_fw2x.c
+++ b/drivers/net/atlantic/hw_atl/hw_atl_utils_fw2x.c
@@ -501,7 +501,7 @@ static int aq_fw2x_get_eeprom(struct aq_hw_s *self, u32 *data, u32 len)
 	/* Write SMBUS request to cfg memory */
 	err = hw_atl_utils_fw_upload_dwords(self, self->rpc_addr,
 				(u32 *)(void *)&request,
-				RTE_ALIGN(sizeof(request), sizeof(u32)));
+				RTE_ALIGN(sizeof(request) / sizeof(u32), sizeof(u32)));
 
 	if (err < 0)
 		return err;
@@ -523,7 +523,7 @@ static int aq_fw2x_get_eeprom(struct aq_hw_s *self, u32 *data, u32 len)
 
 	err = hw_atl_utils_fw_downld_dwords(self, self->rpc_addr + sizeof(u32),
 			&result,
-			RTE_ALIGN(sizeof(result), sizeof(u32)));
+			RTE_ALIGN(sizeof(result) / sizeof(u32), sizeof(u32)));
 
 	if (err < 0)
 		return err;
@@ -558,7 +558,7 @@ static int aq_fw2x_set_eeprom(struct aq_hw_s *self, u32 *data, u32 len)
 	/* Write SMBUS request to cfg memory */
 	err = hw_atl_utils_fw_upload_dwords(self, self->rpc_addr,
 				(u32 *)(void *)&request,
-				RTE_ALIGN(sizeof(request), sizeof(u32)));
+				RTE_ALIGN(sizeof(request) / sizeof(u32), sizeof(u32)));
 
 	if (err < 0)
 		return err;
@@ -589,7 +589,7 @@ static int aq_fw2x_set_eeprom(struct aq_hw_s *self, u32 *data, u32 len)
 	/* Read status of write operation */
 	err = hw_atl_utils_fw_downld_dwords(self, self->rpc_addr + sizeof(u32),
 				&result,
-				RTE_ALIGN(sizeof(result), sizeof(u32)));
+				RTE_ALIGN(sizeof(result) / sizeof(u32), sizeof(u32)));
 
 	if (err < 0)
 		return err;
-- 
2.17.1

[dpdk-dev] [PATCH 05/10] net/atlantic: use eeprom magic as a device address

[Igor Russkikh]

From: Pavel Belous <Pavel.Belous@aquantia.com>

Default dev addr is replaced with magic field from the request.
Length is allowed to be less than maximum.
SMBUS access bit definitions also better organised now.

Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
Signed-off-by: Pavel Belous <Pavel.Belous@aquantia.com>
---
 drivers/net/atlantic/atl_ethdev.c             | 20 ++++++++++------
 drivers/net/atlantic/atl_types.h              |  5 ++--
 drivers/net/atlantic/hw_atl/hw_atl_utils.c    |  4 ++++
 drivers/net/atlantic/hw_atl/hw_atl_utils.h    | 23 ++++++++++--------
 .../net/atlantic/hw_atl/hw_atl_utils_fw2x.c   | 24 ++++++++++---------
 5 files changed, 46 insertions(+), 30 deletions(-)

diff --git a/drivers/net/atlantic/atl_ethdev.c b/drivers/net/atlantic/atl_ethdev.c
index 5bc04f55c..ae1babacf 100644
--- a/drivers/net/atlantic/atl_ethdev.c
+++ b/drivers/net/atlantic/atl_ethdev.c
@@ -1102,24 +1102,27 @@ atl_dev_get_eeprom_length(struct rte_eth_dev *dev __rte_unused)
 	return SFP_EEPROM_SIZE;
 }
 
-static int
-atl_dev_get_eeprom(struct rte_eth_dev *dev, struct rte_dev_eeprom_info *eeprom)
+int atl_dev_get_eeprom(struct rte_eth_dev *dev, struct rte_dev_eeprom_info *eeprom)
 {
 	struct aq_hw_s *hw = ATL_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint32_t dev_addr = SMBUS_DEVICE_ID;
 
 	if (hw->aq_fw_ops->get_eeprom == NULL)
 		return -ENOTSUP;
 
-	if (eeprom->length != SFP_EEPROM_SIZE || eeprom->data == NULL)
+	if (eeprom->length > SFP_EEPROM_SIZE || eeprom->data == NULL)
 		return -EINVAL;
 
-	return hw->aq_fw_ops->get_eeprom(hw, eeprom->data, eeprom->length);
+	if (eeprom->magic)
+		dev_addr = eeprom->magic;
+
+	return hw->aq_fw_ops->get_eeprom(hw, dev_addr, eeprom->data, eeprom->length);
 }
 
-static int
-atl_dev_set_eeprom(struct rte_eth_dev *dev, struct rte_dev_eeprom_info *eeprom)
+int atl_dev_set_eeprom(struct rte_eth_dev *dev, struct rte_dev_eeprom_info *eeprom)
 {
 	struct aq_hw_s *hw = ATL_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint32_t dev_addr = SMBUS_DEVICE_ID;
 
 	if (hw->aq_fw_ops->set_eeprom == NULL)
 		return -ENOTSUP;
@@ -1127,7 +1130,10 @@ atl_dev_set_eeprom(struct rte_eth_dev *dev, struct rte_dev_eeprom_info *eeprom)
 	if (eeprom->length != SFP_EEPROM_SIZE || eeprom->data == NULL)
 		return -EINVAL;
 
-	return hw->aq_fw_ops->set_eeprom(hw, eeprom->data, eeprom->length);
+	if (eeprom->magic)
+		dev_addr = eeprom->magic;
+
+	return hw->aq_fw_ops->set_eeprom(hw, dev_addr, eeprom->data, eeprom->length);
 }
 
 static int
diff --git a/drivers/net/atlantic/atl_types.h b/drivers/net/atlantic/atl_types.h
index 3d90f6cae..ecc515e43 100644
--- a/drivers/net/atlantic/atl_types.h
+++ b/drivers/net/atlantic/atl_types.h
@@ -137,9 +137,10 @@ struct aq_fw_ops {
 
 	int (*led_control)(struct aq_hw_s *self, u32 mode);
 
-	int (*get_eeprom)(struct aq_hw_s *self, u32 *data, u32 len);
+	int (*get_eeprom)(struct aq_hw_s *self, int dev_addr, u32 *data, u32 len);
+
+	int (*set_eeprom)(struct aq_hw_s *self, int dev_addr, u32 *data, u32 len);
 
-	int (*set_eeprom)(struct aq_hw_s *self, u32 *data, u32 len);
 };
 
 struct atl_sw_stats {
diff --git a/drivers/net/atlantic/hw_atl/hw_atl_utils.c b/drivers/net/atlantic/hw_atl/hw_atl_utils.c
index 13f02b9f9..4299b7016 100644
--- a/drivers/net/atlantic/hw_atl/hw_atl_utils.c
+++ b/drivers/net/atlantic/hw_atl/hw_atl_utils.c
@@ -305,6 +305,10 @@ int hw_atl_utils_fw_downld_dwords(struct aq_hw_s *self, u32 a,
 			AQ_HW_WAIT_FOR(!(0x100 & aq_hw_read_reg(self,
 							   HW_ATL_MIF_CMD)),
 				       1, 1000U);
+			if (err) {
+				err = -ETIMEDOUT;
+				goto err_exit;
+			}
 
 		*(p++) = aq_hw_read_reg(self, HW_ATL_MIF_VAL);
 		a += 4;
diff --git a/drivers/net/atlantic/hw_atl/hw_atl_utils.h b/drivers/net/atlantic/hw_atl/hw_atl_utils.h
index 5f3f70847..f2a87826c 100644
--- a/drivers/net/atlantic/hw_atl/hw_atl_utils.h
+++ b/drivers/net/atlantic/hw_atl/hw_atl_utils.h
@@ -8,6 +8,7 @@
 #ifndef HW_ATL_UTILS_H
 #define HW_ATL_UTILS_H
 
+#define BIT(x)  (1UL << (x))
 #define HW_ATL_FLUSH() { (void)aq_hw_read_reg(self, 0x10); }
 
 /* Hardware tx descriptor */
@@ -389,18 +390,8 @@ enum hal_atl_utils_fw_state_e {
 #define HAL_ATLANTIC_UTILS_FW_MSG_OFFLOAD_DEL  10U
 #define HAL_ATLANTIC_UTILS_FW_MSG_CABLE_DIAG   13U // 0xd
 
-#define SMBUS_READ_REQUEST BIT(13)
-#define SMBUS_WRITE_REQUEST BIT(14)
 #define SMBUS_DEVICE_ID 0x50
 
-enum hw_atl_fw2x_rate {
-	FW2X_RATE_100M    = 0x20,
-	FW2X_RATE_1G      = 0x100,
-	FW2X_RATE_2G5     = 0x200,
-	FW2X_RATE_5G      = 0x400,
-	FW2X_RATE_10G     = 0x800,
-};
-
 enum hw_atl_fw2x_caps_lo {
 	CAPS_LO_10BASET_HD = 0x00,
 	CAPS_LO_10BASET_FD,
@@ -414,6 +405,10 @@ enum hw_atl_fw2x_caps_lo {
 	CAPS_LO_2P5GBASET_FD,
 	CAPS_LO_5GBASET_FD,
 	CAPS_LO_10GBASET_FD,
+	CAPS_LO_AUTONEG,
+	CAPS_LO_SMBUS_READ,
+	CAPS_LO_SMBUS_WRITE,
+	CAPS_LO_MACSEC
 };
 
 enum hw_atl_fw2x_caps_hi {
@@ -451,6 +446,14 @@ enum hw_atl_fw2x_caps_hi {
 	CAPS_HI_TRANSACTION_ID,
 };
 
+enum hw_atl_fw2x_rate {
+	FW2X_RATE_100M    = BIT(CAPS_LO_100BASETX_FD),
+	FW2X_RATE_1G      = BIT(CAPS_LO_1000BASET_FD),
+	FW2X_RATE_2G5     = BIT(CAPS_LO_2P5GBASET_FD),
+	FW2X_RATE_5G      = BIT(CAPS_LO_5GBASET_FD),
+	FW2X_RATE_10G     = BIT(CAPS_LO_10GBASET_FD),
+};
+
 struct aq_hw_s;
 struct aq_fw_ops;
 struct aq_hw_link_status_s;
diff --git a/drivers/net/atlantic/hw_atl/hw_atl_utils_fw2x.c b/drivers/net/atlantic/hw_atl/hw_atl_utils_fw2x.c
index a155d4aab..d0eb9f5e9 100644
--- a/drivers/net/atlantic/hw_atl/hw_atl_utils_fw2x.c
+++ b/drivers/net/atlantic/hw_atl/hw_atl_utils_fw2x.c
@@ -129,7 +129,9 @@ static u32 fw2x_to_eee_mask(u32 speed)
 
 static int aq_fw2x_set_link_speed(struct aq_hw_s *self, u32 speed)
 {
-	u32 val = link_speed_mask_2fw2x_ratemask(speed);
+	u32 rate_mask = link_speed_mask_2fw2x_ratemask(speed);
+	u32 reg_val = aq_hw_read_reg(self, HW_ATL_FW2X_MPI_CONTROL_ADDR);
+	u32 val = rate_mask | ((BIT(CAPS_LO_SMBUS_READ) | BIT(CAPS_LO_SMBUS_WRITE) | BIT(CAPS_LO_MACSEC)) & reg_val);
 
 	aq_hw_write_reg(self, HW_ATL_FW2X_MPI_CONTROL_ADDR, val);
 
@@ -484,7 +486,7 @@ static int aq_fw2x_led_control(struct aq_hw_s *self, u32 mode)
 	return 0;
 }
 
-static int aq_fw2x_get_eeprom(struct aq_hw_s *self, u32 *data, u32 len)
+static int aq_fw2x_get_eeprom(struct aq_hw_s *self, int dev_addr, u32 *data, u32 len)
 {
 	int err = 0;
 	struct smbus_read_request request;
@@ -494,7 +496,7 @@ static int aq_fw2x_get_eeprom(struct aq_hw_s *self, u32 *data, u32 len)
 	if (self->fw_ver_actual < HW_ATL_FW_FEATURE_EEPROM)
 		return -EOPNOTSUPP;
 
-	request.device_id = SMBUS_DEVICE_ID;
+	request.device_id = dev_addr;
 	request.address = 0;
 	request.length = len;
 
@@ -506,16 +508,16 @@ static int aq_fw2x_get_eeprom(struct aq_hw_s *self, u32 *data, u32 len)
 	if (err < 0)
 		return err;
 
-	/* Toggle 0x368.SMBUS_READ_REQUEST bit */
+	/* Toggle 0x368.CAPS_LO_SMBUS_READ bit */
 	mpi_opts = aq_hw_read_reg(self, HW_ATL_FW2X_MPI_CONTROL_ADDR);
-	mpi_opts ^= SMBUS_READ_REQUEST;
+	mpi_opts ^= BIT(CAPS_LO_SMBUS_READ);
 
 	aq_hw_write_reg(self, HW_ATL_FW2X_MPI_CONTROL_ADDR, mpi_opts);
 
 	/* Wait until REQUEST_BIT matched in 0x370 */
 
 	AQ_HW_WAIT_FOR((aq_hw_read_reg(self, HW_ATL_FW2X_MPI_STATE_ADDR) &
-		SMBUS_READ_REQUEST) == (mpi_opts & SMBUS_READ_REQUEST),
+		BIT(CAPS_LO_SMBUS_READ)) == (mpi_opts & BIT(CAPS_LO_SMBUS_READ)),
 		10U, 10000U);
 
 	if (err < 0)
@@ -542,7 +544,7 @@ static int aq_fw2x_get_eeprom(struct aq_hw_s *self, u32 *data, u32 len)
 }
 
 
-static int aq_fw2x_set_eeprom(struct aq_hw_s *self, u32 *data, u32 len)
+static int aq_fw2x_set_eeprom(struct aq_hw_s *self, int dev_addr, u32 *data, u32 len)
 {
 	struct smbus_write_request request;
 	u32 mpi_opts, result = 0;
@@ -551,7 +553,7 @@ static int aq_fw2x_set_eeprom(struct aq_hw_s *self, u32 *data, u32 len)
 	if (self->fw_ver_actual < HW_ATL_FW_FEATURE_EEPROM)
 		return -EOPNOTSUPP;
 
-	request.device_id = SMBUS_DEVICE_ID;
+	request.device_id = dev_addr;
 	request.address = 0;
 	request.length = len;
 
@@ -572,15 +574,15 @@ static int aq_fw2x_set_eeprom(struct aq_hw_s *self, u32 *data, u32 len)
 	if (err < 0)
 		return err;
 
-	/* Toggle 0x368.SMBUS_WRITE_REQUEST bit */
+	/* Toggle 0x368.CAPS_LO_SMBUS_WRITE bit */
 	mpi_opts = aq_hw_read_reg(self, HW_ATL_FW2X_MPI_CONTROL_ADDR);
-	mpi_opts ^= SMBUS_WRITE_REQUEST;
+	mpi_opts ^= BIT(CAPS_LO_SMBUS_WRITE);
 
 	aq_hw_write_reg(self, HW_ATL_FW2X_MPI_CONTROL_ADDR, mpi_opts);
 
 	/* Wait until REQUEST_BIT matched in 0x370 */
 	AQ_HW_WAIT_FOR((aq_hw_read_reg(self, HW_ATL_FW2X_MPI_STATE_ADDR) &
-		SMBUS_WRITE_REQUEST) == (mpi_opts & SMBUS_WRITE_REQUEST),
+		BIT(CAPS_LO_SMBUS_WRITE)) == (mpi_opts & (BIT(CAPS_LO_SMBUS_WRITE))),
 		10U, 10000U);
 
 	if (err < 0)
-- 
2.17.1

[dpdk-dev] [PATCH 06/10] net/atlantic: fix eeprom fetching for small and uneven lengths

[Igor Russkikh]

From: Pavel Belous <Pavel.Belous@aquantia.com>

Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
Signed-off-by: Pavel Belous <Pavel.Belous@aquantia.com>
---
 .../net/atlantic/hw_atl/hw_atl_utils_fw2x.c   | 27 ++++++++++++++++---
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/net/atlantic/hw_atl/hw_atl_utils_fw2x.c b/drivers/net/atlantic/hw_atl/hw_atl_utils_fw2x.c
index d0eb9f5e9..efa999268 100644
--- a/drivers/net/atlantic/hw_atl/hw_atl_utils_fw2x.c
+++ b/drivers/net/atlantic/hw_atl/hw_atl_utils_fw2x.c
@@ -531,13 +531,32 @@ static int aq_fw2x_get_eeprom(struct aq_hw_s *self, int dev_addr, u32 *data, u32
 		return err;
 
 	if (result == 0) {
-		err = hw_atl_utils_fw_downld_dwords(self,
+		u32 num_dwords = len / sizeof(u32);
+		u32 bytes_remains = len % sizeof(u32);
+
+		if (num_dwords) {
+			err = hw_atl_utils_fw_downld_dwords(self,
 				self->rpc_addr + sizeof(u32) * 2,
 				data,
-				RTE_ALIGN(len, sizeof(u32)));
+				num_dwords);
 
-		if (err < 0)
-			return err;
+			if (err < 0)
+				return err;
+		}
+
+		if (bytes_remains) {
+			u32 val = 0;
+
+			err = hw_atl_utils_fw_downld_dwords(self,
+				self->rpc_addr + sizeof(u32) * 2 + num_dwords,
+				&val,
+				sizeof(u32));
+
+			if (err < 0)
+				return err;
+
+			rte_memcpy((u8*)data + len - bytes_remains, &val, bytes_remains);
+		}
 	}
 
 	return 0;
-- 
2.17.1

[dpdk-dev] [PATCH 07/10] net/atlantic: fix link configuration

[Igor Russkikh]

In case link speed is re configured after port start, it does not
takes the requested speed value, but instead just sets full autoneg
mask.

Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
---
 drivers/net/atlantic/atl_ethdev.c | 44 ++++++++++++++-----------------
 1 file changed, 20 insertions(+), 24 deletions(-)

diff --git a/drivers/net/atlantic/atl_ethdev.c b/drivers/net/atlantic/atl_ethdev.c
index ae1babacf..ad4e5a717 100644
--- a/drivers/net/atlantic/atl_ethdev.c
+++ b/drivers/net/atlantic/atl_ethdev.c
@@ -465,8 +465,6 @@ atl_dev_start(struct rte_eth_dev *dev)
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
 	struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
 	uint32_t intr_vector = 0;
-	uint32_t *link_speeds;
-	uint32_t speed = 0;
 	int status;
 	int err;
 
@@ -543,6 +541,8 @@ atl_dev_start(struct rte_eth_dev *dev)
 		goto error;
 	}
 
+	err = atl_dev_set_link_up(dev);
+
 	err = hw->aq_fw_ops->update_link_status(hw);
 
 	if (err)
@@ -550,26 +550,6 @@ atl_dev_start(struct rte_eth_dev *dev)
 
 	dev->data->dev_link.link_status = hw->aq_link_status.mbps != 0;
 
-	link_speeds = &dev->data->dev_conf.link_speeds;
-
-	speed = 0x0;
-
-	if (*link_speeds == ETH_LINK_SPEED_AUTONEG) {
-		speed = hw->aq_nic_cfg->link_speed_msk;
-	} else {
-		if (*link_speeds & ETH_LINK_SPEED_10G)
-			speed |= AQ_NIC_RATE_10G;
-		if (*link_speeds & ETH_LINK_SPEED_5G)
-			speed |= AQ_NIC_RATE_5G;
-		if (*link_speeds & ETH_LINK_SPEED_1G)
-			speed |= AQ_NIC_RATE_1G;
-		if (*link_speeds & ETH_LINK_SPEED_2_5G)
-			speed |=  AQ_NIC_RATE_2G5;
-		if (*link_speeds & ETH_LINK_SPEED_100M)
-			speed |= AQ_NIC_RATE_100M;
-	}
-
-	err = hw->aq_fw_ops->set_link_speed(hw, speed);
 	if (err)
 		goto error;
 
@@ -657,9 +637,25 @@ static int
 atl_dev_set_link_up(struct rte_eth_dev *dev)
 {
 	struct aq_hw_s *hw = ATL_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint32_t link_speeds = dev->data->dev_conf.link_speeds;
+	uint32_t speed_mask = 0;
+
+	if (link_speeds == ETH_LINK_SPEED_AUTONEG) {
+		speed_mask = hw->aq_nic_cfg->link_speed_msk;
+	} else {
+		if (link_speeds & ETH_LINK_SPEED_10G)
+			speed_mask |= AQ_NIC_RATE_10G;
+		if (link_speeds & ETH_LINK_SPEED_5G)
+			speed_mask |= AQ_NIC_RATE_5G;
+		if (link_speeds & ETH_LINK_SPEED_1G)
+			speed_mask |= AQ_NIC_RATE_1G;
+		if (link_speeds & ETH_LINK_SPEED_2_5G)
+			speed_mask |=  AQ_NIC_RATE_2G5;
+		if (link_speeds & ETH_LINK_SPEED_100M)
+			speed_mask |= AQ_NIC_RATE_100M;
+	}
 
-	return hw->aq_fw_ops->set_link_speed(hw,
-			hw->aq_nic_cfg->link_speed_msk);
+	return hw->aq_fw_ops->set_link_speed(hw, speed_mask);
 }
 
 /*
-- 
2.17.1

[dpdk-dev] [PATCH 08/10] net/atlantic: eliminate excessive log levels on rx/tx

[Igor Russkikh]

Default rxtx logging used ERR level, that caused logger to always
trigger. That caused perf degradation even if logger was not enabled but
compiled in.

Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
---
 drivers/net/atlantic/atl_rxtx.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/net/atlantic/atl_rxtx.c b/drivers/net/atlantic/atl_rxtx.c
index aea58c0d5..f1519594a 100644
--- a/drivers/net/atlantic/atl_rxtx.c
+++ b/drivers/net/atlantic/atl_rxtx.c
@@ -946,7 +946,7 @@ atl_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 			break;
 		}
 
-		PMD_RX_LOG(ERR, "port_id=%u queue_id=%u tail=%u "
+		PMD_RX_LOG(DEBUG, "port_id=%u queue_id=%u tail=%u "
 			   "eop=0x%x pkt_len=%u hash=0x%x hash_type=0x%x",
 			   (unsigned int)rxq->port_id,
 			   (unsigned int)rxq->queue_id,
@@ -981,7 +981,7 @@ atl_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 		while (true) {
 			new_mbuf = rte_mbuf_raw_alloc(rxq->mb_pool);
 			if (new_mbuf == NULL) {
-				PMD_RX_LOG(ERR,
+				PMD_RX_LOG(DEBUG,
 				   "RX mbuf alloc failed port_id=%u "
 				   "queue_id=%u", (unsigned int)rxq->port_id,
 				   (unsigned int)rxq->queue_id);
@@ -1084,7 +1084,7 @@ atl_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 		adapter->sw_stats.q_ibytes[rxq->queue_id] +=
 			rx_mbuf_first->pkt_len;
 
-		PMD_RX_LOG(ERR, "add mbuf segs=%d pkt_len=%d",
+		PMD_RX_LOG(DEBUG, "add mbuf segs=%d pkt_len=%d",
 			rx_mbuf_first->nb_segs,
 			rx_mbuf_first->pkt_len);
 	}
@@ -1104,7 +1104,7 @@ atl_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 	 */
 	nb_hold = (uint16_t)(nb_hold + rxq->nb_rx_hold);
 	if (nb_hold > rxq->rx_free_thresh) {
-		PMD_RX_LOG(ERR, "port_id=%u queue_id=%u rx_tail=%u "
+		PMD_RX_LOG(DEBUG, "port_id=%u queue_id=%u rx_tail=%u "
 			"nb_hold=%u nb_rx=%u",
 			(unsigned int)rxq->port_id, (unsigned int)rxq->queue_id,
 			(unsigned int)tail, (unsigned int)nb_hold,
@@ -1129,8 +1129,6 @@ atl_xmit_cleanup(struct atl_tx_queue *txq)
 	struct hw_atl_txd_s *txd;
 	int to_clean = 0;
 
-	PMD_INIT_FUNC_TRACE();
-
 	if (txq != NULL) {
 		sw_ring = txq->sw_ring;
 		int head = txq->tx_head;
@@ -1181,11 +1179,7 @@ atl_tso_setup(struct rte_mbuf *tx_pkt, union hw_atl_txc_s *txc)
 	uint32_t tx_cmd = 0;
 	uint64_t ol_flags = tx_pkt->ol_flags;
 
-	PMD_INIT_FUNC_TRACE();
-
 	if (ol_flags & PKT_TX_TCP_SEG) {
-		PMD_DRV_LOG(DEBUG, "xmit TSO pkt");
-
 		tx_cmd |= tx_desc_cmd_lso | tx_desc_cmd_l4cs;
 
 		txc->cmd = 0x4;
@@ -1240,8 +1234,6 @@ atl_xmit_pkt(struct aq_hw_s *hw, struct atl_tx_queue *txq,
 	u32 tx_cmd = 0U;
 	int desc_count = 0;
 
-	PMD_INIT_FUNC_TRACE();
-
 	tail = txq->tx_tail;
 
 	txc = (union hw_atl_txc_s *)&txq->hw_ring[tail];
-- 
2.17.1

[dpdk-dev] [PATCH 09/10] net/atlantic: fix missing vlan filter offload

[Igor Russkikh]

Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
---
 drivers/net/atlantic/atl_ethdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/atlantic/atl_ethdev.c b/drivers/net/atlantic/atl_ethdev.c
index ad4e5a717..c2ba19830 100644
--- a/drivers/net/atlantic/atl_ethdev.c
+++ b/drivers/net/atlantic/atl_ethdev.c
@@ -165,7 +165,8 @@ static struct rte_pci_driver rte_atl_pmd = {
 			| DEV_RX_OFFLOAD_IPV4_CKSUM \
 			| DEV_RX_OFFLOAD_UDP_CKSUM \
 			| DEV_RX_OFFLOAD_TCP_CKSUM \
-			| DEV_RX_OFFLOAD_JUMBO_FRAME)
+			| DEV_RX_OFFLOAD_JUMBO_FRAME \
+			| DEV_RX_OFFLOAD_VLAN_FILTER)
 
 #define ATL_TX_OFFLOADS (DEV_TX_OFFLOAD_VLAN_INSERT \
 			| DEV_TX_OFFLOAD_IPV4_CKSUM \
-- 
2.17.1

[dpdk-dev] [PATCH 10/10] net/atlantic: fix xstats to return correct number of requested items

[Igor Russkikh]

Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
---
 drivers/net/atlantic/atl_ethdev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/atlantic/atl_ethdev.c b/drivers/net/atlantic/atl_ethdev.c
index c2ba19830..d40645283 100644
--- a/drivers/net/atlantic/atl_ethdev.c
+++ b/drivers/net/atlantic/atl_ethdev.c
@@ -758,7 +758,7 @@ atl_dev_xstats_get_names(struct rte_eth_dev *dev __rte_unused,
 		snprintf(xstats_names[i].name, RTE_ETH_XSTATS_NAME_SIZE, "%s",
 			atl_xstats_tbl[i].name);
 
-	return size;
+	return i;
 }
 
 static int
@@ -778,7 +778,7 @@ atl_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *stats,
 					atl_xstats_tbl[i].offset);
 	}
 
-	return n;
+	return i;
 }
 
 static int
-- 
2.17.1

Re: [dpdk-dev] [PATCH 10/10] net/atlantic: fix xstats to return correct number of requested items

[Rami Rosen]

Reviewed-by: Rami Rosen <ramirose@gmail.com>


>

Re: [dpdk-dev] [PATCH 00/10] net/atlantic: bugfixes and code cleanup

[David Marchand]

On Sat, Mar 2, 2019 at 1:10 PM Igor Russkikh <Igor.Russkikh@aquantia.com>
wrote:

> The patchset mainly fixes some Coverity issues,
> couple of minor bugs and overflows.
>
> It also improves eeprom get/set logic with ability of custom
> device address and dump length.
>
> Igor Russkikh (7):
>   net/atlantic: Fix negative error codes
>   net/atlantic: remove unused variable
>   net/atlantic: check for error codes
>   net/atlantic: fix link configuration
>   net/atlantic: eliminate excessive log levels on rx/tx
>   net/atlantic: fix missing vlan filter offload
>   net/atlantic: fix xstats to return correct number of requested items
>
> Pavel Belous (3):
>   net/atlantic: fix buffer overflow
>   net/atlantic: use eeprom magic as a device address
>   net/atlantic: fix eeprom fetching for small and uneven lengths
>
>  drivers/net/atlantic/atl_ethdev.c             | 71 ++++++++++---------
>  drivers/net/atlantic/atl_rxtx.c               | 20 ++----
>  drivers/net/atlantic/atl_types.h              |  5 +-
>  drivers/net/atlantic/hw_atl/hw_atl_b0.c       |  6 +-
>  drivers/net/atlantic/hw_atl/hw_atl_utils.c    |  9 +--
>  drivers/net/atlantic/hw_atl/hw_atl_utils.h    | 23 +++---
>  .../net/atlantic/hw_atl/hw_atl_utils_fw2x.c   | 59 ++++++++++-----
>  7 files changed, 105 insertions(+), 88 deletions(-)
>


Those patches are missing Fixes: tags and copying stable where appropriate.
The commitlogs also need some work.

It looks like those patches were neither checked against checkpatch nor
check-git-log.sh.
Here is what I see (extended the max line length to 100 to avoid all those
80 columns warnings, but this is worth fixing as well).

[dmarchan@dmarchan dpdk]$ DPDK_CHECKPATCH_LINE_LENGTH=100
./devtools/checkpatches.sh

### net/atlantic: Fix negative error codes

WARNING:COMMIT_MESSAGE: Missing commit description - Add an appropriate one

total: 0 errors, 1 warnings, 0 checks, 14 lines checked

### net/atlantic: use eeprom magic as a device address

WARNING:LONG_LINE: line over 100 characters
#163: FILE: drivers/net/atlantic/hw_atl/hw_atl_utils_fw2x.c:134:
+    u32 val = rate_mask | ((BIT(CAPS_LO_SMBUS_READ) |
BIT(CAPS_LO_SMBUS_WRITE) | BIT(CAPS_LO_MACSEC)) & reg_val);

total: 0 errors, 1 warnings, 0 checks, 194 lines checked

### net/atlantic: fix eeprom fetching for small and uneven lengths

WARNING:COMMIT_MESSAGE: Missing commit description - Add an appropriate one

ERROR:POINTER_LOCATION: "(foo*)" should be "(foo *)"
#46: FILE: drivers/net/atlantic/hw_atl/hw_atl_utils_fw2x.c:558:
+            rte_memcpy((u8*)data + len - bytes_remains, &val,
bytes_remains);

total: 1 errors, 1 warnings, 0 checks, 36 lines checked

### net/atlantic: fix missing vlan filter offload

WARNING:COMMIT_MESSAGE: Missing commit description - Add an appropriate one

total: 0 errors, 1 warnings, 0 checks, 9 lines checked

### net/atlantic: fix xstats to return correct number of requested items

WARNING:COMMIT_MESSAGE: Missing commit description - Add an appropriate one

total: 0 errors, 1 warnings, 0 checks, 16 lines checked

5/10 valid patches



[dmarchan@dmarchan dpdk]$ ./devtools/check-git-log.sh
Wrong headline uppercase:
    net/atlantic: Fix negative error codes
Wrong headline lowercase:
    net/atlantic: use eeprom magic as a device address
    net/atlantic: fix eeprom fetching for small and uneven lengths
    net/atlantic: eliminate excessive log levels on rx/tx
    net/atlantic: fix missing vlan filter offload
Headline too long:
    net/atlantic: fix eeprom fetching for small and uneven lengths
    net/atlantic: fix xstats to return correct number of requested items
Missing 'Fixes' tag:
    net/atlantic: Fix negative error codes
    net/atlantic: fix buffer overflow
    net/atlantic: fix eeprom fetching for small and uneven lengths
    net/atlantic: fix link configuration
    net/atlantic: fix missing vlan filter offload
    net/atlantic: fix xstats to return correct number of requested items


-- 
David Marchand

Re: [dpdk-dev] [PATCH 00/10] net/atlantic: bugfixes and code cleanup

[Igor Russkikh]

On 04.03.2019 12:40, David Marchand wrote:

> Those patches are missing Fixes: tags and copying stable where appropriate.
> The commitlogs also need some work.
> 
> It looks like those patches were neither checked against checkpatch nor
> check-git-log.sh.
> Here is what I see (extended the max line length to 100 to avoid all those
> 80 columns warnings, but this is worth fixing as well).

Thanks David, I've indeed forgot about check-git-log.
Will fix these and resubmit.

Regards,
  Igor