diff mbox series

[v3,1/3] doc: announce bonding macro change

Message ID 20230718082856.2235450-2-chaoyong.he@corigine.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series announce bonding macro and function change |

Checks

Context Check Description
ci/checkpatch warning coding style issues

Commit Message

Chaoyong He July 18, 2023, 8:28 a.m. UTC 
  From: Long Wu <long.wu@corigine.com>

In order to support inclusive naming, some of the macro in DPDK will
need to be renamed. Do this through deprecation process now for 23.07.

Signed-off-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
---
 app/test-pmd/testpmd.c                 | 2 +-
 doc/guides/rel_notes/deprecation.rst   | 4 ++++
 drivers/net/bonding/rte_eth_bond_api.c | 6 +++---
 lib/ethdev/rte_ethdev.h                | 5 +++--
 4 files changed, 11 insertions(+), 6 deletions(-)

Comments

Ferruh Yigit July 18, 2023, 11:39 a.m. UTC | #1 
On 7/18/2023 9:28 AM, Chaoyong He wrote:
> From: Long Wu <long.wu@corigine.com>
> 
> In order to support inclusive naming, some of the macro in DPDK will
> need to be renamed. Do this through deprecation process now for 23.07.
> 
> Signed-off-by: Long Wu <long.wu@corigine.com>
> Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
>

Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>
Stephen Hemminger July 18, 2023, 3:53 p.m. UTC | #2 
On Tue, 18 Jul 2023 16:28:54 +0800
Chaoyong He <chaoyong.he@corigine.com> wrote:

> From: Long Wu <long.wu@corigine.com>
> 
> In order to support inclusive naming, some of the macro in DPDK will
> need to be renamed. Do this through deprecation process now for 23.07.
> 
> Signed-off-by: Long Wu <long.wu@corigine.com>
> Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>

Acked-by: Stephen Hemminger <stephen@networkplumber.org>
lihuisong (C) July 19, 2023, 1:24 a.m. UTC | #3 
+1 do not use slave in macro, data and function.
Acked-by: Huisong Li <lihuisong@huawei.com>

在 2023/7/18 16:28, Chaoyong He 写道:
> From: Long Wu <long.wu@corigine.com>
>
> In order to support inclusive naming, some of the macro in DPDK will
> need to be renamed. Do this through deprecation process now for 23.07.
>
> Signed-off-by: Long Wu <long.wu@corigine.com>
> Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
> ---
>   app/test-pmd/testpmd.c                 | 2 +-
>   doc/guides/rel_notes/deprecation.rst   | 4 ++++
>   drivers/net/bonding/rte_eth_bond_api.c | 6 +++---
>   lib/ethdev/rte_ethdev.h                | 5 +++--
>   4 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index c6ad9b18bf..938ca035d4 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -4248,7 +4248,7 @@ uint8_t port_is_bonding_slave(portid_t slave_pid)
>   			slave_pid);
>   		return 0;
>   	}
> -	if ((*dev_info.dev_flags & RTE_ETH_DEV_BONDED_SLAVE) || (port->slave_flag == 1))
> +	if ((*dev_info.dev_flags & RTE_ETH_DEV_BONDING_MEMBER) || (port->slave_flag == 1))
>   		return 1;
>   	return 0;
>   }
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index fb771a0305..f3f2baf0b9 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -118,6 +118,10 @@ Deprecation Notices
>     The legacy actions should be removed
>     once ``MODIFY_FIELD`` alternative is implemented in drivers.
>   
> +* bonding: The macro ``RTE_ETH_DEV_BONDED_SLAVE`` will be deprecated in
> +  DPDK 23.07, and removed in DPDK 23.11. The relevant code can be updated using
> +  ``RTE_ETH_DEV_BONDING_MEMBER``.
> +
>   * cryptodev: The function ``rte_cryptodev_cb_fn`` will be updated
>     to have another parameter ``qp_id`` to return the queue pair ID
>     which got error interrupt to the application,
> diff --git a/drivers/net/bonding/rte_eth_bond_api.c b/drivers/net/bonding/rte_eth_bond_api.c
> index 85d0528b7c..8b6cdce34a 100644
> --- a/drivers/net/bonding/rte_eth_bond_api.c
> +++ b/drivers/net/bonding/rte_eth_bond_api.c
> @@ -472,7 +472,7 @@ __eth_bond_slave_add_lock_free(uint16_t bonded_port_id, uint16_t slave_port_id)
>   		return -1;
>   
>   	slave_eth_dev = &rte_eth_devices[slave_port_id];
> -	if (slave_eth_dev->data->dev_flags & RTE_ETH_DEV_BONDED_SLAVE) {
> +	if (slave_eth_dev->data->dev_flags & RTE_ETH_DEV_BONDING_MEMBER) {
>   		RTE_BOND_LOG(ERR, "Slave device is already a slave of a bonded device");
>   		return -1;
>   	}
> @@ -615,7 +615,7 @@ __eth_bond_slave_add_lock_free(uint16_t bonded_port_id, uint16_t slave_port_id)
>   	}
>   
>   	/* Add slave details to bonded device */
> -	slave_eth_dev->data->dev_flags |= RTE_ETH_DEV_BONDED_SLAVE;
> +	slave_eth_dev->data->dev_flags |= RTE_ETH_DEV_BONDING_MEMBER;
>   
>   	slave_vlan_filter_set(bonded_port_id, slave_port_id);
>   
> @@ -724,7 +724,7 @@ __eth_bond_slave_remove_lock_free(uint16_t bonded_port_id,
>   
>   	slave_eth_dev = &rte_eth_devices[slave_port_id];
>   	slave_remove(internals, slave_eth_dev);
> -	slave_eth_dev->data->dev_flags &= (~RTE_ETH_DEV_BONDED_SLAVE);
> +	slave_eth_dev->data->dev_flags &= (~RTE_ETH_DEV_BONDING_MEMBER);
>   
>   	/*  first slave in the active list will be the primary by default,
>   	 *  otherwise use first device in list */
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index 3d44979b44..04a2564f22 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -2036,8 +2036,9 @@ struct rte_eth_dev_owner {
>   #define RTE_ETH_DEV_FLOW_OPS_THREAD_SAFE  RTE_BIT32(0)
>   /** Device supports link state interrupt */
>   #define RTE_ETH_DEV_INTR_LSC              RTE_BIT32(1)
> -/** Device is a bonded slave */
> -#define RTE_ETH_DEV_BONDED_SLAVE          RTE_BIT32(2)
> +/** Device is a bonding member */
> +#define RTE_ETH_DEV_BONDING_MEMBER        RTE_BIT32(2)
> +#define RTE_ETH_DEV_BONDED_SLAVE RTE_DEPRECATED(RTE_ETH_DEV_BONDED_SLAVE) RTE_ETH_DEV_BONDING_MEMBER
>   /** Device supports device removal interrupt */
>   #define RTE_ETH_DEV_INTR_RMV              RTE_BIT32(3)
>   /** Device is port representor */
Morten Brørup July 26, 2023, 11:47 a.m. UTC | #4 
> From: Chaoyong He [mailto:chaoyong.he@corigine.com]
> Sent: Tuesday, 18 July 2023 10.29
> 
> From: Long Wu <long.wu@corigine.com>
> 
> In order to support inclusive naming, some of the macro in DPDK will
> need to be renamed. Do this through deprecation process now for 23.07.
> 
> Signed-off-by: Long Wu <long.wu@corigine.com>
> Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
> ---
>  app/test-pmd/testpmd.c                 | 2 +-
>  doc/guides/rel_notes/deprecation.rst   | 4 ++++
>  drivers/net/bonding/rte_eth_bond_api.c | 6 +++---
>  lib/ethdev/rte_ethdev.h                | 5 +++--
>  4 files changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index c6ad9b18bf..938ca035d4 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -4248,7 +4248,7 @@ uint8_t port_is_bonding_slave(portid_t slave_pid)
>  			slave_pid);

We might want to get rid of the term "slave" in function, parameter and variable names in testpmd too. This is not important now, and can be postponed to a later patch.

>  		return 0;
>  	}
> -	if ((*dev_info.dev_flags & RTE_ETH_DEV_BONDED_SLAVE) || (port-
> >slave_flag == 1))
> +	if ((*dev_info.dev_flags & RTE_ETH_DEV_BONDING_MEMBER) || (port-
> >slave_flag == 1))

Can we please standardize on using only "bond" and "BOND" everywhere in this driver, instead of both "bond" (in function names) and "BONDING" (in enum values)?

The source code file are also named "rte_eth_bond...", while the directory name is /drivers/net/bonding/.

We are about to rename anyway, so let's do it as good as we can.


