[PATCH 0/3] net/sfc: support FEC feature

[PATCH 0/3] net/sfc: support FEC feature

[Denis Pryazhennikov]

This patch series adds setting and querying of Forward error
correction (FEC). AUTO, BASER and RS modes are supported
by EF10/EF100 hardware. LLRS mode is not supported.

Denis Pryazhennikov (3):
  net/sfc: split link update function
  common/sfc_efx/base: add FEC related macros
  net/sfc: support FEC feature

 doc/guides/nics/features/sfc.ini  |   1 +
 drivers/common/sfc_efx/base/efx.h |   9 +
 drivers/net/sfc/sfc.h             |   2 +
 drivers/net/sfc/sfc_ethdev.c      | 365 +++++++++++++++++++++++++++++-
 drivers/net/sfc/sfc_port.c        |  24 +-
 5 files changed, 381 insertions(+), 20 deletions(-)

-- 
2.37.0 (Apple Git-136)

[PATCH 1/3] net/sfc: split link update function

[Denis Pryazhennikov]

Separate the original link update function into
two functions: state retrieval and update.
This improves code clarity and maintainability.

Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
Reviewed-by: Ivan Malov <ivan.malov@arknetworks.am>
Reviewed-by: Andy Moreton <amoreton@xilinx.com>
---
 drivers/net/sfc/sfc_ethdev.c | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index 320043145495..6d41eb000345 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -248,28 +248,38 @@ sfc_dev_start(struct rte_eth_dev *dev)
 	return -rc;
 }
 
