[PATCH net-next v2 1/2] net/smc: introduce statistics for allocated ringbufs of link group

Wen Gu posted 2 patches 1 year, 4 months ago
There is a newer version of this series
[PATCH net-next v2 1/2] net/smc: introduce statistics for allocated ringbufs of link group
Posted by Wen Gu 1 year, 4 months ago
Currently we have the statistics on sndbuf/RMB sizes of all connections
that have ever been on the link group, namely smc_stats_memsize. However
these statistics are incremental and since the ringbufs of link group
are allowed to be reused, we cannot know the actual allocated buffers
through these. So here introduces the statistic on actual allocated
ringbufs of the link group, it will be incremented when a new ringbuf is
added into buf_list and decremented when it is deleted from buf_list.

Signed-off-by: Wen Gu <guwen@linux.alibaba.com>
---
 include/uapi/linux/smc.h |  4 ++++
 net/smc/smc_core.c       | 50 ++++++++++++++++++++++++++++++++++++----
 net/smc/smc_core.h       |  2 ++
 3 files changed, 52 insertions(+), 4 deletions(-)

diff --git a/include/uapi/linux/smc.h b/include/uapi/linux/smc.h
index b531e3ef011a..d27b8dc50f90 100644
--- a/include/uapi/linux/smc.h
+++ b/include/uapi/linux/smc.h
@@ -127,6 +127,8 @@ enum {
 	SMC_NLA_LGR_R_NET_COOKIE,	/* u64 */
 	SMC_NLA_LGR_R_PAD,		/* flag */
 	SMC_NLA_LGR_R_BUF_TYPE,		/* u8 */
+	SMC_NLA_LGR_R_SNDBUF_ALLOC,	/* u64 */
+	SMC_NLA_LGR_R_RMB_ALLOC,	/* u64 */
 	__SMC_NLA_LGR_R_MAX,
 	SMC_NLA_LGR_R_MAX = __SMC_NLA_LGR_R_MAX - 1
 };
@@ -162,6 +164,8 @@ enum {
 	SMC_NLA_LGR_D_V2_COMMON,	/* nest */
 	SMC_NLA_LGR_D_EXT_GID,		/* u64 */
 	SMC_NLA_LGR_D_PEER_EXT_GID,	/* u64 */
+	SMC_NLA_LGR_D_SNDBUF_ALLOC,	/* u64 */
+	SMC_NLA_LGR_D_DMB_ALLOC,	/* u64 */
 	__SMC_NLA_LGR_D_MAX,
 	SMC_NLA_LGR_D_MAX = __SMC_NLA_LGR_D_MAX - 1
 };
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index 71fb334d8234..b1ff44d72fe4 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -221,6 +221,35 @@ static void smc_lgr_unregister_conn(struct smc_connection *conn)
 	write_unlock_bh(&lgr->conns_lock);
 }
 
+static void smc_lgr_buf_list_add(struct smc_link_group *lgr,
+				 bool is_rmb,
+				 struct list_head *buf_list,
+				 struct smc_buf_desc *buf_desc)
+{
+	list_add(&buf_desc->list, buf_list);
+	if (is_rmb) {
+		lgr->alloc_rmbs += buf_desc->len;
+		lgr->alloc_rmbs +=
+			lgr->is_smcd ? sizeof(struct smcd_cdc_msg) : 0;
+	} else {
+		lgr->alloc_sndbufs += buf_desc->len;
+	}
+}
+
+static void smc_lgr_buf_list_del(struct smc_link_group *lgr,
+				 bool is_rmb,
+				 struct smc_buf_desc *buf_desc)
+{
+	list_del(&buf_desc->list);
+	if (is_rmb) {
+		lgr->alloc_rmbs -= buf_desc->len;
+		lgr->alloc_rmbs -=
+			lgr->is_smcd ? sizeof(struct smcd_cdc_msg) : 0;
+	} else {
+		lgr->alloc_sndbufs -= buf_desc->len;
+	}
+}
+
 int smc_nl_get_sys_info(struct sk_buff *skb, struct netlink_callback *cb)
 {
 	struct smc_nl_dmp_ctx *cb_ctx = smc_nl_dmp_ctx(cb);
@@ -363,6 +392,12 @@ static int smc_nl_fill_lgr(struct smc_link_group *lgr,
 	smc_target[SMC_MAX_PNETID_LEN] = 0;
 	if (nla_put_string(skb, SMC_NLA_LGR_R_PNETID, smc_target))
 		goto errattr;
+	if (nla_put_u64_64bit(skb, SMC_NLA_LGR_R_SNDBUF_ALLOC,
+			      lgr->alloc_sndbufs, SMC_NLA_LGR_R_PAD))
+		goto errattr;
+	if (nla_put_u64_64bit(skb, SMC_NLA_LGR_R_RMB_ALLOC,
+			      lgr->alloc_rmbs, SMC_NLA_LGR_R_PAD))
+		goto errattr;
 	if (lgr->smc_version > SMC_V1) {
 		v2_attrs = nla_nest_start(skb, SMC_NLA_LGR_R_V2_COMMON);
 		if (!v2_attrs)
@@ -541,6 +576,12 @@ static int smc_nl_fill_smcd_lgr(struct smc_link_group *lgr,
 		goto errattr;
 	if (nla_put_u32(skb, SMC_NLA_LGR_D_CHID, smc_ism_get_chid(lgr->smcd)))
 		goto errattr;
+	if (nla_put_u64_64bit(skb, SMC_NLA_LGR_D_SNDBUF_ALLOC,
+			      lgr->alloc_sndbufs, SMC_NLA_LGR_D_PAD))
+		goto errattr;
+	if (nla_put_u64_64bit(skb, SMC_NLA_LGR_D_DMB_ALLOC,
+			      lgr->alloc_rmbs, SMC_NLA_LGR_D_PAD))
+		goto errattr;
 	memcpy(smc_pnet, lgr->smcd->pnetid, SMC_MAX_PNETID_LEN);
 	smc_pnet[SMC_MAX_PNETID_LEN] = 0;
 	if (nla_put_string(skb, SMC_NLA_LGR_D_PNETID, smc_pnet))
@@ -1138,7 +1179,7 @@ static void smcr_buf_unuse(struct smc_buf_desc *buf_desc, bool is_rmb,
 		lock = is_rmb ? &lgr->rmbs_lock :
 				&lgr->sndbufs_lock;
 		down_write(lock);
-		list_del(&buf_desc->list);
+		smc_lgr_buf_list_del(lgr, is_rmb, buf_desc);
 		up_write(lock);
 
 		smc_buf_free(lgr, is_rmb, buf_desc);
@@ -1377,7 +1418,7 @@ static void __smc_lgr_free_bufs(struct smc_link_group *lgr, bool is_rmb)
 			buf_list = &lgr->sndbufs[i];
 		list_for_each_entry_safe(buf_desc, bf_desc, buf_list,
 					 list) {
-			list_del(&buf_desc->list);
+			smc_lgr_buf_list_del(lgr, is_rmb, buf_desc);
 			smc_buf_free(lgr, is_rmb, buf_desc);
 		}
 	}
@@ -2414,7 +2455,7 @@ static int __smc_buf_create(struct smc_sock *smc, bool is_smcd, bool is_rmb)
 		SMC_STAT_RMB_SIZE(smc, is_smcd, is_rmb, bufsize);
 		buf_desc->used = 1;
 		down_write(lock);
-		list_add(&buf_desc->list, buf_list);
+		smc_lgr_buf_list_add(lgr, is_rmb, buf_list, buf_desc);
 		up_write(lock);
 		break; /* found */
 	}
@@ -2496,7 +2537,8 @@ int smc_buf_create(struct smc_sock *smc, bool is_smcd)
 	rc = __smc_buf_create(smc, is_smcd, true);
 	if (rc && smc->conn.sndbuf_desc) {
 		down_write(&smc->conn.lgr->sndbufs_lock);
-		list_del(&smc->conn.sndbuf_desc->list);
+		smc_lgr_buf_list_del(smc->conn.lgr, false,
+				     smc->conn.sndbuf_desc);
 		up_write(&smc->conn.lgr->sndbufs_lock);
 		smc_buf_free(smc->conn.lgr, false, smc->conn.sndbuf_desc);
 		smc->conn.sndbuf_desc = NULL;
diff --git a/net/smc/smc_core.h b/net/smc/smc_core.h
index d93cf51dbd7c..0db4e5f79ac4 100644
--- a/net/smc/smc_core.h
+++ b/net/smc/smc_core.h
@@ -281,6 +281,8 @@ struct smc_link_group {
 	struct rw_semaphore	sndbufs_lock;	/* protects tx buffers */
 	struct list_head	rmbs[SMC_RMBE_SIZES];	/* rx buffers */
 	struct rw_semaphore	rmbs_lock;	/* protects rx buffers */
+	u64			alloc_sndbufs;	/* stats of tx buffers */
+	u64			alloc_rmbs;	/* stats of rx buffers */
 
 	u8			id[SMC_LGR_ID_SIZE];	/* unique lgr id */
 	struct delayed_work	free_work;	/* delayed freeing of an lgr */
-- 
2.32.0.3.g01195cf9f
Re: [PATCH net-next v2 1/2] net/smc: introduce statistics for allocated ringbufs of link group
Posted by Jakub Kicinski 1 year, 4 months ago
On Wed,  7 Aug 2024 15:59:38 +0800 Wen Gu wrote:
> +	if (nla_put_u64_64bit(skb, SMC_NLA_LGR_R_SNDBUF_ALLOC,
> +			      lgr->alloc_sndbufs, SMC_NLA_LGR_R_PAD))

nla_put_uint()
-- 
pw-bot: cr
Re: [PATCH net-next v2 1/2] net/smc: introduce statistics for allocated ringbufs of link group
Posted by Wen Gu 1 year, 4 months ago

On 2024/8/13 08:41, Jakub Kicinski wrote:
> On Wed,  7 Aug 2024 15:59:38 +0800 Wen Gu wrote:
>> +	if (nla_put_u64_64bit(skb, SMC_NLA_LGR_R_SNDBUF_ALLOC,
>> +			      lgr->alloc_sndbufs, SMC_NLA_LGR_R_PAD))
> 
> nla_put_uint()

Hi, Jakub. Thank you for reminder.

I read the commit log and learned the advantages of this helper.
But it seems that the support for corresponding user-space helpers
hasn't kept up yet, e.g. can't find a helper like nla_get_uint in
latest libnl.
Re: [PATCH net-next v2 1/2] net/smc: introduce statistics for allocated ringbufs of link group
Posted by Jakub Kicinski 1 year, 4 months ago
On Tue, 13 Aug 2024 17:55:17 +0800 Wen Gu wrote:
> On 2024/8/13 08:41, Jakub Kicinski wrote:
> > On Wed,  7 Aug 2024 15:59:38 +0800 Wen Gu wrote:  
> >> +	if (nla_put_u64_64bit(skb, SMC_NLA_LGR_R_SNDBUF_ALLOC,
> >> +			      lgr->alloc_sndbufs, SMC_NLA_LGR_R_PAD))  
> > 
> > nla_put_uint()  
> 
> Hi, Jakub. Thank you for reminder.
> 
> I read the commit log and learned the advantages of this helper.
> But it seems that the support for corresponding user-space helpers
> hasn't kept up yet, e.g. can't find a helper like nla_get_uint in
> latest libnl.

Add it, then.
Re: [PATCH net-next v2 1/2] net/smc: introduce statistics for allocated ringbufs of link group
Posted by Wen Gu 1 year, 4 months ago

On 2024/8/13 22:40, Jakub Kicinski wrote:
> On Tue, 13 Aug 2024 17:55:17 +0800 Wen Gu wrote:
>> On 2024/8/13 08:41, Jakub Kicinski wrote:
>>> On Wed,  7 Aug 2024 15:59:38 +0800 Wen Gu wrote:
>>>> +	if (nla_put_u64_64bit(skb, SMC_NLA_LGR_R_SNDBUF_ALLOC,
>>>> +			      lgr->alloc_sndbufs, SMC_NLA_LGR_R_PAD))
>>>
>>> nla_put_uint()
>>
>> Hi, Jakub. Thank you for reminder.
>>
>> I read the commit log and learned the advantages of this helper.
>> But it seems that the support for corresponding user-space helpers
>> hasn't kept up yet, e.g. can't find a helper like nla_get_uint in
>> latest libnl.
> 
> Add it, then.

OK. So I guess we should use nla_put_uint for all 64bit cases from now on?
Re: [PATCH net-next v2 1/2] net/smc: introduce statistics for allocated ringbufs of link group
Posted by Jakub Kicinski 1 year, 4 months ago
On Wed, 14 Aug 2024 11:10:33 +0800 Wen Gu wrote:
> >> Hi, Jakub. Thank you for reminder.
> >>
> >> I read the commit log and learned the advantages of this helper.
> >> But it seems that the support for corresponding user-space helpers
> >> hasn't kept up yet, e.g. can't find a helper like nla_get_uint in
> >> latest libnl.  
> > 
> > Add it, then.  
> 
> OK. So I guess we should use nla_put_uint for all 64bit cases from now on?

Yes, actually I'd go further and say all ints should use the variable
size types, unless we are guaranteed they will never be larger than 32b
(e.g. ifindex is used in many places in uAPI as 31b already, we can't
grow it), or the field is explicitly fixed size (e.g. protocol header
fields defined by standards).
Re: [PATCH net-next v2 1/2] net/smc: introduce statistics for allocated ringbufs of link group
Posted by Ido Schimmel 1 year, 4 months ago
On Tue, Aug 13, 2024 at 07:40:42AM -0700, Jakub Kicinski wrote:
> On Tue, 13 Aug 2024 17:55:17 +0800 Wen Gu wrote:
> > On 2024/8/13 08:41, Jakub Kicinski wrote:
> > > On Wed,  7 Aug 2024 15:59:38 +0800 Wen Gu wrote:  
> > >> +	if (nla_put_u64_64bit(skb, SMC_NLA_LGR_R_SNDBUF_ALLOC,
> > >> +			      lgr->alloc_sndbufs, SMC_NLA_LGR_R_PAD))  
> > > 
> > > nla_put_uint()  
> > 
> > Hi, Jakub. Thank you for reminder.
> > 
> > I read the commit log and learned the advantages of this helper.
> > But it seems that the support for corresponding user-space helpers
> > hasn't kept up yet, e.g. can't find a helper like nla_get_uint in
> > latest libnl.
> 
> Add it, then.

Danielle added one to libmnl:

https://git.netfilter.org/libmnl/commit/?id=102942be401a99943b2c68981b238dadfa788f2d

Intention is to use it in ethtool once it appears in a released version
of libmnl.
Re: [PATCH net-next v2 1/2] net/smc: introduce statistics for allocated ringbufs of link group
Posted by Wen Gu 1 year, 4 months ago

On 2024/8/13 23:13, Ido Schimmel wrote:
> On Tue, Aug 13, 2024 at 07:40:42AM -0700, Jakub Kicinski wrote:
>> On Tue, 13 Aug 2024 17:55:17 +0800 Wen Gu wrote:
>>> On 2024/8/13 08:41, Jakub Kicinski wrote:
>>>> On Wed,  7 Aug 2024 15:59:38 +0800 Wen Gu wrote:
>>>>> +	if (nla_put_u64_64bit(skb, SMC_NLA_LGR_R_SNDBUF_ALLOC,
>>>>> +			      lgr->alloc_sndbufs, SMC_NLA_LGR_R_PAD))
>>>>
>>>> nla_put_uint()
>>>
>>> Hi, Jakub. Thank you for reminder.
>>>
>>> I read the commit log and learned the advantages of this helper.
>>> But it seems that the support for corresponding user-space helpers
>>> hasn't kept up yet, e.g. can't find a helper like nla_get_uint in
>>> latest libnl.
>>
>> Add it, then.
> 
> Danielle added one to libmnl:
> 
> https://git.netfilter.org/libmnl/commit/?id=102942be401a99943b2c68981b238dadfa788f2d
> 
> Intention is to use it in ethtool once it appears in a released version
> of libmnl.

Thanks, that is a good example.
Re: [PATCH net-next v2 1/2] net/smc: introduce statistics for allocated ringbufs of link group
Posted by Jakub Kicinski 1 year, 4 months ago
On Wed, 14 Aug 2024 11:12:41 +0800 Wen Gu wrote:
> > Danielle added one to libmnl:
> > 
> > https://git.netfilter.org/libmnl/commit/?id=102942be401a99943b2c68981b238dadfa788f2d
> > 
> > Intention is to use it in ethtool once it appears in a released version
> > of libmnl.  
> 
> Thanks, that is a good example.

FWIW - technically the kernel version of a uint / sint are only either
32b or 64b, because smaller types get padded to 4bytes in netlink
messages, anyway. But doesn't hurt for the get. For the put make sure
you don't narrow down too much.
Re: [PATCH net-next v2 1/2] net/smc: introduce statistics for allocated ringbufs of link group
Posted by Wen Gu 1 year, 4 months ago

On 2024/8/14 22:48, Jakub Kicinski wrote:
> On Wed, 14 Aug 2024 11:12:41 +0800 Wen Gu wrote:
>>> Danielle added one to libmnl:
>>>
>>> https://git.netfilter.org/libmnl/commit/?id=102942be401a99943b2c68981b238dadfa788f2d
>>>
>>> Intention is to use it in ethtool once it appears in a released version
>>> of libmnl.
>>
>> Thanks, that is a good example.
> 
> FWIW - technically the kernel version of a uint / sint are only either
> 32b or 64b, because smaller types get padded to 4bytes in netlink
> messages, anyway. But doesn't hurt for the get. For the put make sure
> you don't narrow down too much.

Understand. Thanks.