[PATCH] realtek: fix VLAN 0 tag at egress on port 0
Arinc UNAL (Xeront)
arinc.unal at xeront.com
Mon Jun 13 04:32:33 PDT 2022
Hey Birger,
On 09/06/2022 14:30, Birger Koblitz wrote:
> Hi Arinc,
>
> very well spotted! If I could make a suggestion, the RTL93xx tag generation
> functions have the opposite problem, i.e. rtl930x_decode_tag() and
> rtl931x_decode_tag() do not do the check for the destination port being >= 0,
> i.e. defined and the packet not being a broadcast packet.
>
> So I would suggest to remove the
> if (dest_port >= 0) {
> in rtl838x_create_tx_header and rtl839x_create_tx_header() entirely
> and do the check for all 4 SoC families directly in rtl838x_eth_tx():
>
> if (dest_port >= 0)
> priv->r->create_tx_header(h, dest_port, skb->priority >> 1);
>
> this will fix all 4 cases.
Thanks for the suggestion. If I understand this correctly, it should be
something like this?
---
diff --git
a/target/linux/realtek/files-5.10/drivers/net/ethernet/rtl838x_eth.c
b/target/linux/realtek/files-5.10/drivers/net/ethernet/rtl838x_eth.c
index cf6aabc614..f3b7c994c3 100644
--- a/target/linux/realtek/files-5.10/drivers/net/ethernet/rtl838x_eth.c
+++ b/target/linux/realtek/files-5.10/drivers/net/ethernet/rtl838x_eth.c
@@ -96,42 +96,38 @@ static void rtl838x_create_tx_header(struct p_hdr
*h, int dest_port, int prio)
{
prio &= 0x7;
- if (dest_port > 0) {
- // cpu_tag[0] is reserved on the RTL83XX SoCs
- h->cpu_tag[1] = 0x0401; // BIT 10: RTL8380_CPU_TAG, BIT0: L2LEARNING on
- h->cpu_tag[2] = 0x0200; // Set only AS_DPM, to enable DPM settings below
- h->cpu_tag[3] = 0x0000;
- h->cpu_tag[4] = BIT(dest_port) >> 16;
- h->cpu_tag[5] = BIT(dest_port) & 0xffff;
- // Set internal priority and AS_PRIO
- if (prio >= 0)
- h->cpu_tag[2] |= (prio | 0x8) << 12;
- }
+ // cpu_tag[0] is reserved on the RTL83XX SoCs
+ h->cpu_tag[1] = 0x0401; // BIT 10: RTL8380_CPU_TAG, BIT0: L2LEARNING on
+ h->cpu_tag[2] = 0x0200; // Set only AS_DPM, to enable DPM settings below
+ h->cpu_tag[3] = 0x0000;
+ h->cpu_tag[4] = BIT(dest_port) >> 16;
+ h->cpu_tag[5] = BIT(dest_port) & 0xffff;
+ // Set internal priority and AS_PRIO
+ if (prio >= 0)
+ h->cpu_tag[2] |= (prio | 0x8) << 12;
}
static void rtl839x_create_tx_header(struct p_hdr *h, int dest_port,
int prio)
{
prio &= 0x7;
- if (dest_port > 0) {
- // cpu_tag[0] is reserved on the RTL83XX SoCs
- h->cpu_tag[1] = 0x0100; // RTL8390_CPU_TAG marker
- h->cpu_tag[2] = h->cpu_tag[3] = h->cpu_tag[4] = h->cpu_tag[5] = 0;
- // h->cpu_tag[1] |= BIT(1) | BIT(0); // Bypass filter 1/2
- if (dest_port >= 32) {
- dest_port -= 32;
- h->cpu_tag[2] = BIT(dest_port) >> 16;
- h->cpu_tag[3] = BIT(dest_port) & 0xffff;
- } else {
- h->cpu_tag[4] = BIT(dest_port) >> 16;
- h->cpu_tag[5] = BIT(dest_port) & 0xffff;
- }
- h->cpu_tag[2] |= BIT(5); // Enable destination port mask use
- h->cpu_tag[2] |= BIT(8); // Enable L2 Learning
- // Set internal priority and AS_PRIO
- if (prio >= 0)
- h->cpu_tag[1] |= prio | BIT(3);
+ // cpu_tag[0] is reserved on the RTL83XX SoCs
+ h->cpu_tag[1] = 0x0100; // RTL8390_CPU_TAG marker
+ h->cpu_tag[2] = h->cpu_tag[3] = h->cpu_tag[4] = h->cpu_tag[5] = 0;
+ // h->cpu_tag[1] |= BIT(1) | BIT(0); // Bypass filter 1/2
+ if (dest_port >= 32) {
+ dest_port -= 32;
+ h->cpu_tag[2] = BIT(dest_port) >> 16;
+ h->cpu_tag[3] = BIT(dest_port) & 0xffff;
+ } else {
+ h->cpu_tag[4] = BIT(dest_port) >> 16;
+ h->cpu_tag[5] = BIT(dest_port) & 0xffff;
}
+ h->cpu_tag[2] |= BIT(5); // Enable destination port mask use
+ h->cpu_tag[2] |= BIT(8); // Enable L2 Learning
+ // Set internal priority and AS_PRIO
+ if (prio >= 0)
+ h->cpu_tag[1] |= prio | BIT(3);
}
static void rtl930x_create_tx_header(struct p_hdr *h, int dest_port,
int prio)
@@ -1135,6 +1131,9 @@ static int rtl838x_eth_tx(struct sk_buff *skb,
struct net_device *dev)
int dest_port = -1;
int q = skb_get_queue_mapping(skb) % TXRINGS;
+ if (dest_port >= 0)
+ priv->r->create_tx_header(h, dest_port, skb->priority >> 1);
+
if (q) // Check for high prio queue
pr_debug("SKB priority: %d\n", skb->priority);
@@ -1142,7 +1141,7 @@ static int rtl838x_eth_tx(struct sk_buff *skb,
struct net_device *dev)
len = skb->len;
/* Check for DSA tagging at the end of the buffer */
- if (netdev_uses_dsa(dev) && skb->data[len-4] == 0x80 &&
skb->data[len-3] > 0
+ if (netdev_uses_dsa(dev) && skb->data[len-4] == 0x80 &&
skb->data[len-3] >= 0
&& skb->data[len-3] < priv->cpu_port && skb->data[len-2] == 0x10
&& skb->data[len-1] == 0x00) {
/* Reuse tag space for CRC if possible */
Arınç
>
> Cheers,
> Birger
>
> On 09.06.22 10:32, Arinc UNAL (Xeront) wrote:
>> There is a bug on the ethernet driver where the checks for the DSA tag and
>> creating the tx header don't include port 0. This causes any frame
>> egressing from the first port of the switch to have a VLAN 0 tag.
>>
>> Fix this by extending the checks to include port 0.
>>
>> Signed-off-by: Arınç ÜNAL <arinc.unal at xeront.com>
>> ---
>> .../realtek/files-5.10/drivers/net/ethernet/rtl838x_eth.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/target/linux/realtek/files-5.10/drivers/net/ethernet/rtl838x_eth.c b/target/linux/realtek/files-5.10/drivers/net/ethernet/rtl838x_eth.c
>> index cf6aabc614..88a27e78ab 100644
>> --- a/target/linux/realtek/files-5.10/drivers/net/ethernet/rtl838x_eth.c
>> +++ b/target/linux/realtek/files-5.10/drivers/net/ethernet/rtl838x_eth.c
>> @@ -96,7 +96,7 @@ static void rtl838x_create_tx_header(struct p_hdr *h, int dest_port, int prio)
>> {
>> prio &= 0x7;
>>
>> - if (dest_port > 0) {
>> + if (dest_port >= 0) {
>> // cpu_tag[0] is reserved on the RTL83XX SoCs
>> h->cpu_tag[1] = 0x0401; // BIT 10: RTL8380_CPU_TAG, BIT0: L2LEARNING on
>> h->cpu_tag[2] = 0x0200; // Set only AS_DPM, to enable DPM settings below
>> @@ -113,7 +113,7 @@ static void rtl839x_create_tx_header(struct p_hdr *h, int dest_port, int prio)
>> {
>> prio &= 0x7;
>>
>> - if (dest_port > 0) {
>> + if (dest_port >= 0) {
>> // cpu_tag[0] is reserved on the RTL83XX SoCs
>> h->cpu_tag[1] = 0x0100; // RTL8390_CPU_TAG marker
>> h->cpu_tag[2] = h->cpu_tag[3] = h->cpu_tag[4] = h->cpu_tag[5] = 0;
>> @@ -1142,7 +1142,7 @@ static int rtl838x_eth_tx(struct sk_buff *skb, struct net_device *dev)
>> len = skb->len;
>>
>> /* Check for DSA tagging at the end of the buffer */
>> - if (netdev_uses_dsa(dev) && skb->data[len-4] == 0x80 && skb->data[len-3] > 0
>> + if (netdev_uses_dsa(dev) && skb->data[len-4] == 0x80 && skb->data[len-3] >= 0
>> && skb->data[len-3] < priv->cpu_port && skb->data[len-2] == 0x10
>> && skb->data[len-1] == 0x00) {
>> /* Reuse tag space for CRC if possible */
More information about the openwrt-devel
mailing list