[PATCH 0/4] ethdev: rework config restore

[PATCH 0/4] ethdev: rework config restore

[Dariusz Sosnowski]

This patch series reworks the config restore procedure,
so that drivers are able to enable/disable certain parts of it.
Drivers can provide get_restore_flags() callback,
which will indicate to ethdev library what configuration to restore.

If callback is not defined, then ethdev assumes that
all configuration must be restored, preserving the current behavior for all drivers.

This patch series also includes implementation of get_restore_flags()
for mlx5 PMD, which does not require config restore.

RFC: https://inbox.dpdk.org/dev/20240918092201.33772-1-dsosnowski@nvidia.com/

Dariusz Sosnowski (4):
  ethdev: rework config restore
  ethdev: add get restore flags driver callback
  ethdev: restore config only when requested
  net/mlx5: disable config restore

 drivers/net/mlx5/mlx5.c        |  2 ++
 drivers/net/mlx5/mlx5.h        |  3 ++
 drivers/net/mlx5/mlx5_ethdev.c | 19 ++++++++++
 lib/ethdev/ethdev_driver.c     | 11 ++++++
 lib/ethdev/ethdev_driver.h     | 64 ++++++++++++++++++++++++++++++++++
 lib/ethdev/rte_ethdev.c        | 49 ++++++++++++++++++++++----
 lib/ethdev/version.map         |  1 +
 7 files changed, 142 insertions(+), 7 deletions(-)

--
2.39.5

[PATCH 1/4] ethdev: rework config restore

[Dariusz Sosnowski]

Extract promiscuous and all multicast configuration restore
to separate functions.
This change will allow easier integration of disabling
these procedures for supporting PMDs in follow up commits.

Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 lib/ethdev/rte_ethdev.c | 34 +++++++++++++++++++++++++++++-----
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index f1c658f49e..362a1883f0 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -1648,14 +1648,10 @@ eth_dev_mac_restore(struct rte_eth_dev *dev,
 }
 
 static int
-eth_dev_config_restore(struct rte_eth_dev *dev,
-		struct rte_eth_dev_info *dev_info, uint16_t port_id)
+eth_dev_promiscuous_restore(struct rte_eth_dev *dev, uint16_t port_id)
 {
 	int ret;
 
-	if (!(*dev_info->dev_flags & RTE_ETH_DEV_NOLIVE_MAC_ADDR))
-		eth_dev_mac_restore(dev, dev_info);
-
 	/* replay promiscuous configuration */
 	/*
 	 * use callbacks directly since we don't need port_id check and
@@ -1683,6 +1679,14 @@ eth_dev_config_restore(struct rte_eth_dev *dev,
 		}
 	}
 
+	return 0;
+}
+
+static int
+eth_dev_allmulticast_restore(struct rte_eth_dev *dev, uint16_t port_id)
+{
+	int ret;
+
 	/* replay all multicast configuration */
 	/*
 	 * use callbacks directly since we don't need port_id check and
@@ -1713,6 +1717,26 @@ eth_dev_config_restore(struct rte_eth_dev *dev,
 	return 0;
 }
 
+static int
+eth_dev_config_restore(struct rte_eth_dev *dev,
+		struct rte_eth_dev_info *dev_info, uint16_t port_id)
+{
+	int ret;
+
+	if (!(*dev_info->dev_flags & RTE_ETH_DEV_NOLIVE_MAC_ADDR))
+		eth_dev_mac_restore(dev, dev_info);
+
+	ret = eth_dev_promiscuous_restore(dev, port_id);
+	if (ret != 0)
+		return ret;
+
+	ret = eth_dev_allmulticast_restore(dev, port_id);
+	if (ret != 0)
+		return ret;
+
+	return 0;
+}
+
 int
 rte_eth_dev_start(uint16_t port_id)
 {
-- 
2.39.5

[PATCH 2/4] ethdev: add get restore flags driver callback

[Dariusz Sosnowski]

Before this patch, ethdev layer assumed that all drivers require that
it has to forcefully restore:

- MAC addresses
- promiscuous mode setting
- all multicast mode setting

upon rte_eth_dev_start().

This patch introduces a new callback to eth_dev_ops -
get_restore_flags().
Drivers implementing this callback can explicitly enable/disable
certain parts of config restore procedure.

In order to minimize the changes to all the drivers and
preserve the current behavior, it is assumed that
if this callback is not defined, all configuration should be restored.

Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 lib/ethdev/ethdev_driver.c | 11 +++++++
 lib/ethdev/ethdev_driver.h | 64 ++++++++++++++++++++++++++++++++++++++
 lib/ethdev/version.map     |  1 +
 3 files changed, 76 insertions(+)

diff --git a/lib/ethdev/ethdev_driver.c b/lib/ethdev/ethdev_driver.c
index c335a25a82..f78f9fb5c1 100644
--- a/lib/ethdev/ethdev_driver.c
+++ b/lib/ethdev/ethdev_driver.c
@@ -958,3 +958,14 @@ rte_eth_switch_domain_free(uint16_t domain_id)
 
 	return 0;
 }
+
+void
+rte_eth_get_restore_flags(struct rte_eth_dev *dev,
+			  enum rte_eth_dev_operation op,
+			  uint32_t *flags)
+{
+	if (dev->dev_ops->get_restore_flags != NULL)
+		dev->dev_ops->get_restore_flags(dev, op, flags);
+	else
+		*flags = RTE_ETH_RESTORE_ALL;
+}
diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
index ae00ead865..3617c6951d 100644
--- a/lib/ethdev/ethdev_driver.h
+++ b/lib/ethdev/ethdev_driver.h
@@ -1235,6 +1235,47 @@ typedef int (*eth_count_aggr_ports_t)(struct rte_eth_dev *dev);
 typedef int (*eth_map_aggr_tx_affinity_t)(struct rte_eth_dev *dev, uint16_t tx_queue_id,
 					  uint8_t affinity);
 
+/**
+ * @internal
+ * Defines types of operations which can be executed by the application.
+ */
+enum rte_eth_dev_operation {
+	RTE_ETH_START,
+};
+
+/**@{@name Restore flags
+ * Flags returned by get_restore_flags() callback.
+ * They indicate to ethdev layer which configuration is required to be restored.
+ */
+/** If set, ethdev layer will forcefully reapply default and any other added MAC addresses. */
+#define RTE_ETH_RESTORE_MAC_ADDR RTE_BIT32(0)
+/** If set, ethdev layer will forcefully reapply current promiscuous mode setting. */
+#define RTE_ETH_RESTORE_PROMISC  RTE_BIT32(1)
+/** If set, ethdev layer will forcefully reapply current all multicast mode setting. */
+#define RTE_ETH_RESTORE_ALLMULTI RTE_BIT32(2)
+/**@}*/
+
+/** All configuration which can be restored by ethdev layer. */
+#define RTE_ETH_RESTORE_ALL (RTE_ETH_RESTORE_MAC_ADDR | \
+			     RTE_ETH_RESTORE_PROMISC | \
+			     RTE_ETH_RESTORE_ALLMULTI)
+
+/**
+ * @internal
+ * Fetch from the driver what kind of configuration must be restored by ethdev layer,
+ * after certain operations are performed by the application (such as rte_eth_dev_start()).
+ *
+ * @param dev
+ *   Port (ethdev) handle.
+ * @param op
+ *   Type of operation executed by the application.
+ * @param flags
+ *   Flags indicating what configuration must be restored by ethdev layer.
+ */
+typedef void (*eth_get_restore_flags_t)(struct rte_eth_dev *dev,
+					enum rte_eth_dev_operation op,
+					uint32_t *flags);
+
 /**
  * @internal A structure containing the functions exported by an Ethernet driver.
  */
@@ -1474,6 +1515,9 @@ struct eth_dev_ops {
 	eth_count_aggr_ports_t count_aggr_ports;
 	/** Map a Tx queue with an aggregated port of the DPDK port */
 	eth_map_aggr_tx_affinity_t map_aggr_tx_affinity;
+
+	/** Get configuration which ethdev should restore */
+	eth_get_restore_flags_t get_restore_flags;
 };
 
 /**
@@ -2131,6 +2175,26 @@ struct rte_eth_fdir_conf {
 	struct rte_eth_fdir_flex_conf flex_conf;
 };
 
+/**
+ * @internal
+ * Fetch from the driver what kind of configuration must be restarted by ethdev layer,
+ * using get_restore_flags() callback.
+ *
+ * If callback is not defined, it is assumed that all supported configuration must be restored.
+ *
+ * @param dev
+ *   Port (ethdev) handle.
+ * @param op
+ *   Type of operation executed by the application.
+ * @param affinity
+ *   Flags indicating what configuration must be restored by ethdev layer.
+ */
+__rte_internal
+void
+rte_eth_get_restore_flags(struct rte_eth_dev *dev,
+			  enum rte_eth_dev_operation op,
+			  uint32_t *flags);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index 1669055ca5..fa0469e602 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -358,4 +358,5 @@ INTERNAL {
 	rte_eth_switch_domain_alloc;
 	rte_eth_switch_domain_free;
 	rte_flow_fp_default_ops;
+	rte_eth_get_restore_flags;
 };
-- 
2.39.5

[PATCH 3/4] ethdev: restore config only when requested

[Dariusz Sosnowski]

Use get_restore_flags() internal API introduced in previous commits
in rte_eth_dev_start(), to restore only the configuration
requested by the driver.

Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 lib/ethdev/rte_ethdev.c | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 362a1883f0..9e82556374 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -1719,20 +1719,27 @@ eth_dev_allmulticast_restore(struct rte_eth_dev *dev, uint16_t port_id)
 
 static int
 eth_dev_config_restore(struct rte_eth_dev *dev,
-		struct rte_eth_dev_info *dev_info, uint16_t port_id)
+		struct rte_eth_dev_info *dev_info,
+		uint32_t restore_flags,
+		uint16_t port_id)
 {
 	int ret;
 
-	if (!(*dev_info->dev_flags & RTE_ETH_DEV_NOLIVE_MAC_ADDR))
+	if (!(*dev_info->dev_flags & RTE_ETH_DEV_NOLIVE_MAC_ADDR) &&
+	    (restore_flags & RTE_ETH_RESTORE_MAC_ADDR))
 		eth_dev_mac_restore(dev, dev_info);
 
-	ret = eth_dev_promiscuous_restore(dev, port_id);
-	if (ret != 0)
-		return ret;
+	if (restore_flags & RTE_ETH_RESTORE_PROMISC) {
+		ret = eth_dev_promiscuous_restore(dev, port_id);
+		if (ret != 0)
+			return ret;
+	}
 
-	ret = eth_dev_allmulticast_restore(dev, port_id);
-	if (ret != 0)
-		return ret;
+	if (restore_flags & RTE_ETH_RESTORE_ALLMULTI) {
+		ret = eth_dev_allmulticast_restore(dev, port_id);
+		if (ret != 0)
+			return ret;
+	}
 
 	return 0;
 }
