Re: [PATCH net-next] xfrm: rework default policy structure
From: Leon Romanovsky <leon@kernel.org>
Date: 2021-11-18 19:09:23
On Thu, Nov 18, 2021 at 03:29:37PM +0100, Nicolas Dichtel wrote:
quoted hunk ↗ jump to hunk
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 <redacted> --- 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?
quoted hunk ↗ jump to hunk
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