| 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
| ||
|
Date: Tue, 12 Oct 2021 12:56:11 +0000
From: Alvin Šipraga <ALSI@...g-olufsen.dk>
To: Vladimir Oltean <olteanv@...il.com>,
Alvin Šipraga <alvin@...s.dk>
CC: Linus Walleij <linus.walleij@...aro.org>,
Andrew Lunn <andrew@...n.ch>,
Vivien Didelot <vivien.didelot@...il.com>,
Florian Fainelli <f.fainelli@...il.com>,
"David S. Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>,
Rob Herring <robh+dt@...nel.org>,
Heiner Kallweit <hkallweit1@...il.com>,
Russell King <linux@...linux.org.uk>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next 4/6] net: dsa: tag_rtl8_4: add realtek 8 byte
protocol 4 tag
On 10/12/21 2:50 PM, Vladimir Oltean wrote:
> On Tue, Oct 12, 2021 at 02:35:53PM +0200, Alvin Šipraga wrote:
>> From: Alvin Šipraga <alsi@...g-olufsen.dk>
>>
>> This commit implements a basic version of the 8 byte tag protocol used
>> in the Realtek RTL8365MB-VC unmanaged switch, which carries with it a
>> protocol version of 0x04.
>>
>> The implementation itself only handles the parsing of the EtherType
>> value and Realtek protocol version, together with the source or
>> destination port fields. The rest is left unimplemented for now.
>>
>> The tag format is described in a confidential document provided to my
>> company by Realtek Semiconductor Corp. Permission has been granted by
>> the vendor to publish this driver based on that material, together with
>> an extract from the document describing the tag format and its fields.
>> It is hoped that this will help future implementors who do not have
>> access to the material but who wish to extend the functionality of
>> drivers for chips which use this protocol.
>>
>> In addition, two possible values of the REASON field are specified,
>> based on experiments on my end. Realtek does not specify what value this
>> field can take.
>>
>> Signed-off-by: Alvin Šipraga <alsi@...g-olufsen.dk>
>> ---
>
> Reviewed-by: Vladimir Oltean <olteanv@...il.com>
>
>> RFC -> v1:
>> - minor changes to the big comment at the top, including some
>> empirical information about the REASON code
>> - use dev_*_ratelimited() instead of netdev_*() for logging
>> - use warning instead of debug messages
>> - use ETH_P_REALTEK from if_ether.h
>> - set LEARN_DIS on xmit
>> - remove superfluous variables/expressions and use __b16 for tag
>> variable
>> - use new helper functions to insert/remove CPU tag
>> - set offload_fwd_mark properly using helper function
>>
>> include/net/dsa.h | 2 +
>> net/dsa/Kconfig | 6 ++
>> net/dsa/Makefile | 1 +
>> net/dsa/tag_rtl8_4.c | 166 +++++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 175 insertions(+)
>> create mode 100644 net/dsa/tag_rtl8_4.c
>>
>> diff --git a/include/net/dsa.h b/include/net/dsa.h
>> index d784e76113b8..783060e851a9 100644
>> --- a/include/net/dsa.h
>> +++ b/include/net/dsa.h
>> @@ -51,6 +51,7 @@ struct phylink_link_state;
>> #define DSA_TAG_PROTO_SEVILLE_VALUE 21
>> #define DSA_TAG_PROTO_BRCM_LEGACY_VALUE 22
>> #define DSA_TAG_PROTO_SJA1110_VALUE 23
>> +#define DSA_TAG_PROTO_RTL8_4_VALUE 24
>>
>> enum dsa_tag_protocol {
>> DSA_TAG_PROTO_NONE = DSA_TAG_PROTO_NONE_VALUE,
>> @@ -77,6 +78,7 @@ enum dsa_tag_protocol {
>> DSA_TAG_PROTO_OCELOT_8021Q = DSA_TAG_PROTO_OCELOT_8021Q_VALUE,
>> DSA_TAG_PROTO_SEVILLE = DSA_TAG_PROTO_SEVILLE_VALUE,
>> DSA_TAG_PROTO_SJA1110 = DSA_TAG_PROTO_SJA1110_VALUE,
>> + DSA_TAG_PROTO_RTL8_4 = DSA_TAG_PROTO_RTL8_4_VALUE,
>> };
>>
>> struct dsa_switch;
>> diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
>> index 6c7f79e45886..57fc52b17d55 100644
>> --- a/net/dsa/Kconfig
>> +++ b/net/dsa/Kconfig
>> @@ -130,6 +130,12 @@ config NET_DSA_TAG_RTL4_A
>> Realtek switches with 4 byte protocol A tags, sich as found in
>> the Realtek RTL8366RB.
>>
>> +config NET_DSA_TAG_RTL8_4
>> + tristate "Tag driver for Realtek 8 byte protocol 4 tags"
>> + help
>> + Say Y or M if you want to enable support for tagging frames for Realtek
>> + switches with 8 byte protocol 4 tags, such as the Realtek RTL8365MB-VC.
>> +
>> config NET_DSA_TAG_LAN9303
>> tristate "Tag driver for SMSC/Microchip LAN9303 family of switches"
>> help
>> diff --git a/net/dsa/Makefile b/net/dsa/Makefile
>> index f78d537044db..9f75820e7c98 100644
>> --- a/net/dsa/Makefile
>> +++ b/net/dsa/Makefile
>> @@ -16,6 +16,7 @@ obj-$(CONFIG_NET_DSA_TAG_OCELOT) += tag_ocelot.o
>> obj-$(CONFIG_NET_DSA_TAG_OCELOT_8021Q) += tag_ocelot_8021q.o
>> obj-$(CONFIG_NET_DSA_TAG_QCA) += tag_qca.o
>> obj-$(CONFIG_NET_DSA_TAG_RTL4_A) += tag_rtl4_a.o
>> +obj-$(CONFIG_NET_DSA_TAG_RTL8_4) += tag_rtl8_4.o
>> obj-$(CONFIG_NET_DSA_TAG_SJA1105) += tag_sja1105.o
>> obj-$(CONFIG_NET_DSA_TAG_TRAILER) += tag_trailer.o
>> obj-$(CONFIG_NET_DSA_TAG_XRS700X) += tag_xrs700x.o
>> diff --git a/net/dsa/tag_rtl8_4.c b/net/dsa/tag_rtl8_4.c
>> new file mode 100644
>> index 000000000000..da22fd12b645
>> --- /dev/null
>> +++ b/net/dsa/tag_rtl8_4.c
>> @@ -0,0 +1,166 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Handler for Realtek 8 byte switch tags
>> + *
>> + * Copyright (C) 2021 Alvin Šipraga <alsi@...g-olufsen.dk>
>> + *
>> + * NOTE: Currently only supports protocol "4" found in the RTL8365MB, hence
>> + * named tag_rtl8_4.
>> + *
>> + * This tag header has the following format:
>> + *
>> + * -------------------------------------------
>> + * | MAC DA | MAC SA | 8 byte tag | Type | ...
>> + * -------------------------------------------
>> + * _______________/ \______________________________________
>> + * / \
>> + * 0 7|8 15
>> + * |-----------------------------------+-----------------------------------|---
>> + * | (16-bit) | ^
>> + * | Realtek EtherType [0x8899] | |
>> + * |-----------------------------------+-----------------------------------| 8
>> + * | (8-bit) | (8-bit) |
>> + * | Protocol [0x04] | REASON | b
>> + * |-----------------------------------+-----------------------------------| y
>> + * | (1) | (1) | (2) | (1) | (3) | (1) | (1) | (1) | (5) | t
>> + * | FID_EN | X | FID | PRI_EN | PRI | KEEP | X | LEARN_DIS | X | e
>> + * |-----------------------------------+-----------------------------------| s
>
> After our previous discussion I thought you said these bits were an EFID_EN and EFID?
> Or did you reconsider?
No, I did not reconsider - I stand by that observation. I actually sent
an email to our contact at Realtek to get some clarification. But until
I get a response (not guaranteed) or start actually using the field in
the code, I prefer to reproduce their documentation verbatim.
>
>> + * | (1) | (15-bit) | |
>> + * | ALLOW | TX/RX | v
>> + * |-----------------------------------+-----------------------------------|---
>> + *
>> + * With the following field descriptions:
>> + *
>> + * field | description
>> + * ------------+-------------
>> + * Realtek | 0x8899: indicates that this is a proprietary Realtek tag;
>> + * EtherType | note that Realtek uses the same EtherType for
>> + * | other incompatible tag formats (e.g. tag_rtl4_a.c)
>> + * Protocol | 0x04: indicates that this tag conforms to this format
>> + * X | reserved
>> + * ------------+-------------
>> + * REASON | reason for forwarding packet to CPU
>> + * | 0: packet was forwarded or flooded to CPU
>> + * | 80: packet was trapped to CPU
>> + * FID_EN | 1: packet has an FID
>> + * | 0: no FID
>> + * FID | FID of packet (if FID_EN=1)
>> + * PRI_EN | 1: force priority of packet
>> + * | 0: don't force priority
>> + * PRI | priority of packet (if PRI_EN=1)
>> + * KEEP | preserve packet VLAN tag format
>> + * LEARN_DIS | don't learn the source MAC address of the packet
>> + * ALLOW | 1: treat TX/RX field as an allowance port mask, meaning the
>> + * | packet may only be forwarded to ports specified in the
>> + * | mask
>> + * | 0: no allowance port mask, TX/RX field is the forwarding
>> + * | port mask
>> + * TX/RX | TX (switch->CPU): port number the packet was received on
>> + * | RX (CPU->switch): forwarding port mask (if ALLOW=0)
>> + * | allowance port mask (if ALLOW=1)
>> + */
>> +
>> +#include <linux/bits.h>
>> +#include <linux/etherdevice.h>
>> +
>> +#include "dsa_priv.h"
>> +
>> +#define RTL8_4_TAG_LEN 8
>> +/* 0x04 = RTL8365MB DSA protocol
>> + */
>> +#define RTL8_4_PROTOCOL_RTL8365MB 0x04
>> +
>> +static struct sk_buff *rtl8_4_tag_xmit(struct sk_buff *skb,
>> + struct net_device *dev)
>> +{
>> + struct dsa_port *dp = dsa_slave_to_port(dev);
>> + __be16 *tag;
>> +
>> + /* Pad out so the (stripped) packet is at least 64 bytes long
>> + * (including FCS), otherwise the switch will drop the packet.
>> + * Then we need an additional 8 bytes for the Realtek tag.
>> + */
>> + if (unlikely(__skb_put_padto(skb, ETH_ZLEN + RTL8_4_TAG_LEN, false)))
>> + return NULL;
>> +
>> + skb_push(skb, RTL8_4_TAG_LEN);
>> +
>> + dsa_alloc_etype_header(skb, RTL8_4_TAG_LEN);
>> + tag = dsa_etype_header_pos_tx(skb);
>> +
>> + /* Set Realtek EtherType */
>> + tag[0] = htons(ETH_P_REALTEK);
>> +
>> + /* Set Protocol; zero REASON */
>> + tag[1] = htons(RTL8_4_PROTOCOL_RTL8365MB << 8);
>> +
>> + /* Zero FID_EN, FID, PRI_EN, PRI, KEEP; set LEARN_DIS */
>> + tag[2] = htons(1 << 5);
>> +
>> + /* Zero ALLOW; set RX (CPU->switch) forwarding port mask */
>> + tag[3] = htons(BIT(dp->index));
>> +
>> + return skb;
>> +}
>> +
>> +static struct sk_buff *rtl8_4_tag_rcv(struct sk_buff *skb,
>> + struct net_device *dev)
>> +{
>> + __be16 *tag;
>> + u16 etype;
>> + u8 proto;
>> + u8 port;
>> +
>> + if (unlikely(!pskb_may_pull(skb, RTL8_4_TAG_LEN)))
>> + return NULL;
>> +
>> + tag = dsa_etype_header_pos_rx(skb);
>> +
>> + /* Parse Realtek EtherType */
>> + etype = ntohs(tag[0]);
>> + if (unlikely(etype != ETH_P_REALTEK)) {
>> + dev_warn_ratelimited(&dev->dev,
>> + "non-realtek ethertype 0x%04x\n", etype);
>> + return NULL;
>> + }
>> +
>> + /* Parse Protocol */
>> + proto = ntohs(tag[1]) >> 8;
>> + if (unlikely(proto != RTL8_4_PROTOCOL_RTL8365MB)) {
>> + dev_warn_ratelimited(&dev->dev,
>> + "unknown realtek protocol 0x%02x\n",
>> + proto);
>> + return NULL;
>> + }
>> +
>> + /* Parse TX (switch->CPU) */
>> + port = ntohs(tag[3]) & 0xf; /* Port number is the LSB 4 bits */
>> + skb->dev = dsa_master_find_slave(dev, 0, port);
>> + if (!skb->dev) {
>> + dev_warn_ratelimited(&dev->dev,
>> + "could not find slave for port %d\n",
>> + port);
>> + return NULL;
>> + }
>> +
>> + /* Remove tag and recalculate checksum */
>> + skb_pull_rcsum(skb, RTL8_4_TAG_LEN);
>> +
>> + dsa_strip_etype_header(skb, RTL8_4_TAG_LEN);
>> +
>> + dsa_default_offload_fwd_mark(skb);
>> +
>> + return skb;
>> +}
>> +
>> +static const struct dsa_device_ops rtl8_4_netdev_ops = {
>> + .name = "rtl8_4",
>> + .proto = DSA_TAG_PROTO_RTL8_4,
>> + .xmit = rtl8_4_tag_xmit,
>> + .rcv = rtl8_4_tag_rcv,
>> + .needed_headroom = RTL8_4_TAG_LEN,
>> +};
>> +module_dsa_tag_driver(rtl8_4_netdev_ops);
>> +
>> +MODULE_LICENSE("GPL");
>> +MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_RTL8_4);
>> --
>> 2.32.0
>>
Powered by blists - more mailing lists