All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] can: isotp: remove re-binding of bound socket
@ 2022-04-22  8:23 Oliver Hartkopp
  2022-04-28  6:54 ` Marc Kleine-Budde
  0 siblings, 1 reply; 3+ messages in thread
From: Oliver Hartkopp @ 2022-04-22  8:23 UTC (permalink / raw)
  To: linux-can; +Cc: Oliver Hartkopp

As a carry over from the CAN_RAW socket (which allows to change the CAN
interface while mantaining the filter setup) the re-binding of the
CAN_ISOTP socket needs to take care about CAN ID address information and
subscriptions. It turned out that this feature is so limited (e.g. the
sockopts remain fix) that it finally has never been needed/used.

In opposite to the stateless CAN_RAW socket the switching of the CAN ID
subscriptions might additionally lead to an interrupted ongoing PDU
reception. So better remove this unneeded complexity.

Fixes: e057dd3fc20f ("can: add ISO 15765-2:2016 transport protocol")
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
 net/can/isotp.c | 25 +++++--------------------
 1 file changed, 5 insertions(+), 20 deletions(-)

diff --git a/net/can/isotp.c b/net/can/isotp.c
index ff5d7870294e..1e7c6a460ef9 100644
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -1187,24 +1187,25 @@ static int isotp_bind(struct socket *sock, struct sockaddr *uaddr, int len)
 	if (!addr->can_ifindex)
 		return -ENODEV;
 
 	lock_sock(sk);
 
+	if (so->bound) {
+		err = -EINVAL;
+		goto out;
+	}
+
 	/* do not register frame reception for functional addressing */
 	if (so->opt.flags & CAN_ISOTP_SF_BROADCAST)
 		do_rx_reg = 0;
 
 	/* do not validate rx address for functional addressing */
 	if (do_rx_reg && rx_id == tx_id) {
 		err = -EADDRNOTAVAIL;
 		goto out;
 	}
 
-	if (so->bound && addr->can_ifindex == so->ifindex &&
-	    rx_id == so->rxid && tx_id == so->txid)
-		goto out;
-
 	dev = dev_get_by_index(net, addr->can_ifindex);
 	if (!dev) {
 		err = -ENODEV;
 		goto out;
 	}
@@ -1235,26 +1236,10 @@ static int isotp_bind(struct socket *sock, struct sockaddr *uaddr, int len)
 				isotp_rcv_echo, sk, "isotpe", sk);
 	}
 
 	dev_put(dev);
 
-	if (so->bound && do_rx_reg) {
-		/* unregister old filter */
-		if (so->ifindex) {
-			dev = dev_get_by_index(net, so->ifindex);
-			if (dev) {
-				can_rx_unregister(net, dev, so->rxid,
-						  SINGLE_MASK(so->rxid),
-						  isotp_rcv, sk);
-				can_rx_unregister(net, dev, so->txid,
-						  SINGLE_MASK(so->txid),
-						  isotp_rcv_echo, sk);
-				dev_put(dev);
-			}
-		}
-	}
-
 	/* switch to new settings */
 	so->ifindex = ifindex;
 	so->rxid = rx_id;
 	so->txid = tx_id;
 	so->bound = 1;
-- 
2.30.2


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

* Re: [PATCH] can: isotp: remove re-binding of bound socket
  2022-04-22  8:23 [PATCH] can: isotp: remove re-binding of bound socket Oliver Hartkopp
@ 2022-04-28  6:54 ` Marc Kleine-Budde
  2022-04-28  7:05   ` Oliver Hartkopp
  0 siblings, 1 reply; 3+ messages in thread
From: Marc Kleine-Budde @ 2022-04-28  6:54 UTC (permalink / raw)
  To: Oliver Hartkopp; +Cc: linux-can

[-- Attachment #1: Type: text/plain, Size: 1145 bytes --]

Hello Oliver,

On 22.04.2022 10:23:37, Oliver Hartkopp wrote:
> As a carry over from the CAN_RAW socket (which allows to change the CAN
> interface while mantaining the filter setup) the re-binding of the
> CAN_ISOTP socket needs to take care about CAN ID address information and
> subscriptions. It turned out that this feature is so limited (e.g. the
> sockopts remain fix) that it finally has never been needed/used.
> 
> In opposite to the stateless CAN_RAW socket the switching of the CAN ID
> subscriptions might additionally lead to an interrupted ongoing PDU
> reception. So better remove this unneeded complexity.
> 
> Fixes: e057dd3fc20f ("can: add ISO 15765-2:2016 transport protocol")
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>

Which branch should I apply the patch to?

[ ] can
[ ] can+stable
[ ] can-next

regards,
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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH] can: isotp: remove re-binding of bound socket
  2022-04-28  6:54 ` Marc Kleine-Budde
@ 2022-04-28  7:05   ` Oliver Hartkopp
  0 siblings, 0 replies; 3+ messages in thread
From: Oliver Hartkopp @ 2022-04-28  7:05 UTC (permalink / raw)
  To: Marc Kleine-Budde; +Cc: linux-can

[x] can+stable please!

Many thanks,
Oliver

On 28.04.22 08:54, Marc Kleine-Budde wrote:
> Hello Oliver,
> 
> On 22.04.2022 10:23:37, Oliver Hartkopp wrote:
>> As a carry over from the CAN_RAW socket (which allows to change the CAN
>> interface while mantaining the filter setup) the re-binding of the
>> CAN_ISOTP socket needs to take care about CAN ID address information and
>> subscriptions. It turned out that this feature is so limited (e.g. the
>> sockopts remain fix) that it finally has never been needed/used.
>>
>> In opposite to the stateless CAN_RAW socket the switching of the CAN ID
>> subscriptions might additionally lead to an interrupted ongoing PDU
>> reception. So better remove this unneeded complexity.
>>
>> Fixes: e057dd3fc20f ("can: add ISO 15765-2:2016 transport protocol")
>> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> 
> Which branch should I apply the patch to?
> 
> [ ] can
> [ ] can+stable
> [ ] can-next
> 
> regards,
> Marc
> 

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

end of thread, other threads:[~2022-04-28  7:05 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-22  8:23 [PATCH] can: isotp: remove re-binding of bound socket Oliver Hartkopp
2022-04-28  6:54 ` Marc Kleine-Budde
2022-04-28  7:05   ` 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.