[pve-devel] [PATCH pve-common 0/2] add disable bridge learning feature

[pve-devel] [PATCH pve-common 0/2] add disable bridge learning feature

[Alexandre Derumier]

Currently, if bridge receive an unknown dest mac (network bug/attack/..),
we are flooding packets to all bridge ports.

This can waste cpu time, even more with firewall enabled.
Also, if firewall is used with reject action, the src mac of RST
packet is the original unknown dest mac.
(This can block the server at Hetzner for example)

So, we can disable learning && unicast_flood on tap|veth|fwln port interface.
Then mac address need to be add statically in bridge fdb.


Alexandre Derumier (2):
  network: add support for disabling bridge learning on tap|veth|fwln
    ports
  Inotify: add bridge-disable-mac-learning option to bridges.

 src/PVE/INotify.pm |  4 +++-
 src/PVE/Network.pm | 60 +++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 57 insertions(+), 7 deletions(-)

-- 
2.30.2

[pve-devel] [PATCH pve-common 1/2] network: add support for disabling bridge learning on tap|veth|fwln ports

[Alexandre Derumier]

Currently, if bridge receive an unknown dest mac (network bug/attack/..),
we are flooding packets to all bridge ports.

This can waste cpu time, even more with firewall enabled.
Also, if firewall is used with reject action, the src mac of RST
packet is the original unknown dest mac.
(This can block the server at Hetzner for example)

So, we can disable learning && unicast_flood on tap|veth|fwln port interface.
Then mac address need to be add statically in bridge fdb.

Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
---
 src/PVE/Network.pm | 60 +++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 54 insertions(+), 6 deletions(-)

diff --git a/src/PVE/Network.pm b/src/PVE/Network.pm
index 15838a0..2d99af0 100644
--- a/src/PVE/Network.pm
+++ b/src/PVE/Network.pm
@@ -207,6 +207,14 @@ sub disable_ipv6 {
     close($fh);
 }
 
