all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [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.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal