LinuxLists.cc - [PATCH net-next 0/2] net: add local address bind support to vxlan and geneve

2024-02-22 20:49:24

Subject: [PATCH net-next 0/2] net: add local address bind support to vxlan and geneve

This series adds local address bind support to both vxlan
and geneve sockets.

Richard Gobert (2):
net: vxlan: enable local address bind for vxlan sockets
net: geneve: enable local address bind for geneve sockets

drivers/net/geneve.c | 58 +++++++++++++++++++++++++++---
drivers/net/vxlan/vxlan_core.c | 19 +++++++---
include/net/geneve.h | 6 ++++
include/uapi/linux/if_link.h | 2 ++
tools/include/uapi/linux/if_link.h | 2 ++
5 files changed, 77 insertions(+), 10 deletions(-)

--
2.36.1

2024-02-22 20:52:16

by Richard Gobert

[permalink] [raw]

Subject: [PATCH net-next 1/2] net: vxlan: enable local address bind for vxlan sockets

This patch adds support for binding to a local address in vxlan sockets.
It achieves this by using vxlan_addr union to represent a local address
to bind to, and copying it to udp_port_cfg in vxlan_create_sock.

Signed-off-by: Richard Gobert <[email protected]>
---
drivers/net/vxlan/vxlan_core.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
index 16106e088c63..b5f3f946ebcd 100644
--- a/drivers/net/vxlan/vxlan_core.c
+++ b/drivers/net/vxlan/vxlan_core.c
@@ -3479,8 +3479,9 @@ static const struct ethtool_ops vxlan_ethtool_ops = {
.get_link_ksettings = vxlan_get_link_ksettings,
};

