Add support for IFLA_MIN_MTU and IFLA_MAX_MTU in LinkAttrs

[AlekseyMamedoff]

Added attributes MinMTU, MaxMTU to LinkAttrs struct
Added cases for IFLA_MIN_MTU, IFLA_MAX_MTU
Signed-off-by: Aleksey Mamedov alekseymamedoff@gmail.com

[lazysegtree]

@AlekseyMamedoff , Fix for TestXfrmStateWithPcpunumWithoutSADir, the test which is failing in your PR, was recently merged in main. Please rebase with main.

[AlekseyMamedoff]

@lazysegtree Done. Test passed. Thx.

[lazysegtree]

@AlekseyMamedoff
Shouldn't you also add a corresponding method LinkSetMinMTU

If you are adding functionality to parse this attribute via netlink, we should also be able to add it to Links

[lazysegtree]

Also a unit test would be great. Something like

func TestLinkMinMaxMTU(t *testing.T) {
	tearDown := setUpNetlinkTest(t)
	defer tearDown()

	iface := &Dummy{LinkAttrs{
		Name: "foo",
	}}
	if err := LinkAdd(iface); err != nil {
		t.Fatal(err)
	}

	link, err := LinkByName("foo")
	err = LinkSetMinMTU(link, 1000)
	if link.Attrs().MinMTU != 1000 {
		t.Fatal("MinMTU not correct")
	}
}

[aboch]

@AlekseyMamedoff ping

[AlekseyMamedoff]

Hello!
I believe we no need Set funcs and tests for min, max mtu params, cause netlink itself handle it like read-only fields. We can see it, if we dive a bit into netlink source code.

When we ask netlink set some params for dev it will call do_setlink() by the end.

There is handler for IFLA_MTU:

if (tb[IFLA_MTU]) {
		err = netif_set_mtu_ext(dev, nla_get_u32(tb[IFLA_MTU]), extack);
		if (err < 0)
			goto errout;
		status |= DO_SETLINK_MODIFIED;
}

But there is not for IFLA_MIN_MTU or IFLA_MAX_MTU attributes.

So do_setlink() will always fall through directly to the "errout" label and
return 0 (success) without applying any changes.

In your example:

err = LinkSetMinMTU(link, 1000)
if link.Attrs().MinMTU != 1000 {
	t.Fatal("MinMTU not correct")
}

We will never get an error from LinkSetMinMTU(), but always fail when we try check min MTU has changed. The same with max MTU.

P.s. Sorry for delaying with answer.

[EmilyShepherd]

I agree. Adding Setters for MaxMTU and MinMTU sizes does not make sense because they can't be set from userspace. They are a read-only view of what the interface is telling the kernel its minimum or maximum accepted MTU is.

[AlekseyMamedoff]

@aboch ping

[aboch]

Is it possible to extend existing UT to verify these fields are returned, are set?