| Message ID | 20231218164532.411125-2-mailingradian@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 76c945730cdffb572c7767073cc6515fd3f646b4 |
| Headers | show |
| Series | usb: gadget: u_ether: Re-attach netif device to mirror detachment | expand |
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
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);
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(+)