netdev: Fix full duplex capability for 25G ports.

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]>
Signed-off-by: Ilya Maximets <[email protected]>

Author: david-marchand

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);
 

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,         \

lib/netdev-dpdk.c

@@ -4176,6 +4176,23 @@ netdev_dpdk_get_speed(const struct netdev *netdev, uint32_t *current,
     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)
 {
@@ -6888,6 +6905,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

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;

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,

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.
      *

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, &current, 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

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, &current, 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;
     }

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

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, &current, 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);
     }