[PATCH 0/4] net/sfc: support KEEP

DPDK patches and discussions
 help / color / mirror / Atom feed

* [PATCH 0/4] net/sfc: support KEEP_CRC offload
@ 2023-06-01 11:42 Denis Pryazhennikov
  2023-06-01 11:42 ` [PATCH 1/4] common/sfc_efx/base: NIC Partitioning mode discovery using heuristic approach Denis Pryazhennikov
                   ` (7 more replies)
  0 siblings, 8 replies; 28+ messages in thread
From: Denis Pryazhennikov @ 2023-06-01 11:42 UTC (permalink / raw)
  To: dev; +Cc: Viacheslav Galaktionov, Ferruh Yigit, Andrew Rybchenko

This patch series adds support for RTE_ETH_RX_OFFLOAD_KEEP_CRC
offload in the SFC driver.

Denis Pryazhennikov (3):
  common/sfc_efx/base: detect and report FCS include support
  common/sfc_efx/base: add support for configure MAC to keep FCS
  net/sfc: add configurable Rx CRC stripping

Sandilya Bhagi (1):
  common/sfc_efx/base: NIC Partitioning mode discovery using heuristic
    approach

 doc/guides/nics/features/sfc.ini        |   1 +
 doc/guides/nics/sfc_efx.rst             |   6 +-
 drivers/common/sfc_efx/base/ef10_mac.c  |   5 +-
 drivers/common/sfc_efx/base/ef10_nic.c  | 115 ++++++++++++++++++++++++
 drivers/common/sfc_efx/base/efx.h       |  14 +++
 drivers/common/sfc_efx/base/efx_impl.h  |   1 +
 drivers/common/sfc_efx/base/efx_mac.c   |  48 ++++++++++
 drivers/common/sfc_efx/base/siena_nic.c |   1 +
 drivers/common/sfc_efx/version.map      |   1 +
 drivers/net/sfc/sfc.h                   |   1 +
 drivers/net/sfc/sfc_ef10_rx.c           |   3 +-
 drivers/net/sfc/sfc_port.c              |  12 +++
 drivers/net/sfc/sfc_rx.c                |   6 +-
 13 files changed, 208 insertions(+), 6 deletions(-)

-- 
2.37.0 (Apple Git-136)


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 1/4] common/sfc_efx/base: NIC Partitioning mode discovery using heuristic approach
  2023-06-01 11:42 [PATCH 0/4] net/sfc: support KEEP_CRC offload Denis Pryazhennikov
@ 2023-06-01 11:42 ` Denis Pryazhennikov
  2023-06-02  9:43   ` Andrew Rybchenko
  2023-06-01 11:42 ` [PATCH 2/4] common/sfc_efx/base: detect and report FCS include support Denis Pryazhennikov
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 28+ messages in thread
From: Denis Pryazhennikov @ 2023-06-01 11:42 UTC (permalink / raw)
  To: dev
  Cc: Viacheslav Galaktionov, Ferruh Yigit, Andrew Rybchenko,
	Sandilya Bhagi, Andy Moreton

NIC Partitioning mode in SFN devices means multiple PFs per network port.
When NIC Partitioning is configured, apart from the privileged adapter(s) the
other unprivileged adapter(s) will share the same physical port.
Determining NIC Partitioning mode is required to take necessary action(s) for
unprivileged adapter to work seamlessly.
NIC Partitioning is determined using heuristic approach - If the physical ports
are shared between PFs then either NIC Partitioning or SR-IOV is in use.
When NIC Partitioning is in use MAX MTU workaround should be applied so that
the unprivileged functions can seamlessly configure any valid MTU.

hg-changeset: 7f0abee725a8e9c6524e773e5e5d6286a3b027a4

Signed-off-by: Sandilya Bhagi <sbhagi@solarflare.com>
Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
Reviewed-by: Andy Moreton <amoreton@xilinx.com>
---
 drivers/common/sfc_efx/base/ef10_nic.c | 109 +++++++++++++++++++++++++
 drivers/common/sfc_efx/base/efx.h      |   8 ++
 2 files changed, 117 insertions(+)

diff --git a/drivers/common/sfc_efx/base/ef10_nic.c b/drivers/common/sfc_efx/base/ef10_nic.c
index e1709d120093..db4834a65175 100644
--- a/drivers/common/sfc_efx/base/ef10_nic.c
+++ b/drivers/common/sfc_efx/base/ef10_nic.c
@@ -1044,6 +1044,89 @@ ef10_mcdi_get_pf_count(
 	return (rc);
 }
 
+static	__checkReturn	efx_rc_t
+ef10_nic_get_physical_port_usage(
+	__in		efx_nic_t *enp,
+	__in_ecount(pfs_to_ports_size)	uint8_t *pfs_to_ports,
+	__in		size_t pfs_to_ports_size,
+	__out		efx_port_usage_t *port_usagep)
+{
+	efx_nic_cfg_t *encp = &(enp->en_nic_cfg);
+	efx_port_usage_t port_usage;
+	size_t pf;
+	uint8_t phy_port;
+	efx_rc_t rc;
+
+	/*
+	 * The sharing of physical ports between functions are determined
+	 * in the following way.
+	 * 1. If VFs are enabled then the physical port is shared.
+	 * 2. Retrieve PFs to ports assignment.
+	 * 3. If PF 0 assignment cannot be retrieved(ACCESS_DENIED), it
+	 *    implies this is an unprivileged function. An unprivileged
+	 *    function indicates the physical port must be shared with
+	 *    another privileged function.
+	 * 4. If PF 0 assignment can be retrieved, it indicates this
+	 *    function is privileged. Now, read all other PF's physical
+	 *    port number assignment and check if the current PF's physical
+	 *    port is shared with any other PF's physical port.
+	 * NOTE: Sharing of physical ports (using heuristic approach) can
+	 * imply either NIC Partitioning or SR-IOV is in use. This info is
+	 * sufficient to apply the max MTU workaround (WIN-628), but should
+	 * not be used for other purposes.
+	 * NOTE: PF 0 is always privileged function.
+	 */
+
+	if (EFX_PCI_FUNCTION_IS_VF(encp)) {
+		port_usage = EFX_PORT_USAGE_SHARED;
+		goto out;
+	}
+
+	if (pfs_to_ports[0] ==
+	            MC_CMD_GET_CAPABILITIES_V2_OUT_ACCESS_NOT_PERMITTED) {
+		/*
+		 * This is unprivileged function as it do not have sufficient
+		 * privileges to read the value, this implies the physical port
+		 * is shared between this function and another privileged
+		 * function
+		 */
+		port_usage = EFX_PORT_USAGE_SHARED;
+		goto out;
+	}
+
+	if (encp->enc_pf >= pfs_to_ports_size) {
+		rc = EINVAL;
+		goto fail1;
+	}
+	phy_port = pfs_to_ports[encp->enc_pf];
+
+	/*
+	 * This is privileged function as it is able read the value of
+	 * PF 0. Now, check if any other function share the same physical
+	 * port number as this function.
+	 */
+	for (pf = 0; pf < pfs_to_ports_size; pf++) {
+
+		if ((encp->enc_pf != pf) &&
+		    (phy_port == pfs_to_ports[pf])) {
+			/* Found match, PFs share the same physical port */
+			port_usage = EFX_PORT_USAGE_SHARED;
+			goto out;
+		}
+	}
+
+	port_usage = EFX_PORT_USAGE_EXCLUSIVE;
+
+out:
+	*port_usagep = port_usage;
+	return (0);
+
+fail1:
+	EFSYS_PROBE1(fail1, efx_rc_t, rc);
+
+	return (rc);
+}
+
 static	__checkReturn	efx_rc_t
 ef10_get_datapath_caps(
 	__in		efx_nic_t *enp)
@@ -1307,6 +1390,32 @@ ef10_get_datapath_caps(
 		encp->enc_tunnel_config_udp_entries_max = 0;
 	}
 
+#define CAP_PFS_TO_PORTS(_n)						\
+	(MC_CMD_GET_CAPABILITIES_V2_OUT_PFS_TO_PORTS_ASSIGNMENT_ ## _n)
+
+	encp->enc_port_usage = EFX_PORT_USAGE_UNKNOWN;
+
+	if (req.emr_out_length_used >= MC_CMD_GET_CAPABILITIES_V2_OUT_LEN) {
+		/* PFs to ports assignment */
+		uint8_t pfs_to_ports[CAP_PFS_TO_PORTS(NUM)];
+		efx_byte_t *bytep;
+		int i;
+
+		bytep = MCDI_OUT(req, efx_byte_t, CAP_PFS_TO_PORTS(OFST));
+		for (i = 0; i < EFX_ARRAY_SIZE(pfs_to_ports); i++) {
+			pfs_to_ports[i] = EFX_BYTE_FIELD(*bytep, EFX_BYTE_0);
+			bytep += CAP_PFS_TO_PORTS(LEN);
+		}
+
+		if (ef10_nic_get_physical_port_usage(enp,
+		    pfs_to_ports, EFX_ARRAY_SIZE(pfs_to_ports),
+		    &encp->enc_port_usage) != 0) {
+			/* PF to port mapping lookup failed */
+			encp->enc_port_usage = EFX_PORT_USAGE_UNKNOWN;
+		}
+	}
+#undef  CAP_PFS_TO_PORTS
+
 	/*
 	 * Check if firmware reports the VI window mode.
 	 * Medford2 has a variable VI window size (8K, 16K or 64K).
diff --git a/drivers/common/sfc_efx/base/efx.h b/drivers/common/sfc_efx/base/efx.h
index 49e29dcc1c69..93bb4916bfd6 100644
--- a/drivers/common/sfc_efx/base/efx.h
+++ b/drivers/common/sfc_efx/base/efx.h
@@ -311,6 +311,12 @@ efx_nic_check_pcie_link_speed(
 	__in		uint32_t pcie_link_gen,
 	__out		efx_pcie_link_performance_t *resultp);
 
+typedef enum efx_port_usage_e {
+	EFX_PORT_USAGE_UNKNOWN = 0,
+	EFX_PORT_USAGE_EXCLUSIVE,	/* Port only used by this PF */
+	EFX_PORT_USAGE_SHARED,		/* Port shared with other PFs */
+} efx_port_usage_t;
+
 #define	EFX_MAC_ADDR_LEN 6
 
 #if EFSYS_OPT_MCDI
@@ -1680,6 +1686,8 @@ typedef struct efx_nic_cfg_s {
 	uint32_t		enc_assigned_port;
 	/* NIC DMA mapping type */
 	efx_nic_dma_mapping_t	enc_dma_mapping;
+	/* Physical ports shared by PFs */
+	efx_port_usage_t	enc_port_usage;
 } efx_nic_cfg_t;
 
 #define	EFX_PCI_VF_INVALID 0xffff
-- 
2.37.0 (Apple Git-136)


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/4] common/sfc_efx/base: NIC Partitioning mode discovery using heuristic approach
  2023-06-01 11:42 ` [PATCH 1/4] common/sfc_efx/base: NIC Partitioning mode discovery using heuristic approach Denis Pryazhennikov
@ 2023-06-02  9:43   ` Andrew Rybchenko
  0 siblings, 0 replies; 28+ messages in thread
From: Andrew Rybchenko @ 2023-06-02  9:43 UTC (permalink / raw)
  To: Denis Pryazhennikov, dev
  Cc: Viacheslav Galaktionov, Ferruh Yigit, Sandilya Bhagi, Andy Moreton

Summary must not be a statement. Also summary is too long.

On 6/1/23 14:42, Denis Pryazhennikov wrote:
> NIC Partitioning mode in SFN devices means multiple PFs per network port.
> When NIC Partitioning is configured, apart from the privileged adapter(s) the
> other unprivileged adapter(s) will share the same physical port.
> Determining NIC Partitioning mode is required to take necessary action(s) for
> unprivileged adapter to work seamlessly.
> NIC Partitioning is determined using heuristic approach - If the physical ports
> are shared between PFs then either NIC Partitioning or SR-IOV is in use.
> When NIC Partitioning is in use MAX MTU workaround should be applied so that
> the unprivileged functions can seamlessly configure any valid MTU.
> 
> hg-changeset: 7f0abee725a8e9c6524e773e5e5d6286a3b027a4

I'm not sure that it is appropriate here.

> 
> Signed-off-by: Sandilya Bhagi <sbhagi@solarflare.com>

I'd like to understand how is the first author of the patch.
I guess the first signed-off-by. If so, it should be in From
as well.

> Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
> Reviewed-by: Andy Moreton <amoreton@xilinx.com>
> ---
>   drivers/common/sfc_efx/base/ef10_nic.c | 109 +++++++++++++++++++++++++
>   drivers/common/sfc_efx/base/efx.h      |   8 ++
>   2 files changed, 117 insertions(+)
> 
> diff --git a/drivers/common/sfc_efx/base/ef10_nic.c b/drivers/common/sfc_efx/base/ef10_nic.c
> index e1709d120093..db4834a65175 100644
> --- a/drivers/common/sfc_efx/base/ef10_nic.c
> +++ b/drivers/common/sfc_efx/base/ef10_nic.c
> @@ -1044,6 +1044,89 @@ ef10_mcdi_get_pf_count(
>   	return (rc);
>   }
>   
> +static	__checkReturn	efx_rc_t
> +ef10_nic_get_physical_port_usage(
> +	__in		efx_nic_t *enp,
> +	__in_ecount(pfs_to_ports_size)	uint8_t *pfs_to_ports,
> +	__in		size_t pfs_to_ports_size,
> +	__out		efx_port_usage_t *port_usagep)
> +{
> +	efx_nic_cfg_t *encp = &(enp->en_nic_cfg);
> +	efx_port_usage_t port_usage;
> +	size_t pf;
> +	uint8_t phy_port;
> +	efx_rc_t rc;
> +
> +	/*
> +	 * The sharing of physical ports between functions are determined
> +	 * in the following way.
> +	 * 1. If VFs are enabled then the physical port is shared.
> +	 * 2. Retrieve PFs to ports assignment.
> +	 * 3. If PF 0 assignment cannot be retrieved(ACCESS_DENIED), it
> +	 *    implies this is an unprivileged function. An unprivileged
> +	 *    function indicates the physical port must be shared with
> +	 *    another privileged function.
> +	 * 4. If PF 0 assignment can be retrieved, it indicates this
> +	 *    function is privileged. Now, read all other PF's physical
> +	 *    port number assignment and check if the current PF's physical
> +	 *    port is shared with any other PF's physical port.
> +	 * NOTE: Sharing of physical ports (using heuristic approach) can
> +	 * imply either NIC Partitioning or SR-IOV is in use. This info is
> +	 * sufficient to apply the max MTU workaround (WIN-628), but should
> +	 * not be used for other purposes.

I guess you're going to use it for other purpose.

> +	 * NOTE: PF 0 is always privileged function.
> +	 */
> +
> +	if (EFX_PCI_FUNCTION_IS_VF(encp)) {
> +		port_usage = EFX_PORT_USAGE_SHARED;
> +		goto out;
> +	}
> +
> +	if (pfs_to_ports[0] ==
> +	            MC_CMD_GET_CAPABILITIES_V2_OUT_ACCESS_NOT_PERMITTED) {
> +		/*
> +		 * This is unprivileged function as it do not have sufficient
> +		 * privileges to read the value, this implies the physical port
> +		 * is shared between this function and another privileged
> +		 * function
> +		 */
> +		port_usage = EFX_PORT_USAGE_SHARED;
> +		goto out;
> +	}
> +
> +	if (encp->enc_pf >= pfs_to_ports_size) {
> +		rc = EINVAL;
> +		goto fail1;
> +	}
> +	phy_port = pfs_to_ports[encp->enc_pf];
> +
> +	/*
> +	 * This is privileged function as it is able read the value of
> +	 * PF 0. Now, check if any other function share the same physical
> +	 * port number as this function.
> +	 */
> +	for (pf = 0; pf < pfs_to_ports_size; pf++) {
> +
> +		if ((encp->enc_pf != pf) &&
> +		    (phy_port == pfs_to_ports[pf])) {
> +			/* Found match, PFs share the same physical port */
> +			port_usage = EFX_PORT_USAGE_SHARED;
> +			goto out;
> +		}
> +	}
> +
> +	port_usage = EFX_PORT_USAGE_EXCLUSIVE;
> +
> +out:
> +	*port_usagep = port_usage;
> +	return (0);
> +
> +fail1:
> +	EFSYS_PROBE1(fail1, efx_rc_t, rc);
> +
> +	return (rc);
> +}
> +
>   static	__checkReturn	efx_rc_t
>   ef10_get_datapath_caps(
>   	__in		efx_nic_t *enp)
> @@ -1307,6 +1390,32 @@ ef10_get_datapath_caps(
>   		encp->enc_tunnel_config_udp_entries_max = 0;
>   	}
>   
> +#define CAP_PFS_TO_PORTS(_n)						\
> +	(MC_CMD_GET_CAPABILITIES_V2_OUT_PFS_TO_PORTS_ASSIGNMENT_ ## _n)
> +
> +	encp->enc_port_usage = EFX_PORT_USAGE_UNKNOWN;
> +
> +	if (req.emr_out_length_used >= MC_CMD_GET_CAPABILITIES_V2_OUT_LEN) {
> +		/* PFs to ports assignment */
> +		uint8_t pfs_to_ports[CAP_PFS_TO_PORTS(NUM)];
> +		efx_byte_t *bytep;
> +		int i;
> +
> +		bytep = MCDI_OUT(req, efx_byte_t, CAP_PFS_TO_PORTS(OFST));
> +		for (i = 0; i < EFX_ARRAY_SIZE(pfs_to_ports); i++) {
> +			pfs_to_ports[i] = EFX_BYTE_FIELD(*bytep, EFX_BYTE_0);
> +			bytep += CAP_PFS_TO_PORTS(LEN);
> +		}

Sorry, but it looks like memcpy() byte-by-byte.

> +
> +		if (ef10_nic_get_physical_port_usage(enp,
> +		    pfs_to_ports, EFX_ARRAY_SIZE(pfs_to_ports),
> +		    &encp->enc_port_usage) != 0) {

Alignment is misleading above and hard to read. Either correct
an alignment or simply call the function before if.

> +			/* PF to port mapping lookup failed */
> +			encp->enc_port_usage = EFX_PORT_USAGE_UNKNOWN;
> +		}
> +	}
> +#undef  CAP_PFS_TO_PORTS
> +
>   	/*
>   	 * Check if firmware reports the VI window mode.
>   	 * Medford2 has a variable VI window size (8K, 16K or 64K).
> diff --git a/drivers/common/sfc_efx/base/efx.h b/drivers/common/sfc_efx/base/efx.h
> index 49e29dcc1c69..93bb4916bfd6 100644
> --- a/drivers/common/sfc_efx/base/efx.h
> +++ b/drivers/common/sfc_efx/base/efx.h
> @@ -311,6 +311,12 @@ efx_nic_check_pcie_link_speed(
>   	__in		uint32_t pcie_link_gen,
>   	__out		efx_pcie_link_performance_t *resultp);
>   
> +typedef enum efx_port_usage_e {
> +	EFX_PORT_USAGE_UNKNOWN = 0,
> +	EFX_PORT_USAGE_EXCLUSIVE,	/* Port only used by this PF */
> +	EFX_PORT_USAGE_SHARED,		/* Port shared with other PFs */
> +} efx_port_usage_t;
> +
>   #define	EFX_MAC_ADDR_LEN 6
>   
>   #if EFSYS_OPT_MCDI
> @@ -1680,6 +1686,8 @@ typedef struct efx_nic_cfg_s {
>   	uint32_t		enc_assigned_port;
>   	/* NIC DMA mapping type */
>   	efx_nic_dma_mapping_t	enc_dma_mapping;
> +	/* Physical ports shared by PFs */
> +	efx_port_usage_t	enc_port_usage;
>   } efx_nic_cfg_t;
>   
>   #define	EFX_PCI_VF_INVALID 0xffff


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 2/4] common/sfc_efx/base: detect and report FCS include support
  2023-06-01 11:42 [PATCH 0/4] net/sfc: support KEEP_CRC offload Denis Pryazhennikov
  2023-06-01 11:42 ` [PATCH 1/4] common/sfc_efx/base: NIC Partitioning mode discovery using heuristic approach Denis Pryazhennikov
@ 2023-06-01 11:42 ` Denis Pryazhennikov
  2023-06-07  8:27   ` Andrew Rybchenko
  2023-06-01 11:42 ` [PATCH 3/4] common/sfc_efx/base: add support for configure MAC to keep FCS Denis Pryazhennikov
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 28+ messages in thread
From: Denis Pryazhennikov @ 2023-06-01 11:42 UTC (permalink / raw)
  To: dev; +Cc: Viacheslav Galaktionov, Ferruh Yigit, Andrew Rybchenko, Roman Zhukov

A new variable was added to efx_nic_cfg_s to detect and
report if FCS is supported by FW.

Signed-off-by: Roman Zhukov <roman.zhukov@arknetworks.am>
Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
Reviewed-by: Viacheslav Galaktionov <viacheslav.galaktionov@arknetworks.am>
---
 drivers/common/sfc_efx/base/ef10_nic.c  | 6 ++++++
 drivers/common/sfc_efx/base/efx.h       | 1 +
 drivers/common/sfc_efx/base/siena_nic.c | 1 +
 3 files changed, 8 insertions(+)

diff --git a/drivers/common/sfc_efx/base/ef10_nic.c b/drivers/common/sfc_efx/base/ef10_nic.c
index db4834a65175..d5b19af8e811 100644
--- a/drivers/common/sfc_efx/base/ef10_nic.c
+++ b/drivers/common/sfc_efx/base/ef10_nic.c
@@ -1250,6 +1250,12 @@ ef10_get_datapath_caps(
 	/* No limit on maximum number of Rx scatter elements per packet. */
 	encp->enc_rx_scatter_max = -1;
 
+	/* Check if the firmware supports include FCS on RX */
+	if (CAP_FLAGS1(req, RX_INCLUDE_FCS))
+		encp->enc_rx_include_fcs_supported = B_TRUE;
+	else
+		encp->enc_rx_include_fcs_supported = B_FALSE;
+
 	/* Check if the firmware supports packed stream mode */
 	if (CAP_FLAGS1(req, RX_PACKED_STREAM))
 		encp->enc_rx_packed_stream_supported = B_TRUE;
diff --git a/drivers/common/sfc_efx/base/efx.h b/drivers/common/sfc_efx/base/efx.h
index 93bb4916bfd6..ff281167767d 100644
--- a/drivers/common/sfc_efx/base/efx.h
+++ b/drivers/common/sfc_efx/base/efx.h
@@ -1629,6 +1629,7 @@ typedef struct efx_nic_cfg_s {
 	/* Datapath firmware vport reconfigure support */
 	boolean_t		enc_vport_reconfigure_supported;
 	boolean_t		enc_rx_disable_scatter_supported;
+	boolean_t		enc_rx_include_fcs_supported;
 	/* Maximum number of Rx scatter segments supported by HW */
 	uint32_t		enc_rx_scatter_max;
 	boolean_t		enc_allow_set_mac_with_installed_filters;
diff --git a/drivers/common/sfc_efx/base/siena_nic.c b/drivers/common/sfc_efx/base/siena_nic.c
index 9f14faf27168..c0316676eba2 100644
--- a/drivers/common/sfc_efx/base/siena_nic.c
+++ b/drivers/common/sfc_efx/base/siena_nic.c
@@ -187,6 +187,7 @@ siena_board_cfg(
 	encp->enc_allow_set_mac_with_installed_filters = B_TRUE;
 	encp->enc_rx_packed_stream_supported = B_FALSE;
 	encp->enc_rx_var_packed_stream_supported = B_FALSE;
+	encp->enc_rx_include_fcs_supported = B_FALSE;
 	encp->enc_rx_es_super_buffer_supported = B_FALSE;
 	encp->enc_fw_subvariant_no_tx_csum_supported = B_FALSE;
 
-- 
2.37.0 (Apple Git-136)


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 2/4] common/sfc_efx/base: detect and report FCS include support
  2023-06-01 11:42 ` [PATCH 2/4] common/sfc_efx/base: detect and report FCS include support Denis Pryazhennikov
@ 2023-06-07  8:27   ` Andrew Rybchenko
  0 siblings, 0 replies; 28+ messages in thread
From: Andrew Rybchenko @ 2023-06-07  8:27 UTC (permalink / raw)
  To: Denis Pryazhennikov, dev
  Cc: Viacheslav Galaktionov, Ferruh Yigit, Roman Zhukov

On 6/1/23 14:42, Denis Pryazhennikov wrote:
> A new variable was added to efx_nic_cfg_s to detect and
> report if FCS is supported by FW.
> 
> Signed-off-by: Roman Zhukov <roman.zhukov@arknetworks.am>
> Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
> Reviewed-by: Viacheslav Galaktionov <viacheslav.galaktionov@arknetworks.am
Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

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


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 3/4] common/sfc_efx/base: add support for configure MAC to keep FCS
  2023-06-01 11:42 [PATCH 0/4] net/sfc: support KEEP_CRC offload Denis Pryazhennikov
  2023-06-01 11:42 ` [PATCH 1/4] common/sfc_efx/base: NIC Partitioning mode discovery using heuristic approach Denis Pryazhennikov
  2023-06-01 11:42 ` [PATCH 2/4] common/sfc_efx/base: detect and report FCS include support Denis Pryazhennikov
@ 2023-06-01 11:42 ` Denis Pryazhennikov
  2023-06-07  8:28   ` Andrew Rybchenko
  2023-06-01 11:42 ` [PATCH 4/4] net/sfc: add configurable Rx CRC stripping Denis Pryazhennikov
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 28+ messages in thread
From: Denis Pryazhennikov @ 2023-06-01 11:42 UTC (permalink / raw)
  To: dev
  Cc: Viacheslav Galaktionov, Ferruh Yigit, Andrew Rybchenko,
	Roman Zhukov, Andy Moreton

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(-)

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;
-- 
2.37.0 (Apple Git-136)


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 3/4] common/sfc_efx/base: add support for configure MAC to keep FCS
  2023-06-01 11:42 ` [PATCH 3/4] common/sfc_efx/base: add support for configure MAC to keep FCS Denis Pryazhennikov
@ 2023-06-07  8:28   ` Andrew Rybchenko
  0 siblings, 0 replies; 28+ messages in thread
From: Andrew Rybchenko @ 2023-06-07  8:28 UTC (permalink / raw)
  To: Denis Pryazhennikov, dev
  Cc: Viacheslav Galaktionov, Ferruh Yigit, Roman Zhukov, Andy Moreton

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.


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 4/4] net/sfc: add configurable Rx CRC stripping
  2023-06-01 11:42 [PATCH 0/4] net/sfc: support KEEP_CRC offload Denis Pryazhennikov
                   ` (2 preceding siblings ...)
  2023-06-01 11:42 ` [PATCH 3/4] common/sfc_efx/base: add support for configure MAC to keep FCS Denis Pryazhennikov
@ 2023-06-01 11:42 ` Denis Pryazhennikov
  2023-06-07  8:34   ` Andrew Rybchenko
  2023-06-08 16:01 ` [PATCH 0/4] net/sfc: support KEEP_CRC offload Ferruh Yigit
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 28+ messages in thread
From: Denis Pryazhennikov @ 2023-06-01 11:42 UTC (permalink / raw)
  To: dev
  Cc: Viacheslav Galaktionov, Ferruh Yigit, Andrew Rybchenko,
	Roman Zhukov, Andy Moreton

Configurable Rx CRC stripping is allowed only if
running firmware variant supports it and if NIC is
configured with single PF per port and without VFs.

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>
---
 doc/guides/nics/features/sfc.ini |  1 +
 doc/guides/nics/sfc_efx.rst      |  6 ++++--
 drivers/net/sfc/sfc.h            |  1 +
 drivers/net/sfc/sfc_ef10_rx.c    |  3 ++-
 drivers/net/sfc/sfc_port.c       | 12 ++++++++++++
 drivers/net/sfc/sfc_rx.c         |  6 +++++-
 6 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/doc/guides/nics/features/sfc.ini b/doc/guides/nics/features/sfc.ini
index f5ac644278ae..c41c47a63d49 100644
--- a/doc/guides/nics/features/sfc.ini
+++ b/doc/guides/nics/features/sfc.ini
@@ -23,6 +23,7 @@ RSS key update       = Y
 RSS reta update      = Y
 SR-IOV               = Y
 Flow control         = Y
+CRC offload          = Y
 VLAN offload         = P
 L3 checksum offload  = Y
 L4 checksum offload  = Y
diff --git a/doc/guides/nics/sfc_efx.rst b/doc/guides/nics/sfc_efx.rst
index de0656876b96..b3a44e7ddd92 100644
--- a/doc/guides/nics/sfc_efx.rst
+++ b/doc/guides/nics/sfc_efx.rst
@@ -114,6 +114,10 @@ SFC EFX PMD has support for:
 
 - Loopback
 
+- Configurable Rx CRC stripping (if running firmware variant supports it and
+  if NIC is configured with single PF per port and without VFs, otherwise
+  always stripped)
+
 - SR-IOV PF
 
 - Port representors (see :ref: switch_representation)
@@ -126,8 +130,6 @@ The features not yet supported include:
 
 - Priority-based flow control
 
-- Configurable RX CRC stripping (always stripped)
-
 - Header split on receive
 
 - VLAN filtering
diff --git a/drivers/net/sfc/sfc.h b/drivers/net/sfc/sfc.h
index 730d054aea74..5c97d5911eb2 100644
--- a/drivers/net/sfc/sfc.h
+++ b/drivers/net/sfc/sfc.h
@@ -72,6 +72,7 @@ struct sfc_port {
 	unsigned int			flow_ctrl;
 	boolean_t			flow_ctrl_autoneg;
 	size_t				pdu;
+	boolean_t			include_fcs;
 
 	/*
 	 * Flow API isolated mode overrides promisc and allmulti settings;
diff --git a/drivers/net/sfc/sfc_ef10_rx.c b/drivers/net/sfc/sfc_ef10_rx.c
index 7be224c9c412..08fe41fe5d94 100644
--- a/drivers/net/sfc/sfc_ef10_rx.c
+++ b/drivers/net/sfc/sfc_ef10_rx.c
@@ -826,7 +826,8 @@ struct sfc_dp_rx sfc_ef10_rx = {
 	.dev_offload_capa	= RTE_ETH_RX_OFFLOAD_CHECKSUM |
 				  RTE_ETH_RX_OFFLOAD_OUTER_IPV4_CKSUM |
 				  RTE_ETH_RX_OFFLOAD_RSS_HASH,
-	.queue_offload_capa	= RTE_ETH_RX_OFFLOAD_SCATTER,
+	.queue_offload_capa	= RTE_ETH_RX_OFFLOAD_SCATTER |
+				  RTE_ETH_RX_OFFLOAD_KEEP_CRC,
 	.get_dev_info		= sfc_ef10_rx_get_dev_info,
 	.qsize_up_rings		= sfc_ef10_rx_qsize_up_rings,
 	.qcreate		= sfc_ef10_rx_qcreate,
diff --git a/drivers/net/sfc/sfc_port.c b/drivers/net/sfc/sfc_port.c
index 5f312ab1ba83..24d2daf6282b 100644
--- a/drivers/net/sfc/sfc_port.c
+++ b/drivers/net/sfc/sfc_port.c
@@ -250,6 +250,11 @@ sfc_port_start(struct sfc_adapter *sa)
 	if (rc != 0)
 		goto fail_mac_pdu_set;
 
+	sfc_log_init(sa, "set include FCS=%u", port->include_fcs);
+	rc = efx_mac_include_fcs_set(sa->nic, port->include_fcs);
+	if (rc != 0)
+		goto fail_include_fcs_set;
+
 	if (!sfc_sa2shared(sa)->isolated) {
 		struct rte_ether_addr *addr = &port->default_mac_addr;
 
@@ -337,6 +342,7 @@ sfc_port_start(struct sfc_adapter *sa)
 	(void)efx_mac_drain(sa->nic, B_TRUE);
 
 fail_mac_drain:
+fail_include_fcs_set:
 fail_mac_stats_upload:
 	(void)efx_mac_stats_periodic(sa->nic, &port->mac_stats_dma_mem,
 				     0, B_FALSE);
@@ -384,11 +390,17 @@ sfc_port_configure(struct sfc_adapter *sa)
 {
 	const struct rte_eth_dev_data *dev_data = sa->eth_dev->data;
 	struct sfc_port *port = &sa->port;
+	const struct rte_eth_rxmode *rxmode = &dev_data->dev_conf.rxmode;
 
 	sfc_log_init(sa, "entry");
 
 	port->pdu = EFX_MAC_PDU(dev_data->mtu);
 
+	if (rxmode->offloads & RTE_ETH_RX_OFFLOAD_KEEP_CRC)
+		port->include_fcs = true;
+	else
+		port->include_fcs = false;
+
 	return 0;
 }
 
diff --git a/drivers/net/sfc/sfc_rx.c b/drivers/net/sfc/sfc_rx.c
index edd0f0c03842..f80771778c64 100644
--- a/drivers/net/sfc/sfc_rx.c
+++ b/drivers/net/sfc/sfc_rx.c
@@ -655,7 +655,8 @@ struct sfc_dp_rx sfc_efx_rx = {
 	.features		= SFC_DP_RX_FEAT_INTR,
 	.dev_offload_capa	= RTE_ETH_RX_OFFLOAD_CHECKSUM |
 				  RTE_ETH_RX_OFFLOAD_RSS_HASH,
-	.queue_offload_capa	= RTE_ETH_RX_OFFLOAD_SCATTER,
+	.queue_offload_capa	= RTE_ETH_RX_OFFLOAD_SCATTER |
+				  RTE_ETH_RX_OFFLOAD_KEEP_CRC,
 	.qsize_up_rings		= sfc_efx_rx_qsize_up_rings,
 	.qcreate		= sfc_efx_rx_qcreate,
 	.qdestroy		= sfc_efx_rx_qdestroy,
@@ -938,6 +939,9 @@ sfc_rx_get_offload_mask(struct sfc_adapter *sa)
 	if (encp->enc_tunnel_encapsulations_supported == 0)
 		no_caps |= RTE_ETH_RX_OFFLOAD_OUTER_IPV4_CKSUM;
 
+	if (encp->enc_rx_include_fcs_supported == 0)
+		no_caps |= RTE_ETH_RX_OFFLOAD_KEEP_CRC;
+
 	return ~no_caps;
 }
 
-- 
2.37.0 (Apple Git-136)


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 4/4] net/sfc: add configurable Rx CRC stripping
  2023-06-01 11:42 ` [PATCH 4/4] net/sfc: add configurable Rx CRC stripping Denis Pryazhennikov
