Re: [iproute PATCH] libnetlink: Double the dump buffer size

Re: [iproute PATCH] libnetlink: Double the dump buffer size

[Stephen Hemminger]

On Fri, 4 Mar 2016 18:57:28 +0000
Phil Sutter <phil@nwl.cc> wrote:

> There have been reports about 'ip addr' printing "Message truncated" on
> systems with large numbers of VFs. Although I haven't been able to get
> my hands on hardware suitable to reproduce this, increasing the dump
> buffer has been reported to resolve the issue. For want of a better
> idea, just double the buffer size to 32k.
> 
> Feels like this opportunistic buffer size selection is rather
> workarounding a design flaw in libnetlink or maybe even the netlink
> protocol itself.
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
>  lib/libnetlink.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/libnetlink.c b/lib/libnetlink.c
> index d6b5fd3e8a493..245c4ca216753 100644
> --- a/lib/libnetlink.c
> +++ b/lib/libnetlink.c
> @@ -223,7 +223,7 @@ int rtnl_dump_filter_l(struct rtnl_handle *rth,
>  		.msg_iov = &iov,
>  		.msg_iovlen = 1,
>  	};
> -	char buf[16384];
> +	char buf[32768];
>  	int dump_intr = 0;
>  
>  	iov.iov_base = buf;

I thought this was addressed in kernel by making the VF info optional.
The netlink protocol is showing some strain, this is one of them.

Re: [iproute PATCH] libnetlink: Double the dump buffer size

[Phil Sutter]

On Fri, Mar 04, 2016 at 03:35:53PM -0800, Stephen Hemminger wrote:
> On Fri, 4 Mar 2016 18:57:28 +0000
> Phil Sutter <phil@nwl.cc> wrote:
> 
> > There have been reports about 'ip addr' printing "Message truncated" on
> > systems with large numbers of VFs. Although I haven't been able to get
> > my hands on hardware suitable to reproduce this, increasing the dump
> > buffer has been reported to resolve the issue. For want of a better
> > idea, just double the buffer size to 32k.
> > 
> > Feels like this opportunistic buffer size selection is rather
> > workarounding a design flaw in libnetlink or maybe even the netlink
> > protocol itself.
> > 
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > ---
> >  lib/libnetlink.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/lib/libnetlink.c b/lib/libnetlink.c
> > index d6b5fd3e8a493..245c4ca216753 100644
> > --- a/lib/libnetlink.c
> > +++ b/lib/libnetlink.c
> > @@ -223,7 +223,7 @@ int rtnl_dump_filter_l(struct rtnl_handle *rth,
> >  		.msg_iov = &iov,
> >  		.msg_iovlen = 1,
> >  	};
> > -	char buf[16384];
> > +	char buf[32768];
> >  	int dump_intr = 0;
> >  
> >  	iov.iov_base = buf;
> 
> I thought this was addressed in kernel by making the VF info optional.
> The netlink protocol is showing some strain, this is one of them.

Oh, thanks for pointing this out. Testing was done with a RHEL7 kernel
only, so I can't tell whether this still applies to upstream.

What do you mean with 'optional'? Doesn't this imply that it is still
possible to request a dump which will exhaust buffer space in a single
message? Maybe just point me at the relevant kernel commit and I'll go
figure myself.

Thanks, Phil

Re: [iproute PATCH] libnetlink: Double the dump buffer size

[Johannes Berg]

On Fri, 2016-03-04 at 15:35 -0800, Stephen Hemminger wrote:
> 
> > There have been reports about 'ip addr' printing "Message
> > truncated" on
[...]
> I thought this was addressed in kernel by making the VF info
> optional.
> The netlink protocol is showing some strain, this is one of them.

I don't know how the dump is split here, but we had a similar issue
with nl80211 - originally each physical device info had to fit into a
single message (one message during dump for each device), but we fixed
that by having userspace to set a flag when it's able to understand a
multi-message single physical device info.

