Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

network: physical and macvlan nictype MTU support #5745

Open
wants to merge 14 commits into
base: master
from

Conversation

2 participants
@tomponline
Copy link
Member

commented May 9, 2019

This exposes the network_phys_macvlan_mtu support in LXC for setting a boot time custom MTU on physical and macvlan nic device types and updates tests to expect the MTU to be changed if that feature is present.

It also modifies LXD to restore the MTU and MAC of physical devices if changed by the container's settings or from within the container.

Depends on lxc/lxc#2985

@tomponline tomponline force-pushed the tomponline:tp-mtu-lxc branch 3 times, most recently from 89db217 to 40eddff May 10, 2019

@tomponline tomponline marked this pull request as ready for review May 10, 2019

@tomponline tomponline force-pushed the tomponline:tp-mtu-lxc branch 3 times, most recently from 46156f0 to e7a7485 May 10, 2019

@tomponline

This comment has been minimized.

Copy link
Member Author

commented May 12, 2019

jenkins: test this please

@tomponline tomponline force-pushed the tomponline:tp-mtu-lxc branch 3 times, most recently from 7ce6933 to b0e9f8c May 14, 2019

@tomponline

This comment has been minimized.

Copy link
Member Author

commented May 14, 2019

@stgraber this is ready for review now.

The original issue you reported is fixed, that of MTU for vlan passthrough not working.

This also fixes several other issues related to MTU restore once a container is shutdown.

There is an outstanding issue that I have been unable to fix, that is the scenario where a new physical device is hotplugged into a container, has its MTU modified and then the container is shutdown.

Right now I can't figure out a way to restore the MTU after the container has stopped, this will need more discussion with @brauner on the best way to achieve this.

@tomponline tomponline force-pushed the tomponline:tp-mtu-lxc branch 4 times, most recently from de1b88e to a0299d3 May 14, 2019

@stgraber

This comment has been minimized.

Copy link
Member

commented May 14, 2019

jenkins: test this please

@tomponline tomponline force-pushed the tomponline:tp-mtu-lxc branch 8 times, most recently from ed79410 to a8d5b40 May 15, 2019

@tomponline tomponline force-pushed the tomponline:tp-mtu-lxc branch 4 times, most recently from 3f1fb68 to 8736b56 May 16, 2019

@tomponline

This comment has been minimized.

Copy link
Member Author

commented May 19, 2019

@stgraber as it stands, this PR utilises the existing (but renamed) OnPostStop() hook to more accurately reflect when the hook is actually run.

This allows us to restore MTU and MAC for physical passthrough in situations where the devices are hot-unplugged or where LXC knows about them at boot time . However to cover all common scenarios (specifically the scenario where a device is hotplugged and then container is shutdown) I propose adding a new hook to LXD called OnStop() which would be run by LXC's lxc.hook.stop which would then be able to switch into the container's network namespace (only) and move the network devices back to the host (as well as restoring their original config) before LXC destroy's the namespace and the kernel moves them back (or deletes them in the case of dummy devices).

I propose the OnStop hook should iterate through each nic and then attempt to find the device in the container's netns using volatile.eth1.name at which point it can run nsenter --net=${LXC_NET_NS} ip link set ${NAME} netns ${LXD_PID} to move the container back to LXD's netns and then restore it's original name, MTU, MAC etc from the other volatile keys added in this PR.

Are you happy with this general approach?

@tomponline tomponline force-pushed the tomponline:tp-mtu-lxc branch 5 times, most recently from ca502f4 to 033dab7 May 20, 2019

@tomponline

This comment has been minimized.

Copy link
Member Author

commented May 20, 2019

@stgraber I've implemented the physical device MTU/MAC restore based on my proposed approach above.

container: Renames OnStop hook function to OnPostStop
This better reflects the actual LXC hook type being used (lxc.hook.post-stop) and allows the OnStop() function to be added back in the future with different functionality to be run by LXC's lxc.hook.stop hook.

Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>

@tomponline tomponline force-pushed the tomponline:tp-mtu-lxc branch 5 times, most recently from 97fc734 to 4b3396f May 20, 2019

tomponline added some commits May 20, 2019

container: Adds OnStop() function that is run by LXC's stop hook feature
This replaces the old OnStop() function that was renamed to OnPostStop().

This new OnStop() function is run before the container's namespaces have been closed.

Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
api: Exposes LXC network_phys_macvlan_mtu feature
Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
networks/utils: Adds networkGetDevMTU function
Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
networks: Refactors fan mtu detection to use networkGetDevMTU
Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
container/lxc: Stores mtu and mac of parent physical nic before start
Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
container/lxc: Fix copy/paste error in error removeNetworkDevice
Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
container/lxc: Restores physical parent mtu and mac on device removal
Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
test: Tests for physical mtu and mac application and restoration
Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
networks/utils: Adds networkGetDevMAC function
Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
doc: Upates volatile keys used for physical mtu and mac restoration
Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
networks/utils: Adds networkSetDevMAC function
Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
networks/utils: Adds networkSetDevMTU function
Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
container/lxc: Adds snapshotPhysicalNic function
Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>

@tomponline tomponline force-pushed the tomponline:tp-mtu-lxc branch from 4b3396f to ca1e609 May 21, 2019

@stgraber

This comment has been minimized.

Copy link
Member

commented May 21, 2019

jenkins: test this please

func (c *containerLXC) detachInterfaceRename(netns string, ifName string, hostName string) error {
pid := os.Getpid()

_, err := shared.RunCommand("nsenter", fmt.Sprintf("--net=%s", netns), "ip", "link", "set", ifName, "name", hostName, "netns", fmt.Sprintf("%d", pid))

This comment has been minimized.

Copy link
@stgraber

stgraber May 21, 2019

Member

I don't like that we entirely rely on the interface name in the container to have been left intact by the user.

Say you have container that's setup with:

  • eth0 => bridged
  • eth1 => physical

As root in the container I could swap their names, causing LXD to move the veth to the host and then the kernel to move the physical interface to the host.

This now effectively gives me a configured network interface on the host which is attached to a bridge that I can have other containers on, if properly configured, I could use that to take over some specific host traffic.

In general LXD should assume a hostile user in the container and use ways of tracking resources which cannot be modified by the user.

I also think that we should ensure we do an IPv4/IPv6 reset on the interface as it's removed from the container, or at least ensure it's brought down as part of this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.