All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] gianfar: fix bug caused by 87c288c6e9aa31720b72e2bc2d665e24e1653c3e
@ 2011-07-26 10:03 Sebastian Pöhn
  2011-07-26 10:46 ` Jiri Pirko
  2011-07-28  5:43 ` David Miller
  0 siblings, 2 replies; 6+ messages in thread
From: Sebastian Pöhn @ 2011-07-26 10:03 UTC (permalink / raw)
  To: Linux Netdev, jpirko

commit 87c288c6e9aa31720b72e2bc2d665e24e1653c3e "gianfar: do vlan cleanup" has two issues:
# permutation of rx and tx flags
# enabling vlan tag insertion by default (this leads to unusable connections on some configurations)

If VLAN insertion is requested (via ethtool) it will be set at an other point ...

Signed-off-by: Sebastian Poehn <sebastian.poehn@belden.com>
---

 drivers/net/gianfar.c |    6 +-----
 1 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index 835cd25..2659daa 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -388,12 +388,8 @@ static void gfar_init_mac(struct net_device *ndev)
 	if (priv->hwts_rx_en)
 		rctrl |= RCTRL_PRSDEP_INIT | RCTRL_TS_ENABLE;
 
-	/* keep vlan related bits if it's enabled */
-	if (ndev->features & NETIF_F_HW_VLAN_TX)
-		rctrl |= RCTRL_VLEX | RCTRL_PRSDEP_INIT;
-
 	if (ndev->features & NETIF_F_HW_VLAN_RX)
-		tctrl |= TCTRL_VLINS;
+		rctrl |= RCTRL_VLEX | RCTRL_PRSDEP_INIT;
 
 	/* Init rctrl based on our settings */
 	gfar_write(&regs->rctrl, rctrl);



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

* Re: [PATCH] gianfar: fix bug caused by 87c288c6e9aa31720b72e2bc2d665e24e1653c3e
  2011-07-26 10:03 [PATCH] gianfar: fix bug caused by 87c288c6e9aa31720b72e2bc2d665e24e1653c3e Sebastian Pöhn
@ 2011-07-26 10:46 ` Jiri Pirko
  2011-07-26 11:13   ` Sebastian Pöhn
  2011-07-28  5:43 ` David Miller
  1 sibling, 1 reply; 6+ messages in thread
From: Jiri Pirko @ 2011-07-26 10:46 UTC (permalink / raw)
  To: Sebastian Pöhn; +Cc: Linux Netdev

Tue, Jul 26, 2011 at 12:03:13PM CEST, sebastian.belden@googlemail.com wrote:
>commit 87c288c6e9aa31720b72e2bc2d665e24e1653c3e "gianfar: do vlan cleanup" has two issues:
># permutation of rx and tx flags
># enabling vlan tag insertion by default (this leads to unusable connections on some configurations)

How so? What's causing that?

>
>If VLAN insertion is requested (via ethtool) it will be set at an other point ...
>
>Signed-off-by: Sebastian Poehn <sebastian.poehn@belden.com>
>---
>
> drivers/net/gianfar.c |    6 +-----
> 1 files changed, 1 insertions(+), 5 deletions(-)
>
>diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
>index 835cd25..2659daa 100644
>--- a/drivers/net/gianfar.c
>+++ b/drivers/net/gianfar.c
>@@ -388,12 +388,8 @@ static void gfar_init_mac(struct net_device *ndev)
> 	if (priv->hwts_rx_en)
> 		rctrl |= RCTRL_PRSDEP_INIT | RCTRL_TS_ENABLE;
> 
>-	/* keep vlan related bits if it's enabled */
>-	if (ndev->features & NETIF_F_HW_VLAN_TX)
>-		rctrl |= RCTRL_VLEX | RCTRL_PRSDEP_INIT;
>-
> 	if (ndev->features & NETIF_F_HW_VLAN_RX)
>-		tctrl |= TCTRL_VLINS;
>+		rctrl |= RCTRL_VLEX | RCTRL_PRSDEP_INIT;
> 
> 	/* Init rctrl based on our settings */
> 	gfar_write(&regs->rctrl, rctrl);
>
>

If you really need that to be done, you should remove NETIF_F_HW_VLAN_TX
from features (not hw_features) (never add it).


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

* Re: [PATCH] gianfar: fix bug caused by 87c288c6e9aa31720b72e2bc2d665e24e1653c3e
  2011-07-26 10:46 ` Jiri Pirko