@ 2023-06-07  8:34   ` Andrew Rybchenko
  0 siblings, 0 replies; 28+ messages in thread
From: Andrew Rybchenko @ 2023-06-07  8:34 UTC (permalink / raw)
  To: Denis Pryazhennikov, dev
  Cc: Viacheslav Galaktionov, Ferruh Yigit, Roman Zhukov, Andy Moreton

On 6/1/23 14:42, Denis Pryazhennikov wrote:
> Configurable Rx CRC stripping is allowed only if
> running firmware variant supports it and if NIC is
> configured with single PF per port and without VFs.

I'd like to double-check one thing. Doesn't it require fixes on
datapath to report correct length? If no, it would be good to
see notes about it here.

> 
> 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>
> ---
>   doc/guides/nics/features/sfc.ini |  1 +
>   doc/guides/nics/sfc_efx.rst      |  6 ++++--
>   drivers/net/sfc/sfc.h            |  1 +
>   drivers/net/sfc/sfc_ef10_rx.c    |  3 ++-
>   drivers/net/sfc/sfc_port.c       | 12 ++++++++++++
>   drivers/net/sfc/sfc_rx.c         |  6 +++++-
>   6 files changed, 25 insertions(+), 4 deletions(-)
> 
> diff --git a/doc/guides/nics/features/sfc.ini b/doc/guides/nics/features/sfc.ini
> index f5ac644278ae..c41c47a63d49 100644
> --- a/doc/guides/nics/features/sfc.ini
> +++ b/doc/guides/nics/features/sfc.ini
> @@ -23,6 +23,7 @@ RSS key update       = Y
>   RSS reta update      = Y
>   SR-IOV               = Y
>   Flow control         = Y
> +CRC offload          = Y
>   VLAN offload         = P
>   L3 checksum offload  = Y
>   L4 checksum offload  = Y
> diff --git a/doc/guides/nics/sfc_efx.rst b/doc/guides/nics/sfc_efx.rst
> index de0656876b96..b3a44e7ddd92 100644
> --- a/doc/guides/nics/sfc_efx.rst
> +++ b/doc/guides/nics/sfc_efx.rst
> @@ -114,6 +114,10 @@ SFC EFX PMD has support for:
>   
>   - Loopback
>   
> +- Configurable Rx CRC stripping (if running firmware variant supports it and
> +  if NIC is configured with single PF per port and without VFs, otherwise
> +  always stripped)
> +
>   - SR-IOV PF
>   
>   - Port representors (see :ref: switch_representation)
> @@ -126,8 +130,6 @@ The features not yet supported include:
>   
>   - Priority-based flow control
>   
> -- Configurable RX CRC stripping (always stripped)
> -
>   - Header split on receive
>   
>   - VLAN filtering
> diff --git a/drivers/net/sfc/sfc.h b/drivers/net/sfc/sfc.h
> index 730d054aea74..5c97d5911eb2 100644
> --- a/drivers/net/sfc/sfc.h
> +++ b/drivers/net/sfc/sfc.h
> @@ -72,6 +72,7 @@ struct sfc_port {
>   	unsigned int			flow_ctrl;
>   	boolean_t			flow_ctrl_autoneg;
>   	size_t				pdu;
> +	boolean_t			include_fcs;

Please, put it after flow_ctrl_autoneg to use available hole.
There is no requirements to put it after pdu.

>   
>   	/*
>   	 * Flow API isolated mode overrides promisc and allmulti settings;
> diff --git a/drivers/net/sfc/sfc_ef10_rx.c b/drivers/net/sfc/sfc_ef10_rx.c
> index 7be224c9c412..08fe41fe5d94 100644
> --- a/drivers/net/sfc/sfc_ef10_rx.c
> +++ b/drivers/net/sfc/sfc_ef10_rx.c
> @@ -826,7 +826,8 @@ struct sfc_dp_rx sfc_ef10_rx = {
>   	.dev_offload_capa	= RTE_ETH_RX_OFFLOAD_CHECKSUM |
>   				  RTE_ETH_RX_OFFLOAD_OUTER_IPV4_CKSUM |
>   				  RTE_ETH_RX_OFFLOAD_RSS_HASH,
> -	.queue_offload_capa	= RTE_ETH_RX_OFFLOAD_SCATTER,
> +	.queue_offload_capa	= RTE_ETH_RX_OFFLOAD_SCATTER |
> +				  RTE_ETH_RX_OFFLOAD_KEEP_CRC,

Why is it queue level offload? It is configured per port. So,
it should be device level offload if I'm not mistaken.

>   	.get_dev_info		= sfc_ef10_rx_get_dev_info,
>   	.qsize_up_rings		= sfc_ef10_rx_qsize_up_rings,
>   	.qcreate		= sfc_ef10_rx_qcreate,
> diff --git a/drivers/net/sfc/sfc_port.c b/drivers/net/sfc/sfc_port.c
> index 5f312ab1ba83..24d2daf6282b 100644
> --- a/drivers/net/sfc/sfc_port.c
> +++ b/drivers/net/sfc/sfc_port.c
> @@ -250,6 +250,11 @@ sfc_port_start(struct sfc_adapter *sa)
>   	if (rc != 0)
>   		goto fail_mac_pdu_set;
>   
> +	sfc_log_init(sa, "set include FCS=%u", port->include_fcs);
> +	rc = efx_mac_include_fcs_set(sa->nic, port->include_fcs);
> +	if (rc != 0)
> +		goto fail_include_fcs_set;
> +
>   	if (!sfc_sa2shared(sa)->isolated) {
>   		struct rte_ether_addr *addr = &port->default_mac_addr;
>   
> @@ -337,6 +342,7 @@ sfc_port_start(struct sfc_adapter *sa)
>   	(void)efx_mac_drain(sa->nic, B_TRUE);
>   
>   fail_mac_drain:
> +fail_include_fcs_set:
>   fail_mac_stats_upload:
>   	(void)efx_mac_stats_periodic(sa->nic, &port->mac_stats_dma_mem,
>   				     0, B_FALSE);
> @@ -384,11 +390,17 @@ sfc_port_configure(struct sfc_adapter *sa)
>   {
>   	const struct rte_eth_dev_data *dev_data = sa->eth_dev->data;
>   	struct sfc_port *port = &sa->port;
> +	const struct rte_eth_rxmode *rxmode = &dev_data->dev_conf.rxmode;
>   
>   	sfc_log_init(sa, "entry");
>   
>   	port->pdu = EFX_MAC_PDU(dev_data->mtu);
>   
> +	if (rxmode->offloads & RTE_ETH_RX_OFFLOAD_KEEP_CRC)
> +		port->include_fcs = true;
> +	else
> +		port->include_fcs = false;
> +
>   	return 0;
>   }
>   
> diff --git a/drivers/net/sfc/sfc_rx.c b/drivers/net/sfc/sfc_rx.c
> index edd0f0c03842..f80771778c64 100644
> --- a/drivers/net/sfc/sfc_rx.c
> +++ b/drivers/net/sfc/sfc_rx.c
> @@ -655,7 +655,8 @@ struct sfc_dp_rx sfc_efx_rx = {
>   	.features		= SFC_DP_RX_FEAT_INTR,
>   	.dev_offload_capa	= RTE_ETH_RX_OFFLOAD_CHECKSUM |
>   				  RTE_ETH_RX_OFFLOAD_RSS_HASH,
> -	.queue_offload_capa	= RTE_ETH_RX_OFFLOAD_SCATTER,
> +	.queue_offload_capa	= RTE_ETH_RX_OFFLOAD_SCATTER |
> +				  RTE_ETH_RX_OFFLOAD_KEEP_CRC,

It seems it should be device level offload as above.

>   	.qsize_up_rings		= sfc_efx_rx_qsize_up_rings,
>   	.qcreate		= sfc_efx_rx_qcreate,
>   	.qdestroy		= sfc_efx_rx_qdestroy,
> @@ -938,6 +939,9 @@ sfc_rx_get_offload_mask(struct sfc_adapter *sa)
>   	if (encp->enc_tunnel_encapsulations_supported == 0)
>   		no_caps |= RTE_ETH_RX_OFFLOAD_OUTER_IPV4_CKSUM;
>   
> +	if (encp->enc_rx_include_fcs_supported == 0)
> +		no_caps |= RTE_ETH_RX_OFFLOAD_KEEP_CRC;
> +
>   	return ~no_caps;
>   }
>   


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 0/4] net/sfc: support KEEP_CRC offload
  2023-06-01 11:42 [PATCH 0/4] net/sfc: support KEEP_CRC offload Denis Pryazhennikov
                   ` (3 preceding siblings ...)
  2023-06-01 11:42 ` [PATCH 4/4] net/sfc: add configurable Rx CRC stripping Denis Pryazhennikov
@ 2023-06-08 16:01 ` Ferruh Yigit
  2023-06-22  3:47 ` [PATCH v2 " Denis Pryazhennikov
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 28+ messages in thread
From: Ferruh Yigit @ 2023-06-08 16:01 UTC (permalink / raw)
  To: Denis Pryazhennikov, dev; +Cc: Viacheslav Galaktionov, Andrew Rybchenko

On 6/1/2023 12:42 PM, Denis Pryazhennikov wrote:
> This patch series adds support for RTE_ETH_RX_OFFLOAD_KEEP_CRC
> offload in the SFC driver.
> 
> Denis Pryazhennikov (3):
>   common/sfc_efx/base: detect and report FCS include support
>   common/sfc_efx/base: add support for configure MAC to keep FCS
>   net/sfc: add configurable Rx CRC stripping
> 
> Sandilya Bhagi (1):
>   common/sfc_efx/base: NIC Partitioning mode discovery using heuristic
>     approach
>

Can you please fix 'check-git-log' warnings:

Wrong headline uppercase:
        common/sfc_efx/base: NIC Partitioning mode discovery using
heuristic approach
Headline too long:
        common/sfc_efx/base: NIC Partitioning mode discovery using
heuristic approach
        common/sfc_efx/base: add support for configure MAC to keep FCS
Line too long:
        When NIC Partitioning is configured, apart from the privileged
adapter(s) the
        Determining NIC Partitioning mode is required to take necessary
action(s) for
        NIC Partitioning is determined using heuristic approach - If the
physical ports
        When NIC Partitioning is in use MAX MTU workaround should be
applied so that
Contributor name/email mismatch with .mailmap:
        Roman Zhukov <roman.zhukov@arknetworks.am> is not the primary
email address
        Sandilya Bhagi <sbhagi@solarflare.com> is unknown in .mailmap


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v2 0/4] net/sfc: support KEEP_CRC offload
  2023-06-01 11:42 [PATCH 0/4] net/sfc: support KEEP_CRC offload Denis Pryazhennikov
                   ` (4 preceding siblings ...)
  2023-06-08 16:01 ` [PATCH 0/4] net/sfc: support KEEP_CRC offload Ferruh Yigit
@ 2023-06-22  3:47 ` Denis Pryazhennikov
  2023-06-22  3:47   ` [PATCH v2 1/4] common/sfc_efx/base: discover NIC partitioning mode Denis Pryazhennikov
                     ` (3 more replies)
  2023-06-22 11:38 ` [PATCH v3 0/4] net/sfc: support KEEP_CRC offload Denis Pryazhennikov
  2023-06-22 11:47 ` [PATCH v4 0/4] net/sfc: support KEEP_CRC offload Denis Pryazhennikov
  7 siblings, 4 replies; 28+ messages in thread
From: Denis Pryazhennikov @ 2023-06-22  3:47 UTC (permalink / raw)
  To: dev; +Cc: Ferruh Yigit, Andrew Rybchenko

This patch series adds support for RTE_ETH_RX_OFFLOAD_KEEP_CRC
offload in the SFC driver.

Changes in v2:
* Applied review notes;
* Fixed stats handling;
* Added missing release notes to [4/4].

Denis Pryazhennikov (3):
  common/sfc_efx/base: detect and report FCS include support
  common/sfc_efx/base: add support for configure MAC to keep FCS
  net/sfc: add configurable Rx CRC stripping

Sandilya Bhagi (1):
  common/sfc_efx/base: discover NIC partitioning mode

 doc/guides/nics/features/sfc.ini        |   1 +
 doc/guides/nics/sfc_efx.rst             |   6 +-
 doc/guides/rel_notes/release_23_07.rst  |   2 +
 drivers/common/sfc_efx/base/ef10_mac.c  |   5 +-
 drivers/common/sfc_efx/base/ef10_nic.c  | 108 ++++++++++++++++++++++++
 drivers/common/sfc_efx/base/efx.h       |  14 +++
 drivers/common/sfc_efx/base/efx_impl.h  |   1 +
 drivers/common/sfc_efx/base/efx_mac.c   |  48 +++++++++++
 drivers/common/sfc_efx/base/siena_nic.c |   1 +
 drivers/common/sfc_efx/version.map      |   1 +
 drivers/net/sfc/sfc.h                   |   1 +
 drivers/net/sfc/sfc_ef100_rx.c          |   2 +-
 drivers/net/sfc/sfc_ef10_rx.c           |   3 +-
 drivers/net/sfc/sfc_ethdev.c            |   7 +-
 drivers/net/sfc/sfc_port.c              |  12 +++
 drivers/net/sfc/sfc_rx.c                |   6 +-
 16 files changed, 210 insertions(+), 8 deletions(-)

-- 
2.39.2 (Apple Git-143)


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v2 1/4] common/sfc_efx/base: discover NIC partitioning mode
  2023-06-22  3:47 ` [PATCH v2 " Denis Pryazhennikov
@ 2023-06-22  3:47   ` Denis Pryazhennikov
  2023-06-22  9:48     ` Andrew Rybchenko
  2023-06-22  3:47   ` [PATCH v2 2/4] common/sfc_efx/base: detect and report FCS include support Denis Pryazhennikov
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 28+ messages in thread
From: Denis Pryazhennikov @ 2023-06-22  3:47 UTC (permalink / raw)
  To: dev; +Cc: Ferruh Yigit, Andrew Rybchenko, Sandilya Bhagi, Andy Moreton

From: Sandilya Bhagi <sbhagi@solarflare.com>

NIC Partitioning mode in SFC devices means multiple PFs
per network port. When NIC Partitioning is configured,
apart from the privileged adapter(s) the other
unprivileged adapter(s) will share the same physical port.
Determining NIC Partitioning mode is required to take
necessary action(s) for unprivileged adapter to work seamlessly.
BNIC Partitioning is determined using heuristic approach.
If the physical ports are shared between PFs then either
NIC Partitioning or SR-IOV is in use.

Signed-off-by: Sandilya Bhagi <sbhagi@solarflare.com>
Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
Reviewed-by: Andy Moreton <amoreton@xilinx.com>
---
 drivers/common/sfc_efx/base/ef10_nic.c | 102 +++++++++++++++++++++++++
 drivers/common/sfc_efx/base/efx.h      |   8 ++
 2 files changed, 110 insertions(+)

diff --git a/drivers/common/sfc_efx/base/ef10_nic.c b/drivers/common/sfc_efx/base/ef10_nic.c
index e1709d120093..507b784b25cd 100644
--- a/drivers/common/sfc_efx/base/ef10_nic.c
+++ b/drivers/common/sfc_efx/base/ef10_nic.c
@@ -1044,6 +1044,83 @@ ef10_mcdi_get_pf_count(
 	return (rc);
 }
 
+static	__checkReturn			efx_rc_t
+ef10_nic_get_physical_port_usage(
+	__in				efx_nic_t *enp,
+	__in_ecount(pfs_to_ports_size)	uint8_t *pfs_to_ports,
+	__in				size_t pfs_to_ports_size,
+	__out				efx_port_usage_t *port_usagep)
+{
+	efx_nic_cfg_t *encp = &(enp->en_nic_cfg);
+	efx_port_usage_t port_usage;
+	uint8_t phy_port;
+	efx_rc_t rc;
+	size_t pf;
+
+	/*
+	 * The sharing of physical ports between functions are determined
+	 * in the following way.
+	 * 1. If VFs are enabled then the physical port is shared.
+	 * 2. Retrieve PFs to ports assignment.
+	 * 3. If PF 0 assignment cannot be retrieved(ACCESS_DENIED), it
+	 *    implies this is an unprivileged function. An unprivileged
+	 *    function indicates the physical port must be shared with
+	 *    another privileged function.
+	 * 4. If PF 0 assignment can be retrieved, it indicates this
+	 *    function is privileged. Now, read all other PF's physical
+	 *    port number assignment and check if the current PF's physical
+	 *    port is shared with any other PF's physical port.
+	 * NOTE: PF 0 is always privileged function.
+	 */
+
+	if (EFX_PCI_FUNCTION_IS_VF(encp)) {
+		port_usage = EFX_PORT_USAGE_SHARED;
+		goto out;
+	}
+
+	if (pfs_to_ports[0] ==
+	    MC_CMD_GET_CAPABILITIES_V2_OUT_ACCESS_NOT_PERMITTED) {
+		/*
+		 * This is unprivileged function as it do not have sufficient
+		 * privileges to read the value, this implies the physical port
+		 * is shared between this function and another privileged
+		 * function
+		 */
+		port_usage = EFX_PORT_USAGE_SHARED;
+		goto out;
+	}
+
+	if (encp->enc_pf >= pfs_to_ports_size) {
+		rc = EINVAL;
+		goto fail1;
+	}
+	phy_port = pfs_to_ports[encp->enc_pf];
+
+	/*
+	 * This is privileged function as it is able read the value of
+	 * PF 0. Now, check if any other function share the same physical
+	 * port number as this function.
+	 */
+	for (pf = 0; pf < pfs_to_ports_size; pf++) {
+		if ((encp->enc_pf != pf) && (phy_port == pfs_to_ports[pf])) {
+			/* Found match, PFs share the same physical port */
+			port_usage = EFX_PORT_USAGE_SHARED;
+			goto out;
+		}
+	}
+
+	port_usage = EFX_PORT_USAGE_EXCLUSIVE;
+
+out:
+	*port_usagep = port_usage;
+	return (0);
+
+fail1:
+	EFSYS_PROBE1(fail1, efx_rc_t, rc);
+
+	return (rc);
+}
+
 static	__checkReturn	efx_rc_t
 ef10_get_datapath_caps(
 	__in		efx_nic_t *enp)
@@ -1307,6 +1384,31 @@ ef10_get_datapath_caps(
 		encp->enc_tunnel_config_udp_entries_max = 0;
 	}
 
+#define CAP_PFS_TO_PORTS(_n)	\
+	(MC_CMD_GET_CAPABILITIES_V2_OUT_PFS_TO_PORTS_ASSIGNMENT_ ## _n)
+
+	encp->enc_port_usage = EFX_PORT_USAGE_UNKNOWN;
+
+	if (req.emr_out_length_used >= MC_CMD_GET_CAPABILITIES_V2_OUT_LEN) {
+		/* PFs to ports assignment */
+		uint8_t pfs_to_ports[CAP_PFS_TO_PORTS(NUM)];
+
+		EFX_STATIC_ASSERT((CAP_PFS_TO_PORTS(NUM) * CAP_PFS_TO_PORTS(LEN)) ==
+				  EFX_ARRAY_SIZE(pfs_to_ports));
+
+		memcpy(pfs_to_ports, MCDI_OUT(req, efx_byte_t, CAP_PFS_TO_PORTS(OFST)),
+		       EFX_ARRAY_SIZE(pfs_to_ports));
+
+		rc = ef10_nic_get_physical_port_usage(enp, pfs_to_ports,
+						      EFX_ARRAY_SIZE(pfs_to_ports),
+						      &encp->enc_port_usage);
+		if (rc != 0) {
+			/* PF to port mapping lookup failed */
+			encp->enc_port_usage = EFX_PORT_USAGE_UNKNOWN;
+		}
+	}
+#undef  CAP_PFS_TO_PORTS
+
 	/*
 	 * Check if firmware reports the VI window mode.
 	 * Medford2 has a variable VI window size (8K, 16K or 64K).
diff --git a/drivers/common/sfc_efx/base/efx.h b/drivers/common/sfc_efx/base/efx.h
index f156f8b5b31a..a63211612249 100644
--- a/drivers/common/sfc_efx/base/efx.h
+++ b/drivers/common/sfc_efx/base/efx.h
@@ -311,6 +311,12 @@ efx_nic_check_pcie_link_speed(
 	__in		uint32_t pcie_link_gen,
 	__out		efx_pcie_link_performance_t *resultp);
 
+typedef enum efx_port_usage_e {
+	EFX_PORT_USAGE_UNKNOWN = 0,
+	EFX_PORT_USAGE_EXCLUSIVE,	/* Port only used by this PF */
+	EFX_PORT_USAGE_SHARED,		/* Port shared with other PFs */
+} efx_port_usage_t;
+
 #define	EFX_MAC_ADDR_LEN 6
 
 #if EFSYS_OPT_MCDI
@@ -1682,6 +1688,8 @@ typedef struct efx_nic_cfg_s {
 	uint32_t		enc_assigned_port;
 	/* NIC DMA mapping type */
 	efx_nic_dma_mapping_t	enc_dma_mapping;
+	/* Physical ports shared by PFs */
+	efx_port_usage_t	enc_port_usage;
 } efx_nic_cfg_t;
 
 #define	EFX_PCI_VF_INVALID 0xffff
-- 
2.39.2 (Apple Git-143)


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 1/4] common/sfc_efx/base: discover NIC partitioning mode
  2023-06-22  3:47   ` [PATCH v2 1/4] common/sfc_efx/base: discover NIC partitioning mode Denis Pryazhennikov
@ 2023-06-22  9:48     ` Andrew Rybchenko
  0 siblings, 0 replies; 28+ messages in thread
From: Andrew Rybchenko @ 2023-06-22  9:48 UTC (permalink / raw)
  To: Denis Pryazhennikov, dev; +Cc: Ferruh Yigit, Sandilya Bhagi, Andy Moreton

On 6/22/23 06:47, Denis Pryazhennikov wrote:
> From: Sandilya Bhagi <sbhagi@solarflare.com>
> 
> NIC Partitioning mode in SFC devices means multiple PFs
> per network port. When NIC Partitioning is configured,
> apart from the privileged adapter(s) the other
> unprivileged adapter(s) will share the same physical port.
> Determining NIC Partitioning mode is required to take
> necessary action(s) for unprivileged adapter to work seamlessly.
> BNIC Partitioning is determined using heuristic approach.
> If the physical ports are shared between PFs then either
> NIC Partitioning or SR-IOV is in use.
> 
> Signed-off-by: Sandilya Bhagi <sbhagi@solarflare.com>
> Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
> Reviewed-by: Andy Moreton <amoreton@xilinx.com>

Overall LGMT with few style notes:

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

[snip]

> +#define CAP_PFS_TO_PORTS(_n)	\
> +	(MC_CMD_GET_CAPABILITIES_V2_OUT_PFS_TO_PORTS_ASSIGNMENT_ ## _n)
> +
> +	encp->enc_port_usage = EFX_PORT_USAGE_UNKNOWN;
> +
> +	if (req.emr_out_length_used >= MC_CMD_GET_CAPABILITIES_V2_OUT_LEN) {
> +		/* PFs to ports assignment */
> +		uint8_t pfs_to_ports[CAP_PFS_TO_PORTS(NUM)];
> +
> +		EFX_STATIC_ASSERT((CAP_PFS_TO_PORTS(NUM) * CAP_PFS_TO_PORTS(LEN)) ==
> +				  EFX_ARRAY_SIZE(pfs_to_ports));

As far as I remember libefx style, it should be 4 spaces indent above
relative to mail line.

> +
> +		memcpy(pfs_to_ports, MCDI_OUT(req, efx_byte_t, CAP_PFS_TO_PORTS(OFST)),
> +		       EFX_ARRAY_SIZE(pfs_to_ports));

same here, 4 spaces indent

> +
> +		rc = ef10_nic_get_physical_port_usage(enp, pfs_to_ports,
> +						      EFX_ARRAY_SIZE(pfs_to_ports),
> +						      &encp->enc_port_usage);

same here, 4 spaces indent

> +		if (rc != 0) {
> +			/* PF to port mapping lookup failed */
> +			encp->enc_port_usage = EFX_PORT_USAGE_UNKNOWN;
> +		}
> +	}
> +#undef  CAP_PFS_TO_PORTS
> +
>   	/*
>   	 * Check if firmware reports the VI window mode.
>   	 * Medford2 has a variable VI window size (8K, 16K or 64K).

[snip]



^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v2 2/4] common/sfc_efx/base: detect and report FCS include support
  2023-06-22  3:47 ` [PATCH v2 " Denis Pryazhennikov
  2023-06-22  3:47   ` [PATCH v2 1/4] common/sfc_efx/base: discover NIC partitioning mode Denis Pryazhennikov
@ 2023-06-22  3:47   ` Denis Pryazhennikov
  2023-06-22  3:47   ` [PATCH v2 3/4] common/sfc_efx/base: add support for configure MAC to keep FCS Denis Pryazhennikov
  2023-06-22  3:47   ` [PATCH v2 4/4] net/sfc: add configurable Rx CRC stripping Denis Pryazhennikov
  3 siblings, 0 replies; 28+ messages in thread
From: Denis Pryazhennikov @ 2023-06-22  3:47 UTC (permalink / raw)
  To: dev; +Cc: Ferruh Yigit, Andrew Rybchenko, Roman Zhukov, Viacheslav Galaktionov

From: Roman Zhukov <roman.zhukov@arknetworks.am>

A new variable was added to efx_nic_cfg_s to detect and
report if FCS is supported by FW.

Signed-off-by: Roman Zhukov <roman.zhukov@arknetworks.am>
Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
Reviewed-by: Viacheslav Galaktionov <viacheslav.galaktionov@arknetworks.am>
Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
 drivers/common/sfc_efx/base/ef10_nic.c  | 6 ++++++
 drivers/common/sfc_efx/base/efx.h       | 1 +
 drivers/common/sfc_efx/base/siena_nic.c | 1 +
 3 files changed, 8 insertions(+)

diff --git a/drivers/common/sfc_efx/base/ef10_nic.c b/drivers/common/sfc_efx/base/ef10_nic.c
index 507b784b25cd..d4d3ede8cf45 100644
--- a/drivers/common/sfc_efx/base/ef10_nic.c
+++ b/drivers/common/sfc_efx/base/ef10_nic.c
@@ -1244,6 +1244,12 @@ ef10_get_datapath_caps(
 	/* No limit on maximum number of Rx scatter elements per packet. */
 	encp->enc_rx_scatter_max = -1;
 
+	/* Check if the firmware supports include FCS on RX */
+	if (CAP_FLAGS1(req, RX_INCLUDE_FCS))
+		encp->enc_rx_include_fcs_supported = B_TRUE;
+	else
+		encp->enc_rx_include_fcs_supported = B_FALSE;
+
 	/* Check if the firmware supports packed stream mode */
 	if (CAP_FLAGS1(req, RX_PACKED_STREAM))
 		encp->enc_rx_packed_stream_supported = B_TRUE;
diff --git a/drivers/common/sfc_efx/base/efx.h b/drivers/common/sfc_efx/base/efx.h
index a63211612249..b5bd390169ae 100644
--- a/drivers/common/sfc_efx/base/efx.h
+++ b/drivers/common/sfc_efx/base/efx.h
@@ -1629,6 +1629,7 @@ typedef struct efx_nic_cfg_s {
 	/* Datapath firmware vport reconfigure support */
 	boolean_t		enc_vport_reconfigure_supported;
 	boolean_t		enc_rx_disable_scatter_supported;
+	boolean_t		enc_rx_include_fcs_supported;
 	/* Maximum number of Rx scatter segments supported by HW */
 	uint32_t		enc_rx_scatter_max;
 	boolean_t		enc_allow_set_mac_with_installed_filters;
diff --git a/drivers/common/sfc_efx/base/siena_nic.c b/drivers/common/sfc_efx/base/siena_nic.c
index 1f1fb7d2afd1..ca38eefa7e37 100644
--- a/drivers/common/sfc_efx/base/siena_nic.c
+++ b/drivers/common/sfc_efx/base/siena_nic.c
@@ -187,6 +187,7 @@ siena_board_cfg(
 	encp->enc_allow_set_mac_with_installed_filters = B_TRUE;
 	encp->enc_rx_packed_stream_supported = B_FALSE;
 	encp->enc_rx_var_packed_stream_supported = B_FALSE;
+	encp->enc_rx_include_fcs_supported = B_FALSE;
 	encp->enc_rx_es_super_buffer_supported = B_FALSE;
 	encp->enc_fw_subvariant_no_tx_csum_supported = B_FALSE;
 
-- 
2.39.2 (Apple Git-143)


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v2 3/4] common/sfc_efx/base: add support for configure MAC to keep FCS
  2023-06-22  3:47 ` [PATCH v2 " Denis Pryazhennikov
  2023-06-22  3:47   ` [PATCH v2 1/4] common/sfc_efx/base: discover NIC partitioning mode Denis Pryazhennikov
  2023-06-22  3:47   ` [PATCH v2 2/4] common/sfc_efx/base: detect and report FCS include support Denis Pryazhennikov
@ 2023-06-22  3:47   ` Denis Pryazhennikov
  2023-06-22  3:47   ` [PATCH v2 4/4] net/sfc: add configurable Rx CRC stripping Denis Pryazhennikov
  3 siblings, 0 replies; 28+ messages in thread
From: Denis Pryazhennikov @ 2023-06-22  3:47 UTC (permalink / raw)
  To: dev; +Cc: Ferruh Yigit, Andrew Rybchenko, Roman Zhukov, Andy Moreton

From: Roman Zhukov <roman.zhukov@arknetworks.am>

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>
---
 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(-)

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 b5bd390169ae..ef626cc55a7e 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 09b1e95c594c..92a30c34ae28 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 e91bcbcad863..01113bffa7cb 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;
-- 
2.39.2 (Apple Git-143)


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v2 4/4] net/sfc: add configurable Rx CRC stripping
  2023-06-22  3:47 ` [PATCH v2 " Denis Pryazhennikov
                     ` (2 preceding siblings ...)
  2023-06-22  3:47   ` [PATCH v2 3/4] common/sfc_efx/base: add support for configure MAC to keep FCS Denis Pryazhennikov
@ 2023-06-22  3:47   ` Denis Pryazhennikov
  2023-06-22  9:53     ` Andrew Rybchenko
  3 siblings, 1 reply; 28+ messages in thread
From: Denis Pryazhennikov @ 2023-06-22  3:47 UTC (permalink / raw)
  To: dev; +Cc: Ferruh Yigit, Andrew Rybchenko, Roman Zhukov, Andy Moreton

Configurable Rx CRC stripping is allowed only if
running firmware variant supports it and if NIC is
configured with single PF per port and without VFs.
When KEEP_CRC is supported, CRC will be part of
the packet payload. The packet length will also
contain CRC length. At the same time, CRC length
should be removed from stats.

Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
Signed-off-by: Roman Zhukov <roman.zhukov@arknetworks.am>
Reviewed-by: Andy Moreton <amoreton@xilinx.com>
---
 doc/guides/nics/features/sfc.ini       |  1 +
 doc/guides/nics/sfc_efx.rst            |  6 ++++--
 doc/guides/rel_notes/release_23_07.rst |  2 ++
 drivers/net/sfc/sfc.h                  |  1 +
 drivers/net/sfc/sfc_ef100_rx.c         |  2 +-
 drivers/net/sfc/sfc_ef10_rx.c          |  3 ++-
 drivers/net/sfc/sfc_ethdev.c           |  7 ++++++-
 drivers/net/sfc/sfc_port.c             | 12 ++++++++++++
 drivers/net/sfc/sfc_rx.c               |  6 +++++-
 9 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/doc/guides/nics/features/sfc.ini b/doc/guides/nics/features/sfc.ini
index a4b53c619c12..8a9198adcb5b 100644
--- a/doc/guides/nics/features/sfc.ini
+++ b/doc/guides/nics/features/sfc.ini
@@ -23,6 +23,7 @@ RSS key update       = Y
 RSS reta update      = Y
 SR-IOV               = Y
 Flow control         = Y
+CRC offload          = Y
 VLAN offload         = P
 FEC                  = Y
 L3 checksum offload  = Y
diff --git a/doc/guides/nics/sfc_efx.rst b/doc/guides/nics/sfc_efx.rst
index ba82b020932b..24459da33ea8 100644
--- a/doc/guides/nics/sfc_efx.rst
+++ b/doc/guides/nics/sfc_efx.rst
@@ -114,6 +114,10 @@ SFC EFX PMD has support for:
 
 - Loopback
 
+- Configurable Rx CRC stripping (if running firmware variant supports it and
+  if NIC is configured with single PF per port and without VFs, otherwise
+  always stripped)
+
 - SR-IOV PF
 
 - Port representors (see :ref: switch_representation)
@@ -126,8 +130,6 @@ The features not yet supported include:
 
 - Priority-based flow control
 
-- Configurable RX CRC stripping (always stripped)
-
 - Header split on receive
 
 - VLAN filtering
diff --git a/doc/guides/rel_notes/release_23_07.rst b/doc/guides/rel_notes/release_23_07.rst
index 3db443d16018..738d35d9f7c6 100644
--- a/doc/guides/rel_notes/release_23_07.rst
+++ b/doc/guides/rel_notes/release_23_07.rst
@@ -145,6 +145,8 @@ New Features
   * Added support for transfer flow action INDIRECT with subtype COUNT,
     for aggregated statistics.
 
+  * Added support for keeping CRC.
+
 * **Added vmxnet3 version 7 support.**
 
   Added support for vmxnet3 version 7 which includes support
diff --git a/drivers/net/sfc/sfc.h b/drivers/net/sfc/sfc.h
index 89e2c46fa974..25cdeaa5cd25 100644
--- a/drivers/net/sfc/sfc.h
+++ b/drivers/net/sfc/sfc.h
@@ -74,6 +74,7 @@ struct sfc_port {
 
 	unsigned int			flow_ctrl;
 	boolean_t			flow_ctrl_autoneg;
+	boolean_t			include_fcs;
 	size_t				pdu;
 
 	/*
diff --git a/drivers/net/sfc/sfc_ef100_rx.c b/drivers/net/sfc/sfc_ef100_rx.c
index 37b754fa3305..5563bd9a0bd1 100644
--- a/drivers/net/sfc/sfc_ef100_rx.c
+++ b/drivers/net/sfc/sfc_ef100_rx.c
@@ -1004,7 +1004,7 @@ struct sfc_dp_rx sfc_ef100_rx = {
 				  SFC_DP_RX_FEAT_FLOW_MARK |
 				  SFC_DP_RX_FEAT_INTR |
 				  SFC_DP_RX_FEAT_STATS,
-	.dev_offload_capa	= 0,
+	.dev_offload_capa	= RTE_ETH_RX_OFFLOAD_KEEP_CRC,
 	.queue_offload_capa	= RTE_ETH_RX_OFFLOAD_CHECKSUM |
 				  RTE_ETH_RX_OFFLOAD_OUTER_IPV4_CKSUM |
 				  RTE_ETH_RX_OFFLOAD_OUTER_UDP_CKSUM |
diff --git a/drivers/net/sfc/sfc_ef10_rx.c b/drivers/net/sfc/sfc_ef10_rx.c
index 7be224c9c412..30a320d0791c 100644
--- a/drivers/net/sfc/sfc_ef10_rx.c
+++ b/drivers/net/sfc/sfc_ef10_rx.c
@@ -825,7 +825,8 @@ struct sfc_dp_rx sfc_ef10_rx = {
 				  SFC_DP_RX_FEAT_INTR,
 	.dev_offload_capa	= RTE_ETH_RX_OFFLOAD_CHECKSUM |
 				  RTE_ETH_RX_OFFLOAD_OUTER_IPV4_CKSUM |
-				  RTE_ETH_RX_OFFLOAD_RSS_HASH,
+				  RTE_ETH_RX_OFFLOAD_RSS_HASH |
+				  RTE_ETH_RX_OFFLOAD_KEEP_CRC,
 	.queue_offload_capa	= RTE_ETH_RX_OFFLOAD_SCATTER,
 	.get_dev_info		= sfc_ef10_rx_get_dev_info,
 	.qsize_up_rings		= sfc_ef10_rx_qsize_up_rings,
diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index f120ddc5a8d8..1efe64a36a7f 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -689,8 +689,13 @@ sfc_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 
 	sfc_adapter_lock(sa);
 
-	if (have_dp_rx_stats)
+	if (have_dp_rx_stats) {
 		sfc_stats_get_dp_rx(sa, &stats->ipackets, &stats->ibytes);
+		if (dev->data->dev_conf.rxmode.offloads &
+		    RTE_ETH_RX_OFFLOAD_KEEP_CRC) {
+			stats->ibytes -= stats->ipackets * RTE_ETHER_CRC_LEN;
+		}
+	}
 	if (have_dp_tx_stats)
 		sfc_stats_get_dp_tx(sa, &stats->opackets, &stats->obytes);
 
diff --git a/drivers/net/sfc/sfc_port.c b/drivers/net/sfc/sfc_port.c
index 0c887ddedb09..3897facfbc5b 100644
--- a/drivers/net/sfc/sfc_port.c
+++ b/drivers/net/sfc/sfc_port.c
@@ -241,6 +241,11 @@ sfc_port_start(struct sfc_adapter *sa)
 	if (rc != 0)
 		goto fail_mac_pdu_set;
 
+	sfc_log_init(sa, "set include FCS=%u", port->include_fcs);
+	rc = efx_mac_include_fcs_set(sa->nic, port->include_fcs);
+	if (rc != 0)
+		goto fail_include_fcs_set;
+
 	if (!sfc_sa2shared(sa)->isolated) {
 		struct rte_ether_addr *addr = &port->default_mac_addr;
 
@@ -328,6 +333,7 @@ sfc_port_start(struct sfc_adapter *sa)
 	(void)efx_mac_drain(sa->nic, B_TRUE);
 
 fail_mac_drain:
+fail_include_fcs_set:
 fail_mac_stats_upload:
 	(void)efx_mac_stats_periodic(sa->nic, &port->mac_stats_dma_mem,
 				     0, B_FALSE);
@@ -375,11 +381,17 @@ sfc_port_configure(struct sfc_adapter *sa)
 {
 	const struct rte_eth_dev_data *dev_data = sa->eth_dev->data;
 	struct sfc_port *port = &sa->port;
+	const struct rte_eth_rxmode *rxmode = &dev_data->dev_conf.rxmode;
 
 	sfc_log_init(sa, "entry");
 
 	port->pdu = EFX_MAC_PDU(dev_data->mtu);
 
+	if (rxmode->offloads & RTE_ETH_RX_OFFLOAD_KEEP_CRC)
+		port->include_fcs = true;
+	else
+		port->include_fcs = false;
+
 	return 0;
 }
 
diff --git a/drivers/net/sfc/sfc_rx.c b/drivers/net/sfc/sfc_rx.c
index edd0f0c03842..ac94d973de04 100644
--- a/drivers/net/sfc/sfc_rx.c
+++ b/drivers/net/sfc/sfc_rx.c
@@ -654,7 +654,8 @@ struct sfc_dp_rx sfc_efx_rx = {
 	},
 	.features		= SFC_DP_RX_FEAT_INTR,
 	.dev_offload_capa	= RTE_ETH_RX_OFFLOAD_CHECKSUM |
-				  RTE_ETH_RX_OFFLOAD_RSS_HASH,
+				  RTE_ETH_RX_OFFLOAD_RSS_HASH |
+				  RTE_ETH_RX_OFFLOAD_KEEP_CRC,
 	.queue_offload_capa	= RTE_ETH_RX_OFFLOAD_SCATTER,
 	.qsize_up_rings		= sfc_efx_rx_qsize_up_rings,
 	.qcreate		= sfc_efx_rx_qcreate,
@@ -938,6 +939,9 @@ sfc_rx_get_offload_mask(struct sfc_adapter *sa)
 	if (encp->enc_tunnel_encapsulations_supported == 0)
 		no_caps |= RTE_ETH_RX_OFFLOAD_OUTER_IPV4_CKSUM;
 
+	if (encp->enc_rx_include_fcs_supported == 0)
+		no_caps |= RTE_ETH_RX_OFFLOAD_KEEP_CRC;
+
 	return ~no_caps;
 }
 
-- 
2.39.2 (Apple Git-143)


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 4/4] net/sfc: add configurable Rx CRC stripping
  2023-06-22  3:47   ` [PATCH v2 4/4] net/sfc: add configurable Rx CRC stripping Denis Pryazhennikov
@ 2023-06-22  9:53     ` Andrew Rybchenko
  0 siblings, 0 replies; 28+ messages in thread
From: Andrew Rybchenko @ 2023-06-22  9:53 UTC (permalink / raw)
  To: Denis Pryazhennikov, dev; +Cc: Ferruh Yigit, Roman Zhukov, Andy Moreton

On 6/22/23 06:47, Denis Pryazhennikov wrote:
> Configurable Rx CRC stripping is allowed only if
> running firmware variant supports it and if NIC is
> configured with single PF per port and without VFs.
> When KEEP_CRC is supported, CRC will be part of
> the packet payload. The packet length will also
> contain CRC length. At the same time, CRC length
> should be removed from stats.
> 
> Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
> Signed-off-by: Roman Zhukov <roman.zhukov@arknetworks.am>
> Reviewed-by: Andy Moreton <amoreton@xilinx.com>

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



^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v3 0/4] net/sfc: support KEEP_CRC offload
  2023-06-01 11:42 [PATCH 0/4] net/sfc: support KEEP_CRC offload Denis Pryazhennikov
                   ` (5 preceding siblings ...)
  2023-06-22  3:47 ` [PATCH v2 " Denis Pryazhennikov
@ 2023-06-22 11:38 ` Denis Pryazhennikov
  2023-06-22 11:38   ` [PATCH v3 1/4] common/sfc_efx/base: discover NIC partitioning mode Denis Pryazhennikov
                     ` (3 more replies)
  2023-06-22 11:47 ` [PATCH v4 0/4] net/sfc: support KEEP_CRC offload Denis Pryazhennikov
  7 siblings, 4 replies; 28+ messages in thread
From: Denis Pryazhennikov @ 2023-06-22 11:38 UTC (permalink / raw)
  To: dev; +Cc: Ferruh Yigit, Andrew Rybchenko

This patch series adds support for RTE_ETH_RX_OFFLOAD_KEEP_CRC
offload in the SFC driver.

Changes in v2:
* Applied review notes;
* Fixed stats handling;
* Added missing release notes to [4/4].

Changes in v3:
* Fixed wrong indents in [1/4].

Denis Pryazhennikov (3):
  common/sfc_efx/base: detect and report FCS include support
  common/sfc_efx/base: add support for configure MAC to keep FCS
  net/sfc: add configurable Rx CRC stripping

Sandilya Bhagi (1):
  common/sfc_efx/base: discover NIC partitioning mode

 doc/guides/nics/features/sfc.ini        |   1 +
 doc/guides/nics/sfc_efx.rst             |   6 +-
 doc/guides/rel_notes/release_23_07.rst  |   2 +
 drivers/common/sfc_efx/base/ef10_mac.c  |   5 +-
 drivers/common/sfc_efx/base/ef10_nic.c  | 107 ++++++++++++++++++++++++
 drivers/common/sfc_efx/base/efx.h       |  14 ++++
 drivers/common/sfc_efx/base/efx_impl.h  |   1 +
 drivers/common/sfc_efx/base/efx_mac.c   |  48 +++++++++++
 drivers/common/sfc_efx/base/siena_nic.c |   1 +
 drivers/common/sfc_efx/version.map      |   1 +
 drivers/net/sfc/sfc.h                   |   1 +
 drivers/net/sfc/sfc_ef100_rx.c          |   2 +-
 drivers/net/sfc/sfc_ef10_rx.c           |   3 +-
 drivers/net/sfc/sfc_ethdev.c            |   7 +-
 drivers/net/sfc/sfc_port.c              |  12 +++
 drivers/net/sfc/sfc_rx.c                |   6 +-
 16 files changed, 209 insertions(+), 8 deletions(-)

-- 
2.39.2 (Apple Git-143)


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v3 1/4] common/sfc_efx/base: discover NIC partitioning mode
  2023-06-22 11:38 ` [PATCH v3 0/4] net/sfc: support KEEP_CRC offload Denis Pryazhennikov
@ 2023-06-22 11:38   ` Denis Pryazhennikov
  2023-06-22 11:38   ` [PATCH v3 2/4] common/sfc_efx/base: detect and report FCS include support Denis Pryazhennikov
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 28+ messages in thread
From: Denis Pryazhennikov @ 2023-06-22 11:38 UTC (permalink / raw)
  To: dev; +Cc: Ferruh Yigit, Andrew Rybchenko, Sandilya Bhagi, Andy Moreton

From: Sandilya Bhagi <sbhagi@solarflare.com>

NIC Partitioning mode in SFC devices means multiple PFs
per network port. When NIC Partitioning is configured,
apart from the privileged adapter(s) the other
unprivileged adapter(s) will share the same physical port.
Determining NIC Partitioning mode is required to take
necessary action(s) for unprivileged adapter to work seamlessly.
NIC Partitioning is determined using heuristic approach.
If the physical ports are shared between PFs then either
NIC Partitioning or SR-IOV is in use.

Signed-off-by: Sandilya Bhagi <sbhagi@solarflare.com>
Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
Reviewed-by: Andy Moreton <amoreton@xilinx.com>
Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
 drivers/common/sfc_efx/base/ef10_nic.c | 101 +++++++++++++++++++++++++
 drivers/common/sfc_efx/base/efx.h      |   8 ++
 2 files changed, 109 insertions(+)

diff --git a/drivers/common/sfc_efx/base/ef10_nic.c b/drivers/common/sfc_efx/base/ef10_nic.c
index e1709d120093..0c6a2eee4453 100644
--- a/drivers/common/sfc_efx/base/ef10_nic.c
+++ b/drivers/common/sfc_efx/base/ef10_nic.c
@@ -1044,6 +1044,83 @@ ef10_mcdi_get_pf_count(
 	return (rc);
 }
 
+static	__checkReturn			efx_rc_t
+ef10_nic_get_physical_port_usage(
+	__in				efx_nic_t *enp,
+	__in_ecount(pfs_to_ports_size)	uint8_t *pfs_to_ports,
+	__in				size_t pfs_to_ports_size,
+	__out				efx_port_usage_t *port_usagep)
+{
+	efx_nic_cfg_t *encp = &(enp->en_nic_cfg);
+	efx_port_usage_t port_usage;
+	uint8_t phy_port;
+	efx_rc_t rc;
+	size_t pf;
+
+	/*
+	 * The sharing of physical ports between functions are determined
+	 * in the following way.
+	 * 1. If VFs are enabled then the physical port is shared.
+	 * 2. Retrieve PFs to ports assignment.
+	 * 3. If PF 0 assignment cannot be retrieved(ACCESS_DENIED), it
+	 *    implies this is an unprivileged function. An unprivileged
+	 *    function indicates the physical port must be shared with
+	 *    another privileged function.
+	 * 4. If PF 0 assignment can be retrieved, it indicates this
+	 *    function is privileged. Now, read all other PF's physical
+	 *    port number assignment and check if the current PF's physical
+	 *    port is shared with any other PF's physical port.
+	 * NOTE: PF 0 is always privileged function.
+	 */
+
+	if (EFX_PCI_FUNCTION_IS_VF(encp)) {
+		port_usage = EFX_PORT_USAGE_SHARED;
+		goto out;
+	}
+
+	if (pfs_to_ports[0] ==
+	    MC_CMD_GET_CAPABILITIES_V2_OUT_ACCESS_NOT_PERMITTED) {
+		/*
+		 * This is unprivileged function as it do not have sufficient
+		 * privileges to read the value, this implies the physical port
+		 * is shared between this function and another privileged
+		 * function
+		 */
+		port_usage = EFX_PORT_USAGE_SHARED;
+		goto out;
+	}
+
+	if (encp->enc_pf >= pfs_to_ports_size) {
+		rc = EINVAL;
+		goto fail1;
+	}
+	phy_port = pfs_to_ports[encp->enc_pf];
+
+	/*
+	 * This is privileged function as it is able read the value of
+	 * PF 0. Now, check if any other function share the same physical
+	 * port number as this function.
+	 */
+	for (pf = 0; pf < pfs_to_ports_size; pf++) {
+		if ((encp->enc_pf != pf) && (phy_port == pfs_to_ports[pf])) {
+			/* Found match, PFs share the same physical port */
+			port_usage = EFX_PORT_USAGE_SHARED;
+			goto out;
+		}
+	}
+
+	port_usage = EFX_PORT_USAGE_EXCLUSIVE;
+
+out:
+	*port_usagep = port_usage;
+	return (0);
+
+fail1:
+	EFSYS_PROBE1(fail1, efx_rc_t, rc);
+
+	return (rc);
+}
+
 static	__checkReturn	efx_rc_t
 ef10_get_datapath_caps(
 	__in		efx_nic_t *enp)
@@ -1307,6 +1384,30 @@ ef10_get_datapath_caps(
 		encp->enc_tunnel_config_udp_entries_max = 0;
 	}
 
+#define CAP_PFS_TO_PORTS(_n)	\
+	(MC_CMD_GET_CAPABILITIES_V2_OUT_PFS_TO_PORTS_ASSIGNMENT_ ## _n)
+
+	encp->enc_port_usage = EFX_PORT_USAGE_UNKNOWN;
+
+	if (req.emr_out_length_used >= MC_CMD_GET_CAPABILITIES_V2_OUT_LEN) {
+		/* PFs to ports assignment */
+		uint8_t pfs_to_ports[CAP_PFS_TO_PORTS(NUM)];
+
+		EFX_STATIC_ASSERT((CAP_PFS_TO_PORTS(NUM) * CAP_PFS_TO_PORTS(LEN)) ==
+		    EFX_ARRAY_SIZE(pfs_to_ports));
+
+		memcpy(pfs_to_ports, MCDI_OUT(req, efx_byte_t, CAP_PFS_TO_PORTS(OFST)),
+		    EFX_ARRAY_SIZE(pfs_to_ports));
+
+		rc = ef10_nic_get_physical_port_usage(enp, pfs_to_ports,
+		    EFX_ARRAY_SIZE(pfs_to_ports), &encp->enc_port_usage);
+		if (rc != 0) {
+			/* PF to port mapping lookup failed */
+			encp->enc_port_usage = EFX_PORT_USAGE_UNKNOWN;
+		}
+	}
+#undef  CAP_PFS_TO_PORTS
+
 	/*
 	 * Check if firmware reports the VI window mode.
 	 * Medford2 has a variable VI window size (8K, 16K or 64K).
diff --git a/drivers/common/sfc_efx/base/efx.h b/drivers/common/sfc_efx/base/efx.h
index f156f8b5b31a..a63211612249 100644
--- a/drivers/common/sfc_efx/base/efx.h
+++ b/drivers/common/sfc_efx/base/efx.h
@@ -311,6 +311,12 @@ efx_nic_check_pcie_link_speed(
 	__in		uint32_t pcie_link_gen,
 	__out		efx_pcie_link_performance_t *resultp);
 
+typedef enum efx_port_usage_e {
+	EFX_PORT_USAGE_UNKNOWN = 0,
+	EFX_PORT_USAGE_EXCLUSIVE,	/* Port only used by this PF */
+	EFX_PORT_USAGE_SHARED,		/* Port shared with other PFs */
+} efx_port_usage_t;
+
 #define	EFX_MAC_ADDR_LEN 6
 
 #if EFSYS_OPT_MCDI
@@ -1682,6 +1688,8 @@ typedef struct efx_nic_cfg_s {
 	uint32_t		enc_assigned_port;
 	/* NIC DMA mapping type */
 	efx_nic_dma_mapping_t	enc_dma_mapping;
+	/* Physical ports shared by PFs */
+	efx_port_usage_t	enc_port_usage;
 } efx_nic_cfg_t;
 
 #define	EFX_PCI_VF_INVALID 0xffff
-- 
2.39.2 (Apple Git-143)


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v3 2/4] common/sfc_efx/base: detect and report FCS include support
  2023-06-22 11:38 ` [PATCH v3 0/4] net/sfc: support KEEP_CRC offload Denis Pryazhennikov
  2023-06-22 11:38   ` [PATCH v3 1/4] common/sfc_efx/base: discover NIC partitioning mode Denis Pryazhennikov
@ 2023-06-22 11:38   ` Denis Pryazhennikov
  2023-06-22 11:38   ` [PATCH v3 3/4] common/sfc_efx/base: add support for configure MAC to keep FCS Denis Pryazhennikov
  2023-06-22 11:38   ` [PATCH v3 4/4] net/sfc: add configurable Rx CRC stripping Denis Pryazhennikov
  3 siblings, 0 replies; 28+ messages in thread
From: Denis Pryazhennikov @ 2023-06-22 11:38 UTC (permalink / raw)
  To: dev; +Cc: Ferruh Yigit, Andrew Rybchenko, Roman Zhukov, Viacheslav Galaktionov

From: Roman Zhukov <roman.zhukov@arknetworks.am>

A new variable was added to efx_nic_cfg_s to detect and
report if FCS is supported by FW.

Signed-off-by: Roman Zhukov <roman.zhukov@arknetworks.am>
Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
Reviewed-by: Viacheslav Galaktionov <viacheslav.galaktionov@arknetworks.am>
Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
 drivers/common/sfc_efx/base/ef10_nic.c  | 6 ++++++
 drivers/common/sfc_efx/base/efx.h       | 1 +
 drivers/common/sfc_efx/base/siena_nic.c | 1 +
 3 files changed, 8 insertions(+)

diff --git a/drivers/common/sfc_efx/base/ef10_nic.c b/drivers/common/sfc_efx/base/ef10_nic.c
index 0c6a2eee4453..bf9cb9d30990 100644
--- a/drivers/common/sfc_efx/base/ef10_nic.c
+++ b/drivers/common/sfc_efx/base/ef10_nic.c
@@ -1244,6 +1244,12 @@ ef10_get_datapath_caps(
 	/* No limit on maximum number of Rx scatter elements per packet. */
 	encp->enc_rx_scatter_max = -1;
 
+	/* Check if the firmware supports include FCS on RX */
+	if (CAP_FLAGS1(req, RX_INCLUDE_FCS))
+		encp->enc_rx_include_fcs_supported = B_TRUE;
+	else
+		encp->enc_rx_include_fcs_supported = B_FALSE;
+
 	/* Check if the firmware supports packed stream mode */
 	if (CAP_FLAGS1(req, RX_PACKED_STREAM))
 		encp->enc_rx_packed_stream_supported = B_TRUE;
diff --git a/drivers/common/sfc_efx/base/efx.h b/drivers/common/sfc_efx/base/efx.h
index a63211612249..b5bd390169ae 100644
--- a/drivers/common/sfc_efx/base/efx.h
+++ b/drivers/common/sfc_efx/base/efx.h
@@ -1629,6 +1629,7 @@ typedef struct efx_nic_cfg_s {
 	/* Datapath firmware vport reconfigure support */
 	boolean_t		enc_vport_reconfigure_supported;
 	boolean_t		enc_rx_disable_scatter_supported;
+	boolean_t		enc_rx_include_fcs_supported;
 	/* Maximum number of Rx scatter segments supported by HW */
 	uint32_t		enc_rx_scatter_max;
 	boolean_t		enc_allow_set_mac_with_installed_filters;
diff --git a/drivers/common/sfc_efx/base/siena_nic.c b/drivers/common/sfc_efx/base/siena_nic.c
index 1f1fb7d2afd1..ca38eefa7e37 100644
--- a/drivers/common/sfc_efx/base/siena_nic.c
+++ b/drivers/common/sfc_efx/base/siena_nic.c
@@ -187,6 +187,7 @@ siena_board_cfg(
 	encp->enc_allow_set_mac_with_installed_filters = B_TRUE;
 	encp->enc_rx_packed_stream_supported = B_FALSE;
 	encp->enc_rx_var_packed_stream_supported = B_FALSE;
+	encp->enc_rx_include_fcs_supported = B_FALSE;
 	encp->enc_rx_es_super_buffer_supported = B_FALSE;
 	encp->enc_fw_subvariant_no_tx_csum_supported = B_FALSE;
 
-- 
2.39.2 (Apple Git-143)


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v3 3/4] common/sfc_efx/base: add support for configure MAC to keep FCS
  2023-06-22 11:38 ` [PATCH v3 0/4] net/sfc: support KEEP_CRC offload Denis Pryazhennikov
  2023-06-22 11:38   ` [PATCH v3 1/4] common/sfc_efx/base: discover NIC partitioning mode Denis Pryazhennikov
  2023-06-22 11:38   ` [PATCH v3 2/4] common/sfc_efx/base: detect and report FCS include support Denis Pryazhennikov
@ 2023-06-22 11:38   ` Denis Pryazhennikov
  2023-06-22 11:38   ` [PATCH v3 4/4] net/sfc: add configurable Rx CRC stripping Denis Pryazhennikov
  3 siblings, 0 replies; 28+ messages in thread
From: Denis Pryazhennikov @ 2023-06-22 11:38 UTC (permalink / raw)
  To: dev; +Cc: Ferruh Yigit, Andrew Rybchenko, Roman Zhukov, Andy Moreton

From: Roman Zhukov <roman.zhukov@arknetworks.am

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>
---
 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(-)

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 b5bd390169ae..ef626cc55a7e 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 09b1e95c594c..92a30c34ae28 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 e91bcbcad863..01113bffa7cb 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;
-- 
2.39.2 (Apple Git-143)


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v3 4/4] net/sfc: add configurable Rx CRC stripping
  2023-06-22 11:38 ` [PATCH v3 0/4] net/sfc: support KEEP_CRC offload Denis Pryazhennikov
                     ` (2 preceding siblings ...)
  2023-06-22 11:38   ` [PATCH v3 3/4] common/sfc_efx/base: add support for configure MAC to keep FCS Denis Pryazhennikov
@ 2023-06-22 11:38   ` Denis Pryazhennikov
  3 siblings, 0 replies; 28+ messages in thread
From: Denis Pryazhennikov @ 2023-06-22 11:38 UTC (permalink / raw)
  To: dev; +Cc: Ferruh Yigit, Andrew Rybchenko, Roman Zhukov, Andy Moreton

Configurable Rx CRC stripping is allowed only if
running firmware variant supports it and if NIC is
configured with single PF per port and without VFs.
When KEEP_CRC is supported, CRC will be part of
the packet payload. The packet length will also
contain CRC length. At the same time, CRC length
should be removed from stats.

Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
Signed-off-by: Roman Zhukov <roman.zhukov@arknetworks.am>
Reviewed-by: Andy Moreton <amoreton@xilinx.com>
Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
 doc/guides/nics/features/sfc.ini       |  1 +
 doc/guides/nics/sfc_efx.rst            |  6 ++++--
 doc/guides/rel_notes/release_23_07.rst |  2 ++
 drivers/net/sfc/sfc.h                  |  1 +
 drivers/net/sfc/sfc_ef100_rx.c         |  2 +-
 drivers/net/sfc/sfc_ef10_rx.c          |  3 ++-
 drivers/net/sfc/sfc_ethdev.c           |  7 ++++++-
 drivers/net/sfc/sfc_port.c             | 12 ++++++++++++
 drivers/net/sfc/sfc_rx.c               |  6 +++++-
 9 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/doc/guides/nics/features/sfc.ini b/doc/guides/nics/features/sfc.ini
index a4b53c619c12..8a9198adcb5b 100644
--- a/doc/guides/nics/features/sfc.ini
+++ b/doc/guides/nics/features/sfc.ini
@@ -23,6 +23,7 @@ RSS key update       = Y
 RSS reta update      = Y
 SR-IOV               = Y
 Flow control         = Y
+CRC offload          = Y
 VLAN offload         = P
 FEC                  = Y
 L3 checksum offload  = Y
diff --git a/doc/guides/nics/sfc_efx.rst b/doc/guides/nics/sfc_efx.rst
index ba82b020932b..24459da33ea8 100644
--- a/doc/guides/nics/sfc_efx.rst
+++ b/doc/guides/nics/sfc_efx.rst
@@ -114,6 +114,10 @@ SFC EFX PMD has support for:
 
 - Loopback
 
+- Configurable Rx CRC stripping (if running firmware variant supports it and
+  if NIC is configured with single PF per port and without VFs, otherwise
+  always stripped)
+
 - SR-IOV PF
 
 - Port representors (see :ref: switch_representation)
@@ -126,8 +130,6 @@ The features not yet supported include:
 
 - Priority-based flow control
 
-- Configurable RX CRC stripping (always stripped)
-
 - Header split on receive
 
 - VLAN filtering
diff --git a/doc/guides/rel_notes/release_23_07.rst b/doc/guides/rel_notes/release_23_07.rst
index 3db443d16018..738d35d9f7c6 100644
--- a/doc/guides/rel_notes/release_23_07.rst
+++ b/doc/guides/rel_notes/release_23_07.rst
@@ -145,6 +145,8 @@ New Features
   * Added support for transfer flow action INDIRECT with subtype COUNT,
     for aggregated statistics.
 
+  * Added support for keeping CRC.
+
 * **Added vmxnet3 version 7 support.**
 
   Added support for vmxnet3 version 7 which includes support
diff --git a/drivers/net/sfc/sfc.h b/drivers/net/sfc/sfc.h
index 89e2c46fa974..25cdeaa5cd25 100644
--- a/drivers/net/sfc/sfc.h
+++ b/drivers/net/sfc/sfc.h
@@ -74,6 +74,7 @@ struct sfc_port {
 
 	unsigned int			flow_ctrl;
 	boolean_t			flow_ctrl_autoneg;
+	boolean_t			include_fcs;
 	size_t				pdu;
 
 	/*
diff --git a/drivers/net/sfc/sfc_ef100_rx.c b/drivers/net/sfc/sfc_ef100_rx.c
index 37b754fa3305..5563bd9a0bd1 100644
--- a/drivers/net/sfc/sfc_ef100_rx.c
+++ b/drivers/net/sfc/sfc_ef100_rx.c
@@ -1004,7 +1004,7 @@ struct sfc_dp_rx sfc_ef100_rx = {
 				  SFC_DP_RX_FEAT_FLOW_MARK |
 				  SFC_DP_RX_FEAT_INTR |
 				  SFC_DP_RX_FEAT_STATS,
-	.dev_offload_capa	= 0,
+	.dev_offload_capa	= RTE_ETH_RX_OFFLOAD_KEEP_CRC,
 	.queue_offload_capa	= RTE_ETH_RX_OFFLOAD_CHECKSUM |
 				  RTE_ETH_RX_OFFLOAD_OUTER_IPV4_CKSUM |
 				  RTE_ETH_RX_OFFLOAD_OUTER_UDP_CKSUM |
diff --git a/drivers/net/sfc/sfc_ef10_rx.c b/drivers/net/sfc/sfc_ef10_rx.c
index 7be224c9c412..30a320d0791c 100644
--- a/drivers/net/sfc/sfc_ef10_rx.c
+++ b/drivers/net/sfc/sfc_ef10_rx.c
@@ -825,7 +825,8 @@ struct sfc_dp_rx sfc_ef10_rx = {
 				  SFC_DP_RX_FEAT_INTR,
 	.dev_offload_capa	= RTE_ETH_RX_OFFLOAD_CHECKSUM |
 				  RTE_ETH_RX_OFFLOAD_OUTER_IPV4_CKSUM |
-				  RTE_ETH_RX_OFFLOAD_RSS_HASH,
+				  RTE_ETH_RX_OFFLOAD_RSS_HASH |
+				  RTE_ETH_RX_OFFLOAD_KEEP_CRC,
 	.queue_offload_capa	= RTE_ETH_RX_OFFLOAD_SCATTER,
 	.get_dev_info		= sfc_ef10_rx_get_dev_info,
 	.qsize_up_rings		= sfc_ef10_rx_qsize_up_rings,
diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index f120ddc5a8d8..1efe64a36a7f 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -689,8 +689,13 @@ sfc_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 
 	sfc_adapter_lock(sa);
 
-	if (have_dp_rx_stats)
+	if (have_dp_rx_stats) {
 		sfc_stats_get_dp_rx(sa, &stats->ipackets, &stats->ibytes);
+		if (dev->data->dev_conf.rxmode.offloads &
+		    RTE_ETH_RX_OFFLOAD_KEEP_CRC) {
+			stats->ibytes -= stats->ipackets * RTE_ETHER_CRC_LEN;
+		}
+	}
 	if (have_dp_tx_stats)
 		sfc_stats_get_dp_tx(sa, &stats->opackets, &stats->obytes);
 
diff --git a/drivers/net/sfc/sfc_port.c b/drivers/net/sfc/sfc_port.c
index 0c887ddedb09..3897facfbc5b 100644
--- a/drivers/net/sfc/sfc_port.c
+++ b/drivers/net/sfc/sfc_port.c
@@ -241,6 +241,11 @@ sfc_port_start(struct sfc_adapter *sa)
 	if (rc != 0)
 		goto fail_mac_pdu_set;
 
+	sfc_log_init(sa, "set include FCS=%u", port->include_fcs);
+	rc = efx_mac_include_fcs_set(sa->nic, port->include_fcs);
+	if (rc != 0)
+		goto fail_include_fcs_set;
+
 	if (!sfc_sa2shared(sa)->isolated) {
 		struct rte_ether_addr *addr = &port->default_mac_addr;
 
@@ -328,6 +333,7 @@ sfc_port_start(struct sfc_adapter *sa)
 	(void)efx_mac_drain(sa->nic, B_TRUE);
 
 fail_mac_drain:
+fail_include_fcs_set:
 fail_mac_stats_upload:
 	(void)efx_mac_stats_periodic(sa->nic, &port->mac_stats_dma_mem,
 				     0, B_FALSE);
@@ -375,11 +381,17 @@ sfc_port_configure(struct sfc_adapter *sa)
 {
 	const struct rte_eth_dev_data *dev_data = sa->eth_dev->data;
 	struct sfc_port *port = &sa->port;
+	const struct rte_eth_rxmode *rxmode = &dev_data->dev_conf.rxmode;
 
 	sfc_log_init(sa, "entry");
 
 	port->pdu = EFX_MAC_PDU(dev_data->mtu);
 
+	if (rxmode->offloads & RTE_ETH_RX_OFFLOAD_KEEP_CRC)
+		port->include_fcs = true;
+	else
+		port->include_fcs = false;
+
 	return 0;
 }
 
diff --git a/drivers/net/sfc/sfc_rx.c b/drivers/net/sfc/sfc_rx.c
index edd0f0c03842..ac94d973de04 100644
--- a/drivers/net/sfc/sfc_rx.c
+++ b/drivers/net/sfc/sfc_rx.c
@@ -654,7 +654,8 @@ struct sfc_dp_rx sfc_efx_rx = {
 	},
 	.features		= SFC_DP_RX_FEAT_INTR,
 	.dev_offload_capa	= RTE_ETH_RX_OFFLOAD_CHECKSUM |
-				  RTE_ETH_RX_OFFLOAD_RSS_HASH,
+				  RTE_ETH_RX_OFFLOAD_RSS_HASH |
+				  RTE_ETH_RX_OFFLOAD_KEEP_CRC,
 	.queue_offload_capa	= RTE_ETH_RX_OFFLOAD_SCATTER,
 	.qsize_up_rings		= sfc_efx_rx_qsize_up_rings,
 	.qcreate		= sfc_efx_rx_qcreate,
@@ -938,6 +939,9 @@ sfc_rx_get_offload_mask(struct sfc_adapter *sa)
 	if (encp->enc_tunnel_encapsulations_supported == 0)
 		no_caps |= RTE_ETH_RX_OFFLOAD_OUTER_IPV4_CKSUM;
 
+	if (encp->enc_rx_include_fcs_supported == 0)
+		no_caps |= RTE_ETH_RX_OFFLOAD_KEEP_CRC;
+
 	return ~no_caps;
 }
 
-- 
2.39.2 (Apple Git-143)


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v4 0/4] net/sfc: support KEEP_CRC offload
  2023-06-01 11:42 [PATCH 0/4] net/sfc: support KEEP_CRC offload Denis Pryazhennikov
                   ` (6 preceding siblings ...)
  2023-06-22 11:38 ` [PATCH v3 0/4] net/sfc: support KEEP_CRC offload Denis Pryazhennikov
@ 2023-06-22 11:47 ` Denis Pryazhennikov
  2023-06-22 11:47   ` [PATCH v4 1/4] common/sfc_efx/base: discover NIC partitioning mode Denis Pryazhennikov
                     ` (4 more replies)
  7 siblings, 5 replies; 28+ messages in thread
From: Denis Pryazhennikov @ 2023-06-22 11:47 UTC (permalink / raw)
  To: dev; +Cc: Ferruh Yigit, Andrew Rybchenko

This patch series adds support for RTE_ETH_RX_OFFLOAD_KEEP_CRC
offload in the SFC driver.

Changes in v2:
* Applied review notes;
* Fixed stats handling;
* Added missing release notes to [4/4].

Changes in v3:
* Fixed wrong indents in [1/4].

Changes in v4:
* Fixed "From" section in [3/4].

Denis Pryazhennikov (3):
  common/sfc_efx/base: detect and report FCS include support
  common/sfc_efx/base: add support for configure MAC to keep FCS
  net/sfc: add configurable Rx CRC stripping

Sandilya Bhagi (1):
  common/sfc_efx/base: discover NIC partitioning mode

 doc/guides/nics/features/sfc.ini        |   1 +
 doc/guides/nics/sfc_efx.rst             |   6 +-
 doc/guides/rel_notes/release_23_07.rst  |   2 +
 drivers/common/sfc_efx/base/ef10_mac.c  |   5 +-
 drivers/common/sfc_efx/base/ef10_nic.c  | 107 ++++++++++++++++++++++++
 drivers/common/sfc_efx/base/efx.h       |  14 ++++
 drivers/common/sfc_efx/base/efx_impl.h  |   1 +
 drivers/common/sfc_efx/base/efx_mac.c   |  48 +++++++++++
 drivers/common/sfc_efx/base/siena_nic.c |   1 +
 drivers/common/sfc_efx/version.map      |   1 +
 drivers/net/sfc/sfc.h                   |   1 +
 drivers/net/sfc/sfc_ef100_rx.c          |   2 +-
 drivers/net/sfc/sfc_ef10_rx.c           |   3 +-
 drivers/net/sfc/sfc_ethdev.c            |   7 +-
 drivers/net/sfc/sfc_port.c              |  12 +++
 drivers/net/sfc/sfc_rx.c                |   6 +-
 16 files changed, 209 insertions(+), 8 deletions(-)

-- 
2.39.2 (Apple Git-143)


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v4 1/4] common/sfc_efx/base: discover NIC partitioning mode
  2023-06-22 11:47 ` [PATCH v4 0/4] net/sfc: support KEEP_CRC offload Denis Pryazhennikov
@ 2023-06-22 11:47   ` Denis Pryazhennikov
  2023-06-22 11:47   ` [PATCH v4 2/4] common/sfc_efx/base: detect and report FCS include support Denis Pryazhennikov
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 28+ messages in thread
From: Denis Pryazhennikov @ 2023-06-22 11:47 UTC (permalink / raw)
  To: dev; +Cc: Ferruh Yigit, Andrew Rybchenko, Sandilya Bhagi, Andy Moreton

From: Sandilya Bhagi <sbhagi@solarflare.com>

NIC Partitioning mode in SFC devices means multiple PFs
per network port. When NIC Partitioning is configured,
apart from the privileged adapter(s) the other
unprivileged adapter(s) will share the same physical port.
Determining NIC Partitioning mode is required to take
necessary action(s) for unprivileged adapter to work seamlessly.
NIC Partitioning is determined using heuristic approach.
If the physical ports are shared between PFs then either
NIC Partitioning or SR-IOV is in use.

Signed-off-by: Sandilya Bhagi <sbhagi@solarflare.com>
Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
Reviewed-by: Andy Moreton <amoreton@xilinx.com>
Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
 drivers/common/sfc_efx/base/ef10_nic.c | 101 +++++++++++++++++++++++++
 drivers/common/sfc_efx/base/efx.h      |   8 ++
 2 files changed, 109 insertions(+)

diff --git a/drivers/common/sfc_efx/base/ef10_nic.c b/drivers/common/sfc_efx/base/ef10_nic.c
index e1709d120093..0c6a2eee4453 100644
--- a/drivers/common/sfc_efx/base/ef10_nic.c
+++ b/drivers/common/sfc_efx/base/ef10_nic.c
@@ -1044,6 +1044,83 @@ ef10_mcdi_get_pf_count(
 	return (rc);
 }
 
+static	__checkReturn			efx_rc_t
+ef10_nic_get_physical_port_usage(
+	__in				efx_nic_t *enp,
+	__in_ecount(pfs_to_ports_size)	uint8_t *pfs_to_ports,
+	__in				size_t pfs_to_ports_size,
+	__out				efx_port_usage_t *port_usagep)
+{
+	efx_nic_cfg_t *encp = &(enp->en_nic_cfg);
+	efx_port_usage_t port_usage;
+	uint8_t phy_port;
+	efx_rc_t rc;
+	size_t pf;
+
+	/*
+	 * The sharing of physical ports between functions are determined
+	 * in the following way.
+	 * 1. If VFs are enabled then the physical port is shared.
+	 * 2. Retrieve PFs to ports assignment.
+	 * 3. If PF 0 assignment cannot be retrieved(ACCESS_DENIED), it
+	 *    implies this is an unprivileged function. An unprivileged
+	 *    function indicates the physical port must be shared with
+	 *    another privileged function.
+	 * 4. If PF 0 assignment can be retrieved, it indicates this
+	 *    function is privileged. Now, read all other PF's physical
+	 *    port number assignment and check if the current PF's physical
+	 *    port is shared with any other PF's physical port.
+	 * NOTE: PF 0 is always privileged function.
+	 */
+
+	if (EFX_PCI_FUNCTION_IS_VF(encp)) {
+		port_usage = EFX_PORT_USAGE_SHARED;
+		goto out;
+	}
+
+	if (pfs_to_ports[0] ==
+	    MC_CMD_GET_CAPABILITIES_V2_OUT_ACCESS_NOT_PERMITTED) {
+		/*
+		 * This is unprivileged function as it do not have sufficient
+		 * privileges to read the value, this implies the physical port
+		 * is shared between this function and another privileged
+		 * function
+		 */
+		port_usage = EFX_PORT_USAGE_SHARED;
+		goto out;
+	}
+
+	if (encp->enc_pf >= pfs_to_ports_size) {
+		rc = EINVAL;
+		goto fail1;
+	}
+	phy_port = pfs_to_ports[encp->enc_pf];
+
+	/*
+	 * This is privileged function as it is able read the value of
+	 * PF 0. Now, check if any other function share the same physical
+	 * port number as this function.
+	 */
+	for (pf = 0; pf < pfs_to_ports_size; pf++) {
+		if ((encp->enc_pf != pf) && (phy_port == pfs_to_ports[pf])) {
+			/* Found match, PFs share the same physical port */
+			port_usage = EFX_PORT_USAGE_SHARED;
+			goto out;
+		}
+	}
+
+	port_usage = EFX_PORT_USAGE_EXCLUSIVE;
+
+out:
+	*port_usagep = port_usage;
+	return (0);
+
+fail1:
+	EFSYS_PROBE1(fail1, efx_rc_t, rc);
+
+	return (rc);
+}
+
 static	__checkReturn	efx_rc_t
 ef10_get_datapath_caps(
 	__in		efx_nic_t *enp)
@@ -1307,6 +1384,30 @@ ef10_get_datapath_caps(
 		encp->enc_tunnel_config_udp_entries_max = 0;
 	}
 
+#define CAP_PFS_TO_PORTS(_n)	\
+	(MC_CMD_GET_CAPABILITIES_V2_OUT_PFS_TO_PORTS_ASSIGNMENT_ ## _n)
+
+	encp->enc_port_usage = EFX_PORT_USAGE_UNKNOWN;
+
+	if (req.emr_out_length_used >= MC_CMD_GET_CAPABILITIES_V2_OUT_LEN) {
+		/* PFs to ports assignment */
+		uint8_t pfs_to_ports[CAP_PFS_TO_PORTS(NUM)];
+
+		EFX_STATIC_ASSERT((CAP_PFS_TO_PORTS(NUM) * CAP_PFS_TO_PORTS(LEN)) ==
+		    EFX_ARRAY_SIZE(pfs_to_ports));
+
+		memcpy(pfs_to_ports, MCDI_OUT(req, efx_byte_t, CAP_PFS_TO_PORTS(OFST)),
+		    EFX_ARRAY_SIZE(pfs_to_ports));
+
+		rc = ef10_nic_get_physical_port_usage(enp, pfs_to_ports,
+		    EFX_ARRAY_SIZE(pfs_to_ports), &encp->enc_port_usage);
+		if (rc != 0) {
+			/* PF to port mapping lookup failed */
+			encp->enc_port_usage = EFX_PORT_USAGE_UNKNOWN;
+		}
+	}
+#undef  CAP_PFS_TO_PORTS
+
 	/*
 	 * Check if firmware reports the VI window mode.
 	 * Medford2 has a variable VI window size (8K, 16K or 64K).
diff --git a/drivers/common/sfc_efx/base/efx.h b/drivers/common/sfc_efx/base/efx.h
index f156f8b5b31a..a63211612249 100644
--- a/drivers/common/sfc_efx/base/efx.h
+++ b/drivers/common/sfc_efx/base/efx.h
@@ -311,6 +311,12 @@ efx_nic_check_pcie_link_speed(
 	__in		uint32_t pcie_link_gen,
 	__out		efx_pcie_link_performance_t *resultp);
 
+typedef enum efx_port_usage_e {
+	EFX_PORT_USAGE_UNKNOWN = 0,
+	EFX_PORT_USAGE_EXCLUSIVE,	/* Port only used by this PF */
+	EFX_PORT_USAGE_SHARED,		/* Port shared with other PFs */
+} efx_port_usage_t;
+
 #define	EFX_MAC_ADDR_LEN 6
 
 #if EFSYS_OPT_MCDI
@@ -1682,6 +1688,8 @@ typedef struct efx_nic_cfg_s {
 	uint32_t		enc_assigned_port;
 	/* NIC DMA mapping type */
 	efx_nic_dma_mapping_t	enc_dma_mapping;
+	/* Physical ports shared by PFs */
+	efx_port_usage_t	enc_port_usage;
 } efx_nic_cfg_t;
 
 #define	EFX_PCI_VF_INVALID 0xffff
-- 
2.39.2 (Apple Git-143)


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v4 2/4] common/sfc_efx/base: detect and report FCS include support
  2023-06-22 11:47 ` [PATCH v4 0/4] net/sfc: support KEEP_CRC offload Denis Pryazhennikov
  2023-06-22 11:47   ` [PATCH v4 1/4] common/sfc_efx/base: discover NIC partitioning mode Denis Pryazhennikov
