[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <551A94D0.1010608@pengutronix.de>
Date: Tue, 31 Mar 2015 14:36:32 +0200
From: Marc Kleine-Budde <mkl@...gutronix.de>
To: Oliver Hartkopp <socketcan@...tkopp.net>, linux-can@...r.kernel.org
CC: netdev@...r.kernel.org
Subject: Re: [PATCH RFC v2 2/2] can: introduce new raw socket option
to join the given CAN filters
On 03/29/2015 08:09 PM, Oliver Hartkopp wrote:
> The CAN_RAW socket can set multiple CAN identifier specific filters that lead
> to multiple filters in the af_can.c filter processing. These filters are
> indenpendent from each other which leads to logical OR'ed filters when applied.
>
> This socket option joines the given CAN filters in the way that only CAN frames
> are passed to user space that matched *all* given CAN filters. The semantic for
> the applied filters is therefore changed to a logical AND.
>
> This is useful especially when the filterset is a combination of filters where
> the CAN_INV_FILTER flag is set in order to notch single CAN IDs or CAN ID
> ranges from the incoming traffic.
>
> As the can_raw() function is executed from NET_RX softirq the introduced
> variables are implemented as per-CPU variables to avoid extensive locking at
> CAN frame reception time.
>
> Signed-off-by: Oliver Hartkopp <socketcan@...tkopp.net>
> ---
> Documentation/networking/can.txt | 20 ++++++++++++++++++--
> include/uapi/linux/can/raw.h | 1 +
> net/can/raw.c | 36 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 55 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/networking/can.txt b/Documentation/networking/can.txt
> index 0a2859a..5abad1e 100644
> --- a/Documentation/networking/can.txt
> +++ b/Documentation/networking/can.txt
> @@ -22,7 +22,8 @@ This file contains
> 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 option CAN_RAW_FD_FRAMES
> - 4.1.6 RAW socket returned message flags
> + 4.1.6 RAW socket option CAN_RAW_JOIN_FILTERS
> + 4.1.7 RAW socket returned message flags
> 4.2 Broadcast Manager protocol sockets (SOCK_DGRAM)
> 4.2.1 Broadcast Manager operations
> 4.2.2 Broadcast Manager message flags
> @@ -601,7 +602,22 @@ solution for a couple of reasons:
> CAN FD frames by checking if the device maximum transfer unit is CANFD_MTU.
> The CAN device MTU can be retrieved e.g. with a SIOCGIFMTU ioctl() syscall.
>
> - 4.1.6 RAW socket returned message flags
> + 4.1.6 RAW socket option CAN_RAW_JOIN_FILTERS
> +
> + The CAN_RAW socket can set multiple CAN identifier specific filters that
> + lead to multiple filters in the af_can.c filter processing. These filters
> + are indenpendent from each other which leads to logical OR'ed filters when
> + applied (see 4.1.1).
> +
> + This socket option joines the given CAN filters in the way that only CAN
> + frames are passed to user space that matched *all* given CAN filters. The
> + semantic for the applied filters is therefore changed to a logical AND.
> +
> + This is useful especially when the filterset is a combination of filters
> + where the CAN_INV_FILTER flag is set in order to notch single CAN IDs or
> + CAN ID ranges from the incoming traffic.
> +
> + 4.1.7 RAW socket returned message flags
>
> When using recvmsg() call, the msg->msg_flags may contain following flags:
>
> diff --git a/include/uapi/linux/can/raw.h b/include/uapi/linux/can/raw.h
> index 78ec76f..8735f108 100644
> --- a/include/uapi/linux/can/raw.h
> +++ b/include/uapi/linux/can/raw.h
> @@ -57,6 +57,7 @@ enum {
> CAN_RAW_LOOPBACK, /* local loopback (default:on) */
> CAN_RAW_RECV_OWN_MSGS, /* receive my own msgs (default:off) */
> CAN_RAW_FD_FRAMES, /* allow CAN FD frames (default:off) */
> + CAN_RAW_JOIN_FILTERS, /* all filters must match to trigger */
> };
>
> #endif /* !_UAPI_CAN_RAW_H */
> diff --git a/net/can/raw.c b/net/can/raw.c
> index 866a9b3..dc68fa0 100644
> --- a/net/can/raw.c
> +++ b/net/can/raw.c
> @@ -82,6 +82,8 @@ struct raw_sock {
> int loopback;
> int recv_own_msgs;
> int fd_frames;
> + int join_filters;
> + int __percpu *join_rx_count;
> int count; /* number of active filters */
> struct can_filter dfilter; /* default/single filter */
> struct can_filter *filter; /* pointer to filter(s) */
> @@ -128,10 +130,20 @@ static void raw_rcv(struct sk_buff *oskb, void *data)
> /* eliminate multiple filter matches for the same skb */
> if (*this_cpu_ptr(ro->uniq_skb) == oskb &&
> ktime_equal(*this_cpu_ptr(ro->uniq_tstamp), oskb->tstamp)) {
> + if (ro->join_filters) {
> + this_cpu_inc(*ro->join_rx_count);
> + /* drop frame until all enabled filters matched */
> + if (*this_cpu_ptr(ro->join_rx_count) < ro->count)
Can we be sure, that all skbs are processed on the same CPU? So that
it's sufficient to just compare the "ro->join_rx_count" of this CPU with
"ro->count" (or do we have to use join_rx_count of all CPUs?)
> + return;
> + } else
> return;
nitpick: please use { } on both sides of the else.
> } else {
> *this_cpu_ptr(ro->uniq_skb) = oskb;
> *this_cpu_ptr(ro->uniq_tstamp) = oskb->tstamp;
> + *this_cpu_ptr(ro->join_rx_count) = 1;
> + /* drop first frame to check all enabled filters? */
> + if (ro->join_filters && ro->count > 1)
> + return;
> }
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)
Powered by blists - more mailing lists