From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH pve-common] inotify: interfaces: also set type 'bridge' for empty bridges
Date: Fri, 08 Aug 2025 10:40:44 +0200 [thread overview]
Message-ID: <1754642298.y2oj4cigw2.astroid@yuna.none> (raw)
In-Reply-To: <20250807142141.72815-1-h.laimer@proxmox.com>
On August 7, 2025 4:21 pm, Hannes Laimer wrote:
> If a bridge has `bridge_ports` set to `none` we just skip the field.
> Later we use the existance of the field to determine whether the type
> should be `bridge`. This led to bridges without `bridge_ports` not
> being recognized as bridges.
>
> In the `/nodes/{}/network` we do permissions checks but only for ifaces
> with type `bridge`(or `OVSBridge`). So interfaces were returned by the
> endpoint even if the user did not have permissions the correct
> permissions because the interface did not have type `bridge`.
>
> This fixes this by also setting the type to `bridge` for empty bridges.
>
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
Note: this patch is likely a candidate for stable-bookworm as well?
> src/PVE/INotify.pm | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/src/PVE/INotify.pm b/src/PVE/INotify.pm
> index bbcb9f8..5a42ddd 100644
> --- a/src/PVE/INotify.pm
> +++ b/src/PVE/INotify.pm
> @@ -994,7 +994,10 @@ SECTION: while (defined($line = <$fh>)) {
> } elsif ($id eq 'slaves' || $id eq 'bridge_ports') {
this elsif here is taken for both bonds and bridges
> my $devs = {};
> foreach my $p (split(/\s+/, $value)) {
> - next if $p eq 'none';
> + if ($p eq 'none') {
> + $d->{'is_empty_bridge'} = 1;
so this here should be guarded with `if $id eq 'bridge_ports'`..
do we also need special handling of "empty" bonds (which make less sense
in practice, but might still exist?) - in particular if we ever plan on
dropping the naming restrictions there?
> + next;
> + }
> $devs->{$p} = 1;
> }
> my $str = join(' ', sort keys %{$devs});
> @@ -1077,7 +1080,8 @@ OUTER:
>
> my $ip_link = $ip_links->{$altnames->{$iface} // $iface};
>
> - if (defined $d->{'bridge_ports'}) {
> + if (defined $d->{'bridge_ports'} || $d->{'is_empty_bridge'}) {
> + delete $d->{'is_empty_bridge'} if defined $d->{'is_empty_bridge'};
> $d->{type} = 'bridge';
> if (!defined($d->{bridge_stp})) {
> $d->{bridge_stp} = 'off';
> --
> 2.47.2
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
next prev parent reply other threads:[~2025-08-08 8:39 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-08-07 14:21 Hannes Laimer
2025-08-08 8:40 ` Fabian Grünbichler [this message]
2025-08-08 8:58 ` Hannes Laimer
2025-08-08 9:21 ` Fabian Grünbichler
2025-08-08 9:33 ` [pve-devel] superseded: " Hannes Laimer
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=1754642298.y2oj4cigw2.astroid@yuna.none \
--to=f.gruenbichler@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.