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 Acked-by: Eelco Chaudron <[email protected]> Acked-by: Mike Pattrick <[email protected]> Signed-off-by: David Marchand <[email protected]> --- Changes since v4: - separated as patch 5 the removal of netdev_features_is_full_duplex, 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 | 1 + lib/netdev-bsd.c | 20 ++++++++++++++++++++ lib/netdev-dpdk.c | 18 ++++++++++++++++++ lib/netdev-linux-private.h | 1 + lib/netdev-linux.c | 34 +++++++++++++++++++++++++++++++++- lib/netdev-provider.h | 7 +++++++ lib/netdev.c | 31 +++++++++++++++++++++++++++++++ ofproto/ofproto-dpif-sflow.c | 7 +++---- tests/system-interface.at | 12 ++++++++++++ vswitchd/bridge.c | 8 +++----- 10 files changed, 129 insertions(+), 10 deletions(-) diff --git a/include/openvswitch/netdev.h b/include/openvswitch/netdev.h index 83e8633dda..56dff57725 100644 --- a/include/openvswitch/netdev.h +++ b/include/openvswitch/netdev.h @@ -136,6 +136,7 @@ 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 cffbe52191..d29589efde 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. */ @@ -1155,6 +1156,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); @@ -1225,6 +1227,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 @@ -1553,6 +1572,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 45b9f48964..687e1196b5 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -4156,6 +4156,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) { @@ -6868,6 +6885,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 1dc64b4e06..8bf1a29a0f 100644 --- a/lib/netdev-linux.c +++ b/lib/netdev-linux.c @@ -175,10 +175,13 @@ static inline uint32_t rpl_ethtool_cmd_speed(const struct ethtool_cmd *ep) #define ADVERTISED_10000baseR_FEC (1 << 20) #endif -/* Linux 3.2 introduced "unknown" speed. */ +/* Linux 3.2 introduced "unknown" speed and duplex. */ #ifndef SPEED_UNKNOWN #define SPEED_UNKNOWN -1 #endif +#ifndef DUPLEX_UNKNOWN +#define DUPLEX_UNKNOWN 0xff +#endif /* Linux 3.5 introduced supported and advertised flags for * 40G base KR4, CR4, SR4 and LR4. */ @@ -2700,6 +2703,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; @@ -2783,6 +2787,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_, @@ -3907,6 +3937,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, @@ -3924,6 +3955,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..82bb13ce04 100644 --- a/lib/netdev.c +++ b/lib/netdev.c @@ -1298,6 +1298,37 @@ netdev_features_is_full_duplex(enum netdev_features features) | NETDEV_F_100GB_FD | NETDEV_F_1TB_FD)) != 0; } +/* 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) +{ + 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 * successful, otherwise a positive errno value. */ int 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.51.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
