[pve-devel] applied: [PATCH v3] do not allow multicast MAC-addresses

[Thomas Lamprecht]

On 3/12/19 4:07 PM, Stoiko Ivanov wrote:
> As discussed on the list [0] setting a multicast mac-address breaks starting
> containers. This is most evident if you set a mac_prefix for your cluster, with
> leading to multicast macs (odd second hexdigit of the first byte).
> 
> During my quick tests I noticed that setting a multicast mac breaks most/all
> use-cases (LXC refuses to start and seemingly does so since 2010 [1], and Qemu
> happily starts, but the link of the nic cannot be set to up (the guest kernel
> refuses)). The current version enforces unicast mac addresses by:
> * adapting the 'mac-address' format in PVE::JSONSchema
> * registering a standard_option (also in PVE::JSONSchema
> * using this in pve-container and qemu-server
> * registering and using a format 'mac-prefix' in PVE::Cluster
> 
> The fourth patch (for proxmox-widget-toolkit) just provides visual feedback for
> users trying to set such a mac_prefix.
> 
> I assume the potential for regression not to be too high, because a user would
> need to have set a multicast mac_prefix, and in that case they would not get
> a link with/could not start their guests.
> The only other current use-case for the mac-address format (WOL of shutdown
> nodes) should also only need to work for unicast mac-addresses.
> 
> Changes from v2:
> * incorporated Thomas' feeback (thanks!) by using the already present format
>   and removing 2 occurences of the same regex
> * bumped the versioned dependencies on libpve-common-perl in pve-container and
>   qemu-server
> * fixed the tests that broke in pve-container (they worked in v2, because the
>   generated errors from a regex mismatch didn't exit, but were treated as
>   warning)
> 

applied the remaining ones too, two things:
* I reordered the pve-container patches, it's never good to introduce temporary
  build failures, so I moved the "fix test" patch before the change to the new,
  more restrictive, hwaddr format
* it's good to note new (build)-dependencies, but most of the time that's also
  enough, if you send those you often risk that they are not up-to-date once the
  patches are applied, and thus useless. Also you forgot to bump the pve-container
  build dependency.

thanks!

> Changes from v1:
> * put the new regex enforcing uni-cast macs into pve-container/qemu-server's
>   JSONSchema, to provide direct feedback to users while doing any operation
>   with such a guest
> * adapted proxmox-widget-toolkit to provide feedback in the GUI
> 
> [0] https://pve.proxmox.com/pipermail/pve-devel/2019-March/035996.html
> [1] https://lists.linuxcontainers.org/pipermail/lxc-users/2010-August/000783.html
> 
> pve-cluster:
> Stoiko Ivanov (1):
>   mac_prefix: do not allow multicast prefixes
> 
>  data/PVE/Cluster.pm | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> pve-common:
> Stoiko Ivanov (1):
>   JSONSchema: Adapt MAC address format to unicast
> 
>  src/PVE/JSONSchema.pm | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> pve-container:
> Stoiko Ivanov (3):
>   enforce unicast MAC for netconfig
>   fix tests with multicast macs
>   d/control: bump version dependency of libpve-common-perl
> 
>  debian/control                  | 2 +-
>  src/PVE/LXC/Config.pm           | 8 ++------
>  src/test/test-debian-012/config | 4 ++--
>  src/test/test-debian-013/config | 2 +-
>  src/test/test-debian-014/config | 4 ++--
>  5 files changed, 8 insertions(+), 12 deletions(-)
> 
> qemu-server:
> Stoiko Ivanov (2):
>   enforce unicast MAC for $net_fmt
>   d/control: bump2 version dependency of libpve-common-perl
> 
>  PVE/QemuServer.pm | 8 ++------
>  debian/control    | 4 ++--
>  2 files changed, 4 insertions(+), 8 deletions(-)
> 
> proxmox-widget-toolkit:
> Stoiko Ivanov (1):
>   MacPrefix: prevent multicast MacPrefixes
> 
>  Toolkit.js | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>