2023-12-18 16:45:55

by Richard Acayan

[permalink] [raw]
Subject: [PATCH] usb: gadget: u_ether: Re-attach netif device to mirror detachment

In 6.7-rc1, there was a netif_device_detach call added to the
gether_disconnect function. This clears the __LINK_STATE_PRESENT bit of
the netif device and suppresses pings (ICMP messages) and TCP connection
requests from the connected host. If userspace temporarily disconnects
the gadget, such as by temporarily removing configuration in the gadget
configfs interface, network activity should continue to be processed
when the gadget is re-connected. Mirror the netif_device_detach call
with a netif_device_attach call in gether_connect to fix re-connecting
gadgets.

Link: https://gitlab.com/postmarketOS/pmaports/-/tree/6002e51b7090aeeb42947e0ca7ec22278d7227d0/main/postmarketos-base-ui/rootfs-usr-lib-NetworkManager-dispatcher.d-50-tethering.sh
Fixes: f49449fbc21e ("usb: gadget: u_ether: Replace netif_stop_queue with netif_device_detach")
Signed-off-by: Richard Acayan <[email protected]>
---
drivers/usb/gadget/function/u_ether.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c
index 9d1c40c152d8..3c5a6f6ac341 100644
--- a/drivers/usb/gadget/function/u_ether.c
+++ b/drivers/usb/gadget/function/u_ether.c
@@ -1163,6 +1163,8 @@ struct net_device *gether_connect(struct gether *link)
if (netif_running(dev->net))
eth_start(dev, GFP_ATOMIC);

+ netif_device_attach(dev->net);
+
/* on error, disable any endpoints */
} else {
(void) usb_ep_disable(link->out_ep);
--
2.43.0



2023-12-28 22:00:24

by Luca Weiss

[permalink] [raw]
Subject: Re: [PATCH] usb: gadget: u_ether: Re-attach netif device to mirror detachment

On Montag, 18. Dezember 2023 17:45:33 CET Richard Acayan wrote:
> In 6.7-rc1, there was a netif_device_detach call added to the
> gether_disconnect function. This clears the __LINK_STATE_PRESENT bit of
> the netif device and suppresses pings (ICMP messages) and TCP connection
> requests from the connected host. If userspace temporarily disconnects
> the gadget, such as by temporarily removing configuration in the gadget
> configfs interface, network activity should continue to be processed
> when the gadget is re-connected. Mirror the netif_device_detach call
> with a netif_device_attach call in gether_connect to fix re-connecting
> gadgets.

(+Cc Thorsten Leemhuis)

This appears to fix the regression on a 6.7-rc5-based build for
qcom-msm8974pro-fairphone-fp2, that the NCM network gadget doesn't work.
I've also heard reports from qcom-sdm845 and a PXA1908-based phone (if
I see this correctly) about issues on 6.7.

In postmarketOS on the device side the usb0 interface doesn't get the IP
address assigned correctly it seems, but it seems to behave a bit
inconsistently - but always broken.

Anyways, with this patch everything looks good again. I hope this makes
it for 6.7 final still.

Tested-by: Luca Weiss <[email protected]>


>
> Link: https://gitlab.com/postmarketOS/pmaports/-/tree/6002e51b7090aeeb42947e0ca7ec22278d7227d0/main/postmarketos-base-ui/rootfs-usr-lib-NetworkManager-dispatcher.d-50-tethering.sh
> Fixes: f49449fbc21e ("usb: gadget: u_ether: Replace netif_stop_queue with netif_device_detach")
> Signed-off-by: Richard Acayan <[email protected]>
> ---
> drivers/usb/gadget/function/u_ether.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c
> index 9d1c40c152d8..3c5a6f6ac341 100644
> --- a/drivers/usb/gadget/function/u_ether.c
> +++ b/drivers/usb/gadget/function/u_ether.c
> @@ -1163,6 +1163,8 @@ struct net_device *gether_connect(struct gether *link)
> if (netif_running(dev->net))
> eth_start(dev, GFP_ATOMIC);
>
> + netif_device_attach(dev->net);
> +
> /* on error, disable any endpoints */
> } else {
> (void) usb_ep_disable(link->out_ep);





2023-12-28 22:41:47

by Duje Mihanović

[permalink] [raw]
Subject: Re: [PATCH] usb: gadget: u_ether: Re-attach netif device to mirror detachment

On Monday, December 18, 2023 17:45:33 CET, Richard Acayan wrote:
> In 6.7-rc1, there was a netif_device_detach call added to the
> gether_disconnect function. This clears the __LINK_STATE_PRESENT bit of
> the netif device and suppresses pings (ICMP messages) and TCP connection
> requests from the connected host. If userspace temporarily disconnects
> the gadget, such as by temporarily removing configuration in the gadget
> configfs interface, network activity should continue to be processed
> when the gadget is re-connected. Mirror the netif_device_detach call
> with a netif_device_attach call in gether_connect to fix re-connecting
> gadgets.

This fixes the NCM gadget on the PXA1908 phone Luca mentioned on v6.7-rc7.

Tested-by: Duje Mihanović <[email protected]>

Regards,
Duje





2023-12-29 08:44:34

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] usb: gadget: u_ether: Re-attach netif device to mirror detachment

On Thu, Dec 28, 2023 at 10:59:59PM +0100, Luca Weiss wrote:
> On Montag, 18. Dezember 2023 17:45:33 CET Richard Acayan wrote:
> > In 6.7-rc1, there was a netif_device_detach call added to the
> > gether_disconnect function. This clears the __LINK_STATE_PRESENT bit of
> > the netif device and suppresses pings (ICMP messages) and TCP connection
> > requests from the connected host. If userspace temporarily disconnects
> > the gadget, such as by temporarily removing configuration in the gadget
> > configfs interface, network activity should continue to be processed
> > when the gadget is re-connected. Mirror the netif_device_detach call
> > with a netif_device_attach call in gether_connect to fix re-connecting
> > gadgets.
>
> (+Cc Thorsten Leemhuis)
>
> This appears to fix the regression on a 6.7-rc5-based build for
> qcom-msm8974pro-fairphone-fp2, that the NCM network gadget doesn't work.
> I've also heard reports from qcom-sdm845 and a PXA1908-based phone (if
> I see this correctly) about issues on 6.7.
>
> In postmarketOS on the device side the usb0 interface doesn't get the IP
> address assigned correctly it seems, but it seems to behave a bit
> inconsistently - but always broken.
>
> Anyways, with this patch everything looks good again. I hope this makes
> it for 6.7 final still.

It will have to wait until 6.8-rc1, sorry.

2024-01-07 09:56:38

by Thorsten Leemhuis

[permalink] [raw]
Subject: Re: [PATCH] usb: gadget: u_ether: Re-attach netif device to mirror detachment

On 18.12.23 17:45, Richard Acayan wrote:
> In 6.7-rc1, there was a netif_device_detach call added to the
> gether_disconnect function. This clears the __LINK_STATE_PRESENT bit of
> the netif device and suppresses pings (ICMP messages) and TCP connection
> requests from the connected host. If userspace temporarily disconnects
> the gadget, such as by temporarily removing configuration in the gadget
> configfs interface, network activity should continue to be processed
> when the gadget is re-connected. Mirror the netif_device_detach call
> with a netif_device_attach call in gether_connect to fix re-connecting
> gadgets.
>
> Link: https://gitlab.com/postmarketOS/pmaports/-/tree/6002e51b7090aeeb42947e0ca7ec22278d7227d0/main/postmarketos-base-ui/rootfs-usr-lib-NetworkManager-dispatcher.d-50-tethering.sh
> Fixes: f49449fbc21e ("usb: gadget: u_ether: Replace netif_stop_queue with netif_device_detach")
> Signed-off-by: Richard Acayan <[email protected]>

Thanks for the report. To be sure the issue doesn't fall through the
cracks unnoticed, I'm adding it to regzbot, the Linux kernel regression
tracking bot:

#regzbot ^introduced f49449fbc21e
#regzbot title usb: gadget: u_ether: network gadgets don't work
#regzbot fix: usb: gadget: u_ether: Re-attach netif device to mirror
detachment
#regzbot ignore-activity

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
That page also explains what to do if mails like this annoy you.

2024-01-15 20:12:04

by Ferry Toth

[permalink] [raw]
Subject: Re: [PATCH] usb: gadget: u_ether: Re-attach netif device to mirror detachment

Hi,

