[v2] common/mlx5: fix the sysfs port name translation
Checks
Commit Message
With some OFED or upstream kernel of mlx5, the port name fetched from
"/sys/class/net/[DEV]/phys_port_name" may have a tailing "\n" as the
EOL. The sscanf() will return the scanned items number with this EOL.
In such case, the "equal to" condition is considered as false and
the function mlx5_translate_port_name() will recognize the port type
wrongly with UNKNOWN result.
The tailing carriage return character should be removed before
calling the mlx5_translate_port_name(), this was already done in the
NL message handling. In the meanwhile, the possible incorrect line
feed character is also taken into consideration.
Fixes: 654810b56828 ("common/mlx5: share Netlink commands")
Fixes: 420bbdae89f2 ("net/mlx5: fix host physical function representor naming")
Cc: stable@dpdk.org
Signed-off-by: Bing Zhao <bingz@nvidia.com>
---
drivers/net/mlx5/linux/mlx5_ethdev_os.c | 24 ++++++++++++++++++++++--
1 file changed, 22 insertions(+), 2 deletions(-)
Comments
On Wed, 22 Mar 2023 13:34:12 +0200
Bing Zhao <bingz@nvidia.com> wrote:
> +
> + port_name = (char *)malloc(IF_NAMESIZE);
> + if (port_name == NULL) {
> + fclose(file);
> + rte_errno = ENOMEM;
> + return -rte_errno;
> + }
> + line_size = getline(&port_name, &port_name_size, file);
This code is doing unnecessary extra work here.
From getline() man page:
If *lineptr is set to NULL before the call, then getline() will
allocate a buffer for storing the line. This buffer should be
freed by the user program even if getline() failed.
On Wed, 22 Mar 2023 13:34:12 +0200
Bing Zhao <bingz@nvidia.com> wrote:
> + tail_nl = strchr(port_name, '\r');
> + if (tail_nl)
> + *tail_nl = '\0';
This is a file provided by Linux kernel, there is no way
it could have a CR character.
Hi Stephen,
> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Wednesday, March 22, 2023 10:59 PM
> To: Bing Zhao <bingz@nvidia.com>
> Cc: Slava Ovsiienko <viacheslavo@nvidia.com>; Matan Azrad
> <matan@nvidia.com>; dev@dpdk.org; Raslan Darawsheh
> <rasland@nvidia.com>; stable@dpdk.org
> Subject: Re: [PATCH v2] common/mlx5: fix the sysfs port name translation
>
> External email: Use caution opening links or attachments
>
>
> On Wed, 22 Mar 2023 13:34:12 +0200
> Bing Zhao <bingz@nvidia.com> wrote:
>
> > +
> > + port_name = (char *)malloc(IF_NAMESIZE);
> > + if (port_name == NULL) {
> > + fclose(file);
> > + rte_errno = ENOMEM;
> > + return -rte_errno;
> > + }
> > + line_size = getline(&port_name, &port_name_size, file);
>
> This code is doing unnecessary extra work here.
> From getline() man page:
Yes, I also checked the description of this interface before. The original purpose was to allocate / free the memory explicitly.
This will be removed.
>
> If *lineptr is set to NULL before the call, then getline() will
> allocate a buffer for storing the line. This buffer should be
> freed by the user program even if getline() failed.
Hi Stephen,
> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Wednesday, March 22, 2023 11:00 PM
> To: Bing Zhao <bingz@nvidia.com>
> Cc: Slava Ovsiienko <viacheslavo@nvidia.com>; Matan Azrad
> <matan@nvidia.com>; dev@dpdk.org; Raslan Darawsheh
> <rasland@nvidia.com>; stable@dpdk.org
> Subject: Re: [PATCH v2] common/mlx5: fix the sysfs port name translation
>
> External email: Use caution opening links or attachments
>
>
> On Wed, 22 Mar 2023 13:34:12 +0200
> Bing Zhao <bingz@nvidia.com> wrote:
>
> > + tail_nl = strchr(port_name, '\r');
> > + if (tail_nl)
> > + *tail_nl = '\0';
>
> This is a file provided by Linux kernel, there is no way it could have a CR
> character.
Correct, thanks for pointing this. I was overanxious. I will remove this as well.
@@ -1035,7 +1035,8 @@ int
mlx5_sysfs_switch_info(unsigned int ifindex, struct mlx5_switch_info *info)
{
char ifname[IF_NAMESIZE];
- char port_name[IF_NAMESIZE];
+ char *port_name;
+ size_t port_name_size = 0;
FILE *file;
struct mlx5_switch_info data = {
.master = 0,
@@ -1048,6 +1049,7 @@ mlx5_sysfs_switch_info(unsigned int ifindex, struct mlx5_switch_info *info)
bool port_switch_id_set = false;
bool device_dir = false;
char c;
+ ssize_t line_size;
if (!if_indextoname(ifindex, ifname)) {
rte_errno = errno;
@@ -1063,8 +1065,26 @@ mlx5_sysfs_switch_info(unsigned int ifindex, struct mlx5_switch_info *info)
file = fopen(phys_port_name, "rb");
if (file != NULL) {
- if (fgets(port_name, IF_NAMESIZE, file) != NULL)
+ char *tail_nl;
+
+ port_name = (char *)malloc(IF_NAMESIZE);
+ if (port_name == NULL) {
+ fclose(file);
+ rte_errno = ENOMEM;
+ return -rte_errno;
+ }
+ line_size = getline(&port_name, &port_name_size, file);
+ if (line_size > 0) {
+ /* Remove tailing newline character. */
+ tail_nl = strchr(port_name, '\n');
+ if (tail_nl)
+ *tail_nl = '\0';
+ tail_nl = strchr(port_name, '\r');
+ if (tail_nl)
+ *tail_nl = '\0';
mlx5_translate_port_name(port_name, &data);
+ }
+ free(port_name);
fclose(file);
}
file = fopen(phys_switch_id, "rb");