network: allow to specify buffer size for netlink socket

[libvirt] [PATCH v2] network: allow to specify buffer size for netlink socket

Posted by ZhiPeng Lu 7 years, 10 months ago

This patchs allow to set the buffer size for netlink socket in
the libvirtd configuration file. The default buffer size remain
as before at 128k.

Signed-off-by: Zhipeng Lu <lu.zhipeng@zte.com.cn>
---
 daemon/libvirtd-config.c    |  6 ++++++
 daemon/libvirtd-config.h    |  2 ++
 daemon/libvirtd.aug         |  1 +
 daemon/libvirtd.c           | 12 ++++++++++++
 daemon/libvirtd.conf        |  3 +++
 daemon/test_libvirtd.aug.in |  1 +
 src/libvirt_private.syms    |  1 +
 src/util/virnetlink.c       | 19 ++++++++++++++++++-
 src/util/virnetlink.h       |  7 ++++++-
 9 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/daemon/libvirtd-config.c b/daemon/libvirtd-config.c
index 6c0f00e..b2bda28 100644
--- a/daemon/libvirtd-config.c
+++ b/daemon/libvirtd-config.c
@@ -33,6 +33,7 @@
 #include "remote/remote_protocol.h"
 #include "remote/remote_driver.h"
 #include "util/virnetdevopenvswitch.h"
+#include "util/virnetlink.h"
 #include "virstring.h"
 #include "virutil.h"
 
@@ -172,6 +173,8 @@ daemonConfigNew(bool privileged ATTRIBUTE_UNUSED)
     data->admin_keepalive_count = 5;
 
     data->ovs_timeout = VIR_NETDEV_OVS_DEFAULT_TIMEOUT;
