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.
>
> > > + 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
