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> --- include/openvswitch/netdev.h | 2 +- lib/netdev-bsd.c | 31 ++++++++++++++++++++++++++----- lib/netdev-dpdk.c | 20 ++++++++++++++++++++ lib/netdev-linux-private.h | 1 + lib/netdev-linux.c | 35 +++++++++++++++++++++++++++++++++++ lib/netdev-provider.h | 7 +++++++ lib/netdev.c | 30 +++++++++++++++++++++++------- ofproto/ofproto-dpif-sflow.c | 7 +++---- tests/system-interface.at | 12 ++++++++++++ vswitchd/bridge.c | 8 +++----- 10 files changed, 131 insertions(+), 22 deletions(-) diff --git a/include/openvswitch/netdev.h b/include/openvswitch/netdev.h index 83e8633dda..f79bfd1cdd 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 *); 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 c7cd59376a..26e1286ac0 100644 --- a/lib/netdev-bsd.c +++ b/lib/netdev-bsd.c @@ -92,6 +92,7 @@ struct netdev_bsd { struct eth_addr etheraddr; int mtu; int carrier; + bool full_duplex; /* Cached value, see netdev_bsd_get_features(). */ int tap_fd; /* TAP character device, if any, otherwise -1. */ @@ -1107,10 +1108,11 @@ netdev_bsd_parse_media(int media) * passed-in values are set to 0. */ static int -netdev_bsd_get_features(const struct netdev *netdev, +netdev_bsd_get_features(const struct netdev *netdev_, enum netdev_features *current, uint32_t *advertised, enum netdev_features *supported, uint32_t *peer) { + struct netdev_bsd *netdev = netdev_bsd_cast(netdev_); struct ifmediareq ifmr; int *media_list; int i; @@ -1120,7 +1122,7 @@ netdev_bsd_get_features(const struct netdev *netdev, /* XXX Look into SIOCGIFCAP instead of SIOCGIFMEDIA */ memset(&ifmr, 0, sizeof(ifmr)); - ovs_strlcpy(ifmr.ifm_name, netdev_get_name(netdev), sizeof ifmr.ifm_name); + ovs_strlcpy(ifmr.ifm_name, netdev_get_name(netdev_), sizeof ifmr.ifm_name); /* We make two SIOCGIFMEDIA ioctl calls. The first to determine the * number of supported modes, and a second with a buffer to retrieve @@ -1128,7 +1130,7 @@ netdev_bsd_get_features(const struct netdev *netdev, error = af_inet_ioctl(SIOCGIFMEDIA, &ifmr); if (error) { VLOG_DBG_RL(&rl, "%s: ioctl(SIOCGIFMEDIA) failed: %s", - netdev_get_name(netdev), ovs_strerror(error)); + netdev_get_name(netdev_), ovs_strerror(error)); return error; } @@ -1137,7 +1139,7 @@ netdev_bsd_get_features(const struct netdev *netdev, if (IFM_TYPE(ifmr.ifm_current) != IFM_ETHER) { VLOG_DBG_RL(&rl, "%s: doesn't appear to be ethernet", - netdev_get_name(netdev)); + netdev_get_name(netdev_)); error = EINVAL; goto cleanup; } @@ -1145,7 +1147,7 @@ netdev_bsd_get_features(const struct netdev *netdev, error = af_inet_ioctl(SIOCGIFMEDIA, &ifmr); if (error) { VLOG_DBG_RL(&rl, "%s: ioctl(SIOCGIFMEDIA) failed: %s", - netdev_get_name(netdev), ovs_strerror(error)); + netdev_get_name(netdev_), ovs_strerror(error)); goto cleanup; } @@ -1154,6 +1156,7 @@ netdev_bsd_get_features(const struct netdev *netdev, if (!*current) { *current = NETDEV_F_OTHER; } + netdev->full_duplex = ifmr.ifm_active & IFM_FDX; /* Advertised features. */ *advertised = netdev_bsd_parse_media(ifmr.ifm_current); @@ -1194,6 +1197,23 @@ netdev_bsd_get_speed(const struct netdev *netdev, uint32_t *current, return 0; } +static int +netdev_bsd_get_duplex(const struct netdev *netdev_, bool *full_duplex) +{ + enum netdev_features f_current, f_supported, f_advertised, f_peer; + struct netdev_bsd *netdev = netdev_bsd_cast(netdev_); + int error; + + error = netdev_bsd_get_features(netdev_, &f_current, &f_advertised, + &f_supported, &f_peer); + if (error) { + return error; + } + + *full_duplex = netdev->full_duplex; + return 0; +} + /* * 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 @@ -1522,6 +1542,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..5692111b32 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -4177,6 +4177,25 @@ 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; + + 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; + err = 0; + } 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 +6908,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 5dad2e7ed7..fb3cbf0597 100644 --- a/lib/netdev-linux.c +++ b/lib/netdev-linux.c @@ -2682,6 +2682,7 @@ netdev_linux_read_features(struct netdev_linux *netdev) } else { netdev->current = NETDEV_F_OTHER; } + netdev->current_duplex = ecmd.duplex; if (ecmd.port == PORT_TP) { netdev->current |= NETDEV_F_COPPER; @@ -2765,6 +2766,38 @@ netdev_linux_get_speed(const struct netdev *netdev_, uint32_t *current, return error; } +static int +netdev_linux_get_duplex_locked(struct netdev_linux *netdev, bool *full_duplex) +{ + int err; + + if (netdev_linux_netnsid_is_remote(netdev)) { + return EOPNOTSUPP; + } + + netdev_linux_read_features(netdev); + err = netdev->get_features_error; + if (!err && netdev->current_duplex == DUPLEX_UNKNOWN) { + err = EOPNOTSUPP; + } + if (!err) { + *full_duplex = netdev->current_duplex == DUPLEX_FULL; + } + return err; +} + +static int +netdev_linux_get_duplex(const struct netdev *netdev_, bool *full_duplex) +{ + struct netdev_linux *netdev = netdev_linux_cast(netdev_); + int error; + + ovs_mutex_lock(&netdev->mutex); + error = netdev_linux_get_duplex_locked(netdev, full_duplex); + ovs_mutex_unlock(&netdev->mutex); + return error; +} + /* Set the features advertised by 'netdev' to 'advertise'. */ static int netdev_linux_set_advertisements(struct netdev *netdev_, @@ -3889,6 +3922,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, @@ -3906,6 +3940,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..f32003a95c 100644 --- a/lib/netdev.c +++ b/lib/netdev.c @@ -1288,14 +1288,30 @@ 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) +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; + + 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..7b49d1b037 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -2532,9 +2532,9 @@ iface_refresh_netdev_status(struct iface *iface) { struct smap smap; - enum netdev_features current; enum netdev_flags flags; const char *link_state; + bool full_duplex; struct eth_addr mac; int64_t bps, mtu_64, ifindex64, link_resets; int mtu, error; @@ -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.0 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
