Message-ID: <20250320151150.GC889584@horms.kernel.org>
Date: Thu, 20 Mar 2025 15:11:50 +0000
From: Simon Horman <horms@...nel.org>
To: Mark Bloch <mbloch@...dia.com>
Cc: "David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Sabrina Dubroca <sd@...asysnail.net>,
	Kuniyuki Iwashima <kuniyu@...zon.com>,
	Leon Romanovsky <leon@...nel.org>, David Ahern <dsahern@...nel.org>,
	netdev@...r.kernel.org, linux-rdma@...r.kernel.org,
	Mark Zhang <markzhang@...dia.com>,
	Maher Sanalla <msanalla@...dia.com>
Subject: Re: [PATCH net] rtnetlink: Allocate vfinfo size for VF GUIDs when
 supported

On Mon, Mar 17, 2025 at 12:24:19PM +0200, Mark Bloch wrote:
> From: Mark Zhang <markzhang@...dia.com>
> 
> Commit 30aad41721e0 ("net/core: Add support for getting VF GUIDs")
> added support for getting VF port and node GUIDs in netlink ifinfo
> messages, but their size was not taken into consideration in the
> function that allocates the netlink message, causing the following
> warning when a netlink message is filled with many VF port and node
> GUIDs:
>  # echo 64 > /sys/bus/pci/devices/0000\:08\:00.0/sriov_numvfs
>  # ip link show dev ib0
>  RTNETLINK answers: Message too long
>  Cannot send link get request: Message too long
> 
> Kernel warning:
> 
>  ------------[ cut here ]------------
>  WARNING: CPU: 2 PID: 1930 at net/core/rtnetlink.c:4151 rtnl_getlink+0x586/0x5a0
>  Modules linked in: xt_conntrack xt_MASQUERADE nfnetlink xt_addrtype iptable_nat nf_nat br_netfilter overlay mlx5_ib macsec mlx5_core tls rpcrdma rdma_ucm ib_uverbs ib_iser libiscsi scsi_transport_iscsi ib_umad rdma_cm iw_cm ib_ipoib fuse ib_cm ib_core
>  CPU: 2 UID: 0 PID: 1930 Comm: ip Not tainted 6.14.0-rc2+ #1
>  Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
>  RIP: 0010:rtnl_getlink+0x586/0x5a0
>  Code: cb 82 e8 3d af 0a 00 4d 85 ff 0f 84 08 ff ff ff 4c 89 ff 41 be ea ff ff ff e8 66 63 5b ff 49 c7 07 80 4f cb 82 e9 36 fc ff ff <0f> 0b e9 16 fe ff ff e8 de a0 56 00 66 66 2e 0f 1f 84 00 00 00 00
>  RSP: 0018:ffff888113557348 EFLAGS: 00010246
>  RAX: 00000000ffffffa6 RBX: ffff88817e87aa34 RCX: dffffc0000000000
>  RDX: 0000000000000003 RSI: 0000000000000000 RDI: ffff88817e87afb8
>  RBP: 0000000000000009 R08: ffffffff821f44aa R09: 0000000000000000
>  R10: ffff8881260f79a8 R11: ffff88817e87af00 R12: ffff88817e87aa00
>  R13: ffffffff8563d300 R14: 00000000ffffffa6 R15: 00000000ffffffff
>  FS:  00007f63a5dbf280(0000) GS:ffff88881ee00000(0000) knlGS:0000000000000000
>  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>  CR2: 00007f63a5ba4493 CR3: 00000001700fe002 CR4: 0000000000772eb0
>  DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>  DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>  PKRU: 55555554
>  Call Trace:
>   <TASK>
>   ? __warn+0xa5/0x230
>   ? rtnl_getlink+0x586/0x5a0
>   ? report_bug+0x22d/0x240
>   ? handle_bug+0x53/0xa0
>   ? exc_invalid_op+0x14/0x50
>   ? asm_exc_invalid_op+0x16/0x20
>   ? skb_trim+0x6a/0x80
>   ? rtnl_getlink+0x586/0x5a0
>   ? __pfx_rtnl_getlink+0x10/0x10
>   ? rtnetlink_rcv_msg+0x1e5/0x860
>   ? __pfx___mutex_lock+0x10/0x10
>   ? rcu_is_watching+0x34/0x60
>   ? __pfx_lock_acquire+0x10/0x10
>   ? stack_trace_save+0x90/0xd0
>   ? filter_irq_stacks+0x1d/0x70
>   ? kasan_save_stack+0x30/0x40
>   ? kasan_save_stack+0x20/0x40
>   ? kasan_save_track+0x10/0x30
>   rtnetlink_rcv_msg+0x21c/0x860
>   ? entry_SYSCALL_64_after_hwframe+0x76/0x7e
>   ? __pfx_rtnetlink_rcv_msg+0x10/0x10
>   ? arch_stack_walk+0x9e/0xf0
>   ? rcu_is_watching+0x34/0x60
>   ? lock_acquire+0xd5/0x410
>   ? rcu_is_watching+0x34/0x60
>   netlink_rcv_skb+0xe0/0x210
>   ? __pfx_rtnetlink_rcv_msg+0x10/0x10
>   ? __pfx_netlink_rcv_skb+0x10/0x10
>   ? rcu_is_watching+0x34/0x60
>   ? __pfx___netlink_lookup+0x10/0x10
>   ? lock_release+0x62/0x200
>   ? netlink_deliver_tap+0xfd/0x290
>   ? rcu_is_watching+0x34/0x60
>   ? lock_release+0x62/0x200
>   ? netlink_deliver_tap+0x95/0x290
>   netlink_unicast+0x31f/0x480
>   ? __pfx_netlink_unicast+0x10/0x10
>   ? rcu_is_watching+0x34/0x60
>   ? lock_acquire+0xd5/0x410
>   netlink_sendmsg+0x369/0x660
>   ? lock_release+0x62/0x200
>   ? __pfx_netlink_sendmsg+0x10/0x10
>   ? import_ubuf+0xb9/0xf0
>   ? __import_iovec+0x254/0x2b0
>   ? lock_release+0x62/0x200
>   ? __pfx_netlink_sendmsg+0x10/0x10
>   ____sys_sendmsg+0x559/0x5a0
>   ? __pfx_____sys_sendmsg+0x10/0x10
>   ? __pfx_copy_msghdr_from_user+0x10/0x10
>   ? rcu_is_watching+0x34/0x60
>   ? do_read_fault+0x213/0x4a0
>   ? rcu_is_watching+0x34/0x60
>   ___sys_sendmsg+0xe4/0x150
>   ? __pfx____sys_sendmsg+0x10/0x10
>   ? do_fault+0x2cc/0x6f0
>   ? handle_pte_fault+0x2e3/0x3d0
>   ? __pfx_handle_pte_fault+0x10/0x10
>   ? preempt_count_sub+0x14/0xc0
>   ? __down_read_trylock+0x150/0x270
>   ? __handle_mm_fault+0x404/0x8e0
>   ? __pfx___handle_mm_fault+0x10/0x10
>   ? lock_release+0x62/0x200
>   ? __rcu_read_unlock+0x65/0x90
>   ? rcu_is_watching+0x34/0x60
>   __sys_sendmsg+0xd5/0x150
>   ? __pfx___sys_sendmsg+0x10/0x10
>   ? __up_read+0x192/0x480
>   ? lock_release+0x62/0x200
>   ? __rcu_read_unlock+0x65/0x90
>   ? rcu_is_watching+0x34/0x60
>   do_syscall_64+0x6d/0x140
>   entry_SYSCALL_64_after_hwframe+0x76/0x7e
>  RIP: 0033:0x7f63a5b13367
>  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
>  RSP: 002b:00007fff8c726bc8 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
>  RAX: ffffffffffffffda RBX: 0000000067b687c2 RCX: 00007f63a5b13367
>  RDX: 0000000000000000 RSI: 00007fff8c726c30 RDI: 0000000000000004
>  RBP: 00007fff8c726cb8 R08: 0000000000000000 R09: 0000000000000034
>  R10: 00007fff8c726c7c R11: 0000000000000246 R12: 0000000000000001
>  R13: 0000000000000000 R14: 00007fff8c726cd0 R15: 00007fff8c726cd0
>   </TASK>
>  irq event stamp: 0
>  hardirqs last  enabled at (0): [<0000000000000000>] 0x0
>  hardirqs last disabled at (0): [<ffffffff813f9e58>] copy_process+0xd08/0x2830
>  softirqs last  enabled at (0): [<ffffffff813f9e58>] copy_process+0xd08/0x2830
>  softirqs last disabled at (0): [<0000000000000000>] 0x0
>  ---[ end trace 0000000000000000 ]---
> 
> Thus, when calculating ifinfo message size, take VF GUIDs sizes into
> account when supported.
> 
> Fixes: 30aad41721e0 ("net/core: Add support for getting VF GUIDs")
> Signed-off-by: Mark Zhang <markzhang@...dia.com>
> Reviewed-by: Maher Sanalla <msanalla@...dia.com>
> Signed-off-by: Mark Bloch <mbloch@...dia.com>
> ---
>  net/core/rtnetlink.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index d1e559fce918..bfc590e933d9 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -1150,7 +1150,9 @@ static inline int rtnl_vfinfo_size(const struct net_device *dev,
>  			 nla_total_size(sizeof(struct ifla_vf_rate)) +
>  			 nla_total_size(sizeof(struct ifla_vf_link_state)) +
>  			 nla_total_size(sizeof(struct ifla_vf_rss_query_en)) +
> -			 nla_total_size(sizeof(struct ifla_vf_trust)));
> +			 nla_total_size(sizeof(struct ifla_vf_trust)) +
> +			 (dev->netdev_ops->ndo_get_vf_guid ?
> +			  nla_total_size(sizeof(struct ifla_vf_guid)) * 2 : 0));
>  		if (~ext_filter_mask & RTEXT_FILTER_SKIP_STATS) {
>  			size += num_vfs *
>  				(nla_total_size(0) + /* nest IFLA_VF_STATS */

Perhaps I'm over thinking things here,
perhaps the following is easier on the eyes?

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index d1e559fce918..60fac848e092 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1151,6 +1151,9 @@ static inline int rtnl_vfinfo_size(const struct net_device *dev,
 			 nla_total_size(sizeof(struct ifla_vf_link_state)) +
 			 nla_total_size(sizeof(struct ifla_vf_rss_query_en)) +
 			 nla_total_size(sizeof(struct ifla_vf_trust)));
+		if (dev->netdev_ops->ndo_get_vf_guid)
+			size += num_vfs * 2 *
+				nla_total_size(sizeof(struct ifla_vf_guid));
 		if (~ext_filter_mask & RTEXT_FILTER_SKIP_STATS) {
 			size += num_vfs *
 				(nla_total_size(0) + /* nest IFLA_VF_STATS */

In either case, the fix looks good to me.

Reviewed-by: Simon Horman <horms@...nel.org>

Powered by blists - more mailing lists