diff mbox series

[ovs-dev] socket: Increase listen backlog to 64 everywhere.

Message ID 20240412024517.4158617-1-ihrachys@redhat.com
State Accepted
Delegated to: Simon Horman
Headers show
Series [ovs-dev] socket: Increase listen backlog to 64 everywhere. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/intel-ovs-compilation success test: success

Commit Message

Ihar Hrachyshka April 12, 2024, 2:45 a.m. UTC
Before the patch, the size of the backlog depended on the type of socket
(UNIX vs INET) as well as on the language (C vs Python), specifically:

- python used backlog size = 10 for all sockets;
- C used 64 for UNIX sockets but 10 for INET sockets.

This consolidates the values across the board. It effectively bumps the
number of simultaneous connections to python unixctl servers to 64. Also
for INET C servers too.

The rationale to do it, on top of consistency, is as follows:

- fmt_pkt in ovn testsuite is limited by python server listen backlog,
  and as was found out when adopting the tool, it is sometimes useful to
  run lots of parallel calls to fmt_pkt unixctl server in some tests.
  (See [1] for example.)

- there is a recent report [2] on discuss@ ML where the reporter noticed
  significant listen queue overflows in some scenarios (large openstack
  deployments; happens during leader transition when hundreds of neutron
  nodes - with dozens of neutron api workers each - simultaneously
  reconnect to the same northbound leader.) Note: While there is no
  clear indication that this backlog size bump would resolve the
  reported issues, it would probably help somewhat.

[1] https://github.com/ovn-org/ovn/commit/0baca3e519756cbe98a32526ccc637bb73468743
[2] https://mail.openvswitch.org/pipermail/ovs-discuss/2024-April/053049.html

Signed-off-by: Ihar Hrachyshka <ihrachys@redhat.com>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
---
 lib/socket-util.c    | 2 +-
 python/ovs/stream.py | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Simon Horman April 12, 2024, 9:33 a.m. UTC | #1
On Fri, Apr 12, 2024 at 02:45:17AM +0000, Ihar Hrachyshka wrote:
> Before the patch, the size of the backlog depended on the type of socket
> (UNIX vs INET) as well as on the language (C vs Python), specifically:
> 
> - python used backlog size = 10 for all sockets;
> - C used 64 for UNIX sockets but 10 for INET sockets.
> 
> This consolidates the values across the board. It effectively bumps the
> number of simultaneous connections to python unixctl servers to 64. Also
> for INET C servers too.
> 
> The rationale to do it, on top of consistency, is as follows:
> 
> - fmt_pkt in ovn testsuite is limited by python server listen backlog,
>   and as was found out when adopting the tool, it is sometimes useful to
>   run lots of parallel calls to fmt_pkt unixctl server in some tests.
>   (See [1] for example.)
> 
> - there is a recent report [2] on discuss@ ML where the reporter noticed
>   significant listen queue overflows in some scenarios (large openstack
>   deployments; happens during leader transition when hundreds of neutron
>   nodes - with dozens of neutron api workers each - simultaneously
>   reconnect to the same northbound leader.) Note: While there is no
>   clear indication that this backlog size bump would resolve the
>   reported issues, it would probably help somewhat.
> 
> [1] https://github.com/ovn-org/ovn/commit/0baca3e519756cbe98a32526ccc637bb73468743
> [2] https://mail.openvswitch.org/pipermail/ovs-discuss/2024-April/053049.html
> 
> Signed-off-by: Ihar Hrachyshka <ihrachys@redhat.com>
> Acked-by: Eelco Chaudron <echaudro@redhat.com>

Acked-by: Simon Horman <horms@ovn.org>
Simon Horman April 23, 2024, 10:54 a.m. UTC | #2
On Fri, Apr 12, 2024 at 10:33:18AM +0100, Simon Horman wrote:
> On Fri, Apr 12, 2024 at 02:45:17AM +0000, Ihar Hrachyshka wrote:
> > Before the patch, the size of the backlog depended on the type of socket
> > (UNIX vs INET) as well as on the language (C vs Python), specifically:
> > 
> > - python used backlog size = 10 for all sockets;
> > - C used 64 for UNIX sockets but 10 for INET sockets.
> > 
> > This consolidates the values across the board. It effectively bumps the
> > number of simultaneous connections to python unixctl servers to 64. Also
> > for INET C servers too.
> > 
> > The rationale to do it, on top of consistency, is as follows:
> > 
> > - fmt_pkt in ovn testsuite is limited by python server listen backlog,
> >   and as was found out when adopting the tool, it is sometimes useful to
> >   run lots of parallel calls to fmt_pkt unixctl server in some tests.
> >   (See [1] for example.)
> > 
> > - there is a recent report [2] on discuss@ ML where the reporter noticed
> >   significant listen queue overflows in some scenarios (large openstack
> >   deployments; happens during leader transition when hundreds of neutron
> >   nodes - with dozens of neutron api workers each - simultaneously
> >   reconnect to the same northbound leader.) Note: While there is no
> >   clear indication that this backlog size bump would resolve the
> >   reported issues, it would probably help somewhat.
> > 
> > [1] https://github.com/ovn-org/ovn/commit/0baca3e519756cbe98a32526ccc637bb73468743
> > [2] https://mail.openvswitch.org/pipermail/ovs-discuss/2024-April/053049.html
> > 
> > Signed-off-by: Ihar Hrachyshka <ihrachys@redhat.com>
> > Acked-by: Eelco Chaudron <echaudro@redhat.com>
> 
> Acked-by: Simon Horman <horms@ovn.org>

Thanks Ihar,

applied to main:

- socket: Increase listen backlog to 64 everywhere.
  https://github.com/openvswitch/ovs/commit/2b7efee031c3
diff mbox series

Patch

diff --git a/lib/socket-util.c b/lib/socket-util.c
index 3eb3a3816..2d89fce85 100644
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -760,7 +760,7 @@  inet_open_passive(int style, const char *target, int default_port,
     }
 
     /* Listen. */
-    if (style == SOCK_STREAM && listen(fd, 10) < 0) {
+    if (style == SOCK_STREAM && listen(fd, 64) < 0) {
         error = sock_errno();
         VLOG_ERR("%s: listen: %s", target, sock_strerror(error));
         goto error;
diff --git a/python/ovs/stream.py b/python/ovs/stream.py
index 82fbb0d68..dbb6b2e1f 100644
--- a/python/ovs/stream.py
+++ b/python/ovs/stream.py
@@ -620,7 +620,7 @@  class PassiveStream(object):
             raise Exception('Unknown connection string')
 
         try:
-            sock.listen(10)
+            sock.listen(64)
         except socket.error as e:
             vlog.err("%s: listen: %s" % (name, os.strerror(e.error)))
             sock.close()