Re: [PATCH net-next v3 4/5] net/tc: introduce TC_ACT_REINJECT.

[Paolo Abeni <pabeni@redhat.com>]

On Tue, 2018-07-24 at 13:50 -0700, Cong Wang wrote:
> On Tue, Jul 24, 2018 at 1:38 PM Cong Wang <xiyou.wangcong@gmail.com> wrote:
> > 
> > On Tue, Jul 24, 2018 at 1:07 PM Paolo Abeni <pabeni@redhat.com> wrote:
> > > +static inline void skb_tc_reinject(struct sk_buff *skb, struct tcf_result *res)
> > > +{
> > > +       struct gnet_stats_queue *stats = res->qstats;
> > > +       int ret;
> > > +
> > > +       if (res->ingress)
> > > +               ret = netif_receive_skb(skb);
> > > +       else
> > > +               ret = dev_queue_xmit(skb);
> > > +       if (ret && stats)
> > > +               qstats_overlimit_inc(res->qstats);
> > 
> > Why increasing overlimit? Overlimit is typically increased
> > by traffic shapers to indicate there is no bandwidth to send
> > out the packet.
> > 
> > I fail to understand why overlimit is increased in your case
> > here. I guess you want to increase 'drops' instead.
> 
> Hmm, actually the current mirred code increases overlimit too.
> But I still don't think it makes sense.

Yep, I chose to increment 'overlimits' to preserve the current mirred
semantic. 

AFAICS, that was first introduced with:

commit 8919bc13e8d92c5b082c5c0321567383a071f5bc
Author: Jamal Hadi Salim <jhs@mojatatu.com>
Date:   Mon Aug 15 05:25:40 2011 +0000

    net_sched: fix port mirror/redirect stats reporting

Likely increasing 'drops' would be "better", but I'm unsure we can
change this established behavior without affecting some user.

Anyway, I'm fine either way. Please advice.

Cheers,

Paolo