On 12/15/20 12:37 PM, Vincent MAILHOL wrote: >> When implementing BQL we need the CAN frame's DLL len twice: >> 1. When sending the package to the hardware >> 2. After the TX-complete IRQ >> >> We can calculate this information twice, but I thought we might put it into the >> struct can_skb_priv. >> >> https://elixir.bootlin.com/linux/latest/source/include/linux/can/skb.h#L34 >> >> Thoughts? > > I am not knowledgeable enough on this part to guarantee if there will > be no side effects but regardless, I like the idea. > > Also, an u8 is enough to hold the value. ACK > I wonder if it would be fine > to change ifindex to, for example, u16, so that we do not lose any > memory. ifindex is an int: https://elixir.bootlin.com/linux/v5.10.1/source/include/linux/netdevice.h#L1899 > I would look like that: > > struct can_skb_priv { > u16 ifindex; > u8 frame_len; > u8 res; No need for any reserved bytes, as this is not packed and not a stable API. The compiler will take care of the packing. If you want to save space in structures take a look at pahole: https://lwn.net/Articles/206805/ > int skbcnt; > struct can_frame cf[]; > }; > > And the final result in the driver would look as below: > > /* Tx branch */ > can_skb_prv(skb)->frame_len = can_skb_get_frame_len(skb); > netdev_sent_queue(can_skb_prv(skb)->frame_len); > > /* Rx branch */ > netdev_completed_queue(can_skb_prv(skb)->frame_len); I think, I'll add another variety of the get_echo_skb() function, that returns the frame_len. > This is quite neat. 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: OpenPGP digital signature
