netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 1/2] tg3: Fix NETIF_F_LOOPBACK error
@ 2011-05-20  1:11 Matt Carlson
  2011-05-20  1:15 ` Mahesh Bandewar
  0 siblings, 1 reply; 5+ messages in thread
From: Matt Carlson @ 2011-05-20  1:11 UTC (permalink / raw)
  To: davem; +Cc: netdev, mcarlson

Mahesh Bandewar noticed that the features cleanup in commit
0da0606f493c5cdab74bdcc96b12f4305ad94085, entitled
"tg3: Consolidate all netdev feature assignments", mistakenly sets
NETIF_F_LOOPBACK by default.  This patch corrects the error.

Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
---
 drivers/net/tg3.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 012ce70..0b78c5d 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -15080,6 +15080,8 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
 			features |= NETIF_F_TSO_ECN;
 	}
 
+	dev->features |= features;
+
 	/*
 	 * Add loopback capability only for a subset of devices that support
 	 * MAC-LOOPBACK. Eventually this need to be enhanced to allow INT-PHY
@@ -15090,7 +15092,6 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
 		/* Add the loopback capability */
 		features |= NETIF_F_LOOPBACK;
 
-	dev->features |= features;
 	dev->hw_features |= features;
 	dev->vlan_features |= features;
 
-- 
1.7.3.4



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next 1/2] tg3: Fix NETIF_F_LOOPBACK error
  2011-05-20  1:11 [PATCH net-next 1/2] tg3: Fix NETIF_F_LOOPBACK error Matt Carlson
@ 2011-05-20  1:15 ` Mahesh Bandewar
  2011-05-20  1:59   ` Matt Carlson
  0 siblings, 1 reply; 5+ messages in thread
From: Mahesh Bandewar @ 2011-05-20  1:15 UTC (permalink / raw)
  To: Matt Carlson; +Cc: David Miller, linux-netdev

On Thu, May 19, 2011 at 6:11 PM, Matt Carlson <mcarlson@broadcom.com> wrote:
> Mahesh Bandewar noticed that the features cleanup in commit
> 0da0606f493c5cdab74bdcc96b12f4305ad94085, entitled
> "tg3: Consolidate all netdev feature assignments", mistakenly sets
> NETIF_F_LOOPBACK by default.  This patch corrects the error.
>
> Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
> ---
>  drivers/net/tg3.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
> index 012ce70..0b78c5d 100644
> --- a/drivers/net/tg3.c
> +++ b/drivers/net/tg3.c
> @@ -15080,6 +15080,8 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
>                        features |= NETIF_F_TSO_ECN;
>        }
>
> +       dev->features |= features;
> +
>        /*
>         * Add loopback capability only for a subset of devices that support
>         * MAC-LOOPBACK. Eventually this need to be enhanced to allow INT-PHY
> @@ -15090,7 +15092,6 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
>                /* Add the loopback capability */
>                features |= NETIF_F_LOOPBACK;
>
> -       dev->features |= features;
>        dev->hw_features |= features;
>        dev->vlan_features |= features;
I think this line should go up too. Otherwise newly created vlan
device(s) will have spurious loopback bit set.
>
> --
> 1.7.3.4
>
>
> --
> 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
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next 1/2] tg3: Fix NETIF_F_LOOPBACK error
  2011-05-20  1:15 ` Mahesh Bandewar
@ 2011-05-20  1:59   ` Matt Carlson
  2011-05-20  6:57     ` Michał Mirosław
  0 siblings, 1 reply; 5+ messages in thread
From: Matt Carlson @ 2011-05-20  1:59 UTC (permalink / raw)
  To: Mahesh Bandewar; +Cc: Matthew Carlson, David Miller, linux-netdev