+my $bridge_disable_interface_learning = sub {
+    my ($iface) = @_;
+
+    PVE::ProcFSTools::write_proc_entry("/sys/class/net/$iface/brport/unicast_flood", "0");
+    PVE::ProcFSTools::write_proc_entry("/sys/class/net/$iface/brport/learning", "0");
+
+};
+
 my $bridge_add_interface = sub {
     my ($bridge, $iface, $tag, $trunks) = @_;
 
@@ -268,6 +276,43 @@ my $activate_interface = sub {
     die "can't activate interface '$iface' - $@\n" if $@;
 };
 
+sub add_bridge_fdb {
+    my ($iface, $mac) = @_;
+
+    my $learning = PVE::Tools::file_read_firstline("/sys/class/net/$iface/brport/learning");
+    return if $learning;
+
+    my ($vmid, $devid) = &$parse_tap_device_name($iface, 1);
+    return if !defined($vmid);
+
+    PVE::Tools::run_command(['/sbin/bridge', 'fdb', 'append', $mac, 'dev', $iface, 'master', 'static']);
+
+    my ($fwbr, $vethfw, $vethfwpeer, $ovsintport) = &$compute_fwbr_names($vmid, $devid);
+
+    if (-d "/sys/class/net/$vethfwpeer") {
+	PVE::Tools::run_command(['/sbin/bridge', 'fdb', 'append', $mac, 'dev', $vethfwpeer, 'master', 'static']);
+    }
+
+}
+
+sub del_bridge_fdb {
+    my ($iface, $mac) = @_;
+
+    my $learning = PVE::Tools::file_read_firstline("/sys/class/net/$iface/brport/learning");
+    return if $learning;
+
+    my ($vmid, $devid) = &$parse_tap_device_name($iface, 1);
+    return if !defined($vmid);
+
+    PVE::Tools::run_command(['/sbin/bridge', 'fdb', 'del', $mac, 'dev', $iface, 'master', 'static']);
+
+    my ($fwbr, $vethfw, $vethfwpeer, $ovsintport) = &$compute_fwbr_names($vmid, $devid);
+
+    if (-d "/sys/class/net/$vethfwpeer") {
+	PVE::Tools::run_command(['/sbin/bridge', 'fdb', 'del', $mac, 'dev', $vethfwpeer, 'master', 'static']);
+    }
+}
+
 sub tap_create {
     my ($iface, $bridge) = @_;
 
@@ -322,7 +367,7 @@ sub veth_delete {
 }
 
 my $create_firewall_bridge_linux = sub {
-    my ($iface, $bridge, $tag, $trunks) = @_;
+    my ($iface, $bridge, $tag, $trunks, $disablelearning) = @_;
 
     my ($vmid, $devid) = &$parse_tap_device_name($iface);
     my ($fwbr, $vethfw, $vethfwpeer) = &$compute_fwbr_names($vmid, $devid);
@@ -333,14 +378,15 @@ my $create_firewall_bridge_linux = sub {
     copy_bridge_config($bridge, $fwbr);
     veth_create($vethfw, $vethfwpeer, $bridge);
 
-    &$bridge_add_interface($fwbr, $vethfw);
     &$bridge_add_interface($bridge, $vethfwpeer, $tag, $trunks);
+    &$bridge_disable_interface_learning($vethfwpeer) if $disablelearning;
+    &$bridge_add_interface($fwbr, $vethfw);
 
     &$bridge_add_interface($fwbr, $iface);
 };
 
 my $create_firewall_bridge_ovs = sub {
-    my ($iface, $bridge, $tag, $trunks) = @_;
+    my ($iface, $bridge, $tag, $trunks, $disablelearning) = @_;
 
     my ($vmid, $devid) = &$parse_tap_device_name($iface);
     my ($fwbr, undef, undef, $ovsintport) = &$compute_fwbr_names($vmid, $devid);
@@ -359,6 +405,7 @@ my $create_firewall_bridge_ovs = sub {
     PVE::Tools::run_command(['/sbin/ip', 'link', 'set', $ovsintport, 'mtu', $bridgemtu]);
 
     &$bridge_add_interface($fwbr, $ovsintport);
+    &$bridge_disable_interface_learning($ovsintport) if $disablelearning;
 };
 
 my $cleanup_firewall_bridge = sub {
@@ -383,7 +430,7 @@ my $cleanup_firewall_bridge = sub {
 };
 
 sub tap_plug {
-    my ($iface, $bridge, $tag, $firewall, $trunks, $rate) = @_;
+    my ($iface, $bridge, $tag, $firewall, $trunks, $rate, $disablelearning) = @_;
 
     #cleanup old port config from any openvswitch bridge
     eval {run_command("/usr/bin/ovs-vsctl del-port $iface", outfunc => sub {}, errfunc => sub {}) };
@@ -402,16 +449,17 @@ sub tap_plug {
 	}
 
 	if ($firewall) {
-	    &$create_firewall_bridge_linux($iface, $bridge, $tag, $trunks);
+	    &$create_firewall_bridge_linux($iface, $bridge, $tag, $trunks, $disablelearning);
 	} else {
 	    &$bridge_add_interface($bridge, $iface, $tag, $trunks);
 	}
+	&$bridge_disable_interface_learning($iface) if $disablelearning;
 
     } else {
 	&$cleanup_firewall_bridge($iface); # remove stale devices
 
 	if ($firewall) {
-	    &$create_firewall_bridge_ovs($iface, $bridge, $tag, $trunks);
+	    &$create_firewall_bridge_ovs($iface, $bridge, $tag, $trunks, $disablelearning);
 	} else {
 	    &$ovs_bridge_add_port($bridge, $iface, $tag, undef, $trunks);
 	}
-- 
2.30.2

[pve-devel] [PATCH pve-common 2/2] Inotify: add bridge-disable-mac-learning option to bridges.

[Alexandre Derumier]

This is an internal option, only used by proxmox, and not ifupdown1/2

Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
---
 src/PVE/INotify.pm | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/PVE/INotify.pm b/src/PVE/INotify.pm
index 4ff63e8..db942b1 100644
--- a/src/PVE/INotify.pm
+++ b/src/PVE/INotify.pm
@@ -884,7 +884,7 @@ sub __read_etc_network_interfaces {
 	'bridge-fd' => 'bridge_fd',
 	'bridge-stp' => 'bridge_stp',
 	'bridge-ports' => 'bridge_ports',
-	'bridge-vids' => 'bridge_vids'
+	'bridge-vids' => 'bridge_vids',
     };
 
     my $line;
@@ -958,6 +958,7 @@ sub __read_etc_network_interfaces {
 			'bridge-arp-nd-suppress' => 1,
 			'bridge-unicast-flood' => 1,
 			'bridge-multicast-flood' => 1,
+			'bridge-disable-mac-learning' => 1,
 			'bond_miimon' => 1,
 			'bond_xmit_hash_policy' => 1,
 			'bond-primary' => 1,
@@ -1282,6 +1283,7 @@ sub __interface_to_string {
 
 	$raw .= "\tmtu $d->{mtu}\n" if $d->{mtu};
 	$done->{mtu} = 1;
+	$done->{'bridge-disable-mac-learning'} = 1;
 
     } elsif ($d->{type} eq 'bond') {
 
-- 
2.30.2

Re: [pve-devel] [PATCH pve-common 0/2] add disable bridge learning feature

[Thomas Lamprecht]

On 24.09.21 10:48, Alexandre Derumier wrote:
> Currently, if bridge receive an unknown dest mac (network bug/attack/..),
> we are flooding packets to all bridge ports.
> 
> This can waste cpu time, even more with firewall enabled.
> Also, if firewall is used with reject action, the src mac of RST
> packet is the original unknown dest mac.
> (This can block the server at Hetzner for example)
> 
> So, we can disable learning && unicast_flood on tap|veth|fwln port interface.
> Then mac address need to be add statically in bridge fdb.

I'm a bit out of the loop of the with the whole bad hetzner network thingy, is this still
relevant as I'd see if I can get it in finally..

Re: [pve-devel] [PATCH pve-common 0/2] add disable bridge learning feature

[Josef Johansson]

On 11/11/21 11:40, Thomas Lamprecht wrote:
> On 24.09.21 10:48, Alexandre Derumier wrote:
>> Currently, if bridge receive an unknown dest mac (network bug/attack/..),
>> we are flooding packets to all bridge ports.
>>
>> This can waste cpu time, even more with firewall enabled.
>> Also, if firewall is used with reject action, the src mac of RST
>> packet is the original unknown dest mac.
>> (This can block the server at Hetzner for example)
>>
>> So, we can disable learning && unicast_flood on tap|veth|fwln port interface.
>> Then mac address need to be add statically in bridge fdb.
> I'm a bit out of the loop of the with the whole bad hetzner network thingy, is this still
> relevant as I'd see if I can get it in finally..
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>

Hi,

Is it not enough to turn off unicast_flood on fwpr*?

If I have unicast_flood on fwln some scenarios does not work.

I have been running it a while now and it seems to solve all odd quirks
we've had with the networking on PVE.

Regards
Josef

Re: [pve-devel] [PATCH pve-common 0/2] add disable bridge learning feature

[DERUMIER, Alexandre]

Le jeudi 11 novembre 2021 à 11:40 +0100, Thomas Lamprecht a écrit :
On 24.09.21 10:48, Alexandre Derumier wrote:
Currently, if bridge receive an unknown dest mac (network bug/attack/..),
we are flooding packets to all bridge ports.

This can waste cpu time, even more with firewall enabled.
Also, if firewall is used with reject action, the src mac of RST
packet is the original unknown dest mac.
(This can block the server at Hetzner for example)

So, we can disable learning && unicast_flood on tap|veth|fwln port interface.
Then mac address need to be add statically in bridge fdb.

I'm a bit out of the loop of the with the whole bad hetzner network thingy, is this still
relevant as I'd see if I can get it in finally..

yes, it can still help with hetzner.
it should allow to use REJECT rules in firewall, as the reject rst packet src ip,is the original dst ip.
when bad traffic is flooded up to the fwbr bridge with unknown dst mac/ip. (because bridges are in promisc mode).



I think this patch series could be usefull for some sdn controllers,

like evpn, where we can statically register mac, but also ip address (not yet implement in this patch serie) in the controlplane.
(instead of using arp dynamic registration, this can have problem sometime with silent hosts and arp timeout)
they are also some sdn (not yet implement, but faucet sdn or some openflow sdn controller), where mac/ip registration really need to be done statically at vm start.

[pve-devel] applied-series: [PATCH pve-common 0/2] add disable bridge learning feature

[Thomas Lamprecht]

On 24.09.21 10:48, Alexandre Derumier wrote:
> Currently, if bridge receive an unknown dest mac (network bug/attack/..),
> we are flooding packets to all bridge ports.
> 
> This can waste cpu time, even more with firewall enabled.
> Also, if firewall is used with reject action, the src mac of RST
> packet is the original unknown dest mac.
> (This can block the server at Hetzner for example)
> 
> So, we can disable learning && unicast_flood on tap|veth|fwln port interface.
> Then mac address need to be add statically in bridge fdb.
> 
> 
> Alexandre Derumier (2):
>   network: add support for disabling bridge learning on tap|veth|fwln
>     ports
>   Inotify: add bridge-disable-mac-learning option to bridges.
> 
>  src/PVE/INotify.pm |  4 +++-
>  src/PVE/Network.pm | 60 +++++++++++++++++++++++++++++++++++++++++-----
>  2 files changed, 57 insertions(+), 7 deletions(-)
> 



applied, thanks! But I moved from the single flag to an $opts hash for the tap_plug
option, nicer to use than those overly long parameter flags list, that often have
lots of slightly confusing undef mixed in.

You need to adapt the calling site of the relevant open patches though (sorry for
the added work).

Re: [pve-devel] applied-series: [PATCH pve-common 0/2] add disable bridge learning feature

[DERUMIER, Alexandre]

Hi Thomas,

I'm currently on a proxmox training session this week, so I'll not have
time to work on it until next week.

BTW, I have also an pending bugfix for mtu && ovs here, but I think I
need to rebase it now that others patches are applied, so I'll rework
it for next week.

https://lists.proxmox.com/pipermail/pve-devel/2022-February/051808.html


And, If you have time, I'll like to include this patch before release
of qemu 6.2  (new balloon option free-page-reporting)
https://lists.proxmox.com/pipermail/pve-devel/2022-March/051940.html



Thanks for your time !

Alexandre

Le mercredi 16 mars 2022 à 17:33 +0100, Thomas Lamprecht a écrit :
> On 24.09.21 10:48, Alexandre Derumier wrote:
> > Currently, if bridge receive an unknown dest mac (network
> > bug/attack/..),
> > we are flooding packets to all bridge ports.
> > 
> > This can waste cpu time, even more with firewall enabled.
> > Also, if firewall is used with reject action, the src mac of RST
> > packet is the original unknown dest mac.
> > (This can block the server at Hetzner for example)
> > 
> > So, we can disable learning && unicast_flood on tap|veth|fwln port
> > interface.
> > Then mac address need to be add statically in bridge fdb.
> > 
> > 
> > Alexandre Derumier (2):
> >   network: add support for disabling bridge learning on
> > tap|veth|fwln
> >     ports
> >   Inotify: add bridge-disable-mac-learning option to bridges.
> > 
> >  src/PVE/INotify.pm |  4 +++-
> >  src/PVE/Network.pm | 60 +++++++++++++++++++++++++++++++++++++++++-
> > ----
> >  2 files changed, 57 insertions(+), 7 deletions(-)
> > 
> 
> 
> 
> applied, thanks! But I moved from the single flag to an $opts hash
> for the tap_plug
> option, nicer to use than those overly long parameter flags list,
> that often have
> lots of slightly confusing undef mixed in.
> 
> You need to adapt the calling site of the relevant open patches
> though (sorry for
> the added work).
>