containernetworking / plugins

Some reference and example networking plugins, maintained by the CNI team.
Apache License 2.0
2.23k stars 788 forks source link

ipmasq nftables doesn't support multiple ips (dual stack) #1118

Closed champtar closed 3 days ago

champtar commented 2 weeks ago

dual stack setup

{
  "type": "ptp",
  "ipMasq": true,
  "ipMasqBackend": "nftables",
  "ipam": {
    "type": "host-local",
    "ranges": [
      [{"subnet": "198.18.0.0/17"}],
      [{"subnet": "fd61:7465:6d65:1000::/112"}]
    ],
    "routes": [
      { "dst": "0.0.0.0/0" },
      { "dst": "::/0" },
    ]
  }
},

Looking at nft list ruleset, only the ip6 rules are present in cni_plugins_masquerade table Looking at nft monitor rules, we see that the ip rules are added then deleted

https://github.com/containernetworking/plugins/blob/fec2d62676cbe4f2fd587b4840c7fc021bead3f9/pkg/ip/ipmasq_nftables_linux.go#L84-L85 In setupIPMasqNFTablesWithInterface the stale rule logic is incorrect

danwinship commented 2 weeks ago

right... probably should have separate ip and ip6 tables rather than a single inet one... I think I had originally tried to use inet for the portmap plugin too, but eventually decided it worked better with separate tables.

champtar commented 2 weeks ago

There is no limit on the number of ranges, you could have 2 IPv4 and 5 IPv6, so having separate ip/ip6 would only fix the dual stack case. We need the list of all IPs in setupIPMasqNFTablesWithInterface to be able to cleanup and recreate all new rules at once. Bonus point it'll do only 1 nft call instead of 1 per range.

danwinship commented 2 weeks ago

ah. yes, either that or you could include the range in the comment