Wolfgang Grandegger wrote:
> Marc Kleine-Budde wrote:
>>> + if (unlikely(dlc > 8)) {
>>> +
>>> + static int print_dlc_err;
>>> +
>>> + if (!print_dlc_err) {
>>> + dev_err(dev->dev.parent, "illegal dlc %d\n", dlc);
>>> + print_dlc_err = 1;
>>> + }
>> You should use something like "WARN_ONCE" instead of open-coding it.
>
> WARN_ONCE is OK if it's acceptable that dlc > 8 happens.
Yes. I also thought about something like
if (WARN_ONCE(dlc > 8, "%s: Illegal dlc %d\n", DRV_NAME, dlc))
dlc = 8;
Would this be ok?
>> The general question is should a driver print this message or not. The
>> at91 silently limits the dlc to 8. I'm not sure what the other mainline
>> drivers do.
>
> It depends. If it's due to a malfunctioning of the hardware (or
> software), we should inform the user. Unfortunately, the drivers do not
> yet handle that case consistently. I have realized:
>
> - ignore dlc > 8
> - dlc &= 0xf
> - print error message (or oops)
> - min(dlc, 8)
>
The problem is to define what a dlc > 8 provided by the CAN controller (which
IS a BUG inside the CAN controller!) should mean to the rest of the data
inside the registers containing the received CAN frame:
- do we assume the rest to be a valid CAN frame?
- should we drop the frame ?
Is this something that depends on the CAN controller? E.g. is a wrong dlc
tolerable for SJA1000 but not for at91_can ??
Once we've checked that, we should clean this up.
> This is just for incoming messages. The dlc of out-going messages could
> be check by the upper layer. Is that already done? We need to fix that
> issue consistently for all drivers.
The CAN protocols that are using the PF_CAN core should use can_send() which
does this check:
int can_send(struct sk_buff *skb, int loop)
{
struct sk_buff *newskb = NULL;
struct can_frame *cf = (struct can_frame *)skb->data;
int err;
if (skb->len != sizeof(struct can_frame) || cf->can_dlc > 8) {
kfree_skb(skb);
return -EINVAL;
}
(..)
Regards,
Oliver
_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core
