Re: [PATCH net 3/3] ipv6: Fix dst_entry refcnt bugs in ip6_tunnel

On Tue, 2015-09-01 at 11:55 -0700, Martin KaFai Lau wrote:

Problems in the current dst_entry cache in the ip6_tunnel:

1. ip6_tnl_dst_set is racy.  There is no lock to protect it:
   - One major problem is that the dst refcnt gets messed up. F.e.
     the same dst_cache can be released multiple times and then
     triggering the infamous dst refcnt < 0 warning message.
   - Another issue is the inconsistency between dst_cache and
     dst_cookie.

   It can be reproduced by adding and removing the ip6gre tunnel
   while running a super_netperf TCP_CRR test.

2. In ip6_tnl_xmit2() and ip6gre_xmit2(), the outgoing skb does
   not hold a dst_entry's refcnt.

It should not be a problem. refcnt is taken when/if necessary (skb
queued on a qdisc for example)

We have other uses of skb_dst_set_noref()

Please describe the problem ?

This patch:
1. Create a percpu dst_entry cache in ip6_tnl

OK

2. Use a spinlock to protect the dst_cache operations

Well, a seqlock would be better : No need for an atomic operation in
fast path.

3. The outgoing skb always holds the dst_entry's refcnt

Not clear why it is needed. Inexpensive thanks to per cpu dst, but
still...