Marc Kleine-Budde wrote: > Oliver Hartkopp wrote: >> Kurt Van Dijck wrote: >>> On Mon, Dec 14, 2009 at 08:57:37PM +0100, Oliver Hartkopp wrote: >>>> Kurt Van Dijck wrote: >>>>>>>> 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? >>>> Indeed i would like to have a PRINTK_ONCE() or DEV_ERR_ONCE() :-) >>>> >>>> >>>>>> 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. >>>> Using sk_error_report() and leaving out dev_err() is an interesting idea! >>>> >>>> Is that a usual way to provide this kind of error notification, e.g. for >>>> broken PF_PACKET packets? >>> I'm not familiar with PF_PACKET sockets. >>> >>> I encountered this sk_error_report() during my current development. >>> It appears to be _the_ method of saying >>> "something got in error", which must first be set by ->sk_err. >> I wonder if socket layer error reporting is the right thing on netdriver >> level. > >> ~/net-2.6/drivers/net$ find . -name \*.c | xargs grep sk_error > >> returns nothing - and i'm sure accessing skb->sk->sk_error_report(skb->sk) on >> driver level will bounce quite hard on netdev-ML ;-) > > well....ask them. I mean state our problem, give our sk_err solution and > ask if we can make it better.
Good idea. We could combine the question with the patch. Either it goes in or they will advice us to use something else for error reporting to the user. Wolfgang. _______________________________________________ Socketcan-core mailing list [email protected] https://lists.berlios.de/mailman/listinfo/socketcan-core
