Skip to contentSkip to conversation

net: Allow changing IPv4 address protocol

PATCH Applied Petr Machata 3 participants 7 messages Mar 21, 2023 – Mar 23, 2023

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

cover net: Allow changing IPv4 address protocol

IPv4 and IPv6 addresses can be assigned a protocol value that indicates the provenance of the IP address. The attribute is modeled after ip route protocols, and essentially allows the administrator or userspace stack to tag addresses in some way that makes sense to the actor in question.

When IP address protocol field was added in commit 47f0bd503210 ("net: Add new protocol attribute to IP addresses"), the semantics included the ability to change the protocol for IPv6 addresses, but not for IPv4 addresses. It seems this was not deliberate, but rather by accident.

One particular use case is tagging the addresses differently depending on whether the routing stack should advertise them or not. Without support for protocol replacement, this can not be done.

In this patchset, extend IPv4 to allow changing the protocol defined at an address (in patch #1). Then in patches #2 and #3 add selftest coverage for ip address protocols.

Currently the kernel simply ignores the new value. Thus allowing the replacement changes the observable behavior. However, since IPv6 already behaves like this anyway, and since the feature as such is relatively new, it seems like the change is safe to make.

An example session with the feature in action:

	bash-5.2# ip address add dev d 192.0.2.1/28 proto 0xab
	bash-5.2# ip address show dev d
	4: d: <BROADCAST,NOARP> mtu 1500 qdisc noop state DOWN group default qlen 1000
	    link/ether 06:29:74:fd:1f:eb brd ff:ff:ff:ff:ff:ff
	    inet 192.0.2.1/28 scope global proto 0xab d
	       valid_lft forever preferred_lft forever

	bash-5.2# ip address replace dev d 192.0.2.1/28 proto 0x11
	bash-5.2# ip address show dev d
	4: d: <BROADCAST,NOARP> mtu 1500 qdisc noop state DOWN group default qlen 1000
	    link/ether 06:29:74:fd:1f:eb brd ff:ff:ff:ff:ff:ff
	    inet 192.0.2.1/28 scope global proto 0x11 d
	       valid_lft forever preferred_lft forever

Petr Machata (3):
net: ipv4: Allow changing IPv4 address protocol
selftests: rtnetlink: Make the set of tests to run configurable
selftests: rtnetlink: Add an address proto test

 net/ipv4/devinet.c                       |   3 +
 tools/testing/selftests/net/rtnetlink.sh | 181 +++++++++++++++++------
 2 files changed, 142 insertions(+), 42 deletions(-)
Signature · 1 line
2.39.0
#1 lore raw

PATCH 1/3 net: ipv4: Allow changing IPv4 address protocol

When IP address protocol field was added in commit 47f0bd503210 ("net: Add new protocol attribute to IP addresses"), the semantics included the ability to change the protocol for IPv6 addresses, but not for IPv4 addresses. It seems this was not deliberate, but rather by accident.

A userspace that wants to change the protocol of an address might drop and recreate the address, but that disrupts routing and is just impractical.

So in this patch, when an IPv4 address is replaced (through RTM_NEWADDR request with NLM_F_REPLACE flag), update the proto at the address to the one given in the request, or zero if none is given. This matches the behavior of IPv6. Previously, any new value given was simply ignored.

---
 net/ipv4/devinet.c | 3 +++
 1 file changed, 3 insertions(+)
net/ipv4/devinet.c +3 -0
index b0acf6e19aed..5deac0517ef7 100644
@@ -962,6 +962,7 @@ static int inet_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh,
 					 extack);
 	} else {
 		u32 new_metric = ifa->ifa_rt_priority;
+		u8 new_proto = ifa->ifa_proto;
 
 		inet_free_ifa(ifa);
 
@@ -975,6 +976,8 @@ static int inet_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh,
 			ifa->ifa_rt_priority = new_metric;
 		}
 
+		ifa->ifa_proto = new_proto;
+
 		set_ifa_lifetime(ifa, valid_lft, prefered_lft);
 		cancel_delayed_work(&check_lifetime_work);
 		queue_delayed_work(system_power_efficient_wq,
Signature · 1 line
2.39.0
maintainer Reviewed-by on v1 1/3
#2 lore raw

Re: [PATCH net-next 1/3] net: ipv4: Allow changing IPv4 address protocol

Quoted 13 lines · Petr Machata

When IP address protocol field was added in commit 47f0bd503210 ("net: Add new protocol attribute to IP addresses"), the semantics included the ability to change the protocol for IPv6 addresses, but not for IPv4 addresses. It seems this was not deliberate, but rather by accident.

A userspace that wants to change the protocol of an address might drop and recreate the address, but that disrupts routing and is just impractical.

So in this patch, when an IPv4 address is replaced (through RTM_NEWADDR request with NLM_F_REPLACE flag), update the proto at the address to the one given in the request, or zero if none is given. This matches the behavior of IPv6. Previously, any new value given was simply ignored.

Quoted 6 lines
---
 net/ipv4/devinet.c | 3 +++
 1 file changed, 3 insertions(+)
#3 lore raw

PATCH 2/3 selftests: rtnetlink: Make the set of tests to run configurable

In reply to 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.

---
 tools/testing/selftests/net/rtnetlink.sh | 90 +++++++++++++-----------
 1 file changed, 48 insertions(+), 42 deletions(-)
tools/testing/selftests/net/rtnetlink.sh +48 -42
index 275491be3da2..12caf9602353 100755
@@ -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 $?
Signature · 1 line
2.39.0
#4 lore raw

PATCH 3/3 selftests: rtnetlink: Add an address proto test

In reply to Petr Machata

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

---
 tools/testing/selftests/net/rtnetlink.sh | 91 ++++++++++++++++++++++++
 1 file changed, 91 insertions(+)
tools/testing/selftests/net/rtnetlink.sh +91 -0
index 12caf9602353..3b15c686c03f 100755
@@ -26,6 +26,7 @@ ALL_TESTS="
 	kci_test_fdb_get
 	kci_test_neigh_get
 	kci_test_bridge_parent_id
+	kci_test_address_proto
 "
 
 devdummy="test-dummy0"
@@ -1249,6 +1250,96 @@ kci_test_bridge_parent_id()
 	echo "PASS: bridge_parent_id"
 }
 
