[PATCH 1/2] Controller: VirtUtils: remove unused _brctl function

[Jan Tluka]

Signed-off-by: Jan Tluka 
---
 lnst/Controller/VirtUtils.py | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/lnst/Controller/VirtUtils.py b/lnst/Controller/VirtUtils.py
index 7c649e8..c660323 100644
--- a/lnst/Controller/VirtUtils.py
+++ b/lnst/Controller/VirtUtils.py
@@ -36,12 +36,6 @@ def _ip(cmd):
     except ExecCmdFail as err:
         raise VirtUtilsError("ip command error: %s" % err)

-def _brctl(cmd):
-    try:
-        exec_cmd("brctl %s" % cmd)
-    except ExecCmdFail as err:
-        raise VirtUtilsError("brctl error: %s" % err)
-
 def _iptables(cmd):
     try:
         exec_cmd("iptables %s" % cmd)

[PATCH 2/2] Slave: fallback to iproute commands if brctl is missing

[Jan Tluka]

Bridge device can be configured by iproute for quite a long time and brctl
is getting deprecated in recent distributions.

This patch replaces brctl with iproute alternative when creating a bridge
device. For bridge port addition and removal both brctl and iproute method
is provided depending on brctl availability.

Tested on RHEL6, RHEL7 and Fedora28.

Signed-off-by: Jan Tluka 
---
 lnst/Slave/NetConfigDevice.py | 26 +++++++++++++++++++++-----
 lnst/Slave/NmConfigDevice.py  |  2 +-
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/lnst/Slave/NetConfigDevice.py b/lnst/Slave/NetConfigDevice.py
index 03efd40..ab28a0f 100644
--- a/lnst/Slave/NetConfigDevice.py
+++ b/lnst/Slave/NetConfigDevice.py
@@ -18,6 +18,7 @@ from lnst.Slave.NetConfigCommon import get_slaves, get_option, get_slave_option
 from lnst.Slave.NetConfigCommon import parse_netem, get_slave_options
 from lnst.Common.Utils import bool_it
 from lnst.Common.Utils import check_process_running
+from lnst.Common.Utils import is_installed
 from lnst.Slave.NmConfigDevice import type_class_mapping as nm_type_class_mapping
 from lnst.Slave.NmConfigDevice import is_nm_managed

@@ -267,7 +268,8 @@ class NetConfigDeviceBridge(NetConfigDeviceGeneric):
     _modulename = "bridge"

def _add_rm_bridge(self, prefix):
-        exec_cmd("brctl %sbr %s " % (prefix, self._dev_config["name"]))
+        exec_cmd("ip link %s %s type bridge " % (prefix,
+                                                 self._dev_config["name"]))

def _get_bridge_dir(self):
         return "/sys/class/net/%s/bridge" % self._dev_config["name"]
@@ -282,14 +284,28 @@ class NetConfigDeviceBridge(NetConfigDeviceGeneric):
                                             self._get_bridge_dir(),
                                             option))

-    def _add_rm_port(self, prefix, slave_id):
+    def _add_rm_port_iproute(self, action, slave_id):
         port_name = self._if_manager.get_mapped_device(slave_id).get_name()
