all lists on 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 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