On Tue, Feb 18, 2020 at 6:27 AM Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote: > > In these rules, each rule with the same (target type, target class, > filename) values is (in practice) always mapped to the same result type. > Therefore, it is much more efficient to group the rules by (ttype, > tclass, filename). > > Thus, this patch drops the stype field from the key and changes the > datum to be a linked list of one or more structures that contain a > result type and an ebitmap of source types that map the given target to > the given result type under the given filename. The size of the hash > table is also incremented to 2048 to be more optimal for Fedora policy > (which currently has ~2500 unique (ttype, tclass, filename) tuples, > regardless of whether the 'unconfined' module is enabled). > > Not only does this dramtically reduce memory usage when the policy > contains a lot of unconfined domains (ergo a lot of filename based > transitions), but it also slightly reduces memory usage of strongly > confined policies (modeled on Fedora policy with 'unconfined' module > disabled) and significantly reduces lookup times of these rules on > Fedora (roughly matches the performance of the rhashtable conversion > patch [1] posted recently to selinux@xxxxxxxxxxxxxxx). > > An obvious next step is to change binary policy format to match this > layout, so that disk space is also saved. However, since that requires > more work (including matching userspace changes) and this patch is > already beneficial on its own, I'm posting it separately. > > Performance/memory usage comparison: > > Kernel | Policy load | Policy load | Mem usage | Mem usage | openbench > | | (-unconfined) | | (-unconfined) | (createfiles) > -----------------|-------------|---------------|-----------|---------------|-------------- > reference | 1,30s | 0,91s | 90MB | 77MB | 55 us/file > rhashtable patch | 0.98s | 0,85s | 85MB | 75MB | 38 us/file > this patch | 0,95s | 0,87s | 75MB | 75MB | 40 us/file > > (Memory usage is measured after boot. With SELinux disabled the memory > usage was ~60MB on the same system.) > > [1] https://lore.kernel.org/selinux/20200116213937.77795-1-dev@xxxxxxxxxx/T/ > > Signed-off-by: Ondrej Mosnacek <omosnace@xxxxxxxxxx> > --- > > Changes in v3: > - filename_trans_read_one() > - remove the 'newdatum' variable > - uintptr_t -> u32 (leftover from a WIP version of this patch) > - drop 'already_there', just goto out immediately > - allocate 'ft' using kmemdup() instead of kzalloc() > - filename_write_helper() > - move strlen() call outside the loop > - move declaration of 'node' outside the loop > > security/selinux/ss/policydb.c | 166 +++++++++++++++++++-------------- > security/selinux/ss/policydb.h | 8 +- > security/selinux/ss/services.c | 16 ++-- > 3 files changed, 110 insertions(+), 80 deletions(-) Merged into selinux/next, thanks Ondrej. -- paul moore www.paul-moore.com