T4502: firewall: Add software flow offload using flowtable

[vfreex]

Change Summary

The following commands will enable nftables flowtable offload on interfaces eth0 eth1:

set firewall global-options flow-offload software interface <name>
set firewall global-options flow-offload hardware interface <name>

Generated nftables rules:

table inet vyos_offload {
    flowtable VYOS_FLOWTABLE_software {
        hook ingress priority filter - 1; devices = { eth0, eth1, eth2, eth3 };
        counter
    }

    chain VYOS_OFFLOAD_software {
        type filter hook forward priority filter - 1; policy accept;
        ct state { established, related } meta l4proto { tcp, udp } flow add @VYOS_FLOWTABLE_software
    }
}

Use this option to count packets and bytes for each offloaded flow:

set system conntrack flow-accounting 

To verify a connection is offloaded, run

cat /proc/net/nf_conntrack|grep OFFLOAD

This PR follows firewalld's implementation: https://github.com/firewalld/firewalld/blob/e748b97787d685d0ca93f58e8d4292e87d3f0da6/src/firewall/core/nftables.py#L590

A good introduction to nftables flowtable: https://thermalcircle.de/doku.php?id=blog:linux:flowtables_1_a_netfilter_nftables_fastpath

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Code style update (formatting, renaming)
• Refactoring (no functional changes)
• Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
• Other (please describe):

Related Task(s)

• https://vyos.dev/Txxxx

Component(s) name

Proposed changes

How to test

Checklist:

• I have read the CONTRIBUTING document
• I have linked this PR to one or more Phabricator Task(s)
• I have run the components SMOKETESTS if applicable
• My commit headlines contain a valid Task id
• My change requires a change to the documentation
• I have updated the documentation accordingly

[sever-sever]

Why Ethernet only?
It could be tunnel, wireguard or PPPoE, etc.

[vfreex]

I was lazy at when writing this PR XD.
The reason is that when adding a virtual interface (e.g. a bridge, pppoe interface or vlan vif) to a flowtable, it actually adds the underlying physical interface. Therefore all ingress traffic from that physical interface will be offloaded.
e.g. if you add eth0.3 and eth1 to a flowtable, forwarding traffic between eth0.4 and eth1 will be offloaded as well.
So limiting it to ethernet could be less confusing for end-users.

[nicolas-fort]

There's a bigger change for firewall refactoring. #2016

Once we move forward with it, we plan to introduce flowtables and other features.

[zdc]

@vfreex this feature is definitely required and we will be happy to see the same PR after #2016 will be merged. Before that moment all new features in the firewall are frozen to avoid conflicts. Implementing this after #2016 will be easier, by the way.

[vfreex]

Ack. Thanks for the info.

[Apachez-]

@nicolas-fort Any ETA for when that refactoring might occur?

Both https://vyos.dev/T4502 and https://vyos.dev/T5419 shows that implementing flowtable should be a fairly easy task (adding flowtable object which defines which interfaces it will operate on and then adding a line to the chains of IPv4 and IPv6 for it to utilize flowtable as a first option).

Preferly this setting would be set through "set interface ethernet ethX".

@sever-sever It seems that this feature acts on physical hardware only so setting it on tunnels either have not effect or is incorrect. As mentioned when setting it on example eth1 then everything that flows through eth1 will be accelerated.

Im still trying to find out any drawbacks but right now it seems that enabling the software flowtable should be safe for all users (even if this offloading option probably should be disabled by default anyway at least in the first rolling releases) while enabling hardware flowtable depends on which NIC's and drivers you are using (only a few seems to properly support this as of today).

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[nicolas-fort]

Now that firewall refactor was merged, I suggest working on this feature again.
IMO, this feature can be added in firewall ruleset (for forward traffic), with a ruleset similar to this example:

#Flowtables proposal:
set firewall ipv4 foward filter rule 10 inbound-interface interface-name eth0
set firewall ipv4 foward filter rule 10 outbound-interface interface-name eth1
set firewall ipv4 foward filter rule 10 protocol [tcp|udp|tcp_udp]
set firewall ipv4 foward filter rule 10 [source|destination] [address] x.x.x.x
set firewall ipv4 foward filter rule 10 [source|destination] port X
set firewall ipv4 foward filter rule 10 action offload

It should create proper rule, and corresponding flowtable.