+address_get_proto()
+{
+	local addr=$1; shift
+
+	ip -N -j address show dev "$devdummy" |
+	    jq -e -r --arg addr "${addr%/*}" \
+	       '.[].addr_info[] | select(.local == $addr) | .protocol'
+}
+
+address_count()
+{
+	ip -N -j address show dev "$devdummy" "$@" |
+	    jq -e -r '[.[].addr_info[] | .local | select(. != null)] | length'
+}
+
+do_test_address_proto()
+{
+	local what=$1; shift
+	local addr=$1; shift
+	local addr2=${addr%/*}2/${addr#*/}
+	local addr3=${addr%/*}3/${addr#*/}
+	local proto
+	local count
+	local ret=0
+	local err
+
+	ip address add dev "$devdummy" "$addr3"
+	check_err $?
+	proto=$(address_get_proto "$addr3")
+	[[ "$proto" == null ]]
+	check_err $?
+
+	ip address add dev "$devdummy" "$addr2" proto 0x99
+	check_err $?
+	proto=$(address_get_proto "$addr2")
+	[[ "$proto" == 0x99 ]]
+	check_err $?
+
+	ip address add dev "$devdummy" "$addr" proto 0xab
+	check_err $?
+	proto=$(address_get_proto "$addr")
+	[[ "$proto" == 0xab ]]
+	check_err $?
+
+	ip address replace dev "$devdummy" "$addr" proto 0x11
+	proto=$(address_get_proto "$addr")
+	check_err $?
+	[[ "$proto" == 0x11 ]]
+	check_err $?
+
+	count=$(address_count)
+	check_err $?
+	(( count == 3 )) # $addr, $addr2 and $addr3
+
+	count=$(address_count proto 0)
+	check_err $?
+	(( count == 1 )) # just $addr2
+
+	count=$(address_count proto 0x11)
+	check_err $?
+	(( count == 2 )) # $addr and $addr2
+
+	count=$(address_count proto 0xab)
+	check_err $?
+	(( count == 1 )) # just $addr2
+
+	ip address del dev "$devdummy" "$addr"
+	ip address del dev "$devdummy" "$addr2"
+	ip address del dev "$devdummy" "$addr3"
+
+	if [ $ret -ne 0 ]; then
+		echo "FAIL: address proto $what"
+		return 1
+	fi
+	echo "PASS: address proto $what"
+}
+
+kci_test_address_proto()
+{
+	local ret=0
+
+	do_test_address_proto IPv4 192.0.2.1/28
+	check_err $?
+
+	do_test_address_proto IPv6 2001:db8:1::1/64
+	check_err $?
+
+	return $ret
+}
+
 kci_test_rtnl()
 {
 	local current_test
Signature · 1 line
2.39.0
#6 lore raw
In reply to Petr Machata
This series was applied to netdev/net-next.git (main) by David S. Miller <[email protected]>:

Hello:

This series was applied to netdev/net-next.git (main) by David S. Miller <[email protected]>:

Quoted 11 lines · On Tue, 21 Mar 2023 12:51:58 +0100 you wrote:

IPv4 and IPv6 addresses can be assigned a protocol value that indicates the provenance of the IP address. The attribute is modeled after ip route protocols, and essentially allows the administrator or userspace stack to tag addresses in some way that makes sense to the actor in question.

When IP address protocol field was added in commit 47f0bd503210 ("net: Add new protocol attribute to IP addresses"), the semantics included the ability to change the protocol for IPv6 addresses, but not for IPv4 addresses. It seems this was not deliberate, but rather by accident.

[...]

Here is the summary with links:
- [net-next,1/3] net: ipv4: Allow changing IPv4 address protocol

    https://git.kernel.org/netdev/net-next/c/5c4a9aa856c7
  - [net-next,2/3] selftests: rtnetlink: Make the set of tests to run configurable
    https://git.kernel.org/netdev/net-next/c/ecb3c1e675c7
  - [net-next,3/3] selftests: rtnetlink: Add an address proto test
    https://git.kernel.org/netdev/net-next/c/6a414fd77f61

You are awesome, thank you!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html