Hi Arinc,
yes, this would be what I suggest.
Cheers,
Birger
On 13.06.22 13:32, Arinc UNAL (Xeront) wrote:
> 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.u...@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 */
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
