Re: gianfar.c: Unwanted VLAN tagging on TX frames
On Tuesday 25 August 2009 at 01:32:18, Andy Fleming wrote: Hmmmhow have you tested this? This looks like it has a bad race condition. The TCTRL register applies to all packets, which means if you send a packet with VLAN tags, followed by one without, or visa versa, there's a reasonable chance that the second packet's VLAN tags (or lack thereof) will take precedence. Without speaking for the company, I suspect that this is just how the eTSEC works with VLAN -- all, or nothing. Andy Hi Andy, I have tested it by sending a single ping to a station within the VLAN followed by a ping to a station thats not in a VLAN. OK, thats not really a significant test, because I did not send a VLAN tagged frame immediately followed by one without a tag. You are right, this code can enable/disable VLAN tagging before the previous packet is processed. Torsten ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
gianfar.c: Unwanted VLAN tagging on TX frames
Hello everyone,
I have the Freescale's MPC8313erdb eval board and run the latest stable linux
kernel version (linux-2.6.30.5).
After creating a VLAN device (e.g. eth0.2) a VLAN tag is also inserted into
frames that don't relate to a VLAN device. This is the case for frames that
are directly sent through a standard ethernet interface (e.g. eth0).
When creating a VLAN device the gianfar driver enables the hardware supported
VLAN tagging on TX frames. This is done by setting the VLINS bit of the TCTRL
register inside the function gianfar_vlan_rx_register().
The problem is that every outgoing frame will be tagged. For frames from an
interface like eth0 the VLN bit of the FCB isn't set. Therefore the eTSEC uses
the content of the Default VLAN control word register (DFVLAN) to tag the
frame. As long as this register will not be modified after a reset of the
controller the outgoing frames will be tagged with VID = 0 (priority tagged
frames).
The following patch solves this problem.
diff -uprN linux-2.6.30.5_orig//drivers/net/gianfar.c
linux-2.6.30.5/drivers/net/gianfar.c
--- linux-2.6.30.5_orig//drivers/net/gianfar.c 2009-08-16 23:19:38.0
+0200
+++ linux-2.6.30.5/drivers/net/gianfar.c2009-08-22 10:38:28.0
+0200
@@ -1309,6 +1309,7 @@ static int gfar_start_xmit(struct sk_buf
u32 bufaddr;
unsigned long flags;
unsigned int nr_frags, length;
+ u32 tempval;
base = priv-tx_bd_base;
@@ -1385,13 +1386,30 @@ static int gfar_start_xmit(struct sk_buf
gfar_tx_checksum(skb, fcb);
}
- if (priv-vlgrp vlan_tx_tag_present(skb)) {
- if (unlikely(NULL == fcb)) {
- fcb = gfar_add_fcb(skb);
- lstatus |= BD_LFLAG(TXBD_TOE);
- }
+ if (priv-vlgrp) {
+ if (vlan_tx_tag_present(skb)) {
+ if (unlikely(NULL == fcb)) {
+ fcb = gfar_add_fcb(skb);
+ lstatus |= BD_LFLAG(TXBD_TOE);
+ }
+
+ /* Enable VLAN tag insertion for frames from VLAN
devices */
+ tempval = gfar_read(priv-regs-tctrl);
+ if ( !(tempval TCTRL_VLINS) ) {
+ tempval |= TCTRL_VLINS;
+ gfar_write(priv-regs-tctrl, tempval);
+ }
- gfar_tx_vlan(skb, fcb);
+ gfar_tx_vlan(skb, fcb);
+ }
+ else {
+ /* Disable VLAN tag insertion for frames that are not
from a VLAN device */
+ tempval = gfar_read(priv-regs-tctrl);
+ if ( tempval TCTRL_VLINS ) {
+ tempval = ~TCTRL_VLINS;
+ gfar_write(priv-regs-tctrl, tempval);
+ }
+ }
}
/* setup the TxBD length and buffer pointer for the first BD */
@@ -1484,23 +1502,11 @@ static void gfar_vlan_rx_register(struct
priv-vlgrp = grp;
if (grp) {
- /* Enable VLAN tag insertion */
- tempval = gfar_read(priv-regs-tctrl);
- tempval |= TCTRL_VLINS;
-
- gfar_write(priv-regs-tctrl, tempval);
-
/* Enable VLAN tag extraction */
tempval = gfar_read(priv-regs-rctrl);
- tempval |= RCTRL_VLEX;
tempval |= (RCTRL_VLEX | RCTRL_PRSDEP_INIT);
gfar_write(priv-regs-rctrl, tempval);
} else {
- /* Disable VLAN tag insertion */
- tempval = gfar_read(priv-regs-tctrl);
- tempval = ~TCTRL_VLINS;
- gfar_write(priv-regs-tctrl, tempval);
-
/* Disable VLAN tag extraction */
tempval = gfar_read(priv-regs-rctrl);
tempval = ~RCTRL_VLEX;
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: gianfar.c: Unwanted VLAN tagging on TX frames
On Mon, Aug 24, 2009 at 11:10 AM, Torsten Fleischer
to-fleisc...@t-online.de wrote:
Hello everyone,
I have the Freescale's MPC8313erdb eval board and run the latest stable
linux
kernel version (linux-2.6.30.5).
After creating a VLAN device (e.g. eth0.2) a VLAN tag is also inserted into
frames that don't relate to a VLAN device. This is the case for frames that
are directly sent through a standard ethernet interface (e.g. eth0).
When creating a VLAN device the gianfar driver enables the hardware
supported
VLAN tagging on TX frames. This is done by setting the VLINS bit of the
TCTRL
register inside the function gianfar_vlan_rx_register().
The problem is that every outgoing frame will be tagged. For frames from
an
interface like eth0 the VLN bit of the FCB isn't set. Therefore the eTSEC
uses
the content of the Default VLAN control word register (DFVLAN) to tag the
frame. As long as this register will not be modified after a reset of the
controller the outgoing frames will be tagged with VID = 0 (priority tagged
frames).
The following patch solves this problem.
diff -uprN linux-2.6.30.5_orig//drivers/net/gianfar.c
linux-2.6.30.5/drivers/net/gianfar.c
--- linux-2.6.30.5_orig//drivers/net/gianfar.c 2009-08-16
23:19:38.0 +0200
+++ linux-2.6.30.5/drivers/net/gianfar.c2009-08-22
10:38:28.0 +0200
@@ -1309,6 +1309,7 @@ static int gfar_start_xmit(struct sk_buf
u32 bufaddr;
unsigned long flags;
unsigned int nr_frags, length;
+ u32 tempval;
base = priv-tx_bd_base;
@@ -1385,13 +1386,30 @@ static int gfar_start_xmit(struct sk_buf
gfar_tx_checksum(skb, fcb);
}
- if (priv-vlgrp vlan_tx_tag_present(skb)) {
- if (unlikely(NULL == fcb)) {
- fcb = gfar_add_fcb(skb);
- lstatus |= BD_LFLAG(TXBD_TOE);
- }
+ if (priv-vlgrp) {
+ if (vlan_tx_tag_present(skb)) {
+ if (unlikely(NULL == fcb)) {
+ fcb = gfar_add_fcb(skb);
+ lstatus |= BD_LFLAG(TXBD_TOE);
+ }
+
+ /* Enable VLAN tag insertion for frames from VLAN
devices */
+ tempval = gfar_read(priv-regs-tctrl);
+ if ( !(tempval TCTRL_VLINS) ) {
+ tempval |= TCTRL_VLINS;
+ gfar_write(priv-regs-tctrl, tempval);
+ }
- gfar_tx_vlan(skb, fcb);
+ gfar_tx_vlan(skb, fcb);
+ }
+ else {
+ /* Disable VLAN tag insertion for frames that are
not from a VLAN device */
+ tempval = gfar_read(priv-regs-tctrl);
+ if ( tempval TCTRL_VLINS ) {
+ tempval = ~TCTRL_VLINS;
+ gfar_write(priv-regs-tctrl, tempval);
+ }
+ }
}
Hmmmhow have you tested this? This looks like it has a bad race
condition. The TCTRL register applies to all packets, which means if you
send a packet with VLAN tags, followed by one without, or visa versa,
there's a reasonable chance that the second packet's VLAN tags (or lack
thereof) will take precedence.
Without speaking for the company, I suspect that this is just how the eTSEC
works with VLAN -- all, or nothing.
Andy
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
