On 12.07.22 09:17, Marc Kleine-Budde wrote:
On 12.07.2022 09:12:49, Oliver Hartkopp wrote:
On 11.07.22 21:41, Marc Kleine-Budde wrote:
On 11.07.2022 20:34:23, Oliver Hartkopp wrote:
Enable the PF_CAN infrastructure to handle CAN XL frames. A new ethernet
protocol type ETH_P_CANXL is defined to tag skbuffs containing the CAN XL
frame data structure.
As the length information is now a uint16 value for CAN XL a new helper
function can_get_data_len() is introduced to retrieve the data length
from all types of CAN frames.
Signed-off-by: Oliver Hartkopp <socketcan@xxxxxxxxxxxx>
---
include/linux/can/skb.h | 14 ++++++++++
include/uapi/linux/if_ether.h | 1 +
net/can/af_can.c | 49 +++++++++++++++++++++++++++++------
3 files changed, 56 insertions(+), 8 deletions(-)
diff --git a/include/linux/can/skb.h b/include/linux/can/skb.h
index 182749e858b3..d043bc4afd6d 100644
--- a/include/linux/can/skb.h
+++ b/include/linux/can/skb.h
@@ -101,6 +101,20 @@ static inline bool can_is_canfd_skb(const struct sk_buff *skb)
{
/* the CAN specific type of skb is identified by its data length */
return skb->len == CANFD_MTU;
}
+/* get data length inside of CAN frame for all frame types */
+static inline unsigned int can_get_data_len(struct sk_buff *skb)
+{
+ if(skb->len == CANXL_MTU) {
+ const struct canxl_frame *cfx = (struct canxl_frame *)skb->data;
+
+ return cfx->len;
+ } else {
+ const struct canfd_frame *cfd = (struct canfd_frame *)skb->data;
+
+ return cfd->len;
+ }
+}
+
#endif /* !_CAN_SKB_H */
diff --git a/include/uapi/linux/if_ether.h b/include/uapi/linux/if_ether.h
index d370165bc621..69e0457eb200 100644
--- a/include/uapi/linux/if_ether.h
+++ b/include/uapi/linux/if_ether.h
@@ -136,10 +136,11 @@
#define ETH_P_WAN_PPP 0x0007 /* Dummy type for WAN PPP frames*/
#define ETH_P_PPP_MP 0x0008 /* Dummy type for PPP MP frames */
#define ETH_P_LOCALTALK 0x0009 /* Localtalk pseudo type */
#define ETH_P_CAN 0x000C /* CAN: Controller Area Network */
#define ETH_P_CANFD 0x000D /* CANFD: CAN flexible data rate*/
+#define ETH_P_CANXL 0x000E /* CANXL: eXtended frame Length */
#define ETH_P_PPPTALK 0x0010 /* Dummy type for Atalk over PPP*/
#define ETH_P_TR_802_2 0x0011 /* 802.2 frames */
#define ETH_P_MOBITEX 0x0015 /* Mobitex (kaz@xxxxxxxx) */
#define ETH_P_CONTROL 0x0016 /* Card specific control frames */
#define ETH_P_IRDA 0x0017 /* Linux-IrDA */
This file doesn't change that often I suppose. Or does it make sense to
send this change mainline as soon as possible?
AFAIK you only can reserve these values when you have a reference that uses
it. I don't seen any additional pressure here.
There have been added other types that are not used in the kernel (yet).
Ok?!?
I remembered some discussion about PF_FLEXRAY which bounced some years ago.
But I think it should be fine if we get this small patchset discussed
and upstreamed to net-next in this round, right?
diff --git a/net/can/af_can.c b/net/can/af_can.c
index 1fb49d51b25d..2c9f48aa5f1f 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -197,31 +197,32 @@ static int can_create(struct net *net, struct socket *sock, int protocol,
* -EINVAL when the skb->data does not contain a valid CAN frame
*/
int can_send(struct sk_buff *skb, int loop)
{
struct sk_buff *newskb = NULL;
- struct canfd_frame *cfd = (struct canfd_frame *)skb->data;
+ unsigned int len = can_get_data_len(skb);
struct can_pkg_stats *pkg_stats = dev_net(skb->dev)->can.pkg_stats;
int err = -EINVAL;
if (skb->len == CAN_MTU) {
skb->protocol = htons(ETH_P_CAN);
- if (unlikely(cfd->len > CAN_MAX_DLEN))
+ if (unlikely(len > CAN_MAX_DLEN))
goto inval_skb;
} else if (skb->len == CANFD_MTU) {
skb->protocol = htons(ETH_P_CANFD);
- if (unlikely(cfd->len > CANFD_MAX_DLEN))
+ if (unlikely(len > CANFD_MAX_DLEN))
+ goto inval_skb;
+ } else if (skb->len == CANXL_MTU) {
+ skb->protocol = htons(ETH_P_CANXL);
+ if (unlikely(len > CANXL_MAX_DLEN || len == 0))
Do we need a helper for the > CANXL_MAX_DLEN || == 0 check?
Some can_xl_is_valid_data_size(unsigned int len) ?
In other location you're using "canxl" not "can_xl".
I just thought about can_blablabla() as common namespace for our functions.
But canxl_whatever() is also safe ;-)
What about: canxl_valid_data_size()
Fine for me.
Thanks,
Oliver
