On 03/23/2018 08:13 PM, Alexey Kodanev wrote: > On 03/23/2018 06:50 PM, Eric Dumazet wrote: ... >>> + if (connected) >>> + ip6_dst_store(sk, dst, >>> + ipv6_addr_equal(&fl6.daddr, &sk->sk_v6_daddr) ? >>> + &sk->sk_v6_daddr : NULL, >>> +#ifdef CONFIG_IPV6_SUBTREES >>> + ipv6_addr_equal(&fl6.saddr, &np->saddr) ? >>> + &np->saddr : >>> +#endif >>> + NULL); >>> + >> >> What about the MSG_CONFIRM stuff ? >> >>> if (msg->msg_flags&MSG_CONFIRM) >>> goto do_confirm; >>> back_from_confirm: >> >> Should not you move the above code here instead ? > > Ah, you are right, it can release that dst if it go to "do_confirm". > >>> Also ip6_dst_store() does not increment dst refcount. >> >> I fear that as soon as dst is visible to other cpus, it might be stolen. >> > > So we should pass dst_clone(dst) to ip6_dst_store() instead of dst, > because udpv6_err() can release it if it's set the new one. > > Then, I guess, we could left udpv6_sendmsg()/ip6_dst_store() where it > is now in the patch and remove the check for "connected" before dst_relase(), > similar to udp_sendmsg(), right?
And the section "release_dst:" looks redundant after that too. Thanks, Alexey
