From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 896DB93149 for ; Fri, 17 Feb 2023 15:51:06 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 5E2BD7A6D for ; Fri, 17 Feb 2023 15:51:06 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Fri, 17 Feb 2023 15:51:05 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 2899847751 for ; Fri, 17 Feb 2023 15:51:05 +0100 (CET) Date: Fri, 17 Feb 2023 15:51:03 +0100 From: Wolfgang Bumiller To: Christoph Heiss Cc: pve-devel@lists.proxmox.com, Thomas Lamprecht Message-ID: <20230217145103.sl2bgaoq5jtuskp6@fwblub> References: <20230215140245.496507-1-c.heiss@proxmox.com> <20230215140245.496507-4-c.heiss@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230215140245.496507-4-c.heiss@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.186 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH v2 container 3/4] lxc: Add `link-down` config to allow setting interfaces as disconnected X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 17 Feb 2023 14:51:06 -0000 On Wed, Feb 15, 2023 at 03:02:44PM +0100, Christoph Heiss wrote: > If this network option is set, the host-side link will be forced down > and the interface won't be connected to the bridge. > > Signed-off-by: Christoph Heiss > --- > Changes v1 -> v2: > * Split trailing whitespace fix into separate patch > * Rename option to kebap-case Sorry I missed that when Thomas replied to the v1. This is a case where the property already exists in the VM api, so better reuse the snake_case version here. @Thomas? > * Proper option comparison using `safe_boolean_ne` > * Copy option to new network conf like the other options > * Remove the veth interface from the bridge when disconnected > > src/PVE/LXC.pm | 41 ++++++++++++++++++++++++++++++++++------- > src/PVE/LXC/Config.pm | 5 +++++ > src/lxcnetaddbr | 7 +++++-- > 3 files changed, 44 insertions(+), 9 deletions(-) > > diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm > index 0de5ba3..1b93f48 100644 > --- a/src/PVE/LXC.pm > +++ b/src/PVE/LXC.pm > @@ -955,7 +955,8 @@ sub update_net { > } else { > if (safe_string_ne($oldnet->{bridge}, $newnet->{bridge}) || > safe_num_ne($oldnet->{tag}, $newnet->{tag}) || > - safe_num_ne($oldnet->{firewall}, $newnet->{firewall}) > + safe_num_ne($oldnet->{firewall}, $newnet->{firewall}) || > + safe_boolean_ne($oldnet->{'link-down'}, $newnet->{'link-down'}) > ) { > > if ($oldnet->{bridge}) { > @@ -968,10 +969,28 @@ sub update_net { > } > > my ($bridge, $mac, $firewall, $rate) = $newnet->@{'bridge', 'hwaddr', 'firewall', 'rate'}; > - PVE::LXC::net_tap_plug($veth, $bridge, $newnet->{tag}, $firewall, $newnet->{trunks}, $rate, { mac => $mac }); > + > + if (defined($newnet->{'link-down'})) { > + # The interface must not be connected to the designated > + # bridge if the link was requested to be disconnected. > + # Otherwise it could get re-enabled by something like > + # `ifreload`. > + # > + # Thus only force the host-side link down here and skip > + # adding it to the bridge. > + PVE::Tools::run_command(['/sbin/ip', 'link', 'set', 'dev', $veth, 'down']); > + } else { > + # Connect the interface to the bridge > + PVE::LXC::net_tap_plug( > + $veth, $bridge, $newnet->{tag}, $firewall, $newnet->{trunks}, $rate, { mac => $mac }); > + > + # Force the host-side link up if it was previously down. > + PVE::Tools::run_command(['/sbin/ip', 'link', 'set', 'dev', $veth, 'up']) > + if defined($oldnet->{'link-down'}); > + } > > # This includes the rate: > - foreach (qw(bridge tag firewall rate)) { > + foreach (qw(bridge tag firewall rate link-down)) { > $oldnet->{$_} = $newnet->{$_} if $newnet->{$_}; > } > } elsif (safe_string_ne($oldnet->{rate}, $newnet->{rate})) { > @@ -1002,9 +1021,6 @@ sub hotplug_net { > } else { > PVE::Network::veth_create($veth, $vethpeer, $newnet->{bridge}, $newnet->{hwaddr}); > } > - PVE::LXC::net_tap_plug( > - $veth, $newnet->{bridge}, $newnet->{tag}, $newnet->{firewall}, $newnet->{trunks}, > - $newnet->{rate}, { mac => $newnet->{hwaddr} }); > > # attach peer in container > my $cmd = ['lxc-device', '-n', $vmid, 'add', $vethpeer, "$eth" ]; > @@ -1014,8 +1030,19 @@ sub hotplug_net { > $cmd = ['lxc-attach', '-n', $vmid, '-s', 'NETWORK', '--', '/sbin/ip', 'link', 'set', $eth ,'up' ]; > PVE::Tools::run_command($cmd); > > + if (defined($newnet->{'link-down'})) { Any particular reason for moving this down here now? I'm sure *someone* will run into issues with badly and wrongly configured containers with this ;-) It's just nicer for containers to immediately see a readily connected interface, so maybe keep it up there where the original tap_plug call was. > + # In case the network device should be disconnected, force the host-link down .. > + PVE::Tools::run_command(['/sbin/ip', 'link', 'set', 'dev', $veth, 'down']); > + } else { > + # .. otherwise, connect it normally to the bridge. > + # The interface is already up from creation. > + PVE::LXC::net_tap_plug( > + $veth, $newnet->{bridge}, $newnet->{tag}, $newnet->{firewall}, $newnet->{trunks}, > + $newnet->{rate}, { mac => $newnet->{hwaddr} }); > + } > + > my $done = { type => 'veth' }; > - foreach (qw(bridge tag firewall hwaddr name)) { > + foreach (qw(bridge tag firewall hwaddr name link-down)) { > $done->{$_} = $newnet->{$_} if $newnet->{$_}; > } > $conf->{$opt} = PVE::LXC::Config->print_lxc_network($done); > diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm > index af25a96..26a2fac 100644 > --- a/src/PVE/LXC/Config.pm > +++ b/src/PVE/LXC/Config.pm > @@ -814,6 +814,11 @@ our $netconf_desc = { > description => "Apply rate limiting to the interface", > optional => 1, > }, > + 'link-down' => { > + type => 'boolean', > + description => 'Whether this interface should be disconnected (like pulling the plug).', > + optional => 1, > + }, > }; > PVE::JSONSchema::register_format('pve-lxc-network', $netconf_desc); > > diff --git a/src/lxcnetaddbr b/src/lxcnetaddbr > index ebd6baa..c5d724b 100755 > --- a/src/lxcnetaddbr > +++ b/src/lxcnetaddbr > @@ -52,10 +52,13 @@ if (-d "/sys/class/net/$iface") { > #avoid insecure dependency; > ($bridgemtu) = $bridgemtu =~ /(\d+)/; > > - PVE::Tools::run_command("/sbin/ip link set dev $iface up mtu $bridgemtu"); > + my $linkstate = defined($net->{'link-down'}) ? 'down' : 'up'; > + PVE::Tools::run_command("/sbin/ip link set dev $iface $linkstate mtu $bridgemtu"); > PVE::Tools::run_command("/sbin/ip addr add 0.0.0.0/0 dev $iface"); > > - PVE::LXC::net_tap_plug($iface, $bridge, $tag, $firewall, $trunks, $rate, { mac => $hwaddr }); > + # Only plug the interface into the bridge if it is not set as disconnected by the user. > + PVE::LXC::net_tap_plug($iface, $bridge, $tag, $firewall, $trunks, $rate, { mac => $hwaddr }) > + if !defined($net->{'link-down'}); > } > > exit 0; > -- > 2.39.1