[PATCH net-next] net: psample: Introduce stubs to remove NIC driver dependency

[PATCH net-next] net: psample: Introduce stubs to remove NIC driver dependency

[Chris Mi]

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>
---
 include/net/psample.h    | 27 +++++++++++++++++++++++++++
 net/psample/psample.c    | 13 ++++++++++++-
 net/sched/Makefile       |  2 +-
 net/sched/psample_stub.c |  7 +++++++
 4 files changed, 47 insertions(+), 2 deletions(-)
 create mode 100644 net/sched/psample_stub.c

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);
-- 
2.26.2

Re: [PATCH net-next] net: psample: Introduce stubs to remove NIC driver dependency

[Jakub Kicinski]

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?

Re: [PATCH net-next] net: psample: Introduce stubs to remove NIC driver dependency

[Chris Mi]

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

Re: [PATCH net-next] net: psample: Introduce stubs to remove NIC driver dependency

[Saeed Mahameed]

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.

Re: [PATCH net-next] net: psample: Introduce stubs to remove NIC driver dependency

[kernel test robot]

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

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 19518 bytes --]

Re: [PATCH net-next] net: psample: Introduce stubs to remove NIC driver dependency

[Jakub Kicinski]

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! :)

Re: [PATCH net-next] net: psample: Introduce stubs to remove NIC driver dependency

[Saeed Mahameed]

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

Re: [PATCH net-next] net: psample: Introduce stubs to remove NIC driver dependency

[Chris Mi]

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.