Re: [PATCH v4.4,v4.9,v4.14 1/2] usbnet: ipheth: prevent TX queue timeouts when device not ready

[Sasha Levin <sashal@kernel.org>]

On Tue, Apr 30, 2019 at 04:53:29PM -0700, Guenter Roeck wrote:
>From: Alexander Kappner <agk@godking.net>
>
>commit bb1b40c7cb863f0800a6410c7dcb86cf3f28d3b1 upstream.
>
>iOS devices require the host to be "trusted" before servicing network
>packets. Establishing trust requires the user to confirm a dialog on the
>iOS device.Until trust is established, the iOS device will silently discard
>network packets from the host. Currently, the ipheth driver does not detect
>whether an iOS device has established trust with the host, and immediately
>sets up the transmit queues.
>
>This causes the following problems:
>
>- Kernel taint due to WARN() in netdev watchdog.
>- Dmesg spam ("TX timeout").
>- Disruption of user space networking activity (dhcpd, etc...) when new
>interface comes up but cannot be used.
>- Unnecessary host and device wakeups and USB traffic
>
>Example dmesg output:
>
>[ 1101.319778] NETDEV WATCHDOG: eth1 (ipheth): transmit queue 0 timed out
>[ 1101.319817] ------------[ cut here ]------------
>[ 1101.319828] WARNING: CPU: 0 PID: 0 at net/sched/sch_generic.c:316 dev_watchdog+0x20f/0x220
>[ 1101.319831] Modules linked in: ipheth usbmon nvidia_drm(PO) nvidia_modeset(PO) nvidia(PO) iwlmvm mac80211 iwlwifi btusb btrtl btbcm btintel qmi_wwan bluetooth cfg80211 ecdh_generic thinkpad_acpi rfkill [last unloaded: ipheth]
>[ 1101.319861] CPU: 0 PID: 0 Comm: swapper/0 Tainted: P           O    4.13.12.1 #1
>[ 1101.319864] Hardware name: LENOVO 20ENCTO1WW/20ENCTO1WW, BIOS N1EET62W (1.35 ) 11/10/2016
>[ 1101.319867] task: ffffffff81e11500 task.stack: ffffffff81e00000
>[ 1101.319873] RIP: 0010:dev_watchdog+0x20f/0x220
>[ 1101.319876] RSP: 0018:ffff8810a3c03e98 EFLAGS: 00010292
>[ 1101.319880] RAX: 000000000000003a RBX: 0000000000000000 RCX: 0000000000000000
>[ 1101.319883] RDX: ffff8810a3c15c48 RSI: ffffffff81ccbfc2 RDI: 00000000ffffffff
>[ 1101.319886] RBP: ffff880c04ebc41c R08: 0000000000000000 R09: 0000000000000379
>[ 1101.319889] R10: 00000100696589d0 R11: 0000000000000378 R12: ffff880c04ebc000
>[ 1101.319892] R13: 0000000000000000 R14: 0000000000000001 R15: ffff880c2865fc80
>[ 1101.319896] FS:  0000000000000000(0000) GS:ffff8810a3c00000(0000) knlGS:0000000000000000
>[ 1101.319899] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>[ 1101.319902] CR2: 00007f3ff24ac000 CR3: 0000000001e0a000 CR4: 00000000003406f0
>[ 1101.319905] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>[ 1101.319908] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>[ 1101.319910] Call Trace:
>[ 1101.319914]  <IRQ>
>[ 1101.319921]  ? dev_graft_qdisc+0x70/0x70
>[ 1101.319928]  ? dev_graft_qdisc+0x70/0x70
>[ 1101.319934]  ? call_timer_fn+0x2e/0x170
>[ 1101.319939]  ? dev_graft_qdisc+0x70/0x70
>[ 1101.319944]  ? run_timer_softirq+0x1ea/0x440
>[ 1101.319951]  ? timerqueue_add+0x54/0x80
>[ 1101.319956]  ? enqueue_hrtimer+0x38/0xa0
>[ 1101.319963]  ? __do_softirq+0xed/0x2e7
>[ 1101.319970]  ? irq_exit+0xb4/0xc0
>[ 1101.319976]  ? smp_apic_timer_interrupt+0x39/0x50
>[ 1101.319981]  ? apic_timer_interrupt+0x8c/0xa0
>[ 1101.319983]  </IRQ>
>[ 1101.319992]  ? cpuidle_enter_state+0xfa/0x2a0
>[ 1101.319999]  ? do_idle+0x1a3/0x1f0
>[ 1101.320004]  ? cpu_startup_entry+0x5f/0x70
>[ 1101.320011]  ? start_kernel+0x444/0x44c
>[ 1101.320017]  ? early_idt_handler_array+0x120/0x120
>[ 1101.320023]  ? x86_64_start_kernel+0x145/0x154
>[ 1101.320028]  ? secondary_startup_64+0x9f/0x9f
>[ 1101.320033] Code: 20 04 00 00 eb 9f 4c 89 e7 c6 05 59 44 71 00 01 e8 a7 df fd ff 89 d9 4c 89 e6 48 c7 c7 70 b7 cd 81 48 89 c2 31 c0 e8 97 64 90 ff <0f> ff eb bf 66 66 66 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00
>[ 1101.320103] ---[ end trace 0cc4d251e2b57080 ]---
>[ 1101.320110] ipheth 1-5:4.2: ipheth_tx_timeout: TX timeout
>
>The last message "TX timeout" is repeated every 5 seconds until trust is
>established or the device is disconnected, filling up dmesg.
>
>The proposed patch eliminates the problem by, upon connection, keeping the
>TX queue and carrier disabled until a packet is first received from the iOS
>device. This is reflected by the confirmed_pairing variable in the device
>structure. Only after at least one packet has been received from the iOS
>device, the transmit queue and carrier are brought up during the periodic
>device poll in ipheth_carrier_set. Because the iOS device will always send
>a packet immediately upon trust being established, this should not delay
>the interface becoming useable. To prevent failed UBRs in
>ipheth_rcvbulk_callback from perpetually re-enabling the queue if it was
>disabled, a new check is added so only successful transfers re-enable the
>queue, whereas failed transfers only trigger an immediate poll.
>
>This has the added benefit of removing the periodic control requests to the
>iOS device until trust has been established and thus should reduce wakeup
>events on both the host and the iOS device.
>
>Signed-off-by: Alexander Kappner <agk@godking.net>
>Signed-off-by: David S. Miller <davem@davemloft.net>
>[groeck: Fixed context conflict seen because 45611c61dd50 was applied first]
>Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Greg queued it up, thanks :)

--
Thanks,
Sasha