[sever-sever]

As I understand there are no inbound/outbound interfaces

[nicolas-fort]

As I understand there are no inbound/outbound interfaces

Agree.. This is an example in order to re-use current cli..
Do you a better approach?
Maybe skip that interface matcher, and include interfaces under offload?

set firewall ipv4 foward filter rule 10 protocol [tcp|udp|tcp_udp]
set firewall ipv4 foward filter rule 10 [source|destination] [address] x.x.x.x
set firewall ipv4 foward filter rule 10 [source|destination] port X
set firewall ipv4 foward filter rule 10 offload interface eth0
set firewall ipv4 foward filter rule 10 offload interface eth1

[Apachez-]

Given the description at: https://firewalld.org/2023/05/nftables-flowtable

I would suggest to either make it global, something like:

set firewall global-options nft-flowtable none/offload

or per interface:

set interfaces ethernet ethX offload nftables_flowtable

I would prefer to have it named "offload nftables" or "offload nftables_flowtable" rather than just "offload nft" which might be too short.

[sever-sever]

I would prefer to have it named "offload nftables"

I would prefer don’t use nftables at all for name of this feature.

[c-po]

Better CLI nodes would be:

• set firewall global-options offload flowtable
• set interfaces ethernet ethX offload flowtable

So this blends in the current CLI options for offloading in ethernet (but at the cost of adding a dependency call to the firewall script - which is something we try to avoid at all cost) - So I'd stick with the global option only and not make it more complex then necessary.

[Apachez-]

The ones proposed works fine for me :-)

set firewall global-options offload flowtable
set interfaces ethernet ethX offload flowtable

Then regarding if it should be global or not...

"Normally" I think it would make things easier with just

set firewall global-options offload flowtable

Where:

set firewall global-options offload none

is default or if not set.

Because "normally" you want this to be enabled and since it acts on physical interfaces you would either have it enabled for all interfaces or for no interfaces.

It would match the other globals such as:

        all-ping enable
        broadcast-ping disable
        ipv6-receive-redirects disable
        ipv6-src-route disable
        ip-src-route disable
        log-martians enable
        receive-redirects disable
        send-redirects enable
        source-validation strict
        syn-cookies enable
        twa-hazards-protection disable

Because technically one might want to enabled broadcast-pings on just eth2 and no other interface. So having the flowtable setting as a global-option would match that design.

But with that being said there might show up situations where one nic (due to different vendor/model and because of that the driver) for whatever reason is not to happy with flowtable and must have it disabled for whatever reason.

For that case then:

set interfaces ethernet ethX offload flowtable

Would be prefered.

Because I assume VyOS wouldnt like to have it configurable at two places?

Something like:

set firewall global-options all-offload flowtable
set interfaces ethernet eth2 offload none
set interfaces ethernet eth3 offload none

would mean that interface 1-8 have offloading enabled EXCEPT interface eth2 and eth3.

On the other hand having the offloading per interface (even if it would be handy) instead as a global-option can lead to increased amount of support cases where the user for example enabled offload flowtable for eth2 but not eth3 and then dont understand why the traffic is 30-70% worser in one direction than the other.

Perhaps the global-option could have an exclude list which normally is empty?

set firewall global-options offload flowtable
set firewall global-options offload-exclude eth2 eth3

[nicolas-fort]

I would prefer to have it's own configuration as a new section insde firewall.
From netfilter docs:
The *devices* are specified as iifname of the input interface(s) of the traffic that should be offloaded. Devices are required for both traffic directions.
So, since for a flow, it requires 2 interfaces, I would go with something like this:

set firewall ipv4 [fastpath | flowtable | offload | whatever ] rule <rule> interface <iface_1>
set firewall ipv4 [fastpath | flowtable | offload | whatever ] rule <rule> interface <iface_1>
set firewall ipv4 [fastpath | flowtable | offload | whatever ] rule <rule> ... 

Allow some matcher, such as protocol (tcp, udp) and ports. Then generate one flowtable per rule.

[sever-sever]

I would prefer to have it's own configuration as a new section insde firewall. From netfilter docs: The *devices* are specified as iifname of the input interface(s) of the traffic that should be offloaded. Devices are required for both traffic directions. So, since for a flow, it requires 2 interfaces, I would go with something like this:

