DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] net/vmxnet3: support per-queue stats for less queues
@ 2024-10-24 11:01 Morten Brørup
  2024-10-25  9:27 ` [PATCH v2] net/vmxnet3: support per-queue stats for fewer queues Morten Brørup
  2024-11-04 10:52 ` [PATCH v3 1/2] net/vmxnet3: fix potential out of bounds access Morten Brørup
  0 siblings, 2 replies; 9+ messages in thread
From: Morten Brørup @ 2024-10-24 11:01 UTC (permalink / raw)
  To: dev, Jochen Behrens; +Cc: Morten Brørup

Remove the requirement that the configured number of queues to provide
statistics for (RTE_ETHDEV_QUEUE_STAT_CNTRS) cannot be less than the
driver's max number of supported transmit queues (VMXNET3_MAX_TX_QUEUES).

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
---
 drivers/net/vmxnet3/vmxnet3_ethdev.c | 32 +++++++++++++++++-----------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
index 78fac63ab6..8a9bb452c6 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
@@ -1470,42 +1470,52 @@ vmxnet3_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 	struct vmxnet3_hw *hw = dev->data->dev_private;
 	struct UPT1_TxStats txStats;
 	struct UPT1_RxStats rxStats;
+	uint64_t packets, bytes;
 
 	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD, VMXNET3_CMD_GET_STATS);
 
 	for (i = 0; i < hw->num_tx_queues; i++) {
 		vmxnet3_tx_stats_get(hw, i, &txStats);
 
-		stats->q_opackets[i] = txStats.ucastPktsTxOK +
+		packets = txStats.ucastPktsTxOK +
 			txStats.mcastPktsTxOK +
 			txStats.bcastPktsTxOK;
 
-		stats->q_obytes[i] = txStats.ucastBytesTxOK +
+		bytes = txStats.ucastBytesTxOK +
 			txStats.mcastBytesTxOK +
 			txStats.bcastBytesTxOK;
 
-		stats->opackets += stats->q_opackets[i];
-		stats->obytes += stats->q_obytes[i];
+		stats->opackets += packets;
+		stats->obytes += bytes;
 		stats->oerrors += txStats.pktsTxError + txStats.pktsTxDiscard;
+
+		if (i < RTE_ETHDEV_QUEUE_STAT_CNTRS) {
+			stats->q_opackets[i] = packets;
+			stats->q_obytes[i] = bytes;
+		}
 	}
 
 	for (i = 0; i < hw->num_rx_queues; i++) {
 		vmxnet3_rx_stats_get(hw, i, &rxStats);
 
-		stats->q_ipackets[i] = rxStats.ucastPktsRxOK +
+		packets = rxStats.ucastPktsRxOK +
 			rxStats.mcastPktsRxOK +
 			rxStats.bcastPktsRxOK;
 
-		stats->q_ibytes[i] = rxStats.ucastBytesRxOK +
+		bytes = rxStats.ucastBytesRxOK +
 			rxStats.mcastBytesRxOK +
 			rxStats.bcastBytesRxOK;
 
-		stats->ipackets += stats->q_ipackets[i];
-		stats->ibytes += stats->q_ibytes[i];
-
-		stats->q_errors[i] = rxStats.pktsRxError;
+		stats->ipackets += packets;
+		stats->ibytes += bytes;
 		stats->ierrors += rxStats.pktsRxError;
 		stats->imissed += rxStats.pktsRxOutOfBuf;
+
+		if (i < RTE_ETHDEV_QUEUE_STAT_CNTRS) {
+			stats->q_ipackets[i] = packets;
+			stats->q_ibytes[i] = bytes;
+			stats->q_errors[i] = rxStats.pktsRxError;
+		}
 	}
 
 	return 0;
@@ -1521,8 +1531,6 @@ vmxnet3_dev_stats_reset(struct rte_eth_dev *dev)
 
 	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD, VMXNET3_CMD_GET_STATS);
 