@ 2011-07-26 11:13   ` Sebastian Pöhn
  2011-07-26 12:04     ` Jiri Pirko
  0 siblings, 1 reply; 6+ messages in thread
From: Sebastian Pöhn @ 2011-07-26 11:13 UTC (permalink / raw)
  To: Jiri Pirko, Linux Netdev

Am Dienstag, den 26.07.2011, 12:46 +0200 schrieb Jiri Pirko:
> Tue, Jul 26, 2011 at 12:03:13PM CEST, sebastian.belden@googlemail.com wrote:
> >commit 87c288c6e9aa31720b72e2bc2d665e24e1653c3e "gianfar: do vlan cleanup" has two issues:
> ># permutation of rx and tx flags
> ># enabling vlan tag insertion by default (this leads to unusable connections on some configurations)
> 
> How so? What's causing that?
If you enable the VLINS bit of txctrl and do not alter the vlan tag
configuration of the NIC, every packet will get a all zero vlan tag
(0x8100 0000). If you run a network without vlan awareness all packets
will be ignored.

In my configuration gianfar system <-> 3c59x system the 3com system
discards all packets received with the vlan tag.
> 
> >
> >If VLAN insertion is requested (via ethtool) it will be set at an other point ...
> >
> >Signed-off-by: Sebastian Poehn <sebastian.poehn@belden.com>
> >---
> >
> > drivers/net/gianfar.c |    6 +-----
> > 1 files changed, 1 insertions(+), 5 deletions(-)
> >
> >diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
> >index 835cd25..2659daa 100644
> >--- a/drivers/net/gianfar.c
> >+++ b/drivers/net/gianfar.c
> >@@ -388,12 +388,8 @@ static void gfar_init_mac(struct net_device *ndev)
> > 	if (priv->hwts_rx_en)
> > 		rctrl |= RCTRL_PRSDEP_INIT | RCTRL_TS_ENABLE;
> > 
> >-	/* keep vlan related bits if it's enabled */
> >-	if (ndev->features & NETIF_F_HW_VLAN_TX)
> >-		rctrl |= RCTRL_VLEX | RCTRL_PRSDEP_INIT;
> >-
> > 	if (ndev->features & NETIF_F_HW_VLAN_RX)
> >-		tctrl |= TCTRL_VLINS;
> >+		rctrl |= RCTRL_VLEX | RCTRL_PRSDEP_INIT;
> > 
> > 	/* Init rctrl based on our settings */
> > 	gfar_write(&regs->rctrl, rctrl);
> >
> >
> 
> If you really need that to be done, you should remove NETIF_F_HW_VLAN_TX
> from features (not hw_features) (never add it).
> 
The only thing I did is to let the vlan insertion disabled by default.
If someone wants it, it may be enabled via ethtool.



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

