NetBSD Problem Report #57645
From sc.dying@gmail.com Sat Oct 7 09:20:29 2023
Return-Path: <sc.dying@gmail.com>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
by mollari.NetBSD.org (Postfix) with ESMTPS id 151431A923A
for <gnats-bugs@gnats.NetBSD.org>; Sat, 7 Oct 2023 09:20:29 +0000 (UTC)
Message-Id: <0db7cf4a-ad51-4716-a2e6-5a135c522f04@gmail.com>
Date: Sat, 7 Oct 2023 09:20:13 +0000
From: sc.dying@gmail.com
To: gnats-bugs@NetBSD.org
Subject: bridge does not work on raspberry pi
>Number: 57645
>Category: kern
>Synopsis: bridge does not work on raspberry pi
>Confidential: no
>Severity: critical
>Priority: medium
>Responsible: riastradh
>State: closed
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Sat Oct 07 09:25:00 +0000 2023
>Closed-Date: Tue Oct 17 14:29:07 +0000 2023
>Last-Modified: Tue Oct 17 14:29:07 +0000 2023
>Originator: sc.dying@gmail.com
>Release: NetBSD-10.99.9 (and 10.0)
>Organization:
>Environment:
System: raspberrypi,model-b
Architecture: earmv6hf
Machine: evbarm
>Description:
bridge using ethernet dongles on raspberry pi does
not work.
On my testbed (see How-To-Repeat section), tcpdump shows
only broadcast/multicast packets on both interfaces, but
unicast packets are not observed.
>How-To-Repeat:
The testbed consists of a NetBSD rpi (named A) with 2 usb
ethernet devices and two hosts (B, C). In my case two RTL8152
are connected to rpi1b.
Configure A as a bridge, and make sure B and C are connected via A.
ping, ping6, and arping from B to C are unreachable, and vice versa.
>Fix:
I have no good idea.
>Release-Note:
>Audit-Trail:
From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: kern/57645: bridge does not work on raspberry pi
Date: Sat, 7 Oct 2023 11:31:41 +0200
Can you show full ifconfig output for the two ethernet devices and the
bridge?
Martin
From: sc.dying@gmail.com
To: gnats-bugs@netbsd.org, kern-bug-people@netbsd.org, gnats-admin@netbsd.org
Cc:
Subject: Re: kern/57645: bridge does not work on raspberry pi
Date: Sat, 7 Oct 2023 09:58:33 +0000
On 2023/10/07 9:35, Martin Husemann wrote:
> The following reply was made to PR kern/57645; it has been noted by GNATS.
>
> From: Martin Husemann <martin@duskware.de>
> To: gnats-bugs@netbsd.org
> Cc:
> Subject: Re: kern/57645: bridge does not work on raspberry pi
> Date: Sat, 7 Oct 2023 11:31:41 +0200
>
> Can you show full ifconfig output for the two ethernet devices and the
> bridge?
Just I rebooted, so counters are reset.
# ifconfig -va
usmsc0: flags=0x8802<BROADCAST,SIMPLEX,MULTICAST> mtu 1500
ec_capabilities=0x1<VLAN_MTU>
ec_enabled=0
address: b8:27:eb:xx:xx:xx
media: Ethernet autoselect (100baseTX full-duplex)
status: active
input: 0 packets, 0 bytes
output: 0 packets, 0 bytes
ure0: flags=0x8943<UP,BROADCAST,RUNNING,PROMISC,SIMPLEX,MULTICAST> mtu 1500
capabilities=0x3ff00<IP4CSUM_Rx,IP4CSUM_Tx,TCP4CSUM_Rx,TCP4CSUM_Tx>
capabilities=0x3ff00<UDP4CSUM_Rx,UDP4CSUM_Tx,TCP6CSUM_Rx,TCP6CSUM_Tx>
capabilities=0x3ff00<UDP6CSUM_Rx,UDP6CSUM_Tx>
enabled=0
ec_capabilities=0x1<VLAN_MTU>
ec_enabled=0x1<VLAN_MTU>
address: 00:e0:4c:xx:xx:xx
media: Ethernet autoselect (100baseTX full-duplex)
status: active
input: 0 packets, 1440 bytes, 24 multicasts, 24 unknown protocol
output: 24 packets, 1440 bytes
ure1: flags=0x8943<UP,BROADCAST,RUNNING,PROMISC,SIMPLEX,MULTICAST> mtu 1500
capabilities=0x3ff00<IP4CSUM_Rx,IP4CSUM_Tx,TCP4CSUM_Rx,TCP4CSUM_Tx>
capabilities=0x3ff00<UDP4CSUM_Rx,UDP4CSUM_Tx,TCP6CSUM_Rx,TCP6CSUM_Tx>
capabilities=0x3ff00<UDP6CSUM_Rx,UDP6CSUM_Tx>
enabled=0
ec_capabilities=0x1<VLAN_MTU>
ec_enabled=0x1<VLAN_MTU>
address: 00:e0:4c:xx:xx:xx
media: Ethernet autoselect (100baseTX full-duplex)
status: active
input: 24 packets, 1440 bytes, 24 multicasts, 24 unknown protocol
output: 0 packets, 0 bytes
lo0: flags=0x8048<LOOPBACK,RUNNING,MULTICAST> mtu 33176
status: active
input: 0 packets, 0 bytes
output: 0 packets, 0 bytes
bridge0: flags=0x41<UP,RUNNING> mtu 1500
status: active
input: 24 packets, 1440 bytes, 24 multicasts
output: 24 packets, 1440 bytes
#
>
> Martin
>
From: mlelstv@serpens.de (Michael van Elst)
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: kern/57645: bridge does not work on raspberry pi
Date: Sat, 7 Oct 2023 15:04:05 -0000 (UTC)
sc.dying@gmail.com writes:
> On my testbed (see How-To-Repeat section), tcpdump shows
> only broadcast/multicast packets on both interfaces, but
> unicast packets are not observed.
I can replicate this with an axen(4) and the builtin usmsc(4) device.
The underlying problem is that the promiscous mode works for neither
device (completely without bridge).
In the bridge setup this means that only broadcast/multicast packets
work or those destined for the mac address of the interface itself.
From: Taylor R Campbell <riastradh@NetBSD.org>
To: sc.dying@gmail.com
Cc: gnats-bugs@NetBSD.org, Martin Husemann <martin@duskware.de>,
Michael van Elst <mlelstv@serpens.de>
Subject: Re: kern/57645: bridge does not work on raspberry pi
Date: Sat, 7 Oct 2023 23:12:34 +0000
This is a multi-part message in MIME format.
--=_UO/DuVWULAgwAF6z8mwyycn6j85jhood
Can you please try the attached patch and see if it makes a
difference?
--=_UO/DuVWULAgwAF6z8mwyycn6j85jhood
Content-Type: text/plain; charset="ISO-8859-1"; name="pr57646-usbnet-reinit"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: attachment; filename="pr57646-usbnet-reinit.patch"
From c3072c8e296617eb7b6f9ebcec48bfd5f5bab02a Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastradh@NetBSD.org>
Date: Sat, 7 Oct 2023 23:09:16 +0000
Subject: [PATCH] usbnet(4): On if_init, stop/init if IFF_RUNNING -- not noo=
p.
ether_ioctl(9) relies on this to reinitialize an interface when a
flags change returns ENETRESET.
PR kern/57645
XXX pullup-10
---
sys/dev/usb/usbnet.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/sys/dev/usb/usbnet.c b/sys/dev/usb/usbnet.c
index c77e08e07f7e..cdf52af9de08 100644
--- a/sys/dev/usb/usbnet.c
+++ b/sys/dev/usb/usbnet.c
@@ -1300,14 +1300,15 @@ usbnet_if_init(struct ifnet *ifp)
return EIO;
=20
/*
- * If we're already running, nothing to do.
+ * If we're already running, stop the interface first -- we're
+ * reinitializing it.
*
- * XXX This should be an assertion, but it may require some
- * analysis -- and possibly some tweaking -- of sys/net to
- * ensure.
+ * XXX Grody for sys/net to call if_init to reinitialize. This
+ * should be an assertion, not a branch, but it will require
+ * some tweaking of sys/net to avoid.
*/
if (ifp->if_flags & IFF_RUNNING)
- return 0;
+ uno_stop(un, ifp, /*disable*/1/*XXX???*/);
=20
error =3D uno_init(un, ifp);
if (error)
--=_UO/DuVWULAgwAF6z8mwyycn6j85jhood--
From: Taylor R Campbell <riastradh@NetBSD.org>
To: sc.dying@gmail.com
Cc: gnats-bugs@NetBSD.org, Martin Husemann <martin@duskware.de>,
Michael van Elst <mlelstv@serpens.de>
Subject: Re: kern/57645: bridge does not work on raspberry pi
Date: Sat, 7 Oct 2023 23:33:57 +0000
This is a multi-part message in MIME format.
--=_k+R2A0jPCfl/fFvU84G4E4n02s6AxFtx
Correction -- try this patch instead?
--=_k+R2A0jPCfl/fFvU84G4E4n02s6AxFtx
Content-Type: text/plain; charset="ISO-8859-1"; name="pr57646-usbnet-reinit-v2"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: attachment; filename="pr57646-usbnet-reinit-v2.patch"
From c7f5d719c0e8bea099b28f2d57dfe7c8fa980c7e Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastradh@NetBSD.org>
Date: Sat, 7 Oct 2023 23:09:16 +0000
Subject: [PATCH] usbnet(4): On if_init, stop/init if IFF_RUNNING -- not noo=
p.
ether_ioctl(9) relies on this to reinitialize an interface when a
flags change returns ENETRESET.
PR kern/57645
XXX pullup-10
---
sys/dev/usb/usbnet.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/sys/dev/usb/usbnet.c b/sys/dev/usb/usbnet.c
index c77e08e07f7e..8daa904752cf 100644
--- a/sys/dev/usb/usbnet.c
+++ b/sys/dev/usb/usbnet.c
@@ -1112,6 +1112,7 @@ usbnet_stop(struct usbnet *un, struct ifnet *ifp, int=
disable)
USBNETHIST_FUNC(); USBNETHIST_CALLED();
=20
KASSERTMSG(IFNET_LOCKED(ifp), "%s", ifp->if_xname);
+ KASSERTMSG(ifp->if_flags & IFF_RUNNING, "%s", ifp->if_xname);
=20
/*
* For drivers with hardware multicast filter update callbacks:
@@ -1300,14 +1301,16 @@ usbnet_if_init(struct ifnet *ifp)
return EIO;
=20
/*
- * If we're already running, nothing to do.
+ * If we're already running, stop the interface first -- we're
+ * reinitializing it.
*
- * XXX This should be an assertion, but it may require some
- * analysis -- and possibly some tweaking -- of sys/net to
- * ensure.
+ * XXX Grody for sys/net to call if_init to reinitialize. This
+ * should be an assertion, not a branch, but it will require
+ * some tweaking of sys/net to avoid.
*/
if (ifp->if_flags & IFF_RUNNING)
- return 0;
+ usbnet_stop(un, ifp, /*disable*/1/*XXX???*/);
+ KASSERTMSG((ifp->if_flags & IFF_RUNNING) =3D=3D 0, "%s", ifp->if_xname);
=20
error =3D uno_init(un, ifp);
if (error)
--=_k+R2A0jPCfl/fFvU84G4E4n02s6AxFtx--
From: sc.dying@gmail.com
To: Taylor R Campbell <riastradh@NetBSD.org>
Cc: gnats-bugs@NetBSD.org, Martin Husemann <martin@duskware.de>,
Michael van Elst <mlelstv@serpens.de>
Subject: Re: kern/57645: bridge does not work on raspberry pi
Date: Sun, 8 Oct 2023 04:16:56 +0000
On 2023/10/07 23:33, Taylor R Campbell wrote:
> Correction -- try this patch instead?
Not work.
(I removed noisy hub speaking HELLO from ure1 side)
4 arping request (broadcast) sent from ure0 side, 0 replies (unicast).
it doesn't look ure1 receives unicasts.
ure0: flags=0x8943<UP,BROADCAST,RUNNING,PROMISC,SIMPLEX,MULTICAST> mtu 1500
capabilities=0x3ff00<IP4CSUM_Rx,IP4CSUM_Tx,TCP4CSUM_Rx,TCP4CSUM_Tx>
capabilities=0x3ff00<UDP4CSUM_Rx,UDP4CSUM_Tx,TCP6CSUM_Rx,TCP6CSUM_Tx>
capabilities=0x3ff00<UDP6CSUM_Rx,UDP6CSUM_Tx>
enabled=0
ec_capabilities=0x1<VLAN_MTU>
ec_enabled=0x1<VLAN_MTU>
address: 00:e0:4c:xx:xx:xx
media: Ethernet autoselect (100baseTX full-duplex)
status: active
input: 4 packets, 240 bytes, 4 multicasts
output: 0 packets, 0 bytes
ure1: flags=0x8943<UP,BROADCAST,RUNNING,PROMISC,SIMPLEX,MULTICAST> mtu 1500
capabilities=0x3ff00<IP4CSUM_Rx,IP4CSUM_Tx,TCP4CSUM_Rx,TCP4CSUM_Tx>
capabilities=0x3ff00<UDP4CSUM_Rx,UDP4CSUM_Tx,TCP6CSUM_Rx,TCP6CSUM_Tx>
capabilities=0x3ff00<UDP6CSUM_Rx,UDP6CSUM_Tx>
enabled=0
ec_capabilities=0x1<VLAN_MTU>
ec_enabled=0x1<VLAN_MTU>
address: 00:e0:4c:xx:xx:xx
media: Ethernet autoselect (100baseTX full-duplex)
status: active
input: 0 packets, 240 bytes, 4 multicasts
output: 4 packets, 240 bytes
bridge0: flags=0x41<UP,RUNNING> mtu 1500
status: active
input: 4 packets, 240 bytes, 4 multicasts
output: 4 packets, 240 bytes
#
From: mlelstv@serpens.de (Michael van Elst)
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: kern/57645: bridge does not work on raspberry pi
Date: Sun, 8 Oct 2023 10:46:29 -0000 (UTC)
mlelstv@serpens.de (Michael van Elst) writes:
>The following reply was made to PR kern/57645; it has been noted by GNATS.
>From: mlelstv@serpens.de (Michael van Elst)
>To: gnats-bugs@netbsd.org
>Cc:
>Subject: Re: kern/57645: bridge does not work on raspberry pi
>Date: Sat, 7 Oct 2023 15:04:05 -0000 (UTC)
> sc.dying@gmail.com writes:
>
> > On my testbed (see How-To-Repeat section), tcpdump shows
> > only broadcast/multicast packets on both interfaces, but
> > unicast packets are not observed.
>
> I can replicate this with an axen(4) and the builtin usmsc(4) device.
>
> The underlying problem is that the promiscous mode works for neither
> device (completely without bridge).
>
> In the bridge setup this means that only broadcast/multicast packets
> work or those destined for the mac address of the interface itself.
>
The usbnet code has also an inverted logic on when to change settings
so that it effectively never takes place. The following patch works
for me:
Index: sys/dev/usb/usbnet.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/usbnet.c,v
retrieving revision 1.114
diff -p -u -r1.114 usbnet.c
--- sys/dev/usb/usbnet.c 15 Jul 2023 21:41:26 -0000 1.114
+++ sys/dev/usb/usbnet.c 8 Oct 2023 10:40:10 -0000
@@ -1012,7 +1012,7 @@ usbnet_ifflags_cb(struct ethercom *ec)
KASSERTMSG(IFNET_LOCKED(ifp), "%s", ifp->if_xname);
const u_short changed = ifp->if_flags ^ unp->unp_if_flags;
- if ((changed & ~(IFF_CANTCHANGE | IFF_DEBUG)) == 0) {
+ if ((changed & ~(IFF_CANTCHANGE | IFF_DEBUG)) != 0) {
mutex_enter(&unp->unp_mcastlock);
unp->unp_if_flags = ifp->if_flags;
mutex_exit(&unp->unp_mcastlock);
@@ -1030,8 +1030,6 @@ usbnet_ifflags_cb(struct ethercom *ec)
*/
if (changed & IFF_PROMISC)
rv = ENETRESET;
- } else {
- rv = ENETRESET;
}
return rv;
@@ -1065,23 +1063,23 @@ usbnet_if_ioctl(struct ifnet *ifp, u_lon
error = ether_ioctl(ifp, cmd, data);
if (error == ENETRESET) {
+ /*
+ * If there's a hardware multicast filter, and
+ * it has been programmed by usbnet_init_rx_tx
+ * and is active, update it now. Otherwise,
+ * drop the update on the floor -- it will be
+ * observed by usbnet_init_rx_tx next time we
+ * bring the interface up.
+ */
+ if (un->un_ops->uno_mcast) {
+ mutex_enter(&unp->unp_mcastlock);
+ if (unp->unp_mcastactive)
+ (*un->un_ops->uno_mcast)(ifp);
+ mutex_exit(&unp->unp_mcastlock);
+ }
switch (cmd) {
case SIOCADDMULTI:
case SIOCDELMULTI:
- /*
- * If there's a hardware multicast filter, and
- * it has been programmed by usbnet_init_rx_tx
- * and is active, update it now. Otherwise,
- * drop the update on the floor -- it will be
- * observed by usbnet_init_rx_tx next time we
- * bring the interface up.
- */
- if (un->un_ops->uno_mcast) {
- mutex_enter(&unp->unp_mcastlock);
- if (unp->unp_mcastactive)
- (*un->un_ops->uno_mcast)(ifp);
- mutex_exit(&unp->unp_mcastlock);
- }
error = 0;
break;
default:
@@ -1306,8 +1304,16 @@ usbnet_if_init(struct ifnet *ifp)
* analysis -- and possibly some tweaking -- of sys/net to
* ensure.
*/
- if (ifp->if_flags & IFF_RUNNING)
+ if (ifp->if_flags & IFF_RUNNING) {
+ if (un->un_ops->uno_mcast) {
+ struct usbnet_private * const unp __unused = un->un_pri;
+ mutex_enter(&unp->unp_mcastlock);
+ (*un->un_ops->uno_mcast)(ifp);
+ unp->unp_mcastactive = true;
+ mutex_exit(&unp->unp_mcastlock);
+ }
return 0;
+ }
error = uno_init(un, ifp);
if (error)
From: Taylor R Campbell <riastradh@NetBSD.org>
To: sc.dying@gmail.com
Cc: gnats-bugs@NetBSD.org, Martin Husemann <martin@duskware.de>,
Michael van Elst <mlelstv@serpens.de>
Subject: Re: kern/57645: bridge does not work on raspberry pi
Date: Sun, 8 Oct 2023 12:45:33 +0000
> Date: Sun, 8 Oct 2023 04:16:56 +0000
> From: sc.dying@gmail.com
>
> On 2023/10/07 23:33, Taylor R Campbell wrote:
> > Correction -- try this patch instead?
>
> Not work.
Does it work in netbsd-9? Any chance you could bisect? Changes to
usbnet between 9.0 and 10.0 were largely very fine-grained, so
bisection should be able to identify exactly what broke it.
From: sc.dying@gmail.com
To: gnats-bugs@netbsd.org, kern-bug-people@netbsd.org, gnats-admin@netbsd.org
Cc:
Subject: Re: kern/57645: bridge does not work on raspberry pi
Date: Sun, 8 Oct 2023 12:59:15 +0000
On 2023/10/08 10:50, Michael van Elst wrote:
[...]
>
> Index: sys/dev/usb/usbnet.c
> ===================================================================
> RCS file: /cvsroot/src/sys/dev/usb/usbnet.c,v
> retrieving revision 1.114
> diff -p -u -r1.114 usbnet.c
> --- sys/dev/usb/usbnet.c 15 Jul 2023 21:41:26 -0000 1.114
> +++ sys/dev/usb/usbnet.c 8 Oct 2023 10:40:10 -0000
> @@ -1012,7 +1012,7 @@ usbnet_ifflags_cb(struct ethercom *ec)
> KASSERTMSG(IFNET_LOCKED(ifp), "%s", ifp->if_xname);
>
> const u_short changed = ifp->if_flags ^ unp->unp_if_flags;
> - if ((changed & ~(IFF_CANTCHANGE | IFF_DEBUG)) == 0) {
> + if ((changed & ~(IFF_CANTCHANGE | IFF_DEBUG)) != 0) {
> mutex_enter(&unp->unp_mcastlock);
> unp->unp_if_flags = ifp->if_flags;
> mutex_exit(&unp->unp_mcastlock);
> @@ -1030,8 +1030,6 @@ usbnet_ifflags_cb(struct ethercom *ec)
> */
> if (changed & IFF_PROMISC)
> rv = ENETRESET;
> - } else {
> - rv = ENETRESET;
> }
>
> return rv;
I don't think this code works as expected,
as IFF_CANTCHANGE includes IFF_PROMISC.
if ((changed & (~(IFF_CANTCHANGE | IFF_DEBUG) | IFF_PROMISC)) != 0) {
would work.
> @@ -1065,23 +1063,23 @@ usbnet_if_ioctl(struct ifnet *ifp, u_lon
>
> error = ether_ioctl(ifp, cmd, data);
> if (error == ENETRESET) {
> + /*
> + * If there's a hardware multicast filter, and
> + * it has been programmed by usbnet_init_rx_tx
> + * and is active, update it now. Otherwise,
> + * drop the update on the floor -- it will be
> + * observed by usbnet_init_rx_tx next time we
> + * bring the interface up.
> + */
> + if (un->un_ops->uno_mcast) {
> + mutex_enter(&unp->unp_mcastlock);
> + if (unp->unp_mcastactive)
> + (*un->un_ops->uno_mcast)(ifp);
> + mutex_exit(&unp->unp_mcastlock);
> + }
> switch (cmd) {
> case SIOCADDMULTI:
> case SIOCDELMULTI:
> - /*
> - * If there's a hardware multicast filter, and
> - * it has been programmed by usbnet_init_rx_tx
> - * and is active, update it now. Otherwise,
> - * drop the update on the floor -- it will be
> - * observed by usbnet_init_rx_tx next time we
> - * bring the interface up.
> - */
> - if (un->un_ops->uno_mcast) {
> - mutex_enter(&unp->unp_mcastlock);
> - if (unp->unp_mcastactive)
> - (*un->un_ops->uno_mcast)(ifp);
> - mutex_exit(&unp->unp_mcastlock);
> - }
> error = 0;
> break;
> default:
> @@ -1306,8 +1304,16 @@ usbnet_if_init(struct ifnet *ifp)
> * analysis -- and possibly some tweaking -- of sys/net to
> * ensure.
> */
> - if (ifp->if_flags & IFF_RUNNING)
> + if (ifp->if_flags & IFF_RUNNING) {
> + if (un->un_ops->uno_mcast) {
> + struct usbnet_private * const unp __unused = un->un_pri;
> + mutex_enter(&unp->unp_mcastlock);
> + (*un->un_ops->uno_mcast)(ifp);
> + unp->unp_mcastactive = true;
> + mutex_exit(&unp->unp_mcastlock);
> + }
> return 0;
> + }
>
> error = uno_init(un, ifp);
> if (error)
>
>
From: sc.dying@gmail.com
To: Taylor R Campbell <riastradh@NetBSD.org>
Cc: gnats-bugs@NetBSD.org, Martin Husemann <martin@duskware.de>,
Michael van Elst <mlelstv@serpens.de>
Subject: Re: kern/57645: bridge does not work on raspberry pi
Date: Sun, 8 Oct 2023 13:05:46 +0000
On 2023/10/08 12:45, Taylor R Campbell wrote:
>> Date: Sun, 8 Oct 2023 04:16:56 +0000
>> From: sc.dying@gmail.com
>>
>> On 2023/10/07 23:33, Taylor R Campbell wrote:
>>> Correction -- try this patch instead?
>>
>> Not work.
>
> Does it work in netbsd-9? Any chance you could bisect? Changes to
> usbnet between 9.0 and 10.0 were largely very fine-grained, so
> bisection should be able to identify exactly what broke it.
On netbsd-9 bridge with usbnet works well.
I tested only ipv4 and arp.
It may takes long to bisect.
From: mlelstv@serpens.de (Michael van Elst)
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: kern/57645: bridge does not work on raspberry pi
Date: Sun, 8 Oct 2023 13:21:43 -0000 (UTC)
sc.dying@gmail.com writes:
> > - if ((changed & ~(IFF_CANTCHANGE | IFF_DEBUG)) == 0) {
> > + if ((changed & ~(IFF_CANTCHANGE | IFF_DEBUG)) != 0) {
> I don't think this code works as expected,
> as IFF_CANTCHANGE includes IFF_PROMISC.
>
> if ((changed & (~(IFF_CANTCHANGE | IFF_DEBUG) | IFF_PROMISC)) != 0) {
The interesting part is, that it does work, probably because at some
point IFF_UP changes and 'changed' isn't masked.
The IFF_CANTCHANGE mask doesn't look correct to me.
From: mlelstv@serpens.de (Michael van Elst)
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: kern/57645: bridge does not work on raspberry pi
Date: Sun, 8 Oct 2023 13:34:36 -0000 (UTC)
sc.dying@gmail.com writes:
> I don't think this code works as expected,
> as IFF_CANTCHANGE includes IFF_PROMISC.
>
> if ((changed & (~(IFF_CANTCHANGE | IFF_DEBUG) | IFF_PROMISC)) != 0) {
>
> would work.
Here is a better patch. The logic is now the same as in e.g. the bge driver
and still works for me.
Index: sys/dev/usb/usbnet.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/usbnet.c,v
retrieving revision 1.114
diff -p -u -r1.114 usbnet.c
--- sys/dev/usb/usbnet.c 15 Jul 2023 21:41:26 -0000 1.114
+++ sys/dev/usb/usbnet.c 8 Oct 2023 13:27:08 -0000
@@ -1011,11 +1011,12 @@ usbnet_ifflags_cb(struct ethercom *ec)
KASSERTMSG(IFNET_LOCKED(ifp), "%s", ifp->if_xname);
+ mutex_enter(&unp->unp_mcastlock);
const u_short changed = ifp->if_flags ^ unp->unp_if_flags;
- if ((changed & ~(IFF_CANTCHANGE | IFF_DEBUG)) == 0) {
- mutex_enter(&unp->unp_mcastlock);
- unp->unp_if_flags = ifp->if_flags;
- mutex_exit(&unp->unp_mcastlock);
+
+ if ((changed & ~(IFF_CANTCHANGE | IFF_DEBUG)) != 0) {
+ rv = ENETRESET;
+ } else {
/*
* XXX Can we just do uno_mcast synchronously here
* instead of resetting the whole interface?
@@ -1028,12 +1029,13 @@ usbnet_ifflags_cb(struct ethercom *ec)
* Maybe the logic can be unified, but it will require
* an audit and testing of all the usbnet drivers.
*/
- if (changed & IFF_PROMISC)
+ if ((changed & (IFF_PROMISC|IFF_ALLMULTI)) != 0)
rv = ENETRESET;
- } else {
- rv = ENETRESET;
}
+ unp->unp_if_flags = ifp->if_flags;
+ mutex_exit(&unp->unp_mcastlock);
+
return rv;
}
@@ -1065,23 +1067,23 @@ usbnet_if_ioctl(struct ifnet *ifp, u_lon
error = ether_ioctl(ifp, cmd, data);
if (error == ENETRESET) {
+ /*
+ * If there's a hardware multicast filter, and
+ * it has been programmed by usbnet_init_rx_tx
+ * and is active, update it now. Otherwise,
+ * drop the update on the floor -- it will be
+ * observed by usbnet_init_rx_tx next time we
+ * bring the interface up.
+ */
+ if (un->un_ops->uno_mcast) {
+ mutex_enter(&unp->unp_mcastlock);
+ if (unp->unp_mcastactive)
+ (*un->un_ops->uno_mcast)(ifp);
+ mutex_exit(&unp->unp_mcastlock);
+ }
switch (cmd) {
case SIOCADDMULTI:
case SIOCDELMULTI:
- /*
- * If there's a hardware multicast filter, and
- * it has been programmed by usbnet_init_rx_tx
- * and is active, update it now. Otherwise,
- * drop the update on the floor -- it will be
- * observed by usbnet_init_rx_tx next time we
- * bring the interface up.
- */
- if (un->un_ops->uno_mcast) {
- mutex_enter(&unp->unp_mcastlock);
- if (unp->unp_mcastactive)
- (*un->un_ops->uno_mcast)(ifp);
- mutex_exit(&unp->unp_mcastlock);
- }
error = 0;
break;
default:
@@ -1306,8 +1308,16 @@ usbnet_if_init(struct ifnet *ifp)
* analysis -- and possibly some tweaking -- of sys/net to
* ensure.
*/
- if (ifp->if_flags & IFF_RUNNING)
+ if (ifp->if_flags & IFF_RUNNING) {
+ if (un->un_ops->uno_mcast) {
+ struct usbnet_private * const unp = un->un_pri;
+ mutex_enter(&unp->unp_mcastlock);
+ (*un->un_ops->uno_mcast)(ifp);
+ unp->unp_mcastactive = true;
+ mutex_exit(&unp->unp_mcastlock);
+ }
return 0;
+ }
error = uno_init(un, ifp);
if (error)
From: sc.dying@gmail.com
To: gnats-bugs@netbsd.org, kern-bug-people@netbsd.org, gnats-admin@netbsd.org
Cc:
Subject: Re: kern/57645: bridge does not work on raspberry pi
Date: Sun, 8 Oct 2023 21:36:07 +0000
On 2023/10/08 13:35, Michael van Elst wrote:
> The following reply was made to PR kern/57645; it has been noted by GNATS.
>
> From: mlelstv@serpens.de (Michael van Elst)
> To: gnats-bugs@netbsd.org
> Cc:
> Subject: Re: kern/57645: bridge does not work on raspberry pi
> Date: Sun, 8 Oct 2023 13:34:36 -0000 (UTC)
>
> sc.dying@gmail.com writes:
>
> > I don't think this code works as expected,
> > as IFF_CANTCHANGE includes IFF_PROMISC.
> >
> > if ((changed & (~(IFF_CANTCHANGE | IFF_DEBUG) | IFF_PROMISC)) != 0) {
> >
> > would work.
>
>
> Here is a better patch. The logic is now the same as in e.g. the bge driver
> and still works for me.
It works for me, too.
Thank you for rewriting this.
>
>
> Index: sys/dev/usb/usbnet.c
> ===================================================================
> RCS file: /cvsroot/src/sys/dev/usb/usbnet.c,v
> retrieving revision 1.114
> diff -p -u -r1.114 usbnet.c
> --- sys/dev/usb/usbnet.c 15 Jul 2023 21:41:26 -0000 1.114
> +++ sys/dev/usb/usbnet.c 8 Oct 2023 13:27:08 -0000
> @@ -1011,11 +1011,12 @@ usbnet_ifflags_cb(struct ethercom *ec)
>
> KASSERTMSG(IFNET_LOCKED(ifp), "%s", ifp->if_xname);
>
> + mutex_enter(&unp->unp_mcastlock);
> const u_short changed = ifp->if_flags ^ unp->unp_if_flags;
> - if ((changed & ~(IFF_CANTCHANGE | IFF_DEBUG)) == 0) {
> - mutex_enter(&unp->unp_mcastlock);
> - unp->unp_if_flags = ifp->if_flags;
> - mutex_exit(&unp->unp_mcastlock);
> +
> + if ((changed & ~(IFF_CANTCHANGE | IFF_DEBUG)) != 0) {
> + rv = ENETRESET;
> + } else {
> /*
> * XXX Can we just do uno_mcast synchronously here
> * instead of resetting the whole interface?
> @@ -1028,12 +1029,13 @@ usbnet_ifflags_cb(struct ethercom *ec)
> * Maybe the logic can be unified, but it will require
> * an audit and testing of all the usbnet drivers.
> */
> - if (changed & IFF_PROMISC)
> + if ((changed & (IFF_PROMISC|IFF_ALLMULTI)) != 0)
> rv = ENETRESET;
> - } else {
> - rv = ENETRESET;
> }
>
> + unp->unp_if_flags = ifp->if_flags;
> + mutex_exit(&unp->unp_mcastlock);
> +
> return rv;
> }
>
> @@ -1065,23 +1067,23 @@ usbnet_if_ioctl(struct ifnet *ifp, u_lon
>
> error = ether_ioctl(ifp, cmd, data);
> if (error == ENETRESET) {
> + /*
> + * If there's a hardware multicast filter, and
> + * it has been programmed by usbnet_init_rx_tx
> + * and is active, update it now. Otherwise,
> + * drop the update on the floor -- it will be
> + * observed by usbnet_init_rx_tx next time we
> + * bring the interface up.
> + */
> + if (un->un_ops->uno_mcast) {
> + mutex_enter(&unp->unp_mcastlock);
> + if (unp->unp_mcastactive)
> + (*un->un_ops->uno_mcast)(ifp);
> + mutex_exit(&unp->unp_mcastlock);
> + }
> switch (cmd) {
> case SIOCADDMULTI:
> case SIOCDELMULTI:
> - /*
> - * If there's a hardware multicast filter, and
> - * it has been programmed by usbnet_init_rx_tx
> - * and is active, update it now. Otherwise,
> - * drop the update on the floor -- it will be
> - * observed by usbnet_init_rx_tx next time we
> - * bring the interface up.
> - */
> - if (un->un_ops->uno_mcast) {
> - mutex_enter(&unp->unp_mcastlock);
> - if (unp->unp_mcastactive)
> - (*un->un_ops->uno_mcast)(ifp);
> - mutex_exit(&unp->unp_mcastlock);
> - }
> error = 0;
> break;
> default:
> @@ -1306,8 +1308,16 @@ usbnet_if_init(struct ifnet *ifp)
> * analysis -- and possibly some tweaking -- of sys/net to
> * ensure.
> */
> - if (ifp->if_flags & IFF_RUNNING)
> + if (ifp->if_flags & IFF_RUNNING) {
> + if (un->un_ops->uno_mcast) {
> + struct usbnet_private * const unp = un->un_pri;
> + mutex_enter(&unp->unp_mcastlock);
> + (*un->un_ops->uno_mcast)(ifp);
> + unp->unp_mcastactive = true;
> + mutex_exit(&unp->unp_mcastlock);
> + }
> return 0;
> + }
>
> error = uno_init(un, ifp);
> if (error)
>
Responsible-Changed-From-To: kern-bug-people->riastradh
Responsible-Changed-By: riastradh@NetBSD.org
Responsible-Changed-When: Mon, 09 Oct 2023 11:26:23 +0000
Responsible-Changed-Why:
usbnet bugs
From: Taylor R Campbell <riastradh@NetBSD.org>
To: sc.dying@gmail.com
Cc: gnats-bugs@NetBSD.org
Subject: Re: kern/57645: bridge does not work on raspberry pi
Date: Mon, 9 Oct 2023 12:08:57 +0000
This is a multi-part message in MIME format.
--=_3GH/d8a4GiEuk6sY511/abKYWvHzDwXu
Can you please try this updated patch series (git am), or single diff
(git apply, or just patch(1))?
--=_3GH/d8a4GiEuk6sY511/abKYWvHzDwXu
Content-Type: text/plain; charset="ISO-8859-1"; name="pr57645-usbreinit-v3"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: attachment; filename="pr57645-usbreinit-v3.patch"
From 053b62a76f188a38cfba622781b8a7f9a8782617 Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastradh@NetBSD.org>
Date: Mon, 9 Oct 2023 11:59:20 +0000
Subject: [PATCH 1/3] usbnet(9): Make sure unp->unp_if_flags is initialized =
on
init.
usbnet_ifflags_cb is only called if the flags change while up and
running. (XXX Maybe it should be called in other circumstances too
so there's only one path here?)
Out of paranoia, clear the cache on stop.
PR kern/57645
XXX pullup-10
---
sys/dev/usb/usbnet.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/sys/dev/usb/usbnet.c b/sys/dev/usb/usbnet.c
index c77e08e07f7e..76ce9a3f5c61 100644
--- a/sys/dev/usb/usbnet.c
+++ b/sys/dev/usb/usbnet.c
@@ -867,6 +867,8 @@ usbnet_init_rx_tx(struct usbnet * const un)
*/
if (un->un_ops->uno_mcast) {
mutex_enter(&unp->unp_mcastlock);
+ KASSERTMSG(!unp->unp_mcastactive, "%s", ifp->if_xname);
+ unp->unp_if_flags =3D ifp->if_flags;
(*un->un_ops->uno_mcast)(ifp);
unp->unp_mcastactive =3D true;
mutex_exit(&unp->unp_mcastlock);
@@ -1000,6 +1002,13 @@ usbnet_media_upd(struct ifnet *ifp)
=20
/* ioctl */
=20
+/*
+ * usbnet_ifflags_cb(ec)
+ *
+ * Called by if_ethersubr when interface flags change
+ * (SIOCSIFFLAGS), or ethernet capabilities change
+ * (SIOCSETHERCAP), on a running interface.
+ */
static int
usbnet_ifflags_cb(struct ethercom *ec)
{
@@ -1120,7 +1129,9 @@ usbnet_stop(struct usbnet *un, struct ifnet *ifp, int=
disable)
*/
if (un->un_ops->uno_mcast) {
mutex_enter(&unp->unp_mcastlock);
+ KASSERTMSG(unp->unp_mcastactive, "%p", ifp->if_xname);
unp->unp_mcastactive =3D false;
+ unp->unp_if_flags =3D 0;
mutex_exit(&unp->unp_mcastlock);
}
=20
From 761831e5a8234b8197f9feceafcf4bcff539e133 Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastradh@NetBSD.org>
Date: Mon, 9 Oct 2023 12:04:22 +0000
Subject: [PATCH 2/3] usbnet(9): Fix sense of conditional in usbnet_ifflags_=
cb.
This appears to have been mistranscribed in revision 1.1 of usbnet.c.
PR kern/57645
XXX pullup-10
---
sys/dev/usb/usbnet.c | 54 ++++++++++++++++++++++++++------------------
1 file changed, 32 insertions(+), 22 deletions(-)
diff --git a/sys/dev/usb/usbnet.c b/sys/dev/usb/usbnet.c
index 76ce9a3f5c61..771b537bda40 100644
--- a/sys/dev/usb/usbnet.c
+++ b/sys/dev/usb/usbnet.c
@@ -1021,29 +1021,39 @@ usbnet_ifflags_cb(struct ethercom *ec)
KASSERTMSG(IFNET_LOCKED(ifp), "%s", ifp->if_xname);
=20
const u_short changed =3D ifp->if_flags ^ unp->unp_if_flags;
- if ((changed & ~(IFF_CANTCHANGE | IFF_DEBUG)) =3D=3D 0) {
- mutex_enter(&unp->unp_mcastlock);
- unp->unp_if_flags =3D ifp->if_flags;
- mutex_exit(&unp->unp_mcastlock);
- /*
- * XXX Can we just do uno_mcast synchronously here
- * instead of resetting the whole interface?
- *
- * Not yet, because some usbnet drivers (e.g., aue(4))
- * initialize the hardware differently in uno_init
- * depending on IFF_PROMISC. But some (again, aue(4))
- * _also_ need to know whether IFF_PROMISC is set in
- * uno_mcast and do something different with it there.
- * Maybe the logic can be unified, but it will require
- * an audit and testing of all the usbnet drivers.
- */
- if (changed & IFF_PROMISC)
- rv =3D ENETRESET;
- } else {
- rv =3D ENETRESET;
- }
=20
- return rv;
+ /*
+ * If any user-settable flags have changed other than
+ * IFF_DEBUG, just reset the interface.
+ */
+ if ((changed & ~(IFF_CANTCHANGE | IFF_DEBUG)) !=3D 0)
+ return ENETRESET;
+
+ /*
+ * Otherwise, cache the flags change so we can read the flags
+ * under uno_mcastlock for multicast updates in SIOCADDMULTI or
+ * SIOCDELMULTI without IFNET_LOCK.
+ */
+ mutex_enter(&unp->unp_mcastlock);
+ unp->unp_if_flags =3D ifp->if_flags;
+ mutex_exit(&unp->unp_mcastlock);
+
+ /*
+ * If we're switching on or off promiscuous mode, reprogram the
+ * hardware multicast filter now.
+ *
+ * XXX Actually, reset the interface, because some usbnet
+ * drivers (e.g., aue(4)) initialize the hardware differently
+ * in uno_init depending on IFF_PROMISC. But some (again,
+ * aue(4)) _also_ need to know whether IFF_PROMISC is set in
+ * uno_mcast and do something different with it there. Maybe
+ * the logic can be unified, but it will require an audit and
+ * testing of all the usbnet drivers.
+ */
+ if (changed & IFF_PROMISC)
+ return ENETRESET;
+
+ return 0;
}
=20
bool
From ce6d546a34f9689b3a06922e1e407c9f14d1e389 Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastradh@NetBSD.org>
Date: Mon, 9 Oct 2023 12:04:59 +0000
Subject: [PATCH 3/3] usbnet(9): On if_init, stop/init if IFF_RUNNING -- not
noop.
ether_ioctl(9) relies on this to reinitialize an interface when a
flags change returns ENETRESET. We can't just reprogram the hardware
multicast filter because some drivers have logic in if_init that's
conditional on IFF_PROMISC; perhaps we can reduce the cost of this if
we can change those drivers to do it in uno_mcast but that requires
some analysis to determine.
PR kern/57645
XXX pullup-10
---
sys/dev/usb/usbnet.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/sys/dev/usb/usbnet.c b/sys/dev/usb/usbnet.c
index 771b537bda40..5a3479671110 100644
--- a/sys/dev/usb/usbnet.c
+++ b/sys/dev/usb/usbnet.c
@@ -1131,6 +1131,7 @@ usbnet_stop(struct usbnet *un, struct ifnet *ifp, int=
disable)
USBNETHIST_FUNC(); USBNETHIST_CALLED();
=20
KASSERTMSG(IFNET_LOCKED(ifp), "%s", ifp->if_xname);
+ KASSERTMSG(ifp->if_flags & IFF_RUNNING, "%s", ifp->if_xname);
=20
/*
* For drivers with hardware multicast filter update callbacks:
@@ -1321,14 +1322,17 @@ usbnet_if_init(struct ifnet *ifp)
return EIO;
=20
/*
- * If we're already running, nothing to do.
+ * If we're already running, stop the interface first -- we're
+ * reinitializing it.
*
- * XXX This should be an assertion, but it may require some
- * analysis -- and possibly some tweaking -- of sys/net to
- * ensure.
+ * XXX Grody for sys/net to call if_init to reinitialize. This
+ * should be an assertion, not a branch, but it will require
+ * some tweaking of sys/net to avoid. See also the comment in
+ * usbnet_ifflags_cb about if_init vs uno_mcast on reinitalize.
*/
if (ifp->if_flags & IFF_RUNNING)
- return 0;
+ usbnet_stop(un, ifp, /*disable*/1/*XXX???*/);
+ KASSERTMSG((ifp->if_flags & IFF_RUNNING) =3D=3D 0, "%s", ifp->if_xname);
=20
error =3D uno_init(un, ifp);
if (error)
--=_3GH/d8a4GiEuk6sY511/abKYWvHzDwXu
Content-Type: text/plain; charset="ISO-8859-1"; name="pr57645-usbreinit-v3"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: attachment; filename="pr57645-usbreinit-v3.diff"
diff --git a/sys/dev/usb/usbnet.c b/sys/dev/usb/usbnet.c
index c77e08e07f7e..5a3479671110 100644
--- a/sys/dev/usb/usbnet.c
+++ b/sys/dev/usb/usbnet.c
@@ -867,6 +867,8 @@ usbnet_init_rx_tx(struct usbnet * const un)
*/
if (un->un_ops->uno_mcast) {
mutex_enter(&unp->unp_mcastlock);
+ KASSERTMSG(!unp->unp_mcastactive, "%s", ifp->if_xname);
+ unp->unp_if_flags =3D ifp->if_flags;
(*un->un_ops->uno_mcast)(ifp);
unp->unp_mcastactive =3D true;
mutex_exit(&unp->unp_mcastlock);
@@ -1000,6 +1002,13 @@ usbnet_media_upd(struct ifnet *ifp)
=20
/* ioctl */
=20
+/*
+ * usbnet_ifflags_cb(ec)
+ *
+ * Called by if_ethersubr when interface flags change
+ * (SIOCSIFFLAGS), or ethernet capabilities change
+ * (SIOCSETHERCAP), on a running interface.
+ */
static int
usbnet_ifflags_cb(struct ethercom *ec)
{
@@ -1012,29 +1021,39 @@ usbnet_ifflags_cb(struct ethercom *ec)
KASSERTMSG(IFNET_LOCKED(ifp), "%s", ifp->if_xname);
=20
const u_short changed =3D ifp->if_flags ^ unp->unp_if_flags;
- if ((changed & ~(IFF_CANTCHANGE | IFF_DEBUG)) =3D=3D 0) {
- mutex_enter(&unp->unp_mcastlock);
- unp->unp_if_flags =3D ifp->if_flags;
- mutex_exit(&unp->unp_mcastlock);
- /*
- * XXX Can we just do uno_mcast synchronously here
- * instead of resetting the whole interface?
- *
- * Not yet, because some usbnet drivers (e.g., aue(4))
- * initialize the hardware differently in uno_init
- * depending on IFF_PROMISC. But some (again, aue(4))
- * _also_ need to know whether IFF_PROMISC is set in
- * uno_mcast and do something different with it there.
- * Maybe the logic can be unified, but it will require
- * an audit and testing of all the usbnet drivers.
- */
- if (changed & IFF_PROMISC)
- rv =3D ENETRESET;
- } else {
- rv =3D ENETRESET;
- }
=20
- return rv;
+ /*
+ * If any user-settable flags have changed other than
+ * IFF_DEBUG, just reset the interface.
+ */
+ if ((changed & ~(IFF_CANTCHANGE | IFF_DEBUG)) !=3D 0)
+ return ENETRESET;
+
+ /*
+ * Otherwise, cache the flags change so we can read the flags
+ * under uno_mcastlock for multicast updates in SIOCADDMULTI or
+ * SIOCDELMULTI without IFNET_LOCK.
+ */
+ mutex_enter(&unp->unp_mcastlock);
+ unp->unp_if_flags =3D ifp->if_flags;
+ mutex_exit(&unp->unp_mcastlock);
+
+ /*
+ * If we're switching on or off promiscuous mode, reprogram the
+ * hardware multicast filter now.
+ *
+ * XXX Actually, reset the interface, because some usbnet
+ * drivers (e.g., aue(4)) initialize the hardware differently
+ * in uno_init depending on IFF_PROMISC. But some (again,
+ * aue(4)) _also_ need to know whether IFF_PROMISC is set in
+ * uno_mcast and do something different with it there. Maybe
+ * the logic can be unified, but it will require an audit and
+ * testing of all the usbnet drivers.
+ */
+ if (changed & IFF_PROMISC)
+ return ENETRESET;
+
+ return 0;
}
=20
bool
@@ -1112,6 +1131,7 @@ usbnet_stop(struct usbnet *un, struct ifnet *ifp, int=
disable)
USBNETHIST_FUNC(); USBNETHIST_CALLED();
=20
KASSERTMSG(IFNET_LOCKED(ifp), "%s", ifp->if_xname);
+ KASSERTMSG(ifp->if_flags & IFF_RUNNING, "%s", ifp->if_xname);
=20
/*
* For drivers with hardware multicast filter update callbacks:
@@ -1120,7 +1140,9 @@ usbnet_stop(struct usbnet *un, struct ifnet *ifp, int=
disable)
*/
if (un->un_ops->uno_mcast) {
mutex_enter(&unp->unp_mcastlock);
+ KASSERTMSG(unp->unp_mcastactive, "%p", ifp->if_xname);
unp->unp_mcastactive =3D false;
+ unp->unp_if_flags =3D 0;
mutex_exit(&unp->unp_mcastlock);
}
=20
@@ -1300,14 +1322,17 @@ usbnet_if_init(struct ifnet *ifp)
return EIO;
=20
/*
- * If we're already running, nothing to do.
+ * If we're already running, stop the interface first -- we're
+ * reinitializing it.
*
- * XXX This should be an assertion, but it may require some
- * analysis -- and possibly some tweaking -- of sys/net to
- * ensure.
+ * XXX Grody for sys/net to call if_init to reinitialize. This
+ * should be an assertion, not a branch, but it will require
+ * some tweaking of sys/net to avoid. See also the comment in
+ * usbnet_ifflags_cb about if_init vs uno_mcast on reinitalize.
*/
if (ifp->if_flags & IFF_RUNNING)
- return 0;
+ usbnet_stop(un, ifp, /*disable*/1/*XXX???*/);
+ KASSERTMSG((ifp->if_flags & IFF_RUNNING) =3D=3D 0, "%s", ifp->if_xname);
=20
error =3D uno_init(un, ifp);
if (error)
--=_3GH/d8a4GiEuk6sY511/abKYWvHzDwXu--
From: sc.dying@gmail.com
To: Taylor R Campbell <riastradh@NetBSD.org>
Cc: gnats-bugs@NetBSD.org
Subject: Re: kern/57645: bridge does not work on raspberry pi
Date: Mon, 9 Oct 2023 13:50:59 +0000
On 2023/10/09 12:08, Taylor R Campbell wrote:
> Can you please try this updated patch series (git am), or single diff
> (git apply, or just patch(1))?
(`int rv' at line 1019 is now reported unused, I comment it out and build.)
pr57645-usbreinit-v3.diff works.
pr57645-usbreinit-v3.patch works.
Tested by ping and arping.
Thank you.
From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/57645 CVS commit: src/sys/dev/usb
Date: Mon, 9 Oct 2023 17:42:00 +0000
Module Name: src
Committed By: riastradh
Date: Mon Oct 9 17:42:00 UTC 2023
Modified Files:
src/sys/dev/usb: usbnet.c
Log Message:
usbnet(9): Make sure unp->unp_if_flags is initialized on init.
usbnet_ifflags_cb is only called if the flags change while up and
running. (XXX Maybe it should be called in other circumstances too
so there's only one path here?)
Out of paranoia, clear the cache on stop.
PR kern/57645
XXX pullup-10
To generate a diff of this commit:
cvs rdiff -u -r1.114 -r1.115 src/sys/dev/usb/usbnet.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/57645 CVS commit: src/sys/dev/usb
Date: Mon, 9 Oct 2023 17:42:09 +0000
Module Name: src
Committed By: riastradh
Date: Mon Oct 9 17:42:09 UTC 2023
Modified Files:
src/sys/dev/usb: usbnet.c
Log Message:
usbnet(9): Fix sense of conditional in usbnet_ifflags_cb.
This appears to have been mistranscribed in revision 1.1 of usbnet.c.
PR kern/57645
XXX pullup-10
To generate a diff of this commit:
cvs rdiff -u -r1.115 -r1.116 src/sys/dev/usb/usbnet.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/57645 CVS commit: src/sys/dev/usb
Date: Mon, 9 Oct 2023 17:43:01 +0000
Module Name: src
Committed By: riastradh
Date: Mon Oct 9 17:43:01 UTC 2023
Modified Files:
src/sys/dev/usb: usbnet.c
Log Message:
usbnet(9): On if_init, stop/init if IFF_RUNNING -- not noop.
ether_ioctl(9) relies on this to reinitialize an interface when a
flags change returns ENETRESET. We can't just reprogram the hardware
multicast filter because some drivers have logic in if_init that's
conditional on IFF_PROMISC; perhaps we can reduce the cost of this if
we can change those drivers to do it in uno_mcast but that requires
some analysis to determine.
PR kern/57645
XXX pullup-10
To generate a diff of this commit:
cvs rdiff -u -r1.116 -r1.117 src/sys/dev/usb/usbnet.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/57645 CVS commit: src/sys/dev/usb
Date: Mon, 9 Oct 2023 17:44:33 +0000
Module Name: src
Committed By: riastradh
Date: Mon Oct 9 17:44:33 UTC 2023
Modified Files:
src/sys/dev/usb: usbnet.c
Log Message:
usbnet(9): Fix typo in comment.
No functional change intended.
PR kern/57645
XXX pullup-10
To generate a diff of this commit:
cvs rdiff -u -r1.117 -r1.118 src/sys/dev/usb/usbnet.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
State-Changed-From-To: open->needs-pullups
State-Changed-By: riastradh@NetBSD.org
State-Changed-When: Mon, 09 Oct 2023 17:47:01 +0000
State-Changed-Why:
fixed in HEAD, needs pullup-10
(regression since netbsd-9)
State-Changed-From-To: needs-pullups->pending-pullups
State-Changed-By: riastradh@NetBSD.org
State-Changed-When: Thu, 12 Oct 2023 18:35:52 +0000
State-Changed-Why:
pullup-10 #415 https://releng.netbsd.org/cgi-bin/req-10.cgi?show=415
From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/57645 CVS commit: [netbsd-10] src/sys/dev/usb
Date: Sat, 14 Oct 2023 07:03:10 +0000
Module Name: src
Committed By: martin
Date: Sat Oct 14 07:03:10 UTC 2023
Modified Files:
src/sys/dev/usb [netbsd-10]: usbnet.c
Log Message:
Pull up following revision(s) (requested by riastradh in ticket #415):
sys/dev/usb/usbnet.c: revision 1.115
sys/dev/usb/usbnet.c: revision 1.116
sys/dev/usb/usbnet.c: revision 1.117
sys/dev/usb/usbnet.c: revision 1.118
usbnet(9): Make sure unp->unp_if_flags is initialized on init.
usbnet_ifflags_cb is only called if the flags change while up and
running. (XXX Maybe it should be called in other circumstances too
so there's only one path here?)
Out of paranoia, clear the cache on stop.
PR kern/57645
usbnet(9): Fix sense of conditional in usbnet_ifflags_cb.
This appears to have been mistranscribed in revision 1.1 of usbnet.c.
PR kern/57645
usbnet(9): On if_init, stop/init if IFF_RUNNING -- not noop.
ether_ioctl(9) relies on this to reinitialize an interface when a
flags change returns ENETRESET. We can't just reprogram the hardware
multicast filter because some drivers have logic in if_init that's
conditional on IFF_PROMISC; perhaps we can reduce the cost of this if
we can change those drivers to do it in uno_mcast but that requires
some analysis to determine.
PR kern/57645
usbnet(9): Fix typo in comment.
No functional change intended.
PR kern/57645
To generate a diff of this commit:
cvs rdiff -u -r1.113 -r1.113.4.1 src/sys/dev/usb/usbnet.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
State-Changed-From-To: pending-pullups->closed
State-Changed-By: riastradh@NetBSD.org
State-Changed-When: Tue, 17 Oct 2023 14:29:07 +0000
State-Changed-Why:
fixed and pulled up
>Unformatted:
(Contact us)
$NetBSD: query-full-pr,v 1.47 2022/09/11 19:34:41 kim Exp $
$NetBSD: gnats_config.sh,v 1.9 2014/08/02 14:16:04 spz Exp $
Copyright © 1994-2023
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.
Home