Hello Michael,

the patch looks fine to me too.

Technically the solution for "none" will work pretty fine.
> 
> 
> Am 12.04.21 um 12:23 schrieb Michael Tremer:
> > Hello,
> > 
> > > On 12 Apr 2021, at 11:23, Alexander Marx <
> > > alexander.marx(a)ipfire.org> wrote:
> > > 
> > > 
> > > 
> > > Am 12.04.21 um 12:18 schrieb Michael Tremer:
> > > > Hi,
> > > > 
> > > > > On 12 Apr 2021, at 07:05, Alexander Marx <
> > > > > alexander.marx(a)ipfire.org> wrote:
> > > > > 
> > > > > Fixes: #12301
> > > > > 
> > > > > When using hosts with MAC-addresses in a hostgroup,
> > > > > the rule won't be generated if those hosts are selected as
> > > > > target.
> > > > > There is a hint but due to a wrong hashparameter the hint was
> > > > > not shown.
> > > > > 
> > > > > With this patch the hint is shown again.
> > > > > Additionally the rule is skipped when rules.pl creates rules.
> > > > > 
> > > > > There are no bootmessages with failed target "none" anymore.
> > > > > ---
> > > > > config/firewall/firewall-lib.pl | 4 ++--
> > > > > html/cgi-bin/firewall.cgi       | 2 +-
> > > > > 2 files changed, 3 insertions(+), 3 deletions(-)
> > > > > 
> > > > > diff --git a/config/firewall/firewall-lib.pl
> > > > > b/config/firewall/firewall-lib.pl
> > > > > index bc0b30ca5..e7ec30ae0 100644
> > > > > --- a/config/firewall/firewall-lib.pl
> > > > > +++ b/config/firewall/firewall-lib.pl
> > > > > @@ -2,7 +2,7 @@
> > > > > #############################################################
> > > > > ##################
> > > > > #                                                            
> > > > >                  #
> > > > > # IPFire.org - A linux based
> > > > > firewall                                         #
> > > > > -# Copyright (C) 2013 Alexander Marx
> > > > > <amarx(a)ipfire.org>                        #
> > > > > +# Copyright (C) 2021 Alexander Marx
> > > > > <amarx(a)ipfire.org>                        #
> > > > > #                                                            
> > > > >                  #
> > > > > # This program is free software: you can redistribute it
> > > > > and/or modify        #
> > > > > # it under the terms of the GNU General Public License as
> > > > > published by        #
> > > > > @@ -315,7 +315,7 @@ sub get_addresses
> > > > >                 foreach my $grp (sort {$a <=> $b} keys
> > > > > %customgrp) {
> > > > >                         if ($customgrp{$grp}[0] eq $value) {
> > > > >                                 my @address =
> > > > > &get_address($customgrp{$grp}[3], $customgrp{$grp}[2],
> > > > > $type);
> > > > > -
> > > > > +                               next if ($address[0][0] eq
> > > > > 'none');
> > > > A comment for these rather obscure things would not hurt, but
> > > > technically I agree with how this is solved.
> > > > 
> > > > 
> > > > 
> > > > >                                 if (@address) {
> > > > >                                         push(@addresses,
> > > > > @address);
> > > > >                                 }
> > > > > diff --git a/html/cgi-bin/firewall.cgi b/html/cgi-
> > > > > bin/firewall.cgi
> > > > > index 1483e779f..b0851dd3e 100644
> > > > > --- a/html/cgi-bin/firewall.cgi
> > > > > +++ b/html/cgi-bin/firewall.cgi
> > > > > @@ -592,7 +592,7 @@ sub checktarget
> > > > >                 &General::readhasharray("$confighost",
> > > > > \%customhost);
> > > > >                 foreach my $grpkey (sort keys %customgrp){
> > > > >                         foreach my $hostkey (sort keys
> > > > > %customhost){
> > > > > -                               if ($customgrp{$grpkey}[2] eq
> > > > > $customhost{$hostkey}[0] && $customgrp{$grpkey}[2] eq
> > > > > $fwdfwsettings{$fwdfwsettings{'grp2'}} &&
> > > > > $customhost{$hostkey}[1] eq 'mac'){
> > > > > +                               if ($customgrp{$grpkey}[2] eq
> > > > > $customhost{$hostkey}[0] && $customgrp{$grpkey}[0] eq
> > > > > $fwdfwsettings{$fwdfwsettings{'grp2'}} &&
> > > > > $customhost{$hostkey}[1] eq 'mac'){
> > > > What has changed here?
> > > only the hashfield
> > > 
> > > $customgrp{$grpkey}[0] (was 2 before)
> > Yes I saw that, but what does that change?
> > 
> > -Michael
> > 
> > P.S. Do not forget to CC the list
> Thats the indicator to show the Hint. When someone has hostgroups
> with 
> macaddresses as target, the hint is shown.
> Because this Value was 2 instead of 0, the hint was never shown....


Previously the check was performed against the hostgroup name which
never would contain a valid MAC address.

With the changed value now the check for a MAC address will be
performed on each configured host inside the group what is what we
want.
> 
> > 
> > > > >                                         $hint=$Lang::tr{'fwdf
> > > > > w hint mac'};
> > > > >                                         return $hint;
> > > > >                                 }
> > > > > —
> > > > > 2.25.1
> > > > > 
> > > > Best,
> > > > -Michael
> 

Acked-by: Stefan Schantl <stefan.schantl(a)ipfire.org>

Best regards,

-Stefan