Am Mittwoch, den 27.09.2017, 11:51 +0200 schrieb Wolfgang Bumiller: > On Wed, Sep 27, 2017 at 11:09:29AM +0200, Tom Weber wrote: > > > > My goal are defined structures for rules, chains, macros (which i > > think > > are just arrays of "rule templates") etc and code which deals with > > these structures instead of printing out iptables commands in > > various > > places. > That's long overdue anyway and might be useful later on when we want > to > start using nftables. > > > > > While doing this, keeping "input" and "output" identical to whats > > there > > doesn't make this an easy task and therefore I sometimes use these > > 'temporary ugliness' approaches at places that I intend to replace > > anyway. > That's fine. > > > > > > > > > > > > > > @@ -3127,26 +3146,21 @@ sub generate_std_chains { > > > > my $std_chains = $pve_std_chains->{$ipversion} || die > > > > "internal error"; > > > > > > > > my $loglevel = get_option_log_level($options, > > > > 'smurf_log_level'); > > > > - > > > > - my $chain; > > > > - > > > > - if ($ipversion == 4) { > > > > - # same as shorewall smurflog. > > > > - $chain = 'PVEFW-smurflog'; > > > > - $std_chains->{$chain} = []; > > > > - > > > > - push @{$std_chains->{$chain}}, > > > > get_log_rule_base($chain, > > > > 0, "DROP: ", $loglevel) if $loglevel; > > > > - push @{$std_chains->{$chain}}, "-j DROP"; > > > > + my $chain = 'PVEFW-smurflog'; > > > > + if ( $loglevel && $std_chains->{$chain} ) { > > > This shouldn't check for $loglevel as it can also have changed > > > from > > > non-zero to zero in which case the change would happen. Iow. you > > > cannot > > > turn off the smurf logs anymore. > > huh? if $loglevel == 0 it will not turn on logging. > This modifies the fields in the global $pve_std_chains, so it doesn't > need to not turn it on, it needs to turn if off. Such modifications > are > something I'd prefer to reduce if possible.
yes you're right - didn't look at it as running service. so, the ultimate solution would be to have the std_chains template defined in external config files (which i have in mind from the beginning) from which we'll generate our working internal tree which we'd then manipulate according to various settings? > > > > + foreach my $r (@{$std_chains->{$chain}}) { > > > > + $r->{log} = $loglevel; > > > Wouldn't it be better to just give ruleset_generate_rule an > > > optional > > > default loglevel parameter for when rules don't define their own > > > {log}? > > how would this help with turning smurf / tcpflags logging > > separately on > > and off then? (not that I like the current way of setting these - > > but > > that's what's in the frontend right now) > Could be passed depending on $chain in the loop below, then we could > avoid touching the global hashes entirely. > The function would still prefer the rule's ->{log} member over the > default if it exists. _______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel