[PATCH 0/3] net/intel: make link state configurable on device start

[PATCH 0/3] net/intel: make link state configurable on device start

[Roman Khromenok]

Starting a port always brings its link up, so the link partner sees
the port as soon as the application starts it. An application that
enables ports only after its own configuration is applied, such as
a firewall or a switch, cannot keep a disabled port dark: it can only
bring the link down right after rte_eth_dev_start(), and the link
partner may still see the link flap.

The ice driver already has the link_state_on_close device argument to
choose the link state when the port is stopped. This series adds its
counterpart, link_state_on_start, to the ice, i40e and ixgbe drivers:

  -a <pci>,link_state_on_start=down

With "down", the port is started with the link down, and the
application brings it up later with rte_eth_dev_set_link_up().
The default "up" keeps the current behavior.

ixgbe has no firmware command to bring the link down, so "down" leaves
the port in the same state as rte_eth_dev_set_link_down(): the laser
is kept off on fiber links, the copper PHY is powered off right after
the link setup.

Tested with testpmd on E810-C (ice, firmware 4.60) connected to
a network packet broker: with link_state_on_start=down the link stays
down after the port start and the link partner does not see it until
"set link-up"; without the device argument the link comes up on start
as before. i40e and ixgbe are build tested only.

Roman Khromenok (3):
  net/ice: make link state configurable on device start
  net/i40e: make link state configurable on device start
  net/ixgbe: make link state configurable on device start

 doc/guides/nics/i40e.rst               | 13 ++++++
 doc/guides/nics/ice.rst                | 13 ++++++
 doc/guides/nics/ixgbe.rst              | 13 ++++++
 doc/guides/rel_notes/release_26_11.rst | 13 +++++-
 drivers/net/intel/i40e/i40e_ethdev.c   | 58 +++++++++++++++++++++++++-
 drivers/net/intel/i40e/i40e_ethdev.h   |  2 +
 drivers/net/intel/ice/ice_ethdev.c     | 36 +++++++++++++++-
 drivers/net/intel/ice/ice_ethdev.h     |  1 +
 drivers/net/intel/ixgbe/ixgbe_ethdev.c | 44 ++++++++++++++++++-
 drivers/net/intel/ixgbe/ixgbe_ethdev.h |  2 +
 10 files changed, 187 insertions(+), 8 deletions(-)

-- 
2.47.3

[PATCH 1/3] net/ice: make link state configurable on device start

[Roman Khromenok]

The device start always brings the link up, so the port shows up
on the link partner as soon as the application starts it, even if
the application wants to enable it later, for example only after its
configuration is applied.

Introduce the device argument link_state_on_start, the counterpart
of link_state_on_close:
1. up: bring the link up (default, unchanged behavior)
2. down: bring (or keep) the link down; the application brings it up
   later with rte_eth_dev_set_link_up().

Signed-off-by: Roman Khromenok <roma55592@yandex.ru>
---
 doc/guides/nics/ice.rst                | 13 ++++++++++
 doc/guides/rel_notes/release_26_11.rst |  2 ++
 drivers/net/intel/ice/ice_ethdev.c     | 36 ++++++++++++++++++++++++--
 drivers/net/intel/ice/ice_ethdev.h     |  1 +
 4 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/doc/guides/nics/ice.rst b/doc/guides/nics/ice.rst
index ba598742c5..1a78760ff1 100644
--- a/doc/guides/nics/ice.rst
+++ b/doc/guides/nics/ice.rst
@@ -324,6 +324,19 @@ Runtime Configuration
   * ``up``: Leave the link in the up state.
   * ``initial``: Restore the link to the state it was in when the device started.
 
+- ``Link state on start`` (default ``up``)
+
+  The user can request that the link be kept down when the device is started,
+  so that the port does not show up on the link partner until the application
+  calls ``rte_eth_dev_set_link_up()``::
+
+    -a af:00.0,link_state_on_start=<state>
+
+  Supported values for the ``<state>`` parameter:
+
+  * ``up``: Bring the link up.
+  * ``down``: Bring (or keep) the link down.
+
 - ``Low Rx latency`` (default ``0``)
 
   vRAN workloads require low latency DPDK interface for the front haul
diff --git a/doc/guides/rel_notes/release_26_11.rst b/doc/guides/rel_notes/release_26_11.rst
index e8905f5533..a7fda78c5b 100644
--- a/doc/guides/rel_notes/release_26_11.rst
+++ b/doc/guides/rel_notes/release_26_11.rst
@@ -90,6 +90,8 @@ New Features
     enabling QinQ tag insertion and outer IPv4/UDP checksum offloads on those paths.
   * Added support for getting and setting link (802.3x) flow control.
   * Added support for Tx rate limiting per queue.
+  * Added ``link_state_on_start`` devarg to keep the link down
+    when the device is started.
 
 * **Updated Intel ixgbe driver.**
 
diff --git a/drivers/net/intel/ice/ice_ethdev.c b/drivers/net/intel/ice/ice_ethdev.c
index 163304ba1e..51a82830af 100644
--- a/drivers/net/intel/ice/ice_ethdev.c
+++ b/drivers/net/intel/ice/ice_ethdev.c
@@ -45,6 +45,7 @@
 #define ICE_RL_BURST_SIZE_ARG     "rl_burst_size"
 #define ICE_SOURCE_PRUNE_ARG      "source-prune"
 #define ICE_LINK_STATE_ON_CLOSE   "link_state_on_close"
+#define ICE_LINK_STATE_ON_START   "link_state_on_start"
 
 #define ICE_CYCLECOUNTER_MASK  0xffffffffffffffffULL
 
@@ -64,6 +65,7 @@ static const char * const ice_valid_args[] = {
 	ICE_RL_BURST_SIZE_ARG,
 	ICE_SOURCE_PRUNE_ARG,
 	ICE_LINK_STATE_ON_CLOSE,
+	ICE_LINK_STATE_ON_START,
 	NULL
 };
 
@@ -2265,6 +2267,27 @@ parse_link_state_on_close(const char *key, const char *value, void *args)
 	return ret;
 }
 
