[PATCH net-next 0/5] net: Extend address label support

[PATCH net-next 0/5] net: Extend address label support

[Petr Machata]

IPv4 addresses can be tagged with label strings. Unlike IPv6 addrlabels,
which are used for prioritization of IPv6 addresses, these "ip address
labels" are simply tags that the userspace can assign to IP addresses
arbitrarily.

IPv4 has had support for these tags since before Linux was tracked in GIT.
However it has never been possible to change the label after it is once
defined. This limits usefulness of this feature. A userspace that wants to
change a label might drop and recreate the address, but that disrupts
routing and is just impractical.

IPv6 addresses lack support for address labels (in the sense of address
tags) altogether.

In this patchset, extend IPv4 to allow changing the label defined at an
address (in patch #1). Then, in patches #2 and #3, extend IPv6 with a suite
of address label operations fully analogous with those defined for IPv4.
Then in patches #4 and #5 add selftest coverage for the feature.

An example session with the feature in action:

	# ip address add dev d 2001:db8:1::1/64 label foo
	# ip address show dev d
	4: d: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc [...]
	    link/ether 06:29:74:fd:1f:eb brd ff:ff:ff:ff:ff:ff
	    inet6 2001:db8:1::1/64 scope global foo <--
	    valid_lft forever preferred_lft forever
	    inet6 fe80::429:74ff:fefd:1feb/64 scope link d
	    valid_lft forever preferred_lft forever

	# ip address replace dev d 2001:db8:1::1/64 label bar
	# ip address show dev d
	4: d: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc [...]
	    link/ether 06:29:74:fd:1f:eb brd ff:ff:ff:ff:ff:ff
	    inet6 2001:db8:1::1/64 scope global bar <--
	    valid_lft forever preferred_lft forever
	    inet6 fe80::429:74ff:fefd:1feb/64 scope link d
	    valid_lft forever preferred_lft forever

	# ip address del dev d 2001:db8:1::1/64 label foo
	RTNETLINK answers: Cannot assign requested address
	# ip address del dev d 2001:db8:1::1/64 label bar

Petr Machata (5):
  net: ipv4: Allow changing IPv4 labels
  net: ipv6: addrconf: Support IPv6 address labels
  net: ipv6: addrconf: Expose IPv6 address labels through netlink
  selftests: rtnetlink: Make the set of tests to run configurable
  selftests: rtnetlink: Add an address label test

 include/net/addrconf.h                   |   2 +
 include/net/if_inet6.h                   |   1 +
 net/ipv4/devinet.c                       |  10 +-
 net/ipv6/addrconf.c                      |  30 +++-
 tools/testing/selftests/net/rtnetlink.sh | 172 +++++++++++++++++------
 5 files changed, 169 insertions(+), 46 deletions(-)

-- 
2.39.0

[PATCH net-next 1/5] net: ipv4: Allow changing IPv4 labels

[Petr Machata]

IPv4 addresses can be tagged with label strings. Unlike IPv6 addrlabels,
which are used for prioritization of IPv6 addresses, these "ip address
labels" are simply tags that the userspace can assign to IP addresses
arbitrarily.

IPv4 has had support for these tags since before Linux was tracked in GIT.
However it has never been possible to change the label after it is once
defined. This limits usefulness of this feature. A userspace that wants to
change a label might drop and recreate the address, but that disrupts
routing and is just impractical.

In this patch, when an address is replaced (through RTM_NEWADDR request
with NLM_F_REPLACE flag) and a label (IFA_LABEL) is given, update the label
at the address to the one given in the request. So far the new value was
simply ignored.

Signed-off-by: Petr Machata <petrm@nvidia.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
 net/ipv4/devinet.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index b0acf6e19aed..dfb6d20ada9a 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -823,6 +823,7 @@ static void set_ifa_lifetime(struct in_ifaddr *ifa, __u32 valid_lft,
 
 static struct in_ifaddr *rtm_to_ifaddr(struct net *net, struct nlmsghdr *nlh,
 				       __u32 *pvalid_lft, __u32 *pprefered_lft,
+				       const struct nlattr **at_label,
 				       struct netlink_ext_ack *extack)
 {
 	struct nlattr *tb[IFA_MAX+1];
@@ -885,6 +886,8 @@ static struct in_ifaddr *rtm_to_ifaddr(struct net *net, struct nlmsghdr *nlh,
 		nla_strscpy(ifa->ifa_label, tb[IFA_LABEL], IFNAMSIZ);
 	else
 		memcpy(ifa->ifa_label, dev->name, IFNAMSIZ);
+	if (at_label)
+		*at_label = tb[IFA_LABEL];
 
 	if (tb[IFA_RT_PRIORITY])
 		ifa->ifa_rt_priority = nla_get_u32(tb[IFA_RT_PRIORITY]);
@@ -933,6 +936,7 @@ static int inet_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh,
 			    struct netlink_ext_ack *extack)
 {
 	struct net *net = sock_net(skb->sk);
+	const struct nlattr *at_label;
 	struct in_ifaddr *ifa;
 	struct in_ifaddr *ifa_existing;
 	__u32 valid_lft = INFINITY_LIFE_TIME;
@@ -940,7 +944,8 @@ static int inet_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh,
 
 	ASSERT_RTNL();
 
-	ifa = rtm_to_ifaddr(net, nlh, &valid_lft, &prefered_lft, extack);
+	ifa = rtm_to_ifaddr(net, nlh, &valid_lft, &prefered_lft, &at_label,
+			    extack);
 	if (IS_ERR(ifa))
 		return PTR_ERR(ifa);
 
@@ -975,6 +980,9 @@ static int inet_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh,
 			ifa->ifa_rt_priority = new_metric;
 		}
 
+		if (at_label)
+			nla_strscpy(ifa->ifa_label, at_label, IFNAMSIZ);
+
 		set_ifa_lifetime(ifa, valid_lft, prefered_lft);
 		cancel_delayed_work(&check_lifetime_work);
 		queue_delayed_work(system_power_efficient_wq,
-- 
2.39.0

[PATCH net-next 2/5] net: ipv6: addrconf: Support IPv6 address labels

[Petr Machata]

IPv4 addresses can be tagged with label strings. Unlike IPv6 addrlabels,
which are used for prioritization of IPv6 addresses, these "ip address
labels" are simply tags that the userspace can assign to IP addresses
arbitrarily.

In this patch, add to IPv6 support for address labels. This adds the
necessary fields, but does not expose them through UAPI yet -- that will
happen in the next patch.

When not given, the default address label is the same as the netdevice
name. In IPv4, this behavior is due to Linux 2.0 compatibility. While this
is probably not a concern anymore, having the same behavior in IPv6 as we
have in IPv4 will make the feature easier to understand.

Also similarly to IPv4, when address label is part of a request to remove
an address, the label given has to match the label at the address, or else
the address is not removed.

Signed-off-by: Petr Machata <petrm@nvidia.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
 include/net/addrconf.h |  2 ++
 include/net/if_inet6.h |  1 +
 net/ipv6/addrconf.c    | 14 ++++++++++++--
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index c04f359655b8..52214a46cc7e 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -73,6 +73,8 @@ struct ifa6_config {
 	u32			preferred_lft;
 	u32			valid_lft;
 	u16			scope;
+	char			ifa_label[IFNAMSIZ];
+	bool			has_ifa_label;
 };
 
 int addrconf_init(void);
diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h
index c8490729b4ae..2cfb2ac1d1f7 100644
--- a/include/net/if_inet6.h
+++ b/include/net/if_inet6.h
@@ -83,6 +83,7 @@ struct inet6_ifaddr {
 
 	struct rcu_head		rcu;
 	struct in6_addr		peer_addr;
+	char			ifa_label[IFNAMSIZ];
 };
 
 struct ip6_sf_socklist {
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index faa47f9ea73a..5f4f16bb6ef0 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1134,6 +1134,11 @@ ipv6_add_addr(struct inet6_dev *idev, struct ifa6_config *cfg,
 	ifa->rt = f6i;
 
 	ifa->idev = idev;
+	if (cfg->has_ifa_label)
+		memcpy(ifa->ifa_label, cfg->ifa_label, sizeof(cfg->ifa_label));
+	else
+		memcpy(ifa->ifa_label, idev->dev->name, IFNAMSIZ);
+
 	in6_dev_hold(idev);
 
 	/* For caller */
@@ -3000,6 +3005,7 @@ static int inet6_addr_add(struct net *net, int ifindex,
 }
 
 static int inet6_addr_del(struct net *net, int ifindex, u32 ifa_flags,
+			  const char *ifa_label,
 			  const struct in6_addr *pfx, unsigned int plen)
 {
 	struct inet6_ifaddr *ifp;
@@ -3019,6 +3025,8 @@ static int inet6_addr_del(struct net *net, int ifindex, u32 ifa_flags,
 
 	read_lock_bh(&idev->lock);
 	list_for_each_entry(ifp, &idev->addr_list, if_list) {
+		if (ifa_label && strcmp(ifp->ifa_label, ifa_label))
+			continue;
 		if (ifp->prefix_len == plen &&
 		    ipv6_addr_equal(pfx, &ifp->addr)) {
 			in6_ifa_hold(ifp);
@@ -3079,7 +3087,7 @@ int addrconf_del_ifaddr(struct net *net, void __user *arg)
 		return -EFAULT;
 
 	rtnl_lock();
-	err = inet6_addr_del(net, ireq.ifr6_ifindex, 0, &ireq.ifr6_addr,
+	err = inet6_addr_del(net, ireq.ifr6_ifindex, 0, NULL, &ireq.ifr6_addr,
 			     ireq.ifr6_prefixlen);
 	rtnl_unlock();
 	return err;
@@ -4691,7 +4699,7 @@ inet6_rtm_deladdr(struct sk_buff *skb, struct nlmsghdr *nlh,
 	/* We ignore other flags so far. */
 	ifa_flags &= IFA_F_MANAGETEMPADDR;
 
-	return inet6_addr_del(net, ifm->ifa_index, ifa_flags, pfx,
+	return inet6_addr_del(net, ifm->ifa_index, ifa_flags, NULL, pfx,
 			      ifm->ifa_prefixlen);
 }
 
@@ -4792,6 +4800,8 @@ static int inet6_addr_modify(struct net *net, struct inet6_ifaddr *ifp,
 
 	if (cfg->rt_priority && cfg->rt_priority != ifp->rt_priority)
 		ifp->rt_priority = cfg->rt_priority;
+	if (cfg->has_ifa_label)
+		memcpy(ifp->ifa_label, cfg->ifa_label, IFNAMSIZ);
 
 	if (new_peer)
 		ifp->peer_addr = *cfg->peer_pfx;
-- 
2.39.0

[PATCH net-next 3/5] net: ipv6: addrconf: Expose IPv6 address labels through netlink

[Petr Machata]

Support for IPv6 address labels, arbitrary userspace tags associated with
IPv6 addresses, was added in the previous patch. In this patch, expose the
feature through netlink to permit userspace to configure and query address
labels.

An example session with the feature in action:

	# ip address add dev d 2001:db8:1::1/64 label foo
	# ip address show dev d
	4: d: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc [...]
	    link/ether 06:29:74:fd:1f:eb brd ff:ff:ff:ff:ff:ff
	    inet6 2001:db8:1::1/64 scope global foo <--
	    valid_lft forever preferred_lft forever
	    inet6 fe80::429:74ff:fefd:1feb/64 scope link d
	    valid_lft forever preferred_lft forever

	# ip address replace dev d 2001:db8:1::1/64 label bar
	# ip address show dev d
	4: d: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc [...]
	    link/ether 06:29:74:fd:1f:eb brd ff:ff:ff:ff:ff:ff
	    inet6 2001:db8:1::1/64 scope global bar <--
	    valid_lft forever preferred_lft forever
	    inet6 fe80::429:74ff:fefd:1feb/64 scope link d
	    valid_lft forever preferred_lft forever

	# ip address del dev d 2001:db8:1::1/64 label foo
	RTNETLINK answers: Cannot assign requested address
	# ip address del dev d 2001:db8:1::1/64 label bar

Signed-off-by: Petr Machata <petrm@nvidia.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
 net/ipv6/addrconf.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 5f4f16bb6ef0..edd1d08eeadb 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -4671,6 +4671,7 @@ static const struct nla_policy ifa_ipv6_policy[IFA_MAX+1] = {
 	[IFA_RT_PRIORITY]	= { .len = sizeof(u32) },
 	[IFA_TARGET_NETNSID]	= { .type = NLA_S32 },
 	[IFA_PROTO]		= { .type = NLA_U8 },
+	[IFA_LABEL]		= { .type = NLA_STRING, .len = IFNAMSIZ - 1 },
 };
 
 static int
@@ -4681,6 +4682,7 @@ inet6_rtm_deladdr(struct sk_buff *skb, struct nlmsghdr *nlh,
 	struct ifaddrmsg *ifm;
 	struct nlattr *tb[IFA_MAX+1];
 	struct in6_addr *pfx, *peer_pfx;
+	const char *ifa_label;
 	u32 ifa_flags;
 	int err;
 
@@ -4695,11 +4697,12 @@ inet6_rtm_deladdr(struct sk_buff *skb, struct nlmsghdr *nlh,
 		return -EINVAL;
 
 	ifa_flags = tb[IFA_FLAGS] ? nla_get_u32(tb[IFA_FLAGS]) : ifm->ifa_flags;
+	ifa_label = tb[IFA_LABEL] ? nla_data(tb[IFA_LABEL]) : NULL;
 
 	/* We ignore other flags so far. */
 	ifa_flags &= IFA_F_MANAGETEMPADDR;
 
-	return inet6_addr_del(net, ifm->ifa_index, ifa_flags, NULL, pfx,
+	return inet6_addr_del(net, ifm->ifa_index, ifa_flags, ifa_label, pfx,
 			      ifm->ifa_prefixlen);
 }
 
@@ -4915,6 +4918,11 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh,
 	else
 		cfg.ifa_flags = ifm->ifa_flags;
 
+	if (tb[IFA_LABEL]) {
+		nla_strscpy(cfg.ifa_label, tb[IFA_LABEL], IFNAMSIZ);
+		cfg.has_ifa_label = true;
+	}
+
 	/* We ignore other flags so far. */
 	cfg.ifa_flags &= IFA_F_NODAD | IFA_F_HOMEADDRESS |
 			 IFA_F_MANAGETEMPADDR | IFA_F_NOPREFIXROUTE |
@@ -4999,7 +5007,9 @@ static inline int inet6_ifaddr_msgsize(void)
 	       + 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 */;
+	       + nla_total_size(4)  /* IFA_RT_PRIORITY */
+	       + nla_total_size(IFNAMSIZ) /* IFA_LABEL */
+	       ;
 }
 
 enum addr_type_t {
@@ -5082,6 +5092,10 @@ static int inet6_fill_ifaddr(struct sk_buff *skb, struct inet6_ifaddr *ifa,
 	    nla_put_u8(skb, IFA_PROTO, ifa->ifa_proto))
 		goto error;
 
+	if (ifa->ifa_label[0] &&
+	    nla_put_string(skb, IFA_LABEL, ifa->ifa_label))
+		goto error;
+
 	nlmsg_end(skb, nlh);
 	return 0;
 
-- 
2.39.0

[PATCH net-next 4/5] selftests: rtnetlink: Make the set of tests to run configurable

[Petr Machata]

Extract the list of all tests into a variable, ALL_TESTS. Then assume the
environment variable TESTS holds the list of tests to actually run, falling
back to ALL_TESTS if TESTS is empty. This is the same interface that
forwarding selftests use to make the set of tests to run configurable.
In addition to this, allow setting the value explicitly through a command
line option "-t" along the lines of what fib_nexthops.sh does.

Signed-off-by: Petr Machata <petrm@nvidia.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
 tools/testing/selftests/net/rtnetlink.sh | 90 +++++++++++++-----------
 1 file changed, 48 insertions(+), 42 deletions(-)

diff --git a/tools/testing/selftests/net/rtnetlink.sh b/tools/testing/selftests/net/rtnetlink.sh
index 275491be3da2..12caf9602353 100755
--- a/tools/testing/selftests/net/rtnetlink.sh
+++ b/tools/testing/selftests/net/rtnetlink.sh
@@ -4,6 +4,30 @@
 #
 # set -e
 
+ALL_TESTS="
+	kci_test_polrouting
+	kci_test_route_get
+	kci_test_addrlft
+	kci_test_promote_secondaries
+	kci_test_tc
+	kci_test_gre
+	kci_test_gretap
+	kci_test_ip6gretap
+	kci_test_erspan
+	kci_test_ip6erspan
+	kci_test_bridge
+	kci_test_addrlabel
+	kci_test_ifalias
+	kci_test_vrf
+	kci_test_encap
+	kci_test_macsec
+	kci_test_ipsec
+	kci_test_ipsec_offload
+	kci_test_fdb_get
+	kci_test_neigh_get
+	kci_test_bridge_parent_id
+"
+
 devdummy="test-dummy0"
 
 # Kselftest framework requirement - SKIP code is 4.
@@ -1227,60 +1251,34 @@ kci_test_bridge_parent_id()
 
 kci_test_rtnl()
 {
+	local current_test
 	local ret=0
+
 	kci_add_dummy
 	if [ $ret -ne 0 ];then
 		echo "FAIL: cannot add dummy interface"
 		return 1
 	fi
 
-	kci_test_polrouting
-	check_err $?
-	kci_test_route_get
-	check_err $?
-	kci_test_addrlft
-	check_err $?
-	kci_test_promote_secondaries
-	check_err $?
-	kci_test_tc
-	check_err $?
-	kci_test_gre
-	check_err $?
-	kci_test_gretap
-	check_err $?
-	kci_test_ip6gretap
-	check_err $?
-	kci_test_erspan
-	check_err $?
-	kci_test_ip6erspan
-	check_err $?
-	kci_test_bridge
-	check_err $?
-	kci_test_addrlabel
-	check_err $?
-	kci_test_ifalias
-	check_err $?
-	kci_test_vrf
-	check_err $?
-	kci_test_encap
-	check_err $?
-	kci_test_macsec
-	check_err $?
-	kci_test_ipsec
-	check_err $?
-	kci_test_ipsec_offload
-	check_err $?
-	kci_test_fdb_get
-	check_err $?
-	kci_test_neigh_get
-	check_err $?
-	kci_test_bridge_parent_id
-	check_err $?
+	for current_test in ${TESTS:-$ALL_TESTS}; do
+		$current_test
+		check_err $?
+	done
 
 	kci_del_dummy
 	return $ret
 }
 
+usage()
+{
+	cat <<EOF
+usage: ${0##*/} OPTS
+
+        -t <test>   Test(s) to run (default: all)
+                    (options: $(echo $ALL_TESTS))
+EOF
+}
+
 #check for needed privileges
 if [ "$(id -u)" -ne 0 ];then
 	echo "SKIP: Need root privileges"
@@ -1295,6 +1293,14 @@ for x in ip tc;do
 	fi
 done
 
+while getopts t:h o; do
+	case $o in
+		t) TESTS=$OPTARG;;
+		h) usage; exit 0;;
+		*) usage; exit 1;;
+	esac
+done
+
 kci_test_rtnl
 
 exit $?
-- 
2.39.0

[PATCH net-next 5/5] selftests: rtnetlink: Add an address label test

[Petr Machata]

Add coverage of "ip address {add,replace} ... label" support.

Signed-off-by: Petr Machata <petrm@nvidia.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
 tools/testing/selftests/net/rtnetlink.sh | 82 ++++++++++++++++++++++++
 1 file changed, 82 insertions(+)

diff --git a/tools/testing/selftests/net/rtnetlink.sh b/tools/testing/selftests/net/rtnetlink.sh
index 12caf9602353..3f62b202746d 100755
--- a/tools/testing/selftests/net/rtnetlink.sh
+++ b/tools/testing/selftests/net/rtnetlink.sh
@@ -1249,6 +1249,88 @@ kci_test_bridge_parent_id()
 	echo "PASS: bridge_parent_id"
 }
 
+address_get_label()
+{
+	local addr=$1; shift
+
+	ip -j address show dev "$devdummy" |
+	    jq -e -r --arg addr "${addr%/*}" \
+	       '.[].addr_info[] | select(.local == $addr) | .label'
+}
+
+do_test_address_label()
+{
+	local what=$1; shift
+	local addr=$1; shift
+	local label
+	local ret=0
+	local msg
+	local err
+
+	# Test adding an address with a pre-set label.
+	msg=$(ip address add dev "$devdummy" "$addr" label foo 2>&1)
+	err=$?
+	if [[ "$err" -ne 0 && "${msg/be prefixed/}" != "${msg}" ]]; then
+		echo "SKIP: ip does not support arbitrary ip address labels."
+		return $ksft_skip
+	fi
+	check_err $err
+	label=$(address_get_label "$addr")
+	check_err $?
+	[[ "$label" == "foo" ]]
+	check_err $?
+
+	# When deleting an address, if label is given at all, it must match
+	# the label at the deleted address.
+	ip address del dev "$devdummy" "$addr" label bar 2>/dev/null
+	check_fail $?
+	ip address del dev "$devdummy" "$addr" label foo
+	check_err $?
+
+	# When no label is given, it defaults to the name of the netdevice.
+	ip address add dev "$devdummy" "$addr"
+	label=$(address_get_label "$addr")
+	check_err $?
+	[[ "$label" == "$devdummy" ]]
+	check_err $?
+
+	# Setting a label to empty string effectively deletes it -- it is
+	# not reported through netlink.
+	ip address replace dev "$devdummy" "$addr" label ''
+	label=$(address_get_label "$addr")
+	check_fail $?
+
+	# It is possible to set label explicitly.
+	ip address replace dev "$devdummy" "$addr" label foo
+	label=$(address_get_label "$addr")
+	check_err $?
+	[[ "$label" == "foo" ]]
+	check_err $?
+
+	# An address with a label can be deleted without giving a label.
+	ip address del dev "$devdummy" "$addr"
+	check_err $?
+
+	if [ $ret -ne 0 ]; then
+		echo "FAIL: address label $what"
+		return 1
+	fi
+	echo "PASS: address label $what"
+}
+
+kci_test_address_label()
+{
+	local ret=0
+
+	do_test_address_label IPv4 192.0.2.1/28
+	check_err $?
+
+	do_test_address_label IPv6 2001:db8:1::1/64
+	check_err $?
+
+	return $ret
+}
+
 kci_test_rtnl()
 {
 	local current_test
-- 
2.39.0

Re: [PATCH net-next 0/5] net: Extend address label support

[Jakub Kicinski]

On Fri, 10 Mar 2023 12:44:53 +0100 Petr Machata wrote:
> IPv4 addresses can be tagged with label strings. Unlike IPv6 addrlabels,
> which are used for prioritization of IPv6 addresses, these "ip address
> labels" are simply tags that the userspace can assign to IP addresses
> arbitrarily.
> 
> IPv4 has had support for these tags since before Linux was tracked in GIT.
> However it has never been possible to change the label after it is once
> defined. This limits usefulness of this feature. A userspace that wants to
> change a label might drop and recreate the address, but that disrupts
> routing and is just impractical.
> 
> IPv6 addresses lack support for address labels (in the sense of address
> tags) altogether.
> 
> In this patchset, extend IPv4 to allow changing the label defined at an
> address (in patch #1). Then, in patches #2 and #3, extend IPv6 with a suite
> of address label operations fully analogous with those defined for IPv4.
> Then in patches #4 and #5 add selftest coverage for the feature.

Feels a bit like we're missing motivation for this change.
I thought address labels were legacy cruft.
Also the usual concern about allowing to change things is that some
user space will assume it's immutable. The label could until this 
set be used as part of a stable key, right?

Re: [PATCH net-next 0/5] net: Extend address label support

[Petr Machata]

Jakub Kicinski <kuba@kernel.org> writes:

> On Fri, 10 Mar 2023 12:44:53 +0100 Petr Machata wrote:
>> IPv4 addresses can be tagged with label strings. Unlike IPv6 addrlabels,
>> which are used for prioritization of IPv6 addresses, these "ip address
>> labels" are simply tags that the userspace can assign to IP addresses
>> arbitrarily.
>> 
>> IPv4 has had support for these tags since before Linux was tracked in GIT.
>> However it has never been possible to change the label after it is once
>> defined. This limits usefulness of this feature. A userspace that wants to
>> change a label might drop and recreate the address, but that disrupts
>> routing and is just impractical.
>> 
>> IPv6 addresses lack support for address labels (in the sense of address
>> tags) altogether.
>> 
>> In this patchset, extend IPv4 to allow changing the label defined at an
>> address (in patch #1). Then, in patches #2 and #3, extend IPv6 with a suite
>> of address label operations fully analogous with those defined for IPv4.
>> Then in patches #4 and #5 add selftest coverage for the feature.
>
> Feels a bit like we're missing motivation for this change.
> I thought address labels were legacy cruft.

The immutability and lack of IPv6 support is seriously limiting, so the
fact nobody is using this is not that surprising.

> Also the usual concern about allowing to change things is that some
> user space will assume it's immutable. The label could until this 
> set be used as part of a stable key, right?

Maybe. But to change a label, you need to be an admin, so yeah, you can
screw things up if you want to. You could e.g. delete the address
outright. In the end it should be on me as an admin to run a stack that
is not stumbling over itself.

As for the motivation: the use case we are eying in particular is
advertisement of MLAG anycast addresses. One label would be used to mark
anycast addresses if they shouldn't be advertised by the routing stack
yet, a different label for those that can be advertised. Which labels
mean what would be a protocol between the two daemons involved.

Other userspace stacks might use this to their own ends to annotate sets
of addresses according to their needs. Like they can today, if the sets
only involve IPv4 addresses that never migrate from set to set :)

Re: [PATCH net-next 0/5] net: Extend address label support

[Jakub Kicinski]

On Mon, 13 Mar 2023 14:26:56 +0100 Petr Machata wrote:
> > Feels a bit like we're missing motivation for this change.
> > I thought address labels were legacy cruft.  
> 
> The immutability and lack of IPv6 support is seriously limiting, so the
> fact nobody is using this is not that surprising.
> 
> > Also the usual concern about allowing to change things is that some
> > user space will assume it's immutable. The label could until this 
> > set be used as part of a stable key, right?  
> 
> Maybe. But to change a label, you need to be an admin, so yeah, you can
> screw things up if you want to. You could e.g. delete the address
> outright. In the end it should be on me as an admin to run a stack that
> is not stumbling over itself.

I haven't seen that caveat under the "no uAPI-visible regressions"
rule book...  Have you done a github grep for uses of this attr?
I'm guessing that indeed nobody will notice.

> As for the motivation: the use case we are eying in particular is
> advertisement of MLAG anycast addresses. One label would be used to mark
> anycast addresses if they shouldn't be advertised by the routing stack
> yet, a different label for those that can be advertised. Which labels
> mean what would be a protocol between the two daemons involved.

Hm. I see.

> Other userspace stacks might use this to their own ends to annotate sets
> of addresses according to their needs. Like they can today, if the
> sets only involve IPv4 addresses that never migrate from set to set :)

I suspect we may have skipped the feature in v6 for two reasons 
(1) it had no modern use and (2) address label in IPv6 means
the precedence value in address selection, doesn't it?

Re: [PATCH net-next 0/5] net: Extend address label support

[Petr Machata]

Jakub Kicinski <kuba@kernel.org> writes:

> On Mon, 13 Mar 2023 14:26:56 +0100 Petr Machata wrote:
>> > Feels a bit like we're missing motivation for this change.
>> > I thought address labels were legacy cruft.  
>> 
>> The immutability and lack of IPv6 support is seriously limiting, so the
>> fact nobody is using this is not that surprising.
>> 
>> > Also the usual concern about allowing to change things is that some
>> > user space will assume it's immutable. The label could until this 
>> > set be used as part of a stable key, right?  
>> 
>> Maybe. But to change a label, you need to be an admin, so yeah, you can
>> screw things up if you want to. You could e.g. delete the address
>> outright. In the end it should be on me as an admin to run a stack that
>> is not stumbling over itself.
>
> I haven't seen that caveat under the "no uAPI-visible regressions"
> rule book...  Have you done a github grep for uses of this attr?

I didn't realize this before, but the labels do change today as a result
of interface renames. That's... not good. One thing is an admin coming
along and changing a label, which yeah, would change a label. A change
in netdevice name screwing up all the labels is a whole different level.
I guess whatever the original use case for labels was leaks through too
much at this point.

So scratch all this.

I think we will have to use address protocol to do this. IPv6 protocol
already supports replace semantics. Any objections to adding the same
for IPv4?

Like with the labels, address replacement messages with an explicit
IFA_PROTO are not bounced, they just neglect to actually change the
protocol. But it makes no sense to me that someone would issue address
replacement with an explicit proto set which differs from the current
one, but would still rely on the fact that the proto doesn't change...

Re: [PATCH net-next 0/5] net: Extend address label support

[Ido Schimmel]

On Tue, Mar 14, 2023 at 10:44:00AM +0100, Petr Machata wrote:
> Like with the labels, address replacement messages with an explicit
> IFA_PROTO are not bounced, they just neglect to actually change the
> protocol. But it makes no sense to me that someone would issue address
> replacement with an explicit proto set which differs from the current
> one, but would still rely on the fact that the proto doesn't change...

Especially when replace does work with IPv6 addresses. Couple that with
the fact that it's a much newer attribute than the labels (added in
5.18) and that it has no support in iproute2, FRR, libnl etc, the
chances of such a change breaking anyone are slim to none...

Re: [PATCH net-next 0/5] net: Extend address label support

[Jakub Kicinski]

On Tue, 14 Mar 2023 19:43:06 +0200 Ido Schimmel wrote:
> On Tue, Mar 14, 2023 at 10:44:00AM +0100, Petr Machata wrote:
> > Like with the labels, address replacement messages with an explicit
> > IFA_PROTO are not bounced, they just neglect to actually change the
> > protocol. But it makes no sense to me that someone would issue address
> > replacement with an explicit proto set which differs from the current
> > one, but would still rely on the fact that the proto doesn't change...  
> 
> Especially when replace does work with IPv6 addresses. Couple that with
> the fact that it's a much newer attribute than the labels (added in
> 5.18) and that it has no support in iproute2, FRR, libnl etc, the
> chances of such a change breaking anyone are slim to none...

Let's add Jacques, in case he knows something we don't know.

Yes, that sounds fairly safe, we can risk it. Then again we may be
putting different pieces of state into one field? There are holes 
next to ifa_proto in most (all?) structures. It wouldn't cost 
us too much to add a field for your exact use case, it seems.
But no strong feelings, ifa_proto > 3 is a free-for-all, anyway.

Re: [PATCH net-next 0/5] net: Extend address label support

[Stephen Hemminger]

On Fri, 10 Mar 2023 12:44:53 +0100
Petr Machata <petrm@nvidia.com> wrote:

> IPv4 addresses can be tagged with label strings. Unlike IPv6 addrlabels,
> which are used for prioritization of IPv6 addresses, these "ip address
> labels" are simply tags that the userspace can assign to IP addresses
> arbitrarily.
> 
> IPv4 has had support for these tags since before Linux was tracked in GIT.
> However it has never been possible to change the label after it is once
> defined. This limits usefulness of this feature. A userspace that wants to
> change a label might drop and recreate the address, but that disrupts
> routing and is just impractical.
> 
> IPv6 addresses lack support for address labels (in the sense of address
> tags) altogether.
> 
> In this patchset, extend IPv4 to allow changing the label defined at an
> address (in patch #1). Then, in patches #2 and #3, extend IPv6 with a suite
> of address label operations fully analogous with those defined for IPv4.
> Then in patches #4 and #5 add selftest coverage for the feature.
> 
> An example session with the feature in action:
> 
> 	# ip address add dev d 2001:db8:1::1/64 label foo
> 	# ip address show dev d
> 	4: d: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc [...]
> 	    link/ether 06:29:74:fd:1f:eb brd ff:ff:ff:ff:ff:ff
> 	    inet6 2001:db8:1::1/64 scope global foo <--
> 	    valid_lft forever preferred_lft forever
> 	    inet6 fe80::429:74ff:fefd:1feb/64 scope link d
> 	    valid_lft forever preferred_lft forever
> 
> 	# ip address replace dev d 2001:db8:1::1/64 label bar
> 	# ip address show dev d
> 	4: d: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc [...]
> 	    link/ether 06:29:74:fd:1f:eb brd ff:ff:ff:ff:ff:ff
> 	    inet6 2001:db8:1::1/64 scope global bar <--
> 	    valid_lft forever preferred_lft forever
> 	    inet6 fe80::429:74ff:fefd:1feb/64 scope link d
> 	    valid_lft forever preferred_lft forever
> 
> 	# ip address del dev d 2001:db8:1::1/64 label foo
> 	RTNETLINK answers: Cannot assign requested address
> 	# ip address del dev d 2001:db8:1::1/64 label bar

This would add a lot of naming confusion with existing IPv6 address labels.
And MPLS labels.

See man ip-addrlabel for more info.  Can't think of better term for this.
Tag would raise conflicts with vlan/vxlan tag term.
Name would be confusing vs DNS naming.

Also, most of the real world manages addresses through automated services so
doing it with ip address isn't going to help.

Re: [PATCH net-next 0/5] net: Extend address label support

[Petr Machata]

Stephen Hemminger <stephen@networkplumber.org> writes:

> On Fri, 10 Mar 2023 12:44:53 +0100
> Petr Machata <petrm@nvidia.com> wrote:
>
>> IPv4 addresses can be tagged with label strings. Unlike IPv6 addrlabels,
>> which are used for prioritization of IPv6 addresses, these "ip address
>> labels" are simply tags that the userspace can assign to IP addresses
>> arbitrarily.
>> 
>> IPv4 has had support for these tags since before Linux was tracked in GIT.
>> However it has never been possible to change the label after it is once
>> defined. This limits usefulness of this feature. A userspace that wants to
>> change a label might drop and recreate the address, but that disrupts
>> routing and is just impractical.
>> 
>> IPv6 addresses lack support for address labels (in the sense of address
>> tags) altogether.
>> 
>> In this patchset, extend IPv4 to allow changing the label defined at an
>> address (in patch #1). Then, in patches #2 and #3, extend IPv6 with a suite
>> of address label operations fully analogous with those defined for IPv4.
>> Then in patches #4 and #5 add selftest coverage for the feature.
>> 
>> An example session with the feature in action:
>> 
>> 	# ip address add dev d 2001:db8:1::1/64 label foo
>> 	# ip address show dev d
>> 	4: d: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc [...]
>> 	    link/ether 06:29:74:fd:1f:eb brd ff:ff:ff:ff:ff:ff
>> 	    inet6 2001:db8:1::1/64 scope global foo <--
>> 	    valid_lft forever preferred_lft forever
>> 	    inet6 fe80::429:74ff:fefd:1feb/64 scope link d
>> 	    valid_lft forever preferred_lft forever
>> 
>> 	# ip address replace dev d 2001:db8:1::1/64 label bar
>> 	# ip address show dev d
>> 	4: d: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc [...]
>> 	    link/ether 06:29:74:fd:1f:eb brd ff:ff:ff:ff:ff:ff
>> 	    inet6 2001:db8:1::1/64 scope global bar <--
>> 	    valid_lft forever preferred_lft forever
>> 	    inet6 fe80::429:74ff:fefd:1feb/64 scope link d
>> 	    valid_lft forever preferred_lft forever
>> 
>> 	# ip address del dev d 2001:db8:1::1/64 label foo
>> 	RTNETLINK answers: Cannot assign requested address
>> 	# ip address del dev d 2001:db8:1::1/64 label bar
>
> This would add a lot of naming confusion with existing IPv6 address labels.
> And MPLS labels.

The confusion, if any, already exists. ip-address just describes address
labels without any regard to ip-addrlabel, and vice versa. I can
actually add verbiage here to make it clear that these are separate
concepts. Likewise for UAPI headers.

> See man ip-addrlabel for more info.  Can't think of better term for this.
> Tag would raise conflicts with vlan/vxlan tag term.
> Name would be confusing vs DNS naming.
>
> Also, most of the real world manages addresses through automated services so
> doing it with ip address isn't going to help.

Yeah. IFA_LABEL exists now, and likely isn't going away.

FWIW I think all it takes to clear up any confusion is a bit of
documentation in man pages and headers.