diff mbox series

dnsmasq: Ignore carrier status for bridge interfaces

Message ID 185cfa553e3745829776c33345504bbf@4rf.com
State Rejected
Delegated to: Petr Štetiar
Headers show
Series dnsmasq: Ignore carrier status for bridge interfaces | expand

Commit Message

Reuben Dowle July 16, 2020, 12:10 a.m. UTC
dnsmasq sometimes does not listen for DHCP at bootup on lan (see bug FS#1765).

This occurs because netifd can incorrectly indicate carrier down on an
interface through devstatus after issuing a carrier up hotplug event.

This patch ignores carrier status for bridge interfaces, as this does not
reflect media state so is not a useful check.

Signed-off-by: Reuben Dowle <reuben.dowle@4rf.com>
---
 package/network/services/dnsmasq/files/dnsmasq.init | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

--
2.7.4

Comments

Petr Štetiar Dec. 22, 2020, 11:26 a.m. UTC | #1
Reuben Dowle <reuben.dowle@4rf.com> [2020-07-16 00:10:43]:

Hi,

> This occurs because netifd can incorrectly indicate carrier down on an
> interface through devstatus after issuing a carrier up hotplug event.

then it seems like this should be fixed in netifd.

> This patch ignores carrier status for bridge interfaces, as this does not
> reflect media state so is not a useful check.

This looks like a band aid, not a proper fix.

Cheers,

Petr
Reuben Dowle Jan. 10, 2021, 8:17 p.m. UTC | #2
I agree that there does seem to be a bug in netifd here.

That said, it also does not make sense to look at the carrier status of some types of interface that don't have this information. A bridge interface can never have carrier down status. The interface itself can be up or down, but not the carrier. 

-----Original Message-----
From: Petr Štetiar <ynezz@true.cz> 
Sent: Wednesday, 23 December 2020 12:27 am
To: Reuben Dowle <reuben.dowle@4rf.com>
Cc: openwrt-devel@lists.openwrt.org
Subject: Re: [PATCH] dnsmasq: Ignore carrier status for bridge interfaces

Reuben Dowle <reuben.dowle@4rf.com> [2020-07-16 00:10:43]:

Hi,

> This occurs because netifd can incorrectly indicate carrier down on an 
> interface through devstatus after issuing a carrier up hotplug event.

then it seems like this should be fixed in netifd.

> This patch ignores carrier status for bridge interfaces, as this does 
> not reflect media state so is not a useful check.

This looks like a band aid, not a proper fix.

Cheers,

Petr
diff mbox series

Patch

diff --git a/package/network/services/dnsmasq/files/dnsmasq.init b/package/network/services/dnsmasq/files/dnsmasq.init
index 9288971..3aafdf9 100644
--- a/package/network/services/dnsmasq/files/dnsmasq.init
+++ b/package/network/services/dnsmasq/files/dnsmasq.init
@@ -104,9 +104,12 @@  dhcp_check() {

	# If there's no carrier yet, skip this interface.
	# The init script will be called again once the link is up
-	case "$(devstatus "$ifname" | jsonfilter -e @.carrier)" in
-		false) return 1;;
-	esac
+	local carrier
+	local type
+	eval $(devstatus "$ifname" | jsonfilter -e "carrier=@.carrier" -e "type=@.type")
+	if [ "$carrier" = "false" -a "$type" != "bridge" ]; then
+		return 1
+	fi

	udhcpc -n -q -s /bin/true -t 1 -i "$ifname" >&- && rv=1 || rv=0