public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH proxmox-widget-toolkit] ui: mac-prefix-validation
@ 2024-11-08 11:30 Moayad Almalat
  2024-11-08 11:41 ` Shannon Sterz
  0 siblings, 1 reply; 3+ messages in thread
From: Moayad Almalat @ 2024-11-08 11:30 UTC (permalink / raw)
  To: pve-devel


Allow four-octet MAC prefixes in Web UI validation
update the MAC prefix validation in the Web UI to support four-octet
prefixes.

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 8a0138d..42dbfaa 100644
--- a/src/Toolkit.js
+++ b/src/Toolkit.js
@@ -70,7 +70,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,3}:?$/i).test(v);
     },
     MacPrefixMask: /[a-fA-F0-9:]/,
     MacPrefixText: gettext('Example') + ': 02:8f - ' + gettext('only unicast addresses are allowed'),
-- 
2.39.5


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [pve-devel] [PATCH proxmox-widget-toolkit] ui: mac-prefix-validation
  2024-11-08 11:30 [pve-devel] [PATCH proxmox-widget-toolkit] ui: mac-prefix-validation Moayad Almalat
@ 2024-11-08 11:41 ` Shannon Sterz
  2026-02-05 15:56   ` Maximiliano Sandoval
  0 siblings, 1 reply; 3+ messages in thread
From: Shannon Sterz @ 2024-11-08 11:41 UTC (permalink / raw)
  To: Proxmox VE development discussion

On Fri Nov 8, 2024 at 12:30 PM CET, Moayad Almalat wrote:
>
> Allow four-octet MAC prefixes in Web UI validation
> update the MAC prefix validation in the Web UI to support four-octet
> prefixes.
>
> 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 8a0138d..42dbfaa 100644
> --- a/src/Toolkit.js
> +++ b/src/Toolkit.js
> @@ -70,7 +70,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,3}:?$/i).test(v);

this still does not match `BC:24:11:0` as we recommend in our docs as
this regex requires the fourth octet to also be complete and not just a
nible. you could do something like this:

```
/^[a-f0-9][02468ace](?::[a-f0-9]{2}){0,2}(?::[a-f0-9]{0,2})?:?$/i
```

this would allow the last octet to be incomplete too, but i wonder if at
that point we want to generally allow longer prefixes and just make sure
that a prefix address is not a) a multicast prefix b) a broadcast prefix
and c) a complete set of 6 octets. that may be simpler and more future
proof?

>      },
>      MacPrefixMask: /[a-fA-F0-9:]/,
>      MacPrefixText: gettext('Example') + ': 02:8f - ' + gettext('only unicast addresses are allowed'),



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [pve-devel] [PATCH proxmox-widget-toolkit] ui: mac-prefix-validation
  2024-11-08 11:41 ` Shannon Sterz
@ 2026-02-05 15:56   ` Maximiliano Sandoval
  0 siblings, 0 replies; 3+ messages in thread
From: Maximiliano Sandoval @ 2026-02-05 15:56 UTC (permalink / raw)
  To: Shannon Sterz; +Cc: Proxmox VE development discussion

"Shannon Sterz" <s.sterz@proxmox.com> writes:

> On Fri Nov 8, 2024 at 12:30 PM CET, Moayad Almalat wrote:
>>
>> Allow four-octet MAC prefixes in Web UI validation
>> update the MAC prefix validation in the Web UI to support four-octet
>> prefixes.
>>
>> 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 8a0138d..42dbfaa 100644
>> --- a/src/Toolkit.js
>> +++ b/src/Toolkit.js
>> @@ -70,7 +70,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,3}:?$/i).test(v);
>
> this still does not match `BC:24:11:0` as we recommend in our docs as
> this regex requires the fourth octet to also be complete and not just a
> nible. you could do something like this:
>
> ```
> /^[a-f0-9][02468ace](?::[a-f0-9]{2}){0,2}(?::[a-f0-9]{0,2})?:?$/i
> ```
>
> this would allow the last octet to be incomplete too, but i wonder if at
> that point we want to generally allow longer prefixes and just make sure
> that a prefix address is not a) a multicast prefix b) a broadcast prefix
> and c) a complete set of 6 octets. that may be simpler and more future
> proof?

Regarding this point, if you pick an apple from a bag with N apples k
times, returning the apple to the bag each time, the probability for
picking different apples each time is

f(k,N) = 1 * (N - 1)/N * ... * (N - (k - 1)) / N

thus the probability of picking at least one apple more than once (which
would be bad) would be g(k,N) = 1 - f(k, N).

With our current longest prefix `00:00:00:` we have 16^6 = 16.777.216
available mac addresses, and with this patch with the longest prefix
`00:00:00:00:` we are down to 16^4 = 65.536 options. One can check that

g(100, 16777216) = 0.000295
g(100, 1048576) = 0.004710  # with a prefix like 00:00:00:0
g(100, 65536) = 0.072784
g(100, 4096) = 0.072784  # if we allowed 00:00:00:00:0

So the chances of mac address collisions on a system with 100 guest goes
from ~0.03% to ~7.3% with the proposal here, adding one more hex value
would leave us at a ~73% probability of having at least one collision.
Given the effect of such a collision I would rather be conservative on
how much this is extended.

>
>>      },
>>      MacPrefixMask: /[a-fA-F0-9:]/,
>>      MacPrefixText: gettext('Example') + ': 02:8f - ' + gettext('only unicast addresses are allowed'),
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

-- 
Maximiliano




^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-02-05 15:56 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-11-08 11:30 [pve-devel] [PATCH proxmox-widget-toolkit] ui: mac-prefix-validation Moayad Almalat
2024-11-08 11:41 ` Shannon Sterz
2026-02-05 15:56   ` Maximiliano Sandoval

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal