[pve-devel] [PATCH network/container/qemu-server v2 0/3] drop unused `firewall` argument to {add, del}_bridge_fdb()

[pve-devel] [PATCH network/container/qemu-server v2 0/3] drop unused `firewall` argument to {add, del}_bridge_fdb()

[Christoph Heiss]

While working on this code, I noticed that the `firewall` argument is
never used (nor even declared) [0] in both
PVE::Network::{add,del}_bridge_fdb().

Thus drop it everywhere and avoid needlessly passing around things which
are never used anyway.

Did some quick smoke-testing and everything kept working fine, but as
there are really no functional changes, this should not effect anything.

[ Basically just a re-spin of [1], just refreshed all patches such that
they apply cleanly again. No actual changes between v1 and v2. ]

[0] https://git.proxmox.com/?p=pve-common.git;a=blob;f=src/PVE/Network.pm;h=a4f5ba96;hb=HEAD#l299
[1] https://lists.proxmox.com/pipermail/pve-devel/2023-March/055970.html

pve-network:

Christoph Heiss (1):
  sdn: zones: drop unused `firewall` argument to {add, del}_bridge_fdb()

 src/PVE/Network/SDN/Zones.pm | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

pve-container:

Christoph Heiss (1):
  net: drop unused `firewall` argument to add_bridge_fdb()

 src/PVE/LXC.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

qemu-server:

Christoph Heiss (1):
  net: drop unused `firewall` argument to {add, del}_bridge_fdb()

 PVE/QemuServer.pm | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

--
2.39.2

[pve-devel] [PATCH network v2 1/1] sdn: zones: drop unused `firewall` argument to {add, del}_bridge_fdb()

[Christoph Heiss]

PVE::Network::{add,del}_bridge_fdb() do not actually have a `firewall`
parameter, so drop it. And since it wasn't used anywhere else in these
subroutines, drop it completely.

No functional changes.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
  * No changes

 src/PVE/Network/SDN/Zones.pm | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/PVE/Network/SDN/Zones.pm b/src/PVE/Network/SDN/Zones.pm
index 7d70e49..1e678ed 100644
--- a/src/PVE/Network/SDN/Zones.pm
+++ b/src/PVE/Network/SDN/Zones.pm
@@ -326,31 +326,31 @@ sub tap_plug {
 }

 sub add_bridge_fdb {
-    my ($iface, $macaddr, $bridge, $firewall) = @_;
+    my ($iface, $macaddr, $bridge) = @_;

     my $vnet = PVE::Network::SDN::Vnets::get_vnet($bridge, 1);
     if (!$vnet) { # fallback for classic bridge
-	PVE::Network::add_bridge_fdb($iface, $macaddr, $firewall);
+	PVE::Network::add_bridge_fdb($iface, $macaddr);
 	return;
     }

     my $plugin_config = get_plugin_config($vnet);
     my $plugin = PVE::Network::SDN::Zones::Plugin->lookup($plugin_config->{type});
-    PVE::Network::add_bridge_fdb($iface, $macaddr, $firewall) if $plugin_config->{'bridge-disable-mac-learning'};
+    PVE::Network::add_bridge_fdb($iface, $macaddr) if $plugin_config->{'bridge-disable-mac-learning'};
 }

 sub del_bridge_fdb {
-    my ($iface, $macaddr, $bridge, $firewall) = @_;
+    my ($iface, $macaddr, $bridge) = @_;

     my $vnet = PVE::Network::SDN::Vnets::get_vnet($bridge, 1);
     if (!$vnet) { # fallback for classic bridge
-	PVE::Network::del_bridge_fdb($iface, $macaddr, $firewall);
+	PVE::Network::del_bridge_fdb($iface, $macaddr);
 	return;
     }

     my $plugin_config = get_plugin_config($vnet);
     my $plugin = PVE::Network::SDN::Zones::Plugin->lookup($plugin_config->{type});
-    PVE::Network::del_bridge_fdb($iface, $macaddr, $firewall) if $plugin_config->{'bridge-disable-mac-learning'};
+    PVE::Network::del_bridge_fdb($iface, $macaddr) if $plugin_config->{'bridge-disable-mac-learning'};
 }

 1;