-static struct socket *vxlan_create_sock(struct net *net, bool ipv6,
- __be16 port, u32 flags, int ifindex)
+static struct socket *vxlan_create_sock(struct net *net, bool ipv6, __be16 port,
+ u32 flags, int ifindex,
+ union vxlan_addr addr)
{
struct socket *sock;
struct udp_port_cfg udp_conf;
@@ -3493,8 +3494,15 @@ static struct socket *vxlan_create_sock(struct net *net, bool ipv6,
udp_conf.use_udp6_rx_checksums =
!(flags & VXLAN_F_UDP_ZERO_CSUM6_RX);
udp_conf.ipv6_v6only = 1;
+ memcpy(&udp_conf.local_ip6.s6_addr32,
+ &addr.sin6.sin6_addr.s6_addr32,
+ sizeof(addr.sin6.sin6_addr.s6_addr32));
} else {
udp_conf.family = AF_INET;
+ udp_conf.local_ip.s_addr = addr.sin.sin_addr.s_addr;
+ memcpy(&udp_conf.local_ip.s_addr,
+ &addr.sin.sin_addr.s_addr,
+ sizeof(addr.sin.sin_addr.s_addr));
}

udp_conf.local_udp_port = port;
@@ -3512,7 +3520,8 @@ static struct socket *vxlan_create_sock(struct net *net, bool ipv6,
/* Create new listen socket if needed */
static struct vxlan_sock *vxlan_socket_create(struct net *net, bool ipv6,
__be16 port, u32 flags,
- int ifindex)
+ int ifindex,
+ union vxlan_addr addr)
{
struct vxlan_net *vn = net_generic(net, vxlan_net_id);
struct vxlan_sock *vs;
@@ -3527,7 +3536,7 @@ static struct vxlan_sock *vxlan_socket_create(struct net *net, bool ipv6,
for (h = 0; h < VNI_HASH_SIZE; ++h)
INIT_HLIST_HEAD(&vs->vni_list[h]);

- sock = vxlan_create_sock(net, ipv6, port, flags, ifindex);
+ sock = vxlan_create_sock(net, ipv6, port, flags, ifindex, addr);
if (IS_ERR(sock)) {
kfree(vs);
return ERR_CAST(sock);
@@ -3591,7 +3600,7 @@ static int __vxlan_sock_add(struct vxlan_dev *vxlan, bool ipv6)
if (!vs)
vs = vxlan_socket_create(vxlan->net, ipv6,
vxlan->cfg.dst_port, vxlan->cfg.flags,
- l3mdev_index);
+ l3mdev_index, vxlan->cfg.saddr);
if (IS_ERR(vs))
return PTR_ERR(vs);
#if IS_ENABLED(CONFIG_IPV6)
--
2.36.1

2024-02-22 20:54:23

by Richard Gobert

[permalink] [raw]

Subject: [PATCH net-next 2/2] net: geneve: enable local address bind for geneve sockets

This patch adds support for binding to a local address in geneve sockets.
It achieves this by adding a geneve_addr union to represent local address
to bind to, and copying it to udp_port_cfg in geneve_create_sock.

Signed-off-by: Richard Gobert <[email protected]>
---
drivers/net/geneve.c | 58 +++++++++++++++++++++++++++---
include/net/geneve.h | 6 ++++
include/uapi/linux/if_link.h | 2 ++
tools/include/uapi/linux/if_link.h | 2 ++
4 files changed, 63 insertions(+), 5 deletions(-)

diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index 32c51c244153..d0b4cb0e7c51 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -57,6 +57,7 @@ struct geneve_config {
bool ttl_inherit;
enum ifla_geneve_df df;
bool inner_proto_inherit;
+ union geneve_addr saddr;
};

/* Pseudo network device */
@@ -451,7 +452,8 @@ static int geneve_udp_encap_err_lookup(struct sock *sk, struct sk_buff *skb)
}

static struct socket *geneve_create_sock(struct net *net, bool ipv6,
- __be16 port, bool ipv6_rx_csum)
+ __be16 port, bool ipv6_rx_csum,
+ union geneve_addr *local_addr)
{
struct socket *sock;
struct udp_port_cfg udp_conf;
@@ -463,9 +465,15 @@ static struct socket *geneve_create_sock(struct net *net, bool ipv6,
udp_conf.family = AF_INET6;
udp_conf.ipv6_v6only = 1;
udp_conf.use_udp6_rx_checksums = ipv6_rx_csum;
+ memcpy(&udp_conf.local_ip6,
+ &local_addr->sin6.sin6_addr,
+ sizeof(local_addr->sin6.sin6_addr));
} else {
udp_conf.family = AF_INET;
udp_conf.local_ip.s_addr = htonl(INADDR_ANY);
+ memcpy(&udp_conf.local_ip,
+ &local_addr->sin.sin_addr,
+ sizeof(local_addr->sin.sin_addr));
}

udp_conf.local_udp_port = port;
@@ -572,7 +580,8 @@ static int geneve_gro_complete(struct sock *sk, struct sk_buff *skb,

/* Create new listen socket if needed */
static struct geneve_sock *geneve_socket_create(struct net *net, __be16 port,
- bool ipv6, bool ipv6_rx_csum)
+ bool ipv6, bool ipv6_rx_csum,
+ union geneve_addr *local_addr)
{
struct geneve_net *gn = net_generic(net, geneve_net_id);
struct geneve_sock *gs;
@@ -584,7 +593,7 @@ static struct geneve_sock *geneve_socket_create(struct net *net, __be16 port,
if (!gs)
return ERR_PTR(-ENOMEM);

- sock = geneve_create_sock(net, ipv6, port, ipv6_rx_csum);
+ sock = geneve_create_sock(net, ipv6, port, ipv6_rx_csum, local_addr);
if (IS_ERR(sock)) {
kfree(gs);
return ERR_CAST(sock);
@@ -672,7 +681,8 @@ static int geneve_sock_add(struct geneve_dev *geneve, bool ipv6)
}

gs = geneve_socket_create(net, geneve->cfg.info.key.tp_dst, ipv6,
- geneve->cfg.use_udp6_rx_checksums);
+ geneve->cfg.use_udp6_rx_checksums,
+ &geneve->cfg.saddr);
if (IS_ERR(gs))
return PTR_ERR(gs);

@@ -1203,7 +1213,7 @@ static void geneve_setup(struct net_device *dev)
}

static const struct nla_policy geneve_policy[IFLA_GENEVE_MAX + 1] = {
- [IFLA_GENEVE_UNSPEC] = { .strict_start_type = IFLA_GENEVE_INNER_PROTO_INHERIT },
+ [IFLA_GENEVE_UNSPEC] = { .strict_start_type = IFLA_GENEVE_LOCAL6 },
[IFLA_GENEVE_ID] = { .type = NLA_U32 },
[IFLA_GENEVE_REMOTE] = { .len = sizeof_field(struct iphdr, daddr) },
[IFLA_GENEVE_REMOTE6] = { .len = sizeof(struct in6_addr) },
@@ -1218,6 +1228,8 @@ static const struct nla_policy geneve_policy[IFLA_GENEVE_MAX + 1] = {
[IFLA_GENEVE_TTL_INHERIT] = { .type = NLA_U8 },
[IFLA_GENEVE_DF] = { .type = NLA_U8 },
[IFLA_GENEVE_INNER_PROTO_INHERIT] = { .type = NLA_FLAG },
+ [IFLA_GENEVE_LOCAL] = { .len = sizeof_field(struct iphdr, saddr) },
+ [IFLA_GENEVE_LOCAL6] = { .len = sizeof(struct in6_addr) },
};

static int geneve_validate(struct nlattr *tb[], struct nlattr *data[],
@@ -1544,6 +1556,31 @@ static int geneve_nl2info(struct nlattr *tb[], struct nlattr *data[],
cfg->inner_proto_inherit = true;
}

+ if (data[IFLA_GENEVE_LOCAL]) {
+ if (changelink && cfg->saddr.sa.sa_family != AF_INET) {
+ NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_GENEVE_LOCAL], "New local address family does not match old");
+ return -EOPNOTSUPP;
+ }
+
+ cfg->saddr.sin.sin_addr.s_addr = nla_get_in_addr(data[IFLA_GENEVE_LOCAL]);
+ cfg->saddr.sa.sa_family = AF_INET;
+ }
+
+ if (data[IFLA_GENEVE_LOCAL6]) {
+ if (!IS_ENABLED(CONFIG_IPV6)) {
+ NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_GENEVE_LOCAL6], "IPv6 support not enabled in the kernel");
+ return -EPFNOSUPPORT;
+ }
+
+ if (changelink && cfg->saddr.sa.sa_family != AF_INET6) {
+ NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_GENEVE_LOCAL6], "New local address family does not match old");
+ return -EOPNOTSUPP;
+ }
+
+ cfg->saddr.sin6.sin6_addr = nla_get_in6_addr(data[IFLA_VXLAN_LOCAL6]);
+ cfg->saddr.sa.sa_family = AF_INET6;
+ }
+
return 0;
change_notsup:
NL_SET_ERR_MSG_ATTR(extack, data[attrtype],
@@ -1724,6 +1761,7 @@ static size_t geneve_get_size(const struct net_device *dev)
nla_total_size(sizeof(__u8)) + /* IFLA_GENEVE_UDP_ZERO_CSUM6_RX */
nla_total_size(sizeof(__u8)) + /* IFLA_GENEVE_TTL_INHERIT */
nla_total_size(0) + /* IFLA_GENEVE_INNER_PROTO_INHERIT */
+ nla_total_size(sizeof(struct in6_addr)) + /* IFLA_GENEVE_LOCAL{6} */
0;
}

