Patchwork [IGB,2.6.29.3,bug] Re: WARNING at dev_disable_lro when enabling ip_forward

login
register
mail settings
Submitter stephen hemminger
Date May 18, 2009, 10:57 p.m.
Message ID <20090518155714.0f1a3b20@nehalam>
Download mbox | patch
Permalink /patch/27380/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

stephen hemminger - May 18, 2009, 10:57 p.m.
Does this fix it?

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - May 18, 2009, 11 p.m.
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Mon, 18 May 2009 15:57:14 -0700

> Does this fix it?

It should, but I think the ifdef is probably superfluous.

> diff --git a/drivers/net/igb/igb_ethtool.c b/drivers/net/igb/igb_ethtool.c
> index 4bdfc2e..a425d85 100644
> --- a/drivers/net/igb/igb_ethtool.c
> +++ b/drivers/net/igb/igb_ethtool.c
> @@ -2028,6 +2028,10 @@ static struct ethtool_ops igb_ethtool_ops = {
>  	.get_ethtool_stats      = igb_get_ethtool_stats,
>  	.get_coalesce           = igb_get_coalesce,
>  	.set_coalesce           = igb_set_coalesce,
> +#ifdef CONFIG_IGB_LRO
> +	.get_flags		= ethtool_op_get_flags,
> +	.set_flags		= ethtool_op_set_flags,
> +#endif
>  };
>  
>  void igb_set_ethtool_ops(struct net_device *netdev)
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jeff Kirsher - May 18, 2009, 11:07 p.m.
On Mon, May 18, 2009 at 4:00 PM, David Miller <davem@davemloft.net> wrote:
> From: Stephen Hemminger <shemminger@vyatta.com>
> Date: Mon, 18 May 2009 15:57:14 -0700
>
>> Does this fix it?
>
> It should, but I think the ifdef is probably superfluous.
>

I agree with Dave, the #ifdef is not necessary.  Thanks Stephen for
the quick patch.

>> diff --git a/drivers/net/igb/igb_ethtool.c b/drivers/net/igb/igb_ethtool.c
>> index 4bdfc2e..a425d85 100644
>> --- a/drivers/net/igb/igb_ethtool.c
>> +++ b/drivers/net/igb/igb_ethtool.c
>> @@ -2028,6 +2028,10 @@ static struct ethtool_ops igb_ethtool_ops = {
>>       .get_ethtool_stats      = igb_get_ethtool_stats,
>>       .get_coalesce           = igb_get_coalesce,
>>       .set_coalesce           = igb_set_coalesce,
>> +#ifdef CONFIG_IGB_LRO
>> +     .get_flags              = ethtool_op_get_flags,
>> +     .set_flags              = ethtool_op_set_flags,
>> +#endif
>>  };
>>
>>  void igb_set_ethtool_ops(struct net_device *netdev)

If this resolves Sergey's issue, I will push this to the stable tree
(minus the #ifdef).
stephen hemminger - May 18, 2009, 11:08 p.m.
On Mon, 18 May 2009 16:00:29 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:

> From: Stephen Hemminger <shemminger@vyatta.com>
> Date: Mon, 18 May 2009 15:57:14 -0700
> 
> > Does this fix it?
> 
> It should, but I think the ifdef is probably superfluous.
> 
> > diff --git a/drivers/net/igb/igb_ethtool.c b/drivers/net/igb/igb_ethtool.c
> > index 4bdfc2e..a425d85 100644
> > --- a/drivers/net/igb/igb_ethtool.c
> > +++ b/drivers/net/igb/igb_ethtool.c
> > @@ -2028,6 +2028,10 @@ static struct ethtool_ops igb_ethtool_ops = {
> >  	.get_ethtool_stats      = igb_get_ethtool_stats,
> >  	.get_coalesce           = igb_get_coalesce,
> >  	.set_coalesce           = igb_set_coalesce,
> > +#ifdef CONFIG_IGB_LRO
> > +	.get_flags		= ethtool_op_get_flags,
> > +	.set_flags		= ethtool_op_set_flags,
> > +#endif
> >  };
> >  
> >  void igb_set_ethtool_ops(struct net_device *netdev)

For get, the ifdef is superflous, but for set you don't want
the user to turn LRO on if not configured.
David Miller - May 18, 2009, 11:41 p.m.
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Mon, 18 May 2009 16:08:04 -0700

> On Mon, 18 May 2009 16:00:29 -0700 (PDT)
> David Miller <davem@davemloft.net> wrote:
> 
>> > @@ -2028,6 +2028,10 @@ static struct ethtool_ops igb_ethtool_ops = {
>> >  	.get_ethtool_stats      = igb_get_ethtool_stats,
>> >  	.get_coalesce           = igb_get_coalesce,
>> >  	.set_coalesce           = igb_set_coalesce,
>> > +#ifdef CONFIG_IGB_LRO
>> > +	.get_flags		= ethtool_op_get_flags,
>> > +	.set_flags		= ethtool_op_set_flags,
>> > +#endif
>> >  };
>> >  
>> >  void igb_set_ethtool_ops(struct net_device *netdev)
> 
> For get, the ifdef is superflous, but for set you don't want
> the user to turn LRO on if not configured.

Stephen's right, ->set_flags has to be ifdef protected.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sergey Kononenko - May 19, 2009, 8:46 p.m.
> Does this fix it?
> 
> diff --git a/drivers/net/igb/igb_ethtool.c
> b/drivers/net/igb/igb_ethtool.c index 4bdfc2e..a425d85 100644
> --- a/drivers/net/igb/igb_ethtool.c
> +++ b/drivers/net/igb/igb_ethtool.c
> @@ -2028,6 +2028,10 @@ static struct ethtool_ops igb_ethtool_ops = {
>  	.get_ethtool_stats      = igb_get_ethtool_stats,
>  	.get_coalesce           = igb_get_coalesce,
>  	.set_coalesce           = igb_set_coalesce,
> +#ifdef CONFIG_IGB_LRO
> +	.get_flags		= ethtool_op_get_flags,
> +	.set_flags		= ethtool_op_set_flags,
> +#endif
>  };
Yes, patch have fixed the problem. Thank you.

With best regards,
Sergey Kononenko.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jeff Kirsher - May 19, 2009, 10:06 p.m.
On Tue, May 19, 2009 at 1:46 PM, Sergey Kononenko <sergk@sergk.org.ua> wrote:
>> Does this fix it?
>>
>> diff --git a/drivers/net/igb/igb_ethtool.c
>> b/drivers/net/igb/igb_ethtool.c index 4bdfc2e..a425d85 100644
>> --- a/drivers/net/igb/igb_ethtool.c
>> +++ b/drivers/net/igb/igb_ethtool.c
>> @@ -2028,6 +2028,10 @@ static struct ethtool_ops igb_ethtool_ops = {
>>       .get_ethtool_stats      = igb_get_ethtool_stats,
>>       .get_coalesce           = igb_get_coalesce,
>>       .set_coalesce           = igb_set_coalesce,
>> +#ifdef CONFIG_IGB_LRO
>> +     .get_flags              = ethtool_op_get_flags,
>> +     .set_flags              = ethtool_op_set_flags,
>> +#endif
>>  };
> Yes, patch have fixed the problem. Thank you.
>
> With best regards,
> Sergey Kononenko.
> --

Thanks Sergey and Stephen, I will get this queued up for the current
and stable trees.
David Miller - May 19, 2009, 10:14 p.m.
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Tue, 19 May 2009 15:06:57 -0700

> Thanks Sergey and Stephen, I will get this queued up for the current
> and stable trees.

current doesn't need it, since igb uses GRO now.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/net/igb/igb_ethtool.c b/drivers/net/igb/igb_ethtool.c
index 4bdfc2e..a425d85 100644
--- a/drivers/net/igb/igb_ethtool.c
+++ b/drivers/net/igb/igb_ethtool.c
@@ -2028,6 +2028,10 @@  static struct ethtool_ops igb_ethtool_ops = {
 	.get_ethtool_stats      = igb_get_ethtool_stats,
 	.get_coalesce           = igb_get_coalesce,
 	.set_coalesce           = igb_set_coalesce,
+#ifdef CONFIG_IGB_LRO
+	.get_flags		= ethtool_op_get_flags,
+	.set_flags		= ethtool_op_set_flags,
+#endif
 };
 
 void igb_set_ethtool_ops(struct net_device *netdev)