set firewall ipv4 [fastpath | flowtable | offload | whatever ] rule <rule> interface <iface_1>
set firewall ipv4 [fastpath | flowtable | offload | whatever ] rule <rule> interface <iface_1>
set firewall ipv4 [fastpath | flowtable | offload | whatever ] rule <rule> ... 

Allow some matcher, such as protocol (tcp, udp) and ports. Then generate one flowtable per rule.

@nicolas-fort I don't think we need any rules here.
In my opinion it should be as simple as possible (as any other offload on/off option)

@c-po Which interface will it configure without explicitly setting them?

set firewall global-options offload flowtable

[Apachez-]

@nicolas-fort Personally I would vote for the simplicity of:

set firewall global-options offload flowtable
set firewall global-options offload-exclude eth2 eth3

That is its globally enabled but the admin have the ability to for whatever reason exclude one or more interfaces from the flowtable optimization.

The admin shouldnt need to care for adding this per rule or per flow etc since it should operate on the interface itself as described in: https://firewalld.org/2023/05/nftables-flowtable

[sever-sever]

@nicolas-fort Personally I would vote for the simplicity of:

set firewall global-options offload flowtable
set firewall global-options offload-exclude eth2 eth3

That is its globally enabled but the admin have the ability to for whatever reason exclude one or more interfaces from the flowtable optimization.

The admin shouldnt need to care for adding this per rule or per flow etc since it should operate on the interface itself as described in: https://firewalld.org/2023/05/nftables-flowtable

@Apachez- in your example and link setting interfaces are required
This feature can be enabled by setting NftablesFlowtable in /etc/firewalld/firewalld.conf. This setting defaults to off. To enable flowtable support set this value to your list of interfaces for which you want flowtable to be enabled, e.g. NftablesFlowtable=eth0 eth1.

so without setting interfaces it won’t work at all
You can just test it

[Apachez-]

But VyOS already know which interfaces exists (part of generating that interface list in conf-mode).

So use that list (perhaps also match so the interface isnt shutdown/disabled) to define flowtable for all interfaces (if set firewall global-options offload flowtable is set) but exclude the interfaces defined by set firewall global-options offload-exclude.

Whats needed is:

1:

flowtable fastpath {
        hook ingress priority filter + 10
        devices = { eth0, eth1 }
}

where the devices are the interfaces to have this enabled.

2:

Add this as 2nd line in FORWARD (and probably INPUT aswell?) chain:

ct state { established, related } meta l4proto { tcp, udp } flow add @fastpath

3:

Done! :-)

To begin with this (set firewall global-options offload flowtable) could be disabled by default (offload none) but it should be considered to be enabled by default in future.

Or am I missing something here?

[sever-sever]

I’m against global option until it’s doesn’t support natively in nft.
It should be configured on specific interfaces explicitly. As we don’t do global option for ring buffers, lro/gro etc.
As it also has hardware offload flag that will be extended in this/another PR and not all NICs support it. https://marc.info/?l=netfilter&m=162197756905358&w=2

[sarthurdev]

Would like to see a smoketest for the CLI and verifying nftables output too.

[nicolas-fort]

It looks good but I have some doubts:

• Minor and cosmetic: set firewall global-options offload flowtable .. Why both offload and flowtables? Are there any offload actions for firewall?
• Maind doubt: This PR only uses a single flowtable, and offload is done in software. My main doubt is how we should integrate hardware offload when NIC supports its? Moreover, how to confiure offload when we have a mix of interfaces, some of them which supports hw offload, and some of them which doesn't support it. https://docs.kernel.org/networking/nf_flowtable.html#hardware-offload

Maybe it's a good start merging this PR, because benefits from flowtables are more than welcome and needed.. But we should consider and think on how to cover hardware offload too. So far, I had no clear ideas on how to implement it.

[Apachez-]

1. My argument would be to follow the designpattern of offloading for interfaces:

https://docs.vyos.io/en/latest/configuration/interfaces/ethernet.html#offloading

set interfaces ethernet <interface> offload <gro | gso | lro | rps | sg | tso> 

2. Regarding which interfaces to be enabled for offloading that could be dealt with something like:

set firewall offload flowtable <enable | disable>
set firewall offload flowtable interface <none | any | ethX, ethY...>

