all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH container] net: do not force network interface down if `link_down` is to 0
@ 2025-04-30 11:19 Christoph Heiss
  2025-04-30 12:24 ` Fabian Grünbichler
  0 siblings, 1 reply; 3+ messages in thread
From: Christoph Heiss @ 2025-04-30 11:19 UTC (permalink / raw)
  To: pve-devel

Reported in the forum [0].

Currently, the (host) interface is always forced down if the property is
set. Check the actual (boolean) value and honor the users request if set
to `0` aka. off.

[0] https://forum.proxmox.com/threads/bug-container-network-card-turned-off.160691/

Fixes: 9e56948 ("net: Add `link_down` config to allow setting interfaces as disconnected")
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 src/PVE/LXC.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index a58c997..5d9b524 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -995,7 +995,7 @@ sub vm_stop_cleanup {
 sub net_tap_plug : prototype($$) {
     my ($iface, $net) = @_;
 
-    if (defined($net->{link_down})) {
+    if (defined($net->{link_down}) && $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.
-- 
2.49.0



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [pve-devel] [PATCH container] net: do not force network interface down if `link_down` is to 0
  2025-04-30 11:19 [pve-devel] [PATCH container] net: do not force network interface down if `link_down` is to 0 Christoph Heiss
@ 2025-04-30 12:24 ` Fabian Grünbichler
  2025-04-30 12:45   ` Christoph Heiss
  0 siblings, 1 reply; 3+ messages in thread
From: Fabian Grünbichler @ 2025-04-30 12:24 UTC (permalink / raw)
  To: Proxmox VE development discussion

On April 30, 2025 1:19 pm, Christoph Heiss wrote:
> Reported in the forum [0].
> 
> Currently, the (host) interface is always forced down if the property is
> set. Check the actual (boolean) value and honor the users request if set
> to `0` aka. off.
> 
> [0] https://forum.proxmox.com/threads/bug-container-network-card-turned-off.160691/
> 
> Fixes: 9e56948 ("net: Add `link_down` config to allow setting interfaces as disconnected")
> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> ---
>  src/PVE/LXC.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
> index a58c997..5d9b524 100644
> --- a/src/PVE/LXC.pm
> +++ b/src/PVE/LXC.pm
> @@ -995,7 +995,7 @@ sub vm_stop_cleanup {
>  sub net_tap_plug : prototype($$) {
>      my ($iface, $net) = @_;
>  
> -    if (defined($net->{link_down})) {
> +    if (defined($net->{link_down}) && $net->{link_down}) {

the defined here seems kinda strange and misleading, shouldn't we just
check whether it is *true*?

>  	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.
> -- 
> 2.49.0
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [pve-devel] [PATCH container] net: do not force network interface down if `link_down` is to 0
  2025-04-30 12:24 ` Fabian Grünbichler
@ 2025-04-30 12:45   ` Christoph Heiss
  0 siblings, 0 replies; 3+ messages in thread
From: Christoph Heiss @ 2025-04-30 12:45 UTC (permalink / raw)
  To: Fabian Grünbichler; +Cc: Proxmox VE development discussion

On Wed Apr 30, 2025 at 2:24 PM CEST, Fabian Grünbichler wrote:
> On April 30, 2025 1:19 pm, Christoph Heiss wrote:
>> Reported in the forum [0].
>>
>> Currently, the (host) interface is always forced down if the property is
>> set. Check the actual (boolean) value and honor the users request if set
>> to `0` aka. off.
>>
>> [0] https://forum.proxmox.com/threads/bug-container-network-card-turned-off.160691/
>>
>> Fixes: 9e56948 ("net: Add `link_down` config to allow setting interfaces as disconnected")
>> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
>> ---
>>  src/PVE/LXC.pm | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
>> index a58c997..5d9b524 100644
>> --- a/src/PVE/LXC.pm
>> +++ b/src/PVE/LXC.pm
>> @@ -995,7 +995,7 @@ sub vm_stop_cleanup {
>>  sub net_tap_plug : prototype($$) {
>>      my ($iface, $net) = @_;
>>
>> -    if (defined($net->{link_down})) {
>> +    if (defined($net->{link_down}) && $net->{link_down}) {
>
> the defined here seems kinda strange and misleading, shouldn't we just
> check whether it is *true*?

Right, the `defined()` can indeed be dropped, Perl doesn't throw a
warning in such situation anyway.

I'll send a v2 - thanks!


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-04-30 12:45 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-04-30 11:19 [pve-devel] [PATCH container] net: do not force network interface down if `link_down` is to 0 Christoph Heiss
2025-04-30 12:24 ` Fabian Grünbichler
2025-04-30 12:45   ` Christoph Heiss

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal