all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Christoph Heiss <c.heiss@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH v2 container 2/4] lxc: Avoid open-coding normal vs SDN-specific tap_plug()
Date: Wed, 15 Feb 2023 15:02:43 +0100	[thread overview]
Message-ID: <20230215140245.496507-3-c.heiss@proxmox.com> (raw)
In-Reply-To: <20230215140245.496507-1-c.heiss@proxmox.com>

This pattern is used in multiple places, thus extract it into a
subroutine on its own.

No functional changes.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Might not be the best place for net_tap_plug(), putting this logic
inside PVE::Network would probably make more sense. But that would
entail a (bigger) refactoring, since it then also must be done for all
other tap_*() and veth_*() subroutines (and maybe some other things?)
for consistency..
In any case, that definitely would be too much for this series. I can do
that, but I'd do it as a follow-up series.

Changes v1 -> v2:
 * New patch

 src/PVE/LXC.pm  | 28 ++++++++++++++++------------
 src/lxcnetaddbr | 15 ++-------------
 2 files changed, 18 insertions(+), 25 deletions(-)

diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index 345e343..0de5ba3 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -917,6 +917,18 @@ sub vm_stop_cleanup {
     warn $@ if $@; # avoid errors - just warn
 }

+sub net_tap_plug {
+    if ($have_sdn) {
+	my ($iface, $bridge, $tag, $firewall, $trunks, $rate, $opts) = @_;
+
+	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});
+    } else {
+	PVE::Network::tap_plug(@_);
+    }
+}
+
 sub update_net {
     my ($vmid, $conf, $opt, $newnet, $netid, $rootdir) = @_;

@@ -956,14 +968,7 @@ sub update_net {
 		}

 		my ($bridge, $mac, $firewall, $rate) = $newnet->@{'bridge', 'hwaddr', 'firewall', 'rate'};
-		if ($have_sdn) {
-		    PVE::Network::SDN::Zones::tap_plug(
-		        $veth, $bridge, $newnet->{tag}, $firewall, $newnet->{trunks}, $rate);
-		    PVE::Network::SDN::Zones::add_bridge_fdb($veth, $mac, $bridge, $firewall);
-		} else {
-		    PVE::Network::tap_plug(
-		        $veth, $bridge, $newnet->{tag}, $firewall, $newnet->{trunks}, $rate, { mac => $mac });
-		}
+		PVE::LXC::net_tap_plug($veth, $bridge, $newnet->{tag}, $firewall, $newnet->{trunks}, $rate, { mac => $mac });

 		# This includes the rate:
 		foreach (qw(bridge tag firewall rate)) {
@@ -994,13 +999,12 @@ sub hotplug_net {

     if ($have_sdn) {
 	PVE::Network::SDN::Zones::veth_create($veth, $vethpeer, $newnet->{bridge}, $newnet->{hwaddr});
-	PVE::Network::SDN::Zones::tap_plug($veth, $newnet->{bridge}, $newnet->{tag}, $newnet->{firewall}, $newnet->{trunks}, $newnet->{rate});
-	PVE::Network::SDN::Zones::add_bridge_fdb($veth, $newnet->{hwaddr}, $newnet->{bridge}, $newnet->{firewall});
     } else {
 	PVE::Network::veth_create($veth, $vethpeer, $newnet->{bridge}, $newnet->{hwaddr});
-	PVE::Network::tap_plug($veth, $newnet->{bridge}, $newnet->{tag}, $newnet->{firewall}, $newnet->{trunks}, $newnet->{rate});
-	PVE::Network::add_bridge_fdb($veth, $newnet->{hwaddr}, $newnet->{firewall}); # early returns if brport has learning on
     }
+    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" ];
diff --git a/src/lxcnetaddbr b/src/lxcnetaddbr
index 83052e1..ebd6baa 100755
--- a/src/lxcnetaddbr
+++ b/src/lxcnetaddbr
@@ -7,15 +7,8 @@ exit 0 if $ENV{LXC_NAME} && $ENV{LXC_NAME} !~ /^\d+$/;

 use PVE::LXC;
 use PVE::Tools qw(run_command);
-use PVE::Network;
 use PVE::ProcFSTools;

-my $have_sdn;
-eval {
-    require PVE::Network::SDN::Zones;
-    $have_sdn = 1;
-};
-
 die "got unexpected argument count\n" if scalar(@ARGV) != 5;

 my ($vmid, $arg2, $arg3, $type, $iface) = @ARGV;
@@ -48,6 +41,7 @@ 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;

@@ -61,12 +55,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");

-    if ($have_sdn) {
-	PVE::Network::SDN::Zones::tap_plug($iface, $bridge, $tag, $firewall, $trunks, $rate);
-	PVE::Network::SDN::Zones::add_bridge_fdb($iface, $net->{hwaddr}, $bridge, $firewall);
-    } else {
-	PVE::Network::tap_plug($iface, $bridge, $tag, $firewall, $trunks, $rate, { mac => $net->{hwaddr}});
-    }
+    PVE::LXC::net_tap_plug($iface, $bridge, $tag, $firewall, $trunks, $rate, { mac => $hwaddr });
 }

 exit 0;
--
2.39.1





  parent reply	other threads:[~2023-02-15 14:03 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-15 14:02 [pve-devel] [PATCH v2 container/manager 0/4] fix #3413: Add `Disconnect` option for LXC networks Christoph Heiss
2023-02-15 14:02 ` [pve-devel] [PATCH v2 container 1/4] lxc: Fix some trailing whitespace Christoph Heiss
2023-02-17 14:22   ` Wolfgang Bumiller
2023-02-20 11:13     ` Christoph Heiss
2023-02-15 14:02 ` Christoph Heiss [this message]
2023-02-17 14:34   ` [pve-devel] [PATCH v2 container 2/4] lxc: Avoid open-coding normal vs SDN-specific tap_plug() Wolfgang Bumiller
2023-02-20 11:17     ` Christoph Heiss
2023-02-15 14:02 ` [pve-devel] [PATCH v2 container 3/4] lxc: Add `link-down` config to allow setting interfaces as disconnected Christoph Heiss
2023-02-17 14:51   ` Wolfgang Bumiller
2023-02-17 16:38     ` Thomas Lamprecht
2023-02-20 11:51       ` Christoph Heiss
2023-02-20 12:33         ` Wolfgang Bumiller
2023-02-20 12:37           ` Thomas Lamprecht
2023-02-15 14:02 ` [pve-devel] [PATCH v2 manager 4/4] lxc: Add `Disconnect` option for network interfaces Christoph Heiss

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230215140245.496507-3-c.heiss@proxmox.com \
    --to=c.heiss@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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