all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH v3 common/docs/widget-toolkit/manager/firewall 0/6] drop vmbr prefix for bridges
Date: Thu, 11 Apr 2024 16:21:48 +0200	[thread overview]
Message-ID: <1712845023.55m9ezau3w.astroid@yuna.none> (raw)
In-Reply-To: <20240229104104.111188-1-s.hanreich@proxmox.com>

Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>

with some small nits for the docs patch, see comment there.

the pve-common patch should probably get the bug number (545) in the
subject as well.

some hint about the inter-dependencies of the patches would be nice, AFAIU:
- pve-manager requires proxmox-widget-toolkit
- the UI changes require pve-common, but since that is cluster-wide, a
  dep there only helps the local node as usual (still, it might be a
  good idea so at least that combo is covered for sure ;))
- the firewall change is rather independent, since it's just the
  simulator that is affected at worst it won't handle a new interface
  name.

I think this one would make quite a few users happy (at least those that
haven't yet switched to SDN with its more flexible naming scheme ;))

On February 29, 2024 11:40 am, Stefan Hanreich wrote:
> Original patch series by Jillian Morgan <jillian.morgan@primordial.ca>
> 
> I've refrained from adding arbitrary bond names in this patch series, since
> that would require a bigger amount of changes in the firewall simulator. I'll
> look into adding that in a future patch series.
> 
> Changes from v2 -> v3:
>   * limit bridge names to 10 characters in Web UI
>   * introduce common variable for bridge names in firewall simulator
>   * update docs to reflect changes
>   * add warning for interfaces named vmbrX that are not bridges
> 
> Changes from v1 -> v2:
>   * limit name to 15 characters
>   * properly validate bridge names in vlan/qinq zones
>   * properly handle OVS bridges
>   * handle new bridge names in firewall simulator
> 
> 
> 
> common:
> 
> Stefan Hanreich (1):
>   interfaces: allow arbitrary bridge names in network config
> 
>  src/PVE/INotify.pm | 28 +++++++++++++++-------------
>  1 file changed, 15 insertions(+), 13 deletions(-)
> 
> 
> docs:
> 
> Stefan Hanreich (1):
>   network: update specification for bridge names
> 
>  pve-network.adoc | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> 
> widget-toolkit:
> 
> Stefan Hanreich (1):
>   network: allow bridges to have any valid interface name
> 
>  src/Toolkit.js          | 4 ++--
>  src/node/NetworkEdit.js | 4 +++-
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> 
> manager:
> 
> Stefan Hanreich (2):
>   sdn: qinq: vlan: properly validate bridge name
>   sdn: vlan: fix indentation in vlan edit dialogue
> 
>  www/manager6/sdn/zones/QinQEdit.js |  3 +++
>  www/manager6/sdn/zones/VlanEdit.js | 11 +++++++----
>  2 files changed, 10 insertions(+), 4 deletions(-)
> 
> 
> firewall:
> 
> Stefan Hanreich (1):
>   simulator: use new bridge naming scheme
> 
>  src/PVE/FirewallSimulator.pm    | 29 +++++++++++++++++++----------
>  src/PVE/Service/pve_firewall.pm |  5 +++--
>  2 files changed, 22 insertions(+), 12 deletions(-)
> 
> 
> Summary over all repositories:
>   8 files changed, 61 insertions(+), 38 deletions(-)
> 
> -- 
> murpp v0.4.0
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 




  parent reply	other threads:[~2024-04-11 14:22 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-29 10:40 Stefan Hanreich
2024-02-29 10:40 ` [pve-devel] [PATCH v3 common 1/6] interfaces: allow arbitrary bridge names in network config Stefan Hanreich
2024-02-29 10:41 ` [pve-devel] [PATCH v3 docs 2/6] network: update specification for bridge names Stefan Hanreich
2024-04-11 14:21   ` Fabian Grünbichler
2024-02-29 10:41 ` [pve-devel] [PATCH v3 widget-toolkit 3/6] network: allow bridges to have any valid interface name Stefan Hanreich
2024-02-29 10:41 ` [pve-devel] [PATCH v3 manager 4/6] sdn: qinq: vlan: properly validate bridge name Stefan Hanreich
2024-02-29 10:41 ` [pve-devel] [PATCH v3 manager 5/6] sdn: vlan: fix indentation in vlan edit dialogue Stefan Hanreich
2024-02-29 10:41 ` [pve-devel] [PATCH v3 firewall 6/6] simulator: use new bridge naming scheme Stefan Hanreich
2024-04-11 14:21 ` Fabian Grünbichler [this message]
2024-04-12  7:46   ` [pve-devel] [PATCH v3 common/docs/widget-toolkit/manager/firewall 0/6] drop vmbr prefix for bridges Stefan Hanreich

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=1712845023.55m9ezau3w.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.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal