Re: [PATCH net v4 0/3] ipv6: udp6: set dst cache for a connected sk if current not valid

2018-03-30 Thread Martin KaFai Lau
On Fri, Mar 30, 2018 at 08:53:06PM +0300, Alexey Kodanev wrote:
> A new RTF_CACHE route can be created with the socket's dst cache
> update between the below calls in udpv6_sendmsg(), when datagram
> sending results to ICMPV6_PKT_TOOBIG error:
> 
>dst = ip6_sk_dst_lookup_flow(...)
>...
> release_dst:
> if (dst) {
> if (connected) {
> ip6_dst_store(sk, dst)
> 
> Therefore, the new socket's dst cache reset to the old one on
> "release_dst:".
> 
> The first two patches prepare the code to store dst cache
> with ip6_sk_dst_lookup_flow():
> 
>   * the first patch adds ip6_dst_store_flow() function with
> commonly used source and destiantion addresses checks using
> the flow information.
> 
>   * the second patch adds new argument to ip6_sk_dst_lookup_flow()
> and ability to store dst in the socket's cache. Also, the two
> users of the function are updated without enabling the new
> behavior: pingv6_sendmsg() and udpv6_sendmsg().
> 
> The last patch contains the actual fix that removes sk dst cache
> update in the end of udpv6_sendmsg(), and allows to do it in
> ip6_sk_dst_lookup_flow().
> 
> v4: * fix the error in the build of ip_dst_store_flow() reported by
>   kbuild test robot due to missing checks for CONFIG_IPV6: add
>   new function to ip6_output.c instead of ip6_route.h
Thanks for the patches!

Instead of ip6_output.c, would net/ipv6/route.c be a better
place for the new ip6_sk_dst_store_flow()?

Others LGTM.  

> * add 'const' to struct flowi6 in ip6_dst_store_flow()
> * minor commit messages fixes
> 
> v3: * instead of moving ip6_dst_store() above udp_v6_send_skb(),
>   update socket's dst cache inside ip6_sk_dst_lookup_flow()
>   if the current one is invalid
> * the issue not reproduced in 4.1, but starting from 4.2. Add
>   one more 'Fixes:' commit that creates new RTF_CACHE route.
>   Though, it is also mentioned in the first one
> 
> 
> Alexey Kodanev (3):
>   ipv6: add a wrapper for ip6_dst_store() with flowi6 checks
>   ipv6: allow to cache dst for a connected sk in ip6_sk_dst_lookup_flow()
>   ipv6: udp6: set dst cache for a connected sk if current not valid
> 
>  include/net/ipv6.h|  6 --
>  net/ipv6/datagram.c   |  9 +
>  net/ipv6/ip6_output.c | 32 +---
>  net/ipv6/ping.c   |  2 +-
>  net/ipv6/udp.c| 21 ++---
>  5 files changed, 37 insertions(+), 33 deletions(-)
> 
> -- 
> 1.8.3.1
> 


[PATCH net v4 0/3] ipv6: udp6: set dst cache for a connected sk if current not valid

2018-03-30 Thread Alexey Kodanev
A new RTF_CACHE route can be created with the socket's dst cache
update between the below calls in udpv6_sendmsg(), when datagram
sending results to ICMPV6_PKT_TOOBIG error:

   dst = ip6_sk_dst_lookup_flow(...)
   ...
release_dst:
if (dst) {
if (connected) {
ip6_dst_store(sk, dst)

Therefore, the new socket's dst cache reset to the old one on
"release_dst:".

The first two patches prepare the code to store dst cache
with ip6_sk_dst_lookup_flow():

  * the first patch adds ip6_dst_store_flow() function with
commonly used source and destiantion addresses checks using
the flow information.

  * the second patch adds new argument to ip6_sk_dst_lookup_flow()
and ability to store dst in the socket's cache. Also, the two
users of the function are updated without enabling the new
behavior: pingv6_sendmsg() and udpv6_sendmsg().

The last patch contains the actual fix that removes sk dst cache
update in the end of udpv6_sendmsg(), and allows to do it in
ip6_sk_dst_lookup_flow().

v4: * fix the error in the build of ip_dst_store_flow() reported by
  kbuild test robot due to missing checks for CONFIG_IPV6: add
  new function to ip6_output.c instead of ip6_route.h
* add 'const' to struct flowi6 in ip6_dst_store_flow()
* minor commit messages fixes

v3: * instead of moving ip6_dst_store() above udp_v6_send_skb(),
  update socket's dst cache inside ip6_sk_dst_lookup_flow()
  if the current one is invalid
* the issue not reproduced in 4.1, but starting from 4.2. Add
  one more 'Fixes:' commit that creates new RTF_CACHE route.
  Though, it is also mentioned in the first one


Alexey Kodanev (3):
  ipv6: add a wrapper for ip6_dst_store() with flowi6 checks
  ipv6: allow to cache dst for a connected sk in ip6_sk_dst_lookup_flow()
  ipv6: udp6: set dst cache for a connected sk if current not valid

 include/net/ipv6.h|  6 --
 net/ipv6/datagram.c   |  9 +
 net/ipv6/ip6_output.c | 32 +---
 net/ipv6/ping.c   |  2 +-
 net/ipv6/udp.c| 21 ++---
 5 files changed, 37 insertions(+), 33 deletions(-)

-- 
1.8.3.1