Change in vdsm[master]: netinfo: optimize ipaddr retrieval

asegurap at redhat.com asegurap at redhat.com
Tue Jan 21 15:05:59 UTC 2014


Antoni Segura Puimedon has uploaded a new change for review.

Change subject: netinfo: optimize ipaddr retrieval
......................................................................

netinfo: optimize ipaddr retrieval

We used to retrieve IP address information for each device when
getting capabilities or in general doing netinfo.get. This in turn
was calling python ethtool that was doing:
One netlink roundtrip to get the etherinfo object. One round trip
for ipv4 and one for ipv6.

For 200 bridged, vlanned, bonded (with 2 nics) networks this meant:
200bridges + 200vlans + 1 bond + 2 nics = 403 devices
+
200 networks
= 603 calls to getIpInfo for 0.7867s

This patch changes the implementation so that we use netlink to
retrieve all the ipaddrs and then we match them to the devices
we have, saving most of the work.

= 603 calls to getIpInfo for 0.009371s (0.03327s spent on vdsm.netlink from
netinfo.get)

Change-Id: I571529f6dd379ffb317625fd9e2abd6b5ae88696
Signed-off-by: Antoni S. Puimedon <asegurap at redhat.com>
---
M lib/vdsm/netinfo.py
M lib/vdsm/netlink.py
2 files changed, 99 insertions(+), 28 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/18/23518/1

diff --git a/lib/vdsm/netinfo.py b/lib/vdsm/netinfo.py
index 7af05fd..e089483 100644
--- a/lib/vdsm/netinfo.py
+++ b/lib/vdsm/netinfo.py
@@ -18,7 +18,7 @@
 # Refer to the README and COPYING files for full details of the license
 #
 
-from collections import namedtuple
+from collections import defaultdict, namedtuple
 import errno
 from glob import iglob
 from itertools import chain
@@ -42,6 +42,7 @@
 from .ipwrapper import linkShowDev
 from .utils import anyFnmatch
 from .netconfpersistence import RunningConfig
+from .netlink import iter_addrs
 
 
 NET_CONF_DIR = '/etc/sysconfig/network-scripts/'
@@ -288,7 +289,7 @@
 def prefix2netmask(prefix):
     if not 0 <= prefix <= 32:
         raise ValueError('%s is not a valid prefix value. It must be between '
-                         '0 and 32')
+                         '0 and 32' % prefix)
     return socket.inet_ntoa(
         struct.pack("!I", int('1' * prefix + '0' * (32 - prefix), 2)))
 
@@ -310,15 +311,17 @@
     return gateways.get(dev, '')
 
 
-def getIpInfo(dev):
-    devInfo = ethtool.get_interfaces_info(dev.encode('utf8'))[0]
-    addr = devInfo.ipv4_address
-    netmask = devInfo.ipv4_netmask
-    ipv6addrs = devInfo.get_ipv6_addresses()
-
-    return (addr if addr else '',
-            prefix2netmask(netmask) if netmask else '',
-            [addr6.address + '/' + str(addr6.netmask) for addr6 in ipv6addrs])
+def getIpInfo(dev, ipAddrs):
+    ipv4addr = ''
+    ipv4netmask = ''
+    ipv6addrs = []
+    for addr in ipAddrs[dev]:
+        if addr['family'] == 'inet':
+            ipv4addr, prefix = addr['address'].split('/')
+            ipv4netmask = prefix2netmask(addr['prefixlen'])
+        else:
+            ipv6addrs.append(addr['address'])
+    return ipv4addr, ipv4netmask, ipv6addrs
 
 
 def getipv6addrs(dev):
@@ -470,7 +473,8 @@
     return paddr
 
 
-def _getNetInfo(iface, bridged, gateways, ipv6routes, qosInbound, qosOutbound):
+def _getNetInfo(iface, bridged, gateways, ipv6routes, ipAddrs, qosInbound,
+                qosOutbound):
     '''Returns a dictionary of properties about the network's interface status.
     Raises a KeyError if the iface does not exist.'''
     data = {}
@@ -484,7 +488,7 @@
             # comment when the version is no longer supported.
             data['interface'] = iface
 
