| 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: Mon, 28 Feb 2022 16:50:58 +0200
From: Vladimir Oltean <olteanv@...il.com>
To: Luiz Angelo Daros de Luca <luizluca@...il.com>
Cc: netdev@...r.kernel.org, linus.walleij@...aro.org, andrew@...n.ch,
vivien.didelot@...il.com, f.fainelli@...il.com,
davem@...emloft.net, kuba@...nel.org, alsi@...g-olufsen.dk,
arinc.unal@...nc9.com
Subject: Re: [PATCH net-next v4 2/3] net: dsa: tag_rtl8_4: add rtl8_4t
trailing variant
On Sun, Feb 27, 2022 at 12:59:19AM -0300, Luiz Angelo Daros de Luca wrote:
> Realtek switches supports the same tag both before ethertype or between
> payload and the CRC.
>
> Reviewed-by: Alvin Šipraga <alsi@...g-olufsen.dk>
> Signed-off-by: Luiz Angelo Daros de Luca <luizluca@...il.com>
> ---
> include/net/dsa.h | 2 +
> net/dsa/tag_rtl8_4.c | 150 +++++++++++++++++++++++++++++++++----------
> 2 files changed, 117 insertions(+), 35 deletions(-)
>
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index fd1f62a6e0a8..b688ced04b0e 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -52,6 +52,7 @@ struct phylink_link_state;
> #define DSA_TAG_PROTO_BRCM_LEGACY_VALUE 22
> #define DSA_TAG_PROTO_SJA1110_VALUE 23
> #define DSA_TAG_PROTO_RTL8_4_VALUE 24
> +#define DSA_TAG_PROTO_RTL8_4T_VALUE 25
>
> enum dsa_tag_protocol {
> DSA_TAG_PROTO_NONE = DSA_TAG_PROTO_NONE_VALUE,
> @@ -79,6 +80,7 @@ enum dsa_tag_protocol {
> 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,
> + DSA_TAG_PROTO_RTL8_4T = DSA_TAG_PROTO_RTL8_4T_VALUE,
> };
>
> struct dsa_switch;
> diff --git a/net/dsa/tag_rtl8_4.c b/net/dsa/tag_rtl8_4.c
> index 02686ad4045d..bf34b257b193 100644
> --- a/net/dsa/tag_rtl8_4.c
> +++ b/net/dsa/tag_rtl8_4.c
> @@ -9,11 +9,6 @@
> *
> * This tag header has the following format:
If you generalize the comments, then it is no longer a "header".
> *
> - * -------------------------------------------
> - * | MAC DA | MAC SA | 8 byte tag | Type | ...
> - * -------------------------------------------
> - * _______________/ \______________________________________
> - * / \
> * 0 7|8 15
> * |-----------------------------------+-----------------------------------|---
> * | (16-bit) | ^
> @@ -58,6 +53,24 @@
> * 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)
> + *
> + * The tag can be positioned before Ethertype, using tag "rtl8_4":
> + *
> + * +--------+--------+------------+------+-----
> + * | MAC DA | MAC SA | 8 byte tag | Type | ...
> + * +--------+--------+------------+------+-----
> + *
> + * The tag can also appear between the end of the payload and before the CRC,
> + * using tag "rtl8_4t":
> + *
> + * +--------+--------+------+-----+---------+------------+-----+
> + * | MAC DA | MAC SA | TYPE | ... | payload | 8-byte tag | CRC |
> + * +--------+--------+------+-----+---------+------------+-----+
> + *
> + * The added bytes after the payload will break most checksums, either in
> + * software or hardware. To avoid this issue, if the checksum is still pending,
> + * this tagger checksum the packet in software before adding the tag.
s/checksum/checksums/
> + *
> */
>
> #include <linux/bitfield.h>
> @@ -84,87 +97,133 @@
> #define RTL8_4_TX GENMASK(3, 0)
> #define RTL8_4_RX GENMASK(10, 0)
>
> -static struct sk_buff *rtl8_4_tag_xmit(struct sk_buff *skb,
> - struct net_device *dev)
> +static void rtl8_4_write_tag(struct sk_buff *skb, struct net_device *dev,
> + void *tag)
> {
> struct dsa_port *dp = dsa_slave_to_port(dev);
> - __be16 *tag;
> -
> - skb_push(skb, RTL8_4_TAG_LEN);
> -
> - dsa_alloc_etype_header(skb, RTL8_4_TAG_LEN);
> - tag = dsa_etype_header_pos_tx(skb);
> + __be16 tag16[RTL8_4_TAG_LEN / 2];
>
> /* Set Realtek EtherType */
> - tag[0] = htons(ETH_P_REALTEK);
> + tag16[0] = htons(ETH_P_REALTEK);
>
> /* Set Protocol; zero REASON */
> - tag[1] = htons(FIELD_PREP(RTL8_4_PROTOCOL, RTL8_4_PROTOCOL_RTL8365MB));
> + tag16[1] = htons(FIELD_PREP(RTL8_4_PROTOCOL, RTL8_4_PROTOCOL_RTL8365MB));
>
> /* Zero FID_EN, FID, PRI_EN, PRI, KEEP; set LEARN_DIS */
> - tag[2] = htons(FIELD_PREP(RTL8_4_LEARN_DIS, 1));
> + tag16[2] = htons(FIELD_PREP(RTL8_4_LEARN_DIS, 1));
>
> /* Zero ALLOW; set RX (CPU->switch) forwarding port mask */
> - tag[3] = htons(FIELD_PREP(RTL8_4_RX, BIT(dp->index)));
> + tag16[3] = htons(FIELD_PREP(RTL8_4_RX, BIT(dp->index)));
> +
> + memcpy(tag, tag16, RTL8_4_TAG_LEN);
> +}
> +
> +static struct sk_buff *rtl8_4_tag_xmit(struct sk_buff *skb,
> + struct net_device *dev)
> +{
> + skb_push(skb, RTL8_4_TAG_LEN);
> +
> + dsa_alloc_etype_header(skb, RTL8_4_TAG_LEN);
> +
> + rtl8_4_write_tag(skb, dev, dsa_etype_header_pos_tx(skb));
>
> return skb;
> }
>
> -static struct sk_buff *rtl8_4_tag_rcv(struct sk_buff *skb,
> - struct net_device *dev)
> +static struct sk_buff *rtl8_4t_tag_xmit(struct sk_buff *skb,
> + struct net_device *dev)
> +{
> + /* Calculate the checksum here if not done yet as trailing tags will
> + * break either software and hardware based checksum
"either" is typically used with "or", not with "and"
> + */
> + if (skb->ip_summed == CHECKSUM_PARTIAL && skb_checksum_help(skb))
> + return NULL;
> +
> + rtl8_4_write_tag(skb, dev, skb_put(skb, RTL8_4_TAG_LEN));
> +
> + return skb;
> +}
> +
> +static int rtl8_4_read_tag(struct sk_buff *skb, struct net_device *dev,
> + void *tag)
> {
> - __be16 *tag;
> + __be16 tag16[RTL8_4_TAG_LEN / 2];
> u16 etype;
> u8 reason;
> u8 proto;
> u8 port;
>
> - if (unlikely(!pskb_may_pull(skb, RTL8_4_TAG_LEN)))
> - return NULL;
> -
> - tag = dsa_etype_header_pos_rx(skb);
> + memcpy(tag16, tag, RTL8_4_TAG_LEN);
>
> /* Parse Realtek EtherType */
> - etype = ntohs(tag[0]);
> + etype = ntohs(tag16[0]);
> if (unlikely(etype != ETH_P_REALTEK)) {
> dev_warn_ratelimited(&dev->dev,
> "non-realtek ethertype 0x%04x\n", etype);
> - return NULL;
> + return -EPROTO;
> }
>
> /* Parse Protocol */
> - proto = FIELD_GET(RTL8_4_PROTOCOL, ntohs(tag[1]));
> + proto = FIELD_GET(RTL8_4_PROTOCOL, ntohs(tag16[1]));
> if (unlikely(proto != RTL8_4_PROTOCOL_RTL8365MB)) {
> dev_warn_ratelimited(&dev->dev,
> "unknown realtek protocol 0x%02x\n",
> proto);
> - return NULL;
> + return -EPROTO;
> }
>
> /* Parse REASON */
> - reason = FIELD_GET(RTL8_4_REASON, ntohs(tag[1]));
> + reason = FIELD_GET(RTL8_4_REASON, ntohs(tag16[1]));
>
> /* Parse TX (switch->CPU) */
> - port = FIELD_GET(RTL8_4_TX, ntohs(tag[3]));
> + port = FIELD_GET(RTL8_4_TX, ntohs(tag16[3]));
> 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;
> + return -ENOENT;
> }
>
> + if (reason != RTL8_4_REASON_TRAP)
> + dsa_default_offload_fwd_mark(skb);
> +
> + return 0;
> +}
> +
> +static struct sk_buff *rtl8_4_tag_rcv(struct sk_buff *skb,
> + struct net_device *dev)
> +{
> + if (unlikely(!pskb_may_pull(skb, RTL8_4_TAG_LEN)))
> + return NULL;
> +
> + if (unlikely(rtl8_4_read_tag(skb, dev, dsa_etype_header_pos_rx(skb))))
> + return NULL;
> +
> /* Remove tag and recalculate checksum */
> skb_pull_rcsum(skb, RTL8_4_TAG_LEN);
>
> dsa_strip_etype_header(skb, RTL8_4_TAG_LEN);
>
> - if (reason != RTL8_4_REASON_TRAP)
> - dsa_default_offload_fwd_mark(skb);
> + return skb;
> +}
> +
> +static struct sk_buff *rtl8_4t_tag_rcv(struct sk_buff *skb,
> + struct net_device *dev)
> +{
> + if (skb_linearize(skb))
> + return NULL;
> +
> + if (unlikely(rtl8_4_read_tag(skb, dev, skb_tail_pointer(skb) - RTL8_4_TAG_LEN)))
> + return NULL;
> +
> + if (pskb_trim_rcsum(skb, skb->len - RTL8_4_TAG_LEN))
> + return NULL;
>
> return skb;
> }
>
> +/* Ethertype version */
> static const struct dsa_device_ops rtl8_4_netdev_ops = {
> .name = "rtl8_4",
> .proto = DSA_TAG_PROTO_RTL8_4,
> @@ -172,7 +231,28 @@ static const struct dsa_device_ops rtl8_4_netdev_ops = {
> .rcv = rtl8_4_tag_rcv,
> .needed_headroom = RTL8_4_TAG_LEN,
> };
> -module_dsa_tag_driver(rtl8_4_netdev_ops);
>
> -MODULE_LICENSE("GPL");
> +DSA_TAG_DRIVER(rtl8_4_netdev_ops);
> +
> MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_RTL8_4);
> +
> +/* Tail version */
> +static const struct dsa_device_ops rtl8_4t_netdev_ops = {
> + .name = "rtl8_4t",
> + .proto = DSA_TAG_PROTO_RTL8_4T,
> + .xmit = rtl8_4t_tag_xmit,
> + .rcv = rtl8_4t_tag_rcv,
> + .needed_tailroom = RTL8_4_TAG_LEN,
> +};
> +
> +DSA_TAG_DRIVER(rtl8_4t_netdev_ops);
> +
> +MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_RTL8_4L);
> +
> +static struct dsa_tag_driver *dsa_tag_drivers[] = {
> + &DSA_TAG_DRIVER_NAME(rtl8_4_netdev_ops),
> + &DSA_TAG_DRIVER_NAME(rtl8_4t_netdev_ops),
> +};
> +module_dsa_tag_drivers(dsa_tag_drivers);
> +
> +MODULE_LICENSE("GPL");
> --
> 2.35.1
>
Powered by blists - more mailing lists