Re: [added to the 3.18 stable tree] can: fix loss of CAN frames in raw_rcv

[Oliver Hartkopp <socketcan@hartkopp.net>]

Hello Sasha,

this patch fixes commit 514ac99c64b "can: fix multiple delivery of a single 
CAN frame for overlapping CAN filters" which is currently not on your list for 
3.18.

Indeed I would suggest to omit either commit 514ac99c64b and the patch below 
for 3.18 stable.

Commit 514ac99c64b changes the number of returned frames in some cases and by 
now developers can trust on the fact that this behaviour change comes with 
Linux 4.1.

So nothing breaks when we omit commit 514ac99c64b and 36c01245eb804 for 3.18.

The fact that the patch below emerged on the stable ML is that it is relevant 
for 4.1 which was released in this phase.

Ok?

Many thanks,
Oliver

On 04.07.2015 05:02, Sasha Levin wrote:
> From: Oliver Hartkopp <socketcan@hartkopp.net>
>
> This patch has been added to the 3.18 stable tree. If you have any
> objections, please let us know.
>
> ===============
>
> [ Upstream commit 36c01245eb8046c16eee6431e7dbfbb302635fa8 ]
>
> As reported by Manfred Schlaegl here
>
>     http://marc.info/?l=linux-netdev&m=143482089824232&w=2
>
> 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.
>
> As net timestamping is influenced by several players (netstamp_needed and
> netdev_tstamp_prequeue) Manfred missed a proper timestamp which leads to
> CAN frame loss.
>
> As skb timestamping became now mandatory for CAN related skbs this patch
> makes sure that received CAN skbs always have a proper timestamp set.
> Maybe there's a better solution in the future but this patch fixes the
> CAN frame loss so far.
>
> Reported-by: Manfred Schlaegl <manfred.schlaegl@gmx.at>
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> Cc: linux-stable <stable@vger.kernel.org>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
> ---
>   drivers/net/can/dev.c   | 5 +++++
>   drivers/net/can/slcan.c | 1 +
>   drivers/net/can/vcan.c  | 3 +++
>   net/can/af_can.c        | 6 +++++-
>   4 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
> index 573b53b..bb686e1 100644
> --- a/drivers/net/can/dev.c
> +++ b/drivers/net/can/dev.c
> @@ -360,6 +360,9 @@ unsigned int can_get_echo_skb(struct net_device *dev, unsigned int idx)
>   		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;
>
> @@ -496,6 +499,7 @@ struct sk_buff *alloc_can_skb(struct net_device *dev, struct can_frame **cf)
>   	if (unlikely(!skb))
>   		return NULL;
>
> +	__net_timestamp(skb);
>   	skb->protocol = htons(ETH_P_CAN);
>   	skb->pkt_type = PACKET_BROADCAST;
>   	skb->ip_summed = CHECKSUM_UNNECESSARY;
> @@ -524,6 +528,7 @@ struct sk_buff *alloc_canfd_skb(struct net_device *dev,
>   	if (unlikely(!skb))
>   		return NULL;
>
> +	__net_timestamp(skb);
>   	skb->protocol = htons(ETH_P_CANFD);
>   	skb->pkt_type = PACKET_BROADCAST;
>   	skb->ip_summed = CHECKSUM_UNNECESSARY;
> diff --git a/drivers/net/can/slcan.c b/drivers/net/can/slcan.c
> index acb5b92..cb6b472 100644
> --- a/drivers/net/can/slcan.c
> +++ b/drivers/net/can/slcan.c
> @@ -210,6 +210,7 @@ static void slc_bump(struct slcan *sl)
>   	if (!skb)
>   		return;
>
> +	__net_timestamp(skb);
>   	skb->dev = sl->dev;
>   	skb->protocol = htons(ETH_P_CAN);
>   	skb->pkt_type = PACKET_BROADCAST;
> diff --git a/drivers/net/can/vcan.c b/drivers/net/can/vcan.c
> index 4e94057..30e4627 100644
> --- a/drivers/net/can/vcan.c
> +++ b/drivers/net/can/vcan.c
> @@ -81,6 +81,9 @@ static void vcan_rx(struct sk_buff *skb, struct net_device *dev)
>   	skb->dev       = dev;
>   	skb->ip_summed = CHECKSUM_UNNECESSARY;
>
> +	if (!(skb->tstamp.tv64))
> +		__net_timestamp(skb);
> +
>   	netif_rx_ni(skb);
>   }
>
> diff --git a/net/can/af_can.c b/net/can/af_can.c
> index d6030d6..9a32449 100644
> --- a/net/can/af_can.c
> +++ b/net/can/af_can.c
> @@ -313,8 +313,12 @@ int can_send(struct sk_buff *skb, int loop)
>   		return err;
>   	}
>
> -	if (newskb)
> +	if (newskb) {
> +		if (!(newskb->tstamp.tv64))
> +			__net_timestamp(newskb);
> +
>   		netif_rx_ni(newskb);
> +	}
>
>   	/* update statistics */
>   	can_stats.tx_frames++;
>