[PATCH] net: usb: ipheth: stop data URBs on ndo_stop

[PATCH] net: usb: ipheth: stop data URBs on ndo_stop

[raoxu]

From: Xu Rao <raoxu@uniontech.com>

ipheth_open() submits the RX URB and ipheth_tx() can submit the TX URB
while the netdev is running.  A successful RX completion resubmits the
RX URB from ipheth_rcvbulk_callback(), so the receive path remains
active until the URB is explicitly stopped.

ipheth_close() stops the netdev queue and disables the carrier work, but
it leaves any submitted data URBs running.  After an administrative link
down, an already submitted RX URB can therefore complete successfully,
account the received packet and pass it to the networking stack, and
then submit the RX URB again even though the interface has been stopped.

The disconnect path already kills the URBs after unregister_netdev(), so
unplug testing eventually quiesces the data path.  The gap is therefore
specific to an administrative close while the USB device remains
connected.

Kill the data URBs from ndo_stop as well.  Do this after disabling
carrier_work: ipheth_sndbulk_callback() schedules the work on TX URB
errors, while disable_delayed_work_sync() prevents a completion caused by
usb_kill_urb() from re-arming it.  usb_kill_urb() also waits for pending
completion handlers and prevents the RX completion from successfully
resubmitting its URB.

Fixes: a19259c3d589 ("drivers/net/usb: Add new driver ipheth")
Cc: stable@vger.kernel.org
Signed-off-by: Xu Rao <raoxu@uniontech.com>
---
 drivers/net/usb/ipheth.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/usb/ipheth.c b/drivers/net/usb/ipheth.c
index 2b490114d232..f127aeab7031 100644
--- a/drivers/net/usb/ipheth.c
+++ b/drivers/net/usb/ipheth.c
@@ -505,6 +505,7 @@ static int ipheth_close(struct net_device *net)
 	 * it, so that such a schedule_delayed_work() is a no-op.
 	 */
 	disable_delayed_work_sync(&dev->carrier_work);
+	ipheth_kill_urbs(dev);
 	return 0;
 }
 
-- 
2.50.1

Re: [PATCH] net: usb: ipheth: stop data URBs on ndo_stop

[Oliver Neukum]

On 31.08.26 11:36, raoxu wrote:
> The disconnect path already kills the URBs after unregister_netdev(), so
> unplug testing eventually quiesces the data path.  The gap is therefore
> specific to an administrative close while the USB device remains
> connected.
> 
> Kill the data URBs from ndo_stop as well.  Do this after disabling
> carrier_work: ipheth_sndbulk_callback() schedules the work on TX URB
> errors, while disable_delayed_work_sync() prevents a completion caused by
> usb_kill_urb() from re-arming it.  usb_kill_urb() also waits for pending
> completion handlers and prevents the RX completion from successfully
> resubmitting its URB.

Hi,

this also kills the transmit data path. The packets in that URB
have already been reported back as successfully transmitted.
Is this really the correct approach?

	Regards
		Oliver