datapath: Strip down vport interface : OVS_VPORT_ATTR_MTU

There is no need to have vport attribute MTU (OVS_VPORT_ATTR_MTU) as
linux net-dev-ioctl can be used to get/set MTU for linux device.
Following patch removes OVS_VPORT_ATTR_MTU from datapath protocol.

This patch also adds netdev_set_mtu interface. So that MTU adjustments
can be done from OVS userspace. get_mtu() interface is also changed, now
get_mtu() returns EOPNOTSUPP rather than returning 0 and setting *pmtu
to INT_MAX in case there is no MTU attribute for given device.

Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>

diff --git a/datapath/datapath.c b/datapath/datapath.c
index 6abd6918571b31487da6d91a191e701ef7487589..79df5f8988383ce52c0b1927a334a7610a2c93fa 100644 (file)
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -1551,7 +1551,6 @@ static const struct nla_policy vport_policy[OVS_VPORT_ATTR_MAX + 1] = {
        [OVS_VPORT_ATTR_STATS] = { .minlen = sizeof(struct rtnl_link_stats64) },
        [OVS_VPORT_ATTR_ADDRESS] = { .minlen = ETH_ALEN },
 #endif
-       [OVS_VPORT_ATTR_MTU] = { .type = NLA_U32 },
        [OVS_VPORT_ATTR_OPTIONS] = { .type = NLA_NESTED },
 };
 