@@ -1742,6 +1749,7 @@ rte_eth_dev_start(uint16_t port_id)
 {
 	struct rte_eth_dev *dev;
 	struct rte_eth_dev_info dev_info;
+	uint32_t restore_flags;
 	int diag;
 	int ret, ret_stop;
 
@@ -1769,8 +1777,11 @@ rte_eth_dev_start(uint16_t port_id)
 	if (ret != 0)
 		return ret;
 
+	rte_eth_get_restore_flags(dev, RTE_ETH_START, &restore_flags);
+
 	/* Lets restore MAC now if device does not support live change */
-	if (*dev_info.dev_flags & RTE_ETH_DEV_NOLIVE_MAC_ADDR)
+	if ((*dev_info.dev_flags & RTE_ETH_DEV_NOLIVE_MAC_ADDR) &&
+	    (restore_flags & RTE_ETH_RESTORE_MAC_ADDR))
 		eth_dev_mac_restore(dev, &dev_info);
 
 	diag = (*dev->dev_ops->dev_start)(dev);
@@ -1779,7 +1790,7 @@ rte_eth_dev_start(uint16_t port_id)
 	else
 		return eth_err(port_id, diag);
 
-	ret = eth_dev_config_restore(dev, &dev_info, port_id);
+	ret = eth_dev_config_restore(dev, &dev_info, restore_flags, port_id);
 	if (ret != 0) {
 		RTE_ETHDEV_LOG_LINE(ERR,
 			"Error during restoring configuration for device (port %u): %s",
-- 
2.39.5

[PATCH 4/4] net/mlx5: disable config restore

[Dariusz Sosnowski]

mlx5 PMD does not require configuration restore
on rte_eth_dev_start().
Add implementation of get_restore_flags() indicating that.

Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 drivers/net/mlx5/mlx5.c        |  2 ++
 drivers/net/mlx5/mlx5.h        |  3 +++
 drivers/net/mlx5/mlx5_ethdev.c | 19 +++++++++++++++++++
 3 files changed, 24 insertions(+)

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 8d266b0e64..9b6acaf7f1 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -2571,6 +2571,7 @@ const struct eth_dev_ops mlx5_dev_ops = {
 	.count_aggr_ports = mlx5_count_aggr_ports,
 	.map_aggr_tx_affinity = mlx5_map_aggr_tx_affinity,
 	.rx_metadata_negotiate = mlx5_flow_rx_metadata_negotiate,
+	.get_restore_flags = mlx5_get_restore_flags,
 };

 /* Available operations from secondary process. */
@@ -2663,6 +2664,7 @@ const struct eth_dev_ops mlx5_dev_ops_isolate = {
 	.get_monitor_addr = mlx5_get_monitor_addr,
 	.count_aggr_ports = mlx5_count_aggr_ports,
 	.map_aggr_tx_affinity = mlx5_map_aggr_tx_affinity,
+	.get_restore_flags = mlx5_get_restore_flags,
 };

 /**
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 869aac032b..a5829fb71a 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -2228,6 +2228,9 @@ eth_rx_burst_t mlx5_select_rx_function(struct rte_eth_dev *dev);
 struct mlx5_priv *mlx5_port_to_eswitch_info(uint16_t port, bool valid);
 struct mlx5_priv *mlx5_dev_to_eswitch_info(struct rte_eth_dev *dev);
 int mlx5_dev_configure_rss_reta(struct rte_eth_dev *dev);
+void mlx5_get_restore_flags(struct rte_eth_dev *dev,
+			    enum rte_eth_dev_operation op,
+			    uint32_t *flags);

 /* mlx5_ethdev_os.c */

diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index 6a678d6dcc..8b78efc3fd 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -796,3 +796,22 @@ mlx5_hairpin_cap_get(struct rte_eth_dev *dev, struct rte_eth_hairpin_cap *cap)
 	cap->tx_cap.rte_memory = hca_attr->hairpin_sq_wq_in_host_mem;
 	return 0;
 }
+
+/**
+ * Indicate to ethdev layer, what configuration must be restored.
+ *
+ * @param[in] dev
+ *   Pointer to Ethernet device structure.
+ * @param[in] op
+ *   Type of operation which might require config restore.
+ * @param[out] flags
+ *   Restore flags will be stored here.
+ */
+void
+mlx5_get_restore_flags(__rte_unused struct rte_eth_dev *dev,
+		       __rte_unused enum rte_eth_dev_operation op,
+		       uint32_t *flags)
+{
+	/* mlx5 PMD does not require any configuration restore. */
+	*flags = 0;
+}
--
2.39.5

[PATCH v2 0/4] ethdev: rework config restore

[Dariusz Sosnowski]

This patch series reworks the config restore procedure,
so that drivers are able to enable/disable certain parts of it.
Drivers can provide get_restore_flags() callback,
which will indicate to ethdev library what configuration to restore.

If callback is not defined, then ethdev assumes that
all configuration must be restored, preserving the current behavior for all drivers.

This patch series also includes implementation of get_restore_flags()
for mlx5 PMD, which does not require config restore.

RFC: https://inbox.dpdk.org/dev/20240918092201.33772-1-dsosnowski@nvidia.com/

v2:
- Fix typos in API docs.

Dariusz Sosnowski (4):
  ethdev: rework config restore
  ethdev: add get restore flags driver callback
  ethdev: restore config only when requested
  net/mlx5: disable config restore

 drivers/net/mlx5/mlx5.c        |  2 ++
 drivers/net/mlx5/mlx5.h        |  3 ++
 drivers/net/mlx5/mlx5_ethdev.c | 19 ++++++++++
 lib/ethdev/ethdev_driver.c     | 11 ++++++
 lib/ethdev/ethdev_driver.h     | 64 ++++++++++++++++++++++++++++++++++
 lib/ethdev/rte_ethdev.c        | 49 ++++++++++++++++++++++----
 lib/ethdev/version.map         |  1 +
 7 files changed, 142 insertions(+), 7 deletions(-)

--
2.39.5

[PATCH v2 1/4] ethdev: rework config restore

[Dariusz Sosnowski]

Extract promiscuous and all multicast configuration restore
to separate functions.
This change will allow easier integration of disabling
these procedures for supporting PMDs in follow up commits.

Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 lib/ethdev/rte_ethdev.c | 34 +++++++++++++++++++++++++++++-----
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index f1c658f49e..362a1883f0 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -1648,14 +1648,10 @@ eth_dev_mac_restore(struct rte_eth_dev *dev,
 }
 
 static int
-eth_dev_config_restore(struct rte_eth_dev *dev,
-		struct rte_eth_dev_info *dev_info, uint16_t port_id)
+eth_dev_promiscuous_restore(struct rte_eth_dev *dev, uint16_t port_id)
 {
 	int ret;
 
-	if (!(*dev_info->dev_flags & RTE_ETH_DEV_NOLIVE_MAC_ADDR))
-		eth_dev_mac_restore(dev, dev_info);
-
 	/* replay promiscuous configuration */
 	/*
 	 * use callbacks directly since we don't need port_id check and
@@ -1683,6 +1679,14 @@ eth_dev_config_restore(struct rte_eth_dev *dev,
 		}
 	}
 
+	return 0;
+}
+
+static int
+eth_dev_allmulticast_restore(struct rte_eth_dev *dev, uint16_t port_id)
+{
+	int ret;
+
 	/* replay all multicast configuration */
 	/*
 	 * use callbacks directly since we don't need port_id check and
@@ -1713,6 +1717,26 @@ eth_dev_config_restore(struct rte_eth_dev *dev,
 	return 0;
 }
 
+static int
+eth_dev_config_restore(struct rte_eth_dev *dev,
+		struct rte_eth_dev_info *dev_info, uint16_t port_id)
+{
+	int ret;
+
+	if (!(*dev_info->dev_flags & RTE_ETH_DEV_NOLIVE_MAC_ADDR))
+		eth_dev_mac_restore(dev, dev_info);
+
+	ret = eth_dev_promiscuous_restore(dev, port_id);
+	if (ret != 0)
+		return ret;
+
+	ret = eth_dev_allmulticast_restore(dev, port_id);
+	if (ret != 0)
+		return ret;
+
+	return 0;
+}
+
 int
 rte_eth_dev_start(uint16_t port_id)
 {
-- 
2.39.5

[PATCH v2 2/4] ethdev: add get restore flags driver callback

[Dariusz Sosnowski]

Before this patch, ethdev layer assumed that all drivers require that
it has to forcefully restore:

- MAC addresses
- promiscuous mode setting
- all multicast mode setting

upon rte_eth_dev_start().

This patch introduces a new callback to eth_dev_ops -
get_restore_flags().
Drivers implementing this callback can explicitly enable/disable
certain parts of config restore procedure.

In order to minimize the changes to all the drivers and
preserve the current behavior, it is assumed that
if this callback is not defined, all configuration should be restored.

Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 lib/ethdev/ethdev_driver.c | 11 +++++++
 lib/ethdev/ethdev_driver.h | 64 ++++++++++++++++++++++++++++++++++++++
 lib/ethdev/version.map     |  1 +
 3 files changed, 76 insertions(+)

diff --git a/lib/ethdev/ethdev_driver.c b/lib/ethdev/ethdev_driver.c
index c335a25a82..f78f9fb5c1 100644
--- a/lib/ethdev/ethdev_driver.c
+++ b/lib/ethdev/ethdev_driver.c
@@ -958,3 +958,14 @@ rte_eth_switch_domain_free(uint16_t domain_id)
 
 	return 0;
 }
+
+void
+rte_eth_get_restore_flags(struct rte_eth_dev *dev,
+			  enum rte_eth_dev_operation op,
+			  uint32_t *flags)
+{
+	if (dev->dev_ops->get_restore_flags != NULL)
+		dev->dev_ops->get_restore_flags(dev, op, flags);
+	else
+		*flags = RTE_ETH_RESTORE_ALL;
+}
diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
index ae00ead865..8ac5328521 100644
--- a/lib/ethdev/ethdev_driver.h
+++ b/lib/ethdev/ethdev_driver.h
@@ -1235,6 +1235,47 @@ typedef int (*eth_count_aggr_ports_t)(struct rte_eth_dev *dev);
 typedef int (*eth_map_aggr_tx_affinity_t)(struct rte_eth_dev *dev, uint16_t tx_queue_id,
 					  uint8_t affinity);
 
+/**
+ * @internal
+ * Defines types of operations which can be executed by the application.
+ */
+enum rte_eth_dev_operation {
+	RTE_ETH_START,
+};
+
+/**@{@name Restore flags
+ * Flags returned by get_restore_flags() callback.
+ * They indicate to ethdev layer which configuration is required to be restored.
+ */
+/** If set, ethdev layer will forcefully restore default and any other added MAC addresses. */
+#define RTE_ETH_RESTORE_MAC_ADDR RTE_BIT32(0)
+/** If set, ethdev layer will forcefully restore current promiscuous mode setting. */
+#define RTE_ETH_RESTORE_PROMISC  RTE_BIT32(1)
+/** If set, ethdev layer will forcefully restore current all multicast mode setting. */
+#define RTE_ETH_RESTORE_ALLMULTI RTE_BIT32(2)
+/**@}*/
+
+/** All configuration which can be restored by ethdev layer. */
+#define RTE_ETH_RESTORE_ALL (RTE_ETH_RESTORE_MAC_ADDR | \
+			     RTE_ETH_RESTORE_PROMISC | \
+			     RTE_ETH_RESTORE_ALLMULTI)
+
+/**
+ * @internal
+ * Fetch from the driver what kind of configuration must be restored by ethdev layer,
+ * after certain operations are performed by the application (such as rte_eth_dev_start()).
+ *
+ * @param dev
+ *   Port (ethdev) handle.
+ * @param op
+ *   Type of operation executed by the application.
+ * @param flags
+ *   Flags indicating what configuration must be restored by ethdev layer.
+ */
+typedef void (*eth_get_restore_flags_t)(struct rte_eth_dev *dev,
+					enum rte_eth_dev_operation op,
+					uint32_t *flags);
+
 /**
  * @internal A structure containing the functions exported by an Ethernet driver.
  */
@@ -1474,6 +1515,9 @@ struct eth_dev_ops {
 	eth_count_aggr_ports_t count_aggr_ports;
 	/** Map a Tx queue with an aggregated port of the DPDK port */
 	eth_map_aggr_tx_affinity_t map_aggr_tx_affinity;
+
+	/** Get configuration which ethdev should restore */
+	eth_get_restore_flags_t get_restore_flags;
 };
 
 /**
@@ -2131,6 +2175,26 @@ struct rte_eth_fdir_conf {
 	struct rte_eth_fdir_flex_conf flex_conf;
 };
 
+/**
+ * @internal
+ * Fetch from the driver what kind of configuration must be restored by ethdev layer,
+ * using get_restore_flags() callback.
+ *
+ * If callback is not defined, it is assumed that all supported configuration must be restored.
+ *
+ * @param dev
+ *   Port (ethdev) handle.
+ * @param op
+ *   Type of operation executed by the application.
+ * @param flags
+ *   Flags indicating what configuration must be restored by ethdev layer.
+ */
+__rte_internal
+void
+rte_eth_get_restore_flags(struct rte_eth_dev *dev,
+			  enum rte_eth_dev_operation op,
+			  uint32_t *flags);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index 1669055ca5..fa0469e602 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -358,4 +358,5 @@ INTERNAL {
 	rte_eth_switch_domain_alloc;
 	rte_eth_switch_domain_free;
 	rte_flow_fp_default_ops;
+	rte_eth_get_restore_flags;
 };
-- 
2.39.5

RE: [PATCH v2 2/4] ethdev: add get restore flags driver callback

[Konstantin Ananyev]

> Before this patch, ethdev layer assumed that all drivers require that
> it has to forcefully restore:
> 
> - MAC addresses
> - promiscuous mode setting
> - all multicast mode setting
> 
> upon rte_eth_dev_start().
> 
> This patch introduces a new callback to eth_dev_ops -
> get_restore_flags().
> Drivers implementing this callback can explicitly enable/disable
> certain parts of config restore procedure.
> 
> In order to minimize the changes to all the drivers and
> preserve the current behavior, it is assumed that
> if this callback is not defined, all configuration should be restored.
> 
> Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>

LGTM, just few nits/suggestions, pls see below.
Series-Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>

> ---
>  lib/ethdev/ethdev_driver.c | 11 +++++++
>  lib/ethdev/ethdev_driver.h | 64 ++++++++++++++++++++++++++++++++++++++
>  lib/ethdev/version.map     |  1 +
>  3 files changed, 76 insertions(+)
> 
> diff --git a/lib/ethdev/ethdev_driver.c b/lib/ethdev/ethdev_driver.c
> index c335a25a82..f78f9fb5c1 100644
> --- a/lib/ethdev/ethdev_driver.c
> +++ b/lib/ethdev/ethdev_driver.c
> @@ -958,3 +958,14 @@ rte_eth_switch_domain_free(uint16_t domain_id)
> 
>  	return 0;
>  }
> +
> +void
> +rte_eth_get_restore_flags(struct rte_eth_dev *dev,
> +			  enum rte_eth_dev_operation op,
> +			  uint32_t *flags)

I would go with uint64_t for flags (to avoid limitations in future).
Also, If it is void anyway, might be just return flags?
i.e:
uint64_t  +rte_eth_get_restore_flags(struct rte_eth_dev *dev, enum rte_eth_dev_operation op);

Another thing - do we need to do update docs?
PMD or PG sections, release notes?

> +{
> +	if (dev->dev_ops->get_restore_flags != NULL)
> +		dev->dev_ops->get_restore_flags(dev, op, flags);
> +	else
> +		*flags = RTE_ETH_RESTORE_ALL;
> +}
> diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
> index ae00ead865..8ac5328521 100644
> --- a/lib/ethdev/ethdev_driver.h
> +++ b/lib/ethdev/ethdev_driver.h
> @@ -1235,6 +1235,47 @@ typedef int (*eth_count_aggr_ports_t)(struct rte_eth_dev *dev);
>  typedef int (*eth_map_aggr_tx_affinity_t)(struct rte_eth_dev *dev, uint16_t tx_queue_id,
>  					  uint8_t affinity);
> 
> +/**
> + * @internal
> + * Defines types of operations which can be executed by the application.
> + */
> +enum rte_eth_dev_operation {
> +	RTE_ETH_START,
> +};
> +
> +/**@{@name Restore flags
> + * Flags returned by get_restore_flags() callback.
> + * They indicate to ethdev layer which configuration is required to be restored.
> + */
> +/** If set, ethdev layer will forcefully restore default and any other added MAC addresses. */
> +#define RTE_ETH_RESTORE_MAC_ADDR RTE_BIT32(0)
> +/** If set, ethdev layer will forcefully restore current promiscuous mode setting. */
> +#define RTE_ETH_RESTORE_PROMISC  RTE_BIT32(1)
> +/** If set, ethdev layer will forcefully restore current all multicast mode setting. */
> +#define RTE_ETH_RESTORE_ALLMULTI RTE_BIT32(2)
> +/**@}*/
> +
> +/** All configuration which can be restored by ethdev layer. */
> +#define RTE_ETH_RESTORE_ALL (RTE_ETH_RESTORE_MAC_ADDR | \
> +			     RTE_ETH_RESTORE_PROMISC | \
> +			     RTE_ETH_RESTORE_ALLMULTI)
> +
> +/**
> + * @internal
> + * Fetch from the driver what kind of configuration must be restored by ethdev layer,
> + * after certain operations are performed by the application (such as rte_eth_dev_start()).
> + *
> + * @param dev
> + *   Port (ethdev) handle.
> + * @param op
> + *   Type of operation executed by the application.
> + * @param flags
> + *   Flags indicating what configuration must be restored by ethdev layer.
> + */
> +typedef void (*eth_get_restore_flags_t)(struct rte_eth_dev *dev,
> +					enum rte_eth_dev_operation op,
> +					uint32_t *flags);
> +
>  /**
>   * @internal A structure containing the functions exported by an Ethernet driver.
>   */
> @@ -1474,6 +1515,9 @@ struct eth_dev_ops {
>  	eth_count_aggr_ports_t count_aggr_ports;
>  	/** Map a Tx queue with an aggregated port of the DPDK port */
>  	eth_map_aggr_tx_affinity_t map_aggr_tx_affinity;
> +
> +	/** Get configuration which ethdev should restore */
> +	eth_get_restore_flags_t get_restore_flags;
>  };
> 
>  /**
> @@ -2131,6 +2175,26 @@ struct rte_eth_fdir_conf {
>  	struct rte_eth_fdir_flex_conf flex_conf;
>  };
> 
> +/**
> + * @internal
> + * Fetch from the driver what kind of configuration must be restored by ethdev layer,
> + * using get_restore_flags() callback.
> + *
> + * If callback is not defined, it is assumed that all supported configuration must be restored.
> + *
> + * @param dev
> + *   Port (ethdev) handle.
> + * @param op
> + *   Type of operation executed by the application.
> + * @param flags
> + *   Flags indicating what configuration must be restored by ethdev layer.
> + */
> +__rte_internal

