CAN RAW sockets allow userspace to tell if a received CAN frame comes
from the same socket, another socket on the same host, or another host.
See commit 1e55659ce6dd ("can-raw: add msg_flags to distinguish local
traffic"). However, this feature is missing in CAN BCM sockets.
Add the same feature to CAN BCM sockets. When reading a received frame
(opcode RX_CHANGED) using recvmsg, two flags in msg->msg_flags may be
set following the previous convention (from CAN RAW), to distinguish
between 'own', 'local' and 'remote' CAN traffic.
Update the documentation to reflect this change.
Signed-off-by: Nicolas Maier <[email protected]>
---
Documentation/networking/can.rst | 34 ++++++++++++++------------
net/can/bcm.c | 42 +++++++++++++++++++++++++++++---
2 files changed, 56 insertions(+), 20 deletions(-)
diff --git a/Documentation/networking/can.rst b/Documentation/networking/can.rst
index d7e1ada905b2..62519d38c58b 100644
--- a/Documentation/networking/can.rst
+++ b/Documentation/networking/can.rst
@@ -444,6 +444,24 @@ definitions are specified for CAN specific MTUs in include/linux/can.h:
#define CANFD_MTU (sizeof(struct canfd_frame)) == 72 => CAN FD frame
+Returned Message Flags
+----------------------
+
+When using the system call recvmsg(2) on a RAW or a BCM socket, the
+msg->msg_flags field may contain the following flags:
+
+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.
+ This flag can be interpreted as a 'transmission confirmation' when the
+ CAN driver supports the echo of frames on driver level, see
+ :ref:`socketcan-local-loopback1` and :ref:`socketcan-local-loopback2`.
+ (Note: In order to receive such messages on a RAW socket,
+ CAN_RAW_RECV_OWN_MSGS must be set.)
+
+
.. _socketcan-raw-sockets:
RAW Protocol Sockets with can_filters (SOCK_RAW)
@@ -693,22 +711,6 @@ where the CAN_INV_FILTER flag is set in order to notch single CAN IDs or
CAN ID ranges from the incoming traffic.
-RAW Socket Returned Message Flags
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-
-When using recvmsg() call, the msg->msg_flags may contain following flags:
-
-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.
- This flag can be interpreted as a 'transmission confirmation' when the
- CAN driver supports the echo of frames on driver level, see
- :ref:`socketcan-local-loopback1` and :ref:`socketcan-local-loopback2`.
- In order to receive such messages, CAN_RAW_RECV_OWN_MSGS must be set.
-
-
Broadcast Manager Protocol Sockets (SOCK_DGRAM)
-----------------------------------------------
diff --git a/net/can/bcm.c b/net/can/bcm.c
index 9168114fc87f..32345e155006 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -72,9 +72,11 @@
#define BCM_TIMER_SEC_MAX (400 * 24 * 60 * 60)
/* use of last_frames[index].flags */
+#define RX_LOCAL 0x10 /* frame was created on the local host */
+#define RX_OWN 0x20 /* frame was sent via the socket it was received on */
#define RX_RECV 0x40 /* received data for this element */
#define RX_THR 0x80 /* element not been sent due to throttle feature */
-#define BCM_CAN_FLAGS_MASK 0x3F /* to clean private flags after usage */
+#define BCM_CAN_FLAGS_MASK 0x0F /* to clean private flags after usage */
/* get best masking value for can_rx_register() for a given single can_id */
#define REGMASK(id) ((id & CAN_EFF_FLAG) ? \
@@ -138,6 +140,19 @@ static LIST_HEAD(bcm_notifier_list);
static DEFINE_SPINLOCK(bcm_notifier_lock);
static struct bcm_sock *bcm_busy_notifier;
+/* Return pointer to store the extra msg flags for bcm_recvmsg().
+ * We use the space of one unsigned int beyond the 'struct sockaddr_can'
+ * in skb->cb.
+ */
+static inline unsigned int *bcm_flags(struct sk_buff *skb)
+{
+ sock_skb_cb_check_size(sizeof(struct sockaddr_can) +
+ sizeof(unsigned int));
+
+ /* return pointer after struct sockaddr_can */
+ return (unsigned int *)(&((struct sockaddr_can *)skb->cb)[1]);
+}
+
static inline struct bcm_sock *bcm_sk(const struct sock *sk)
{
return (struct bcm_sock *)sk;
@@ -325,6 +340,7 @@ static void bcm_send_to_user(struct bcm_op *op, struct bcm_msg_head *head,
struct sock *sk = op->sk;
unsigned int datalen = head->nframes * op->cfsiz;
int err;
+ unsigned int *pflags;
skb = alloc_skb(sizeof(*head) + datalen, gfp_any());
if (!skb)
@@ -344,8 +360,16 @@ static void bcm_send_to_user(struct bcm_op *op, struct bcm_msg_head *head,
* relevant for updates that are generated by the
* BCM, where nframes is 1
*/
- if (head->nframes == 1)
+ if (head->nframes == 1) {
+ pflags = bcm_flags(skb);
+ *pflags = 0;
+ if (firstframe->flags & RX_LOCAL)
+ *pflags |= MSG_DONTROUTE;
+ if (firstframe->flags & RX_OWN)
+ *pflags |= MSG_CONFIRM;
+
firstframe->flags &= BCM_CAN_FLAGS_MASK;
+ }
}
if (has_timestamp) {
@@ -444,7 +468,7 @@ static void bcm_rx_changed(struct bcm_op *op, struct canfd_frame *data)
op->frames_filtered = op->frames_abs = 0;
/* this element is not throttled anymore */
- data->flags &= (BCM_CAN_FLAGS_MASK|RX_RECV);
+ data->flags &= ~RX_THR;
memset(&head, 0, sizeof(head));
head.opcode = RX_CHANGED;
@@ -642,7 +666,7 @@ static enum hrtimer_restart bcm_rx_thr_handler(struct hrtimer *hrtimer)
static void bcm_rx_handler(struct sk_buff *skb, void *data)
{
struct bcm_op *op = (struct bcm_op *)data;
- const struct canfd_frame *rxframe = (struct canfd_frame *)skb->data;
+ struct canfd_frame *rxframe = (struct canfd_frame *)skb->data;
unsigned int i;
if (op->can_id != rxframe->can_id)
@@ -657,6 +681,13 @@ static void bcm_rx_handler(struct sk_buff *skb, void *data)
return;
}
+ /* add flags to distinguish between own/local/remote CAN traffic */
+ if (skb->sk) {
+ rxframe->flags |= RX_LOCAL;
+ if (skb->sk == op->sk)
+ rxframe->flags |= RX_OWN;
+ }
+
/* disable timeout */
hrtimer_cancel(&op->timer);
@@ -1675,6 +1706,9 @@ static int bcm_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
memcpy(msg->msg_name, skb->cb, msg->msg_namelen);
}
+ /* assign the flags that have been recorded in bcm_send_to_user() */
+ msg->msg_flags |= *(bcm_flags(skb));
+
skb_free_datagram(sk, skb);
return size;
--
2.34.1
Hi Nicolas,
thanks for the patch!
On 07.01.24 07:44, Nicolas Maier wrote:
> @@ -642,7 +666,7 @@ static enum hrtimer_restart bcm_rx_thr_handler(struct hrtimer *hrtimer)
> static void bcm_rx_handler(struct sk_buff *skb, void *data)
> {
> struct bcm_op *op = (struct bcm_op *)data;
> - const struct canfd_frame *rxframe = (struct canfd_frame *)skb->data;
> + struct canfd_frame *rxframe = (struct canfd_frame *)skb->data;
> unsigned int i;
>
> if (op->can_id != rxframe->can_id)
> @@ -657,6 +681,13 @@ static void bcm_rx_handler(struct sk_buff *skb, void *data)
> return;
> }
>
> + /* add flags to distinguish between own/local/remote CAN traffic */
> + if (skb->sk) {
> + rxframe->flags |= RX_LOCAL;
> + if (skb->sk == op->sk)
> + rxframe->flags |= RX_OWN;
> + }
> +
> /* disable timeout */
> hrtimer_cancel(&op->timer);
>
No. You are writing to a read-only skbuff, which is not yet cloned or
copied. The read-only handling of the skb is done in the NET_RX_SOFTIRQ
triggering can_rcv_filter() and friends.
See this note:
https://elixir.bootlin.com/linux/v6.7/source/net/can/af_can.c#L430
When you are also changing the CAN frames' content you need to
skb_copy() the provided skb, see:
https://elixir.bootlin.com/linux/v6.7/source/net/can/gw.c#L504
So if you want to pass these new flags to the user space you should
think of extending the parameter list of bcm_rx_update_and_send() and
bcm_rx_cmp_to_index().
But in the first place I'm interested to know what the use-case for this
extension is.
Best regards,
Oliver
Hello,
Thanks a lot for the feedback!
On 08/01/2024 19:57, Oliver Hartkopp wrote:
> But in the first place I'm interested to know what the use-case for this
> extension is.
My goal with this extension is to be able to implement a reliable
'CAN send' function in userspace which would check that the message has
been correctly transmitted, or retry if no confirmation was received
after a timeout (I am using a device which empties some TX buffer when
recovering from the Bus-Off state, therefore some messages will be lost,
which is why the reliability needs to be handled elsewhere). For this
purpose, knowing the origin of a received message is needed.
This was already possible with RAW sockets, and the goal here is to
provide the same information when using BCM sockets.
Best regards,
Nicolas