From: Aaron Lauterer <a.lauterer@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: Re: [pve-devel] [PATCH common 1/3] fix #3893: network: make bridge vids configurable
Date: Fri, 26 May 2023 10:33:52 +0200 [thread overview]
Message-ID: <94344ee1-ce15-5196-3d0b-1984df902b24@proxmox.com> (raw)
In-Reply-To: <20230413151021.2882582-1-a.lauterer@proxmox.com>
ping?
On 4/13/23 17:10, Aaron Lauterer wrote:
> For that we need to add a new format option that checks against valid
> VLAN tags and ranges, for example: 2 4 100-200
>
> The check, if the default value should be used, needs to fail not just
> when not defined, but also in case it is an empty string.
>
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
>
> Notes:
> I think replacing the 'defined' check with 'length' should be fine.
> We need to also handle the situation that the parameter is defined,
> but an empty string. There should be no autovivification happening.
> If I missed a side effect, let me know.
>
> For the new format option I went with singular for the name as it
> only checks a single VLAN ID/range from the list, 'pve-bridge-vid',
> but I am not sure if it wouldn't be better to call it the actual
> parameter name 'pve-bridge-vids'.
>
> src/PVE/INotify.pm | 2 +-
> src/PVE/JSONSchema.pm | 32 ++++++++++++++++++++++++++++++++
> 2 files changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/src/PVE/INotify.pm b/src/PVE/INotify.pm
> index bc33a8f..14f40ac 100644
> --- a/src/PVE/INotify.pm
> +++ b/src/PVE/INotify.pm
> @@ -1270,7 +1270,7 @@ sub __interface_to_string {
>
> if (defined($d->{bridge_vlan_aware})) {
> $raw .= "\tbridge-vlan-aware yes\n";
> - my $vlans = defined($d->{bridge_vids}) ? $d->{bridge_vids} : "2-4094";
> + my $vlans = length($d->{bridge_vids}) ? $d->{bridge_vids} : "2-4094";
> $raw .= "\tbridge-vids $vlans\n";
> }
> $done->{bridge_vlan_aware} = 1;
> diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
> index 527e409..d81a567 100644
> --- a/src/PVE/JSONSchema.pm
> +++ b/src/PVE/JSONSchema.pm
> @@ -78,6 +78,12 @@ register_standard_option('pve-iface', {
> minLength => 2, maxLength => 20,
> });
>
> +register_standard_option('pve-bridge-vid', {
> + description => "Bridge VLAN ID.",
> + type => 'string', format => 'pve-bridge-vid',
> + minLength => 1, maxLength => 9,
> +});
> +
> register_standard_option('pve-storage-id', {
> description => "The storage identifier.",
> type => 'string', format => 'pve-storage-id',
> @@ -588,6 +594,32 @@ sub pve_verify_iface {
> return $id;
> }
>
> +# bridge vlan id (vids)
> +register_format('pve-bridge-vid', \&pve_verify_bridge_vid);
> +sub pve_verify_bridge_vid {
> + my ($vlan, $noerr) = @_;
> +
> + my $check_vid = sub {
> + my $id = shift;
> + if ( $id < 2 || $id > 4094) {
> + return undef if $noerr;
> + die "invalid VLAN tag '$id'\n";
> + }
> + };
> +
> + if ($vlan !~ m/^(\d+)([-](\d+))?$/i) {
> + return undef if $noerr;
> + die "invalid VLAN configuration '$vlan'\n";
> + }
> + $check_vid->($1);
> + if ($3) {
> + $check_vid->($3);
> + die "VLAN range must go from lower to higher tag '$vlan'" if $1 > $3 && !$noerr;
> + }
> +
> + return $vlan;
> +}
> +
> # general addresses by name or IP
> register_format('address', \&pve_verify_address);
> sub pve_verify_address {
prev parent reply other threads:[~2023-05-26 8:34 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-13 15:10 Aaron Lauterer
2023-04-13 15:10 ` [pve-devel] [PATCH manager 2/3] fix #3893: api: network: add bridge_vids parameter Aaron Lauterer
2023-04-13 15:10 ` [pve-devel] [PATCH widget-toolkit 3/3] fix #3892: NetworkEdit: add bridge vids field for bridge_vids Aaron Lauterer
2023-06-01 14:24 ` Thomas Lamprecht
2023-06-02 13:10 ` Aaron Lauterer
2023-06-07 16:12 ` Thomas Lamprecht
2023-05-26 8:33 ` Aaron Lauterer [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=94344ee1-ce15-5196-3d0b-1984df902b24@proxmox.com \
--to=a.lauterer@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