netdev - Re: [PATCH] tbf scheduler: TSO support

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: <20070512.174930.80276554.taka@valinux.co.jp>
Date:	Sat, 12 May 2007 17:49:30 +0900 (JST)
From:	Hirokazu Takahashi <taka@...inux.co.jp>
To:	kaber@...sh.net
Cc:	davem@...emloft.net, linux-net@...r.kernel.org,
	netdev@...r.kernel.org
Subject: Re: [PATCH] tbf scheduler: TSO support

Hi,

> > I think the concept of TBF is quit good but the userspace tools have
> > become old that it doesn't fit to Gb ethernet environment.
> > The tools should be updated to care about much faster network and
> > GbE jumbo frames. I agree with you at this point.
> > 
> > On the other hand, handling TSO packet should be a kernel issue.
> > A TSO packet is logically just a multiple segmented packet
> > including several ordinary packets. This feature should be kept
> > invisible from userspace.
> 
> 
> Putting aside this question, this cannot work properly without userspace
> since the rate table is only built for the given MTU. Your patch tries
> to work around that by summing up the result of L2T in q->max_size
> steps, which gives incorrect results with MPU or overhead settings.

You have a point. sch->dev->mtu should be used instead of q->max_size,
which gives precisely correct results. Every TSO packet will be split
into several packets with sch->dev->mtu length, which administrators
specify. The splitting is automatically done by TSO supporting controllers.

To minimize the overhead of the calculation I'd re-implement it like:

   segs = skb->len/sch->dev->mtu;
   rest = skb->len - sch->dev->mtu*segs;
   toks -= L2T(q, sch->dev->mtu) * segs;
   if (rest)
        toks -= L2T(q, rest);

I think the problem is that sch->dev->mtu might not be set correctly,
so some error checking is needed.

> I think we can only do two things without userspace support:
> 
> - disable TSO (not a good idea)

This is just what I'm doing now as a workaround.

> - split the skb into q->max_size chunks on dequeue

It sure is difficult because the splitting deeply depends on the
protocol stack which made the skb and it will make some overhead,
which is why TSO and other offloading features are introduced.

I'm now thinking I can make it just hold a TSO packet until p->tokens
reaches the size of the packet. I think it is straightforward
implementation. I'll try this.

> The later would allow people to take full advantage of TSO with properly
> configured TBF, but it would still at least work with a too small mtu
> value.

Thanks,
Hirokazu Takahashi.

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