netdev - Re: [PATCH bpf-next 2/5] net/smc: allow smc to negotiate protocols on policies

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a8555236-2bef-b0fb-d8a8-dde3058a2271@linux.alibaba.com>
Date:   Thu, 27 Apr 2023 11:30:42 +0800
From:   "D. Wythe" <alibuda@...ux.alibaba.com>
To:     Kui-Feng Lee <sinquersw@...il.com>, kgraul@...ux.ibm.com,
        wenjia@...ux.ibm.com, jaka@...ux.ibm.com, ast@...nel.org,
        daniel@...earbox.net, andrii@...nel.org, martin.lau@...ux.dev,
        pabeni@...hat.com, song@...nel.org, sdf@...gle.com,
        haoluo@...gle.com, yhs@...com, edumazet@...gle.com,
        john.fastabend@...il.com, kpsingh@...nel.org, jolsa@...nel.org,
        guwen@...ux.alibaba.com
Cc:     kuba@...nel.org, davem@...emloft.net, netdev@...r.kernel.org,
        linux-s390@...r.kernel.org, linux-rdma@...r.kernel.org,
        bpf@...r.kernel.org
Subject: Re: [PATCH bpf-next 2/5] net/smc: allow smc to negotiate protocols on
 policies


Hi Lee,


On 4/27/23 12:47 AM, Kui-Feng Lee wrote:
>
>
> On 4/26/23 02:24, D. Wythe wrote:
>> From: "D. Wythe" <alibuda@...ux.alibaba.com>
>> diff --git a/net/smc/bpf_smc.c b/net/smc/bpf_smc.c
>> new file mode 100644
>> index 0000000..0c0ec05
>> --- /dev/null
>> +++ b/net/smc/bpf_smc.c
>> @@ -0,0 +1,201 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
> ... cut ...

Will fix it, Thanks.

>> +
>> +/* register ops */
>> +int smc_sock_register_negotiator_ops(struct smc_sock_negotiator_ops 
>> *ops)
>> +{
>> +    int ret;
>> +
>> +    ret = smc_sock_validate_negotiator_ops(ops);
>> +    if (ret)
>> +        return ret;
>> +
>> +    /* calt key by name hash */
>> +    ops->key = jhash(ops->name, sizeof(ops->name), strlen(ops->name));
>> +
>> +    spin_lock(&smc_sock_negotiator_list_lock);
>> +    if (smc_negotiator_ops_get_by_key(ops->key)) {
>> +        pr_notice("smc: %s negotiator already registered\n", 
>> ops->name);
>> +        ret = -EEXIST;
>> +    } else {
>> +        list_add_tail_rcu(&ops->list, &smc_sock_negotiator_list);
>> +    }
>> +    spin_unlock(&smc_sock_negotiator_list_lock);
>> +    return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(smc_sock_register_negotiator_ops);
>
> This and following functions are not specific to BPF, right?
> I found you have more BPF specific code in this file in following
> patches.  But, I feel these function should not in this file since
> they are not BPF specific because file name "bpf_smc.c" hints.

Yes. Logically those functions are not suitable for being placed in 
"bpf_smc.c".
However, since SMC is compiled as modules by default, and currently
struct ops needs to be built in, or specific symbols will not be found 
during linking.

Of course, I can separate those this function in another new file, which 
can also be built in.
I may have to introduce a new KConfig likes SMC_NEGOTIATOR. But this 
feature is  only effective
when eBPF exists, so from the perspective of SMC, it would also be kind 
of weird.

But whatever, if you do think it's necessary, I can split it into two files.

Besh wishes.
D. Wythe