-	RTE_BUILD_BUG_ON(RTE_ETHDEV_QUEUE_STAT_CNTRS < VMXNET3_MAX_TX_QUEUES);
-
 	for (i = 0; i < hw->num_tx_queues; i++) {
 		vmxnet3_hw_tx_stats_get(hw, i, &txStats);
 		memcpy(&hw->snapshot_tx_stats[i], &txStats,
-- 
2.43.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2] net/vmxnet3: support per-queue stats for fewer queues
  2024-10-24 11:01 [PATCH] net/vmxnet3: support per-queue stats for less queues Morten Brørup
@ 2024-10-25  9:27 ` Morten Brørup
  2024-10-27  8:47   ` Morten Brørup
  2024-11-01  0:28   ` Ferruh Yigit
  2024-11-04 10:52 ` [PATCH v3 1/2] net/vmxnet3: fix potential out of bounds access Morten Brørup
  1 sibling, 2 replies; 9+ messages in thread
From: Morten Brørup @ 2024-10-25  9:27 UTC (permalink / raw)
  To: dev, Jochen Behrens; +Cc: Morten Brørup

Removed the requirement that the configured number of queues to provide
statistics for (RTE_ETHDEV_QUEUE_STAT_CNTRS) cannot be less than the
driver's max number of supported transmit queues (VMXNET3_MAX_TX_QUEUES).

Also improved support for virtual hardware version 6.

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
---
v2:
* Virtual hardware version 6 supports more queues; updated some arrays
  accordingly.
* Added support for larger MTU with virtual hardware version 6.
---
 drivers/net/vmxnet3/vmxnet3_ethdev.c | 34 +++++++++++++++++-----------
 drivers/net/vmxnet3/vmxnet3_ethdev.h |  4 ++--
 2 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
index 78fac63ab6..1752c58069 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
@@ -1470,42 +1470,52 @@ vmxnet3_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 	struct vmxnet3_hw *hw = dev->data->dev_private;
 	struct UPT1_TxStats txStats;
 	struct UPT1_RxStats rxStats;
+	uint64_t packets, bytes;
 
 	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD, VMXNET3_CMD_GET_STATS);
 
 	for (i = 0; i < hw->num_tx_queues; i++) {
 		vmxnet3_tx_stats_get(hw, i, &txStats);
 
-		stats->q_opackets[i] = txStats.ucastPktsTxOK +
+		packets = txStats.ucastPktsTxOK +
 			txStats.mcastPktsTxOK +
 			txStats.bcastPktsTxOK;
 
-		stats->q_obytes[i] = txStats.ucastBytesTxOK +
+		bytes = txStats.ucastBytesTxOK +
 			txStats.mcastBytesTxOK +
 			txStats.bcastBytesTxOK;
 
-		stats->opackets += stats->q_opackets[i];
-		stats->obytes += stats->q_obytes[i];
+		stats->opackets += packets;
+		stats->obytes += bytes;
 		stats->oerrors += txStats.pktsTxError + txStats.pktsTxDiscard;
+
+		if (i < RTE_ETHDEV_QUEUE_STAT_CNTRS) {
+			stats->q_opackets[i] = packets;
+			stats->q_obytes[i] = bytes;
+		}
 	}
 
 	for (i = 0; i < hw->num_rx_queues; i++) {
 		vmxnet3_rx_stats_get(hw, i, &rxStats);
 
-		stats->q_ipackets[i] = rxStats.ucastPktsRxOK +
+		packets = rxStats.ucastPktsRxOK +
 			rxStats.mcastPktsRxOK +
 			rxStats.bcastPktsRxOK;
 
-		stats->q_ibytes[i] = rxStats.ucastBytesRxOK +
+		bytes = rxStats.ucastBytesRxOK +
 			rxStats.mcastBytesRxOK +
 			rxStats.bcastBytesRxOK;
 
-		stats->ipackets += stats->q_ipackets[i];
-		stats->ibytes += stats->q_ibytes[i];
-
-		stats->q_errors[i] = rxStats.pktsRxError;
+		stats->ipackets += packets;
+		stats->ibytes += bytes;
 		stats->ierrors += rxStats.pktsRxError;
 		stats->imissed += rxStats.pktsRxOutOfBuf;
+
+		if (i < RTE_ETHDEV_QUEUE_STAT_CNTRS) {
+			stats->q_ipackets[i] = packets;
+			stats->q_ibytes[i] = bytes;
+			stats->q_errors[i] = rxStats.pktsRxError;
+		}
 	}
 
 	return 0;
@@ -1521,8 +1531,6 @@ vmxnet3_dev_stats_reset(struct rte_eth_dev *dev)
 
 	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD, VMXNET3_CMD_GET_STATS);
 