Also, are the bonding and balancing modes defined in /drivers/net/bonding/rte_eth_bond.h missing the RTE_ prefix, or are they private?


PS: Sorry about joining the discussion late (and possibly ignoring previous discussions on this). I reacted to the poll for ACKs to this series.
diff mbox series

Patch

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index c6ad9b18bf..938ca035d4 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -4248,7 +4248,7 @@  uint8_t port_is_bonding_slave(portid_t slave_pid)
 			slave_pid);
 		return 0;
 	}
-	if ((*dev_info.dev_flags & RTE_ETH_DEV_BONDED_SLAVE) || (port->slave_flag == 1))
+	if ((*dev_info.dev_flags & RTE_ETH_DEV_BONDING_MEMBER) || (port->slave_flag == 1))
 		return 1;
 	return 0;
 }
diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index fb771a0305..f3f2baf0b9 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -118,6 +118,10 @@  Deprecation Notices
   The legacy actions should be removed
   once ``MODIFY_FIELD`` alternative is implemented in drivers.
 
+* bonding: The macro ``RTE_ETH_DEV_BONDED_SLAVE`` will be deprecated in
+  DPDK 23.07, and removed in DPDK 23.11. The relevant code can be updated using
+  ``RTE_ETH_DEV_BONDING_MEMBER``.
+
 * cryptodev: The function ``rte_cryptodev_cb_fn`` will be updated
   to have another parameter ``qp_id`` to return the queue pair ID
   which got error interrupt to the application,
diff --git a/drivers/net/bonding/rte_eth_bond_api.c b/drivers/net/bonding/rte_eth_bond_api.c
index 85d0528b7c..8b6cdce34a 100644
--- a/drivers/net/bonding/rte_eth_bond_api.c
+++ b/drivers/net/bonding/rte_eth_bond_api.c
@@ -472,7 +472,7 @@  __eth_bond_slave_add_lock_free(uint16_t bonded_port_id, uint16_t slave_port_id)
 		return -1;
 
 	slave_eth_dev = &rte_eth_devices[slave_port_id];
-	if (slave_eth_dev->data->dev_flags & RTE_ETH_DEV_BONDED_SLAVE) {
+	if (slave_eth_dev->data->dev_flags & RTE_ETH_DEV_BONDING_MEMBER) {
 		RTE_BOND_LOG(ERR, "Slave device is already a slave of a bonded device");
 		return -1;
 	}
@@ -615,7 +615,7 @@  __eth_bond_slave_add_lock_free(uint16_t bonded_port_id, uint16_t slave_port_id)
 	}
 
 	/* Add slave details to bonded device */
-	slave_eth_dev->data->dev_flags |= RTE_ETH_DEV_BONDED_SLAVE;
+	slave_eth_dev->data->dev_flags |= RTE_ETH_DEV_BONDING_MEMBER;
 
 	slave_vlan_filter_set(bonded_port_id, slave_port_id);
 
@@ -724,7 +724,7 @@  __eth_bond_slave_remove_lock_free(uint16_t bonded_port_id,
 
 	slave_eth_dev = &rte_eth_devices[slave_port_id];
 	slave_remove(internals, slave_eth_dev);
-	slave_eth_dev->data->dev_flags &= (~RTE_ETH_DEV_BONDED_SLAVE);
+	slave_eth_dev->data->dev_flags &= (~RTE_ETH_DEV_BONDING_MEMBER);
 
 	/*  first slave in the active list will be the primary by default,
 	 *  otherwise use first device in list */
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index 3d44979b44..04a2564f22 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -2036,8 +2036,9 @@  struct rte_eth_dev_owner {
 #define RTE_ETH_DEV_FLOW_OPS_THREAD_SAFE  RTE_BIT32(0)
 /** Device supports link state interrupt */
 #define RTE_ETH_DEV_INTR_LSC              RTE_BIT32(1)
-/** Device is a bonded slave */
-#define RTE_ETH_DEV_BONDED_SLAVE          RTE_BIT32(2)
+/** Device is a bonding member */
+#define RTE_ETH_DEV_BONDING_MEMBER        RTE_BIT32(2)
+#define RTE_ETH_DEV_BONDED_SLAVE RTE_DEPRECATED(RTE_ETH_DEV_BONDED_SLAVE) RTE_ETH_DEV_BONDING_MEMBER
 /** Device supports device removal interrupt */
 #define RTE_ETH_DEV_INTR_RMV              RTE_BIT32(3)
 /** Device is port representor */