On 06/16/2014 09:16 PM, vaughan wrote:
> Hi experts,
[ALL CAPS SUBJECT LINE MAKES IT SOUND LIKE YOU ARE SHOUTING]
>
> Release: OL7
> Kernel: 3.10.0-121.el7.x86_64
>
> Noticed below error on OL7 server, while loading Intel 10gigabit nic
> driver module , ixgbe in syslog
> --------------------------------------------------------------------
> journal: nl_recv returned with error: No buffer space available
> -------------------------------------------------------------------
>
>
> This problem can be addressed by increasing the size of the socket
> receive buffer via nl_socket_set_buffer_size(). However, since reliable
> transmissions from kernel to user are impossible in any case, I don't
> know if we need to ensure enough buffer and how much it will be enough.
> In my test, 32768*4 bytes of receive buffer size is ok. Below is my patch.
Thanks for the submission. Can you use 'git send-email' next time, as
that format is easier for maintainers to apply your patch, than having
to manually extract your inlined patch out of the body of a larger message?
>
>>From 4505e0001563bcef814dd2c6dc02b2d83e521867 Mon Sep 17 00:00:00 2001
> From: rpm-build <rpm-build>
> Date: Mon, 16 Jun 2014 20:05:08 +0800
> Subject: [PATCH] set nl socket buffer size to 32768*4
>
> Signed-off-by: Vaughan Cao <vaughan.cao@xxxxxxxxxx>
Your commit message is rather sparse. You should include some of the
information from your larger email as part of your commit message (in
particular, that you determined that the ixgbe driver module fails with
the default netlink buffer size, and that requesting a larger libnl
buffer resulted in clearing up the log messages that you were seeing).
> ---
> src/util/virnetlink.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
> index 5849ed7..cf44ad2 100644
> --- a/src/util/virnetlink.c
> +++ b/src/util/virnetlink.c
> @@ -561,6 +561,15 @@ virNetlinkEventServiceStart(unsigned int protocol,
> unsigned int groups)
> goto error_server;
> }
>
> + if (nl_socket_set_buffer_size(srv->netlinknh, (32768*4), 0)) {
Spaces on both sides of binary operators.
> + virReportSystemError(errno, "%s",
> + _("cannot set netlink socket buffer size to 32768*4"));
Unusual indentation. Also, are we guaranteed that 'errno' is a sane
value at this point?
http://libnl2.sourcearchive.com/documentation/2.0-1/group__socket_gaec9b1f3b0fdbf4e6e0fb10a233b5df68.html
merely states that nl_socket_set_buffer_size() returns a negative error
code on failure, not that it sets errno.
> + goto error_server;
> + } else {
> + virReportSystemError(0, "%s",
> + _("set netlink socket buffer size to 32768*4"));
virReportSystemError() is the wrong function to use here; in particular,
reporting 0 as the error value will make for a non-sensical message:
set netlink socket buffer size to 32768*4: Success
virReportError() would be sufficient.
Also, do you have to repeat the 32768*4 value in your two error
messages, or can you merely just state "cannot resize socket buffer" and
be done with it (so that the value itself can be changed without having
to retranslate messages down the road).
> + }
> +
> if ((srv->eventwatch = virEventAddHandle(fd,
> VIR_EVENT_HANDLE_READABLE,
> virNetlinkEventCallback,
>
I'd feel better if Laine actually approves the patch, but I've pointed
out enough issues that it is worth you posting a v2.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list