DPDK patches and discussions
 help / color / mirror / Atom feed
From: Shahaf Shuler <shahafs@mellanox.com>
To: Dekel Peled <dekelp@mellanox.com>, Yongseok Koh <yskoh@mellanox.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, Ori Kam <orika@mellanox.com>,
	Dekel Peled <dekelp@mellanox.com>
Subject: Re: [dpdk-dev] [PATCH v2] net/mlx5: support new representors' naming	format
Date: Thu, 7 Mar 2019 08:57:34 +0000	[thread overview]
Message-ID: <AM0PR0502MB3795A9041B5D6F95B7F0C8B6C34C0@AM0PR0502MB3795.eurprd05.prod.outlook.com> (raw)
In-Reply-To: <1551599028-14591-1-git-send-email-dekelp@mellanox.com>

Sunday, March 3, 2019 9:44 AM, Dekel Peled:
> Subject: [dpdk-dev] [PATCH v2] net/mlx5: support new representors'
> naming format

Representors' -> representor 

> 
> Kernel update [1] introduce new format of representors names.
> This patch implements RFC [2], updating MLX5 PMD to support the new
> format, while maintaining support of the existing format.

As a high level comment, I would expect to have some function called translate_phys_port_name to translate the netlink/sysfs output to the representor number.
This function should be called from both sysfs and netlink routines. 