@@ -1745,6 +1783,11 @@ static int geneve_fill_info(struct sk_buff *skb, const struct net_device *dev)
if (nla_put_in_addr(skb, IFLA_GENEVE_REMOTE,
info->key.u.ipv4.dst))
goto nla_put_failure;
+
+ if (nla_put_in_addr(skb, IFLA_GENEVE_LOCAL,
+ info->key.u.ipv4.src))
+ goto nla_put_failure;
+
if (nla_put_u8(skb, IFLA_GENEVE_UDP_CSUM,
!!(info->key.tun_flags & TUNNEL_CSUM)))
goto nla_put_failure;
@@ -1754,6 +1797,11 @@ static int geneve_fill_info(struct sk_buff *skb, const struct net_device *dev)
if (nla_put_in6_addr(skb, IFLA_GENEVE_REMOTE6,
&info->key.u.ipv6.dst))
goto nla_put_failure;
+
+ if (nla_put_in6_addr(skb, IFLA_GENEVE_LOCAL6,
+ &info->key.u.ipv6.src))
+ goto nla_put_failure;
+
if (nla_put_u8(skb, IFLA_GENEVE_UDP_ZERO_CSUM6_TX,
!(info->key.tun_flags & TUNNEL_CSUM)))
goto nla_put_failure;
diff --git a/include/net/geneve.h b/include/net/geneve.h
index 5c96827a487e..8dcd7fff2c0f 100644
--- a/include/net/geneve.h
+++ b/include/net/geneve.h
@@ -68,6 +68,12 @@ static inline bool netif_is_geneve(const struct net_device *dev)
!strcmp(dev->rtnl_link_ops->kind, "geneve");
}

+union geneve_addr {
+ struct sockaddr_in sin;
+ struct sockaddr_in6 sin6;
+ struct sockaddr sa;
+};
+
#ifdef CONFIG_INET
struct net_device *geneve_dev_create_fb(struct net *net, const char *name,
u8 name_assign_type, u16 dst_port);
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index ab9bcff96e4d..e4a0cdea734b 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -1419,6 +1419,8 @@ enum {
IFLA_GENEVE_TTL_INHERIT,
IFLA_GENEVE_DF,
IFLA_GENEVE_INNER_PROTO_INHERIT,
+ IFLA_GENEVE_LOCAL,
+ IFLA_GENEVE_LOCAL6,
__IFLA_GENEVE_MAX
};
#define IFLA_GENEVE_MAX (__IFLA_GENEVE_MAX - 1)
diff --git a/tools/include/uapi/linux/if_link.h b/tools/include/uapi/linux/if_link.h
index a0aa05a28cf2..438bd867ec38 100644
--- a/tools/include/uapi/linux/if_link.h
+++ b/tools/include/uapi/linux/if_link.h
@@ -888,6 +888,8 @@ enum {
IFLA_GENEVE_TTL_INHERIT,
IFLA_GENEVE_DF,
IFLA_GENEVE_INNER_PROTO_INHERIT,
+ IFLA_GENEVE_LOCAL,
+ IFLA_GENEVE_LOCAL6,
__IFLA_GENEVE_MAX
};
#define IFLA_GENEVE_MAX (__IFLA_GENEVE_MAX - 1)
--
2.36.1

2024-02-22 22:31:27

by Eyal Birger

[permalink] [raw]

Subject: Re: [PATCH net-next 2/2] net: geneve: enable local address bind for geneve sockets

Hi,

On Thu, Feb 22, 2024 at 12:54 PM Richard Gobert
<[email protected]> wrote:
>
> This patch adds support for binding to a local address in geneve sockets.

Thanks for adding this.

> It achieves this by adding a geneve_addr union to represent local address
> to bind to, and copying it to udp_port_cfg in geneve_create_sock.