-static int
-sfc_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete)
+static void
+sfc_dev_get_rte_link(struct rte_eth_dev *dev, int wait_to_complete,
+		     struct rte_eth_link *link)
 {
 	struct sfc_adapter *sa = sfc_adapter_by_eth_dev(dev);
-	struct rte_eth_link current_link;
-	int ret;
 
-	sfc_log_init(sa, "entry");
+	SFC_ASSERT(link != NULL);
 
 	if (sa->state != SFC_ETHDEV_STARTED) {
-		sfc_port_link_mode_to_info(EFX_LINK_UNKNOWN, &current_link);
+		sfc_port_link_mode_to_info(EFX_LINK_UNKNOWN, link);
 	} else if (wait_to_complete) {
 		efx_link_mode_t link_mode;
 
 		if (efx_port_poll(sa->nic, &link_mode) != 0)
 			link_mode = EFX_LINK_UNKNOWN;
-		sfc_port_link_mode_to_info(link_mode, &current_link);
-
+		sfc_port_link_mode_to_info(link_mode, link);
 	} else {
 		sfc_ev_mgmt_qpoll(sa);
-		rte_eth_linkstatus_get(dev, &current_link);
+		rte_eth_linkstatus_get(dev, link);
 	}
+}
+
+static int
+sfc_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete)
+{
+	struct sfc_adapter *sa = sfc_adapter_by_eth_dev(dev);
+	struct rte_eth_link current_link;
+	int ret;
+
+	sfc_log_init(sa, "entry");
+
+	sfc_dev_get_rte_link(dev, wait_to_complete, &current_link);
 
 	ret = rte_eth_linkstatus_set(dev, &current_link);
 	if (ret == 0)
-- 
2.37.0 (Apple Git-136)

[PATCH 2/3] common/sfc_efx/base: add FEC related macros

[Denis Pryazhennikov]

Added new macros to simplify working with FEC bits

Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
Reviewed-by: Andy Moreton <amoreton@xilinx.com>
---
 drivers/common/sfc_efx/base/efx.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/common/sfc_efx/base/efx.h b/drivers/common/sfc_efx/base/efx.h
index 49e29dcc1c69..10c412bcd7dc 100644
--- a/drivers/common/sfc_efx/base/efx.h
+++ b/drivers/common/sfc_efx/base/efx.h
@@ -3958,6 +3958,15 @@ typedef enum efx_phy_fec_type_e {
 	EFX_PHY_FEC_RS
 } efx_phy_fec_type_t;
 
+#define EFX_PHY_CAP_FEC_BIT(_fec_bit) (1U << EFX_PHY_CAP_##_fec_bit)
+#define EFX_PHY_CAP_FEC_MASK \
+	(EFX_PHY_CAP_FEC_BIT(BASER_FEC) |		\
+	 EFX_PHY_CAP_FEC_BIT(25G_BASER_FEC) |		\
+	 EFX_PHY_CAP_FEC_BIT(BASER_FEC_REQUESTED) |	\
+	 EFX_PHY_CAP_FEC_BIT(25G_BASER_FEC_REQUESTED) |	\
+	 EFX_PHY_CAP_FEC_BIT(RS_FEC) |			\
+	 EFX_PHY_CAP_FEC_BIT(RS_FEC_REQUESTED))
+
 LIBEFX_API
 extern	__checkReturn	efx_rc_t
 efx_phy_fec_type_get(
-- 
2.37.0 (Apple Git-136)

[PATCH 3/3] net/sfc: support FEC feature

[Denis Pryazhennikov]

Support ethdev methods to query and set FEC information.
Limitations: ignoring rte_eth_fec_get_capability() results
can lead to NOFEC if the device is not strated.

Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
Reviewed-by: Ivan Malov <ivan.malov@arknetworks.am>
Reviewed-by: Andy Moreton <amoreton@xilinx.com>
Reviewed-by: Ferruh Yigit <ferruh.yigit@amd.com>
---
 doc/guides/nics/features/sfc.ini |   1 +
 drivers/net/sfc/sfc.h            |   2 +
 drivers/net/sfc/sfc_ethdev.c     | 337 +++++++++++++++++++++++++++++++
 drivers/net/sfc/sfc_port.c       |  24 ++-
 4 files changed, 353 insertions(+), 11 deletions(-)

diff --git a/doc/guides/nics/features/sfc.ini b/doc/guides/nics/features/sfc.ini
index f5ac644278ae..e0b9bfb7f7bf 100644
--- a/doc/guides/nics/features/sfc.ini
+++ b/doc/guides/nics/features/sfc.ini
@@ -24,6 +24,7 @@ RSS reta update      = Y
 SR-IOV               = Y
 Flow control         = Y
 VLAN offload         = P
+FEC                  = Y
 L3 checksum offload  = Y
 L4 checksum offload  = Y
 Inner L3 checksum    = Y
diff --git a/drivers/net/sfc/sfc.h b/drivers/net/sfc/sfc.h
index 730d054aea74..e42abe42cb8a 100644
--- a/drivers/net/sfc/sfc.h
+++ b/drivers/net/sfc/sfc.h
@@ -68,6 +68,8 @@ struct sfc_port {
 
 	uint32_t			phy_adv_cap_mask;
 	uint32_t			phy_adv_cap;
+	uint32_t			fec_cfg;
+	bool				fec_auto;
 
 	unsigned int			flow_ctrl;
 	boolean_t			flow_ctrl_autoneg;
diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index 6d41eb000345..05fc70541b10 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -2343,6 +2343,340 @@ sfc_rx_metadata_negotiate(struct rte_eth_dev *dev, uint64_t *features)
 	return 0;
 }
 
+static unsigned int
+sfc_fec_get_capa_speed_to_fec(uint32_t supported_caps,
+			      struct rte_eth_fec_capa *speed_fec_capa)
+{
+	int num = 0;
+
+	if (supported_caps & (1u << EFX_PHY_CAP_10000FDX)) {
+		if (speed_fec_capa) {
+			speed_fec_capa[num].speed = RTE_ETH_SPEED_NUM_10G;
+			speed_fec_capa[num].capa =
+				RTE_ETH_FEC_MODE_CAPA_MASK(NOFEC) |
+				RTE_ETH_FEC_MODE_CAPA_MASK(AUTO) |
+				RTE_ETH_FEC_MODE_CAPA_MASK(BASER);
+		}
+		num++;
+	}
+	if (supported_caps & (1u << EFX_PHY_CAP_25000FDX)) {
+		if (speed_fec_capa) {
+			speed_fec_capa[num].speed = RTE_ETH_SPEED_NUM_25G;
+			speed_fec_capa[num].capa =
+				RTE_ETH_FEC_MODE_CAPA_MASK(NOFEC) |
+				RTE_ETH_FEC_MODE_CAPA_MASK(AUTO) |
+				RTE_ETH_FEC_MODE_CAPA_MASK(BASER) |
+				RTE_ETH_FEC_MODE_CAPA_MASK(RS);
+		}
+		num++;
+	}
+	if (supported_caps & (1u << EFX_PHY_CAP_40000FDX)) {
+		if (speed_fec_capa) {
+			speed_fec_capa[num].speed = RTE_ETH_SPEED_NUM_40G;
+			speed_fec_capa[num].capa =
+				RTE_ETH_FEC_MODE_CAPA_MASK(NOFEC) |
+				RTE_ETH_FEC_MODE_CAPA_MASK(AUTO) |
+				RTE_ETH_FEC_MODE_CAPA_MASK(BASER);
+		}
+		num++;
+	}
+	if (supported_caps & (1u << EFX_PHY_CAP_50000FDX)) {
+		if (speed_fec_capa) {
+			speed_fec_capa[num].speed = RTE_ETH_SPEED_NUM_50G;
+			speed_fec_capa[num].capa =
+				RTE_ETH_FEC_MODE_CAPA_MASK(NOFEC) |
+				RTE_ETH_FEC_MODE_CAPA_MASK(AUTO) |
+				RTE_ETH_FEC_MODE_CAPA_MASK(BASER) |
+				RTE_ETH_FEC_MODE_CAPA_MASK(RS);
+		}
+		num++;
+	}
+	if (supported_caps & (1u << EFX_PHY_CAP_100000FDX)) {
+		if (speed_fec_capa) {
+			speed_fec_capa[num].speed = RTE_ETH_SPEED_NUM_100G;
+			speed_fec_capa[num].capa =
+				RTE_ETH_FEC_MODE_CAPA_MASK(NOFEC) |
+				RTE_ETH_FEC_MODE_CAPA_MASK(AUTO) |
+				RTE_ETH_FEC_MODE_CAPA_MASK(RS);
+		}
+		num++;
+	}
+
+	return num;
+}
+
+static int
+sfc_fec_get_capability(struct rte_eth_dev *dev,
+		       struct rte_eth_fec_capa *speed_fec_capa,
+		       unsigned int num)
+{
+	struct sfc_adapter *sa = sfc_adapter_by_eth_dev(dev);
+	unsigned int num_entries;
+	uint32_t supported_caps;
+
+	sfc_adapter_lock(sa);
+
+	efx_phy_adv_cap_get(sa->nic, EFX_PHY_CAP_PERM, &supported_caps);
+
+	num_entries = sfc_fec_get_capa_speed_to_fec(supported_caps, NULL);
+	if (!speed_fec_capa || num < num_entries)
+		goto adapter_unlock;
+
+	num_entries = sfc_fec_get_capa_speed_to_fec(supported_caps,
+						    speed_fec_capa);
+
+adapter_unlock:
+	sfc_adapter_unlock(sa);
+
+	return num_entries;
+}
+
+static uint32_t
+sfc_efx_caps_to_fec(uint32_t caps, bool is_25g)
+{
+	bool rs_req = caps & EFX_PHY_CAP_FEC_BIT(RS_FEC_REQUESTED);
+	bool rs = caps & EFX_PHY_CAP_FEC_BIT(RS_FEC);
+	uint32_t fec_capa = 0;
+	bool baser_req;
+	bool baser;
+
+	if (is_25g) {
+		baser = caps & EFX_PHY_CAP_FEC_BIT(25G_BASER_FEC);
+		baser_req = caps & EFX_PHY_CAP_FEC_BIT(25G_BASER_FEC_REQUESTED);
+	} else {
+		baser = caps & EFX_PHY_CAP_FEC_BIT(BASER_FEC);
+		baser_req = caps & EFX_PHY_CAP_FEC_BIT(BASER_FEC_REQUESTED);
+	}
+
+	if (!baser && !rs)
+		return RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_NOFEC);
+
+	if (rs_req)
+		fec_capa |= RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_RS);
+
+	if (baser_req)
+		fec_capa |= RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_BASER);
+
+
+	return fec_capa;
+}
+
+static int
+sfc_fec_get(struct rte_eth_dev *dev, uint32_t *fec_capa)
+{
+	struct sfc_adapter *sa = sfc_adapter_by_eth_dev(dev);
+	struct sfc_port *port = &sa->port;
+	struct rte_eth_link current_link;
+	efx_phy_fec_type_t active_fec;
+	bool is_25g = false;
+	int rc = 0;
+
+	sfc_adapter_lock(sa);
+
+	sfc_dev_get_rte_link(dev, 1, &current_link);
+
+	if (current_link.link_status == RTE_ETH_LINK_DOWN) {
+		uint32_t speed = current_link.link_speed;
+
+		if (port->fec_auto) {
+			*fec_capa = RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_AUTO);
+			goto adapter_unlock;
+		}
+
+		is_25g = (speed == RTE_ETH_SPEED_NUM_25G ||
+			  speed == RTE_ETH_SPEED_NUM_50G);
+
+		*fec_capa = sfc_efx_caps_to_fec(port->fec_cfg, is_25g);
+
+		goto adapter_unlock;
+	}
+
+	rc = efx_phy_fec_type_get(sa->nic, &active_fec);
+	if (rc != 0)
+		goto adapter_unlock;
+
+	switch (active_fec) {
+	case EFX_PHY_FEC_NONE:
+		*fec_capa = RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_NOFEC);
+		break;
+	case EFX_PHY_FEC_BASER:
+		*fec_capa = RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_BASER);
+		break;
+	case EFX_PHY_FEC_RS:
+		*fec_capa = RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_RS);
+		break;
+	default:
+		break;
+	}
+
+adapter_unlock:
+	sfc_adapter_unlock(sa);
+
+	return rc;
+}
+
+static int
+sfc_fec_capa_check(struct rte_eth_dev *dev, uint32_t fec_capa,
+		   uint32_t supported_caps)
+{
+	struct rte_eth_fec_capa *speed_fec_capa;
+	struct rte_eth_link current_link;
+	bool is_supported = false;
+	unsigned int num_entries;
+	bool auto_fec = false;
+	unsigned int i;
+
+	struct sfc_adapter *sa = sfc_adapter_by_eth_dev(dev);
+
+	if (sa->state != SFC_ETHDEV_STARTED)
+		return 0;
+
+	if (fec_capa & RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_AUTO)) {
+		auto_fec = true;
+		fec_capa &= ~RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_AUTO);
+	}
+
+	/*
+	 * If only the AUTO bit is set, the decision on which FEC
+	 * mode to use will be made by HW/FW or driver.
+	 */
+	if (auto_fec && fec_capa == 0)
+		return 0;
+
+	sfc_dev_get_rte_link(dev, 1, &current_link);
+
+	num_entries = sfc_fec_get_capa_speed_to_fec(supported_caps, NULL);
+	if (num_entries == 0)
+		return -ENOTSUP;
+
+	speed_fec_capa = rte_calloc("fec_capa", num_entries,
+				    sizeof(*speed_fec_capa), 0);
+	num_entries = sfc_fec_get_capa_speed_to_fec(supported_caps,
+						    speed_fec_capa);
+
+	for (i = 0; i < num_entries; i++) {
+		if (speed_fec_capa[i].speed == current_link.link_speed) {
+			if ((fec_capa & speed_fec_capa[i].capa) != 0)
+				is_supported = true;
+
+			break;
+		}
+	}
+
+	rte_free(speed_fec_capa);
+
+	if (is_supported)
+		return 0;
+
+	return -ENOTSUP;
+}
+
+static int
+sfc_fec_capa_to_efx(uint32_t supported_caps, uint32_t fec_capa,
+		    uint32_t *efx_fec_caps)
+{
+	bool fec_is_set = false;
+	bool auto_fec = false;
+	bool nofec = false;
+	uint32_t ret = 0;
+
+	if (efx_fec_caps == NULL)
+		return -EINVAL;
+
+	if (fec_capa & RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_AUTO))
+		auto_fec = true;
+
+	if (fec_capa & RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_NOFEC))
+		nofec = true;
+
+	if (fec_capa == RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_AUTO)) {
+		ret |= (EFX_PHY_CAP_FEC_BIT(BASER_FEC) |
+			EFX_PHY_CAP_FEC_BIT(25G_BASER_FEC) |
+			EFX_PHY_CAP_FEC_BIT(RS_FEC)) & supported_caps;
+		goto done;
+	}
+
+	if (fec_capa & RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_RS)) {
+		fec_is_set = true;
+
+		if (supported_caps & EFX_PHY_CAP_FEC_BIT(RS_FEC)) {
+			ret |= EFX_PHY_CAP_FEC_BIT(RS_FEC) |
+			       EFX_PHY_CAP_FEC_BIT(RS_FEC_REQUESTED);
+		}
+	}
+	if (fec_capa & RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_BASER)) {
+		if (!auto_fec && fec_is_set)
+			return -EINVAL;
+
+		if (supported_caps & EFX_PHY_CAP_FEC_BIT(BASER_FEC)) {
+			ret |= EFX_PHY_CAP_FEC_BIT(BASER_FEC) |
+			       EFX_PHY_CAP_FEC_BIT(BASER_FEC_REQUESTED);
+		}
+		if (supported_caps & EFX_PHY_CAP_FEC_BIT(25G_BASER_FEC)) {
+			ret |= EFX_PHY_CAP_FEC_BIT(25G_BASER_FEC) |
+			       EFX_PHY_CAP_FEC_BIT(25G_BASER_FEC_REQUESTED);
+		}
+	}
+
+	if (ret == 0 && !nofec)
+		return -ENOTSUP;
+
+done:
+	*efx_fec_caps = ret;
+	return 0;
+}
+
+static int
+sfc_fec_set(struct rte_eth_dev *dev, uint32_t fec_capa)
+{
+	struct sfc_adapter *sa = sfc_adapter_by_eth_dev(dev);
+	struct sfc_port *port = &sa->port;
+	uint32_t supported_caps;
+	uint32_t efx_fec_caps;
+	uint32_t updated_caps;
+	int rc = 0;
+
+	sfc_adapter_lock(sa);
+
+	efx_phy_adv_cap_get(sa->nic, EFX_PHY_CAP_PERM, &supported_caps);
+
+	rc = sfc_fec_capa_check(dev, fec_capa, supported_caps);
+	if (rc != 0)
+		goto adapter_unlock;
+
+	rc = sfc_fec_capa_to_efx(supported_caps, fec_capa, &efx_fec_caps);
+	if (rc != 0)
+		goto adapter_unlock;
+
+	if (sa->state == SFC_ETHDEV_STARTED) {
+		efx_phy_adv_cap_get(sa->nic, EFX_PHY_CAP_CURRENT,
+				    &updated_caps);
+		updated_caps = updated_caps & ~EFX_PHY_CAP_FEC_MASK;
+		updated_caps |= efx_fec_caps;
+
+		rc = efx_phy_adv_cap_set(sa->nic, updated_caps);
+		if (rc != 0)
+			goto adapter_unlock;
+	}
+
+	port->fec_cfg = efx_fec_caps;
+	/*
+	 * There is no chance to recognize AUTO mode from the
+	 * saved FEC capabilities as AUTO mode can have the same
+	 * set of bits as any other mode from the EFX point of view.
+	 * Save it in the proper variable.
+	 */
+	if (fec_capa & RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_AUTO))
+		port->fec_auto = true;
+	else
+		port->fec_auto = false;
+
+adapter_unlock:
+	sfc_adapter_unlock(sa);
+
+	return rc;
+}
+
 static const struct eth_dev_ops sfc_eth_dev_ops = {
 	.dev_configure			= sfc_dev_configure,
 	.dev_start			= sfc_dev_start,
@@ -2392,6 +2726,9 @@ static const struct eth_dev_ops sfc_eth_dev_ops = {
 	.pool_ops_supported		= sfc_pool_ops_supported,
 	.representor_info_get		= sfc_representor_info_get,
 	.rx_metadata_negotiate		= sfc_rx_metadata_negotiate,
+	.fec_get_capability		= sfc_fec_get_capability,
+	.fec_get			= sfc_fec_get,
+	.fec_set			= sfc_fec_set,
 };
 
 struct sfc_ethdev_init_data {
diff --git a/drivers/net/sfc/sfc_port.c b/drivers/net/sfc/sfc_port.c
index 5f312ab1ba83..0c887ddedb09 100644
--- a/drivers/net/sfc/sfc_port.c
+++ b/drivers/net/sfc/sfc_port.c
@@ -228,17 +228,8 @@ sfc_port_start(struct sfc_adapter *sa)
 	/* Preserve pause capabilities set by above efx_mac_fcntl_set()  */
 	efx_phy_adv_cap_get(sa->nic, EFX_PHY_CAP_CURRENT, &phy_adv_cap);
 	SFC_ASSERT((port->phy_adv_cap & phy_pause_caps) == 0);
-	phy_adv_cap = port->phy_adv_cap | (phy_adv_cap & phy_pause_caps);
-
-	/*
-	 * No controls for FEC yet. Use default FEC mode.
-	 * I.e. advertise everything supported (*_FEC=1), but do not request
-	 * anything explicitly (*_FEC_REQUESTED=0).
-	 */
-	phy_adv_cap |= port->phy_adv_cap_mask &
-		(1u << EFX_PHY_CAP_BASER_FEC |
-		 1u << EFX_PHY_CAP_RS_FEC |
-		 1u << EFX_PHY_CAP_25G_BASER_FEC);
+	phy_adv_cap = port->phy_adv_cap | (phy_adv_cap & phy_pause_caps) |
+			port->fec_cfg;
 
 	sfc_log_init(sa, "set phy adv caps to %#x", phy_adv_cap);
 	rc = efx_phy_adv_cap_set(sa->nic, phy_adv_cap);
@@ -469,6 +460,17 @@ sfc_port_attach(struct sfc_adapter *sa)
 
 	port->mac_stats_update_period_ms = kvarg_stats_update_period_ms;
 
+	/*
+	 * Set default FEC mode.
+	 * I.e. advertise everything supported (*_FEC=1), but do not request
+	 * anything explicitly (*_FEC_REQUESTED=0).
+	 */
+	port->fec_cfg = port->phy_adv_cap_mask &
+		(EFX_PHY_CAP_FEC_BIT(BASER_FEC) |
+		 EFX_PHY_CAP_FEC_BIT(RS_FEC) |
+		 EFX_PHY_CAP_FEC_BIT(25G_BASER_FEC));
+	port->fec_auto = true;
+
 	sfc_log_init(sa, "done");
 	return 0;
 
-- 
2.37.0 (Apple Git-136)

Re: [PATCH 1/3] net/sfc: split link update function

[Andrew Rybchenko]

On 6/2/23 01:23, Denis Pryazhennikov wrote:
> Separate the original link update function into
> two functions: state retrieval and update.
> This improves code clarity and maintainability.
> 
> Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
> Reviewed-by: Ivan Malov <ivan.malov@arknetworks.am>
> Reviewed-by: Andy Moreton <amoreton@xilinx.com>

Acked-by: Andrew Rybchenko <arybchenko@oktetlabs.ru>

Re: [PATCH 2/3] common/sfc_efx/base: add FEC related macros

[Andrew Rybchenko]

On 6/2/23 01:23, Denis Pryazhennikov wrote:
> Added new macros to simplify working with FEC bits

Full stop missing above.

> 
> Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
> Reviewed-by: Andy Moreton <amoreton@xilinx.com>

Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

Re: [PATCH 3/3] net/sfc: support FEC feature

[Andrew Rybchenko]

On 6/2/23 01:23, Denis Pryazhennikov wrote:
> Support ethdev methods to query and set FEC information.
> Limitations: ignoring rte_eth_fec_get_capability() results
> can lead to NOFEC if the device is not strated.
> 
> Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
> Reviewed-by: Ivan Malov <ivan.malov@arknetworks.am>
> Reviewed-by: Andy Moreton <amoreton@xilinx.com>
> Reviewed-by: Ferruh Yigit <ferruh.yigit@amd.com>
> ---
>   doc/guides/nics/features/sfc.ini |   1 +
>   drivers/net/sfc/sfc.h            |   2 +
>   drivers/net/sfc/sfc_ethdev.c     | 337 +++++++++++++++++++++++++++++++
>   drivers/net/sfc/sfc_port.c       |  24 ++-
>   4 files changed, 353 insertions(+), 11 deletions(-)

Let's advertise it in release notes.

> 
> diff --git a/doc/guides/nics/features/sfc.ini b/doc/guides/nics/features/sfc.ini
> index f5ac644278ae..e0b9bfb7f7bf 100644
> --- a/doc/guides/nics/features/sfc.ini
> +++ b/doc/guides/nics/features/sfc.ini
> @@ -24,6 +24,7 @@ RSS reta update      = Y
>   SR-IOV               = Y
>   Flow control         = Y
>   VLAN offload         = P
> +FEC                  = Y
>   L3 checksum offload  = Y
>   L4 checksum offload  = Y
>   Inner L3 checksum    = Y
> diff --git a/drivers/net/sfc/sfc.h b/drivers/net/sfc/sfc.h
> index 730d054aea74..e42abe42cb8a 100644
> --- a/drivers/net/sfc/sfc.h
> +++ b/drivers/net/sfc/sfc.h
> @@ -68,6 +68,8 @@ struct sfc_port {
>   
>   	uint32_t			phy_adv_cap_mask;
>   	uint32_t			phy_adv_cap;
> +	uint32_t			fec_cfg;
> +	bool				fec_auto;
>   
>   	unsigned int			flow_ctrl;
>   	boolean_t			flow_ctrl_autoneg;
> diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
> index 6d41eb000345..05fc70541b10 100644
> --- a/drivers/net/sfc/sfc_ethdev.c
> +++ b/drivers/net/sfc/sfc_ethdev.c
> @@ -2343,6 +2343,340 @@ sfc_rx_metadata_negotiate(struct rte_eth_dev *dev, uint64_t *features)
>   	return 0;
>   }
>   
> +static unsigned int
> +sfc_fec_get_capa_speed_to_fec(uint32_t supported_caps,
> +			      struct rte_eth_fec_capa *speed_fec_capa)
> +{
> +	int num = 0;
> +
> +	if (supported_caps & (1u << EFX_PHY_CAP_10000FDX)) {
> +		if (speed_fec_capa) {

Compare vs NULL as DPDK coding style says. Here and in similar cases below.

> +			speed_fec_capa[num].speed = RTE_ETH_SPEED_NUM_10G;
> +			speed_fec_capa[num].capa =
> +				RTE_ETH_FEC_MODE_CAPA_MASK(NOFEC) |
> +				RTE_ETH_FEC_MODE_CAPA_MASK(AUTO) |
> +				RTE_ETH_FEC_MODE_CAPA_MASK(BASER);

I'd like to see a comment which explicilty says that supported
FEC depends on supported link speed only. So, there is no
special capability bits.

> +		}
> +		num++;
> +	}
> +	if (supported_caps & (1u << EFX_PHY_CAP_25000FDX)) {
> +		if (speed_fec_capa) {
> +			speed_fec_capa[num].speed = RTE_ETH_SPEED_NUM_25G;
> +			speed_fec_capa[num].capa =
> +				RTE_ETH_FEC_MODE_CAPA_MASK(NOFEC) |
> +				RTE_ETH_FEC_MODE_CAPA_MASK(AUTO) |
> +				RTE_ETH_FEC_MODE_CAPA_MASK(BASER) |
> +				RTE_ETH_FEC_MODE_CAPA_MASK(RS);
> +		}
> +		num++;
> +	}
> +	if (supported_caps & (1u << EFX_PHY_CAP_40000FDX)) {
> +		if (speed_fec_capa) {
> +			speed_fec_capa[num].speed = RTE_ETH_SPEED_NUM_40G;
> +			speed_fec_capa[num].capa =
> +				RTE_ETH_FEC_MODE_CAPA_MASK(NOFEC) |
> +				RTE_ETH_FEC_MODE_CAPA_MASK(AUTO) |
> +				RTE_ETH_FEC_MODE_CAPA_MASK(BASER);
> +		}
> +		num++;
> +	}
> +	if (supported_caps & (1u << EFX_PHY_CAP_50000FDX)) {
> +		if (speed_fec_capa) {
> +			speed_fec_capa[num].speed = RTE_ETH_SPEED_NUM_50G;
> +			speed_fec_capa[num].capa =
> +				RTE_ETH_FEC_MODE_CAPA_MASK(NOFEC) |
> +				RTE_ETH_FEC_MODE_CAPA_MASK(AUTO) |
> +				RTE_ETH_FEC_MODE_CAPA_MASK(BASER) |
> +				RTE_ETH_FEC_MODE_CAPA_MASK(RS);
> +		}
> +		num++;
> +	}
> +	if (supported_caps & (1u << EFX_PHY_CAP_100000FDX)) {
> +		if (speed_fec_capa) {
> +			speed_fec_capa[num].speed = RTE_ETH_SPEED_NUM_100G;
> +			speed_fec_capa[num].capa =
> +				RTE_ETH_FEC_MODE_CAPA_MASK(NOFEC) |
> +				RTE_ETH_FEC_MODE_CAPA_MASK(AUTO) |
> +				RTE_ETH_FEC_MODE_CAPA_MASK(RS);
> +		}
> +		num++;
> +	}
> +
> +	return num;
> +}
> +
> +static int
> +sfc_fec_get_capability(struct rte_eth_dev *dev,
> +		       struct rte_eth_fec_capa *speed_fec_capa,
> +		       unsigned int num)
> +{
> +	struct sfc_adapter *sa = sfc_adapter_by_eth_dev(dev);
> +	unsigned int num_entries;
> +	uint32_t supported_caps;
> +
> +	sfc_adapter_lock(sa);
> +
> +	efx_phy_adv_cap_get(sa->nic, EFX_PHY_CAP_PERM, &supported_caps);
> +
> +	num_entries = sfc_fec_get_capa_speed_to_fec(supported_caps, NULL);
> +	if (!speed_fec_capa || num < num_entries)

Compare vs NULL as DPDK coding style says.

> +		goto adapter_unlock;
> +
> +	num_entries = sfc_fec_get_capa_speed_to_fec(supported_caps,
> +						    speed_fec_capa);
> +
> +adapter_unlock:
> +	sfc_adapter_unlock(sa);
> +
> +	return num_entries;
> +}
> +
> +static uint32_t
> +sfc_efx_caps_to_fec(uint32_t caps, bool is_25g)
> +{
> +	bool rs_req = caps & EFX_PHY_CAP_FEC_BIT(RS_FEC_REQUESTED);
> +	bool rs = caps & EFX_PHY_CAP_FEC_BIT(RS_FEC);
> +	uint32_t fec_capa = 0;
> +	bool baser_req;
> +	bool baser;
> +
> +	if (is_25g) {
> +		baser = caps & EFX_PHY_CAP_FEC_BIT(25G_BASER_FEC);
> +		baser_req = caps & EFX_PHY_CAP_FEC_BIT(25G_BASER_FEC_REQUESTED);
> +	} else {
> +		baser = caps & EFX_PHY_CAP_FEC_BIT(BASER_FEC);
> +		baser_req = caps & EFX_PHY_CAP_FEC_BIT(BASER_FEC_REQUESTED);
> +	}
> +
> +	if (!baser && !rs)
> +		return RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_NOFEC);
> +
> +	if (rs_req)
> +		fec_capa |= RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_RS);
> +
> +	if (baser_req)
> +		fec_capa |= RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_BASER);
> +
> +

