[pve-devel] [RFC firewall] ebtables: remove PVE chains properly

[Fabian Grünbichler]

On Mon, Jul 08, 2019 at 10:38:26AM +0200, Thomas Lamprecht wrote:
> Am 7/8/19 um 9:33 AM schrieb Fabian Grünbichler:
> > when globally disabling the FW, or on shutdown of firewall service.
> > otherwise, ebtables rules are leftover (and perpetually displayed as
> > pending changes as well).
> > 
> > the actual removal is done by taking the same code path as when
> > disabling just ebtables on the cluster level, i.e. applying an empty
> > ruleset.
> > 
> > Signed-off-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
> > ---
> > 
> > Notes:
> >     another approach would be to make ebtables_get_chains more like
> >     iptables_get_chains, and then re-use remove_pvefw_chains_iptables..
> >     
> >     should backport cleanly to stable-5
> > 
> >  src/PVE/Firewall.pm | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
> > index 96c45e9..4147f87 100644
> > --- a/src/PVE/Firewall.pm
> > +++ b/src/PVE/Firewall.pm
> > @@ -4269,6 +4269,7 @@ sub remove_pvefw_chains {
> >      PVE::Firewall::remove_pvefw_chains_iptables("iptables");
> >      PVE::Firewall::remove_pvefw_chains_iptables("ip6tables");
> >      PVE::Firewall::remove_pvefw_chains_ipset();
> > +    PVE::Firewall::remove_pvefw_chains_ebtables();
> >  
> >  }
> >  
> > @@ -4314,6 +4315,12 @@ sub remove_pvefw_chains_ipset {
> >      ipset_restore_cmdlist($cmdlist) if $cmdlist;
> >  }
> >  
> > +sub remove_pvefw_chains_ebtables {
> > +    # empty ruleset == ebtables disabled
> > +    my ($cmdlist, $changes) = get_ebtables_cmdlist({});
> > +    ebtables_restore_cmdlist($cmdlist) if $changes && $cmdlist;
> 
> $cmdlist is always true here..

true, and $changes is only 1 for anything besides exists/ignore/delete
(the latter seems incorrect IMHO, since both ipset and iptables treat
deletions as changes).

will send a v2..

> Also while it is not too useful to flush the rules if no changes
> (i.e., already emptied ebtables ruleset) is detected we could do
> it anyway, e.g. a simple (untested):
> 
> ebtables_restore_cmdlist("*filter\n");

that would also remove rules set by the admin, AFAICT?

> 
> > +}
> > +
> >  sub init {
> >      my $cluster_conf = load_clusterfw_conf();
> >      my $cluster_options = $cluster_conf->{options};
> > 
>