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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 4/26/23 20:30, D. Wythe wrote:

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@xxxxxxxxxxxxxxxxx>
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.
On the other hand, this feature is only effective when SMC exists.
Even without BPF, you still can implement a negotiator in a module.
Since you have exported these symbols, I suspect that you expect
negotiators in modules or builtin, right?  If I am wrong about exports,
perhaps you should stop exporting since they are used locally only.


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

Besh wishes.
D. Wythe






[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux