[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZWirsc6i-8n4qSAo@hog>
Date: Thu, 30 Nov 2023 16:35:13 +0100
From: Sabrina Dubroca <sd@...asysnail.net>
To: Christian Hopps <chopps@...pps.org>
Cc: devel@...ux-ipsec.org, Steffen Klassert <steffen.klassert@...unet.com>,
netdev@...r.kernel.org, Christian Hopps <chopps@...n.net>
Subject: Re: [RFC ipsec-next v2 6/8] iptfs: xfrm: Add mode_cbs module
functionality
2023-11-12, 22:52:17 -0500, Christian Hopps wrote:
> From: Christian Hopps <chopps@...n.net>
>
> 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.
> 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)
> 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)
--
Sabrina
Powered by blists - more mailing lists