For new function we probably need 'rte_experimental'...
But from other side - it is internal.
Not sure what are rules here for such case.

> +void
> +rte_eth_get_restore_flags(struct rte_eth_dev *dev,
> +			  enum rte_eth_dev_operation op,
> +			  uint32_t *flags);
> +
>  #ifdef __cplusplus
>  }
>  #endif
> diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
> index 1669055ca5..fa0469e602 100644
> --- a/lib/ethdev/version.map
> +++ b/lib/ethdev/version.map
> @@ -358,4 +358,5 @@ INTERNAL {
>  	rte_eth_switch_domain_alloc;
>  	rte_eth_switch_domain_free;
>  	rte_flow_fp_default_ops;
> +	rte_eth_get_restore_flags;
>  };
> --
> 2.39.5

RE: [PATCH v2 2/4] ethdev: add get restore flags driver callback

[Dariusz Sosnowski]

> -----Original Message-----
> From: Konstantin Ananyev <konstantin.ananyev@huawei.com>
> Sent: Friday, October 11, 2024 14:54
> To: Dariusz Sosnowski <dsosnowski@nvidia.com>; NBU-Contact-Thomas
> Monjalon (EXTERNAL) <thomas@monjalon.net>; Ferruh Yigit
> <ferruh.yigit@amd.com>; Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH v2 2/4] ethdev: add get restore flags driver callback
> 
> External email: Use caution opening links or attachments
> 
> 
> > Before this patch, ethdev layer assumed that all drivers require that
> > it has to forcefully restore:
> >
> > - MAC addresses
> > - promiscuous mode setting
> > - all multicast mode setting
> >
> > upon rte_eth_dev_start().
> >
> > This patch introduces a new callback to eth_dev_ops -
> > get_restore_flags().
> > Drivers implementing this callback can explicitly enable/disable
> > certain parts of config restore procedure.
> >
> > In order to minimize the changes to all the drivers and preserve the
> > current behavior, it is assumed that if this callback is not defined,
> > all configuration should be restored.
> >
> > Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
> 
> LGTM, just few nits/suggestions, pls see below.
> Series-Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
> 
> > ---
> >  lib/ethdev/ethdev_driver.c | 11 +++++++  lib/ethdev/ethdev_driver.h |
> > 64 ++++++++++++++++++++++++++++++++++++++
> >  lib/ethdev/version.map     |  1 +
> >  3 files changed, 76 insertions(+)
> >
> > diff --git a/lib/ethdev/ethdev_driver.c b/lib/ethdev/ethdev_driver.c
> > index c335a25a82..f78f9fb5c1 100644
> > --- a/lib/ethdev/ethdev_driver.c
> > +++ b/lib/ethdev/ethdev_driver.c
> > @@ -958,3 +958,14 @@ rte_eth_switch_domain_free(uint16_t domain_id)
> >
> >       return 0;
> >  }
> > +
> > +void
> > +rte_eth_get_restore_flags(struct rte_eth_dev *dev,
> > +                       enum rte_eth_dev_operation op,
> > +                       uint32_t *flags)
> 
> I would go with uint64_t for flags (to avoid limitations in future).

+1

> Also, If it is void anyway, might be just return flags?
> i.e:
> uint64_t  +rte_eth_get_restore_flags(struct rte_eth_dev *dev, enum
> rte_eth_dev_operation op);

+1

> 
> Another thing - do we need to do update docs?
> PMD or PG sections, release notes?

IIRC new additions and internal changes in ethdev are not advertised in release notes.
@Ferruh Yigit: Should these changes be documented/announced somewhere?

> 
> > +{
> > +     if (dev->dev_ops->get_restore_flags != NULL)
> > +             dev->dev_ops->get_restore_flags(dev, op, flags);
> > +     else
> > +             *flags = RTE_ETH_RESTORE_ALL; }
> > diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
> > index ae00ead865..8ac5328521 100644
> > --- a/lib/ethdev/ethdev_driver.h
> > +++ b/lib/ethdev/ethdev_driver.h
> > @@ -1235,6 +1235,47 @@ typedef int (*eth_count_aggr_ports_t)(struct
> > rte_eth_dev *dev);  typedef int (*eth_map_aggr_tx_affinity_t)(struct
> rte_eth_dev *dev, uint16_t tx_queue_id,
> >                                         uint8_t affinity);
> >
> > +/**
> > + * @internal
> > + * Defines types of operations which can be executed by the application.
> > + */
> > +enum rte_eth_dev_operation {
> > +     RTE_ETH_START,
> > +};
> > +
> > +/**@{@name Restore flags
> > + * Flags returned by get_restore_flags() callback.
> > + * They indicate to ethdev layer which configuration is required to be restored.
> > + */
> > +/** If set, ethdev layer will forcefully restore default and any
> > +other added MAC addresses. */ #define RTE_ETH_RESTORE_MAC_ADDR
> > +RTE_BIT32(0)
> > +/** If set, ethdev layer will forcefully restore current promiscuous
> > +mode setting. */ #define RTE_ETH_RESTORE_PROMISC  RTE_BIT32(1)
> > +/** If set, ethdev layer will forcefully restore current all
> > +multicast mode setting. */ #define RTE_ETH_RESTORE_ALLMULTI
> > +RTE_BIT32(2) /**@}*/
> > +
> > +/** All configuration which can be restored by ethdev layer. */
> > +#define RTE_ETH_RESTORE_ALL (RTE_ETH_RESTORE_MAC_ADDR | \
> > +                          RTE_ETH_RESTORE_PROMISC | \
> > +                          RTE_ETH_RESTORE_ALLMULTI)
> > +
> > +/**
> > + * @internal
> > + * Fetch from the driver what kind of configuration must be restored
> > +by ethdev layer,
> > + * after certain operations are performed by the application (such as
> rte_eth_dev_start()).
> > + *
> > + * @param dev
> > + *   Port (ethdev) handle.
> > + * @param op
> > + *   Type of operation executed by the application.
> > + * @param flags
> > + *   Flags indicating what configuration must be restored by ethdev layer.
> > + */
> > +typedef void (*eth_get_restore_flags_t)(struct rte_eth_dev *dev,
> > +                                     enum rte_eth_dev_operation op,
> > +                                     uint32_t *flags);
> > +
> >  /**
> >   * @internal A structure containing the functions exported by an Ethernet
> driver.
> >   */
> > @@ -1474,6 +1515,9 @@ struct eth_dev_ops {
> >       eth_count_aggr_ports_t count_aggr_ports;
> >       /** Map a Tx queue with an aggregated port of the DPDK port */
> >       eth_map_aggr_tx_affinity_t map_aggr_tx_affinity;
> > +
> > +     /** Get configuration which ethdev should restore */
> > +     eth_get_restore_flags_t get_restore_flags;
> >  };
> >
> >  /**
> > @@ -2131,6 +2175,26 @@ struct rte_eth_fdir_conf {
> >       struct rte_eth_fdir_flex_conf flex_conf;  };
> >
> > +/**
> > + * @internal
> > + * Fetch from the driver what kind of configuration must be restored
> > +by ethdev layer,
> > + * using get_restore_flags() callback.
> > + *
> > + * If callback is not defined, it is assumed that all supported configuration must
> be restored.
> > + *
> > + * @param dev
> > + *   Port (ethdev) handle.
> > + * @param op
> > + *   Type of operation executed by the application.
> > + * @param flags
> > + *   Flags indicating what configuration must be restored by ethdev layer.
> > + */
> > +__rte_internal
> 
> For new function we probably need 'rte_experimental'...
> But from other side - it is internal.
> Not sure what are rules here for such case.
> 
> > +void
> > +rte_eth_get_restore_flags(struct rte_eth_dev *dev,
> > +                       enum rte_eth_dev_operation op,
> > +                       uint32_t *flags);
> > +
> >  #ifdef __cplusplus
> >  }
> >  #endif
> > diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map index
> > 1669055ca5..fa0469e602 100644
> > --- a/lib/ethdev/version.map
> > +++ b/lib/ethdev/version.map
> > @@ -358,4 +358,5 @@ INTERNAL {
> >       rte_eth_switch_domain_alloc;
> >       rte_eth_switch_domain_free;
> >       rte_flow_fp_default_ops;
> > +     rte_eth_get_restore_flags;
> >  };
> > --
> > 2.39.5

[PATCH v2 3/4] ethdev: restore config only when requested

[Dariusz Sosnowski]

Use get_restore_flags() internal API introduced in previous commits
in rte_eth_dev_start(), to restore only the configuration
requested by the driver.

Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 lib/ethdev/rte_ethdev.c | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 362a1883f0..9e82556374 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -1719,20 +1719,27 @@ eth_dev_allmulticast_restore(struct rte_eth_dev *dev, uint16_t port_id)
 
 static int
 eth_dev_config_restore(struct rte_eth_dev *dev,
-		struct rte_eth_dev_info *dev_info, uint16_t port_id)
+		struct rte_eth_dev_info *dev_info,
+		uint32_t restore_flags,
+		uint16_t port_id)
 {
 	int ret;
 
-	if (!(*dev_info->dev_flags & RTE_ETH_DEV_NOLIVE_MAC_ADDR))
+	if (!(*dev_info->dev_flags & RTE_ETH_DEV_NOLIVE_MAC_ADDR) &&
+	    (restore_flags & RTE_ETH_RESTORE_MAC_ADDR))
 		eth_dev_mac_restore(dev, dev_info);
 
-	ret = eth_dev_promiscuous_restore(dev, port_id);
-	if (ret != 0)
-		return ret;
+	if (restore_flags & RTE_ETH_RESTORE_PROMISC) {
+		ret = eth_dev_promiscuous_restore(dev, port_id);
+		if (ret != 0)
+			return ret;
+	}
 
-	ret = eth_dev_allmulticast_restore(dev, port_id);
-	if (ret != 0)
-		return ret;
+	if (restore_flags & RTE_ETH_RESTORE_ALLMULTI) {
+		ret = eth_dev_allmulticast_restore(dev, port_id);
+		if (ret != 0)
+			return ret;
+	}
 
 	return 0;
 }
@@ -1742,6 +1749,7 @@ rte_eth_dev_start(uint16_t port_id)
 {
 	struct rte_eth_dev *dev;
 	struct rte_eth_dev_info dev_info;
+	uint32_t restore_flags;
 	int diag;
 	int ret, ret_stop;
 
@@ -1769,8 +1777,11 @@ rte_eth_dev_start(uint16_t port_id)
 	if (ret != 0)
 		return ret;
 
+	rte_eth_get_restore_flags(dev, RTE_ETH_START, &restore_flags);
+
 	/* Lets restore MAC now if device does not support live change */
-	if (*dev_info.dev_flags & RTE_ETH_DEV_NOLIVE_MAC_ADDR)
+	if ((*dev_info.dev_flags & RTE_ETH_DEV_NOLIVE_MAC_ADDR) &&
+	    (restore_flags & RTE_ETH_RESTORE_MAC_ADDR))
 		eth_dev_mac_restore(dev, &dev_info);
 
 	diag = (*dev->dev_ops->dev_start)(dev);
@@ -1779,7 +1790,7 @@ rte_eth_dev_start(uint16_t port_id)
 	else
 		return eth_err(port_id, diag);
 
-	ret = eth_dev_config_restore(dev, &dev_info, port_id);
+	ret = eth_dev_config_restore(dev, &dev_info, restore_flags, port_id);
 	if (ret != 0) {
 		RTE_ETHDEV_LOG_LINE(ERR,
 			"Error during restoring configuration for device (port %u): %s",
-- 
2.39.5

[PATCH v2 4/4] net/mlx5: disable config restore

[Dariusz Sosnowski]

mlx5 PMD does not require configuration restore
on rte_eth_dev_start().
Add implementation of get_restore_flags() indicating that.

Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 drivers/net/mlx5/mlx5.c        |  2 ++
 drivers/net/mlx5/mlx5.h        |  3 +++
 drivers/net/mlx5/mlx5_ethdev.c | 19 +++++++++++++++++++
 3 files changed, 24 insertions(+)

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 8d266b0e64..9b6acaf7f1 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -2571,6 +2571,7 @@ const struct eth_dev_ops mlx5_dev_ops = {
 	.count_aggr_ports = mlx5_count_aggr_ports,
 	.map_aggr_tx_affinity = mlx5_map_aggr_tx_affinity,
 	.rx_metadata_negotiate = mlx5_flow_rx_metadata_negotiate,
+	.get_restore_flags = mlx5_get_restore_flags,
 };
 
 /* Available operations from secondary process. */
@@ -2663,6 +2664,7 @@ const struct eth_dev_ops mlx5_dev_ops_isolate = {
 	.get_monitor_addr = mlx5_get_monitor_addr,
 	.count_aggr_ports = mlx5_count_aggr_ports,
 	.map_aggr_tx_affinity = mlx5_map_aggr_tx_affinity,
+	.get_restore_flags = mlx5_get_restore_flags,
 };
 
 /**
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 869aac032b..a5829fb71a 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -2228,6 +2228,9 @@ eth_rx_burst_t mlx5_select_rx_function(struct rte_eth_dev *dev);
 struct mlx5_priv *mlx5_port_to_eswitch_info(uint16_t port, bool valid);
 struct mlx5_priv *mlx5_dev_to_eswitch_info(struct rte_eth_dev *dev);
 int mlx5_dev_configure_rss_reta(struct rte_eth_dev *dev);
+void mlx5_get_restore_flags(struct rte_eth_dev *dev,
+			    enum rte_eth_dev_operation op,
+			    uint32_t *flags);
 
 /* mlx5_ethdev_os.c */
 
diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index 6a678d6dcc..8b78efc3fd 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -796,3 +796,22 @@ mlx5_hairpin_cap_get(struct rte_eth_dev *dev, struct rte_eth_hairpin_cap *cap)
 	cap->tx_cap.rte_memory = hca_attr->hairpin_sq_wq_in_host_mem;
 	return 0;
 }
+
+/**
+ * Indicate to ethdev layer, what configuration must be restored.
+ *
+ * @param[in] dev
+ *   Pointer to Ethernet device structure.
+ * @param[in] op
+ *   Type of operation which might require.
+ * @param[out] flags
+ *   Restore flags will be stored here.
+ */
+void
+mlx5_get_restore_flags(__rte_unused struct rte_eth_dev *dev,
+		       __rte_unused enum rte_eth_dev_operation op,
+		       uint32_t *flags)
+{
+	/* mlx5 PMD does not require any configuration restore. */
+	*flags = 0;
+}
-- 
2.39.5

[PATCH v3 0/4] ethdev: rework config restore

[Dariusz Sosnowski]

This patch series reworks the config restore procedure,
so that drivers are able to enable/disable certain parts of it.
Drivers can provide get_restore_flags() callback,
which will indicate to ethdev library what configuration to restore.

If callback is not defined, then ethdev assumes that
all configuration must be restored, preserving the current behavior for all drivers.

This patch series also includes implementation of get_restore_flags()
for mlx5 PMD, which does not require config restore.

RFC: https://inbox.dpdk.org/dev/20240918092201.33772-1-dsosnowski@nvidia.com/

v3:
- Change type for restore flags to uint64_t.
- Return restore flags from rte_eth_get_restore_flags() directly,
  instead of returning through pointer.
v2:
- Fix typos in API docs.

Dariusz Sosnowski (4):
  ethdev: rework config restore
  ethdev: add get restore flags driver callback
  ethdev: restore config only when requested
  net/mlx5: disable config restore

 drivers/net/mlx5/mlx5.c        |  2 ++
 drivers/net/mlx5/mlx5.h        |  2 ++
 drivers/net/mlx5/mlx5_ethdev.c | 18 ++++++++++
 lib/ethdev/ethdev_driver.c     |  9 +++++
 lib/ethdev/ethdev_driver.h     | 66 ++++++++++++++++++++++++++++++++++
 lib/ethdev/rte_ethdev.c        | 49 +++++++++++++++++++++----
 lib/ethdev/version.map         |  1 +
 7 files changed, 140 insertions(+), 7 deletions(-)

--
2.39.5

[PATCH v3 1/4] ethdev: rework config restore

[Dariusz Sosnowski]

Extract promiscuous and all multicast configuration restore
to separate functions.
This change will allow easier integration of disabling
these procedures for supporting PMDs in follow up commits.

Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 lib/ethdev/rte_ethdev.c | 34 +++++++++++++++++++++++++++++-----
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index f1c658f49e..362a1883f0 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -1648,14 +1648,10 @@ eth_dev_mac_restore(struct rte_eth_dev *dev,
 }
 
 static int
-eth_dev_config_restore(struct rte_eth_dev *dev,
-		struct rte_eth_dev_info *dev_info, uint16_t port_id)
+eth_dev_promiscuous_restore(struct rte_eth_dev *dev, uint16_t port_id)
 {
 	int ret;
 
-	if (!(*dev_info->dev_flags & RTE_ETH_DEV_NOLIVE_MAC_ADDR))
-		eth_dev_mac_restore(dev, dev_info);
-
 	/* replay promiscuous configuration */
 	/*
 	 * use callbacks directly since we don't need port_id check and
@@ -1683,6 +1679,14 @@ eth_dev_config_restore(struct rte_eth_dev *dev,
 		}
 	}
 
+	return 0;
+}
+
+static int
+eth_dev_allmulticast_restore(struct rte_eth_dev *dev, uint16_t port_id)
+{
+	int ret;
+
 	/* replay all multicast configuration */
 	/*
 	 * use callbacks directly since we don't need port_id check and
@@ -1713,6 +1717,26 @@ eth_dev_config_restore(struct rte_eth_dev *dev,
 	return 0;
 }
 
+static int
+eth_dev_config_restore(struct rte_eth_dev *dev,
+		struct rte_eth_dev_info *dev_info, uint16_t port_id)
+{
+	int ret;
+
+	if (!(*dev_info->dev_flags & RTE_ETH_DEV_NOLIVE_MAC_ADDR))
+		eth_dev_mac_restore(dev, dev_info);
+
+	ret = eth_dev_promiscuous_restore(dev, port_id);
+	if (ret != 0)
+		return ret;
+
+	ret = eth_dev_allmulticast_restore(dev, port_id);
+	if (ret != 0)
+		return ret;
+
+	return 0;
+}
+
 int
 rte_eth_dev_start(uint16_t port_id)
 {
-- 
2.39.5

[PATCH v3 2/4] ethdev: add get restore flags driver callback

[Dariusz Sosnowski]

Before this patch, ethdev layer assumed that all drivers require that
it has to forcefully restore:

- MAC addresses
- promiscuous mode setting
- all multicast mode setting

upon rte_eth_dev_start().

This patch introduces a new callback to eth_dev_ops -
get_restore_flags().
Drivers implementing this callback can explicitly enable/disable
certain parts of config restore procedure.

In order to minimize the changes to all the drivers and
preserve the current behavior, it is assumed that
if this callback is not defined, all configuration should be restored.

Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 lib/ethdev/ethdev_driver.c |  9 ++++++
 lib/ethdev/ethdev_driver.h | 66 ++++++++++++++++++++++++++++++++++++++
 lib/ethdev/version.map     |  1 +
 3 files changed, 76 insertions(+)

diff --git a/lib/ethdev/ethdev_driver.c b/lib/ethdev/ethdev_driver.c
index c335a25a82..9afef06431 100644
--- a/lib/ethdev/ethdev_driver.c
+++ b/lib/ethdev/ethdev_driver.c
@@ -958,3 +958,12 @@ rte_eth_switch_domain_free(uint16_t domain_id)
 
 	return 0;
 }
+
+uint64_t
+rte_eth_get_restore_flags(struct rte_eth_dev *dev, enum rte_eth_dev_operation op)
+{
+	if (dev->dev_ops->get_restore_flags != NULL)
+		return dev->dev_ops->get_restore_flags(dev, op);
+	else
+		return RTE_ETH_RESTORE_ALL;
+}
diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
index ae00ead865..3974512a5c 100644
--- a/lib/ethdev/ethdev_driver.h
+++ b/lib/ethdev/ethdev_driver.h
@@ -1235,6 +1235,48 @@ typedef int (*eth_count_aggr_ports_t)(struct rte_eth_dev *dev);
 typedef int (*eth_map_aggr_tx_affinity_t)(struct rte_eth_dev *dev, uint16_t tx_queue_id,
 					  uint8_t affinity);
 
+/**
+ * @internal
+ * Defines types of operations which can be executed by the application.
+ */
+enum rte_eth_dev_operation {
+	RTE_ETH_START,
+};
+
+/**@{@name Restore flags
+ * Flags returned by get_restore_flags() callback.
+ * They indicate to ethdev layer which configuration is required to be restored.
+ */
+/** If set, ethdev layer will forcefully restore default and any other added MAC addresses. */
+#define RTE_ETH_RESTORE_MAC_ADDR RTE_BIT64(0)
+/** If set, ethdev layer will forcefully restore current promiscuous mode setting. */
+#define RTE_ETH_RESTORE_PROMISC  RTE_BIT64(1)
+/** If set, ethdev layer will forcefully restore current all multicast mode setting. */
+#define RTE_ETH_RESTORE_ALLMULTI RTE_BIT64(2)
+/**@}*/
+
+/** All configuration which can be restored by ethdev layer. */
+#define RTE_ETH_RESTORE_ALL (RTE_ETH_RESTORE_MAC_ADDR | \
+			     RTE_ETH_RESTORE_PROMISC | \
+			     RTE_ETH_RESTORE_ALLMULTI)
+
+/**
+ * @internal
+ * Fetch from the driver what kind of configuration must be restored by ethdev layer,
+ * after certain operations are performed by the application (such as rte_eth_dev_start()).
+ *
+ * @param dev
+ *   Port (ethdev) handle.
+ * @param op
+ *   Type of operation executed by the application.
+ *
+ * @return
+ *   ORed restore flags indicating which configuration should be restored by ethdev.
+ *   0 if no restore is required by the driver.
+ */
+typedef uint64_t (*eth_get_restore_flags_t)(struct rte_eth_dev *dev,
+					    enum rte_eth_dev_operation op);
+
 /**
  * @internal A structure containing the functions exported by an Ethernet driver.
  */
@@ -1474,6 +1516,9 @@ struct eth_dev_ops {
 	eth_count_aggr_ports_t count_aggr_ports;
 	/** Map a Tx queue with an aggregated port of the DPDK port */
 	eth_map_aggr_tx_affinity_t map_aggr_tx_affinity;
+
+	/** Get configuration which ethdev should restore */
+	eth_get_restore_flags_t get_restore_flags;
 };
 
 /**
@@ -2131,6 +2176,27 @@ struct rte_eth_fdir_conf {
 	struct rte_eth_fdir_flex_conf flex_conf;
 };
 
+/**
+ * @internal
+ * Fetch from the driver what kind of configuration must be restored by ethdev layer,
+ * using get_restore_flags() callback.
+ *
+ * If callback is not defined, it is assumed that all supported configuration must be restored.
+ *
+ * @param dev
+ *   Port (ethdev) handle.
+ * @param op
+ *   Type of operation executed by the application.
+ *
+ * @return
+ *   ORed restore flags indicating which configuration should be restored by ethdev.
+ *   0 if no restore is required by the driver.
+ */
+__rte_internal
+uint64_t
+rte_eth_get_restore_flags(struct rte_eth_dev *dev,
+			  enum rte_eth_dev_operation op);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index 1669055ca5..fa0469e602 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -358,4 +358,5 @@ INTERNAL {
 	rte_eth_switch_domain_alloc;
 	rte_eth_switch_domain_free;
 	rte_flow_fp_default_ops;
+	rte_eth_get_restore_flags;
 };
-- 
2.39.5

[PATCH v3 3/4] ethdev: restore config only when requested

[Dariusz Sosnowski]

Use get_restore_flags() internal API introduced in previous commits
in rte_eth_dev_start(), to restore only the configuration
requested by the driver.

Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 lib/ethdev/rte_ethdev.c | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 362a1883f0..c46107c4de 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -1719,20 +1719,27 @@ eth_dev_allmulticast_restore(struct rte_eth_dev *dev, uint16_t port_id)
 
 static int
 eth_dev_config_restore(struct rte_eth_dev *dev,
-		struct rte_eth_dev_info *dev_info, uint16_t port_id)
+		struct rte_eth_dev_info *dev_info,
+		uint64_t restore_flags,
+		uint16_t port_id)
 {
 	int ret;
 
-	if (!(*dev_info->dev_flags & RTE_ETH_DEV_NOLIVE_MAC_ADDR))
+	if (!(*dev_info->dev_flags & RTE_ETH_DEV_NOLIVE_MAC_ADDR) &&
+	    (restore_flags & RTE_ETH_RESTORE_MAC_ADDR))
 		eth_dev_mac_restore(dev, dev_info);
 
-	ret = eth_dev_promiscuous_restore(dev, port_id);
-	if (ret != 0)
-		return ret;
+	if (restore_flags & RTE_ETH_RESTORE_PROMISC) {
+		ret = eth_dev_promiscuous_restore(dev, port_id);
+		if (ret != 0)
+			return ret;
+	}
 
-	ret = eth_dev_allmulticast_restore(dev, port_id);
-	if (ret != 0)
-		return ret;
+	if (restore_flags & RTE_ETH_RESTORE_ALLMULTI) {
+		ret = eth_dev_allmulticast_restore(dev, port_id);
+		if (ret != 0)
+			return ret;
+	}
 
 	return 0;
 }
@@ -1742,6 +1749,7 @@ rte_eth_dev_start(uint16_t port_id)
 {
 	struct rte_eth_dev *dev;
 	struct rte_eth_dev_info dev_info;
+	uint64_t restore_flags;
 	int diag;
 	int ret, ret_stop;
 
@@ -1769,8 +1777,11 @@ rte_eth_dev_start(uint16_t port_id)
 	if (ret != 0)
 		return ret;
 
+	restore_flags = rte_eth_get_restore_flags(dev, RTE_ETH_START);
+
 	/* Lets restore MAC now if device does not support live change */
-	if (*dev_info.dev_flags & RTE_ETH_DEV_NOLIVE_MAC_ADDR)
+	if ((*dev_info.dev_flags & RTE_ETH_DEV_NOLIVE_MAC_ADDR) &&
+	    (restore_flags & RTE_ETH_RESTORE_MAC_ADDR))
 		eth_dev_mac_restore(dev, &dev_info);
 
 	diag = (*dev->dev_ops->dev_start)(dev);
@@ -1779,7 +1790,7 @@ rte_eth_dev_start(uint16_t port_id)
 	else
 		return eth_err(port_id, diag);
 
-	ret = eth_dev_config_restore(dev, &dev_info, port_id);
+	ret = eth_dev_config_restore(dev, &dev_info, restore_flags, port_id);
 	if (ret != 0) {
 		RTE_ETHDEV_LOG_LINE(ERR,
 			"Error during restoring configuration for device (port %u): %s",
-- 
2.39.5

[PATCH v3 4/4] net/mlx5: disable config restore

[Dariusz Sosnowski]

mlx5 PMD does not require configuration restore
on rte_eth_dev_start().
Add implementation of get_restore_flags() indicating that.

Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 drivers/net/mlx5/mlx5.c        |  2 ++
 drivers/net/mlx5/mlx5.h        |  2 ++
 drivers/net/mlx5/mlx5_ethdev.c | 18 ++++++++++++++++++
 3 files changed, 22 insertions(+)

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 8d266b0e64..9b6acaf7f1 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -2571,6 +2571,7 @@ const struct eth_dev_ops mlx5_dev_ops = {
 	.count_aggr_ports = mlx5_count_aggr_ports,
 	.map_aggr_tx_affinity = mlx5_map_aggr_tx_affinity,
 	.rx_metadata_negotiate = mlx5_flow_rx_metadata_negotiate,
+	.get_restore_flags = mlx5_get_restore_flags,
 };
 
 /* Available operations from secondary process. */
@@ -2663,6 +2664,7 @@ const struct eth_dev_ops mlx5_dev_ops_isolate = {
 	.get_monitor_addr = mlx5_get_monitor_addr,
 	.count_aggr_ports = mlx5_count_aggr_ports,
 	.map_aggr_tx_affinity = mlx5_map_aggr_tx_affinity,
+	.get_restore_flags = mlx5_get_restore_flags,
 };
 
 /**
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 869aac032b..33568bc3d3 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -2228,6 +2228,8 @@ eth_rx_burst_t mlx5_select_rx_function(struct rte_eth_dev *dev);
 struct mlx5_priv *mlx5_port_to_eswitch_info(uint16_t port, bool valid);
 struct mlx5_priv *mlx5_dev_to_eswitch_info(struct rte_eth_dev *dev);
 int mlx5_dev_configure_rss_reta(struct rte_eth_dev *dev);
+uint64_t mlx5_get_restore_flags(struct rte_eth_dev *dev,
+				enum rte_eth_dev_operation op);
 
 /* mlx5_ethdev_os.c */
 
diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index 6a678d6dcc..6f24d649e0 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -796,3 +796,21 @@ mlx5_hairpin_cap_get(struct rte_eth_dev *dev, struct rte_eth_hairpin_cap *cap)
 	cap->tx_cap.rte_memory = hca_attr->hairpin_sq_wq_in_host_mem;
 	return 0;
 }
+
+/**
+ * Indicate to ethdev layer, what configuration must be restored.
+ *
+ * @param[in] dev
+ *   Pointer to Ethernet device structure.
+ * @param[in] op
+ *   Type of operation which might require.
+ * @param[out] flags
+ *   Restore flags will be stored here.
+ */
+uint64_t
+mlx5_get_restore_flags(__rte_unused struct rte_eth_dev *dev,
+		       __rte_unused enum rte_eth_dev_operation op)
+{
+	/* mlx5 PMD does not require any configuration restore. */
+	return 0;
+}
-- 
2.39.5

Re: [PATCH v3 0/4] ethdev: rework config restore

[Ferruh Yigit]

On 10/11/2024 7:51 PM, Dariusz Sosnowski wrote:
> This patch series reworks the config restore procedure,
> so that drivers are able to enable/disable certain parts of it.
> Drivers can provide get_restore_flags() callback,
> which will indicate to ethdev library what configuration to restore.
> 
> If callback is not defined, then ethdev assumes that
> all configuration must be restored, preserving the current behavior for all drivers.
> 
> This patch series also includes implementation of get_restore_flags()
> for mlx5 PMD, which does not require config restore.
> 
> RFC: https://inbox.dpdk.org/dev/20240918092201.33772-1-dsosnowski@nvidia.com/
> 
> v3:
> - Change type for restore flags to uint64_t.
> - Return restore flags from rte_eth_get_restore_flags() directly,
>   instead of returning through pointer.
> v2:
> - Fix typos in API docs.
> 
> Dariusz Sosnowski (4):
>   ethdev: rework config restore
>   ethdev: add get restore flags driver callback
>   ethdev: restore config only when requested
>   net/mlx5: disable config restore
> 

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

Series applied to dpdk-next-net/main, thanks.