Re: [PATCH net-next v3 1/3] net: ipv6: ioam6_iptunnel: mitigate 2-realloc issue

[Vadim Fedorenko]

On 28/10/2024 22:36, Justin Iurman wrote:
> This patch mitigates the two-reallocations issue with ioam6_iptunnel by
> providing the dst_entry (in the cache) to the first call to
> skb_cow_head(). As a result, the very first iteration would still
> trigger two reallocations (i.e., empty cache), while next iterations
> would only trigger a single reallocation.
>
> Performance tests before/after applying this patch, which clearly shows
> the improvement:
> - inline mode:
>    - before: https://ibb.co/LhQ8V63
>    - after: https://ibb.co/x5YT2bS
> - encap mode:
>    - before: https://ibb.co/3Cjm5m0
>    - after: https://ibb.co/TwpsxTC
> - encap mode with tunsrc:
>    - before: https://ibb.co/Gpy9QPg
>    - after: https://ibb.co/PW1bZFT
>
> This patch also fixes an incorrect behavior: after the insertion, the
> second call to skb_cow_head() makes sure that the dev has enough
> headroom in the skb for layer 2 and stuff. In that case, the "old"
> dst_entry was used, which is now fixed. After discussing with Paolo, it
> appears that both patches can be merged into a single one -this one-
> (for the sake of readability) and target net-next.
>
> Signed-off-by: Justin Iurman <justin.iurman@xxxxxxxxx>
> ---
>   net/ipv6/ioam6_iptunnel.c | 90 +++++++++++++++++++++------------------
>   1 file changed, 49 insertions(+), 41 deletions(-)
>
> diff --git a/net/ipv6/ioam6_iptunnel.c b/net/ipv6/ioam6_iptunnel.c
> index beb6b4cfc551..07bfd557e08a 100644
> --- a/net/ipv6/ioam6_iptunnel.c
> +++ b/net/ipv6/ioam6_iptunnel.c
> @@ -254,15 +254,24 @@ static int ioam6_do_fill(struct net *net, struct sk_buff *skb)
>   	return 0;
>   }
>   
> +static inline int dev_overhead(struct dst_entry *dst, struct sk_buff *skb)
> +{
> +	if (likely(dst))
> +		return LL_RESERVED_SPACE(dst->dev);
> +
> +	return skb->mac_len;
> +}

static inline functions in .c files are not welcome.
consider to move this helper to some header, probably dev.h or dst.h
and reuse it in other tunnels.

And please honor 24h rule before the next submission.


>   static int ioam6_do_inline(struct net *net, struct sk_buff *skb,
> -			   struct ioam6_lwt_encap *tuninfo)
> +			   struct ioam6_lwt_encap *tuninfo,
> +			   struct dst_entry *dst)
>   {
>   	struct ipv6hdr *oldhdr, *hdr;
>   	int hdrlen, err;
>   
>   	hdrlen = (tuninfo->eh.hdrlen + 1) << 3;
>   
> -	err = skb_cow_head(skb, hdrlen + skb->mac_len);
> +	err = skb_cow_head(skb, hdrlen + dev_overhead(dst, skb));
>   	if (unlikely(err))
>   		return err;
>   
[.. snip ..]