Set veth MTU using CALICO_LIBNETWORK_VETH_MTU

[ti-mo]

Description

Ref. https://github.com/projectcalico/calicoctl/issues/488

I've introduced an environment variable CALICO_LIBNETWORK_VETH_MTU to set the MTU of the veth pairs created by the Calico libnetwork plugin. As discussed in the issue above, this allows FELIX_IPINIPMTU to be controlled separately from the veth MTU.

Worth noting is that this extends the signature of netns.CreateVeth, and I'm not sure if there are any other packages that use this function.

This was tested manually, could not get the tests on master to run. I'll need some help/pointers on how to add this to the test suite.

Will this ever make it into 2.6, or should I add this to the 3.0 docs?

Todos

• Tests
• Documentation
• Release note

Release Note

MTU of veth endpoints created by calico-libnetwork can now be configured using `CALICO_LIBNETWORK_VETH_MTU`.

[CLAassistant]

All committers have signed the CLA.

[caseydavenport]

Just one minor comment from me - looks good!

I think your tests are failing due to a known issue: #162

Would appreciate a secondary review from @fasaxc or @robbrockbank since I'm not super familiar with this code.

Thanks!

[caseydavenport]

Nit: we generally prefer logs which use WithField (though I know a lot of the logs in this file don't use that approach!)

e.g.

log.WithField("mtu", mtu).Info("Parsed veth MTU")

[caseydavenport]

Just merged #163 which should fix your CI errors if you rebase.

[ti-mo]

@caseydavenport Thanks, tests are passing now, just need a review.

To repeat my original question: is this going to be backported to 2.6 or will it land in 3.0? Where should this be documented?

[fasaxc]

@ti-mo would it be safer to have the default as 0 (i.e. the current behaviour) instead of 1500? Unless we're sure that the kernel's default in all circumstances is 1500 then changing this default might break systems that rely on the kernel's behaviour. WDYT?

[ti-mo]

Crap, I was worried about this. I couldn't find the default anywhere, but it happened to default to 1500 on the interface types (veth) we create. This will definitely not the case for overlay interface types.

There does not seem to be a canonical default defined per interface type in the netlink package either, so, logically, this is up to the kernel to infer when it receives a zero MTU value in a netlink message. I'll just remove this so vethMTU is always 0 unless overridden.

(Since this code only ever creates veths, I don't see this having any impact)

Thanks!

[fasaxc]

Yeah, lets have it default to "what we do now", or to a smaller value: with IPIP enabled, it should probably be 1460 to allow for the IPIP header.

[caseydavenport]

To repeat my original question: is this going to be backported to 2.6 or will it land in 3.0? Where should this be documented?

Sorry for missing that - I think we should get this into a v2.6 patch release. However, this will also make it into v3.1, so we'll want docs in both places.

• v2.6: https://github.com/projectcalico/calico/tree/master/v2.6
• master (v3.1): https://github.com/projectcalico/calico/tree/master/master

[caseydavenport]

@ti-mo if you could squash your commits, I'd be happy to merge (provided @fasaxc has no more feedback)

[ti-mo]

@caseydavenport Done, working on the documentation!