public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH pve-common] network: disable unicast flooding on tap|veth|fwln ports
@ 2021-09-14  0:26 Alexandre Derumier
  2021-09-14  6:32 ` alexandre derumier
  0 siblings, 1 reply; 9+ messages in thread
From: Alexandre Derumier @ 2021-09-14  0:26 UTC (permalink / raw)
  To: pve-devel

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 unicast_flood on tap|veth|fwln port interface.
bridge will learn mac address of the vm|ct, when it send traffic
or when It'll reply to arp requests coming from outside.

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

diff --git a/src/PVE/Network.pm b/src/PVE/Network.pm
index 15838a0..119340f 100644
--- a/src/PVE/Network.pm
+++ b/src/PVE/Network.pm
@@ -207,6 +207,12 @@ sub disable_ipv6 {
     close($fh);
 }
 
+my $bridge_disable_interface_flooding = sub {
+    my ($iface) = @_;
+
+    PVE::ProcFSTools::write_proc_entry("/sys/class/net/$iface/brport/unicast_flood", "0");
+};
+
 my $bridge_add_interface = sub {
     my ($bridge, $iface, $tag, $trunks) = @_;
 
@@ -334,6 +340,7 @@ my $create_firewall_bridge_linux = sub {
     veth_create($vethfw, $vethfwpeer, $bridge);
 
     &$bridge_add_interface($fwbr, $vethfw);
+    &$bridge_disable_interface_flooding($vethfw);
     &$bridge_add_interface($bridge, $vethfwpeer, $tag, $trunks);
 
     &$bridge_add_interface($fwbr, $iface);
@@ -359,6 +366,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_flooding($ovsintport);
 };
 
 my $cleanup_firewall_bridge = sub {
@@ -406,6 +414,7 @@ sub tap_plug {
 	} else {
 	    &$bridge_add_interface($bridge, $iface, $tag, $trunks);
 	}
+	&$bridge_disable_interface_flooding($iface);
 
     } else {
 	&$cleanup_firewall_bridge($iface); # remove stale devices
-- 
2.30.2




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

* Re: [pve-devel] [PATCH pve-common] network: disable unicast flooding on tap|veth|fwln ports
  2021-09-14  0:26 [pve-devel] [PATCH pve-common] network: disable unicast flooding on tap|veth|fwln ports Alexandre Derumier
@ 2021-09-14  6:32 ` alexandre derumier
  2021-09-15 15:33   ` alexandre derumier
  0 siblings, 1 reply; 9+ messages in thread
From: alexandre derumier @ 2021-09-14  6:32 UTC (permalink / raw)
  To: pve-devel

Thinking a little bit more about this,
I think we should add an option in vm/ct nic options, to enable it.

It could break some network where arp timeout is bigger than default
brige ageing-time (5min by default), or with special asymetric
networks.


Le mardi 14 septembre 2021 à 02:26 +0200, Alexandre Derumier a écrit :
> 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 unicast_flood on tap|veth|fwln port interface.
> bridge will learn mac address of the vm|ct, when it send traffic
> or when It'll reply to arp requests coming from outside.
> 
> Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
> ---
>  src/PVE/Network.pm | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/src/PVE/Network.pm b/src/PVE/Network.pm
> index 15838a0..119340f 100644
> --- a/src/PVE/Network.pm
> +++ b/src/PVE/Network.pm
> @@ -207,6 +207,12 @@ sub disable_ipv6 {
>      close($fh);
>  }
>  
> +my $bridge_disable_interface_flooding = sub {
> +    my ($iface) = @_;
> +
> +   
> PVE::ProcFSTools::write_proc_entry("/sys/class/net/$iface/brport/unic
> ast_flood", "0");
> +};
> +
>  my $bridge_add_interface = sub {
>      my ($bridge, $iface, $tag, $trunks) = @_;
>  
> @@ -334,6 +340,7 @@ my $create_firewall_bridge_linux = sub {
>      veth_create($vethfw, $vethfwpeer, $bridge);
>  
>      &$bridge_add_interface($fwbr, $vethfw);
> +    &$bridge_disable_interface_flooding($vethfw);
>      &$bridge_add_interface($bridge, $vethfwpeer, $tag, $trunks);
>  
>      &$bridge_add_interface($fwbr, $iface);
> @@ -359,6 +366,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_flooding($ovsintport);
>  };
>  
>  my $cleanup_firewall_bridge = sub {
> @@ -406,6 +414,7 @@ sub tap_plug {
>         } else {
>             &$bridge_add_interface($bridge, $iface, $tag, $trunks);
>         }
> +       &$bridge_disable_interface_flooding($iface);
>  
>      } else {
>         &$cleanup_firewall_bridge($iface); # remove stale devices





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

* Re: [pve-devel] [PATCH pve-common] network: disable unicast flooding on tap|veth|fwln ports
  2021-09-14  6:32 ` alexandre derumier
@ 2021-09-15 15:33   ` alexandre derumier
  2021-09-15 17:09     ` Thomas Lamprecht
  0 siblings, 1 reply; 9+ messages in thread
From: alexandre derumier @ 2021-09-15 15:33 UTC (permalink / raw)
  To: pve-devel

I have looked at other hypervisors implementations (as it don't see to
have problem with hetzner),


https://listman.redhat.com/archives/libvir-list/2014-December/msg00173.html


https://docs.vmware.com/en/VMware-NSX-T-Data-Center/3.1/administration/GUID-C5752084-A582-4AEA-BD5D-03FE5DBC746E.html


Both vmware && libvirt have a mode to manually manage fdb entries in
bridge mac table.

This will work if only 1mac is behind 1 nic, so it should be an option
(nested hypervisor for examples).

but for classic vm , it could allow to disable unicast_flood &&
learning for the tap interface, but also promisc mode on tap interface!

I was think about add an option on vmbrX  or vnetX directly to
enable/disable.

I'm going to do tests, testing vlan aware && live migration too.




Le mardi 14 septembre 2021 à 08:32 +0200, alexandre derumier a écrit :
> Thinking a little bit more about this,
> I think we should add an option in vm/ct nic options, to enable it.
> 
> It could break some network where arp timeout is bigger than default
> brige ageing-time (5min by default), or with special asymetric
> networks.
> 
> 
> Le mardi 14 septembre 2021 à 02:26 +0200, Alexandre Derumier a écrit :
> > 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 unicast_flood on tap|veth|fwln port interface.
> > bridge will learn mac address of the vm|ct, when it send traffic
> > or when It'll reply to arp requests coming from outside.
> > 
> > Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
> > ---
> >  src/PVE/Network.pm | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/src/PVE/Network.pm b/src/PVE/Network.pm
> > index 15838a0..119340f 100644
> > --- a/src/PVE/Network.pm
> > +++ b/src/PVE/Network.pm
> > @@ -207,6 +207,12 @@ sub disable_ipv6 {
> >      close($fh);
> >  }
> >  
> > +my $bridge_disable_interface_flooding = sub {
> > +    my ($iface) = @_;
> > +
> > +   
> > PVE::ProcFSTools::write_proc_entry("/sys/class/net/$iface/brport/unic
> > ast_flood", "0");
> > +};
> > +
> >  my $bridge_add_interface = sub {
> >      my ($bridge, $iface, $tag, $trunks) = @_;
> >  
> > @@ -334,6 +340,7 @@ my $create_firewall_bridge_linux = sub {
> >      veth_create($vethfw, $vethfwpeer, $bridge);
> >  
> >      &$bridge_add_interface($fwbr, $vethfw);
> > +    &$bridge_disable_interface_flooding($vethfw);
> >      &$bridge_add_interface($bridge, $vethfwpeer, $tag, $trunks);
> >  
> >      &$bridge_add_interface($fwbr, $iface);
> > @@ -359,6 +366,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_flooding($ovsintport);
> >  };
> >  
> >  my $cleanup_firewall_bridge = sub {
> > @@ -406,6 +414,7 @@ sub tap_plug {
> >         } else {
> >             &$bridge_add_interface($bridge, $iface, $tag, $trunks);
> >         }
> > +       &$bridge_disable_interface_flooding($iface);
> >  
> >      } else {
> >         &$cleanup_firewall_bridge($iface); # remove stale devices
> 





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

* Re: [pve-devel] [PATCH pve-common] network: disable unicast flooding on tap|veth|fwln ports
  2021-09-15 15:33   ` alexandre derumier
@ 2021-09-15 17:09     ` Thomas Lamprecht
  2021-09-16 21:48       ` alexandre derumier
  0 siblings, 1 reply; 9+ messages in thread
From: Thomas Lamprecht @ 2021-09-15 17:09 UTC (permalink / raw)
  To: Proxmox VE development discussion, alexandre derumier

On 15.09.21 17:33, alexandre derumier wrote:
> I have looked at other hypervisors implementations (as it don't see to
> have problem with hetzner),
> 
> 
> https://listman.redhat.com/archives/libvir-list/2014-December/msg00173.html
> 
> 
> https://docs.vmware.com/en/VMware-NSX-T-Data-Center/3.1/administration/GUID-C5752084-A582-4AEA-BD5D-03FE5DBC746E.html
> 
> 
> Both vmware && libvirt have a mode to manually manage fdb entries in
> bridge mac table.
> 
> This will work if only 1mac is behind 1 nic, so it should be an option
> (nested hypervisor for examples).
> 
> but for classic vm , it could allow to disable unicast_flood &&
> learning for the tap interface, but also promisc mode on tap interface!
> 
> I was think about add an option on vmbrX  or vnetX directly to
> enable/disable.

As this would be on the VM tap devices it would sound somewhat reasonable to
have it as per vNIC setting, but naturally it would then be a bit annoying to
change for all; a tradeoff could be to allow setting the default value per
bridge, node or datacenter (I'd do only one of those).

What do you think?

> 
> I'm going to do tests, testing vlan aware && live migration too.

great, thanks for your work on this!




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

* Re: [pve-devel] [PATCH pve-common] network: disable unicast flooding on tap|veth|fwln ports
  2021-09-15 17:09     ` Thomas Lamprecht
@ 2021-09-16 21:48       ` alexandre derumier
  2022-01-14 10:51         ` Wolfgang Bumiller
  0 siblings, 1 reply; 9+ messages in thread
From: alexandre derumier @ 2021-09-16 21:48 UTC (permalink / raw)
  To: Thomas Lamprecht, Proxmox VE development discussion

Le mercredi 15 septembre 2021 à 19:09 +0200, Thomas Lamprecht a écrit :
> On 15.09.21 17:33, alexandre derumier wrote:
> > I have looked at other hypervisors implementations (as it don't see
> > to
> > have problem with hetzner),
> > 
> > 
> > https://listman.redhat.com/archives/libvir-list/2014-December/msg00173.html
> > 
> > 
> > https://docs.vmware.com/en/VMware-NSX-T-Data-Center/3.1/administration/GUID-C5752084-A582-4AEA-BD5D-03FE5DBC746E.html
> > 
> > 
> > Both vmware && libvirt have a mode to manually manage fdb entries
> > in
> > bridge mac table.
> > 
> > This will work if only 1mac is behind 1 nic, so it should be an
> > option
> > (nested hypervisor for examples).
> > 
> > but for classic vm , it could allow to disable unicast_flood &&
> > learning for the tap interface, but also promisc mode on tap
> > interface!
> > 
> > I was think about add an option on vmbrX  or vnetX directly to
> > enable/disable.
> 
> As this would be on the VM tap devices it would sound somewhat
> reasonable to
> have it as per vNIC setting, but naturally it would then be a bit
> annoying to
> change for all; a tradeoff could be to allow setting the default
> value per
> bridge, node or datacenter (I'd do only one of those).
> 
> What do you think?
> 
I have done test, I think the best way is to enable it on the bridge
 or vnet for sdn.
because ovs don't support it for example, or its not needed for routed
setup or vxlan.
I don't known too much where add this option for classic vmbr ? in
/etc/network/interfaces ?.
I can't reuse bridge-unicast-flood off, bridge-learning off on vmbr
with ifupdown, because it's apply on all ports (ethX), and we don't
want that.
I could add a custom attribute, but I need to parse
/etc/network/interfaces in this case  for the tap_plug sub. 
For vnet, it's easy.

the worktlow is:

1)
- plug veth/tap iface (+fwbr if firewall)
   - disable unicast_flood + bridge_learning on the tap|veth + fwpr
interfaces

2)
then add static mac with "bridge fdb add <mac> dev <tap|veth|fwpr>
master static.

for 2), for live migration, we need to do it just before the resume of
the target vm
         for normal start, just after the start or after a nic hotplug.

static mac is autoremoved if the interface is removed, so it should be
auto handle by cleanup on vm crash too



As bonus, the benefit to configure it at bridge level, is that if all
egress ports (tap|veth|fwpb) have unicast_flood && learning disable,
the only remaining port (physical ethX ingress), is auto set in non-
promiscous, so bad traffic never go inside the server.
(another requirement is that the bridge need to be vlan-aware, but I
think it could work at herzner is default pvid 1 for untagged traffic)
https://git.backbone.ws/kolan/backbone-sources/commit/2796d0c648c9

https://legacy.netdevconf.info/0.1/docs/netdev_tutorial_bridge_makita_150213.pdf

I'ill try to send patches next week, I'm a bit busy at work this week.


Alexandre


> > 
> > I'm going to do tests, testing vlan aware && live migration too.
> 
> great, thanks for your work on this!
> 



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

* Re: [pve-devel] [PATCH pve-common] network: disable unicast flooding on tap|veth|fwln ports
  2021-09-16 21:48       ` alexandre derumier
@ 2022-01-14 10:51         ` Wolfgang Bumiller
  2022-01-14 11:23           ` Josef Johansson
  2022-01-14 16:50           ` DERUMIER, Alexandre
  0 siblings, 2 replies; 9+ messages in thread
From: Wolfgang Bumiller @ 2022-01-14 10:51 UTC (permalink / raw)
  To: alexandre derumier; +Cc: Thomas Lamprecht, Proxmox VE development discussion

On Thu, Sep 16, 2021 at 11:48:15PM +0200, alexandre derumier wrote:
> Le mercredi 15 septembre 2021 à 19:09 +0200, Thomas Lamprecht a écrit :
> > On 15.09.21 17:33, alexandre derumier wrote:
> > > I have looked at other hypervisors implementations (as it don't see
> > > to
> > > have problem with hetzner),
> > > 
> > > 
> > > https://listman.redhat.com/archives/libvir-list/2014-December/msg00173.html
> > > 
> > > 
> > > https://docs.vmware.com/en/VMware-NSX-T-Data-Center/3.1/administration/GUID-C5752084-A582-4AEA-BD5D-03FE5DBC746E.html
> > > 
> > > 
> > > Both vmware && libvirt have a mode to manually manage fdb entries
> > > in
> > > bridge mac table.
> > > 
> > > This will work if only 1mac is behind 1 nic, so it should be an
> > > option
> > > (nested hypervisor for examples).
> > > 
> > > but for classic vm , it could allow to disable unicast_flood &&
> > > learning for the tap interface, but also promisc mode on tap
> > > interface!
> > > 
> > > I was think about add an option on vmbrX  or vnetX directly to
> > > enable/disable.
> > 
> > As this would be on the VM tap devices it would sound somewhat
> > reasonable to
> > have it as per vNIC setting, but naturally it would then be a bit
> > annoying to
> > change for all; a tradeoff could be to allow setting the default
> > value per
> > bridge, node or datacenter (I'd do only one of those).
> > 
> > What do you think?
> > 
> I have done test, I think the best way is to enable it on the bridge
>  or vnet for sdn.
> because ovs don't support it for example, or its not needed for routed
> setup or vxlan.
> I don't known too much where add this option for classic vmbr ? in
> /etc/network/interfaces ?.
> I can't reuse bridge-unicast-flood off, bridge-learning off on vmbr
> with ifupdown, because it's apply on all ports (ethX), and we don't
> want that.
> I could add a custom attribute, but I need to parse
> /etc/network/interfaces in this case  for the tap_plug sub. 

As far as I can tell, ifupdown2 only applies this to the ports it knows
about, so in theory we *could* start to honor this for the interfaces we
crate for VMs as a default, and have an on/off/auto value on VM network
interfaces (override or use whatever /e/n/interfaces says).

Or do you mean you typically want this to be on for VMs but off
specifically for the physical port? Then /e/n/interfaces won't fit.

Although it *does* allow listing ports and doesn't seem to mind if a
port does not exist, so we *may* get away with saying we expect
something like this:

    bridge-unicast-flood eth0=on _pve=off

Either way, it's a port setting, so I wonder a by-vm-interface optional
override probably makes sense, not sure (but would be easy enough to
do).




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

* Re: [pve-devel] [PATCH pve-common] network: disable unicast flooding on tap|veth|fwln ports
  2022-01-14 10:51         ` Wolfgang Bumiller
@ 2022-01-14 11:23           ` Josef Johansson
  2022-01-28  3:39             ` Josef Johansson
  2022-01-14 16:50           ` DERUMIER, Alexandre
  1 sibling, 1 reply; 9+ messages in thread
From: Josef Johansson @ 2022-01-14 11:23 UTC (permalink / raw)
  To: pve-devel

On 1/14/22 11:51, Wolfgang Bumiller wrote:
> On Thu, Sep 16, 2021 at 11:48:15PM +0200, alexandre derumier wrote:
>> Le mercredi 15 septembre 2021 à 19:09 +0200, Thomas Lamprecht a écrit :
>>> On 15.09.21 17:33, alexandre derumier wrote:
>>>> I have looked at other hypervisors implementations (as it don't see
>>>> to
>>>> have problem with hetzner),
>>>>
>>>>
>>>> https://listman.redhat.com/archives/libvir-list/2014-December/msg00173.html
>>>>
>>>>
>>>> https://docs.vmware.com/en/VMware-NSX-T-Data-Center/3.1/administration/GUID-C5752084-A582-4AEA-BD5D-03FE5DBC746E.html
>>>>
>>>>
>>>> Both vmware && libvirt have a mode to manually manage fdb entries
>>>> in
>>>> bridge mac table.
>>>>
>>>> This will work if only 1mac is behind 1 nic, so it should be an
>>>> option
>>>> (nested hypervisor for examples).
>>>>
>>>> but for classic vm , it could allow to disable unicast_flood &&
>>>> learning for the tap interface, but also promisc mode on tap
>>>> interface!
>>>>
>>>> I was think about add an option on vmbrX  or vnetX directly to
>>>> enable/disable.
>>> As this would be on the VM tap devices it would sound somewhat
>>> reasonable to
>>> have it as per vNIC setting, but naturally it would then be a bit
>>> annoying to
>>> change for all; a tradeoff could be to allow setting the default
>>> value per
>>> bridge, node or datacenter (I'd do only one of those).
>>>
>>> What do you think?
>>>
>> I have done test, I think the best way is to enable it on the bridge
>>  or vnet for sdn.
>> because ovs don't support it for example, or its not needed for routed
>> setup or vxlan.
>> I don't known too much where add this option for classic vmbr ? in
>> /etc/network/interfaces ?.
>> I can't reuse bridge-unicast-flood off, bridge-learning off on vmbr
>> with ifupdown, because it's apply on all ports (ethX), and we don't
>> want that.
>> I could add a custom attribute, but I need to parse
>> /etc/network/interfaces in this case  for the tap_plug sub. 
> As far as I can tell, ifupdown2 only applies this to the ports it knows
> about, so in theory we *could* start to honor this for the interfaces we
> crate for VMs as a default, and have an on/off/auto value on VM network
> interfaces (override or use whatever /e/n/interfaces says).
>
> Or do you mean you typically want this to be on for VMs but off
> specifically for the physical port? Then /e/n/interfaces won't fit.
>
> Although it *does* allow listing ports and doesn't seem to mind if a
> port does not exist, so we *may* get away with saying we expect
> something like this:
>
>     bridge-unicast-flood eth0=on _pve=off
>
> Either way, it's a port setting, so I wonder a by-vm-interface optional
> override probably makes sense, not sure (but would be easy enough to
> do).
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>

Maybe something along the lines of

--- Network.pm	2021-05-25 16:35:27.000000000 +0200
+++ Network.pm.new	2022-01-14 12:20:48.181632198 +0100
@@ -309,6 +309,7 @@ sub veth_create {
     disable_ipv6($vethpeer);
     &$activate_interface($veth);
     &$activate_interface($vethpeer);
+    PVE::Tools::run_command(['/sbin/bridge', 'link', 'set', 'dev', $vethpeer, 'flood', 'off']);
 }
 
 sub veth_delete {


This is basically what I do right to solve this problem. I leave everything else with unicast_flood on.
I do not use ovs yet, so basic bridging.

If I enable it on fwln some ARP-functionality stops working.

Regards
Josef




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

* Re: [pve-devel] [PATCH pve-common] network: disable unicast flooding on tap|veth|fwln ports
  2022-01-14 10:51         ` Wolfgang Bumiller
  2022-01-14 11:23           ` Josef Johansson
@ 2022-01-14 16:50           ` DERUMIER, Alexandre
  1 sibling, 0 replies; 9+ messages in thread
From: DERUMIER, Alexandre @ 2022-01-14 16:50 UTC (permalink / raw)
  To: w.bumiller, aderumier; +Cc: pve-devel, t.lamprecht

Hi Wolfgang,
Thanks for reviewing this.

> 
> 
> As far as I can tell, ifupdown2 only applies this to the ports it
> knows
> about, so in theory we *could* start to honor this for the interfaces
> we
> crate for VMs as a default, and have an on/off/auto value on VM
> network
> interfaces (override or use whatever /e/n/interfaces says).
> 
> Or do you mean you typically want this to be on for VMs but off
> specifically for the physical port? Then /e/n/interfaces won't fit.
> 
yes.

bridge_learning && unicast_flood should be keep "on" on physical
interfaces. (bridge need to forward unknown dest mac to the outside
world and register macs when they coming from outside).

bridge_learning && unicast_flood can be set "off" on the vms.
(and we need to register manually mac address in bridge fdb)
https://lists.proxmox.com/pipermail/pve-devel/2021-September/050089.html
This avoid to flood unknown mac traffic from vmbr0 to vm tap or fwbr
bridge. 



if bridge_learning && unicast_flood is disabled on all vms ports,
and that we only have 1 interface (the physical etX) with
bridge_learning on, prosmic mode is disabled on the bridge.
That mean than traffic with wrong mac, will not enter to the server.


So, this is more secure, but in this case it make more sense to have a
global option instead vm by vm option.

That's why I have added a custom  "bridge-disable-mac-learning" global
bridge option in this patch:
https://lists.proxmox.com/pipermail/pve-devel/2021-September/050088.html



> Although it *does* allow listing ports and doesn't seem to mind if a
> port does not exist, so we *may* get away with saying we expect
> something like this:
> 
>     bridge-unicast-flood eth0=on _pve=off
> 
> Either way, it's a port setting, so I wonder a by-vm-interface
> optional
> override probably makes sense, not sure (but would be easy enough to
> do).
> 

AFAIK, the only problem is if user have multiple mac addresses inside
the vms that we don't known (mainly nested virtualisation).


Vmware have a simple option "promisc" on/off on the vswitch (and I
think it's off by default, they register manually mac). And it need to
be disable when they need to do nested virtu.




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

* Re: [pve-devel] [PATCH pve-common] network: disable unicast flooding on tap|veth|fwln ports
  2022-01-14 11:23           ` Josef Johansson
@ 2022-01-28  3:39             ` Josef Johansson
  0 siblings, 0 replies; 9+ messages in thread
From: Josef Johansson @ 2022-01-28  3:39 UTC (permalink / raw)
  To: pve-devel, Alexandre Derumier

On 1/14/22 12:23, Josef Johansson wrote:
> On 1/14/22 11:51, Wolfgang Bumiller wrote:
>> On Thu, Sep 16, 2021 at 11:48:15PM +0200, alexandre derumier wrote:
>>> Le mercredi 15 septembre 2021 à 19:09 +0200, Thomas Lamprecht a écrit :
>>>> On 15.09.21 17:33, alexandre derumier wrote:
>>>>> I have looked at other hypervisors implementations (as it don't see
>>>>> to
>>>>> have problem with hetzner),
>>>>>
>>>>>
>>>>> https://listman.redhat.com/archives/libvir-list/2014-December/msg00173.html
>>>>>
>>>>>
>>>>> https://docs.vmware.com/en/VMware-NSX-T-Data-Center/3.1/administration/GUID-C5752084-A582-4AEA-BD5D-03FE5DBC746E.html
>>>>>
>>>>>
>>>>> Both vmware && libvirt have a mode to manually manage fdb entries
>>>>> in
>>>>> bridge mac table.
>>>>>
>>>>> This will work if only 1mac is behind 1 nic, so it should be an
>>>>> option
>>>>> (nested hypervisor for examples).
>>>>>
>>>>> but for classic vm , it could allow to disable unicast_flood &&
>>>>> learning for the tap interface, but also promisc mode on tap
>>>>> interface!
>>>>>
>>>>> I was think about add an option on vmbrX  or vnetX directly to
>>>>> enable/disable.
>>>> As this would be on the VM tap devices it would sound somewhat
>>>> reasonable to
>>>> have it as per vNIC setting, but naturally it would then be a bit
>>>> annoying to
>>>> change for all; a tradeoff could be to allow setting the default
>>>> value per
>>>> bridge, node or datacenter (I'd do only one of those).
>>>>
>>>> What do you think?
>>>>
>>> I have done test, I think the best way is to enable it on the bridge
>>>  or vnet for sdn.
>>> because ovs don't support it for example, or its not needed for routed
>>> setup or vxlan.
>>> I don't known too much where add this option for classic vmbr ? in
>>> /etc/network/interfaces ?.
>>> I can't reuse bridge-unicast-flood off, bridge-learning off on vmbr
>>> with ifupdown, because it's apply on all ports (ethX), and we don't
>>> want that.
>>> I could add a custom attribute, but I need to parse
>>> /etc/network/interfaces in this case  for the tap_plug sub. 
>> As far as I can tell, ifupdown2 only applies this to the ports it knows
>> about, so in theory we *could* start to honor this for the interfaces we
>> crate for VMs as a default, and have an on/off/auto value on VM network
>> interfaces (override or use whatever /e/n/interfaces says).
>>
>> Or do you mean you typically want this to be on for VMs but off
>> specifically for the physical port? Then /e/n/interfaces won't fit.
>>
>> Although it *does* allow listing ports and doesn't seem to mind if a
>> port does not exist, so we *may* get away with saying we expect
>> something like this:
>>
>>     bridge-unicast-flood eth0=on _pve=off
>>
>> Either way, it's a port setting, so I wonder a by-vm-interface optional
>> override probably makes sense, not sure (but would be easy enough to
>> do).
>>
>>
>> _______________________________________________
>> pve-devel mailing list
>> pve-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>>
> Maybe something along the lines of
>
> --- Network.pm	2021-05-25 16:35:27.000000000 +0200
> +++ Network.pm.new	2022-01-14 12:20:48.181632198 +0100
> @@ -309,6 +309,7 @@ sub veth_create {
>      disable_ipv6($vethpeer);
>      &$activate_interface($veth);
>      &$activate_interface($vethpeer);
> +    PVE::Tools::run_command(['/sbin/bridge', 'link', 'set', 'dev', $vethpeer, 'flood', 'off']);
>  }
>  
>  sub veth_delete {
>
>
> This is basically what I do right to solve this problem. I leave everything else with unicast_flood on.
> I do not use ovs yet, so basic bridging.
>
> If I enable it on fwln some ARP-functionality stops working.
>
> Regards
> Josef
>
I'm landing on these commits, I've tested this one and it seems to work
properly.
--- a/usr/share/perl5/PVE/Network.pm    2022-01-28 04:20:22.704806437 +0100
+++ b/usr/share/perl5/PVE/Network.pm.new    2022-01-28
04:20:10.256959699 +0100
@@ -335,6 +335,13 @@ my $create_firewall_bridge_linux = sub {
     &$bridge_add_interface($fwbr, $vethfw);
     &$bridge_add_interface($bridge, $vethfwpeer, $tag, $trunks);

+    # When the server does not know where to send traffic, it will
broadcast
+    # the traffic onto every port on a bridge. There's two cases where
this is
+    # not wanted. Servers can siphone traffic not intended for it but also
+    # cause the first request (between correct servers) to fail. Leave
$vethpeer
+    # since otherwise ARP between HV and VM will not work.
+    PVE::Tools::run_command(['/sbin/bridge', 'link', 'set', 'dev',
$vethfwpeer, 'flood', 'off']);
+
     &$bridge_add_interface($fwbr, $iface);
 };

This one is not tested, but should in theory be the same. One thing that
can happen is that outbound traffic from this server is not working
properly. I will do some testing with this soon.

--- /usr/share/perl5/PVE/Network.pm    2022-01-28 04:21:10.524217677 +0100
+++ /usr/share/perl5/PVE/Network.pm.new    2022-01-28 04:35:23.921695488
+0100
@@ -411,6 +411,9 @@ sub tap_plug {
         &$create_firewall_bridge_linux($iface, $bridge, $tag, $trunks);
     } else {
         &$bridge_add_interface($bridge, $iface, $tag, $trunks);
+
+        # Do not allow VMs to siphone traffic not destined for them
+        PVE::Tools::run_command(['/sbin/bridge', 'link', 'set', 'dev',
$iface, 'flood', 'off']);
     }
 
     } else {


Let me know what you think! And I'll make proper patches.
Regards
- Josef



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

end of thread, other threads:[~2022-01-28  3:39 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-14  0:26 [pve-devel] [PATCH pve-common] network: disable unicast flooding on tap|veth|fwln ports Alexandre Derumier
2021-09-14  6:32 ` alexandre derumier
2021-09-15 15:33   ` alexandre derumier
2021-09-15 17:09     ` Thomas Lamprecht
2021-09-16 21:48       ` alexandre derumier
2022-01-14 10:51         ` Wolfgang Bumiller
2022-01-14 11:23           ` Josef Johansson
2022-01-28  3:39             ` Josef Johansson
2022-01-14 16:50           ` DERUMIER, Alexandre

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