Thanks, PSB. > -----Original Message----- > From: Shahaf Shuler > Sent: Thursday, March 7, 2019 10:58 AM > To: Dekel Peled <dek...@mellanox.com>; Yongseok Koh > <ys...@mellanox.com> > Cc: dev@dpdk.org; Ori Kam <or...@mellanox.com>; Dekel Peled > <dek...@mellanox.com> > Subject: RE: [dpdk-dev] [PATCH v2] net/mlx5: support new representors' > naming format > > Sunday, March 3, 2019 9:44 AM, Dekel Peled: > > Subject: [dpdk-dev] [PATCH v2] net/mlx5: support new representors' > > naming format > > Representors' -> representor I'll amend it.
> > > > > 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. I'll add it. > > > > > [1] > > > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit > > > hub.com%2Ftorvalds%2Flinux%2Fcommit%2Fc12ecc2&data=02%7C01% > > > 7Cshahafs%40mellanox.com%7C98910a4923c64d7af0cc08d69fac0de0%7Ca65 > > > 2971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636871958646189312& > > > sdata=vMvheEzZJEWRTtSK7z2qjJSxR2o50KnLOGATBAVfueM%3D&reser > > ved=0 > > [2] > > > https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmail > > s.dpdk.org%2Farchives%2Fdev%2F2019- > > > March%2F125676.html&data=02%7C01%7Cshahafs%40mellanox.com%7 > > > C98910a4923c64d7af0cc08d69fac0de0%7Ca652971c7d2e4d9ba6a4d149256f46 > > > 1b%7C0%7C0%7C636871958646189312&sdata=UWxPP9SXclFAz23dtBHU > > eIF7v9ZRqbEU4nXUsirDr%2Bg%3D&reserved=0 > > > > --- > > v2: Use public link to kernel patch, add link to RFC. > > --- > > > > Signed-off-by: Dekel Peled <dek...@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. I'll reorder. > > > 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? I don't need this name, only the representors names. e.g. pf0vf0 will result in name "0". P0 will also result in name "0" but it is not valid. > > > + */ > > + 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. I'll reorder. > > > - 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? See comment above. > > > + 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
