Change in vdsm[ovirt-3.4]: netlink: Introduce socket pool

asegurap at redhat.com asegurap at redhat.com
Wed Feb 26 07:41:39 UTC 2014


Hello Nir Soffer, Dan Kenigsberg,

I'd like you to do a code review.  Please visit

    http://gerrit.ovirt.org/25056

to review the following change.

Change subject: netlink: Introduce socket pool
......................................................................

netlink: Introduce socket pool

If a lot of netlink requests happened _nl_connect would fail at
the C level, more concretely when doing bind, which would return
EADDRINUSE.

This patch addresses that issue as well as reduces the the resource
consumption of the module by reusing the netlink sockets. The reusing
is done by a semaphore protected socket pool.

Change-Id: I657ac3d3e0c2661ce73bdef9aa807ead888a42eb
Signed-off-by: Antoni S. Puimedon <asegurap at redhat.com>
Reviewed-on: http://gerrit.ovirt.org/24603
Reviewed-by: Nir Soffer <nsoffer at redhat.com>
Reviewed-by: Dan Kenigsberg <danken at redhat.com>
---
M lib/vdsm/netlink.py
1 file changed, 61 insertions(+), 27 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/56/25056/1

diff --git a/lib/vdsm/netlink.py b/lib/vdsm/netlink.py
index 68b9e2c..020792f 100644
--- a/lib/vdsm/netlink.py
+++ b/lib/vdsm/netlink.py
@@ -20,9 +20,12 @@
 from contextlib import contextmanager
 from ctypes import (CDLL, CFUNCTYPE, c_char, c_char_p, c_int, c_void_p,
                     c_size_t, get_errno, sizeof)
+from Queue import Empty, Queue
+from threading import BoundedSemaphore
 import errno
 
 NETLINK_ROUTE = 0
+_POOL_SIZE = 5
 CHARBUFFSIZE = 32
 LIBNL = CDLL('libnl.so.1', use_errno=True)
 
@@ -38,6 +41,8 @@
 _nl_connect = CFUNCTYPE(c_int, c_void_p, c_int)(('nl_connect', LIBNL))
 _nl_handle_alloc = CFUNCTYPE(c_void_p)(('nl_handle_alloc', LIBNL))
 _nl_handle_destroy = CFUNCTYPE(None, c_void_p)(('nl_handle_destroy', LIBNL))
+
+_nl_geterror = CFUNCTYPE(c_char_p)(('nl_geterror', LIBNL))
 
 _nl_cache_free = CFUNCTYPE(None, c_void_p)(('nl_cache_free', LIBNL))
 _nl_cache_get_first = _void_proto(('nl_cache_get_first', LIBNL))
@@ -65,40 +70,69 @@
     'rtnl_link_operstate2str', LIBNL))
 
 
+class NLSocketPool(object):
+    """Pool of netlink sockets."""
+    def __init__(self, size):
+        if size <= 0:
+            raise ValueError('Invalid socket pool size %r. Must be positive')
+        self._semaphore = BoundedSemaphore(size)
+        self._sockets = Queue(maxsize=size)
+
+    @contextmanager
+    def socket(self):
+        """Returns a socket from the pool (creating it when needed)."""
+        with self._semaphore:
+            try:
+                sock = self._sockets.get_nowait()
+            except Empty:
+                sock = _open_socket()
+            try:
+                yield sock
+            finally:
+                self._sockets.put_nowait(sock)
+
+
+_pool = NLSocketPool(_POOL_SIZE)
+
+
+def _open_socket():
+    """Returns an open netlink socket."""
+    handle = _nl_handle_alloc()
+    if handle is None:
+        raise IOError(get_errno(), 'Failed to allocate netlink handle')
+
+    err = _nl_connect(handle, NETLINK_ROUTE)
+    if err:
+        _nl_handle_destroy(handle)
+        raise IOError(-err, _nl_geterror())
+    return handle
+
+
+def _close_socket(sock):
+    """Closes and frees the resources of the passed netlink socket."""
+    _nl_handle_destroy(sock)
+
+
 def iter_links():
     """Generator that yields an information dictionary for each link of the
     system."""
-    with _nl_link_cache() as cache:
-        link = _nl_cache_get_first(cache)
-        while link:
-            yield _link_info(cache, link)
-            link = _nl_cache_get_next(link)
+    with _pool.socket() as sock:
+        with _nl_link_cache(sock) as cache:
+            link = _nl_cache_get_first(cache)
+            while link:
+                yield _link_info(cache, link)
+                link = _nl_cache_get_next(link)
 
 
 def get_link(name):
     """Returns the information dictionary of the name specified link."""
-    with _nl_link_cache() as cache:
-        link = _rtnl_link_get_by_name(cache, name)
-        if not link:
-            raise IOError(errno.ENODEV, '%s is not present in the system' %
-                          name)
-        return _link_info(cache, link)
-
-
- at contextmanager
-def _open_nl_socket():
-    """Provides a Netlink socket and closes and destroys it upon exit."""
-    handle = _nl_handle_alloc()
-    if handle is None:
-        raise IOError(get_errno(), 'Failed to allocate netlink handle')
-    try:
-        err = _nl_connect(handle, NETLINK_ROUTE)
-        if err:
-            raise IOError(-err, 'Failed to connect to netlink socket.')
-        yield handle
-    finally:
-        # handle is automatically disconnected on destroy.
-        _nl_handle_destroy(handle)
+    with _pool.socket() as sock:
+        with _nl_link_cache(sock) as cache:
+            link = _rtnl_link_get_by_name(cache, name)
+            if not link:
+                raise IOError(errno.ENODEV, '%s is not present in the system' %
+                              name)
+            return _link_info(cache, link)
 
 
 @contextmanager


-- 
To view, visit http://gerrit.ovirt.org/25056
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I657ac3d3e0c2661ce73bdef9aa807ead888a42eb
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.4
Gerrit-Owner: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>


More information about the vdsm-patches mailing list