| Message ID | 1305853864-2135-2-git-send-email-mcarlson@broadcom.com |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Miller |
| Headers | show |
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 > -- 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
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. -- 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
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 -- 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
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 > -- 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
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;
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(-)