[PATCH ipsec-next v3] xfrm: Use xfrm_ip2inner_mode() unconditionally

[PATCH ipsec-next v3] xfrm: Use xfrm_ip2inner_mode() unconditionally

[Jianbo Liu]

Commit c9500d7b7de8 ("xfrm: store xfrm_mode directly, not its
address") changed how the xfrm_mode is stored in the xfrm state. The
inner_mode NULL check is redundant as xfrm_ip2inner_mode() now returns
the address of an embedded structure, which cannot be NULL.

Additionally, commit 61fafbee6cfe ("xfrm: Determine inner GSO type
from packet inner protocol") updated xfrm_ip2inner_mode() to
explicitly check x->sel.family. If the selector family is specified
(i.e., not AF_UNSPEC), the helper now correctly returns &x->inner_mode
directly.

This means the manual branching which checked for AF_UNSPEC before
deciding whether to call the helper or use the state's inner mode
directly is no longer necessary.

This patch simplifies the code by calling xfrm_ip2inner_mode()
unconditionally and removing the NULL checking.

Signed-off-by: Jianbo Liu <jianbol@nvidia.com>
Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
---
V3:
 - Change the commit subject (was "xfrm: Remove redundant state inner mode check").
 - Call xfrm_ip2inner_mode() unconditionally and update the commit message accordingly.

V2:
 - Change subject prefix, and send separately to "ipsec-next".

 net/ipv4/ip_vti.c              | 11 +----------
 net/ipv6/ip6_vti.c             | 11 +----------
 net/xfrm/xfrm_interface_core.c | 11 +----------
 net/xfrm/xfrm_policy.c         | 11 +----------
 4 files changed, 4 insertions(+), 40 deletions(-)

diff --git a/net/ipv4/ip_vti.c b/net/ipv4/ip_vti.c
index 95b6bb78fcd2..89784976c65e 100644
--- a/net/ipv4/ip_vti.c
+++ b/net/ipv4/ip_vti.c
@@ -118,16 +118,7 @@ static int vti_rcv_cb(struct sk_buff *skb, int err)
 
 	x = xfrm_input_state(skb);
 
-	inner_mode = &x->inner_mode;
-
-	if (x->sel.family == AF_UNSPEC) {
-		inner_mode = xfrm_ip2inner_mode(x, XFRM_MODE_SKB_CB(skb)->protocol);
-		if (inner_mode == NULL) {
-			XFRM_INC_STATS(dev_net(skb->dev),
-				       LINUX_MIB_XFRMINSTATEMODEERROR);
-			return -EINVAL;
-		}
-	}
+	inner_mode = xfrm_ip2inner_mode(x, XFRM_MODE_SKB_CB(skb)->protocol);
 
 	family = inner_mode->family;
 
diff --git a/net/ipv6/ip6_vti.c b/net/ipv6/ip6_vti.c
index ad5290be4dd6..fd56831837de 100644
--- a/net/ipv6/ip6_vti.c
+++ b/net/ipv6/ip6_vti.c
@@ -362,16 +362,7 @@ static int vti6_rcv_cb(struct sk_buff *skb, int err)
 
 	x = xfrm_input_state(skb);
 
-	inner_mode = &x->inner_mode;
-
-	if (x->sel.family == AF_UNSPEC) {
-		inner_mode = xfrm_ip2inner_mode(x, XFRM_MODE_SKB_CB(skb)->protocol);
-		if (inner_mode == NULL) {
-			XFRM_INC_STATS(dev_net(skb->dev),
-				       LINUX_MIB_XFRMINSTATEMODEERROR);
-			return -EINVAL;
-		}
-	}
+	inner_mode = xfrm_ip2inner_mode(x, XFRM_MODE_SKB_CB(skb)->protocol);
 
 	family = inner_mode->family;
 
diff --git a/net/xfrm/xfrm_interface_core.c b/net/xfrm/xfrm_interface_core.c
index 330a05286a56..802a54569df9 100644
--- a/net/xfrm/xfrm_interface_core.c
+++ b/net/xfrm/xfrm_interface_core.c
@@ -387,16 +387,7 @@ static int xfrmi_rcv_cb(struct sk_buff *skb, int err)
 	xnet = !net_eq(xi->net, dev_net(skb->dev));
 
 	if (xnet) {
-		inner_mode = &x->inner_mode;
-
-		if (x->sel.family == AF_UNSPEC) {
-			inner_mode = xfrm_ip2inner_mode(x, XFRM_MODE_SKB_CB(skb)->protocol);
-			if (inner_mode == NULL) {
-				XFRM_INC_STATS(dev_net(skb->dev),
-					       LINUX_MIB_XFRMINSTATEMODEERROR);
-				return -EINVAL;
-			}
-		}
+		inner_mode = xfrm_ip2inner_mode(x, XFRM_MODE_SKB_CB(skb)->protocol);
 
 		if (!xfrm_policy_check(NULL, XFRM_POLICY_IN, skb,
 				       inner_mode->family))
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 62486f866975..a609b1fa3109 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -2711,16 +2711,7 @@ static struct dst_entry *xfrm_bundle_create(struct xfrm_policy *policy,
 			 */
 			xfrm_dst_set_child(xdst_prev, &xdst->u.dst);
 
-		if (xfrm[i]->sel.family == AF_UNSPEC) {
-			inner_mode = xfrm_ip2inner_mode(xfrm[i],
-							xfrm_af2proto(family));
-			if (!inner_mode) {
-				err = -EAFNOSUPPORT;
-				dst_release(dst);
-				goto put_states;
-			}
-		} else
-			inner_mode = &xfrm[i]->inner_mode;
+		inner_mode = xfrm_ip2inner_mode(xfrm[i], xfrm_af2proto(family));
 
 		xdst->route = dst;
 		dst_copy_metrics(dst1, dst);
-- 
2.49.0

Re: [PATCH ipsec-next v3] xfrm: Use xfrm_ip2inner_mode() unconditionally

[Sabrina Dubroca]

2025-11-28, 05:48:04 +0200, Jianbo Liu wrote:
> Commit c9500d7b7de8 ("xfrm: store xfrm_mode directly, not its
> address") changed how the xfrm_mode is stored in the xfrm state. The
> inner_mode NULL check is redundant as xfrm_ip2inner_mode() now returns
> the address of an embedded structure, which cannot be NULL.
> 
> Additionally, commit 61fafbee6cfe ("xfrm: Determine inner GSO type
> from packet inner protocol") updated xfrm_ip2inner_mode() to
> explicitly check x->sel.family. If the selector family is specified
> (i.e., not AF_UNSPEC), the helper now correctly returns &x->inner_mode
> directly.

Note: that commit is not in ipsec-next yet, only in ipsec. This patch
should only be applied to ipsec-next once the trees have been merged
together.

> This means the manual branching which checked for AF_UNSPEC before
> deciding whether to call the helper or use the state's inner mode
> directly is no longer necessary.
> 
> This patch simplifies the code by calling xfrm_ip2inner_mode()
> unconditionally and removing the NULL checking.
> 
> Signed-off-by: Jianbo Liu <jianbol@nvidia.com>
> Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
> ---
> V3:
>  - Change the commit subject (was "xfrm: Remove redundant state inner mode check").
>  - Call xfrm_ip2inner_mode() unconditionally and update the commit message accordingly.

Other than the tree scheduling comment above, the patch looks ok,
thanks.

Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>

-- 
Sabrina