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

From: Eyal Birger
Date: Thu Feb 22 2024 - 17:31:24 EST

Next message: Huang, Kai: "Re: [PATCH v9 08/15] x86/sgx: Implement EPC reclamation flows for cgroup"
Previous message: David Laight: "RE: [PATCH v2] checkpatch: add check for snprintf to scnprintf"
In reply to: Richard Gobert: "[PATCH net-next 2/2] net: geneve: enable local address bind for geneve sockets"
Next in thread: Jakub Kicinski: "Re: [PATCH net-next 0/2] net: add local address bind support to vxlan and geneve"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi,

On Thu, Feb 22, 2024 at 12:54 PM Richard Gobert
<richardbgobert@xxxxxxxxx> 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 <richardbgobert@xxxxxxxxx>
> ---
> 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
>

Next message: Huang, Kai: "Re: [PATCH v9 08/15] x86/sgx: Implement EPC reclamation flows for cgroup"
Previous message: David Laight: "RE: [PATCH v2] checkpatch: add check for snprintf to scnprintf"
In reply to: Richard Gobert: "[PATCH net-next 2/2] net: geneve: enable local address bind for geneve sockets"
Next in thread: Jakub Kicinski: "Re: [PATCH net-next 0/2] net: add local address bind support to vxlan and geneve"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]