-	RTE_BUILD_BUG_ON(RTE_ETHDEV_QUEUE_STAT_CNTRS < VMXNET3_MAX_TX_QUEUES);
-
 	for (i = 0; i < hw->num_tx_queues; i++) {
 		vmxnet3_hw_tx_stats_get(hw, i, &txStats);
 		memcpy(&hw->snapshot_tx_stats[i], &txStats,
@@ -1566,7 +1574,7 @@ vmxnet3_dev_info_get(struct rte_eth_dev *dev,
 	dev_info->min_rx_bufsize = 1518 + RTE_PKTMBUF_HEADROOM;
 	dev_info->max_rx_pktlen = 16384; /* includes CRC, cf MAXFRS register */
 	dev_info->min_mtu = VMXNET3_MIN_MTU;
-	dev_info->max_mtu = VMXNET3_MAX_MTU;
+	dev_info->max_mtu = VMXNET3_VERSION_GE_6(hw) ? VMXNET3_V6_MAX_MTU : VMXNET3_MAX_MTU;
 	dev_info->speed_capa = RTE_ETH_LINK_SPEED_10G;
 	dev_info->max_mac_addrs = VMXNET3_MAX_MAC_ADDRS;
 
diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.h b/drivers/net/vmxnet3/vmxnet3_ethdev.h
index 2b3e2c4caa..e9ded6663d 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.h
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.h
@@ -121,8 +121,8 @@ struct vmxnet3_hw {
 #define VMXNET3_VFT_TABLE_SIZE     (VMXNET3_VFT_SIZE * sizeof(uint32_t))
 	UPT1_TxStats	      saved_tx_stats[VMXNET3_EXT_MAX_TX_QUEUES];
 	UPT1_RxStats	      saved_rx_stats[VMXNET3_EXT_MAX_RX_QUEUES];
-	UPT1_TxStats          snapshot_tx_stats[VMXNET3_MAX_TX_QUEUES];
-	UPT1_RxStats          snapshot_rx_stats[VMXNET3_MAX_RX_QUEUES];
+	UPT1_TxStats          snapshot_tx_stats[VMXNET3_EXT_MAX_TX_QUEUES];
+	UPT1_RxStats          snapshot_rx_stats[VMXNET3_EXT_MAX_RX_QUEUES];
 	uint16_t              tx_prod_offset;
 	uint16_t              rx_prod_offset[2];
 	/* device capability bit map */
-- 
2.43.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH v2] net/vmxnet3: support per-queue stats for fewer queues
  2024-10-25  9:27 ` [PATCH v2] net/vmxnet3: support per-queue stats for fewer queues Morten Brørup
@ 2024-10-27  8:47   ` Morten Brørup
  2024-11-01  0:28   ` Ferruh Yigit
  1 sibling, 0 replies; 9+ messages in thread
From: Morten Brørup @ 2024-10-27  8:47 UTC (permalink / raw)
  To: Jochen Behrens, Ronak Doshi; +Cc: dev, Roger B Melton

Jochen, Ronak, (Roger,)

Please review the below patch, so it can go into the 24.11 LTS release.

> From: Morten Brørup [mailto:mb@smartsharesystems.com]
> Sent: Friday, 25 October 2024 11.28
> 
> Removed the requirement that the configured number of queues to provide
> statistics for (RTE_ETHDEV_QUEUE_STAT_CNTRS) cannot be less than the
> driver's max number of supported transmit queues
> (VMXNET3_MAX_TX_QUEUES).
> 
> Also improved support for virtual hardware version 6.
> 
> Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> ---
> v2:
> * Virtual hardware version 6 supports more queues; updated some arrays
>   accordingly.
> * Added support for larger MTU with virtual hardware version 6.
> ---
>  drivers/net/vmxnet3/vmxnet3_ethdev.c | 34 +++++++++++++++++-----------
>  drivers/net/vmxnet3/vmxnet3_ethdev.h |  4 ++--
>  2 files changed, 23 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c
> b/drivers/net/vmxnet3/vmxnet3_ethdev.c
> index 78fac63ab6..1752c58069 100644
> --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
> +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
> @@ -1470,42 +1470,52 @@ vmxnet3_dev_stats_get(struct rte_eth_dev *dev,
> struct rte_eth_stats *stats)
>  	struct vmxnet3_hw *hw = dev->data->dev_private;
>  	struct UPT1_TxStats txStats;
>  	struct UPT1_RxStats rxStats;
> +	uint64_t packets, bytes;
> 
>  	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD,
> VMXNET3_CMD_GET_STATS);
> 
>  	for (i = 0; i < hw->num_tx_queues; i++) {
>  		vmxnet3_tx_stats_get(hw, i, &txStats);
> 
> -		stats->q_opackets[i] = txStats.ucastPktsTxOK +
> +		packets = txStats.ucastPktsTxOK +
>  			txStats.mcastPktsTxOK +
>  			txStats.bcastPktsTxOK;
> 
> -		stats->q_obytes[i] = txStats.ucastBytesTxOK +
> +		bytes = txStats.ucastBytesTxOK +
>  			txStats.mcastBytesTxOK +
>  			txStats.bcastBytesTxOK;
> 
> -		stats->opackets += stats->q_opackets[i];
> -		stats->obytes += stats->q_obytes[i];
> +		stats->opackets += packets;
> +		stats->obytes += bytes;
>  		stats->oerrors += txStats.pktsTxError +
> txStats.pktsTxDiscard;
> +
> +		if (i < RTE_ETHDEV_QUEUE_STAT_CNTRS) {
> +			stats->q_opackets[i] = packets;
> +			stats->q_obytes[i] = bytes;
> +		}
>  	}
> 
>  	for (i = 0; i < hw->num_rx_queues; i++) {
>  		vmxnet3_rx_stats_get(hw, i, &rxStats);
> 
> -		stats->q_ipackets[i] = rxStats.ucastPktsRxOK +
> +		packets = rxStats.ucastPktsRxOK +
>  			rxStats.mcastPktsRxOK +
>  			rxStats.bcastPktsRxOK;
> 
> -		stats->q_ibytes[i] = rxStats.ucastBytesRxOK +
> +		bytes = rxStats.ucastBytesRxOK +
>  			rxStats.mcastBytesRxOK +
>  			rxStats.bcastBytesRxOK;
> 
> -		stats->ipackets += stats->q_ipackets[i];
> -		stats->ibytes += stats->q_ibytes[i];
> -
> -		stats->q_errors[i] = rxStats.pktsRxError;
> +		stats->ipackets += packets;
> +		stats->ibytes += bytes;
>  		stats->ierrors += rxStats.pktsRxError;
>  		stats->imissed += rxStats.pktsRxOutOfBuf;
> +
> +		if (i < RTE_ETHDEV_QUEUE_STAT_CNTRS) {
> +			stats->q_ipackets[i] = packets;
> +			stats->q_ibytes[i] = bytes;
> +			stats->q_errors[i] = rxStats.pktsRxError;
> +		}
>  	}
> 
>  	return 0;
> @@ -1521,8 +1531,6 @@ vmxnet3_dev_stats_reset(struct rte_eth_dev *dev)
> 
>  	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD,
> VMXNET3_CMD_GET_STATS);
> 
> -	RTE_BUILD_BUG_ON(RTE_ETHDEV_QUEUE_STAT_CNTRS <
> VMXNET3_MAX_TX_QUEUES);
> -
>  	for (i = 0; i < hw->num_tx_queues; i++) {
>  		vmxnet3_hw_tx_stats_get(hw, i, &txStats);
>  		memcpy(&hw->snapshot_tx_stats[i], &txStats,
> @@ -1566,7 +1574,7 @@ vmxnet3_dev_info_get(struct rte_eth_dev *dev,
>  	dev_info->min_rx_bufsize = 1518 + RTE_PKTMBUF_HEADROOM;
>  	dev_info->max_rx_pktlen = 16384; /* includes CRC, cf MAXFRS
> register */
>  	dev_info->min_mtu = VMXNET3_MIN_MTU;
> -	dev_info->max_mtu = VMXNET3_MAX_MTU;
> +	dev_info->max_mtu = VMXNET3_VERSION_GE_6(hw) ? VMXNET3_V6_MAX_MTU
> : VMXNET3_MAX_MTU;
>  	dev_info->speed_capa = RTE_ETH_LINK_SPEED_10G;
>  	dev_info->max_mac_addrs = VMXNET3_MAX_MAC_ADDRS;
> 
> diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.h
> b/drivers/net/vmxnet3/vmxnet3_ethdev.h
> index 2b3e2c4caa..e9ded6663d 100644
> --- a/drivers/net/vmxnet3/vmxnet3_ethdev.h
> +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.h
> @@ -121,8 +121,8 @@ struct vmxnet3_hw {
>  #define VMXNET3_VFT_TABLE_SIZE     (VMXNET3_VFT_SIZE *
> sizeof(uint32_t))
>  	UPT1_TxStats
> saved_tx_stats[VMXNET3_EXT_MAX_TX_QUEUES];
>  	UPT1_RxStats
> saved_rx_stats[VMXNET3_EXT_MAX_RX_QUEUES];
> -	UPT1_TxStats          snapshot_tx_stats[VMXNET3_MAX_TX_QUEUES];
> -	UPT1_RxStats          snapshot_rx_stats[VMXNET3_MAX_RX_QUEUES];
> +	UPT1_TxStats
> snapshot_tx_stats[VMXNET3_EXT_MAX_TX_QUEUES];
> +	UPT1_RxStats
> snapshot_rx_stats[VMXNET3_EXT_MAX_RX_QUEUES];
>  	uint16_t              tx_prod_offset;
>  	uint16_t              rx_prod_offset[2];
>  	/* device capability bit map */
> --
> 2.43.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] net/vmxnet3: support per-queue stats for fewer queues
  2024-10-25  9:27 ` [PATCH v2] net/vmxnet3: support per-queue stats for fewer queues Morten Brørup
  2024-10-27  8:47   ` Morten Brørup
@ 2024-11-01  0:28   ` Ferruh Yigit
  1 sibling, 0 replies; 9+ messages in thread
From: Ferruh Yigit @ 2024-11-01  0:28 UTC (permalink / raw)
  To: Morten Brørup, dev, Jochen Behrens

On 10/25/2024 10:27 AM, Morten Brørup wrote:
> Removed the requirement that the configured number of queues to provide
> statistics for (RTE_ETHDEV_QUEUE_STAT_CNTRS) cannot be less than the
> driver's max number of supported transmit queues (VMXNET3_MAX_TX_QUEUES).
> 
> Also improved support for virtual hardware version 6.
> 
> Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> ---
> v2:
> * Virtual hardware version 6 supports more queues; updated some arrays
>   accordingly.
> * Added support for larger MTU with virtual hardware version 6.
> ---
>  drivers/net/vmxnet3/vmxnet3_ethdev.c | 34 +++++++++++++++++-----------
>  drivers/net/vmxnet3/vmxnet3_ethdev.h |  4 ++--
>  2 files changed, 23 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
> index 78fac63ab6..1752c58069 100644
> --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
> +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
> @@ -1470,42 +1470,52 @@ vmxnet3_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
>  	struct vmxnet3_hw *hw = dev->data->dev_private;
>  	struct UPT1_TxStats txStats;
>  	struct UPT1_RxStats rxStats;
> +	uint64_t packets, bytes;
>  
>  	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD, VMXNET3_CMD_GET_STATS);
>  
>  	for (i = 0; i < hw->num_tx_queues; i++) {
>  		vmxnet3_tx_stats_get(hw, i, &txStats);
>  
> -		stats->q_opackets[i] = txStats.ucastPktsTxOK +
> +		packets = txStats.ucastPktsTxOK +
>  			txStats.mcastPktsTxOK +
>  			txStats.bcastPktsTxOK;
>  
> -		stats->q_obytes[i] = txStats.ucastBytesTxOK +
> +		bytes = txStats.ucastBytesTxOK +
>  			txStats.mcastBytesTxOK +
>  			txStats.bcastBytesTxOK;
>  
> -		stats->opackets += stats->q_opackets[i];
> -		stats->obytes += stats->q_obytes[i];
> +		stats->opackets += packets;
> +		stats->obytes += bytes;
>  		stats->oerrors += txStats.pktsTxError + txStats.pktsTxDiscard;
> +
> +		if (i < RTE_ETHDEV_QUEUE_STAT_CNTRS) {
> +			stats->q_opackets[i] = packets;
> +			stats->q_obytes[i] = bytes;
> +		}
>  	}
>  
>  	for (i = 0; i < hw->num_rx_queues; i++) {
>  		vmxnet3_rx_stats_get(hw, i, &rxStats);
>  
> -		stats->q_ipackets[i] = rxStats.ucastPktsRxOK +
> +		packets = rxStats.ucastPktsRxOK +
>  			rxStats.mcastPktsRxOK +
>  			rxStats.bcastPktsRxOK;
>  
> -		stats->q_ibytes[i] = rxStats.ucastBytesRxOK +
> +		bytes = rxStats.ucastBytesRxOK +
>  			rxStats.mcastBytesRxOK +
>  			rxStats.bcastBytesRxOK;
>  
> -		stats->ipackets += stats->q_ipackets[i];
> -		stats->ibytes += stats->q_ibytes[i];
> -
> -		stats->q_errors[i] = rxStats.pktsRxError;
> +		stats->ipackets += packets;
> +		stats->ibytes += bytes;
>  		stats->ierrors += rxStats.pktsRxError;
>  		stats->imissed += rxStats.pktsRxOutOfBuf;
> +
> +		if (i < RTE_ETHDEV_QUEUE_STAT_CNTRS) {
> +			stats->q_ipackets[i] = packets;
> +			stats->q_ibytes[i] = bytes;
> +			stats->q_errors[i] = rxStats.pktsRxError;
> +		}
>  	}
>

Ack, this is also a fix, since queue size can be up to
'VMXNET3_EXT_MAX_RX_QUEUES' (32) for version 6, even if
'RTE_ETHDEV_QUEUE_STAT_CNTRS > VMXNET3_MAX_TX_QUEUES',
'RTE_ETHDEV_QUEUE_STAT_CNTRS ' still can be smaller than
'VMXNET3_EXT_MAX_RX_QUEUES', which will cause accessing out of boundary
to queue stats array.


>  
>  	return 0;
> @@ -1521,8 +1531,6 @@ vmxnet3_dev_stats_reset(struct rte_eth_dev *dev)
>  
>  	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD, VMXNET3_CMD_GET_STATS);
>  
> -	RTE_BUILD_BUG_ON(RTE_ETHDEV_QUEUE_STAT_CNTRS < VMXNET3_MAX_TX_QUEUES);
> -
>  	for (i = 0; i < hw->num_tx_queues; i++) {
>  		vmxnet3_hw_tx_stats_get(hw, i, &txStats);
>  		memcpy(&hw->snapshot_tx_stats[i], &txStats,
> @@ -1566,7 +1574,7 @@ vmxnet3_dev_info_get(struct rte_eth_dev *dev,
>  	dev_info->min_rx_bufsize = 1518 + RTE_PKTMBUF_HEADROOM;
>  	dev_info->max_rx_pktlen = 16384; /* includes CRC, cf MAXFRS register */
>  	dev_info->min_mtu = VMXNET3_MIN_MTU;
> -	dev_info->max_mtu = VMXNET3_MAX_MTU;
> +	dev_info->max_mtu = VMXNET3_VERSION_GE_6(hw) ? VMXNET3_V6_MAX_MTU : VMXNET3_MAX_MTU;
>

Hi Morten,

The MTU update is unrelated with the main purpose of this patch, what do
you think to separate above to its own function.

>  	dev_info->speed_capa = RTE_ETH_LINK_SPEED_10G;
>  	dev_info->max_mac_addrs = VMXNET3_MAX_MAC_ADDRS;
>  
> diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.h b/drivers/net/vmxnet3/vmxnet3_ethdev.h
> index 2b3e2c4caa..e9ded6663d 100644
> --- a/drivers/net/vmxnet3/vmxnet3_ethdev.h
> +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.h
> @@ -121,8 +121,8 @@ struct vmxnet3_hw {
>  #define VMXNET3_VFT_TABLE_SIZE     (VMXNET3_VFT_SIZE * sizeof(uint32_t))
>  	UPT1_TxStats	      saved_tx_stats[VMXNET3_EXT_MAX_TX_QUEUES];
>  	UPT1_RxStats	      saved_rx_stats[VMXNET3_EXT_MAX_RX_QUEUES];
> -	UPT1_TxStats          snapshot_tx_stats[VMXNET3_MAX_TX_QUEUES];
> -	UPT1_RxStats          snapshot_rx_stats[VMXNET3_MAX_RX_QUEUES];
> +	UPT1_TxStats          snapshot_tx_stats[VMXNET3_EXT_MAX_TX_QUEUES];
> +	UPT1_RxStats          snapshot_rx_stats[VMXNET3_EXT_MAX_RX_QUEUES];
>

Ack, probably above needs to be fixed anyway, even with
"VMXNET3_MAX_TX_QUEUES < RTE_ETHDEV_QUEUE_STAT_CNTRS" restriction (as
RTE_ETHDEV_QUEUE_STAT_CNTRS can be configured to be a bigger value
during build).


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v3 1/2] net/vmxnet3: fix potential out of bounds access
  2024-10-24 11:01 [PATCH] net/vmxnet3: support per-queue stats for less queues Morten Brørup
  2024-10-25  9:27 ` [PATCH v2] net/vmxnet3: support per-queue stats for fewer queues Morten Brørup
@ 2024-11-04 10:52 ` Morten Brørup
  2024-11-04 10:52   ` [PATCH v3 2/2] net/vmxnet3: support larger MTU with version 6 Morten Brørup
  2024-11-06  2:04   ` [PATCH v3 1/2] net/vmxnet3: fix potential out of bounds access Ferruh Yigit
  1 sibling, 2 replies; 9+ messages in thread
From: Morten Brørup @ 2024-11-04 10:52 UTC (permalink / raw)
  To: dev, Ferruh Yigit, Jochen Behrens, Ronak Doshi; +Cc: Morten Brørup

With virtual hardware version 6, the max number of RX queues was increased
to VMXNET3_EXT_MAX_RX_QUEUES (32) from VMXNET3_MAX_RX_QUEUES (16),
similarly, the max number of TX queues was increased to
VMXNET3_EXT_MAX_TX_QUEUES (32) from VMXNET3_MAX_TX_QUEUES (8).
These increases were not fully considered in the PMD...

The configured number of queues to provide statistics for
(RTE_ETHDEV_QUEUE_STAT_CNTRS) can be smaller than driver's max number of
supported transmit queues for virtual hardware version 6
(VMXNET3_EXT_MAX_RX_QUEUES) (32), which will cause accessing the queue
stats array out of boundary if the application uses more than
RTE_ETHDEV_QUEUE_STAT_CNTRS queues.

This patch fixes this by two modifications:
- Increased stats array size to support hardware version 6.
- Respect RTE_ETHDEV_QUEUE_STAT_CNTRS when getting the per-queue counters.

The build time check
RTE_BUILD_BUG_ON(RTE_ETHDEV_QUEUE_STAT_CNTRS < VMXNET3_MAX_TX_QUEUES)
has become irrelevant, so it is removed.
With this removal, per-queue stats for fewer queues is supported.

Fixes: b1584dd0affe ("net/vmxnet3: support version 6")

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
---
v3:
* This is a bugfix, not just an improvement. (Ferruh Yigit)
* Moved support for larger MTU with virtual hardware version 6 to separate
  patch. (Ferruh Yigit)
v2:
* Virtual hardware version 6 supports more queues; updated some arrays
  accordingly.
* Added support for larger MTU with virtual hardware version 6.
---
 drivers/net/vmxnet3/vmxnet3_ethdev.c | 32 +++++++++++++++++-----------
 drivers/net/vmxnet3/vmxnet3_ethdev.h |  4 ++--
 2 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
index 78fac63ab6..8a9bb452c6 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
@@ -1470,42 +1470,52 @@ vmxnet3_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 	struct vmxnet3_hw *hw = dev->data->dev_private;
 	struct UPT1_TxStats txStats;
 	struct UPT1_RxStats rxStats;
+	uint64_t packets, bytes;
 
 	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD, VMXNET3_CMD_GET_STATS);
 
 	for (i = 0; i < hw->num_tx_queues; i++) {
 		vmxnet3_tx_stats_get(hw, i, &txStats);
 
-		stats->q_opackets[i] = txStats.ucastPktsTxOK +
+		packets = txStats.ucastPktsTxOK +
 			txStats.mcastPktsTxOK +
 			txStats.bcastPktsTxOK;
 
-		stats->q_obytes[i] = txStats.ucastBytesTxOK +
+		bytes = txStats.ucastBytesTxOK +
 			txStats.mcastBytesTxOK +
 			txStats.bcastBytesTxOK;
 
-		stats->opackets += stats->q_opackets[i];
-		stats->obytes += stats->q_obytes[i];
+		stats->opackets += packets;
+		stats->obytes += bytes;
 		stats->oerrors += txStats.pktsTxError + txStats.pktsTxDiscard;
+
+		if (i < RTE_ETHDEV_QUEUE_STAT_CNTRS) {
+			stats->q_opackets[i] = packets;
+			stats->q_obytes[i] = bytes;
+		}
 	}
 
 	for (i = 0; i < hw->num_rx_queues; i++) {
 		vmxnet3_rx_stats_get(hw, i, &rxStats);
 
-		stats->q_ipackets[i] = rxStats.ucastPktsRxOK +
+		packets = rxStats.ucastPktsRxOK +
 			rxStats.mcastPktsRxOK +
 			rxStats.bcastPktsRxOK;
 
-		stats->q_ibytes[i] = rxStats.ucastBytesRxOK +
+		bytes = rxStats.ucastBytesRxOK +
 			rxStats.mcastBytesRxOK +
 			rxStats.bcastBytesRxOK;
 
-		stats->ipackets += stats->q_ipackets[i];
-		stats->ibytes += stats->q_ibytes[i];
-
-		stats->q_errors[i] = rxStats.pktsRxError;
+		stats->ipackets += packets;
+		stats->ibytes += bytes;
 		stats->ierrors += rxStats.pktsRxError;
 		stats->imissed += rxStats.pktsRxOutOfBuf;
+
+		if (i < RTE_ETHDEV_QUEUE_STAT_CNTRS) {
+			stats->q_ipackets[i] = packets;
+			stats->q_ibytes[i] = bytes;
+			stats->q_errors[i] = rxStats.pktsRxError;
+		}
 	}
 
 	return 0;
@@ -1521,8 +1531,6 @@ vmxnet3_dev_stats_reset(struct rte_eth_dev *dev)
 
 	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD, VMXNET3_CMD_GET_STATS);
 
-	RTE_BUILD_BUG_ON(RTE_ETHDEV_QUEUE_STAT_CNTRS < VMXNET3_MAX_TX_QUEUES);
-
 	for (i = 0; i < hw->num_tx_queues; i++) {
 		vmxnet3_hw_tx_stats_get(hw, i, &txStats);
 		memcpy(&hw->snapshot_tx_stats[i], &txStats,
diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.h b/drivers/net/vmxnet3/vmxnet3_ethdev.h
index 2b3e2c4caa..e9ded6663d 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.h
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.h
@@ -121,8 +121,8 @@ struct vmxnet3_hw {
 #define VMXNET3_VFT_TABLE_SIZE     (VMXNET3_VFT_SIZE * sizeof(uint32_t))
 	UPT1_TxStats	      saved_tx_stats[VMXNET3_EXT_MAX_TX_QUEUES];
 	UPT1_RxStats	      saved_rx_stats[VMXNET3_EXT_MAX_RX_QUEUES];
-	UPT1_TxStats          snapshot_tx_stats[VMXNET3_MAX_TX_QUEUES];
-	UPT1_RxStats          snapshot_rx_stats[VMXNET3_MAX_RX_QUEUES];
+	UPT1_TxStats          snapshot_tx_stats[VMXNET3_EXT_MAX_TX_QUEUES];
+	UPT1_RxStats          snapshot_rx_stats[VMXNET3_EXT_MAX_RX_QUEUES];
 	uint16_t              tx_prod_offset;
 	uint16_t              rx_prod_offset[2];
 	/* device capability bit map */
-- 
2.43.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v3 2/2] net/vmxnet3: support larger MTU with version 6
  2024-11-04 10:52 ` [PATCH v3 1/2] net/vmxnet3: fix potential out of bounds access Morten Brørup
@ 2024-11-04 10:52   ` Morten Brørup
  2024-11-04 21:44     ` Jochen Behrens
  2024-11-06  2:04   ` [PATCH v3 1/2] net/vmxnet3: fix potential out of bounds access Ferruh Yigit
  1 sibling, 1 reply; 9+ messages in thread
From: Morten Brørup @ 2024-11-04 10:52 UTC (permalink / raw)
  To: dev, Ferruh Yigit, Jochen Behrens, Ronak Doshi; +Cc: Morten Brørup

Virtual hardware version 6 supports larger max MTU, but the device
information (dev_info) did not reflect this, so it could not be used.

Fixes: b1584dd0affe ("net/vmxnet3: support version 6")

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
---
v3:
* Moved support for larger MTU with virtual hardware version 6 to separate
  patch. (Ferruh Yigit)
---
 drivers/net/vmxnet3/vmxnet3_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
index 8a9bb452c6..1752c58069 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
@@ -1574,7 +1574,7 @@ vmxnet3_dev_info_get(struct rte_eth_dev *dev,
 	dev_info->min_rx_bufsize = 1518 + RTE_PKTMBUF_HEADROOM;
 	dev_info->max_rx_pktlen = 16384; /* includes CRC, cf MAXFRS register */
 	dev_info->min_mtu = VMXNET3_MIN_MTU;
-	dev_info->max_mtu = VMXNET3_MAX_MTU;
+	dev_info->max_mtu = VMXNET3_VERSION_GE_6(hw) ? VMXNET3_V6_MAX_MTU : VMXNET3_MAX_MTU;
 	dev_info->speed_capa = RTE_ETH_LINK_SPEED_10G;
 	dev_info->max_mac_addrs = VMXNET3_MAX_MAC_ADDRS;
 
-- 
2.43.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 2/2] net/vmxnet3: support larger MTU with version 6
  2024-11-04 10:52   ` [PATCH v3 2/2] net/vmxnet3: support larger MTU with version 6 Morten Brørup
@ 2024-11-04 21:44     ` Jochen Behrens
  2024-11-06  2:04       ` Ferruh Yigit
  0 siblings, 1 reply; 9+ messages in thread
From: Jochen Behrens @ 2024-11-04 21:44 UTC (permalink / raw)
  To: Morten Brørup, dev, Ferruh Yigit, Ronak Doshi

Hi Morten,

thanks for fixing the MTU setting!

Acked-by: Jochen Behrens jochen.behrens@broadcom.com

On 11/4/24 02:52, Morten Brørup wrote:
> Virtual hardware version 6 supports larger max MTU, but the device
> information (dev_info) did not reflect this, so it could not be used.
> 
> Fixes: b1584dd0affe ("net/vmxnet3: support version 6")
> 
> Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> ---
> v3:
> * Moved support for larger MTU with virtual hardware version 6 to separate
>    patch. (Ferruh Yigit)
> ---
>   drivers/net/vmxnet3/vmxnet3_ethdev.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
> index 8a9bb452c6..1752c58069 100644
> --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
> +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
> @@ -1574,7 +1574,7 @@ vmxnet3_dev_info_get(struct rte_eth_dev *dev,
>   	dev_info->min_rx_bufsize = 1518 + RTE_PKTMBUF_HEADROOM;
>   	dev_info->max_rx_pktlen = 16384; /* includes CRC, cf MAXFRS register */
>   	dev_info->min_mtu = VMXNET3_MIN_MTU;
> -	dev_info->max_mtu = VMXNET3_MAX_MTU;
> +	dev_info->max_mtu = VMXNET3_VERSION_GE_6(hw) ? VMXNET3_V6_MAX_MTU : VMXNET3_MAX_MTU;
>   	dev_info->speed_capa = RTE_ETH_LINK_SPEED_10G;
>   	dev_info->max_mac_addrs = VMXNET3_MAX_MAC_ADDRS;
>   

-- 
This electronic communication and the information and any files transmitted 
with it, or attached to it, are confidential and are intended solely for 
the use of the individual or entity to whom it is addressed and may contain 
information that is confidential, legally privileged, protected by privacy 
laws, or otherwise restricted from disclosure to anyone else. If you are 
not the intended recipient or the person responsible for delivering the 
e-mail to the intended recipient, you are hereby notified that any use, 
copying, distributing, dissemination, forwarding, printing, or copying of 
this e-mail is strictly prohibited. If you received this e-mail in error, 
please return the e-mail to the sender, delete it from your computer, and 
destroy any printed copy of it.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 1/2] net/vmxnet3: fix potential out of bounds access
  2024-11-04 10:52 ` [PATCH v3 1/2] net/vmxnet3: fix potential out of bounds access Morten Brørup
  2024-11-04 10:52   ` [PATCH v3 2/2] net/vmxnet3: support larger MTU with version 6 Morten Brørup
@ 2024-11-06  2:04   ` Ferruh Yigit
  1 sibling, 0 replies; 9+ messages in thread
From: Ferruh Yigit @ 2024-11-06  2:04 UTC (permalink / raw)
  To: Morten Brørup, dev, Jochen Behrens, Ronak Doshi

On 11/4/2024 10:52 AM, Morten Brørup wrote:
> With virtual hardware version 6, the max number of RX queues was increased
> to VMXNET3_EXT_MAX_RX_QUEUES (32) from VMXNET3_MAX_RX_QUEUES (16),
> similarly, the max number of TX queues was increased to
> VMXNET3_EXT_MAX_TX_QUEUES (32) from VMXNET3_MAX_TX_QUEUES (8).
> These increases were not fully considered in the PMD...
> 
> The configured number of queues to provide statistics for
> (RTE_ETHDEV_QUEUE_STAT_CNTRS) can be smaller than driver's max number of
> supported transmit queues for virtual hardware version 6
> (VMXNET3_EXT_MAX_RX_QUEUES) (32), which will cause accessing the queue
> stats array out of boundary if the application uses more than
> RTE_ETHDEV_QUEUE_STAT_CNTRS queues.
> 
> This patch fixes this by two modifications:
> - Increased stats array size to support hardware version 6.
> - Respect RTE_ETHDEV_QUEUE_STAT_CNTRS when getting the per-queue counters.
> 
> The build time check
> RTE_BUILD_BUG_ON(RTE_ETHDEV_QUEUE_STAT_CNTRS < VMXNET3_MAX_TX_QUEUES)
> has become irrelevant, so it is removed.
> With this removal, per-queue stats for fewer queues is supported.
> 
> Fixes: b1584dd0affe ("net/vmxnet3: support version 6")
> 
> Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
>

Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>

Applied to dpdk-next-net/main, thanks.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 2/2] net/vmxnet3: support larger MTU with version 6
  2024-11-04 21:44     ` Jochen Behrens
@ 2024-11-06  2:04       ` Ferruh Yigit
  0 siblings, 0 replies; 9+ messages in thread
From: Ferruh Yigit @ 2024-11-06  2:04 UTC (permalink / raw)
  To: Jochen Behrens, Morten Brørup, dev, Ronak Doshi

On 11/4/2024 9:44 PM, Jochen Behrens wrote:

> On 11/4/24 02:52, Morten Brørup wrote:
>> Virtual hardware version 6 supports larger max MTU, but the device
>> information (dev_info) did not reflect this, so it could not be used.
>>
>> Fixes: b1584dd0affe ("net/vmxnet3: support version 6")
>>
>> Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
>
> Acked-by: Jochen Behrens jochen.behrens@broadcom.com
>

Applied to dpdk-next-net/main, thanks.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2024-11-06  2:05 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-10-24 11:01 [PATCH] net/vmxnet3: support per-queue stats for less queues Morten Brørup
2024-10-25  9:27 ` [PATCH v2] net/vmxnet3: support per-queue stats for fewer queues Morten Brørup
2024-10-27  8:47   ` Morten Brørup
2024-11-01  0:28   ` Ferruh Yigit
2024-11-04 10:52 ` [PATCH v3 1/2] net/vmxnet3: fix potential out of bounds access Morten Brørup
2024-11-04 10:52   ` [PATCH v3 2/2] net/vmxnet3: support larger MTU with version 6 Morten Brørup
2024-11-04 21:44     ` Jochen Behrens
2024-11-06  2:04       ` Ferruh Yigit
2024-11-06  2:04   ` [PATCH v3 1/2] net/vmxnet3: fix potential out of bounds access Ferruh Yigit

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).