[v2] net/mlx5: support new representors' naming format
Checks
Commit Message
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.
[1] https://github.com/torvalds/linux/commit/c12ecc2
[2] http://mails.dpdk.org/archives/dev/2019-March/125676.html
---
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(-)
Comments
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&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 <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
Thanks, PSB.
> -----Original Message-----
> From: Shahaf Shuler
> Sent: Thursday, March 7, 2019 10:58 AM
> To: Dekel Peled <dekelp@mellanox.com>; Yongseok Koh
> <yskoh@mellanox.com>
> Cc: 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
>
> 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 <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.
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
@@ -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 */
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)
+ */
+ 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");
@@ -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);
- 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);
+ port_name_set = true;
+ }
+ if (errno)
+ goto error;
+ } else {
+ port_name_set = true;
+ }
break;
case IFLA_PHYS_SWITCH_ID:
info.switch_id = 0;