mwan3: improvements to route creation

handle creation of routing tables in mwan3rtmon to avoid race
conditions and potentially missing routes

handle ipv6 routes that have expiry

update directly connected ipset when routes are added or deleted

add fall through rules so that the default routing table is not
used if no rule in the interface-specific routing table matches

add option to comply with mwan3 source based routing

get default route parameters from main routing table

Signed-off-by: Aaron Goodman <aaronjg@stanford.edu>

net/mwan3/files/etc/hotplug.d/iface/15-mwan3

@@ -33,8 +33,8 @@ $IPT4 -S mwan3_hook &>/dev/null || {
 
 mwan3_init
 [ "$MWAN3_STARTUP" = 1 ] || {
-	mwan3_set_connected_iptables
+	config_get family $INTERFACE family ipv4
-	mwan3_set_custom_ipset
+	mwan3_set_connected_${family}
 }
 
 if [ "$MWAN3_STARTUP" != 1 ]; then
@@ -68,8 +68,7 @@ case "$ACTION" in
 	ifup|connected)
 		mwan3_create_iface_iptables $INTERFACE $DEVICE
 		mwan3_create_iface_rules $INTERFACE $DEVICE
-		mwan3_create_iface_route $INTERFACE $DEVICE
+		[ "$MWAN3_STARTUP" != 1 ] && mwan3_create_iface_route $INTERFACE $DEVICE
-		[ "$MWAN3_STARTUP" != 1 ] && mwan3_add_non_default_iface_route $INTERFACE $DEVICE
 		mwan3_set_iface_hotplug_state $INTERFACE "$binary_status"
 
 		mwan3_get_src_ip src_ip "$TRUE_INTERFACE"

net/mwan3/files/lib/mwan3/mwan3.sh

@@ -294,16 +294,16 @@ mwan3_set_connected_ipv4()
 
 	$IPS swap mwan3_connected_v4_temp mwan3_connected_v4
 	$IPS destroy mwan3_connected_v4_temp
+	$IPS -! add mwan3_connected mwan3_connected_v4
 
 }
 
-mwan3_set_connected_iptables()
+mwan3_set_connected_ipv6()
 {
 	local connected_network_v6 source_network_v6 error
 	local update=""
-	mwan3_set_connected_ipv4
+	[ $NO_IPV6 -eq 0 ] || return
 
-	[ $NO_IPV6 -eq 0 ] && {
 	mwan3_push_update -! create mwan3_connected_v6 hash:net family inet6
 	mwan3_push_update flush mwan3_connected_v6
 
@@ -315,19 +315,27 @@ mwan3_set_connected_iptables()
 	for source_network_v6 in $($IP6 addr ls | sed -ne 's/ *inet6 \([^ \/]*\).* scope global.*/\1/p'); do
 		mwan3_push_update -! add mwan3_source_v6 "$source_network_v6"
 	done
-	}
+	mwan3_push_update -! add mwan3_connected mwan3_connected_v6
+	error=$(echo "$update" | $IPS restore 2>&1) || LOG error "set_connected_ipv6: $error"
+}
+
+mwan3_set_connected_ipset()
+{
+	local error
+	local update=""
 
 	mwan3_push_update -! create mwan3_connected list:set
 	mwan3_push_update flush mwan3_connected
-	mwan3_push_update -! add mwan3_connected mwan3_connected_v4
-	[ $NO_IPV6 -eq 0 ] && mwan3_push_update -! add mwan3_connected mwan3_connected_v6
 
 	mwan3_push_update -! create mwan3_dynamic_v4 hash:net
 	mwan3_push_update -! add mwan3_connected mwan3_dynamic_v4
 
-	[ $NO_IPV6 -eq 0 ] && mwan3_push_update -! create mwan3_dynamic_v6 hash:net family inet6
+	if [ $NO_IPV6 -eq 0 ]; then
-	[ $NO_IPV6 -eq 0 ] && mwan3_push_update -! add mwan3_connected mwan3_dynamic_v6
+		mwan3_push_update -! create mwan3_dynamic_v6 hash:net family inet6
-	error=$(echo "$update" | $IPS restore 2>&1) || LOG error "set_connected_iptables: $error"
+		mwan3_push_update -! add mwan3_connected mwan3_dynamic_v6
+	fi
+
+	error=$(echo "$update" | $IPS restore 2>&1) || LOG error "set_connected_ipset: $error"
 }
 
 mwan3_set_general_rules()
@@ -439,7 +447,7 @@ mwan3_set_general_iptables()
 
 mwan3_create_iface_iptables()
 {
-	local id family connected_name IPT IPTR current update error
+	local id family IPT IPTR current update error
 
 	config_get family "$1" family ipv4
 	mwan3_get_iface_id id "$1"
@@ -447,16 +455,11 @@ mwan3_create_iface_iptables()
 	[ -n "$id" ] || return 0
 
 	if [ "$family" = "ipv4" ]; then
-		connected_name=mwan3_connected
 		IPT="$IPT4"
 		IPTR="$IPT4R"
-		$IPS -! create $connected_name list:set
-
 	elif [ "$family" = "ipv6" ] && [ $NO_IPV6 -eq 0 ]; then
-		connected_name=mwan3_connected_v6
 		IPT="$IPT6"
 		IPTR="$IPT6R"
-		$IPS -! create $connected_name hash:net family inet6
 	else
 		return
 	fi
@@ -474,7 +477,7 @@ mwan3_create_iface_iptables()
 
 	mwan3_push_update -A "mwan3_iface_in_$1" \
 			  -i "$2" \
-			  -m set --match-set $connected_name src \
+			  -m set --match-set mwan3_connected src \
 			  -m mark --mark "0x0/$MMX_MASK" \
 			  -m comment --comment "default" \
 			  -j MARK --set-xmark "$MMX_DEFAULT/$MMX_MASK"
@@ -521,45 +524,17 @@ mwan3_delete_iface_iptables()
 
 }
 
-mwan3_create_iface_route()
+mwan3_get_routes()
 {
-	local id via metric V V_ IP family
+	local source_routing
-	local iface device cmd true_iface
+	config_get_bool source_routing globals source_routing 0
-
+	[ $source_routing -eq 0 ] && unset source_routing
-	iface=$1
+	$IP route list table main | sed -ne "/^linkdown/T; s/expires \([0-9]\+\)sec//;s/error [0-9]\+//; ${source_routing:+s/default\(.*\) from [^ ]*/default\1/;} p" | uniq
-	device=$2
-	config_get family "$iface" family ipv4
-	mwan3_get_iface_id id "$iface"
-
-	[ -n "$id" ] || return 0
-
-	mwan3_get_true_iface true_iface $iface
-	if [ "$family" = "ipv4" ]; then
-		V_=""
-		IP="$IP4"
-	elif [ "$family" = "ipv6" ]; then
-		V_=6
-		IP="$IP6"
-	fi
-
-	network_get_gateway${V_} via "$true_iface"
-
-	{ [ -z "$via" ] || [ "$via" = "0.0.0.0" ] || [ "$via" = "::" ] ; } && unset via
-
-	network_get_metric metric "$true_iface"
-
-	$IP route flush table "$id"
-	cmd="$IP route add table $id default \
-	     ${via:+via} $via \
-	     ${metric:+metric} $metric \
-	     dev $2"
-	$cmd || LOG warn "ip cmd failed $cmd"
-
 }
 
-mwan3_add_non_default_iface_route()
+mwan3_create_iface_route()
 {
-	local tid route_line family IP id
+	local tid route_line family IP id tbl
 	config_get family "$1" family ipv4
 	mwan3_get_iface_id id "$1"
 
@@ -571,10 +546,15 @@ mwan3_add_non_default_iface_route()
 		IP="$IP6"
 	fi
 
+	tbl=$($IP route list table $id 2>/dev/null)$'\n'
 	mwan3_update_dev_to_table
-	$IP route list table main | grep -v "^default\|linkdown\|^::/0\|^fe80::/64\|^unreachable" | while read -r route_line; do
+	mwan3_get_routes | while read -r route_line; do
 		mwan3_route_line_dev "tid" "$route_line" "$family"
+		{ [ -z "${route_line##default*}" ] || [ -z "${route_line##fe80::/64*}" ]; } && [ "$tid" != "$id" ] && continue
 		if [ -z "$tid" ] || [ "$tid" = "$id" ]; then
+			# possible that routes are already in the table
+			# if 'connected' was called after 'ifup'
+			[ -n "$tbl" ] && [ -z "${tbl##*$route_line$'\n'*}" ] && continue
 			$IP route add table $id $route_line ||
 				LOG warn "failed to add $route_line to table $id"
 		fi
@@ -582,63 +562,21 @@ mwan3_add_non_default_iface_route()
 	done
 }
 
-mwan3_add_all_nondefault_routes()
-{
-	local tid IP route_line ipv family active_tbls tid
-
-	add_active_tbls()
-	{
-		let tid++
-		config_get family "$1" family ipv4
-		[ "$family" != "$ipv" ] && return
-		$IP route list table $tid 2>/dev/null | grep -q "^default\|^::/0" && {
-			active_tbls="$active_tbls${tid} "
-		}
-	}
-
-	add_route()
-	{
-		let tid++
-		[ -n "${active_tbls##* $tid *}" ] && return
-		$IP route add table $tid $route_line ||
-			LOG warn "failed to add $route_line to table $tid"
-	}
-
-	mwan3_update_dev_to_table
-	for ipv in ipv4 ipv6; do
-		[ "$ipv" = "ipv6" ] && [ $NO_IPV6 -ne 0 ] && continue
-		if [ "$ipv" = "ipv4" ]; then
-			IP="$IP4"
-		elif [ "$ipv" = "ipv6" ]; then
-			IP="$IP6"
-		fi
-		tid=0
-		active_tbls=" "
-		config_foreach add_active_tbls interface
-		$IP route list table main | grep -v "^default\|linkdown\|^::/0\|^fe80::/64\|^unreachable" | while read -r route_line; do
-			mwan3_route_line_dev "tid" "$route_line" "$ipv"
-			if [ -n "$tid" ]; then
-				$IP route add table $tid $route_line
-			else
-				config_foreach add_route interface
-			fi
-		done
-	done
-}
 mwan3_delete_iface_route()
 {
-	local id
+	local id family
 
 	config_get family "$1" family ipv4
 	mwan3_get_iface_id id "$1"
 
-	[ -n "$id" ] || return 0
+	if [ -z "$id" ]; then
+		LOG warn "delete_iface_route: could not find table id for interface $1"
+		return 0
+	fi
 
 	if [ "$family" = "ipv4" ]; then
 		$IP4 route flush table "$id"
-	fi
+	elif [ "$family" = "ipv6" ] && [ $NO_IPV6 -eq 0 ]; then
-
-	if [ "$family" = "ipv6" ] && [ $NO_IPV6 -eq 0 ]; then
 		$IP6 route flush table "$id"
 	fi
 }
@@ -660,21 +598,16 @@ mwan3_create_iface_rules()
 		return
 	fi
 
-	while [ -n "$($IP rule list | awk '$1 == "'$((id+1000)):'"')" ]; do
+	mwan3_delete_iface_rules "$1"
-		$IP rule del pref $((id+1000))
-	done
-
-	while [ -n "$($IP rule list | awk '$1 == "'$((id+2000)):'"')" ]; do
-		$IP rule del pref $((id+2000))
-	done
 
 	$IP rule add pref $((id+1000)) iif "$2" lookup "$id"
 	$IP rule add pref $((id+2000)) fwmark "$(mwan3_id2mask id MMX_MASK)/$MMX_MASK" lookup "$id"
