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 <david.march...@redhat.com> --- 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); int netdev_set_advertisements(struct netdev *, enum netdev_features advertise); void netdev_features_format(struct ds *, enum netdev_features); diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c index 7718c30a04..be27b86e52 100644 --- a/lib/netdev-bsd.c +++ b/lib/netdev-bsd.c @@ -97,6 +97,7 @@ struct netdev_bsd { enum netdev_features current; enum netdev_features advertised; enum netdev_features supported; + bool full_duplex; int tap_fd; /* TAP character device, if any, otherwise -1. */ @@ -1150,6 +1151,7 @@ netdev_bsd_read_features(struct netdev_bsd *netdev) if (!netdev->current) { netdev->current = NETDEV_F_OTHER; } + netdev->full_duplex = ifmr.ifm_active & IFM_FDX; /* Advertised features. */ netdev->advertised = netdev_bsd_parse_media(ifmr.ifm_current); @@ -1219,6 +1221,23 @@ netdev_bsd_get_speed(const struct netdev *netdev_, uint32_t *current, return error; } +static int +netdev_bsd_get_duplex(const struct netdev *netdev_, bool *full_duplex) +{ + struct netdev_bsd *netdev = netdev_bsd_cast(netdev_); + int error; + + ovs_mutex_lock(&netdev->mutex); + netdev_bsd_read_features(netdev); + if (!netdev->get_features_error) { + *full_duplex = netdev->full_duplex; + } + error = netdev->get_features_error; + ovs_mutex_unlock(&netdev->mutex); + + return error; +} + /* * Assigns 'addr' as 'netdev''s IPv4 address and 'mask' as its netmask. If * 'addr' is INADDR_ANY, 'netdev''s IPv4 address is cleared. Returns a @@ -1547,6 +1566,7 @@ netdev_bsd_update_flags(struct netdev *netdev_, enum netdev_flags off, .get_stats = netdev_bsd_get_stats, \ .get_features = netdev_bsd_get_features, \ .get_speed = netdev_bsd_get_speed, \ + .get_duplex = netdev_bsd_get_duplex, \ .set_in4 = netdev_bsd_set_in4, \ .get_addr_list = netdev_bsd_get_addr_list, \ .get_next_hop = netdev_bsd_get_next_hop, \ diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 17b4d66779..d1495ee955 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -4177,6 +4177,23 @@ out: return 0; } +static int +netdev_dpdk_get_duplex(const struct netdev *netdev, bool *full_duplex) +{ + struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); + int err = 0; + + ovs_mutex_lock(&dev->mutex); + if (dev->link.link_speed != RTE_ETH_SPEED_NUM_UNKNOWN) { + *full_duplex = dev->link.link_duplex == RTE_ETH_LINK_FULL_DUPLEX; + } else { + err = EOPNOTSUPP; + } + ovs_mutex_unlock(&dev->mutex); + + return err; +} + static struct ingress_policer * netdev_dpdk_policer_construct(uint32_t rate, uint32_t burst) { @@ -6889,6 +6906,7 @@ parse_vhost_config(const struct smap *ovs_other_config) .get_custom_stats = netdev_dpdk_get_custom_stats, \ .get_features = netdev_dpdk_get_features, \ .get_speed = netdev_dpdk_get_speed, \ + .get_duplex = netdev_dpdk_get_duplex, \ .get_status = netdev_dpdk_get_status, \ .reconfigure = netdev_dpdk_reconfigure, \ .rxq_recv = netdev_dpdk_rxq_recv diff --git a/lib/netdev-linux-private.h b/lib/netdev-linux-private.h index 8e572e3b3b..4b5b606755 100644 --- a/lib/netdev-linux-private.h +++ b/lib/netdev-linux-private.h @@ -94,6 +94,7 @@ struct netdev_linux { enum netdev_features advertised; /* Cached from ETHTOOL_GSET. */ enum netdev_features supported; /* Cached from ETHTOOL_GSET. */ uint32_t current_speed; /* Cached from ETHTOOL_GSET. */ + uint8_t current_duplex; /* Cached from ETHTOOL_GSET. */ struct ethtool_drvinfo drvinfo; /* Cached from ETHTOOL_GDRVINFO. */ struct tc *tc; diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c index 2147659485..f9f5ce4917 100644 --- a/lib/netdev-linux.c +++ b/lib/netdev-linux.c @@ -90,6 +90,9 @@ #ifndef SPEED_100000 #define SPEED_100000 100000 #endif +#ifndef DUPLEX_UNKNOWN +#define DUPLEX_UNKNOWN 0xff +#endif VLOG_DEFINE_THIS_MODULE(netdev_linux); @@ -2696,6 +2699,7 @@ netdev_linux_read_features(struct netdev_linux *netdev) } else { netdev->current = 0; } + netdev->current_duplex = ecmd.duplex; if (ecmd.port == PORT_TP) { netdev->current |= NETDEV_F_COPPER; @@ -2779,6 +2783,32 @@ netdev_linux_get_speed(const struct netdev *netdev_, uint32_t *current, return error; } +static int +netdev_linux_get_duplex(const struct netdev *netdev_, bool *full_duplex) +{ + struct netdev_linux *netdev = netdev_linux_cast(netdev_); + int err; + + ovs_mutex_lock(&netdev->mutex); + + if (netdev_linux_netnsid_is_remote(netdev)) { + err = EOPNOTSUPP; + goto exit; + } + + netdev_linux_read_features(netdev); + err = netdev->get_features_error; + if (!err && netdev->current_duplex == DUPLEX_UNKNOWN) { + err = EOPNOTSUPP; + goto exit; + } + *full_duplex = netdev->current_duplex == DUPLEX_FULL; + +exit: + ovs_mutex_unlock(&netdev->mutex); + return err; +} + /* Set the features advertised by 'netdev' to 'advertise'. */ static int netdev_linux_set_advertisements(struct netdev *netdev_, @@ -3903,6 +3933,7 @@ const struct netdev_class netdev_linux_class = { .get_stats = netdev_linux_get_stats, .get_features = netdev_linux_get_features, .get_speed = netdev_linux_get_speed, + .get_duplex = netdev_linux_get_duplex, .get_status = netdev_linux_get_status, .get_block_id = netdev_linux_get_block_id, .send = netdev_linux_send, @@ -3920,6 +3951,7 @@ const struct netdev_class netdev_tap_class = { .get_stats = netdev_tap_get_stats, .get_features = netdev_linux_get_features, .get_speed = netdev_linux_get_speed, + .get_duplex = netdev_linux_get_duplex, .get_status = netdev_linux_get_status, .send = netdev_linux_send, .rxq_construct = netdev_linux_rxq_construct, diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h index 5ae3794699..33a68c49c2 100644 --- a/lib/netdev-provider.h +++ b/lib/netdev-provider.h @@ -514,6 +514,13 @@ struct netdev_class { int (*get_speed)(const struct netdev *netdev, uint32_t *current, uint32_t *max); + /* Stores the current duplex status of 'netdev' into '*full_duplex'. + * 'true' means full duplex, 'false' means half duplex. + * + * This function may be set to null if it would always return EOPNOTSUPP. + */ + int (*get_duplex)(const struct netdev *netdev, bool *full_duplex); + /* Set the features advertised by 'netdev' to 'advertise', which is a * set of NETDEV_F_* bits. * diff --git a/lib/netdev.c b/lib/netdev.c index df5b35232d..6a05e9a7e5 100644 --- a/lib/netdev.c +++ b/lib/netdev.c @@ -1288,14 +1288,35 @@ netdev_features_to_bps(enum netdev_features features, : default_bps); } -/* Returns true if any of the NETDEV_F_* bits that indicate a full-duplex link - * are set in 'features', otherwise false. */ -bool -netdev_features_is_full_duplex(enum netdev_features features) +/* Stores the duplex capability of 'netdev' into 'full_duplex'. + * + * Some network devices may not implement support for this function. + * In such cases this function will always return EOPNOTSUPP. */ +int +netdev_get_duplex(const struct netdev *netdev, bool *full_duplex) { - return (features & (NETDEV_F_10MB_FD | NETDEV_F_100MB_FD | NETDEV_F_1GB_FD - | NETDEV_F_10GB_FD | NETDEV_F_40GB_FD - | NETDEV_F_100GB_FD | NETDEV_F_1TB_FD)) != 0; + int error; + + *full_duplex = false; + error = netdev->netdev_class->get_duplex + ? netdev->netdev_class->get_duplex(netdev, full_duplex) + : EOPNOTSUPP; + + if (error == EOPNOTSUPP) { + enum netdev_features current; + + error = netdev_get_features(netdev, ¤t, NULL, NULL, NULL); + if (!error && (current & NETDEV_F_OTHER)) { + error = EOPNOTSUPP; + } + if (!error) { + *full_duplex = (current & (NETDEV_F_10MB_FD | NETDEV_F_100MB_FD + | NETDEV_F_1GB_FD | NETDEV_F_10GB_FD + | NETDEV_F_40GB_FD | NETDEV_F_100GB_FD + | NETDEV_F_1TB_FD)) != 0; + } + } + return error; } /* Set the features advertised by 'netdev' to 'advertise'. Returns 0 if diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c index c5403e27aa..e043d7cbc8 100644 --- a/ofproto/ofproto-dpif-sflow.c +++ b/ofproto/ofproto-dpif-sflow.c @@ -298,7 +298,6 @@ sflow_agent_get_counters(void *ds_, SFLPoller *poller, struct dpif_sflow *ds = ds_; SFLCounters_sample_element elem, lacp_elem, of_elem, name_elem; SFLCounters_sample_element eth_elem; - enum netdev_features current; struct dpif_sflow_port *dsp; SFLIf_counters *counters; SFLEthernet_counters* eth_counters; @@ -307,6 +306,7 @@ sflow_agent_get_counters(void *ds_, SFLPoller *poller, struct lacp_member_stats lacp_stats; uint32_t curr_speed; const char *ifName; + bool full_duplex; dsp = dpif_sflow_find_port(ds, u32_to_odp(poller->bridgePort)); if (!dsp) { @@ -317,11 +317,10 @@ sflow_agent_get_counters(void *ds_, SFLPoller *poller, counters = &elem.counterBlock.generic; counters->ifIndex = SFL_DS_INDEX(poller->dsi); counters->ifType = 6; - if (!netdev_get_features(dsp->ofport->netdev, ¤t, NULL, NULL, NULL)) { + if (!netdev_get_duplex(dsp->ofport->netdev, &full_duplex)) { /* The values of ifDirection come from MAU MIB (RFC 2668): 0 = unknown, 1 = full-duplex, 2 = half-duplex, 3 = in, 4=out */ - counters->ifDirection = (netdev_features_is_full_duplex(current) - ? 1 : 2); + counters->ifDirection = full_duplex ? 1 : 2; } else { counters->ifDirection = 0; } diff --git a/tests/system-interface.at b/tests/system-interface.at index cb0835ad6b..15c4a0e2e1 100644 --- a/tests/system-interface.at +++ b/tests/system-interface.at @@ -207,5 +207,17 @@ AT_CHECK([ovs-vsctl get interface tap0 link_speed], [0], [dnl 50000000000 ]) +AT_CHECK([ovs-vsctl get interface tap0 duplex], [0], [dnl +full +]) + +AT_CHECK([ip link set dev tap0 down]) +AT_CHECK([ethtool -s tap0 duplex half]) +AT_CHECK([ip link set dev tap0 up]) + +AT_CHECK([ovs-vsctl get interface tap0 duplex], [0], [dnl +half +]) + OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index 456b784c01..475eefefaa 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -2532,11 +2532,11 @@ iface_refresh_netdev_status(struct iface *iface) { struct smap smap; - enum netdev_features current; enum netdev_flags flags; const char *link_state; struct eth_addr mac; int64_t bps, mtu_64, ifindex64, link_resets; + bool full_duplex; int mtu, error; uint32_t mbps; @@ -2576,11 +2576,9 @@ iface_refresh_netdev_status(struct iface *iface) link_resets = netdev_get_carrier_resets(iface->netdev); ovsrec_interface_set_link_resets(iface->cfg, &link_resets, 1); - error = netdev_get_features(iface->netdev, ¤t, NULL, NULL, NULL); + error = netdev_get_duplex(iface->netdev, &full_duplex); if (!error) { - ovsrec_interface_set_duplex(iface->cfg, - netdev_features_is_full_duplex(current) - ? "full" : "half"); + ovsrec_interface_set_duplex(iface->cfg, full_duplex ? "full" : "half"); } else { ovsrec_interface_set_duplex(iface->cfg, NULL); } -- 2.50.1 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
