public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Hannes Laimer <h.laimer@proxmox.com>
To: Gabriel Goller <g.goller@proxmox.com>
Cc: pve-devel@lists.proxmox.com
Subject: Re: [PATCH pve-network] sdn: zones: vxlan: restore the flood entries after an FRR reload
Date: Mon, 5 Oct 2026 15:32:25 +0200	[thread overview]
Message-ID: <b2a3058f-cb41-4dfb-92ba-4d476d3abde9@proxmox.com> (raw)
In-Reply-To: <asOhXfszaAQPyo9q@luna.proxmox.com>

On 2026-10-05 15:13, Gabriel Goller wrote:
> On 05.10.2026 15:01, Hannes Laimer wrote:
>> On 2026-10-05 14:31, Gabriel Goller wrote:
>>> On 29.09.2026 11:28, Hannes Laimer wrote:
>>>> [snip]
>>>> diff --git a/src/PVE/Network/SDN.pm b/src/PVE/Network/SDN.pm
>>>> index 33a3cf3..42af00d 100644
>>>> --- a/src/PVE/Network/SDN.pm
>>>> +++ b/src/PVE/Network/SDN.pm
>>>> @@ -492,7 +492,12 @@ sub generate_frr_config {
>>>>      my $raw_config = PVE::Network::SDN::generate_frr_raw_config($running_config, $fabric_config);
>>>>      PVE::Network::SDN::Frr::write_raw_config($raw_config);
>>>>  
>>>> -    PVE::Network::SDN::Frr::apply($needs_restart) if $apply;
>>>> +    return if !$apply;
>>>> +
>>>> +    PVE::Network::SDN::Frr::apply($needs_restart);
>>>> +
>>>> +    # zebra removes a VXLAN zone's static flood entries with the VTEPs it withdraws
>>>> +    PVE::Network::SDN::Zones::restore_vxlan_flood_entries();
>>>
>>> Hmm so this is tricky, on reload bgpd tells zebra to remove these changes
>>> and zebra enqueues this to the dplane, so this is asynchronous in many
>>> ways. So we can't guarantee that the entries have been removed when running
>>> restore_vxlan_flood_entries().
>>>
>>
>> yes, this is unfortunately not something we can guarantee. restart
>> happens already at the end of FRR::apply, we could force a `reload` of
>> there is a removed evpn controller and an existing vxlan zone. I'm not
>> sure there is a difference for this here tough..
> 
> Hmm the problem is with the reload though isn't it? A restart would simply kill
> all the daemons and zebra on startup doesn't nuke the fdb entries right?
> 

actually not sure, we did write the frr config already at that point,
but without the controller.. no, i guess, as you say it shouldn't

>>> Not sure what we could do here.
>>>
>>> Three options that came to my mind where:
>>> 1) sleep(5)
>>
>> not sure what kind of timeframes are realistic here, since we can't
>> guarantee anything anyway, something like 1s is probably enough. but in
>> my (limited) testing i also couldn't hit this without any sleep
>>
>>> 2) `bridge monitor fdb` before running the frr reload/restart and then check if the entries have been removed (kind of overkill)
>>
>> we still wouldn't know how long to wait..
> 
> True. At least we could shortcut it when they are gone.

yes, but as you said, a (or no) sleep is probably better here

> 
>>> 3) force a frr restart (not 100% sure if this works)
>>
>> ideally we could just tell frr 'hands off these VNIs..', but we can't,
>> and given `advertise-all-vni` that might also be weird/wrong to begin
>> with
> 
> Yeah, don't think thats possible.
> But IMO forcing a restart when advertise-all-vni is removed wouldn't even be so
> bad. Of course that would break other connections the user has open.

that would actually work i think.. could be seen as an unnecessary one
though, depending how easily the "remove after our add" would actually
be hit..

> 
>>>>  }
>>>>  
>>>>  sub generate_dhcp_config {
>>>> diff --git a/src/PVE/Network/SDN/Zones.pm b/src/PVE/Network/SDN/Zones.pm
>>>> index f668303..2c3c69c 100644
>>>> --- a/src/PVE/Network/SDN/Zones.pm
>>>> +++ b/src/PVE/Network/SDN/Zones.pm
>>>> @@ -178,6 +178,24 @@ sub generate_etc_network_config {
>>>>      return $raw_network_config;
>>>>  }
>>>>  
>>>> +sub restore_vxlan_flood_entries {
>>>> +    my $raw_config = eval { PVE::Tools::file_get_contents($local_network_sdn_file) };
>>>> +    return if !defined($raw_config);
>>>
>>> Maybe an error here would be nice instead of a silent return. Although this
>>> probably can't happen anyway...
>>>
>>
>> yeah, not really an error imho.. but a warning should be fine,
>> because the caller assumed there is a config file
>>
>>
>> thanks for taking a look! :)
>>
>>>> +
>>>> [snip]





      reply	other threads:[~2026-10-05 13:32 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-29  9:28 [PATCH pve-network] sdn: zones: vxlan: restore the flood entries after an FRR reload Hannes Laimer
2026-10-05 12:31 ` Gabriel Goller
2026-10-05 13:01   ` Hannes Laimer
2026-10-05 13:13     ` Gabriel Goller
2026-10-05 13:32       ` Hannes Laimer [this message]

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=b2a3058f-cb41-4dfb-92ba-4d476d3abde9@proxmox.com \
    --to=h.laimer@proxmox.com \
    --cc=g.goller@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