On 9/16/25 3:56 PM, David Marchand wrote: > As OVS does not know of the 25G speed, matching on a list of known speed > for deducing full duplex capability is broken. > > Add a new netdev op to retrieve the duplex status for the existing netdev > implementations. > > Reported-at: https://issues.redhat.com/browse/FDP-883 > Signed-off-by: David Marchand <[email protected]> > --- > Changes since v2: > - reworked BSD update on top of additional cleanup for > concurrent accesses, > - preserved compatibility with older Linux kernels (wrt > DUPLEX_UNKNOWN availability), > - added some description to netdev_get_duplex(), > - (hopefully) fixed reverse xmas tree, > > --- > include/openvswitch/netdev.h | 2 +- > lib/netdev-bsd.c | 20 ++++++++++++++++++++ > lib/netdev-dpdk.c | 18 ++++++++++++++++++ > lib/netdev-linux-private.h | 1 + > lib/netdev-linux.c | 32 ++++++++++++++++++++++++++++++++ > lib/netdev-provider.h | 7 +++++++ > lib/netdev.c | 35 ++++++++++++++++++++++++++++------- > ofproto/ofproto-dpif-sflow.c | 7 +++---- > tests/system-interface.at | 12 ++++++++++++ > vswitchd/bridge.c | 8 +++----- > 10 files changed, 125 insertions(+), 17 deletions(-) > > diff --git a/include/openvswitch/netdev.h b/include/openvswitch/netdev.h > index 83e8633dda..8212ffb00e 100644 > --- a/include/openvswitch/netdev.h > +++ b/include/openvswitch/netdev.h > @@ -135,7 +135,7 @@ int netdev_get_features(const struct netdev *, > int netdev_get_speed(const struct netdev *, uint32_t *current, uint32_t > *max); > uint64_t netdev_features_to_bps(enum netdev_features features, > uint64_t default_bps); > -bool netdev_features_is_full_duplex(enum netdev_features features); > +int netdev_get_duplex(const struct netdev *, bool *full_duplex);
Hi, David. The code in this patch looks mostly good to me, but I have an issue with the API change here. The patch is technically a bug fix, because we're currently wrongly reporting duplex value for some NICs. So, it makes sense to me if we backported this fix down to LTS. But we shouldn't really backport a backwards incompatible change in the public API. The description of netdev_features_is_full_duplex() is fairly clear and while it overlaps with the new netdev_get_duplex(), they can co-exist without issues even semantically. So, maybe we should split this patch in two: one that adds and uses the new API and the one that removes the old function? This way we could backport the fix and only clean up the API on main. What do you think? Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
