Re: [PATCH] network: allow for forward dev to be a transient interface

On Fri, Jun 07, 2024 at 01:33:30PM -0400, Laine Stump wrote: > A user reported that if they set <forward mode='nat|route' dev='blah'> > starting the network would fail if the device 'blah' didn't already > exist. > > This is caused by using "iif" and "oif" in nftables rules to check for > the forwarding device - these two commands work by saving the named > interface's ifindex (an unsigned integer) when the rule is added, and > comparing it to the ifindex associated with the packet's path at > runtime. This works great if the interface both 1) exists when the > rule is added, and 2) is never deleted and re-created after the rule > is added (since it would end up with a different ifindex). > > When checking for the network's bridge device, it is okay for us to > use "iif" and "oif", because the bridge device is created before the > firewall rules are added, and will continue to exist until just after > the firewall rules are deleted when the network is shutdown. > > But since the forward device might be deleted/re-added during the > lifetime of the network's firewall rules, we must instead us "oifname" > and "iifname" - these are much less efficient than "Xif" because they > do a string compare of the interface's name rather than just comparing > two integers (ifindex), but they don't require the interface to exist > when the rule is added, and they can properly cope with the named > interface being deleted and re-added later. > > Fixes: a4f38f6ffe6a9edc001d18890ccfc3f38e72fb94 > Signed-off-by: Laine Stump <laine(a)redhat.com&gt; > --- > src/network/network_nftables.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) Surprised that tests/networkxml2firewalldata didn't need an update - guess we've got missing coverage of this feature.

So Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; but would be good if you can add more test coverage in a followup too.