Re: [PATCH net-next v15 02/14] net: netdev netlink api to bind dma-buf to a net device

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)

+};