On Mon, 2024-05-06 at 20:08 +0300, Ilpo Järvinen wrote: > On Thu, 2 May 2024, Christoph Fritz wrote: > > > Enhance CAN broadcast manager with RX_LIN_SETUP and RX_LIN_DELETE > > operations to setup automatic LIN frame responses in responder mode. > > > > Additionally, the patch introduces the LIN_EVENT_FRAME flag to > > setup event-triggered LIN frames. > > > > Signed-off-by: Christoph Fritz <christoph.fritz@xxxxxxxxx> > > --- > > include/uapi/linux/can/bcm.h | 5 ++- > > net/can/bcm.c | 74 +++++++++++++++++++++++++++++++++++- > > 2 files changed, 77 insertions(+), 2 deletions(-) > > > > diff --git a/include/uapi/linux/can/bcm.h b/include/uapi/linux/can/bcm.h > > index f1e45f533a72c..c46268a114078 100644 > > --- a/include/uapi/linux/can/bcm.h > > +++ b/include/uapi/linux/can/bcm.h > > @@ -86,7 +86,9 @@ enum { > > TX_EXPIRED, /* notification on performed transmissions (count=0) */ > > RX_STATUS, /* reply to RX_READ request */ > > RX_TIMEOUT, /* cyclic message is absent */ > > - RX_CHANGED /* updated CAN frame (detected content change) */ > > + RX_CHANGED, /* updated CAN frame (detected content change) */ > > + RX_LIN_SETUP, /* create auto-response for LIN frame */ > > + RX_LIN_DELETE, /* remove auto-response for LIN frame */ > > }; > > > > #define SETTIMER 0x0001 > > @@ -101,5 +103,6 @@ enum { > > #define TX_RESET_MULTI_IDX 0x0200 > > #define RX_RTR_FRAME 0x0400 > > #define CAN_FD_FRAME 0x0800 > > +#define LIN_EVENT_FRAME 0x1000 > > > > #endif /* !_UAPI_CAN_BCM_H */ > > diff --git a/net/can/bcm.c b/net/can/bcm.c > > index 27d5fcf0eac9d..a717e594234d1 100644 > > --- a/net/can/bcm.c > > +++ b/net/can/bcm.c > > @@ -59,6 +59,7 @@ > > #include <linux/can/bcm.h> > > #include <linux/slab.h> > > #include <net/sock.h> > > +#include <net/lin.h> > > #include <net/net_namespace.h> > > > > /* > > @@ -1330,6 +1331,59 @@ static int bcm_tx_send(struct msghdr *msg, int ifindex, struct sock *sk, > > return cfsiz + MHSIZ; > > } > > > > +static int bcm_lin_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, > > + int ifindex, struct sock *sk, int cfsiz, int is_active) > > +{ > > + struct lin_responder_answer answ; > > + struct net_device *dev; > > + struct sk_buff *skb; > > + struct canfd_frame cf; > > + netdevice_tracker tracker; > > + size_t sz; > > + int ret; > > + > > + if (msg_head->nframes > 1) > > + return -EINVAL; > > + > > + if (!(msg_head->flags & CAN_FD_FRAME)) > > + return -EINVAL; > > + > > + ret = memcpy_from_msg(&cf, msg, cfsiz); > > + if (ret < 0) > > + return ret; > > + > > + answ.lf.lin_id = cf.can_id & LIN_ID_MASK; > > + answ.is_active = is_active; > > + answ.is_event_frame = !!(msg_head->flags & LIN_EVENT_FRAME); > > + answ.event_associated_id = msg_head->can_id; > > + answ.lf.len = min(cf.len, LIN_MAX_DLEN); > > + memcpy(answ.lf.data, cf.data, answ.lf.len); > > + sz = min(sizeof(struct lin_responder_answer), sizeof(cf.data)); > > + cf.can_id |= LIN_RXOFFLOAD_DATA_FLAG; > > + memcpy(cf.data, &answ, sz); > > + > > + dev = netdev_get_by_index(sock_net(sk), ifindex, &tracker, GFP_KERNEL); > > + if (!dev) > > + return -ENODEV; > > + > > + skb = alloc_skb(cfsiz + sizeof(struct can_skb_priv), gfp_any()); > > You just called the other function with GFP_KERNEL and you now need > gfp_any(). Which is correct?? I guess GFP_KERNEL but I'm not sure so I'll let gfp_any() decide for netdev_get_by_index() too. > > > + if (!skb) > > + goto lin_out; > > + > > + can_skb_reserve(skb); > > + can_skb_prv(skb)->ifindex = dev->ifindex; > > + can_skb_prv(skb)->skbcnt = 0; > > + skb_put_data(skb, &cf, cfsiz); > > + > > + skb->dev = dev; > > + can_skb_set_owner(skb, sk); > > + ret = can_send(skb, 1); /* send with loopback */ > > + > > +lin_out: > > + netdev_put(dev, &tracker); > > + return ret; > > +} > > + > > /* > > * bcm_sendmsg - process BCM commands (opcodes) from the userspace > > */ > > @@ -1429,12 +1483,30 @@ static int bcm_sendmsg(struct socket *sock, struct msghdr *msg, size_t size) > > > > case TX_SEND: > > /* we need exactly one CAN frame behind the msg head */ > > - if ((msg_head.nframes != 1) || (size != cfsiz + MHSIZ)) > > + if (msg_head.nframes != 1 || size != cfsiz + MHSIZ) > > Unrelated style fix, doesn't belong to this patch. OK ... thanks -- Christoph