Thanks Andrew for your comments. Sent v3 with your review comments. Thanks, Satha.
-----Original Message----- From: Andrew Rybchenko <[email protected]> Sent: Monday, September 26, 2022 6:48 PM To: Satha Koteswara Rao Kottidi <[email protected]>; Aman Singh <[email protected]>; Yuying Zhang <[email protected]>; Ajit Khaparde <[email protected]>; Somnath Kotur <[email protected]>; Nithin Kumar Dabilpuram <[email protected]>; Kiran Kumar Kokkilagadda <[email protected]>; Sunil Kumar Kori <[email protected]>; Qiming Yang <[email protected]>; Wenjun Wu <[email protected]>; Jiawen Wu <[email protected]>; Jian Wang <[email protected]>; Thomas Monjalon <[email protected]>; Ferruh Yigit <[email protected]> Cc: [email protected]; [email protected]; [email protected]; [email protected]; Jerin Jacob Kollanukkaran <[email protected]> Subject: [EXT] Re: [PATCH v2] ethdev: queue rate parameter changed from 16b to 32b External Email ---------------------------------------------------------------------- On 9/23/22 16:45, [email protected] wrote: > From: Satha Rao <[email protected]> > > The rate parameter modified to uint32_t, so that it can work for more > than 64 Gbps. > > Signed-off-by: Satha Rao <[email protected]> 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]
