From: Jo-Philipp Wich <jo@mein.io>
Date: Mon, 9 May 2022 13:09:50 +0000 (+0200)
Subject: fw4: filter non hw-offload capable devices when resolving lower devices
X-Git-Url: http://git.lede-project.org./?a=commitdiff_plain;h=b2682251a173103490a4a2556fb87e4df1d627b3;p=project%2Ffirewall4.git

fw4: filter non hw-offload capable devices when resolving lower devices

Make sure to ignore devices not capable of hardware offloading when resolving
lower devices for the flowtable declaration.

Commit 57984e0 ("fw4: always resolve lower flowtable devices") changed the
behaviour of fw4 to always resolve lower devices, even for soft offloading
but removed some a crucial check to omit incapable devices in the hardware
offloading case, regressing previously working setups due to the inclusion
of wireless devices into the hardware offloaded table declaration.

Since we need to reintroduce ubus device status information for this change,
we can utilize the devinfo value exposed there instead or resolving it from
sysfs ourselves. Also make sure to sort the deduplicated device list to
produce a deterministic result.

Fixes: 57984e0 ("fw4: always resolve lower flowtable devices")
Signed-off-by: Jo-Philipp Wich <jo@mein.io>
---

diff --git a/root/usr/share/ucode/fw4.uc b/root/usr/share/ucode/fw4.uc
index 12e0d8c..39e8507 100644
--- a/root/usr/share/ucode/fw4.uc
+++ b/root/usr/share/ucode/fw4.uc
@@ -356,53 +356,33 @@ function map_setmatch(set, match, proto) {
 	return fields;
 }
 
-function determine_device_type(devname) {
-	let uevent = fs.open(`/sys/class/net/${devname}/uevent`),
-	    devtype = null;
-
-	if (uevent) {
-		let line;
-
-		while ((line = uevent.read('line')) != null) {
-			let m = match(line, /^DEVTYPE=(\w+)/);
-
-			if (m) {
-				devtype = m[1];
-				break;
-			}
-		}
-
-		uevent.close();
-	}
-
-	return devtype;
-}
-
-function resolve_lower_devices(devname) {
-	switch (determine_device_type(devname)) {
-	case null:
-		return [];
-
-	case 'vlan':
-	case 'bridge':
-		let dir = fs.opendir(`/sys/class/net/${devname}`);
-		let lower = [];
-
-		if (dir) {
+function resolve_lower_devices(devstatus, devname, require_hwoffload) {
+	let dir = fs.opendir(`/sys/class/net/${devname}`);
+	let devs = [];
+
+	if (dir) {
+		switch (devstatus[devname]?.devtype) {
+		case 'vlan':
+		case 'bridge':
 			let e;
 
 			while ((e = dir.read()) != null)
 				if (index(e, "lower_") === 0)
-					push(lower, ...resolve_lower_devices(substr(e, 6)));
+					push(devs, ...resolve_lower_devices(devstatus, substr(e, 6), require_hwoffload));
 
-			dir.close();
-		}
+			break;
+
+		default:
+			if (!require_hwoffload || devstatus[devname]?.["hw-tc-offload"])
+				push(devs, devname);
 
-		return lower;
+			break;
+		}
 
-	default:
-		return [ devname ];
+		dir.close();
 	}
+
+	return devs;
 }
 
 function nft_json_command(...args) {
@@ -460,24 +440,41 @@ return {
 	},
 
 	resolve_offload_devices: function() {
+		if (!this.default_option("flow_offloading"))
+			return [];
+
+		let devstatus = null;
 		let devices = [];
+		let bus = ubus.connect();
 
-		if (this.default_option("flow_offloading")) {
+		if (bus) {
+			devstatus = bus.call("network.device", "status") || {};
+			bus.disconnect();
+		}
+
+		if (this.default_option("flow_offloading_hw")) {
 			for (let zone in this.zones())
 				for (let device in zone.related_physdevs)
-					push(devices, ...resolve_lower_devices(device));
+					push(devices, ...resolve_lower_devices(devstatus, device, true));
 
 			if (length(devices)) {
 				devices = sort(uniq(devices));
 
-				if (this.default_option("flow_offloading_hw") && !nft_try_hw_offload(devices)) {
-					this.warn('Hardware flow offloading unavailable, falling back to software offloading');
-					this.state.defaults.flow_offloading_hw = false;
-				}
+				if (nft_try_hw_offload(devices))
+					return devices;
+
+				this.warn('Hardware flow offloading unavailable, falling back to software offloading');
+				this.state.defaults.flow_offloading_hw = false;
+
+				devices = [];
 			}
 		}
 
-		return devices;
+		for (let zone in this.zones())
+			for (let device in zone.related_physdevs)
+				push(devices, ...resolve_lower_devices(devstatus, device, false));
+
+		return sort(uniq(devices));
 	},
 
 	check_set_types: function() {
diff --git a/tests/01_configuration/01_ruleset b/tests/01_configuration/01_ruleset
index fa0e7a4..65cddd1 100644
--- a/tests/01_configuration/01_ruleset
+++ b/tests/01_configuration/01_ruleset
@@ -296,10 +296,10 @@ table inet fw4 {
 [!] Section @defaults[0] specifies unknown option 'unknown_defaults_option'
 [!] Section @rule[9] (Test-Deprecated-Rule-Option) option '_name' is deprecated by fw4
 [!] Section @rule[9] (Test-Deprecated-Rule-Option) specifies unknown option 'unknown_rule_option'
-[call] fs.open path </sys/class/net/br-lan/uevent> mode <null>
+[call] ctx.call object <network.device> method <status> args <null>
 [call] fs.opendir path </sys/class/net/br-lan>
-[call] fs.open path </sys/class/net/eth0/uevent> mode <null>
-[call] fs.open path </sys/class/net/eth1/uevent> mode <null>
+[call] fs.opendir path </sys/class/net/eth0>
+[call] fs.opendir path </sys/class/net/eth1>
 [call] system command </usr/sbin/nft -c '
 		add table inet fw4-hw-offload-test;
 		add flowtable inet fw4-hw-offload-test ft {