From 9b02078077b62e4277e84c7f39382ce09986cf6b Mon Sep 17 00:00:00 2001 From: Pravin Shelar Date: Mon, 12 Sep 2011 17:12:52 -0700 Subject: [PATCH] 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 Acked-by: Jesse Gross --- datapath/datapath.c | 8 ------ include/openvswitch/datapath-protocol.h | 7 ++--- lib/dpif-linux.c | 10 ------- lib/dpif-linux.h | 1 - lib/dpif-netdev.c | 4 +-- lib/netdev-dummy.c | 11 +++++++ lib/netdev-linux.c | 38 ++++++++++++++++++++----- lib/netdev-provider.h | 8 +++++- lib/netdev-vport.c | 20 ++++++------- lib/netdev.c | 29 +++++++++++++++---- lib/netdev.h | 1 + vswitchd/bridge.c | 2 +- 12 files changed, 88 insertions(+), 51 deletions(-) diff --git a/datapath/datapath.c b/datapath/datapath.c index 6abd6918..79df5f89 100644 --- 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 c1c9ef04..5687792d 100644 --- 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 8d655f35..9533e148 100644 --- 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 3a2c816a..c673beeb 100644 --- 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 8aefaf9b..b48be861 100644 --- 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 4fb11514..c2c53111 100644 --- 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 6d44581b..fc61d452 100644 --- 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 5214be3f..b8f65291 100644 --- 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 c4c1e6b8..f215ec6e 100644 --- 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 12ac81d6..9fba077b 100644 --- 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 13d2ee76..2644708b 100644 --- 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 ba5fbc6f..f1c306d4 100644 --- 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); } -- 2.30.2