Re: [PATCH 1/2 v2] mv643xx_eth: use sw csum for big packets

From: Lennert Buytenhek
Date: Sat Jun 12 2010 - 04:31:43 EST


On Thu, Jun 10, 2010 at 10:38:05AM +0300, Saeed Bishara wrote:

> >> diff --git a/drivers/net/mv643xx_eth.c b/drivers/net/mv643xx_eth.c
> >> index e345ec8..2d0e06b 100644
> >> --- a/drivers/net/mv643xx_eth.c
> >> +++ b/drivers/net/mv643xx_eth.c
> >> @@ -289,6 +289,7 @@ struct mv643xx_eth_shared_private {
> >> unsigned int t_clk;
> >> int extended_rx_coal_limit;
> >> int tx_bw_control;
> >> + int tx_csum_limit;
> >> };
> >>
> >> #define TX_BW_CONTROL_ABSENT 0
> >> @@ -782,7 +783,8 @@ static int txq_submit_skb(struct
> >tx_queue *txq, struct sk_buff *skb)
> >> skb->protocol != htons(ETH_P_8021Q));
> >>
> >> tag_bytes = (void *)ip_hdr(skb) - (void
> >*)skb->data - ETH_HLEN;
> >> - if (unlikely(tag_bytes & ~12)) {
> >> + if (unlikely(tag_bytes & ~12) ||
> >> + skb->len > mp->shared->tx_csum_limit) {
> >
> >Please line up skb->len with unlikely() on the line above it.
> >
> >
> >> @@ -2666,6 +2668,9 @@ static int
> >mv643xx_eth_shared_probe(struct platform_device *pdev)
> >> * Detect hardware parameters.
> >> */
> >> msp->t_clk = (pd != NULL && pd->t_clk != 0) ? pd->t_clk
> >: 133000000;
> >> + msp->tx_csum_limit = pd->tx_csum_limit ?
> >pd->tx_csum_limit : 9 * 1024;
> >> + /* 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
> >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"?

I.e.:

> >I.e. what happens if skb->len is 9 * 1024 + 15, but there is one VLAN
> >tag present -- will the hardware be able to do the checksum offload or
> >not?
--
To unsubscribe from this list: send the line "unsubscribe linux-net" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html