* [pve-devel] [PATCH v4 container 1/3] net: Pass network config directly to net_tap_plug()
2023-02-22 12:49 [pve-devel] [PATCH v4 container/manager 0/3] fix #3413: Add `Disconnect` option for LXC networks Christoph Heiss
@ 2023-02-22 12:49 ` Christoph Heiss
2023-02-22 12:49 ` [pve-devel] [PATCH v4 container 2/3] net: Add `link_down` config to allow setting interfaces as disconnected Christoph Heiss
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Christoph Heiss @ 2023-02-22 12:49 UTC (permalink / raw)
To: pve-devel
No functional changes.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v3 -> v4:
* New patch
A note: The check in net_tap_plug() whether the MAC address is actually
passed in $opts is removed, as this was always the case anyway.
src/PVE/LXC.pm | 20 +++++++++-----------
src/lxcnetaddbr | 9 +--------
2 files changed, 10 insertions(+), 19 deletions(-)
diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index 54ee0d9..54afd97 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -918,15 +918,16 @@ sub vm_stop_cleanup {
warn $@ if $@; # avoid errors - just warn
}
-sub net_tap_plug : prototype($$$$$$;$) {
- my ($iface, $bridge, $tag, $firewall, $trunks, $rate, $opts) = @_;
+sub net_tap_plug : prototype($$) {
+ my ($iface, $net) = @_;
+ my ($bridge, $tag, $firewall, $trunks, $rate, $hwaddr) =
+ $net->@{'bridge', 'tag', 'firewall', 'trunks', 'rate', 'hwaddr'};
if ($have_sdn) {
PVE::Network::SDN::Zones::tap_plug($iface, $bridge, $tag, $firewall, $trunks, $rate);
- PVE::Network::SDN::Zones::add_bridge_fdb($iface, $opts->{mac}, $bridge, $firewall)
- if defined($opts->{mac});
+ PVE::Network::SDN::Zones::add_bridge_fdb($iface, $hwaddr, $bridge, $firewall);
} else {
- PVE::Network::tap_plug($iface, $bridge, $tag, $firewall, $trunks, $rate, $opts);
+ PVE::Network::tap_plug($iface, $bridge, $tag, $firewall, $trunks, $rate, { mac => $hwaddr });
}
}
@@ -968,8 +969,7 @@ sub update_net {
PVE::LXC::Config->write_config($vmid, $conf);
}
- 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 });
+ PVE::LXC::net_tap_plug($veth, $newnet);
# This includes the rate:
foreach (qw(bridge tag firewall rate)) {
@@ -1003,10 +1003,8 @@ 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} },
- );
+
+ PVE::LXC::net_tap_plug($veth, $newnet);
# attach peer in container
my $cmd = ['lxc-device', '-n', $vmid, 'add', $vethpeer, "$eth" ];
diff --git a/src/lxcnetaddbr b/src/lxcnetaddbr
index ebd6baa..f2db433 100755
--- a/src/lxcnetaddbr
+++ b/src/lxcnetaddbr
@@ -36,13 +36,7 @@ die "unable to find network definition for interface '$iface'\n"
my $net = PVE::LXC::Config->parse_lxc_network($netconf);
-my $tag = $net->{tag};
-my $firewall = $net->{firewall};
my $bridge = $net->{bridge};
-my $trunks = $net->{trunks};
-my $rate = $net->{rate};
-my $hwaddr = $net->{hwaddr};
-
die "missing bridge configuration" if !$bridge;
if (-d "/sys/class/net/$iface") {
@@ -54,8 +48,7 @@ if (-d "/sys/class/net/$iface") {
PVE::Tools::run_command("/sbin/ip link set dev $iface up 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 });
+ PVE::LXC::net_tap_plug($iface, $net);
}
exit 0;
--
2.39.1
^ permalink raw reply [flat|nested] 7+ messages in thread* [pve-devel] [PATCH v4 container 2/3] net: Add `link_down` config to allow setting interfaces as disconnected
2023-02-22 12:49 [pve-devel] [PATCH v4 container/manager 0/3] fix #3413: Add `Disconnect` option for LXC networks Christoph Heiss
2023-02-22 12:49 ` [pve-devel] [PATCH v4 container 1/3] net: Pass network config directly to net_tap_plug() Christoph Heiss
@ 2023-02-22 12:49 ` Christoph Heiss
2023-02-22 12:49 ` [pve-devel] [PATCH v4 manager 3/3] lxc: Add `Disconnect` option for network interfaces Christoph Heiss
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Christoph Heiss @ 2023-02-22 12:49 UTC (permalink / raw)
To: pve-devel
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 <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* Split trailing whitespace fix into separate patch
* Rename option to kebap-case
* 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
Changes v2 -> v3:
* Rename option to snake_case again
* Move option hotplug-handling before LXC attach again
Changes v3 -> v4:
* Rebase
* Shorten and remove some comments as appropriate
* Update `link_down` schema comment
* Move `link_down` logic to net_tap_plug()
A note regarding the last change:
The interface is now always set UP if `link_down` is unset. This saves
us from passing the old network configuration to net_tap_plug() and
should not have any effect as setting an interface UP/DOWN is
(hopefully?) idempotent anyway - if it already is UP it does nothing and
if it is currently DOWN we want it UP anyway at that point.
src/PVE/LXC.pm | 17 ++++++++++++++---
src/PVE/LXC/Config.pm | 6 ++++++
2 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index 54afd97..c4d53e8 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -920,6 +920,14 @@ sub vm_stop_cleanup {
sub net_tap_plug : prototype($$) {
my ($iface, $net) = @_;
+
+ if (defined($net->{link_down})) {
+ PVE::Tools::run_command(['/sbin/ip', 'link', 'set', 'dev', $iface, 'down']);
+ # Don't add disconnected interfaces to the bridge, otherwise e.g. applying any network
+ # change (e.g. `ifreload -a`) could (re-)activate it unintentionally.
+ return;
+ }
+
my ($bridge, $tag, $firewall, $trunks, $rate, $hwaddr) =
$net->@{'bridge', 'tag', 'firewall', 'trunks', 'rate', 'hwaddr'};
@@ -929,6 +937,8 @@ sub net_tap_plug : prototype($$) {
} else {
PVE::Network::tap_plug($iface, $bridge, $tag, $firewall, $trunks, $rate, { mac => $hwaddr });
}
+
+ PVE::Tools::run_command(['/sbin/ip', 'link', 'set', 'dev', $iface, 'up']);
}
sub update_net {
@@ -957,7 +967,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}) {
@@ -972,7 +983,7 @@ sub update_net {
PVE::LXC::net_tap_plug($veth, $newnet);
# 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})) {
@@ -1015,7 +1026,7 @@ sub hotplug_net {
PVE::Tools::run_command($cmd);
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..bf424f9 100644
--- a/src/PVE/LXC/Config.pm
+++ b/src/PVE/LXC/Config.pm
@@ -814,6 +814,12 @@ our $netconf_desc = {
description => "Apply rate limiting to the interface",
optional => 1,
},
+ # TODO: Rename this option and the qemu-server one to `link-down` for PVE 8.0
+ 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);
--
2.39.1
^ permalink raw reply [flat|nested] 7+ messages in thread* [pve-devel] [PATCH v4 manager 3/3] lxc: Add `Disconnect` option for network interfaces
2023-02-22 12:49 [pve-devel] [PATCH v4 container/manager 0/3] fix #3413: Add `Disconnect` option for LXC networks Christoph Heiss
2023-02-22 12:49 ` [pve-devel] [PATCH v4 container 1/3] net: Pass network config directly to net_tap_plug() Christoph Heiss
2023-02-22 12:49 ` [pve-devel] [PATCH v4 container 2/3] net: Add `link_down` config to allow setting interfaces as disconnected Christoph Heiss
@ 2023-02-22 12:49 ` Christoph Heiss
2023-03-16 15:06 ` [pve-devel] applied: " Wolfgang Bumiller
2023-02-23 13:54 ` [pve-devel] [PATCH v4 container/manager 0/3] fix #3413: Add `Disconnect` option for LXC networks Friedrich Weber
2023-03-16 11:51 ` [pve-devel] applied: " Thomas Lamprecht
4 siblings, 1 reply; 7+ messages in thread
From: Christoph Heiss @ 2023-02-22 12:49 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* Rename option to kebap-case
Changes v2 -> v3:
* Rename option to snake_case again
Changes v3 -> v4:
* Rebase
Q: Since this depends on an updated API endpoint from pve-container, the
minimal required 'Depends' version for pve-container probably needs a
bump? How is something like this usually handled?
www/manager6/Parser.js | 3 +++
www/manager6/lxc/Network.js | 13 +++++++++++++
2 files changed, 16 insertions(+)
diff --git a/www/manager6/Parser.js b/www/manager6/Parser.js
index 9f7b2c84..c3772d3b 100644
--- a/www/manager6/Parser.js
+++ b/www/manager6/Parser.js
@@ -298,6 +298,8 @@ Ext.define('PVE.Parser', {
data[match_res[1]] = match_res[2];
} else if ((match_res = p.match(/^firewall=(\d+)$/)) !== null) {
data.firewall = PVE.Parser.parseBoolean(match_res[1]);
+ } else if ((match_res = p.match(/^link_down=(\d+)$/)) !== null) {
+ data.link_down = PVE.Parser.parseBoolean(match_res[1]);
} else if (!p.match(/^type=\S+$/)) {
console.warn(`could not parse LXC network string ${p}`);
}
@@ -319,6 +321,7 @@ Ext.define('PVE.Parser', {
name: 1,
rate: 1,
tag: 1,
+ link_down: 1,
};
return Object.entries(config)
.filter(([k, v]) => v !== undefined && v !== '' && knownKeys[k])
diff --git a/www/manager6/lxc/Network.js b/www/manager6/lxc/Network.js
index 85033bd8..b2cd9410 100644
--- a/www/manager6/lxc/Network.js
+++ b/www/manager6/lxc/Network.js
@@ -273,6 +273,12 @@ Ext.define('PVE.lxc.NetworkInputPanel', {
];
me.advancedColumn1 = [
+ {
+ xtype: 'proxmoxcheckbox',
+ fieldLabel: gettext('Disconnect'),
+ name: 'link_down',
+ value: cdata.link_down,
+ },
{
xtype: 'proxmoxintegerfield',
fieldLabel: 'MTU',
@@ -539,6 +545,12 @@ Ext.define('PVE.lxc.NetworkView', {
width: 80,
dataIndex: 'mtu',
},
+ {
+ header: gettext('Disconnected'),
+ width: 100,
+ dataIndex: 'link_down',
+ renderer: Proxmox.Utils.format_boolean,
+ },
],
listeners: {
activate: me.load,
@@ -564,6 +576,7 @@ Ext.define('PVE.lxc.NetworkView', {
'tag',
'firewall',
'mtu',
+ 'link_down',
],
});
});
--
2.39.1
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [pve-devel] [PATCH v4 container/manager 0/3] fix #3413: Add `Disconnect` option for LXC networks
2023-02-22 12:49 [pve-devel] [PATCH v4 container/manager 0/3] fix #3413: Add `Disconnect` option for LXC networks Christoph Heiss
` (2 preceding siblings ...)
2023-02-22 12:49 ` [pve-devel] [PATCH v4 manager 3/3] lxc: Add `Disconnect` option for network interfaces Christoph Heiss
@ 2023-02-23 13:54 ` Friedrich Weber
2023-03-16 11:51 ` [pve-devel] applied: " Thomas Lamprecht
4 siblings, 0 replies; 7+ messages in thread
From: Friedrich Weber @ 2023-02-23 13:54 UTC (permalink / raw)
To: pve-devel
As I also missed that feature, I applied the patches to my PVE instance
with pre-existing containers -- all interfaces stayed up as expected,
and disconnecting/reconnecting interfaces for running and stopped
containers via the Web UI worked nicely.
Tested-by: Friedrich Weber <f.weber@proxmox.com>
On 22/02/2023 13:49, Christoph Heiss wrote:
> Add a `Disconnect` option for network interfaces on LXC containers, much
> like it already exists for VMs. This has been requested in #3413 [0] and
> seems useful, especially considering we already support the same thing
> for VMs.
>
> One thing to note is that LXC does not seem to support the notion of
> setting an interface down. The `flags` property would suggest that this
> possible [1], but AFAICS it does not work. I tried setting the value as
> empty and to something else than "up" (since that is really the only
> supported option [2][3]), which both had absolutely no effect.
>
> Thus force the host-side link of the container network down and avoid
> adding it to the designated bridge if the new option is set, effectively
> disconnecting the container network.
>
> The first patch is cleanup only and does not change anything regarding
> functionality.
>
> Testing
> -------
> Testing was done by starting a LXC container (w/ and w/o `link_down`
> set), checking if the interface has (or not) LOWERLAYERDOWN set inside
> the container (`ip address eth0`) and if packet transit works (or not)
> using a simple `ping`. Same thing after toggeling the option on the
> interface. Further, the interface(s) should (or should not) be listed
> in `brctl show`. Same thing was done for hotplugged interfaces to a
> running container.
>
> Also tested with `ifreload -a` (thanks Wolfgang!) thrown in, which did
> nothing unexpected: If `link_down` was set, interfaces stayed in
> LOWERLAYERDOWN and unplugged from the bridge, and stayed UP and plugged
> into the bridge when `link_down` was unset.
>
> [0] https://bugzilla.proxmox.com/show_bug.cgi?id=3413
> [1] https://linuxcontainers.org/lxc/manpages/man5/lxc.container.conf.5.html#lbAO
> [2] https://github.com/lxc/lxc/blob/08f0e769/src/lxc/confile.c#L453-L467
> [3] https://github.com/lxc/lxc/blob/08f0e769/src/lxc/confile.c#L5933-L5952
>
> v1: https://lists.proxmox.com/pipermail/pve-devel/2023-February/055762.html
> v2: https://lists.proxmox.com/pipermail/pve-devel/2023-February/055795.html
> v3: https://lists.proxmox.com/pipermail/pve-devel/2023-February/055839.html
>
> pve-container:
>
> Christoph Heiss (2):
> net: Pass network config directly to net_tap_plug()
> net: Add `link_down` config to allow setting interfaces as disconnected
>
> src/PVE/LXC.pm | 37 +++++++++++++++++++++++--------------
> src/PVE/LXC/Config.pm | 6 ++++++
> src/lxcnetaddbr | 9 +--------
> 3 files changed, 30 insertions(+), 22 deletions(-)
>
> pve-manager:
>
> Christoph Heiss (1):
> lxc: Add `Disconnect` option for network interfaces
>
> www/manager6/Parser.js | 3 +++
> www/manager6/lxc/Network.js | 13 +++++++++++++
> 2 files changed, 16 insertions(+)
>
> --
> 2.39.1
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread* [pve-devel] applied: [PATCH v4 container/manager 0/3] fix #3413: Add `Disconnect` option for LXC networks
2023-02-22 12:49 [pve-devel] [PATCH v4 container/manager 0/3] fix #3413: Add `Disconnect` option for LXC networks Christoph Heiss
` (3 preceding siblings ...)
2023-02-23 13:54 ` [pve-devel] [PATCH v4 container/manager 0/3] fix #3413: Add `Disconnect` option for LXC networks Friedrich Weber
@ 2023-03-16 11:51 ` Thomas Lamprecht
4 siblings, 0 replies; 7+ messages in thread
From: Thomas Lamprecht @ 2023-03-16 11:51 UTC (permalink / raw)
To: Proxmox VE development discussion, Christoph Heiss
Am 22/02/2023 um 13:49 schrieb Christoph Heiss:
> Add a `Disconnect` option for network interfaces on LXC containers, much
> like it already exists for VMs. This has been requested in #3413 [0] and
> seems useful, especially considering we already support the same thing
> for VMs.
>
> One thing to note is that LXC does not seem to support the notion of
> setting an interface down. The `flags` property would suggest that this
> possible [1], but AFAICS it does not work. I tried setting the value as
> empty and to something else than "up" (since that is really the only
> supported option [2][3]), which both had absolutely no effect.
>
> Thus force the host-side link of the container network down and avoid
> adding it to the designated bridge if the new option is set, effectively
> disconnecting the container network.
>
> The first patch is cleanup only and does not change anything regarding
> functionality.
>
> Testing
> -------
> Testing was done by starting a LXC container (w/ and w/o `link_down`
> set), checking if the interface has (or not) LOWERLAYERDOWN set inside
> the container (`ip address eth0`) and if packet transit works (or not)
> using a simple `ping`. Same thing after toggeling the option on the
> interface. Further, the interface(s) should (or should not) be listed
> in `brctl show`. Same thing was done for hotplugged interfaces to a
> running container.
>
> Also tested with `ifreload -a` (thanks Wolfgang!) thrown in, which did
> nothing unexpected: If `link_down` was set, interfaces stayed in
> LOWERLAYERDOWN and unplugged from the bridge, and stayed UP and plugged
> into the bridge when `link_down` was unset.
>
> [0] https://bugzilla.proxmox.com/show_bug.cgi?id=3413
> [1] https://linuxcontainers.org/lxc/manpages/man5/lxc.container.conf.5.html#lbAO
> [2] https://github.com/lxc/lxc/blob/08f0e769/src/lxc/confile.c#L453-L467
> [3] https://github.com/lxc/lxc/blob/08f0e769/src/lxc/confile.c#L5933-L5952
>
> v1: https://lists.proxmox.com/pipermail/pve-devel/2023-February/055762.html
> v2: https://lists.proxmox.com/pipermail/pve-devel/2023-February/055795.html
> v3: https://lists.proxmox.com/pipermail/pve-devel/2023-February/055839.html
>
> pve-container:
>
> Christoph Heiss (2):
> net: Pass network config directly to net_tap_plug()
> net: Add `link_down` config to allow setting interfaces as disconnected
>
> src/PVE/LXC.pm | 37 +++++++++++++++++++++++--------------
> src/PVE/LXC/Config.pm | 6 ++++++
> src/lxcnetaddbr | 9 +--------
> 3 files changed, 30 insertions(+), 22 deletions(-)
>
applied above two, with the relevant bits of the cover letter added to the commit
message of the second container patch, thanks!
^ permalink raw reply [flat|nested] 7+ messages in thread