-        ipv4addr, ipv4netmask, ipv6addrs = getIpInfo(iface)
+        ipv4addr, ipv4netmask, ipv6addrs = getIpInfo(iface, ipAddrs)
         data.update({'iface': iface, 'bridged': bridged,
                      'addr': ipv4addr, 'netmask': ipv4netmask,
                      'gateway': getgateway(gateways, iface),
@@ -504,41 +508,48 @@
     return data
 
 
-def _bridgeinfo(link, gateways, ipv6routes):
-    info = _devinfo(link)
+def _bridgeinfo(link, gateways, ipv6routes, ipAddrs):
+    info = _devinfo(link, ipAddrs)
     info.update({'gateway': getgateway(gateways, link.name),
                 'ipv6gateway': ipv6routes.get(link.name, '::'),
                 'ports': ports(link.name), 'stp': bridge_stp_state(link.name)})
     return info
 
 
-def _nicinfo(link, paddr):
-    info = _devinfo(link)
+def _nicinfo(link, paddr, ipAddrs):
+    info = _devinfo(link, ipAddrs)
     info.update({'hwaddr': link.address, 'speed': nicSpeed(link.name)})
     if paddr.get(link.name):
         info['permhwaddr'] = paddr[link.name]
     return info
 
 
-def _bondinfo(link):
-    info = _devinfo(link)
+def _bondinfo(link, ipAddrs):
+    info = _devinfo(link, ipAddrs)
     info.update({'hwaddr': link.address, 'slaves': slaves(link.name)})
     return info
 
 
-def _vlaninfo(link):
-    info = _devinfo(link)
+def _vlaninfo(link, ipAddrs):
+    info = _devinfo(link, ipAddrs)
     info.update({'iface': link.device, 'vlanid': link.vlanid})
     return info
 
 
-def _devinfo(link):
-    ipv4addr, ipv4netmask, ipv6addrs = getIpInfo(link.name)
+def _devinfo(link, ipAddrs):
+    ipv4addr, ipv4netmask, ipv6addrs = getIpInfo(link.name, ipAddrs)
     return {'addr': ipv4addr,
             'cfg': getIfaceCfg(link.name),
             'ipv6addrs': ipv6addrs,
             'mtu': str(link.mtu),
             'netmask': ipv4netmask}
+
+
+def _getIpAddrs():
+    addrs = defaultdict(list)
+    for addr in iter_addrs():
+        addrs[addr['label']].append(addr)
+    return addrs
 
 
 def get():
@@ -547,12 +558,13 @@
     gateways = getRoutes()
     ipv6routes = getIPv6Routes()
     paddr = permAddr()
+    ipAddrs = _getIpAddrs()
 
     for net, netAttr in networks().iteritems():
         try:
             d['networks'][net] = _getNetInfo(netAttr.get('iface', net),
                                              netAttr['bridged'], gateways,
-                                             ipv6routes,
+                                             ipv6routes, ipAddrs,
                                              netAttr.get('qosInbound'),
                                              netAttr.get('qosOutbound'))
         except KeyError:
@@ -561,13 +573,13 @@
     for dev in (link for link in getLinks() if not link.isHidden()):
         if dev.isBRIDGE():
             d['bridges'][dev.name] = \
-                _bridgeinfo(dev, gateways, ipv6routes)
+                _bridgeinfo(dev, gateways, ipv6routes, ipAddrs)
         elif dev.isNICLike():
-            d['nics'][dev.name] = _nicinfo(dev, paddr)
+            d['nics'][dev.name] = _nicinfo(dev, paddr, ipAddrs)
         elif dev.isBOND():
-            d['bondings'][dev.name] = _bondinfo(dev)
+            d['bondings'][dev.name] = _bondinfo(dev, ipAddrs)
         elif dev.isVLAN():
-            d['vlans'][dev.name] = _vlaninfo(dev)
+            d['vlans'][dev.name] = _vlaninfo(dev, ipAddrs)
 
     return d
 
diff --git a/lib/vdsm/netlink.py b/lib/vdsm/netlink.py
index 68b9e2c..8b42ccc 100644
--- a/lib/vdsm/netlink.py
+++ b/lib/vdsm/netlink.py
@@ -43,6 +43,7 @@
 _nl_cache_get_first = _void_proto(('nl_cache_get_first', LIBNL))
 _nl_cache_get_next = _void_proto(('nl_cache_get_next', LIBNL))
 _rtnl_link_alloc_cache = _void_proto(('rtnl_link_alloc_cache', LIBNL))
+_rtnl_addr_alloc_cache = _void_proto(('rtnl_addr_alloc_cache', LIBNL))
 
 _rtnl_link_get_addr = _void_proto(('rtnl_link_get_addr', LIBNL))
 _rtnl_link_get_flags = _int_proto(('rtnl_link_get_flags', LIBNL))
@@ -55,6 +56,14 @@
 _rtnl_link_get_qdisc = _char_proto(('rtnl_link_get_qdisc', LIBNL))
 _rtnl_link_vlan_get_id = _int_proto(('rtnl_link_vlan_get_id', LIBNL))
 
+_rtnl_addr_get_label = _char_proto(('rtnl_addr_get_label', LIBNL))
+_rtnl_addr_get_ifindex = _int_proto(('rtnl_addr_get_ifindex', LIBNL))
+_rtnl_addr_get_family = _int_proto(('rtnl_addr_get_family', LIBNL))
+_rtnl_addr_get_prefixlen = _int_proto(('rtnl_addr_get_prefixlen', LIBNL))
+_rtnl_addr_get_scope = _int_proto(('rtnl_addr_get_scope', LIBNL))
+_rtnl_addr_get_flags = _int_proto(('rtnl_addr_get_flags', LIBNL))
+_rtnl_addr_get_local = _void_proto(('rtnl_addr_get_local', LIBNL))
+
 _nl_addr2str = CFUNCTYPE(c_char_p, c_void_p, c_char_p, c_int)((
     'nl_addr2str', LIBNL))
 _rtnl_link_get_by_name = CFUNCTYPE(c_void_p, c_void_p, c_char_p)((
@@ -63,6 +72,9 @@
     'rtnl_link_i2name', LIBNL))
 _rtnl_link_operstate2str = CFUNCTYPE(c_char_p, c_int, c_char_p, c_size_t)((
     'rtnl_link_operstate2str', LIBNL))
+_nl_af2str = CFUNCTYPE(c_char_p, c_int, c_char_p, c_int)(('nl_af2str', LIBNL))
+_rtnl_scope2str = CFUNCTYPE(c_char_p, c_int, c_char_p, c_int)((
+    'rtnl_scope2str', LIBNL))
 
 
 def iter_links():
@@ -73,6 +85,14 @@
         while link:
             yield _link_info(cache, link)
             link = _nl_cache_get_next(link)
+
+
+def iter_addrs():
+    with _nl_addr_cache() as cache:
+        addr = _nl_cache_get_first(cache)
+        while addr:
+            yield _addr_info(cache, addr)
+            addr = _nl_cache_get_next(addr)
 
 
 def get_link(name):
@@ -112,6 +132,30 @@
             yield cache
         finally:
             _nl_cache_free(cache)
+
+
+ at contextmanager
+def _nl_addr_cache():
+    """Provides a link cache and frees it and its links upon exit."""
+    with _open_nl_socket() as sock:
+        cache = _rtnl_addr_alloc_cache(sock)
+        if cache is None:
+            raise IOError(get_errno(), 'Failed to allocate link cache.')
+        try:
+            yield cache
+        finally:
+            _nl_cache_free(cache)
+
+
+def _addr_info(cache, addr):
+    return {'label': _rtnl_addr_get_label(addr),
+            'index': _rtnl_addr_get_ifindex(addr),
+            'family': _addr_family(addr),
+            'prefixlen': _rtnl_addr_get_prefixlen(addr),
+            'scope': _addr_scope(addr),
+            'flags': _rtnl_addr_get_flags(addr),
+            'address': _addr_local(addr),
+            }
 
 
 def _link_info(cache, link):
@@ -167,3 +211,18 @@
     state = _rtnl_link_get_operstate(link)
     operstate = (c_char * CHARBUFFSIZE)()
     return _rtnl_link_operstate2str(state, operstate, sizeof(operstate))
+
+
+def _addr_scope(addr):
+    scope = (c_char * CHARBUFFSIZE)()
+    return _rtnl_scope2str(_rtnl_addr_get_scope(addr), scope, sizeof(scope))
+
+
+def _addr_family(addr):
+    family = (c_char * CHARBUFFSIZE)()
+    return _nl_af2str(_rtnl_addr_get_family(addr), family, sizeof(family))
+
+
+def _addr_local(addr):
+    address = (c_char * CHARBUFFSIZE)()
+    return _nl_addr2str(_rtnl_addr_get_local(addr), address, sizeof(address))


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I571529f6dd379ffb317625fd9e2abd6b5ae88696
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon <asegurap at redhat.com>


More information about the vdsm-patches mailing list