* [PATCH 1/3] net: usb: cdc_ncm: emit dev_err on error paths
@ 2021-01-16 5:26 Grant Grundler
2021-01-16 5:26 ` [PATCH 2/3] net: usb: log errors to dmesg/syslog Grant Grundler
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Grant Grundler @ 2021-01-16 5:26 UTC (permalink / raw)
To: Oliver Neukum
Cc: David S. Miller, Jakub Kicinski, Greg Kroah-Hartman, linux-usb,
netdev, linux-kernel, Grant Grundler
Several error paths in bind/probe code will only emit
output using dev_dbg. But if we are going to fail the
bind/probe, emit related output with "err" priority.
Signed-off-by: Grant Grundler <grundler@chromium.org>
---
drivers/net/usb/cdc_ncm.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 5a78848db93f..25498c311551 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -849,17 +849,17 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
/* check if we got everything */
if (!ctx->data) {
- dev_dbg(&intf->dev, "CDC Union missing and no IAD found\n");
+ dev_err(&intf->dev, "CDC Union missing and no IAD found\n");
goto error;
}
if (cdc_ncm_comm_intf_is_mbim(intf->cur_altsetting)) {
if (!ctx->mbim_desc) {
- dev_dbg(&intf->dev, "MBIM functional descriptor missing\n");
+ dev_err(&intf->dev, "MBIM functional descriptor missing\n");
goto error;
}
} else {
if (!ctx->ether_desc || !ctx->func_desc) {
- dev_dbg(&intf->dev, "NCM or ECM functional descriptors missing\n");
+ dev_err(&intf->dev, "NCM or ECM functional descriptors missing\n");
goto error;
}
}
@@ -868,7 +868,7 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
if (ctx->data != ctx->control) {
temp = usb_driver_claim_interface(driver, ctx->data, dev);
if (temp) {
- dev_dbg(&intf->dev, "failed to claim data intf\n");
+ dev_err(&intf->dev, "failed to claim data intf\n");
goto error;
}
}
@@ -924,7 +924,7 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
if (ctx->ether_desc) {
temp = usbnet_get_ethernet_addr(dev, ctx->ether_desc->iMACAddress);
if (temp) {
- dev_dbg(&intf->dev, "failed to get mac address\n");
+ dev_err(&intf->dev, "failed to get mac address\n");
goto error2;
}
dev_info(&intf->dev, "MAC-Address: %pM\n", dev->net->dev_addr);
--
2.29.2
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 2/3] net: usb: log errors to dmesg/syslog
2021-01-16 5:26 [PATCH 1/3] net: usb: cdc_ncm: emit dev_err on error paths Grant Grundler
@ 2021-01-16 5:26 ` Grant Grundler
2021-01-18 22:02 ` Grant Grundler
2021-01-16 5:26 ` [PATCH 3/3] net: usb: cdc_ncm: don't spew notifications Grant Grundler
2021-01-18 22:01 ` [PATCH 1/3] net: usb: cdc_ncm: emit dev_err on error paths Grant Grundler
2 siblings, 1 reply; 12+ messages in thread
From: Grant Grundler @ 2021-01-16 5:26 UTC (permalink / raw)
To: Oliver Neukum
Cc: David S. Miller, Jakub Kicinski, Greg Kroah-Hartman, linux-usb,
netdev, linux-kernel, Grant Grundler
Errors in protocol should be logged when the driver aborts operations.
If the driver can carry on and "humor" the device, then emitting
the message as debug output level is fine.
Signed-off-by: Grant Grundler <grundler@chromium.org>
---
drivers/net/usb/usbnet.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 1447da1d5729..bc7b93399bd5 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -887,7 +887,7 @@ int usbnet_open (struct net_device *net)
// insist peer be connected
if (info->check_connect && (retval = info->check_connect (dev)) < 0) {
- netif_dbg(dev, ifup, dev->net, "can't open; %d\n", retval);
+ netif_err(dev, ifup, dev->net, "can't open; %d\n", retval);
goto done;
}
--
2.29.2
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 2/3] net: usb: log errors to dmesg/syslog
2021-01-16 5:26 ` [PATCH 2/3] net: usb: log errors to dmesg/syslog Grant Grundler
@ 2021-01-18 22:02 ` Grant Grundler
0 siblings, 0 replies; 12+ messages in thread
From: Grant Grundler @ 2021-01-18 22:02 UTC (permalink / raw)
To: Grant Grundler, nic_swsd
Cc: Oliver Neukum, David S. Miller, Jakub Kicinski,
Greg Kroah-Hartman, linux-usb, netdev, LKML
+nic_swsd [adding per Realtek developer team request]
On Sat, Jan 16, 2021 at 5:27 AM Grant Grundler <grundler@chromium.org> wrote:
>
> Errors in protocol should be logged when the driver aborts operations.
> If the driver can carry on and "humor" the device, then emitting
> the message as debug output level is fine.
>
> Signed-off-by: Grant Grundler <grundler@chromium.org>
> ---
> drivers/net/usb/usbnet.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
> index 1447da1d5729..bc7b93399bd5 100644
> --- a/drivers/net/usb/usbnet.c
> +++ b/drivers/net/usb/usbnet.c
> @@ -887,7 +887,7 @@ int usbnet_open (struct net_device *net)
>
> // insist peer be connected
> if (info->check_connect && (retval = info->check_connect (dev)) < 0) {
> - netif_dbg(dev, ifup, dev->net, "can't open; %d\n", retval);
> + netif_err(dev, ifup, dev->net, "can't open; %d\n", retval);
> goto done;
> }
>
> --
> 2.29.2
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/3] net: usb: cdc_ncm: don't spew notifications
2021-01-16 5:26 [PATCH 1/3] net: usb: cdc_ncm: emit dev_err on error paths Grant Grundler
2021-01-16 5:26 ` [PATCH 2/3] net: usb: log errors to dmesg/syslog Grant Grundler
@ 2021-01-16 5:26 ` Grant Grundler
2021-01-18 22:02 ` Grant Grundler
2021-01-19 21:45 ` Jakub Kicinski
2021-01-18 22:01 ` [PATCH 1/3] net: usb: cdc_ncm: emit dev_err on error paths Grant Grundler
2 siblings, 2 replies; 12+ messages in thread
From: Grant Grundler @ 2021-01-16 5:26 UTC (permalink / raw)
To: Oliver Neukum
Cc: David S. Miller, Jakub Kicinski, Greg Kroah-Hartman, linux-usb,
netdev, linux-kernel, Grant Grundler
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(-)
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)
--
2.29.2
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 3/3] net: usb: cdc_ncm: don't spew notifications
2021-01-16 5:26 ` [PATCH 3/3] net: usb: cdc_ncm: don't spew notifications Grant Grundler
@ 2021-01-18 22:02 ` Grant Grundler
2021-01-19 1:23 ` Hayes Wang
2021-01-19 21:45 ` Jakub Kicinski
1 sibling, 1 reply; 12+ messages in thread
From: Grant Grundler @ 2021-01-18 22:02 UTC (permalink / raw)
To: Grant Grundler, nic_swsd
Cc: Oliver Neukum, David S. Miller, Jakub Kicinski,
Greg Kroah-Hartman, linux-usb, netdev, LKML
+nic_swsd [adding per Realtek developer team request]
On Sat, Jan 16, 2021 at 5:27 AM Grant Grundler <grundler@chromium.org> wrote:
>
> 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(-)
>
> 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)
> --
> 2.29.2
>
^ permalink raw reply [flat|nested] 12+ messages in thread* RE: [PATCH 3/3] net: usb: cdc_ncm: don't spew notifications
2021-01-18 22:02 ` Grant Grundler
@ 2021-01-19 1:23 ` Hayes Wang
0 siblings, 0 replies; 12+ messages in thread
From: Hayes Wang @ 2021-01-19 1:23 UTC (permalink / raw)
To: Grant Grundler, nic_swsd
Cc: Oliver Neukum, David S. Miller, Jakub Kicinski,
Greg Kroah-Hartman, linux-usb, netdev, LKML
Grant Grundler <grundler@chromium.org>
> Sent: Tuesday, January 19, 2021 6:03 AM
> Subject: Re: [PATCH 3/3] 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
> >
> > ...
> > [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>
Reviewed-by: Hayes Wang <hayeswang@realtek.com>
Best Regards,
Hayes
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] net: usb: cdc_ncm: don't spew notifications
2021-01-16 5:26 ` [PATCH 3/3] net: usb: cdc_ncm: don't spew notifications Grant Grundler
2021-01-18 22:02 ` Grant Grundler
@ 2021-01-19 21:45 ` Jakub Kicinski
2021-01-20 0:55 ` Grant Grundler
1 sibling, 1 reply; 12+ messages in thread
From: Jakub Kicinski @ 2021-01-19 21:45 UTC (permalink / raw)
To: Grant Grundler
Cc: Oliver Neukum, David S. Miller, Greg Kroah-Hartman, linux-usb,
netdev, linux-kernel
On Fri, 15 Jan 2021 21:26:23 -0800 Grant Grundler wrote:
> 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.
> @@ -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:
Thanks for the patch, this looks like an improvement over:
59b4a8fa27f5 ("CDC-NCM: remove "connected" log message")
right? Should we bring the "network connection: connected" message back?
Do you want all of these patches to be applied to 5.11 and backported?
Feels to me like the last one is a fix and the rest can go into -next,
WDYT?
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 3/3] net: usb: cdc_ncm: don't spew notifications
2021-01-19 21:45 ` Jakub Kicinski
@ 2021-01-20 0:55 ` Grant Grundler
2021-01-20 0:59 ` Grant Grundler
0 siblings, 1 reply; 12+ messages in thread
From: Grant Grundler @ 2021-01-20 0:55 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Grant Grundler, Oliver Neukum, David S. Miller,
Greg Kroah-Hartman, linux-usb, netdev, LKML
On Tue, Jan 19, 2021 at 9:46 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Fri, 15 Jan 2021 21:26:23 -0800 Grant Grundler wrote:
> > 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.
>
> > @@ -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:
>
> Thanks for the patch, this looks like an improvement over:
>
> 59b4a8fa27f5 ("CDC-NCM: remove "connected" log message")
>
> right? Should we bring the "network connection: connected" message back?
Yes, we can revert Roland's patch. I didn't see that one.
> Do you want all of these patches to be applied to 5.11 and backported?
Yes to 5.11. Only the 3rd one really needs to be applied to stable kernels.
> Feels to me like the last one is a fix and the rest can go into -next,
> WDYT?
Exactly.
Thanks!
grant
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 3/3] net: usb: cdc_ncm: don't spew notifications
2021-01-20 0:55 ` Grant Grundler
@ 2021-01-20 0:59 ` Grant Grundler
2021-01-20 1:05 ` Jakub Kicinski
0 siblings, 1 reply; 12+ messages in thread
From: Grant Grundler @ 2021-01-20 0:59 UTC (permalink / raw)
To: Grant Grundler
Cc: Jakub Kicinski, Oliver Neukum, David S. Miller,
Greg Kroah-Hartman, linux-usb, netdev, LKML
On Wed, Jan 20, 2021 at 12:55 AM Grant Grundler <grundler@chromium.org> wrote:
>
> On Tue, Jan 19, 2021 at 9:46 PM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > On Fri, 15 Jan 2021 21:26:23 -0800 Grant Grundler wrote:
> > > 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.
> >
> > > @@ -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:
> >
> > Thanks for the patch, this looks like an improvement over:
> >
> > 59b4a8fa27f5 ("CDC-NCM: remove "connected" log message")
> >
> > right? Should we bring the "network connection: connected" message back?
>
> Yes, we can revert Roland's patch. I didn't see that one.
>
> > Do you want all of these patches to be applied to 5.11 and backported?
>
> Yes to 5.11. Only the 3rd one really needs to be applied to stable kernels.
Sorry - I was thinking 5.11 was -next (and that's incorrect).
As you suggested below, only the 3rd one really needs to be applied to
5.11 and other stable kernels.
cheers,
grant
>
> > Feels to me like the last one is a fix and the rest can go into -next,
> > WDYT?
>
> Exactly.
>
> Thanks!
> grant
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 3/3] net: usb: cdc_ncm: don't spew notifications
2021-01-20 0:59 ` Grant Grundler
@ 2021-01-20 1:05 ` Jakub Kicinski
2021-01-20 1:14 ` Grant Grundler
0 siblings, 1 reply; 12+ messages in thread
From: Jakub Kicinski @ 2021-01-20 1:05 UTC (permalink / raw)
To: Grant Grundler
Cc: Oliver Neukum, David S. Miller, Greg Kroah-Hartman, linux-usb,
netdev, LKML
On Wed, 20 Jan 2021 00:59:17 +0000 Grant Grundler wrote:
> > > Thanks for the patch, this looks like an improvement over:
> > >
> > > 59b4a8fa27f5 ("CDC-NCM: remove "connected" log message")
> > >
> > > right? Should we bring the "network connection: connected" message back?
> >
> > Yes, we can revert Roland's patch. I didn't see that one.
> >
> > > Do you want all of these patches to be applied to 5.11 and backported?
> >
> > Yes to 5.11. Only the 3rd one really needs to be applied to stable kernels.
>
> Sorry - I was thinking 5.11 was -next (and that's incorrect).
>
> As you suggested below, only the 3rd one really needs to be applied to
> 5.11 and other stable kernels.
Cool, would you mind reposting just the 3rd patch separately, and
tagged as [PATCH net] so that CI can give it a shaking? We'll go
from there with the rest.
> >
> > > Feels to me like the last one is a fix and the rest can go into -next,
> > > WDYT?
> >
> > Exactly.
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 3/3] net: usb: cdc_ncm: don't spew notifications
2021-01-20 1:05 ` Jakub Kicinski
@ 2021-01-20 1:14 ` Grant Grundler
0 siblings, 0 replies; 12+ messages in thread
From: Grant Grundler @ 2021-01-20 1:14 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Grant Grundler, Oliver Neukum, David S. Miller,
Greg Kroah-Hartman, linux-usb, netdev, LKML
On Wed, Jan 20, 2021 at 1:05 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Wed, 20 Jan 2021 00:59:17 +0000 Grant Grundler wrote:
> > > > Thanks for the patch, this looks like an improvement over:
> > > >
> > > > 59b4a8fa27f5 ("CDC-NCM: remove "connected" log message")
> > > >
> > > > right? Should we bring the "network connection: connected" message back?
> > >
> > > Yes, we can revert Roland's patch. I didn't see that one.
> > >
> > > > Do you want all of these patches to be applied to 5.11 and backported?
> > >
> > > Yes to 5.11. Only the 3rd one really needs to be applied to stable kernels.
> >
> > Sorry - I was thinking 5.11 was -next (and that's incorrect).
> >
> > As you suggested below, only the 3rd one really needs to be applied to
> > 5.11 and other stable kernels.
>
> Cool, would you mind reposting just the 3rd patch separately, and
> tagged as [PATCH net] so that CI can give it a shaking?
Done :)
> We'll go from there with the rest.
Awesome - thank you!
cheers,
grant
>
> > >
> > > > Feels to me like the last one is a fix and the rest can go into -next,
> > > > WDYT?
> > >
> > > Exactly.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] net: usb: cdc_ncm: emit dev_err on error paths
2021-01-16 5:26 [PATCH 1/3] net: usb: cdc_ncm: emit dev_err on error paths Grant Grundler
2021-01-16 5:26 ` [PATCH 2/3] net: usb: log errors to dmesg/syslog Grant Grundler
2021-01-16 5:26 ` [PATCH 3/3] net: usb: cdc_ncm: don't spew notifications Grant Grundler
@ 2021-01-18 22:01 ` Grant Grundler
2 siblings, 0 replies; 12+ messages in thread
From: Grant Grundler @ 2021-01-18 22:01 UTC (permalink / raw)
To: Grant Grundler, nic_swsd
Cc: Oliver Neukum, David S. Miller, Jakub Kicinski,
Greg Kroah-Hartman, linux-usb, netdev, LKML
+nic_swsd [adding per Realtek developer team request]
On Sat, Jan 16, 2021 at 5:26 AM Grant Grundler <grundler@chromium.org> wrote:
>
> Several error paths in bind/probe code will only emit
> output using dev_dbg. But if we are going to fail the
> bind/probe, emit related output with "err" priority.
>
> Signed-off-by: Grant Grundler <grundler@chromium.org>
> ---
> drivers/net/usb/cdc_ncm.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
> index 5a78848db93f..25498c311551 100644
> --- a/drivers/net/usb/cdc_ncm.c
> +++ b/drivers/net/usb/cdc_ncm.c
> @@ -849,17 +849,17 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
>
> /* check if we got everything */
> if (!ctx->data) {
> - dev_dbg(&intf->dev, "CDC Union missing and no IAD found\n");
> + dev_err(&intf->dev, "CDC Union missing and no IAD found\n");
> goto error;
> }
> if (cdc_ncm_comm_intf_is_mbim(intf->cur_altsetting)) {
> if (!ctx->mbim_desc) {
> - dev_dbg(&intf->dev, "MBIM functional descriptor missing\n");
> + dev_err(&intf->dev, "MBIM functional descriptor missing\n");
> goto error;
> }
> } else {
> if (!ctx->ether_desc || !ctx->func_desc) {
> - dev_dbg(&intf->dev, "NCM or ECM functional descriptors missing\n");
> + dev_err(&intf->dev, "NCM or ECM functional descriptors missing\n");
> goto error;
> }
> }
> @@ -868,7 +868,7 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
> if (ctx->data != ctx->control) {
> temp = usb_driver_claim_interface(driver, ctx->data, dev);
> if (temp) {
> - dev_dbg(&intf->dev, "failed to claim data intf\n");
> + dev_err(&intf->dev, "failed to claim data intf\n");
> goto error;
> }
> }
> @@ -924,7 +924,7 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
> if (ctx->ether_desc) {
> temp = usbnet_get_ethernet_addr(dev, ctx->ether_desc->iMACAddress);
> if (temp) {
> - dev_dbg(&intf->dev, "failed to get mac address\n");
> + dev_err(&intf->dev, "failed to get mac address\n");
> goto error2;
> }
> dev_info(&intf->dev, "MAC-Address: %pM\n", dev->net->dev_addr);
> --
> 2.29.2
>
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-01-20 1:17 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-16 5:26 [PATCH 1/3] net: usb: cdc_ncm: emit dev_err on error paths Grant Grundler
2021-01-16 5:26 ` [PATCH 2/3] net: usb: log errors to dmesg/syslog Grant Grundler
2021-01-18 22:02 ` Grant Grundler
2021-01-16 5:26 ` [PATCH 3/3] net: usb: cdc_ncm: don't spew notifications Grant Grundler
2021-01-18 22:02 ` Grant Grundler
2021-01-19 1:23 ` Hayes Wang
2021-01-19 21:45 ` Jakub Kicinski
2021-01-20 0:55 ` Grant Grundler
2021-01-20 0:59 ` Grant Grundler
2021-01-20 1:05 ` Jakub Kicinski
2021-01-20 1:14 ` Grant Grundler
2021-01-18 22:01 ` [PATCH 1/3] net: usb: cdc_ncm: emit dev_err on error paths Grant Grundler
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 NNTP newsgroup(s).