On 09/18/2018 11:40 AM, Oleksij Rempel wrote:
> Current CAN framework can't guarantee proper/chronological order
> of RX and TX-ECHO messages. To make this possible, drivers should use
> this functions instead of can_get_echo_skb().
>
> Signed-off-by: Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx>
> ---
> drivers/net/can/rx-offload.c | 47 ++++++++++++++++++++++++++++++++++
> include/linux/can/rx-offload.h | 4 +++
> 2 files changed, 51 insertions(+)
>
> diff --git a/drivers/net/can/rx-offload.c b/drivers/net/can/rx-offload.c
> index d94dae216820..aafd84f79969 100644
> --- a/drivers/net/can/rx-offload.c
> +++ b/drivers/net/can/rx-offload.c
> @@ -209,6 +209,53 @@ int can_rx_offload_irq_offload_fifo(struct can_rx_offload *offload)
> }
> EXPORT_SYMBOL_GPL(can_rx_offload_irq_offload_fifo);
>
> +int can_rx_offload_queue_sorted(struct can_rx_offload *offload,
> + struct sk_buff *skb, u32 timestamp)
> +{
> + struct can_rx_offload_cb *cb;
> + unsigned long flags;
> +
> + if (skb_queue_len(&offload->skb_queue) >
> + offload->skb_queue_len_max)
> + return -ENOMEM;
> +
> + cb = can_rx_offload_get_cb(skb);
> + cb->timestamp = timestamp;
> +
> + spin_lock_irqsave(&offload->skb_queue.lock, flags);
> + __skb_queue_add_sort(&offload->skb_queue, skb, can_rx_offload_compare);
> + spin_unlock_irqrestore(&offload->skb_queue.lock, flags);
> +
> + can_rx_offload_schedule(offload);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(can_rx_offload_queue_sorted);
> +
> +unsigned int can_rx_offload_get_echo_skb(struct can_rx_offload *offload,
> + unsigned int idx, u32 timestamp)
> +{
> + struct can_priv *priv = netdev_priv(offload->dev);
> +
> + BUG_ON(idx >= priv->echo_skb_max);
> +
> + if (priv->echo_skb[idx]) {
> + struct sk_buff *skb = priv->echo_skb[idx];
> + struct can_frame *cf = (struct can_frame *)skb->data;
> + u8 dlc = cf->can_dlc;
> +
> + if (can_rx_offload_queue_sorted(offload, skb, timestamp))
In the current implementation the only error returned is an ENOMEM, if
the queue is full...
> + return 0;
> +
> + priv->echo_skb[idx] = NULL;
...but the stack echo_skb handling relies on the fact, that the place at
idx is NULL. So better drop the packet, set echo_skb to NULL and
increase some error counter.
https://elixir.bootlin.com/linux/latest/source/include/linux/netdevice.h#L165
What about rx_errors, tx_fifo_errors or both?
> +
> + return dlc;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(can_rx_offload_get_echo_skb);
> +
> int can_rx_offload_irq_queue_err_skb(struct can_rx_offload *offload, struct sk_buff *skb)
> {
> if (skb_queue_len(&offload->skb_queue) >
> diff --git a/include/linux/can/rx-offload.h b/include/linux/can/rx-offload.h
> index cb31683bbe15..01a7c9e5d8d8 100644
> --- a/include/linux/can/rx-offload.h
> +++ b/include/linux/can/rx-offload.h
> @@ -41,6 +41,10 @@ int can_rx_offload_add_timestamp(struct net_device *dev, struct can_rx_offload *
> int can_rx_offload_add_fifo(struct net_device *dev, struct can_rx_offload *offload, unsigned int weight);
> int can_rx_offload_irq_offload_timestamp(struct can_rx_offload *offload, u64 reg);
> int can_rx_offload_irq_offload_fifo(struct can_rx_offload *offload);
> +int can_rx_offload_queue_sorted(struct can_rx_offload *offload,
> + struct sk_buff *skb, u32 timestamp);
> +unsigned int can_rx_offload_get_echo_skb(struct can_rx_offload *offload,
> + unsigned int idx, u32 timestamp);
> int can_rx_offload_irq_queue_err_skb(struct can_rx_offload *offload, struct sk_buff *skb);
> void can_rx_offload_reset(struct can_rx_offload *offload);
> void can_rx_offload_del(struct can_rx_offload *offload);
>
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 |
Attachment:
signature.asc
Description: OpenPGP digital signature
