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 <echaudro@redhat.com>
Acked-by: Mike Pattrick <mkp@redhat.com>
Signed-off-by: David Marchand <david.marchand@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>

diff --git a/include/openvswitch/netdev.h b/include/openvswitch/netdev.h
index 83e8633dda6d5547c7ceba59aeb7771d42260f4d..56dff57725a1858672b094de0e61cd59b549c9b3 100644 (file)
--- 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 cffbe52191161b2fdb2288907c2c26cdf9f69936..d29589efdea70b76ea8b8f6c6cdd57a060075948 100644 (file)
--- 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 45b9f489641e74213b74f00a83cadfc43d9e1991..687e1196b529a3682e89b654494b9d82893aca56 100644 (file)
--- 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 8e572e3b3b1429dae480326c502c693823e46c7b..4b5b606755e9286e3147143552994328e0d2b4d5 100644 (file)
--- 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 1dc64b4e0657a3eda418e3b7fe242a2617a56df4..8bf1a29a0f43b666e0df1c3dd4f355ce87624536 100644 (file)
--- 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 5ae3794699c2a626ef5c4af9dcb88c58113c3ab4..33a68c49c278c41ce89c3f0c198e3b3b36f03ae6 100644 (file)
--- 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 df5b35232d1f16d9d019d7610ce2629059ae1ced..82bb13ce04c6e3c1201c6424cc3eedd1d914e388 100644 (file)
--- 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, &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

diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c
index c5403e27aa0a51924e419880101965c025006a26..e043d7cbc8faf207e3ad4b55c7813120ce6b4147 100644 (file)
--- 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, &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;
     }

diff --git a/tests/system-interface.at b/tests/system-interface.at
index cb0835ad6b1928d2d1e29d6d26fca4ee15fa987b..15c4a0e2e178256f9ce7df0ca2c2ba50f6ba1bff 100644 (file)
--- 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 456b784c01d00c335e426892637f2bc5fe4e78a6..475eefefaaeaf1fc77d6d627c226262d8cd0c048 100644 (file)
--- 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, &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);
     }