Rule loading speed optimizations

[rafal-krypa]

When a project has large Smack policy and cares about time needed to boot the system, the Smack policy loading time becomes interesting. It's the only functionality of this project that could use speed optimizations.
In Tizen 2.x, with policy of 20K rules and 600 different labels, on the ARM development targets, smackload needs 570 ms to load the whole policy. It's also important, that the policy is read from dozens of files.
Current implementation of policy loading isn't very efficient. Every file is read line by line, each line is parsed and stored on the list (parsing has benefit of format error detection). Then each rule is converted back to textual form and sent to kernel, via either load2 or change-rule interface, one by one.
This can be improved by:

• Using multiple rules per write, as enabled by Smack since kernel 3.12
• Improving implementation in user space
• Further optimizations of kernel interface, if needed (probably not)

[rafal-krypa]

Few months ago I started experimenting with such optimizations. An alternative tool was written aiming for possibly shortest run time at some costs (i.e. code complexity, memory overhead). It was also the program I used for testing multi-rule kernel interface. Summing all gains, it improved rule loading speed by 40%.
I have uploaded the tool to Github repo rafal-krypa/smackload-fast for discussion. You can find a README file there describing what's going on inside.
I don't believe that this program is a candidate for inclusion here. But I'd like to discuss it and take some parts and concepts from it to optimizing libsmack.
I may have gone too far with some optimizations, do some things optimally and so on. This surely isn't a perfect thing, but a prototype that works quite well. Please share your thoughts.

[jarkkojs]

I have some other things to do right now but I don't think this collides with smack_accesses_apply_from_file. This would optimize existing routines.

To be more clear if doing no parsing and uploading rules as a chunk, I will put that under --no-modify parameter for smackload and smackctl. The more generic version (where this performance optimization would go into) would be used by default.

I would merge this first and foremost before even considering smack_accesses_apply_from_file. As I said in #37, you guys should finish #14 by using uthash as you suggested.

[jarkkojs]

This issue does not have a clear end condition and collides heavily with #14, #44 and #37. I would prefer if issues were much more concrete. This more like a discussion than issue.

That said I understand why you created this. We don't have a discussion forum for purely libsmack so things that really don't fit as issues (such as this) end up issues anyway.

I can propose something: please and your experiment as a comment to #37 (with short explanation). I will create a mailing list for purely libsmack discussions. After these things are done this issue can be closed.

[jarkkojs]

I'll promise to investigate your program as soon as I have time. I think this is in all cases good investigation work to do! Don't get me wrong here. I got some ideas from your README file. I'm going to push one patch myself for you to look at and I have an idea for another patch that you could work with Jan. More to be followed. Let's keep this stuff here for now and try to close all relevant issues when we have sufficient performance, OK?

In the meanwhile I will try to figure out decent mailing list platform to link to this project. BTW, one thing that could help us to be better connected would be an IRC @ freenode? Does that make sense to you?

[jarkkojs]

You could do something like this to probe multi-rule support (#40):

static int has_accesses_apply_multi = 1;

int smack_accesses_apply(struct smack_accesses *handle)
{
    int ret;

    if (has_accesses_apply_multi) {
        ret = accesses_apply_multi(handle, 0);
        if (!ret)
            return 0;
    }

    return accesses_apply(handle, 0); 
}

Then inside accesses_apply_multi() you can set has_accesses_apply_multi to zero if writing multiple rules does not work properly.

You have a separate internal function for applying multiple rules (accesses_apply_multi() or whatever). You first try that function. If it reports an error you use accesses_apply. You should also have a global static variable to remember the choice (initially has_multi_rule=1 (again, just a random name to serve as an example).

[rafal-krypa]

I'm afraid it's not that simple. Old kernel Smack implementation parsing data in load2 was buggy. When fed with input containing multiple rules separated by LF, it will kmalloc() buffer for the entire data, parse the first rule, report and claim that it processed entire input. Libsmack will see that a call to write(load_fd, buf, cnt) returned cnt, but kernel will apply only the first rule.

I have a working idea for detecting this, but it's quite hacky. Based on details in Smack kernel implementation, multi-line support can be determined by the following function:

static int test_multiline(int load_fd)
{
    if (write(load_fd, "", 0) == 0)
        return 1; /* Multiline capable kernels can also handle 0-byte write */
    else
        return 0; /* Legacy kernels return an error */
}

Another possibility would be to write a longer, valid input and check if it was applied correctly. But this would require putting some junk in the policy.

Such function could be called during library initialization as a part of run-time Smack functionality detection (#40).

[jarkkojs]

Hey, I'm happy with any detection that works but one comment. We don't have to circulate every kernel regression unless it is trivial to circulate. Those need to be fixed in the kernel and backported to distribution kernels. Your detection is only mandatory to work in the case when given kernel has necessary regression fixes.

[jarkkojs]

How is this going?

[rafal-krypa]

This (writing multiple rules to kernel at once) is currently blocked by #39 and #40. Rule merging is needed to reliably split change rules and set rules. Kernel Smack feature detection is needed for checking multi-rule support in kernel. I could do it simply inside smack_accesses_apply(), but it will be better in libsmack init code.
I am working on moving that two issues forward, but haven't published my work yet.

[jarkkojs]

OK. For the merging wouldn't it make sense to just revise patch @jobol for #88 to use integer identifiers instead of strings for the rule tree? See my suggestion in that issue. I think you should somehow sync your work and I really should push to get mailing list ramped up for discussions like this..

[jarkkojs]

@rafal-krypa: we can close this now, right?

[rafal-krypa]

I have two more commits that belong under this issue. This strictly depends on #39, so I had to wait.
First commit adds support for multi-rule write to load2 and change-rule, if such feature is supported by kernel. Second one is an optimization of accesses_print(). Together they give ~11% performance gain (7% + 4%). It's smaller that I expected, but still worth it.

I anticipate that you won't like memory allocation in smack_accesses_save() and accesses_apply(). But please check if the general approach is fine for you, then I can try to adjust the patches.