* [PATCH net-next v2 1/2] can-isotp: implement cleanups / improvements from review @ 2020-10-11 9:24 Oliver Hartkopp 2020-10-11 9:24 ` [PATCH net-next v2 2/2] can: remove obsolete version strings Oliver Hartkopp ` (3 more replies) 0 siblings, 4 replies; 6+ messages in thread From: Oliver Hartkopp @ 2020-10-11 9:24 UTC (permalink / raw) To: kuba, netdev; +Cc: mkl, davem, linux-can, Oliver Hartkopp 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_KERNEL instead of gfp_any() in soft hrtimer context - make use of pr_fmt() [suggested my Marc Kleine-Budde] 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 | 1 - net/can/Kconfig | 3 ++- net/can/isotp.c | 19 ++++++++++--------- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/include/uapi/linux/can/isotp.h b/include/uapi/linux/can/isotp.h index 553006509f4e..7793b26aa154 100644 --- a/include/uapi/linux/can/isotp.h +++ b/include/uapi/linux/can/isotp.h @@ -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 */ 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..22187669c5c9 100644 --- a/net/can/isotp.c +++ b/net/can/isotp.c @@ -79,6 +79,8 @@ MODULE_LICENSE("Dual BSD/GPL"); MODULE_AUTHOR("Oliver Hartkopp <socketcan@hartkopp.net>"); MODULE_ALIAS("can-proto-6"); +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + #define SINGLE_MASK(id) (((id) & CAN_EFF_FLAG) ? \ (CAN_EFF_MASK | CAN_EFF_FLAG | CAN_RTR_FLAG) : \ (CAN_SFF_MASK | CAN_EFF_FLAG | CAN_RTR_FLAG)) @@ -222,8 +224,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("%s: can_send_ret %d\n", + __func__, can_send_ret); dev_put(dev); @@ -769,7 +771,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 +800,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("%s: can_send_ret %d\n", + __func__, can_send_ret); if (so->tx.idx >= so->tx.len) { /* we are done */ @@ -942,8 +944,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", - __func__, err); + pr_notice_once("%s: can_send_ret %d\n", __func__, err); return err; } @@ -1408,11 +1409,11 @@ static __init int isotp_module_init(void) { int err; - pr_info("can: isotp protocol (rev " CAN_ISOTP_VERSION ")\n"); + pr_info("isotp protocol (rev " CAN_ISOTP_VERSION ")\n"); err = can_proto_register(&isotp_can_proto); if (err < 0) - pr_err("can: registration of isotp protocol failed\n"); + pr_err("registration of isotp protocol failed\n"); return err; } -- 2.28.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net-next v2 2/2] can: remove obsolete version strings 2020-10-11 9:24 [PATCH net-next v2 1/2] can-isotp: implement cleanups / improvements from review Oliver Hartkopp @ 2020-10-11 9:24 ` Oliver Hartkopp 2020-10-11 15:44 ` [PATCH net-next v2 1/2] can-isotp: implement cleanups / improvements from review Jakub Kicinski ` (2 subsequent siblings) 3 siblings, 0 replies; 6+ messages in thread From: Oliver Hartkopp @ 2020-10-11 9:24 UTC (permalink / raw) To: kuba, netdev; +Cc: mkl, davem, linux-can, Oliver Hartkopp As pointed out by Jakub Kicinski here: 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> --- 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 22187669c5c9..132ca6c471d6 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>"); @@ -1409,7 +1407,7 @@ static __init int isotp_module_init(void) { int err; - pr_info("isotp protocol (rev " CAN_ISOTP_VERSION ")\n"); + pr_info("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] 6+ messages in thread
* Re: [PATCH net-next v2 1/2] can-isotp: implement cleanups / improvements from review 2020-10-11 9:24 [PATCH net-next v2 1/2] can-isotp: implement cleanups / improvements from review Oliver Hartkopp 2020-10-11 9:24 ` [PATCH net-next v2 2/2] can: remove obsolete version strings Oliver Hartkopp @ 2020-10-11 15:44 ` Jakub Kicinski 2020-10-12 7:41 ` Oliver Hartkopp 2020-10-11 15:45 ` Jakub Kicinski 2020-10-11 17:59 ` kernel test robot 3 siblings, 1 reply; 6+ messages in thread From: Jakub Kicinski @ 2020-10-11 15:44 UTC (permalink / raw) To: Oliver Hartkopp; +Cc: netdev, mkl, davem, linux-can On Sun, 11 Oct 2020 11:24:07 +0200 Oliver Hartkopp wrote: > diff --git a/net/can/isotp.c b/net/can/isotp.c > index e6ff032b5426..22187669c5c9 100644 > --- a/net/can/isotp.c > +++ b/net/can/isotp.c > @@ -79,6 +79,8 @@ MODULE_LICENSE("Dual BSD/GPL"); > MODULE_AUTHOR("Oliver Hartkopp <socketcan@hartkopp.net>"); > MODULE_ALIAS("can-proto-6"); > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt You need to move this before the includes: net/can/isotp.c:82: warning: "pr_fmt" redefined 82 | #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt | In file included from ../include/linux/kernel.h:15, from ../include/linux/list.h:9, from ../include/linux/module.h:12, from ../net/can/isotp.c:56: include/linux/printk.h:297: note: this is the location of the previous definition 297 | #define pr_fmt(fmt) fmt | net/can/isotp.c:82:9: warning: preprocessor token pr_fmt redefined net/can/isotp.c: note: in included file (through ../include/linux/kernel.h, ../include/linux/list.h, ../include/linux/module.h): include/linux/printk.h:297:9: this was the original definition ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next v2 1/2] can-isotp: implement cleanups / improvements from review 2020-10-11 15:44 ` [PATCH net-next v2 1/2] can-isotp: implement cleanups / improvements from review Jakub Kicinski @ 2020-10-12 7:41 ` Oliver Hartkopp 0 siblings, 0 replies; 6+ messages in thread From: Oliver Hartkopp @ 2020-10-12 7:41 UTC (permalink / raw) To: Jakub Kicinski; +Cc: netdev, mkl, davem, linux-can On 11.10.20 17:44, Jakub Kicinski wrote: > On Sun, 11 Oct 2020 11:24:07 +0200 Oliver Hartkopp wrote: >> diff --git a/net/can/isotp.c b/net/can/isotp.c >> index e6ff032b5426..22187669c5c9 100644 >> --- a/net/can/isotp.c >> +++ b/net/can/isotp.c >> @@ -79,6 +79,8 @@ MODULE_LICENSE("Dual BSD/GPL"); >> MODULE_AUTHOR("Oliver Hartkopp <socketcan@hartkopp.net>"); >> MODULE_ALIAS("can-proto-6"); >> >> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > You need to move this before the includes: > > net/can/isotp.c:82: warning: "pr_fmt" redefined > 82 | #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > | > In file included from ../include/linux/kernel.h:15, > from ../include/linux/list.h:9, > from ../include/linux/module.h:12, > from ../net/can/isotp.c:56: > include/linux/printk.h:297: note: this is the location of the previous definition > 297 | #define pr_fmt(fmt) fmt > | > net/can/isotp.c:82:9: warning: preprocessor token pr_fmt redefined > net/can/isotp.c: note: in included file (through ../include/linux/kernel.h, ../include/linux/list.h, ../include/linux/module.h): > include/linux/printk.h:297:9: this was the original definition > Hm - don't know why my build process didn't complain about it. Or why I possibly overlooked it. I'll do the cosmetic pr_fmt() improvements for the entire CAN network layer stuff later as it is not relevant for this net-next window. The v3 patch will fix the GFP_ATOMIC too. Thanks for your patience, Oliver ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next v2 1/2] can-isotp: implement cleanups / improvements from review 2020-10-11 9:24 [PATCH net-next v2 1/2] can-isotp: implement cleanups / improvements from review Oliver Hartkopp 2020-10-11 9:24 ` [PATCH net-next v2 2/2] can: remove obsolete version strings Oliver Hartkopp 2020-10-11 15:44 ` [PATCH net-next v2 1/2] can-isotp: implement cleanups / improvements from review Jakub Kicinski @ 2020-10-11 15:45 ` Jakub Kicinski 2020-10-11 17:59 ` kernel test robot 3 siblings, 0 replies; 6+ messages in thread From: Jakub Kicinski @ 2020-10-11 15:45 UTC (permalink / raw) To: Oliver Hartkopp; +Cc: netdev, mkl, davem, linux-can On Sun, 11 Oct 2020 11:24:07 +0200 Oliver Hartkopp wrote: > @@ -769,7 +771,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); hrtimer will need GFP_ATOMIC ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next v2 1/2] can-isotp: implement cleanups / improvements from review 2020-10-11 9:24 [PATCH net-next v2 1/2] can-isotp: implement cleanups / improvements from review Oliver Hartkopp ` (2 preceding siblings ...) 2020-10-11 15:45 ` Jakub Kicinski @ 2020-10-11 17:59 ` kernel test robot 3 siblings, 0 replies; 6+ messages in thread From: kernel test robot @ 2020-10-11 17:59 UTC (permalink / raw) To: Oliver Hartkopp, kuba, netdev Cc: kbuild-all, mkl, davem, linux-can, Oliver Hartkopp [-- Attachment #1: Type: text/plain, Size: 1926 bytes --] Hi Oliver, I love your patch! Perhaps something to improve: [auto build test WARNING on net-next/master] url: https://github.com/0day-ci/linux/commits/Oliver-Hartkopp/can-isotp-implement-cleanups-improvements-from-review/20201012-000055 base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git df41c19abbeaae6e24c8e31cff5fdb48632be380 config: xtensa-allyesconfig (attached as .config) compiler: xtensa-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/693c9b4e8c5710e7b536ec9d6a969a7d7343a100 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Oliver-Hartkopp/can-isotp-implement-cleanups-improvements-from-review/20201012-000055 git checkout 693c9b4e8c5710e7b536ec9d6a969a7d7343a100 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=xtensa If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): >> net/can/isotp.c:82: warning: "pr_fmt" redefined 82 | #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt | In file included from include/linux/kernel.h:15, from include/linux/list.h:9, from include/linux/module.h:12, from net/can/isotp.c:56: include/linux/printk.h:297: note: this is the location of the previous definition 297 | #define pr_fmt(fmt) fmt | vim +/pr_fmt +82 net/can/isotp.c 81 > 82 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt 83 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 65141 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-10-12 7:41 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-10-11 9:24 [PATCH net-next v2 1/2] can-isotp: implement cleanups / improvements from review Oliver Hartkopp 2020-10-11 9:24 ` [PATCH net-next v2 2/2] can: remove obsolete version strings Oliver Hartkopp 2020-10-11 15:44 ` [PATCH net-next v2 1/2] can-isotp: implement cleanups / improvements from review Jakub Kicinski 2020-10-12 7:41 ` Oliver Hartkopp 2020-10-11 15:45 ` Jakub Kicinski 2020-10-11 17:59 ` kernel test robot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).