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

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

[Michal Kubecek]

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

[Michal Kubecek]

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 <mkubecek@suse.cz>
---
 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()

[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.

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 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

[Stephen Hemminger]

On Fri,  1 Sep 2017 18:39:06 +0200 (CEST)
Michal Kubecek <mkubecek@suse.cz> 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.