netdev - Re: [PATCH net-next] ipv6: do lazy dst->__use update when per cpu dst is available

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 27 Sep 2017 11:03:33 -0700
From:   Eric Dumazet <edumazet@...gle.com>
To:     Wei Wang <weiwan@...gle.com>
Cc:     Paolo Abeni <pabeni@...hat.com>, Martin KaFai Lau <kafai@...com>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Hannes Frederic Sowa <hannes@...essinduktion.org>
Subject: Re: [PATCH net-next] ipv6: do lazy dst->__use update when per cpu dst
 is available

>> diff --git a/net/ipv6/route.c b/net/ipv6/route.c

>
> Hi Paolo,
>
> Eric and I discussed about this issue recently as well :).
>
> What about the following change:
>
> diff --git a/include/net/dst.h b/include/net/dst.h
> index 93568bd0a352..33e1d86bcef6 100644
> --- a/include/net/dst.h
> +++ b/include/net/dst.h
> @@ -258,14 +258,18 @@ static inline void dst_hold(struct dst_entry *dst)
>  static inline void dst_use(struct dst_entry *dst, unsigned long time)
>  {
>         dst_hold(dst);
> -       dst->__use++;
> -       dst->lastuse = time;
> +       if (dst->lastuse != time) {
> +               dst->__use++;
> +               dst->lastuse = time;
> +       }
>  }
>
>  static inline void dst_use_noref(struct dst_entry *dst, unsigned long time)
>  {
> -       dst->__use++;
> -       dst->lastuse = time;
> +       if (dst->lastuse != time) {
> +               dst->__use++;
> +               dst->lastuse = time;
> +       }
>  }
>
>  static inline struct dst_entry *dst_clone(struct dst_entry *dst)
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 26cc9f483b6d..e195f093add3 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -1170,8 +1170,7 @@ struct rt6_info *ip6_pol_route(struct net *net,
> struct fib6_table *table,
>
>                 struct rt6_info *pcpu_rt;
>
> -               rt->dst.lastuse = jiffies;
> -               rt->dst.__use++;
> +               dst_use_noref(rt, jiffies);
>                 pcpu_rt = rt6_get_pcpu_route(rt);
>
>                 if (pcpu_rt) {
>
>
> This way we always only update dst->__use and dst->lastuse at most
> once per jiffy. And we don't really need to update pcpu and then do
> the copy over from pcpu_rt to rt operation.
>
> Another thing is that I don't really see any places making use of
> dst->__use. So maybe we can also get rid of this dst->__use field?
>
> Thanks.
> Wei

Paolo, given we are very close to send Wei awesome work about IPv6
routing cache,
could we ask you to wait few days before doing the same work from your side ?

Main issue is the rwlock, and we are converting it to full RCU.

You are sending patches that are making our job very difficult IMO.

We chose to mimic the change done in neighbour code years ago
( 0ed8ddf4045fcfcac36bad753dc4046118c603ec )

Thanks.