All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ben Hutchings <bhutchings@solarflare.com>
To: Stanislaw Gruszka <sgruszka@redhat.com>
Cc: Amit Salecha <amit.salecha@qlogic.com>,
	Eric Dumazet <eric.dumazet@gmail.com>,
	Jesper Dangaard Brouer <hawk@comx.dk>,
	Alexander Duyck <alexander.h.duyck@intel.com>,
	netdev <netdev@vger.kernel.org>,
	Neil Horman <nhorman@tuxdriver.com>
Subject: Re: LRO disable warnings on kernel 2.6.38
Date: Mon, 21 Mar 2011 13:52:23 +0000	[thread overview]
Message-ID: <1300715543.26693.349.camel@localhost> (raw)
In-Reply-To: <20110321134647.GA3126@redhat.com>

On Mon, 2011-03-21 at 14:46 +0100, Stanislaw Gruszka wrote:
> On Mon, Mar 21, 2011 at 01:34:06PM +0100, Stanislaw Gruszka wrote:
[...]
> > I'm afraid some other drivers can have similar problem after
> > adding ETH_FLAG_{TX,RX}VLAN. I think we need to distinguish features
> > that are configurable at runtime from features that are hardcoded.
> > I'm going to look at that.
> 
> Other drivers have this bug too. I'm going to prepare patch with similar fix
> like Amit proposed, but also for other drivers. Something like below:

This may be useful temporarily, but the new netdev-features API seems to
make this easier to get right.  We should be moving drivers over to that
instead.

Ben.

> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index c1a71bb..38fd0cb 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -141,9 +141,17 @@ u32 ethtool_op_get_flags(struct net_device *dev)
>  }
>  EXPORT_SYMBOL(ethtool_op_get_flags);
>  
> +bool ethtool_invalid_flags(struct net_device *dev, u32 data, u32 supported)
> +{
> +	if ((dev->features & ~supported) != (data & ~supported))
> +		return true;
> +	else
> +		return false;
> +}
> +
>  int ethtool_op_set_flags(struct net_device *dev, u32 data, u32 supported)
>  {
> -	if (data & ~supported)
> +	if (ethtool_invalid_flags(dev, data, supported);
>  		return -EINVAL;
>  
>  	dev->features = ((dev->features & ~flags_dup_features) |
> diff --git a/drivers/net/vmxnet3/vmxnet3_ethtool.c b/drivers/net/vmxnet3/vmxnet3_ethtool.c
> index 81254be..7a662f1 100644
> --- a/drivers/net/vmxnet3/vmxnet3_ethtool.c
> +++ b/drivers/net/vmxnet3/vmxnet3_ethtool.c
> @@ -304,8 +304,8 @@ vmxnet3_set_flags(struct net_device *netdev, u32 data)
>  	u8 lro_present = (netdev->features & NETIF_F_LRO) == 0 ? 0 : 1;
>  	unsigned long flags;
>  
> -	if (data & ~ETH_FLAG_LRO)
> -		return -EOPNOTSUPP;
> +	if (ethtool_invalid_flags(netdev, data, ETH_FLAG_LRO))
> +		return -EINVAL;
>  
>  	if (lro_requested ^ lro_present) {
>  		/* toggle the LRO feature*/

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


  reply	other threads:[~2011-03-21 13:53 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-18 11:12 LRO disable warnings on kernel 2.6.38 Jesper Dangaard Brouer
2011-03-18 13:05 ` Eric Dumazet
2011-03-18 14:17   ` Ben Hutchings
2011-03-18 14:33     ` Eric Dumazet
2011-03-18 15:15       ` Stephen Hemminger
2011-03-18 19:52         ` David Miller
2011-03-18 19:58           ` Ben Hutchings
2011-03-18 19:59             ` David Miller
2011-03-18 15:40 ` Ben Hutchings
2011-03-18 16:18 ` Alexander Duyck
2011-03-21  9:21   ` Jesper Dangaard Brouer
2011-03-21  9:45     ` Eric Dumazet
2011-03-21  9:53       ` Eric Dumazet
2011-03-21 10:04         ` Jesper Dangaard Brouer
2011-03-21 10:11           ` Eric Dumazet
2011-03-21 10:27             ` Jesper Dangaard Brouer
2011-03-21 10:00       ` Amit Salecha
2011-03-21 12:34         ` Stanislaw Gruszka
2011-03-21 13:46           ` Stanislaw Gruszka
2011-03-21 13:52             ` Ben Hutchings [this message]
2011-03-21 15:10               ` [RFC] net: fix ethtool->set_flags not intended -EINVAL return value Stanislaw Gruszka
2011-03-21 15:15                 ` Ben Hutchings
2011-03-22  2:41                 ` Jesse Gross

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1300715543.26693.349.camel@localhost \
    --to=bhutchings@solarflare.com \
    --cc=alexander.h.duyck@intel.com \
    --cc=amit.salecha@qlogic.com \
    --cc=eric.dumazet@gmail.com \
    --cc=hawk@comx.dk \
    --cc=netdev@vger.kernel.org \
    --cc=nhorman@tuxdriver.com \
    --cc=sgruszka@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.