* [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.