| Message ID | 20240628003253.1694510-3-almasrymina@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Device Memory TCP | expand |
Mina Almasry <almasrymina@google.com> writes: > + - > + name: bind-dmabuf > + attributes: > + - > + name: ifindex > + doc: netdev ifindex to bind the dma-buf to. Minor nit: The series uses a mix of dmabuf and dma-buf but the doc additions (devmem.rst) consistently uses dmabuf. I think it would be helpful to be consistent here and say 'devmem dmabuf' in the docstring to highlight whos dmabuf it is and keep the generated netdev docs in alignment. > + type: u32 > + checks: > + min: 1 > + - > + name: queues > + doc: receive queues to bind the dma-buf to. And here. > + type: nest > + nested-attributes: queue-dmabuf > + multi-attr: true > + - > + name: dmabuf-fd > + doc: dmabuf file descriptor to bind. > + type: u32 > + - > + name: dmabuf-id > + doc: id of the dmabuf binding > + type: u32 > + checks: > + min: 1 > + > > - > name: qstats > @@ -579,6 +618,20 @@ operations: > attributes: > - ifindex > reply: *queue-get-op > + - > + name: bind-rx > + doc: Bind dmabuf to netdev And here. > + attribute-set: bind-dmabuf > + flags: [ admin-perm ] > + do: > + request: > + attributes: > + - ifindex > + - dmabuf-fd > + - queues > + reply: > + attributes: > + - dmabuf-id
On Fri, Jun 28, 2024 at 3:10 AM Donald Hunter <donald.hunter@gmail.com> wrote: > > Mina Almasry <almasrymina@google.com> writes: > > + - > > + name: bind-dmabuf > > + attributes: > > + - > > + name: ifindex > > + doc: netdev ifindex to bind the dma-buf to. > > Minor nit: > > The series uses a mix of dmabuf and dma-buf but the doc additions > (devmem.rst) consistently uses dmabuf. I think it would be helpful to be > consistent here and say 'devmem dmabuf' in the docstring to highlight > whos dmabuf it is and keep the generated netdev docs in alignment. > To be honest, even the dmabuf docs mixes 'dma-buf' and 'dmabuf', to my eye: https://docs.kernel.org/driver-api/dma-buf.html I can edit these docs I'm adding so these are consistent. But on 'devmem dmabuf', not sure to be honest. Technically all dmabufs are supported, even non-devmem ones. I'm not sure non-devmem dmabufs are common at all, the only example I can think of is udmabuf whose primary user is qemu and testing, so it's somewhat implied that the dmabuf is devmem, and even if it isn't, it would be supported. I prefer to keep the docs saying just 'dmabuf' as technically all are supported. Maybe I should add a note about this somewhere in the dedicated docs.
On Mon, 1 Jul 2024 at 20:05, Mina Almasry <almasrymina@google.com> wrote: > > On Fri, Jun 28, 2024 at 3:10 AM Donald Hunter <donald.hunter@gmail.com> wrote: > > > > Mina Almasry <almasrymina@google.com> writes: > > > + - > > > + name: bind-dmabuf > > > + attributes: > > > + - > > > + name: ifindex > > > + doc: netdev ifindex to bind the dma-buf to. > > > > Minor nit: > > > > The series uses a mix of dmabuf and dma-buf but the doc additions > > (devmem.rst) consistently uses dmabuf. I think it would be helpful to be > > consistent here and say 'devmem dmabuf' in the docstring to highlight > > whos dmabuf it is and keep the generated netdev docs in alignment. > > To be honest, even the dmabuf docs mixes 'dma-buf' and 'dmabuf', to my eye: > > https://docs.kernel.org/driver-api/dma-buf.html > > I can edit these docs I'm adding so these are consistent. > > But on 'devmem dmabuf', not sure to be honest. Technically all dmabufs > are supported, even non-devmem ones. I'm not sure non-devmem dmabufs > are common at all, the only example I can think of is udmabuf whose > primary user is qemu and testing, so it's somewhat implied that the > dmabuf is devmem, and even if it isn't, it would be supported. I > prefer to keep the docs saying just 'dmabuf' as technically all are > supported. Maybe I should add a note about this somewhere in the > dedicated docs. That's a fair point. If you could mention it in the docs, that would be great. Thanks, Donald.
On Fri, 28 Jun 2024 00:32:39 +0000 Mina Almasry wrote: > API takes the dma-buf fd as input, and binds it to the netdevice. The > user can specify the rx queues to bind the dma-buf to. > > Suggested-by: Stanislav Fomichev <sdf@google.com> > Signed-off-by: Mina Almasry <almasrymina@google.com> > diff --git a/Documentation/netlink/specs/netdev.yaml b/Documentation/netlink/specs/netdev.yaml > index 959755be4d7f9..899ac0882a098 100644 > --- a/Documentation/netlink/specs/netdev.yaml > +++ b/Documentation/netlink/specs/netdev.yaml > @@ -268,6 +268,45 @@ attribute-sets: > name: napi-id > doc: ID of the NAPI instance which services this queue. > type: u32 > + - > + name: queue-dmabuf > + attributes: > + - > + name: type > + doc: rx or tx queue > + type: u8 > + enum: queue-type > + - > + name: idx > + doc: queue index > + type: u32 u8 is a waste of space, since attrs are rounded up to 4B and we don't use "idx" How about we use a subset of queue attrs? name: queue-id subset-of: queue attributes: - name: id - name: type > + - > + name: bind-dmabuf The naming is a bit too command specific, how about pp-buf ? Or just dmabuf ? > + attributes: > + - > + name: ifindex > + doc: netdev ifindex to bind the dma-buf to. > + type: u32 > + checks: > + min: 1 > + - > + name: queues > + doc: receive queues to bind the dma-buf to. > + type: nest > + nested-attributes: queue-dmabuf > + multi-attr: true > + - > + name: dmabuf-fd > + doc: dmabuf file descriptor to bind. > + type: u32 > + - > + name: dmabuf-id > + doc: id of the dmabuf binding > + type: u32 > + checks: > + min: 1 > + We need some form of introspection. Can we add both in the queue dump and page pool dump some info (dmabuf-id?) to indicate there is a DMABUF bound to the queue / page pool? > - > name: qstats > @@ -579,6 +618,20 @@ operations: > attributes: > - ifindex > reply: *queue-get-op > + - > + name: bind-rx > + doc: Bind dmabuf to netdev > + attribute-set: bind-dmabuf > + flags: [ admin-perm ] > + do: > + request: > + attributes: > + - ifindex > + - dmabuf-fd > + - queues > + reply: > + attributes: > + - dmabuf-id The ops end up getting rendered as an enum, so the ordering matters. You can't insert in the middle without breaking uAPI. For attribute sets (which you also added before qstat) it technically doesn't matter but would be good to have them in order to match ops. > diff --git a/include/uapi/linux/netdev.h b/include/uapi/linux/netdev.h > index 43742ac5b00da..190a504a62358 100644 > --- a/include/uapi/linux/netdev.h > +++ b/include/uapi/linux/netdev.h > @@ -136,6 +136,24 @@ enum { > NETDEV_A_QUEUE_MAX = (__NETDEV_A_QUEUE_MAX - 1) > }; > > +enum { > + NETDEV_A_QUEUE_DMABUF_TYPE = 1, > + NETDEV_A_QUEUE_DMABUF_IDX, > + > + __NETDEV_A_QUEUE_DMABUF_MAX, > + NETDEV_A_QUEUE_DMABUF_MAX = (__NETDEV_A_QUEUE_DMABUF_MAX - 1) > +}; > + > +enum { > + NETDEV_A_BIND_DMABUF_IFINDEX = 1, > + NETDEV_A_BIND_DMABUF_QUEUES, > + NETDEV_A_BIND_DMABUF_DMABUF_FD, > + NETDEV_A_BIND_DMABUF_DMABUF_ID, This does look kinda repetitive, maybe let's drop the dmabuf from attr names? > + __NETDEV_A_BIND_DMABUF_MAX, > + NETDEV_A_BIND_DMABUF_MAX = (__NETDEV_A_BIND_DMABUF_MAX - 1) > +};
diff --git a/Documentation/netlink/specs/netdev.yaml b/Documentation/netlink/specs/netdev.yaml index 959755be4d7f9..899ac0882a098 100644 --- a/Documentation/netlink/specs/netdev.yaml +++ b/Documentation/netlink/specs/netdev.yaml @@ -268,6 +268,45 @@ attribute-sets: name: napi-id doc: ID of the NAPI instance which services this queue. type: u32 + - + name: queue-dmabuf + attributes: + - + name: type + doc: rx or tx queue + type: u8 + enum: queue-type + - + name: idx + doc: queue index + type: u32 + + - + name: bind-dmabuf + attributes: + - + name: ifindex + doc: netdev ifindex to bind the dma-buf to. + type: u32 + checks: + min: 1 + - + name: queues + doc: receive queues to bind the dma-buf to. + type: nest + nested-attributes: queue-dmabuf + multi-attr: true + - + name: dmabuf-fd + doc: dmabuf file descriptor to bind. + type: u32 + - + name: dmabuf-id + doc: id of the dmabuf binding + type: u32 + checks: + min: 1 + - name: qstats @@ -579,6 +618,20 @@ operations: attributes: - ifindex reply: *queue-get-op + - + name: bind-rx + doc: Bind dmabuf to netdev + attribute-set: bind-dmabuf + flags: [ admin-perm ] + do: + request: + attributes: + - ifindex + - dmabuf-fd + - queues + reply: + attributes: + - dmabuf-id - name: napi-get doc: Get information about NAPI instances configured on the system. diff --git a/include/uapi/linux/netdev.h b/include/uapi/linux/netdev.h index 43742ac5b00da..190a504a62358 100644 --- a/include/uapi/linux/netdev.h +++ b/include/uapi/linux/netdev.h @@ -136,6 +136,24 @@ enum { NETDEV_A_QUEUE_MAX = (__NETDEV_A_QUEUE_MAX - 1) }; +enum { + NETDEV_A_QUEUE_DMABUF_TYPE = 1, + NETDEV_A_QUEUE_DMABUF_IDX, + + __NETDEV_A_QUEUE_DMABUF_MAX, + NETDEV_A_QUEUE_DMABUF_MAX = (__NETDEV_A_QUEUE_DMABUF_MAX - 1) +}; + +enum { + NETDEV_A_BIND_DMABUF_IFINDEX = 1, + NETDEV_A_BIND_DMABUF_QUEUES, + NETDEV_A_BIND_DMABUF_DMABUF_FD, + NETDEV_A_BIND_DMABUF_DMABUF_ID, + + __NETDEV_A_BIND_DMABUF_MAX, + NETDEV_A_BIND_DMABUF_MAX = (__NETDEV_A_BIND_DMABUF_MAX - 1) +}; + enum { NETDEV_A_QSTATS_IFINDEX = 1, NETDEV_A_QSTATS_QUEUE_TYPE, @@ -184,6 +202,7 @@ enum { NETDEV_CMD_PAGE_POOL_CHANGE_NTF, NETDEV_CMD_PAGE_POOL_STATS_GET, NETDEV_CMD_QUEUE_GET, + NETDEV_CMD_BIND_RX, NETDEV_CMD_NAPI_GET, NETDEV_CMD_QSTATS_GET, diff --git a/net/core/netdev-genl-gen.c b/net/core/netdev-genl-gen.c index 8350a0afa9ec7..9acd0d893765a 100644 --- a/net/core/netdev-genl-gen.c +++ b/net/core/netdev-genl-gen.c @@ -27,6 +27,11 @@ const struct nla_policy netdev_page_pool_info_nl_policy[NETDEV_A_PAGE_POOL_IFIND [NETDEV_A_PAGE_POOL_IFINDEX] = NLA_POLICY_FULL_RANGE(NLA_U32, &netdev_a_page_pool_ifindex_range), }; +const struct nla_policy netdev_queue_dmabuf_nl_policy[NETDEV_A_QUEUE_DMABUF_IDX + 1] = { + [NETDEV_A_QUEUE_DMABUF_TYPE] = NLA_POLICY_MAX(NLA_U8, 1), + [NETDEV_A_QUEUE_DMABUF_IDX] = { .type = NLA_U32, }, +}; + /* NETDEV_CMD_DEV_GET - do */ static const struct nla_policy netdev_dev_get_nl_policy[NETDEV_A_DEV_IFINDEX + 1] = { [NETDEV_A_DEV_IFINDEX] = NLA_POLICY_MIN(NLA_U32, 1), @@ -58,6 +63,13 @@ static const struct nla_policy netdev_queue_get_dump_nl_policy[NETDEV_A_QUEUE_IF [NETDEV_A_QUEUE_IFINDEX] = NLA_POLICY_MIN(NLA_U32, 1), }; +/* NETDEV_CMD_BIND_RX - do */ +static const struct nla_policy netdev_bind_rx_nl_policy[NETDEV_A_BIND_DMABUF_DMABUF_FD + 1] = { + [NETDEV_A_BIND_DMABUF_IFINDEX] = NLA_POLICY_MIN(NLA_U32, 1), + [NETDEV_A_BIND_DMABUF_DMABUF_FD] = { .type = NLA_U32, }, + [NETDEV_A_BIND_DMABUF_QUEUES] = NLA_POLICY_NESTED(netdev_queue_dmabuf_nl_policy), +}; + /* NETDEV_CMD_NAPI_GET - do */ static const struct nla_policy netdev_napi_get_do_nl_policy[NETDEV_A_NAPI_ID + 1] = { [NETDEV_A_NAPI_ID] = { .type = NLA_U32, }, @@ -130,6 +142,13 @@ static const struct genl_split_ops netdev_nl_ops[] = { .maxattr = NETDEV_A_QUEUE_IFINDEX, .flags = GENL_CMD_CAP_DUMP, }, + { + .cmd = NETDEV_CMD_BIND_RX, + .doit = netdev_nl_bind_rx_doit, + .policy = netdev_bind_rx_nl_policy, + .maxattr = NETDEV_A_BIND_DMABUF_DMABUF_FD, + .flags = GENL_ADMIN_PERM | GENL_CMD_CAP_DO, + }, { .cmd = NETDEV_CMD_NAPI_GET, .doit = netdev_nl_napi_get_doit, diff --git a/net/core/netdev-genl-gen.h b/net/core/netdev-genl-gen.h index 4db40fd5b4a9e..ca5a0983f2834 100644 --- a/net/core/netdev-genl-gen.h +++ b/net/core/netdev-genl-gen.h @@ -13,6 +13,7 @@ /* Common nested types */ extern const struct nla_policy netdev_page_pool_info_nl_policy[NETDEV_A_PAGE_POOL_IFINDEX + 1]; +extern const struct nla_policy netdev_queue_dmabuf_nl_policy[NETDEV_A_QUEUE_DMABUF_IDX + 1]; int netdev_nl_dev_get_doit(struct sk_buff *skb, struct genl_info *info); int netdev_nl_dev_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb); @@ -26,6 +27,7 @@ int netdev_nl_page_pool_stats_get_dumpit(struct sk_buff *skb, int netdev_nl_queue_get_doit(struct sk_buff *skb, struct genl_info *info); int netdev_nl_queue_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb); +int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info); int netdev_nl_napi_get_doit(struct sk_buff *skb, struct genl_info *info); int netdev_nl_napi_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb); int netdev_nl_qstats_get_dumpit(struct sk_buff *skb, diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c index 05f9515d2c05c..2d726e65211dd 100644 --- a/net/core/netdev-genl.c +++ b/net/core/netdev-genl.c @@ -721,6 +721,12 @@ int netdev_nl_qstats_get_dumpit(struct sk_buff *skb, return err; } +/* Stub */ +int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info) +{ + return 0; +} + static int netdev_genl_netdevice_event(struct notifier_block *nb, unsigned long event, void *ptr) { diff --git a/tools/include/uapi/linux/netdev.h b/tools/include/uapi/linux/netdev.h index 43742ac5b00da..190a504a62358 100644 --- a/tools/include/uapi/linux/netdev.h +++ b/tools/include/uapi/linux/netdev.h @@ -136,6 +136,24 @@ enum { NETDEV_A_QUEUE_MAX = (__NETDEV_A_QUEUE_MAX - 1) }; +enum { + NETDEV_A_QUEUE_DMABUF_TYPE = 1, + NETDEV_A_QUEUE_DMABUF_IDX, + + __NETDEV_A_QUEUE_DMABUF_MAX, + NETDEV_A_QUEUE_DMABUF_MAX = (__NETDEV_A_QUEUE_DMABUF_MAX - 1) +}; + +enum { + NETDEV_A_BIND_DMABUF_IFINDEX = 1, + NETDEV_A_BIND_DMABUF_QUEUES, + NETDEV_A_BIND_DMABUF_DMABUF_FD, + NETDEV_A_BIND_DMABUF_DMABUF_ID, + + __NETDEV_A_BIND_DMABUF_MAX, + NETDEV_A_BIND_DMABUF_MAX = (__NETDEV_A_BIND_DMABUF_MAX - 1) +}; + enum { NETDEV_A_QSTATS_IFINDEX = 1, NETDEV_A_QSTATS_QUEUE_TYPE, @@ -184,6 +202,7 @@ enum { NETDEV_CMD_PAGE_POOL_CHANGE_NTF, NETDEV_CMD_PAGE_POOL_STATS_GET, NETDEV_CMD_QUEUE_GET, + NETDEV_CMD_BIND_RX, NETDEV_CMD_NAPI_GET, NETDEV_CMD_QSTATS_GET,
API takes the dma-buf fd as input, and binds it to the netdevice. The user can specify the rx queues to bind the dma-buf to. Suggested-by: Stanislav Fomichev <sdf@google.com> Signed-off-by: Mina Almasry <almasrymina@google.com> --- v7: - Use flags: [ admin-perm ] instead of a CAP_NET_ADMIN check. Changes in v1: - Add rx-queue-type to distingish rx from tx (Jakub) - Return dma-buf ID from netlink API (David, Stan) Changes in RFC-v3: - Support binding multiple rx rx-queues --- Documentation/netlink/specs/netdev.yaml | 53 +++++++++++++++++++++++++ include/uapi/linux/netdev.h | 19 +++++++++ net/core/netdev-genl-gen.c | 19 +++++++++ net/core/netdev-genl-gen.h | 2 + net/core/netdev-genl.c | 6 +++ tools/include/uapi/linux/netdev.h | 19 +++++++++ 6 files changed, 118 insertions(+)