Re: [libvirt] [PATCH v4] libvirtd: Increase NL buffer size for lots of interface

On 01/20/2016 08:51 AM, Leno Hou wrote: > 1. When switching CPUs to offline/online in a system more than 128 cpus > 2. When using virsh to destroy domain in a system with more interface > All of above happens nl_recv returned with error: No buffer space > available. > > This patch sets the socket buffer size to 128K and turns on message > peeking > on new function virNetlinkAlloc,as this would solve this problem totally > and permanetly. > > Signed-off-by: Leno Hou <houqy(a)linux.vnet.ibm.com&gt; > Cc: Wenyi Gao <wenyi(a)linux.vnet.ibm.com&gt; > CC: Laine Stump <laine(a)laine.org&gt; > CC: Michal Privoznik <mprivozn(a)redhat.com&gt; > --- > src/util/virnetlink.c | 42 ++++++++++++++++++++++++++++++++++++++---- > src/util/virnetlink.h | 7 +++++++ > 2 files changed, 45 insertions(+), 4 deletions(-) > > diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c > index 679b48e..02f5215 100644 > --- a/src/util/virnetlink.c > +++ b/src/util/virnetlink.c > @@ -64,13 +64,11 @@ struct virNetlinkEventHandle { > }; > # ifdef HAVE_LIBNL1 > -# define virNetlinkAlloc nl_handle_alloc > +# define virSocketSetBufferSize nl_set_buffer_size > # define virNetlinkFree nl_handle_destroy > -typedef struct nl_handle virNetlinkHandle; > # else > -# define virNetlinkAlloc nl_socket_alloc > +# define virSocketSetBufferSize nl_socket_set_buffer_size > # define virNetlinkFree nl_socket_free > -typedef struct nl_sock virNetlinkHandle; > # endif > typedef struct _virNetlinkEventSrvPrivate virNetlinkEventSrvPrivate; > @@ -108,6 +106,42 @@ static virNetlinkHandle *placeholder_nlhandle; > /* Function definitions */ > /** > + * virNetlinkAlloc > + * > + * Perform netlink allocation which was defined differently for libnl3 > + * vs libnl-1, and did all of three in one function > + * 1) create socket > + * 2) set larger buffer size > + * 3) turn on message peeking > + * > + * see the following email message: > + * > https://www.redhat.com/archives/libvir-list/2016-January/msg00536.html > + * > + * Returns virNetlinkHandle > + */ > +virNetlinkHandle *virNetlinkAlloc(void) > +{ > + virNetlinkHandle *netlinknh; > + #ifdef HAVE_LIBNL1 > + netlinknh = nl_handle_alloc(); > + #else > + netlinknh = nl_socket_alloc(); > + #endif > + > + if (virSocketSetBufferSize(netlinknh,131702,0) >= 0) { > + nl_socket_enable_msg_peek(netlinknh); > + return netlinknh; > + } > + > + virReportSystemError(errno, > + "%s",_("can not set socket buffer size to 128k")); > + > + virNetlinkFree(netlinknh); > + > + return NULL; > +} > + > +/** > * virNetlinkStartup: > * > * Perform any initialization that needs to take place before the > diff --git a/src/util/virnetlink.h b/src/util/virnetlink.h > index 0664a7a..b4a2010 100644 > --- a/src/util/virnetlink.h > +++ b/src/util/virnetlink.h > @@ -44,6 +44,13 @@ struct nlmsghdr; > # endif /* __linux__ */ > +# ifdef HAVE_LIBNL1 > +typedef struct nl_handle virNetlinkHandle; > +# else > +typedef struct nl_sock virNetlinkHandle; > +# endif > +virNetlinkHandle *virNetlinkAlloc(void); > + > int virNetlinkStartup(void); > void virNetlinkShutdown(void); I had earlier tried modifying your previous patch to match this one, and found that the attempt to set the buffer size failed if the socket wasn't already connected. I solved this by making a separate function virNetLinkCreateSocket(int protocol) which would create and connect a socket, then set the buffer size and turn on message peeking. I then called this function from all of the places that were previously calling virNetlinkAlloc() (except one - the nlhandle created virNetlinkStartup() is never connected, so has no protocol, but it also is never used to receive anything, so it doesn't need a larger buffer or message peeking). In my tests anyway, this worked properly. I've attached my modified patch below. Since it started out from your patch, I left it attributed to you and just added my name to a Signed-off-by:. Assuming it fixes your problem, are you okay with this patch going in?