diff mbox series

[3/4] common/sfc_efx/base: add support for configure MAC to keep FCS

Message ID 20230601114220.17796-4-denis.pryazhennikov@arknetworks.am (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series net/sfc: support KEEP_CRC offload |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Denis Pryazhennikov June 1, 2023, 11:42 a.m. UTC 
  Drivers cannot determine if received packet includes the FCS or not.
Only packets with an external port have the FCS included and functions
without link control privilege cannot determine the MAC configuration.
This patch is trying to make assumptions that: if PF is the only function
(there are no VFs or additional PFs); it can set the MAC configuration and
it never expects packets it sends to be looped back then it can assume that
changed the MAC configuration to include the FCS is safe and that all
received packets will include their FCS.

Signed-off-by: Roman Zhukov <roman.zhukov@arknetworks.am>
Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
Reviewed-by: Andy Moreton <amoreton@xilinx.com>
---
 drivers/common/sfc_efx/base/ef10_mac.c |  5 +--
 drivers/common/sfc_efx/base/efx.h      |  5 +++
 drivers/common/sfc_efx/base/efx_impl.h |  1 +
 drivers/common/sfc_efx/base/efx_mac.c  | 48 ++++++++++++++++++++++++++
 drivers/common/sfc_efx/version.map     |  1 +
 5 files changed, 58 insertions(+), 2 deletions(-)

Comments

Andrew Rybchenko June 7, 2023, 8:28 a.m. UTC | #1 
On 6/1/23 14:42, Denis Pryazhennikov wrote:
> Drivers cannot determine if received packet includes the FCS or not.
> Only packets with an external port have the FCS included and functions
> without link control privilege cannot determine the MAC configuration.
> This patch is trying to make assumptions that: if PF is the only function
> (there are no VFs or additional PFs); it can set the MAC configuration and
> it never expects packets it sends to be looped back then it can assume that
> changed the MAC configuration to include the FCS is safe and that all
> received packets will include their FCS.
> 
> Signed-off-by: Roman Zhukov <roman.zhukov@arknetworks.am>
> Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
> Reviewed-by: Andy Moreton <amoreton@xilinx.com>

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

Please, ensure that initial author of the patch is used in From.
diff mbox series

Patch

diff --git a/drivers/common/sfc_efx/base/ef10_mac.c b/drivers/common/sfc_efx/base/ef10_mac.c
index 28228a9fb784..bfc82b80c7e5 100644
--- a/drivers/common/sfc_efx/base/ef10_mac.c
+++ b/drivers/common/sfc_efx/base/ef10_mac.c
@@ -307,9 +307,10 @@  ef10_mac_reconfigure(
 	 */
 	MCDI_IN_SET_DWORD(req, SET_MAC_IN_FCNTL, MC_CMD_FCNTL_AUTO);
 
-	/* Do not include the Ethernet frame checksum in RX packets */
+	/* Include the Ethernet frame checksum in RX packets if it's required */
 	MCDI_IN_POPULATE_DWORD_1(req, SET_MAC_IN_FLAGS,
-				    SET_MAC_IN_FLAG_INCLUDE_FCS, 0);
+				    SET_MAC_IN_FLAG_INCLUDE_FCS,
+				    epp->ep_include_fcs ? 1 : 0);
 
 	efx_mcdi_execute_quiet(enp, &req);
 
diff --git a/drivers/common/sfc_efx/base/efx.h b/drivers/common/sfc_efx/base/efx.h
index ff281167767d..b52baaa7a452 100644
--- a/drivers/common/sfc_efx/base/efx.h
+++ b/drivers/common/sfc_efx/base/efx.h
@@ -747,6 +747,11 @@  efx_mac_fcntl_get(
 	__out		unsigned int *fcntl_wantedp,
 	__out		unsigned int *fcntl_linkp);
 
+LIBEFX_API
+extern	__checkReturn	efx_rc_t
+efx_mac_include_fcs_set(
+	__in efx_nic_t *enp,
+	__in boolean_t enabled);
 
 #if EFSYS_OPT_MAC_STATS
 
diff --git a/drivers/common/sfc_efx/base/efx_impl.h b/drivers/common/sfc_efx/base/efx_impl.h
index 45e99d01c58f..b6461c14399e 100644
--- a/drivers/common/sfc_efx/base/efx_impl.h
+++ b/drivers/common/sfc_efx/base/efx_impl.h
@@ -363,6 +363,7 @@  typedef struct efx_port_s {
 	uint32_t		ep_default_adv_cap_mask;
 	uint32_t		ep_phy_cap_mask;
 	boolean_t		ep_mac_drain;
+	boolean_t		ep_include_fcs;
 #if EFSYS_OPT_BIST
 	efx_bist_type_t		ep_current_bist;
 #endif
diff --git a/drivers/common/sfc_efx/base/efx_mac.c b/drivers/common/sfc_efx/base/efx_mac.c
index c51e86b52c29..13cac5a75130 100644
--- a/drivers/common/sfc_efx/base/efx_mac.c
+++ b/drivers/common/sfc_efx/base/efx_mac.c
@@ -527,6 +527,54 @@  efx_mac_filter_default_rxq_clear(
 		emop->emo_filter_default_rxq_clear(enp);
 }
 
+	__checkReturn	efx_rc_t
+efx_mac_include_fcs_set(
+	__in		efx_nic_t *enp,
+	__in		boolean_t enabled)
+{
+	efx_port_t *epp = &(enp->en_port);
+	efx_nic_cfg_t *encp = &(enp->en_nic_cfg);
+	const efx_mac_ops_t *emop = epp->ep_emop;
+	efx_rc_t rc;
+
+	EFSYS_ASSERT3U(enp->en_magic, ==, EFX_NIC_MAGIC);
+	EFSYS_ASSERT3U(enp->en_mod_flags, &, EFX_MOD_PORT);
+	EFSYS_ASSERT(emop != NULL);
+
+	if (enabled && !encp->enc_rx_include_fcs_supported) {
+		rc = ENOTSUP;
+		goto fail1;
+	}
+
+	/*
+	 * Enabling 'include FCS' changes link control state and affects
+	 * behaviour for all PCI functions on the port, so to avoid this it
+	 * can be enabled only if the PCI function is exclusive port user
+	 */
+	if (enabled && encp->enc_port_usage != EFX_PORT_USAGE_EXCLUSIVE) {
+		rc = EACCES;
+		goto fail2;
+	}
+
+	if (epp->ep_include_fcs != enabled) {
+		epp->ep_include_fcs = enabled;
+
+		rc = emop->emo_reconfigure(enp);
+		if (rc != 0)
+			goto fail3;
+	}
+
+	return 0;
+
+fail3:
+	EFSYS_PROBE(fail3);
+fail2:
+	EFSYS_PROBE(fail2);
+fail1:
+	EFSYS_PROBE1(fail1, efx_rc_t, rc);
+
+	return rc;
+}
 
 #if EFSYS_OPT_MAC_STATS
 
diff --git a/drivers/common/sfc_efx/version.map b/drivers/common/sfc_efx/version.map
index d9b04a611d25..a54dba81f578 100644
--- a/drivers/common/sfc_efx/version.map
+++ b/drivers/common/sfc_efx/version.map
@@ -71,6 +71,7 @@  INTERNAL {
 	efx_mac_drain;
 	efx_mac_fcntl_get;
 	efx_mac_fcntl_set;
+	efx_mac_include_fcs_set;
 	efx_mac_filter_default_rxq_clear;
 	efx_mac_filter_default_rxq_set;
 	efx_mac_filter_get_all_ucast_mcast;