Use ip on systems where brctl is not available

[Xen-devel] [PATCH] [tools/hotplug] Use ip on systems where brctl is not available

Posted by Steven Haigh 4 years, 4 months ago

Newer distros like CentOS 8 do not have brctl available. As such, we
can't use it to configure networking anymore.

This patch will fall back to 'ip' or 'bridge' commands if brctl is not
available in the working PATH.

This would be a likely backport candidate to any version expected to be
built on CentOS 8 etc.

---
 tools/hotplug/Linux/colo-proxy-setup      | 30 +++++++++++++++++------
 tools/hotplug/Linux/vif-bridge            | 16 ++++++++----
 tools/hotplug/Linux/vif2                  | 12 +++++++--
 tools/hotplug/Linux/xen-network-common.sh | 16 +++++++++---
 4 files changed, 55 insertions(+), 19 deletions(-)

diff --git a/tools/hotplug/Linux/colo-proxy-setup b/tools/hotplug/Linux/colo-proxy-setup
index 94e2034452..d709146c47 100755
--- a/tools/hotplug/Linux/colo-proxy-setup
+++ b/tools/hotplug/Linux/colo-proxy-setup
@@ -76,10 +76,17 @@ function teardown_primary()
 
 function setup_secondary()
 {
-    do_without_error brctl delif $bridge $vifname
-    do_without_error brctl addbr $forwardbr
-    do_without_error brctl addif $forwardbr $vifname
-    do_without_error brctl addif $forwardbr $forwarddev
+    if which brctl >&/dev/null; then
+        do_without_error brctl delif $bridge $vifname
+        do_without_error brctl addbr $forwardbr
+        do_without_error brctl addif $forwardbr $vifname
+        do_without_error brctl addif $forwardbr $forwarddev
+    else
+        do_without_error ip link set $vifname nomaster
+        do_without_error ip link add name $forwardbr type bridge
+        do_without_error ip link set $vifname master $forwardbr
+        do_without_error ip link set $forwarddev master $forwardbr
+    fi
     do_without_error ip link set dev $forwardbr up
     do_without_error modprobe xt_SECCOLO
 
@@ -91,10 +98,17 @@ function setup_secondary()
 
 function teardown_secondary()
 {
-    do_without_error brctl delif $forwardbr $forwarddev
-    do_without_error brctl delif $forwardbr $vifname
-    do_without_error brctl delbr $forwardbr
-    do_without_error brctl addif $bridge $vifname
+    if which brctl >&/dev/null; then
+        do_without_error brctl delif $forwardbr $forwarddev
+        do_without_error brctl delif $forwardbr $vifname
+        do_without_error brctl delbr $forwardbr
+        do_without_error brctl addif $bridge $vifname
+    else
+        do_without_error ip link set $forwarddev nomaster
+        do_without_error ip link set $vifname nomaster
+        do_without_error ip link delete $forwardbr type bridge
+        do_without_error ip link set $vifname master $bridge
+    fi
 
     do_without_error iptables -t mangle -D PREROUTING -m physdev --physdev-in \
         $vifname -j SECCOLO --index $index
diff --git a/tools/hotplug/Linux/vif-bridge b/tools/hotplug/Linux/vif-bridge
index 6956dea66a..e722090ca8 100644
--- a/tools/hotplug/Linux/vif-bridge
+++ b/tools/hotplug/Linux/vif-bridge
@@ -31,10 +31,12 @@ dir=$(dirname "$0")
 bridge=${bridge:-}
 bridge=$(xenstore_read_default "$XENBUS_PATH/bridge" "$bridge")
 
-if [ -z "$bridge" ]
-then
-  bridge=$(brctl show | awk 'NR==2{print$1}')
-
+if [ -z "$bridge" ]; then
+    if which brctl >&/dev/null; then
+        bridge=$(brctl show | awk 'NR==2{print$1}')
+    else
+        bridge=$(bridge link | cut -d" " -f7)
+    fi
   if [ -z "$bridge" ]
   then
      fatal "Could not find bridge, and none was specified"
@@ -82,7 +84,11 @@ case "$command" in
         ;;
 
     offline)
-        do_without_error brctl delif "$bridge" "$dev"
+        if which brctl >&/dev/null; then
+            do_without_error brctl delif "$bridge" "$dev"
+        else
+            do_without_error ip link set "$dev" nomaster
+        fi
         do_without_error ifconfig "$dev" down
         ;;
 
diff --git a/tools/hotplug/Linux/vif2 b/tools/hotplug/Linux/vif2
index 2c155be68c..5bd555c6f0 100644
--- a/tools/hotplug/Linux/vif2
+++ b/tools/hotplug/Linux/vif2
@@ -7,13 +7,21 @@ dir=$(dirname "$0")
 bridge=$(xenstore_read_default "$XENBUS_PATH/bridge" "$bridge")
 if [ -z "$bridge" ]
     then
-    nr_bridges=$(($(brctl show | cut -f 1 | grep -v "^$" | wc -l) - 1))
+    if which brctl >&/dev/null; then
+        nr_bridges=$(($(brctl show | cut -f 1 | grep -v "^$" | wc -l) - 1))
+    else
+        nr_bridges=$(bridge link | wc -l)
+    fi
     if [ "$nr_bridges" != 1 ]
 	then
 	fatal "no bridge specified, and don't know which one to use ($nr_bridges found)"
     fi
-    bridge=$(brctl show | cut -d "
+    if which brctl >&/dev/null; then
+        bridge=$(brctl show | cut -d "
 " -f 2 | cut -f 1)
+    else
+        bridge=$(bridge link | cut -d" " -f6)
+    fi
 fi
 
 command="$1"
diff --git a/tools/hotplug/Linux/xen-network-common.sh b/tools/hotplug/Linux/xen-network-common.sh
index 92ffa603f7..8dd3a62068 100644
--- a/tools/hotplug/Linux/xen-network-common.sh
+++ b/tools/hotplug/Linux/xen-network-common.sh
@@ -111,9 +111,13 @@ create_bridge () {
 
     # Don't create the bridge if it already exists.
     if [ ! -e "/sys/class/net/${bridge}/bridge" ]; then
-	brctl addbr ${bridge}
-	brctl stp ${bridge} off
-	brctl setfd ${bridge} 0
+        if which brctl >&/dev/null; then
+            brctl addbr ${bridge}
+            brctl stp ${bridge} off
+            brctl setfd ${bridge} 0
+        else
+            ip link add name ${bridge} type bridge stp_state 0 forward_delay 0
+        fi
     fi
 }
 
@@ -127,7 +131,11 @@ add_to_bridge () {
 	ip link set dev ${dev} up || true
 	return
     fi
-    brctl addif ${bridge} ${dev}
+    if which brctl >&/dev/null; then
+        brctl addif ${bridge} ${dev}
+    else
+        ip link set ${dev} master ${bridge}
+    fi
     ip link set dev ${dev} up
 }
 
-- 
2.24.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] [tools/hotplug] Use ip on systems where brctl is not available

Posted by Wei Liu 4 years, 4 months ago

On Wed, Dec 18, 2019 at 12:15:23PM +1100, Steven Haigh wrote:
> Newer distros like CentOS 8 do not have brctl available. As such, we
> can't use it to configure networking anymore.
> 
> This patch will fall back to 'ip' or 'bridge' commands if brctl is not
> available in the working PATH.
> 
> This would be a likely backport candidate to any version expected to be
> built on CentOS 8 etc.

Acked-by: Wei Liu <wl@xen.org>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] [tools/hotplug] Use ip on systems where brctl is not available

Posted by Ian Jackson 4 years, 4 months ago

Steven Haigh writes ("[PATCH] [tools/hotplug] Use ip on systems where brctl is not available"):
> Newer distros like CentOS 8 do not have brctl available. As such, we
> can't use it to configure networking anymore.
> 
> This patch will fall back to 'ip' or 'bridge' commands if brctl is not
> available in the working PATH.

This looks good to me at least in the brctl case.  I have two minor
comments.

For the avoidance of doubt, I guess you have tested this in the
`ip'/`bridge' case ?  How thoroughly ? :-)

> -if [ -z "$bridge" ]
> -then
> -  bridge=$(brctl show | awk 'NR==2{print$1}')
> -
> +if [ -z "$bridge" ]; then

The presumably-unintentional style change makes the review slightly
harder...

> -    bridge=$(brctl show | cut -d "
> +    if which brctl >&/dev/null; then

Maybe introduce
   have_brctl () { ... }
so we can say
   if have_brctl; then
?

Regards,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] [tools/hotplug] Use ip on systems where brctl is not available

Posted by Steven Haigh 4 years, 4 months ago

On 2019-12-19 02:42, Ian Jackson wrote:
> Steven Haigh writes ("[PATCH] [tools/hotplug] Use ip on systems where
> brctl is not available"):
>> Newer distros like CentOS 8 do not have brctl available. As such, we
>> can't use it to configure networking anymore.
>> 
>> This patch will fall back to 'ip' or 'bridge' commands if brctl is not
>> available in the working PATH.
> 
> This looks good to me at least in the brctl case.  I have two minor
> comments.
> 
> For the avoidance of doubt, I guess you have tested this in the
> `ip'/`bridge' case ?  How thoroughly ? :-)

I have tested it to the point that it's almost a port of the Fedora 
patch - however the Fedora patch removes brctl completely in favour of 
the ip / bridge commands. While I haven't specifically debugged the 
result on Fedora, the networking works successfully when running a 
Domain-0 in Fedora 31 - which was the source of the 'ip' commands to 
run.

> 
>> -if [ -z "$bridge" ]
>> -then
>> -  bridge=$(brctl show | awk 'NR==2{print$1}')
>> -
>> +if [ -z "$bridge" ]; then
> 
> The presumably-unintentional style change makes the review slightly
> harder...

I'm intending to submit a new patch series after this (to make 
backporting this easier) that cleans up formatting / whitespace / syntax 
across the majority of scripts in the Linux directory. It'll look like a 
hot mess when submitting the next lot of patches - but its better than 
nothing.

>> -    bridge=$(brctl show | cut -d "
>> +    if which brctl >&/dev/null; then
> 
> Maybe introduce
>    have_brctl () { ... }
> so we can say
>    if have_brctl; then
> ?

I don't really have a preference. brctl is used through quite a few 
scripts - none of which really have a standard method of operation or 
common presentation. Some scripts call xen-network-common.sh - some do 
not.

Would I be correct in thinking that your proposal would be to ensure all 
network scripts source xen-network-common.sh - but this would be a more 
invasive change for backporting - hence I've tried to keep it as simple 
as possible for now.

Would a restructure of these things be better for something to be 
committed as yet another patch set (after formatting/style cleanups) 
that makes things a little more consistent?

-- 
Steven Haigh

? netwiz@crc.id.au     ? https://www.crc.id.au

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel