On Wed, 25 Nov 2020 07:10:43 +0800 wenxu wrote:
> 在 2020/11/25 3:24, Jakub Kicinski 写道:
> > On Fri, 20 Nov 2020 07:38:36 +0800 we...@ucloud.cn wrote:
> >> +int tcf_dev_queue_xmit(struct sk_buff *skb, int (*xmit)(struct sk_buff
> >> *skb))
> >> +{
> >> + xmit_hook_func *xmit_hook;
> >> +
> >> + xmit_hook = rcu_dereference(tcf_xmit_hook);
> >> + if (xmit_hook)
> >> + return xmit_hook(skb, xmit);
> >> + else
> >> + return xmit(skb);
> >> +}
> >> +EXPORT_SYMBOL_GPL(tcf_dev_queue_xmit);
> > I'm concerned about the performance impact of these indirect calls.
> >
> > Did you check what code compiler will generate? What the impact with
> > retpolines enabled is going to be?
> >
> > Now that sch_frag is no longer a module this could be simplified.
> >
> > First of all - xmit_hook can only be sch_frag_xmit_hook, so please use
> > that directly.
> >
> > if (READ_ONCE(tcf_xmit_hook_count))
> > sch_frag_xmit_hook(...
> > else
> > dev_queue_xmit(...
> >
> > The abstraction is costly and not necessary right now IMO.
> >
> > Then probably the counter should be:
> >
> > u32 __read_mostly tcf_xmit_hook_count;
> >
> > To avoid byte loads and having it be places in an unlucky cache line.
> Maybe a static key replace tcf_xmit_hook_count is more simplified？
>
> DEFINE_STATIC_KEY_FALSE(tcf_xmit_hook_in_use);
I wasn't sure if static key would work with the module (mirred being a
module) but thinking about it again, if tcf_dev_queue_xmit() is not an
static inline but a normal function, it should work. Sounds good!
