Re: [PATCH v2] ethdev: queue rate parameter changed from 16b to 32b

[Andrew Rybchenko]

On 9/23/22 16:45, skotesh...@marvell.com wrote:

From: Satha Rao <skotesh...@marvell.com>
The rate parameter modified to uint32_t, so that it can work
for more than 64 Gbps.

Signed-off-by: Satha Rao <skotesh...@marvell.com>

Overall LGTM, but please update release notes and cleanup
deprecation in the next version.

However, the patch requires Acks from bnxt and ixgbe
maintainers.

diff --git a/drivers/net/bnxt/rte_pmd_bnxt.c b/drivers/net/bnxt/rte_pmd_bnxt.c
index 77ecbef..4dc38a2 100644
--- a/drivers/net/bnxt/rte_pmd_bnxt.c
+++ b/drivers/net/bnxt/rte_pmd_bnxt.c
@@ -172,12 +172,12 @@ int rte_pmd_bnxt_set_vf_mac_addr(uint16_t port, uint16_t 
vf,
  }

int rte_pmd_bnxt_set_vf_rate_limit(uint16_t port, uint16_t vf,

-                               uint16_t tx_rate, uint64_t q_msk)
+                               uint32_t tx_rate, uint64_t q_msk)

Deprecateion announces rte_eth_set_queue_rate_limit() changes,
but above is a separate API which is not directly related.
I'm OK with the change, but it requires maintainer Ack.

  {
        struct rte_eth_dev *eth_dev;
        struct rte_eth_dev_info dev_info;
        struct bnxt *bp;
-       uint16_t tot_rate = 0;
+       uint32_t tot_rate = 0;
        uint64_t idx;
        int rc;

[snip]

diff --git a/drivers/net/cnxk/cnxk_ethdev.h b/drivers/net/cnxk/cnxk_ethdev.h
index c09e9bf..17c820a 100644
--- a/drivers/net/cnxk/cnxk_ethdev.h
+++ b/drivers/net/cnxk/cnxk_ethdev.h
@@ -557,17 +557,14 @@ int cnxk_nix_timesync_write_time(struct rte_eth_dev 
*eth_dev,

uint64_t cnxk_nix_rxq_mbuf_setup(struct cnxk_eth_dev *dev);

  int cnxk_nix_tm_ops_get(struct rte_eth_dev *eth_dev, void *ops);
-int cnxk_nix_tm_set_queue_rate_limit(struct rte_eth_dev *eth_dev,
-                                    uint16_t queue_idx, uint16_t tx_rate);
-int cnxk_nix_tm_mark_vlan_dei(struct rte_eth_dev *eth_dev, int mark_green,
-                             int mark_yellow, int mark_red,
-                             struct rte_tm_error *error);
-int cnxk_nix_tm_mark_ip_ecn(struct rte_eth_dev *eth_dev, int mark_green,
-                           int mark_yellow, int mark_red,
-                           struct rte_tm_error *error);
-int cnxk_nix_tm_mark_ip_dscp(struct rte_eth_dev *eth_dev, int mark_green,
-                            int mark_yellow, int mark_red,
-                            struct rte_tm_error *error);

Above changes for 3 functions look unrelated. Please, avoid it.

+int cnxk_nix_tm_set_queue_rate_limit(struct rte_eth_dev *eth_dev, uint16_t 
queue_idx,
+                                    uint32_t tx_rate);
+int cnxk_nix_tm_mark_vlan_dei(struct rte_eth_dev *eth_dev, int mark_green, int 
mark_yellow,
+                             int mark_red, struct rte_tm_error *error);
+int cnxk_nix_tm_mark_ip_ecn(struct rte_eth_dev *eth_dev, int mark_green, int 
mark_yellow,
+                           int mark_red, struct rte_tm_error *error);
+int cnxk_nix_tm_mark_ip_dscp(struct rte_eth_dev *eth_dev, int mark_green, int 
mark_yellow,
+                            int mark_red, struct rte_tm_error *error);

/* MTR */

  int cnxk_nix_mtr_ops_get(struct rte_eth_dev *dev, void *ops);
diff --git a/drivers/net/cnxk/cnxk_tm.c b/drivers/net/cnxk/cnxk_tm.c
index d45e70a..a36f45d 100644

[snip]

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 1dfad0e..9ff8ee0 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -2475,7 +2475,7 @@ static int eth_ixgbevf_pci_remove(struct rte_pci_device 
*pci_dev)

int

  ixgbe_set_vf_rate_limit(struct rte_eth_dev *dev, uint16_t vf,
-                       uint16_t tx_rate, uint64_t q_msk)
+                       uint32_t tx_rate, uint64_t q_msk)

Requires Ack from ixgbe maintainer

[snip]