On Tue, May 28, 2024 at 09:59:17AM -0700, Andrea Bolognani wrote:
> On Tue, May 28, 2024 at 12:50:51PM GMT, Laine Stump wrote:
> > On 5/28/24 12:31 PM, Pavel Hrdina wrote:
> > > On Tue, May 28, 2024 at 05:49:19PM +0200, Andrea Bolognani wrote:
> > > > + if (not firewall_backend_priority.contains('nftables') or
> > > > + not firewall_backend_priority.contains('iptables') or
> > > > + firewall_backend_priority.length() != 2)
> > >
> > > No need to have a check for specific values. Meson will already check if
> > > they are from the list of choices defined in meson_options.txt .
> >
> > But we don't just need to check that the values in the list are all valid
> > options; we also want to make sure that nobody has entered the same value
> > multiple times (which this ends up doing by making sure that there is at
> > least one entry for each valid value, and that the list is the same length
> > as the number of valid values).
>
> Yes, that was exactly the idea.
True, that is not checked so we still need to duplicate the list here
that I wanted to avoid.
> > Or do we not care if someone repeats the same value? Maybe somebody wants to
> > include iptables support in the build, but not look for it automatically
> > (instead only use it if it's explicitly requested in network.conf). One way
> > of doing that would be to sent firewall_backend_priority = nftables,nftables
> >
> > (that does seem a bit obtuse; perhaps it would be better to allow limiting
> > the length of the option list to 1)
>
> If that's something that we want to allow, then we should include
> explicit support for it rather than make it possible through obscure
> runes :)
>
> I'm not sure we really need to bother, but I don't feel strongly
> either way so I could be persuaded to look into it. Perhaps as an
> after-release follow up, though?
>
> > > > +option('firewall_backend_priority', type: 'array', choices: ['nftables', 'iptables'], description: 'firewall backends to try, preferred ones first')
> > >
> > > What about "order of firewall backends to try"? The part "preferred ones
> > > first" sounds strange to me.
>
> Sure, that works too.
Reviewed-by: Pavel Hrdina <phrdina@xxxxxxxxxx>
Attachment:
signature.asc
Description: PGP signature