* Re: [PATCH] gianfar: fix bug caused by 87c288c6e9aa31720b72e2bc2d665e24e1653c3e
  2011-07-26 11:13   ` Sebastian Pöhn
@ 2011-07-26 12:04     ` Jiri Pirko
  2011-07-26 12:21       ` Sebastian Pöhn
  0 siblings, 1 reply; 6+ messages in thread
From: Jiri Pirko @ 2011-07-26 12:04 UTC (permalink / raw)
  To: Sebastian Pöhn; +Cc: Linux Netdev

Tue, Jul 26, 2011 at 01:13:41PM CEST, sebastian.belden@googlemail.com wrote:
>Am Dienstag, den 26.07.2011, 12:46 +0200 schrieb Jiri Pirko:
>> Tue, Jul 26, 2011 at 12:03:13PM CEST, sebastian.belden@googlemail.com wrote:
>> >commit 87c288c6e9aa31720b72e2bc2d665e24e1653c3e "gianfar: do vlan cleanup" has two issues:
>> ># permutation of rx and tx flags
>> ># enabling vlan tag insertion by default (this leads to unusable connections on some configurations)
>> 
>> How so? What's causing that?
>If you enable the VLINS bit of txctrl and do not alter the vlan tag
>configuration of the NIC, every packet will get a all zero vlan tag
>(0x8100 0000). If you run a network without vlan awareness all packets
>will be ignored.


Interesting hw... I would guess that if gfar_tx_vlan() is not called, no
tag would be put there...

>
>In my configuration gianfar system <-> 3c59x system the 3com system
>discards all packets received with the vlan tag.
>> 
>> >
>> >If VLAN insertion is requested (via ethtool) it will be set at an other point ...
>> >
>> >Signed-off-by: Sebastian Poehn <sebastian.poehn@belden.com>
>> >---
>> >
>> > drivers/net/gianfar.c |    6 +-----
>> > 1 files changed, 1 insertions(+), 5 deletions(-)
>> >
>> >diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
>> >index 835cd25..2659daa 100644
>> >--- a/drivers/net/gianfar.c
>> >+++ b/drivers/net/gianfar.c
>> >@@ -388,12 +388,8 @@ static void gfar_init_mac(struct net_device *ndev)
>> > 	if (priv->hwts_rx_en)
>> > 		rctrl |= RCTRL_PRSDEP_INIT | RCTRL_TS_ENABLE;
>> > 
>> >-	/* keep vlan related bits if it's enabled */
>> >-	if (ndev->features & NETIF_F_HW_VLAN_TX)
>> >-		rctrl |= RCTRL_VLEX | RCTRL_PRSDEP_INIT;
>> >-
>> > 	if (ndev->features & NETIF_F_HW_VLAN_RX)
>> >-		tctrl |= TCTRL_VLINS;
>> >+		rctrl |= RCTRL_VLEX | RCTRL_PRSDEP_INIT;
>> > 
>> > 	/* Init rctrl based on our settings */
>> > 	gfar_write(&regs->rctrl, rctrl);
>> >
>> >
>> 
>> If you really need that to be done, you should remove NETIF_F_HW_VLAN_TX
>> from features (not hw_features) (never add it).
>> 
>The only thing I did is to let the vlan insertion disabled by default.
>If someone wants it, it may be enabled via ethtool.
>
>

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

* Re: [PATCH] gianfar: fix bug caused by 87c288c6e9aa31720b72e2bc2d665e24e1653c3e
  2011-07-26 12:04     ` Jiri Pirko
