[PATCH net 0/2] net: ipv4: fix nexthop route delete warning

[PATCH net 0/2] net: ipv4: fix nexthop route delete warning

[Nikolay Aleksandrov]

Hi,
The first patch fixes a warning that can be triggered by deleting a
nexthop route and specifying a device (more info in its commit msg).
And the second patch adds a selftest for that case.

For the fix the alternative would be to do matching on the nexthop's
attributes but I think we shouldn't since it's old-style route deletion
and nexthops are managed through a different interface, so I chose to
don't do any matching if the fib_info is a nexthop route and the user
specified old-style attributes to match on (e.g. device in this case)
which preserves the current behaviour and avoids the warning.

Thanks,
 Nik

Nikolay Aleksandrov (2):
  net: ipv4: fix route with nexthop object delete warning
  selftests: net: add delete nexthop route warning test

 net/ipv4/fib_semantics.c                    |  7 ++++++-
 tools/testing/selftests/net/fib_nexthops.sh | 13 +++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

-- 
2.35.1

[PATCH net 1/2] net: ipv4: fix route with nexthop object delete warning

[Nikolay Aleksandrov]

FRR folks have hit a kernel warning[1] while deleting routes[2] which is
caused by trying to delete a route pointing to a nexthop id without
specifying nhid but matching on an interface. That is, a route is found
but we hit a warning while matching it. The warning is from
fib_info_nh() in include/net/nexthop.h because we run it on a fib_info
with nexthop object. The call chain is:
 inet_rtm_delroute -> fib_table_delete -> fib_nh_match (called with a
nexthop fib_info and also with fc_oif set thus calling fib_info_nh on
the fib_info and triggering the warning). The fix is to not do any
matching in that branch if the fi has a nexthop object because those are
managed separately.

[1]
 [  523.462226] ------------[ cut here ]------------
 [  523.462230] WARNING: CPU: 14 PID: 22893 at include/net/nexthop.h:468 fib_nh_match+0x210/0x460
 [  523.462236] Modules linked in: dummy rpcsec_gss_krb5 xt_socket nf_socket_ipv4 nf_socket_ipv6 ip6table_raw iptable_raw bpf_preload xt_statistic ip_set ip_vs_sh ip_vs_wrr ip_vs_rr ip_vs xt_mark nf_tables xt_nat veth nf_conntrack_netlink nfnetlink xt_addrtype br_netfilter overlay dm_crypt nfsv3 nfs fscache netfs vhost_net vhost vhost_iotlb tap tun xt_CHECKSUM xt_MASQUERADE xt_conntrack 8021q garp mrp ipt_REJECT nf_reject_ipv4 ip6table_mangle ip6table_nat iptable_mangle iptable_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 iptable_filter bridge stp llc rfcomm snd_seq_dummy snd_hrtimer rpcrdma rdma_cm iw_cm ib_cm ib_core ip6table_filter xt_comment ip6_tables vboxnetadp(OE) vboxnetflt(OE) vboxdrv(OE) qrtr bnep binfmt_misc xfs vfat fat squashfs loop nvidia_drm(POE) nvidia_modeset(POE) nvidia_uvm(POE) nvidia(POE) intel_rapl_msr intel_rapl_common snd_hda_codec_realtek snd_hda_codec_generic ledtrig_audio snd_hda_codec_hdmi btusb btrtl iwlmvm uvcvideo btbcm snd_hda_intel edac_mce_amd
 [  523.462274]  videobuf2_vmalloc videobuf2_memops btintel snd_intel_dspcfg videobuf2_v4l2 snd_intel_sdw_acpi bluetooth snd_usb_audio snd_hda_codec mac80211 snd_usbmidi_lib joydev snd_hda_core videobuf2_common kvm_amd snd_rawmidi snd_hwdep snd_seq videodev ccp snd_seq_device libarc4 ecdh_generic mc snd_pcm kvm iwlwifi snd_timer drm_kms_helper snd cfg80211 cec soundcore irqbypass rapl wmi_bmof i2c_piix4 rfkill k10temp pcspkr acpi_cpufreq nfsd auth_rpcgss nfs_acl lockd grace sunrpc drm zram ip_tables crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel nvme sp5100_tco r8169 nvme_core wmi ipmi_devintf ipmi_msghandler fuse
 [  523.462300] CPU: 14 PID: 22893 Comm: ip Tainted: P           OE     5.16.18-200.fc35.x86_64 #1
 [  523.462302] Hardware name: Micro-Star International Co., Ltd. MS-7C37/MPG X570 GAMING EDGE WIFI (MS-7C37), BIOS 1.C0 10/29/2020
 [  523.462303] RIP: 0010:fib_nh_match+0x210/0x460
 [  523.462304] Code: 7c 24 20 48 8b b5 90 00 00 00 e8 bb ee f4 ff 48 8b 7c 24 20 41 89 c4 e8 ee eb f4 ff 45 85 e4 0f 85 2e fe ff ff e9 4c ff ff ff <0f> 0b e9 17 ff ff ff 3c 0a 0f 85 61 fe ff ff 48 8b b5 98 00 00 00
 [  523.462306] RSP: 0018:ffffaa53d4d87928 EFLAGS: 00010286
 [  523.462307] RAX: 0000000000000000 RBX: ffffaa53d4d87a90 RCX: ffffaa53d4d87bb0
 [  523.462308] RDX: ffff9e3d2ee6be80 RSI: ffffaa53d4d87a90 RDI: ffffffff920ed380
 [  523.462309] RBP: ffff9e3d2ee6be80 R08: 0000000000000064 R09: 0000000000000000
 [  523.462310] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000031
 [  523.462310] R13: 0000000000000020 R14: 0000000000000000 R15: ffff9e3d331054e0
 [  523.462311] FS:  00007f245517c1c0(0000) GS:ffff9e492ed80000(0000) knlGS:0000000000000000
 [  523.462313] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 [  523.462313] CR2: 000055e5dfdd8268 CR3: 00000003ef488000 CR4: 0000000000350ee0
 [  523.462315] Call Trace:
 [  523.462316]  <TASK>
 [  523.462320]  fib_table_delete+0x1a9/0x310
 [  523.462323]  inet_rtm_delroute+0x93/0x110
 [  523.462325]  rtnetlink_rcv_msg+0x133/0x370
 [  523.462327]  ? _copy_to_iter+0xb5/0x6f0
 [  523.462330]  ? rtnl_calcit.isra.0+0x110/0x110
 [  523.462331]  netlink_rcv_skb+0x50/0xf0
 [  523.462334]  netlink_unicast+0x211/0x330
 [  523.462336]  netlink_sendmsg+0x23f/0x480
 [  523.462338]  sock_sendmsg+0x5e/0x60
 [  523.462340]  ____sys_sendmsg+0x22c/0x270
 [  523.462341]  ? import_iovec+0x17/0x20
 [  523.462343]  ? sendmsg_copy_msghdr+0x59/0x90
 [  523.462344]  ? __mod_lruvec_page_state+0x85/0x110
 [  523.462348]  ___sys_sendmsg+0x81/0xc0
 [  523.462350]  ? netlink_seq_start+0x70/0x70
 [  523.462352]  ? __dentry_kill+0x13a/0x180
 [  523.462354]  ? __fput+0xff/0x250
 [  523.462356]  __sys_sendmsg+0x49/0x80
 [  523.462358]  do_syscall_64+0x3b/0x90
 [  523.462361]  entry_SYSCALL_64_after_hwframe+0x44/0xae
 [  523.462364] RIP: 0033:0x7f24552aa337
 [  523.462365] Code: 0e 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b9 0f 1f 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 89 54 24 1c 48 89 74 24 10
 [  523.462366] RSP: 002b:00007fff7f05a838 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
 [  523.462368] RAX: ffffffffffffffda RBX: 000000006245bf91 RCX: 00007f24552aa337
 [  523.462368] RDX: 0000000000000000 RSI: 00007fff7f05a8a0 RDI: 0000000000000003
 [  523.462369] RBP: 0000000000000000 R08: 0000000000000001 R09: 0000000000000000
 [  523.462370] R10: 0000000000000008 R11: 0000000000000246 R12: 0000000000000001
 [  523.462370] R13: 00007fff7f05ce08 R14: 0000000000000000 R15: 000055e5dfdd1040
 [  523.462373]  </TASK>
 [  523.462374] ---[ end trace ba537bc16f6bf4ed ]---

