From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <o.bektas@proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256)
(No client certificate requested)
by lists.proxmox.com (Postfix) with ESMTPS id 3EC7D7D6F1
for <pve-devel@lists.proxmox.com>; Tue, 9 Nov 2021 11:58:16 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 14D03A625
for <pve-devel@lists.proxmox.com>; Tue, 9 Nov 2021 11:57:46 +0100 (CET)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
[94.136.29.106])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
key-exchange X25519 server-signature RSA-PSS (2048 bits))
(No client certificate requested)
by firstgate.proxmox.com (Proxmox) with ESMTPS id 22520A61A
for <pve-devel@lists.proxmox.com>; Tue, 9 Nov 2021 11:57:45 +0100 (CET)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
by proxmox-new.maurer-it.com (Proxmox) with ESMTP id E7F3F4273D
for <pve-devel@lists.proxmox.com>; Tue, 9 Nov 2021 11:57:44 +0100 (CET)
Date: Tue, 9 Nov 2021 11:57:42 +0100
From: Oguz Bektas <o.bektas@proxmox.com>
To: Thomas Lamprecht <t.lamprecht@proxmox.com>
Cc: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Message-ID: <YYpUJlC8CMsbc5J4@gaia>
Mail-Followup-To: Oguz Bektas <o.bektas@proxmox.com>,
Thomas Lamprecht <t.lamprecht@proxmox.com>,
Proxmox VE development discussion <pve-devel@lists.proxmox.com>
References: <20211108134400.1884660-1-o.bektas@proxmox.com>
<20211108134400.1884660-2-o.bektas@proxmox.com>
<1546d7cd-9a15-6444-075d-9fe1268d8b63@proxmox.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1546d7cd-9a15-6444-075d-9fe1268d8b63@proxmox.com>
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.815 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
information. [gentoo.pm, openwrt.pm, ubuntu.pm, fedora.pm, setup.pm,
unmanaged.pm, suse.pm]
Subject: Re: [pve-devel] [PATCH container 1/2] fix #2044: add openwrt support
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Tue, 09 Nov 2021 10:58:16 -0000
hi,
thanks for the comments :)
On Mon, Nov 08, 2021 at 04:11:06PM +0100, Thomas Lamprecht wrote:
> no sign-off and zero commit message stating thoughts about design choices
> (like just adding the last named configured vNIC which as single interface
> to the CT network config?!) what openWRT is based on (e.g., that it basically
> just uses busybox) and a, at least basic, pve-docs patch would be also good
> to have..
>
> On 08.11.21 14:43, Oguz Bektas wrote:
> > ---
> > src/PVE/LXC/Setup.pm | 4 ++
> > src/PVE/LXC/Setup/Makefile | 1 +
> > src/PVE/LXC/Setup/OpenWrt.pm | 82 ++++++++++++++++++++++++++++++++++++
>
> file/module name should be cased OpenWRT
hmm, they call it 'OpenWrt' everywhere on their website though, also on
wikipedia etc. it's spelled in this way ;)
>
> > 3 files changed, 87 insertions(+)
> > create mode 100644 src/PVE/LXC/Setup/OpenWrt.pm
> >
> > diff --git a/src/PVE/LXC/Setup.pm b/src/PVE/LXC/Setup.pm
> > index 5cc56af..e89886f 100644
> > --- a/src/PVE/LXC/Setup.pm
> > +++ b/src/PVE/LXC/Setup.pm
> > @@ -17,6 +17,7 @@ use PVE::LXC::Setup::Fedora;
> > use PVE::LXC::Setup::Gentoo;
> > use PVE::LXC::Setup::SUSE;
> > use PVE::LXC::Setup::Ubuntu;
> > +use PVE::LXC::Setup::OpenWrt;
> > use PVE::LXC::Setup::Unmanaged;
> >
> > my $plugins = {
> > @@ -28,6 +29,7 @@ my $plugins = {
> > fedora => 'PVE::LXC::Setup::Fedora',
> > gentoo => 'PVE::LXC::Setup::Gentoo',
> > opensuse => 'PVE::LXC::Setup::SUSE',
> > + openwrt => 'PVE::LXC::Setup::OpenWrt',
> > ubuntu => 'PVE::LXC::Setup::Ubuntu',
> > unmanaged => 'PVE::LXC::Setup::Unmanaged',
> > };
> > @@ -75,6 +77,8 @@ my $autodetect_type = sub {
> > return "alpine";
> > } elsif (-f "$rootdir/etc/gentoo-release") {
> > return "gentoo";
> > + } elsif (-f "$rootdir/etc/openwrt_release") {
> > + return "openwrt";
> > } elsif (-f "$rootdir/etc/os-release") {
> > die "unable to detect OS distribution\n";
> > } else {
> > diff --git a/src/PVE/LXC/Setup/Makefile b/src/PVE/LXC/Setup/Makefile
> > index 04ee2e4..22702ca 100644
> > --- a/src/PVE/LXC/Setup/Makefile
> > +++ b/src/PVE/LXC/Setup/Makefile
> > @@ -9,6 +9,7 @@ SOURCES=\
> > Fedora.pm \
> > Gentoo.pm \
> > SUSE.pm \
> > + OpenWrt.pm \
> > Ubuntu.pm \
> > Unmanaged.pm \
> >
> > diff --git a/src/PVE/LXC/Setup/OpenWrt.pm b/src/PVE/LXC/Setup/OpenWrt.pm
> > new file mode 100644
> > index 0000000..89ea740
> > --- /dev/null
> > +++ b/src/PVE/LXC/Setup/OpenWrt.pm
> > @@ -0,0 +1,82 @@
> > +package PVE::LXC::Setup::OpenWrt;
> > +
> > +use strict;
> > +use warnings;
> > +
> > +use PVE::LXC;
> > +use PVE::Network;
> > +use File::Path;
> > +
> > +use PVE::LXC::Setup::Base;
> > +use base qw(PVE::LXC::Setup::Base);
> > +
> > +my $known_versions = {
> > + '21.02.1' => 1,
>
> so a simple upgrade from 21.02.1 -> 21.02.2 may already break CT start?
>
> Busybox based CTs tend to be quite forgiving and not that likely to change,
> so I'd not fatally die on an unknown (newer) version, maybe just checking
> for > 21.02 would be enough of a safety net.
yes that makes sense, i was mainly concerned with the current version
working but should be fine to allow future newer versions as well.
>
> > +};
> > +
> > +sub new {
> > + my ($class, $conf, $rootdir) = @_;
> > +
> > + my $version;
> > + my $release = PVE::Tools::file_get_contents("$rootdir/etc/openwrt_release");
> > + if ($release =~ m/^DISTRIB_RELEASE=\'(\d+\.\d+\.\d+)\'$/mi) {
> > + $version = $1;
> > + }
> > + die "unsupported OpenWrt version '$version'\n"
> > + if !$known_versions->{$version};
> > +
> > + my $self = { conf => $conf, rootdir => $rootdir, version => $version };
> > +
> > + $conf->{ostype} = "openwrt";
> > +
> > + return bless $self, $class;
> > +}
> > +
> > +sub template_fixup {
> > + my ($self, $conf) = @_;
> > +}
> > +
> > +sub setup_init {
> > + my ($self, $conf) = @_;
> > +}
> > +
> > +sub setup_network {
> > + my ($self, $conf) = @_;
> > +
> > + my $d;
> > + foreach my $k (keys %$conf) {
> > + next if $k !~ m/^net(\d+)$/;
> > + $d = PVE::LXC::Config->parse_lxc_network($conf->{$k});
> > + next if !$d->{name};
>
> what is this and why does it selects it a random (perl's `keys` is actively shuffling
> stuff around) different netX config to use on each new start?!
>
> I'd figure that such nondeterministic behavior would be rather confusing and undesired
> for an user..
ah yes. i guess it would make sense to sort the keys before. i wanted to
add the interfaces in a loop but then got lost, i'll fix that in v2.
>
> > + }
> > +
> > + my $proto = ($d->{ip} eq 'dhcp') ? 'dhcp' : 'static';
> > + my $ip = "";
> > + if ($proto eq 'static') {
> > + $ip = $d->{ip};
> > + }
> > +
> > + my $data = <<"DATA";
> > +config interface 'loopback'
> > + option proto 'static'
> > + option ipaddr '127.0.0.1'
> > + option netmask '255.0.0.0'
> > + option device 'lo'
> > +
> > +config interface 'wan'
> > + option proto '$proto'
> > + option device '$d->{name}'
> > + option ipaddr '$ip'
> > + option netmask '255.255.255.0'
>
> so no IPv6 and not multiple interfaces and no dhcp (for testing)? I'd imagine that some
> user may not be too happy with that, especially as they can configure such things without
> any complaint from the backend, at least some good reasoning in a commit message and short
> hints in code comments would be good _if_ we go for that overly simple config printer, I
> mean we surely do not need to support all of openWRT, nor can't we, but at least a minimum
> basic set of common used stuff would be nice, as would be multiple interfaces.
dhcp works (rather hackily but it does :D), see the line with 'proto'
above. we just leave $ip empty in case of dhcp, otherwise it takes the
'static' param in there.
ipv6 can be configured in the same way by adding 'wan6' interface and
doing the same.