[PATCH iproute2 0/2] fix "ip link show dev ..." for NICs with many VFs

STALE3295d

4 messages, 2 authors, 2017-09-01 · open the first message on its own page

[PATCH iproute2 0/2] fix "ip link show dev ..." for NICs with many VFs

From: Michal Kubecek <hidden>
Date: 2017-09-01 16:39:09

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(-)

-- 
2.14.1

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

From: Michal Kubecek <hidden>
Date: 2017-09-01 16:39:16

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.

Signed-off-by: Michal Kubecek <redacted>
---
 ip/iplink.c | 5 +++++
 1 file changed, 5 insertions(+)
diff --git a/ip/iplink.c b/ip/iplink.c
index 5aff2fde38da..790e3a138bb0 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -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);
-- 
2.14.1

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

From: Michal Kubecek <hidden>
Date: 2017-09-01 16:39:20

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.

Signed-off-by: Michal Kubecek <redacted>
---
 ip/iplink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ip/iplink.c b/ip/iplink.c
index 790e3a138bb0..72c347932068 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -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) {
-- 
2.14.1

Re: [PATCH iproute2 0/2] fix "ip link show dev ..." for NICs with many VFs

From: Stephen Hemminger <stephen@networkplumber.org>
Date: 2017-09-01 21:16:43

On Fri,  1 Sep 2017 18:39:06 +0200 (CEST)
Michal Kubecek [off-list ref] wrote:
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(-)
Looks like the best set of solutions to the kernel side API issue.
Applied, thanks Michal.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help