Re: [PATCH] ehea: Fixing LRO configuration

[Jesse Gross <jesse@nicira.com>]

On Wed, Dec 8, 2010 at 10:31 AM,  <leitao@linux.vnet.ibm.com> wrote:
> In order to set LRO on ehea, the user must set a module parameter, which
> is not the standard way to do so. This patch adds a way to set LRO using
> the ethtool tool.
>
> Signed-off-by: Breno Leitao <leitao@linux.vnet.ibm.com>
>
> diff --git a/drivers/net/ehea/ehea_ethtool.c b/drivers/net/ehea/ehea_ethtool.c
> index 75b099c..1f37ee6 100644
> --- a/drivers/net/ehea/ehea_ethtool.c
> +++ b/drivers/net/ehea/ehea_ethtool.c
> @@ -261,6 +261,13 @@ static void ehea_get_ethtool_stats(struct net_device *dev,
>
>  }
>
> +static int ehea_set_flags(struct net_device *dev, u32 data)
> +{
> +       return ethtool_op_set_flags(dev, data, ETH_FLAG_LRO
> +                                       | ETH_FLAG_TXVLAN
> +                                       | ETH_FLAG_RXVLAN);

I don't think that this should enable those vlan offloading flags.  I
don't see any logic to actually disable vlan stripping on receive if
that flag is toggled.  Transmit might be OK, since it will cause the
networking core to insert the tag in software if offloading is
disabled.  However, I see some places where skb->protocol is accessed
to determine the protocol of the packet being transmitted.  In most
drivers this causes a problem when transmit vlan offloading is
disabled because the packet type becomes ETH_P_8021Q, instead of the
expected protocol.  On the other hand, it appears that there aren't
any offloads in vlan_features, so maybe it doesn't matter.