pqarmitage
commented
7 months ago Many thanks for this patch.
I can confirm that it works fine when using network namespaces, but it did not handle ipvlans.
Based on your patch I have added the following further patches:
4883311 use addattr32() for setting IFLA_GROUP rather than addattr_l(). I have changed the original code where IFLA_GROUP was being set as well.
2f3c193 Make setting the interface group to the parent's interface group work for ipvlans.
5fcaa2d3 This commit adds an option to explicitly set the interface group for a VMAC or ipvlan, so it can be independent of the the interface group of the parent interface.
Once again, many thanks for your patch, and I think it has led to further enhancements to keepalived.
It is useful in many instances to set up firewall rules based on interface groups so that sets of interfaces may be aggregated by group and matched with a single rule rather than by listing them all.
Prior to this change, when use_vmac or use_ipvlan is used, new interfaces are created with the default group, which breaks this ability.
Further complicating the issue is that nftables resolves interface names to ifindex at load time. This is problematic with keepalived's interface creation, which usually comes after the firewall loading, forcing the use of iifname, oifname instead (similar to iptables -i, -o).
By copying the group value, such firewall rules can continue to work regardless of the use_vmac or use_ipvlan settings, since packets may now arrive on, or be routed out from, the new interfaces.
I've tested that this works with use_vmac, but I'm not sure about use_ipvlan or cross-namespace cases. There is already a function for setting interface groups, netlink_link_group(), but it was introduced to work around a netlink issue by setting it to the current value, and it creates a whole new request, which is unnecessary here.
I've thought about other ways to make firewall rules that play more nicely with the use_vmac interfaces, but so far the best I've come up with is interface groups and iifname, oifname for all of the other cases. Eventually, maybe it would make sense for the kernel to update nft findexes dynamically whenever an interface name appears, rather than doing it only at load time.