Before:
 msg1: phy1: A, B, C
 msg2: phy2: A, B, C

After:
 msg1: phy1: A
 msg2: phy1: B
 msg3: phy1: C
 msg4: phy1: D
 msg5: phy2: A
 [...]

For userspace not setting the flag, it only get partial info today for
compatibility (A, B, C, not D), but in our particular case this was
perfectly reasonable since it would be unaware of the new capabilities
anyway.

I don't know precisely enough what the issue at hand is to comment
whether such an approach will be feasible here, but it seems it could
be.

johannes

Re: [iproute PATCH] libnetlink: Double the dump buffer size

[Phil Sutter]

On Sat, Mar 05, 2016 at 01:57:46AM +0100, Phil Sutter wrote:
> On Fri, Mar 04, 2016 at 03:35:53PM -0800, Stephen Hemminger wrote:
> > On Fri, 4 Mar 2016 18:57:28 +0000
> > Phil Sutter <phil@nwl.cc> wrote:
> > 
> > > There have been reports about 'ip addr' printing "Message truncated" on
> > > systems with large numbers of VFs. Although I haven't been able to get
> > > my hands on hardware suitable to reproduce this, increasing the dump
> > > buffer has been reported to resolve the issue. For want of a better
> > > idea, just double the buffer size to 32k.
> > > 
> > > Feels like this opportunistic buffer size selection is rather
> > > workarounding a design flaw in libnetlink or maybe even the netlink
> > > protocol itself.
> > > 
> > > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > > ---
> > >  lib/libnetlink.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/lib/libnetlink.c b/lib/libnetlink.c
> > > index d6b5fd3e8a493..245c4ca216753 100644
> > > --- a/lib/libnetlink.c
> > > +++ b/lib/libnetlink.c
> > > @@ -223,7 +223,7 @@ int rtnl_dump_filter_l(struct rtnl_handle *rth,
> > >  		.msg_iov = &iov,
> > >  		.msg_iovlen = 1,
> > >  	};
> > > -	char buf[16384];
> > > +	char buf[32768];
> > >  	int dump_intr = 0;
> > >  
> > >  	iov.iov_base = buf;
> > 
> > I thought this was addressed in kernel by making the VF info optional.
> > The netlink protocol is showing some strain, this is one of them.
> 
> Oh, thanks for pointing this out. Testing was done with a RHEL7 kernel
> only, so I can't tell whether this still applies to upstream.
> 
> What do you mean with 'optional'? Doesn't this imply that it is still
> possible to request a dump which will exhaust buffer space in a single
> message? Maybe just point me at the relevant kernel commit and I'll go
> figure myself.

Guess I found it: c53864fd60227 ("rtnetlink: Only supply IFLA_VF_PORTS
information when RTEXT_FILTER_VF is set"). Bad news though, this patch
is already present in RHEL7 kernels. So in order to overcome this
Message Truncated problem, increasing the dump buffer size remains the
solution to go with.

Thanks, Phil

[iproute PATCH] libnetlink: Double the dump buffer size

[Phil Sutter]

There have been reports about 'ip addr' printing "Message truncated" on
systems with large numbers of VFs. Although I haven't been able to get
my hands on hardware suitable to reproduce this, increasing the dump
buffer has been reported to resolve the issue. For want of a better
idea, just double the buffer size to 32k.

Feels like this opportunistic buffer size selection is rather
workarounding a design flaw in libnetlink or maybe even the netlink
protocol itself.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 lib/libnetlink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index d6b5fd3e8a493..245c4ca216753 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -223,7 +223,7 @@ int rtnl_dump_filter_l(struct rtnl_handle *rth,
 		.msg_iov = &iov,
 		.msg_iovlen = 1,
 	};
-	char buf[16384];
+	char buf[32768];
 	int dump_intr = 0;
 
 	iov.iov_base = buf;
-- 
2.7.2