* [PATCH proxmox-widget-toolkit v2] fix #2685: ui: allow 4-bit mac_prefix suffix
@ 2026-02-24 12:05 Moayad Almalat
2026-02-24 13:00 ` Fabian Grünbichler
2026-02-24 15:27 ` Maximiliano Sandoval
0 siblings, 2 replies; 5+ messages in thread
From: Moayad Almalat @ 2026-02-24 12:05 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Moayad Almalat <m.almalat@proxmox.com>
---
src/Toolkit.js | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/Toolkit.js b/src/Toolkit.js
index d4e579b..39513b8 100644
--- a/src/Toolkit.js
+++ b/src/Toolkit.js
@@ -72,7 +72,7 @@ Ext.apply(Ext.form.field.VTypes, {
MacAddressText: gettext('Example') + ': 01:23:45:67:89:ab',
MacPrefix: function (v) {
- return /^[a-f0-9][02468ace](?::[a-f0-9]{2}){0,2}:?$/i.test(v);
+ return /^(?:[a-f0-9][02468ace](?::[a-f0-9]{2}){0,2}:?|[a-f0-9][02468ace](?::[a-f0-9]{2}){2}:[a-f0-9])$/i.test(v);
},
MacPrefixMask: /[a-fA-F0-9:]/,
MacPrefixText:
--
2.47.3
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH proxmox-widget-toolkit v2] fix #2685: ui: allow 4-bit mac_prefix suffix
2026-02-24 12:05 [PATCH proxmox-widget-toolkit v2] fix #2685: ui: allow 4-bit mac_prefix suffix Moayad Almalat
@ 2026-02-24 13:00 ` Fabian Grünbichler
2026-02-24 14:29 ` Moayad Almalat
2026-02-24 15:27 ` Maximiliano Sandoval
1 sibling, 1 reply; 5+ messages in thread
From: Fabian Grünbichler @ 2026-02-24 13:00 UTC (permalink / raw)
To: Moayad Almalat, pve-devel
typo in the subject - a 4-bit prefix makes no sense ;)
On February 24, 2026 1:05 pm, Moayad Almalat wrote:
> Signed-off-by: Moayad Almalat <m.almalat@proxmox.com>
> ---
> src/Toolkit.js | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/Toolkit.js b/src/Toolkit.js
> index d4e579b..39513b8 100644
> --- a/src/Toolkit.js
> +++ b/src/Toolkit.js
> @@ -72,7 +72,7 @@ Ext.apply(Ext.form.field.VTypes, {
> MacAddressText: gettext('Example') + ': 01:23:45:67:89:ab',
>
> MacPrefix: function (v) {
> - return /^[a-f0-9][02468ace](?::[a-f0-9]{2}){0,2}:?$/i.test(v);
> + return /^(?:[a-f0-9][02468ace](?::[a-f0-9]{2}){0,2}:?|[a-f0-9][02468ace](?::[a-f0-9]{2}){2}:[a-f0-9])$/i.test(v);
I think this is not quite correct - the [02468ace] part here is for only
allowing unicast addresses, but that only applies to the first octet!
the old regex allowed 1-3 octets (with the first octet always being
restricted to not allow multicast), and you want to extend it to 4
octets AFAIU? that would simply require replacing the `{0,2}` with a
`{0,3}`, I think?
but note that the backend in PVE has the same checks and would need to
be adapted as well..
> },
> MacPrefixMask: /[a-fA-F0-9:]/,
> MacPrefixText:
> --
> 2.47.3
>
>
>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH proxmox-widget-toolkit v2] fix #2685: ui: allow 4-bit mac_prefix suffix
2026-02-24 13:00 ` Fabian Grünbichler
@ 2026-02-24 14:29 ` Moayad Almalat
2026-02-24 15:13 ` Shannon Sterz
0 siblings, 1 reply; 5+ messages in thread
From: Moayad Almalat @ 2026-02-24 14:29 UTC (permalink / raw)
To: Fabian Grünbichler, pve-devel
Hi,
Thank you!
On 2/24/26 2:00 PM, Fabian Grünbichler wrote:
> typo in the subject - a 4-bit prefix makes no sense 😉
In the subject I meant to say 4-bit suffix extension to mac_prefix. I
can send v3 if needed.
>
> On February 24, 2026 1:05 pm, Moayad Almalat wrote:
>> Signed-off-by: Moayad Almalat <m.almalat@proxmox.com>
>> ---
>> src/Toolkit.js | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/Toolkit.js b/src/Toolkit.js
>> index d4e579b..39513b8 100644
>> --- a/src/Toolkit.js
>> +++ b/src/Toolkit.js
>> @@ -72,7 +72,7 @@ Ext.apply(Ext.form.field.VTypes, {
>> MacAddressText: gettext('Example') + ': 01:23:45:67:89:ab',
>> MacPrefix: function (v) {
>> - return /^[a-f0-9][02468ace](?::[a-f0-9]{2}){0,2}:?$/i.test(v);
>> + return
/^(?:[a-f0-9][02468ace](?::[a-f0-9]{2}){0,2}:?|[a-f0-9][02468ace](?::[a-f0-9]{2}){2}:[a-f0-9])$/i.test(v);
>
> I think this is not quite correct - the [02468ace] part here is for only
> allowing unicast addresses, but that only applies to the first octet!
>
> the old regex allowed 1-3 octets (with the first octet always being
> restricted to not allow multicast), and you want to extend it to 4
> octets AFAIU? that would simply require replacing the `{0,2}` with a
> `{0,3}`, I think?
>
> but note that the backend in PVE has the same checks and would need to
> be adapted as well..
>
The unicast restrection applies only to the first octet, both
alternatives start at the beginning, so that part is unchanged.
The intent is to allow a single extra hex nibble after 3 full octets
e.g., `BC:24:11:0` and I tested that in 3 LXC:
```
root@pve-a:/tmp# pct config 102 | grep hwaddr
....hwaddr=BC:24:11:0E:0B:F3,ip=dhcp,ip6=auto,type=veth
root@pve-a:/tmp# pct config 103 | grep hwaddr
....hwaddr=BC:24:11:0A:DD:DD,ip=dhcp,ip6=auto,type=veth
root@pve-a:/tmp# pct config 104 | grep hwaddr
....hwaddr=BC:24:11:06:58:1C,ip=dhcp,ip6=auto,type=veth
root@pve-a:/tmp#
```
>> },
>> MacPrefixMask: /[a-fA-F0-9:]/,
>> MacPrefixText:
>> --
>> 2.47.3
>>
>>
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH proxmox-widget-toolkit v2] fix #2685: ui: allow 4-bit mac_prefix suffix
2026-02-24 14:29 ` Moayad Almalat
@ 2026-02-24 15:13 ` Shannon Sterz
0 siblings, 0 replies; 5+ messages in thread
From: Shannon Sterz @ 2026-02-24 15:13 UTC (permalink / raw)
To: Moayad Almalat; +Cc: pve-devel
On Tue Feb 24, 2026 at 3:29 PM CET, Moayad Almalat wrote:
> Hi,
>
> Thank you!
>
> On 2/24/26 2:00 PM, Fabian Grünbichler wrote:
> > typo in the subject - a 4-bit prefix makes no sense 😉
> In the subject I meant to say 4-bit suffix extension to mac_prefix. I
> can send v3 if needed.
> >
> > On February 24, 2026 1:05 pm, Moayad Almalat wrote:
> >> Signed-off-by: Moayad Almalat <m.almalat@proxmox.com>
> >> ---
> >> src/Toolkit.js | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/src/Toolkit.js b/src/Toolkit.js
> >> index d4e579b..39513b8 100644
> >> --- a/src/Toolkit.js
> >> +++ b/src/Toolkit.js
> >> @@ -72,7 +72,7 @@ Ext.apply(Ext.form.field.VTypes, {
> >> MacAddressText: gettext('Example') + ': 01:23:45:67:89:ab',
> >> MacPrefix: function (v) {
> >> - return /^[a-f0-9][02468ace](?::[a-f0-9]{2}){0,2}:?$/i.test(v);
> >> + return
> /^(?:[a-f0-9][02468ace](?::[a-f0-9]{2}){0,2}:?|[a-f0-9][02468ace](?::[a-f0-9]{2}){2}:[a-f0-9])$/i.test(v);
> >
> > I think this is not quite correct - the [02468ace] part here is for only
> > allowing unicast addresses, but that only applies to the first octet!
> >
> > the old regex allowed 1-3 octets (with the first octet always being
> > restricted to not allow multicast), and you want to extend it to 4
> > octets AFAIU? that would simply require replacing the `{0,2}` with a
> > `{0,3}`, I think?
> >
> > but note that the backend in PVE has the same checks and would need to
> > be adapted as well..
> >
> The unicast restrection applies only to the first octet, both
> alternatives start at the beginning, so that part is unchanged.
>
> The intent is to allow a single extra hex nibble after 3 full octets
> e.g., `BC:24:11:0` and I tested that in 3 LXC:
> ```
> root@pve-a:/tmp# pct config 102 | grep hwaddr
> ....hwaddr=BC:24:11:0E:0B:F3,ip=dhcp,ip6=auto,type=veth
> root@pve-a:/tmp# pct config 103 | grep hwaddr
> ....hwaddr=BC:24:11:0A:DD:DD,ip=dhcp,ip6=auto,type=veth
> root@pve-a:/tmp# pct config 104 | grep hwaddr
> ....hwaddr=BC:24:11:06:58:1C,ip=dhcp,ip6=auto,type=veth
> root@pve-a:/tmp#
> ```
>
the regex is more confusing than it needs to be tho. why not simply do
the the following:
/^[a-f0-9][02468ace](?::[a-f0-9]{2}){0,2}(?::[a-f0-9]?)?$/i
this simply extends the existing regex with the additional prefix
instead of duplicating most of the regex. also i think what fabian meant
is, that this setting is also validated on the backend. so the regex
will need to be adapted in `pve_verify_mac_prefix()` in
`pve-cluster/src/PVE/DataCenterConfig.pm` for this to work as intended.
please also adjust the commit message to something that reflects the
intent better too. thanks!
> >> },
> >> MacPrefixMask: /[a-fA-F0-9:]/,
> >> MacPrefixText:
> >> --
> >> 2.47.3
> >>
> >>
> >>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH proxmox-widget-toolkit v2] fix #2685: ui: allow 4-bit mac_prefix suffix
2026-02-24 12:05 [PATCH proxmox-widget-toolkit v2] fix #2685: ui: allow 4-bit mac_prefix suffix Moayad Almalat
2026-02-24 13:00 ` Fabian Grünbichler
@ 2026-02-24 15:27 ` Maximiliano Sandoval
1 sibling, 0 replies; 5+ messages in thread
From: Maximiliano Sandoval @ 2026-02-24 15:27 UTC (permalink / raw)
To: Moayad Almalat; +Cc: pve-devel
Moayad Almalat <m.almalat@proxmox.com> writes:
> Signed-off-by: Moayad Almalat <m.almalat@proxmox.com>
> ---
> src/Toolkit.js | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/Toolkit.js b/src/Toolkit.js
> index d4e579b..39513b8 100644
> --- a/src/Toolkit.js
> +++ b/src/Toolkit.js
> @@ -72,7 +72,7 @@ Ext.apply(Ext.form.field.VTypes, {
> MacAddressText: gettext('Example') + ': 01:23:45:67:89:ab',
>
> MacPrefix: function (v) {
> - return /^[a-f0-9][02468ace](?::[a-f0-9]{2}){0,2}:?$/i.test(v);
> + return /^(?:[a-f0-9][02468ace](?::[a-f0-9]{2}){0,2}:?|[a-f0-9][02468ace](?::[a-f0-9]{2}){2}:[a-f0-9])$/i.test(v);
> },
> MacPrefixMask: /[a-fA-F0-9:]/,
> MacPrefixText:
I had the following stashed locally:
```
modified 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:
new file 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();
```
Perhaps this can be added here either in a followup or in v3.
This is pre-existing issue, but at the moment we only allow for 8bit,
16bit or 24bit suffixes, e.g. 00:00:0 is not allowed. The test above
will test any possible prefix lenght from 00 to 00:00:00:0.
--
Maximiliano
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-02-24 15:27 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-02-24 12:05 [PATCH proxmox-widget-toolkit v2] fix #2685: ui: allow 4-bit mac_prefix suffix Moayad Almalat
2026-02-24 13:00 ` Fabian Grünbichler
2026-02-24 14:29 ` Moayad Almalat
2026-02-24 15:13 ` Shannon Sterz
2026-02-24 15:27 ` Maximiliano Sandoval
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.