From: Ferry Toth <fntoth@gmail.com>
To: Ferry Toth <fntoth@gmail.com>,
Hardik Gajjar <hgajjar@de.adit-jv.com>,
Andy Shevchenko <andriy.shevchenko@intel.com>
Cc: gregkh@linuxfoundation.org, s.hauer@pengutronix.de,
jonathanh@nvidia.com, linux-usb@vger.kernel.org,
linux-kernel@vger.kernel.org, quic_linyyuan@quicinc.com,
paul@crapouillou.net, quic_eserrao@quicinc.com,
erosca@de.adit-jv.com
Subject: Re: [PATCH v4] usb: gadget: u_ether: Replace netif_stop_queue with netif_device_detach
Date: Wed, 3 Apr 2024 23:01:58 +0200 [thread overview]
Message-ID: <aeee83d8-dee3-42ed-b705-988b17800721@gmail.com> (raw)
In-Reply-To: <f25283fc-4550-4725-960b-2ea783fd62e1@gmail.com>
Hi,
Op 15-01-2024 om 21:10 schreef Ferry Toth:
> Hi,
>
> Op 15-01-2024 om 14:27 schreef Hardik Gajjar:
>> On Sun, Jan 14, 2024 at 06:59:19PM +0200, Andy Shevchenko wrote:
>>> +Cc: Ferry.
>>>
>>> On Fri, Oct 06, 2023 at 05:56:46PM +0200, Hardik Gajjar wrote:
>>>> This patch replaces the usage of netif_stop_queue with
>>>> netif_device_detach
>>>> in the u_ether driver. The netif_device_detach function not only
>>>> stops all
>>>> tx queues by calling netif_tx_stop_all_queues but also marks the
>>>> device as
>>>> removed by clearing the __LINK_STATE_PRESENT bit.
>>>>
>>>> This change helps notify user space about the disconnection of the
>>>> device
>>>> more effectively, compared to netif_stop_queue, which only stops a
>>>> single
>>>> transmit queue.
>>>
>>> This change effectively broke my USB ether setup.
>>>
>>> git bisect start
>>> # status: waiting for both good and bad commits
>>> # good: [1f24458a1071f006e3f7449c08ae0f12af493923] Merge tag
>>> 'tty-6.7-rc1' of
>>> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty
>>> git bisect good 1f24458a1071f006e3f7449c08ae0f12af493923
>>> # status: waiting for bad commit, 1 good commit known
>>> # bad: [2c40c1c6adab90ee4660caf03722b3a3ec67767b] Merge tag
>>> 'usb-6.7-rc1' of
>>> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb
>>> git bisect bad 2c40c1c6adab90ee4660caf03722b3a3ec67767b
>>> # bad: [17d6b82d2d6d467149874b883cdba844844b996d] usb/usbip: fix
>>> wrong data added to platform device
>>> git bisect bad 17d6b82d2d6d467149874b883cdba844844b996d
>>> # good: [ba6b83a910b6d8a9379bda55cbf06cb945473a96] usb: xhci-mtk:
>>> add a bandwidth budget table
>>> git bisect good ba6b83a910b6d8a9379bda55cbf06cb945473a96
>>> # good: [dddc00f255415b826190cfbaa5d6dbc87cd9ded1] Revert "usb:
>>> gadget: uvc: cleanup request when not in correct state"
>>> git bisect good dddc00f255415b826190cfbaa5d6dbc87cd9ded1
>>> # bad: [8f999ce60ea3d47886b042ef1f22bb184b6e9c59] USB: typec:
>>> tps6598x: Refactor tps6598x port registration
>>> git bisect bad 8f999ce60ea3d47886b042ef1f22bb184b6e9c59
>>> # bad: [f49449fbc21e7e9550a5203902d69c8ae7dfd918] usb: gadget:
>>> u_ether: Replace netif_stop_queue with netif_device_detach
>>> git bisect bad f49449fbc21e7e9550a5203902d69c8ae7dfd918
>>> # good: [97475763484245916735a1aa9a3310a01d46b008] USB: usbip: fix
>>> stub_dev hub disconnect
>>> git bisect good 97475763484245916735a1aa9a3310a01d46b008
>>> # good: [0f5aa1b01263b8b621bc4f031a1f2983ef8517b7] usb: usbtest: fix
>>> a type promotion bug
>>> git bisect good 0f5aa1b01263b8b621bc4f031a1f2983ef8517b7
>>> # first bad commit: [f49449fbc21e7e9550a5203902d69c8ae7dfd918] usb:
>>> gadget: u_ether: Replace netif_stop_queue with netif_device_detach
>>>
>>> Note, revert indeed helps. Should I send a revert?
>>>
>>> I use configfs to setup USB EEM function and it worked till this
>>> commit.
>>> If needed, I can share my scripts, but I believe it's not needed as
>>> here
>>> we see a clear regression.
>>>
>>> --
>>> With Best Regards,
>>> Andy Shevchenko
>>>
>>>
>>
>> Without this patch, there may be a potential crash in a race
>> condition, as __LINK_STATE_PRESENT is monitored at many places in the
>> Network stack to determine the status of the link.
>>
>> Could you please provide details on how this patch affects your
>> functionality? Are you experiencing connection problems or data
>> transfer interruptions?
>
> In my case on mrfld (Intel Edison Arduino) using configfs with this
> patch no config from host through dhcp is received. Manual setting
> correct ipv4 addr / mask / gw still no connection.
>
>> Instead of reverting this patch, consider trying the upcoming patch
>> (soon to be available in the mainline) to see if it resolves your issue.
>>
>> https://lore.kernel.org/lkml/2023122900-commence-agenda-db2c@gregkh/T/#m36a812d3f1e5d744ee32381f6ae4185940b376de
>>
>
> This patch works for me with v6.7.0.
I need to revisit this. The patch in this topic landed in v6.7.0-rc1
(f49449fbc21e) and breaks the gadget mrfld (Intel Edison Arduino) and
other platforms as well.
The mentioned fix "usb: gadget: u_ether: Re-attach netif device to
mirror detachment*" * has landed in v6.8.0-rc1 (76c945730). What it does
fix: I am able to make a USB EEM function again.
However, now a hidden issue appears. With mrfld there is an external
switch to easily switch between host and device mode.
What is not fixed:
- when in device mode and unplugging/plugging the cable when using
`ifconfig usb0` the line "usb0:
flags=4163<UP,BROADCAST,RUNNING,MULTICAST>" changes to "usb0:
flags=4099<UP,BROADCAST,MULTICAST>" as is supposed to, the route table
is updated and the dir `/sys/class/net/usb0` exists and in the dir `cat
carrier*` shows the carrier up and down counts. This is the expected
behavior.
- when in device mode and switching to host mode `ifconfig usb0`
continues to show "RUNNING", the route table is not modified and the dir
`/sys/class/net/usb0` no longer exists.
- switching to device mode again, USB EEM works fine, no changes to
RUNNING or the route table happen and the dir `/sys/class/net/usb0`
still is non- existing.
- unplugging/plugging the cable in device mode after this does not
restore the original situation.
This behavior I tested on v6.9.0-rc2 (with a few unrelated but essential
patches on top) and bisected back to this patch in v6.70-rc1.
It seems `netif_device_detach` does not completely clean up as expected
and `netif_device_attach` does not completely rebuild.
I am wondering if on other platforms this can be reproduced? If so, inho
it would be best to revert the both patches until the issue is resolved.
Thanks,
Ferry
>> Thanks,
>> Hardik
>
next prev parent reply other threads:[~2024-04-03 21:02 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-06 14:12 [PATCH] usb: gadget: u_ether: Replace netif_stop_queue with netif_device_detach Hardik Gajjar
2023-10-06 14:21 ` Greg KH
2023-10-06 14:50 ` Hardik Gajjar
2023-10-06 14:53 ` [PATCH v2] " Hardik Gajjar
2023-10-06 14:59 ` Greg KH
2023-10-06 15:38 ` [PATCH v3] " Hardik Gajjar
2023-10-06 15:56 ` [PATCH v4] " Hardik Gajjar
2024-01-14 16:59 ` Andy Shevchenko
2024-01-15 13:27 ` Hardik Gajjar
2024-01-15 20:10 ` Ferry Toth
2024-04-03 21:01 ` Ferry Toth [this message]
2024-04-05 11:38 ` Hardik Gajjar
2024-04-07 20:51 ` Ferry Toth
2024-04-10 17:37 ` Andy Shevchenko
2024-04-11 14:26 ` Hardik Gajjar
2024-04-11 16:39 ` Andy Shevchenko
2024-04-11 20:52 ` Ferry Toth
2024-04-16 13:48 ` Andy Shevchenko
2024-04-17 15:13 ` Hardik Gajjar
2024-04-25 21:27 ` Ferry Toth
2024-04-28 21:07 ` Ferry Toth
2024-04-30 15:32 ` Hardik Gajjar
2024-04-30 19:40 ` Ferry Toth
2024-04-30 21:12 ` Ferry Toth
2024-05-02 15:29 ` Hardik Gajjar
2024-05-02 15:31 ` Andy Shevchenko
2024-05-02 16:16 ` Hardik Gajjar
2024-05-03 7:24 ` Linux regression tracking (Thorsten Leemhuis)
2024-05-03 9:15 ` Hardik Gajjar
2024-05-03 12:39 ` Linux regression tracking (Thorsten Leemhuis)
2024-05-02 20:13 ` Ferry Toth
2024-05-02 20:32 ` Ferry Toth
2024-05-10 9:45 ` Hardik Gajjar
2024-05-15 18:38 ` Ferry Toth
2024-05-26 20:52 ` Ferry Toth
2024-05-27 6:29 ` Linux regression tracking (Thorsten Leemhuis)
2024-05-28 7:18 ` Hardik Gajjar
2024-05-29 9:23 ` Andy Shevchenko
2024-05-30 19:06 ` Ferry Toth
2024-06-06 7:58 ` Thorsten Leemhuis
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aeee83d8-dee3-42ed-b705-988b17800721@gmail.com \
--to=fntoth@gmail.com \
--cc=andriy.shevchenko@intel.com \
--cc=erosca@de.adit-jv.com \
--cc=gregkh@linuxfoundation.org \
--cc=hgajjar@de.adit-jv.com \
--cc=jonathanh@nvidia.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=paul@crapouillou.net \
--cc=quic_eserrao@quicinc.com \
--cc=quic_linyyuan@quicinc.com \
--cc=s.hauer@pengutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).