[PATCH net-next 1/1] net: Add new protocol attribute to IP addresses

Subsystems: networking drivers, networking [general], networking [ipv4/ipv6], the rest

STALE1694d

7 messages, 3 authors, 2022-02-07 · open the first message on its own page

[PATCH net-next 1/1] net: Add new protocol attribute to IP addresses

From: Jacques de Laval <hidden>
Date: 2022-02-03 16:31:53

This patch adds a new protocol attribute to IPv4 and IPv6 addresses.
Inspiration was taken from the protocol attribute of routes. User space
applications like iproute2 can set/get the protocol with the Netlink API.

The attribute is stored as an 8-bit unsigned int. Only IFAPROT_UNSPEC is
defined. The rest of the available ids are available for user space to
define.

Grouping addresses on their origin is useful in scenarios where you want
to distinguish between addresses coming from a specific protocol like DHCP
and addresses that have been statically set.

Tagging addresses with a string label is an existing feature that could be
used as a solution. Unfortunately the max length of a label is
15 characters, and for compatibility reasons the label must be prefixed
with the name of the device followed by a colon. Since device names also
have a max length of 15 characters, only -1 characters is guaranteed to be
available for any origin tag, which is not that much.

A reference implementation of user space setting and getting protocols
is available for iproute2:

Link: https://github.com/westermo/iproute2/commit/9a6ea18bd79f47f293e5edc7780f315ea42ff540

Signed-off-by: Jacques de Laval <redacted>
---
 include/linux/inetdevice.h   |  1 +
 include/net/addrconf.h       |  1 +
 include/net/if_inet6.h       |  2 ++
 include/uapi/linux/if_addr.h |  4 ++++
 net/ipv4/devinet.c           |  8 ++++++++
 net/ipv6/addrconf.c          | 12 ++++++++++++
 6 files changed, 28 insertions(+)
diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
index a038feb63f23..caa6b7a5b5ac 100644
--- a/include/linux/inetdevice.h
+++ b/include/linux/inetdevice.h
@@ -148,6 +148,7 @@ struct in_ifaddr {
 	unsigned char		ifa_prefixlen;
 	__u32			ifa_flags;
 	char			ifa_label[IFNAMSIZ];
+	unsigned char		ifa_proto;
 
 	/* In seconds, relative to tstamp. Expiry is at tstamp + HZ * lft. */
 	__u32			ifa_valid_lft;
diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index 78ea3e332688..e53d8f4f4166 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -69,6 +69,7 @@ struct ifa6_config {
 	u32			preferred_lft;
 	u32			valid_lft;
 	u16			scope;
+	u8			ifa_proto;
 };
 
 int addrconf_init(void);
diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h
index 653e7d0f65cb..f7c270b24167 100644
--- a/include/net/if_inet6.h
+++ b/include/net/if_inet6.h
@@ -73,6 +73,8 @@ struct inet6_ifaddr {
 
 	struct rcu_head		rcu;
 	struct in6_addr		peer_addr;
+
+	__u8			ifa_proto;
 };
 
 struct ip6_sf_socklist {
diff --git a/include/uapi/linux/if_addr.h b/include/uapi/linux/if_addr.h
index dfcf3ce0097f..2aa46b9c9961 100644
--- a/include/uapi/linux/if_addr.h
+++ b/include/uapi/linux/if_addr.h
@@ -35,6 +35,7 @@ enum {
 	IFA_FLAGS,
 	IFA_RT_PRIORITY,  /* u32, priority/metric for prefix route */
 	IFA_TARGET_NETNSID,
+	IFA_PROTO,
 	__IFA_MAX,
 };
 
@@ -69,4 +70,7 @@ struct ifa_cacheinfo {
 #define IFA_PAYLOAD(n) NLMSG_PAYLOAD(n,sizeof(struct ifaddrmsg))
 #endif
 
+/* ifa_protocol */
+#define IFAPROT_UNSPEC	0
+
 #endif
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 4744c7839de5..369b229d0de4 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -102,6 +102,7 @@ static const struct nla_policy ifa_ipv4_policy[IFA_MAX+1] = {
 	[IFA_FLAGS]		= { .type = NLA_U32 },
 	[IFA_RT_PRIORITY]	= { .type = NLA_U32 },
 	[IFA_TARGET_NETNSID]	= { .type = NLA_S32 },
+	[IFA_PROTO]		= { .type = NLA_U8 },
 };
 
 struct inet_fill_args {
@@ -887,6 +888,11 @@ static struct in_ifaddr *rtm_to_ifaddr(struct net *net, struct nlmsghdr *nlh,
 	if (tb[IFA_RT_PRIORITY])
 		ifa->ifa_rt_priority = nla_get_u32(tb[IFA_RT_PRIORITY]);
 
+	if (tb[IFA_PROTO])
+		ifa->ifa_proto = nla_get_u8(tb[IFA_PROTO]);
+	else
+		ifa->ifa_proto = IFAPROT_UNSPEC;
+
 	if (tb[IFA_CACHEINFO]) {
 		struct ifa_cacheinfo *ci;
 
@@ -1623,6 +1629,7 @@ static size_t inet_nlmsg_size(void)
 	       + nla_total_size(4) /* IFA_BROADCAST */
 	       + nla_total_size(IFNAMSIZ) /* IFA_LABEL */
 	       + nla_total_size(4)  /* IFA_FLAGS */
+	       + nla_total_size(1)  /* IFA_PROTO */
 	       + nla_total_size(4)  /* IFA_RT_PRIORITY */
 	       + nla_total_size(sizeof(struct ifa_cacheinfo)); /* IFA_CACHEINFO */
 }
@@ -1697,6 +1704,7 @@ static int inet_fill_ifaddr(struct sk_buff *skb, struct in_ifaddr *ifa,
 	     nla_put_in_addr(skb, IFA_BROADCAST, ifa->ifa_broadcast)) ||
 	    (ifa->ifa_label[0] &&
 	     nla_put_string(skb, IFA_LABEL, ifa->ifa_label)) ||
+	    nla_put_u8(skb, IFA_PROTO, ifa->ifa_proto) ||
 	    nla_put_u32(skb, IFA_FLAGS, ifa->ifa_flags) ||
 	    (ifa->ifa_rt_priority &&
 	     nla_put_u32(skb, IFA_RT_PRIORITY, ifa->ifa_rt_priority)) ||
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 846037e73723..a74a3e73fcbf 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1117,6 +1117,7 @@ ipv6_add_addr(struct inet6_dev *idev, struct ifa6_config *cfg,
 	ifa->prefix_len = cfg->plen;
 	ifa->rt_priority = cfg->rt_priority;
 	ifa->flags = cfg->ifa_flags;
+	ifa->ifa_proto = cfg->ifa_proto;
 	/* No need to add the TENTATIVE flag for addresses with NODAD */
 	if (!(cfg->ifa_flags & IFA_F_NODAD))
 		ifa->flags |= IFA_F_TENTATIVE;
@@ -4626,6 +4627,7 @@ static const struct nla_policy ifa_ipv6_policy[IFA_MAX+1] = {
 	[IFA_FLAGS]		= { .len = sizeof(u32) },
 	[IFA_RT_PRIORITY]	= { .len = sizeof(u32) },
 	[IFA_TARGET_NETNSID]	= { .type = NLA_S32 },
+	[IFA_PROTO]             = { .len = sizeof(u8) },
 };
 
 static int
@@ -4750,6 +4752,7 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, struct ifa6_config *cfg)
 	ifp->tstamp = jiffies;
 	ifp->valid_lft = cfg->valid_lft;
 	ifp->prefered_lft = cfg->preferred_lft;
+	ifp->ifa_proto = cfg->ifa_proto;
 
 	if (cfg->rt_priority && cfg->rt_priority != ifp->rt_priority)
 		ifp->rt_priority = cfg->rt_priority;
@@ -4843,6 +4846,11 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh,
 	if (tb[IFA_RT_PRIORITY])
 		cfg.rt_priority = nla_get_u32(tb[IFA_RT_PRIORITY]);
 
+	if (tb[IFA_PROTO])
+		cfg.ifa_proto = nla_get_u8(tb[IFA_PROTO]);
+	else
+		cfg.ifa_proto = IFAPROT_UNSPEC;
+
 	cfg.valid_lft = INFINITY_LIFE_TIME;
 	cfg.preferred_lft = INFINITY_LIFE_TIME;
 
@@ -4946,6 +4954,7 @@ static inline int inet6_ifaddr_msgsize(void)
 	       + nla_total_size(16) /* IFA_ADDRESS */
 	       + nla_total_size(sizeof(struct ifa_cacheinfo))
 	       + nla_total_size(4)  /* IFA_FLAGS */
+	       + nla_total_size(1)  /* IFA_PROTO */
 	       + nla_total_size(4)  /* IFA_RT_PRIORITY */;
 }
 
@@ -5023,6 +5032,9 @@ static int inet6_fill_ifaddr(struct sk_buff *skb, struct inet6_ifaddr *ifa,
 	if (nla_put_u32(skb, IFA_FLAGS, ifa->flags) < 0)
 		goto error;
 
+	if (nla_put_u8(skb, IFA_PROTO, ifa->ifa_proto))
+		goto error;
+
 	nlmsg_end(skb, nlh);
 	return 0;
 
-- 
2.33.0

Re: [PATCH net-next 1/1] net: Add new protocol attribute to IP addresses

From: Jakub Kicinski <kuba@kernel.org>
Date: 2022-02-04 03:08:05

On Thu, 3 Feb 2022 17:31:06 +0100 Jacques de Laval wrote:
quoted hunk
diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h
index 653e7d0f65cb..f7c270b24167 100644
--- a/include/net/if_inet6.h
+++ b/include/net/if_inet6.h
@@ -73,6 +73,8 @@ struct inet6_ifaddr {
 
 	struct rcu_head		rcu;
 	struct in6_addr		peer_addr;
+
+	__u8			ifa_proto;
nit: the __ types are for uAPI, you can use a normal u8 here.
quoted hunk
 };
 
 struct ip6_sf_socklist {
diff --git a/include/uapi/linux/if_addr.h b/include/uapi/linux/if_addr.h
index dfcf3ce0097f..2aa46b9c9961 100644
--- a/include/uapi/linux/if_addr.h
+++ b/include/uapi/linux/if_addr.h
quoted hunk
@@ -69,4 +70,7 @@ struct ifa_cacheinfo {
 #define IFA_PAYLOAD(n) NLMSG_PAYLOAD(n,sizeof(struct ifaddrmsg))
 #endif
 
+/* ifa_protocol */
+#define IFAPROT_UNSPEC	0
+
 #endif
What's the purpose of defining this as a constant?

Re: [PATCH net-next 1/1] net: Add new protocol attribute to IP addresses

From: David Ahern <hidden>
Date: 2022-02-04 04:16:54

On 2/3/22 9:31 AM, Jacques de Laval wrote:
quoted hunk
This patch adds a new protocol attribute to IPv4 and IPv6 addresses.
Inspiration was taken from the protocol attribute of routes. User space
applications like iproute2 can set/get the protocol with the Netlink API.

The attribute is stored as an 8-bit unsigned int. Only IFAPROT_UNSPEC is
defined. The rest of the available ids are available for user space to
define.

Grouping addresses on their origin is useful in scenarios where you want
to distinguish between addresses coming from a specific protocol like DHCP
and addresses that have been statically set.

Tagging addresses with a string label is an existing feature that could be
used as a solution. Unfortunately the max length of a label is
15 characters, and for compatibility reasons the label must be prefixed
with the name of the device followed by a colon. Since device names also
have a max length of 15 characters, only -1 characters is guaranteed to be
available for any origin tag, which is not that much.

A reference implementation of user space setting and getting protocols
is available for iproute2:

Link: https://github.com/westermo/iproute2/commit/9a6ea18bd79f47f293e5edc7780f315ea42ff540

Signed-off-by: Jacques de Laval <redacted>
---
 include/linux/inetdevice.h   |  1 +
 include/net/addrconf.h       |  1 +
 include/net/if_inet6.h       |  2 ++
 include/uapi/linux/if_addr.h |  4 ++++
 net/ipv4/devinet.c           |  8 ++++++++
 net/ipv6/addrconf.c          | 12 ++++++++++++
 6 files changed, 28 insertions(+)
diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
index a038feb63f23..caa6b7a5b5ac 100644
--- a/include/linux/inetdevice.h
+++ b/include/linux/inetdevice.h
@@ -148,6 +148,7 @@ struct in_ifaddr {
 	unsigned char		ifa_prefixlen;
 	__u32			ifa_flags;
 	char			ifa_label[IFNAMSIZ];
+	unsigned char		ifa_proto;
there is a hole after ifa_prefixlen where this can go and not affect
struct size.
quoted hunk
 
 	/* In seconds, relative to tstamp. Expiry is at tstamp + HZ * lft. */
 	__u32			ifa_valid_lft;
diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index 78ea3e332688..e53d8f4f4166 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -69,6 +69,7 @@ struct ifa6_config {
 	u32			preferred_lft;
 	u32			valid_lft;
 	u16			scope;
+	u8			ifa_proto;
 };
 
 int addrconf_init(void);
diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h
index 653e7d0f65cb..f7c270b24167 100644
--- a/include/net/if_inet6.h
+++ b/include/net/if_inet6.h
@@ -73,6 +73,8 @@ struct inet6_ifaddr {
 
 	struct rcu_head		rcu;
 	struct in6_addr		peer_addr;
+
+	__u8			ifa_proto;
similarly for this struct; couple of holes that you can put this.

quoted hunk
 };
 
 struct ip6_sf_socklist {
diff --git a/include/uapi/linux/if_addr.h b/include/uapi/linux/if_addr.h
index dfcf3ce0097f..2aa46b9c9961 100644
--- a/include/uapi/linux/if_addr.h
+++ b/include/uapi/linux/if_addr.h
@@ -35,6 +35,7 @@ enum {
 	IFA_FLAGS,
 	IFA_RT_PRIORITY,  /* u32, priority/metric for prefix route */
 	IFA_TARGET_NETNSID,
+	IFA_PROTO,
 	__IFA_MAX,
 };
 
@@ -69,4 +70,7 @@ struct ifa_cacheinfo {
 #define IFA_PAYLOAD(n) NLMSG_PAYLOAD(n,sizeof(struct ifaddrmsg))
 #endif
 
+/* ifa_protocol */
+#define IFAPROT_UNSPEC	0
*If* the value is just a passthrough (userspace to kernel and back), no
need for this uapi. However, have you considered builtin protocol labels
- e.g. for autoconf, LLA, etc. Kernel generated vs RAs vs userspace
adding it.

Re: [PATCH net-next 1/1] net: Add new protocol attribute to IP addresses

From: Jacques de Laval <hidden>
Date: 2022-02-04 18:08:39

On 2022-02-04 16:16 UTC, David Ahern wrote:
quoted
diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
index a038feb63f23..caa6b7a5b5ac 100644
--- a/include/linux/inetdevice.h
+++ b/include/linux/inetdevice.h
@@ -148,6 +148,7 @@ struct in_ifaddr {
 	unsigned char		ifa_prefixlen;
 	__u32			ifa_flags;
 	char			ifa_label[IFNAMSIZ];
+	unsigned char		ifa_proto;
there is a hole after ifa_prefixlen where this can go and not affect
struct size.
quoted
 
 	/* In seconds, relative to tstamp. Expiry is at tstamp + HZ * lft. */
 	__u32			ifa_valid_lft;
diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index 78ea3e332688..e53d8f4f4166 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -69,6 +69,7 @@ struct ifa6_config {
 	u32			preferred_lft;
 	u32			valid_lft;
 	u16			scope;
+	u8			ifa_proto;
 };
 
 int addrconf_init(void);
diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h
index 653e7d0f65cb..f7c270b24167 100644
--- a/include/net/if_inet6.h
+++ b/include/net/if_inet6.h
@@ -73,6 +73,8 @@ struct inet6_ifaddr {
 
 	struct rcu_head		rcu;
 	struct in6_addr		peer_addr;
+
+	__u8			ifa_proto;
similarly for this struct; couple of holes that you can put this.
Thank you! I will fix this in v2.
quoted
 };
 
 struct ip6_sf_socklist {
diff --git a/include/uapi/linux/if_addr.h b/include/uapi/linux/if_addr.h
index dfcf3ce0097f..2aa46b9c9961 100644
--- a/include/uapi/linux/if_addr.h
+++ b/include/uapi/linux/if_addr.h
@@ -35,6 +35,7 @@ enum {
 	IFA_FLAGS,
 	IFA_RT_PRIORITY,  /* u32, priority/metric for prefix route */
 	IFA_TARGET_NETNSID,
+	IFA_PROTO,
 	__IFA_MAX,
 };
 
@@ -69,4 +70,7 @@ struct ifa_cacheinfo {
 #define IFA_PAYLOAD(n) NLMSG_PAYLOAD(n,sizeof(struct ifaddrmsg))
 #endif
 
+/* ifa_protocol */
+#define IFAPROT_UNSPEC	0
*If* the value is just a passthrough (userspace to kernel and back), no
need for this uapi. However, have you considered builtin protocol labels
- e.g. for autoconf, LLA, etc. Kernel generated vs RAs vs userspace
adding it.
Agreed. For my own (very isolated) use case I only need the passthrough,
but I can see that it would make sense to standardize some labels.
I was trying to give this some thought but I have to admit I copped out
because of my limited knowledge on what labels would be reasonable to
reserve.

Based on what you mention, do you think the list bellow would make sense?

#define IFAPROT_UNSPEC		0  /* unspecified */
#define IFAPROT_KERNEL_LO	1  /* loopback */
#define IFAPROT_KERNEL_RA	2  /* auto assigned by kernel from router announcement */
#define IFAPROT_KERNEL_LL	3  /* link-local set by kernel */
#define IFAPROT_STATIC		4  /* set by admin */
#define IFAPROT_AUTO		5  /* DHCP, BOOTP etc. */
#define IFAPROT_LL		6  /* link-local set by userspace */

Or do you think it needs more granularity?

Re: [PATCH net-next 1/1] net: Add new protocol attribute to IP addresses

From: Jacques de Laval <hidden>
Date: 2022-02-04 18:40:50

On 2022-02-04 19:07 -0800, David Ahern wrote:
On Thu, 3 Feb 2022 17:31:06 +0100 Jacques de Laval wrote:
quoted
diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h
index 653e7d0f65cb..f7c270b24167 100644
--- a/include/net/if_inet6.h
+++ b/include/net/if_inet6.h
@@ -73,6 +73,8 @@ struct inet6_ifaddr {
 
 	struct rcu_head		rcu;
 	struct in6_addr		peer_addr;
+
+	__u8			ifa_proto;
nit: the __ types are for uAPI, you can use a normal u8 here.
Thanks you! I will fix this in v2.
quoted
 };
 
 struct ip6_sf_socklist {
diff --git a/include/uapi/linux/if_addr.h b/include/uapi/linux/if_addr.h
index dfcf3ce0097f..2aa46b9c9961 100644
--- a/include/uapi/linux/if_addr.h
+++ b/include/uapi/linux/if_addr.h
quoted
@@ -69,4 +70,7 @@ struct ifa_cacheinfo {
 #define IFA_PAYLOAD(n) NLMSG_PAYLOAD(n,sizeof(struct ifaddrmsg))
 #endif
 
+/* ifa_protocol */
+#define IFAPROT_UNSPEC	0
+
 #endif
What's the purpose of defining this as a constant?
Agreed, there's not much point if no other protocol labels are reserved.
If I can't come up with a list of labels that a reasonable to reserve I
should remove the constant in v2.

Re: [PATCH net-next 1/1] net: Add new protocol attribute to IP addresses

From: David Ahern <hidden>
Date: 2022-02-05 04:10:57

On 2/4/22 11:07 AM, Jacques de Laval wrote:
quoted
quoted
@@ -69,4 +70,7 @@ struct ifa_cacheinfo {
 #define IFA_PAYLOAD(n) NLMSG_PAYLOAD(n,sizeof(struct ifaddrmsg))
 #endif
 
+/* ifa_protocol */
+#define IFAPROT_UNSPEC	0
*If* the value is just a passthrough (userspace to kernel and back), no
need for this uapi. However, have you considered builtin protocol labels
- e.g. for autoconf, LLA, etc. Kernel generated vs RAs vs userspace
adding it.
Agreed. For my own (very isolated) use case I only need the passthrough,
but I can see that it would make sense to standardize some labels.
I was trying to give this some thought but I have to admit I copped out
because of my limited knowledge on what labels would be reasonable to
reserve.

Based on what you mention, do you think the list bellow would make sense?

#define IFAPROT_UNSPEC		0  /* unspecified */
#define IFAPROT_KERNEL_LO	1  /* loopback */
#define IFAPROT_KERNEL_RA	2  /* auto assigned by kernel from router announcement */
#define IFAPROT_KERNEL_LL	3  /* link-local set by kernel */
Those above look good to me.
#define IFAPROT_STATIC		4  /* set by admin */
#define IFAPROT_AUTO		5  /* DHCP, BOOTP etc. */
#define IFAPROT_LL		6  /* link-local set by userspace */

Or do you think it needs more granularity?
anything coming from userspace can just be a passthrough, so protocol
label is only set if it is an autonomous action by the kernel or some
app passed in a value.

Re: [PATCH net-next 1/1] net: Add new protocol attribute to IP addresses

From: Jacques de Laval <hidden>
Date: 2022-02-07 14:16:39

On Fri, Feb 04, 2022 at 09:10:51PM -0700, David Ahern wrote:
On 2/4/22 11:07 AM, Jacques de Laval wrote:
quoted
quoted
quoted
@@ -69,4 +70,7 @@ struct ifa_cacheinfo {
 #define IFA_PAYLOAD(n) NLMSG_PAYLOAD(n,sizeof(struct ifaddrmsg))
 #endif
 
+/* ifa_protocol */
+#define IFAPROT_UNSPEC	0
*If* the value is just a passthrough (userspace to kernel and back), no
need for this uapi. However, have you considered builtin protocol labels
- e.g. for autoconf, LLA, etc. Kernel generated vs RAs vs userspace
adding it.
Agreed. For my own (very isolated) use case I only need the passthrough,
but I can see that it would make sense to standardize some labels.
I was trying to give this some thought but I have to admit I copped out
because of my limited knowledge on what labels would be reasonable to
reserve.

Based on what you mention, do you think the list bellow would make sense?

#define IFAPROT_UNSPEC		0  /* unspecified */
#define IFAPROT_KERNEL_LO	1  /* loopback */
#define IFAPROT_KERNEL_RA	2  /* auto assigned by kernel from router announcement */
#define IFAPROT_KERNEL_LL	3  /* link-local set by kernel */
Those above look good to me.
quoted
#define IFAPROT_STATIC		4  /* set by admin */
#define IFAPROT_AUTO		5  /* DHCP, BOOTP etc. */
#define IFAPROT_LL		6  /* link-local set by userspace */

Or do you think it needs more granularity?
anything coming from userspace can just be a passthrough, so protocol
label is only set if it is an autonomous action by the kernel or some
app passed in a value.
Thanks David, that makes sense. Will include the first defines in v2
and try to set them when appropriate.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help