On Thu, May 19, 2011 at 06:15:18PM -0700, Mahesh Bandewar wrote:
> On Thu, May 19, 2011 at 6:11 PM, Matt Carlson <mcarlson@broadcom.com> wrote:
> > Mahesh Bandewar noticed that the features cleanup in commit
> > 0da0606f493c5cdab74bdcc96b12f4305ad94085, entitled
> > "tg3: Consolidate all netdev feature assignments", mistakenly sets
> > NETIF_F_LOOPBACK by default. ?This patch corrects the error.
> >
> > Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
> > ---
> > ?drivers/net/tg3.c | ? ?3 ++-
> > ?1 files changed, 2 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
> > index 012ce70..0b78c5d 100644
> > --- a/drivers/net/tg3.c
> > +++ b/drivers/net/tg3.c
> > @@ -15080,6 +15080,8 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
> > ? ? ? ? ? ? ? ? ? ? ? ?features |= NETIF_F_TSO_ECN;
> > ? ? ? ?}
> >
> > + ? ? ? dev->features |= features;
> > +
> > ? ? ? ?/*
> > ? ? ? ? * Add loopback capability only for a subset of devices that support
> > ? ? ? ? * MAC-LOOPBACK. Eventually this need to be enhanced to allow INT-PHY
> > @@ -15090,7 +15092,6 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
> > ? ? ? ? ? ? ? ?/* Add the loopback capability */
> > ? ? ? ? ? ? ? ?features |= NETIF_F_LOOPBACK;
> >
> > - ? ? ? dev->features |= features;
> > ? ? ? ?dev->hw_features |= features;
> > ? ? ? ?dev->vlan_features |= features;
> I think this line should go up too. Otherwise newly created vlan
> device(s) will have spurious loopback bit set.

Yes.  You are right.  I thought vlan_features functioned like
hw_features.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next 1/2] tg3: Fix NETIF_F_LOOPBACK error
  2011-05-20  1:59   ` Matt Carlson
@ 2011-05-20  6:57     ` Michał Mirosław
  2011-05-20 17:56       ` Mahesh Bandewar
  0 siblings, 1 reply; 5+ messages in thread
From: Michał Mirosław @ 2011-05-20  6:57 UTC (permalink / raw)
  To: Matt Carlson; +Cc: Mahesh Bandewar, David Miller, linux-netdev

2011/5/20 Matt Carlson <mcarlson@broadcom.com>:
> On Thu, May 19, 2011 at 06:15:18PM -0700, Mahesh Bandewar wrote:
>> On Thu, May 19, 2011 at 6:11 PM, Matt Carlson <mcarlson@broadcom.com> wrote:
>> > Mahesh Bandewar noticed that the features cleanup in commit
>> > 0da0606f493c5cdab74bdcc96b12f4305ad94085, entitled
>> > "tg3: Consolidate all netdev feature assignments", mistakenly sets
>> > NETIF_F_LOOPBACK by default. ?This patch corrects the error.
>> >
>> > Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
>> > ---
>> > ?drivers/net/tg3.c | ? ?3 ++-
>> > ?1 files changed, 2 insertions(+), 1 deletions(-)
>> >
>> > diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
>> > index 012ce70..0b78c5d 100644
>> > --- a/drivers/net/tg3.c
>> > +++ b/drivers/net/tg3.c
>> > @@ -15080,6 +15080,8 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
>> > ? ? ? ? ? ? ? ? ? ? ? ?features |= NETIF_F_TSO_ECN;
>> > ? ? ? ?}
>> >
>> > + ? ? ? dev->features |= features;
>> > +
>> > ? ? ? ?/*
>> > ? ? ? ? * Add loopback capability only for a subset of devices that support
>> > ? ? ? ? * MAC-LOOPBACK. Eventually this need to be enhanced to allow INT-PHY
>> > @@ -15090,7 +15092,6 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
>> > ? ? ? ? ? ? ? ?/* Add the loopback capability */
>> > ? ? ? ? ? ? ? ?features |= NETIF_F_LOOPBACK;
>> >
>> > - ? ? ? dev->features |= features;
>> > ? ? ? ?dev->hw_features |= features;
>> > ? ? ? ?dev->vlan_features |= features;
>> I think this line should go up too. Otherwise newly created vlan
>> device(s) will have spurious loopback bit set.
> Yes.  You are right.  I thought vlan_features functioned like
> hw_features.

Probably NETIF_F_LOOPBACK should be forcibly set on VLAN devices when
underlying device has it enabled. Just a quick thought for discussion.

Best Regards,
Michał Mirosław

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next 1/2] tg3: Fix NETIF_F_LOOPBACK error
  2011-05-20  6:57     ` Michał Mirosław
@ 2011-05-20 17:56       ` Mahesh Bandewar
  0 siblings, 0 replies; 5+ messages in thread
From: Mahesh Bandewar @ 2011-05-20 17:56 UTC (permalink / raw)
  To: Michał Mirosław; +Cc: Matt Carlson, David Miller, linux-netdev

On Thu, May 19, 2011 at 11:57 PM, Michał Mirosław <mirqus@gmail.com> wrote:
> 2011/5/20 Matt Carlson <mcarlson@broadcom.com>:
>> On Thu, May 19, 2011 at 06:15:18PM -0700, Mahesh Bandewar wrote:
>>> On Thu, May 19, 2011 at 6:11 PM, Matt Carlson <mcarlson@broadcom.com> wrote:
>>> > Mahesh Bandewar noticed that the features cleanup in commit
>>> > 0da0606f493c5cdab74bdcc96b12f4305ad94085, entitled
>>> > "tg3: Consolidate all netdev feature assignments", mistakenly sets
>>> > NETIF_F_LOOPBACK by default. ?This patch corrects the error.
>>> >
>>> > Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
>>> > ---
>>> > ?drivers/net/tg3.c | ? ?3 ++-
>>> > ?1 files changed, 2 insertions(+), 1 deletions(-)
>>> >
>>> > diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
>>> > index 012ce70..0b78c5d 100644
>>> > --- a/drivers/net/tg3.c
>>> > +++ b/drivers/net/tg3.c
>>> > @@ -15080,6 +15080,8 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
>>> > ? ? ? ? ? ? ? ? ? ? ? ?features |= NETIF_F_TSO_ECN;
>>> > ? ? ? ?}
>>> >
>>> > + ? ? ? dev->features |= features;
>>> > +
>>> > ? ? ? ?/*
>>> > ? ? ? ? * Add loopback capability only for a subset of devices that support
>>> > ? ? ? ? * MAC-LOOPBACK. Eventually this need to be enhanced to allow INT-PHY
>>> > @@ -15090,7 +15092,6 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
>>> > ? ? ? ? ? ? ? ?/* Add the loopback capability */
>>> > ? ? ? ? ? ? ? ?features |= NETIF_F_LOOPBACK;
>>> >
>>> > - ? ? ? dev->features |= features;
>>> > ? ? ? ?dev->hw_features |= features;
>>> > ? ? ? ?dev->vlan_features |= features;
>>> I think this line should go up too. Otherwise newly created vlan
>>> device(s) will have spurious loopback bit set.
>> Yes.  You are right.  I thought vlan_features functioned like
>> hw_features.
>
> Probably NETIF_F_LOOPBACK should be forcibly set on VLAN devices when
> underlying device has it enabled. Just a quick thought for discussion.
>
I think that's a good idea for the sake of correctness and should be
done when we enable / disable loopback.

--mahesh..

> Best Regards,
> Michał Mirosław
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2011-05-20 17:56 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-05-20  1:11 [PATCH net-next 1/2] tg3: Fix NETIF_F_LOOPBACK error Matt Carlson
2011-05-20  1:15 ` Mahesh Bandewar
2011-05-20  1:59   ` Matt Carlson
2011-05-20  6:57     ` Michał Mirosław
2011-05-20 17:56       ` Mahesh Bandewar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).