Re: [PATCH net-next 1/3] net: prestera: acl: migrate to new vTCAM api

Hi Jakub,

	Thanks for reviewing the changes.

On Tue, 23 Nov 2021 18:58:00 +0200 Volodymyr Mytnyk wrote:

quoted

From: Volodymyr Mytnyk <redacted>

- Add new vTCAM HW API to configure HW ACLs.
- Migrate acl to use new vTCAM HW API.
- No counter support in this patch-set.

Co-developed-by: Yevhen Orlov <redacted>
Signed-off-by: Yevhen Orlov <redacted>
Signed-off-by: Volodymyr Mytnyk <redacted>

quoted

 struct prestera_acl_ruleset {
+     struct rhash_head ht_node; /* Member of acl HT */
+     struct prestera_acl_ruleset_ht_key ht_key;
       struct rhashtable rule_ht;
-     struct prestera_switch *sw;
-     u16 id;
+     struct prestera_acl *acl;
+     unsigned long rule_count;
+     refcount_t refcount;
+     void *keymask;
+     bool offload;
+     u32 vtcam_id;
+     u16 pcl_id;

put the pcl_id earlier for better packing?

Fixed in v2, checked in all places, uploaded the changes today.

quoted

 };

quoted

+struct prestera_acl_vtcam {
+     struct list_head list;
+     __be32 keymask[__PRESTERA_ACL_RULE_MATCH_TYPE_MAX];
+     bool is_keymask_set;
+     refcount_t refcount;
+     u8 lookup;

same here, 1B types together

Fixed

quoted

       u32 id;
 };

quoted

+int prestera_acl_ruleset_keymask_set(struct prestera_acl_ruleset *ruleset,
+                                  void *keymask)
 {
-     prestera_hw_acl_ruleset_del(ruleset->sw, ruleset->id);
-     rhashtable_destroy(&ruleset->rule_ht);
-     kfree(ruleset);
+     void *__keymask;
+
+     if (!keymask || !ruleset)

Can this legitimately happen? No defensive programming, please.

This function is unused here, so just removed from this patch.

quoted

+             return -EINVAL;
+
+     __keymask = kmalloc(ACL_KEYMASK_SIZE, GFP_KERNEL);
+     if (!__keymask)
+             return -ENOMEM;
+
+     memcpy(__keymask, keymask, ACL_KEYMASK_SIZE);

kmemdup()

quoted

+     ruleset->keymask = __keymask;
+
+     return 0;
 }

Regards,
	Volodymyr