On Thu, Nov 3, 2016 at 10:31 PM, Eric Dumazet <eric.duma...@gmail.com> wrote: > On Thu, 2016-11-03 at 21:39 +0800, Gao Feng wrote: >> Hi Eric, >> >> On Thu, Nov 3, 2016 at 9:30 PM, Eric Dumazet <eric.duma...@gmail.com> wrote: >> > On Thu, 2016-11-03 at 21:03 +0800, f...@ikuai8.com wrote: >> >> From: Gao Feng <f...@ikuai8.com> >> >> >> >> The dropped count of veth is located in struct veth_priv, but other >> >> statistics like packets and bytes are in another struct pcpu_vstats. >> >> Now keep these three counters in the same struct. >> >> >> >> Signed-off-by: Gao Feng <f...@ikuai8.com> >> >> --- >> >> v2: Use right "peer" instead of "dev"; >> >> v1: Initial version >> > >> > May I ask : Why ? >> >> Just because I think statistics should be in the same struct. > > That is not a good reason then.
Because other net devices put the statistics together. Take tun/tap as example, it is a virtual device, but its all statistics are percpu including dropped. Regards Feng > >> >> > >> > We did that because there was no point making per-cpu requirements >> > bigger, for a counter that is hardly ever updated. >> > >> > Do you have a real case where performance dropping packets in a driver >> > is needed ? >> >> No, I haven't met the performance issue now. > > OK then kill this patch. > >> >> > >> > At some point we will have to stop dumb percpu explosion, when we have >> > 128+ cores per host. Folding all these percpu counters is taking a lot >> > of time too. >> > >> > >> > >> Ok, I get it. It is designed specially to put the dropped counter as >> atomic counter, not percpu. >> But I have one question that when put the counters as percpu, and when not? > > Because the regular fast path needs to be fast ? > > Try to _use_ veth without these percpu stats and be prepared to be > shocked. > > >
