netdev - [RFC PATCH 1/2] net: phylink: Add support for link initialization w/ a "rolling start"

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: 
 <174493418638.1021855.3276376104762124654.stgit@ahduyck-xeon-server.home.arpa>
Date: Thu, 17 Apr 2025 16:56:26 -0700
From: Alexander Duyck <alexander.duyck@...il.com>
To: linux@...linux.org.uk
Cc: netdev@...r.kernel.org, andrew@...n.ch, kuba@...nel.org
Subject: [RFC PATCH 1/2] net: phylink: Add support for link initialization w/
 a "rolling start"

From: Alexander Duyck <alexanderduyck@...com>

This change adds support for what I am referring to as a rolling start to
phylink. The general idea is that when phylink is loading on a MAC/PCS/PHY,
and it already has the link up and is passing traffic for something such as
a BMC, we don't want to disrupt that. To guarantee that a few things are
expected of phylink and of the driver.

>From phylink we would expect:
1. phylink_resume must not call mac_link_down to rebalance the link.
2. The first call to phylink_resolve after calling phylink_start/resume
   must trigger a call to phylink_link_up/down to sync up either the
   netdev carrier state, or the MAC link state.

>From the driver we would expect:
1. mac_prepare must perform the actions of mac_link_down if configuration
   requires changes that will cause loss of link, even if only temporary.
2. Calls to pcs_link_up, mac_link_up and mac_link_down must be idempotent.

With these changes the current expectation is that without changing any
link settings the link should remain up without any issues. So going
through a driver load/unload or ip link up/down should have no effect on
the BMC link. If link level changes are made such as changing autoneg, flow
control, speed, FEC, or number of lanes then the link should at least
momemtarily drop, and this is to be expected.

Signed-off-by: Alexander Duyck <alexanderduyck@...com>
---
 drivers/net/ethernet/meta/fbnic/fbnic_phylink.c |    1 +
 drivers/net/phy/phylink.c                       |    8 ++++++--
 include/linux/phylink.h                         |    4 ++++
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_phylink.c b/drivers/net/ethernet/meta/fbnic/fbnic_phylink.c
index 860b02b22c15..bbd13bf08eff 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_phylink.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_phylink.c
@@ -142,6 +142,7 @@ int fbnic_phylink_init(struct net_device *netdev)
 					       MAC_40000FD | MAC_50000FD |
 					       MAC_100000FD;
 	fbn->phylink_config.default_an_inband = true;
+	fbn->phylink_config.rolling_start = true;
 
 	__set_bit(PHY_INTERFACE_MODE_XGMII,
 		  fbn->phylink_config.supported_interfaces);
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 1bdd5d8bb5b0..66cd866959ef 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -52,6 +52,7 @@ struct phylink {
 	struct phylink_pcs *pcs;
 	struct device *dev;
 	unsigned int old_link_state:1;
+	unsigned int rolling_start:1;
 
 	unsigned long phylink_disable_state; /* bitmask of disables */
 	struct phy_device *phydev;
@@ -1666,8 +1667,9 @@ static void phylink_resolve(struct work_struct *w)
 	if (pl->major_config_failed)
 		link_state.link = false;
 
-	if (link_state.link != cur_link_state) {
+	if (link_state.link != cur_link_state || pl->rolling_start) {
 		pl->old_link_state = link_state.link;
+		pl->rolling_start = false;
 		if (!link_state.link)
 			phylink_link_down(pl);
 		else
@@ -2603,10 +2605,12 @@ void phylink_resume(struct phylink *pl)
 {
 	ASSERT_RTNL();
 
+	pl->rolling_start = pl->config->rolling_start;
+
 	if (test_bit(PHYLINK_DISABLE_MAC_WOL, &pl->phylink_disable_state)) {
 		/* Wake-on-Lan enabled, MAC handling */
 
-		if (pl->suspend_link_up) {
+		if (pl->suspend_link_up && !pl->rolling_start) {
 			/* Call mac_link_down() so we keep the overall state
 			 * balanced. Do this under the state_mutex lock for
 			 * consistency. This will cause a "Link Down" message
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index 1f5773ab5660..2d044f1a141c 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -156,6 +156,9 @@ enum phylink_op_type {
  * @lpi_capabilities: MAC speeds which can support LPI signalling
  * @lpi_timer_default: Default EEE LPI timer setting.
  * @eee_enabled_default: If set, EEE will be enabled by phylink at creation time
+ * @rolling_start: If set MAC may start in the "up" state, and if possible we
+ * 		   should avoid bringing it down unecessarily during
+ * 		   phylink_start/resume.
  */
 struct phylink_config {
 	struct device *dev;
@@ -165,6 +168,7 @@ struct phylink_config {
 	bool mac_requires_rxc;
 	bool default_an_inband;
 	bool eee_rx_clk_stop_enable;
+	bool rolling_start;
 	void (*get_fixed_state)(struct phylink_config *config,
 				struct phylink_link_state *state);
 	DECLARE_PHY_INTERFACE_MASK(supported_interfaces);