On 19.03.2021 10:55:07, Oliver Hartkopp wrote: > > What about skb_put_zero(), which I mentioned in my initial cover letter: > > Yes, that would indeed be more elegant. Will send a v2 patch. > > > > > Note here the "B" and "E" flags are set. Another possibility is to use > > > > skb_put_zero() instead of skb_put(), but with a bigger overhead. A 3. > > > > option is to only memset() the non-data part of the struct canfd_frame. > > > > http://lore.kernel.org/r/20210218215434.1708249-1-mkl@xxxxxxxxxxxxxx > > I modified candump in a way that it always prints the entire frame > independent from can(fd)_frame::len > > diff --git a/candump.c b/candump.c > index 7bb854a..9683fc9 100644 > --- a/candump.c > +++ b/candump.c > @@ -719,13 +719,13 @@ int main(int argc, char **argv) > perror("read"); > return 1; > } > > if ((size_t)nbytes == CAN_MTU) > - maxdlen = CAN_MAX_DLEN; > + frame.len = maxdlen = CAN_MAX_DLEN; > else if ((size_t)nbytes == CANFD_MTU) > - maxdlen = CANFD_MAX_DLEN; > + frame.len = maxdlen = CANFD_MAX_DLEN; > else { > fprintf(stderr, "read: incomplete CAN > frame\n"); > return 1; > } > > And there you can see that in flow-control (FC) frames and consecutive > frames (CF) especially at the end of the PDU you see uninitialized content. > > So it does not help to only clear the non-data part of the struct > canfd_frame. Makes sense, my emphasis was on skb_put_zero() instead of skb_put(). 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