Re: [PATCH] ipvlan: fix ipvlan MTU limits

On Mon, Jan 8, 2018 at 10:48 PM,  [off-list ref] wrote:

quoted

From: Keefe Liu <redacted>

The MTU of ipvlan interface should not bigger than the phy device,
When we run following scripts, we will find there are some problems.
Step1:
        ip link add link eth0 name ipv1 type ipvlan mode l2
        ip netns add net1
        ip link set dev ipv1 netns net1
Step2:
        ip netns exec net1 ip link set dev ipv1 mtu 1501
        RTNETLINK answers: Invalid argument
        dmesg info: "ipv1: Invalid MTU 1501 requested, hw max 1500"
Step3:
        ip link set dev eth0 mtu 1600
        ip netns exec net1 ip link set dev ipv1 mtu 1501
        RTNETLINK answers: Invalid argument
        dmesg info: "ipv1: Invalid MTU 1501 requested, hw max 1500"
Step4:
        ip link set dev eth0 mtu 1400
        ip netns exec net1 ip link set dev ipv1 mtu 1500 The result of
Step2 is we expected, but the result of Step3 and Step4 are not.

This patch set ipvlan's maximum MTU to ETH_MAX_MTU, and when we

change

quoted

the ipvlan device's MTU, ipvlan_change_mtu() will make sure the new
MTU no larger than the phy device's MTU.

Signed-off-by: Keefe Liu <redacted>
---
 drivers/net/ipvlan/ipvlan_main.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/net/ipvlan/ipvlan_main.c

b/drivers/net/ipvlan/ipvlan_main.c
index 30cb803..84c007d 100644

--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c

@@ -380,12 +380,24 @@ static int ipvlan_get_iflink(const struct net_device

*dev)

quoted

        return ipvlan->phy_dev->ifindex;  }

+static int ipvlan_change_mtu(struct net_device *dev, int new_mtu) {
+       struct ipvl_dev *ipvlan = netdev_priv(dev);
+
+       if (ipvlan->phy_dev->mtu < new_mtu)
+               return -EINVAL;
+
+       dev->mtu = new_mtu;
+       return 0;
+}
+
 static const struct net_device_ops ipvlan_netdev_ops = {
        .ndo_init               = ipvlan_init,
        .ndo_uninit             = ipvlan_uninit,
        .ndo_open               = ipvlan_open,
        .ndo_stop               = ipvlan_stop,
        .ndo_start_xmit         = ipvlan_start_xmit,
+       .ndo_change_mtu         = ipvlan_change_mtu,
        .ndo_fix_features       = ipvlan_fix_features,
        .ndo_change_rx_flags    = ipvlan_change_rx_flags,
        .ndo_set_rx_mode        = ipvlan_set_multicast_mac_filter,

@@ -680,6 +692,8 @@ void ipvlan_link_setup(struct net_device *dev)  {
        ether_setup(dev);

+       dev->min_mtu = 0;

should be ETH_MIN_MTU since we expect the underlying device to be
ETH_ARPHDR and IPvlan deals with IPv4/IPv6.

[liuqifa] Yes, I agree with you. But I have another question, I found in commit 9157208,  macvlan's min_mtu has been changed from ETH_MIN_MTU to 0, I'd like to know what's the difference between macvlan and ipvlan.

quoted

+       dev->max_mtu = ETH_MAX_MTU;
        dev->priv_flags &= ~(IFF_XMIT_DST_RELEASE | IFF_TX_SKB_SHARING);
        dev->priv_flags |= IFF_UNICAST_FLT | IFF_NO_QUEUE;
        dev->netdev_ops = &ipvlan_netdev_ops;
--
1.8.3.1

These changes are not sufficient if you want to have different per-slave mtu
settings. One can always change the MTU of the master device and all  per-
slave settings will get wiped. I don't think that's a desired outcome.

[liuqifa] I agree with you, I will reprogram the code to realize this goal.