[PATCH] realtek: correct egress frame port verification
Arinc UNAL (Xeront)
arinc.unal at xeront.com
Sun Jun 19 02:45:06 PDT 2022
On 19.06.2022 11:56, Sander Vanheule wrote:
> Destination switch ports for outgoing frame can range from 0 to
> CPU_PORT-1, and frame priorities should also always be positive.
>
> Refactor the code to only generate egress frame CPU headers when the a
> valid destination port number is available, and make the code a bit more
> consistent between different switch generations. Change the dest_port
> and prio argument types from 'int' to 'unsigned int', since only
> positive values are valid.
>
> This fixes the issue where egress frames on switch port 0 did not
> receive a VLAN tag, because they are sent out without a CPU header.
> Also fixes a potential issue with invalid (negative) egress port numbers
> on RTL93xx switches.
>
> Reported-by: Arınç ÜNAL <arinc.unal at xeront.com>
> Reported-by: Birger Koblitz <mail at birger-koblitz.de>
> Signed-off-by: Sander Vanheule <sander at svanheule.net>
Thanks for doing this!
Acked-by: Arınç ÜNAL <arinc.unal at xeront.com>
> ---
> .../drivers/net/ethernet/rtl838x_eth.c | 86 +++++++++----------
> .../drivers/net/ethernet/rtl838x_eth.h | 2 +-
> 2 files changed, 40 insertions(+), 48 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 cf6aabc6142f..241a5787f820 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
> @@ -92,54 +92,44 @@ struct notify_b {
> u32 reserved2[8];
> };
>
> -static void rtl838x_create_tx_header(struct p_hdr *h, int dest_port, int prio)
> +static void rtl838x_create_tx_header(struct p_hdr *h, unsigned int dest_port, unsigned 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
> + h->cpu_tag[2] |= ((prio & 0x7) | BIT(3)) << 12;
> }
>
> -static void rtl839x_create_tx_header(struct p_hdr *h, int dest_port, int prio)
> +static void rtl839x_create_tx_header(struct p_hdr *h, unsigned int dest_port, unsigned 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
> + h->cpu_tag[1] |= (prio & 0x7) | BIT(3);
> }
>
> -static void rtl930x_create_tx_header(struct p_hdr *h, int dest_port, int prio)
> +static void rtl930x_create_tx_header(struct p_hdr *h, unsigned int dest_port, unsigned int prio)
> {
> h->cpu_tag[0] = 0x8000; // CPU tag marker
> - h->cpu_tag[1] = h->cpu_tag[2] = 0;
> - if (prio >= 0)
> - h->cpu_tag[2] = BIT(13) | prio << 8; // Enable and set Priority Queue
> + h->cpu_tag[1] = 0;
> + /* Enable (AS_QID) and set Priority Queue (QID) */
> + h->cpu_tag[2] = (BIT(5) | (prio & 0x1f)) << 8;
> h->cpu_tag[3] = 0;
> h->cpu_tag[4] = 0;
> h->cpu_tag[5] = 0;
> @@ -147,12 +137,12 @@ static void rtl930x_create_tx_header(struct p_hdr *h, int dest_port, int prio)
> h->cpu_tag[7] = BIT(dest_port) & 0xffff;
> }
>
> -static void rtl931x_create_tx_header(struct p_hdr *h, int dest_port, int prio)
> +static void rtl931x_create_tx_header(struct p_hdr *h, unsigned int dest_port, unsigned int prio)
> {
> h->cpu_tag[0] = 0x8000; // CPU tag marker
> - h->cpu_tag[1] = h->cpu_tag[2] = 0;
> - if (prio >= 0)
> - h->cpu_tag[2] = BIT(13) | prio << 8; // Enable and set Priority Queue
> + h->cpu_tag[1] = 0;
> + /* Enable (AS_QID) and set Priority Queue (QID) */
> + h->cpu_tag[2] = (BIT(5) | (prio & 0x1f)) << 8;
> h->cpu_tag[3] = 0;
> h->cpu_tag[4] = h->cpu_tag[5] = h->cpu_tag[6] = h->cpu_tag[7] = 0;
> if (dest_port >= 32) {
> @@ -1142,9 +1132,10 @@ 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
> - && skb->data[len-3] < priv->cpu_port && skb->data[len-2] == 0x10
> - && skb->data[len-1] == 0x00) {
> + if (netdev_uses_dsa(dev) && skb->data[len-4] == 0x80
> + && skb->data[len-3] < priv->cpu_port
> + && skb->data[len-2] == 0x10
> + && skb->data[len-1] == 0x00) {
> /* Reuse tag space for CRC if possible */
> dest_port = skb->data[len-3];
> skb->data[len-4] = skb->data[len-3] = skb->data[len-2] = skb->data[len-1] = 0x00;
> @@ -1171,7 +1162,8 @@ static int rtl838x_eth_tx(struct sk_buff *skb, struct net_device *dev)
> h->len -= 4;
> }
>
> - priv->r->create_tx_header(h, dest_port, skb->priority >> 1);
> + if (dest_port >= 0)
> + priv->r->create_tx_header(h, dest_port, skb->priority >> 1);
>
> /* Copy packet data to tx buffer */
> memcpy((void *)KSEG1ADDR(h->buf), skb->data, len);
> diff --git a/target/linux/realtek/files-5.10/drivers/net/ethernet/rtl838x_eth.h b/target/linux/realtek/files-5.10/drivers/net/ethernet/rtl838x_eth.h
> index 2d1f80dc9d2c..3e633558351d 100644
> --- a/target/linux/realtek/files-5.10/drivers/net/ethernet/rtl838x_eth.h
> +++ b/target/linux/realtek/files-5.10/drivers/net/ethernet/rtl838x_eth.h
> @@ -436,7 +436,7 @@ struct rtl838x_eth_reg {
> int mac;
> int l2_tbl_flush_ctrl;
> void (*update_cntr)(int r, int work_done);
> - void (*create_tx_header)(struct p_hdr *h, int dest_port, int prio);
> + void (*create_tx_header)(struct p_hdr *h, unsigned int dest_port, unsigned int prio);
> bool (*decode_tag)(struct p_hdr *h, struct dsa_tag *tag);
> };
>
More information about the openwrt-devel
mailing list