@@ -1574,7 +1573,6 @@ static int ovs_vport_cmd_fill_info(struct vport *vport, struct sk_buff *skb,
        struct ovs_header *ovs_header;
        struct nlattr *nla;
        int ifindex;
-       int mtu;
        int err;
 
        ovs_header = genlmsg_put(skb, pid, seq, &dp_vport_genl_family,
@@ -1596,10 +1594,6 @@ static int ovs_vport_cmd_fill_info(struct vport *vport, struct sk_buff *skb,
 
        NLA_PUT(skb, OVS_VPORT_ATTR_ADDRESS, ETH_ALEN, vport_get_addr(vport));
 
-       mtu = vport_get_mtu(vport);
-       if (mtu)
-               NLA_PUT_U32(skb, OVS_VPORT_ATTR_MTU, mtu);
-
        err = vport_get_options(vport, skb);
        if (err == -EMSGSIZE)
                goto error;
@@ -1679,8 +1673,6 @@ static int change_vport(struct vport *vport, struct nlattr *a[OVS_VPORT_ATTR_MAX
                err = vport_set_stats(vport, nla_data(a[OVS_VPORT_ATTR_STATS]));
        if (!err && a[OVS_VPORT_ATTR_ADDRESS])
                err = vport_set_addr(vport, nla_data(a[OVS_VPORT_ATTR_ADDRESS]));
-       if (!err && a[OVS_VPORT_ATTR_MTU])
-               err = vport_set_mtu(vport, nla_get_u32(a[OVS_VPORT_ATTR_MTU]));
        return err;
 }
 

diff --git a/include/openvswitch/datapath-protocol.h b/include/openvswitch/datapath-protocol.h
index c1c9ef04ec17f2f72dd5a2d75ba2bcec8c301192..5687792dff708408048e2afcc70c38d04ddaff7f 100644 (file)
--- a/include/openvswitch/datapath-protocol.h
+++ b/include/openvswitch/datapath-protocol.h
@@ -222,8 +222,6 @@ enum ovs_vport_cmd {
  * @OVS_VPORT_ATTR_STATS: A &struct rtnl_link_stats64 giving statistics for
  * packets sent or received through the vport.
  * @OVS_VPORT_ATTR_ADDRESS: A 6-byte Ethernet address for the vport.
- * @OVS_VPORT_ATTR_MTU: MTU for the vport.  Omitted if the vport does not have
- * an MTU as, e.g., some tunnels do not.
  * @OVS_VPORT_ATTR_IFINDEX: ifindex of the underlying network device, if any.
  *
  * These attributes follow the &struct ovs_header within the Generic Netlink
@@ -237,8 +235,8 @@ enum ovs_vport_cmd {
  * %OVS_VPORT_ATTR_NAME attributes are required.  %OVS_VPORT_ATTR_PORT_NO is
  * optional; if not specified a free port number is automatically selected.
  * Whether %OVS_VPORT_ATTR_OPTIONS is required or optional depends on the type
- * of vport.  %OVS_VPORT_ATTR_STATS, %OVS_VPORT_ATTR_ADDRESS, and
- * %OVS_VPORT_ATTR_MTU are optional, and other attributes are ignored.
+ * of vport.  %OVS_VPORT_ATTR_STATS and %OVS_VPORT_ATTR_ADDRESS are optional,
+ * and other attributes are ignored.
  *
  * For other requests, if %OVS_VPORT_ATTR_NAME is specified then it is used to
  * look up the vport to operate on; otherwise dp_idx from the &struct
@@ -251,7 +249,6 @@ enum ovs_vport_attr {
        OVS_VPORT_ATTR_NAME,    /* string name, up to IFNAMSIZ bytes long */
        OVS_VPORT_ATTR_STATS,   /* struct rtnl_link_stats64 */
        OVS_VPORT_ATTR_ADDRESS, /* hardware address */
-       OVS_VPORT_ATTR_MTU,     /* 32-bit maximum transmission unit */
        OVS_VPORT_ATTR_OPTIONS, /* nested attributes, varies by vport type */
        OVS_VPORT_ATTR_IFINDEX, /* 32-bit ifindex of backing netdev */
        __OVS_VPORT_ATTR_MAX

diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
index 8d655f35597d64992e114e4f57c36d296a8e8640..9533e1489a83aadd84f36eb77e6c3811ec9ab340 100644 (file)
--- a/lib/dpif-linux.c
+++ b/lib/dpif-linux.c
@@ -1238,7 +1238,6 @@ dpif_linux_vport_from_ofpbuf(struct dpif_linux_vport *vport,
                                      .min_len = ETH_ADDR_LEN,
                                      .max_len = ETH_ADDR_LEN,
                                      .optional = true },
-        [OVS_VPORT_ATTR_MTU] = { .type = NL_A_U32, .optional = true },
         [OVS_VPORT_ATTR_OPTIONS] = { .type = NL_A_NESTED, .optional = true },
         [OVS_VPORT_ATTR_IFINDEX] = { .type = NL_A_U32, .optional = true },
     };
@@ -1273,11 +1272,6 @@ dpif_linux_vport_from_ofpbuf(struct dpif_linux_vport *vport,
     if (a[OVS_VPORT_ATTR_ADDRESS]) {
         vport->address = nl_attr_get(a[OVS_VPORT_ATTR_ADDRESS]);
     }
-    if (a[OVS_VPORT_ATTR_MTU]) {
-        vport->mtu = nl_attr_get_u32(a[OVS_VPORT_ATTR_MTU]);
-    } else {
-        vport->mtu = INT_MAX;
-    }
     if (a[OVS_VPORT_ATTR_OPTIONS]) {
         vport->options = nl_attr_get(a[OVS_VPORT_ATTR_OPTIONS]);
         vport->options_len = nl_attr_get_size(a[OVS_VPORT_ATTR_OPTIONS]);
@@ -1324,10 +1318,6 @@ dpif_linux_vport_to_ofpbuf(const struct dpif_linux_vport *vport,
                           vport->address, ETH_ADDR_LEN);
     }
 
-    if (vport->mtu && vport->mtu != INT_MAX) {
-        nl_msg_put_u32(buf, OVS_VPORT_ATTR_MTU, vport->mtu);
-    }
-
     if (vport->options) {
         nl_msg_put_nested(buf, OVS_VPORT_ATTR_OPTIONS,
                           vport->options, vport->options_len);

diff --git a/lib/dpif-linux.h b/lib/dpif-linux.h
index 3a2c816abca366f5e2c703eb5d041a65496ebec8..c673beeb3c1c42c3d8fdaab3c7ded514d05ab16b 100644 (file)
--- a/lib/dpif-linux.h
+++ b/lib/dpif-linux.h
@@ -36,7 +36,6 @@ struct dpif_linux_vport {
     const char *name;                      /* OVS_VPORT_ATTR_NAME. */
     const struct rtnl_link_stats64 *stats; /* OVS_VPORT_ATTR_STATS. */
     const uint8_t *address;                /* OVS_VPORT_ATTR_ADDRESS. */
-    int mtu;                               /* OVS_VPORT_ATTR_MTU. */
     const struct nlattr *options;          /* OVS_VPORT_ATTR_OPTIONS. */
     size_t options_len;
     int ifindex;                           /* OVS_VPORT_ATTR_IFINDEX. */

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 8aefaf9b0340fa45e1fb6ec792af096e2ccc4de7..b48be86173c2c486de1948164209c042cc453b40 100644 (file)
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -378,8 +378,8 @@ do_add_port(struct dp_netdev *dp, const char *devname, const char *type,
     port->netdev = netdev;
     port->internal = internal;
 
-    netdev_get_mtu(netdev, &mtu);
-    if (mtu != INT_MAX && mtu > max_mtu) {
+    error = netdev_get_mtu(netdev, &mtu);
+    if (!error) {
         max_mtu = mtu;
     }
 

diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
index 4fb11514cbc67ec9fa553793e27877d7cdc9a067..c2c531116cc4426a60490f9b683da741d94deeb0 100644 (file)
--- a/lib/netdev-dummy.c
+++ b/lib/netdev-dummy.c
@@ -171,6 +171,16 @@ netdev_dummy_get_mtu(const struct netdev *netdev, int *mtup)
     return 0;
 }
 
+static int
+netdev_dummy_set_mtu(const struct netdev *netdev, int mtu)
+{
+    struct netdev_dev_dummy *dev =
+        netdev_dev_dummy_cast(netdev_get_dev(netdev));
+
+    dev->mtu = mtu;
+    return 0;
+}
+
 static int
 netdev_dummy_get_stats(const struct netdev *netdev, struct netdev_stats *stats)
 {
@@ -259,6 +269,7 @@ static const struct netdev_class dummy_class = {
     netdev_dummy_set_etheraddr,
     netdev_dummy_get_etheraddr,
     netdev_dummy_get_mtu,
+    netdev_dummy_set_mtu,
     NULL,                       /* get_ifindex */
     NULL,                       /* get_carrier */
     NULL,                       /* get_miimon */

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 6d44581bf4ccaaebfaaa7a9e7a5c07e5f5d96695..fc61d452d266915304988ce1ca0510cb3fa5f06c 100644 (file)
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -1000,6 +1000,29 @@ netdev_linux_get_mtu(const struct netdev *netdev_, int *mtup)
     return 0;
 }
 
+/* Sets the maximum size of transmitted (MTU) for given device using linux
+ * networking ioctl interface.
+ */
+static int
+netdev_linux_set_mtu(const struct netdev *netdev_, int mtu)
+{
+    struct netdev_dev_linux *netdev_dev =
+                                netdev_dev_linux_cast(netdev_get_dev(netdev_));
+    struct ifreq ifr;
+    int error;
+
+    ifr.ifr_mtu = mtu;
+    error = netdev_linux_do_ioctl(netdev_get_name(netdev_), &ifr,
+                                  SIOCSIFMTU, "SIOCSIFMTU");
+    if (error) {
+        return error;
+    }
+
+    netdev_dev->mtu = ifr.ifr_mtu;
+    netdev_dev->cache_valid |= VALID_MTU;
+    return 0;
+}
+
 /* Returns the ifindex of 'netdev', if successful, as a positive number.
  * On failure, returns a negative errno value. */
 static int
@@ -2269,6 +2292,7 @@ netdev_linux_change_seq(const struct netdev *netdev)
     netdev_linux_set_etheraddr,                                 \
     netdev_linux_get_etheraddr,                                 \
     netdev_linux_get_mtu,                                       \
+    netdev_linux_set_mtu,                                       \
     netdev_linux_get_ifindex,                                   \
     netdev_linux_get_carrier,                                   \
     netdev_linux_set_miimon_interval,                           \
@@ -2412,11 +2436,11 @@ htb_setup_class__(struct netdev *netdev, unsigned int handle,
     int error;
     int mtu;
 
-    netdev_get_mtu(netdev, &mtu);
-    if (mtu == INT_MAX) {
+    error = netdev_get_mtu(netdev, &mtu);
+    if (error) {
         VLOG_WARN_RL(&rl, "cannot set up HTB on device %s that lacks MTU",
                      netdev_get_name(netdev));
-        return EINVAL;
+        return error;
     }
 
     memset(&opt, 0, sizeof opt);
@@ -2535,13 +2559,13 @@ htb_parse_class_details__(struct netdev *netdev,
     const char *max_rate_s = shash_find_data(details, "max-rate");
     const char *burst_s = shash_find_data(details, "burst");
     const char *priority_s = shash_find_data(details, "priority");
-    int mtu;
+    int mtu, error;
 
-    netdev_get_mtu(netdev, &mtu);
-    if (mtu == INT_MAX) {
+    error = netdev_get_mtu(netdev, &mtu);
+    if (error) {
         VLOG_WARN_RL(&rl, "cannot parse HTB class on device %s that lacks MTU",
                      netdev_get_name(netdev));
-        return EINVAL;
+        return error;
     }
 
     /* HTB requires at least an mtu sized min-rate to send any traffic even

diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h
index 5214be3fdbb5cfef4400b2999cea25ab82aca7d4..b8f65291927c2d334d988c4788c96f31d9dac288 100644 (file)
--- a/lib/netdev-provider.h
+++ b/lib/netdev-provider.h
@@ -248,9 +248,15 @@ struct netdev_class {
      * bytes for Ethernet devices.
      *
      * If 'netdev' does not have an MTU (e.g. as some tunnels do not), then
-     * this function should set '*mtup' to INT_MAX. */
+     * this function should return EOPNOTSUPP. */
     int (*get_mtu)(const struct netdev *netdev, int *mtup);
 
+    /* Sets 'netdev''s MTU to 'mtu'.
+     *
+     * If 'netdev' does not have an MTU (e.g. as some tunnels do not), then
+     * this function should return EOPNOTSUPP. */
+    int (*set_mtu)(const struct netdev *netdev, int mtu);
+
     /* Returns the ifindex of 'netdev', if successful, as a positive number.
      * On failure, returns a negative errno value.
      *

diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index c4c1e6b8de61fed11a2b8ecfe5a84363a4da78bf..f215ec6e06e9c34fced530b3dba3cd46dfdb5ec6 100644 (file)
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -362,18 +362,17 @@ netdev_vport_get_etheraddr(const struct netdev *netdev,
 }
 
 static int
-netdev_vport_get_mtu(const struct netdev *netdev, int *mtup)
+netdev_vport_get_mtu(const struct netdev *netdev OVS_UNUSED,
+                     int *mtup OVS_UNUSED)
 {
-    struct dpif_linux_vport reply;
-    struct ofpbuf *buf;
-    int error;
+    return EOPNOTSUPP;
+}
 
-    error = dpif_linux_vport_get(netdev_get_name(netdev), &reply, &buf);
-    if (!error) {
-        *mtup = reply.mtu;
-        ofpbuf_delete(buf);
-    }
-    return error;
+static int
+netdev_vport_set_mtu(const struct netdev *netdev OVS_UNUSED,
+                     int mtu OVS_UNUSED)
+{
+    return EOPNOTSUPP;
 }
 
 int
@@ -874,6 +873,7 @@ unparse_patch_config(const char *name OVS_UNUSED, const char *type OVS_UNUSED,
     netdev_vport_set_etheraddr,                             \
     netdev_vport_get_etheraddr,                             \
     netdev_vport_get_mtu,                                   \
+    netdev_vport_set_mtu,                                   \
     NULL,                       /* get_ifindex */           \
     NULL,                       /* get_carrier */           \
     NULL,                       /* get_miimon */            \

diff --git a/lib/netdev.c b/lib/netdev.c
index 12ac81d6edf0b67a0b263f25f142f7925efbf333..9fba077b6982c9f31e5db6cd1b5c54b1c61ed93f 100644 (file)
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -510,19 +510,36 @@ netdev_get_name(const struct netdev *netdev)
  * (and received) packets, in bytes, not including the hardware header; thus,
  * this is typically 1500 bytes for Ethernet devices.
  *
- * If successful, returns 0 and stores the MTU size in '*mtup'.  Stores INT_MAX
- * in '*mtup' if 'netdev' does not have an MTU (as e.g. some tunnels do not).On
- * failure, returns a positive errno value and stores ETH_PAYLOAD_MAX (1500) in
- * '*mtup'. */
+ * If successful, returns 0 and stores the MTU size in '*mtup'.  Returns
+ * EOPNOTSUPP if 'netdev' does not have an MTU (as e.g. some tunnels do not).
+ * On other failure, returns a positive errno value. */
 int
 netdev_get_mtu(const struct netdev *netdev, int *mtup)
 {
     int error = netdev_get_dev(netdev)->netdev_class->get_mtu(netdev, mtup);
-    if (error) {
+    if (error && error != EOPNOTSUPP) {
+        VLOG_WARN_RL(&rl, "failed to retrieve MTU for network device %s: %s",
+                     netdev_get_name(netdev), strerror(error));
+    }
+    return error;
+}
+
+/* Sets the MTU of 'netdev'.  The MTU is the maximum size of transmitted
+ * (and received) packets, in bytes.
+ *
+ * If successful, returns 0.  Returns EOPNOTSUPP if 'netdev' does not have an
+ * MTU (as e.g. some tunnels do not).  On other failure, returns a positive
+ * errno value. */
+int
+netdev_set_mtu(const struct netdev *netdev, int mtu)
+{
+    int error = netdev_get_dev(netdev)->netdev_class->set_mtu(netdev, mtu);
+
+    if (error && error != EOPNOTSUPP) {
         VLOG_WARN_RL(&rl, "failed to retrieve MTU for network device %s: %s",
                      netdev_get_name(netdev), strerror(error));
-        *mtup = ETH_PAYLOAD_MAX;
     }
+
     return error;
 }
 

diff --git a/lib/netdev.h b/lib/netdev.h
index 13d2ee76e8e66ae54fd4a5d17d81577c1a4490b5..2644708b3520fbe92d06fd94fa4a658ef1ec04c1 100644 (file)
--- a/lib/netdev.h
+++ b/lib/netdev.h
@@ -100,6 +100,7 @@ int netdev_get_config(const struct netdev *, struct shash *);
 const char *netdev_get_name(const struct netdev *);
 const char *netdev_get_type(const struct netdev *);
 int netdev_get_mtu(const struct netdev *, int *mtup);
+int netdev_set_mtu(const struct netdev *, int mtu);
 int netdev_get_ifindex(const struct netdev *);
 
 /* Packet send and receive. */

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index ba5fbc6fe01e39b3d3a71ba46477cf65e4180f3a..f1c306d48dcc365f04b6d0a57535b1e30899cac7 100644 (file)
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -1248,7 +1248,7 @@ iface_refresh_status(struct iface *iface)
                                     iface_get_carrier(iface) ? "up" : "down");
 
     error = netdev_get_mtu(iface->netdev, &mtu);
-    if (!error && mtu != INT_MAX) {
+    if (!error) {
         mtu_64 = mtu;
         ovsrec_interface_set_mtu(iface->cfg, &mtu_64, 1);
     }