Re: [PATCH 1/2] mv643xx_eth: disable csum offloading if the hw limited by max size

From: Lennert Buytenhek
Date: Tue Jun 08 2010 - 13:26:29 EST


On Sun, Jun 06, 2010 at 03:28:55PM +0300, Saeed Bishara wrote:

> diff --git a/drivers/net/mv643xx_eth.c b/drivers/net/mv643xx_eth.c
> index e345ec8..79243dd 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
> @@ -2468,6 +2469,14 @@ static int mv643xx_eth_change_mtu(struct net_device *dev, int new_mtu)
> mv643xx_eth_recalc_skb_size(mp);
> tx_set_rate(mp, 1000000000, 16777216);
>
> + if (mp->shared->tx_csum_limit && dev->mtu > mp->shared->tx_csum_limit) {
> + dev->features &= ~NETIF_F_IP_CSUM;
> + dev->vlan_features &= ~NETIF_F_IP_CSUM;
> + } else {
> + dev->features |= NETIF_F_IP_CSUM;
> + dev->vlan_features |= NETIF_F_IP_CSUM;
> + }

I wonder if it would be better to still advertise NETIF_F_IP_CSUM in
this case, but just call skb_checksum_help() if skb->len is > dev->mtu
in txq_submit_skb(), like we do for the case where the ethernet header
is of an unsupported length -- that way, we'd still have hardware
checksum offload for TX packets that _do_ fit inside the hardware TX
fifo, e.g. when communicating with hosts on another subnet where the
default MTU is used.


> diff --git a/include/linux/mv643xx_eth.h b/include/linux/mv643xx_eth.h
> index cbbbe9b..7402718 100644
> --- a/include/linux/mv643xx_eth.h
> +++ b/include/linux/mv643xx_eth.h
> @@ -19,6 +19,10 @@ struct mv643xx_eth_shared_platform_data {
> struct mbus_dram_target_info *dram;
> struct platform_device *shared_smi;
> unsigned int t_clk;
> + /*
> + * Max packet size for Tx IP/Layer 4 checksum (0 -> no limit)

There's always a limit. :-) You might as well just fill in the right
values for orion5x/kw/dd/loki while you're at it.


thanks,
Lennert
--
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