From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8ED30C433F5 for ; Thu, 18 Nov 2021 19:09:23 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 762E461131 for ; Thu, 18 Nov 2021 19:09:23 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232778AbhKRTMW (ORCPT ); Thu, 18 Nov 2021 14:12:22 -0500 Received: from mail.kernel.org ([198.145.29.99]:58026 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232137AbhKRTMU (ORCPT ); Thu, 18 Nov 2021 14:12:20 -0500 Received: by mail.kernel.org (Postfix) with ESMTPSA id 0E4EE6124B; Thu, 18 Nov 2021 19:09:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1637262559; bh=bWCmM9yI3QPu0lOm+Nc7dYXiy1spQVos53w9OZJHk58=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=g9g7Iar4wrehdQrtnA17hFSHlGlswPVNNiQ2aEMsyYqyA/u7xFVRiIf3rWviMN6y/ sWEXUF8zTDwbFhKWMRNJRGeJc5ynTHYWTN8vV2cJWuPgJHOiyWSLlaRCNWNTd2qMu2 Wxi+bKjNdaZDYhLmrF5G/9qkldU00GaB84xJdEYgEaxAp3V/ivx+uSLLA5QsK86T08 Mi+IL8SdHxFumyn7XP5JrD+bZdnEntRNuAkdG6H2oCa+GgUWBjXTyut5yyIKl3yFNP 3WU2GmW/8/sVqQ0nLJDTXRLZGf91A71ZJkJT37FOkML7jMrc+In8Qjr/8VjGaqmAvq 3G+GVPGQczydw== Date: Thu, 18 Nov 2021 21:09:15 +0200 From: Leon Romanovsky To: Nicolas Dichtel Cc: steffen.klassert@secunet.com, herbert@gondor.apana.org.au, antony.antony@secunet.com, davem@davemloft.net, kuba@kernel.org, netdev@vger.kernel.org Subject: Re: [PATCH net-next] xfrm: rework default policy structure Message-ID: References: <20211118142937.5425-1-nicolas.dichtel@6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20211118142937.5425-1-nicolas.dichtel@6wind.com> Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Thu, Nov 18, 2021 at 03:29:37PM +0100, Nicolas Dichtel wrote: > This is a follow up of commit f8d858e607b2 ("xfrm: make user policy API > complete"). The goal is to align userland API to the internal structures. > > Signed-off-by: Nicolas Dichtel > --- > > This patch targets ipsec-next, but because ipsec-next has not yet been > rebased on top of net-next, I based the patch on top of net-next. > > include/net/netns/xfrm.h | 6 +----- > include/net/xfrm.h | 38 ++++++++--------------------------- > net/xfrm/xfrm_policy.c | 10 +++++++--- > net/xfrm/xfrm_user.c | 43 +++++++++++++++++----------------------- > 4 files changed, 34 insertions(+), 63 deletions(-) > > diff --git a/include/net/netns/xfrm.h b/include/net/netns/xfrm.h > index 947733a639a6..bd7c3be4af5d 100644 > --- a/include/net/netns/xfrm.h > +++ b/include/net/netns/xfrm.h > @@ -66,11 +66,7 @@ struct netns_xfrm { > int sysctl_larval_drop; > u32 sysctl_acq_expires; > > - u8 policy_default; > -#define XFRM_POL_DEFAULT_IN 1 > -#define XFRM_POL_DEFAULT_OUT 2 > -#define XFRM_POL_DEFAULT_FWD 4 > -#define XFRM_POL_DEFAULT_MASK 7 > + u8 policy_default[XFRM_POLICY_MAX]; > > #ifdef CONFIG_SYSCTL > struct ctl_table_header *sysctl_hdr; > diff --git a/include/net/xfrm.h b/include/net/xfrm.h > index 2308210793a0..3fd1e052927e 100644 > --- a/include/net/xfrm.h > +++ b/include/net/xfrm.h > @@ -1075,22 +1075,6 @@ xfrm_state_addr_cmp(const struct xfrm_tmpl *tmpl, const struct xfrm_state *x, un > } > > #ifdef CONFIG_XFRM > -static inline bool > -xfrm_default_allow(struct net *net, int dir) > -{ > - u8 def = net->xfrm.policy_default; > - > - switch (dir) { > - case XFRM_POLICY_IN: > - return def & XFRM_POL_DEFAULT_IN ? false : true; > - case XFRM_POLICY_OUT: > - return def & XFRM_POL_DEFAULT_OUT ? false : true; > - case XFRM_POLICY_FWD: > - return def & XFRM_POL_DEFAULT_FWD ? false : true; > - } > - return false; > -} > - > int __xfrm_policy_check(struct sock *, int dir, struct sk_buff *skb, > unsigned short family); > > @@ -1104,13 +1088,10 @@ static inline int __xfrm_policy_check2(struct sock *sk, int dir, > if (sk && sk->sk_policy[XFRM_POLICY_IN]) > return __xfrm_policy_check(sk, ndir, skb, family); > > - if (xfrm_default_allow(net, dir)) > - return (!net->xfrm.policy_count[dir] && !secpath_exists(skb)) || > - (skb_dst(skb) && (skb_dst(skb)->flags & DST_NOPOLICY)) || > - __xfrm_policy_check(sk, ndir, skb, family); > - else > - return (skb_dst(skb) && (skb_dst(skb)->flags & DST_NOPOLICY)) || > - __xfrm_policy_check(sk, ndir, skb, family); > + return (net->xfrm.policy_default[dir] == XFRM_USERPOLICY_ACCEPT && > + (!net->xfrm.policy_count[dir] && !secpath_exists(skb))) || > + (skb_dst(skb) && (skb_dst(skb)->flags & DST_NOPOLICY)) || > + __xfrm_policy_check(sk, ndir, skb, family); > } This is completely unreadable. What is the advantage of writing like this? > > static inline int xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb, unsigned short family) > @@ -1162,13 +1143,10 @@ static inline int xfrm_route_forward(struct sk_buff *skb, unsigned short family) > { > struct net *net = dev_net(skb->dev); > > - if (xfrm_default_allow(net, XFRM_POLICY_FWD)) > - return !net->xfrm.policy_count[XFRM_POLICY_OUT] || > - (skb_dst(skb)->flags & DST_NOXFRM) || > - __xfrm_route_forward(skb, family); > - else > - return (skb_dst(skb)->flags & DST_NOXFRM) || > - __xfrm_route_forward(skb, family); > + return (net->xfrm.policy_default[XFRM_POLICY_FWD] == XFRM_USERPOLICY_ACCEPT && > + !net->xfrm.policy_count[XFRM_POLICY_OUT]) || > + (skb_dst(skb)->flags & DST_NOXFRM) || > + __xfrm_route_forward(skb, family); Ditto. Thanks