-        exec_cmd("brctl %sif %s %s" % (prefix, self._dev_config["name"],
+        if action == "add":
+            exec_cmd("ip link set %s master %s" % (port_name,
+                                                   self._dev_config["name"]))
+        elif action == "del":
+            exec_cmd("ip link set %s nomaster" % port_name)
+
+    def _add_rm_port_brctl(self, action, slave_id):
+        port_name = self._if_manager.get_mapped_device(slave_id).get_name()
+        exec_cmd("brctl %sif %s %s" % (action, self._dev_config["name"],
                                        port_name))

-    def _add_rm_ports(self, prefix):
+    def _add_rm_port(self, action, slave_id):
+        if is_installed("brctl"):
+            self._add_rm_port_brctl(action, slave_id)
+        else:
+            self._add_rm_port_iproute(action, slave_id)
+
+    def _add_rm_ports(self, action):
         for slave_id in get_slaves(self._dev_config):
-            self._add_rm_port(prefix, slave_id)
+            self._add_rm_port(action, slave_id)

def create(self):
         self._add_rm_bridge("add")
diff --git a/lnst/Slave/NmConfigDevice.py b/lnst/Slave/NmConfigDevice.py
index 29c2c75..2c04948 100644
--- a/lnst/Slave/NmConfigDevice.py
+++ b/lnst/Slave/NmConfigDevice.py
@@ -614,7 +614,7 @@ class NmConfigDeviceBridge(NmConfigDeviceGeneric):
         if get_nm_version() < "0.9.9":
             try:
                 exec_cmd("ip link set %s down" % config["name"])
-                exec_cmd("brctl delbr %s " % config["name"])
+                exec_cmd("ip link del %s type bridge" % config["name"])
             except:
                 pass

[PATCH 2/2] Slave: fallback to iproute commands if brctl is missing

[Ondrej Lichtner]

On Wed, Oct 10, 2018 at 10:23:16AM +0200, Jan Tluka wrote:
> Bridge device can be configured by iproute for quite a long time and brctl
> is getting deprecated in recent distributions.
>
> This patch replaces brctl with iproute alternative when creating a bridge
> device. For bridge port addition and removal both brctl and iproute method
> is provided depending on brctl availability.
>
> Tested on RHEL6, RHEL7 and Fedora28.
> Signed-off-by: Jan Tluka
>
> lnst/Slave/NetConfigDevice.py | 26 +++++++++++++++++++++-----
>  lnst/Slave/NmConfigDevice.py  |  2 +-
>  2 files changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/lnst/Slave/NetConfigDevice.py b/lnst/Slave/NetConfigDevice.py
> index 03efd40..ab28a0f 100644
> --- a/lnst/Slave/NetConfigDevice.py
> +++ b/lnst/Slave/NetConfigDevice.py
> @@ -18,6 +18,7 @@ from lnst.Slave.NetConfigCommon import get_slaves, get_option, get_slave_option
>  from lnst.Slave.NetConfigCommon import parse_netem, get_slave_options
>  from lnst.Common.Utils import bool_it
>  from lnst.Common.Utils import check_process_running
> +from lnst.Common.Utils import is_installed
>  from lnst.Slave.NmConfigDevice import type_class_mapping as nm_type_class_mapping
>  from lnst.Slave.NmConfigDevice import is_nm_managed
>
> @@ -267,7 +268,8 @@ class NetConfigDeviceBridge(NetConfigDeviceGeneric):
>      _modulename = "bridge"
>  def _add_rm_bridge(self, prefix):
>    exec_cmd("brctl %sbr %s " % (prefix, self._dev_config["name"]))
>    exec_cmd("ip link %s %s type bridge " % (prefix,
>                                             self._dev_config["name"]))
>
> def _get_bridge_dir(self):
>        return "/sys/class/net/%s/bridge" % self._dev_config["name"]
>
> @@ -282,14 +284,28 @@ class NetConfigDeviceBridge(NetConfigDeviceGeneric):
>                                              self._get_bridge_dir(),
>                                              option))
> def _add_rm_port(self, prefix, slave_id):
> def _add_rm_port_iproute(self, action, slave_id):
>     port_name = self._if_manager.get_mapped_device(slave_id).get_name()
>    exec_cmd("brctl %sif %s %s" % (prefix, self._dev_config["name"],
>    if action == "add":
>        exec_cmd("ip link set %s master %s" % (port_name,
>                                               self._dev_config["name"]))
>    elif action == "del":
>        exec_cmd("ip link set %s nomaster" % port_name)
> def _add_rm_port_brctl(self, action, slave_id):
>    port_name = self._if_manager.get_mapped_device(slave_id).get_name()
>    exec_cmd("brctl %sif %s %s" % (action, self._dev_config["name"],
>                                   port_name))
>
> def _add_rm_ports(self, prefix):
> def _add_rm_port(self, action, slave_id):
>    if is_installed("brctl"):
>        self._add_rm_port_brctl(action, slave_id)
>    else:
>        self._add_rm_port_iproute(action, slave_id)
> def _add_rm_ports(self, action):
>     for slave_id in get_slaves(self._dev_config):
>        self._add_rm_port(prefix, slave_id)
>        self._add_rm_port(action, slave_id)
>
> def create(self):
>        self._add_rm_bridge("add")
>
> diff --git a/lnst/Slave/NmConfigDevice.py b/lnst/Slave/NmConfigDevice.py
> index 29c2c75..2c04948 100644
> --- a/lnst/Slave/NmConfigDevice.py
> +++ b/lnst/Slave/NmConfigDevice.py
> @@ -614,7 +614,7 @@ class NmConfigDeviceBridge(NmConfigDeviceGeneric):
>          if get_nm_version() < "0.9.9":
>              try:
>                  exec_cmd("ip link set %s down" % config["name"])
>            exec_cmd("brctl delbr %s " % config["name"])
>            exec_cmd("ip link del %s type bridge" % config["name"])
>        except:
>            pass
>
> -- 
> 2.14.4
> _______________________________________________
> LNST-developers mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]
> Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
> List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
> List Archives: https://lists.fedorahosted.org/archives/list/[e-mail]...

Looks ok. Feel free to push to master. Also consider pushing it to the
python3 branch.

Acked-by: Ondrej Lichtner

[PATCH 2/2] Slave: fallback to iproute commands if brctl is missing

[Jan Tluka]

Wed, Oct 10, 2018 at 10:35:44AM CEST, [e-mail] wrote:
> On Wed, Oct 10, 2018 at 10:23:16AM +0200, Jan Tluka wrote:
> > Bridge device can be configured by iproute for quite a long time and brctl
> > is getting deprecated in recent distributions.
> >
> > This patch replaces brctl with iproute alternative when creating a bridge
> > device. For bridge port addition and removal both brctl and iproute method
> > is provided depending on brctl availability.
> >
> > Tested on RHEL6, RHEL7 and Fedora28.
> >
> > Signed-off-by: Jan Tluka

Hello Jiri, I'd appreciate any comments on this from your side.
In particular I'm interested if there are any differences when the bridge
is configured by brctl or iproute - do they change any of the bridge
defaults, is there difference in bridge port settings?

Additionally, does this change impact any of Mellanox testing? I'd like
to avoid any breakage.

Thanks,
Jan

[PATCH 2/2] Slave: fallback to iproute commands if brctl is missing

[Jan Tluka]

Wed, Oct 10, 2018 at 10:35:44AM CEST, [e-mail] wrote:
> On Wed, Oct 10, 2018 at 10:23:16AM +0200, Jan Tluka wrote:
> > Bridge device can be configured by iproute for quite a long time and brctl
> > is getting deprecated in recent distributions.
> >
> > This patch replaces brctl with iproute alternative when creating a bridge
> > device. For bridge port addition and removal both brctl and iproute method
> > is provided depending on brctl availability.
> >
> > Tested on RHEL6, RHEL7 and Fedora28.
> > Signed-off-by: Jan Tluka
> >
> > lnst/Slave/NetConfigDevice.py | 26 +++++++++++++++++++++-----
> >  lnst/Slave/NmConfigDevice.py  |  2 +-
> >  2 files changed, 22 insertions(+), 6 deletions(-)
> >
> > diff --git a/lnst/Slave/NetConfigDevice.py b/lnst/Slave/NetConfigDevice.py
> > index 03efd40..ab28a0f 100644
> > --- a/lnst/Slave/NetConfigDevice.py
> > +++ b/lnst/Slave/NetConfigDevice.py
> > @@ -18,6 +18,7 @@ from lnst.Slave.NetConfigCommon import get_slaves, get_option, get_slave_option
> >  from lnst.Slave.NetConfigCommon import parse_netem, get_slave_options
> >  from lnst.Common.Utils import bool_it
> >  from lnst.Common.Utils import check_process_running
> > +from lnst.Common.Utils import is_installed
> >  from lnst.Slave.NmConfigDevice import type_class_mapping as nm_type_class_mapping
> >  from lnst.Slave.NmConfigDevice import is_nm_managed
> >
> > @@ -267,7 +268,8 @@ class NetConfigDeviceBridge(NetConfigDeviceGeneric):
> >      _modulename = "bridge"
> >  def _add_rm_bridge(self, prefix):
> >    exec_cmd("brctl %sbr %s " % (prefix, self._dev_config["name"]))
> >    exec_cmd("ip link %s %s type bridge " % (prefix,
> >                                             self._dev_config["name"]))
> >
> > def _get_bridge_dir(self):
> >        return "/sys/class/net/%s/bridge" % self._dev_config["name"]
> >
> > @@ -282,14 +284,28 @@ class NetConfigDeviceBridge(NetConfigDeviceGeneric):
> >                                              self._get_bridge_dir(),
> >                                              option))
> > def _add_rm_port(self, prefix, slave_id):
> > def _add_rm_port_iproute(self, action, slave_id):
> >     port_name = self._if_manager.get_mapped_device(slave_id).get_name()
> >    exec_cmd("brctl %sif %s %s" % (prefix, self._dev_config["name"],
> >    if action == "add":
> >        exec_cmd("ip link set %s master %s" % (port_name,
> >                                               self._dev_config["name"]))
> >    elif action == "del":
> >        exec_cmd("ip link set %s nomaster" % port_name)
> > def _add_rm_port_brctl(self, action, slave_id):
> >    port_name = self._if_manager.get_mapped_device(slave_id).get_name()
> >    exec_cmd("brctl %sif %s %s" % (action, self._dev_config["name"],
> >                                   port_name))
> >
> > def _add_rm_ports(self, prefix):
> > def _add_rm_port(self, action, slave_id):
> >    if is_installed("brctl"):
> >        self._add_rm_port_brctl(action, slave_id)
> >    else:
> >        self._add_rm_port_iproute(action, slave_id)
> > def _add_rm_ports(self, action):
> >     for slave_id in get_slaves(self._dev_config):
> >        self._add_rm_port(prefix, slave_id)
> >        self._add_rm_port(action, slave_id)
> >
> > def create(self):
> >        self._add_rm_bridge("add")
> >
> > diff --git a/lnst/Slave/NmConfigDevice.py b/lnst/Slave/NmConfigDevice.py
> > index 29c2c75..2c04948 100644
> > --- a/lnst/Slave/NmConfigDevice.py
> > +++ b/lnst/Slave/NmConfigDevice.py
> > @@ -614,7 +614,7 @@ class NmConfigDeviceBridge(NmConfigDeviceGeneric):
> >          if get_nm_version() < "0.9.9":
> >              try:
> >                  exec_cmd("ip link set %s down" % config["name"])
> >            exec_cmd("brctl delbr %s " % config["name"])
> >            exec_cmd("ip link del %s type bridge" % config["name"])
> >        except:
> >            pass
> >
> > -- 
> > 2.14.4
> > _______________________________________________
> > LNST-developers mailing list -- [e-mail]
> > To unsubscribe send an email to [e-mail]
> > Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
> > List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
> > List Archives: https://lists.fedorahosted.org/archives/list/[e-mail]...
>
> Looks ok. Feel free to push to master. Also consider pushing it to the
> python3 branch.
>
> Acked-by: Ondrej Lichtner

Ok, I'll wait for Jiri's reply.

Btw, the patches apply cleanly on master-py3.

Thanks
-Jan

[PATCH 2/2] Slave: fallback to iproute commands if brctl is missing

[Jan Tluka]

Wed, Oct 10, 2018 at 10:23:16AM CEST, [e-mail] wrote:
> Bridge device can be configured by iproute for quite a long time and brctl
> is getting deprecated in recent distributions.
>
> This patch replaces brctl with iproute alternative when creating a bridge
> device. For bridge port addition and removal both brctl and iproute method
> is provided depending on brctl availability.
>
> Tested on RHEL6, RHEL7 and Fedora28.
>
> Signed-off-by: Jan Tluka

I pushed the series.

-Jan