On Mon, Dec 14, 2009 at 02:06:12PM +0100, Wolfgang Grandegger wrote:
> Kurt Van Dijck wrote:
> >>>> I still believe that the message and the backtrace does not help the
> >>>> user to understand that he has done something wrong. Nevertheless, if
> >>>> there is nobody else sharing my concerns, please go ahead preparing the
> >>>> patch as you suggested above. The future will show if they are
> >>>> reasonable.
> >>> I share a bit the concern, but if I understand well, this is to avoid
> >>> PF_PACKET sockets to break CAN rules. When regular PF_CAN is used, the
> >>> user will be informed via can_send in af_can.c, isn't it?
> >> Yes, that's also my understanding:
> >>
> >> http://lxr.linux.no/#linux+v2.6.32/net/can/af_can.c#L218
> >
> > Wolfgang,
> >
> > To address your concern, would a construction like this suit, when
> > fitted in Oliver's proposal?
> >
> > inline int no_can_skb((struct sk_buff *skb)
> > {
> > struct can_frame *cf = (struct can_frame *)skb->data
> >
> > if ((skb->len != sizeof(*cf)) || ((cf->can_dlc > 8)) {
> > if (skb->sk && !sock_flag(skb->sk, SOCK_DEAD)) {
> > skb->sk->sk_err = EINVAL;
> > skb->sk->sk_error_report(skb->sk); /* can this block?*/
> > }
> >
> > WARN_ONCE(1, "non conform skbuf: ...");
> > "Dropped non conform skbuf: len %d, can_dlc %d\n",
> > skb->len, cf->can_dlc);
> > return 1;
> > }
> > return 0;
> > }
>
> My primary concern is about using *WARN_ONCE*. The BUG, WARN, functions
> and friends indicate to the user that there is a problem with the
> kernel, e.g. a bug and therefore I prefer a simple dev_err(). Also the
I see.
I agree that a dev_err seems more appropriate here.
Since this is TX path, kernel message flood is with respect to local
activity?
> word "skbuf" does say little to the normal Linux users. I find
> s/skbuf/packet/ more intuitive. Of course, if there is a better way to
ack.
> inform the user we should use it. Unfortunately, I can't tell if your
> approach will work.
I went into the code, in net/core/sock.c:sock_def_error_report.
that looks like atomic code to me, so the above sk_error_report() stuff
should work.
no_can_skb() would not even need a dev_err in that case.
>
> Wolfgang.
_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core
