Commit ac911bfe authored by Oliver Hartkopp's avatar Oliver Hartkopp Committed by Marc Kleine-Budde
Browse files

can: isotp: implement cleanups / improvements from review 

As pointed out by Jakub Kicinski here:
http://lore.kernel.org/r/20201009175751.5c54097f@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com


this patch addresses the remarked issues:

- remove empty line in comment
- remove default=y for CAN_ISOTP in Kconfig
- make use of pr_notice_once()
- use GFP_ATOMIC instead of gfp_any() in soft hrtimer context

The version strings in the CAN subsystem are removed by a separate patch.

Signed-off-by: default avatarOliver Hartkopp <socketcan@hartkopp.net>
Link: https://lore.kernel.org/r/20201012074354.25839-1-socketcan@hartkopp.net


Signed-off-by: default avatarMarc Kleine-Budde <mkl@pengutronix.de>
parent bc081a69

include/uapi/linux/can/isotp.h

+0 −1
Original line number Diff line number Diff line
@@ -160,7 +160,6 @@ struct can_isotp_ll_options { 
 * these default settings can be changed via sockopts.

 * For that reason the STmin value is intentionally _not_ checked for

 * consistency and copied directly into the flow control (FC) frame.

 *

 */



#endif /* !_UAPI_CAN_ISOTP_H */

net/can/Kconfig

+2 −1
Original line number Diff line number Diff line
@@ -57,7 +57,6 @@ source "net/can/j1939/Kconfig" 


config CAN_ISOTP

	tristate "ISO 15765-2:2016 CAN transport protocol"

	default y

	help

	  CAN Transport Protocols offer support for segmented Point-to-Point

	  communication between CAN nodes via two defined CAN Identifiers.
@@ -67,6 +66,8 @@ config CAN_ISOTP 
	  vehicle diagnosis (UDS, ISO 14229) or IP-over-CAN traffic.

	  This protocol driver implements data transfers according to

	  ISO 15765-2:2016 for 'classic' CAN and CAN FD frame types.

	  If you want to perform automotive vehicle diagnostic services (UDS),

	  say 'y'.



source "drivers/net/can/Kconfig"

net/can/isotp.c

+7 −7
Original line number Diff line number Diff line
@@ -222,7 +222,7 @@ static int isotp_send_fc(struct sock *sk, int ae, u8 flowstatus) 


	can_send_ret = can_send(nskb, 1);

	if (can_send_ret)

		printk_once(KERN_NOTICE "can-isotp: %s: can_send_ret %d\n",

		pr_notice_once("can-isotp: %s: can_send_ret %d\n",

			       __func__, can_send_ret);



	dev_put(dev);
@@ -769,7 +769,7 @@ static enum hrtimer_restart isotp_tx_timer_handler(struct hrtimer *hrtimer) 


isotp_tx_burst:

		skb = alloc_skb(so->ll.mtu + sizeof(struct can_skb_priv),

				gfp_any());

				GFP_ATOMIC);

		if (!skb) {

			dev_put(dev);

			break;
@@ -798,7 +798,7 @@ isotp_tx_burst: 


		can_send_ret = can_send(skb, 1);

		if (can_send_ret)

			printk_once(KERN_NOTICE "can-isotp: %s: can_send_ret %d\n",

			pr_notice_once("can-isotp: %s: can_send_ret %d\n",

				       __func__, can_send_ret);



		if (so->tx.idx >= so->tx.len) {
@@ -942,7 +942,7 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size) 
	err = can_send(skb, 1);

	dev_put(dev);

	if (err) {

		printk_once(KERN_NOTICE "can-isotp: %s: can_send_ret %d\n",

		pr_notice_once("can-isotp: %s: can_send_ret %d\n",

			       __func__, err);

		return err;

	}

CRA Git | Maintained and supported by SUSTech CRA and CCSE