fix "ip link show dev ..." for NICs with many VFs
Subsystems (2)
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
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
@@ -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
PATCH 2/2 iplink: double the buffer size also in iplink_get()
In reply to Michal KubecekCommit 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
@@ -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
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 iniplink_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 iniplink_get()
iplink: double the buffer size also iniplink_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.