+    
+    data->netlink_sock_buffer_size = VIRT_NETLINK_SOCK_BUFFER_SIZE;
 
     localhost = virGetHostname();
     if (localhost == NULL) {
@@ -394,6 +397,9 @@ daemonConfigLoadOptions(struct daemonConfig *data,
     if (virConfGetValueUInt(conf, "ovs_timeout", &data->ovs_timeout) < 0)
         goto error;
 
+    if (virConfGetValueUInt(conf, "netlink_sock_buffer_size", &data->netlink_sock_buffer_size) < 0)
+        goto error;
+
     return 0;
 
  error:
diff --git a/daemon/libvirtd-config.h b/daemon/libvirtd-config.h
index 1edf5fa..22befd1 100644
--- a/daemon/libvirtd-config.h
+++ b/daemon/libvirtd-config.h
@@ -94,6 +94,8 @@ struct daemonConfig {
     unsigned int admin_keepalive_count;
 
     unsigned int ovs_timeout;
+    
+    unsigned int netlink_sock_buffer_size;
 };
 
 
diff --git a/daemon/libvirtd.aug b/daemon/libvirtd.aug
index 24fdf44..84ea00a 100644
--- a/daemon/libvirtd.aug
+++ b/daemon/libvirtd.aug
@@ -89,6 +89,7 @@ module Libvirtd =
    let misc_entry = str_entry "host_uuid"
                   | str_entry "host_uuid_source"
                   | int_entry "ovs_timeout"
+                  | int_entry "netlink_sock_buffer_size"
 
    (* Each enty in the config is one of the following three ... *)
    let entry = network_entry
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index a558458..34db23a 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -648,6 +648,16 @@ daemonSetupNetDevOpenvswitch(struct daemonConfig *config)
 
 
 /*
+ * Set up the netlink socket  buffer size
+ */
+static void
+daemonSetupNetLink(struct daemonConfig *config)
+{
+    virNetLinkSetBufferSize(config->netlink_sock_buffer_size);
+}
+
+
+/*
  * Set up the logging environment
  * By default if daemonized all errors go to the logfile libvirtd.log,
  * but if verbose or error debugging is asked for then also output
@@ -1257,6 +1267,8 @@ int main(int argc, char **argv) {
         exit(EXIT_FAILURE);
     }
 
+    daemonSetupNetLink(config);
+
     daemonSetupNetDevOpenvswitch(config);
 
     if (daemonSetupAccessManager(config) < 0) {
diff --git a/daemon/libvirtd.conf b/daemon/libvirtd.conf
index e83e9a1..b174767 100644
--- a/daemon/libvirtd.conf
+++ b/daemon/libvirtd.conf
@@ -476,3 +476,6 @@
 # potential infinite waits blocking libvirt.
 #
 #ovs_timeout = 5
+
+# This allow to specify buffer size for netlink socket.
+#netlink_sock_buffer_size = 131072
diff --git a/daemon/test_libvirtd.aug.in b/daemon/test_libvirtd.aug.in
index 1200952..0a1207f 100644
--- a/daemon/test_libvirtd.aug.in
+++ b/daemon/test_libvirtd.aug.in
@@ -64,3 +64,4 @@ module Test_libvirtd =
         { "admin_keepalive_interval" = "5" }
         { "admin_keepalive_count" = "5" }
         { "ovs_timeout" = "5" }
+        { "netlink_sock_buffer_size" = "131072" }
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 888412a..83be39d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2248,6 +2248,7 @@ virNetlinkEventServiceStart;
 virNetlinkEventServiceStop;
 virNetlinkEventServiceStopAll;
 virNetlinkGetErrorCode;
+virNetLinkSetBufferSize;
 virNetlinkShutdown;
 virNetlinkStartup;
 
diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
index d732fe8..0a549b7 100644
--- a/src/util/virnetlink.c
+++ b/src/util/virnetlink.c
@@ -49,6 +49,7 @@ VIR_LOG_INIT("util.netlink");
 
 #define NETLINK_ACK_TIMEOUT_S  (2*1000)
 
+
 #if defined(__linux__) && defined(HAVE_LIBNL)
 /* State for a single netlink event handle */
 struct virNetlinkEventHandle {
@@ -104,6 +105,22 @@ static int nextWatch = 1;
 static virNetlinkEventSrvPrivatePtr server[MAX_LINKS] = {NULL};
 static virNetlinkHandle *placeholder_nlhandle;
 
+/*
+ * Set netlink  default buffer size
+ */
+static unsigned int virNetLinkBufferSize = VIRT_NETLINK_SOCK_BUFFER_SIZE; 
+
+/**
+ * virNetLinkSetBufferSize:
+ * @size: the buffer size
+ *
+ * Set netlink socket buffer size
+ */
+void
+virNetLinkSetBufferSize(unsigned int size)
+{
+    virNetLinkBufferSize = size;
+}
 /* Function definitions */
 
 /**
@@ -189,7 +206,7 @@ virNetlinkCreateSocket(int protocol)
         goto error;
     }
 
-    if (virNetlinkSetBufferSize(nlhandle, 131702, 0) < 0) {
+    if (virNetlinkSetBufferSize(nlhandle, virNetLinkBufferSize, 0) < 0) {
         virReportSystemError(errno, "%s",
                              _("cannot set netlink socket buffer "
                                "size to 128k"));
diff --git a/src/util/virnetlink.h b/src/util/virnetlink.h
index 088b013..2d6e762 100644
--- a/src/util/virnetlink.h
+++ b/src/util/virnetlink.h
@@ -22,7 +22,6 @@
 
 # include "internal.h"
 # include "virmacaddr.h"
-
 # if defined(__linux__) && defined(HAVE_LIBNL)
 
 /* Work around a bug where older libnl-1 headers expected older gcc
@@ -44,6 +43,8 @@ struct nlmsghdr;
 
 # endif /* __linux__ */
 
+#define VIRT_NETLINK_SOCK_BUFFER_SIZE (131072)
+
 int virNetlinkStartup(void);
 void virNetlinkShutdown(void);
 
@@ -120,5 +121,9 @@ int virNetlinkEventAddClient(virNetlinkEventHandleCallback handleCB,
  */
 int virNetlinkEventRemoveClient(int watch, const virMacAddr *macaddr,
                                 unsigned int protocol);
+/**
+ *virNetLinkSetBufferSize: set netlink socket buffer size
+ */
+void virNetLinkSetBufferSize(unsigned int size);
 
 #endif /* __VIR_NETLINK_H__ */
-- 
1.8.3.1


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2] network: allow to specify buffer size for netlink socket

Posted by Laine Stump 7 years, 10 months ago

On 07/11/2017 09:01 PM, ZhiPeng Lu wrote:
> This patchs allow to set the buffer size for netlink socket in
> the libvirtd configuration file. The default buffer size remain
> as before at 128k.

See my more detailed response to your earlier patch here:


  https://www.redhat.com/archives/libvir-list/2017-July/msg00566.html

There should be no need to configure the initial libnl buffer size,
because we enable MSG_PEEK on the libnl sockets (and recent versions of
libnl have it turned on by default anyway). If that's not permitting the
buffer to auto-grow as necessary, then there is a different bug somewhere.

>
> Signed-off-by: Zhipeng Lu <lu.zhipeng@zte.com.cn>
> ---
>  daemon/libvirtd-config.c    |  6 ++++++
>  daemon/libvirtd-config.h    |  2 ++
>  daemon/libvirtd.aug         |  1 +
>  daemon/libvirtd.c           | 12 ++++++++++++
>  daemon/libvirtd.conf        |  3 +++
>  daemon/test_libvirtd.aug.in |  1 +
>  src/libvirt_private.syms    |  1 +
>  src/util/virnetlink.c       | 19 ++++++++++++++++++-
>  src/util/virnetlink.h       |  7 ++++++-
>  9 files changed, 50 insertions(+), 2 deletions(-)
>
> diff --git a/daemon/libvirtd-config.c b/daemon/libvirtd-config.c
> index 6c0f00e..b2bda28 100644
> --- a/daemon/libvirtd-config.c
> +++ b/daemon/libvirtd-config.c
> @@ -33,6 +33,7 @@
>  #include "remote/remote_protocol.h"
>  #include "remote/remote_driver.h"
>  #include "util/virnetdevopenvswitch.h"
> +#include "util/virnetlink.h"
>  #include "virstring.h"
>  #include "virutil.h"
>  
> @@ -172,6 +173,8 @@ daemonConfigNew(bool privileged ATTRIBUTE_UNUSED)
>      data->admin_keepalive_count = 5;
>  
>      data->ovs_timeout = VIR_NETDEV_OVS_DEFAULT_TIMEOUT;
> +    
> +    data->netlink_sock_buffer_size = VIRT_NETLINK_SOCK_BUFFER_SIZE;
>  
>      localhost = virGetHostname();
>      if (localhost == NULL) {
> @@ -394,6 +397,9 @@ daemonConfigLoadOptions(struct daemonConfig *data,
>      if (virConfGetValueUInt(conf, "ovs_timeout", &data->ovs_timeout) < 0)
>          goto error;
>  
> +    if (virConfGetValueUInt(conf, "netlink_sock_buffer_size", &data->netlink_sock_buffer_size) < 0)
> +        goto error;
> +
>      return 0;
>  
>   error:
> diff --git a/daemon/libvirtd-config.h b/daemon/libvirtd-config.h
> index 1edf5fa..22befd1 100644
> --- a/daemon/libvirtd-config.h
> +++ b/daemon/libvirtd-config.h
> @@ -94,6 +94,8 @@ struct daemonConfig {
>      unsigned int admin_keepalive_count;
>  
>      unsigned int ovs_timeout;
> +    
> +    unsigned int netlink_sock_buffer_size;
>  };
>  
>  
> diff --git a/daemon/libvirtd.aug b/daemon/libvirtd.aug
> index 24fdf44..84ea00a 100644
> --- a/daemon/libvirtd.aug
> +++ b/daemon/libvirtd.aug
> @@ -89,6 +89,7 @@ module Libvirtd =
>     let misc_entry = str_entry "host_uuid"
>                    | str_entry "host_uuid_source"
>                    | int_entry "ovs_timeout"
> +                  | int_entry "netlink_sock_buffer_size"
>  
>     (* Each enty in the config is one of the following three ... *)
>     let entry = network_entry
> diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
> index a558458..34db23a 100644
> --- a/daemon/libvirtd.c
> +++ b/daemon/libvirtd.c
> @@ -648,6 +648,16 @@ daemonSetupNetDevOpenvswitch(struct daemonConfig *config)
>  
>  
>  /*
> + * Set up the netlink socket  buffer size
> + */
> +static void
> +daemonSetupNetLink(struct daemonConfig *config)
> +{
> +    virNetLinkSetBufferSize(config->netlink_sock_buffer_size);
> +}
> +
> +
> +/*
>   * Set up the logging environment
>   * By default if daemonized all errors go to the logfile libvirtd.log,
>   * but if verbose or error debugging is asked for then also output
> @@ -1257,6 +1267,8 @@ int main(int argc, char **argv) {
>          exit(EXIT_FAILURE);
>      }
>  
> +    daemonSetupNetLink(config);
> +
>      daemonSetupNetDevOpenvswitch(config);
>  
>      if (daemonSetupAccessManager(config) < 0) {
> diff --git a/daemon/libvirtd.conf b/daemon/libvirtd.conf
> index e83e9a1..b174767 100644
> --- a/daemon/libvirtd.conf
> +++ b/daemon/libvirtd.conf
> @@ -476,3 +476,6 @@
>  # potential infinite waits blocking libvirt.
>  #
>  #ovs_timeout = 5
> +
> +# This allow to specify buffer size for netlink socket.
> +#netlink_sock_buffer_size = 131072
> diff --git a/daemon/test_libvirtd.aug.in b/daemon/test_libvirtd.aug.in
> index 1200952..0a1207f 100644
> --- a/daemon/test_libvirtd.aug.in
> +++ b/daemon/test_libvirtd.aug.in
> @@ -64,3 +64,4 @@ module Test_libvirtd =
>          { "admin_keepalive_interval" = "5" }
>          { "admin_keepalive_count" = "5" }
>          { "ovs_timeout" = "5" }
> +        { "netlink_sock_buffer_size" = "131072" }
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 888412a..83be39d 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2248,6 +2248,7 @@ virNetlinkEventServiceStart;
>  virNetlinkEventServiceStop;
>  virNetlinkEventServiceStopAll;
>  virNetlinkGetErrorCode;
> +virNetLinkSetBufferSize;
>  virNetlinkShutdown;
>  virNetlinkStartup;
>  
> diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
> index d732fe8..0a549b7 100644
> --- a/src/util/virnetlink.c
> +++ b/src/util/virnetlink.c
> @@ -49,6 +49,7 @@ VIR_LOG_INIT("util.netlink");
>  
>  #define NETLINK_ACK_TIMEOUT_S  (2*1000)
>  
> +
>  #if defined(__linux__) && defined(HAVE_LIBNL)
>  /* State for a single netlink event handle */
>  struct virNetlinkEventHandle {
> @@ -104,6 +105,22 @@ static int nextWatch = 1;
>  static virNetlinkEventSrvPrivatePtr server[MAX_LINKS] = {NULL};
>  static virNetlinkHandle *placeholder_nlhandle;
>  
> +/*
> + * Set netlink  default buffer size
> + */
> +static unsigned int virNetLinkBufferSize = VIRT_NETLINK_SOCK_BUFFER_SIZE; 
> +
> +/**
> + * virNetLinkSetBufferSize:
> + * @size: the buffer size
> + *
> + * Set netlink socket buffer size
> + */
> +void
> +virNetLinkSetBufferSize(unsigned int size)
> +{
> +    virNetLinkBufferSize = size;
> +}
>  /* Function definitions */
>  
>  /**
> @@ -189,7 +206,7 @@ virNetlinkCreateSocket(int protocol)
>          goto error;
>      }
>  
> -    if (virNetlinkSetBufferSize(nlhandle, 131702, 0) < 0) {
> +    if (virNetlinkSetBufferSize(nlhandle, virNetLinkBufferSize, 0) < 0) {
>          virReportSystemError(errno, "%s",
>                               _("cannot set netlink socket buffer "
>                                 "size to 128k"));
> diff --git a/src/util/virnetlink.h b/src/util/virnetlink.h
> index 088b013..2d6e762 100644
> --- a/src/util/virnetlink.h
> +++ b/src/util/virnetlink.h
> @@ -22,7 +22,6 @@
>  
>  # include "internal.h"
>  # include "virmacaddr.h"
> -
>  # if defined(__linux__) && defined(HAVE_LIBNL)
>  
>  /* Work around a bug where older libnl-1 headers expected older gcc
> @@ -44,6 +43,8 @@ struct nlmsghdr;
>  
>  # endif /* __linux__ */
>  
> +#define VIRT_NETLINK_SOCK_BUFFER_SIZE (131072)
> +
>  int virNetlinkStartup(void);
>  void virNetlinkShutdown(void);
>  
> @@ -120,5 +121,9 @@ int virNetlinkEventAddClient(virNetlinkEventHandleCallback handleCB,
>   */
>  int virNetlinkEventRemoveClient(int watch, const virMacAddr *macaddr,
>                                  unsigned int protocol);
> +/**
> + *virNetLinkSetBufferSize: set netlink socket buffer size
> + */
> +void virNetLinkSetBufferSize(unsigned int size);
>  
>  #endif /* __VIR_NETLINK_H__ */


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2] network: allow to specify buffer size for netlink socket

Posted by John Ferlan 7 years, 10 months ago


On 07/16/2017 05:04 PM, Laine Stump wrote:
> On 07/11/2017 09:01 PM, ZhiPeng Lu wrote:
>> This patchs allow to set the buffer size for netlink socket in
>> the libvirtd configuration file. The default buffer size remain
>> as before at 128k.
> 
> See my more detailed response to your earlier patch here:
> 
> 
>   https://www.redhat.com/archives/libvir-list/2017-July/msg00566.html
> 
> There should be no need to configure the initial libnl buffer size,
> because we enable MSG_PEEK on the libnl sockets (and recent versions of
> libnl have it turned on by default anyway). If that's not permitting the
> buffer to auto-grow as necessary, then there is a different bug somewhere.
> 

If an old version of libnl is the problem, then perhaps a patch which
just adds a comment in virNetlinkCreateSocket to "summarize" what gets
discovered w/r/t MSG_PEEK and the "correct" minimum version of libnl so
that the "next" person to come this way will have a chance at
understanding what needs to be done without going through the submit a
patch changing the size again!

All that said, having it be configurable could be useful for someone who
has a system that doesn't have that version, while still working as
expected for the right version.

John

>>
>> Signed-off-by: Zhipeng Lu <lu.zhipeng@zte.com.cn>
>> ---
>>  daemon/libvirtd-config.c    |  6 ++++++
>>  daemon/libvirtd-config.h    |  2 ++
>>  daemon/libvirtd.aug         |  1 +
>>  daemon/libvirtd.c           | 12 ++++++++++++
>>  daemon/libvirtd.conf        |  3 +++
>>  daemon/test_libvirtd.aug.in |  1 +
>>  src/libvirt_private.syms    |  1 +
>>  src/util/virnetlink.c       | 19 ++++++++++++++++++-
>>  src/util/virnetlink.h       |  7 ++++++-
>>  9 files changed, 50 insertions(+), 2 deletions(-)
>>
>> diff --git a/daemon/libvirtd-config.c b/daemon/libvirtd-config.c
>> index 6c0f00e..b2bda28 100644
>> --- a/daemon/libvirtd-config.c
>> +++ b/daemon/libvirtd-config.c
>> @@ -33,6 +33,7 @@
>>  #include "remote/remote_protocol.h"
>>  #include "remote/remote_driver.h"
>>  #include "util/virnetdevopenvswitch.h"
>> +#include "util/virnetlink.h"
>>  #include "virstring.h"
>>  #include "virutil.h"
>>  
>> @@ -172,6 +173,8 @@ daemonConfigNew(bool privileged ATTRIBUTE_UNUSED)
>>      data->admin_keepalive_count = 5;
>>  
>>      data->ovs_timeout = VIR_NETDEV_OVS_DEFAULT_TIMEOUT;
>> +    
>> +    data->netlink_sock_buffer_size = VIRT_NETLINK_SOCK_BUFFER_SIZE;
>>  
>>      localhost = virGetHostname();
>>      if (localhost == NULL) {
>> @@ -394,6 +397,9 @@ daemonConfigLoadOptions(struct daemonConfig *data,
>>      if (virConfGetValueUInt(conf, "ovs_timeout", &data->ovs_timeout) < 0)
>>          goto error;
>>  
>> +    if (virConfGetValueUInt(conf, "netlink_sock_buffer_size", &data->netlink_sock_buffer_size) < 0)
>> +        goto error;
>> +
>>      return 0;
>>  
>>   error:
>> diff --git a/daemon/libvirtd-config.h b/daemon/libvirtd-config.h
>> index 1edf5fa..22befd1 100644
>> --- a/daemon/libvirtd-config.h
>> +++ b/daemon/libvirtd-config.h
>> @@ -94,6 +94,8 @@ struct daemonConfig {
>>      unsigned int admin_keepalive_count;
>>  
>>      unsigned int ovs_timeout;
>> +    
>> +    unsigned int netlink_sock_buffer_size;
>>  };
>>  
>>  
>> diff --git a/daemon/libvirtd.aug b/daemon/libvirtd.aug
>> index 24fdf44..84ea00a 100644
>> --- a/daemon/libvirtd.aug
>> +++ b/daemon/libvirtd.aug
>> @@ -89,6 +89,7 @@ module Libvirtd =
>>     let misc_entry = str_entry "host_uuid"
>>                    | str_entry "host_uuid_source"
>>                    | int_entry "ovs_timeout"
>> +                  | int_entry "netlink_sock_buffer_size"
>>  
>>     (* Each enty in the config is one of the following three ... *)
>>     let entry = network_entry
>> diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
>> index a558458..34db23a 100644
>> --- a/daemon/libvirtd.c
>> +++ b/daemon/libvirtd.c
>> @@ -648,6 +648,16 @@ daemonSetupNetDevOpenvswitch(struct daemonConfig *config)
>>  
>>  
>>  /*
>> + * Set up the netlink socket  buffer size
>> + */
>> +static void
>> +daemonSetupNetLink(struct daemonConfig *config)
>> +{
>> +    virNetLinkSetBufferSize(config->netlink_sock_buffer_size);
>> +}
>> +
>> +
>> +/*
>>   * Set up the logging environment
>>   * By default if daemonized all errors go to the logfile libvirtd.log,
>>   * but if verbose or error debugging is asked for then also output
>> @@ -1257,6 +1267,8 @@ int main(int argc, char **argv) {
>>          exit(EXIT_FAILURE);
>>      }
>>  
>> +    daemonSetupNetLink(config);
>> +
>>      daemonSetupNetDevOpenvswitch(config);
>>  
>>      if (daemonSetupAccessManager(config) < 0) {
>> diff --git a/daemon/libvirtd.conf b/daemon/libvirtd.conf
>> index e83e9a1..b174767 100644
>> --- a/daemon/libvirtd.conf
>> +++ b/daemon/libvirtd.conf
>> @@ -476,3 +476,6 @@
>>  # potential infinite waits blocking libvirt.
>>  #
>>  #ovs_timeout = 5
>> +
>> +# This allow to specify buffer size for netlink socket.
>> +#netlink_sock_buffer_size = 131072
>> diff --git a/daemon/test_libvirtd.aug.in b/daemon/test_libvirtd.aug.in
>> index 1200952..0a1207f 100644
>> --- a/daemon/test_libvirtd.aug.in
>> +++ b/daemon/test_libvirtd.aug.in
>> @@ -64,3 +64,4 @@ module Test_libvirtd =
>>          { "admin_keepalive_interval" = "5" }
>>          { "admin_keepalive_count" = "5" }
>>          { "ovs_timeout" = "5" }
>> +        { "netlink_sock_buffer_size" = "131072" }
>> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
>> index 888412a..83be39d 100644
>> --- a/src/libvirt_private.syms
>> +++ b/src/libvirt_private.syms
>> @@ -2248,6 +2248,7 @@ virNetlinkEventServiceStart;
>>  virNetlinkEventServiceStop;
>>  virNetlinkEventServiceStopAll;
>>  virNetlinkGetErrorCode;
>> +virNetLinkSetBufferSize;
>>  virNetlinkShutdown;
>>  virNetlinkStartup;
>>  
>> diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
>> index d732fe8..0a549b7 100644
>> --- a/src/util/virnetlink.c
>> +++ b/src/util/virnetlink.c
>> @@ -49,6 +49,7 @@ VIR_LOG_INIT("util.netlink");
>>  
>>  #define NETLINK_ACK_TIMEOUT_S  (2*1000)
>>  
>> +
>>  #if defined(__linux__) && defined(HAVE_LIBNL)
>>  /* State for a single netlink event handle */
>>  struct virNetlinkEventHandle {
>> @@ -104,6 +105,22 @@ static int nextWatch = 1;
>>  static virNetlinkEventSrvPrivatePtr server[MAX_LINKS] = {NULL};
>>  static virNetlinkHandle *placeholder_nlhandle;
>>  
>> +/*
>> + * Set netlink  default buffer size
>> + */
>> +static unsigned int virNetLinkBufferSize = VIRT_NETLINK_SOCK_BUFFER_SIZE; 
>> +
>> +/**
>> + * virNetLinkSetBufferSize:
>> + * @size: the buffer size
>> + *
>> + * Set netlink socket buffer size
>> + */
>> +void
>> +virNetLinkSetBufferSize(unsigned int size)
>> +{
>> +    virNetLinkBufferSize = size;
>> +}
>>  /* Function definitions */
>>  
>>  /**
>> @@ -189,7 +206,7 @@ virNetlinkCreateSocket(int protocol)
>>          goto error;
>>      }
>>  
>> -    if (virNetlinkSetBufferSize(nlhandle, 131702, 0) < 0) {
>> +    if (virNetlinkSetBufferSize(nlhandle, virNetLinkBufferSize, 0) < 0) {
>>          virReportSystemError(errno, "%s",
>>                               _("cannot set netlink socket buffer "
>>                                 "size to 128k"));
>> diff --git a/src/util/virnetlink.h b/src/util/virnetlink.h
>> index 088b013..2d6e762 100644
>> --- a/src/util/virnetlink.h
>> +++ b/src/util/virnetlink.h
>> @@ -22,7 +22,6 @@
>>  
>>  # include "internal.h"
>>  # include "virmacaddr.h"
>> -
>>  # if defined(__linux__) && defined(HAVE_LIBNL)
>>  
>>  /* Work around a bug where older libnl-1 headers expected older gcc
>> @@ -44,6 +43,8 @@ struct nlmsghdr;
>>  
>>  # endif /* __linux__ */
>>  
>> +#define VIRT_NETLINK_SOCK_BUFFER_SIZE (131072)
>> +
>>  int virNetlinkStartup(void);
>>  void virNetlinkShutdown(void);
>>  
>> @@ -120,5 +121,9 @@ int virNetlinkEventAddClient(virNetlinkEventHandleCallback handleCB,
>>   */
>>  int virNetlinkEventRemoveClient(int watch, const virMacAddr *macaddr,
>>                                  unsigned int protocol);
>> +/**
>> + *virNetLinkSetBufferSize: set netlink socket buffer size
>> + */
>> +void virNetLinkSetBufferSize(unsigned int size);
>>  
>>  #endif /* __VIR_NETLINK_H__ */
> 
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2] network: allow to specify buffer size for netlink socket

Posted by Laine Stump 7 years, 9 months ago

On 07/17/2017 08:00 AM, John Ferlan wrote:
>
> On 07/16/2017 05:04 PM, Laine Stump wrote:
>> On 07/11/2017 09:01 PM, ZhiPeng Lu wrote:
>>> This patchs allow to set the buffer size for netlink socket in
>>> the libvirtd configuration file. The default buffer size remain
>>> as before at 128k.
>> See my more detailed response to your earlier patch here:
>>
>>
>>   https://www.redhat.com/archives/libvir-list/2017-July/msg00566.html
>>
>> There should be no need to configure the initial libnl buffer size,
>> because we enable MSG_PEEK on the libnl sockets (and recent versions of
>> libnl have it turned on by default anyway). If that's not permitting the
>> buffer to auto-grow as necessary, then there is a different bug somewhere.
>>
> If an old version of libnl is the problem, then perhaps a patch which
> just adds a comment in virNetlinkCreateSocket to "summarize" what gets
> discovered w/r/t MSG_PEEK and the "correct" minimum version of libnl so
> that the "next" person to come this way will have a chance at
> understanding what needs to be done without going through the submit a
> patch changing the size again!
>
> All that said, having it be configurable could be useful for someone who
> has a system that doesn't have that version, while still working as
> expected for the right version.

I think it may need to be a fairly unusual combination of kernel,
libvirt, and libnl versions, combined with pretty "big" hardware in
order for that to happen. More information from ZhiPeng about the
versions of those packages might allow us to make a better informed
decision.

Workarounds are okay when necessary. But adding a config parameter is
something that would need to be left in forever, leaving more code to
maintain, and all for a bug that shouldn't even be there today, much
less 6 months or a year from now - turning on message peek was supposed
to "eliminate this problem totally and permanently". If it didn't, I'd
like to know why.

ZhipPeng - can you tell us more about your setup? package versions,
hardware, example XML, gdb backtrace at the instant the error message is
logged?

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list