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

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]

Date:   Tue, 30 Nov 2021 12:26:07 +0000
From:   "Volodymyr Mytnyk [C]" <vmytnyk@...vell.com>
To:     Jakub Kicinski <kuba@...nel.org>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Taras Chornyi <taras.chornyi@...ision.eu>,
        Mickey Rachamim <mickeyr@...vell.com>,
        Serhiy Pshyk <serhiy.pshyk@...ision.eu>,
        "Taras Chornyi [C]" <tchornyi@...vell.com>,
        "David S. Miller" <davem@...emloft.net>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Yevhen Orlov <yevhen.orlov@...ision.eu>
Subject: 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:
> > From: Volodymyr Mytnyk <vmytnyk@...vell.com>
> > 
> > - 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 <yevhen.orlov@...ision.eu>
> > Signed-off-by: Yevhen Orlov <yevhen.orlov@...ision.eu>
> > Signed-off-by: Volodymyr Mytnyk <vmytnyk@...vell.com>
> 
> >  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.

> 
> >  };
> 
> > +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

> 
> >        u32 id;
> >  };
> 
> > +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.

> 
> > +             return -EINVAL;
> > +
> > +     __keymask = kmalloc(ACL_KEYMASK_SIZE, GFP_KERNEL);
> > +     if (!__keymask)
> > +             return -ENOMEM;
> > +
> > +     memcpy(__keymask, keymask, ACL_KEYMASK_SIZE);
> 
> kmemdup()
> 
> > +     ruleset->keymask = __keymask;
> > +
> > +     return 0;
> >  }

Regards,
	Volodymyr