AFICT in geneve_sock_add(), geneve_socket_create() is only called if there's
no existing open socket with the GENEVE destination port. As such, wouldn't
this bind work only for the first socket in the namespace?

If that is the case, then perhaps binding the socket isn't the right
approach, and instead geneve_lookup() should search for the tunnel based on
both the source and destination IPs.

Am I missing something?

Eyal.
>
> Signed-off-by: Richard Gobert <[email protected]>
> ---
> drivers/net/geneve.c | 58 +++++++++++++++++++++++++++---
> include/net/geneve.h | 6 ++++
> include/uapi/linux/if_link.h | 2 ++
> tools/include/uapi/linux/if_link.h | 2 ++
> 4 files changed, 63 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
> index 32c51c244153..d0b4cb0e7c51 100644
> --- a/drivers/net/geneve.c
> +++ b/drivers/net/geneve.c
> @@ -57,6 +57,7 @@ struct geneve_config {
> bool ttl_inherit;
> enum ifla_geneve_df df;
> bool inner_proto_inherit;
> + union geneve_addr saddr;
> };
>
> /* Pseudo network device */
> @@ -451,7 +452,8 @@ static int geneve_udp_encap_err_lookup(struct sock *sk, struct sk_buff *skb)
> }
>
> static struct socket *geneve_create_sock(struct net *net, bool ipv6,
> - __be16 port, bool ipv6_rx_csum)
> + __be16 port, bool ipv6_rx_csum,
> + union geneve_addr *local_addr)
> {
> struct socket *sock;
> struct udp_port_cfg udp_conf;
> @@ -463,9 +465,15 @@ static struct socket *geneve_create_sock(struct net *net, bool ipv6,
> udp_conf.family = AF_INET6;
> udp_conf.ipv6_v6only = 1;
> udp_conf.use_udp6_rx_checksums = ipv6_rx_csum;
> + memcpy(&udp_conf.local_ip6,
> + &local_addr->sin6.sin6_addr,
> + sizeof(local_addr->sin6.sin6_addr));
> } else {
> udp_conf.family = AF_INET;
> udp_conf.local_ip.s_addr = htonl(INADDR_ANY);
> + memcpy(&udp_conf.local_ip,
> + &local_addr->sin.sin_addr,
> + sizeof(local_addr->sin.sin_addr));
> }
>
> udp_conf.local_udp_port = port;
> @@ -572,7 +580,8 @@ static int geneve_gro_complete(struct sock *sk, struct sk_buff *skb,
>
> /* Create new listen socket if needed */
> static struct geneve_sock *geneve_socket_create(struct net *net, __be16 port,
> - bool ipv6, bool ipv6_rx_csum)
> + bool ipv6, bool ipv6_rx_csum,
> + union geneve_addr *local_addr)
> {
> struct geneve_net *gn = net_generic(net, geneve_net_id);
> struct geneve_sock *gs;
> @@ -584,7 +593,7 @@ static struct geneve_sock *geneve_socket_create(struct net *net, __be16 port,
> if (!gs)
> return ERR_PTR(-ENOMEM);
>
> - sock = geneve_create_sock(net, ipv6, port, ipv6_rx_csum);
> + sock = geneve_create_sock(net, ipv6, port, ipv6_rx_csum, local_addr);
> if (IS_ERR(sock)) {
> kfree(gs);
> return ERR_CAST(sock);
> @@ -672,7 +681,8 @@ static int geneve_sock_add(struct geneve_dev *geneve, bool ipv6)
> }
>
> gs = geneve_socket_create(net, geneve->cfg.info.key.tp_dst, ipv6,
> - geneve->cfg.use_udp6_rx_checksums);
> + geneve->cfg.use_udp6_rx_checksums,
> + &geneve->cfg.saddr);
> if (IS_ERR(gs))
> return PTR_ERR(gs);
>
> @@ -1203,7 +1213,7 @@ static void geneve_setup(struct net_device *dev)
> }
>
> static const struct nla_policy geneve_policy[IFLA_GENEVE_MAX + 1] = {
> - [IFLA_GENEVE_UNSPEC] = { .strict_start_type = IFLA_GENEVE_INNER_PROTO_INHERIT },
> + [IFLA_GENEVE_UNSPEC] = { .strict_start_type = IFLA_GENEVE_LOCAL6 },
> [IFLA_GENEVE_ID] = { .type = NLA_U32 },
> [IFLA_GENEVE_REMOTE] = { .len = sizeof_field(struct iphdr, daddr) },
> [IFLA_GENEVE_REMOTE6] = { .len = sizeof(struct in6_addr) },
> @@ -1218,6 +1228,8 @@ static const struct nla_policy geneve_policy[IFLA_GENEVE_MAX + 1] = {
> [IFLA_GENEVE_TTL_INHERIT] = { .type = NLA_U8 },
> [IFLA_GENEVE_DF] = { .type = NLA_U8 },
> [IFLA_GENEVE_INNER_PROTO_INHERIT] = { .type = NLA_FLAG },
> + [IFLA_GENEVE_LOCAL] = { .len = sizeof_field(struct iphdr, saddr) },
> + [IFLA_GENEVE_LOCAL6] = { .len = sizeof(struct in6_addr) },
> };
>
> static int geneve_validate(struct nlattr *tb[], struct nlattr *data[],
> @@ -1544,6 +1556,31 @@ static int geneve_nl2info(struct nlattr *tb[], struct nlattr *data[],
> cfg->inner_proto_inherit = true;
> }
>
> + if (data[IFLA_GENEVE_LOCAL]) {
> + if (changelink && cfg->saddr.sa.sa_family != AF_INET) {
> + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_GENEVE_LOCAL], "New local address family does not match old");
> + return -EOPNOTSUPP;
> + }
> +
> + cfg->saddr.sin.sin_addr.s_addr = nla_get_in_addr(data[IFLA_GENEVE_LOCAL]);
> + cfg->saddr.sa.sa_family = AF_INET;
> + }
> +
> + if (data[IFLA_GENEVE_LOCAL6]) {
> + if (!IS_ENABLED(CONFIG_IPV6)) {
> + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_GENEVE_LOCAL6], "IPv6 support not enabled in the kernel");
> + return -EPFNOSUPPORT;
> + }
> +
> + if (changelink && cfg->saddr.sa.sa_family != AF_INET6) {
> + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_GENEVE_LOCAL6], "New local address family does not match old");
> + return -EOPNOTSUPP;
> + }
> +
> + cfg->saddr.sin6.sin6_addr = nla_get_in6_addr(data[IFLA_VXLAN_LOCAL6]);
> + cfg->saddr.sa.sa_family = AF_INET6;
> + }
> +
> return 0;
> change_notsup:
> NL_SET_ERR_MSG_ATTR(extack, data[attrtype],
> @@ -1724,6 +1761,7 @@ static size_t geneve_get_size(const struct net_device *dev)
> nla_total_size(sizeof(__u8)) + /* IFLA_GENEVE_UDP_ZERO_CSUM6_RX */
> nla_total_size(sizeof(__u8)) + /* IFLA_GENEVE_TTL_INHERIT */
> nla_total_size(0) + /* IFLA_GENEVE_INNER_PROTO_INHERIT */
> + nla_total_size(sizeof(struct in6_addr)) + /* IFLA_GENEVE_LOCAL{6} */
> 0;
> }
>
> @@ -1745,6 +1783,11 @@ static int geneve_fill_info(struct sk_buff *skb, const struct net_device *dev)
> if (nla_put_in_addr(skb, IFLA_GENEVE_REMOTE,
> info->key.u.ipv4.dst))
> goto nla_put_failure;
> +
> + if (nla_put_in_addr(skb, IFLA_GENEVE_LOCAL,
> + info->key.u.ipv4.src))
> + goto nla_put_failure;
> +
> if (nla_put_u8(skb, IFLA_GENEVE_UDP_CSUM,
> !!(info->key.tun_flags & TUNNEL_CSUM)))
> goto nla_put_failure;
> @@ -1754,6 +1797,11 @@ static int geneve_fill_info(struct sk_buff *skb, const struct net_device *dev)
> if (nla_put_in6_addr(skb, IFLA_GENEVE_REMOTE6,
> &info->key.u.ipv6.dst))
> goto nla_put_failure;
> +
> + if (nla_put_in6_addr(skb, IFLA_GENEVE_LOCAL6,
> + &info->key.u.ipv6.src))
> + goto nla_put_failure;
> +
> if (nla_put_u8(skb, IFLA_GENEVE_UDP_ZERO_CSUM6_TX,
> !(info->key.tun_flags & TUNNEL_CSUM)))
> goto nla_put_failure;
> diff --git a/include/net/geneve.h b/include/net/geneve.h
> index 5c96827a487e..8dcd7fff2c0f 100644
> --- a/include/net/geneve.h
> +++ b/include/net/geneve.h
> @@ -68,6 +68,12 @@ static inline bool netif_is_geneve(const struct net_device *dev)
> !strcmp(dev->rtnl_link_ops->kind, "geneve");
> }
>
> +union geneve_addr {
> + struct sockaddr_in sin;
> + struct sockaddr_in6 sin6;
> + struct sockaddr sa;
> +};
> +
> #ifdef CONFIG_INET
> struct net_device *geneve_dev_create_fb(struct net *net, const char *name,
> u8 name_assign_type, u16 dst_port);
> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index ab9bcff96e4d..e4a0cdea734b 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -1419,6 +1419,8 @@ enum {
> IFLA_GENEVE_TTL_INHERIT,
> IFLA_GENEVE_DF,
> IFLA_GENEVE_INNER_PROTO_INHERIT,
> + IFLA_GENEVE_LOCAL,
> + IFLA_GENEVE_LOCAL6,
> __IFLA_GENEVE_MAX
> };
> #define IFLA_GENEVE_MAX (__IFLA_GENEVE_MAX - 1)
> diff --git a/tools/include/uapi/linux/if_link.h b/tools/include/uapi/linux/if_link.h
> index a0aa05a28cf2..438bd867ec38 100644
> --- a/tools/include/uapi/linux/if_link.h
> +++ b/tools/include/uapi/linux/if_link.h
> @@ -888,6 +888,8 @@ enum {
> IFLA_GENEVE_TTL_INHERIT,
> IFLA_GENEVE_DF,
> IFLA_GENEVE_INNER_PROTO_INHERIT,
> + IFLA_GENEVE_LOCAL,
> + IFLA_GENEVE_LOCAL6,
> __IFLA_GENEVE_MAX
> };
> #define IFLA_GENEVE_MAX (__IFLA_GENEVE_MAX - 1)
> --
> 2.36.1
>

