On Friday, 10 October 2025 at 17:46:25 Matias Ezequiel Vara Larsen
<[email protected]> wrote:
> On Thu, Sep 11, 2025 at 10:59:40PM +0200, Francesco Valla wrote:
> > Hello Mikhail, Harald,
> >
> > hoping there will be a v6 of this patch soon, a few comments:
> >
>
> I am working on the v6 by addressing the comments in this thread.
>
> > On Monday, 8 January 2024 at 14:10:35 Mikhail Golubev-Ciuchea
> > <[email protected]> wrote:
> >
> > [...]
> >
> > > +
> > > +/* virtio_can private data structure */
> > > +struct virtio_can_priv {
> > > + struct can_priv can; /* must be the first member */
> > > + /* NAPI for RX messages */
> > > + struct napi_struct napi;
> > > + /* NAPI for TX messages */
> > > + struct napi_struct napi_tx;
> > > + /* The network device we're associated with */
> > > + struct net_device *dev;
> > > + /* The virtio device we're associated with */
> > > + struct virtio_device *vdev;
> > > + /* The virtqueues */
> > > + struct virtqueue *vqs[VIRTIO_CAN_QUEUE_COUNT];
> > > + /* I/O callback function pointers for the virtqueues */
> > > + vq_callback_t *io_callbacks[VIRTIO_CAN_QUEUE_COUNT];
> > > + /* Lock for TX operations */
> > > + spinlock_t tx_lock;
> > > + /* Control queue lock. Defensive programming, may be not needed */
> > > + struct mutex ctrl_lock;
> > > + /* Wait for control queue processing without polling */
> > > + struct completion ctrl_done;
> > > + /* List of virtio CAN TX message */
> > > + struct list_head tx_list;
> > > + /* Array of receive queue messages */
> > > + struct virtio_can_rx rpkt[128];
> >
> > This array should probably be allocated dynamically at probe - maybe
> > using a module parameter instead of a hardcoded value as length?
> >
>
> If I allocate this array in probe(), I would not know sdu[] in advance
> if I defined it as a flexible array. That made me wonder: can sdu[] be
> defined as flexible array for rx?
>
> Thanks.
>
One thing that can be done is to define struct virtio_can_rx as:
struct virtio_can_rx {
#define VIRTIO_CAN_RX 0x0101
__le16 msg_type;
__le16 length; /* 0..8 CC, 0..64 CAN-FD, 0..2048 CAN-XL, 12 bits */
__u8 reserved_classic_dlc; /* If CAN classic length = 8 then DLC can be
8..15 */
__u8 padding;
__le16 reserved_xl_priority; /* May be needed for CAN XL priority */
__le32 flags;
__le32 can_id;
__u8 sdu[] __counted_by(length);
};
and then allocate the rpkt[] array using the maximum length for SDU:
priv->rpkt = kcalloc(num_rx_buffers,
sizeof(struct virtio_can_rx) + VIRTIO_CAN_MAX_DLEN,
GFP_KERNEL);
In this way, the size of each member of rpkt[] is known and is thus
suitable for virtio_can_populate_vqs().
Regards,
Francesco
