Commit dbfc4fb7d578d4f224faa6b60deb40804dfdc1b1
Committed by
David S. Miller
David S. Miller
1 parent
c19be735c9
Exists in
ti-lsk-linux-4.1.y
and in
10 other branches
dst: no need to take reference on DST_NOCACHE dsts
Since commit f8864972126899 ("ipv4: fix dst race in sk_dst_get()") DST_NOCACHE dst_entries get freed by RCU. So there is no need to get a reference on them when we are in rcu protected sections. Cc: Eric Dumazet <edumazet@google.com> Cc: Julian Anastasov <ja@ssi.bg> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org> Reviewed-by: Julian Anastasov <ja@ssi.bg> Signed-off-by: David S. Miller <davem@davemloft.net>
Showing 3 changed files with 4 additions and 47 deletions Side-by-side Diff
include/linux/skbuff.h
| ... | ... | @@ -717,9 +717,6 @@ |
| 717 | 717 | skb->_skb_refdst = (unsigned long)dst; |
| 718 | 718 | } |
| 719 | 719 | |
| 720 | -void __skb_dst_set_noref(struct sk_buff *skb, struct dst_entry *dst, | |
| 721 | - bool force); | |
| 722 | - | |
| 723 | 720 | /** |
| 724 | 721 | * skb_dst_set_noref - sets skb dst, hopefully, without taking reference |
| 725 | 722 | * @skb: buffer |
| ... | ... | @@ -732,24 +729,8 @@ |
| 732 | 729 | */ |
| 733 | 730 | static inline void skb_dst_set_noref(struct sk_buff *skb, struct dst_entry *dst) |
| 734 | 731 | { |
| 735 | - __skb_dst_set_noref(skb, dst, false); | |
| 736 | -} | |
| 737 | - | |
| 738 | -/** | |
| 739 | - * skb_dst_set_noref_force - sets skb dst, without taking reference | |
| 740 | - * @skb: buffer | |
| 741 | - * @dst: dst entry | |
| 742 | - * | |
| 743 | - * Sets skb dst, assuming a reference was not taken on dst. | |
| 744 | - * No reference is taken and no dst_release will be called. While for | |
| 745 | - * cached dsts deferred reclaim is a basic feature, for entries that are | |
| 746 | - * not cached it is caller's job to guarantee that last dst_release for | |
| 747 | - * provided dst happens when nobody uses it, eg. after a RCU grace period. | |
| 748 | - */ | |
| 749 | -static inline void skb_dst_set_noref_force(struct sk_buff *skb, | |
| 750 | - struct dst_entry *dst) | |
| 751 | -{ | |
| 752 | - __skb_dst_set_noref(skb, dst, true); | |
| 732 | + WARN_ON(!rcu_read_lock_held() && !rcu_read_lock_bh_held()); | |
| 733 | + skb->_skb_refdst = (unsigned long)dst | SKB_DST_NOREF; | |
| 753 | 734 | } |
| 754 | 735 | |
| 755 | 736 | /** |
net/core/dst.c
| ... | ... | @@ -327,30 +327,6 @@ |
| 327 | 327 | } |
| 328 | 328 | EXPORT_SYMBOL(__dst_destroy_metrics_generic); |
| 329 | 329 | |
| 330 | -/** | |
| 331 | - * __skb_dst_set_noref - sets skb dst, without a reference | |
| 332 | - * @skb: buffer | |
| 333 | - * @dst: dst entry | |
| 334 | - * @force: if force is set, use noref version even for DST_NOCACHE entries | |
| 335 | - * | |
| 336 | - * Sets skb dst, assuming a reference was not taken on dst | |
| 337 | - * skb_dst_drop() should not dst_release() this dst | |
| 338 | - */ | |
| 339 | -void __skb_dst_set_noref(struct sk_buff *skb, struct dst_entry *dst, bool force) | |
| 340 | -{ | |
| 341 | - WARN_ON(!rcu_read_lock_held() && !rcu_read_lock_bh_held()); | |
| 342 | - /* If dst not in cache, we must take a reference, because | |
| 343 | - * dst_release() will destroy dst as soon as its refcount becomes zero | |
| 344 | - */ | |
| 345 | - if (unlikely((dst->flags & DST_NOCACHE) && !force)) { | |
| 346 | - dst_hold(dst); | |
| 347 | - skb_dst_set(skb, dst); | |
| 348 | - } else { | |
| 349 | - skb->_skb_refdst = (unsigned long)dst | SKB_DST_NOREF; | |
| 350 | - } | |
| 351 | -} | |
| 352 | -EXPORT_SYMBOL(__skb_dst_set_noref); | |
| 353 | - | |
| 354 | 330 | /* Dirty hack. We did it in 2.2 (in __dst_free), |
| 355 | 331 | * we have _very_ good reasons not to repeat |
| 356 | 332 | * this mistake in 2.3, but we have no choice |
net/netfilter/ipvs/ip_vs_xmit.c
| ... | ... | @@ -343,7 +343,7 @@ |
| 343 | 343 | skb_dst_drop(skb); |
| 344 | 344 | if (noref) { |
| 345 | 345 | if (!local) |
| 346 | - skb_dst_set_noref_force(skb, &rt->dst); | |
| 346 | + skb_dst_set_noref(skb, &rt->dst); | |
| 347 | 347 | else |
| 348 | 348 | skb_dst_set(skb, dst_clone(&rt->dst)); |
| 349 | 349 | } else |
| ... | ... | @@ -487,7 +487,7 @@ |
| 487 | 487 | skb_dst_drop(skb); |
| 488 | 488 | if (noref) { |
| 489 | 489 | if (!local) |
| 490 | - skb_dst_set_noref_force(skb, &rt->dst); | |
| 490 | + skb_dst_set_noref(skb, &rt->dst); | |
| 491 | 491 | else |
| 492 | 492 | skb_dst_set(skb, dst_clone(&rt->dst)); |
| 493 | 493 | } else |