2024-02-23 14:11:11

by Jakub Kicinski

[permalink] [raw]

Subject: Re: [PATCH net-next 0/2] net: add local address bind support to vxlan and geneve

On Thu, 22 Feb 2024 21:48:39 +0100 Richard Gobert wrote:
> This series adds local address bind support to both vxlan
> and geneve sockets.

Looks like this breaks a lot of vxlan selftests.
--
pw-bot: cr

2024-02-23 17:52:00

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH net-next 1/2] net: vxlan: enable local address bind for vxlan sockets

2024-02-24 05:58:55

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH net-next 1/2] net: vxlan: enable local address bind for vxlan sockets

Hi Richard,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url: https://github.com/intel-lab-lkp/linux/commits/Richard-Gobert/net-vxlan-enable-local-address-bind-for-vxlan-sockets/20240223-045600
base: net-next/main
patch link: https://lore.kernel.org/r/a4cd1adb-74d4-4eea-9f74-0d0ac3d79e44%40gmail.com
patch subject: [PATCH net-next 1/2] net: vxlan: enable local address bind for vxlan sockets
config: x86_64-randconfig-122-20240224 (https://download.01.org/0day-ci/archive/20240224/[email protected]/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240224/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

>> drivers/net/vxlan/vxlan_core.c:3498:20: error: no member named 'local_ip6' in 'struct udp_port_cfg'
3498 | memcpy(&udp_conf.local_ip6.s6_addr32,
| ~~~~~~~~ ^
include/linux/fortify-string.h:639:17: note: expanded from macro 'memcpy'
639 | __struct_size(p), __struct_size(q), \
| ^
include/linux/compiler_types.h:358:56: note: expanded from macro '__struct_size'
358 | #define __struct_size(p) __builtin_dynamic_object_size(p, 0)
| ^
include/linux/fortify-string.h:582:27: note: expanded from macro '__fortify_memcpy_chk'
582 | const size_t __p_size = (p_size); \
| ^~~~~~
>> drivers/net/vxlan/vxlan_core.c:3498:20: error: no member named 'local_ip6' in 'struct udp_port_cfg'
3498 | memcpy(&udp_conf.local_ip6.s6_addr32,
| ~~~~~~~~ ^
include/linux/fortify-string.h:640:17: note: expanded from macro 'memcpy'
640 | __member_size(p), __member_size(q), \
| ^
include/linux/compiler_types.h:359:56: note: expanded from macro '__member_size'
359 | #define __member_size(p) __builtin_dynamic_object_size(p, 1)
| ^
include/linux/fortify-string.h:584:33: note: expanded from macro '__fortify_memcpy_chk'
584 | const size_t __p_size_field = (p_size_field); \
| ^~~~~~~~~~~~
>> drivers/net/vxlan/vxlan_core.c:3498:20: error: no member named 'local_ip6' in 'struct udp_port_cfg'
3498 | memcpy(&udp_conf.local_ip6.s6_addr32,
| ~~~~~~~~ ^
include/linux/fortify-string.h:638:47: note: expanded from macro 'memcpy'
638 | #define memcpy(p, q, s) __fortify_memcpy_chk(p, q, s, \
| ^
include/linux/fortify-string.h:593:20: note: expanded from macro '__fortify_memcpy_chk'
593 | __underlying_##op(p, q, __fortify_size); \
| ^
3 errors generated.

vim +3498 drivers/net/vxlan/vxlan_core.c

3482
3483 static struct socket *vxlan_create_sock(struct net *net, bool ipv6, __be16 port,
3484 u32 flags, int ifindex,
3485 union vxlan_addr addr)
3486 {
3487 struct socket *sock;
3488 struct udp_port_cfg udp_conf;
3489 int err;
3490
3491 memset(&udp_conf, 0, sizeof(udp_conf));
3492
3493 if (ipv6) {
3494 udp_conf.family = AF_INET6;
3495 udp_conf.use_udp6_rx_checksums =
3496 !(flags & VXLAN_F_UDP_ZERO_CSUM6_RX);
3497 udp_conf.ipv6_v6only = 1;
> 3498 memcpy(&udp_conf.local_ip6.s6_addr32,
3499 &addr.sin6.sin6_addr.s6_addr32,
3500 sizeof(addr.sin6.sin6_addr.s6_addr32));
3501 } else {
3502 udp_conf.family = AF_INET;
3503 udp_conf.local_ip.s_addr = addr.sin.sin_addr.s_addr;
3504 memcpy(&udp_conf.local_ip.s_addr,
3505 &addr.sin.sin_addr.s_addr,
3506 sizeof(addr.sin.sin_addr.s_addr));
3507 }
3508
3509 udp_conf.local_udp_port = port;
3510 udp_conf.bind_ifindex = ifindex;
3511
3512 /* Open UDP socket */
3513 err = udp_sock_create(net, &udp_conf, &sock);
3514 if (err < 0)
3515 return ERR_PTR(err);
3516
3517 udp_allow_gso(sock->sk);
3518 return sock;
3519 }
3520

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2024-02-24 09:01:51

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH net-next 2/2] net: geneve: enable local address bind for geneve sockets

Hi Richard,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url: https://github.com/intel-lab-lkp/linux/commits/Richard-Gobert/net-vxlan-enable-local-address-bind-for-vxlan-sockets/20240223-045600
base: net-next/main
patch link: https://lore.kernel.org/r/79a8ba83-86bf-4c22-845c-8f285c2d1396%40gmail.com
patch subject: [PATCH net-next 2/2] net: geneve: enable local address bind for geneve sockets
config: x86_64-randconfig-012-20240224 (https://download.01.org/0day-ci/archive/20240224/[email protected]/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240224/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

In file included from include/linux/string.h:292,
from include/linux/bitmap.h:12,
from include/linux/ethtool.h:16,
from drivers/net/geneve.c:10:
drivers/net/geneve.c: In function 'geneve_create_sock':
>> drivers/net/geneve.c:469:34: error: 'struct udp_port_cfg' has no member named 'local_ip6'; did you mean 'local_ip'?
469 | memcpy(&udp_conf.local_ip6,
| ^~~~~~~~~
include/linux/fortify-string.h:582:34: note: in definition of macro '__fortify_memcpy_chk'
582 | const size_t __p_size = (p_size); \
| ^~~~~~
include/linux/fortify-string.h:639:17: note: in expansion of macro '__struct_size'
639 | __struct_size(p), __struct_size(q), \
| ^~~~~~~~~~~~~
drivers/net/geneve.c:469:17: note: in expansion of macro 'memcpy'
469 | memcpy(&udp_conf.local_ip6,
| ^~~~~~
>> drivers/net/geneve.c:469:34: error: 'struct udp_port_cfg' has no member named 'local_ip6'; did you mean 'local_ip'?
469 | memcpy(&udp_conf.local_ip6,
| ^~~~~~~~~
include/linux/fortify-string.h:584:40: note: in definition of macro '__fortify_memcpy_chk'
584 | const size_t __p_size_field = (p_size_field); \
| ^~~~~~~~~~~~
include/linux/fortify-string.h:640:17: note: in expansion of macro '__member_size'
640 | __member_size(p), __member_size(q), \
| ^~~~~~~~~~~~~
drivers/net/geneve.c:469:17: note: in expansion of macro 'memcpy'
469 | memcpy(&udp_conf.local_ip6,
| ^~~~~~
>> drivers/net/geneve.c:469:34: error: 'struct udp_port_cfg' has no member named 'local_ip6'; did you mean 'local_ip'?
469 | memcpy(&udp_conf.local_ip6,
| ^~~~~~~~~
include/linux/fortify-string.h:593:27: note: in definition of macro '__fortify_memcpy_chk'
593 | __underlying_##op(p, q, __fortify_size); \
| ^
drivers/net/geneve.c:469:17: note: in expansion of macro 'memcpy'
469 | memcpy(&udp_conf.local_ip6,
| ^~~~~~

vim +469 drivers/net/geneve.c

454
455 static struct socket *geneve_create_sock(struct net *net, bool ipv6,
456 __be16 port, bool ipv6_rx_csum,
457 union geneve_addr *local_addr)
458 {
459 struct socket *sock;
460 struct udp_port_cfg udp_conf;
461 int err;
462
463 memset(&udp_conf, 0, sizeof(udp_conf));
464
465 if (ipv6) {
466 udp_conf.family = AF_INET6;
467 udp_conf.ipv6_v6only = 1;
468 udp_conf.use_udp6_rx_checksums = ipv6_rx_csum;
> 469 memcpy(&udp_conf.local_ip6,
470 &local_addr->sin6.sin6_addr,
471 sizeof(local_addr->sin6.sin6_addr));
472 } else {
473 udp_conf.family = AF_INET;
474 udp_conf.local_ip.s_addr = htonl(INADDR_ANY);
475 memcpy(&udp_conf.local_ip,
476 &local_addr->sin.sin_addr,
477 sizeof(local_addr->sin.sin_addr));
478 }
479
480 udp_conf.local_udp_port = port;
481
482 /* Open UDP socket */
483 err = udp_sock_create(net, &udp_conf, &sock);
484 if (err < 0)
485 return ERR_PTR(err);
486
487 udp_allow_gso(sock->sk);
488 return sock;
489 }
490

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2024-02-24 10:06:55

by Jiri Benc

[permalink] [raw]

Subject: Re: [PATCH net-next 2/2] net: geneve: enable local address bind for geneve sockets

On Thu, 22 Feb 2024 21:53:50 +0100, Richard Gobert wrote:
> static const struct nla_policy geneve_policy[IFLA_GENEVE_MAX + 1] = {
> - [IFLA_GENEVE_UNSPEC] = { .strict_start_type = IFLA_GENEVE_INNER_PROTO_INHERIT },
> + [IFLA_GENEVE_UNSPEC] = { .strict_start_type = IFLA_GENEVE_LOCAL6 },

The strict_start_type value should stay at
IFLA_GENEVE_INNER_PROTO_INHERIT. We don't want to relax the strict
checking for that attribute and we want strict checking for the newly
added IFLA_GENEVE_LOCAL.

See the documentation in include/net/netlink.h.

Jiri

2024-02-27 09:18:44

by Richard Gobert

[permalink] [raw]

Subject: Re: [PATCH net-next 2/2] net: geneve: enable local address bind for geneve sockets

Eyal Birger wrote:
> Hi,
>
> On Thu, Feb 22, 2024 at 12:54 PM Richard Gobert
> <[email protected]> wrote:
>>
>> This patch adds support for binding to a local address in geneve sockets.
>
> Thanks for adding this.
>
>> It achieves this by adding a geneve_addr union to represent local address
>> to bind to, and copying it to udp_port_cfg in geneve_create_sock.
>
> AFICT in geneve_sock_add(), geneve_socket_create() is only called if there's
> no existing open socket with the GENEVE destination port. As such, wouldn't
> this bind work only for the first socket in the namespace?
>
> If that is the case, then perhaps binding the socket isn't the right
> approach, and instead geneve_lookup() should search for the tunnel based on
> both the source and destination IPs.
>
> Am I missing something?
>
> Eyal

You are right, I missed it.
Binding the socket is the main reason for the patch, to prevent exposing
the geneve port on all interfaces.
I think it should be searched in geneve{6}_lookup and in geneve_find_sock:

static struct geneve_sock *geneve_find_sock(struct geneve_net *gn,
sa_family_t family,
union geneve_addr *saddr)
{
struct geneve_sock *gs;

list_for_each_entry(gs, &gn->sock_list, list) {
struct inet_sock *inet = inet_sk(gs->sock->sk);

if (inet->inet_sport == dst_port && geneve_get_sk_family(gs) == family) {
if (family == AF_INET && inet->inet_rcv_saddr == saddr->sin.sin_addr.s_addr)
return gs;
...

This is also true for VXLAN
What do you think?
Thanks

2024-02-28 17:52:43

by Eyal Birger

[permalink] [raw]

Subject: Re: [PATCH net-next 2/2] net: geneve: enable local address bind for geneve sockets

On Tue, Feb 27, 2024 at 1:03 AM Richard Gobert <[email protected]> wrote:
>
> Eyal Birger wrote:
> > Hi,
> >
> > On Thu, Feb 22, 2024 at 12:54 PM Richard Gobert
> > <[email protected]> wrote:
> >>
> >> This patch adds support for binding to a local address in geneve sockets.
> >
> > Thanks for adding this.
> >
> >> It achieves this by adding a geneve_addr union to represent local address
> >> to bind to, and copying it to udp_port_cfg in geneve_create_sock.
> >
> > AFICT in geneve_sock_add(), geneve_socket_create() is only called if there's
> > no existing open socket with the GENEVE destination port. As such, wouldn't
> > this bind work only for the first socket in the namespace?
> >
> > If that is the case, then perhaps binding the socket isn't the right
> > approach, and instead geneve_lookup() should search for the tunnel based on
> > both the source and destination IPs.
> >
> > Am I missing something?
> >
> > Eyal
>
> You are right, I missed it.
> Binding the socket is the main reason for the patch, to prevent exposing
> the geneve port on all interfaces.

I see. The use case I had in mind is allowing to differentiate between
tunnels based on local IP, but not exposing the port sounds like
a good use case too.

> I think it should be searched in geneve{6}_lookup and in geneve_find_sock:

If the socket is bound to a specific IP, i'm not sure you'd need to
change geneve{6}_lookup() - only packets matching that IP would arrive
there no? In that case I think the change you suggested to geneve_find_sock()
should be enough.

>
> static struct geneve_sock *geneve_find_sock(struct geneve_net *gn,
> sa_family_t family,
> union geneve_addr *saddr)
> {
> struct geneve_sock *gs;
>
> list_for_each_entry(gs, &gn->sock_list, list) {
> struct inet_sock *inet = inet_sk(gs->sock->sk);
>
> if (inet->inet_sport == dst_port && geneve_get_sk_family(gs) == family) {
> if (family == AF_INET && inet->inet_rcv_saddr == saddr->sin.sin_addr.s_addr)
> return gs;
> ...
>
> This is also true for VXLAN

I haven't looked at the VXLAN code, but if the lookup is similar there
I would guess the same.

Eyal.