can: replace timestamp as unique skb attribute
Commit 514ac99c64b "can: fix multiple delivery of a single CAN frame for
overlapping CAN filters" requires the skb->tstamp to be set to check for
identical CAN skbs.
Without timestamping to be required by user space applications this timestamp
was not generated which lead to commit 36c01245eb8 "can: fix loss of CAN frames
in raw_rcv" - which forces the timestamp to be set in all CAN related skbuffs
by introducing several __net_timestamp() calls.
This forces e.g. out of tree drivers which are not using alloc_can{,fd}_skb()
to add __net_timestamp() after skbuff creation to prevent the frame loss fixed
in mainline Linux.
This patch removes the timestamp dependency and uses an atomic counter to
create an unique identifier together with the skbuff pointer.
Btw: the new skbcnt element introduced in struct can_skb_priv has to be
initialized with zero in out-of-tree drivers which are not using
alloc_can{,fd}_skb() too.
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index e9b1810..aede704 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -440,9 +440,6 @@
struct can_frame *cf = (struct can_frame *)skb->data;
u8 dlc = cf->can_dlc;
- if (!(skb->tstamp.tv64))
- __net_timestamp(skb);
-
netif_rx(priv->echo_skb[idx]);
priv->echo_skb[idx] = NULL;
@@ -578,7 +575,6 @@
if (unlikely(!skb))
return NULL;
- __net_timestamp(skb);
skb->protocol = htons(ETH_P_CAN);
skb->pkt_type = PACKET_BROADCAST;
skb->ip_summed = CHECKSUM_UNNECESSARY;
@@ -589,6 +585,7 @@
can_skb_reserve(skb);
can_skb_prv(skb)->ifindex = dev->ifindex;
+ can_skb_prv(skb)->skbcnt = 0;
*cf = (struct can_frame *)skb_put(skb, sizeof(struct can_frame));
memset(*cf, 0, sizeof(struct can_frame));
@@ -607,7 +604,6 @@
if (unlikely(!skb))
return NULL;
- __net_timestamp(skb);
skb->protocol = htons(ETH_P_CANFD);
skb->pkt_type = PACKET_BROADCAST;
skb->ip_summed = CHECKSUM_UNNECESSARY;
@@ -618,6 +614,7 @@
can_skb_reserve(skb);
can_skb_prv(skb)->ifindex = dev->ifindex;
+ can_skb_prv(skb)->skbcnt = 0;
*cfd = (struct canfd_frame *)skb_put(skb, sizeof(struct canfd_frame));
memset(*cfd, 0, sizeof(struct canfd_frame));
diff --git a/drivers/net/can/slcan.c b/drivers/net/can/slcan.c
index f64f529..a23a7af 100644
--- a/drivers/net/can/slcan.c
+++ b/drivers/net/can/slcan.c
@@ -207,7 +207,6 @@
if (!skb)
return;
- __net_timestamp(skb);
skb->dev = sl->dev;
skb->protocol = htons(ETH_P_CAN);
skb->pkt_type = PACKET_BROADCAST;
@@ -215,6 +214,7 @@
can_skb_reserve(skb);
can_skb_prv(skb)->ifindex = sl->dev->ifindex;
+ can_skb_prv(skb)->skbcnt = 0;
memcpy(skb_put(skb, sizeof(struct can_frame)),
&cf, sizeof(struct can_frame));
diff --git a/drivers/net/can/vcan.c b/drivers/net/can/vcan.c
index 0ce868d..674f367 100644
--- a/drivers/net/can/vcan.c
+++ b/drivers/net/can/vcan.c
@@ -78,9 +78,6 @@
skb->dev = dev;
skb->ip_summed = CHECKSUM_UNNECESSARY;
- if (!(skb->tstamp.tv64))
- __net_timestamp(skb);
-
netif_rx_ni(skb);
}
diff --git a/include/linux/can/skb.h b/include/linux/can/skb.h
index b6a52a4..51bb653 100644
--- a/include/linux/can/skb.h
+++ b/include/linux/can/skb.h
@@ -27,10 +27,12 @@
/**
* struct can_skb_priv - private additional data inside CAN sk_buffs
* @ifindex: ifindex of the first interface the CAN frame appeared on
+ * @skbcnt: atomic counter to have an unique id together with skb pointer
* @cf: align to the following CAN frame at skb->data
*/
struct can_skb_priv {
int ifindex;
+ int skbcnt;
struct can_frame cf[0];
};
diff --git a/net/can/af_can.c b/net/can/af_can.c
index 7933e62..166d436 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -89,6 +89,8 @@
struct s_stats can_stats; /* packet statistics */
struct s_pstats can_pstats; /* receive list statistics */
+static atomic_t skbcounter = ATOMIC_INIT(0);
+
/*
* af_can socket functions
*/
@@ -310,12 +312,8 @@
return err;
}
- if (newskb) {
- if (!(newskb->tstamp.tv64))
- __net_timestamp(newskb);
-
+ if (newskb)
netif_rx_ni(newskb);
- }
/* update statistics */
can_stats.tx_frames++;
@@ -683,6 +681,10 @@
can_stats.rx_frames++;
can_stats.rx_frames_delta++;
+ /* create non-zero unique skb identifier together with *skb */
+ while (!(can_skb_prv(skb)->skbcnt))
+ can_skb_prv(skb)->skbcnt = atomic_inc_return(&skbcounter);
+
rcu_read_lock();
/* deliver the packet to sockets listening on all devices */
diff --git a/net/can/bcm.c b/net/can/bcm.c
index b523453..a1ba687 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -261,6 +261,7 @@
can_skb_reserve(skb);
can_skb_prv(skb)->ifindex = dev->ifindex;
+ can_skb_prv(skb)->skbcnt = 0;
memcpy(skb_put(skb, CFSIZ), cf, CFSIZ);
@@ -1217,6 +1218,7 @@
}
can_skb_prv(skb)->ifindex = dev->ifindex;
+ can_skb_prv(skb)->skbcnt = 0;
skb->dev = dev;
can_skb_set_owner(skb, sk);
err = can_send(skb, 1); /* send with loopback */
diff --git a/net/can/raw.c b/net/can/raw.c
index 31b9748..2e67b14 100644
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -75,7 +75,7 @@
*/
struct uniqframe {
- ktime_t tstamp;
+ int skbcnt;
const struct sk_buff *skb;
unsigned int join_rx_count;
};
@@ -133,7 +133,7 @@
/* 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)) {
+ this_cpu_ptr(ro->uniq)->skbcnt == can_skb_prv(oskb)->skbcnt) {
if (ro->join_filters) {
this_cpu_inc(ro->uniq->join_rx_count);
/* drop frame until all enabled filters matched */
@@ -144,7 +144,7 @@
}
} else {
this_cpu_ptr(ro->uniq)->skb = oskb;
- this_cpu_ptr(ro->uniq)->tstamp = oskb->tstamp;
+ this_cpu_ptr(ro->uniq)->skbcnt = can_skb_prv(oskb)->skbcnt;
this_cpu_ptr(ro->uniq)->join_rx_count = 1;
/* drop first frame to check all enabled filters? */
if (ro->join_filters && ro->count > 1)
@@ -749,6 +749,7 @@
can_skb_reserve(skb);
can_skb_prv(skb)->ifindex = dev->ifindex;
+ can_skb_prv(skb)->skbcnt = 0;
err = memcpy_from_msg(skb_put(skb, size), msg, size);
if (err < 0)