On Mon, Jul 31, 2017 at 03:35:02PM -0700, Cong Wang wrote: > On Mon, Jul 31, 2017 at 3:19 PM, Shaohua Li <s...@kernel.org> wrote: > > static inline __be32 ip6_make_flowlabel(struct net *net, struct sk_buff > > *skb, > > __be32 flowlabel, bool autolabel, > > - struct flowi6 *fl6) > > + struct flowi6 *fl6, u32 hash) > > { > > - u32 hash; > > - > > /* @flowlabel may include more than a flow label, eg, the traffic > > class. > > * Here we want only the flow label value. > > */ > > @@ -788,7 +786,8 @@ static inline __be32 ip6_make_flowlabel(struct net > > *net, struct sk_buff *skb, > > net->ipv6.sysctl.auto_flowlabels != > > IP6_AUTO_FLOW_LABEL_FORCED)) > > return flowlabel; > > > > - hash = skb_get_hash_flowi6(skb, fl6); > > + if (skb) > > + hash = skb_get_hash_flowi6(skb, fl6); > > > Why not just move skb_get_hash_flowi6() to its caller? > This check is not necessary. If you don't want to touch > existing callers, you can just introduce a wrapper: > > > static inline __be32 ip6_make_flowlabel(struct net *net, struct sk_buff *skb, > __be32 flowlabel, bool autolabel, > struct flowi6 *fl6) > { > u32 hash = skb_get_hash_flowi6(skb, fl6); > return __ip6_make_flowlabel(net, flowlabel, autolabel, hash); > }
this will always call skb_get_hash_flowi6 for the fast path even auto flowlabel is disabled. I thought we should avoid this. > > And your code can just call: > > __ip6_make_flowlabel(net, flowlabel, autolabel, sk->sk_txhash);