Skip to contentSkip to conversation

fix "ip link show dev ..." for NICs with many VFs

PATCH In review Michal Kubecek 2 participants 4 messages Sep 1, 2017

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover fix "ip link show dev ..." for NICs with many VFs

Two of our customers recently encountered problems with processing of large messages produced by kernel in response to "ip link show" for NICs with many (120-128) virtual functions. While some of them have been already addressed in recent versions of iproute2, some still persist.

Patch 1 adds check to handle the case when a message fits into the buffer in rtnl_talk() but not into the buffer in iplink_get().

Patch 2 increases the buffer size in iplink_get() to suffice even for NICs with 128 VFs.

Note: after applying patch 2, patch 1 seems useless as both buffers have the same size so that the check cannot actually trigger. However, as we cannot guarantee they will always stay the same, I believe the check should still be added.

Michal Kubecek (2):
iplink: check for message truncation in iplink_get()
iplink: double the buffer size also in iplink_get()

 ip/iplink.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
Signature · 1 line
2.14.1
#1 lore raw

PATCH 1/2 iplink: check for message truncation in iplink_get()

If message length exceeds maxlen argument of rtnl_talk(), it is truncated to maxlen but unlike in the case of truncation to the length of local buffer in rtnl_talk(), the caller doesn't get any indication of a problem.

In particular, iplink_get() passes the truncated message on and parsing it results in various warnings and sometimes even a segfault (observed with "ip link show dev ..." for a NIC with 125 VFs).

Handle message truncation in iplink_get() the same way as truncation in rtnl_talk() would be handled: return an error.

---
 ip/iplink.c | 5 +++++
 1 file changed, 5 insertions(+)
ip/iplink.c +5 -0
index 5aff2fde38da..790e3a138bb0 100644
@@ -1040,6 +1040,11 @@ int iplink_get(unsigned int flags, char *name, __u32 filt_mask)
 
 	if (rtnl_talk(&rth, &req.n, &answer.n, sizeof(answer)) < 0)
 		return -2;
+	if (answer.n.nlmsg_len > sizeof(answer.buf)) {
+		fprintf(stderr, "Message truncated from %u to %lu\n",
+			answer.n.nlmsg_len, sizeof(answer.buf));
+		return -2;
+	}
 
 	if (brief)
 		print_linkinfo_brief(NULL, &answer.n, stdout, NULL);
Signature · 1 line
2.14.1
#2 lore raw

PATCH 2/2 iplink: double the buffer size also in iplink_get()

In reply to Michal Kubecek

Commit 72b365e8e0fd ("libnetlink: Double the dump buffer size") increased the buffer size for "ip link show" command to 32 KB to handle NICs with large number of VFs. With "dev" filter, a different code path is taken and iplink_get() still uses only 16 KB buffer.

The size of 32768 is not very future-proof as NICs supporting 120-128 VFs are already in use so that single RTM_NEWLINK message in the dump can exceed 30000 bytes. But it's what rtnl_talk() and rtnl_dump_filter_l() use so let's be consistent. Once this proves insufficient, all three sizes should be increased.

---
 ip/iplink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
ip/iplink.c +1 -1
index 790e3a138bb0..72c347932068 100644
@@ -1024,7 +1024,7 @@ int iplink_get(unsigned int flags, char *name, __u32 filt_mask)
 	};
 	struct {
 		struct nlmsghdr n;
-		char buf[16384];
+		char buf[32768];
 	} answer;
 
 	if (name) {
Signature · 1 line
2.14.1
#3 lore raw
In reply to Michal Kubecek
Quoted 16 lines · On Fri, 1 Sep 2017 18:39:06 +0200 (CEST) Michal Kubecek <[email protected]>…

Two of our customers recently encountered problems with processing of large messages produced by kernel in response to "ip link show" for NICs with many (120-128) virtual functions. While some of them have been already addressed in recent versions of iproute2, some still persist.

Patch 1 adds check to handle the case when a message fits into the buffer in rtnl_talk() but not into the buffer in iplink_get().

Patch 2 increases the buffer size in iplink_get() to suffice even for NICs with 128 VFs.

Note: after applying patch 2, patch 1 seems useless as both buffers have the same size so that the check cannot actually trigger. However, as we cannot guarantee they will always stay the same, I believe the check should still be added.

Quoted 7 lines

Michal Kubecek (2):
iplink: check for message truncation in iplink_get()
iplink: double the buffer size also in iplink_get()

 ip/iplink.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Looks like the best set of solutions to the kernel side API issue. Applied, thanks Michal.