From: Michael Tremer <michael.tremer@ipfire.org>
Date: Sat, 22 Jul 2017 20:10:48 +0000 (+0200)
Subject: Remove support for Rapid Spanning Tree Protocol
X-Git-Tag: 009~120
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4e0bc0937911ddbe4dad471ba99793c5461e8886;p=network.git

Remove support for Rapid Spanning Tree Protocol

The userspace daemon is not very stable and unfortunately
not very well tested so that reliable use of it is impossible
right now.

We keep supporting STP as implemented in the Linux kernel
which has some disadvantages, but has proven to be more solid.

Signed-off-by: Michael Tremer <michael.tremer@ipfire.org>
---

diff --git a/man/network-zone-bridge.xml b/man/network-zone-bridge.xml
index efa8d11d..a77118bc 100644
--- a/man/network-zone-bridge.xml
+++ b/man/network-zone-bridge.xml
@@ -108,38 +108,10 @@
 		</variablelist>
 
 		<para>
-			Spanning Tree Protocol configuration options:
+			Spanning Tree Protocol (802.1D) configuration options:
 		</para>
 
 		<variablelist>
-			<varlistentry>
-				<term>
-					<option>--stp-mode=[<emphasis>rstp</emphasis>|stp]</option>
-				</term>
-
-				<listitem>
-					<para>
-						When STP is enabled, this option will set the operational
-						mode which can either be:
-					</para>
-					<itemizedlist>
-						<listitem>
-							<para>
-								<emphasis>Spanning Tree Protocol IEEE 802.1D</emphasis>
-								(stp) was introduced in 1998 by Radia Perlman.
-							</para>
-						</listitem>
-						<listitem>
-							<para>
-								<emphasis>Rapid Spanning Tree Protocol IEEE 802.1W</emphasis>
-								(rstp) which has much faster convergence times and was
-								introduced in 2004. This is the default option.
-							</para>
-						</listitem>
-					</itemizedlist>
-				</listitem>
-			</varlistentry>
-
 			<varlistentry>
 				<term>
 					<option>--stp-forward-delay=<replaceable>0</replaceable></option>
diff --git a/src/functions/functions.constants b/src/functions/functions.constants
index 8db9bea1..d9ece3ea 100644
--- a/src/functions/functions.constants
+++ b/src/functions/functions.constants
@@ -69,6 +69,10 @@ EXIT_DAD_OK=0
 EXIT_DAD_FAILED=8
 EXIT_DAD_TENTATIVE=8
 
+# Spanning Tree Protocol
+EXIT_STP_KERNEL=1
+EXIT_STP_ERROR=2
+
 STATUS_UP=0
 STATUS_DOWN=1
 STATUS_NOCARRIER=2
diff --git a/src/functions/functions.stp b/src/functions/functions.stp
index e9e08c43..7155c0f3 100644
--- a/src/functions/functions.stp
+++ b/src/functions/functions.stp
@@ -19,15 +19,6 @@
 #                                                                             #
 ###############################################################################
 
