netdev - Re: [PATCH net-next v3 4/5] net/tc: introduce TC_ACT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6a9fd3c8f861c203c7e12a2a2e477796c5e093d5.camel@redhat.com>
Date:   Wed, 25 Jul 2018 10:29:13 +0200
From:   Paolo Abeni <pabeni@...hat.com>
To:     Cong Wang <xiyou.wangcong@...il.com>,
        Jamal Hadi Salim <jhs@...atatu.com>
Cc:     Linux Kernel Network Developers <netdev@...r.kernel.org>,
        Jiri Pirko <jiri@...nulli.us>,
        Daniel Borkmann <daniel@...earbox.net>,
        Marcelo Ricardo Leitner <marcelo.leitner@...il.com>,
        Eyal Birger <eyal.birger@...il.com>,
        David Miller <davem@...emloft.net>
Subject: Re: [PATCH net-next v3 4/5] net/tc: introduce TC_ACT_REINJECT.

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@...il.com> wrote:
> > 
> > On Tue, Jul 24, 2018 at 1:07 PM Paolo Abeni <pabeni@...hat.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@...atatu.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