Thread (27 messages) 27 messages, 2 authors, 2026-07-31

Re: [PATCH v22 net-next 04/12] net/nebula-matrix: add channel layer

flat view

From: Jakub Kicinski <kuba@kernel.org>
Date: 2026-07-31 01:27:20
Also in: linux-doc, lkml

On Thu, 23 Jul 2026 12:00:56 +0800 illusion.wang wrote:
quoted hunk ↗ jump to hunk
+#define FNV_PRIME_32 0x01000193
+#define FNV_OFFSET_32 0x811C9DC5
+static u32 nbl_common_calc_hash_key(void *key, u32 key_size, u32 bucket_size)
+{
+	u32 hash = FNV_OFFSET_32;
+	u8 *p = (u8 *)key;
+	u32 i;
+
+	if (bucket_size == 0 || bucket_size == NBL_HASH_TBL_LIST_BUCKET_SIZE)
+		return 0;
+
+	for (i = 0; i < key_size; i++) {
+		hash ^= p[i];
+		hash *= FNV_PRIME_32;
+	}
+	/* Use bitmask if bucket_size is a power of 2 */
+	if ((bucket_size & (bucket_size - 1)) == 0)
+		return hash & (bucket_size - 1);
+	else
+		return hash % bucket_size;
+}
Why are you implementing your own hashing function and your own hash
table? Can't one of existing implementations in the kernel be used?
quoted hunk ↗ jump to hunk
+int nbl_common_alloc_hash_node(struct nbl_hash_tbl_mgt *tbl_mgt, void *key,
+			       void *data, void **out_data)
+{
+	struct nbl_hash_entry_node *hash_node;
+	u16 data_size;
+	u32 hash_val;
+	u16 key_size;
+
+	hash_node = devm_kzalloc(tbl_mgt->tbl_key.dev, sizeof(*hash_node),
+				 GFP_KERNEL);
Don't use devm_ for inherently dynamically allocated memory...
quoted hunk ↗ jump to hunk
+static void nbl_common_detach_hash_node(struct nbl_hash_tbl_mgt *tbl_mgt,
+					struct nbl_hash_entry_node *hash_node)
+{
+	hlist_del(&hash_node->node);
+	devm_kfree(tbl_mgt->tbl_key.dev, hash_node->key);
+	devm_kfree(tbl_mgt->tbl_key.dev, hash_node->data);
+	devm_kfree(tbl_mgt->tbl_key.dev, hash_node);
... if you ever have to explicitly free something, chances are you
shouldn't be using devm_ in the first place.
quoted hunk ↗ jump to hunk
index 7ae331959ca1..d6b7bfff3cc6 100644
--- a/drivers/net/ethernet/nebula-matrix/nbl/nbl_core.h
+++ b/drivers/net/ethernet/nebula-matrix/nbl/nbl_core.h
@@ -11,23 +11,34 @@
 #include "nbl_include/nbl_def_common.h"
 
 struct nbl_hw_mgt;
+struct nbl_hw_ops_tbl;
+struct nbl_channel_ops_tbl;
+struct nbl_channel_mgt;
 
Lots of unnecessary forward declarations.
Using type as a member implicitly forward declares.
quoted hunk ↗ jump to hunk
 struct nbl_adapter *nbl_core_init(struct pci_dev *pdev,
 				  struct nbl_init_param *param);
 void nbl_core_remove(struct nbl_adapter *adapter);
+
 #endif
This chunk should be squashed into an earlier patch
quoted hunk ↗ jump to hunk
+#define NBL_CHAN_SEND(chan_send, dst_id, mesg_type, argument, arg_length,\
+		      response, resp_length, need_ack)			\
+do {									\
+	typeof(chan_send)	*__chan_send = &(chan_send);		\
+	__chan_send->dstid	= (dst_id);				\
+	__chan_send->msg_type	= (mesg_type);				\
+	__chan_send->arg	= (argument);				\
+	__chan_send->arg_len	= (arg_length);				\
+	__chan_send->resp	= (response);				\
+	__chan_send->resp_len	= (resp_length);			\
+	__chan_send->ack	= (need_ack);				\
+} while (0)
+
+#define NBL_CHAN_ACK(chan_ack, dst_id, mesg_type, msg_id, err_code, ack_data, \
+		     data_length)					\
+do {									\
+	typeof(chan_ack)	*__chan_ack = &(chan_ack);		\
+	__chan_ack->dstid	= (dst_id);				\
+	__chan_ack->msg_type	= (mesg_type);				\
+	__chan_ack->msgid	= (msg_id);				\
+	__chan_ack->err		= (err_code);				\
+	__chan_ack->data	= (ack_data);				\
+	__chan_ack->data_len	= (data_length);			\
+} while (0)
+
Why are these macros and not C code?
You only seem to pass struct nbl_chan_ack_info into NBL_CHAN_ACK()
+struct nbl_hash_tbl_mgt;
This forward declaration here makes no sense.
Please look at your patches before you post them.
quoted hunk ↗ jump to hunk
 struct nbl_common_info {
 	struct pci_dev *pdev;
 	struct device *dev;
@@ -29,4 +31,19 @@ struct nbl_common_info {
 	u8 has_net;
 };
 
+struct nbl_hash_tbl_key {
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help