Re: [PATCH iproute2-next 0/4] Introduce FRMR pools
From: David Ahern <hidden>
Date: 2026-03-07 01:45:41
Also in:
linux-rdma
On 3/2/26 8:51 AM, Chiara Meiohas wrote:
From Michael: This series adds support for managing Fast Registration Memory Region (FRMR) pools in rdma tool, enabling users to monitor and configure FRMR pool behavior.
Claude has some quibbles with the patches: 1. Type mismatch: RDMA_NLDEV_ATTR_RES_FRMR_POOL_AGING_PERIOD In rdma/include/uapi/rdma/rdma_netlink.h:
+ RDMA_NLDEV_ATTR_RES_FRMR_POOL_AGING_PERIOD, /* u64 */
In rdma/res-frmr-pools.c (res_frmr_pools_one_set_aging):
+ uint32_t aging_period; + mnl_attr_put_u32(rd->nlh,
RDMA_NLDEV_ATTR_RES_FRMR_POOL_AGING_PERIOD,
+ aging_period);
The uapi header documents this attribute as u64, but the code declares a uint32_t variable and sends it with mnl_attr_put_u32(). This mismatch means userspace will send a 4-byte attribute when the kernel expects 8 bytes, leading to either a parse error or silent data truncation. Fix: use uint64_t and mnl_attr_put_u64() to match the uapi annotation, or correct the uapi comment to reflect the intended wire type. --- 2. Type mismatch: RDMA_NLDEV_ATTR_RES_FRMR_POOL_PINNED In rdma/include/uapi/rdma/rdma_netlink.h:
+ RDMA_NLDEV_ATTR_RES_FRMR_POOL_PINNED, /* u8 */
In rdma/res-frmr-pools.c (res_frmr_pools_line):
+ uint32_t queue_handles = 0, pinned_handles = 0; + if (nla_line[RDMA_NLDEV_ATTR_RES_FRMR_POOL_PINNED]) + pinned_handles = mnl_attr_get_u32( + nla_line[RDMA_NLDEV_ATTR_RES_FRMR_POOL_PINNED]);
In rdma/res-frmr-pools.c (res_frmr_pools_one_set_pinned):
+ uint32_t pinned_value; + mnl_attr_put_u32(rd->nlh, RDMA_NLDEV_ATTR_RES_FRMR_POOL_PINNED, + pinned_value);
The uapi header marks this as u8, but the code reads and writes it as u32 using mnl_attr_get_u32() and mnl_attr_put_u32(). The type used in userspace must match the kernel's attribute definition. Fix: decide on the actual wire type and make the uapi comment, variable declaration, and mnl accessor consistent. If u32 is correct, change the comment to /* u32 */; if u8 is correct, use uint8_t and mnl_attr_put_u8(). --- 3. Declared but unimplemented: res_frmr_pools_idx_parse_cb In rdma/res.h:
+int res_frmr_pools_idx_parse_cb(const struct nlmsghdr *nlh, void *data);
This function is declared alongside all the other *_idx_parse_cb symbols, but there is no corresponding definition in rdma/res-frmr-pools.c. Because res_frmr_pools uses id=0 in RES_FUNC:
+RES_FUNC(res_frmr_pools, RDMA_NLDEV_CMD_RES_FRMR_POOLS_GET, + frmr_pools_valid_filters, true, 0);
the macro's idx path is never triggered, so this won't cause a link error in practice. However, the declaration is misleading and inconsistent with the other resource types where the idx callback exists because they pass a non-zero id. Either implement the function (if per-index lookup is planned) or remove the declaration from res.h. ---