Op 18-12-2023 om 17:45 schreef Richard Acayan:
> In 6.7-rc1, there was a netif_device_detach call added to the
> gether_disconnect function. This clears the __LINK_STATE_PRESENT bit of
> the netif device and suppresses pings (ICMP messages) and TCP connection
> requests from the connected host. If userspace temporarily disconnects
> the gadget, such as by temporarily removing configuration in the gadget
> configfs interface, network activity should continue to be processed
> when the gadget is re-connected. Mirror the netif_device_detach call
> with a netif_device_attach call in gether_connect to fix re-connecting
> gadgets.
>
> Link: https://gitlab.com/postmarketOS/pmaports/-/tree/6002e51b7090aeeb42947e0ca7ec22278d7227d0/main/postmarketos-base-ui/rootfs-usr-lib-NetworkManager-dispatcher.d-50-tethering.sh
> Fixes: f49449fbc21e ("usb: gadget: u_ether: Replace netif_stop_queue with netif_device_detach")
> Signed-off-by: Richard Acayan <[email protected]>
> ---
> drivers/usb/gadget/function/u_ether.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c
> index 9d1c40c152d8..3c5a6f6ac341 100644
> --- a/drivers/usb/gadget/function/u_ether.c
> +++ b/drivers/usb/gadget/function/u_ether.c
> @@ -1163,6 +1163,8 @@ struct net_device *gether_connect(struct gether *link)
> if (netif_running(dev->net))
> eth_start(dev, GFP_ATOMIC);
>
> + netif_device_attach(dev->net);
> +
> /* on error, disable any endpoints */
> } else {
> (void) usb_ep_disable(link->out_ep);
This works mrfld (Intel Edison Arduino) using configfs with v6.7.0.
Tested using `iperf3 -s` on mrfld,
iperf3 --bidir -c edison-usb
[ ID][Role] Interval Transfer Bitrate Retr
[ 5][TX-C] 0.00-10.00 sec 130 MBytes 109 Mbits/sec 0
sender
[ 5][TX-C] 0.00-9.99 sec 129 MBytes 108 Mbits/sec
receiver
[ 7][RX-C] 0.00-10.00 sec 167 MBytes 140 Mbits/sec 0
sender
[ 7][RX-C] 0.00-9.99 sec 166 MBytes 139 Mbits/sec
receiver

and
iperf3 -c edison-usb
[ 5] 0.00-10.00 sec 247 MBytes 207 Mbits/sec 0 sender
[ 5] 0.00-9.99 sec 246 MBytes 206 Mbits/sec
receiver


Tested-by: Ferry Toth <[email protected]> [mrfld]

2024-01-21 14:46:09

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH] usb: gadget: u_ether: Re-attach netif device to mirror detachment

On Sun, Jan 21, 2024 at 03:23:42PM +0200, Andy Shevchenko wrote:
> On Mon, Dec 18, 2023 at 11:45:33AM -0500, Richard Acayan wrote:
> > In 6.7-rc1, there was a netif_device_detach call added to the
> > gether_disconnect function. This clears the __LINK_STATE_PRESENT bit of
> > the netif device and suppresses pings (ICMP messages) and TCP connection
> > requests from the connected host. If userspace temporarily disconnects
> > the gadget, such as by temporarily removing configuration in the gadget
> > configfs interface, network activity should continue to be processed
> > when the gadget is re-connected. Mirror the netif_device_detach call
> > with a netif_device_attach call in gether_connect to fix re-connecting
> > gadgets.
>
> Tested-by: From: Andy Shevchenko <[email protected]> # Intel Merrifield

Sorry, now correct tag.

Tested-by: Andy Shevchenko <[email protected]> # Intel Merrifield

--
With Best Regards,
Andy Shevchenko



2024-01-21 17:00:17

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH] usb: gadget: u_ether: Re-attach netif device to mirror detachment

On Mon, Dec 18, 2023 at 11:45:33AM -0500, Richard Acayan wrote:
> In 6.7-rc1, there was a netif_device_detach call added to the
> gether_disconnect function. This clears the __LINK_STATE_PRESENT bit of
> the netif device and suppresses pings (ICMP messages) and TCP connection
> requests from the connected host. If userspace temporarily disconnects
> the gadget, such as by temporarily removing configuration in the gadget
> configfs interface, network activity should continue to be processed
> when the gadget is re-connected. Mirror the netif_device_detach call
> with a netif_device_attach call in gether_connect to fix re-connecting
> gadgets.

Tested-by: From: Andy Shevchenko <[email protected]> # Intel Merrifield

--
With Best Regards,
Andy Shevchenko