Re: gianfar.c: Unwanted VLAN tagging on TX frames
From: Andy Fleming <hidden>
Date: 2009-08-24 23:32:21
On Mon, Aug 24, 2009 at 11:10 AM, Torsten Fleischer < to-fleischer@t-online.de> wrote:
quoted hunk ↗ jump to hunk
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-1623:19:38.000000000 +0200+++ linux-2.6.30.5/drivers/net/gianfar.c 2009-08-2210:38:28.000000000 +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 VLANdevices */ + 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); + } + } }
Hmmm....how 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