-# The default mode.
-#   We default to RSTP, because it has the better user experience and
-#   faster convergence times. Despite of that, it completely downgradeable
-#   to plain STP.
-STP_DEFAULT_MODE="rstp"
-
-# Allowed modes of the spanning tree protocol.
-STP_ALLOWED_MODES="rstp stp"
-
 stp_enable() {
 	local bridge=${1}
 	assert isset bridge
@@ -60,92 +51,6 @@ stp_is_enabled() {
 	esac
 }
 
-stp_is_userspace() {
-	local bridge=${1}
-	assert isset bridge
-
-	local state=$(__device_get_file ${bridge} bridge/stp_state)
-	case "${state}" in
-		2)
-			return ${EXIT_TRUE}
-			;;
-		*)
-			return ${EXIT_FALSE}
-			;;
-	esac
-}
-
-stp_get_name() {
-	local proto=${1}
-
-	case "${proto}" in
-		stp)
-			echo "Spanning Tree Protocol"
-			;;
-		rstp)
-			echo "Rapid Spanning Tree Protocol"
-			;;
-		mstp)
-			echo "Multiple Spanning Tree Protocol"
-			;;
-	esac
-
-	return ${EXIT_OK}
-}
-
-stp_bridge_set_protocol() {
-	local bridge=${1}
-	assert isset bridge
-
-	local mode=${2}
-	assert isset mode
-
-	if ! list_match ${mode} ${STP_ALLOWED_MODES}; then
-		log WARNING "Unknown protocol version: ${mode}."
-		log WARNING "Using default mode."
-
-		mode="${STP_DEFAULT_MODE}"
-	fi
-
-	cmd mstpctl setforcevers ${bridge} ${mode}
-	assert [ $? -eq 0 ]
-}
-
-stp_bridge_get_protocol() {
-	local bridge=${1}
-
-	assert isset bridge
-
-	# Let's check what the kernel is telling us about it's STP state.
-	local state=$(__device_get_file ${bridge} "bridge/stp_state")
-
-	case "${state}" in
-		0)
-			# STP is disabled.
-			return ${EXIT_OK}
-			;;
-		1)
-			# Kernel mode STP is running.
-			echo "stp"
-			return ${EXIT_OK}
-			;;
-		2)
-			# User-space STP is running.
-			;;
-		*)
-			log ERROR "Kernel is running in an unknown STP state."
-			return ${EXIT_ERROR}
-			;;
-	esac
-
-	# We get here, when STP is running in user-space mode.
-
-	# Get the current protocol version.
-	mstpctl showbridge ${bridge} force-protocol-version 2>/dev/null
-
-	return ${EXIT_OK}
-}
-
 stp_bridge_get_id() {
 	local bridge=${1}
 	assert isset bridge
@@ -159,12 +64,8 @@ stp_bridge_get_forward_delay() {
 	local bridge=${1}
 	assert isset bridge
 
-	if stp_is_userspace ${bridge}; then
-		cmd mstpctl showbridge ${bridge} forward-delay
-	else
-		local output=$(__device_get_file ${bridge} bridge/forward_delay)
-		__stp_div_100 ${output}
-	fi
+	local output=$(__device_get_file ${bridge} bridge/forward_delay)
+	__stp_div_100 ${output}
 
 	return ${EXIT_OK}
 }
@@ -280,11 +181,7 @@ stp_bridge_get_designated_root() {
 
 	local output
 
-	if stp_is_userspace ${bridge}; then
-		output=$(cmd mstpctl showbridge ${bridge} designated-root)
-	else
-		output=$(__device_get_file ${bridge} bridge/root_id)
-	fi
+	output=$(__device_get_file ${bridge} bridge/root_id)
 	output=${output:6}
 
 	# Print output (lowercase).
@@ -301,33 +198,14 @@ stp_bridge_get_root_path_cost() {
 	local bridge=${1}
 	assert isset bridge
 
-	if stp_is_userspace ${bridge}; then
-		cmd mstpctl showbridge ${bridge} path-cost
-	else
-		__device_get_file ${bridge} bridge/root_path_cost
-	fi
-
-	return ${EXIT_OK}
+	__device_get_file ${bridge} bridge/root_path_cost
 }
 
 stp_bridge_get_root_port_id() {
 	local bridge=${1}
 	assert isset bridge
 
-	if stp_is_userspace ${bridge}; then
-		local root_port=$(cmd mstpctl showbridge ${bridge} root-port)
-
-		# Return error, when there is no root port.
-		if [ "${root_port}" = "none" ]; then
-			return ${EXIT_ERROR}
-		fi
-
-		print "${root_port}"
-	else
-		__device_get_file ${bridge} bridge/root_port_id
-	fi
-
-	return ${EXIT_OK}
+	__device_get_file ${bridge} bridge/root_port_id
 }
 
 stp_bridge_get_root_port() {
@@ -366,26 +244,14 @@ stp_bridge_get_topology_change_count() {
 	local bridge=${1}
 	assert isset bridge
 
-	if stp_is_userspace ${bridge}; then
-		cmd mstpctl showbridge ${bridge} topology-change-count
-	else
-		__device_get_file ${bridge} bridge/topology_change
-	fi
-
-	return ${EXIT_OK}
+	__device_get_file ${bridge} bridge/topology_change
 }
 
 stp_bridge_get_topology_change_timer() {
 	local bridge=${1}
 	assert isset bridge
 
-	if stp_is_userspace ${bridge}; then
-		cmd mstpctl showbridge ${bridge} time-since-topology-change
-	else
-		__device_get_file ${bridge} bridge/topology_change_timer
-	fi
-
-	return ${EXIT_OK}
+	__device_get_file ${bridge} bridge/topology_change_timer
 }
 
 stp_bridge_get_topology_change_detected() {
@@ -394,11 +260,7 @@ stp_bridge_get_topology_change_detected() {
 
 	local change
 
-	if stp_is_userspace ${bridge}; then
-		change=$(mstpctl showbridge ${bridge} topology-change)
-	else
-		change=$(__device_get_file ${bridge} bridge/topology_change_detected)
-	fi
+	change=$(__device_get_file ${bridge} bridge/topology_change_detected)
 
 	if enabled change; then
 		print "yes"
@@ -416,34 +278,28 @@ stp_port_get_state() {
 	local port=${2}
 	assert isset port
 
-	local space
-	if stp_is_userspace ${bridge}; then
-		state=$(mstpctl showportdetail ${bridge} ${port} state)
-		print "${state^^}"
-	else
-		state=$(__device_get_file ${bridge} brif/${port}/state)
-
-		case "${state}" in
-			0)
-				print "DISABLED"
-				;;
-			1)
-				print "LISTENING"
-				;;
-			2)
-				print "LEARNING"
-				;;
-			3)
-				print "FORWARDING"
-				;;
-			4)
-				print "BLOCKING"
-				;;
-			*)
-				return ${EXIT_ERROR}
-				;;
-		esac
-	fi
+	local state=$(__device_get_file ${bridge} brif/${port}/state)
+
+	case "${state}" in
+		0)
+			print "DISABLED"
+			;;
+		1)
+			print "LISTENING"
+			;;
+		2)
+			print "LEARNING"
+			;;
+		3)
+			print "FORWARDING"
+			;;
+		4)
+			print "BLOCKING"
+			;;
+		*)
+			return ${EXIT_ERROR}
+			;;
+	esac
 
 	return ${EXIT_OK}
 }
