mirror/CumulusNetworks-ifupdown2
1
0
Fork 0
mirror of https://github.com/CumulusNetworks/ifupdown2.git synced 2024-05-06 15:54:50 +00:00
Code Activity

performance fix: better handling fd to allow subprocess.close_fds=False and code re-organisation

Browse Source 
Ticket: None
Reviewed By: CCR-4692
Testing Done: smoke + scale tests

If called with close_fds=True the subprocess module will try to close every fd
from 3 to MAXFD before executing the specified command. This is done in Python
not even with a C-implementation which truly affecting performances.

This patch aims to better handle the file descriptor used by ifupdown2. Either
by closing them after use or by setting the close-on-exec flag for the file
descriptor, which causes the file descriptor to be automatically
(and atomically) closed when any of the exec-family functions succeed.

With the actual patch all tests are passing, I can't think of any future issue
but if any a possible future modification might be to use the parameter
'preexec_fn', which allows us to set function which will be executed in the
child process before executing the command line. We can always manually close
any remaining open file descriptors with something like:

>>> os.listdir('/proc/self/fd/')
['0', '1', '2', ‘3’, etc..]
>>> for fd in os.listdir('/proc/self/fd/')
>>>    if int(fd) > 2:
>>>    	  os.close(fd)

This patch is also totally re-organising the use of subprocesses. By removing
all subprocess code redundancy.
This commit is contained in:
Julien Fortin
2016-05-13 19:52:57 +02:00
parent afe5125163
commit a193d8d1c0
18 changed files with 343 additions and 464 deletions
Download Patch File Download Diff File Expand all files Collapse all files

98
ifupdownaddons/bridgeutils.py
View File

@@ -9,6 +9,7 @@ from utilsbase import *
import os
import re
import logging
from ifupdown.utils import utils
import ifupdown.ifupdownflags as ifupdownflags
from cache import *
@@ -63,7 +64,7 @@ class brctl(utilsBase):
battrs = {}
bports = {}
brout = self.exec_command('/sbin/brctl showstp %s' %bridgename)
brout = utils.exec_command('/sbin/brctl showstp %s' % bridgename)
chunks = re.split(r'\n\n', brout, maxsplit=0, flags=re.MULTILINE)
try:
@@ -133,9 +134,9 @@ class brctl(utilsBase):
except:
pass
if not bridgename:
brctlout = self.exec_command('/sbin/brctl show')
brctlout = utils.exec_command('/sbin/brctl show')
else:
brctlout = self.exec_command('/sbin/brctl show ' + bridgename)
brctlout = utils.exec_command('/sbin/brctl show %s' % bridgename)
if not brctlout:
return
@@ -202,13 +203,13 @@ class brctl(utilsBase):
def create_bridge(self, bridgename):
if self.bridge_exists(bridgename):
return
self.exec_command('/sbin/brctl addbr %s' %bridgename)
utils.exec_command('/sbin/brctl addbr %s' % bridgename)
self._cache_update([bridgename], {})
def delete_bridge(self, bridgename):
if not self.bridge_exists(bridgename):
return
self.exec_command('/sbin/brctl delbr %s' %bridgename)
utils.exec_command('/sbin/brctl delbr %s' % bridgename)
self._cache_invalidate()
def add_bridge_port(self, bridgename, bridgeportname):
@@ -216,8 +217,8 @@ class brctl(utilsBase):
ports = self._cache_get([bridgename, 'linkinfo', 'ports'])
if ports and ports.get(bridgeportname):
return
self.exec_command('/sbin/brctl addif ' + bridgename + ' ' +
bridgeportname)
utils.exec_command('/sbin/brctl addif %s %s' %
(bridgename, bridgeportname))
self._cache_update([bridgename, 'linkinfo', 'ports',
bridgeportname], {})
@@ -226,8 +227,8 @@ class brctl(utilsBase):
ports = self._cache_get([bridgename, 'linkinfo', 'ports'])
if not ports or not ports.get(bridgeportname):
return
self.exec_command('/sbin/brctl delif ' + bridgename + ' ' +
bridgeportname)
utils.exec_command('/sbin/brctl delif %s %s' %
(bridgename, bridgeportname))
self._cache_delete([bridgename, 'linkinfo', 'ports',
'bridgeportname'])
@@ -240,16 +241,19 @@ class brctl(utilsBase):
curval = portattrs.get(k)
if curval and curval == v:
continue
self.exec_command('/sbin/brctl set%s %s %s %s'
%(k, bridgename, bridgeportname, v))
utils.exec_command('/sbin/brctl set%s %s %s %s' %
(k, bridgename, bridgeportname, v))
def set_bridgeport_attr(self, bridgename, bridgeportname,
attrname, attrval):
if self._cache_check([bridgename, 'linkinfo', 'ports',
bridgeportname, attrname], attrval):
return
self.exec_command('/sbin/brctl set%s %s %s %s' %(attrname, bridgename,
bridgeportname, attrval))
utils.exec_command('/sbin/brctl set%s %s %s %s' %
(attrname,
bridgename,
bridgeportname,
attrval))
def set_bridge_attrs(self, bridgename, attrdict):
for k, v in attrdict.iteritems():
@@ -258,8 +262,8 @@ class brctl(utilsBase):
if self._cache_check([bridgename, 'linkinfo', k], v):
continue
try:
self.exec_command('/sbin/brctl set%s %s %s'
%(k, bridgename, v))
cmd = '/sbin/brctl set%s %s %s' % (k, bridgename, v)
utils.exec_command(cmd)
except Exception, e:
self.logger.warn('%s: %s' %(bridgename, str(e)))
pass
@@ -267,8 +271,8 @@ class brctl(utilsBase):
def set_bridge_attr(self, bridgename, attrname, attrval):
if self._cache_check([bridgename, 'linkinfo', attrname], attrval):
return
self.exec_command('/sbin/brctl set%s %s %s'
%(attrname, bridgename, attrval))
utils.exec_command('/sbin/brctl set%s %s %s' %
(attrname, bridgename, attrval))
def get_bridge_attrs(self, bridgename):
return self._cache_get([bridgename, 'linkinfo'])
@@ -282,7 +286,7 @@ class brctl(utilsBase):
bridgeportname, attrname])
def set_stp(self, bridge, stp_state):
self.exec_command('/sbin/brctl stp ' + bridge + ' ' + stp_state)
utils.exec_command('/sbin/brctl stp %s %s' % (bridge, stp_state))
def get_stp(self, bridge):
sysfs_stpstate = '/sys/class/net/%s/bridge/stp_state' %bridge
@@ -314,22 +318,21 @@ class brctl(utilsBase):
return preprocess_func(value)
def set_ageing(self, bridge, ageing):
self.exec_command('/sbin/brctl setageing ' + bridge + ' ' + ageing)
utils.exec_command('/sbin/brctl setageing %s %s' % (bridge, ageing))
def get_ageing(self, bridge):
return self.read_value_from_sysfs('/sys/class/net/%s/bridge/ageing_time'
%bridge, self.conv_value_to_user)
def set_bridgeprio(self, bridge, bridgeprio):
self.exec_command('/sbin/brctl setbridgeprio ' + bridge + ' ' +
bridgeprio)
def set_bridgeprio(self, bridge, prio):
utils.exec_command('/sbin/brctl setbridgeprio %s %s' % (bridge, prio))
def get_bridgeprio(self, bridge):
return self.read_file_oneline(
'/sys/class/net/%s/bridge/priority' %bridge)
def set_fd(self, bridge, fd):
self.exec_command('/sbin/brctl setfd ' + bridge + ' ' + fd)
utils.exec_command('/sbin/brctl setfd %s %s' % (bridge, fd))
def get_fd(self, bridge):
return self.read_value_from_sysfs(
@@ -341,51 +344,51 @@ class brctl(utilsBase):
raise Exception('set_gcint not implemented')
def set_hello(self, bridge, hello):
self.exec_command('/sbin/brctl sethello ' + bridge + ' ' + hello)
utils.exec_command('/sbin/brctl sethello %s %s' % (bridge, hello))
def get_hello(self, bridge):
return self.read_value_from_sysfs('/sys/class/net/%s/bridge/hello_time'
%bridge, self.conv_value_to_user)
def set_maxage(self, bridge, maxage):
self.exec_command('/sbin/brctl setmaxage ' + bridge + ' ' + maxage)
utils.exec_command('/sbin/brctl setmaxage %s %s' % (bridge, maxage))
def get_maxage(self, bridge):
return self.read_value_from_sysfs('/sys/class/net/%s/bridge/max_age'
%bridge, self.conv_value_to_user)
def set_pathcost(self, bridge, port, pathcost):
self.exec_command('/sbin/brctl setpathcost %s' %bridge + ' %s' %port +
' %s' %pathcost)
utils.exec_command('/sbin/brctl setpathcost %s %s %s' %
(bridge, port, pathcost))
def get_pathcost(self, bridge, port):
return self.read_file_oneline('/sys/class/net/%s/brport/path_cost'
%port)
def set_portprio(self, bridge, port, prio):
self.exec_command('/sbin/brctl setportprio %s' %bridge + ' %s' %port +
' %s' %prio)
utils.exec_command('/sbin/brctl setportprio %s %s %s' %
(bridge, port, prio))
def get_portprio(self, bridge, port):
return self.read_file_oneline('/sys/class/net/%s/brport/priority'
%port)
def set_hashmax(self, bridge, hashmax):
self.exec_command('/sbin/brctl sethashmax %s' %bridge + ' %s' %hashmax)
utils.exec_command('/sbin/brctl sethashmax %s %s' % (bridge, hashmax))
def get_hashmax(self, bridge):
return self.read_file_oneline('/sys/class/net/%s/bridge/hash_max'
%bridge)
def set_hashel(self, bridge, hashel):
self.exec_command('/sbin/brctl sethashel %s' %bridge + ' %s' %hashel)
utils.exec_command('/sbin/brctl sethashel %s %s' % (bridge, hashel))
def get_hashel(self, bridge):
return self.read_file_oneline('/sys/class/net/%s/bridge/hash_elasticity'
%bridge)
def set_mclmc(self, bridge, mclmc):
self.exec_command('/sbin/brctl setmclmc %s' %bridge + ' %s' %mclmc)
utils.exec_command('/sbin/brctl setmclmc %s %s' % (bridge, mclmc))
def get_mclmc(self, bridge):
return self.read_file_oneline(
@@ -393,24 +396,21 @@ class brctl(utilsBase):
%bridge)
def set_mcrouter(self, bridge, mcrouter):
self.exec_command('/sbin/brctl setmcrouter %s' %bridge +
' %s' %mcrouter)
utils.exec_command('/sbin/brctl setmcrouter %s %s' % (bridge, mcrouter))
def get_mcrouter(self, bridge):
return self.read_file_oneline(
'/sys/class/net/%s/bridge/multicast_router' %bridge)
def set_mcsnoop(self, bridge, mcsnoop):
self.exec_command('/sbin/brctl setmcsnoop %s' %bridge +
' %s' %mcsnoop)
utils.exec_command('/sbin/brctl setmcsnoop %s %s' % (bridge, mcsnoop))
def get_mcsnoop(self, bridge):
return self.read_file_oneline(
'/sys/class/net/%s/bridge/multicast_snooping' %bridge)
def set_mcsqc(self, bridge, mcsqc):
self.exec_command('/sbin/brctl setmcsqc %s' %bridge +
' %s' %mcsqc)
utils.exec_command('/sbin/brctl setmcsqc %s %s' % (bridge, mcsqc))
def get_mcsqc(self, bridge):
return self.read_file_oneline(
@@ -418,8 +418,8 @@ class brctl(utilsBase):
%bridge)
def set_mcqifaddr(self, bridge, mcqifaddr):
self.exec_command('/sbin/brctl setmcqifaddr %s' %bridge +
' %s' %mcqifaddr)
utils.exec_command('/sbin/brctl setmcqifaddr %s %s' %
(bridge, mcqifaddr))
def get_mcqifaddr(self, bridge):
return self.read_file_oneline(
@@ -427,8 +427,8 @@ class brctl(utilsBase):
%bridge)
def set_mcquerier(self, bridge, mcquerier):
self.exec_command('/sbin/brctl setmcquerier %s' %bridge +
' %s' %mcquerier)
utils.exec_command('/sbin/brctl setmcquerier %s %s' %
(bridge, mcquerier))
def get_mcquerier(self, bridge):
return self.read_file_oneline(
@@ -448,15 +448,15 @@ class brctl(utilsBase):
self.logger.warn('mcqv4src \'%s\' invalid IPv4 address' %mcquerier)
return
self.exec_command('/sbin/brctl setmcqv4src %s' %bridge +
' %d %s' %(vlan, mcquerier))
utils.exec_command('/sbin/brctl setmcqv4src %s %d %s' %
(bridge, vlan, mcquerier))
def del_mcqv4src(self, bridge, vlan):
self.exec_command('/sbin/brctl delmcqv4src %s %d' %(bridge, vlan))
utils.exec_command('/sbin/brctl delmcqv4src %s %d' % (bridge, vlan))
def get_mcqv4src(self, bridge, vlan=None):
mcqv4src = {}
mcqout = self.exec_command('/sbin/brctl showmcqv4src %s' %bridge)
mcqout = utils.exec_command('/sbin/brctl showmcqv4src %s' % bridge)
if not mcqout: return None
mcqlines = mcqout.splitlines()
for l in mcqlines[1:]:
@@ -470,8 +470,7 @@ class brctl(utilsBase):
return mcqv4src
def set_mclmi(self, bridge, mclmi):
self.exec_command('/sbin/brctl setmclmi %s' %bridge +
' %s' %mclmi)
utils.exec_command('/sbin/brctl setmclmi %s %s' % (bridge, mclmi))
def get_mclmi(self, bridge):
return self.read_file_oneline(
@@ -479,8 +478,7 @@ class brctl(utilsBase):
%bridge)
def set_mcmi(self, bridge, mcmi):
self.exec_command('/sbin/brctl setmcmi %s' %bridge +
' %s' %mcmi)
utils.exec_command('/sbin/brctl setmcmi %s %s' % (bridge, mcmi))
def get_mcmi(self, bridge):
return self.read_file_oneline(