On Mon, Jun 14, 2010 at 4:32 AM, Lennert Buytenhek
[off-list ref] wrote:

On Sun, Jun 13, 2010 at 10:38:10AM +0300, Saeed Bishara wrote:

quoted

quoted

quoted

quoted

quoted

@@ -2666,6 +2668,9 @@ static int

mv643xx_eth_shared_probe(struct platform_device *pdev)

quoted

? ? ? ? ?* Detect hardware parameters.
? ? ? ? ?*/
? ? ? ? msp->t_clk = (pd != NULL && pd->t_clk != 0) ? pd->t_clk

: 133000000;

quoted

+ ? ? ? msp->tx_csum_limit = pd->tx_csum_limit ?

pd->tx_csum_limit : 9 * 1024;

quoted

+ ? ? ? /* add header count so we can compare against skb->len */
+ ? ? ? msp->tx_csum_limit += ETH_HLEN;
? ? ? ? infer_hw_params(msp);

Is the limit 9 * 1024 + 14 for the whole packet, or 9 *

1024 for the IP

quoted

quoted

part?

the limit is for the IP part, but I thought that adding the

header length, then comparing agains skb->len will be the
same. What do you suggest?

Right, but for the header length you take 14, while if to include VLAN
tags or DSA tags the header might actually be longer -- how does that
affect the ability of the hardware to compute the checksum?

I.e. is the restriction "total packet length must be < N + 14 bytes"
or is it "the IP part must be < N bytes and it doesn't matter whether
there are VLAN tags or not"?

It's the second option: "the IP part must be < N" regardless to the L2 header.

Then I suppose you want to do something like:

? ? ? ?int hdr_len;
? ? ? ?int tag_bytes;

? ? ? ?hdr_len = (void *)ip_hdr(skb) - (void *)skb->data;
? ? ? ?tag_bytes = hdr_len - 4;

that should be 14 instead of 4, right?

? ? ? ?if (skb->len - hdr_len > the limit || unlikely(tag_bytes & ~12)) {

(And then keep the limit as maximum number of bytes in the packet
starting from the IP header part.)

ok. here is the updated patch: