All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] apparmor: Fix network performance issue in aa_label_sk_perm
@ 2018-09-07  4:33 ` Tony Jones
  0 siblings, 0 replies; 6+ messages in thread
From: Tony Jones @ 2018-09-07  4:33 UTC (permalink / raw)
  To: john.johansen; +Cc: seth.arnold, linux-kernel, linux-security-module

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 <tonyj@suse.de>
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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] apparmor: Fix network performance issue in aa_label_sk_perm
@ 2018-09-07  4:33 ` Tony Jones
  0 siblings, 0 replies; 6+ messages in thread
From: Tony Jones @ 2018-09-07  4:33 UTC (permalink / raw)
  To: linux-security-module

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 <tonyj@suse.de>
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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] apparmor: Fix network performance issue in aa_label_sk_perm
  2018-09-07  4:33 ` Tony Jones
@ 2018-09-07 16:37   ` John Johansen
  -1 siblings, 0 replies; 6+ messages in thread
From: John Johansen @ 2018-09-07 16:37 UTC (permalink / raw)
  To: Tony Jones; +Cc: seth.arnold, linux-kernel, linux-security-module

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 <tonyj@suse.de>
> 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)
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] apparmor: Fix network performance issue in aa_label_sk_perm
@ 2018-09-07 16:37   ` John Johansen
  0 siblings, 0 replies; 6+ messages in thread
From: John Johansen @ 2018-09-07 16:37 UTC (permalink / raw)
  To: linux-security-module

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 <tonyj@suse.de>
> 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)
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] apparmor: Fix network performance issue in aa_label_sk_perm
  2018-09-07 16:37   ` John Johansen
@ 2018-09-07 23:53     ` Tony Jones
  -1 siblings, 0 replies; 6+ messages in thread
From: Tony Jones @ 2018-09-07 23:53 UTC (permalink / raw)
  To: John Johansen; +Cc: seth.arnold, linux-kernel, linux-security-module

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>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] apparmor: Fix network performance issue in aa_label_sk_perm
@ 2018-09-07 23:53     ` Tony Jones
  0 siblings, 0 replies; 6+ messages in thread
From: Tony Jones @ 2018-09-07 23:53 UTC (permalink / raw)
  To: linux-security-module

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>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-09-07 23:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-07  4:33 [PATCH] apparmor: Fix network performance issue in aa_label_sk_perm Tony Jones
2018-09-07  4:33 ` Tony Jones
2018-09-07 16:37 ` John Johansen
2018-09-07 16:37   ` John Johansen
2018-09-07 23:53   ` Tony Jones
2018-09-07 23:53     ` Tony Jones

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.