+	$IP rule add pref $((id+3000)) fwmark "$(mwan3_id2mask id MMX_MASK)/$MMX_MASK" unreachable
 }
 
 mwan3_delete_iface_rules()
 {
-	local id family IP
+	local id family IP rule_id
 
 	config_get family "$1" family ipv4
 	mwan3_get_iface_id id "$1"
@@ -689,12 +622,8 @@ mwan3_delete_iface_rules()
 		return
 	fi
 
-	while [ -n "$($IP rule list | awk '$1 == "'$((id+1000)):'"')" ]; do
+	for rule_id in $(ip rule list | awk '$1 % 1000 == '$id' && $1 > 1000 && $1 < 4000 {print substr($1,0,4)}'); do
-		$IP rule del pref $((id+1000))
+		$IP rule del pref $rule_id
-	done
-
-	while [ -n "$($IP rule list | awk '$1 == "'$((id+2000)):'"')" ]; do
-		$IP rule del pref $((id+2000))
 	done
 }
 
@@ -952,6 +881,10 @@ mwan3_set_user_iptables_rule()
 	config_get global_logging globals logging 0
 	config_get loglevel globals loglevel notice
 
+	[ "$ipv" = "ipv6" ] && [ $NO_IPV6 -ne 0 ] && return
+	[ "$family" = "ipv4" ] && [ "$ipv" = "ipv6" ] && return
+	[ "$family" = "ipv6" ] && [ "$ipv" = "ipv4" ] && return
+
 	if [ -n "$src_iface" ]; then
 		network_get_device src_dev "$src_iface"
 		if [ -z "$src_dev" ]; then
@@ -1013,10 +946,6 @@ mwan3_set_user_iptables_rule()
 		fi
 	fi
 
-	[ "$ipv" = "ipv6" ] && [ $NO_IPV6 -ne 0 ] && return
-	[ "$family" = "ipv4" ] && [ "$ipv" = "ipv6" ] && return
-	[ "$family" = "ipv6" ] && [ "$ipv" = "ipv4" ] && return
-
 	if [ $rule_policy -eq 1 ] && [ -n "${current##*-N $policy*}" ]; then
 		mwan3_push_update -N "$policy"
 	fi
@@ -1172,6 +1101,7 @@ mwan3_report_iface_status()
 		result="offline"
 	elif [ -n "$($IP rule | awk '$1 == "'$((id+1000)):'"')" ] && \
 		     [ -n "$($IP rule | awk '$1 == "'$((id+2000)):'"')" ] && \
+		     [ -n "$($IP rule | awk '$1 == "'$((id+3000)):'"')" ] && \
 		     [ -n "$($IPT -S mwan3_iface_in_$1 2> /dev/null)" ] && \
 		     [ -n "$($IP route list table $id default dev $device 2> /dev/null)" ]; then
 		json_init

net/mwan3/files/usr/sbin/mwan3

@@ -163,7 +163,7 @@ start()
 	mwan3_set_general_iptables
 	config_foreach ifup interface
 	wait $hotplug_pids
-	mwan3_add_all_nondefault_routes
+	mwan3_add_all_routes
 	mwan3_set_policies_iptables
 	mwan3_set_user_rules
 

net/mwan3/files/usr/sbin/mwan3rtmon

@@ -5,81 +5,139 @@
 . /lib/mwan3/mwan3.sh
 . /lib/mwan3/common.sh
 
+trap_with_arg()
+{
+	func="$1" ; shift
+	pid="$1" ; shift
+	for sig ; do
+		# shellcheck disable=SC2064
+		trap "$func $sig $pid" "$sig"
+	done
+}
+
+func_trap()
+{
+	kill -${1} ${2} 2>/dev/null
+}
+
+mwan3_add_all_routes()
+{
+	local tid IP IPT route_line family active_tbls tid initial_state
+	local ipv=$1
+
+	add_active_tbls()
+	{
+		let tid++
+		config_get family "$1" family ipv4
+		config_get initial_state "$1" initial_state "online"
+		[ "$family" != "$ipv" ] && return
+		if [ "$initial_state" = "online" ] && $IPT -S "mwan3_iface_in_$1" &> /dev/null; then
+			active_tbls="$active_tbls${tid} "
+		fi
+	}
+
+	add_route()
+	{
+		let tid++
+		[ -n "${active_tbls##* $tid *}" ] && return
+		$IP route add table $tid $route_line ||
+			LOG warn "failed to add $route_line to table $tid"
+	}
+
+	mwan3_update_dev_to_table
+	[ "$ipv" = "ipv6" ] && [ $NO_IPV6 -ne 0 ] && return
+	if [ "$ipv" = "ipv4" ]; then
+		IP="$IP4"
+		IPT="$IPT4"
+	elif [ "$ipv" = "ipv6" ]; then
+		IP="$IP6"
+		IPT="$IPT6"
+	fi
+	tid=0
+	active_tbls=" "
+	config_foreach add_active_tbls interface
+	[ $active_tbls = " " ] && return
+	mwan3_get_routes | while read -r route_line; do
+		mwan3_route_line_dev "tid" "$route_line" "$ipv"
+		if [ -n "$tid" ] && [ -z "${active_tbls##* $tid *}" ]; then
+			$IP route add table $tid $route_line
+		elif [ -n "${route_line##default*}" ] && [ -n "${route_line##fe80::/64*}" ]; then
+			config_foreach add_route interface
+		fi
+	done
+}
+
 mwan3_rtmon_route_handle()
 {
-	config_load mwan3
+	local action route_line family tbl device line route_line_exp tid source_routing
-	local section action route_line family tbl device metric tos dst line tid
+
 	route_line=${1##"Deleted "}
 	route_family=$2
 
+	config_get_boolean source_routing globals source_routing 0
+	[ $source_routing -eq 0 ] && unset source_routing
+
+	if [ "$route_line" = "$1" ]; then
+		action="replace"
+		route_line_exp="s/expires \([0-9]\+\)sec//;s/error [0-9]\+//; ${source_routing:+s/default\(.*\) from [^ ]*/default\1/}"
+		$IPS -! add mwan3_connected_${route_family##ip} ${route_line%% *}
+	else
+		action="del"
+		route_line_exp="s/expires [0-9]\+sec//;s/error [0-9]\+//; ${source_routing:+s/default\(.*\) from [^ ]*/default\1/}"
+		mwan3_set_connected_${route_family}
+	fi
+
 	if [ "$route_family" = "ipv4" ]; then
 		IP="$IP4"
 	elif [ "$route_family" = "ipv6" ] && [ $NO_IPV6 -eq 0 ]; then
 		IP="$IP6"
+		route_line=$(echo "$route_line" | sed "$route_line_exp")
 	else
+		LOG warn "route update called with invalid family - $route_family"
 		return
 	fi
 
-	if [ "$route_line" == "$1" ]; then
+	# don't try to add routes when link has gone down
-		action="add"
+	if [ -z "${route_line##linkdown*}" ]; then
-	else
+		LOG debug "not adding route due to linkdown - skipping $route_line"
-		action="del"
+		return
 	fi
 
-	# never add default route lines, since this is handled elsewhere
-	[ -z "${route_line##default*}" ] && return
-	[ -z "${route_line##::/0*}" ] && return
-	route_line=${route_line%% linkdown*}
-	route_line=${route_line%% unreachable*}
-	mwan3_update_dev_to_table
-	mwan3_route_line_dev "tid" "$route_line" "$route_family"
 	handle_route() {
-		tbl=$($IP route list table $tid)
+		local iface=$1
-		if [ $action = "add" ]; then
+		tbl=$($IP route list table $tid 2>/dev/null)$'\n'
-			echo "$tbl" | grep -q "^default\|^::/0" || return
-		else
-			[ -z "$tbl" ] && return
-		fi
-		# check that action needs to be performed. May not need to take action if:
-		# Got a route update on ipv6 where route is already in the table
-		# Got a delete event, but table was already flushed
 
-		[ $action = "add" ] && [ -z "${tbl##*$route_line*}" ] && return
+		if [ "$(cat /var/run/mwan3track/$iface/STATUS)" != "online" ]; then
-		[ $action = "del" ] && [ -n "${tbl##*$route_line*}" ] && return
+			LOG debug "interface $iface is offline - skipping $route_line";
-		network_get_device device "$section"
+			return
-		LOG debug "adjusting route $device: $IP route \"$action\" table $tid $route_line"
+		fi
+
+		# check that action needs to be performed. May not need to take action if we
+		# got a delete event, but table was already flushed
+		if [ $action = "del" ] && [ -n "${tbl##*$route_line$'\n'*}" ]; then
+			LOG debug "skipping already deleted route table $tid - skipping $route_line"
+			return
+		fi
+
+		network_get_device device "$iface"
+		LOG debug "adjusting route $device: $IP route $action table $tid $route_line"
 		$IP route "$action" table $tid $route_line ||
 			LOG warn "failed: $IP route $action table $tid $route_line"
 	}
 	handle_route_cb(){
+		local iface=$1
 		let tid++
-		config_get family "$section" family ipv4
+		config_get family "$iface" family ipv4
 		[ "$family" != "$route_family" ] && return
-		handle_route
+		handle_route "$iface"
 	}
 
-	if [ $action = "add" ]; then
+	mwan3_update_dev_to_table
-		## handle old routes from 'change' or 'replace'
+	mwan3_route_line_dev "tid" "$route_line" "$route_family"
-		metric=${route_line##*metric }
-		[ "$metric" = "$route_line" ] && unset metric || metric=${metric%% *}
-
-		tos=${route_line##*tos }
-		[ "$tos" = "$route_line" ] && unset tos || tos=${tos%% *}
-
-		dst=${route_line%% *}
-		grep_line="$dst ${tos:+tos $tos}.*table [0-9].*${metric:+metric $metric}"
-		$IP route list table all | grep "$grep_line" | while read -r line; do
-			tbl=${line##*table }
-			tbl=${tbl%% *}
-			[ $tbl -gt $MWAN3_INTERFACE_MAX ] && continue
-			LOG debug "removing route on ip route change/replace: $line"
-			$IP route del $line
-		done
-	fi
 
 	if [ -n "$tid" ]; then
 		handle_route
-	else
+	elif [ -n "${route_line##default*}" ] && [ -n "${route_line##fe80::/64*}" ]; then
 		config_foreach handle_route_cb interface
 	fi
 }
@@ -91,19 +149,35 @@ main()
 	config_load mwan3
 	family=$1
 	[ -z $family ] && family=ipv4
-	if [ "$family" = ipv6 ]; then
+	if [ "$family" = "ipv6" ]; then
+		if [ $NO_IPV6 -ne 0 ]; then
+			LOG warn "mwan3rtmon started for ipv6, but ipv6 not enabled on system"
+			exit 1
+		fi
 		IP="$IP6"
 	else
 		IP="$IP4"
 	fi
 	mwan3_init
-
+	mwan3_lock "mwan3rtmon" "start"
-	$IP monitor route | while read -r line; do
+	sh -c "echo \$\$; exec $IP monitor route" | {
+		read -r monitor_pid
+		trap_with_arg func_trap "$monitor_pid" SIGINT SIGTERM SIGKILL
+		while read -r line; do
 			[ -z "${line##*table*}" ] && continue
 			LOG debug "handling route update $family $line"
 			mwan3_lock "service" "mwan3rtmon"
 			mwan3_rtmon_route_handle "$line" "$family"
 			mwan3_unlock "service" "mwan3rtmon"
 		done
+	} &
+	child=$!
+	kill -SIGSTOP $child
+	trap_with_arg func_trap "$child" SIGINT SIGTERM SIGKILL
+	mwan3_set_connected_${family}
+	mwan3_add_all_routes ${family}
+	mwan3_unlock "mwan3rtmon" "start"
+	kill -SIGCONT $child
+	wait $!
 }
 main "$@"