two many empty lines, just one is sufficient

> +	return fec_capa;
> +}
> +
> +static int
> +sfc_fec_get(struct rte_eth_dev *dev, uint32_t *fec_capa)
> +{
> +	struct sfc_adapter *sa = sfc_adapter_by_eth_dev(dev);
> +	struct sfc_port *port = &sa->port;
> +	struct rte_eth_link current_link;
> +	efx_phy_fec_type_t active_fec;
> +	bool is_25g = false;
> +	int rc = 0;
> +
> +	sfc_adapter_lock(sa);
> +
> +	sfc_dev_get_rte_link(dev, 1, &current_link);
> +
> +	if (current_link.link_status == RTE_ETH_LINK_DOWN) {
> +		uint32_t speed = current_link.link_speed;
> +
> +		if (port->fec_auto) {
> +			*fec_capa = RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_AUTO);
> +			goto adapter_unlock;
> +		}
> +
> +		is_25g = (speed == RTE_ETH_SPEED_NUM_25G ||
> +			  speed == RTE_ETH_SPEED_NUM_50G);
> +
> +		*fec_capa = sfc_efx_caps_to_fec(port->fec_cfg, is_25g);
> +
> +		goto adapter_unlock;
> +	}
> +
> +	rc = efx_phy_fec_type_get(sa->nic, &active_fec);
> +	if (rc != 0)
> +		goto adapter_unlock;
> +
> +	switch (active_fec) {
> +	case EFX_PHY_FEC_NONE:
> +		*fec_capa = RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_NOFEC);
> +		break;
> +	case EFX_PHY_FEC_BASER:
> +		*fec_capa = RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_BASER);
> +		break;
> +	case EFX_PHY_FEC_RS:
> +		*fec_capa = RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_RS);
> +		break;
> +	default:

Don't we need an error message here and return failure?

> +		break;
> +	}
> +
> +adapter_unlock:
> +	sfc_adapter_unlock(sa);
> +
> +	return rc;
> +}
> +
> +static int
> +sfc_fec_capa_check(struct rte_eth_dev *dev, uint32_t fec_capa,
> +		   uint32_t supported_caps)
> +{
> +	struct rte_eth_fec_capa *speed_fec_capa;
> +	struct rte_eth_link current_link;
> +	bool is_supported = false;
> +	unsigned int num_entries;
> +	bool auto_fec = false;
> +	unsigned int i;
> +
> +	struct sfc_adapter *sa = sfc_adapter_by_eth_dev(dev);
> +
> +	if (sa->state != SFC_ETHDEV_STARTED)
> +		return 0;
> +
> +	if (fec_capa & RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_AUTO)) {
> +		auto_fec = true;
> +		fec_capa &= ~RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_AUTO);
> +	}
> +
> +	/*
> +	 * If only the AUTO bit is set, the decision on which FEC
> +	 * mode to use will be made by HW/FW or driver.
> +	 */
> +	if (auto_fec && fec_capa == 0)
> +		return 0;
> +
> +	sfc_dev_get_rte_link(dev, 1, &current_link);
> +
> +	num_entries = sfc_fec_get_capa_speed_to_fec(supported_caps, NULL);
> +	if (num_entries == 0)
> +		return -ENOTSUP;
> +
> +	speed_fec_capa = rte_calloc("fec_capa", num_entries,
> +				    sizeof(*speed_fec_capa), 0);
> +	num_entries = sfc_fec_get_capa_speed_to_fec(supported_caps,
> +						    speed_fec_capa);
> +
> +	for (i = 0; i < num_entries; i++) {
> +		if (speed_fec_capa[i].speed == current_link.link_speed) {
> +			if ((fec_capa & speed_fec_capa[i].capa) != 0)
> +				is_supported = true;
> +
> +			break;
> +		}
> +	}
> +
> +	rte_free(speed_fec_capa);
> +
> +	if (is_supported)
> +		return 0;
> +
> +	return -ENOTSUP;
> +}
> +
> +static int
> +sfc_fec_capa_to_efx(uint32_t supported_caps, uint32_t fec_capa,
> +		    uint32_t *efx_fec_caps)
> +{
> +	bool fec_is_set = false;
> +	bool auto_fec = false;
> +	bool nofec = false;
> +	uint32_t ret = 0;
> +
> +	if (efx_fec_caps == NULL)
> +		return -EINVAL;
> +
> +	if (fec_capa & RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_AUTO))
> +		auto_fec = true;
> +
> +	if (fec_capa & RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_NOFEC))
> +		nofec = true;
> +
> +	if (fec_capa == RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_AUTO)) {
> +		ret |= (EFX_PHY_CAP_FEC_BIT(BASER_FEC) |
> +			EFX_PHY_CAP_FEC_BIT(25G_BASER_FEC) |
> +			EFX_PHY_CAP_FEC_BIT(RS_FEC)) & supported_caps;
> +		goto done;
> +	}
> +
> +	if (fec_capa & RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_RS)) {
> +		fec_is_set = true;
> +
> +		if (supported_caps & EFX_PHY_CAP_FEC_BIT(RS_FEC)) {
> +			ret |= EFX_PHY_CAP_FEC_BIT(RS_FEC) |
> +			       EFX_PHY_CAP_FEC_BIT(RS_FEC_REQUESTED);
> +		}
> +	}
> +	if (fec_capa & RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_BASER)) {
> +		if (!auto_fec && fec_is_set)
> +			return -EINVAL;
> +
> +		if (supported_caps & EFX_PHY_CAP_FEC_BIT(BASER_FEC)) {
> +			ret |= EFX_PHY_CAP_FEC_BIT(BASER_FEC) |
> +			       EFX_PHY_CAP_FEC_BIT(BASER_FEC_REQUESTED);
> +		}
> +		if (supported_caps & EFX_PHY_CAP_FEC_BIT(25G_BASER_FEC)) {
> +			ret |= EFX_PHY_CAP_FEC_BIT(25G_BASER_FEC) |
> +			       EFX_PHY_CAP_FEC_BIT(25G_BASER_FEC_REQUESTED);
> +		}
> +	}
> +
> +	if (ret == 0 && !nofec)
> +		return -ENOTSUP;
> +
> +done:
> +	*efx_fec_caps = ret;
> +	return 0;
> +}
> +
> +static int
> +sfc_fec_set(struct rte_eth_dev *dev, uint32_t fec_capa)
> +{
> +	struct sfc_adapter *sa = sfc_adapter_by_eth_dev(dev);
> +	struct sfc_port *port = &sa->port;
> +	uint32_t supported_caps;
> +	uint32_t efx_fec_caps;
> +	uint32_t updated_caps;
> +	int rc = 0;
> +
> +	sfc_adapter_lock(sa);
> +
> +	efx_phy_adv_cap_get(sa->nic, EFX_PHY_CAP_PERM, &supported_caps);
> +
> +	rc = sfc_fec_capa_check(dev, fec_capa, supported_caps);
> +	if (rc != 0)
> +		goto adapter_unlock;
> +
> +	rc = sfc_fec_capa_to_efx(supported_caps, fec_capa, &efx_fec_caps);
> +	if (rc != 0)
> +		goto adapter_unlock;
> +
> +	if (sa->state == SFC_ETHDEV_STARTED) {
> +		efx_phy_adv_cap_get(sa->nic, EFX_PHY_CAP_CURRENT,
> +				    &updated_caps);
> +		updated_caps = updated_caps & ~EFX_PHY_CAP_FEC_MASK;
> +		updated_caps |= efx_fec_caps;
> +
> +		rc = efx_phy_adv_cap_set(sa->nic, updated_caps);
> +		if (rc != 0)
> +			goto adapter_unlock;
> +	}
> +
> +	port->fec_cfg = efx_fec_caps;
> +	/*
> +	 * There is no chance to recognize AUTO mode from the
> +	 * saved FEC capabilities as AUTO mode can have the same
> +	 * set of bits as any other mode from the EFX point of view.
> +	 * Save it in the proper variable.
> +	 */
> +	if (fec_capa & RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_AUTO))
> +		port->fec_auto = true;
> +	else
> +		port->fec_auto = false;
> +
> +adapter_unlock:
> +	sfc_adapter_unlock(sa);
> +
> +	return rc;
> +}
> +
>   static const struct eth_dev_ops sfc_eth_dev_ops = {
>   	.dev_configure			= sfc_dev_configure,
>   	.dev_start			= sfc_dev_start,
> @@ -2392,6 +2726,9 @@ static const struct eth_dev_ops sfc_eth_dev_ops = {
>   	.pool_ops_supported		= sfc_pool_ops_supported,
>   	.representor_info_get		= sfc_representor_info_get,
>   	.rx_metadata_negotiate		= sfc_rx_metadata_negotiate,
> +	.fec_get_capability		= sfc_fec_get_capability,
> +	.fec_get			= sfc_fec_get,
> +	.fec_set			= sfc_fec_set,
>   };
>   
>   struct sfc_ethdev_init_data {

[snip]

[PATCH v2 0/3] net/sfc: support FEC feature

[Denis Pryazhennikov]

This patch series adds setting and querying of Forward error
correction (FEC). AUTO, BASER and RS modes are supported
by EF10/EF100 hardware. LLRS mode is not supported.

Changes in v2:
* Applied review notes in [2/3] and [3/3];
* Added a check for the supported by a device FEC modes
  in sfc_fec_get_capa_speed_to_fec();
* Fixed error handling.

Denis Pryazhennikov (3):
  net/sfc: split link update function
  common/sfc_efx/base: add FEC related macros
  net/sfc: support FEC feature

 doc/guides/nics/features/sfc.ini       |   1 +
 doc/guides/rel_notes/release_23_07.rst |   6 +
 drivers/common/sfc_efx/base/efx.h      |   9 +
 drivers/net/sfc/sfc.h                  |   2 +
 drivers/net/sfc/sfc_ethdev.c           | 406 ++++++++++++++++++++++++-
 drivers/net/sfc/sfc_port.c             |  24 +-
 6 files changed, 428 insertions(+), 20 deletions(-)

-- 
2.37.0 (Apple Git-136)

[PATCH v2 1/3] net/sfc: split link update function

[Denis Pryazhennikov]

Separate the original link update function into
two functions: state retrieval and update.
This improves code clarity and maintainability.

Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
Reviewed-by: Ivan Malov <ivan.malov@arknetworks.am>
Reviewed-by: Andy Moreton <amoreton@xilinx.com>
Acked-by: Andrew Rybchenko <arybchenko@oktetlabs.ru>
---
 drivers/net/sfc/sfc_ethdev.c | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index 320043145495..6d41eb000345 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -248,28 +248,38 @@ sfc_dev_start(struct rte_eth_dev *dev)
 	return -rc;
 }
 
-static int
-sfc_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete)
+static void
+sfc_dev_get_rte_link(struct rte_eth_dev *dev, int wait_to_complete,
+		     struct rte_eth_link *link)
 {
 	struct sfc_adapter *sa = sfc_adapter_by_eth_dev(dev);
-	struct rte_eth_link current_link;
-	int ret;
 
-	sfc_log_init(sa, "entry");
+	SFC_ASSERT(link != NULL);
 
 	if (sa->state != SFC_ETHDEV_STARTED) {
-		sfc_port_link_mode_to_info(EFX_LINK_UNKNOWN, &current_link);
+		sfc_port_link_mode_to_info(EFX_LINK_UNKNOWN, link);
 	} else if (wait_to_complete) {
 		efx_link_mode_t link_mode;
 
 		if (efx_port_poll(sa->nic, &link_mode) != 0)
 			link_mode = EFX_LINK_UNKNOWN;
-		sfc_port_link_mode_to_info(link_mode, &current_link);
-
+		sfc_port_link_mode_to_info(link_mode, link);
 	} else {
 		sfc_ev_mgmt_qpoll(sa);
-		rte_eth_linkstatus_get(dev, &current_link);
+		rte_eth_linkstatus_get(dev, link);
 	}
+}
+
+static int
+sfc_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete)
+{
+	struct sfc_adapter *sa = sfc_adapter_by_eth_dev(dev);
+	struct rte_eth_link current_link;
+	int ret;
+
+	sfc_log_init(sa, "entry");
+
+	sfc_dev_get_rte_link(dev, wait_to_complete, &current_link);
 
 	ret = rte_eth_linkstatus_set(dev, &current_link);
 	if (ret == 0)
-- 
2.37.0 (Apple Git-136)

[PATCH v2 2/3] common/sfc_efx/base: add FEC related macros

[Denis Pryazhennikov]

Added new macros to simplify working with FEC bits.

Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
Reviewed-by: Andy Moreton <amoreton@xilinx.com>
Acked-by: Andrew Rybchenko <arybchenko@oktetlabs.ru>
---
 drivers/common/sfc_efx/base/efx.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/common/sfc_efx/base/efx.h b/drivers/common/sfc_efx/base/efx.h
index 49e29dcc1c69..10c412bcd7dc 100644
--- a/drivers/common/sfc_efx/base/efx.h
+++ b/drivers/common/sfc_efx/base/efx.h
@@ -3958,6 +3958,15 @@ typedef enum efx_phy_fec_type_e {
 	EFX_PHY_FEC_RS
 } efx_phy_fec_type_t;
 
+#define EFX_PHY_CAP_FEC_BIT(_fec_bit) (1U << EFX_PHY_CAP_##_fec_bit)
+#define EFX_PHY_CAP_FEC_MASK \
+	(EFX_PHY_CAP_FEC_BIT(BASER_FEC) |		\
+	 EFX_PHY_CAP_FEC_BIT(25G_BASER_FEC) |		\
+	 EFX_PHY_CAP_FEC_BIT(BASER_FEC_REQUESTED) |	\
+	 EFX_PHY_CAP_FEC_BIT(25G_BASER_FEC_REQUESTED) |	\
+	 EFX_PHY_CAP_FEC_BIT(RS_FEC) |			\
+	 EFX_PHY_CAP_FEC_BIT(RS_FEC_REQUESTED))
+
 LIBEFX_API
 extern	__checkReturn	efx_rc_t
 efx_phy_fec_type_get(
-- 
2.37.0 (Apple Git-136)

[PATCH v2 3/3] net/sfc: support FEC feature

[Denis Pryazhennikov]

Support ethdev methods to query and set FEC information.
Limitations: ignoring rte_eth_fec_get_capability() results
can lead to NOFEC if the device is not strated.

Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
Reviewed-by: Ivan Malov <ivan.malov@arknetworks.am>
Reviewed-by: Andy Moreton <amoreton@xilinx.com>
Reviewed-by: Ferruh Yigit <ferruh.yigit@amd.com>
---
 doc/guides/nics/features/sfc.ini       |   1 +
 doc/guides/rel_notes/release_23_07.rst |   6 +
 drivers/net/sfc/sfc.h                  |   2 +
 drivers/net/sfc/sfc_ethdev.c           | 378 +++++++++++++++++++++++++
 drivers/net/sfc/sfc_port.c             |  24 +-
 5 files changed, 400 insertions(+), 11 deletions(-)

diff --git a/doc/guides/nics/features/sfc.ini b/doc/guides/nics/features/sfc.ini
index f5ac644278ae..e0b9bfb7f7bf 100644
--- a/doc/guides/nics/features/sfc.ini
+++ b/doc/guides/nics/features/sfc.ini
@@ -24,6 +24,7 @@ RSS reta update      = Y
 SR-IOV               = Y
 Flow control         = Y
 VLAN offload         = P
+FEC                  = Y
 L3 checksum offload  = Y
 L4 checksum offload  = Y
 Inner L3 checksum    = Y
diff --git a/doc/guides/rel_notes/release_23_07.rst b/doc/guides/rel_notes/release_23_07.rst
index 027ae7bd2d64..59a215a4a4f8 100644
--- a/doc/guides/rel_notes/release_23_07.rst
+++ b/doc/guides/rel_notes/release_23_07.rst
@@ -170,6 +170,12 @@ New Features
 
   See :doc:`../prog_guide/pdcp_lib` for more information.
 
+* **Updated Solarflare network PMD.**
+
+  Updated the Solarflare ``sfc_efx`` driver with changes including:
+
+  * Added support for configuring FEC mode, querying FEC capabilities and
+    current FEC mode from a device.
 
 Removed Items
 -------------
diff --git a/drivers/net/sfc/sfc.h b/drivers/net/sfc/sfc.h
index 730d054aea74..e42abe42cb8a 100644
--- a/drivers/net/sfc/sfc.h
+++ b/drivers/net/sfc/sfc.h
@@ -68,6 +68,8 @@ struct sfc_port {
 
 	uint32_t			phy_adv_cap_mask;
 	uint32_t			phy_adv_cap;
+	uint32_t			fec_cfg;
+	bool				fec_auto;
 
 	unsigned int			flow_ctrl;
 	boolean_t			flow_ctrl_autoneg;
diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index 6d41eb000345..f120ddc5a8d8 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -2343,6 +2343,381 @@ sfc_rx_metadata_negotiate(struct rte_eth_dev *dev, uint64_t *features)
 	return 0;
 }
 
+static unsigned int
+sfc_fec_get_capa_speed_to_fec(uint32_t supported_caps,
+			      struct rte_eth_fec_capa *speed_fec_capa)
+{
+	unsigned int num = 0;
+	bool baser = false;
+	bool rs = false;
+
+	if (supported_caps & EFX_PHY_CAP_FEC_BIT(BASER_FEC))
+		baser = true;
+	if (supported_caps & EFX_PHY_CAP_FEC_BIT(RS_FEC))
+		rs = true;
+
+	/*
+	 * NOFEC and AUTO FEC modes are always supported.
+	 * FW does not provide information about the supported
+	 * FEC modes per the link speed.
+	 * Supported FEC depends on supported link speeds and
+	 * supported FEC modes by a device.
+	 */
+	if (supported_caps & (1u << EFX_PHY_CAP_10000FDX)) {
+		if (speed_fec_capa != NULL) {
+			speed_fec_capa[num].speed = RTE_ETH_SPEED_NUM_10G;
+			speed_fec_capa[num].capa =
+				RTE_ETH_FEC_MODE_CAPA_MASK(NOFEC) |
+				RTE_ETH_FEC_MODE_CAPA_MASK(AUTO);
+			if (baser) {
+				speed_fec_capa[num].capa |=
+					RTE_ETH_FEC_MODE_CAPA_MASK(BASER);
+			}
+		}
+		num++;
+	}
+	if (supported_caps & (1u << EFX_PHY_CAP_25000FDX)) {
+		if (speed_fec_capa != NULL) {
+			speed_fec_capa[num].speed = RTE_ETH_SPEED_NUM_25G;
+			speed_fec_capa[num].capa =
+				RTE_ETH_FEC_MODE_CAPA_MASK(NOFEC) |
+				RTE_ETH_FEC_MODE_CAPA_MASK(AUTO);
+			if (baser) {
+				speed_fec_capa[num].capa |=
+					RTE_ETH_FEC_MODE_CAPA_MASK(BASER);
+			}
+			if (rs) {
+				speed_fec_capa[num].capa |=
+					RTE_ETH_FEC_MODE_CAPA_MASK(RS);
+			}
+		}
+		num++;
+	}
+	if (supported_caps & (1u << EFX_PHY_CAP_40000FDX)) {
+		if (speed_fec_capa != NULL) {
+			speed_fec_capa[num].speed = RTE_ETH_SPEED_NUM_40G;
+			speed_fec_capa[num].capa =
+				RTE_ETH_FEC_MODE_CAPA_MASK(NOFEC) |
+				RTE_ETH_FEC_MODE_CAPA_MASK(AUTO);
+			if (baser) {
+				speed_fec_capa[num].capa |=
+					RTE_ETH_FEC_MODE_CAPA_MASK(BASER);
+			}
+		}
+		num++;
+	}
+	if (supported_caps & (1u << EFX_PHY_CAP_50000FDX)) {
+		if (speed_fec_capa != NULL) {
+			speed_fec_capa[num].speed = RTE_ETH_SPEED_NUM_50G;
+			speed_fec_capa[num].capa =
+				RTE_ETH_FEC_MODE_CAPA_MASK(NOFEC) |
+				RTE_ETH_FEC_MODE_CAPA_MASK(AUTO);
+			if (baser) {
+				speed_fec_capa[num].capa |=
+					RTE_ETH_FEC_MODE_CAPA_MASK(BASER);
+			}
+			if (rs) {
+				speed_fec_capa[num].capa |=
+					RTE_ETH_FEC_MODE_CAPA_MASK(RS);
+			}
+		}
+		num++;
+	}
+	if (supported_caps & (1u << EFX_PHY_CAP_100000FDX)) {
+		if (speed_fec_capa != NULL) {
+			speed_fec_capa[num].speed = RTE_ETH_SPEED_NUM_100G;
+			speed_fec_capa[num].capa =
+				RTE_ETH_FEC_MODE_CAPA_MASK(NOFEC) |
+				RTE_ETH_FEC_MODE_CAPA_MASK(AUTO);
+			if (rs) {
+				speed_fec_capa[num].capa |=
+					RTE_ETH_FEC_MODE_CAPA_MASK(RS);
+			}
+		}
+		num++;
+	}
+
+	return num;
+}
+
+static int
+sfc_fec_get_capability(struct rte_eth_dev *dev,
+		       struct rte_eth_fec_capa *speed_fec_capa,
+		       unsigned int num)
+{
+	struct sfc_adapter *sa = sfc_adapter_by_eth_dev(dev);
+	unsigned int num_entries;
+	uint32_t supported_caps;
+
+	sfc_adapter_lock(sa);
+
+	efx_phy_adv_cap_get(sa->nic, EFX_PHY_CAP_PERM, &supported_caps);
+
+	num_entries = sfc_fec_get_capa_speed_to_fec(supported_caps, NULL);
+	if (speed_fec_capa == NULL || num < num_entries)
+		goto adapter_unlock;
+
+	num_entries = sfc_fec_get_capa_speed_to_fec(supported_caps,
+						    speed_fec_capa);
+
+adapter_unlock:
+	sfc_adapter_unlock(sa);
+
+	return num_entries;
+}
+
+static uint32_t
+sfc_efx_caps_to_fec(uint32_t caps, bool is_25g)
+{
+	bool rs_req = caps & EFX_PHY_CAP_FEC_BIT(RS_FEC_REQUESTED);
+	bool rs = caps & EFX_PHY_CAP_FEC_BIT(RS_FEC);
+	bool baser_req;
+	bool baser;
+
+	if (is_25g) {
+		baser = caps & EFX_PHY_CAP_FEC_BIT(25G_BASER_FEC);
+		baser_req = caps & EFX_PHY_CAP_FEC_BIT(25G_BASER_FEC_REQUESTED);
+	} else {
+		baser = caps & EFX_PHY_CAP_FEC_BIT(BASER_FEC);
+		baser_req = caps & EFX_PHY_CAP_FEC_BIT(BASER_FEC_REQUESTED);
+	}
+
+	if (!baser && !rs)
+		return RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_NOFEC);
+
+	if (rs_req)
+		return RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_RS);
+
+	if (baser_req)
+		return RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_BASER);
+
+	return 0;
+}
+
+static int
+sfc_fec_get(struct rte_eth_dev *dev, uint32_t *fec_capa)
+{
+	struct sfc_adapter *sa = sfc_adapter_by_eth_dev(dev);
+	struct sfc_port *port = &sa->port;
+	struct rte_eth_link current_link;
+	efx_phy_fec_type_t active_fec;
+	bool is_25g = false;
+	int rc = 0;
+
+	sfc_adapter_lock(sa);
+
+	sfc_dev_get_rte_link(dev, 1, &current_link);
+
+	if (current_link.link_status == RTE_ETH_LINK_DOWN) {
+		uint32_t speed = current_link.link_speed;
+
+		if (port->fec_auto) {
+			*fec_capa = RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_AUTO);
+			goto adapter_unlock;
+		}
+
+		is_25g = (speed == RTE_ETH_SPEED_NUM_25G ||
+			  speed == RTE_ETH_SPEED_NUM_50G);
+
+		*fec_capa = sfc_efx_caps_to_fec(port->fec_cfg, is_25g);
+		if (*fec_capa == 0)
+			rc = ENOTSUP;
+
+		goto adapter_unlock;
+	}
+
+	rc = efx_phy_fec_type_get(sa->nic, &active_fec);
+	if (rc != 0)
+		goto adapter_unlock;
+
+	switch (active_fec) {
+	case EFX_PHY_FEC_NONE:
+		*fec_capa = RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_NOFEC);
+		break;
+	case EFX_PHY_FEC_BASER:
+		*fec_capa = RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_BASER);
+		break;
+	case EFX_PHY_FEC_RS:
+		*fec_capa = RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_RS);
+		break;
+	default:
+		rc = ENOTSUP;
+		break;
+	}
+
+adapter_unlock:
+	sfc_adapter_unlock(sa);
+
+	if (rc != 0)
+		sfc_err(sa, "failed to get FEC mode");
+
+	SFC_ASSERT(rc >= 0);
+	return -rc;
+}
+
+static int
+sfc_fec_capa_check(struct rte_eth_dev *dev, uint32_t fec_capa,
+		   uint32_t supported_caps)
+{
+	struct rte_eth_fec_capa *speed_fec_capa;
+	struct rte_eth_link current_link;
+	bool is_supported = false;
+	unsigned int num_entries;
+	bool auto_fec = false;
+	unsigned int i;
+
+	struct sfc_adapter *sa = sfc_adapter_by_eth_dev(dev);
+
+	if (sa->state != SFC_ETHDEV_STARTED)
+		return 0;
+
+	if (fec_capa & RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_AUTO)) {
+		auto_fec = true;
+		fec_capa &= ~RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_AUTO);
+	}
+
+	/*
+	 * If only the AUTO bit is set, the decision on which FEC
+	 * mode to use will be made by HW/FW or driver.
+	 */
+	if (auto_fec && fec_capa == 0)
+		return 0;
+
+	sfc_dev_get_rte_link(dev, 1, &current_link);
+
+	num_entries = sfc_fec_get_capa_speed_to_fec(supported_caps, NULL);
+	if (num_entries == 0)
+		return ENOTSUP;
+
+	speed_fec_capa = rte_calloc("fec_capa", num_entries,
+				    sizeof(*speed_fec_capa), 0);
+	num_entries = sfc_fec_get_capa_speed_to_fec(supported_caps,
+						    speed_fec_capa);
+
+	for (i = 0; i < num_entries; i++) {
+		if (speed_fec_capa[i].speed == current_link.link_speed) {
+			if ((fec_capa & speed_fec_capa[i].capa) != 0)
+				is_supported = true;
+
+			break;
+		}
+	}
+
+	rte_free(speed_fec_capa);
+
+	if (is_supported)
+		return 0;
+
+	return ENOTSUP;
+}
+
+static int
+sfc_fec_capa_to_efx(uint32_t supported_caps, uint32_t fec_capa,
+		    uint32_t *efx_fec_caps)
+{
+	bool fec_is_set = false;
+	bool auto_fec = false;
+	bool nofec = false;
+	uint32_t ret = 0;
+
+	if (efx_fec_caps == NULL)
+		return EINVAL;
+
+	if (fec_capa & RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_AUTO))
+		auto_fec = true;
+
+	if (fec_capa & RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_NOFEC))
+		nofec = true;
+
+	if (fec_capa == RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_AUTO)) {
+		ret |= (EFX_PHY_CAP_FEC_BIT(BASER_FEC) |
+			EFX_PHY_CAP_FEC_BIT(25G_BASER_FEC) |
+			EFX_PHY_CAP_FEC_BIT(RS_FEC)) & supported_caps;
+		goto done;
+	}
+
+	if (fec_capa & RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_RS)) {
+		fec_is_set = true;
+
+		if (supported_caps & EFX_PHY_CAP_FEC_BIT(RS_FEC)) {
+			ret |= EFX_PHY_CAP_FEC_BIT(RS_FEC) |
+			       EFX_PHY_CAP_FEC_BIT(RS_FEC_REQUESTED);
+		}
+	}
+	if (fec_capa & RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_BASER)) {
+		if (!auto_fec && fec_is_set)
+			return EINVAL;
+
+		if (supported_caps & EFX_PHY_CAP_FEC_BIT(BASER_FEC)) {
+			ret |= EFX_PHY_CAP_FEC_BIT(BASER_FEC) |
+			       EFX_PHY_CAP_FEC_BIT(BASER_FEC_REQUESTED);
+		}
+		if (supported_caps & EFX_PHY_CAP_FEC_BIT(25G_BASER_FEC)) {
+			ret |= EFX_PHY_CAP_FEC_BIT(25G_BASER_FEC) |
+			       EFX_PHY_CAP_FEC_BIT(25G_BASER_FEC_REQUESTED);
+		}
+	}
+
+	if (ret == 0 && !nofec)
+		return ENOTSUP;
+
+done:
+	*efx_fec_caps = ret;
+	return 0;
+}
+
+static int
+sfc_fec_set(struct rte_eth_dev *dev, uint32_t fec_capa)
+{
+	struct sfc_adapter *sa = sfc_adapter_by_eth_dev(dev);
+	struct sfc_port *port = &sa->port;
+	uint32_t supported_caps;
+	uint32_t efx_fec_caps;
+	uint32_t updated_caps;
+	int rc = 0;
+
+	sfc_adapter_lock(sa);
+
+	efx_phy_adv_cap_get(sa->nic, EFX_PHY_CAP_PERM, &supported_caps);
+
+	rc = sfc_fec_capa_check(dev, fec_capa, supported_caps);
+	if (rc != 0)
+		goto adapter_unlock;
+
+	rc = sfc_fec_capa_to_efx(supported_caps, fec_capa, &efx_fec_caps);
+	if (rc != 0)
+		goto adapter_unlock;
+
+	if (sa->state == SFC_ETHDEV_STARTED) {
+		efx_phy_adv_cap_get(sa->nic, EFX_PHY_CAP_CURRENT,
+				    &updated_caps);
+		updated_caps = updated_caps & ~EFX_PHY_CAP_FEC_MASK;
+		updated_caps |= efx_fec_caps;
+
+		rc = efx_phy_adv_cap_set(sa->nic, updated_caps);
+		if (rc != 0)
+			goto adapter_unlock;
+	}
+
+	port->fec_cfg = efx_fec_caps;
+	/*
+	 * There is no chance to recognize AUTO mode from the
+	 * saved FEC capabilities as AUTO mode can have the same
+	 * set of bits as any other mode from the EFX point of view.
+	 * Save it in the proper variable.
+	 */
+	if (fec_capa & RTE_ETH_FEC_MODE_TO_CAPA(RTE_ETH_FEC_AUTO))
+		port->fec_auto = true;
+	else
+		port->fec_auto = false;
+
+adapter_unlock:
+	sfc_adapter_unlock(sa);
+
+	SFC_ASSERT(rc >= 0);
+	return -rc;
+}
+
 static const struct eth_dev_ops sfc_eth_dev_ops = {
 	.dev_configure			= sfc_dev_configure,
 	.dev_start			= sfc_dev_start,
@@ -2392,6 +2767,9 @@ static const struct eth_dev_ops sfc_eth_dev_ops = {
 	.pool_ops_supported		= sfc_pool_ops_supported,
 	.representor_info_get		= sfc_representor_info_get,
 	.rx_metadata_negotiate		= sfc_rx_metadata_negotiate,
+	.fec_get_capability		= sfc_fec_get_capability,
+	.fec_get			= sfc_fec_get,
+	.fec_set			= sfc_fec_set,
 };
 
 struct sfc_ethdev_init_data {
diff --git a/drivers/net/sfc/sfc_port.c b/drivers/net/sfc/sfc_port.c
index 5f312ab1ba83..0c887ddedb09 100644
--- a/drivers/net/sfc/sfc_port.c
+++ b/drivers/net/sfc/sfc_port.c
@@ -228,17 +228,8 @@ sfc_port_start(struct sfc_adapter *sa)
 	/* Preserve pause capabilities set by above efx_mac_fcntl_set()  */
 	efx_phy_adv_cap_get(sa->nic, EFX_PHY_CAP_CURRENT, &phy_adv_cap);
 	SFC_ASSERT((port->phy_adv_cap & phy_pause_caps) == 0);
-	phy_adv_cap = port->phy_adv_cap | (phy_adv_cap & phy_pause_caps);
-
-	/*
-	 * No controls for FEC yet. Use default FEC mode.
-	 * I.e. advertise everything supported (*_FEC=1), but do not request
-	 * anything explicitly (*_FEC_REQUESTED=0).
-	 */
-	phy_adv_cap |= port->phy_adv_cap_mask &
-		(1u << EFX_PHY_CAP_BASER_FEC |
-		 1u << EFX_PHY_CAP_RS_FEC |
-		 1u << EFX_PHY_CAP_25G_BASER_FEC);
+	phy_adv_cap = port->phy_adv_cap | (phy_adv_cap & phy_pause_caps) |
+			port->fec_cfg;
 
 	sfc_log_init(sa, "set phy adv caps to %#x", phy_adv_cap);
 	rc = efx_phy_adv_cap_set(sa->nic, phy_adv_cap);
@@ -469,6 +460,17 @@ sfc_port_attach(struct sfc_adapter *sa)
 
 	port->mac_stats_update_period_ms = kvarg_stats_update_period_ms;
 
+	/*
+	 * Set default FEC mode.
+	 * I.e. advertise everything supported (*_FEC=1), but do not request
+	 * anything explicitly (*_FEC_REQUESTED=0).
+	 */
+	port->fec_cfg = port->phy_adv_cap_mask &
+		(EFX_PHY_CAP_FEC_BIT(BASER_FEC) |
+		 EFX_PHY_CAP_FEC_BIT(RS_FEC) |
+		 EFX_PHY_CAP_FEC_BIT(25G_BASER_FEC));
+	port->fec_auto = true;
+
 	sfc_log_init(sa, "done");
 	return 0;
 
-- 
2.37.0 (Apple Git-136)

Re: [PATCH v2 3/3] net/sfc: support FEC feature

[Andrew Rybchenko]

On 6/15/23 11:38, Denis Pryazhennikov wrote:
> Support ethdev methods to query and set FEC information.
> Limitations: ignoring rte_eth_fec_get_capability() results
> can lead to NOFEC if the device is not strated.
> 
> Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
> Reviewed-by: Ivan Malov <ivan.malov@arknetworks.am>
> Reviewed-by: Andy Moreton <amoreton@xilinx.com>
> Reviewed-by: Ferruh Yigit <ferruh.yigit@amd.com>

Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

> diff --git a/doc/guides/rel_notes/release_23_07.rst b/doc/guides/rel_notes/release_23_07.rst
> index 027ae7bd2d64..59a215a4a4f8 100644
> --- a/doc/guides/rel_notes/release_23_07.rst
> +++ b/doc/guides/rel_notes/release_23_07.rst
> @@ -170,6 +170,12 @@ New Features
>   
>     See :doc:`../prog_guide/pdcp_lib` for more information.
>   
> +* **Updated Solarflare network PMD.**
> +
> +  Updated the Solarflare ``sfc_efx`` driver with changes including:
> +
> +  * Added support for configuring FEC mode, querying FEC capabilities and
> +    current FEC mode from a device.

Extra empty line is required here. Two empty lines before the next
section.

>   
>   Removed Items
>   -------------

[snip]

Re: [PATCH 1/3] net/sfc: split link update function

[Ferruh Yigit]

On 6/2/2023 9:52 AM, Andrew Rybchenko wrote:
> On 6/2/23 01:23, Denis Pryazhennikov wrote:
>> Separate the original link update function into
>> two functions: state retrieval and update.
>> This improves code clarity and maintainability.
>>
>> Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
>> Reviewed-by: Ivan Malov <ivan.malov@arknetworks.am>
>> Reviewed-by: Andy Moreton <amoreton@xilinx.com>
> 
> Acked-by: Andrew Rybchenko <arybchenko@oktetlabs.ru>
> 
> 

Hi Andrew,

You have other email address [1] recorded in .mailmap, I will keep using
it, if this is new preferred address please update .mailmap

[1]
Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

Re: [PATCH v2 3/3] net/sfc: support FEC feature

[Ferruh Yigit]

On 6/19/2023 10:39 AM, Andrew Rybchenko wrote:
> On 6/15/23 11:38, Denis Pryazhennikov wrote:
>> Support ethdev methods to query and set FEC information.
>> Limitations: ignoring rte_eth_fec_get_capability() results
>> can lead to NOFEC if the device is not strated.
>>
>> Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
>> Reviewed-by: Ivan Malov <ivan.malov@arknetworks.am>
>> Reviewed-by: Andy Moreton <amoreton@xilinx.com>
>> Reviewed-by: Ferruh Yigit <ferruh.yigit@amd.com>
> 
> Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> 
>> diff --git a/doc/guides/rel_notes/release_23_07.rst
>> b/doc/guides/rel_notes/release_23_07.rst
>> index 027ae7bd2d64..59a215a4a4f8 100644
>> --- a/doc/guides/rel_notes/release_23_07.rst
>> +++ b/doc/guides/rel_notes/release_23_07.rst
>> @@ -170,6 +170,12 @@ New Features
>>       See :doc:`../prog_guide/pdcp_lib` for more information.
>>   +* **Updated Solarflare network PMD.**
>> +
>> +  Updated the Solarflare ``sfc_efx`` driver with changes including:
>> +
>> +  * Added support for configuring FEC mode, querying FEC capabilities
>> and
>> +    current FEC mode from a device.
> 
> Extra empty line is required here. Two empty lines before the next
> section.
> 

Location of the update is wrong, please see section comment on where to
place the update, I will fix while merging.

Re: [PATCH v2 2/3] common/sfc_efx/base: add FEC related macros

[Ferruh Yigit]

On 6/15/2023 9:38 AM, Denis Pryazhennikov wrote:
> Added new macros to simplify working with FEC bits.
> 
> Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
> Reviewed-by: Andy Moreton <amoreton@xilinx.com>
> Acked-by: Andrew Rybchenko <arybchenko@oktetlabs.ru>
> ---
>  drivers/common/sfc_efx/base/efx.h | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/common/sfc_efx/base/efx.h b/drivers/common/sfc_efx/base/efx.h
> index 49e29dcc1c69..10c412bcd7dc 100644
> --- a/drivers/common/sfc_efx/base/efx.h
> +++ b/drivers/common/sfc_efx/base/efx.h
> @@ -3958,6 +3958,15 @@ typedef enum efx_phy_fec_type_e {
>  	EFX_PHY_FEC_RS
>  } efx_phy_fec_type_t;
>  
> +#define EFX_PHY_CAP_FEC_BIT(_fec_bit) (1U << EFX_PHY_CAP_##_fec_bit)
> +#define EFX_PHY_CAP_FEC_MASK \
> +	(EFX_PHY_CAP_FEC_BIT(BASER_FEC) |		\
> +	 EFX_PHY_CAP_FEC_BIT(25G_BASER_FEC) |		\
> +	 EFX_PHY_CAP_FEC_BIT(BASER_FEC_REQUESTED) |	\
> +	 EFX_PHY_CAP_FEC_BIT(25G_BASER_FEC_REQUESTED) |	\
> +	 EFX_PHY_CAP_FEC_BIT(RS_FEC) |			\
> +	 EFX_PHY_CAP_FEC_BIT(RS_FEC_REQUESTED))
> +
>  LIBEFX_API
>  extern	__checkReturn	efx_rc_t
>  efx_phy_fec_type_get(

will squash to commit 3/3 where actual functionality is added and these
macros are used.

Re: [PATCH 1/3] net/sfc: split link update function

[Andrew Rybchenko]

On 6/20/23 14:25, Ferruh Yigit wrote:
> On 6/2/2023 9:52 AM, Andrew Rybchenko wrote:
>> On 6/2/23 01:23, Denis Pryazhennikov wrote:
>>> Separate the original link update function into
>>> two functions: state retrieval and update.
>>> This improves code clarity and maintainability.
>>>
>>> Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
>>> Reviewed-by: Ivan Malov <ivan.malov@arknetworks.am>
>>> Reviewed-by: Andy Moreton <amoreton@xilinx.com>
>>
>> Acked-by: Andrew Rybchenko <arybchenko@oktetlabs.ru>
>>
>>
> 
> Hi Andrew,
> 
> You have other email address [1] recorded in .mailmap, I will keep using
> it, if this is new preferred address please update .mailmap
> 
> [1]
> Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> 

Thanks Ferruh, it was some kind of typo. [1] is the right one.

Re: [PATCH v2 0/3] net/sfc: support FEC feature

[Ferruh Yigit]

On 6/15/2023 9:38 AM, Denis Pryazhennikov wrote:
> This patch series adds setting and querying of Forward error
> correction (FEC). AUTO, BASER and RS modes are supported
> by EF10/EF100 hardware. LLRS mode is not supported.
> 
> Changes in v2:
> * Applied review notes in [2/3] and [3/3];
> * Added a check for the supported by a device FEC modes
>   in sfc_fec_get_capa_speed_to_fec();
> * Fixed error handling.
> 
> Denis Pryazhennikov (3):
>   net/sfc: split link update function
>   common/sfc_efx/base: add FEC related macros
>   net/sfc: support FEC feature
>

2/3 squashed into 3/3,

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