On 7/23/19 3:00 PM, Oliver Hartkopp wrote: > Introduce CAN FD support which needs an extension of the netlink API to pass > CAN FD type content to the kernel which has a different size to Classic CAN. > Additionally the struct canfd_frame has a new 'flags' element that can now > be modified with can-gw. > > The new CGW_FLAGS_CAN_FD option flag defines whether the routing job handles > Classic CAN or CAN FD frames. This setting is very strict at reception time > and enables the new possibilities, e.g. CGW_FDMOD_* and modifying the flags > element of struct canfd_frame, only when CGW_FLAGS_CAN_FD ist set. > > Signed-off-by: Oliver Hartkopp <socketcan@xxxxxxxxxxxx> > --- I've added these patches to the testing branch: https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git/log/?h=testing While applying I've fixes some checkpatch warnings/errors. > include/uapi/linux/can/gw.h | 14 ++- > net/can/gw.c | 233 ++++++++++++++++++++++++++++++++---- > 2 files changed, 220 insertions(+), 27 deletions(-) > > diff --git a/include/uapi/linux/can/gw.h b/include/uapi/linux/can/gw.h > index ed811bc463b5..3aea5388c8e4 100644 > --- a/include/uapi/linux/can/gw.h > +++ b/include/uapi/linux/can/gw.h > @@ -80,6 +80,10 @@ enum { > CGW_DELETED, /* number of deleted CAN frames (see max_hops param) */ > CGW_LIM_HOPS, /* limit the number of hops of this specific rule */ > CGW_MOD_UID, /* user defined identifier for modification updates */ > + CGW_FDMOD_AND, /* CAN FD frame modification binary AND */ > + CGW_FDMOD_OR, /* CAN FD frame modification binary OR */ > + CGW_FDMOD_XOR, /* CAN FD frame modification binary XOR */ > + CGW_FDMOD_SET, /* CAN FD frame modification set alternate values */ > __CGW_MAX > }; > > @@ -88,6 +92,7 @@ enum { > #define CGW_FLAGS_CAN_ECHO 0x01 > #define CGW_FLAGS_CAN_SRC_TSTAMP 0x02 > #define CGW_FLAGS_CAN_IIF_TX_OK 0x04 > +#define CGW_FLAGS_CAN_FD 0x08 > > #define CGW_MOD_FUNCS 4 /* AND OR XOR SET */ > > @@ -96,8 +101,9 @@ enum { > #define CGW_MOD_DLC 0x02 /* contains the data length in bytes */ > #define CGW_MOD_LEN CGW_MOD_DLC /* CAN FD length representation */ > #define CGW_MOD_DATA 0x04 > +#define CGW_MOD_FLAGS 0x08 /* CAN FD flags */ > > -#define CGW_FRAME_MODS 3 /* ID DLC/LEN DATA */ > +#define CGW_FRAME_MODS 4 /* ID DLC/LEN DATA FLAGS */ > > #define MAX_MODFUNCTIONS (CGW_MOD_FUNCS * CGW_FRAME_MODS) > > @@ -106,7 +112,13 @@ struct cgw_frame_mod { > __u8 modtype; > } __attribute__((packed)); > > +struct cgw_fdframe_mod { > + struct canfd_frame cf; > + __u8 modtype; > +} __attribute__((packed)); > + > #define CGW_MODATTR_LEN sizeof(struct cgw_frame_mod) > +#define CGW_FDMODATTR_LEN sizeof(struct cgw_fdframe_mod) > > struct cgw_csum_xor { > __s8 from_idx; > diff --git a/net/can/gw.c b/net/can/gw.c > index aecccc15c972..b75c0897540a 100644 > --- a/net/can/gw.c > +++ b/net/can/gw.c > @@ -60,7 +60,7 @@ > #include <net/net_namespace.h> > #include <net/sock.h> > > -#define CAN_GW_VERSION "20170425" > +#define CAN_GW_VERSION "20190105" > #define CAN_GW_NAME "can-gw" > > MODULE_DESCRIPTION("PF_CAN netlink gateway"); > @@ -150,6 +150,18 @@ struct cgw_job { > u16 flags; > }; > > +/* make sure we get a valid CAN frame data length after manipulation */ > +const u8 validate_len[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, /* 0 - 8 */ > + 12, 12, 12, 12, /* 9 - 12 */ > + 16, 16, 16, 16, /* 13 - 16 */ > + 20, 20, 20, 20, /* 17 - 20 */ > + 24, 24, 24, 24, /* 21 - 24 */ > + 32, 32, 32, 32, 32, 32, 32, 32, /* 25 - 32 */ > + 48, 48, 48, 48, 48, 48, 48, 48, /* 33 - 40 */ > + 48, 48, 48, 48, 48, 48, 48, 48, /* 41 - 48 */ > + 64, 64, 64, 64, 64, 64, 64, 64, /* 49 - 56 */ > + 64, 64, 64, 64, 64, 64, 64, 64}; /* 57 - 64 */ > + > /* modification functions that are invoked in the hot path in can_can_gw_rcv */ > > #define MODFUNC(func, op) static void func(struct canfd_frame *cf, \ > @@ -157,17 +169,56 @@ struct cgw_job { > > MODFUNC(mod_and_id, cf->can_id &= mod->modframe.and.can_id) > MODFUNC(mod_and_len, cf->len &= mod->modframe.and.len) > +MODFUNC(mod_and_flags, cf->flags &= mod->modframe.and.flags) > MODFUNC(mod_and_data, *(u64 *)cf->data &= *(u64 *)mod->modframe.and.data) > MODFUNC(mod_or_id, cf->can_id |= mod->modframe.or.can_id) > MODFUNC(mod_or_len, cf->len |= mod->modframe.or.len) > +MODFUNC(mod_or_flags, cf->flags |= mod->modframe.or.flags) > MODFUNC(mod_or_data, *(u64 *)cf->data |= *(u64 *)mod->modframe.or.data) > MODFUNC(mod_xor_id, cf->can_id ^= mod->modframe.xor.can_id) > MODFUNC(mod_xor_len, cf->len ^= mod->modframe.xor.len) > +MODFUNC(mod_xor_flags, cf->flags ^= mod->modframe.xor.flags) > MODFUNC(mod_xor_data, *(u64 *)cf->data ^= *(u64 *)mod->modframe.xor.data) > MODFUNC(mod_set_id, cf->can_id = mod->modframe.set.can_id) > MODFUNC(mod_set_len, cf->len = mod->modframe.set.len) > +MODFUNC(mod_set_flags, cf->flags = mod->modframe.set.flags) > MODFUNC(mod_set_data, *(u64 *)cf->data = *(u64 *)mod->modframe.set.data) > > +static void mod_and_fddata(struct canfd_frame *cf, struct cf_mod *mod) > +{ > + int i; > + > + for(i = 0; i < CANFD_MAX_DLEN; i += 8) > + *(u64 *)(cf->data + i) &= *(u64 *)(mod->modframe.and.data + i); > + > + return; > +} > + > +static void mod_or_fddata(struct canfd_frame *cf, struct cf_mod *mod) > +{ > + int i; > + > + for(i = 0; i < CANFD_MAX_DLEN; i += 8) > + *(u64 *)(cf->data + i) |= *(u64 *)(mod->modframe.or.data + i); > + > + return; > +} > + > +static void mod_xor_fddata(struct canfd_frame *cf, struct cf_mod *mod) > +{ > + int i; > + > + for(i = 0; i < CANFD_MAX_DLEN; i += 8) > + *(u64 *)(cf->data + i) ^= *(u64 *)(mod->modframe.xor.data + i); > + > + return; > +} > + > +static void mod_set_fddata(struct canfd_frame *cf, struct cf_mod *mod) > +{ > + memcpy(cf->data, mod->modframe.set.data, CANFD_MAX_DLEN); > +} > + > static void canframecpy(struct canfd_frame *dst, struct can_frame *src) > { > /* > @@ -181,10 +232,27 @@ static void canframecpy(struct canfd_frame *dst, struct can_frame *src) > *(u64 *)dst->data = *(u64 *)src->data; > } > > -static int cgw_chk_csum_parms(s8 fr, s8 to, s8 re) > +static void canfdframecpy(struct canfd_frame *dst, struct canfd_frame *src) > +{ > + /* > + * Copy the struct members separately to ensure that no uninitialized > + * data are copied in the 2 bytes hole of the struct. This is needed > + * to make easy compares of the data in the struct cf_mod. > + */ > + > + dst->can_id = src->can_id; > + dst->flags = src->flags; > + dst->len = src->len; > + memcpy(dst->data, src->data, CANFD_MAX_DLEN); > +} > + > +static int cgw_chk_csum_parms(s8 fr, s8 to, s8 re, struct rtcanmsg *r) > { > s8 dlen = CAN_MAX_DLEN; > > + if (r->flags & CGW_FLAGS_CAN_FD) > + dlen = CANFD_MAX_DLEN; > + > /* > * absolute dlc values 0 .. 7 => 0 .. 7, e.g. data [0] > * relative to received dlc -1 .. -8 : > @@ -355,6 +423,15 @@ static void can_can_gw_rcv(struct sk_buff *skb, void *data) > struct sk_buff *nskb; > int modidx = 0; > > + /* process strictly Classic CAN or CAN FD frames */ > + if (gwj->flags & CGW_FLAGS_CAN_FD) { > + if (skb->len != CANFD_MTU) > + return; > + } else { > + if (skb->len != CAN_MTU) > + return; > + } > + > /* > * Do not handle CAN frames routed more than 'max_hops' times. > * In general we should never catch this delimiter which is intended > @@ -425,23 +502,22 @@ static void can_can_gw_rcv(struct sk_buff *skb, void *data) > int max_len = nskb->len - offsetof(struct canfd_frame, data); I know, this is original code...but max_len can either be 8 (for CAN frames) or 64 (for CAN-FD frames)? Because we always have full can_frame or canfd_frame in the skb, right? I assume a lot more will break if the len neither 8 nor 64. > > /* dlc may have changed, make sure it fits to the CAN frame */ > - if (cf->len > max_len) > - goto out_delete; > + if (cf->len > max_len) { > + /* delete frame due to misconfiguration */ > + gwj->deleted_frames++; > + kfree_skb(nskb); > + return; > + } > > - /* check for checksum updates in classic CAN length only */ > - if (gwj->mod.csumfunc.crc8) { > - if (cf->len > 8) > - goto out_delete; > + /* ensure a valid CAN (FD) frame data length */ > + cf->len = validate_len[cf->len]; This looks strange to me, but I cannot say if I don't userstand this or if there really is a potential problem: - first you calculate max_len - the cf->len > max_len is discarded - but then cf->len is "rounded up" via validate_len[]. What's the purpose of the last step? 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
