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 <mailingradian@gmail.com>
---
 drivers/usb/gadget/function/u_ether.c | 2 ++
 1 file changed, 2 insertions(+)
  

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 <luca@z3ntu.xyz>


> 
> 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 <mailingradian@gmail.com>
> ---
>  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);
  

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ć <duje.mihanovic@skole.hr>

Regards,
Duje
  

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.
  

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 <mailingradian@gmail.com>

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.
  

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 <mailingradian@gmail.com>
> ---
>   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 <fntoth@gmail.com> [mrfld]
  

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 <andriy.shevchenko@linux.intel.com> # Intel Merrifield
  

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 <andriy.shevchenko@linux.intel.com> # Intel Merrifield

Sorry, now correct tag.

Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> # Intel Merrifield