-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 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. cheers, Marc - -- Pengutronix e.K. | Marc Kleine-Budde | Linux Solutions for Science and Industry | Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEARECAAYFAksn7M4ACgkQjTAFq1RaXHNRtQCeN3mCMZC9E9QYaQ0YTx7mErG1 kSsAn244AKW+Nkx4iVj7tLShufhZOnt2 =EZ+6 -----END PGP SIGNATURE----- _______________________________________________ Socketcan-core mailing list [email protected] https://lists.berlios.de/mailman/listinfo/socketcan-core