Where default is (to follow the designpattern of offloading for interfaces which are default disabled):

set firewall offload flowtable 'disable'
set firewall offload flowtable interface 'none'

2.1 When it comes to adding support for hw-offloading this must be tested how things work out if you enable hw-offloading for an interface which doesnt support HW_OFFLOAD. I mean so the admin doesnt enable hw-offloading and after reboot lost access to the VyOS device.

A check for HW_OFFLOAD flag during commit would be prefered and issue a log warning that a particular interface might not support the asked feature (in case admin have selected a non-supporting interface for hw-offloading through flowtable).

2.2 When it comes to naming I like systems which are self-explanatory as in you dont have to spend hours of reading manual and training to do basic configuration.

That is one suggestion would be to in future updates separate flowtable into software-flowtable and hardware-flowtable. Checks might be needed if software and hardware offloading can be runned at the same time for a particular interface.

set firewall offload sw-flowtable <enable | disable>
set firewall offload sw-flowtable interface <none | any | ethX, ethY...>
set firewall offload hw-flowtable <enable | disable>
set firewall offload hw-flowtable interface <none | any | ethX, ethY...>

Where default is (again to match the design pattern of offloading for interfaces):

set firewall offload sw-flowtable 'disable'
set firewall offload sw-flowtable interface 'none'
set firewall offload hw-flowtable 'disable'
set firewall offload hw-flowtable interface 'none'

[vfreex]

@nicolas-fort nicolas-fort

It looks good but I have some doubts:

• Minor and cosmetic: set firewall global-options offload flowtable .. Why both offload and flowtables? Are there any offload actions for firewall?

I was following other people's suggestion for the CLI structure. I guess it means we can add another offload approach in the future (e.g. hardware offload). Maybe it is better to be called set firewall global-options offload software?

• Maind doubt: This PR only uses a single flowtable, and offload is done in software. My main doubt is how we should integrate hardware offload when NIC supports its? Moreover, how to confiure offload when we have a mix of interfaces, some of them which supports hw offload, and some of them which doesn't support it. https://docs.kernel.org/networking/nf_flowtable.html#hardware-offload

Maybe it's a good start merging this PR, because benefits from flowtables are more than welcome and needed.. But we should consider and think on how to cover hardware offload too. So far, I had no clear ideas on how to implement it.

I don't have a hardware supporting hardware offload, but I saw OpenWRT demonstrated how they do hardware offload on mt7621 SoC:

    flowtable ft {
            hook ingress priority 0;
            devices = { "lan4", "lan2", "lan3", "lan1", "wan" };
            flags offload;
    }

The Linux Kernel Documentation says "You can create as many flowtables as you want in case you need to perform resource partitioning." and "If your network device provides hardware offload support, you can turn it on by means of the 'offload' flag in your flowtable definition, e.g.", but I didn't see a clear description about the behavior if we mix interfaces. So more information or testing is needed.

[vfreex]

@Apachez-

Regarding which interfaces to be enabled for offloading that could be dealt with something like:
set firewall offload flowtable <enable | disable>
set firewall offload flowtable interface <none | any | ethX, ethY...>
Where default is (to follow the designpattern of offloading for interfaces which are default disabled):
set firewall offload flowtable 'disable'
set firewall offload flowtable interface 'none'

I like the idea of having disable option to disable offloading without removing the interfaces from the configuration (might be good for testing).

I am not sure if the implication of interface none. interface is a multi-value option here, which I expect to list all interfaces for offloading. If we don't need any interfaces, we need to remove it.

Also for interface any, this can be added in the future. For now, it is not very clear to me that which kinds of interfaces support flowtable offloading. I would like to keep it simple in the beginning and extend it in the future.

[Apachez-]

@vfreex In my case the below would yield the same result (as in no flowtable is configured in nft):

set firewall offload flowtable 'disable'
set firewall offload flowtable interface 'ethx'

set firewall offload flowtable 'enable'
set firewall offload flowtable interface 'none'

But if the above would be an issue in the VyOS config engine then "none" can be removed from available options for "flowtable interfaces".

The "any" option for "flowtable interface" would be really nice if that could be implemented in short future.

Mainly for the case where (again must be verified with testing) you can just blindly enable both software- and hardware-flowtable for all interfaces and those who doesnt support hw but supports sw will utilize that sw-acceleration and those who support neither will work just as if they werent part of that interface list (and those who support both will well have both hw- and sw-acceleration - again must be tested so it isnt that an ingress interface can only be hw OR sw accelerated).

Flowtable acts on ingress physical interface (so ethX in VyOS case) but since 5.13 also supports bridges. What happens in this case is that the kernel will resolve which physical interfaces are part of lets say br0 and apply the flowtable for the ingress of these interfaces according to https://docs.kernel.org/networking/nf_flowtable.html

[vfreex]

Thanks for the review. I've updated this PR as follows:

1. PR is rebased.
2. CLI is changed to:

set firewall global-options flow-offload software interface <name>
set firewall global-options flow-offload hardware interface <name>

3. Generated nftables config looks like:

table inet vyos_offload {
	flowtable VYOS_FLOWTABLE_hardware {
		hook ingress priority filter - 2; devices = { eth0, eth1 }
                 flags offload;
		counter
	}

	chain VYOS_FLOWTABLE_hardware {
		type filter hook forward priority filter - 2; policy accept;
		ct state { established, related } meta l4proto { tcp, udp } flow add @VYOS_FLOWTABLE_hardware
	}

       	flowtable VYOS_FLOWTABLE_software {
		hook ingress priority filter - 1; devices = { eth2, eth3 }
                 flags offload;
		counter
	}

	chain VYOS_OFFLOAD_software {
		type filter hook forward priority filter - 1; policy accept;
		ct state { established, related } meta l4proto { tcp, udp } flow add @VYOS_FLOWTABLE_software
	}
}

Note I don't really have a hardware that supports hardware offload. This setup totally comes from my understanding of the documentation. When I tried to add an unsupported interface for hardware offload, I got Error: Could not process rule: No such file or directory ct state { established, related } meta l4proto { tcp, udp } flow add @VYOS_FLOWTABLE_hardware.

4. Smoketest added, however I don't have an environment to actually run the smoketest.

[vfreex]

@Apachez- I couldn't add an interface that doesn't support hardware offload to hardware-flowtable. I got the following error:

Error: Could not
process rule: Operation not supported flowtable VYOS_FLOWTABLE_hardware
{           ^^^^^^^^^^^^^^^^^^^^^^^ /run/nftables.conf:82:69-101: Error:
Could not process rule: No such file or directory         ct state {
established, related } meta l4proto { tcp, udp } flow add
@VYOS_FLOWTABLE_hardware
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This is the nftables config that I wanted to apply:

    flowtable VYOS_FLOWTABLE_hardware {
        hook ingress priority filter - 2; devices = { eth0 };
        flags offload;
        counter
    }

    chain VYOS_OFFLOAD_hardware {
        type filter hook forward priority filter - 2; policy accept;
        ct state { established, related } meta l4proto { tcp, udp } flow add @VYOS_FLOWTABLE_hardware
    }

Not sure if I did something wrong or it worked as expected.

Based on my assumption that hardware flowtable can only contain interfaces that support hardware offload, I split this to 2 separate flowtables: One for software, another for hardware. I also removed the restriction to allow any interface to be added to flowtable. So nftables will complain if the interface is not supported.

[sarthurdev]

I wonder if we should instead follow the new CLI of the refactor when defining and applying flowtables?

Example:

set firewall flowtable FT_TEST description 'Test Flowtable'
set firewall flowtable FT_TEST interface eth0
set firewall flowtable FT_TEST interface eth1
set firewall flowtable FT_TEST offload [software|hardware]

set firewall ... rule 1 state established
set firewall ... rule 1 state related
set firewall ... rule 1 protocol tcp_udp
set firewall ... rule 1 action offload
set firewall ... rule 1 offload-target FT_TEST

This way you can create as many sw/hw offloaded flowtables as needed and the CLI is consistent with the refactor. (e.g. action jump and jump-target <name>)

[vfreex]

@sarthurdev As discussed earlier, using rules to configure flowtable could be an overkill for now. Personally I prefer to keep it simple at least for now.

[sarthurdev]

@sarthurdev As discussed earlier, using rules to configure flowtable could be an overkill for now. Personally I prefer to keep it simple at least for now.

I would prefer explicitly defining the table and rules in CLI. That way you can see in the firewall config, the flowtable definition and the rules that reference it and know how it should operate - instead of the creation and rules being done for you behind the scenes.

[sever-sever]

@sarthurdev As discussed earlier, using rules to configure flowtable could be an overkill for now. Personally I prefer to keep it simple at least for now.

+1

[vfreex]

PR rebased

[24fpsDaVinci]

no plans to merge this?

[Apachez-]

Yeah, any ETA for merge?

Both software and hardware flowtables is a welcomed addition in terms of added performance.

[24fpsDaVinci]

vyos@vyos# set firewall global-options flow-offload software interface eth1 
[edit]
vyos@vyos# set firewall global-options flow-offload software interface eth2
[edit]
vyos@vyos# commit

Failed to apply firewall: /run/nftables.conf:86:1-1: Error: syntax
error, unexpected '}' } ^

