| Message ID | 20200408182416.30995-9-siarhei.liakh@concurrent-rt.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/9] SELinux: Introduce "Advanced Hashing" Kconfig option | expand |
Hi, On Wed, Apr 8, 2020 at 8:24 PM <siarhei.liakh@concurrent-rt.com> wrote: > > From: Siarhei Liakh <siarhei.liakh@concurrent-rt.com> > > This patch replaces local copy of custom hash function with existing > implementation of lookup3 from the standard Linux library. This change > allows to reduce the amount of custom code with has to be maintained, while > potentially improving overall performance of the hash table in question. > > Signed-off-by: Siarhei Liakh <siarhei.liakh@concurrent-rt.com> > --- > Please CC me directly in all replies. > > security/selinux/ss/symtab.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/security/selinux/ss/symtab.c b/security/selinux/ss/symtab.c > index dc2ce94165d3..8d189d7683d1 100644 > --- a/security/selinux/ss/symtab.c > +++ b/security/selinux/ss/symtab.c > @@ -9,6 +9,16 @@ > #include <linux/errno.h> > #include "symtab.h" > > +#ifdef CONFIG_SECURITY_SELINUX_ADVANCED_HASHING > +#include <linux/jhash.h> > + > +static unsigned int symhash(struct hashtab *h, const void *key) > +{ > + return jhash(key, strlen((const char *) key), 0) & (h->size - 1); Did you consider using full_name_hash() here instead? It is used in other places (mainly filesystem code) to hash strings. I wonder how it compares to jhash both in terms of speed and in terms of randomness of the resulting hash. It would be nice if you could do some benchmarks and provide some numbers to support the choice. > +} > + > +#else /* #ifdef CONFIG_SECURITY_SELINUX_ADVANCED_HASHING */ > + > static unsigned int symhash(struct hashtab *h, const void *key) > { > const char *p, *keyp; > @@ -23,6 +33,8 @@ static unsigned int symhash(struct hashtab *h, const void *key) > return val & (h->size - 1); > } > > +#endif /* #else #ifdef CONFIG_SECURITY_SELINUX_ADVANCED_HASHING */ > + > static int symcmp(struct hashtab *h, const void *key1, const void *key2) > { > const char *keyp1, *keyp2; > -- > 2.17.1 > -- Ondrej Mosnacek <omosnace at redhat dot com> Software Engineer, Security Technologies Red Hat, Inc.
The 04/14/2020 13:06, Ondrej Mosnacek wrote: > Hi, > > On Wed, Apr 8, 2020 at 8:24 PM <siarhei.liakh@concurrent-rt.com> wrote: > > > > From: Siarhei Liakh <siarhei.liakh@concurrent-rt.com> > > > > This patch replaces local copy of custom hash function with existing > > implementation of lookup3 from the standard Linux library. This change > > allows to reduce the amount of custom code with has to be maintained, while > > potentially improving overall performance of the hash table in question. > > > > Signed-off-by: Siarhei Liakh <siarhei.liakh@concurrent-rt.com> > > --- > > Please CC me directly in all replies. > > > > security/selinux/ss/symtab.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/security/selinux/ss/symtab.c b/security/selinux/ss/symtab.c > > index dc2ce94165d3..8d189d7683d1 100644 > > --- a/security/selinux/ss/symtab.c > > +++ b/security/selinux/ss/symtab.c > > @@ -9,6 +9,16 @@ > > #include <linux/errno.h> > > #include "symtab.h" > > > > +#ifdef CONFIG_SECURITY_SELINUX_ADVANCED_HASHING > > +#include <linux/jhash.h> > > + > > +static unsigned int symhash(struct hashtab *h, const void *key) > > +{ > > + return jhash(key, strlen((const char *) key), 0) & (h->size - 1); > > Did you consider using full_name_hash() here instead? It is used in > other places (mainly filesystem code) to hash strings. I wonder how it > compares to jhash both in terms of speed and in terms of randomness of > the resulting hash. It would be nice if you could do some benchmarks > and provide some numbers to support the choice. No, I have not considered other hashes, as my goal is to simply eliminate local copies of custom hashes in favor of existing ones from the Linux library. So, any other standard has will do, as far as I am concerned... However, this whole conversation brings up an interesting point: looks like we need an official hashing guide with benchmarks and references to official sources/research behind each hash function available. Sounds like an excellent project for some student somewhere! :-) I'll see what I can quickly put together within the scope of this discussion, though. Thank you!
diff --git a/security/selinux/ss/symtab.c b/security/selinux/ss/symtab.c index dc2ce94165d3..8d189d7683d1 100644 --- a/security/selinux/ss/symtab.c +++ b/security/selinux/ss/symtab.c @@ -9,6 +9,16 @@ #include <linux/errno.h> #include "symtab.h" +#ifdef CONFIG_SECURITY_SELINUX_ADVANCED_HASHING +#include <linux/jhash.h> + +static unsigned int symhash(struct hashtab *h, const void *key) +{ + return jhash(key, strlen((const char *) key), 0) & (h->size - 1); +} + +#else /* #ifdef CONFIG_SECURITY_SELINUX_ADVANCED_HASHING */ + static unsigned int symhash(struct hashtab *h, const void *key) { const char *p, *keyp; @@ -23,6 +33,8 @@ static unsigned int symhash(struct hashtab *h, const void *key) return val & (h->size - 1); } +#endif /* #else #ifdef CONFIG_SECURITY_SELINUX_ADVANCED_HASHING */ + static int symcmp(struct hashtab *h, const void *key1, const void *key2) { const char *keyp1, *keyp2;