2018-09-07 04:35:37

by Tony Jones

[permalink] [raw]
Subject: [PATCH] apparmor: Fix network performance issue in aa_label_sk_perm

The netperf benchmark shows a 5.73% reduction in throughput for
small (64 byte) transfers by unconfined tasks.

DEFINE_AUDIT_SK() in aa_label_sk_perm() should not be performed
unconditionally, rather only when the label is confined.

netperf-tcp
56974a6fc^ 56974a6fc
Min 64 563.48 ( 0.00%) 531.17 ( -5.73%)
Min 128 1056.92 ( 0.00%) 999.44 ( -5.44%)
Min 256 1945.95 ( 0.00%) 1867.97 ( -4.01%)
Min 1024 6761.40 ( 0.00%) 6364.23 ( -5.87%)
Min 2048 11110.53 ( 0.00%) 10606.20 ( -4.54%)
Min 3312 13692.67 ( 0.00%) 13158.41 ( -3.90%)
Min 4096 14926.29 ( 0.00%) 14457.46 ( -3.14%)
Min 8192 18399.34 ( 0.00%) 18091.65 ( -1.67%)
Min 16384 21384.13 ( 0.00%) 21158.05 ( -1.06%)
Hmean 64 564.96 ( 0.00%) 534.38 ( -5.41%)
Hmean 128 1064.42 ( 0.00%) 1010.12 ( -5.10%)
Hmean 256 1965.85 ( 0.00%) 1879.16 ( -4.41%)
Hmean 1024 6839.77 ( 0.00%) 6478.70 ( -5.28%)
Hmean 2048 11154.80 ( 0.00%) 10671.13 ( -4.34%)
Hmean 3312 13838.12 ( 0.00%) 13249.01 ( -4.26%)
Hmean 4096 15009.99 ( 0.00%) 14561.36 ( -2.99%)
Hmean 8192 18975.57 ( 0.00%) 18326.54 ( -3.42%)
Hmean 16384 21440.44 ( 0.00%) 21324.59 ( -0.54%)
Stddev 64 1.24 ( 0.00%) 2.85 (-130.64%)
Stddev 128 4.51 ( 0.00%) 6.53 ( -44.84%)
Stddev 256 11.67 ( 0.00%) 8.50 ( 27.16%)
Stddev 1024 48.33 ( 0.00%) 75.07 ( -55.34%)
Stddev 2048 54.82 ( 0.00%) 65.16 ( -18.86%)
Stddev 3312 153.57 ( 0.00%) 56.29 ( 63.35%)
Stddev 4096 100.25 ( 0.00%) 88.50 ( 11.72%)
Stddev 8192 358.13 ( 0.00%) 169.99 ( 52.54%)
Stddev 16384 43.99 ( 0.00%) 141.82 (-222.39%)

Signed-off-by: Tony Jones <[email protected]>
Fixes: 56974a6fcfef ("apparmor: add base infastructure for socket
mediation")
---
security/apparmor/net.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/security/apparmor/net.c b/security/apparmor/net.c
index bb24cfa0a164..d5d72dd1ca1f 100644
--- a/security/apparmor/net.c
+++ b/security/apparmor/net.c
@@ -146,17 +146,20 @@ int aa_af_perm(struct aa_label *label, const char *op, u32 request, u16 family,
static int aa_label_sk_perm(struct aa_label *label, const char *op, u32 request,
struct sock *sk)
{
- struct aa_profile *profile;
- DEFINE_AUDIT_SK(sa, op, sk);
+ int error = 0;

AA_BUG(!label);
AA_BUG(!sk);

- if (unconfined(label))
- return 0;
+ if (!unconfined(label)) {
+ struct aa_profile *profile;
+ DEFINE_AUDIT_SK(sa, op, sk);

- return fn_for_each_confined(label, profile,
- aa_profile_af_sk_perm(profile, &sa, request, sk));
+ error = fn_for_each_confined(label, profile,
+ aa_profile_af_sk_perm(profile, &sa, request, sk));
+ }
+
+ return error;
}

int aa_sk_perm(const char *op, u32 request, struct sock *sk)
--
2.18.0



2018-09-07 16:39:26

by John Johansen

[permalink] [raw]
Subject: Re: [PATCH] apparmor: Fix network performance issue in aa_label_sk_perm

On 09/06/2018 09:33 PM, Tony Jones wrote:
> The netperf benchmark shows a 5.73% reduction in throughput for
> small (64 byte) transfers by unconfined tasks.
>
> DEFINE_AUDIT_SK() in aa_label_sk_perm() should not be performed
> unconditionally, rather only when the label is confined.
>
> netperf-tcp
> 56974a6fc^ 56974a6fc
> Min 64 563.48 ( 0.00%) 531.17 ( -5.73%)
> Min 128 1056.92 ( 0.00%) 999.44 ( -5.44%)
> Min 256 1945.95 ( 0.00%) 1867.97 ( -4.01%)
> Min 1024 6761.40 ( 0.00%) 6364.23 ( -5.87%)
> Min 2048 11110.53 ( 0.00%) 10606.20 ( -4.54%)
> Min 3312 13692.67 ( 0.00%) 13158.41 ( -3.90%)
> Min 4096 14926.29 ( 0.00%) 14457.46 ( -3.14%)
> Min 8192 18399.34 ( 0.00%) 18091.65 ( -1.67%)
> Min 16384 21384.13 ( 0.00%) 21158.05 ( -1.06%)
> Hmean 64 564.96 ( 0.00%) 534.38 ( -5.41%)
> Hmean 128 1064.42 ( 0.00%) 1010.12 ( -5.10%)
> Hmean 256 1965.85 ( 0.00%) 1879.16 ( -4.41%)
> Hmean 1024 6839.77 ( 0.00%) 6478.70 ( -5.28%)
> Hmean 2048 11154.80 ( 0.00%) 10671.13 ( -4.34%)
> Hmean 3312 13838.12 ( 0.00%) 13249.01 ( -4.26%)
> Hmean 4096 15009.99 ( 0.00%) 14561.36 ( -2.99%)
> Hmean 8192 18975.57 ( 0.00%) 18326.54 ( -3.42%)
> Hmean 16384 21440.44 ( 0.00%) 21324.59 ( -0.54%)
> Stddev 64 1.24 ( 0.00%) 2.85 (-130.64%)
> Stddev 128 4.51 ( 0.00%) 6.53 ( -44.84%)
> Stddev 256 11.67 ( 0.00%) 8.50 ( 27.16%)
> Stddev 1024 48.33 ( 0.00%) 75.07 ( -55.34%)
> Stddev 2048 54.82 ( 0.00%) 65.16 ( -18.86%)
> Stddev 3312 153.57 ( 0.00%) 56.29 ( 63.35%)
> Stddev 4096 100.25 ( 0.00%) 88.50 ( 11.72%)
> Stddev 8192 358.13 ( 0.00%) 169.99 ( 52.54%)
> Stddev 16384 43.99 ( 0.00%) 141.82 (-222.39%)
>
> Signed-off-by: Tony Jones <[email protected]>
> Fixes: 56974a6fcfef ("apparmor: add base infastructure for socket
> mediation")

hey Tony,

thanks for the patch, I am curious did you're investigation look
into what parts of DEFINE_AUDIT_SK are causing the issue?

regardless, I have pulled it into apparmor next

> ---
> security/apparmor/net.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/security/apparmor/net.c b/security/apparmor/net.c
> index bb24cfa0a164..d5d72dd1ca1f 100644
> --- a/security/apparmor/net.c
> +++ b/security/apparmor/net.c
> @@ -146,17 +146,20 @@ int aa_af_perm(struct aa_label *label, const char *op, u32 request, u16 family,
> static int aa_label_sk_perm(struct aa_label *label, const char *op, u32 request,
> struct sock *sk)
> {
> - struct aa_profile *profile;
> - DEFINE_AUDIT_SK(sa, op, sk);
> + int error = 0;
>
> AA_BUG(!label);
> AA_BUG(!sk);
>
> - if (unconfined(label))
> - return 0;
> + if (!unconfined(label)) {
> + struct aa_profile *profile;
> + DEFINE_AUDIT_SK(sa, op, sk);
>
> - return fn_for_each_confined(label, profile,
> - aa_profile_af_sk_perm(profile, &sa, request, sk));
> + error = fn_for_each_confined(label, profile,
> + aa_profile_af_sk_perm(profile, &sa, request, sk));
> + }
> +
> + return error;
> }
>
> int aa_sk_perm(const char *op, u32 request, struct sock *sk)
>


2018-09-07 23:57:00

by Tony Jones

[permalink] [raw]
Subject: Re: [PATCH] apparmor: Fix network performance issue in aa_label_sk_perm

On 09/07/2018 09:37 AM, John Johansen wrote:

> hey Tony,
>
> thanks for the patch, I am curious did you're investigation look
> into what parts of DEFINE_AUDIT_SK are causing the issue?

Hi JJ.

Attached are the perf annotations for DEFINE_AUDIT_SK (percentages are relative to the fn).
Our kernel performance testing is carried out with default installs which means AppArmor
is enabled but the performance tests are unconfined. It was obvious that the overhead of
DEFINE_AUDIT_SK was significant for smaller packet sizes (typical of synthetic benchmarks)
and that it didn't need to execute for the unconfined case, hence the patch. I didn't
spend any time looking at the performance of confined tasks. It may be worth your time to
look at this.

Comparing my current tip (2601dd392dd1) to tip+patch I'm seeing an increase of 3-6% in netperf
throughput for packet sizes 64-1024.

HTH

Tony

Percent | Source code & Disassembly of vmlinux for cycles:ppp (117 samples)
---------------------------------------------------------------------------------
:
:
:
: Disassembly of section .text:
:
: ffffffff813fbec0 <aa_label_sk_perm>:
: aa_label_sk_perm():
: type));
: }
:
: static int aa_label_sk_perm(struct aa_label *label, const char *op, u32 request,
: struct sock *sk)
: {
0.00 : ffffffff813fbec0: callq ffffffff81a017f0 <__fentry__>
2.56 : ffffffff813fbec5: push %r14
0.00 : ffffffff813fbec7: mov %rcx,%r14
: struct aa_profile *profile;
: DEFINE_AUDIT_SK(sa, op, sk);
0.00 : ffffffff813fbeca: mov $0x7,%ecx
: {
0.00 : ffffffff813fbecf: push %r13
3.42 : ffffffff813fbed1: mov %edx,%r13d
0.00 : ffffffff813fbed4: push %r12
0.00 : ffffffff813fbed6: push %rbp
0.00 : ffffffff813fbed7: mov %rdi,%rbp
5.13 : ffffffff813fbeda: push %rbx
0.00 : ffffffff813fbedb: sub $0xb8,%rsp
: DEFINE_AUDIT_SK(sa, op, sk);
0.00 : ffffffff813fbee2: movzwl 0x10(%r14),%r9d
: {
1.71 : ffffffff813fbee7: mov %gs:0x28,%rax
0.00 : ffffffff813fbef0: mov %rax,0xb0(%rsp)
0.00 : ffffffff813fbef8: xor %eax,%eax
: DEFINE_AUDIT_SK(sa, op, sk);
0.00 : ffffffff813fbefa: lea 0x78(%rsp),%rdx
1.71 : ffffffff813fbeff: lea 0x20(%rsp),%r8
0.00 : ffffffff813fbf04: movq $0x0,(%rsp)
0.00 : ffffffff813fbf0c: movq $0x0,0x10(%rsp)
0.00 : ffffffff813fbf15: mov %rdx,%rdi
14.53 : ffffffff813fbf18: rep stos %rax,%es:(%rdi)
1.71 : ffffffff813fbf1b: mov $0xb,%ecx
0.00 : ffffffff813fbf20: mov %r8,%rdi
0.00 : ffffffff813fbf23: mov %r14,0x80(%rsp)
18.80 : ffffffff813fbf2b: rep stos %rax,%es:(%rdi)
0.00 : ffffffff813fbf2e: mov %rsi,0x28(%rsp)
1.71 : ffffffff813fbf33: mov %r9w,0x88(%rsp)
0.00 : ffffffff813fbf3c: cmp $0x1,%r9w
0.00 : ffffffff813fbf41: je ffffffff813fbfa1 <aa_label_sk_perm+0xe1>
0.00 : ffffffff813fbf43: mov $0x2,%eax
0.00 : ffffffff813fbf48: test %r14,%r14
0.00 : ffffffff813fbf4b: je ffffffff813fbfa1 <aa_label_sk_perm+0xe1>
14.53 : ffffffff813fbf4d: mov %al,(%rsp)
0.00 : ffffffff813fbf50: movzwl 0x1ea(%r14),%eax
: AA_BUG(!sk);
:
: if (unconfined(label))
: return 0;
:
: return fn_for_each_confined(label, profile,
0.00 : ffffffff813fbf58: xor %r12d,%r12d
: DEFINE_AUDIT_SK(sa, op, sk);
0.00 : ffffffff813fbf5b: mov %r8,0x18(%rsp)
8.55 : ffffffff813fbf60: mov %eax,0x58(%rsp)
0.00 : ffffffff813fbf64: movzbl 0x1e9(%r14),%eax
0.00 : ffffffff813fbf6c: mov %rdx,0x8(%rsp)
0.00 : ffffffff813fbf71: mov %eax,0x5c(%rsp)
: if (unconfined(label))
8.55 : ffffffff813fbf75: testb $0x2,0x40(%rbp)
0.00 : ffffffff813fbf79: je ffffffff813fbfa8 <aa_label_sk_perm+0xe8>
: aa_profile_af_sk_perm(profile, &sa, request, sk));
: }
0.00 : ffffffff813fbf7b: mov 0xb0(%rsp),%rdx
0.00 : ffffffff813fbf83: xor %gs:0x28,%rdx
4.27 : ffffffff813fbf8c: mov %r12d,%eax
0.00 : ffffffff813fbf8f: jne ffffffff813fbfe5 <aa_label_sk_perm+0x125>
0.00 : ffffffff813fbf91: add $0xb8,%rsp
0.00 : ffffffff813fbf98: pop %rbx
5.13 : ffffffff813fbf99: pop %rbp
0.00 : ffffffff813fbf9a: pop %r12
0.00 : ffffffff813fbf9c: pop %r13
0.00 : ffffffff813fbf9e: pop %r14
7.69 : ffffffff813fbfa0: retq
: DEFINE_AUDIT_SK(sa, op, sk);
0.00 : ffffffff813fbfa1: mov $0x7,%eax
0.00 : ffffffff813fbfa6: jmp ffffffff813fbf4d <aa_label_sk_perm+0x8d>
: return fn_for_each_confined(label, profile,
0.00 : ffffffff813fbfa8: xor %esi,%esi
0.00 : ffffffff813fbfaa: jmp ffffffff813fbfcd <aa_label_sk_perm+0x10d>
: aa_profile_af_sk_perm():
: static inline int aa_profile_af_sk_perm(struct aa_profile *profile,
: struct common_audit_data *sa,
: u32 request,
: struct sock *sk)
: {
: return aa_profile_af_perm(profile, sa, request, sk->sk_family,
0.00 : ffffffff813fbfac: movzwl 0x10(%r14),%ecx
0.00 : ffffffff813fbfb1: movzwl 0x1ea(%r14),%r8d
0.00 : ffffffff813fbfb9: mov %rsp,%rsi
0.00 : ffffffff813fbfbc: mov %r13d,%edx
0.00 : ffffffff813fbfbf: callq ffffffff813fbdf0 <aa_profile_af_perm>
: aa_label_sk_perm():
0.00 : ffffffff813fbfc4: lea 0x1(%rbx),%esi
0.00 : ffffffff813fbfc7: test %eax,%eax
0.00 : ffffffff813fbfc9: cmovne %eax,%r12d
0.00 : ffffffff813fbfcd: mov %rbp,%rdi
0.00 : ffffffff813fbfd0: callq ffffffff813f7310 <aa_label_next_confined>
0.00 : ffffffff813fbfd5: mov %eax,%ebx
0.00 : ffffffff813fbfd7: cltq
0.00 : ffffffff813fbfd9: mov 0x50(%rbp,%rax,8),%rdi
0.00 : ffffffff813fbfde: test %rdi,%rdi
0.00 : ffffffff813fbfe1: jne ffffffff813fbfac <aa_label_sk_perm+0xec>
0.00 : ffffffff813fbfe3: jmp ffffffff813fbf7b <aa_label_sk_perm+0xbb>
: }
0.00 : ffffffff813fbfe5: callq ffffffff81090d60 <__stack_chk_fail>