linux-kernel - Re: [PATCH v4] usb: gadget: u_ether: Replace netif_stop_queue with netif_device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f25283fc-4550-4725-960b-2ea783fd62e1@gmail.com>
Date: Mon, 15 Jan 2024 21:10:58 +0100
From: Ferry Toth <fntoth@...il.com>
To: Hardik Gajjar <hgajjar@...adit-jv.com>,
 Andy Shevchenko <andriy.shevchenko@...el.com>
Cc: Ferry Toth <ftoth@...londelft.nl>, gregkh@...uxfoundation.org,
 s.hauer@...gutronix.de, jonathanh@...dia.com, linux-usb@...r.kernel.org,
 linux-kernel@...r.kernel.org, quic_linyyuan@...cinc.com,
 paul@...pouillou.net, quic_eserrao@...cinc.com, erosca@...adit-jv.com
Subject: Re: [PATCH v4] usb: gadget: u_ether: Replace netif_stop_queue with
 netif_device_detach

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.

> Thanks,
> Hardik