Re: [PATCH pve-cluster v3] fix #2685: datacenter: allow 4-bit suffix in mac_prefix

[Maximiliano Sandoval <m.sandoval@proxmox.com>]

Moayad Almalat <m.almalat@proxmox.com> 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 <m.sandoval@proxmox.com>

-- 
Maximiliano