On Mon, Jun 1, 2026 at 7:34 PM Jakub Kicinski [off-list ref] wrote:

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
geneve: Introduce IFLA_GENEVE_LOCAL and IFLA_GENEVE_LOCAL6.

This patch adds IFLA_GENEVE_LOCAL and IFLA_GENEVE_LOCAL6 attributes so a
GENEVE device's underlying UDP socket can be bound to a specific local
IPv4 or IPv6 address rather than the wildcard, and updates lookup paths
and xmit paths accordingly.

quoted

diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index 3a62d132a8c4..c5372b33a305 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c

[ ... ]

quoted

@@ -1972,6 +2043,55 @@ static int geneve_nl2info(struct nlattr *tb[], struct nlattr *data[],
 #endif
      }

+     if (data[IFLA_GENEVE_LOCAL]) {
+             if (changelink) {
+                     attrtype = IFLA_GENEVE_LOCAL;
+                     goto change_notsup;
+             }
+
+             info->key.u.ipv4.src = nla_get_in_addr(data[IFLA_GENEVE_LOCAL]);
+
+             if (ipv4_is_multicast(info->key.u.ipv4.src)) {
+                     NL_SET_ERR_MSG_ATTR(extack, data[IFLA_GENEVE_LOCAL],
+                                         "Local IPv4 address cannot be Multicast");
+                     return -EINVAL;
+             }
+
+             cfg->dualstack = false;
+     }
+
+     if (data[IFLA_GENEVE_LOCAL6]) {
+#if IS_ENABLED(CONFIG_IPV6)
+             int addr_type;
+
+             if (changelink) {
+                     attrtype = IFLA_GENEVE_LOCAL6;
+                     goto change_notsup;
+             }

[Medium]
Should the changelink rejection here mirror the IFLA_GENEVE_REMOTE
behavior in this same function? IFLA_GENEVE_REMOTE only fails the
changelink when the family changes:

REMOTE can be changed easily for each xmit, but LOCAL requires
re-bind(), which would unnecessarily complicate the code, so
I just disallowed changelink for simplicity.

        if (data[IFLA_GENEVE_REMOTE]) {
                if (changelink && (ip_tunnel_info_af(info) == AF_INET6)) {
                        attrtype = IFLA_GENEVE_REMOTE;
                        goto change_notsup;
                }

whereas IFLA_GENEVE_LOCAL and IFLA_GENEVE_LOCAL6 fail unconditionally
whenever the attribute is present, even when the value matches the
existing configuration.

Combined with the geneve_fill_info() change below, would this break the
common dump-then-changelink round trip used by some management tools on
pre-existing devices, returning -EOPNOTSUPP even though LOCAL was never
explicitly configured?

[ ... ]

quoted

@@ -2320,6 +2441,20 @@ static int geneve_fill_info(struct sk_buff *skb, const struct net_device *dev)
 #endif
      }

+     if (!geneve->cfg.dualstack) {
+             if (ip_tunnel_info_af(info) == AF_INET) {
+                     if (nla_put_in_addr(skb, IFLA_GENEVE_LOCAL,
+                                         info->key.u.ipv4.src))
+                             goto nla_put_failure;
+#if IS_ENABLED(CONFIG_IPV6)
+             } else {
+                     if (nla_put_in6_addr(skb, IFLA_GENEVE_LOCAL6,
+                                          &info->key.u.ipv6.src))
+                             goto nla_put_failure;
+#endif
+             }
+     }
+

[Medium]
Does this emit IFLA_GENEVE_LOCAL or IFLA_GENEVE_LOCAL6 on every
non-collect_md device, including legacy devices created without any
local address?

cfg->dualstack is only set true for collect_md devices, so for any
device where the user never specified a local address, info->key.u.ipv4.src
is 0 (or info->key.u.ipv6.src is ::) but the attribute is still emitted.
This would change the visible output of ip -d link show and netlink
dumps for existing GENEVE devices, adding a synthetic local 0.0.0.0 or
local :: that was not present before.

Would gating this on a non-zero source address (or tracking whether the
user explicitly set LOCAL) avoid the dump-output change for unaffected
devices?

Honestly I don't have strong preference on this.

New iproute2 will show 0.0.0.0 or :: even for devices created without
the attribute indeed, but I guess it's almost always the case that
the dump command shows every attribute when we add a new one...?

The commit message describes the new behavior on devices that set
LOCAL/LOCAL6 but does not mention that fill_info now emits these
attributes for every previously-created non-collect_md device, nor that
changelink will reject any presence of LOCAL/LOCAL6 even when the value
is unchanged. Could the commit message call out these compatibility
implications?