all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Josef Johansson <josef@oderland.se>
To: pve-devel@lists.proxmox.com, Alexandre Derumier <aderumier@odiso.com>
Subject: Re: [pve-devel] [PATCH pve-common] network: disable unicast flooding on tap|veth|fwln ports
Date: Fri, 28 Jan 2022 04:39:22 +0100	[thread overview]
Message-ID: <72aee949-c0bc-03c7-181c-880f97c3d1f9@oderland.se> (raw)
In-Reply-To: <6bc3b9db-a4f2-1172-c717-7802c971e54f@oderland.se>

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



  reply	other threads:[~2022-01-28  3:39 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-14  0:26 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 [this message]
2022-01-14 16:50           ` DERUMIER, Alexandre

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=72aee949-c0bc-03c7-181c-880f97c3d1f9@oderland.se \
    --to=josef@oderland.se \
    --cc=aderumier@odiso.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