From: Stephen Smalley <firstname.lastname@example.org> To: Ondrej Mosnacek <email@example.com> Cc: SElinux list <firstname.lastname@example.org>, Paul Moore <email@example.com>, Michal Sekletar <firstname.lastname@example.org> Subject: Re: [PATCH v2] selinux: cache the SID -> context string translation Date: Thu, 7 Nov 2019 12:59:55 -0500 Message-ID: <email@example.com> (raw) In-Reply-To: <CAFqZXNvfovdHTFaPH3QA2dRyibtf8Qhd_3G3HKP2qyex5cfirstname.lastname@example.org> On 11/7/19 7:41 AM, Ondrej Mosnacek wrote: > On Wed, Nov 6, 2019 at 8:48 PM Stephen Smalley <email@example.com> wrote: >> On 11/6/19 2:22 PM, Stephen Smalley wrote: >>> On 11/6/19 11:11 AM, Stephen Smalley wrote: >>>> On 11/6/19 3:26 AM, Ondrej Mosnacek wrote: >>>>> Translating a context struct to string can be quite slow, especially if >>>>> the context has a lot of category bits set. This can cause quite >>>>> noticeable performance impact in situations where the translation needs >>>>> to be done repeatedly. A common example is a UNIX datagram socket with >>>>> the SO_PASSSEC option enabled, which is used e.g. by systemd-journald >>>>> when receiving log messages via datagram socket. This scenario can be >>>>> reproduced with: >>>>> >>>>> cat /dev/urandom | base64 | logger & >>>>> timeout 30s perf record -p $(pidof systemd-journald) -a -g >>>>> kill %1 >>>>> perf report -g none --pretty raw | grep security_secid_to_secctx >>>>> >>>>> Before the caching introduced by this patch, computing the context >>>>> string (security_secid_to_secctx() function) takes up ~65% of >>>>> systemd-journald's CPU time (assuming a context with 1024 categories >>>>> set and Fedora x86_64 release kernel configs). After this patch >>>>> (assuming near-perfect cache hit ratio) this overhead is reduced to just >>>>> ~2%. >>>>> >>>>> This patch addresses the issue by caching a certain number (compile-time >>>>> configurable) of recently used context strings to speed up repeated >>>>> translations of the same context, while using only a small amount of >>>>> memory. >>>>> >>>>> The cache is integrated into the existing sidtab table by adding a field >>>>> to each entry, which when not NULL contains an RCU-protected pointer to >>>>> a cache entry containing the cached string. The cache entries are kept >>>>> in a linked list sorted according to how recently they were used. On a >>>>> cache miss when the cache is full, the least recently used entry is >>>>> removed to make space for the new entry. >>>>> >>>>> The patch migrates security_sid_to_context_core() to use the cache (also >>>>> a few other functions where it was possible without too much fuss, but >>>>> these mostly use the translation for logging in case of error, which is >>>>> rare). >>>>> >>>>> Link: https://bugzilla.redhat.com/show_bug.cgi?id=1733259 >>>>> Cc: Michal Sekletar <firstname.lastname@example.org> >>>>> Signed-off-by: Ondrej Mosnacek <email@example.com> >>>> >>>> This looks good to me and didn't trigger any issues during testing. >>>> Might want to run it by the RCU wizards, e.g. Paul McKenney, to >>>> confirm that it is correct and optimal usage of RCU. I didn't see >>>> anywhere near the same degree of performance improvement in running >>>> your reproducer above but I also have all the kernel debugging options >>>> enabled so perhaps those are creating noise or perhaps the reproducer >>>> doesn't yield stable results. >>> >>> Rebuilding with the stock Fedora x86_64 kernel config was closer to your >>> results albeit still different, ~35% before and ~2% after. >> >> Ah, I can reproduce your ~65% result on the 5.3-based Fedora kernel, but >> not with mainline 5.4.0-rc1. Only SELinux change between those two that >> seems potentially relevant is your "selinux: avoid atomic_t usage in >> sidtab" patch. > > Hm... did you use the stock Fedora kernel RPM as the baseline? If so, > this could be because on stable Fedora releases the kernel package is > built with release config and kernel-debug with debug config, while on > Rawhide there is only one kernel package, which is built with debug > config. Under debug Fedora config the numbers are completely different > due to the overhead of various debug checks. I don't remember the > "after" value that I got when testing the patched Rawhide kernel with > the default debug config, but on stock Rawhide I got 43%, and on > Rawhide kernel rebuild with release config I got 65% again. I first built selinux/next (based on 5.4-rc1) before and after your patch using a config with many debug options enabled (including KASAN), which yielded wildly different percentages both before and after - don't remember the exact values (but still improved by your patch). Then I built the same sources using the stock Fedora 31 release kernel config and got the 35% versus 2% figures, likewise an improvement. Then I ran your reproducer just on the stock Fedora 31 release kernel, which happened to be 5.3-based, and got the 65% versus 2% figures that matched your results. So I'm not sure what accounts for the difference in the before results between the stock Fedora 31 release kernel and selinux/next using the same config. Could be a 5.3 vs 5.4-rc1 change, a change in selinux/next on top of 5.4-rc1, or something different in the build toolchain/environment used for the stock Fedora kernel versus my build host (which was running Fedora 31). Regardless, I do see a significant improvement in all cases from the patch, only the degree of improvement differs. So I'm fine with it. Might want to get a 2nd opinion from Paul McKenney all the same on the RCU bits.
prev parent reply index Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-11-06 8:26 Ondrej Mosnacek 2019-11-06 16:11 ` Stephen Smalley 2019-11-06 19:22 ` Stephen Smalley 2019-11-06 19:47 ` Stephen Smalley 2019-11-07 12:41 ` Ondrej Mosnacek 2019-11-07 17:59 ` Stephen Smalley [this message]
Reply instructions: You may reply publically to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
SELinux Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/selinux/0 selinux/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 selinux selinux/ https://lore.kernel.org/selinux \ firstname.lastname@example.org public-inbox-index selinux Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.kernel.vger.selinux AGPL code for this site: git clone https://public-inbox.org/public-inbox.git