Hi Nelio, Friday, July 13, 2018 4:54 PM, Nelio Laranjeiro: > Subject: [PATCH] net/mlx5: fix representors detection > > On systems where the required Netlink commands are not supported but > Mellanox OFED is installed, representors information must be retrieved > through sysfs. > > Fixes: 4c10141488fe ("net/mlx5: add port representor awareness") > > Signed-off-by: Nelio Laranjeiro <nelio.laranje...@6wind.com> > Acked-by: Adrien Mazarguil <adrien.mazarg...@6wind.com> > --- > drivers/net/mlx5/mlx5.c | 7 +++-- > drivers/net/mlx5/mlx5.h | 2 ++ > drivers/net/mlx5/mlx5_ethdev.c | 53 > ++++++++++++++++++++++++++++++++++ > 3 files changed, 60 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c index > df7f39844..8174947f3 100644 > --- a/drivers/net/mlx5/mlx5.c > +++ b/drivers/net/mlx5/mlx5.c > @@ -1306,7 +1306,8 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv > __rte_unused, > * Netlink calls assuming kernel drivers are recent enough to > * support them. > * > - * In the event of identification failure through Netlink, either: > + * In the event of identification failure through Netlink, try again > + * through sysfs, then either: > * > * 1. No device matches (n == 0), complain and bail out. > * 2. A single IB device matches (n == 1) and is not a representor, @@ > -1325,7 +1326,9 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv > __rte_unused, > if (nl_route < 0 || > !list[i].ifindex || > mlx5_nl_switch_info(nl_route, list[i].ifindex, > - &list[i].info)) { > + &list[i].info) || > + ((!list[i].info.representor && !list[i].info.master) && > + mlx5_sysfs_switch_info(list[i].ifindex, &list[i].info))) {
I find this logic a bit complex to be under a single if statement. Can you simplify it w/ local variables? > list[i].ifindex = 0; > memset(&list[i].info, 0, sizeof(list[i].info)); > continue; > diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index > cc01310e0..fff88b1d6 100644 > --- a/drivers/net/mlx5/mlx5.h > +++ b/drivers/net/mlx5/mlx5.h > @@ -254,6 +254,8 @@ eth_rx_burst_t mlx5_select_rx_function(struct > rte_eth_dev *dev); unsigned int mlx5_dev_to_port_id(const struct > rte_device *dev, > uint16_t *port_list, > unsigned int port_list_n); > +int mlx5_sysfs_switch_info(unsigned int ifindex, > + struct mlx5_switch_info *info); > > /* mlx5_mac.c */ > > diff --git a/drivers/net/mlx5/mlx5_ethdev.c > b/drivers/net/mlx5/mlx5_ethdev.c index 05f66f7b6..101be4791 100644 > --- a/drivers/net/mlx5/mlx5_ethdev.c > +++ b/drivers/net/mlx5/mlx5_ethdev.c > @@ -1296,3 +1296,56 @@ mlx5_dev_to_port_id(const struct rte_device > *dev, uint16_t *port_list, > } > return n; > } > + > +/** > + * Get switch information associated with network interface. > + * > + * @param ifindex > + * Network interface index. > + * @param[out] info > + * Switch information object, populated in case of success. > + * > + * @return > + * 0 on success, a negative errno value otherwise and rte_errno is set. > + */ > +int > +mlx5_sysfs_switch_info(unsigned int ifindex, struct mlx5_switch_info > +*info) { > + char ifname[IF_NAMESIZE]; > + FILE *file; > + struct mlx5_switch_info data = { .master = 0, }; > + bool port_name_set = false; > + bool port_switch_id_set = false; > + char c; > + > + if (!if_indextoname(ifindex, ifname)) { > + rte_errno = errno; > + return -rte_errno; > + } > + > + MKSTR(phys_port_name, "/sys/class/net/%s/phys_port_name", > + ifname); > + MKSTR(phys_switch_id, "/sys/class/net/%s/phys_switch_id", > + ifname); > + > + file = fopen(phys_port_name, "rb"); > + if (file == NULL) > + goto error; This goto is not correct. Representors will indeed be able to read, but PFs will not be able to open the phys_port_name (as there is none). The phys_switch_id is accessible form both. I pasted a suggestion below[1], let me know what you think. > + port_name_set = fscanf(file, "%d%c", &data.port_name, &c) == 2 > && > + c == '\n'; > + fclose(file); > + file = fopen(phys_switch_id, "rb"); > + if (file == NULL) > + goto error; > + port_switch_id_set = > + fscanf(file, "%" SCNx64 "%c", &data.switch_id, &c) == 2 && > + c == '\n'; > + fclose(file); > + data.master = port_switch_id_set && !port_name_set; > + data.representor = port_switch_id_set && port_name_set; > + *info = data; > + return 0; > +error: > + rte_errno = errno; > + return -rte_errno; > +} > -- > 2.18.0 [1] diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c index 6cfe02d..b64eb20 100644 --- a/drivers/net/mlx5/mlx5_ethdev.c +++ b/drivers/net/mlx5/mlx5_ethdev.c @@ -1328,23 +1328,20 @@ struct ethtool_link_settings { ifname); file = fopen(phys_port_name, "rb"); - if (file == NULL) - goto error; - port_name_set = fscanf(file, "%d%c", &data.port_name, &c) == 2 && - c == '\n'; - fclose(file); + if (file) { + port_name_set = fscanf(file, "%d%c", &data.port_name, &c) == 2 && + c == '\n'; + fclose(file); + } file = fopen(phys_switch_id, "rb"); - if (file == NULL) - goto error; - port_switch_id_set = - fscanf(file, "%" SCNx64 "%c", &data.switch_id, &c) == 2 && - c == '\n'; - fclose(file); + if (file) { + port_switch_id_set = + fscanf(file, "%" SCNx64 "%c", &data.switch_id, &c) == 2 && + c == '\n'; + fclose(file); + } data.master = port_switch_id_set && !port_name_set; data.representor = port_switch_id_set && port_name_set;