@@ -466,13 +322,7 @@ stp_port_get_cost() {
 	local port=${2}
 	assert isset port
 
-	if stp_is_userspace ${bridge}; then
-		cmd mstpctl showportdetail ${bridge} ${port} external-port-cost
-	else
-		__device_get_file ${bridge} brif/${port}/path_cost
-	fi
-
-	return ${EXIT_ERROR}
+	__device_get_file ${bridge} brif/${port}/path_cost
 }
 
 stp_port_set_cost() {
@@ -489,11 +339,7 @@ stp_port_set_cost() {
 
 	log DEBUG "Setting STP path costs of port '${port}' (bridge '${bridge}') to '${cost}'"
 
-	if stp_is_userspace "${bridge}"; then
-		cmd mstpctl setportpathcost "${bridge}" "${port}" "${cost}"
-	else
-		__device_set_file "${bridge}" "brif/${port}/path_cost" "${cost}"
-	fi
+	__device_set_file "${bridge}" "brif/${port}/path_cost" "${cost}"
 }
 
 stp_port_get_designated_root() {
@@ -503,18 +349,10 @@ stp_port_get_designated_root() {
 	local port=${2}
 	assert isset port
 
-	local output
-
-	if stp_is_userspace ${bridge}; then
-		output=$(cmd mstpctl showportdetail ${bridge} ${port} designated-root)
-		output=${output:6}
-	else
-		output=$(__device_get_file ${bridge} brif/${port}/designated_root)
-		output=${output:5}
-	fi
+	local output=$(__device_get_file ${bridge} brif/${port}/designated_root)
 
 	if isset output; then
-		mac_format ${output}
+		mac_format ${output:5}
 		return ${EXIT_OK}
 	fi
 
diff --git a/src/helpers/bridge-stp b/src/helpers/bridge-stp
index b25af6a8..30745385 100644
--- a/src/helpers/bridge-stp
+++ b/src/helpers/bridge-stp
@@ -19,9 +19,6 @@
 #                                                                             #
 ###############################################################################
 
-EXIT_USERSPACE_STP=0
-EXIT_KERNEL_STP=1
-
 # Change LOG_FACILITY that we will find our messages in syslog.
 LOG_FACILITY=$(basename ${0})
 
@@ -43,60 +40,25 @@ if ! zone_exists ${zone}; then
 fi
 
 # Read zone settings
-zone_settings_read "${zone}" --ignore-superfluous-settings \
-	STP STP_MODE
+zone_settings_read "${zone}" --ignore-superfluous-settings STP
 
 # Make sure STP is enabled for this zone.
-assert enabled STP
-
-log DEBUG "Configured STP mode is '${STP_MODE}'"
-
-case "${STP_MODE}" in
-	rstp)
-		# Check if mstpd is running. If not, try to start it.
-		if ! service_is_active mstpd; then
-			service_start "mstpd.service"
-
-			if ! service_is_active "mstpd.service"; then
-				log ERROR "mstpd is not running. STP might not work."
-				exit 1
-			fi
-		fi
-
-		# Set the right protocol that should be used.
-		# Do this after the bridge has been added.
-		(
-			sleep 2
-			stp_bridge_set_protocol "${zone}" "${STP_MODE}"
-		) &
-
-		# Tell mstpd that STP has to be enabled/disabled.
-		case "${action}" in
-			start)
-				log DEBUG "Enabling userspace STP for zone '${zone}'"
-				exec mstpctl addbridge ${zone}
-				;;
-			stop)
-				log DEBUG "Disabling userspace STP for zone '${zone}'"
-				exec mstpctl delbridge ${zone}
-				;;
-		esac
+if ! enabled STP; then
+	log ERROR "The kernel tried to enable STP for zone ${zone}"
+	log ERROR "but our configuration disagrees"
+	exit ${EXIT_STP_ERROR}
+fi
 