@ 2023-06-22 11:47   ` Denis Pryazhennikov
  2023-06-22 11:47   ` [PATCH v4 3/4] common/sfc_efx/base: add support for configure MAC to keep FCS Denis Pryazhennikov
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 28+ messages in thread
From: Denis Pryazhennikov @ 2023-06-22 11:47 UTC (permalink / raw)
  To: dev; +Cc: Ferruh Yigit, Andrew Rybchenko, Roman Zhukov, Viacheslav Galaktionov

From: Roman Zhukov <roman.zhukov@arknetworks.am>

A new variable was added to efx_nic_cfg_s to detect and
report if FCS is supported by FW.

Signed-off-by: Roman Zhukov <roman.zhukov@arknetworks.am>
Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
Reviewed-by: Viacheslav Galaktionov <viacheslav.galaktionov@arknetworks.am>
Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
 drivers/common/sfc_efx/base/ef10_nic.c  | 6 ++++++
 drivers/common/sfc_efx/base/efx.h       | 1 +
 drivers/common/sfc_efx/base/siena_nic.c | 1 +
 3 files changed, 8 insertions(+)

diff --git a/drivers/common/sfc_efx/base/ef10_nic.c b/drivers/common/sfc_efx/base/ef10_nic.c
index 0c6a2eee4453..bf9cb9d30990 100644
--- a/drivers/common/sfc_efx/base/ef10_nic.c
+++ b/drivers/common/sfc_efx/base/ef10_nic.c
@@ -1244,6 +1244,12 @@ ef10_get_datapath_caps(
 	/* No limit on maximum number of Rx scatter elements per packet. */
 	encp->enc_rx_scatter_max = -1;
 
+	/* Check if the firmware supports include FCS on RX */
+	if (CAP_FLAGS1(req, RX_INCLUDE_FCS))
+		encp->enc_rx_include_fcs_supported = B_TRUE;
+	else
+		encp->enc_rx_include_fcs_supported = B_FALSE;
+
 	/* Check if the firmware supports packed stream mode */
 	if (CAP_FLAGS1(req, RX_PACKED_STREAM))
 		encp->enc_rx_packed_stream_supported = B_TRUE;
diff --git a/drivers/common/sfc_efx/base/efx.h b/drivers/common/sfc_efx/base/efx.h
index a63211612249..b5bd390169ae 100644
--- a/drivers/common/sfc_efx/base/efx.h
+++ b/drivers/common/sfc_efx/base/efx.h
@@ -1629,6 +1629,7 @@ typedef struct efx_nic_cfg_s {
 	/* Datapath firmware vport reconfigure support */
 	boolean_t		enc_vport_reconfigure_supported;
 	boolean_t		enc_rx_disable_scatter_supported;
+	boolean_t		enc_rx_include_fcs_supported;
 	/* Maximum number of Rx scatter segments supported by HW */
 	uint32_t		enc_rx_scatter_max;
 	boolean_t		enc_allow_set_mac_with_installed_filters;
diff --git a/drivers/common/sfc_efx/base/siena_nic.c b/drivers/common/sfc_efx/base/siena_nic.c
index 1f1fb7d2afd1..ca38eefa7e37 100644
--- a/drivers/common/sfc_efx/base/siena_nic.c
+++ b/drivers/common/sfc_efx/base/siena_nic.c
@@ -187,6 +187,7 @@ siena_board_cfg(
 	encp->enc_allow_set_mac_with_installed_filters = B_TRUE;
 	encp->enc_rx_packed_stream_supported = B_FALSE;
 	encp->enc_rx_var_packed_stream_supported = B_FALSE;
+	encp->enc_rx_include_fcs_supported = B_FALSE;
 	encp->enc_rx_es_super_buffer_supported = B_FALSE;
 	encp->enc_fw_subvariant_no_tx_csum_supported = B_FALSE;
 
-- 
2.39.2 (Apple Git-143)


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v4 3/4] common/sfc_efx/base: add support for configure MAC to keep FCS
  2023-06-22 11:47 ` [PATCH v4 0/4] net/sfc: support KEEP_CRC offload Denis Pryazhennikov
  2023-06-22 11:47   ` [PATCH v4 1/4] common/sfc_efx/base: discover NIC partitioning mode Denis Pryazhennikov
  2023-06-22 11:47   ` [PATCH v4 2/4] common/sfc_efx/base: detect and report FCS include support Denis Pryazhennikov
@ 2023-06-22 11:47   ` Denis Pryazhennikov
  2023-06-22 11:47   ` [PATCH v4 4/4] net/sfc: add configurable Rx CRC stripping Denis Pryazhennikov
  2023-06-22 13:09   ` [PATCH v4 0/4] net/sfc: support KEEP_CRC offload Ferruh Yigit
  4 siblings, 0 replies; 28+ messages in thread
From: Denis Pryazhennikov @ 2023-06-22 11:47 UTC (permalink / raw)
  To: dev; +Cc: Ferruh Yigit, Andrew Rybchenko, Roman Zhukov, Andy Moreton

From: Roman Zhukov <roman.zhukov@arknetworks.am>

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>
---
 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(-)

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 b5bd390169ae..ef626cc55a7e 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 09b1e95c594c..92a30c34ae28 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 e91bcbcad863..01113bffa7cb 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;
-- 
2.39.2 (Apple Git-143)


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v4 4/4] net/sfc: add configurable Rx CRC stripping
  2023-06-22 11:47 ` [PATCH v4 0/4] net/sfc: support KEEP_CRC offload Denis Pryazhennikov
                     ` (2 preceding siblings ...)
  2023-06-22 11:47   ` [PATCH v4 3/4] common/sfc_efx/base: add support for configure MAC to keep FCS Denis Pryazhennikov
@ 2023-06-22 11:47   ` Denis Pryazhennikov
  2023-06-22 13:09   ` [PATCH v4 0/4] net/sfc: support KEEP_CRC offload Ferruh Yigit
  4 siblings, 0 replies; 28+ messages in thread
From: Denis Pryazhennikov @ 2023-06-22 11:47 UTC (permalink / raw)
  To: dev; +Cc: Ferruh Yigit, Andrew Rybchenko, Roman Zhukov, Andy Moreton

Configurable Rx CRC stripping is allowed only if
running firmware variant supports it and if NIC is
configured with single PF per port and without VFs.
When KEEP_CRC is supported, CRC will be part of
the packet payload. The packet length will also
contain CRC length. At the same time, CRC length
should be removed from stats.

Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
Signed-off-by: Roman Zhukov <roman.zhukov@arknetworks.am>
Reviewed-by: Andy Moreton <amoreton@xilinx.com>
Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
 doc/guides/nics/features/sfc.ini       |  1 +
 doc/guides/nics/sfc_efx.rst            |  6 ++++--
 doc/guides/rel_notes/release_23_07.rst |  2 ++
 drivers/net/sfc/sfc.h                  |  1 +
 drivers/net/sfc/sfc_ef100_rx.c         |  2 +-
 drivers/net/sfc/sfc_ef10_rx.c          |  3 ++-
 drivers/net/sfc/sfc_ethdev.c           |  7 ++++++-
 drivers/net/sfc/sfc_port.c             | 12 ++++++++++++
 drivers/net/sfc/sfc_rx.c               |  6 +++++-
 9 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/doc/guides/nics/features/sfc.ini b/doc/guides/nics/features/sfc.ini
index a4b53c619c12..8a9198adcb5b 100644
--- a/doc/guides/nics/features/sfc.ini
+++ b/doc/guides/nics/features/sfc.ini
@@ -23,6 +23,7 @@ RSS key update       = Y
 RSS reta update      = Y
 SR-IOV               = Y
 Flow control         = Y
+CRC offload          = Y
 VLAN offload         = P
 FEC                  = Y
 L3 checksum offload  = Y
diff --git a/doc/guides/nics/sfc_efx.rst b/doc/guides/nics/sfc_efx.rst
index ba82b020932b..24459da33ea8 100644
--- a/doc/guides/nics/sfc_efx.rst
+++ b/doc/guides/nics/sfc_efx.rst
@@ -114,6 +114,10 @@ SFC EFX PMD has support for:
 
 - Loopback
 
+- Configurable Rx CRC stripping (if running firmware variant supports it and
+  if NIC is configured with single PF per port and without VFs, otherwise
+  always stripped)
+
 - SR-IOV PF
 
 - Port representors (see :ref: switch_representation)
@@ -126,8 +130,6 @@ The features not yet supported include:
 
 - Priority-based flow control
 
-- Configurable RX CRC stripping (always stripped)
-
 - Header split on receive
 
 - VLAN filtering
diff --git a/doc/guides/rel_notes/release_23_07.rst b/doc/guides/rel_notes/release_23_07.rst
index 3db443d16018..738d35d9f7c6 100644
--- a/doc/guides/rel_notes/release_23_07.rst
+++ b/doc/guides/rel_notes/release_23_07.rst
@@ -145,6 +145,8 @@ New Features
   * Added support for transfer flow action INDIRECT with subtype COUNT,
     for aggregated statistics.
 
+  * Added support for keeping CRC.
+
 * **Added vmxnet3 version 7 support.**
 
   Added support for vmxnet3 version 7 which includes support
diff --git a/drivers/net/sfc/sfc.h b/drivers/net/sfc/sfc.h
index 89e2c46fa974..25cdeaa5cd25 100644
--- a/drivers/net/sfc/sfc.h
+++ b/drivers/net/sfc/sfc.h
@@ -74,6 +74,7 @@ struct sfc_port {
 
 	unsigned int			flow_ctrl;
 	boolean_t			flow_ctrl_autoneg;
+	boolean_t			include_fcs;
 	size_t				pdu;
 
 	/*
diff --git a/drivers/net/sfc/sfc_ef100_rx.c b/drivers/net/sfc/sfc_ef100_rx.c
index 37b754fa3305..5563bd9a0bd1 100644
--- a/drivers/net/sfc/sfc_ef100_rx.c
+++ b/drivers/net/sfc/sfc_ef100_rx.c
@@ -1004,7 +1004,7 @@ struct sfc_dp_rx sfc_ef100_rx = {
 				  SFC_DP_RX_FEAT_FLOW_MARK |
 				  SFC_DP_RX_FEAT_INTR |
 				  SFC_DP_RX_FEAT_STATS,
-	.dev_offload_capa	= 0,
+	.dev_offload_capa	= RTE_ETH_RX_OFFLOAD_KEEP_CRC,
 	.queue_offload_capa	= RTE_ETH_RX_OFFLOAD_CHECKSUM |
 				  RTE_ETH_RX_OFFLOAD_OUTER_IPV4_CKSUM |
 				  RTE_ETH_RX_OFFLOAD_OUTER_UDP_CKSUM |
diff --git a/drivers/net/sfc/sfc_ef10_rx.c b/drivers/net/sfc/sfc_ef10_rx.c
index 7be224c9c412..30a320d0791c 100644
--- a/drivers/net/sfc/sfc_ef10_rx.c
+++ b/drivers/net/sfc/sfc_ef10_rx.c
@@ -825,7 +825,8 @@ struct sfc_dp_rx sfc_ef10_rx = {
 				  SFC_DP_RX_FEAT_INTR,
 	.dev_offload_capa	= RTE_ETH_RX_OFFLOAD_CHECKSUM |
 				  RTE_ETH_RX_OFFLOAD_OUTER_IPV4_CKSUM |
-				  RTE_ETH_RX_OFFLOAD_RSS_HASH,
+				  RTE_ETH_RX_OFFLOAD_RSS_HASH |
+				  RTE_ETH_RX_OFFLOAD_KEEP_CRC,
 	.queue_offload_capa	= RTE_ETH_RX_OFFLOAD_SCATTER,
 	.get_dev_info		= sfc_ef10_rx_get_dev_info,
 	.qsize_up_rings		= sfc_ef10_rx_qsize_up_rings,
diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index f120ddc5a8d8..1efe64a36a7f 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -689,8 +689,13 @@ sfc_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 
 	sfc_adapter_lock(sa);
 
-	if (have_dp_rx_stats)
+	if (have_dp_rx_stats) {
 		sfc_stats_get_dp_rx(sa, &stats->ipackets, &stats->ibytes);
+		if (dev->data->dev_conf.rxmode.offloads &
+		    RTE_ETH_RX_OFFLOAD_KEEP_CRC) {
+			stats->ibytes -= stats->ipackets * RTE_ETHER_CRC_LEN;
+		}
+	}
 	if (have_dp_tx_stats)
 		sfc_stats_get_dp_tx(sa, &stats->opackets, &stats->obytes);
 
diff --git a/drivers/net/sfc/sfc_port.c b/drivers/net/sfc/sfc_port.c
index 0c887ddedb09..3897facfbc5b 100644
--- a/drivers/net/sfc/sfc_port.c
+++ b/drivers/net/sfc/sfc_port.c
@@ -241,6 +241,11 @@ sfc_port_start(struct sfc_adapter *sa)
 	if (rc != 0)
 		goto fail_mac_pdu_set;
 
+	sfc_log_init(sa, "set include FCS=%u", port->include_fcs);
+	rc = efx_mac_include_fcs_set(sa->nic, port->include_fcs);
+	if (rc != 0)
+		goto fail_include_fcs_set;
+
 	if (!sfc_sa2shared(sa)->isolated) {
 		struct rte_ether_addr *addr = &port->default_mac_addr;
 
@@ -328,6 +333,7 @@ sfc_port_start(struct sfc_adapter *sa)
 	(void)efx_mac_drain(sa->nic, B_TRUE);
 
 fail_mac_drain:
+fail_include_fcs_set:
 fail_mac_stats_upload:
 	(void)efx_mac_stats_periodic(sa->nic, &port->mac_stats_dma_mem,
 				     0, B_FALSE);
@@ -375,11 +381,17 @@ sfc_port_configure(struct sfc_adapter *sa)
 {
 	const struct rte_eth_dev_data *dev_data = sa->eth_dev->data;
 	struct sfc_port *port = &sa->port;
+	const struct rte_eth_rxmode *rxmode = &dev_data->dev_conf.rxmode;
 
 	sfc_log_init(sa, "entry");
 
 	port->pdu = EFX_MAC_PDU(dev_data->mtu);
 
+	if (rxmode->offloads & RTE_ETH_RX_OFFLOAD_KEEP_CRC)
+		port->include_fcs = true;
+	else
+		port->include_fcs = false;
+
 	return 0;
 }
 
diff --git a/drivers/net/sfc/sfc_rx.c b/drivers/net/sfc/sfc_rx.c
index edd0f0c03842..ac94d973de04 100644
--- a/drivers/net/sfc/sfc_rx.c
+++ b/drivers/net/sfc/sfc_rx.c
@@ -654,7 +654,8 @@ struct sfc_dp_rx sfc_efx_rx = {
 	},
 	.features		= SFC_DP_RX_FEAT_INTR,
 	.dev_offload_capa	= RTE_ETH_RX_OFFLOAD_CHECKSUM |
-				  RTE_ETH_RX_OFFLOAD_RSS_HASH,
+				  RTE_ETH_RX_OFFLOAD_RSS_HASH |
+				  RTE_ETH_RX_OFFLOAD_KEEP_CRC,
 	.queue_offload_capa	= RTE_ETH_RX_OFFLOAD_SCATTER,
 	.qsize_up_rings		= sfc_efx_rx_qsize_up_rings,
 	.qcreate		= sfc_efx_rx_qcreate,
@@ -938,6 +939,9 @@ sfc_rx_get_offload_mask(struct sfc_adapter *sa)
 	if (encp->enc_tunnel_encapsulations_supported == 0)
 		no_caps |= RTE_ETH_RX_OFFLOAD_OUTER_IPV4_CKSUM;
 
+	if (encp->enc_rx_include_fcs_supported == 0)
+		no_caps |= RTE_ETH_RX_OFFLOAD_KEEP_CRC;
+
 	return ~no_caps;
 }
 
-- 
2.39.2 (Apple Git-143)


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v4 0/4] net/sfc: support KEEP_CRC offload
  2023-06-22 11:47 ` [PATCH v4 0/4] net/sfc: support KEEP_CRC offload Denis Pryazhennikov
                     ` (3 preceding siblings ...)
  2023-06-22 11:47   ` [PATCH v4 4/4] net/sfc: add configurable Rx CRC stripping Denis Pryazhennikov
@ 2023-06-22 13:09   ` Ferruh Yigit
  4 siblings, 0 replies; 28+ messages in thread
From: Ferruh Yigit @ 2023-06-22 13:09 UTC (permalink / raw)
  To: Denis Pryazhennikov, dev; +Cc: Andrew Rybchenko

On 6/22/2023 12:47 PM, Denis Pryazhennikov wrote:
> This patch series adds support for RTE_ETH_RX_OFFLOAD_KEEP_CRC
> offload in the SFC driver.
> 
> Changes in v2:
> * Applied review notes;
> * Fixed stats handling;
> * Added missing release notes to [4/4].
> 
> Changes in v3:
> * Fixed wrong indents in [1/4].
> 
> Changes in v4:
> * Fixed "From" section in [3/4].
> 
> Denis Pryazhennikov (3):
>   common/sfc_efx/base: detect and report FCS include support
>   common/sfc_efx/base: add support for configure MAC to keep FCS
>   net/sfc: add configurable Rx CRC stripping
> 
> Sandilya Bhagi (1):
>   common/sfc_efx/base: discover NIC partitioning mode
> 

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


^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2023-06-22 13:09 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-01 11:42 [PATCH 0/4] net/sfc: support KEEP_CRC offload Denis Pryazhennikov
2023-06-01 11:42 ` [PATCH 1/4] common/sfc_efx/base: NIC Partitioning mode discovery using heuristic approach Denis Pryazhennikov
2023-06-02  9:43   ` Andrew Rybchenko
2023-06-01 11:42 ` [PATCH 2/4] common/sfc_efx/base: detect and report FCS include support Denis Pryazhennikov
2023-06-07  8:27   ` Andrew Rybchenko
2023-06-01 11:42 ` [PATCH 3/4] common/sfc_efx/base: add support for configure MAC to keep FCS Denis Pryazhennikov
2023-06-07  8:28   ` Andrew Rybchenko
2023-06-01 11:42 ` [PATCH 4/4] net/sfc: add configurable Rx CRC stripping Denis Pryazhennikov
2023-06-07  8:34   ` Andrew Rybchenko
2023-06-08 16:01 ` [PATCH 0/4] net/sfc: support KEEP_CRC offload Ferruh Yigit
2023-06-22  3:47 ` [PATCH v2 " Denis Pryazhennikov
2023-06-22  3:47   ` [PATCH v2 1/4] common/sfc_efx/base: discover NIC partitioning mode Denis Pryazhennikov
2023-06-22  9:48     ` Andrew Rybchenko
2023-06-22  3:47   ` [PATCH v2 2/4] common/sfc_efx/base: detect and report FCS include support Denis Pryazhennikov
2023-06-22  3:47   ` [PATCH v2 3/4] common/sfc_efx/base: add support for configure MAC to keep FCS Denis Pryazhennikov
2023-06-22  3:47   ` [PATCH v2 4/4] net/sfc: add configurable Rx CRC stripping Denis Pryazhennikov
2023-06-22  9:53     ` Andrew Rybchenko
2023-06-22 11:38 ` [PATCH v3 0/4] net/sfc: support KEEP_CRC offload Denis Pryazhennikov
2023-06-22 11:38   ` [PATCH v3 1/4] common/sfc_efx/base: discover NIC partitioning mode Denis Pryazhennikov
2023-06-22 11:38   ` [PATCH v3 2/4] common/sfc_efx/base: detect and report FCS include support Denis Pryazhennikov
2023-06-22 11:38   ` [PATCH v3 3/4] common/sfc_efx/base: add support for configure MAC to keep FCS Denis Pryazhennikov
2023-06-22 11:38   ` [PATCH v3 4/4] net/sfc: add configurable Rx CRC stripping Denis Pryazhennikov
2023-06-22 11:47 ` [PATCH v4 0/4] net/sfc: support KEEP_CRC offload Denis Pryazhennikov
2023-06-22 11:47   ` [PATCH v4 1/4] common/sfc_efx/base: discover NIC partitioning mode Denis Pryazhennikov
2023-06-22 11:47   ` [PATCH v4 2/4] common/sfc_efx/base: detect and report FCS include support Denis Pryazhennikov
2023-06-22 11:47   ` [PATCH v4 3/4] common/sfc_efx/base: add support for configure MAC to keep FCS Denis Pryazhennikov
2023-06-22 11:47   ` [PATCH v4 4/4] net/sfc: add configurable Rx CRC stripping Denis Pryazhennikov
2023-06-22 13:09   ` [PATCH v4 0/4] net/sfc: support KEEP_CRC offload Ferruh Yigit

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).