--
2.41.0

[pve-devel] [PATCH container v2 2/2] net: drop unused `firewall` argument to add_bridge_fdb()

[Christoph Heiss]

PVE::Network::SDN::Zones::add_bridge_fdb() does not actually have a
`firewall` parameter, so drop it.

No functional changes.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
  * No changes

 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 a531ea5..b5bb04b 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -935,7 +935,7 @@ sub net_tap_plug : prototype($$) {

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

[pve-devel] [PATCH qemu-server v2 3/3] net: drop unused `firewall` argument to {add, del}_bridge_fdb()

[Christoph Heiss]

PVE::Network::{add,del}_bridge_fdb() do not actually have a `firewall`
parameter, so drop it. Same for the SDN equivalents.

No functional changes.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
  * No changes

 PVE/QemuServer.pm | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 430661a..589ba2a 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -8634,9 +8634,9 @@ sub add_nets_bridge_fdb {
 	    next;
 	}
 	if ($have_sdn) {
-	    PVE::Network::SDN::Zones::add_bridge_fdb($iface, $mac, $bridge, $net->{firewall});
+	    PVE::Network::SDN::Zones::add_bridge_fdb($iface, $mac, $bridge);
 	} elsif (-d "/sys/class/net/$bridge/bridge") { # avoid fdb management with OVS for now
-	    PVE::Network::add_bridge_fdb($iface, $mac, $net->{firewall});
+	    PVE::Network::add_bridge_fdb($iface, $mac);
 	}
     }
 }
@@ -8653,9 +8653,9 @@ sub del_nets_bridge_fdb {

 	my $bridge = $net->{bridge};
 	if ($have_sdn) {
-	    PVE::Network::SDN::Zones::del_bridge_fdb($iface, $mac, $bridge, $net->{firewall});
+	    PVE::Network::SDN::Zones::del_bridge_fdb($iface, $mac, $bridge);
 	} elsif (-d "/sys/class/net/$bridge/bridge") { # avoid fdb management with OVS for now
-	    PVE::Network::del_bridge_fdb($iface, $mac, $net->{firewall});
+	    PVE::Network::del_bridge_fdb($iface, $mac);
 	}
     }
 }
--
2.41.0

Re: [pve-devel] [PATCH network/container/qemu-server v2 0/3] drop unused `firewall` argument to {add, del}_bridge_fdb()

[Thomas Lamprecht]

Am 05/07/2023 um 16:38 schrieb Christoph Heiss:
> While working on this code, I noticed that the `firewall` argument is
> never used (nor even declared) [0] in both
> PVE::Network::{add,del}_bridge_fdb().
> 
> Thus drop it everywhere and avoid needlessly passing around things which
> are never used anyway.
> 
> Did some quick smoke-testing and everything kept working fine, but as
> there are really no functional changes, this should not effect anything.
> 
> [ Basically just a re-spin of [1], just refreshed all patches such that
> they apply cleanly again. No actual changes between v1 and v2. ]
> 
> [0] https://git.proxmox.com/?p=pve-common.git;a=blob;f=src/PVE/Network.pm;h=a4f5ba96;hb=HEAD#l299
> [1] https://lists.proxmox.com/pipermail/pve-devel/2023-March/055970.html
> 
> pve-network:
> 
> Christoph Heiss (1):
>   sdn: zones: drop unused `firewall` argument to {add, del}_bridge_fdb()
> 
>  src/PVE/Network/SDN/Zones.pm | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> pve-container:
> 
> Christoph Heiss (1):
>   net: drop unused `firewall` argument to add_bridge_fdb()
> 
>  src/PVE/LXC.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> qemu-server:
> 
> Christoph Heiss (1):
>   net: drop unused `firewall` argument to {add, del}_bridge_fdb()
> 
>  PVE/QemuServer.pm | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> --
> 2.39.2
> 
> 

for the record, this was replaced by a series from Alexandre
https://lore.proxmox.com/pve-devel/20230926073942.3212260-1-aderumier@odiso.com/


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