From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id B38941FF13F for ; Thu, 26 Feb 2026 11:37:02 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 567BB1BF25; Thu, 26 Feb 2026 11:37:57 +0100 (CET) From: Maximiliano Sandoval To: Moayad Almalat Subject: Re: [PATCH pve-cluster v3] fix #2685: datacenter: allow 4-bit suffix in mac_prefix In-Reply-To: <20260224161418.320892-1-m.almalat@proxmox.com> (Moayad Almalat's message of "Tue, 24 Feb 2026 17:14:18 +0100") References: <20260224161418.320892-1-m.almalat@proxmox.com> User-Agent: mu4e 1.12.9; emacs 30.1 Date: Thu, 26 Feb 2026 11:37:22 +0100 Message-ID: MIME-Version: 1.0 Content-Type: text/plain X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1772102224643 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.094 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: ADUAEKN26XOIYURGBYZRLS62YT3IXDT2 X-Message-ID-Hash: ADUAEKN26XOIYURGBYZRLS62YT3IXDT2 X-MailFrom: m.sandoval@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: pve-devel@lists.proxmox.com X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Moayad Almalat writes: > --- > src/PVE/DataCenterConfig.pm | 2 +- > src/test/Makefile | 9 ++++++++- > src/test/test_mac_prefix.pl | 24 ++++++++++++++++++++++++ > 3 files changed, 33 insertions(+), 2 deletions(-) > create mode 100644 src/test/test_mac_prefix.pl > > diff --git a/src/PVE/DataCenterConfig.pm b/src/PVE/DataCenterConfig.pm > index 514c867..d88b167 100644 > --- a/src/PVE/DataCenterConfig.pm > +++ b/src/PVE/DataCenterConfig.pm > @@ -224,7 +224,7 @@ PVE::JSONSchema::register_format('mac-prefix', \&pve_verify_mac_prefix); > sub pve_verify_mac_prefix { > my ($mac_prefix, $noerr) = @_; > > - if ($mac_prefix !~ m/^[a-f0-9][02468ace](?::[a-f0-9]{2}){0,2}:?$/i) { > + if ($mac_prefix !~ m/^[a-f0-9][02468ace](?::[a-f0-9]{2}){0,2}(?::[a-f0-9]?)?$/i) { > return undef if $noerr; > die "value is not a valid unicast MAC address prefix\n"; > } > diff --git a/src/test/Makefile b/src/test/Makefile > index 757f35f..cdd37d0 100644 > --- a/src/test/Makefile > +++ b/src/test/Makefile > @@ -4,8 +4,15 @@ cpgtest: cpgtest.c > gcc -Wall cpgtest.c $(shell pkg-config --cflags --libs libcpg libqb) -o cpgtest > > .PHONY: check install clean distclean > -check: > +check: corosync-parser-test test-mac-prefix > + > +.PHONY: corosync-parser-test > +corosync-parser-test: > ./corosync_parser_test.pl > > +.PHONY: test-mac-prefix > +test-mac-prefix: > + perl test_mac_prefix.pl > + > distclean: clean > clean: > diff --git a/src/test/test_mac_prefix.pl b/src/test/test_mac_prefix.pl > new file mode 100644 > index 0000000..631874c > --- /dev/null > +++ b/src/test/test_mac_prefix.pl > @@ -0,0 +1,24 @@ > +use strict; > +use warnings; > + > +use Test::More; > + > +use lib ('.', '..'); > + > +use PVE::DataCenterConfig; > + > +my $longest_prefix_len = 10; > +my $prefix = "00:00:00:00:00:00"; > + > +# test that all sub-strings of $prefix longer than "00" and strictly shorter than > +# "00:00:00:00" are OK > +for (my $i = 0; $i <= length($prefix); $i++) { > + my $sub_prefix = substr($prefix, 0, $i); > + if (2 <= $i && $i <= $longest_prefix_len) { > + ok(PVE::DataCenterConfig::pve_verify_mac_prefix($sub_prefix), "$sub_prefix is a valid mac prefix"); > + } else { > + is(PVE::DataCenterConfig::pve_verify_mac_prefix($sub_prefix, 1), undef, "$sub_prefix is not valid mac prefix"); > + } > +} > + > +done_testing(); Looks good on my end. Some comments on the previous state: The prefixes "00:0" and "00:00:00:0" (we have "BC:24:11:0" as an example at the docs and schema's verbose_description) where not allowed by the regex before this patch, now: ``` $ make -C src/test test-mac-prefix make: Entering directory '/home/msandoval/Projects/pve-cluster/src/test' perl test_mac_prefix.pl ok 1 - is not valid mac prefix ok 2 - 0 is not valid mac prefix ok 3 - 00 is a valid mac prefix ok 4 - 00: is a valid mac prefix ok 5 - 00:0 is a valid mac prefix ok 6 - 00:00 is a valid mac prefix ok 7 - 00:00: is a valid mac prefix ok 8 - 00:00:0 is a valid mac prefix ok 9 - 00:00:00 is a valid mac prefix ok 10 - 00:00:00: is a valid mac prefix ok 11 - 00:00:00:0 is a valid mac prefix ok 12 - 00:00:00:00 is not valid mac prefix ok 13 - 00:00:00:00: is not valid mac prefix ok 14 - 00:00:00:00:0 is not valid mac prefix ok 15 - 00:00:00:00:00 is not valid mac prefix ok 16 - 00:00:00:00:00: is not valid mac prefix ok 17 - 00:00:00:00:00:0 is not valid mac prefix ok 18 - 00:00:00:00:00:00 is not valid mac prefix 1..18 $ echo $? 0 ``` note that while "0" is still invalid as a prefix, this patch is a welcome improvement. Reviewed-by: Maximiliano Sandoval -- Maximiliano