Re: [PATCH 09/14] ipv6: hand dst refs to cork setup

From: Willem de Bruijn
Date: Tue Jan 11 2022 - 10:40:11 EST


On Mon, Jan 10, 2022 at 8:25 PM Pavel Begunkov <asml.silence@xxxxxxxxx> wrote:
>
> During cork->dst setup, ip6_make_skb() gets an additional reference to
> a passed in dst. However, udpv6_sendmsg() doesn't need dst after calling
> ip6_make_skb(), and so we can save two additional atomics by passing
> dst references to ip6_make_skb(). udpv6_sendmsg() is the only caller, so
> it's enough to make sure it doesn't use dst afterwards.
>
> Signed-off-by: Pavel Begunkov <asml.silence@xxxxxxxxx>
> ---

There are two patches 9/14

> net/ipv6/ip6_output.c | 9 ++++++---
> net/ipv6/udp.c | 3 ++-
> 2 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 0cc490f2cfbf..6a7bba4dd04d 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -1356,6 +1356,8 @@ static int ip6_setup_cork(struct sock *sk, struct inet_cork_full *cork,
> unsigned int mtu;
> struct ipv6_txoptions *nopt, *opt = ipc6->opt;
>
> + cork->base.dst = &rt->dst;
> +

Is there a reason to move this up from its original location next to
the other cork initialization assignments?

That the reference is taken in ip6_append_data for corked requests
(once, in setup cork branch) and inherited from udpv6_send_skb
otherwise is non-trivial. Worth a comment.

> /*
> * setup for corking
> */
> @@ -1389,8 +1391,6 @@ static int ip6_setup_cork(struct sock *sk, struct inet_cork_full *cork,
>
> /* need source address above miyazawa*/
> }
> - dst_hold(&rt->dst);
> - cork->base.dst = &rt->dst;
> v6_cork->hop_limit = ipc6->hlimit;
> v6_cork->tclass = ipc6->tclass;
> if (rt->dst.flags & DST_XFRM_TUNNEL)
> @@ -1784,6 +1784,7 @@ int ip6_append_data(struct sock *sk,
> /*
> * setup for corking
> */
> + dst_hold(&rt->dst);
> err = ip6_setup_cork(sk, &inet->cork, &np->cork,
> ipc6, rt);
> if (err)
> @@ -1974,8 +1975,10 @@ struct sk_buff *ip6_make_skb(struct sock *sk,
> int exthdrlen = (ipc6->opt ? ipc6->opt->opt_flen : 0);
> int err;
>
> - if (flags & MSG_PROBE)
> + if (flags & MSG_PROBE) {
> + dst_release(&rt->dst);
> return NULL;
> + }
>
> __skb_queue_head_init(&queue);
>
> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
> index eec83e34ae27..3039dff7fe64 100644
> --- a/net/ipv6/udp.c
> +++ b/net/ipv6/udp.c
> @@ -1541,7 +1541,8 @@ int udpv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
> err = PTR_ERR(skb);
> if (!IS_ERR_OR_NULL(skb))
> err = udp_v6_send_skb(skb, fl6, &cork.base);
> - goto out;
> + /* ip6_make_skb steals dst reference */
> + goto out_no_dst;
> }
>
> lock_sock(sk);
> --
> 2.34.1
>