From 9197df76b46ff6fbe1f7a522961730ffc55a860d Mon Sep 17 00:00:00 2001 From: Justin Pettit Date: Mon, 12 Sep 2011 22:13:30 -0700 Subject: [PATCH] Set MTU in userspace rather than kernel. Currently the kernel automatically sets the MTU of any internal interfaces to the minimum of all attached interfaces because the Linux bridge does this. Userspace can do this with more knowledge and flexibility. Feature #7323 Signed-off-by: Justin Pettit Acked-by: Jesse Gross --- datapath/datapath.c | 47 ------------------------- datapath/datapath.h | 2 -- datapath/dp_notify.c | 5 --- datapath/vport-internal_dev.c | 6 ---- datapath/vport-netdev.c | 7 ---- datapath/vport-netdev.h | 1 - datapath/vport.c | 30 ---------------- datapath/vport.h | 3 -- ofproto/ofproto-provider.h | 1 + ofproto/ofproto.c | 65 +++++++++++++++++++++++++++++++++++ 10 files changed, 66 insertions(+), 101 deletions(-) diff --git a/datapath/datapath.c b/datapath/datapath.c index 232b26504..0238c5f3b 100644 --- a/datapath/datapath.c +++ b/datapath/datapath.c @@ -749,52 +749,6 @@ static void get_dp_stats(struct datapath *dp, struct ovs_dp_stats *stats) } } -/* MTU of the dp pseudo-device: ETH_DATA_LEN or the minimum of the ports. - * Called with RTNL lock. - */ -int dp_min_mtu(const struct datapath *dp) -{ - struct vport *p; - int mtu = 0; - - ASSERT_RTNL(); - - list_for_each_entry (p, &dp->port_list, node) { - int dev_mtu; - - /* Skip any internal ports, since that's what we're trying to - * set. */ - if (is_internal_vport(p)) - continue; - - dev_mtu = vport_get_mtu(p); - if (!dev_mtu) - continue; - if (!mtu || dev_mtu < mtu) - mtu = dev_mtu; - } - - return mtu ? mtu : ETH_DATA_LEN; -} - -/* Sets the MTU of all datapath devices to the minimum of the ports - * Called with RTNL lock. - */ -void set_internal_devs_mtu(const struct datapath *dp) -{ - struct vport *p; - int mtu; - - ASSERT_RTNL(); - - mtu = dp_min_mtu(dp); - - list_for_each_entry (p, &dp->port_list, node) { - if (is_internal_vport(p)) - vport_set_mtu(p, mtu); - } -} - static const struct nla_policy flow_policy[OVS_FLOW_ATTR_MAX + 1] = { [OVS_FLOW_ATTR_KEY] = { .type = NLA_NESTED }, [OVS_FLOW_ATTR_ACTIONS] = { .type = NLA_NESTED }, @@ -1740,7 +1694,6 @@ static int ovs_vport_cmd_new(struct sk_buff *skb, struct genl_info *info) if (IS_ERR(vport)) goto exit_unlock; - set_internal_devs_mtu(dp); dp_sysfs_add_if(vport); err = change_vport(vport, a); diff --git a/datapath/datapath.h b/datapath/datapath.h index 4b5f7ec5e..ff8ac104d 100644 --- a/datapath/datapath.h +++ b/datapath/datapath.h @@ -150,8 +150,6 @@ extern int (*dp_ioctl_hook)(struct net_device *dev, struct ifreq *rq, int cmd); void dp_process_received_packet(struct vport *, struct sk_buff *); void dp_detach_port(struct vport *); int dp_upcall(struct datapath *, struct sk_buff *, const struct dp_upcall_info *); -int dp_min_mtu(const struct datapath *dp); -void set_internal_devs_mtu(const struct datapath *dp); struct datapath *get_dp(int dp_idx); const char *dp_name(const struct datapath *dp); diff --git a/datapath/dp_notify.c b/datapath/dp_notify.c index 7b3b219ac..942d628cb 100644 --- a/datapath/dp_notify.c +++ b/datapath/dp_notify.c @@ -59,11 +59,6 @@ static int dp_device_event(struct notifier_block *unused, unsigned long event, dp_sysfs_add_if(vport); } break; - - case NETDEV_CHANGEMTU: - if (!is_internal_dev(dev)) - set_internal_devs_mtu(dp); - break; } return NOTIFY_DONE; } diff --git a/datapath/vport-internal_dev.c b/datapath/vport-internal_dev.c index 82079bd11..04f51eb33 100644 --- a/datapath/vport-internal_dev.c +++ b/datapath/vport-internal_dev.c @@ -117,14 +117,9 @@ static const struct ethtool_ops internal_dev_ethtool_ops = { static int internal_dev_change_mtu(struct net_device *netdev, int new_mtu) { - struct vport *vport = internal_dev_get_vport(netdev); - if (new_mtu < 68) return -EINVAL; - if (new_mtu > dp_min_mtu(vport->dp)) - return -EINVAL; - netdev->mtu = new_mtu; return 0; } @@ -274,7 +269,6 @@ const struct vport_ops internal_vport_ops = { .flags = VPORT_F_REQUIRED | VPORT_F_GEN_STATS | VPORT_F_FLOW, .create = internal_dev_create, .destroy = internal_dev_destroy, - .set_mtu = netdev_set_mtu, .set_addr = netdev_set_addr, .get_name = netdev_get_name, .get_addr = netdev_get_addr, diff --git a/datapath/vport-netdev.c b/datapath/vport-netdev.c index 478f2716b..7cd6c6b3b 100644 --- a/datapath/vport-netdev.c +++ b/datapath/vport-netdev.c @@ -189,12 +189,6 @@ static void netdev_destroy(struct vport *vport) vport_free(vport); } -int netdev_set_mtu(struct vport *vport, int mtu) -{ - struct netdev_vport *netdev_vport = netdev_vport_priv(vport); - return dev_set_mtu(netdev_vport->dev, mtu); -} - int netdev_set_addr(struct vport *vport, const unsigned char *addr) { struct netdev_vport *netdev_vport = netdev_vport_priv(vport); @@ -422,7 +416,6 @@ const struct vport_ops netdev_vport_ops = { .exit = netdev_exit, .create = netdev_create, .destroy = netdev_destroy, - .set_mtu = netdev_set_mtu, .set_addr = netdev_set_addr, .get_name = netdev_get_name, .get_addr = netdev_get_addr, diff --git a/datapath/vport-netdev.h b/datapath/vport-netdev.h index 3f64767dc..7a743d24b 100644 --- a/datapath/vport-netdev.h +++ b/datapath/vport-netdev.h @@ -25,7 +25,6 @@ netdev_vport_priv(const struct vport *vport) return vport_priv(vport); } -int netdev_set_mtu(struct vport *, int mtu); int netdev_set_addr(struct vport *, const unsigned char *addr); const char *netdev_get_name(const struct vport *); const unsigned char *netdev_get_addr(const struct vport *); diff --git a/datapath/vport.c b/datapath/vport.c index b1418a4c3..ca802b4b2 100644 --- a/datapath/vport.c +++ b/datapath/vport.c @@ -284,36 +284,6 @@ void vport_del(struct vport *vport) vport->ops->destroy(vport); } -/** - * vport_set_mtu - set device MTU (for kernel callers) - * - * @vport: vport on which to set MTU. - * @mtu: New MTU. - * - * Sets the MTU of the given device. Some devices may not support setting the - * MTU, in which case the result will always be -EOPNOTSUPP. RTNL lock must - * be held. - */ -int vport_set_mtu(struct vport *vport, int mtu) -{ - ASSERT_RTNL(); - - if (mtu < 68) - return -EINVAL; - - if (vport->ops->set_mtu) { - int ret; - - ret = vport->ops->set_mtu(vport, mtu); - - if (!ret && !is_internal_vport(vport)) - set_internal_devs_mtu(vport->dp); - - return ret; - } else - return -EOPNOTSUPP; -} - /** * vport_set_addr - set device Ethernet address (for kernel callers) * diff --git a/datapath/vport.h b/datapath/vport.h index bbbb21414..6c4da24fb 100644 --- a/datapath/vport.h +++ b/datapath/vport.h @@ -30,7 +30,6 @@ void vport_del(struct vport *); struct vport *vport_locate(const char *name); -int vport_set_mtu(struct vport *, int mtu); int vport_set_addr(struct vport *, const unsigned char *); int vport_set_stats(struct vport *, struct rtnl_link_stats64 *); @@ -156,7 +155,6 @@ struct vport_parms { * @get_options: Appends vport-specific attributes for the configuration of an * existing vport to a &struct sk_buff. May be %NULL for a vport that does not * have any configuration. - * @set_mtu: Set the device's MTU. May be null if not supported. * @set_addr: Set the device's MAC address. May be null if not supported. * @get_name: Get the device's name. * @get_addr: Get the device's MAC address. @@ -190,7 +188,6 @@ struct vport_ops { int (*set_options)(struct vport *, struct nlattr *); int (*get_options)(const struct vport *, struct sk_buff *); - int (*set_mtu)(struct vport *, int mtu); int (*set_addr)(struct vport *, const unsigned char *); /* Called with rcu_read_lock or RTNL lock. */ diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h index 3037bb2e9..f596abcba 100644 --- a/ofproto/ofproto-provider.h +++ b/ofproto/ofproto-provider.h @@ -93,6 +93,7 @@ struct ofport { struct ofp_phy_port opp; uint16_t ofp_port; /* OpenFlow port number. */ unsigned int change_seq; + int mtu; }; /* An OpenFlow flow within a "struct ofproto". diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 7748ef1b8..1309b492a 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -147,6 +147,7 @@ static int handle_flow_mod__(struct ofproto *, struct ofconn *, static void update_port(struct ofproto *, const char *devname); static int init_ports(struct ofproto *); static void reinit_ports(struct ofproto *); +static void set_internal_devs_mtu(struct ofproto *); static void ofproto_unixctl_init(void); @@ -1193,6 +1194,7 @@ ofport_install(struct ofproto *p, { const char *netdev_name = netdev_get_name(netdev); struct ofport *ofport; + int dev_mtu; int error; /* Create ofport. */ @@ -1211,6 +1213,13 @@ ofport_install(struct ofproto *p, hmap_insert(&p->ports, &ofport->hmap_node, hash_int(ofport->ofp_port, 0)); shash_add(&p->port_by_name, netdev_name, ofport); + if (!netdev_get_mtu(netdev, &dev_mtu)) { + set_internal_devs_mtu(p); + ofport->mtu = dev_mtu; + } else { + ofport->mtu = 0; + } + /* Let the ofproto_class initialize its private data. */ error = p->ofproto_class->port_construct(ofport); if (error) { @@ -1337,12 +1346,22 @@ update_port(struct ofproto *ofproto, const char *name) port = ofproto_get_port(ofproto, ofproto_port.ofp_port); if (port && !strcmp(netdev_get_name(port->netdev), name)) { struct netdev *old_netdev = port->netdev; + int dev_mtu; /* 'name' hasn't changed location. Any properties changed? */ if (!ofport_equal(&port->opp, &opp)) { ofport_modified(port, &opp); } + /* If this is a non-internal port and the MTU changed, check + * if the datapath's MTU needs to be updated. */ + if (strcmp(netdev_get_type(netdev), "internal") + && !netdev_get_mtu(netdev, &dev_mtu) + && port->mtu != dev_mtu) { + set_internal_devs_mtu(ofproto); + port->mtu = dev_mtu; + } + /* Install the newly opened netdev in case it has changed. * Don't close the old netdev yet in case port_modified has to * remove a retained reference to it.*/ @@ -1398,6 +1417,52 @@ init_ports(struct ofproto *p) return 0; } + +/* Find the minimum MTU of all non-datapath devices attached to 'p'. + * Returns ETH_PAYLOAD_MAX or the minimum of the ports. */ +static int +find_min_mtu(struct ofproto *p) +{ + struct ofport *ofport; + int mtu = 0; + + HMAP_FOR_EACH (ofport, hmap_node, &p->ports) { + struct netdev *netdev = ofport->netdev; + int dev_mtu; + + /* Skip any internal ports, since that's what we're trying to + * set. */ + if (!strcmp(netdev_get_type(netdev), "internal")) { + continue; + } + + if (netdev_get_mtu(netdev, &dev_mtu)) { + continue; + } + if (!mtu || dev_mtu < mtu) { + mtu = dev_mtu; + } + } + + return mtu ? mtu: ETH_PAYLOAD_MAX; +} + +/* Set the MTU of all datapath devices on 'p' to the minimum of the + * non-datapath ports. */ +static void +set_internal_devs_mtu(struct ofproto *p) +{ + struct ofport *ofport; + int mtu = find_min_mtu(p); + + HMAP_FOR_EACH (ofport, hmap_node, &p->ports) { + struct netdev *netdev = ofport->netdev; + + if (!strcmp(netdev_get_type(netdev), "internal")) { + netdev_set_mtu(netdev, mtu); + } + } +} static void ofproto_rule_destroy__(struct rule *rule) -- 2.43.0