Re: kern/57645: bridge does not work on raspberry pi

[sc.dying%gmail.com@localhost]

The following reply was made to PR kern/57645; it has been noted by GNATS.

From: sc.dying%gmail.com@localhost
To: gnats-bugs%netbsd.org@localhost, kern-bug-people%netbsd.org@localhost, gnats-admin%netbsd.org@localhost
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)
 >  
 >