On Thu, Sep 8, 2011 at 8:03 PM, Pravin Shelar <[email protected]> wrote:
> diff --git a/datapath/datapath.c b/datapath/datapath.c
> index b92c198..aa33f63 100644
> --- a/datapath/datapath.c
> +++ b/datapath/datapath.c
> @@ -130,7 +130,6 @@ static inline size_t br_nlmsg_size(void)
> + nla_total_size(IFNAMSIZ) /* IFLA_IFNAME */
> + nla_total_size(MAX_ADDR_LEN) /* IFLA_ADDRESS */
> + nla_total_size(4) /* IFLA_MASTER */
> - + nla_total_size(4) /* IFLA_MTU */
> + nla_total_size(4) /* IFLA_LINK */
> + nla_total_size(1); /* IFLA_OPERSTATE */
> }
> @@ -163,7 +162,6 @@ static int dp_fill_ifinfo(struct sk_buff *skb,
> NLA_PUT_STRING(skb, IFLA_IFNAME, vport_get_name(port));
> NLA_PUT_U32(skb, IFLA_MASTER,
> vport_get_ifindex(get_vport_protected(dp, OVSP_LOCAL)));
> - NLA_PUT_U32(skb, IFLA_MTU, vport_get_mtu(port));
I think it is much more questionable to drop this from the
compatibility code then IFLA_LINK because this is actually included in
devices that we see. It also seems more likely to actually get used
in practice.
> -/* 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);
> - }
> -}
Until the equivalent functionality is replicated in userspace you
can't remove this from the kernel because it will break things in the
interim.
> diff --git a/datapath/vport-internal_dev.c b/datapath/vport-internal_dev.c
> index f777637..492bef7 100644
> --- a/datapath/vport-internal_dev.c
> +++ b/datapath/vport-internal_dev.c
> -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;
> -}
You can't remove this function because this is what the network stack
calls to change the MTU of the device. Removing it means that nothing
can ever change the MTU.
> diff --git a/include/openvswitch/datapath-protocol.h
> b/include/openvswitch/datapath-protocol.h
> index 97a7c04..adc39d1 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
> @@ -238,7 +236,6 @@ enum ovs_vport_cmd {
> * 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.
Looks like you removed the end of the sentence by accident.
> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
> index 15bf8bd..3bdb022 100644
> --- a/lib/netdev-vport.c
> +++ b/lib/netdev-vport.c
> 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 ENOTSUP;
> +}
All of the other netdev functions return EOPNOTSUPP in this case, so I
think that these functions should do the same.
In dpif-netdev.c there's another instance of the caller of
netdev_get_mtu() checking for INT_MAX.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
