All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] can: remove CAN FD compatibility for CAN 2.0 sockets
@ 2014-02-27 20:16 Oliver Hartkopp
  2014-02-28 15:23 ` Stephane Grosjean
  0 siblings, 1 reply; 3+ messages in thread
From: Oliver Hartkopp @ 2014-02-27 20:16 UTC (permalink / raw)
  To: linux-can; +Cc: Stephane Grosjean

In commit e2d265d3b587 (canfd: add support for CAN FD in CAN_RAW sockets)
CAN FD frames with a payload length up to 8 byte are passed to legacy
sockets where the CAN FD support was not enabled by the application.

After some discussions with developers at a fair this well meant feature
leads to confusion as no clean switch for CAN / CAN FD is provided to the
application programmer. Additionally a compatibility like this for legacy
CAN_RAW sockets requires some compatibility handling for the sending, e.g.
make CAN2.0 frames a CAN FD frame with BRS at transmission time (?!?).

This will become a mess when people start to develop applications with
real CAN FD hardware. This patch reverts the bad compatibility code.

Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>

---

diff --git a/net/can/raw.c b/net/can/raw.c
index 8be757c..081e81f 100644
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -121,13 +121,9 @@ static void raw_rcv(struct sk_buff *oskb, void *data)
 	if (!ro->recv_own_msgs && oskb->sk == sk)
 		return;
 
-	/* do not pass frames with DLC > 8 to a legacy socket */
-	if (!ro->fd_frames) {
-		struct canfd_frame *cfd = (struct canfd_frame *)oskb->data;
-
-		if (unlikely(cfd->len > CAN_MAX_DLEN))
-			return;
-	}
+	/* do not pass non-CAN2.0 frames to a legacy socket */
+	if (!ro->fd_frames && oskb->len != CAN_MTU)
+		return;
 
 	/* clone the given skb to be able to enqueue it into the rcv queue */
 	skb = skb_clone(oskb, GFP_ATOMIC);
@@ -738,9 +734,7 @@ static int raw_recvmsg(struct kiocb *iocb, struct socket *sock,
 		       struct msghdr *msg, size_t size, int flags)
 {
 	struct sock *sk = sock->sk;
-	struct raw_sock *ro = raw_sk(sk);
 	struct sk_buff *skb;
-	int rxmtu;
 	int err = 0;
 	int noblock;
 
@@ -751,20 +745,10 @@ static int raw_recvmsg(struct kiocb *iocb, struct socket *sock,
 	if (!skb)
 		return err;
 
-	/*
-	 * when serving a legacy socket the DLC <= 8 is already checked inside
-	 * raw_rcv(). Now check if we need to pass a canfd_frame to a legacy
-	 * socket and cut the possible CANFD_MTU/CAN_MTU length to CAN_MTU
-	 */
-	if (!ro->fd_frames)
-		rxmtu = CAN_MTU;
-	else
-		rxmtu = skb->len;
-
-	if (size < rxmtu)
+	if (size < skb->len)
 		msg->msg_flags |= MSG_TRUNC;
 	else
-		size = rxmtu;
+		size = skb->len;
 
 	err = memcpy_toiovec(msg->msg_iov, skb->data, size);
 	if (err < 0) {



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] can: remove CAN FD compatibility for CAN 2.0 sockets
  2014-02-27 20:16 [PATCH] can: remove CAN FD compatibility for CAN 2.0 sockets Oliver Hartkopp
@ 2014-02-28 15:23 ` Stephane Grosjean
  2014-03-01 14:31   ` Oliver Hartkopp
  0 siblings, 1 reply; 3+ messages in thread
From: Stephane Grosjean @ 2014-02-28 15:23 UTC (permalink / raw)
  To: Oliver Hartkopp, linux-can

Hi Oliver,

I agree with this change, so you also have my Acked-by ...

Le 27/02/2014 21:16, Oliver Hartkopp a écrit :
> In commit e2d265d3b587 (canfd: add support for CAN FD in CAN_RAW sockets)
> CAN FD frames with a payload length up to 8 byte are passed to legacy
> sockets where the CAN FD support was not enabled by the application.
>
> After some discussions with developers at a fair this well meant feature
> leads to confusion as no clean switch for CAN / CAN FD is provided to the
> application programmer. Additionally a compatibility like this for legacy
> CAN_RAW sockets requires some compatibility handling for the sending, e.g.
> make CAN2.0 frames a CAN FD frame with BRS at transmission time (?!?).
>
> This will become a mess when people start to develop applications with
> real CAN FD hardware. This patch reverts the bad compatibility code.
>
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Acked-by: Stephane Grosjean <s.grosjean@peak-system.com>
> ---
>
> diff --git a/net/can/raw.c b/net/can/raw.c
> index 8be757c..081e81f 100644
> --- a/net/can/raw.c
> +++ b/net/can/raw.c
> @@ -121,13 +121,9 @@ static void raw_rcv(struct sk_buff *oskb, void *data)
>   	if (!ro->recv_own_msgs && oskb->sk == sk)
>   		return;
>   
> -	/* do not pass frames with DLC > 8 to a legacy socket */
> -	if (!ro->fd_frames) {
> -		struct canfd_frame *cfd = (struct canfd_frame *)oskb->data;
> -
> -		if (unlikely(cfd->len > CAN_MAX_DLEN))
> -			return;
> -	}
> +	/* do not pass non-CAN2.0 frames to a legacy socket */
> +	if (!ro->fd_frames && oskb->len != CAN_MTU)
> +		return;
>   
>   	/* clone the given skb to be able to enqueue it into the rcv queue */
>   	skb = skb_clone(oskb, GFP_ATOMIC);
> @@ -738,9 +734,7 @@ static int raw_recvmsg(struct kiocb *iocb, struct socket *sock,
>   		       struct msghdr *msg, size_t size, int flags)
>   {
>   	struct sock *sk = sock->sk;
> -	struct raw_sock *ro = raw_sk(sk);
>   	struct sk_buff *skb;
> -	int rxmtu;
>   	int err = 0;
>   	int noblock;
>   
> @@ -751,20 +745,10 @@ static int raw_recvmsg(struct kiocb *iocb, struct socket *sock,
>   	if (!skb)
>   		return err;
>   
> -	/*
> -	 * when serving a legacy socket the DLC <= 8 is already checked inside
> -	 * raw_rcv(). Now check if we need to pass a canfd_frame to a legacy
> -	 * socket and cut the possible CANFD_MTU/CAN_MTU length to CAN_MTU
> -	 */
> -	if (!ro->fd_frames)
> -		rxmtu = CAN_MTU;
> -	else
> -		rxmtu = skb->len;
> -
> -	if (size < rxmtu)
> +	if (size < skb->len)
>   		msg->msg_flags |= MSG_TRUNC;
>   	else
> -		size = rxmtu;
> +		size = skb->len;
>   
>   	err = memcpy_toiovec(msg->msg_iov, skb->data, size);
>   	if (err < 0) {
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-can" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
PEAK-System Technik GmbH, Otto-Roehm-Strasse 69, D-64293 Darmstadt 
Geschaeftsleitung: A.Gach/U.Wilhelm,St.Nr.:007/241/13586 FA Darmstadt 
HRB-9183 Darmstadt, Ust.IdNr.:DE 202220078, WEE-Reg.-Nr.: DE39305391 
Tel.+49 (0)6151-817320 / Fax:+49 (0)6151-817329, info@peak-system.com

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] can: remove CAN FD compatibility for CAN 2.0 sockets
  2014-02-28 15:23 ` Stephane Grosjean
@ 2014-03-01 14:31   ` Oliver Hartkopp
  0 siblings, 0 replies; 3+ messages in thread
From: Oliver Hartkopp @ 2014-03-01 14:31 UTC (permalink / raw)
  To: Marc Kleine-Budde; +Cc: Stephane Grosjean, linux-can

On 28.02.2014 16:23, Stephane Grosjean wrote:
> I agree with this change, so you also have my Acked-by ...
> 

(..)

>>
>> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> Acked-by: Stephane Grosjean <s.grosjean@peak-system.com>

Hi Marc,

there's also some documentation to be removed when reverting this
compatibility stuff.

Will send a V2 to have this in one single patch in a minute.

Regards,
Oliver




^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2014-03-01 14:31 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-27 20:16 [PATCH] can: remove CAN FD compatibility for CAN 2.0 sockets Oliver Hartkopp
2014-02-28 15:23 ` Stephane Grosjean
2014-03-01 14:31   ` Oliver Hartkopp

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.