diff mbox

net_sched: factorize qdisc stats handling

Message ID Pine.LNX.4.64.1101172243470.15896@ask.diku.dk
State RFC, archived
Delegated to: David Miller
Headers show

Commit Message

Jesper Dangaard Brouer Jan. 17, 2011, 9:55 p.m. UTC 
On Sat, 8 Jan 2011, Eric Dumazet wrote:
>> Changli Gao <xiaosuo@gmail.com> wrote:
>>
>>>> +       cl->bstats.packets += skb_is_gso(skb)?skb_shinfo(skb)->gso_segs:1;

What about the qlen when we enqueue a GSO packet? Should we account for 
the number of internal GSO packets, or just account a GSO packet as a 
single packet?


Is is at all legal to modify the q.qlen this way?

Cheers
   Jesper Brouer

--
-------------------------------------------------------------------
MSc. Master of Computer Science
Dept. of Computer Science, University of Copenhagen
Author of http://www.adsl-optimizer.dk
-------------------------------------------------------------------
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Eric Dumazet Jan. 17, 2011, 10:04 p.m. UTC | #1 
Le lundi 17 janvier 2011 à 22:55 +0100, Jesper Dangaard Brouer a écrit :
> 
> On Sat, 8 Jan 2011, Eric Dumazet wrote:
> >> Changli Gao <xiaosuo@gmail.com> wrote:
> >>
> >>>> +       cl->bstats.packets += skb_is_gso(skb)?skb_shinfo(skb)->gso_segs:1;
> 
> What about the qlen when we enqueue a GSO packet? Should we account for 
> the number of internal GSO packets, or just account a GSO packet as a 
> single packet?
> 
> diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
> index 0918834..1a8c0a3 100644
> --- a/net/sched/sch_generic.c
> +++ b/net/sched/sch_generic.c
> @@ -453,7 +453,8 @@ static int pfifo_fast_enqueue(struct sk_buff *skb, 
> struct Qdisc* qdisc)
>                  struct sk_buff_head *list = band2list(priv, band);
> 
>                  priv->bitmap |= (1 << band);
> -               qdisc->q.qlen++;
> +               /* Should the number of GSO packets be taken into account?*/
> +               qdisc->q.qlen += skb_is_gso(skb)?skb_shinfo(skb)->gso_segs:1;
>                  return __qdisc_enqueue_tail(skb, qdisc, list);
>          }
> 
> Is is at all legal to modify the q.qlen this way?

nope ;)

q.qlen is really number of skbs here.



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 0918834..1a8c0a3 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -453,7 +453,8 @@  static int pfifo_fast_enqueue(struct sk_buff *skb, 
struct Qdisc* qdisc)
                 struct sk_buff_head *list = band2list(priv, band);

                 priv->bitmap |= (1 << band);
-               qdisc->q.qlen++;
+               /* Should the number of GSO packets be taken into account?*/
+               qdisc->q.qlen += skb_is_gso(skb)?skb_shinfo(skb)->gso_segs:1;
                 return __qdisc_enqueue_tail(skb, qdisc, list);
         }