netdev - Re: [net-next PATCH 1/5] net: dsa: tag_rtl4

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200603135244.GA869823@lunn.ch>
Date:   Wed, 3 Jun 2020 15:52:44 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Linus Walleij <linus.walleij@...aro.org>
Cc:     Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        netdev@...r.kernel.org, DENG Qingfang <dqfext@...il.com>
Subject: Re: [net-next PATCH 1/5] net: dsa: tag_rtl4_a: Implement Realtek 4
 byte A tag

> +static struct sk_buff *rtl4a_tag_rcv(struct sk_buff *skb,
> +				     struct net_device *dev,
> +				     struct packet_type *pt)
> +{
> +	u16 protport;
> +	__be16 *p;
> +	u16 etype;
> +	u8 flags;
> +	u8 *tag;
> +	u8 prot;
> +	u8 port;
> +
> +	if (unlikely(!pskb_may_pull(skb, RTL4_A_HDR_LEN)))
> +		return NULL;
> +
> +	/* The RTL4 header has its own custom Ethertype 0x8899 and that
> +	 * starts right at the beginning of the packet, after the src
> +	 * ethernet addr. Apparantly skb->data always points 2 bytes in,
> +	 * behind the Ethertype.
> +	 */
> +	tag = skb->data - 2;
> +	p = (__be16 *)tag;
> +	etype = ntohs(*p);
> +	if (etype != RTL4_A_ETHERTYPE) {
> +		/* Not custom, just pass through */
> +		netdev_dbg(dev, "non-realtek ethertype 0x%04x\n", etype);
> +		return skb;
> +	}
> +	p = (__be16 *)(tag + 2);
> +	protport = ntohs(*p);
> +	/* The 4 upper bits are the protocol */
> +	prot = (protport >> RTL4_A_PROTOCOL_SHIFT) & 0x0f;
> +	if (prot != RTL4_A_PROTOCOL_RTL8366RB) {
> +		netdev_err(dev, "unknown realtek protocol 0x%01x\n", prot);
> +		return NULL;
> +	}
> +	netdev_dbg(dev, "realtek protocol 0x%02x\n", prot);
> +	port = protport & 0xff;
> +	netdev_dbg(dev, "realtek port origin 0x%02x\n", port);
> +
> +	/* Remove RTL4 tag and recalculate checksum */
> +	skb_pull_rcsum(skb, RTL4_A_HDR_LEN);
> +
> +	/* Move ethernet DA and SA in front of the data */
> +	memmove(skb->data - ETH_HLEN,
> +		skb->data - ETH_HLEN - RTL4_A_HDR_LEN,
> +		2 * ETH_ALEN);
> +
> +	skb->dev = dsa_master_find_slave(dev, 0, port);
> +	if (!skb->dev) {
> +		netdev_dbg(dev, "could not find slave for port %d\n", port);
> +		return NULL;
> +	}
> +	netdev_dbg(skb->dev, "forwarded packet to slave port %d\n", port);
> +
> +	skb->offload_fwd_mark = 1;
> +
> +	return skb;
> +}

Hi Linus

Do you think you are passed basic debug/reverse engineering? There are
a lot of netdev_dbg() statements here. It would be nice to remove most
of them, if you think the code is stable.

Is there any hint in OpenRRPC that tags can be used in the other
direction? Where is spanning tree performed? In the switch, or by the
host? That is one example where the host needs to be able to
send/receive frames on specific ports.

       Andrew