Antoni Segura Puimedon has uploaded a new change for review. Change subject: ipwrapper: switch link polling to netlink ......................................................................
ipwrapper: switch link polling to netlink getting the links from iproute2 implied creating a process each time and then parsing the output. Due to the fact that this opearations happen periodically and sometimes several times per second, the cost was too high. Change-Id: I09a120155e3c5be15c237171620e5c996c2af681 Signed-off-by: Antoni S. Puimedon <[email protected]> --- M debian/vdsm-python.install M lib/vdsm/Makefile.am M lib/vdsm/ipwrapper.py A lib/vdsm/netlink.py M tests/ipwrapperTests.py M tests/netinfoTests.py M vdsm.spec.in 7 files changed, 163 insertions(+), 40 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/48/23248/1 diff --git a/debian/vdsm-python.install b/debian/vdsm-python.install index 5ff4848..7a78042 100644 --- a/debian/vdsm-python.install +++ b/debian/vdsm-python.install @@ -10,6 +10,7 @@ ./usr/lib/python2.7/dist-packages/vdsm/libvirtconnection.py ./usr/lib/python2.7/dist-packages/vdsm/netconfpersistence.py ./usr/lib/python2.7/dist-packages/vdsm/netinfo.py +./usr/lib/python2.7/dist-packages/vdsm/netlink.py ./usr/lib/python2.7/dist-packages/vdsm/qemuImg.py ./usr/lib/python2.7/dist-packages/vdsm/tool/__init__.py ./usr/lib/python2.7/dist-packages/vdsm/tool/dummybr.py diff --git a/lib/vdsm/Makefile.am b/lib/vdsm/Makefile.am index 5bdf593..416e1e6 100644 --- a/lib/vdsm/Makefile.am +++ b/lib/vdsm/Makefile.am @@ -29,6 +29,7 @@ libvirtconnection.py \ netconfpersistence.py \ netinfo.py \ + netlink.py \ qemuImg.py \ SecureXMLRPCServer.py \ utils.py \ diff --git a/lib/vdsm/ipwrapper.py b/lib/vdsm/ipwrapper.py index 75f4ad9..23e7dac 100644 --- a/lib/vdsm/ipwrapper.py +++ b/lib/vdsm/ipwrapper.py @@ -37,6 +37,7 @@ from .utils import CommandPath from .utils import execCmd from .utils import grouper +from . import netlink _IP_BINARY = CommandPath('ip', '/sbin/ip') @@ -136,46 +137,24 @@ self.address) @classmethod + def fromDict(cls, data): + # TODO: Tune the following line to reuse the type when libnl1 type + # getting is fixed. + data['linkType'] = _detectType(data['name']) + return cls(**data) + + @classmethod def fromText(cls, text): """Creates a Link object from the textual representation from iproute2's "ip -o -d link show" command.""" attrs = _parseLinkLine(text) if 'linkType' not in attrs: - attrs['linkType'] = cls._detectType(attrs['name']) + attrs['linkType'] = _detectType(attrs['name']) if attrs['linkType'] in (LinkType.VLAN, LinkType.MACVLAN): name, device = attrs['name'].split('@') attrs['name'] = name attrs['device'] = device return cls(**attrs) - - @staticmethod - def _detectType(name): - """Returns the LinkType for the specified device.""" - # TODO: Add support for virtual functions - detectedType = None - try: - driver = _drvinfo(name) - except IOError as ioe: - if ioe.errno == errno.EOPNOTSUPP: - if name == 'lo': - detectedType = LinkType.LOOPBACK - else: - detectedType = LinkType.DUMMY - return detectedType - else: - raise # Reraise other errors like ENODEV - if driver in (LinkType.BRIDGE, LinkType.MACVLAN, LinkType.TUN, - LinkType.OVS, LinkType.TEAM): - detectedType = driver - elif driver == 'bonding': - detectedType = LinkType.BOND - elif 'VLAN' in driver or 'vlan' in driver: - detectedType = LinkType.VLAN - elif os.path.exists('/sys/class/net/%s/device/physfn/' % name): - detectedType = LinkType.VF - else: - detectedType = LinkType.NIC - return detectedType def isBOND(self): return self.type == LinkType.BOND @@ -232,7 +211,7 @@ for path in iglob('/sys/class/net/*/address'): dev = os.path.basename(os.path.dirname(path)) if (dev != self.name and _read_stripped(path) == self.address and - self._detectType(dev) == LinkType.MACVLAN): + _detectType(dev) == LinkType.MACVLAN): return True return False @@ -268,13 +247,12 @@ def getLinks(): """Returns a list of Link objects for each link in the system.""" - return [Link.fromText(line) for line in linksShowDetailed() if - not line.startswith(' ')] + return [Link.fromDict(data) for data in netlink.getLinks()] def getLink(dev): """Returns the Link object for the specified dev.""" - return Link.fromText(linksShowDetailed(dev=dev)[0]) + return Link.fromDict(netlink.getLink(dev)) @equals @@ -657,3 +635,32 @@ def _dev_sysfs_exists(devName): return os.path.exists(os.path.join(NET_SYSFS, devName)) + + +def _detectType(name): + """Returns the LinkType for the specified device.""" + # TODO: Add support for virtual functions + detectedType = None + try: + driver = _drvinfo(name) + except IOError as ioe: + if ioe.errno == errno.EOPNOTSUPP: + if name == 'lo': + detectedType = LinkType.LOOPBACK + else: + detectedType = LinkType.DUMMY + return detectedType + else: + raise # Reraise other errors like ENODEV + if driver in (LinkType.BRIDGE, LinkType.MACVLAN, LinkType.TUN, + LinkType.OVS, LinkType.TEAM): + detectedType = driver + elif driver == 'bonding': + detectedType = LinkType.BOND + elif 'VLAN' in driver or 'vlan' in driver: + detectedType = LinkType.VLAN + elif os.path.exists('/sys/class/net/%s/device/physfn/' % name): + detectedType = LinkType.VF + else: + detectedType = LinkType.NIC + return detectedType diff --git a/lib/vdsm/netlink.py b/lib/vdsm/netlink.py new file mode 100644 index 0000000..7998184 --- /dev/null +++ b/lib/vdsm/netlink.py @@ -0,0 +1,94 @@ +from contextlib import contextmanager +import ctypes +import errno + + +NETLINK_ROUTE = 0 +LIBNL = ctypes.cdll.LoadLibrary('libnl.so.1') + + +def _link_info(cache, link): + data = {'index': LIBNL.rtnl_link_get_ifindex(link)} + data['name'] = ctypes.cast(LIBNL.rtnl_link_get_name(link), + ctypes.c_char_p).value + nl_addr = LIBNL.rtnl_link_get_addr(link) + address = (ctypes.c_char * 32)() + if nl_addr: + LIBNL.nl_addr2str(nl_addr, address, + ctypes.sizeof(address)) + data['address'] = address.value + else: + data['address'] = None + # TODO: libnl-1 has a bug when getting type. Add the following line when + # fixed. + #data['type'] = ctypes.cast(LIBNL.rtnl_link_get_info_type(link), + #ctypes.c_char_p).value + data['flags'] = LIBNL.rtnl_link_get_flags(link) + data['qdisc'] = ctypes.cast(LIBNL.rtnl_link_get_qdisc(link), + ctypes.c_char_p).value + data['mtu'] = LIBNL.rtnl_link_get_mtu(link) + + vlanid = LIBNL.rtnl_link_vlan_get_id(link) + if vlanid >= 0: + data['vlanid'] = vlanid + + master = LIBNL.rtnl_link_get_master(link) + if master >= 1: + master_name = (ctypes.c_char * 32)() + data['master'] = ctypes.cast( + LIBNL.rtnl_link_i2name(cache, master, master_name, + ctypes.sizeof(master_name)), + ctypes.c_char_p).value + + state = LIBNL.rtnl_link_get_operstate(link) + operstate = (ctypes.c_char * 16)() + LIBNL.rtnl_link_operstate2str(state, operstate, ctypes.sizeof(operstate)) + data['state'] = operstate.value + return data + + +def getLinks(): + with _nl_link_cache() as cache: + link = LIBNL.nl_cache_get_first(cache) + links = [] + while link: + links.append(_link_info(cache, link)) + link = LIBNL.nl_cache_get_next(link) + return links + + +def getLink(name): + with _nl_link_cache() as cache: + link = LIBNL.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 +def _open_nl_socket(): + handle = LIBNL.nl_handle_alloc() + if handle is None: + raise IOError(ctypes.get_errno(), 'Failed to allocate netlink handle') + try: + err = LIBNL.nl_connect(handle, NETLINK_ROUTE) + if err: + raise IOError(err, 'Failed to connect to netlink socket.') + yield handle + finally: + # handle is closed automatically on destroy. + LIBNL.nl_handle_destroy(handle) + + +@contextmanager +def _nl_link_cache(): + """Provides a link cache and frees it and its links upon exit.""" + with _open_nl_socket() as sock: + cache = LIBNL.rtnl_link_alloc_cache(sock) + if cache is None: + raise IOError('Failed to allocate link cache.') + try: + yield cache + finally: + LIBNL.nl_cache_free(cache) diff --git a/tests/ipwrapperTests.py b/tests/ipwrapperTests.py index eb28f73..1689aa4 100644 --- a/tests/ipwrapperTests.py +++ b/tests/ipwrapperTests.py @@ -39,7 +39,7 @@ class TestIpwrapper(TestCaseBase): - @MonkeyPatch(Link, '_detectType', partial(_fakeTypeDetection, Link)) + @MonkeyPatch(ipwrapper, '_detectType', partial(_fakeTypeDetection, Link)) def testLinkFromF20Text(self): lines = ('1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state ' 'UNKNOWN mode DEFAULT group default \\ link/loopback ' @@ -92,7 +92,7 @@ self.assertEqual(devices[-5].type, LinkType.DUMMY) self.assertEqual(devices[-3].master, devices[-2].name) - @MonkeyPatch(Link, '_detectType', partial(_fakeTypeDetection, Link)) + @MonkeyPatch(ipwrapper, '_detectType', partial(_fakeTypeDetection, Link)) def testLinkFromRHEL64Text(self): lines = ( '1: lo: <LOOPBACK,UP,LOWER_UP> mtu 16436 qdisc noqueue state ' @@ -285,6 +285,25 @@ iterator.next() +class TestLinks(TestCaseBase): + _bridge = tcTests._Bridge() + + @ValidateRunningAsRoot + def setUp(self): + tcTests._checkDependencies() + self._bridge.addDevice() + + def tearDown(self): + self._bridge.delDevice() + + def testGetLink(self): + link = ipwrapper.getLink(self._bridge.devName) + self.assertTrue(link.isBRIDGE) + self.assertEqual(link.master, None) + self.assertEqual(link.mtu, 1500) + self.assertEqual(link.name, self._bridge.devName) + + class TestDrvinfo(TestCaseBase): _bridge = tcTests._Bridge() diff --git a/tests/netinfoTests.py b/tests/netinfoTests.py index 857dccd..3df4e71 100644 --- a/tests/netinfoTests.py +++ b/tests/netinfoTests.py @@ -53,13 +53,13 @@ self.assertRaises(ValueError, netinfo.prefix2netmask, -1) self.assertRaises(ValueError, netinfo.prefix2netmask, 33) - @MonkeyPatch(ipwrapper.Link, '_detectType', + @MonkeyPatch(ipwrapper, '_detectType', partial(_fakeTypeDetection, ipwrapper.Link)) def testSpeedInvalidNic(self): nicName = '0' * 20 # devices can't have so long names self.assertEqual(netinfo.nicSpeed(nicName), 0) - @MonkeyPatch(ipwrapper.Link, '_detectType', + @MonkeyPatch(ipwrapper, '_detectType', partial(_fakeTypeDetection, ipwrapper.Link)) def testSpeedInRange(self): for d in netinfo.nics(): @@ -90,7 +90,7 @@ for s, addr in zip(inputs, ip): self.assertEqual(addr, netinfo.ipv6StrToAddress(s)) - @MonkeyPatch(ipwrapper.Link, '_detectType', + @MonkeyPatch(ipwrapper, '_detectType', partial(_fakeTypeDetection, ipwrapper.Link)) @MonkeyPatch(netinfo, 'networks', lambda: {'fake': {'bridged': True}}) def testGetNonExistantBridgeInfo(self): @@ -180,7 +180,7 @@ self._testNics), (ipwrapper, '_bondExists', lambda x: x == 'jbond'), - (ipwrapper.Link, '_detectType', + (ipwrapper, '_detectType', partial(_fakeTypeDetection, ipwrapper.Link)), (ipwrapper.Link, '_fakeNics', ['fake*']), (ipwrapper.Link, '_hiddenBonds', ['jb*']), diff --git a/vdsm.spec.in b/vdsm.spec.in index a3fc93d..51095de 100644 --- a/vdsm.spec.in +++ b/vdsm.spec.in @@ -1096,6 +1096,7 @@ %{python_sitearch}/%{vdsm_name}/ipwrapper.py* %{python_sitearch}/%{vdsm_name}/libvirtconnection.py* %{python_sitearch}/%{vdsm_name}/netinfo.py* +%{python_sitearch}/%{vdsm_name}/netlink.py* %{python_sitearch}/%{vdsm_name}/qemuImg.py* %{python_sitearch}/%{vdsm_name}/SecureXMLRPCServer.py* %{python_sitearch}/%{vdsm_name}/netconfpersistence.py* -- To view, visit http://gerrit.ovirt.org/23248 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I09a120155e3c5be15c237171620e5c996c2af681 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon <[email protected]> _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
