Thread (11 messages) 11 messages, 3 authors, 2026-03-16

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.

---

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help