From: Oliver Hartkopp <socketcan-fJ+pQTUTwRTk1uMJSBkQmQ@public•gmane.org>
To: Kurt Van Dijck <kurt.van.dijck-/BeEPy95v10@public•gmane.org>
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public•gmane.org,
netdev <netdev-u79uwXL29TY76Z2rM5mHXA@public•gmane.org>
Subject: Re: [PATCH V2 net-next] can-raw: add msg_flags to distinguish local traffic
Date: Fri, 15 Oct 2010 18:15:51 +0200 [thread overview]
Message-ID: <4CB87E37.4070908@hartkopp.net> (raw)
In-Reply-To: <20101015153829.GA317-MxZ6Iy/zr/UdbCeoMzGj59i2O/JbrIOy@public.gmane.org>
Hello Kurt,
thanks for the patch.
I really appreciate this extension.
Some remarks inline ...
On 15.10.2010 17:38, Kurt Van Dijck wrote:
> diff --git a/Documentation/networking/can.txt b/Documentation/networking/can.txt
> index cd79735..95341aa 100644
> --- a/Documentation/networking/can.txt
> +++ b/Documentation/networking/can.txt
> @@ -22,6 +22,7 @@ This file contains
> 4.1.2 RAW socket option CAN_RAW_ERR_FILTER
> 4.1.3 RAW socket option CAN_RAW_LOOPBACK
> 4.1.4 RAW socket option CAN_RAW_RECV_OWN_MSGS
> + 4.1.5 RAW socket returned flags
> 4.2 Broadcast Manager protocol sockets (SOCK_DGRAM)
> 4.3 connected transport protocols (SOCK_SEQPACKET)
> 4.4 unconnected transport protocols (SOCK_DGRAM)
> @@ -471,6 +472,16 @@ solution for a couple of reasons:
> setsockopt(s, SOL_CAN_RAW, CAN_RAW_RECV_OWN_MSGS,
> &recv_own_msgs, sizeof(recv_own_msgs));
>
> + 4.1.5 RAW socket returned flags
> +
> + When using recvmsg() call, the msg->msg_flags may contain following flags:
> +
> + MSG_DONTROUTE: set when the frame was sent via the same physical device,
> + ie. a loopback frame.
According the code MSG_DONTROUTE indicates frames that have been originated on
the local host. Nothing more.
Only when you enabled CAN_RAW_RECV_OWN_MSGS and you bound the socket to a
specific CAN interface, the description becomes correct.
Better write:
MSG_DONTROUTE : set when the received frame was created on the local host.
> + MSG_CONFIRM: set when the frame was sent via the socket it is received on.
ok.
> + This flag acts as a 'transmission confirmation'.
Better:
This flag can be interpreted as a 'transmission confirmation' when the CAN
driver supports the echo of CAN frames on driver level, see 3.2 and 6.2
> + In order to receive such messages, CAN_RAW_RECV_OWN_MSGS must be set.
> +
> 4.2 Broadcast Manager protocol sockets (SOCK_DGRAM)
> 4.3 connected transport protocols (SOCK_SEQPACKET)
> 4.4 unconnected transport protocols (SOCK_DGRAM)
> diff --git a/net/can/raw.c b/net/can/raw.c
> index 7d77e67..9020c4f 100644
> --- a/net/can/raw.c
> +++ b/net/can/raw.c
> @@ -90,23 +90,39 @@ struct raw_sock {
> can_err_mask_t err_mask;
> };
>
> +/*
> + * return some space to store extra msg flags in.
> + * We use 1 int beyond the 'struct sockaddr_can' in skb->cb
> + * to store those.
> + * These flags will be use in raw_recvmsg()
> + */
/*
* Return pointer to store the extra msg flags for raw_recvmsg().
* We use the space of one unsigned int beyond the 'struct sockaddr_can'
* in skb->cb.
*/
> +static inline unsigned int *raw_flags(struct sk_buff *skb)
> +{
> + BUILD_BUG_ON(sizeof(skb->cb) <= (sizeof(struct sockaddr_can)
> + + sizeof(unsigned int)));
One empty line would be nice here ...
> + /* return pointer after struct sockaddr_can */
> + return (unsigned int *)(&((struct sockaddr_can *)skb->cb)[1]);
> +}
> +
> static inline struct raw_sock *raw_sk(const struct sock *sk)
> {
> return (struct raw_sock *)sk;
> }
>
> -static void raw_rcv(struct sk_buff *skb, void *data)
> +static void raw_rcv(struct sk_buff *oskb, void *data)
> {
> struct sock *sk = (struct sock *)data;
> struct raw_sock *ro = raw_sk(sk);
> struct sockaddr_can *addr;
> + struct sk_buff *skb;
> + unsigned int *pflags;
>
> /* check the received tx sock reference */
> - if (!ro->recv_own_msgs && skb->sk == sk)
> + if (!ro->recv_own_msgs && oskb->sk == sk)
> return;
>
> /* clone the given skb to be able to enqueue it into the rcv queue */
> - skb = skb_clone(skb, GFP_ATOMIC);
> + skb = skb_clone(oskb, GFP_ATOMIC);
> if (!skb)
> return;
>
> @@ -123,6 +139,14 @@ static void raw_rcv(struct sk_buff *skb, void *data)
> addr->can_family = AF_CAN;
> addr->can_ifindex = skb->dev->ifindex;
>
> + /* prepare the flags for raw_recvmsg() */
> + pflags = raw_flags(skb);
> + *pflags = 0;
> + if (oskb->sk)
> + *pflags |= MSG_DONTROUTE;
> + if (oskb->sk == sk)
> + *pflags |= MSG_CONFIRM;
> +
A good improvement since your first proof-of-concept code :-)
> if (sock_queue_rcv_skb(sk, skb) < 0)
> kfree_skb(skb);
> }
> @@ -707,6 +731,9 @@ static int raw_recvmsg(struct kiocb *iocb, struct socket *sock,
> memcpy(msg->msg_name, skb->cb, msg->msg_namelen);
> }
>
> + /* assign the flags that have been recorded in raw_rcv() */
> + msg->msg_flags |= *(raw_flags(skb));
> +
dito :-)
> skb_free_datagram(sk, skb);
>
> return size;
Tnx & regards,
Oliver
next prev parent reply other threads:[~2010-10-15 16:15 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-10-15 15:38 [PATCH V2 net-next] can-raw: add msg_flags to distinguish local traffic Kurt Van Dijck
[not found] ` <20101015153829.GA317-MxZ6Iy/zr/UdbCeoMzGj59i2O/JbrIOy@public.gmane.org>
2010-10-15 16:15 ` Oliver Hartkopp [this message]
[not found] ` <4CB87E37.4070908-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>
2010-10-16 7:37 ` Kurt Van Dijck
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4CB87E37.4070908@hartkopp.net \
--to=socketcan-fj+pqtutwrtk1umjsbkqmq@public$(echo .)gmane.org \
--cc=kurt.van.dijck-/BeEPy95v10@public$(echo .)gmane.org \
--cc=netdev-u79uwXL29TY76Z2rM5mHXA@public$(echo .)gmane.org \
--cc=socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public$(echo .)gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox