* [PATCH net-next 1/2] can-isotp: implement cleanups / improvements from review
@ 2020-10-10 20:49 Oliver Hartkopp
2020-10-10 20:49 ` [PATCH net-next 2/2] can: remove obsolete version strings Oliver Hartkopp
2020-10-11 7:15 ` [PATCH net-next 1/2] can-isotp: implement cleanups / improvements from review Marc Kleine-Budde
0 siblings, 2 replies; 4+ messages in thread
From: Oliver Hartkopp @ 2020-10-10 20:49 UTC (permalink / raw)
To: kuba, netdev; +Cc: mkl, davem, linux-can, Oliver Hartkopp
As pointed out by Jakub Kicinski here:
https://marc.info/?l=linux-can&m=160229286216008
this patch addresses the remarked issues:
- remove empty lines in comment
- remove default=y for CAN_ISOTP in Kconfig
- make use of pr_notice_once()
- use GFP_KERNEL instead of gfp_any() in soft hrtimer context
The version strings in the CAN subsystem are removed by a separate patch.
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
include/uapi/linux/can/isotp.h | 4 +---
net/can/Kconfig | 3 ++-
net/can/isotp.c | 14 +++++++-------
3 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/include/uapi/linux/can/isotp.h b/include/uapi/linux/can/isotp.h
index 553006509f4e..accf0efa46f4 100644
--- a/include/uapi/linux/can/isotp.h
+++ b/include/uapi/linux/can/isotp.h
@@ -151,8 +151,7 @@ struct can_isotp_ll_options {
#define CAN_ISOTP_DEFAULT_LL_TX_DL CAN_MAX_DLEN
#define CAN_ISOTP_DEFAULT_LL_TX_FLAGS 0
-/*
- * Remark on CAN_ISOTP_DEFAULT_RECV_* values:
+/* Remark on CAN_ISOTP_DEFAULT_RECV_* values:
*
* We can strongly assume, that the Linux Kernel implementation of
* CAN_ISOTP is capable to run with BS=0, STmin=0 and WFTmax=0.
@@ -160,7 +159,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 */
diff --git a/net/can/Kconfig b/net/can/Kconfig
index 021fe03a8ed6..224e5e0283a9 100644
--- a/net/can/Kconfig
+++ b/net/can/Kconfig
@@ -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"
diff --git a/net/can/isotp.c b/net/can/isotp.c
index e6ff032b5426..bc3a722c200b 100644
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -222,8 +222,8 @@ 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",
- __func__, can_send_ret);
+ 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_KERNEL);
if (!skb) {
dev_put(dev);
break;
@@ -798,8 +798,8 @@ static enum hrtimer_restart isotp_tx_timer_handler(struct hrtimer *hrtimer)
can_send_ret = can_send(skb, 1);
if (can_send_ret)
- printk_once(KERN_NOTICE "can-isotp: %s: can_send_ret %d\n",
- __func__, can_send_ret);
+ pr_notice_once("can-isotp: %s: can_send_ret %d\n",
+ __func__, can_send_ret);
if (so->tx.idx >= so->tx.len) {
/* we are done */
@@ -942,8 +942,8 @@ 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",
- __func__, err);
+ pr_notice_once("can-isotp: %s: can_send_ret %d\n",
+ __func__, err);
return err;
}
--
2.28.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH net-next 2/2] can: remove obsolete version strings
2020-10-10 20:49 [PATCH net-next 1/2] can-isotp: implement cleanups / improvements from review Oliver Hartkopp
@ 2020-10-10 20:49 ` Oliver Hartkopp
2020-10-11 7:22 ` Marc Kleine-Budde
2020-10-11 7:15 ` [PATCH net-next 1/2] can-isotp: implement cleanups / improvements from review Marc Kleine-Budde
1 sibling, 1 reply; 4+ messages in thread
From: Oliver Hartkopp @ 2020-10-10 20:49 UTC (permalink / raw)
To: kuba, netdev; +Cc: mkl, davem, linux-can, Oliver Hartkopp
As pointed out by Jakub Kicinski here:
https://marc.info/?l=linux-can&m=160229286216008
this patch removes the obsolete version information of the different
CAN protocols and the AF_CAN core module.
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
include/linux/can/core.h | 7 -------
include/net/netns/can.h | 1 -
net/can/af_can.c | 2 +-
net/can/bcm.c | 4 +---
net/can/gw.c | 4 +---
net/can/isotp.c | 4 +---
net/can/proc.c | 12 ------------
net/can/raw.c | 4 +---
8 files changed, 5 insertions(+), 33 deletions(-)
diff --git a/include/linux/can/core.h b/include/linux/can/core.h
index 7da9f1f82e8e..5fb8d0e3f9c1 100644
--- a/include/linux/can/core.h
+++ b/include/linux/can/core.h
@@ -18,13 +18,6 @@
#include <linux/skbuff.h>
#include <linux/netdevice.h>
-#define CAN_VERSION "20170425"
-
-/* increment this number each time you change some user-space interface */
-#define CAN_ABI_VERSION "9"
-
-#define CAN_VERSION_STRING "rev " CAN_VERSION " abi " CAN_ABI_VERSION
-
#define DNAME(dev) ((dev) ? (dev)->name : "any")
/**
diff --git a/include/net/netns/can.h b/include/net/netns/can.h
index b6ab7d1530d7..52fbd8291a96 100644
--- a/include/net/netns/can.h
+++ b/include/net/netns/can.h
@@ -15,7 +15,6 @@ struct can_rcv_lists_stats;
struct netns_can {
#if IS_ENABLED(CONFIG_PROC_FS)
struct proc_dir_entry *proc_dir;
- struct proc_dir_entry *pde_version;
struct proc_dir_entry *pde_stats;
struct proc_dir_entry *pde_reset_stats;
struct proc_dir_entry *pde_rcvlist_all;
diff --git a/net/can/af_can.c b/net/can/af_can.c
index b7d0f6500893..6373ab9c5507 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -875,7 +875,7 @@ static __init int can_init(void)
offsetof(struct can_frame, data) !=
offsetof(struct canfd_frame, data));
- pr_info("can: controller area network core (" CAN_VERSION_STRING ")\n");
+ pr_info("can: controller area network core\n");
rcv_cache = kmem_cache_create("can_receiver", sizeof(struct receiver),
0, 0, NULL);
diff --git a/net/can/bcm.c b/net/can/bcm.c
index 4253915800e6..0e5c37be4a2b 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -81,8 +81,6 @@
(CAN_EFF_MASK | CAN_EFF_FLAG | CAN_RTR_FLAG) : \
(CAN_SFF_MASK | CAN_EFF_FLAG | CAN_RTR_FLAG))
-#define CAN_BCM_VERSION "20170425"
-
MODULE_DESCRIPTION("PF_CAN broadcast manager protocol");
MODULE_LICENSE("Dual BSD/GPL");
MODULE_AUTHOR("Oliver Hartkopp <oliver.hartkopp@volkswagen.de>");
@@ -1696,7 +1694,7 @@ static int __init bcm_module_init(void)
{
int err;
- pr_info("can: broadcast manager protocol (rev " CAN_BCM_VERSION " t)\n");
+ pr_info("can: broadcast manager protocol\n");
err = can_proto_register(&bcm_can_proto);
if (err < 0) {
diff --git a/net/can/gw.c b/net/can/gw.c
index 49b4e3d91ad6..6b790b6ff8d2 100644
--- a/net/can/gw.c
+++ b/net/can/gw.c
@@ -59,7 +59,6 @@
#include <net/net_namespace.h>
#include <net/sock.h>
-#define CAN_GW_VERSION "20190810"
#define CAN_GW_NAME "can-gw"
MODULE_DESCRIPTION("PF_CAN netlink gateway");
@@ -1194,8 +1193,7 @@ static __init int cgw_module_init(void)
/* sanitize given module parameter */
max_hops = clamp_t(unsigned int, max_hops, CGW_MIN_HOPS, CGW_MAX_HOPS);
- pr_info("can: netlink gateway (rev " CAN_GW_VERSION ") max_hops=%d\n",
- max_hops);
+ pr_info("can: netlink gateway - max_hops=%d\n", max_hops);
ret = register_pernet_subsys(&cangw_pernet_ops);
if (ret)
diff --git a/net/can/isotp.c b/net/can/isotp.c
index bc3a722c200b..5338e990bde1 100644
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -72,8 +72,6 @@
#include <net/sock.h>
#include <net/net_namespace.h>
-#define CAN_ISOTP_VERSION "20200928"
-
MODULE_DESCRIPTION("PF_CAN isotp 15765-2:2016 protocol");
MODULE_LICENSE("Dual BSD/GPL");
MODULE_AUTHOR("Oliver Hartkopp <socketcan@hartkopp.net>");
@@ -1408,7 +1406,7 @@ static __init int isotp_module_init(void)
{
int err;
- pr_info("can: isotp protocol (rev " CAN_ISOTP_VERSION ")\n");
+ pr_info("can: isotp protocol\n");
err = can_proto_register(&isotp_can_proto);
if (err < 0)
diff --git a/net/can/proc.c b/net/can/proc.c
index a4eb06c9eb70..550928b8b8a2 100644
--- a/net/can/proc.c
+++ b/net/can/proc.c
@@ -54,7 +54,6 @@
* proc filenames for the PF_CAN core
*/
-#define CAN_PROC_VERSION "version"
#define CAN_PROC_STATS "stats"
#define CAN_PROC_RESET_STATS "reset_stats"
#define CAN_PROC_RCVLIST_ALL "rcvlist_all"
@@ -293,12 +292,6 @@ static int can_reset_stats_proc_show(struct seq_file *m, void *v)
return 0;
}
-static int can_version_proc_show(struct seq_file *m, void *v)
-{
- seq_printf(m, "%s\n", CAN_VERSION_STRING);
- return 0;
-}
-
static inline void can_rcvlist_proc_show_one(struct seq_file *m, int idx,
struct net_device *dev,
struct can_dev_rcv_lists *dev_rcv_lists)
@@ -441,8 +434,6 @@ void can_init_proc(struct net *net)
}
/* own procfs entries from the AF_CAN core */
- net->can.pde_version = proc_create_net_single(CAN_PROC_VERSION, 0644,
- net->can.proc_dir, can_version_proc_show, NULL);
net->can.pde_stats = proc_create_net_single(CAN_PROC_STATS, 0644,
net->can.proc_dir, can_stats_proc_show, NULL);
net->can.pde_reset_stats = proc_create_net_single(CAN_PROC_RESET_STATS,
@@ -471,9 +462,6 @@ void can_init_proc(struct net *net)
*/
void can_remove_proc(struct net *net)
{
- if (net->can.pde_version)
- remove_proc_entry(CAN_PROC_VERSION, net->can.proc_dir);
-
if (net->can.pde_stats)
remove_proc_entry(CAN_PROC_STATS, net->can.proc_dir);
diff --git a/net/can/raw.c b/net/can/raw.c
index ea70850f9152..6ec8aa1d0da4 100644
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -55,8 +55,6 @@
#include <net/sock.h>
#include <net/net_namespace.h>
-#define CAN_RAW_VERSION CAN_VERSION
-
MODULE_DESCRIPTION("PF_CAN raw protocol");
MODULE_LICENSE("Dual BSD/GPL");
MODULE_AUTHOR("Urs Thuermann <urs.thuermann@volkswagen.de>");
@@ -885,7 +883,7 @@ static __init int raw_module_init(void)
{
int err;
- pr_info("can: raw protocol (rev " CAN_RAW_VERSION ")\n");
+ pr_info("can: raw protocol\n");
err = can_proto_register(&raw_can_proto);
if (err < 0)
--
2.28.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net-next 1/2] can-isotp: implement cleanups / improvements from review
2020-10-10 20:49 [PATCH net-next 1/2] can-isotp: implement cleanups / improvements from review Oliver Hartkopp
2020-10-10 20:49 ` [PATCH net-next 2/2] can: remove obsolete version strings Oliver Hartkopp
@ 2020-10-11 7:15 ` Marc Kleine-Budde
1 sibling, 0 replies; 4+ messages in thread
From: Marc Kleine-Budde @ 2020-10-11 7:15 UTC (permalink / raw)
To: Oliver Hartkopp, kuba, netdev; +Cc: davem, linux-can
[-- Attachment #1.1: Type: text/plain, Size: 4523 bytes --]
On 10/10/20 10:49 PM, Oliver Hartkopp wrote:
> As pointed out by Jakub Kicinski here:
> https://marc.info/?l=linux-can&m=160229286216008
> this patch addresses the remarked issues:
>
> - remove empty lines in comment
> - remove default=y for CAN_ISOTP in Kconfig
> - make use of pr_notice_once()
> - use GFP_KERNEL instead of gfp_any() in soft hrtimer context
>
> The version strings in the CAN subsystem are removed by a separate patch.
>
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> ---
> include/uapi/linux/can/isotp.h | 4 +---
> net/can/Kconfig | 3 ++-
> net/can/isotp.c | 14 +++++++-------
> 3 files changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/include/uapi/linux/can/isotp.h b/include/uapi/linux/can/isotp.h
> index 553006509f4e..accf0efa46f4 100644
> --- a/include/uapi/linux/can/isotp.h
> +++ b/include/uapi/linux/can/isotp.h
> @@ -151,8 +151,7 @@ struct can_isotp_ll_options {
> #define CAN_ISOTP_DEFAULT_LL_TX_DL CAN_MAX_DLEN
> #define CAN_ISOTP_DEFAULT_LL_TX_FLAGS 0
>
> -/*
> - * Remark on CAN_ISOTP_DEFAULT_RECV_* values:
> +/* Remark on CAN_ISOTP_DEFAULT_RECV_* values:
I think for uapi headers we use the default commenting style, not the netdev one.
> *
> * We can strongly assume, that the Linux Kernel implementation of
> * CAN_ISOTP is capable to run with BS=0, STmin=0 and WFTmax=0.
> @@ -160,7 +159,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 */
> diff --git a/net/can/Kconfig b/net/can/Kconfig
> index 021fe03a8ed6..224e5e0283a9 100644
> --- a/net/can/Kconfig
> +++ b/net/can/Kconfig
> @@ -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"
>
> diff --git a/net/can/isotp.c b/net/can/isotp.c
> index e6ff032b5426..bc3a722c200b 100644
> --- a/net/can/isotp.c
> +++ b/net/can/isotp.c
> @@ -222,8 +222,8 @@ 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",
> - __func__, can_send_ret);
> + pr_notice_once("can-isotp: %s: can_send_ret %d\n",
> + __func__, can_send_ret);
please define a pr_fmt for the "can-isotp: " prefix.
>
> 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_KERNEL);
> if (!skb) {
> dev_put(dev);
> break;
> @@ -798,8 +798,8 @@ static enum hrtimer_restart isotp_tx_timer_handler(struct hrtimer *hrtimer)
>
> can_send_ret = can_send(skb, 1);
> if (can_send_ret)
> - printk_once(KERN_NOTICE "can-isotp: %s: can_send_ret %d\n",
> - __func__, can_send_ret);
> + pr_notice_once("can-isotp: %s: can_send_ret %d\n",
> + __func__, can_send_ret);
>
> if (so->tx.idx >= so->tx.len) {
> /* we are done */
> @@ -942,8 +942,8 @@ 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",
> - __func__, err);
> + pr_notice_once("can-isotp: %s: can_send_ret %d\n",
> + __func__, err);
> return err;
> }
>
>
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: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net-next 2/2] can: remove obsolete version strings
2020-10-10 20:49 ` [PATCH net-next 2/2] can: remove obsolete version strings Oliver Hartkopp
@ 2020-10-11 7:22 ` Marc Kleine-Budde
0 siblings, 0 replies; 4+ messages in thread
From: Marc Kleine-Budde @ 2020-10-11 7:22 UTC (permalink / raw)
To: Oliver Hartkopp, kuba, netdev; +Cc: davem, linux-can
[-- Attachment #1.1: Type: text/plain, Size: 801 bytes --]
On 10/10/20 10:49 PM, Oliver Hartkopp wrote:
> As pointed out by Jakub Kicinski here:
> https://marc.info/?l=linux-can&m=160229286216008
Better use https://lore.kernel.org for this. The URL is constructed by adding
"/r/$MESSAGE_ID":
http://lore.kernel.org/r/20201009175751.5c54097f@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com
> this patch removes the obsolete version information of the different
> CAN protocols and the AF_CAN core module.
>
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
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: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-10-11 7:22 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-10 20:49 [PATCH net-next 1/2] can-isotp: implement cleanups / improvements from review Oliver Hartkopp
2020-10-10 20:49 ` [PATCH net-next 2/2] can: remove obsolete version strings Oliver Hartkopp
2020-10-11 7:22 ` Marc Kleine-Budde
2020-10-11 7:15 ` [PATCH net-next 1/2] can-isotp: implement cleanups / improvements from review Marc Kleine-Budde
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.