On Fri. 22 Jul. 2022 at 18:58, Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx> wrote:
> 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.
Here it is:
https://elixir.bootlin.com/linux/latest/source/net/ipv4/udp.c#L1846
The relevant function is sk_peek_offset:
https://elixir.bootlin.com/linux/latest/source/include/net/sock.h#L617
And to finish, there is an nearly eponym field in struct sock: sk_peek_off
https://elixir.bootlin.com/linux/latest/source/include/net/sock.h#L457
> > 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.
Arg, I did not have enough sleep.
By the way, do you have a preference between the flexible array member
and the data[CANXL_MAX_DLEN]? So far, I have been pushing the idea of
the flexible array member but if I am the only one to support this
idea, we can call it a day and use the data[CANXL_MAX_DLEN] approach.
Yours sincerely,
Vincent Mailhol
