On 14.07.2022 18:05:39, Oliver Hartkopp wrote:
> Extend the CAN device driver infrastructure to handle CAN XL frames.
> This especially addresses the increased data length which is extended
> to uint16 for CAN XL.
>
> Signed-off-by: Oliver Hartkopp <socketcan@xxxxxxxxxxxx>
> ---
> drivers/net/can/dev/rx-offload.c | 2 +-
> drivers/net/can/dev/skb.c | 55 ++++++++++++++++++++++++++------
> include/linux/can/skb.h | 6 +++-
> 3 files changed, 52 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/can/dev/rx-offload.c b/drivers/net/can/dev/rx-offload.c
> index a32a01c172d4..8505e547e922 100644
> --- a/drivers/net/can/dev/rx-offload.c
> +++ b/drivers/net/can/dev/rx-offload.c
> @@ -245,11 +245,11 @@ unsigned int can_rx_offload_get_echo_skb(struct can_rx_offload *offload,
> unsigned int *frame_len_ptr)
> {
> struct net_device *dev = offload->dev;
> struct net_device_stats *stats = &dev->stats;
> struct sk_buff *skb;
> - u8 len;
> + unsigned int len;
> int err;
>
> skb = __can_get_echo_skb(dev, idx, &len, frame_len_ptr);
> if (!skb)
> return 0;
> diff --git a/drivers/net/can/dev/skb.c b/drivers/net/can/dev/skb.c
> index 8bb62dd864c8..c4283fa680be 100644
> --- a/drivers/net/can/dev/skb.c
> +++ b/drivers/net/can/dev/skb.c
> @@ -53,11 +53,12 @@ int can_put_echo_skb(struct sk_buff *skb, struct net_device *dev,
> BUG_ON(idx >= priv->echo_skb_max);
>
> /* check flag whether this packet has to be looped back */
> if (!(dev->flags & IFF_ECHO) ||
> (skb->protocol != htons(ETH_P_CAN) &&
> - skb->protocol != htons(ETH_P_CANFD))) {
> + skb->protocol != htons(ETH_P_CANFD) &&
> + skb->protocol != htons(ETH_P_CANXL))) {
> kfree_skb(skb);
> return 0;
> }
>
> if (!priv->echo_skb[idx]) {
> @@ -86,12 +87,12 @@ int can_put_echo_skb(struct sk_buff *skb, struct net_device *dev,
> return 0;
> }
> EXPORT_SYMBOL_GPL(can_put_echo_skb);
>
> struct sk_buff *
> -__can_get_echo_skb(struct net_device *dev, unsigned int idx, u8 *len_ptr,
> - unsigned int *frame_len_ptr)
> +__can_get_echo_skb(struct net_device *dev, unsigned int idx,
> + unsigned int *len_ptr, unsigned int *frame_len_ptr)
> {
> struct can_priv *priv = netdev_priv(dev);
>
> if (idx >= priv->echo_skb_max) {
> netdev_err(dev, "%s: BUG! Trying to access can_priv::echo_skb out of bounds (%u/max %u)\n",
> @@ -103,17 +104,13 @@ __can_get_echo_skb(struct net_device *dev, unsigned int idx, u8 *len_ptr,
> /* Using "struct canfd_frame::len" for the frame
> * length is supported on both CAN and CANFD frames.
> */
> struct sk_buff *skb = priv->echo_skb[idx];
> struct can_skb_priv *can_skb_priv = can_skb_prv(skb);
> - struct canfd_frame *cf = (struct canfd_frame *)skb->data;
>
> /* get the real payload length for netdev statistics */
> - if (cf->can_id & CAN_RTR_FLAG)
> - *len_ptr = 0;
> - else
> - *len_ptr = cf->len;
> + *len_ptr = can_skb_get_data_len(skb);
>
> if (frame_len_ptr)
> *frame_len_ptr = can_skb_priv->frame_len;
>
> priv->echo_skb[idx] = NULL;
> @@ -139,11 +136,11 @@ __can_get_echo_skb(struct net_device *dev, unsigned int idx, u8 *len_ptr,
> */
> unsigned int can_get_echo_skb(struct net_device *dev, unsigned int idx,
> unsigned int *frame_len_ptr)
> {
> struct sk_buff *skb;
> - u8 len;
> + unsigned int len;
>
> skb = __can_get_echo_skb(dev, idx, &len, frame_len_ptr);
> if (!skb)
> return 0;
>
> @@ -244,10 +241,47 @@ struct sk_buff *alloc_canfd_skb(struct net_device *dev,
>
> return skb;
> }
> EXPORT_SYMBOL_GPL(alloc_canfd_skb);
>
> +struct sk_buff *alloc_canxl_skb(struct net_device *dev,
> + struct canxl_frame **cfx,
> + unsigned int datalen)
> +{
> + struct sk_buff *skb;
> +
> + if (datalen < CANXL_MIN_DLEN || datalen > CANXL_MAX_DLEN)
> + goto out_error;
> +
> + skb = netdev_alloc_skb(dev, sizeof(struct can_skb_priv) +
> + CANXL_HEAD_SZ + datalen);
> + if (unlikely(!skb))
> + goto out_error;
> +
> + skb->protocol = htons(ETH_P_CANXL);
> + skb->pkt_type = PACKET_BROADCAST;
> + skb->ip_summed = CHECKSUM_UNNECESSARY;
> +
> + skb_reset_mac_header(skb);
> + skb_reset_network_header(skb);
> + skb_reset_transport_header(skb);
> +
> + can_skb_reserve(skb);
> + can_skb_prv(skb)->ifindex = dev->ifindex;
> + can_skb_prv(skb)->skbcnt = 0;
> +
> + *cfx = skb_put_zero(skb, CANXL_HEAD_SZ + datalen);
Should the CANXL_XLF be set here?
I have a bad feeling if we have a struct canxl_frame with a fixed size,
but it might not completely be backed by data.....
For example, I've updated the gs_usb driver to work with flexible arrays
to accommodate the different USB frame length:
https://elixir.bootlin.com/linux/latest/source/drivers/net/can/usb/gs_usb.c#L216
Maybe we should talk to Kees Cook what's best to use here.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Attachment:
signature.asc
Description: PGP signature
