DPDK patches and discussions
 help / color / mirror / Atom feed
From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
To: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>, dev@dpdk.org
Cc: Viacheslav Galaktionov <viacheslav.galaktionov@arknetworks.am>,
	Ferruh Yigit <ferruh.yigit@amd.com>,
	Sandilya Bhagi <sbhagi@solarflare.com>,
	Andy Moreton <amoreton@xilinx.com>
Subject: Re: [PATCH 1/4] common/sfc_efx/base: NIC Partitioning mode discovery using heuristic approach
Date: Fri, 2 Jun 2023 12:43:58 +0300	[thread overview]
Message-ID: <50881453-53c4-4179-cceb-dd6b75fe6f7c@oktetlabs.ru> (raw)
In-Reply-To: <20230601114220.17796-2-denis.pryazhennikov@arknetworks.am>

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


  reply	other threads:[~2023-06-02  9:44 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=50881453-53c4-4179-cceb-dd6b75fe6f7c@oktetlabs.ru \
    --to=andrew.rybchenko@oktetlabs.ru \
    --cc=amoreton@xilinx.com \
    --cc=denis.pryazhennikov@arknetworks.am \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@amd.com \
    --cc=sbhagi@solarflare.com \
    --cc=viacheslav.galaktionov@arknetworks.am \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).