@ 2011-07-26 12:21       ` Sebastian Pöhn
  0 siblings, 0 replies; 6+ messages in thread
From: Sebastian Pöhn @ 2011-07-26 12:21 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: Linux Netdev

Am Dienstag, den 26.07.2011, 14:04 +0200 schrieb Jiri Pirko:
> Tue, Jul 26, 2011 at 01:13:41PM CEST, sebastian.belden@googlemail.com wrote:
> >Am Dienstag, den 26.07.2011, 12:46 +0200 schrieb Jiri Pirko:
> >> Tue, Jul 26, 2011 at 12:03:13PM CEST, sebastian.belden@googlemail.com wrote:
> >> >commit 87c288c6e9aa31720b72e2bc2d665e24e1653c3e "gianfar: do vlan cleanup" has two issues:
> >> ># permutation of rx and tx flags
> >> ># enabling vlan tag insertion by default (this leads to unusable connections on some configurations)
> >> 
> >> How so? What's causing that?
> >If you enable the VLINS bit of txctrl and do not alter the vlan tag
> >configuration of the NIC, every packet will get a all zero vlan tag
> >(0x8100 0000). If you run a network without vlan awareness all packets
> >will be ignored.
> 
> 
> Interesting hw... I would guess that if gfar_tx_vlan() is not called, no
> tag would be put there...
The Freescale documentation says in detail:

if(FCB has valid VLAN field)
	send packet with this one
else
	send packet with default vlan tag

default vlan tag = dfvlan register = 0x8100 0000 by default
> 
> >
> >In my configuration gianfar system <-> 3c59x system the 3com system
> >discards all packets received with the vlan tag.
> >> 
> >> >
> >> >If VLAN insertion is requested (via ethtool) it will be set at an other point ...
> >> >
> >> >Signed-off-by: Sebastian Poehn <sebastian.poehn@belden.com>
> >> >---
> >> >
> >> > drivers/net/gianfar.c |    6 +-----
> >> > 1 files changed, 1 insertions(+), 5 deletions(-)
> >> >
> >> >diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
> >> >index 835cd25..2659daa 100644
> >> >--- a/drivers/net/gianfar.c
> >> >+++ b/drivers/net/gianfar.c
> >> >@@ -388,12 +388,8 @@ static void gfar_init_mac(struct net_device *ndev)
> >> > 	if (priv->hwts_rx_en)
> >> > 		rctrl |= RCTRL_PRSDEP_INIT | RCTRL_TS_ENABLE;
> >> > 
> >> >-	/* keep vlan related bits if it's enabled */
> >> >-	if (ndev->features & NETIF_F_HW_VLAN_TX)
> >> >-		rctrl |= RCTRL_VLEX | RCTRL_PRSDEP_INIT;
> >> >-
> >> > 	if (ndev->features & NETIF_F_HW_VLAN_RX)
> >> >-		tctrl |= TCTRL_VLINS;
> >> >+		rctrl |= RCTRL_VLEX | RCTRL_PRSDEP_INIT;
> >> > 
> >> > 	/* Init rctrl based on our settings */
> >> > 	gfar_write(&regs->rctrl, rctrl);
> >> >
> >> >
> >> 
> >> If you really need that to be done, you should remove NETIF_F_HW_VLAN_TX
> >> from features (not hw_features) (never add it).
> >> 
> >The only thing I did is to let the vlan insertion disabled by default.
> >If someone wants it, it may be enabled via ethtool.
> >
> >



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

* Re: [PATCH] gianfar: fix bug caused by 87c288c6e9aa31720b72e2bc2d665e24e1653c3e
  2011-07-26 10:03 [PATCH] gianfar: fix bug caused by 87c288c6e9aa31720b72e2bc2d665e24e1653c3e Sebastian Pöhn
  2011-07-26 10:46 ` Jiri Pirko
@ 2011-07-28  5:43 ` David Miller
  1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2011-07-28  5:43 UTC (permalink / raw)
  To: sebastian.belden; +Cc: netdev, jpirko

From: "Sebastian Pöhn" <sebastian.belden@googlemail.com>
Date: Tue, 26 Jul 2011 12:03:13 +0200

> commit 87c288c6e9aa31720b72e2bc2d665e24e1653c3e "gianfar: do vlan cleanup" has two issues:
> # permutation of rx and tx flags
> # enabling vlan tag insertion by default (this leads to unusable connections on some configurations)
> 
> If VLAN insertion is requested (via ethtool) it will be set at an other point ...
> 
> Signed-off-by: Sebastian Poehn <sebastian.poehn@belden.com>

Applied.

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

end of thread, other threads:[~2011-07-28  5:43 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-26 10:03 [PATCH] gianfar: fix bug caused by 87c288c6e9aa31720b72e2bc2d665e24e1653c3e Sebastian Pöhn
2011-07-26 10:46 ` Jiri Pirko
2011-07-26 11:13   ` Sebastian Pöhn
2011-07-26 12:04     ` Jiri Pirko
2011-07-26 12:21       ` Sebastian Pöhn
2011-07-28  5:43 ` David Miller

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.