On Mon, Jul 17, 2023 at 10:28 AM Paolo Abeni <pabeni@xxxxxxxxxx> wrote:
>
> Perf traces of network-related workload shows a measurable overhead
> inside the network-related selinux hooks while zeroing the
> lsm_network_audit struct.
>
> In most cases we can delay the initialization of such structure to the
> usage point, avoiding such overhead in a few cases.
>
> Additionally, the audit code accesses the IP address information only
> for AF_INET* families, and selinux_parse_skb() will fill-out the
> relevant fields in such cases. When the family field is zeroed or the
> initialization is followed by the mentioned parsing, the zeroing can be
> limited to the sk, family and netif fields.
>
> By factoring out the audit-data initialization to new helpers, this
> patch removes some duplicate code and gives small but measurable
> performance gain under UDP flood.
>
> Signed-off-by: Paolo Abeni <pabeni@xxxxxxxxxx>
> ---
> Note the performance gain is small, but measurable and let the selinux
> hooks almost disappear from the perf traces I collect. The only
> remaining perf-related pain-point I see is the indirect call at the
> security_ level, and tackling it looks much more difficult... :(
> ---
> security/selinux/hooks.c | 84 ++++++++++++++++++++--------------------
> 1 file changed, 43 insertions(+), 41 deletions(-)
Overall this looks good to me, I just have some bikeshed-y comments
around function names (below) ...
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index d06e350fedee..9a75b3bcff2b 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -4495,18 +4495,41 @@ static int socket_sockcreate_sid(const struct task_security_struct *tsec,
> secclass, NULL, socksid);
> }
>
> +static void __ad_init_net(struct common_audit_data *ad,
> + struct lsm_network_audit *net,
> + int ifindex, struct sock *sk, u16 family)
> +{
> + ad->type = LSM_AUDIT_DATA_NET;
> + ad->u.net = net;
> + net->netif = ifindex;
> + net->sk = sk;
> + net->family = family;
> +}
Let's call this "__ad_net_init()".
> +static void ad_init_net_from_sk(struct common_audit_data *ad,
> + struct lsm_network_audit *net,
> + struct sock *sk)
> +{
> + __ad_init_net(ad, net, 0, sk, 0);
> +}
Similarly, let's call this "ad_net_init_from_sk()"
> +static void ad_init_net_from_netif_family(struct common_audit_data *ad,
> + struct lsm_network_audit *net,
> + int ifindex, u16 family)
> +{
> + __ad_init_net(ad, net, ifindex, 0, family);
> +}
... and I think this should be "ad_net_init_from_iif()".
--
paul-moore.com
