| Message ID | 20210120011208.3768105-1-grundler@chromium.org |
|---|---|
| State | New |
| Headers | show |
| Series | [net] net: usb: cdc_ncm: don't spew notifications | expand |
On Wed, 20 Jan 2021 03:38:32 +0000 Hayes Wang wrote: > Grant Grundler <grundler@chromium.org> > > Sent: Wednesday, January 20, 2021 9:12 AM > > Subject: [PATCH net] net: usb: cdc_ncm: don't spew notifications > > > > RTL8156 sends notifications about every 32ms. > > Only display/log notifications when something changes. > > > > This issue has been reported by others: > > https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1832472 > > https://lkml.org/lkml/2020/8/27/1083 > > > > Chrome OS cannot support RTL8156 until this is fixed. > > > > Signed-off-by: Grant Grundler <grundler@chromium.org> > > Reviewed-by: Hayes Wang <hayeswang@realtek.com> Applied, thanks! net should be merged back into net-next by the end of the day, so if the other patches depend on this one to apply cleanly please keep an eye and post after that happens. If there is no conflict you can just post them with [PATCH net-next] now.
On Wed, Jan 20, 2021 at 5:04 PM Jakub Kicinski <kuba@kernel.org> wrote: > > On Wed, 20 Jan 2021 03:38:32 +0000 Hayes Wang wrote: > > Grant Grundler <grundler@chromium.org> > > > Sent: Wednesday, January 20, 2021 9:12 AM > > > Subject: [PATCH net] net: usb: cdc_ncm: don't spew notifications > > > > > > RTL8156 sends notifications about every 32ms. > > > Only display/log notifications when something changes. > > > > > > This issue has been reported by others: > > > https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1832472 > > > https://lkml.org/lkml/2020/8/27/1083 > > > > > > Chrome OS cannot support RTL8156 until this is fixed. > > > > > > Signed-off-by: Grant Grundler <grundler@chromium.org> > > > > Reviewed-by: Hayes Wang <hayeswang@realtek.com> > > Applied, thanks! > > net should be merged back into net-next by the end of the day, so > if the other patches depend on this one to apply cleanly please keep > an eye and post after that happens. If there is no conflict you can > just post them with [PATCH net-next] now. Jakub, sorry, I "dropped the ball" on this one. I'll repost with "net-next" a bit later today. cheers, grant
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c index 25498c311551..5de096545b86 100644 --- a/drivers/net/usb/cdc_ncm.c +++ b/drivers/net/usb/cdc_ncm.c @@ -1827,6 +1827,15 @@ cdc_ncm_speed_change(struct usbnet *dev, uint32_t rx_speed = le32_to_cpu(data->DLBitRRate); uint32_t tx_speed = le32_to_cpu(data->ULBitRate); + /* if the speed hasn't changed, don't report it. + * RTL8156 shipped before 2021 sends notification about every 32ms. + */ + if (dev->rx_speed == rx_speed && dev->tx_speed == tx_speed) + return; + + dev->rx_speed = rx_speed; + dev->tx_speed = tx_speed; + /* * Currently the USB-NET API does not support reporting the actual * device speed. Do print it instead. @@ -1867,7 +1876,8 @@ static void cdc_ncm_status(struct usbnet *dev, struct urb *urb) * USB_CDC_NOTIFY_NETWORK_CONNECTION notification shall be * sent by device after USB_CDC_NOTIFY_SPEED_CHANGE. */ - usbnet_link_change(dev, !!event->wValue, 0); + if (netif_carrier_ok(dev->net) != !!event->wValue) + usbnet_link_change(dev, !!event->wValue, 0); break; case USB_CDC_NOTIFY_SPEED_CHANGE: diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h index 88a7673894d5..cfbfd6fe01df 100644 --- a/include/linux/usb/usbnet.h +++ b/include/linux/usb/usbnet.h @@ -81,6 +81,8 @@ struct usbnet { # define EVENT_LINK_CHANGE 11 # define EVENT_SET_RX_MODE 12 # define EVENT_NO_IP_ALIGN 13 + u32 rx_speed; /* in bps - NOT Mbps */ + u32 tx_speed; /* in bps - NOT Mbps */ }; static inline struct usb_driver *driver_of(struct usb_interface *intf)
RTL8156 sends notifications about every 32ms. Only display/log notifications when something changes. This issue has been reported by others: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1832472 https://lkml.org/lkml/2020/8/27/1083 ... [785962.779840] usb 1-1: new high-speed USB device number 5 using xhci_hcd [785962.929944] usb 1-1: New USB device found, idVendor=0bda, idProduct=8156, bcdDevice=30.00 [785962.929949] usb 1-1: New USB device strings: Mfr=1, Product=2, SerialNumber=6 [785962.929952] usb 1-1: Product: USB 10/100/1G/2.5G LAN [785962.929954] usb 1-1: Manufacturer: Realtek [785962.929956] usb 1-1: SerialNumber: 000000001 [785962.991755] usbcore: registered new interface driver cdc_ether [785963.017068] cdc_ncm 1-1:2.0: MAC-Address: 00:24:27:88:08:15 [785963.017072] cdc_ncm 1-1:2.0: setting rx_max = 16384 [785963.017169] cdc_ncm 1-1:2.0: setting tx_max = 16384 [785963.017682] cdc_ncm 1-1:2.0 usb0: register 'cdc_ncm' at usb-0000:00:14.0-1, CDC NCM, 00:24:27:88:08:15 [785963.019211] usbcore: registered new interface driver cdc_ncm [785963.023856] usbcore: registered new interface driver cdc_wdm [785963.025461] usbcore: registered new interface driver cdc_mbim [785963.038824] cdc_ncm 1-1:2.0 enx002427880815: renamed from usb0 [785963.089586] cdc_ncm 1-1:2.0 enx002427880815: network connection: disconnected [785963.121673] cdc_ncm 1-1:2.0 enx002427880815: network connection: disconnected [785963.153682] cdc_ncm 1-1:2.0 enx002427880815: network connection: disconnected ... This is about 2KB per second and will overwrite all contents of a 1MB dmesg buffer in under 10 minutes rendering them useless for debugging many kernel problems. This is also an extra 180 MB/day in /var/logs (or 1GB per week) rendering the majority of those logs useless too. When the link is up (expected state), spew amount is >2x higher: ... [786139.600992] cdc_ncm 2-1:2.0 enx002427880815: network connection: connected [786139.632997] cdc_ncm 2-1:2.0 enx002427880815: 2500 mbit/s downlink 2500 mbit/s uplink [786139.665097] cdc_ncm 2-1:2.0 enx002427880815: network connection: connected [786139.697100] cdc_ncm 2-1:2.0 enx002427880815: 2500 mbit/s downlink 2500 mbit/s uplink [786139.729094] cdc_ncm 2-1:2.0 enx002427880815: network connection: connected [786139.761108] cdc_ncm 2-1:2.0 enx002427880815: 2500 mbit/s downlink 2500 mbit/s uplink ... Chrome OS cannot support RTL8156 until this is fixed. Signed-off-by: Grant Grundler <grundler@chromium.org> --- drivers/net/usb/cdc_ncm.c | 12 +++++++++++- include/linux/usb/usbnet.h | 2 ++ 2 files changed, 13 insertions(+), 1 deletion(-)