Re: [RFC PATCH v5 3/5] can: dev: add CAN XL support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 22.07.2022 17:33:08, Vincent Mailhol wrote:
> On Fri. 22 Jul. 2022 at 16:27, Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx> wrote:
> > On 22.07.2022 12:20:43, Vincent Mailhol wrote:
> > > I do not buy your argument that you can not do sizeof(struct
> > > canxl_frame) because of naming.
> > >
> > > With a flexible array member, I can do:
> > >
> > > struct canxl_frame {
> > >          canid_t prio;  /* 11 bit priority for arbitration (canid_t) */
> > >          __u8    flags; /* additional flags for CAN XL */
> > >          __u8    sdt;   /* SDU (service data unit) type */
> > >          __u16   len;   /* frame payload length in byte */
> > >          __u32   af;    /* acceptance field */
> > >          __u8    data[];
> > > };
> > >
> > > void foo (int s)
> > > {
> > >          struct canxl_frame cxl_hdr = { 0 };
> > >          struct canxl_frame *cxl1, *cxl2;
> > >          size_t cxl2_data_len, cxl2_frame_len;
> > >
> > >          /* read header */
> > >          assert(read(s, &cxl_hdr, sizeof(cxl_hdr)) == sizeof(cxl_hdr));
> > >          cxl1 = malloc(sizeof(*cxl1) + cxl_hdr.len);
> > >          memcpy(cxl1, &cxl_hdr, sizeof(cxl_hdr));
> > >          /* read remaining data */
> > >          assert(read(s, cxl1->data, cxl1->len) == cxl1->len);
> >
> > For performance reasons you probable don't want to split the read of a
> > single CAN frame over 2 read() syscalls.
> 
> ACK. I wrote the code to illustrate how to do header manipulations.
> The goal of this example was not to be optimal but to show off how
> sizeof(struct canxl_frame) could be used. Sorry if the example was not
> natural and a bit too forced.
> 
> > Also the CAN_RAW doesn't
> > support "split"-read now, but could be extended.
> 
> Interesting! I naively thought that split read() was handled at a
> higher level of the socket API. I did not know that it was per
> protocol.

The CAN_RAW protocol implements raw_recvmsg():

| https://elixir.bootlin.com/linux/latest/source/net/can/raw.c#L843

> It could make sense to allow such split read() for CANXL.

Then we have to track the number of already read bytes inside the
socket. The POSIX API offers some interesting flags to recvmsg(). With
MSG_PEEK you can read but not remove the read data from the queue and/or
MSG_TRUNC to get the total size of the CAN frame.

I have not tested these flags, but I think support for them has to be
implemented in CAN_RAW, too.

Also, we should have a look at the UDP code.

> One example
> I can directly think of is the Packet API. Correct me if I am wrong
> but if writing generic code to dump packets on any interfaces, you do
> not know in advance the MTU. And so, I guess you have to provide a
> buffer of an arbitrary size. A generic program using a buffer of, let
> say, 2048 bytes (one page) will not be enough to get a CANXL frame in
> one single shot.

Nitpick: the page size is arch and/or kernel config dependent and the
smallest page size that Linux supports is 4k.

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Automotive Discussions]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]     [CAN Bus]

  Powered by Linux