Sabrina Dubroca [off-list ref] writes:

2023-11-12, 22:52:17 -0500, Christian Hopps wrote:

quoted

From: Christian Hopps <redacted>

Add a set of callbacks xfrm_mode_cbs to xfrm_state. These callbacks
enable the addition of new xfrm modes, such as IP-TFS to be defined
in modules.

Not a big fan of bringing back modes in modules :(
Florian's work made the code a lot more readable.

quoted

diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
index 662c83beb345..4390c111410d 100644
--- a/net/xfrm/xfrm_output.c
+++ b/net/xfrm/xfrm_output.c

@@ -280,7 +280,9 @@ static int xfrm4_tunnel_encap_add(struct xfrm_state *x, struct sk_buff *skb)
 	skb_set_inner_network_header(skb, skb_network_offset(skb));
 	skb_set_inner_transport_header(skb, skb_transport_offset(skb));

-	skb_set_network_header(skb, -x->props.header_len);
+	/* backup to add space for the outer encap */
+	skb_set_network_header(skb,
+			       -x->props.header_len + x->props.enc_hdr_len);

Since this only gets called for XFRM_MODE_TUNNEL, and only iptfs sets
enc_hdr_len, do we need this change? (and same for xfrm6_tunnel_encap_add)

You're right, removed. This particular code actually predated the callbacks.

quoted

 	skb->mac_header = skb->network_header +
 			  offsetof(struct iphdr, protocol);
 	skb->transport_header = skb->network_header + sizeof(*top_iph);

@@ -325,7 +327,8 @@ static int xfrm6_tunnel_encap_add(struct xfrm_state *x, struct sk_buff *skb)
 	skb_set_inner_network_header(skb, skb_network_offset(skb));
 	skb_set_inner_transport_header(skb, skb_transport_offset(skb));

-	skb_set_network_header(skb, -x->props.header_len);
+	skb_set_network_header(skb,
+			       -x->props.header_len + x->props.enc_hdr_len);
 	skb->mac_header = skb->network_header +
 			  offsetof(struct ipv6hdr, nexthdr);
 	skb->transport_header = skb->network_header + sizeof(*top_iph);

@@ -472,6 +475,8 @@ static int xfrm_outer_mode_output(struct xfrm_state *x, struct sk_buff *skb)
 		WARN_ON_ONCE(1);
 		break;
 	default:
+		if (x->mode_cbs->prepare_output)

Can x->mode_cbs be NULL here? Every other use of mode_cbs does
    if (x->mode_cbs && x->mode_cbs->FOO)

(I think not at the moment since only IPTFS (and IN_TRIGGER) can reach
this, but this inconsistency with the rest of the series struck me)

Still worth putting the guard in, fixed.

Thanks,
Chris.