The FDP team is no longer accepting new bugs in Bugzilla. Please report your issues under FDP project in Jira. Thanks.
Bug 2137567 - [RFE] Add support for 25, 50, 100 and other Gbps link speed detection
Summary: [RFE] Add support for 25, 50, 100 and other Gbps link speed detection
Keywords:
Status: CLOSED UPSTREAM
Alias: None
Product: Red Hat Enterprise Linux Fast Datapath
Classification: Red Hat
Component: openvswitch3.0
Version: FDP 22.L
Hardware: Unspecified
OS: Unspecified
high
medium
Target Milestone: ---
: ---
Assignee: Adrián Moreno
QA Contact: Hekai Wang
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2022-10-25 14:10 UTC by Ilya Maximets
Modified: 2023-09-28 13:20 UTC (History)
6 users (show)

Fixed In Version: openvswitch3.2-3.2.0-0.2.el9fdp
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2023-09-28 13:20:40 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Issue Tracker FD-2399 0 None None None 2022-10-25 14:17:25 UTC

Description Ilya Maximets 2022-10-25 14:10:55 UTC
OVS currently doesn't detect link speeds that are not defined in the
OF 1.5 spec.  For example, fairly common 25 Gbps links.  However, some
configuration aspects like QoS max-rate may depend on the detected link
speed if not overridden by the user.

It is possible to detect other different link speeds while not reporting
them to OpenFlow to still be complaint with it.  OpenFlow will just need
to report the 'OTHER' speed to the controller.

The infrastructure for netdev_features and OpenFlow structures is fairly
separate in the code, so it should not be a big problem.

One big issue is that OVS is using deprecated ETHTOOL_GSET/SSET to query
netdev features.  This API doesn't support any new link speeds.
New code should first try ETHTOOL_GLINKSETTINGS/SLINKSETTINGS first and
fall back to ETHTOOL_GSET/SSET only if the new API fails.

ETHTOOL_GLINKSETTINGS/SLINKSETTINGS API is able to provide information
about all the modern-age link speeds, but it is more complex to use.

Comment 2 Adrián Moreno 2023-03-03 10:31:56 UTC
Hi Ilya, I've started looking into this.

(In reply to Ilya Maximets from comment #0)
> OVS currently doesn't detect link speeds that are not defined in the
> OF 1.5 spec.  For example, fairly common 25 Gbps links.  However, some
> configuration aspects like QoS max-rate may depend on the detected link
> speed if not overridden by the user.
> 
> It is possible to detect other different link speeds while not reporting
> them to OpenFlow to still be complaint with it.  OpenFlow will just need
> to report the 'OTHER' speed to the controller.
> 
> The infrastructure for netdev_features and OpenFlow structures is fairly
> separate in the code, so it should not be a big problem.
> 
> One big issue is that OVS is using deprecated ETHTOOL_GSET/SSET to query
> netdev features.  This API doesn't support any new link speeds.
> New code should first try ETHTOOL_GLINKSETTINGS/SLINKSETTINGS first and
> fall back to ETHTOOL_GSET/SSET only if the new API fails.
> 
> ETHTOOL_GLINKSETTINGS/SLINKSETTINGS API is able to provide information
> about all the modern-age link speeds, but it is more complex to use.

I'm thinking maybe this is not the only approach, or it may not be enough.

Currently, we have "enum netdev_features" that matches bit-to-bit with the OFPPF_ features bits:

> BUILD_ASSERT_DECL((int) NETDEV_F_10MB_HD    == OFPPF_10MB_HD);     /* bit 0 */
> BUILD_ASSERT_DECL((int) NETDEV_F_10MB_FD    == OFPPF_10MB_FD);     /* bit 1 */
> BUILD_ASSERT_DECL((int) NETDEV_F_100MB_HD   == OFPPF_100MB_HD);    /* bit 2 */
> BUILD_ASSERT_DECL((int) NETDEV_F_100MB_FD   == OFPPF_100MB_FD);    /* bit 3 */
> BUILD_ASSERT_DECL((int) NETDEV_F_1GB_HD     == OFPPF_1GB_HD);      /* bit 4 */
> BUILD_ASSERT_DECL((int) NETDEV_F_1GB_FD     == OFPPF_1GB_FD);      /* bit 5 */
> BUILD_ASSERT_DECL((int) NETDEV_F_10GB_FD    == OFPPF_10GB_FD);     /* bit 6 */
> BUILD_ASSERT_DECL((int) NETDEV_F_40GB_FD    == OFPPF11_40GB_FD);   /* bit 7 */
> BUILD_ASSERT_DECL((int) NETDEV_F_100GB_FD   == OFPPF11_100GB_FD);  /* bit 8 */
> BUILD_ASSERT_DECL((int) NETDEV_F_1TB_FD     == OFPPF11_1TB_FD);    /* bit 9 */

Surely, ETHTOOL_GLINKSETTINGS API exposes much more link speeds, which are constantly being added  (last one in October 22). In fact, looking at the API it's clear the authors designed it to allow for future extension. There are currently [1] much more options than in DPDK [2]. Being ethtool the superset of DPDK's, we could expose that in the NETDEV_* layer but that will probably mean we have to update it as more features are added in order to stay accurate. This can be a maintenance burden.

Considering that we're not exposing them to OpenFlow layer, for me the problem is we're actually using these feature bits to determine the link speeds (via netdev_features_to_bps). However, both the rte_eth_link and ethtool APIs report the current link speed using big enough fields (32 bits in 1Mbps units).

How about we add another function in the netdev API to retrieve the link speeds directly? We could get both maximum and currently configured speed. In netdev-linux we can still use the new ethtool API to try to determine the maximum possible link speed. Even if we cannot follow the pace of DPDK's or ethtool.h's updates in feature bits, it's very likely we'll be able to get an accurate current speed which will make QoS work fine for any link in both linux and DPDK netdevs.

Does this make any sense?

[1] https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/ethtool.h#L1636
[2] https://github.com/DPDK/dpdk/blob/4ef69b2877a24ddb89afaf4bb6f4e73bb52a605b/lib/ethdev/rte_ethdev.h#L312

Comment 3 Ilya Maximets 2023-03-10 11:19:00 UTC
Hi, Adrian.

That makes sense to me.  If we can have a netdev API that reports e.g. a number
as a link speed instead of feature flags that would be nice.  Most users do
convert the bits to a number anyway and ETHTOOL_GLINKSETTINGS reports a number.

I didn't see your code, but the idea sounds good as long as we keep OpenFlow
interfaces intact.

Comment 4 Eelco Chaudron 2023-09-28 13:20:40 UTC
Fixed upstream closing ERFE.


Note You need to log in before you can comment on or make changes to this bug.