> 
> [1]
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit
> hub.com%2Ftorvalds%2Flinux%2Fcommit%2Fc12ecc2&amp;data=02%7C01%
> 7Cshahafs%40mellanox.com%7C98910a4923c64d7af0cc08d69fac0de0%7Ca65
> 2971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636871958646189312&amp;
> sdata=vMvheEzZJEWRTtSK7z2qjJSxR2o50KnLOGATBAVfueM%3D&amp;reser
> ved=0
> [2]
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmail
> s.dpdk.org%2Farchives%2Fdev%2F2019-
> March%2F125676.html&amp;data=02%7C01%7Cshahafs%40mellanox.com%7
> C98910a4923c64d7af0cc08d69fac0de0%7Ca652971c7d2e4d9ba6a4d149256f46
> 1b%7C0%7C0%7C636871958646189312&amp;sdata=UWxPP9SXclFAz23dtBHU
> eIF7v9ZRqbEU4nXUsirDr%2Bg%3D&amp;reserved=0
> 
> ---
> v2: Use public link to kernel patch, add link to RFC.
> ---
> 
> Signed-off-by: Dekel Peled <dekelp@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5_ethdev.c | 14 +++++++++++++-
>  drivers/net/mlx5/mlx5_nl.c     | 20 +++++++++++++++-----
>  2 files changed, 28 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_ethdev.c
> b/drivers/net/mlx5/mlx5_ethdev.c index 795c9c7..664f485 100644
> --- a/drivers/net/mlx5/mlx5_ethdev.c
> +++ b/drivers/net/mlx5/mlx5_ethdev.c
> @@ -1359,7 +1359,8 @@ int mlx5_fw_version_get(struct rte_eth_dev *dev,
> char *fw_ver, size_t fw_size)
>  	bool port_name_set = false;
>  	bool port_switch_id_set = false;
>  	bool device_dir = false;
> -	char c;
> +	char c, pf_c1, pf_c2, vf_c1, vf_c2;
> +	int32_t pf_num;
> 
>  	if (!if_indextoname(ifindex, ifname)) {
>  		rte_errno = errno;
> @@ -1375,9 +1376,20 @@ int mlx5_fw_version_get(struct rte_eth_dev
> *dev, char *fw_ver, size_t fw_size)
> 
>  	file = fopen(phys_port_name, "rb");
>  	if (file != NULL) {
> +		/* Check for port-name as a number (support kernel ver <
> 5.0 */

Logically it should be the other way around.
First check for new kernel, if not, fall back to older one. 

>  		port_name_set =
>  			fscanf(file, "%d%c", &data.port_name, &c) == 2 &&
>  			c == '\n';
> +		if (!port_name_set) {
> +			/*
> +			 * Check for port-name as a string of the form pf0vf0
> +			 * (support kernel ver >= 5.0)

According to the kernel commit, naming of type p0 or p1 (for the uplink vport). Will it work w/ below fscanf? 

> +			 */
> +			port_name_set = (fscanf(file, "%c%c%d%c%c%d%c",
> &pf_c1,
> +						&pf_c2, &pf_num, &vf_c1,
> &vf_c2,
> +						&data.port_name, &c) == 7)
> &&
> +					c == '\n';
> +		}


>  		fclose(file);
>  	}
>  	file = fopen(phys_switch_id, "rb");
> diff --git a/drivers/net/mlx5/mlx5_nl.c b/drivers/net/mlx5/mlx5_nl.c index
> 0bf6845..cc7b4b8 100644
> --- a/drivers/net/mlx5/mlx5_nl.c
> +++ b/drivers/net/mlx5/mlx5_nl.c
> @@ -849,7 +849,7 @@ struct mlx5_nl_ifindex_data {
>  	while (off < nh->nlmsg_len) {
>  		struct rtattr *ra = (void *)((uintptr_t)nh + off);
>  		void *payload = RTA_DATA(ra);
> -		char *end;
> +		char *end, *vf_str;
>  		unsigned int i;
> 
>  		if (ra->rta_len > nh->nlmsg_len - off) @@ -861,10 +861,20
> @@ struct mlx5_nl_ifindex_data {
>  		case IFLA_PHYS_PORT_NAME:
>  			errno = 0;
>  			info.port_name = strtol(payload, &end, 0);

Again, the logic should be first try the new naming and fallback to old ones. 

> -			if (errno ||
> -			    (size_t)(end - (char *)payload) != strlen(payload))
> -				goto error;
> -			port_name_set = true;
> +			if (errno || ((size_t)(end - (char *)payload) !=
> +				      strlen(payload))) {
> +				vf_str = strstr(payload, "vf");
> +				if (vf_str) {
> +					errno = 0;
> +					info.port_name = strtol(vf_str + 2,
> +								&end, 0);

Again, true also in case port name is p0? 

> +					port_name_set = true;
> +				}
> +				if (errno)
> +					goto error;
> +			} else {
> +				port_name_set = true;
> +			}
>  			break;
>  		case IFLA_PHYS_SWITCH_ID:
>  			info.switch_id = 0;
> --
> 1.8.3.1

  reply	other threads:[~2019-03-07  8:57 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-28 15:25 [dpdk-dev] [PATCH] " Dekel Peled
2019-03-03  7:43 ` [dpdk-dev] [PATCH v2] " Dekel Peled
2019-03-07  8:57   ` Shahaf Shuler [this message]
2019-03-10 14:55     ` Dekel Peled
2019-03-10 15:21   ` [dpdk-dev] [PATCH v3] net/mlx5: support new representor " Dekel Peled
2019-03-10 16:06     ` [dpdk-dev] [PATCH v4] " Dekel Peled
2019-03-17  6:23       ` [dpdk-dev] [PATCH v5] " Dekel Peled
2019-03-17  6:23         ` Dekel Peled
2019-03-18  6:29         ` Slava Ovsiienko
2019-03-18  6:29           ` Slava Ovsiienko
2019-03-18  6:59           ` Shahaf Shuler
2019-03-18  6:59             ` Shahaf Shuler

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=AM0PR0502MB3795A9041B5D6F95B7F0C8B6C34C0@AM0PR0502MB3795.eurprd05.prod.outlook.com \
    --to=shahafs@mellanox.com \
    --cc=dekelp@mellanox.com \
    --cc=dev@dpdk.org \
    --cc=orika@mellanox.com \
    --cc=yskoh@mellanox.com \
    /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).