Re: [PATCH] net: fix softnet_stat

[Changli Gao <xiaosuo@gmail.com>]

On Thu, Apr 15, 2010 at 11:28 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> I think this was nice, because we can see number of frames received by
> this cpu, before giving them to another cpu.
>
> Maybe we want a new counter ?
>
>        __get_cpu_var(netdev_rx_stat).inpackets++;
>
>
>>
>>       rps_lock(queue);
>>       if (queue->input_pkt_queue.qlen <= netdev_max_backlog) {
>> @@ -2366,9 +2365,9 @@ enqueue:
>>               goto enqueue;
>>       }
>>
>> +     per_cpu(netdev_rx_stat, cpu).dropped++;
>
> This is a bit expensive, and could be a queue->rx_stat.dropped++
> if netdev_rx_stat is moved inside queue structure.
>
>
>>       rps_unlock(queue);
>>
>> -     __get_cpu_var(netdev_rx_stat).dropped++;
>>       local_irq_restore(flags);
>>
>>       kfree_skb(skb);
>> @@ -3565,7 +3564,7 @@ static int softnet_seq_show(struct seq_file *seq, void *v)
>>       struct netif_rx_stats *s = v;
>>
>>       seq_printf(seq, "%08x %08x %08x %08x %08x %08x %08x %08x %08x %08x\n",
>> -                s->total, s->dropped, s->time_squeeze, 0,
>> +                s->total + s->dropped, s->dropped, s->time_squeeze, 0,
>
> This is wrong I believe... I prefer to add a new column, s->inpackets ?
>

the first column total should be the number of the packets, which are
processed by this CPU before RPS involved. If RPS is enabled, it
should keep the old meaning. If new statistics data is needed by RPS,
we would add it in another patch.


-- 
Regards，
Changli Gao(xiaosuo@gmail.com)