| Message ID | 20210126145929.7404-1-cmi@nvidia.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [net-next] net: psample: Introduce stubs to remove NIC driver dependency | expand |
On Tue, 26 Jan 2021 22:59:29 +0800 Chris Mi wrote: > In order to send sampled packets to userspace, NIC driver calls > psample api directly. But it creates a hard dependency on module > psample. Introduce psample_ops to remove the hard dependency. > It is initialized when psample module is loaded and set to NULL > when the module is unloaded. > > Signed-off-by: Chris Mi <cmi@nvidia.com> > Reviewed-by: Jiri Pirko <jiri@nvidia.com> This adds a bunch of sparse warnings. MelVidia has some patch checking infra, right? Any reason this was not run through it?
Hi Jakub, On 1/27/2021 10:49 AM, Jakub Kicinski wrote: > On Tue, 26 Jan 2021 22:59:29 +0800 Chris Mi wrote: >> In order to send sampled packets to userspace, NIC driver calls >> psample api directly. But it creates a hard dependency on module >> psample. Introduce psample_ops to remove the hard dependency. >> It is initialized when psample module is loaded and set to NULL >> when the module is unloaded. >> >> Signed-off-by: Chris Mi <cmi@nvidia.com> >> Reviewed-by: Jiri Pirko <jiri@nvidia.com> > This adds a bunch of sparse warnings. > > MelVidia has some patch checking infra, right? Any reason this was not > run through it? Could you please tell me what's sparse warnings you hit? Just now I ran ./scripts/checkpatch.pl again without "--ignore FILE_PATH_CHANGES", I got the following warning: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating? #128: new file mode 100644 I'll change it. But I'm not sure if this is the only thing I need to change. So could you please elaborate? I'll pay attention to it in the future. Thanks, Chris
On Tue, 2021-01-26 at 18:49 -0800, Jakub Kicinski wrote: > On Tue, 26 Jan 2021 22:59:29 +0800 Chris Mi wrote: > > In order to send sampled packets to userspace, NIC driver calls > > psample api directly. But it creates a hard dependency on module > > psample. Introduce psample_ops to remove the hard dependency. > > It is initialized when psample module is loaded and set to NULL > > when the module is unloaded. > > > > Signed-off-by: Chris Mi <cmi@nvidia.com> > > Reviewed-by: Jiri Pirko <jiri@nvidia.com> > > This adds a bunch of sparse warnings. > > MelVidia has some patch checking infra, right? Any reason this was > not > run through it? we do but we don't test warnings on non mlnx files, as we rely on the fact that our mlnx files are clean, We simply don't check for "added/diff" warnings, we check that mlnx files are kept clean, easier :) .. Anyway I am working internally to clone/copy nipa into our CI.. hope it will work smoothly .. Thanks, Saeed.
Hi Chris,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Chris-Mi/net-psample-Introduce-stubs-to-remove-NIC-driver-dependency/20210127-082451
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 02c26940908fd31bb112e9742adedfb06eca19e1
config: s390-randconfig-s031-20210126 (attached as .config)
compiler: s390-linux-gcc (GCC) 9.3.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.3-212-g56dbccf5-dirty
# https://github.com/0day-ci/linux/commit/f2df98afc1a1f1809d9e8a178b2d4766cbca8bf7
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Chris-Mi/net-psample-Introduce-stubs-to-remove-NIC-driver-dependency/20210127-082451
git checkout f2df98afc1a1f1809d9e8a178b2d4766cbca8bf7
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=s390
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
"sparse warnings: (new ones prefixed by >>)"
>> net/psample/psample.c:468:17: sparse: sparse: incompatible types in comparison expression (different address spaces):
>> net/psample/psample.c:468:17: sparse: struct psample_ops const [noderef] __rcu *
>> net/psample/psample.c:468:17: sparse: struct psample_ops const *
net/psample/psample.c:474:9: sparse: sparse: incompatible types in comparison expression (different address spaces):
net/psample/psample.c:474:9: sparse: struct psample_ops const [noderef] __rcu *
net/psample/psample.c:474:9: sparse: struct psample_ops const *
vim +468 net/psample/psample.c
461
462 static int __init psample_module_init(void)
463 {
464 int ret;
465
466 ret = genl_register_family(&psample_nl_family);
467 if (!ret)
> 468 RCU_INIT_POINTER(psample_ops, &psample_sample_ops);
469 return ret;
470 }
471
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Tue, 26 Jan 2021 20:50:28 -0800 Saeed Mahameed wrote: > On Tue, 2021-01-26 at 18:49 -0800, Jakub Kicinski wrote: > > On Tue, 26 Jan 2021 22:59:29 +0800 Chris Mi wrote: > > > In order to send sampled packets to userspace, NIC driver calls > > > psample api directly. But it creates a hard dependency on module > > > psample. Introduce psample_ops to remove the hard dependency. > > > It is initialized when psample module is loaded and set to NULL > > > when the module is unloaded. > > > > > > Signed-off-by: Chris Mi <cmi@nvidia.com> > > > Reviewed-by: Jiri Pirko <jiri@nvidia.com> > > > > This adds a bunch of sparse warnings. > > > > MelVidia has some patch checking infra, right? Any reason this was > > not > > run through it? > > we do but we don't test warnings on non mlnx files, as we rely on the > fact that our mlnx files are clean, We simply don't check for > "added/diff" warnings, we check that mlnx files are kept clean, easier > :) .. > > Anyway I am working internally to clone/copy nipa into our CI.. hope it > will work smoothly .. Exciting! :)
On Wed, 2021-01-27 at 11:42 +0800, Chris Mi wrote:
> Could you please tell me what's sparse warnings you hit?
https://patchwork.kernel.org/project/netdevbpf/patch/20210127101648.513562-1-cmi@nvidia.com/
build allmodconfig and build32
On 1/28/2021 7:03 AM, Saeed Mahameed wrote: > On Wed, 2021-01-27 at 11:42 +0800, Chris Mi wrote: > >> Could you please tell me what's sparse warnings you hit? > https://patchwork.kernel.org/project/netdevbpf/patch/20210127101648.513562-1-cmi@nvidia.com/ > > build allmodconfig and build32 OK, I see. Thanks, Saeed.
diff --git a/include/net/psample.h b/include/net/psample.h index 68ae16bb0a4a..ff0e4484fc12 100644 --- a/include/net/psample.h +++ b/include/net/psample.h @@ -4,6 +4,7 @@ #include <uapi/linux/psample.h> #include <linux/list.h> +#include <linux/skbuff.h> struct psample_group { struct list_head list; @@ -14,6 +15,15 @@ struct psample_group { struct rcu_head rcu; }; +struct psample_ops { + void (*sample_packet)(struct psample_group *group, struct sk_buff *skb, + u32 trunc_size, int in_ifindex, int out_ifindex, + u32 sample_rate); + +}; + +extern const struct psample_ops *psample_ops; + struct psample_group *psample_group_get(struct net *net, u32 group_num); void psample_group_take(struct psample_group *group); void psample_group_put(struct psample_group *group); @@ -35,4 +45,21 @@ static inline void psample_sample_packet(struct psample_group *group, #endif +static inline void +psample_nic_sample_packet(struct psample_group *group, + struct sk_buff *skb, u32 trunc_size, + int in_ifindex, int out_ifindex, + u32 sample_rate) +{ + const struct psample_ops *ops; + + rcu_read_lock(); + ops = rcu_dereference(psample_ops); + if (ops) + psample_ops->sample_packet(group, skb, trunc_size, + in_ifindex, out_ifindex, + sample_rate); + rcu_read_unlock(); +} + #endif /* __NET_PSAMPLE_H */ diff --git a/net/psample/psample.c b/net/psample/psample.c index 33e238c965bd..d98fc4f000dc 100644 --- a/net/psample/psample.c +++ b/net/psample/psample.c @@ -8,6 +8,7 @@ #include <linux/kernel.h> #include <linux/skbuff.h> #include <linux/module.h> +#include <linux/rcupdate.h> #include <net/net_namespace.h> #include <net/sock.h> #include <net/netlink.h> @@ -35,6 +36,10 @@ static const struct genl_multicast_group psample_nl_mcgrps[] = { static struct genl_family psample_nl_family __ro_after_init; +static const struct psample_ops psample_sample_ops = { + .sample_packet = psample_sample_packet, +}; + static int psample_group_nl_fill(struct sk_buff *msg, struct psample_group *group, enum psample_command cmd, u32 portid, u32 seq, @@ -456,11 +461,17 @@ EXPORT_SYMBOL_GPL(psample_sample_packet); static int __init psample_module_init(void) { - return genl_register_family(&psample_nl_family); + int ret; + + ret = genl_register_family(&psample_nl_family); + if (!ret) + RCU_INIT_POINTER(psample_ops, &psample_sample_ops); + return ret; } static void __exit psample_module_exit(void) { + RCU_INIT_POINTER(psample_ops, NULL); genl_unregister_family(&psample_nl_family); } diff --git a/net/sched/Makefile b/net/sched/Makefile index dd14ef413fda..0d92bb98bb26 100644 --- a/net/sched/Makefile +++ b/net/sched/Makefile @@ -3,7 +3,7 @@ # Makefile for the Linux Traffic Control Unit. # -obj-y := sch_generic.o sch_mq.o +obj-y := sch_generic.o sch_mq.o psample_stub.o obj-$(CONFIG_INET) += sch_frag.o obj-$(CONFIG_NET_SCHED) += sch_api.o sch_blackhole.o diff --git a/net/sched/psample_stub.c b/net/sched/psample_stub.c new file mode 100644 index 000000000000..0451d1b3bd7d --- /dev/null +++ b/net/sched/psample_stub.c @@ -0,0 +1,7 @@ +// SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB +/* Copyright (c) 2021 Mellanox Technologies. */ + +#include <net/psample.h> + +const struct psample_ops __rcu *psample_ops __read_mostly; +EXPORT_SYMBOL_GPL(psample_ops);