| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <c62ee575-49e0-c5d6-f855-ead5775af141@nvidia.com>
Date: Mon, 1 Feb 2021 10:00:48 +0800
From: Chris Mi <cmi@...dia.com>
To: Ido Schimmel <idosch@...sch.org>
CC: <netdev@...r.kernel.org>, <kuba@...nel.org>, <jiri@...dia.com>,
<saeedm@...dia.com>, kernel test robot <lkp@...el.com>
Subject: Re: [PATCH net-next v5] net: psample: Introduce stubs to remove NIC
driver dependency
On 1/30/2021 10:52 PM, Ido Schimmel wrote:
> On Sat, Jan 30, 2021 at 10:33:19AM +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.
>>
>> Reported-by: kernel test robot <lkp@...el.com>
> This belongs in the changelog (that should be part of the commit
> message). Something like "Fix xxx reported by kernel test robot".
But I see existing commits have it. And Jakub didn't ask me to do it. So
I think
I'd better keep it.
>
>> Signed-off-by: Chris Mi <cmi@...dia.com>
>> Reviewed-by: Jiri Pirko <jiri@...dia.com>
>> ---
>> v1->v2:
>> - fix sparse errors
>> v2->v3:
>> - remove inline
>> v3->v4:
>> - add inline back
>> v4->v5:
>> - address Jakub's comments
>>
>> include/net/psample.h | 26 ++++++++++++++++++++++++++
>> net/psample/psample.c | 14 +++++++++++++-
>> net/sched/Makefile | 2 +-
>> net/sched/psample_stub.c | 5 +++++
>> 4 files changed, 45 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..d0f1cfc56f6f 100644
>> --- a/include/net/psample.h
>> +++ b/include/net/psample.h
>> @@ -14,6 +14,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 __rcu *psample_ops __read_mostly;
>> +
>> 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 +44,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)
>> + 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..983ca5b698fe 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,18 @@ 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_assign_pointer(psample_ops, NULL);
>> + synchronize_rcu();
>> 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
> Why the stub is under net/sched/ when psample is at net/psample/?
> psample is not the same as 'act_sample'.
If this stub is in net/psample and compiled to psample kernel module, NIC
driver still depends on it. It must be in kernel. And I think net/sched
is the
only place we can put. If we put it in somewhere else, like net/, I
guess we'll
get more objections.
>
>>
>> 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..0541b8c5100d
>> --- /dev/null
>> +++ b/net/sched/psample_stub.c
>> @@ -0,0 +1,5 @@
>> +// SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB
> psample has "// SPDX-License-Identifier: GPL-2.0-only"
Done. Thanks.
>
>> +/* Copyright (c) 2021 Mellanox Technologies. */
>> +
>> +const struct psample_ops __rcu *psample_ops __read_mostly;
>> +EXPORT_SYMBOL_GPL(psample_ops);
>> --
>> 2.26.2
>>
Powered by blists - more mailing lists