From: Chenna Arnoori <[email protected]>
When OVS-DPDK reconciles port state, it calls flow_ctrl_get followed
by flow_ctrl_set if the returned configuration differs from its
database. The driver was reporting autoneg=1 whenever the firmware
had auto_pause set (including the AUTONEG_PAUSE bit 0x4), even
though no pause autoneg was actually requested. This mismatch
caused OVS to repeatedly call flow_ctrl_set, which triggered a
full link reconfig with PHY reset, flapping the link every time
any interface change occurred on the system.
Two problems were fixed. First, flow_ctrl_set was clearing all
autoneg bits instead of only the flow-control autoneg bit,
which also wiped the speed autoneg state. Second, the pause
set path was abandoning its own HWRM request and calling the
full link config function, which built a separate request
without the pause fields that set_pause_common would have
computed.
Port the kernel bnxt_en driver's approach: add a
set_link_common helper that merges link and speed fields into
an existing HWRM request, and rework set_pause to build a
single combined request with both pause and link fields when
a full reconfig is needed. When only pause changes without an
auto-to-force transition, a pause-only PHY config is sent
without a full link reprogram.
Fixes: 8aaf473bbed6 ("net/bnxt: add flow control operations")
Cc: [email protected]
Signed-off-by: Chenna Arnoori <[email protected]>
Signed-off-by: Mohammad Shuab Siddique <[email protected]>
---
doc/guides/rel_notes/release_26_07.rst | 25 ++++
drivers/net/bnxt/bnxt.h | 10 ++
drivers/net/bnxt/bnxt_ethdev.c | 12 +-
drivers/net/bnxt/bnxt_hwrm.c | 151 +++++++++++++++++++++++++
drivers/net/bnxt/bnxt_hwrm.h | 1 +
5 files changed, 197 insertions(+), 2 deletions(-)
diff --git a/doc/guides/rel_notes/release_26_07.rst
b/doc/guides/rel_notes/release_26_07.rst
index b5285af5fe..ee80a37055 100644
--- a/doc/guides/rel_notes/release_26_07.rst
+++ b/doc/guides/rel_notes/release_26_07.rst
@@ -209,6 +209,31 @@ ABI Changes
* No ABI change that would break compatibility with 25.11.
+Bug Fixes and Other Changes
+---------------------------
+
+.. This section should contain bug fixes added to the relevant
+ stable branch. Sample format:
+
+ * **code/area: Fixed issue in <component>.**
+
+ Fixed a specific issue with the following impact, caused by the following
+ action, and the resolution.
+
+ This section is a comment. Do not overwrite or remove it.
+ Also, make sure to start the actual text at the margin.
+ =======================================================
+
+* **net/bnxt: Fixed link flapping on flow control configuration.**
+
+ Fixed an issue where setting flow control parameters via
+ ``rte_eth_dev_flow_ctrl_set()`` triggered an unconditional PHY reset,
+ causing repeated link flaps in environments such as OVS-DPDK that
+ periodically reconcile port state. The fix ports the kernel bnxt_en
+ approach of building a combined pause+link HWRM request rather than
+ delegating to the full link reconfiguration path.
+
+
Known Issues
------------
diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
index 7515f0564f..0ea18fb134 100644
--- a/drivers/net/bnxt/bnxt.h
+++ b/drivers/net/bnxt/bnxt.h
@@ -88,6 +88,8 @@
#define HWRM_VERSION_1_9_1 0x10901
#define HWRM_VERSION_1_9_2 0x10903
#define HWRM_VERSION_1_10_2_13 0x10a020d
+/* Minimum spec version that supports AUTONEG_PAUSE bit in auto_pause field */
+#define HWRM_SPEC_CODE_AUTONEG_PAUSE 0x10201
#define BNXT_MAX_MTU 9574
#define BNXT_NUM_VLANS 2
@@ -333,6 +335,14 @@ struct bnxt_link_info {
uint8_t active_lanes;
uint8_t option_flags;
uint16_t pmd_speed_lanes;
+ /* Bitmask tracking which autoneg modes are active */
+ uint8_t autoneg;
+#define BNXT_AUTONEG_SPEED 1 /* speed autoneg enabled */
+#define BNXT_AUTONEG_FLOW_CTRL 2 /* pause/flow-ctrl autoneg enabled */
+ /* True after autoneg->forced FC transition; cleared once set_pause
+ * sends the combined pause+link HWRM request successfully.
+ */
+ bool link_reconfig_needed;
};
#define BNXT_COS_QUEUE_COUNT 8
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 1b8cf3a52a..467b551bd9 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -2565,7 +2565,7 @@ static int bnxt_flow_ctrl_get_op(struct rte_eth_dev *dev,
return rc;
memset(fc_conf, 0, sizeof(*fc_conf));
- if (bp->link_info->auto_pause)
+ if (bp->link_info->autoneg & BNXT_AUTONEG_FLOW_CTRL)
fc_conf->autoneg = 1;
switch (bp->link_info->pause) {
case 0:
@@ -2601,6 +2601,14 @@ static int bnxt_flow_ctrl_set_op(struct rte_eth_dev *dev,
return -ENOTSUP;
}
+ if (fc_conf->autoneg) {
+ bp->link_info->autoneg |= BNXT_AUTONEG_FLOW_CTRL;
+ } else {
+ if (bp->link_info->autoneg & BNXT_AUTONEG_FLOW_CTRL)
+ bp->link_info->link_reconfig_needed = true;
+ bp->link_info->autoneg &= ~BNXT_AUTONEG_FLOW_CTRL;
+ }
+
switch (fc_conf->mode) {
case RTE_ETH_FC_NONE:
bp->link_info->auto_pause = 0;
@@ -2642,7 +2650,7 @@ static int bnxt_flow_ctrl_set_op(struct rte_eth_dev *dev,
}
break;
}
- return bnxt_set_hwrm_link_config(bp, true);
+ return bnxt_hwrm_set_pause(bp);
}
/* Add UDP tunneling port */
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 0c82935de9..a0983183c0 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -1876,6 +1876,22 @@ static int bnxt_hwrm_port_phy_qcfg(struct bnxt *bp,
link_info->auto_pause = resp->auto_pause;
link_info->force_pause = resp->force_pause;
link_info->auto_mode = resp->auto_mode;
+ link_info->autoneg = 0;
+
+ if (link_info->auto_mode != HWRM_PORT_PHY_QCFG_OUTPUT_AUTO_MODE_NONE) {
+ link_info->autoneg = BNXT_AUTONEG_SPEED;
+ if (bp->hwrm_spec_code >= HWRM_SPEC_CODE_AUTONEG_PAUSE) {
+ if (link_info->auto_pause &
+ HWRM_PORT_PHY_CFG_INPUT_AUTO_PAUSE_AUTONEG_PAUSE)
+ link_info->autoneg |=
+ BNXT_AUTONEG_FLOW_CTRL;
+ } else {
+ link_info->autoneg |= BNXT_AUTONEG_FLOW_CTRL;
+ }
+ } else {
+ link_info->autoneg = 0;
+ }
+
link_info->phy_type = resp->phy_type;
link_info->media_type = resp->media_type;
@@ -4048,6 +4064,141 @@ static int bnxt_hwrm_port_phy_cfg_v2(struct bnxt *bp,
struct bnxt_link_info *con
return rc;
}
+static void
+bnxt_hwrm_set_pause_common(struct bnxt *bp,
+ struct hwrm_port_phy_cfg_input *req)
+{
+ struct bnxt_link_info *link_info = bp->link_info;
+
+ if (link_info->autoneg & BNXT_AUTONEG_FLOW_CTRL) {
+ if (bp->hwrm_spec_code >= HWRM_SPEC_CODE_AUTONEG_PAUSE)
+ req->auto_pause =
+ HWRM_PORT_PHY_CFG_INPUT_AUTO_PAUSE_AUTONEG_PAUSE;
+ if (link_info->auto_pause &
+ HWRM_PORT_PHY_CFG_INPUT_AUTO_PAUSE_RX)
+ req->auto_pause |=
+ HWRM_PORT_PHY_CFG_INPUT_AUTO_PAUSE_RX;
+ if (link_info->auto_pause &
+ HWRM_PORT_PHY_CFG_INPUT_AUTO_PAUSE_TX)
+ req->auto_pause |=
+ HWRM_PORT_PHY_CFG_INPUT_AUTO_PAUSE_TX;
+ req->enables |=
+
rte_cpu_to_le_32(HWRM_PORT_PHY_CFG_INPUT_ENABLES_AUTO_PAUSE);
+ } else {
+ if (link_info->force_pause &
+ HWRM_PORT_PHY_CFG_INPUT_FORCE_PAUSE_RX)
+ req->force_pause |=
+ HWRM_PORT_PHY_CFG_INPUT_FORCE_PAUSE_RX;
+ if (link_info->force_pause &
+ HWRM_PORT_PHY_CFG_INPUT_FORCE_PAUSE_TX)
+ req->force_pause |=
+ HWRM_PORT_PHY_CFG_INPUT_FORCE_PAUSE_TX;
+ req->enables |=
+
rte_cpu_to_le_32(HWRM_PORT_PHY_CFG_INPUT_ENABLES_FORCE_PAUSE);
+ if (bp->hwrm_spec_code >= HWRM_SPEC_CODE_AUTONEG_PAUSE) {
+ req->auto_pause = req->force_pause;
+ req->enables |=
+
rte_cpu_to_le_32(HWRM_PORT_PHY_CFG_INPUT_ENABLES_AUTO_PAUSE);
+ }
+ }
+}
+
+static void
+bnxt_hwrm_set_link_common(struct bnxt *bp, struct hwrm_port_phy_cfg_input *req)
+{
+ struct rte_eth_conf *dev_conf = &bp->eth_dev->data->dev_conf;
+ uint16_t autoneg, speed;
+
+ autoneg = bnxt_check_eth_link_autoneg(dev_conf->link_speeds);
+
+ if (BNXT_CHIP_P5(bp) &&
+ dev_conf->link_speeds & RTE_ETH_LINK_SPEED_40G)
+ autoneg = 0;
+
+ if (autoneg == 1 && BNXT_CHIP_P5(bp) &&
+ bp->link_info->auto_mode == 0 &&
+ bp->link_info->force_pam4_link_speed ==
+ HWRM_PORT_PHY_CFG_INPUT_FORCE_PAM4_LINK_SPEED_200GB)
+ autoneg = 0;
+
+ speed = bnxt_parse_eth_link_speed(bp, dev_conf->link_speeds,
+ bp->link_info);
+ req->flags |=
+ rte_cpu_to_le_32(HWRM_PORT_PHY_CFG_INPUT_FLAGS_RESET_PHY);
+
+ if (autoneg == 1 &&
+ (bp->link_info->support_auto_speeds ||
+ bp->link_info->support_pam4_auto_speeds)) {
+ uint16_t spd_mask;
+ uint32_t en;
+
+ req->flags |=
+
rte_cpu_to_le_32(HWRM_PORT_PHY_CFG_INPUT_FLAGS_RESTART_AUTONEG);
+ spd_mask = bnxt_parse_eth_link_speed_mask(bp,
+
dev_conf->link_speeds);
+ req->auto_link_speed_mask = rte_cpu_to_le_16(spd_mask);
+ req->auto_link_pam4_speed_mask =
+
rte_cpu_to_le_16(bp->link_info->auto_pam4_link_speed_mask);
+ en = HWRM_PORT_PHY_CFG_IN_EN_AUTO_LINK_SPEED_MASK |
+ HWRM_PORT_PHY_CFG_IN_EN_AUTO_PAM4_LINK_SPD_MASK |
+ HWRM_PORT_PHY_CFG_INPUT_ENABLES_AUTO_MODE;
+ req->enables |= rte_cpu_to_le_32(en);
+ req->auto_mode =
+ HWRM_PORT_PHY_CFG_INPUT_AUTO_MODE_SPEED_MASK;
+ } else {
+ uint32_t en;
+
+ req->flags |=
+ rte_cpu_to_le_32(HWRM_PORT_PHY_CFG_INPUT_FLAGS_FORCE);
+ if (speed) {
+ if (bp->link_info->link_signal_mode) {
+ req->force_pam4_link_speed =
+ rte_cpu_to_le_16(speed);
+ en =
HWRM_PORT_PHY_CFG_IN_EN_FORCE_PAM4_LINK_SPEED;
+ req->enables |= rte_cpu_to_le_32(en);
+ } else {
+ req->force_link_speed =
+ rte_cpu_to_le_16(speed);
+ }
+ }
+ }
+ req->auto_duplex =
+ bnxt_parse_eth_link_duplex(dev_conf->link_speeds);
+ req->enables |=
+ rte_cpu_to_le_32(HWRM_PORT_PHY_CFG_INPUT_ENABLES_AUTO_DUPLEX);
+}
+
+int bnxt_hwrm_set_pause(struct bnxt *bp)
+{
+ struct hwrm_port_phy_cfg_input req = {0};
+ struct hwrm_port_phy_cfg_output *resp = bp->hwrm_cmd_resp_addr;
+ struct bnxt_link_info *link_info = bp->link_info;
+ int rc;
+
+ HWRM_PREP(&req, HWRM_PORT_PHY_CFG, BNXT_USE_CHIMP_MB);
+
+ bnxt_hwrm_set_pause_common(bp, &req);
+
+ if ((link_info->autoneg & BNXT_AUTONEG_FLOW_CTRL) ||
+ link_info->link_reconfig_needed)
+ bnxt_hwrm_set_link_common(bp, &req);
+
+ rc = bnxt_hwrm_send_message(bp, &req, sizeof(req), BNXT_USE_CHIMP_MB);
+
+ HWRM_CHECK_RESULT();
+
+ if (!rc) {
+ if (!(link_info->autoneg & BNXT_AUTONEG_FLOW_CTRL)) {
+ link_info->pause = link_info->force_pause;
+ link_info->auto_pause = 0;
+ }
+ link_info->link_reconfig_needed = false;
+ }
+
+ HWRM_UNLOCK();
+ return rc;
+}
+
static int bnxt_set_hwrm_link_config_v2(struct bnxt *bp, bool link_up)
{
struct rte_eth_conf *dev_conf = &bp->eth_dev->data->dev_conf;
diff --git a/drivers/net/bnxt/bnxt_hwrm.h b/drivers/net/bnxt/bnxt_hwrm.h
index fc56223ab4..3034803023 100644
--- a/drivers/net/bnxt/bnxt_hwrm.h
+++ b/drivers/net/bnxt/bnxt_hwrm.h
@@ -261,6 +261,7 @@ void bnxt_free_hwrm_rx_ring(struct bnxt *bp, int
queue_index);
int bnxt_alloc_hwrm_resources(struct bnxt *bp);
int bnxt_get_hwrm_link_config(struct bnxt *bp, struct rte_eth_link *link);
int bnxt_set_hwrm_link_config(struct bnxt *bp, bool link_up);
+int bnxt_hwrm_set_pause(struct bnxt *bp);
int bnxt_hwrm_func_qcfg(struct bnxt *bp, uint16_t *mtu);
int bnxt_hwrm_func_resc_qcaps(struct bnxt *bp);
int bnxt_hwrm_func_reserve_vf_resc(struct bnxt *bp, bool test);
--
2.47.3