[[firewall]] failed
Commit failed
[edit]

DEBUG - ======================================================================
DEBUG - ERROR: test_conntrack_ignore (__main__.TestSystemConntrack.test_conntrack_ignore)
DEBUG - ----------------------------------------------------------------------
DEBUG - Traceback (most recent call last):
DEBUG -   File "/usr/libexec/vyos/tests/smoke/cli/test_system_conntrack.py", line 274, in test_conntrack_ignore
DEBUG -     self.cli_commit()
DEBUG -   File "/usr/libexec/vyos/tests/smoke/cli/base_vyostest_shim.py", line 76, in cli_commit
DEBUG -     self._session.commit()
DEBUG -   File "/usr/lib/python3/dist-packages/vyos/configsession.py", line 183, in commit
DEBUG -     out = self.__run_command([COMMIT])
DEBUG -           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
DEBUG -   File "/usr/lib/python3/dist-packages/vyos/configsession.py", line 139, in __run_command
DEBUG -     raise ConfigSessionError(output)
DEBUG - vyos.configsession.ConfigSessionError: [ firewall ]
DEBUG - Failed to apply firewall: /run/nftables.conf:98:1-1: Error: syntax
DEBUG - error, unexpected '}' } ^
DEBUG - 
DEBUG - [[firewall]] failed
DEBUG - Commit failed

@vfreex I built the iso with this PR but getting a strange error... also appears in the smoke test

https://github.com/vyos/vyos-rolling-nightly-builds/actions/runs/6192281781/job/16812090664

[Apachez-]

So that means that there is one "}" too many in the rendered config by /usr/share/vyos/templates/firewall/nftables.j2:

https://github.com/vyos/vyos-1x/blob/249f391f6898b9b4617b135952815d6f4675e9ee/data/templates/firewall/nftables.j2

https://github.com/vyos/vyos-1x/compare/1f50ab569d5dfd31854aaa91967d8e44d2fc918a..f909c17aca4d48598d5eaee0df81bf64967902f0#diff-f907b1a7bc83a9dc85d52b21c9ebbceefb18f6d9a82f00f028d4fff26baebab3

I think the error is at line 275-276:

vyos-1x/data/templates/firewall/nftables.j2

Lines 275 to 276 in 249f391

	{{ group_tmpl.groups(group, True) }}
	}

For "Bridge Firewall" the table is deleted "delete table bridge vyos_filter".

And then only created if "{% if bridge is vyos_defined %}".

But then comes these two lines out of nowhere:

{{ group_tmpl.groups(group, True) }}
}

Possible for you to comment out line 275 and 276 in /usr/share/vyos/templates/firewall/nftables.j2 at your installation and try to run the commit again?

You might need to either reboot the box or restart vyos-configd before doing so:

systemctl restart vyos-configd

Also before doing so try to take a backup copy of /run/nftables.conf after that failed commit and then another backup after commit when you have done the above modification.

If possible please upload both to this thread.

[24fpsDaVinci]

@Apachez- thank you very much, it appears to be working now
fixed_nftables.conf
failed_nftables.conf

[vfreex]

@Apachez- Those lines were mistakenly included during rebase. I saw @c-po had removed the extra } in 45cfd56. I created #2272 to remove the extra {{ group_tmpl.groups(group, True) }}

[sarthurdev]

Note to peoeple using CLI in this PR, next rolling image has changed flowtable CLI without automated migration (Due to being so close together, and not between release versions). Recommended to delete flowtable node prior updating to a newer rolling image.