Re: [Cerowrt-devel] some kernel updates

[Jesper Dangaard Brouer <jbrouer@redhat.com>]

On Fri, 23 Aug 2013 14:37:47 +0200
Sebastian Moeller <moeller0@gmx.de> wrote:

> On Aug 23, 2013, at 13:16 , Jesper Dangaard Brouer <jbrouer@redhat.com> wrote:
> 
> > On Fri, 23 Aug 2013 12:15:12 +0200
> > Sebastian Moeller <moeller0@gmx.de> wrote:

[...]

> >>>>> Especially since the kernel already fudges
> >>>>> the packet size to account for the ethernet header and then some, so this
> >>>>> path should receive more scrutiny by virtue of having more users?
> >>> 
> >>> As you mention, the default kernel path (not tc stab) fudges the packet
> >>> size for Ethernet headers, AND I made a mistake (back in approx 2006,
> >>> sorry) that the "overhead" cannot be a negative number.  
> >> 
> >> 	Mmh, does this also apply to stab?
> > 
> > This seems to be two question...
> > 
> > Yes, the Ethernet header size gets adjusted/added before the "stab"
> > call.
> > For reference
> > See: net/core/dev.c function __dev_xmit_skb()
> > Call qdisc_pkt_len_init(skb); // adjust Ethernet and account for GSO
> > Call qdisc_calculate_pkt_len(skb, q); // is the stab call
> >  (ps calls __qdisc_calculate_pkt_len() in net/sched/sch_api.c)
> > 
> > The qdisc_pkt_len_init() call were introduced by Eric in
> > v3.9-rc1~139^2~411.
> 
> 	So I look at 3.10 here:
> 
> net/core/dev.c, qdisc_pkt_len_init
> line 2628: 	qdisc_skb_cb(skb)->pkt_len = skb->len;
> and in 
> line 2650: qdisc_skb_cb(skb)->pkt_len += (gso_segs - 1) * hdr_len;
> so the adjusted size does not seem to end in skb->len
> 
> 
> and then in 
> net/sched/sch_api.c, __qdisc_calculate_pkt_len
> line440: pkt_len = skb->len + stab->szopts.overhead;
> 
> So to my eyes this looks like stab is not honoring the changes made in qdisc_pkt_len_init, no? At least I fail to see where 
> skb->len is assigned qdisc_skb_cb(skb)->pkt_len
> But I happily admit that I am truly a novice in these matters and easily intimidated by C code.


You are absolutely correct, and I were wrong. Guess I didn't read
__qdisc_calculate_pkt_len() carefully enough, sorry.

When stab is enabled, it will override the skb pkt_len.


> > Thus, in kernels >= 3.9, you would need to change/reduce your tc
> > "overhead" parameter with -14 bytes (iif you accounted encapsulated
> > Ethernet header before)
> 
> 	That is what I thought before, but my kernel spelunking made
> me reconsider and switch to not subtract the 14 bytes since as I
> understand it the kernel actively does not do it if stab is used.

You are correct, and I was wrong.  Good to have more eyeballs on the
code :-)

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Sr. Network Kernel Developer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer