* [PATCH] usbnet: Set duplex status to unknown in the absence of MII @ 2025-07-21 7:10 yicongsrfy 2025-07-21 13:51 ` Andrew Lunn 0 siblings, 1 reply; 14+ messages in thread From: yicongsrfy @ 2025-07-21 7:10 UTC (permalink / raw) To: oneukum, andrew+netdev, davem; +Cc: netdev, linux-usb, Yi Cong From: Yi Cong <yicong@kylinos.cn> CDC device don't use mdio to get link status, duplex is set half as default. Now cdc_ncm can't get duplex, set it UNKNOWN instead of half which might actually be in an error state. Signed-off-by: Yi Cong <yicong@kylinos.cn> --- drivers/net/usb/usbnet.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c index 6a3cca104af9..c612f8f606e5 100644 --- a/drivers/net/usb/usbnet.c +++ b/drivers/net/usb/usbnet.c @@ -1013,6 +1013,11 @@ int usbnet_get_link_ksettings_internal(struct net_device *net, else cmd->base.speed = SPEED_UNKNOWN; + /* Now we can't get link duplex without MII, + * set it DUPLEX_UNKNOWN instead of default DUPLEX_HALF + */ + cmd->base.duplex = DUPLEX_UNKNOWN; + return 0; } EXPORT_SYMBOL_GPL(usbnet_get_link_ksettings_internal); -- 2.25.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] usbnet: Set duplex status to unknown in the absence of MII 2025-07-21 7:10 [PATCH] usbnet: Set duplex status to unknown in the absence of MII yicongsrfy @ 2025-07-21 13:51 ` Andrew Lunn 2025-07-22 2:09 ` yicongsrfy 0 siblings, 1 reply; 14+ messages in thread From: Andrew Lunn @ 2025-07-21 13:51 UTC (permalink / raw) To: yicongsrfy; +Cc: oneukum, andrew+netdev, davem, netdev, linux-usb, Yi Cong On Mon, Jul 21, 2025 at 03:10:22PM +0800, yicongsrfy@163.com wrote: > From: Yi Cong <yicong@kylinos.cn> > > CDC device don't use mdio to get link status, duplex is set > half as default. > > Now cdc_ncm can't get duplex, set it UNKNOWN instead of half > which might actually be in an error state. It appears that CDC has a transfer to report the link status: usbnet_cdc_status(). There is a u32 which contains the link speed. Is the duplex also included in this transfer? What does the standard say about duplex? Are 1/2 duplex modes are supported? Is it clear stated they are not supported? 1G/half not be supported depending on the MAC/PHY pair? Andrew ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] usbnet: Set duplex status to unknown in the absence of MII 2025-07-21 13:51 ` Andrew Lunn @ 2025-07-22 2:09 ` yicongsrfy 2025-07-22 13:06 ` Andrew Lunn 0 siblings, 1 reply; 14+ messages in thread From: yicongsrfy @ 2025-07-22 2:09 UTC (permalink / raw) To: andrew; +Cc: andrew+netdev, davem, linux-usb, netdev, oneukum, yicong Thanks for your reply! According to the "Universal Serial Bus Class Definitions for Communications Devices v1.2": In Section 6.3, which describes notifications such as NetworkConnection and ConnectionSpeedChange, there is no mention of duplex status.In particular, for ConnectionSpeedChange, its data payload only contains two 32-bit unsigned integers, corresponding to the uplink and downlink speeds. Since CDC has no way to obtain the duplex status of the device, ethtool displays a default value of "Half". I think it would be better to display "unknown" instead of potentially showing incorrect information — for example, my device is actually operating at 1Gbps Full-duplex, but ethtool shows 1Gbps Half-duplex. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] usbnet: Set duplex status to unknown in the absence of MII 2025-07-22 2:09 ` yicongsrfy @ 2025-07-22 13:06 ` Andrew Lunn 2025-07-23 1:29 ` yicongsrfy 0 siblings, 1 reply; 14+ messages in thread From: Andrew Lunn @ 2025-07-22 13:06 UTC (permalink / raw) To: yicongsrfy; +Cc: andrew+netdev, davem, linux-usb, netdev, oneukum, yicong On Tue, Jul 22, 2025 at 10:09:33AM +0800, yicongsrfy@163.com wrote: > Thanks for your reply! > > According to the "Universal Serial Bus Class Definitions for Communications Devices v1.2": > In Section 6.3, which describes notifications such as NetworkConnection and ConnectionSpeedChange, > there is no mention of duplex status.In particular, for ConnectionSpeedChange, its data payload > only contains two 32-bit unsigned integers, corresponding to the uplink and downlink speeds. Thanks for checking this. Just one more question. This is kind of flipping the question on its head. Does the standard say devices are actually allowed to support 1/2 duplex? Does it say they are not allowed to support 1/2 duplex? If duplex is not reported, maybe it is because 1/2 duplex is simply not allowed, so there is no need to report it. > Since CDC has no way to obtain the duplex status of the device, ethtool displays a default > value of "Half". I think it would be better to display "unknown" instead of potentially showing > incorrect information — for example, my device is actually operating at 1Gbps Full-duplex, > but ethtool shows 1Gbps Half-duplex. I agree that reporting 1/2 is probably wrong. But we have to decide between "unknown" and "full". Andrew ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] usbnet: Set duplex status to unknown in the absence of MII 2025-07-22 13:06 ` Andrew Lunn @ 2025-07-23 1:29 ` yicongsrfy 2025-07-23 7:17 ` Oliver Neukum 2025-07-23 13:05 ` Andrew Lunn 0 siblings, 2 replies; 14+ messages in thread From: yicongsrfy @ 2025-07-23 1:29 UTC (permalink / raw) To: andrew; +Cc: andrew+netdev, davem, linux-usb, netdev, oneukum, yicong, yicongsrfy On Tue, 22 Jul 2025 15:06:37 +0200 [thread overview] Andrew <andrew@lunn.ch> wrote: > > On Tue, Jul 22, 2025 at 10:09:33AM +0800, yicongsrfy@163.com wrote: > > Thanks for your reply! > > > > According to the "Universal Serial Bus Class Definitions for Communications Devices v1.2": > > In Section 6.3, which describes notifications such as NetworkConnection and ConnectionSpeedChange, > > there is no mention of duplex status.In particular, for ConnectionSpeedChange, its data payload > > only contains two 32-bit unsigned integers, corresponding to the uplink and downlink speeds. > > Thanks for checking this. > > Just one more question. This is kind of flipping the question on its > head. Does the standard say devices are actually allowed to support > 1/2 duplex? Does it say they are not allowed to support 1/2 duplex? > > If duplex is not reported, maybe it is because 1/2 duplex is simply > not allowed, so there is no need to report it. > No, the standard does not mention anything about duplex at all. However, Chapter 2 of the standard describes the scope of devices covered by CDC, including wired and wireless network adapters, among others. We know that wireless communication is inherently half-duplex; for wired network adapters, the duplex status depends on the capabilities of both communication ends. One of the USB network adapters I have (AX88179) supports both the vendor-specific driver and the cdc_ncm driver. When using the vendor-specific driver, all operational states function normally, and the information reported by ethtool matches the actual hardware behavior — which means the hardware definitely supports both full-duplex and half-duplex modes. The issue we are discussing only occurs when using the cdc_ncm driver. To further investigate, I conducted the following tests using the cdc_ncm driver: 1. I set the peer network adapter (r8169) to auto-negotiation and connected it to the device under test. On the peer adapter, I was able to observe that the link was operating in full-duplex mode. 2. I disabled auto-negotiation on the peer adapter and manually set it to half-duplex. The setting was successfully applied, and communication remained functional. From these two tests, we can conclude that both full-duplex and half-duplex modes are supported — the problem is simply that the duplex status cannot be retrieved in the absence of MII support. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] usbnet: Set duplex status to unknown in the absence of MII 2025-07-23 1:29 ` yicongsrfy @ 2025-07-23 7:17 ` Oliver Neukum 2025-07-23 8:44 ` yicongsrfy 2025-07-23 13:05 ` Andrew Lunn 1 sibling, 1 reply; 14+ messages in thread From: Oliver Neukum @ 2025-07-23 7:17 UTC (permalink / raw) To: yicongsrfy, andrew Cc: andrew+netdev, davem, linux-usb, netdev, oneukum, yicong On 23.07.25 03:29, yicongsrfy@163.com wrote: > No, the standard does not mention anything about duplex at all. > > However, Chapter 2 of the standard describes the scope of devices > covered by CDC, including wired and wireless network adapters, > among others. > > We know that wireless communication is inherently half-duplex; Well, no. 802.11 is half-duplex. Cell phones are capable of full duplex. CDC is not just network cards. > for wired network adapters, the duplex status depends on the > capabilities of both communication ends. On ethernet. Again CDC is not limited to ethernet. > From these two tests, we can conclude that both full-duplex > and half-duplex modes are supported — the problem is simply > that the duplex status cannot be retrieved in the absence of > MII support. Sort of. You are asking a generic driver to apply a concept from ethernet. It cannot. Ethernet even if it is half-duplex is very much symmetrical in speed. Cable modems do not, just to give an example. I think we need to centralize the reaction to stuff that is not ethernet. Regards Oliver ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] usbnet: Set duplex status to unknown in the absence of MII 2025-07-23 7:17 ` Oliver Neukum @ 2025-07-23 8:44 ` yicongsrfy 2025-07-23 10:03 ` Oliver Neukum 2025-07-23 13:04 ` Andrew Lunn 0 siblings, 2 replies; 14+ messages in thread From: yicongsrfy @ 2025-07-23 8:44 UTC (permalink / raw) To: oneukum; +Cc: andrew+netdev, andrew, davem, linux-usb, netdev, yicong On Wed, 23 Jul 2025 09:17:02 +0200 Oliver <oneukum@suse.com> wrote: > > On 23.07.25 03:29, yicongsrfy@163.com wrote: > > > From these two tests, we can conclude that both full-duplex > > and half-duplex modes are supported — the problem is simply > > that the duplex status cannot be retrieved in the absence of > > MII support. > > Sort of. You are asking a generic driver to apply a concept > from ethernet. It cannot. Ethernet even if it is half-duplex > is very much symmetrical in speed. Cable modems do not, just > to give an example. > > I think we need to centralize the reaction to stuff that is not ethernet. Thanks! I think I understand what you mean now. You're suggesting to create a unified interface or framework to retrieve the duplex status of all CDC protocol-supported devices? This seems like a rather big undertaking, and one of the key reasons is that the CDC protocol itself does not define anything related to duplex status — unlike the 802.3 standard, which clearly defines how to obtain this information via MDIO. Coming back to the issue described in this patch, usbnet_get_link_ksettings_internal is currently only used in cdc_ether.c and cdc_ncm.c as a callback for ethtool. Can we assume that this part only concerns Ethernet devices (and that, at least for now, none of the existing devices can retrieve the duplex status through this interface)? ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] usbnet: Set duplex status to unknown in the absence of MII 2025-07-23 8:44 ` yicongsrfy @ 2025-07-23 10:03 ` Oliver Neukum 2025-07-23 13:04 ` Andrew Lunn 1 sibling, 0 replies; 14+ messages in thread From: Oliver Neukum @ 2025-07-23 10:03 UTC (permalink / raw) To: yicongsrfy, oneukum Cc: andrew+netdev, andrew, davem, linux-usb, netdev, yicong On 23.07.25 10:44, yicongsrfy@163.com wrote: > On Wed, 23 Jul 2025 09:17:02 +0200 Oliver <oneukum@suse.com> wrote: >> >> On 23.07.25 03:29, yicongsrfy@163.com wrote: >> >>> From these two tests, we can conclude that both full-duplex >>> and half-duplex modes are supported — the problem is simply >>> that the duplex status cannot be retrieved in the absence of >>> MII support. >> >> Sort of. You are asking a generic driver to apply a concept >> from ethernet. It cannot. Ethernet even if it is half-duplex >> is very much symmetrical in speed. Cable modems do not, just >> to give an example. >> >> I think we need to centralize the reaction to stuff that is not ethernet. > > Thanks! > > I think I understand what you mean now. > You're suggesting to create a unified interface or > framework to retrieve the duplex status of all CDC > protocol-supported devices? Well no. We have to understand that the difference in duplex status apply only to a subset of devices. In a way the network layer is deficient in only having an unknown status rather than an unknown and an inapplicable status. If we are to retain a single status for simplicity, then I'd say that the default being half duplex rather than unknown is wrong. > This seems like a rather big undertaking, and one of the key > reasons is that the CDC protocol itself does not define anything > related to duplex status — unlike the 802.3 standard, which > clearly defines how to obtain this information via MDIO. CDC is not a network protocol. CDC is a protocol for talking to network devices. CDC can define a way to transmit duplex information. If the protocol under CDC does not have the notion, this will be of no use. > Coming back to the issue described in this patch, > usbnet_get_link_ksettings_internal is currently only used in > cdc_ether.c and cdc_ncm.c as a callback for ethtool. > Can we assume that this part only concerns Ethernet devices > (and that, at least for now, none of the existing devices can > retrieve the duplex status through this interface)? Yes. It is not really that important. But strictly speaking the network layer is using the wrong default. Regards Oliver ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] usbnet: Set duplex status to unknown in the absence of MII 2025-07-23 8:44 ` yicongsrfy 2025-07-23 10:03 ` Oliver Neukum @ 2025-07-23 13:04 ` Andrew Lunn 1 sibling, 0 replies; 14+ messages in thread From: Andrew Lunn @ 2025-07-23 13:04 UTC (permalink / raw) To: yicongsrfy; +Cc: oneukum, andrew+netdev, davem, linux-usb, netdev, yicong > Coming back to the issue described in this patch, > usbnet_get_link_ksettings_internal is currently only used in > cdc_ether.c and cdc_ncm.c as a callback for ethtool. > Can we assume that this part only concerns Ethernet devices > (and that, at least for now, none of the existing devices can > retrieve the duplex status through this interface)? "ethtool" can be used with any sort of interface, if the driver implements the op. So you should not assume it is only used for Ethernet. Andrew ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] usbnet: Set duplex status to unknown in the absence of MII 2025-07-23 1:29 ` yicongsrfy 2025-07-23 7:17 ` Oliver Neukum @ 2025-07-23 13:05 ` Andrew Lunn 2025-07-23 22:21 ` Jakub Kicinski 1 sibling, 1 reply; 14+ messages in thread From: Andrew Lunn @ 2025-07-23 13:05 UTC (permalink / raw) To: yicongsrfy; +Cc: andrew+netdev, davem, linux-usb, netdev, oneukum, yicong On Wed, Jul 23, 2025 at 09:29:26AM +0800, yicongsrfy@163.com wrote: > On Tue, 22 Jul 2025 15:06:37 +0200 [thread overview] Andrew <andrew@lunn.ch> wrote: > > > > On Tue, Jul 22, 2025 at 10:09:33AM +0800, yicongsrfy@163.com wrote: > > > Thanks for your reply! > > > > > > According to the "Universal Serial Bus Class Definitions for Communications Devices v1.2": > > > In Section 6.3, which describes notifications such as NetworkConnection and ConnectionSpeedChange, > > > there is no mention of duplex status.In particular, for ConnectionSpeedChange, its data payload > > > only contains two 32-bit unsigned integers, corresponding to the uplink and downlink speeds. > > > > Thanks for checking this. > > > > Just one more question. This is kind of flipping the question on its > > head. Does the standard say devices are actually allowed to support > > 1/2 duplex? Does it say they are not allowed to support 1/2 duplex? > > > > If duplex is not reported, maybe it is because 1/2 duplex is simply > > not allowed, so there is no need to report it. > > > > No, the standard does not mention anything about duplex at all. O.K. So please set duplex to unknown. Andrew ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] usbnet: Set duplex status to unknown in the absence of MII 2025-07-23 13:05 ` Andrew Lunn @ 2025-07-23 22:21 ` Jakub Kicinski 2025-07-24 1:31 ` [PATCH v2] " yicongsrfy 0 siblings, 1 reply; 14+ messages in thread From: Jakub Kicinski @ 2025-07-23 22:21 UTC (permalink / raw) To: Andrew Lunn Cc: yicongsrfy, andrew+netdev, davem, linux-usb, netdev, oneukum, yicong On Wed, 23 Jul 2025 15:05:27 +0200 Andrew Lunn wrote: > > > Thanks for checking this. > > > > > > Just one more question. This is kind of flipping the question on its > > > head. Does the standard say devices are actually allowed to support > > > 1/2 duplex? Does it say they are not allowed to support 1/2 duplex? > > > > > > If duplex is not reported, maybe it is because 1/2 duplex is simply > > > not allowed, so there is no need to report it. > > > > > > > No, the standard does not mention anything about duplex at all. > > O.K. So please set duplex to unknown. .. and update the commit message and the code comment to reflect the outcome of the discussion better. -- pw-bot: cr ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2] usbnet: Set duplex status to unknown in the absence of MII 2025-07-23 22:21 ` Jakub Kicinski @ 2025-07-24 1:31 ` yicongsrfy 2025-07-24 9:03 ` Oliver Neukum 2025-07-25 18:10 ` patchwork-bot+netdevbpf 0 siblings, 2 replies; 14+ messages in thread From: yicongsrfy @ 2025-07-24 1:31 UTC (permalink / raw) To: kuba, andrew+netdev, andrew, oneukum; +Cc: davem, linux-usb, netdev, yicong From: Yi Cong <yicong@kylinos.cn> Currently, USB CDC devices that do not use MDIO to get link status have their duplex mode set to half-duplex by default. However, since the CDC specification does not define a duplex status, this can be misleading. This patch changes the default to DUPLEX_UNKNOWN in the absence of MII, which more accurately reflects the state of the link and avoids implying an incorrect or error state. v2: rewrote commmit messages and code comments Link: https://lore.kernel.org/all/20250723152151.70a8034b@kernel.org/ Signed-off-by: Yi Cong <yicong@kylinos.cn> --- drivers/net/usb/usbnet.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c index 6a3cca104af9..b870e7c6d6a0 100644 --- a/drivers/net/usb/usbnet.c +++ b/drivers/net/usb/usbnet.c @@ -1013,6 +1013,13 @@ int usbnet_get_link_ksettings_internal(struct net_device *net, else cmd->base.speed = SPEED_UNKNOWN; + /* The standard "Universal Serial Bus Class Definitions + * for Communications Devices v1.2" does not specify + * anything about duplex status. + * So set it DUPLEX_UNKNOWN instead of default DUPLEX_HALF. + */ + cmd->base.duplex = DUPLEX_UNKNOWN; + return 0; } EXPORT_SYMBOL_GPL(usbnet_get_link_ksettings_internal); -- 2.25.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2] usbnet: Set duplex status to unknown in the absence of MII 2025-07-24 1:31 ` [PATCH v2] " yicongsrfy @ 2025-07-24 9:03 ` Oliver Neukum 2025-07-25 18:10 ` patchwork-bot+netdevbpf 1 sibling, 0 replies; 14+ messages in thread From: Oliver Neukum @ 2025-07-24 9:03 UTC (permalink / raw) To: yicongsrfy, kuba, andrew+netdev, andrew, oneukum Cc: davem, linux-usb, netdev, yicong On 24.07.25 03:31, yicongsrfy@163.com wrote: > From: Yi Cong <yicong@kylinos.cn> > > Currently, USB CDC devices that do not use MDIO to get link status have > their duplex mode set to half-duplex by default. However, since the CDC > specification does not define a duplex status, this can be misleading. > > This patch changes the default to DUPLEX_UNKNOWN in the absence of MII, > which more accurately reflects the state of the link and avoids implying > an incorrect or error state. > > v2: rewrote commmit messages and code comments > > Link: https://lore.kernel.org/all/20250723152151.70a8034b@kernel.org/ > Signed-off-by: Yi Cong <yicong@kylinos.cn> Acked-by: Oliver Neukum <oneukum@suse.com> ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] usbnet: Set duplex status to unknown in the absence of MII 2025-07-24 1:31 ` [PATCH v2] " yicongsrfy 2025-07-24 9:03 ` Oliver Neukum @ 2025-07-25 18:10 ` patchwork-bot+netdevbpf 1 sibling, 0 replies; 14+ messages in thread From: patchwork-bot+netdevbpf @ 2025-07-25 18:10 UTC (permalink / raw) To: None; +Cc: kuba, andrew+netdev, andrew, oneukum, davem, linux-usb, netdev, yicong Hello: This patch was applied to netdev/net-next.git (main) by Jakub Kicinski <kuba@kernel.org>: On Thu, 24 Jul 2025 09:31:33 +0800 you wrote: > From: Yi Cong <yicong@kylinos.cn> > > Currently, USB CDC devices that do not use MDIO to get link status have > their duplex mode set to half-duplex by default. However, since the CDC > specification does not define a duplex status, this can be misleading. > > This patch changes the default to DUPLEX_UNKNOWN in the absence of MII, > which more accurately reflects the state of the link and avoids implying > an incorrect or error state. > > [...] Here is the summary with links: - [v2] usbnet: Set duplex status to unknown in the absence of MII https://git.kernel.org/netdev/net-next/c/a75afcd188e1 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2025-07-25 18:10 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-07-21 7:10 [PATCH] usbnet: Set duplex status to unknown in the absence of MII yicongsrfy 2025-07-21 13:51 ` Andrew Lunn 2025-07-22 2:09 ` yicongsrfy 2025-07-22 13:06 ` Andrew Lunn 2025-07-23 1:29 ` yicongsrfy 2025-07-23 7:17 ` Oliver Neukum 2025-07-23 8:44 ` yicongsrfy 2025-07-23 10:03 ` Oliver Neukum 2025-07-23 13:04 ` Andrew Lunn 2025-07-23 13:05 ` Andrew Lunn 2025-07-23 22:21 ` Jakub Kicinski 2025-07-24 1:31 ` [PATCH v2] " yicongsrfy 2025-07-24 9:03 ` Oliver Neukum 2025-07-25 18:10 ` patchwork-bot+netdevbpf
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).