Attachment 255301 Details for Bug 252165 – usb_ethernet Sync ioctl/tick media status changes v3

[patch] usb_ethernet Sync ioctl/tick media status changes v3

0001-usb_ethernet-Sync-tick-ioctl-calls.patch (text/plain), 3.10 KB, created by Ali Abdallah on 2024-11-19 15:17:39 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Ali Abdallah

Created: 2024-11-19 15:17:39 UTC

Size: 3.10 KB

patch

obsolete

>From 1a7962def3e73eb18a84c230e2c22a1b6d1b71ef Mon Sep 17 00:00:00 2001
>From: Ali Abdallah <ali.abdallah@suse.com>
>Date: Tue, 19 Nov 2024 16:14:28 +0100
>Subject: [PATCH] usb_ethernet: Sync tick/ioctl calls
>
>By design usbd_do_request_flags releases mtx lock and then acquire it
>later, but this cause the following issue.
>
>1. ue_tick_task acquired the lock
>2. uether ioctl SIOCGIFMEDIA/SIOCSIFMEDIA waiting for the lock
>3. usbd_do_request_flags code called by ue_tick_task releases the lock
>4. uether ioctl acquires the lock, and continues.
>
>The above causes a race in setting mii->mii_media_status and mii_media_active
>which causes the link status to flicker. It can also occur between two
>different ioctl threads as well!
>
>link state changed to UP
>link state changed to DOWN
>link state changed to UP
>link state changed to DOWN
>...
>
>see https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=252165
>for more details
>---
> sys/dev/usb/net/usb_ethernet.c | 22 +++++++++++++++++++++-
> sys/dev/usb/net/usb_ethernet.h |  7 +++++++
> 2 files changed, 28 insertions(+), 1 deletion(-)
>
>diff --git a/sys/dev/usb/net/usb_ethernet.c b/sys/dev/usb/net/usb_ethernet.c
>index 2b90c9249855..6259a8a653ab 100644
>--- a/sys/dev/usb/net/usb_ethernet.c
>+++ b/sys/dev/usb/net/usb_ethernet.c
>@@ -176,6 +176,11 @@ uether_ifattach(struct usb_ether *ue)
> 		goto error;
> 	}
> 
>+	ue->ioctl_cnt = 0;
>+	ue->ticking = 0;
>+	cv_init(&ue->tick_cv, "uether_tick_cv");
>+	cv_init(&ue->ioctl_cv, "uether_ioctl_cv");
>+
> 	/* fork rest of the attach code */
> 	UE_LOCK(ue);
> 	ue_queue_command(ue, ue_attach_post_task,
>@@ -327,6 +332,9 @@ uether_ifdetach(struct usb_ether *ue)
> 			bus_topo_unlock();
> 		}
> 
>+		cv_destroy(&ue->ioctl_cv);
>+		cv_destroy(&ue->tick_cv);
>+
> 		/* free interface instance */
> 		if_free(ifp);
> 
>@@ -499,10 +507,13 @@ ue_tick_task(struct usb_proc_msg *_task)
> 	struct usb_ether *ue = task->ue;
> 	if_t ifp = ue->ue_ifp;
> 
>+
> 	if ((if_getdrvflags(ifp) & IFF_DRV_RUNNING) == 0)
> 		return;
>-
>+	ue->ticking = 1;
> 	ue->ue_methods->ue_tick(ue);
>+	cv_signal(&ue->tick_cv);
>+	ue->ticking = 0;
> }
> 
> int
>@@ -543,8 +554,17 @@ uether_ioctl(if_t ifp, u_long command, caddr_t data)
> 	case SIOCGIFMEDIA:
> 	case SIOCSIFMEDIA:
> 		if (ue->ue_miibus != NULL) {
>+			UE_LOCK(ue);
>+			while(ue->ticking)
>+				cv_wait(&ue->tick_cv, ue->ue_mtx);
>+			while(ue->ioctl_cnt)
>+				cv_wait(&ue->ioctl_cv, ue->ue_mtx);
>+			UE_UNLOCK(ue);
> 			mii = device_get_softc(ue->ue_miibus);
>+			ue->ioctl_cnt = 1;
> 			error = ifmedia_ioctl(ifp, ifr, &mii->mii_media, command);
>+			ue->ioctl_cnt = 0;
>+			cv_signal(&ue->ioctl_cv);
> 		} else
> 			error = ether_ioctl(ifp, command, data);
> 		break;
>diff --git a/sys/dev/usb/net/usb_ethernet.h b/sys/dev/usb/net/usb_ethernet.h
>index 1f0e121af7a3..bd56868aa733 100644
>--- a/sys/dev/usb/net/usb_ethernet.h
>+++ b/sys/dev/usb/net/usb_ethernet.h
>@@ -75,6 +75,13 @@ struct usb_ether {
> 	/* NOTE: the "ue_ifp" pointer must be first --hps */
> 	if_t			ue_ifp;
> 	struct mtx		*ue_mtx;
>+
>+	/* Sync ticking/ioctls calls */
>+	struct cv		tick_cv;
>+	int			ticking;
>+	struct cv		ioctl_cv;
>+	int			ioctl_cnt;
>+
> 	const struct usb_ether_methods *ue_methods;
> 	struct sysctl_oid	*ue_sysctl_oid;
> 	void			*ue_sc;
>-- 
>2.46.2
>

Actions: View | Diff

Attachments on bug 252165: 221274 | 236901 | 251770 | 255142 | 255162 | 255301