[2] https://github.com/FRRouting/frr/issues/6412

Fixes: 4c7e8084fd46 ("ipv4: Plumb support for nexthop object in a fib_info")
Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
 net/ipv4/fib_semantics.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index cc8e84ef2ae4..ccb62038f6a4 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -889,8 +889,13 @@ int fib_nh_match(struct net *net, struct fib_config *cfg, struct fib_info *fi,
 	}
 
 	if (cfg->fc_oif || cfg->fc_gw_family) {
-		struct fib_nh *nh = fib_info_nh(fi, 0);
+		struct fib_nh *nh;
+
+		/* cannot match on nexthop object attributes */
+		if (fi->nh)
+			return 1;
 
+		nh = fib_info_nh(fi, 0);
 		if (cfg->fc_encap) {
 			if (fib_encap_match(net, cfg->fc_encap_type,
 					    cfg->fc_encap, nh, cfg, extack))
-- 
2.35.1

Re: [PATCH net 1/2] net: ipv4: fix route with nexthop object delete warning

[David Ahern]

On 3/31/22 9:46 AM, Nikolay Aleksandrov wrote:
> FRR folks have hit a kernel warning[1] while deleting routes[2] which is
> caused by trying to delete a route pointing to a nexthop id without
> specifying nhid but matching on an interface. That is, a route is found
> but we hit a warning while matching it. The warning is from
> fib_info_nh() in include/net/nexthop.h because we run it on a fib_info
> with nexthop object. The call chain is:
>  inet_rtm_delroute -> fib_table_delete -> fib_nh_match (called with a
> nexthop fib_info and also with fc_oif set thus calling fib_info_nh on
> the fib_info and triggering the warning). The fix is to not do any
> matching in that branch if the fi has a nexthop object because those are
> managed separately.
> 
> [1]
>  [  523.462226] ------------[ cut here ]------------
>  [  523.462230] WARNING: CPU: 14 PID: 22893 at include/net/nexthop.h:468 fib_nh_match+0x210/0x460
>  [  523.462236] Modules linked in: dummy rpcsec_gss_krb5 xt_socket nf_socket_ipv4 nf_socket_ipv6 ip6table_raw iptable_raw bpf_preload xt_statistic ip_set ip_vs_sh ip_vs_wrr ip_vs_rr ip_vs xt_mark nf_tables xt_nat veth nf_conntrack_netlink nfnetlink xt_addrtype br_netfilter overlay dm_crypt nfsv3 nfs fscache netfs vhost_net vhost vhost_iotlb tap tun xt_CHECKSUM xt_MASQUERADE xt_conntrack 8021q garp mrp ipt_REJECT nf_reject_ipv4 ip6table_mangle ip6table_nat iptable_mangle iptable_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 iptable_filter bridge stp llc rfcomm snd_seq_dummy snd_hrtimer rpcrdma rdma_cm iw_cm ib_cm ib_core ip6table_filter xt_comment ip6_tables vboxnetadp(OE) vboxnetflt(OE) vboxdrv(OE) qrtr bnep binfmt_misc xfs vfat fat squashfs loop nvidia_drm(POE) nvidia_modeset(POE) nvidia_uvm(POE) nvidia(POE) intel_rapl_msr intel_rapl_common snd_hda_codec_realtek snd_hda_codec_generic ledtrig_audio snd_hda_codec_hdmi btusb btrtl iwlmvm uvcvideo btbcm snd_hda_intel edac_mce_amd
>  [  523.462274]  videobuf2_vmalloc videobuf2_memops btintel snd_intel_dspcfg videobuf2_v4l2 snd_intel_sdw_acpi bluetooth snd_usb_audio snd_hda_codec mac80211 snd_usbmidi_lib joydev snd_hda_core videobuf2_common kvm_amd snd_rawmidi snd_hwdep snd_seq videodev ccp snd_seq_device libarc4 ecdh_generic mc snd_pcm kvm iwlwifi snd_timer drm_kms_helper snd cfg80211 cec soundcore irqbypass rapl wmi_bmof i2c_piix4 rfkill k10temp pcspkr acpi_cpufreq nfsd auth_rpcgss nfs_acl lockd grace sunrpc drm zram ip_tables crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel nvme sp5100_tco r8169 nvme_core wmi ipmi_devintf ipmi_msghandler fuse
>  [  523.462300] CPU: 14 PID: 22893 Comm: ip Tainted: P           OE     5.16.18-200.fc35.x86_64 #1
>  [  523.462302] Hardware name: Micro-Star International Co., Ltd. MS-7C37/MPG X570 GAMING EDGE WIFI (MS-7C37), BIOS 1.C0 10/29/2020
>  [  523.462303] RIP: 0010:fib_nh_match+0x210/0x460
>  [  523.462304] Code: 7c 24 20 48 8b b5 90 00 00 00 e8 bb ee f4 ff 48 8b 7c 24 20 41 89 c4 e8 ee eb f4 ff 45 85 e4 0f 85 2e fe ff ff e9 4c ff ff ff <0f> 0b e9 17 ff ff ff 3c 0a 0f 85 61 fe ff ff 48 8b b5 98 00 00 00
>  [  523.462306] RSP: 0018:ffffaa53d4d87928 EFLAGS: 00010286
>  [  523.462307] RAX: 0000000000000000 RBX: ffffaa53d4d87a90 RCX: ffffaa53d4d87bb0
>  [  523.462308] RDX: ffff9e3d2ee6be80 RSI: ffffaa53d4d87a90 RDI: ffffffff920ed380
>  [  523.462309] RBP: ffff9e3d2ee6be80 R08: 0000000000000064 R09: 0000000000000000
>  [  523.462310] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000031
>  [  523.462310] R13: 0000000000000020 R14: 0000000000000000 R15: ffff9e3d331054e0
>  [  523.462311] FS:  00007f245517c1c0(0000) GS:ffff9e492ed80000(0000) knlGS:0000000000000000
>  [  523.462313] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>  [  523.462313] CR2: 000055e5dfdd8268 CR3: 00000003ef488000 CR4: 0000000000350ee0
>  [  523.462315] Call Trace:
>  [  523.462316]  <TASK>
>  [  523.462320]  fib_table_delete+0x1a9/0x310
>  [  523.462323]  inet_rtm_delroute+0x93/0x110
>  [  523.462325]  rtnetlink_rcv_msg+0x133/0x370
>  [  523.462327]  ? _copy_to_iter+0xb5/0x6f0
>  [  523.462330]  ? rtnl_calcit.isra.0+0x110/0x110
>  [  523.462331]  netlink_rcv_skb+0x50/0xf0
>  [  523.462334]  netlink_unicast+0x211/0x330
>  [  523.462336]  netlink_sendmsg+0x23f/0x480
>  [  523.462338]  sock_sendmsg+0x5e/0x60
>  [  523.462340]  ____sys_sendmsg+0x22c/0x270
>  [  523.462341]  ? import_iovec+0x17/0x20
>  [  523.462343]  ? sendmsg_copy_msghdr+0x59/0x90
>  [  523.462344]  ? __mod_lruvec_page_state+0x85/0x110
>  [  523.462348]  ___sys_sendmsg+0x81/0xc0
>  [  523.462350]  ? netlink_seq_start+0x70/0x70
>  [  523.462352]  ? __dentry_kill+0x13a/0x180
>  [  523.462354]  ? __fput+0xff/0x250
>  [  523.462356]  __sys_sendmsg+0x49/0x80
>  [  523.462358]  do_syscall_64+0x3b/0x90
>  [  523.462361]  entry_SYSCALL_64_after_hwframe+0x44/0xae
>  [  523.462364] RIP: 0033:0x7f24552aa337
>  [  523.462365] Code: 0e 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b9 0f 1f 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 89 54 24 1c 48 89 74 24 10
>  [  523.462366] RSP: 002b:00007fff7f05a838 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
>  [  523.462368] RAX: ffffffffffffffda RBX: 000000006245bf91 RCX: 00007f24552aa337
>  [  523.462368] RDX: 0000000000000000 RSI: 00007fff7f05a8a0 RDI: 0000000000000003
>  [  523.462369] RBP: 0000000000000000 R08: 0000000000000001 R09: 0000000000000000
>  [  523.462370] R10: 0000000000000008 R11: 0000000000000246 R12: 0000000000000001
>  [  523.462370] R13: 00007fff7f05ce08 R14: 0000000000000000 R15: 000055e5dfdd1040
>  [  523.462373]  </TASK>
>  [  523.462374] ---[ end trace ba537bc16f6bf4ed ]---
> 
> [2] https://github.com/FRRouting/frr/issues/6412
> 
> Fixes: 4c7e8084fd46 ("ipv4: Plumb support for nexthop object in a fib_info")
> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
> ---
>  net/ipv4/fib_semantics.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
> index cc8e84ef2ae4..ccb62038f6a4 100644
> --- a/net/ipv4/fib_semantics.c
> +++ b/net/ipv4/fib_semantics.c
> @@ -889,8 +889,13 @@ int fib_nh_match(struct net *net, struct fib_config *cfg, struct fib_info *fi,
>  	}
>  
>  	if (cfg->fc_oif || cfg->fc_gw_family) {
> -		struct fib_nh *nh = fib_info_nh(fi, 0);
> +		struct fib_nh *nh;
> +
> +		/* cannot match on nexthop object attributes */
> +		if (fi->nh)
> +			return 1;
>  
> +		nh = fib_info_nh(fi, 0);
>  		if (cfg->fc_encap) {
>  			if (fib_encap_match(net, cfg->fc_encap_type,
>  					    cfg->fc_encap, nh, cfg, extack))

I think the right fix is something like this:
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index cc8e84ef2ae4..c70775f5e155 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -886,6 +886,8 @@ int fib_nh_match(struct net *net, struct fib_config
*cfg, struct fib_info *fi,
                if (fi->nh && cfg->fc_nh_id == fi->nh->id)
                        return 0;
                return 1;
+       } else if (fi->nh) {
+               return 1;
        }

ie., if the cfg has a nexthop id it needs to match fib_info.
if the cfg does not have a nexthop id, but fib_info does then it is not
a match

Re: [PATCH net 1/2] net: ipv4: fix route with nexthop object delete warning

[Nikolay Aleksandrov]

On 31 March 2022 20:05:43 EEST, David Ahern <dsahern@kernel.org> wrote:
>On 3/31/22 9:46 AM, Nikolay Aleksandrov wrote:
>> FRR folks have hit a kernel warning[1] while deleting routes[2] which is
>> caused by trying to delete a route pointing to a nexthop id without
>> specifying nhid but matching on an interface. That is, a route is found
>> but we hit a warning while matching it. The warning is from
>> fib_info_nh() in include/net/nexthop.h because we run it on a fib_info
>> with nexthop object. The call chain is:
>>  inet_rtm_delroute -> fib_table_delete -> fib_nh_match (called with a
>> nexthop fib_info and also with fc_oif set thus calling fib_info_nh on
>> the fib_info and triggering the warning). The fix is to not do any
>> matching in that branch if the fi has a nexthop object because those are
>> managed separately.
>> 
>> [1]
>>  [  523.462226] ------------[ cut here ]------------
>>  [  523.462230] WARNING: CPU: 14 PID: 22893 at include/net/nexthop.h:468 fib_nh_match+0x210/0x460
>>  [  523.462236] Modules linked in: dummy rpcsec_gss_krb5 xt_socket nf_socket_ipv4 nf_socket_ipv6 ip6table_raw iptable_raw bpf_preload xt_statistic ip_set ip_vs_sh ip_vs_wrr ip_vs_rr ip_vs xt_mark nf_tables xt_nat veth nf_conntrack_netlink nfnetlink xt_addrtype br_netfilter overlay dm_crypt nfsv3 nfs fscache netfs vhost_net vhost vhost_iotlb tap tun xt_CHECKSUM xt_MASQUERADE xt_conntrack 8021q garp mrp ipt_REJECT nf_reject_ipv4 ip6table_mangle ip6table_nat iptable_mangle iptable_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 iptable_filter bridge stp llc rfcomm snd_seq_dummy snd_hrtimer rpcrdma rdma_cm iw_cm ib_cm ib_core ip6table_filter xt_comment ip6_tables vboxnetadp(OE) vboxnetflt(OE) vboxdrv(OE) qrtr bnep binfmt_misc xfs vfat fat squashfs loop nvidia_drm(POE) nvidia_modeset(POE) nvidia_uvm(POE) nvidia(POE) intel_rapl_msr intel_rapl_common snd_hda_codec_realtek snd_hda_codec_generic ledtrig_audio snd_hda_codec_hdmi btusb btrtl iwlmvm uvcvideo btbcm snd_hda_intel edac_mce_amd
>>  [  523.462274]  videobuf2_vmalloc videobuf2_memops btintel snd_intel_dspcfg videobuf2_v4l2 snd_intel_sdw_acpi bluetooth snd_usb_audio snd_hda_codec mac80211 snd_usbmidi_lib joydev snd_hda_core videobuf2_common kvm_amd snd_rawmidi snd_hwdep snd_seq videodev ccp snd_seq_device libarc4 ecdh_generic mc snd_pcm kvm iwlwifi snd_timer drm_kms_helper snd cfg80211 cec soundcore irqbypass rapl wmi_bmof i2c_piix4 rfkill k10temp pcspkr acpi_cpufreq nfsd auth_rpcgss nfs_acl lockd grace sunrpc drm zram ip_tables crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel nvme sp5100_tco r8169 nvme_core wmi ipmi_devintf ipmi_msghandler fuse
>>  [  523.462300] CPU: 14 PID: 22893 Comm: ip Tainted: P           OE     5.16.18-200.fc35.x86_64 #1
>>  [  523.462302] Hardware name: Micro-Star International Co., Ltd. MS-7C37/MPG X570 GAMING EDGE WIFI (MS-7C37), BIOS 1.C0 10/29/2020
>>  [  523.462303] RIP: 0010:fib_nh_match+0x210/0x460
>>  [  523.462304] Code: 7c 24 20 48 8b b5 90 00 00 00 e8 bb ee f4 ff 48 8b 7c 24 20 41 89 c4 e8 ee eb f4 ff 45 85 e4 0f 85 2e fe ff ff e9 4c ff ff ff <0f> 0b e9 17 ff ff ff 3c 0a 0f 85 61 fe ff ff 48 8b b5 98 00 00 00
>>  [  523.462306] RSP: 0018:ffffaa53d4d87928 EFLAGS: 00010286
>>  [  523.462307] RAX: 0000000000000000 RBX: ffffaa53d4d87a90 RCX: ffffaa53d4d87bb0
>>  [  523.462308] RDX: ffff9e3d2ee6be80 RSI: ffffaa53d4d87a90 RDI: ffffffff920ed380
>>  [  523.462309] RBP: ffff9e3d2ee6be80 R08: 0000000000000064 R09: 0000000000000000
>>  [  523.462310] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000031
>>  [  523.462310] R13: 0000000000000020 R14: 0000000000000000 R15: ffff9e3d331054e0
>>  [  523.462311] FS:  00007f245517c1c0(0000) GS:ffff9e492ed80000(0000) knlGS:0000000000000000
>>  [  523.462313] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>  [  523.462313] CR2: 000055e5dfdd8268 CR3: 00000003ef488000 CR4: 0000000000350ee0
>>  [  523.462315] Call Trace:
>>  [  523.462316]  <TASK>
>>  [  523.462320]  fib_table_delete+0x1a9/0x310
>>  [  523.462323]  inet_rtm_delroute+0x93/0x110
>>  [  523.462325]  rtnetlink_rcv_msg+0x133/0x370
>>  [  523.462327]  ? _copy_to_iter+0xb5/0x6f0
>>  [  523.462330]  ? rtnl_calcit.isra.0+0x110/0x110
>>  [  523.462331]  netlink_rcv_skb+0x50/0xf0
>>  [  523.462334]  netlink_unicast+0x211/0x330
>>  [  523.462336]  netlink_sendmsg+0x23f/0x480
>>  [  523.462338]  sock_sendmsg+0x5e/0x60
>>  [  523.462340]  ____sys_sendmsg+0x22c/0x270
>>  [  523.462341]  ? import_iovec+0x17/0x20
>>  [  523.462343]  ? sendmsg_copy_msghdr+0x59/0x90
>>  [  523.462344]  ? __mod_lruvec_page_state+0x85/0x110
>>  [  523.462348]  ___sys_sendmsg+0x81/0xc0
>>  [  523.462350]  ? netlink_seq_start+0x70/0x70
>>  [  523.462352]  ? __dentry_kill+0x13a/0x180
>>  [  523.462354]  ? __fput+0xff/0x250
>>  [  523.462356]  __sys_sendmsg+0x49/0x80
>>  [  523.462358]  do_syscall_64+0x3b/0x90
>>  [  523.462361]  entry_SYSCALL_64_after_hwframe+0x44/0xae
>>  [  523.462364] RIP: 0033:0x7f24552aa337
>>  [  523.462365] Code: 0e 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b9 0f 1f 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 89 54 24 1c 48 89 74 24 10
>>  [  523.462366] RSP: 002b:00007fff7f05a838 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
>>  [  523.462368] RAX: ffffffffffffffda RBX: 000000006245bf91 RCX: 00007f24552aa337
>>  [  523.462368] RDX: 0000000000000000 RSI: 00007fff7f05a8a0 RDI: 0000000000000003
>>  [  523.462369] RBP: 0000000000000000 R08: 0000000000000001 R09: 0000000000000000
>>  [  523.462370] R10: 0000000000000008 R11: 0000000000000246 R12: 0000000000000001
>>  [  523.462370] R13: 00007fff7f05ce08 R14: 0000000000000000 R15: 000055e5dfdd1040
>>  [  523.462373]  </TASK>
>>  [  523.462374] ---[ end trace ba537bc16f6bf4ed ]---
>> 
>> [2] https://github.com/FRRouting/frr/issues/6412
>> 
>> Fixes: 4c7e8084fd46 ("ipv4: Plumb support for nexthop object in a fib_info")
>> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
>> ---
>>  net/ipv4/fib_semantics.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>> 
>> diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
>> index cc8e84ef2ae4..ccb62038f6a4 100644
>> --- a/net/ipv4/fib_semantics.c
>> +++ b/net/ipv4/fib_semantics.c
>> @@ -889,8 +889,13 @@ int fib_nh_match(struct net *net, struct fib_config *cfg, struct fib_info *fi,
>>  	}
>>  
>>  	if (cfg->fc_oif || cfg->fc_gw_family) {
>> -		struct fib_nh *nh = fib_info_nh(fi, 0);
>> +		struct fib_nh *nh;
>> +
>> +		/* cannot match on nexthop object attributes */
>> +		if (fi->nh)
>> +			return 1;
>>  
>> +		nh = fib_info_nh(fi, 0);
>>  		if (cfg->fc_encap) {
>>  			if (fib_encap_match(net, cfg->fc_encap_type,
>>  					    cfg->fc_encap, nh, cfg, extack))
>
>I think the right fix is something like this:
>diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
>index cc8e84ef2ae4..c70775f5e155 100644
>--- a/net/ipv4/fib_semantics.c
>+++ b/net/ipv4/fib_semantics.c
>@@ -886,6 +886,8 @@ int fib_nh_match(struct net *net, struct fib_config
>*cfg, struct fib_info *fi,
>                if (fi->nh && cfg->fc_nh_id == fi->nh->id)
>                        return 0;
>                return 1;
>+       } else if (fi->nh) {
>+               return 1;
>        }
>
>ie., if the cfg has a nexthop id it needs to match fib_info.
>if the cfg does not have a nexthop id, but fib_info does then it is not
>a match


Right, that is also correct and I was tempted to cut it all short in that case
 but seemed riskier so I went with the more narrow fix for that specific case.
Anyway, I'll respin with that change.

Cheers,
 Nik

Re: [PATCH net 1/2] net: ipv4: fix route with nexthop object delete warning

[Nikolay Aleksandrov]

On 31 March 2022 20:17:14 EEST, Nikolay Aleksandrov <razor@blackwall.org> wrote:
>On 31 March 2022 20:05:43 EEST, David Ahern <dsahern@kernel.org> wrote:
>>On 3/31/22 9:46 AM, Nikolay Aleksandrov wrote:
>>> FRR folks have hit a kernel warning[1] while deleting routes[2] which is
>>> caused by trying to delete a route pointing to a nexthop id without
>>> specifying nhid but matching on an interface. That is, a route is found
>>> but we hit a warning while matching it. The warning is from
>>> fib_info_nh() in include/net/nexthop.h because we run it on a fib_info
>>> with nexthop object. The call chain is:
>>>  inet_rtm_delroute -> fib_table_delete -> fib_nh_match (called with a
>>> nexthop fib_info and also with fc_oif set thus calling fib_info_nh on
>>> the fib_info and triggering the warning). The fix is to not do any
>>> matching in that branch if the fi has a nexthop object because those are
>>> managed separately.
>>> 
>>> [1]
>>>  [  523.462226] ------------[ cut here ]------------
>>>  [  523.462230] WARNING: CPU: 14 PID: 22893 at include/net/nexthop.h:468 fib_nh_match+0x210/0x460
>>>  [  523.462236] Modules linked in: dummy rpcsec_gss_krb5 xt_socket nf_socket_ipv4 nf_socket_ipv6 ip6table_raw iptable_raw bpf_preload xt_statistic ip_set ip_vs_sh ip_vs_wrr ip_vs_rr ip_vs xt_mark nf_tables xt_nat veth nf_conntrack_netlink nfnetlink xt_addrtype br_netfilter overlay dm_crypt nfsv3 nfs fscache netfs vhost_net vhost vhost_iotlb tap tun xt_CHECKSUM xt_MASQUERADE xt_conntrack 8021q garp mrp ipt_REJECT nf_reject_ipv4 ip6table_mangle ip6table_nat iptable_mangle iptable_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 iptable_filter bridge stp llc rfcomm snd_seq_dummy snd_hrtimer rpcrdma rdma_cm iw_cm ib_cm ib_core ip6table_filter xt_comment ip6_tables vboxnetadp(OE) vboxnetflt(OE) vboxdrv(OE) qrtr bnep binfmt_misc xfs vfat fat squashfs loop nvidia_drm(POE) nvidia_modeset(POE) nvidia_uvm(POE) nvidia(POE) intel_rapl_msr intel_rapl_common snd_hda_codec_realtek snd_hda_codec_generic ledtrig_audio snd_hda_codec_hdmi btusb btrtl iwlmvm uvcvideo btbcm snd_hda_intel edac_mce_amd
>>>  [  523.462274]  videobuf2_vmalloc videobuf2_memops btintel snd_intel_dspcfg videobuf2_v4l2 snd_intel_sdw_acpi bluetooth snd_usb_audio snd_hda_codec mac80211 snd_usbmidi_lib joydev snd_hda_core videobuf2_common kvm_amd snd_rawmidi snd_hwdep snd_seq videodev ccp snd_seq_device libarc4 ecdh_generic mc snd_pcm kvm iwlwifi snd_timer drm_kms_helper snd cfg80211 cec soundcore irqbypass rapl wmi_bmof i2c_piix4 rfkill k10temp pcspkr acpi_cpufreq nfsd auth_rpcgss nfs_acl lockd grace sunrpc drm zram ip_tables crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel nvme sp5100_tco r8169 nvme_core wmi ipmi_devintf ipmi_msghandler fuse
>>>  [  523.462300] CPU: 14 PID: 22893 Comm: ip Tainted: P           OE     5.16.18-200.fc35.x86_64 #1
>>>  [  523.462302] Hardware name: Micro-Star International Co., Ltd. MS-7C37/MPG X570 GAMING EDGE WIFI (MS-7C37), BIOS 1.C0 10/29/2020
>>>  [  523.462303] RIP: 0010:fib_nh_match+0x210/0x460
>>>  [  523.462304] Code: 7c 24 20 48 8b b5 90 00 00 00 e8 bb ee f4 ff 48 8b 7c 24 20 41 89 c4 e8 ee eb f4 ff 45 85 e4 0f 85 2e fe ff ff e9 4c ff ff ff <0f> 0b e9 17 ff ff ff 3c 0a 0f 85 61 fe ff ff 48 8b b5 98 00 00 00
>>>  [  523.462306] RSP: 0018:ffffaa53d4d87928 EFLAGS: 00010286
>>>  [  523.462307] RAX: 0000000000000000 RBX: ffffaa53d4d87a90 RCX: ffffaa53d4d87bb0
>>>  [  523.462308] RDX: ffff9e3d2ee6be80 RSI: ffffaa53d4d87a90 RDI: ffffffff920ed380
>>>  [  523.462309] RBP: ffff9e3d2ee6be80 R08: 0000000000000064 R09: 0000000000000000
>>>  [  523.462310] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000031
>>>  [  523.462310] R13: 0000000000000020 R14: 0000000000000000 R15: ffff9e3d331054e0
>>>  [  523.462311] FS:  00007f245517c1c0(0000) GS:ffff9e492ed80000(0000) knlGS:0000000000000000
>>>  [  523.462313] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>>  [  523.462313] CR2: 000055e5dfdd8268 CR3: 00000003ef488000 CR4: 0000000000350ee0
>>>  [  523.462315] Call Trace:
>>>  [  523.462316]  <TASK>
>>>  [  523.462320]  fib_table_delete+0x1a9/0x310
>>>  [  523.462323]  inet_rtm_delroute+0x93/0x110
>>>  [  523.462325]  rtnetlink_rcv_msg+0x133/0x370
>>>  [  523.462327]  ? _copy_to_iter+0xb5/0x6f0
>>>  [  523.462330]  ? rtnl_calcit.isra.0+0x110/0x110
>>>  [  523.462331]  netlink_rcv_skb+0x50/0xf0
>>>  [  523.462334]  netlink_unicast+0x211/0x330
>>>  [  523.462336]  netlink_sendmsg+0x23f/0x480
>>>  [  523.462338]  sock_sendmsg+0x5e/0x60
>>>  [  523.462340]  ____sys_sendmsg+0x22c/0x270
>>>  [  523.462341]  ? import_iovec+0x17/0x20
>>>  [  523.462343]  ? sendmsg_copy_msghdr+0x59/0x90
>>>  [  523.462344]  ? __mod_lruvec_page_state+0x85/0x110
>>>  [  523.462348]  ___sys_sendmsg+0x81/0xc0
>>>  [  523.462350]  ? netlink_seq_start+0x70/0x70
>>>  [  523.462352]  ? __dentry_kill+0x13a/0x180
>>>  [  523.462354]  ? __fput+0xff/0x250
>>>  [  523.462356]  __sys_sendmsg+0x49/0x80
>>>  [  523.462358]  do_syscall_64+0x3b/0x90
>>>  [  523.462361]  entry_SYSCALL_64_after_hwframe+0x44/0xae
>>>  [  523.462364] RIP: 0033:0x7f24552aa337
>>>  [  523.462365] Code: 0e 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b9 0f 1f 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 89 54 24 1c 48 89 74 24 10
>>>  [  523.462366] RSP: 002b:00007fff7f05a838 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
>>>  [  523.462368] RAX: ffffffffffffffda RBX: 000000006245bf91 RCX: 00007f24552aa337
>>>  [  523.462368] RDX: 0000000000000000 RSI: 00007fff7f05a8a0 RDI: 0000000000000003
>>>  [  523.462369] RBP: 0000000000000000 R08: 0000000000000001 R09: 0000000000000000
>>>  [  523.462370] R10: 0000000000000008 R11: 0000000000000246 R12: 0000000000000001
>>>  [  523.462370] R13: 00007fff7f05ce08 R14: 0000000000000000 R15: 000055e5dfdd1040
>>>  [  523.462373]  </TASK>
>>>  [  523.462374] ---[ end trace ba537bc16f6bf4ed ]---
>>> 
>>> [2] https://github.com/FRRouting/frr/issues/6412
>>> 
>>> Fixes: 4c7e8084fd46 ("ipv4: Plumb support for nexthop object in a fib_info")
>>> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
>>> ---
>>>  net/ipv4/fib_semantics.c | 7 ++++++-
>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
>>> index cc8e84ef2ae4..ccb62038f6a4 100644
>>> --- a/net/ipv4/fib_semantics.c
>>> +++ b/net/ipv4/fib_semantics.c
>>> @@ -889,8 +889,13 @@ int fib_nh_match(struct net *net, struct fib_config *cfg, struct fib_info *fi,
>>>  	}
>>>  
>>>  	if (cfg->fc_oif || cfg->fc_gw_family) {
>>> -		struct fib_nh *nh = fib_info_nh(fi, 0);
>>> +		struct fib_nh *nh;
>>> +
>>> +		/* cannot match on nexthop object attributes */
>>> +		if (fi->nh)
>>> +			return 1;
>>>  
>>> +		nh = fib_info_nh(fi, 0);
>>>  		if (cfg->fc_encap) {
>>>  			if (fib_encap_match(net, cfg->fc_encap_type,
>>>  					    cfg->fc_encap, nh, cfg, extack))
>>
>>I think the right fix is something like this:
>>diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
>>index cc8e84ef2ae4..c70775f5e155 100644
>>--- a/net/ipv4/fib_semantics.c
>>+++ b/net/ipv4/fib_semantics.c
>>@@ -886,6 +886,8 @@ int fib_nh_match(struct net *net, struct fib_config
>>*cfg, struct fib_info *fi,
>>                if (fi->nh && cfg->fc_nh_id == fi->nh->id)
>>                        return 0;
>>                return 1;
>>+       } else if (fi->nh) {
>>+               return 1;
>>        }
>>
>>ie., if the cfg has a nexthop id it needs to match fib_info.
>>if the cfg does not have a nexthop id, but fib_info does then it is not
>>a match
>
>
>Right, that is also correct and I was tempted to cut it all short in that case
> but seemed riskier so I went with the more narrow fix for that specific case.
>Anyway, I'll respin with that change.
>
>Cheers,
> Nik
>

Actually I just remembered another reason - ip route del default
should work regardless of specifying nhid or not. I can't test right now,
but I suspect that may break if the nh match code is invoked with that change.

I'll verify it once I'm back in front of a pc.

Re: [PATCH net 1/2] net: ipv4: fix route with nexthop object delete warning

[Nikolay Aleksandrov]

On 31/03/2022 20:34, Nikolay Aleksandrov wrote:
> On 31 March 2022 20:17:14 EEST, Nikolay Aleksandrov <razor@blackwall.org> wrote:
>> On 31 March 2022 20:05:43 EEST, David Ahern <dsahern@kernel.org> wrote:
>>> On 3/31/22 9:46 AM, Nikolay Aleksandrov wrote:
>>>> FRR folks have hit a kernel warning[1] while deleting routes[2] which is
>>>> caused by trying to delete a route pointing to a nexthop id without
>>>> specifying nhid but matching on an interface. That is, a route is found
>>>> but we hit a warning while matching it. The warning is from
>>>> fib_info_nh() in include/net/nexthop.h because we run it on a fib_info
>>>> with nexthop object. The call chain is:
>>>>  inet_rtm_delroute -> fib_table_delete -> fib_nh_match (called with a
>>>> nexthop fib_info and also with fc_oif set thus calling fib_info_nh on
>>>> the fib_info and triggering the warning). The fix is to not do any
>>>> matching in that branch if the fi has a nexthop object because those are
>>>> managed separately.
>>>>
[snip]
>>>> [2] https://github.com/FRRouting/frr/issues/6412
>>>>
>>>> Fixes: 4c7e8084fd46 ("ipv4: Plumb support for nexthop object in a fib_info")
>>>> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
>>>> ---
>>>>  net/ipv4/fib_semantics.c | 7 ++++++-
>>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
>>>> index cc8e84ef2ae4..ccb62038f6a4 100644
>>>> --- a/net/ipv4/fib_semantics.c
>>>> +++ b/net/ipv4/fib_semantics.c
>>>> @@ -889,8 +889,13 @@ int fib_nh_match(struct net *net, struct fib_config *cfg, struct fib_info *fi,
>>>>  	}
>>>>  
>>>>  	if (cfg->fc_oif || cfg->fc_gw_family) {
>>>> -		struct fib_nh *nh = fib_info_nh(fi, 0);
>>>> +		struct fib_nh *nh;
>>>> +
>>>> +		/* cannot match on nexthop object attributes */
>>>> +		if (fi->nh)
>>>> +			return 1;
>>>>  
>>>> +		nh = fib_info_nh(fi, 0);
>>>>  		if (cfg->fc_encap) {
>>>>  			if (fib_encap_match(net, cfg->fc_encap_type,
>>>>  					    cfg->fc_encap, nh, cfg, extack))
>>>
>>> I think the right fix is something like this:
>>> diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
>>> index cc8e84ef2ae4..c70775f5e155 100644
>>> --- a/net/ipv4/fib_semantics.c
>>> +++ b/net/ipv4/fib_semantics.c
>>> @@ -886,6 +886,8 @@ int fib_nh_match(struct net *net, struct fib_config
>>> *cfg, struct fib_info *fi,
>>>                if (fi->nh && cfg->fc_nh_id == fi->nh->id)
>>>                        return 0;
>>>                return 1;
>>> +       } else if (fi->nh) {
>>> +               return 1;
>>>        }
>>>
>>> ie., if the cfg has a nexthop id it needs to match fib_info.
>>> if the cfg does not have a nexthop id, but fib_info does then it is not
>>> a match
>>
>>
>> Right, that is also correct and I was tempted to cut it all short in that case
>> but seemed riskier so I went with the more narrow fix for that specific case.
>> Anyway, I'll respin with that change.
>>
>> Cheers,
>> Nik
>>
> 
> Actually I just remembered another reason - ip route del default
> should work regardless of specifying nhid or not. I can't test right now,
> but I suspect that may break if the nh match code is invoked with that change.
> 
> I'll verify it once I'm back in front of a pc.
> 

I just tested it and yes - the change breaks the standard ip route del <addr> case.
I.e.:
$ ip r add 1.2.3.4/32 nhid 12
$ ip r del 1.2.3.4/32
RTNETLINK answers: No such process

Even though:
$ ip r show 1.2.3.4/32
1.2.3.4 nhid 12 via 192.168.11.2 dev dummy0 

Before that change it worked, so I think we should proceed with my fix instead.
The command must not match if there's any nexthop specification after the route
as it doesn't right now, and it must match if there isn't any nh spec.

Cheers,
 Nik

Re: [PATCH net 1/2] net: ipv4: fix route with nexthop object delete warning

[David Ahern]

On 3/31/22 9:46 AM, Nikolay Aleksandrov wrote:
> FRR folks have hit a kernel warning[1] while deleting routes[2] which is
> caused by trying to delete a route pointing to a nexthop id without
> specifying nhid but matching on an interface. That is, a route is found
> but we hit a warning while matching it. The warning is from
> fib_info_nh() in include/net/nexthop.h because we run it on a fib_info
> with nexthop object. The call chain is:
>  inet_rtm_delroute -> fib_table_delete -> fib_nh_match (called with a
> nexthop fib_info and also with fc_oif set thus calling fib_info_nh on
> the fib_info and triggering the warning). The fix is to not do any
> matching in that branch if the fi has a nexthop object because those are
> managed separately.
> 
> [1]
>  [  523.462226] ------------[ cut here ]------------
>  [  523.462230] WARNING: CPU: 14 PID: 22893 at include/net/nexthop.h:468 fib_nh_match+0x210/0x460
>  [  523.462236] Modules linked in: dummy rpcsec_gss_krb5 xt_socket nf_socket_ipv4 nf_socket_ipv6 ip6table_raw iptable_raw bpf_preload xt_statistic ip_set ip_vs_sh ip_vs_wrr ip_vs_rr ip_vs xt_mark nf_tables xt_nat veth nf_conntrack_netlink nfnetlink xt_addrtype br_netfilter overlay dm_crypt nfsv3 nfs fscache netfs vhost_net vhost vhost_iotlb tap tun xt_CHECKSUM xt_MASQUERADE xt_conntrack 8021q garp mrp ipt_REJECT nf_reject_ipv4 ip6table_mangle ip6table_nat iptable_mangle iptable_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 iptable_filter bridge stp llc rfcomm snd_seq_dummy snd_hrtimer rpcrdma rdma_cm iw_cm ib_cm ib_core ip6table_filter xt_comment ip6_tables vboxnetadp(OE) vboxnetflt(OE) vboxdrv(OE) qrtr bnep binfmt_misc xfs vfat fat squashfs loop nvidia_drm(POE) nvidia_modeset(POE) nvidia_uvm(POE) nvidia(POE) intel_rapl_msr intel_rapl_common snd_hda_codec_realtek snd_hda_codec_generic ledtrig_audio snd_hda_codec_hdmi btusb btrtl iwlmvm uvcvideo btbcm snd_hda_intel edac_mce_amd
>  [  523.462274]  videobuf2_vmalloc videobuf2_memops btintel snd_intel_dspcfg videobuf2_v4l2 snd_intel_sdw_acpi bluetooth snd_usb_audio snd_hda_codec mac80211 snd_usbmidi_lib joydev snd_hda_core videobuf2_common kvm_amd snd_rawmidi snd_hwdep snd_seq videodev ccp snd_seq_device libarc4 ecdh_generic mc snd_pcm kvm iwlwifi snd_timer drm_kms_helper snd cfg80211 cec soundcore irqbypass rapl wmi_bmof i2c_piix4 rfkill k10temp pcspkr acpi_cpufreq nfsd auth_rpcgss nfs_acl lockd grace sunrpc drm zram ip_tables crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel nvme sp5100_tco r8169 nvme_core wmi ipmi_devintf ipmi_msghandler fuse
>  [  523.462300] CPU: 14 PID: 22893 Comm: ip Tainted: P           OE     5.16.18-200.fc35.x86_64 #1
>  [  523.462302] Hardware name: Micro-Star International Co., Ltd. MS-7C37/MPG X570 GAMING EDGE WIFI (MS-7C37), BIOS 1.C0 10/29/2020
>  [  523.462303] RIP: 0010:fib_nh_match+0x210/0x460
>  [  523.462304] Code: 7c 24 20 48 8b b5 90 00 00 00 e8 bb ee f4 ff 48 8b 7c 24 20 41 89 c4 e8 ee eb f4 ff 45 85 e4 0f 85 2e fe ff ff e9 4c ff ff ff <0f> 0b e9 17 ff ff ff 3c 0a 0f 85 61 fe ff ff 48 8b b5 98 00 00 00
>  [  523.462306] RSP: 0018:ffffaa53d4d87928 EFLAGS: 00010286
>  [  523.462307] RAX: 0000000000000000 RBX: ffffaa53d4d87a90 RCX: ffffaa53d4d87bb0
>  [  523.462308] RDX: ffff9e3d2ee6be80 RSI: ffffaa53d4d87a90 RDI: ffffffff920ed380
>  [  523.462309] RBP: ffff9e3d2ee6be80 R08: 0000000000000064 R09: 0000000000000000
>  [  523.462310] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000031
>  [  523.462310] R13: 0000000000000020 R14: 0000000000000000 R15: ffff9e3d331054e0
>  [  523.462311] FS:  00007f245517c1c0(0000) GS:ffff9e492ed80000(0000) knlGS:0000000000000000
>  [  523.462313] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>  [  523.462313] CR2: 000055e5dfdd8268 CR3: 00000003ef488000 CR4: 0000000000350ee0
>  [  523.462315] Call Trace:
>  [  523.462316]  <TASK>
>  [  523.462320]  fib_table_delete+0x1a9/0x310
>  [  523.462323]  inet_rtm_delroute+0x93/0x110
>  [  523.462325]  rtnetlink_rcv_msg+0x133/0x370
>  [  523.462327]  ? _copy_to_iter+0xb5/0x6f0
>  [  523.462330]  ? rtnl_calcit.isra.0+0x110/0x110
>  [  523.462331]  netlink_rcv_skb+0x50/0xf0
>  [  523.462334]  netlink_unicast+0x211/0x330
>  [  523.462336]  netlink_sendmsg+0x23f/0x480
>  [  523.462338]  sock_sendmsg+0x5e/0x60
>  [  523.462340]  ____sys_sendmsg+0x22c/0x270
>  [  523.462341]  ? import_iovec+0x17/0x20
>  [  523.462343]  ? sendmsg_copy_msghdr+0x59/0x90
>  [  523.462344]  ? __mod_lruvec_page_state+0x85/0x110
>  [  523.462348]  ___sys_sendmsg+0x81/0xc0
>  [  523.462350]  ? netlink_seq_start+0x70/0x70
>  [  523.462352]  ? __dentry_kill+0x13a/0x180
>  [  523.462354]  ? __fput+0xff/0x250
>  [  523.462356]  __sys_sendmsg+0x49/0x80
>  [  523.462358]  do_syscall_64+0x3b/0x90
>  [  523.462361]  entry_SYSCALL_64_after_hwframe+0x44/0xae
>  [  523.462364] RIP: 0033:0x7f24552aa337
>  [  523.462365] Code: 0e 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b9 0f 1f 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 89 54 24 1c 48 89 74 24 10
>  [  523.462366] RSP: 002b:00007fff7f05a838 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
>  [  523.462368] RAX: ffffffffffffffda RBX: 000000006245bf91 RCX: 00007f24552aa337
>  [  523.462368] RDX: 0000000000000000 RSI: 00007fff7f05a8a0 RDI: 0000000000000003
>  [  523.462369] RBP: 0000000000000000 R08: 0000000000000001 R09: 0000000000000000
>  [  523.462370] R10: 0000000000000008 R11: 0000000000000246 R12: 0000000000000001
>  [  523.462370] R13: 00007fff7f05ce08 R14: 0000000000000000 R15: 000055e5dfdd1040
>  [  523.462373]  </TASK>
>  [  523.462374] ---[ end trace ba537bc16f6bf4ed ]---
> 
> [2] https://github.com/FRRouting/frr/issues/6412
> 
> Fixes: 4c7e8084fd46 ("ipv4: Plumb support for nexthop object in a fib_info")
> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
> ---
>  net/ipv4/fib_semantics.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 

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

[PATCH net 2/2] selftests: net: add delete nexthop route warning test

[Nikolay Aleksandrov]

Add a test which causes a WARNING on kernels which treat a
nexthop route like a normal route when comparing for deletion and a
device is specified. That is, a route is found but we hit a warning while
matching it. The warning is from fib_info_nh() in include/net/nexthop.h
because we run it on a fib_info with nexthop object. The call chain is:
 inet_rtm_delroute -> fib_table_delete -> fib_nh_match (called with a
nexthop fib_info and also with fc_oif set thus calling fib_info_nh on
the fib_info and triggering the warning).

Repro steps:
 $ ip nexthop add id 12 via 172.16.1.3 dev veth1
 $ ip route add 172.16.101.1/32 nhid 12
 $ ip route delete 172.16.101.1/32 dev veth1

Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
 tools/testing/selftests/net/fib_nexthops.sh | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tools/testing/selftests/net/fib_nexthops.sh b/tools/testing/selftests/net/fib_nexthops.sh
index d444ee6aa3cb..371e3e0c91b7 100755
--- a/tools/testing/selftests/net/fib_nexthops.sh
+++ b/tools/testing/selftests/net/fib_nexthops.sh
@@ -1208,6 +1208,19 @@ ipv4_fcnal()
 	set +e
 	check_nexthop "dev veth1" ""
 	log_test $? 0 "Nexthops removed on admin down"
+
+	# nexthop route delete warning
+	run_cmd "$IP li set dev veth1 up"
+	run_cmd "$IP nexthop add id 12 via 172.16.1.3 dev veth1"
+	out1=`dmesg | grep "WARNING:.*fib_nh_match.*" | wc -l`
+	run_cmd "$IP route add 172.16.101.1/32 nhid 12"
+	run_cmd "$IP route delete 172.16.101.1/32 dev veth1"
+	out2=`dmesg | grep "WARNING:.*fib_nh_match.*" | wc -l`
+	[ $out1 -eq $out2 ]
+	rc=$?
+	log_test $rc 0 "Delete nexthop route warning"
+	run_cmd "$IP ip route delete 172.16.101.1/32 nhid 12"
+	run_cmd "$IP ip nexthop del id 12"
 }
 
 ipv4_grp_fcnal()
-- 
2.35.1

Re: [PATCH net 2/2] selftests: net: add delete nexthop route warning test

[David Ahern]

On 3/31/22 9:46 AM, Nikolay Aleksandrov wrote:
> Add a test which causes a WARNING on kernels which treat a
> nexthop route like a normal route when comparing for deletion and a
> device is specified. That is, a route is found but we hit a warning while
> matching it. The warning is from fib_info_nh() in include/net/nexthop.h
> because we run it on a fib_info with nexthop object. The call chain is:
>  inet_rtm_delroute -> fib_table_delete -> fib_nh_match (called with a
> nexthop fib_info and also with fc_oif set thus calling fib_info_nh on
> the fib_info and triggering the warning).
> 
> Repro steps:
>  $ ip nexthop add id 12 via 172.16.1.3 dev veth1
>  $ ip route add 172.16.101.1/32 nhid 12
>  $ ip route delete 172.16.101.1/32 dev veth1
> 
> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
> ---
>  tools/testing/selftests/net/fib_nexthops.sh | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/tools/testing/selftests/net/fib_nexthops.sh b/tools/testing/selftests/net/fib_nexthops.sh
> index d444ee6aa3cb..371e3e0c91b7 100755
> --- a/tools/testing/selftests/net/fib_nexthops.sh
> +++ b/tools/testing/selftests/net/fib_nexthops.sh
> @@ -1208,6 +1208,19 @@ ipv4_fcnal()
>  	set +e
>  	check_nexthop "dev veth1" ""
>  	log_test $? 0 "Nexthops removed on admin down"
> +
> +	# nexthop route delete warning
> +	run_cmd "$IP li set dev veth1 up"
> +	run_cmd "$IP nexthop add id 12 via 172.16.1.3 dev veth1"
> +	out1=`dmesg | grep "WARNING:.*fib_nh_match.*" | wc -l`
> +	run_cmd "$IP route add 172.16.101.1/32 nhid 12"
> +	run_cmd "$IP route delete 172.16.101.1/32 dev veth1"
> +	out2=`dmesg | grep "WARNING:.*fib_nh_match.*" | wc -l`
> +	[ $out1 -eq $out2 ]
> +	rc=$?
> +	log_test $rc 0 "Delete nexthop route warning"
> +	run_cmd "$IP ip route delete 172.16.101.1/32 nhid 12"
> +	run_cmd "$IP ip nexthop del id 12"
>  }
>  
>  ipv4_grp_fcnal()

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

Re: [PATCH net 0/2] net: ipv4: fix nexthop route delete warning

[Nikolay Aleksandrov]

On 31/03/2022 18:46, Nikolay Aleksandrov wrote:
> Hi,
> The first patch fixes a warning that can be triggered by deleting a
> nexthop route and specifying a device (more info in its commit msg).
> And the second patch adds a selftest for that case.
> 
> For the fix the alternative would be to do matching on the nexthop's
> attributes but I think we shouldn't since it's old-style route deletion
> and nexthops are managed through a different interface, so I chose to
> don't do any matching if the fib_info is a nexthop route and the user
> specified old-style attributes to match on (e.g. device in this case)
> which preserves the current behaviour and avoids the warning.
> 
> Thanks,
>  Nik
> 
> Nikolay Aleksandrov (2):
>   net: ipv4: fix route with nexthop object delete warning
>   selftests: net: add delete nexthop route warning test
> 
>  net/ipv4/fib_semantics.c                    |  7 ++++++-
>  tools/testing/selftests/net/fib_nexthops.sh | 13 +++++++++++++
>  2 files changed, 19 insertions(+), 1 deletion(-)
> 

I'll send v2 with an updated comment in the selftest and added tags.

Thanks,
 Nik