[PATCH net-next 0/3] net: Allow changing IPv4 address protocol

[PATCH net-next 0/3] net: Allow changing IPv4 address protocol

[Petr Machata]

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(-)

-- 
2.39.0

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

[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.

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

diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index b0acf6e19aed..5deac0517ef7 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -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,
-- 
2.39.0

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

[David Ahern]

On 3/21/23 5:51 AM, Petr Machata wrote:
> 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.
> 
> Signed-off-by: Petr Machata <petrm@nvidia.com>
> Reviewed-by: Ido Schimmel <idosch@nvidia.com>
> ---
>  net/ipv4/devinet.c | 3 +++
>  1 file changed, 3 insertions(+)
> 

Reviewed-by: David Ahern <dsahern@kernel.org>

[PATCH net-next 2/3] 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 3/3] selftests: rtnetlink: Add an address proto test

[Petr Machata]

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

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

diff --git a/tools/testing/selftests/net/rtnetlink.sh b/tools/testing/selftests/net/rtnetlink.sh
index 12caf9602353..3b15c686c03f 100755
--- a/tools/testing/selftests/net/rtnetlink.sh
+++ b/tools/testing/selftests/net/rtnetlink.sh
@@ -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
-- 
2.39.0

Re: [PATCH net-next 3/3] selftests: rtnetlink: Add an address proto test

[David Ahern]

On 3/21/23 5:52 AM, Petr Machata wrote:
> Add coverage of "ip address {add,replace} ... proto" support.
> 
> Signed-off-by: Petr Machata <petrm@nvidia.com>
> Reviewed-by: Ido Schimmel <idosch@nvidia.com>
> ---
>  tools/testing/selftests/net/rtnetlink.sh | 91 ++++++++++++++++++++++++
>  1 file changed, 91 insertions(+)
> 

Reviewed-by: David Ahern <dsahern@kernel.org>

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

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:

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!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html