-		log ERROR "Could not properly exec mstpctl."
+case "${action}" in
+	start)
+		log INFO "STP activated for ${zone}"
+		exit ${EXIT_STP_KERNEL}
 		;;
-	stp)
-		case "${action}" in
-			start)
-				log DEBUG "Enabling kernel STP for zone '${zone}'"
-				exit ${EXIT_KERNEL_STP}
-				;;
-			stop)
-				log DEBUG "Disabling kernel STP for zone '${zone}'"
-				exit ${EXIT_OK}
-				;;
-		esac
+	stop)
+		log INFO "STP deactivated for ${zone}"
+		exit ${EXIT_OK}
 		;;
+	*)
+		log ERROR "Unknown action: ${action}"
+		exit ${EXIT_STP_ERROR}
 esac
-
-# Fall back to kernel STP.
-exit ${EXIT_KERNEL_STP}
diff --git a/src/hooks/zones/bridge b/src/hooks/zones/bridge
index 63ca004a..e0d839b3 100644
--- a/src/hooks/zones/bridge
+++ b/src/hooks/zones/bridge
@@ -23,7 +23,7 @@
 
 HOOK_MANPAGE="network-zone-bridge"
 
-HOOK_SETTINGS="HOOK STP STP_FORWARD_DELAY STP_HELLO STP_MAXAGE STP_MODE"
+HOOK_SETTINGS="HOOK STP STP_FORWARD_DELAY STP_HELLO STP_MAXAGE"
 HOOK_SETTINGS="${HOOK_SETTINGS} STP_PRIORITY MAC MTU"
 
 HOOK_PORT_SETTINGS="COST PRIORITY"
@@ -32,7 +32,6 @@ HOOK_PORT_SETTINGS="COST PRIORITY"
 MAC=""
 MTU=1500
 STP="on"
-STP_MODE="rstp"
 STP_FORWARD_DELAY=0
 STP_HELLO=2
 STP_MAXAGE=20
@@ -41,7 +40,6 @@ STP_PRIORITY=512
 hook_check_settings() {
 	assert ismac MAC
 	assert isbool STP
-	assert isoneof STP_MODE stp rstp
 	assert isinteger STP_HELLO
 	assert isinteger STP_FORWARD_DELAY
 	assert isinteger STP_PRIORITY
@@ -54,9 +52,6 @@ hook_parse_cmdline() {
 			--stp=*)
 				STP=${1#--stp=}
 				;;
-			--stp-mode=*)
-				STP_MODE=${1#--stp-mode=}
-				;;
 			--stp-hello=*)
 				STP_HELLO=${1#--stp-hello=}
 				;;
@@ -203,9 +198,6 @@ hook_status() {
 
 	cli_headline 2 "Spanning Tree Protocol information"
 	if stp_is_enabled "${zone}"; then
-		local proto=$(stp_bridge_get_protocol ${zone})
-
-		cli_print_fmt1 2 "Version"	"$(stp_get_name ${proto})"
 		cli_print_fmt1 2 "ID"		"$(stp_bridge_get_id ${zone})"
 		cli_print_fmt1 2 "Priority"	"$(stp_bridge_get_priority ${zone})"