+static int
+parse_link_state_on_start(const char *key, const char *value, void *args)
+{
+	bool *link_down = args;
+
+	if (value == NULL || link_down == NULL)
+		return -EINVAL;
+
+	if (strcmp(value, "down") == 0) {
+		*link_down = true;
+	} else if (strcmp(value, "up") == 0) {
+		*link_down = false;
+	} else {
+		PMD_DRV_LOG(WARNING, "%s: Invalid value \"%s\", "
+				"should be \"down\" or \"up\"", key, value);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int
 lookup_pps_type(const char *pps_name)
 {
@@ -2529,6 +2552,11 @@ static int ice_parse_devargs(struct rte_eth_dev *dev)
 
 	ret = rte_kvargs_process(kvlist, ICE_LINK_STATE_ON_CLOSE,
 				 &parse_link_state_on_close, &ad->devargs.link_state_on_close);
+	if (ret)
+		goto bail;
+
+	ret = rte_kvargs_process(kvlist, ICE_LINK_STATE_ON_START,
+				 &parse_link_state_on_start, &ad->devargs.link_down_on_start);
 
 bail:
 	rte_kvargs_free(kvlist);
@@ -4856,7 +4884,10 @@ ice_dev_start(struct rte_eth_dev *dev)
 
 	ice_get_init_link_status(dev);
 
-	ice_dev_set_link_up(dev);
+	if (pf->adapter->devargs.link_down_on_start)
+		ice_dev_set_link_down(dev);
+	else
+		ice_dev_set_link_up(dev);
 
 	/* Call get_link_info aq command to enable/disable LSE */
 	ice_link_update(dev, 0);
@@ -8118,7 +8149,8 @@ RTE_PMD_REGISTER_PARAM_STRING(net_ice,
 			      ICE_RL_BURST_SIZE_ARG "=<N>"
 			      ICE_SOURCE_PRUNE_ARG "=<0|1>"
 			      ICE_RX_LOW_LATENCY_ARG "=<0|1>"
-			      ICE_LINK_STATE_ON_CLOSE "=<down|up|initial>");
+			      ICE_LINK_STATE_ON_CLOSE "=<down|up|initial>"
+			      ICE_LINK_STATE_ON_START "=<down|up>");
 
 RTE_LOG_REGISTER_SUFFIX(ice_logtype_init, init, NOTICE);
 RTE_LOG_REGISTER_SUFFIX(ice_logtype_driver, driver, NOTICE);
diff --git a/drivers/net/intel/ice/ice_ethdev.h b/drivers/net/intel/ice/ice_ethdev.h
index 8d5e17e2df..8794870c34 100644
--- a/drivers/net/intel/ice/ice_ethdev.h
+++ b/drivers/net/intel/ice/ice_ethdev.h
@@ -640,6 +640,7 @@ struct ice_devargs {
 	bool source_prune;
 	uint32_t rl_burst_size;
 	int link_state_on_close;
+	bool link_down_on_start;
 	int xtr_field_offs;
 	uint8_t xtr_flag_offs[PROTO_XTR_MAX];
 	/* Name of the field. */
-- 
2.47.3

[PATCH 2/3] net/i40e: make link state configurable on device start

[Roman Khromenok]

The device start always applies the link configuration, which brings
the link up, so the port shows up on the link partner as soon as
the application starts it, even if the application wants to enable
it later, for example only after its configuration is applied.

Introduce the device argument link_state_on_start, as for the ice
driver:
1. up: bring the link up (default, unchanged behavior)
2. down: bring (or keep) the link down; the application brings it up
   later with rte_eth_dev_set_link_up(), which applies the link
   configuration.

Signed-off-by: Roman Khromenok <roma55592@yandex.ru>
---
 doc/guides/nics/i40e.rst               | 13 ++++++
 doc/guides/rel_notes/release_26_11.rst |  5 +++
 drivers/net/intel/i40e/i40e_ethdev.c   | 58 +++++++++++++++++++++++++-
 drivers/net/intel/i40e/i40e_ethdev.h   |  2 +
 4 files changed, 76 insertions(+), 2 deletions(-)

diff --git a/doc/guides/nics/i40e.rst b/doc/guides/nics/i40e.rst
index 29234dbf0f..1242737611 100644
--- a/doc/guides/nics/i40e.rst
+++ b/doc/guides/nics/i40e.rst
@@ -193,6 +193,19 @@ Runtime Configuration
   * ``segment``: Check number of mbuf segments not exceed hw limitation.
   * ``offload``: Check any unsupported offload flag.
 
+- ``Link state on start`` (default ``up``)
+
+  The user can request that the link be kept down when the device is started,
+  so that the port does not show up on the link partner until the application
+  calls ``rte_eth_dev_set_link_up()``::
+
+    -a 84:00.0,link_state_on_start=<state>
+
+  Supported values for the ``<state>`` parameter:
+
+  * ``up``: Bring the link up.
+  * ``down``: Bring (or keep) the link down.
+
 QinQ Configuration
 ~~~~~~~~~~~~~~~~~~
 
diff --git a/doc/guides/rel_notes/release_26_11.rst b/doc/guides/rel_notes/release_26_11.rst
index a7fda78c5b..f5831a7e4e 100644
--- a/doc/guides/rel_notes/release_26_11.rst
+++ b/doc/guides/rel_notes/release_26_11.rst
@@ -74,6 +74,11 @@ New Features
     ``xdp_meta_rx_ts_valid_mask``.
   * Added ``read_clock`` operation to query the PTP hardware clock.
 
+* **Updated Intel i40e driver.**
+
+  * Added ``link_state_on_start`` devarg to keep the link down
+    when the device is started.
+
 * **Updated Intel iavf driver.**
 
   * Runtime Rx/Tx queue setup is now automatically disabled
diff --git a/drivers/net/intel/i40e/i40e_ethdev.c b/drivers/net/intel/i40e/i40e_ethdev.c
index 5d6102cae7..bc5adffbbc 100644
--- a/drivers/net/intel/i40e/i40e_ethdev.c
+++ b/drivers/net/intel/i40e/i40e_ethdev.c
@@ -49,6 +49,7 @@
 #define ETH_I40E_QUEUE_NUM_PER_VF_ARG	"queue-num-per-vf"
 #define ETH_I40E_VF_MSG_CFG		"vf_msg_cfg"
 #define ETH_I40E_MBUF_CHECK_ARG       "mbuf_check"
+#define ETH_I40E_LINK_STATE_ON_START  "link_state_on_start"
 
 #define I40E_CLEAR_PXE_WAIT_MS     200
 #define I40E_VSI_TSR_QINQ_STRIP		0x4010
@@ -418,6 +419,7 @@ static const char *const valid_keys[] = {
 	ETH_I40E_QUEUE_NUM_PER_VF_ARG,
 	ETH_I40E_VF_MSG_CFG,
 	ETH_I40E_MBUF_CHECK_ARG,
+	ETH_I40E_LINK_STATE_ON_START,
 	NULL};
 
 static const struct rte_pci_id pci_id_i40e_map[] = {
@@ -1332,6 +1334,53 @@ i40e_support_multi_driver(struct rte_eth_dev *dev)
 	return 0;
 }
 
+static int
+i40e_parse_link_state_on_start_handler(const char *key, const char *value,
+				       void *opaque)
+{
+	bool *link_down = opaque;
+
+	if (value == NULL || link_down == NULL)
+		return -EINVAL;
+
+	if (strcmp(value, "down") == 0) {
+		*link_down = true;
+	} else if (strcmp(value, "up") == 0) {
+		*link_down = false;
+	} else {
+		PMD_DRV_LOG(WARNING, "%s: Invalid value \"%s\", "
+			    "should be \"down\" or \"up\"", key, value);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int
+i40e_parse_link_state_on_start(struct rte_eth_dev *dev)
+{
+	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
+	struct rte_kvargs *kvlist;
+	int ret;
+
+	/* Bring the link up on device start by default */
+	pf->link_down_on_start = false;
+
+	if (!dev->device->devargs)
+		return 0;
+
+	kvlist = rte_kvargs_parse(dev->device->devargs->args, valid_keys);
+	if (!kvlist)
+		return -EINVAL;
+
+	ret = rte_kvargs_process(kvlist, ETH_I40E_LINK_STATE_ON_START,
+				 i40e_parse_link_state_on_start_handler,
+				 &pf->link_down_on_start);
+
+	rte_kvargs_free(kvlist);
+	return ret;
+}
+
 static int
 i40e_aq_debug_write_global_register(struct i40e_hw *hw,
 				    uint32_t reg_addr, uint64_t reg_val,
@@ -1593,6 +1642,7 @@ eth_i40e_dev_init(struct rte_eth_dev *dev, void *init_params __rte_unused)
 	i40e_parse_mbuf_check(dev);
 	/* Check if need to support multi-driver */
 	i40e_support_multi_driver(dev);
+	i40e_parse_link_state_on_start(dev);
 
 	/* Make sure all is clean before doing PF reset */
 	i40e_clear_hw(hw);
@@ -2551,7 +2601,10 @@ i40e_dev_start(struct rte_eth_dev *dev)
 	}
 
 	/* Apply link configure */
-	ret = i40e_apply_link_speed(dev);
+	if (pf->link_down_on_start)
+		ret = i40e_dev_set_link_down(dev);
+	else
+		ret = i40e_apply_link_speed(dev);
 	if (I40E_SUCCESS != ret) {
 		PMD_DRV_LOG(ERR, "Fail to apply link setting");
 		goto tx_err;
@@ -12500,4 +12553,5 @@ RTE_PMD_REGISTER_PARAM_STRING(net_i40e,
 			      ETH_I40E_FLOATING_VEB_ARG "=1"
 			      ETH_I40E_FLOATING_VEB_LIST_ARG "=<string>"
 			      ETH_I40E_QUEUE_NUM_PER_VF_ARG "=1|2|4|8|16"
-			      ETH_I40E_SUPPORT_MULTI_DRIVER "=1");
+			      ETH_I40E_SUPPORT_MULTI_DRIVER "=1"
+			      ETH_I40E_LINK_STATE_ON_START "=<down|up>");
diff --git a/drivers/net/intel/i40e/i40e_ethdev.h b/drivers/net/intel/i40e/i40e_ethdev.h
index 1e64a2d280..c4ddfab0be 100644
--- a/drivers/net/intel/i40e/i40e_ethdev.h
+++ b/drivers/net/intel/i40e/i40e_ethdev.h
@@ -1203,6 +1203,8 @@ struct i40e_pf {
 	bool fw8_3gt;
 	/* MAC config needs re-applying when link first comes up */
 	bool mac_config_on_link_up;
+	/* Keep the link down on device start */
+	bool link_down_on_start;
 	/* true when interrupt path unavailable */
 	bool use_aq_polling;
 
-- 
2.47.3

[PATCH 3/3] net/ixgbe: make link state configurable on device start

[Roman Khromenok]

The device start always turns on the laser or the copper PHY and
sets up the link, so the port shows up on the link partner as soon
as the application starts it, even if the application wants to
enable it later, for example only after its configuration is applied.

Introduce the device argument link_state_on_start, as for the ice
and i40e drivers:
1. up: bring the link up (default, unchanged behavior)
2. down: keep the laser off on fiber links and leave the port in the
   same state as after rte_eth_dev_set_link_down(); the application
   brings it up later with rte_eth_dev_set_link_up().

On copper links the PHY is still powered on to set up the link
and powered off right after it.

Signed-off-by: Roman Khromenok <roma55592@yandex.ru>
---
 doc/guides/nics/ixgbe.rst              | 13 ++++++++
 doc/guides/rel_notes/release_26_11.rst |  6 ++--
 drivers/net/intel/ixgbe/ixgbe_ethdev.c | 44 ++++++++++++++++++++++++--
 drivers/net/intel/ixgbe/ixgbe_ethdev.h |  2 ++
 4 files changed, 61 insertions(+), 4 deletions(-)

diff --git a/doc/guides/nics/ixgbe.rst b/doc/guides/nics/ixgbe.rst
index 1d14e292e2..dbf0c2c5e4 100644
--- a/doc/guides/nics/ixgbe.rst
+++ b/doc/guides/nics/ixgbe.rst
@@ -187,6 +187,19 @@ It must be passed as part of EAL arguments. For example,
   This value is read only at probe time.
   Changing it requires restarting the process with a new EAL argument.
 
+- ``link_state_on_start`` (default **up**)
+
+  The user can request that the link be kept down when the device is started,
+  so that the port does not show up on the link partner until the application
+  calls ``rte_eth_dev_set_link_up()``::
+
+    -a 81:00.0,link_state_on_start=<state>
+
+  Supported values for the ``<state>`` parameter:
+
+  * ``up``: Bring the link up.
+  * ``down``: Turn off the laser on fiber links or the PHY on copper links.
+
 .. note::
 
    The driver rejects Flow Director when DCB is enabled.
diff --git a/doc/guides/rel_notes/release_26_11.rst b/doc/guides/rel_notes/release_26_11.rst
index f5831a7e4e..3aadcf94a1 100644
--- a/doc/guides/rel_notes/release_26_11.rst
+++ b/doc/guides/rel_notes/release_26_11.rst
@@ -100,8 +100,10 @@ New Features
 
 * **Updated Intel ixgbe driver.**
 
-  Added ``fdir_buffer_size`` devarg to select the Flow Director table size
-  (``64k``, ``128k`` or ``256k``) at probe time. The default remains ``64k``.
+  * Added ``fdir_buffer_size`` devarg to select the Flow Director table size
+    (``64k``, ``128k`` or ``256k``) at probe time. The default remains ``64k``.
+  * Added ``link_state_on_start`` devarg to keep the link down
+    when the device is started.
 
 * **Updated NXP DPAA2 ethernet driver.**
 
diff --git a/drivers/net/intel/ixgbe/ixgbe_ethdev.c b/drivers/net/intel/ixgbe/ixgbe_ethdev.c
index 1745070e59..2adedaff8e 100644
--- a/drivers/net/intel/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/intel/ixgbe/ixgbe_ethdev.c
@@ -130,10 +130,12 @@
 
 #define IXGBE_DEVARG_FIBER_SDP3_NOT_TX_DISABLE	"fiber_sdp3_no_tx_disable"
 #define IXGBE_DEVARG_FDIR_BUFFER_SIZE		"fdir_buffer_size"
+#define IXGBE_DEVARG_LINK_STATE_ON_START	"link_state_on_start"
 
 static const char * const ixgbe_valid_arguments[] = {
 	IXGBE_DEVARG_FIBER_SDP3_NOT_TX_DISABLE,
 	IXGBE_DEVARG_FDIR_BUFFER_SIZE,
+	IXGBE_DEVARG_LINK_STATE_ON_START,
 	NULL
 };
 
@@ -985,6 +987,27 @@ devarg_handle_fdir_buffer_size(const char *key, const char *value,
 	return 0;
 }
 
+static int
+devarg_handle_link_state_on_start(const char *key, const char *value,
+				  void *extra_args)
+{
+	bool *link_down = extra_args;
+
+	if (value == NULL || extra_args == NULL)
+		return -EINVAL;
+
+	if (strcmp(value, "down") == 0) {
+		*link_down = true;
+	} else if (strcmp(value, "up") == 0) {
+		*link_down = false;
+	} else {
+		PMD_INIT_LOG(ERR, "invalid %s='%s', use down or up", key, value);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int
 ixgbe_parse_devargs(struct ixgbe_adapter *adapter,
 		      struct rte_devargs *devargs)
@@ -1016,6 +1039,12 @@ ixgbe_parse_devargs(struct ixgbe_adapter *adapter,
 			adapter->fdir_conf.pballoc = pballoc;
 	}
 
+	if (rte_kvargs_count(kvlist, IXGBE_DEVARG_LINK_STATE_ON_START) != 0 &&
+	    rte_kvargs_process(kvlist, IXGBE_DEVARG_LINK_STATE_ON_START,
+			       devarg_handle_link_state_on_start,
+			       &adapter->link_down_on_start) != 0)
+		ret = -EINVAL;
+
 	rte_kvargs_free(kvlist);
 	return ret;
 }
@@ -2720,8 +2749,11 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
 	}
 
 	if (hw->mac.ops.get_media_type(hw) == ixgbe_media_type_copper) {
-		/* Turn on the copper */
+		/* Turn on the copper, the PHY is needed to set up the link */
 		ixgbe_set_phy_power(hw, true);
+	} else if (adapter->link_down_on_start) {
+		/* Keep the laser off */
+		ixgbe_disable_tx_laser(hw);
 	} else {
 		/* Turn on the laser */
 		ixgbe_enable_tx_laser(hw);
@@ -2806,6 +2838,13 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
 	if (err)
 		goto error;
 
+	if (adapter->link_down_on_start) {
+		/* The link is brought up later by ixgbe_dev_set_link_up() */
+		err = ixgbe_dev_set_link_down(dev);
+		if (err)
+			goto error;
+	}
+
 skip_link_setup:
 
 	if (rte_intr_allow_others(intr_handle)) {
@@ -8661,7 +8700,8 @@ RTE_PMD_REGISTER_PCI_TABLE(net_ixgbe, pci_id_ixgbe_map);
 RTE_PMD_REGISTER_KMOD_DEP(net_ixgbe, "* igb_uio | uio_pci_generic | vfio-pci");
 RTE_PMD_REGISTER_PARAM_STRING(net_ixgbe,
 			      IXGBE_DEVARG_FIBER_SDP3_NOT_TX_DISABLE "=<0|1>"
-			      IXGBE_DEVARG_FDIR_BUFFER_SIZE "=<64k|128k|256k>");
+			      IXGBE_DEVARG_FDIR_BUFFER_SIZE "=<64k|128k|256k>"
+			      IXGBE_DEVARG_LINK_STATE_ON_START "=<down|up>");
 RTE_PMD_REGISTER_PCI(net_ixgbe_vf, rte_ixgbevf_pmd);
 RTE_PMD_REGISTER_PCI_TABLE(net_ixgbe_vf, pci_id_ixgbevf_map);
 RTE_PMD_REGISTER_KMOD_DEP(net_ixgbe_vf, "* igb_uio | vfio-pci");
diff --git a/drivers/net/intel/ixgbe/ixgbe_ethdev.h b/drivers/net/intel/ixgbe/ixgbe_ethdev.h
index 2e9d166e29..60206d00d5 100644
--- a/drivers/net/intel/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/intel/ixgbe/ixgbe_ethdev.h
@@ -492,6 +492,8 @@ struct ixgbe_adapter {
 
 	/* Used for limiting SDP3 TX_DISABLE checks */
 	uint8_t sdp3_no_tx_disable;
+	/* Keep the link down on device start */
+	bool link_down_on_start;
 	uint16_t max_vfs;
 
 	/* Used for VF link sync with PF's physical and logical (by checking
-- 
2.47.3

Re: [PATCH 0/3] net/intel: make link state configurable on device start

[Stephen Hemminger]

On Fri,  2 Oct 2026 21:43:35 +0200
Roman Khromenok <roma55592@yandex.ru> wrote:

> Starting a port always brings its link up, so the link partner sees
> the port as soon as the application starts it. An application that
> enables ports only after its own configuration is applied, such as
> a firewall or a switch, cannot keep a disabled port dark: it can only
> bring the link down right after rte_eth_dev_start(), and the link
> partner may still see the link flap.
> 
> The ice driver already has the link_state_on_close device argument to
> choose the link state when the port is stopped. This series adds its
> counterpart, link_state_on_start, to the ice, i40e and ixgbe drivers:
> 
>   -a <pci>,link_state_on_start=down

NAK.

DPDK devices should not be like snowflakes, everyone unique.

When I see patches like this I think one or more of the following:
  - a user does not understand how DPDK devices work and does not
    want to change their application
  - developer refuses to say no to the user.
  - DPDK devices semantics are poorly documented.
  - DPDK devices do not implement the link states in RFC2863 but should.
    RFC 2863 separates ifAdminStatus from ifOperStatus; ethdev should as well.

If you want something done better it must done at ethdev layer
and you must coordinate across all existing devices.
No more specials.

Take user input as problem description, not as directive on how
to solve it.

The link_state_on_close argument in ice is not a precedent. It was
a workaround for hotplug problems and should not have been a devarg
either. Adding a matching on_start option to three drivers repeats
the mistake. I should have objected stronger then.

Re: [PATCH 0/3] net/intel: make link state configurable on device start

[Roman Khromenok]

On Sat, 3 Oct 2026, Stephen Hemminger wrote:
> If you want something done better it must done at ethdev layer
> and you must coordinate across all existing devices.

Thanks, understood. I withdraw this series.

The problem behind it: a firewall starts its ports at boot but must keep
the ports that are disabled in its configuration down, so that the link
partner never sees them. Today rte_eth_dev_start() brings the link up
on most drivers, and the application can only bring it down afterwards.

Would an RFC along these lines be acceptable?
- ethdev keeps the administrative link state (RFC 2863 ifAdminStatus)
  set by rte_eth_dev_set_link_down()/up(), including before the port
  is started;
- rte_eth_dev_start() honors it: drivers that support it keep the link
  down on start, and for the others ethdev calls dev_set_link_down()
  right after dev_start(), so the semantics are the same for all devices;
- a new feature in the features matrix to track driver support.

Or would you prefer a separate API for the administrative state instead
of changing the behavior of rte_eth_dev_set_link_down()?

Roman

Re: [PATCH 0/3] net/intel: make link state configurable on device start

[Stephen Hemminger]

On Fri,  2 Oct 2026 21:43:35 +0200
Roman Khromenok <roma55592@yandex.ru> wrote:

> Starting a port always brings its link up, so the link partner sees
> the port as soon as the application starts it. An application that
> enables ports only after its own configuration is applied, such as
> a firewall or a switch, cannot keep a disabled port dark: it can only
> bring the link down right after rte_eth_dev_start(), and the link
> partner may still see the link flap.

The point of start is to signal that the application is ready to
send and receive packets. I would expect the normal startup sequence is:

  1. EAL init
  2. Detect devices
  3. Configure internal state
  4. Configure devices.
  5. Start devices

A device should be dark from 2 to 5. No packets coming in or out
and link is down.

Re: [PATCH 0/3] net/intel: make link state configurable on device start

[Stephen Hemminger]

On Sat,  3 Oct 2026 23:53:45 +0200
Roman Khromenok <roma55592@yandex.ru> wrote:

> On Sat, 3 Oct 2026, Stephen Hemminger wrote:
> > If you want something done better it must done at ethdev layer
> > and you must coordinate across all existing devices.  
> 
> Thanks, understood. I withdraw this series.
> 
> The problem behind it: a firewall starts its ports at boot but must keep
> the ports that are disabled in its configuration down, so that the link
> partner never sees them. Today rte_eth_dev_start() brings the link up
> on most drivers, and the application can only bring it down afterwards.
> 
> Would an RFC along these lines be acceptable?
> - ethdev keeps the administrative link state (RFC 2863 ifAdminStatus)
>   set by rte_eth_dev_set_link_down()/up(), including before the port
>   is started;
> - rte_eth_dev_start() honors it: drivers that support it keep the link
>   down on start, and for the others ethdev calls dev_set_link_down()
>   right after dev_start(), so the semantics are the same for all devices;
> - a new feature in the features matrix to track driver support.
> 
> Or would you prefer a separate API for the administrative state instead
> of changing the behavior of rte_eth_dev_set_link_down()?
> 
> Roman

No. Application should not call device start until it expects
to bring link up.

The RFC 2863 stuff is for layered devices where lower layers are
separate from the upper ones (ie. tunnels, bonding, etc).