public inbox for pve-devel@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 v3 container 2/4] lxc: Avoid open-coding normal vs SDN-specific tap_plug()
Date: Tue, 21 Feb 2023 09:05:48 +0100	[thread overview]
Message-ID: <20230221080550.43336-3-c.heiss@proxmox.com> (raw)
In-Reply-To: <20230221080550.43336-1-c.heiss@proxmox.com>

This pattern is used in multiple places, thus just extract it into a sub
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

Changes v2 -> v3:
 * Add prototype to net_tap_plug()

 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 cbbb82d..d419124 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -918,6 +918,18 @@ sub vm_stop_cleanup {
     warn $@ if $@; # avoid errors - just warn
 }

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

@@ -957,14 +969,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)) {
@@ -995,13 +1000,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-21  8:06 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-21  8:05 [pve-devel] [PATCH v3 container/manager 0/4] fix #3413: Add `Disconnect` option for LXC networks Christoph Heiss
2023-02-21  8:05 ` [pve-devel] [PATCH v3 container 1/4] lxc: Fix some trailing whitespace Christoph Heiss
2023-02-21 16:59   ` [pve-devel] applied: " Thomas Lamprecht
2023-02-21  8:05 ` Christoph Heiss [this message]
2023-02-21 17:07   ` [pve-devel] applied: [PATCH v3 container 2/4] lxc: Avoid open-coding normal vs SDN-specific tap_plug() Thomas Lamprecht
2023-02-22  9:34     ` Christoph Heiss
2023-02-21  8:05 ` [pve-devel] [PATCH v3 container 3/4] lxc: Add `link_down` config to allow setting interfaces as disconnected Christoph Heiss
2023-02-21 17:25   ` Thomas Lamprecht
2023-02-22  9:51     ` Christoph Heiss
2023-02-21  8:05 ` [pve-devel] [PATCH v3 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=20230221080550.43336-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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal