[PATCH net-next] Improving `ip link show` output in iproute2 for PF/VF association

[PATCH net-next] Improving `ip link show` output in iproute2 for PF/VF association

[skakishi]

Hello all,

The current version of iproute2 does not display clear associations between
Physical Functions (PFs) and Virtual Functions (VFs). To address this limitation,
I've added the following enhancement.

Proposed Changes
When the 'detail' option is enabled, the command will display additional details,
including VF IDs and their respective names.

For non-SR-IOV legacy mode:
$ ip link show -d
...
1: enp6s0f0np0: ...
vf 0 link/ether ... name enp6s0f1v0
vf 1 link/ether ... name enp6s0f1v1
...

For SR-IOV switchdev mode (including VF representor information):
$ ip link show -d
...
1: enp6s0f0np0: ...
vf 0 link/ether ... name enp6s0f0v0, representor enp6s0f0npf0vf0
vf 1 link/ether ... name enp6s0f0v1, representor enp6s0f0npf0vf1
...

Technical Details
I've taken the additional data from sysfs.

Current Work Status
I am actively working on implementing this extension.

Request for Feedback
Do you find this feature useful?
Do you think getting data from sysfs is an appropriate approach? Are there any
alternative methods you would recommend?

Best regards,
---
 ip/ipaddress.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 77 insertions(+), 2 deletions(-)

diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 8197709d..d48fc04e 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -19,6 +19,7 @@
 #include <arpa/inet.h>
 #include <string.h>
 #include <fnmatch.h>
+#include <dirent.h>
 
 #include <linux/netdevice.h>
 #include <linux/if_arp.h>
@@ -347,7 +348,76 @@ static void print_af_spec(FILE *fp, struct rtattr *af_spec_attr)
 
 static void print_vf_stats64(FILE *fp, struct rtattr *vfstats);
 
-static void print_vfinfo(FILE *fp, struct ifinfomsg *ifi, struct rtattr *vfinfo)
+static const char *SYS_CLASS_NET_PATH = "/sys/class/net";
+
+static void print_vf_names(const char *pfname, int vf_index)
+{
+	char vf_list_path[256];
+
+	snprintf(vf_list_path, sizeof(vf_list_path),
+		 "%s/%s/device/virtfn%d/net", SYS_CLASS_NET_PATH, pfname,
+		 vf_index);
+	DIR *vf_dir = opendir(vf_list_path);
+	if (vf_dir) {
+		struct dirent *vf_entry;
+		while ((vf_entry = readdir(vf_dir))) {
+			if (vf_entry->d_name[0] == '.')
+				continue;
+			print_string(PRINT_ANY, "name", ", name %s",
+				     vf_entry->d_name);
+			break;
+		}
+		closedir(vf_dir);
+	}
+}
+
+static void print_vf_representor(const char *pfname, int vf_index)
+{
+	char pf_device_path[512], vf_device_path[512];
+	char pf_link_target[256] = { 0 }, vf_link_target[256] = { 0 };
+	int found_count = 0;
+
+	snprintf(pf_device_path, sizeof(pf_device_path), "%s/%s/device",
+		 SYS_CLASS_NET_PATH, pfname);
+
+	if (readlink(pf_device_path, pf_link_target,
+		     sizeof(pf_link_target) - 1) == -1) {
+		perror("Could not read PF device symbolic link");
+		return;
+	}
+
+	DIR *net_dir = opendir(SYS_CLASS_NET_PATH);
+	if (!net_dir) {
+		perror("Could not open net directory");
+		return;
+	}
+
+	struct dirent *net_entry;
+	while ((net_entry = readdir(net_dir))) {
+		if (net_entry->d_name[0] == '.' ||
+		    strcmp(net_entry->d_name, pfname) == 0)
+			continue;
+
+		snprintf(vf_device_path, sizeof(vf_device_path), "%s/%s/device",
+			 SYS_CLASS_NET_PATH, net_entry->d_name);
+		if (readlink(vf_device_path, vf_link_target,
+			     sizeof(vf_link_target) - 1) == -1)
+			continue;
+
+		if (strcmp(pf_link_target, vf_link_target) != 0)
+			continue;
+
+		found_count++;
+		if (found_count == vf_index + 1) {
+			print_string(PRINT_ANY, "representor",
+				     ", representor %s", net_entry->d_name);
+			break;
+		}
+	}
+	closedir(net_dir);
+}
+
+static void print_vfinfo(FILE *fp, const char *pfname, struct ifinfomsg *ifi, struct rtattr *vfinfo, bool show_details)
 {
 	struct ifla_vf_mac *vf_mac;
 	struct ifla_vf_broadcast *vf_broadcast;
@@ -541,6 +611,11 @@ static void print_vfinfo(FILE *fp, struct ifinfomsg *ifi, struct rtattr *vfinfo)
 				   rss_query->setting);
 	}
 
+	if (show_details) {
+		print_vf_names(pfname, vf_mac->vf);
+		print_vf_representor(pfname, vf_mac->vf);
+	}
+
 	if (vf[IFLA_VF_STATS] && show_stats)
 		print_vf_stats64(fp, vf[IFLA_VF_STATS]);
 }
@@ -1343,7 +1418,7 @@ int print_linkinfo(struct nlmsghdr *n, void *arg)
 		open_json_array(PRINT_JSON, "vfinfo_list");
 		for (i = RTA_DATA(vflist); RTA_OK(i, rem); i = RTA_NEXT(i, rem)) {
 			open_json_object(NULL);
-			print_vfinfo(fp, ifi, i);
+			print_vfinfo(fp, name, ifi, i, show_details);
 			close_json_object();
 			count++;
 		}
-- 
2.39.2 (Apple Git-143)

Re: [PATCH net-next] Improving `ip link show` output in iproute2 for PF/VF association

[Stephen Hemminger]

On Wed, 20 Sep 2023 14:28:27 +0900
skakishi <skakishi@yahoo-corp.jp> wrote:

> Hello all,
> 
> The current version of iproute2 does not display clear associations between
> Physical Functions (PFs) and Virtual Functions (VFs). To address this limitation,
> I've added the following enhancement.
> 
> Proposed Changes
> When the 'detail' option is enabled, the command will display additional details,
> including VF IDs and their respective names.
> 
> For non-SR-IOV legacy mode:
> $ ip link show -d
> ...
> 1: enp6s0f0np0: ...
> vf 0 link/ether ... name enp6s0f1v0
> vf 1 link/ether ... name enp6s0f1v1
> ...
> 
> For SR-IOV switchdev mode (including VF representor information):
> $ ip link show -d
> ...
> 1: enp6s0f0np0: ...
> vf 0 link/ether ... name enp6s0f0v0, representor enp6s0f0npf0vf0
> vf 1 link/ether ... name enp6s0f0v1, representor enp6s0f0npf0vf1
> ...
> 
> Technical Details
> I've taken the additional data from sysfs.
> 
> Current Work Status
> I am actively working on implementing this extension.
> 
> Request for Feedback
> Do you find this feature useful?
> Do you think getting data from sysfs is an appropriate approach? Are there any
> alternative methods you would recommend?
> 
> Best regards,

I prefer that functionality in iproute2 comes from netlink.
The kernel may need a change to put the vfinfo in the current netlink data.
Sysfs becomes less likely to work in cases where containers or other security parameters
are involved.