| 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
| ||
|
Message-ID: <7c14ff72-95d5-2e97-4ff5-8e41fcfab2c6@gmail.com>
Date: Tue, 23 Mar 2021 09:30:06 -0700
From: Florian Fainelli <f.fainelli@...il.com>
To: Tobias Waldekranz <tobias@...dekranz.com>,
Vladimir Oltean <olteanv@...il.com>
Cc: davem@...emloft.net, kuba@...nel.org, andrew@...n.ch,
vivien.didelot@...il.com, netdev@...r.kernel.org
Subject: Re: [PATCH net-next] net: dsa: mv88e6xxx: Allow dynamic
reconfiguration of tag protocol
On 3/23/2021 7:48 AM, Tobias Waldekranz wrote:
> On Tue, Mar 23, 2021 at 13:35, Vladimir Oltean <olteanv@...il.com> wrote:
>> On Tue, Mar 23, 2021 at 11:23:26AM +0100, Tobias Waldekranz wrote:
>>> All devices are capable of using regular DSA tags. Support for
>>> Ethertyped DSA tags sort into three categories:
>>>
>>> 1. No support. Older chips fall into this category.
>>>
>>> 2. Full support. Datasheet explicitly supports configuring the CPU
>>> port to receive FORWARDs with a DSA tag.
>>>
>>> 3. Undocumented support. Datasheet lists the configuration from
>>> category 2 as "reserved for future use", but does empirically
>>> behave like a category 2 device.
>>>
>>> Because there are ethernet controllers that do not handle regular DSA
>>> tags in all cases, it is sometimes preferable to rely on the
>>> undocumented behavior, as the alternative is a very crippled
>>> system. But, in those cases, make sure to log the fact that an
>>> undocumented feature has been enabled.
>>>
>>> Signed-off-by: Tobias Waldekranz <tobias@...dekranz.com>
>>> ---
>>>
>>> In a system using an NXP T1023 SoC connected to a 6390X switch, we
>>> noticed that TO_CPU frames where not reaching the CPU. This only
>>> happened on hardware port 8. Looking at the DSA master interface
>>> (dpaa-ethernet) we could see that an Rx error counter was bumped at
>>> the same rate. The logs indicated a parser error.
>>>
>>> It just so happens that a TO_CPU coming in on device 0, port 8, will
>>> result in the first two bytes of the DSA tag being one of:
>>>
>>> 00 40
>>> 00 44
>>> 00 46
>>>
>>> My guess is that since these values look like 802.3 length fields, the
>>> controller's parser will signal an error if the frame length does not
>>> match what is in the header.
>>
>> Interesting assumption.
>> Could you please try this patch out, just for my amusement? It is only
>> compile-tested.
>>
>> -----------------------------[ cut here ]-----------------------------
>> From ab75b63d1bfeccc3032060e6e6dbd2ea8f1d31ed Mon Sep 17 00:00:00 2001
>> From: Vladimir Oltean <vladimir.oltean@....com>
>> Date: Tue, 23 Mar 2021 13:03:34 +0200
>> Subject: [PATCH] fsl/fman: ignore RX parse errors on ports that are DSA
>> masters
>>
>> Tobias reports that when an FMan port receives a Marvell DSA tagged
>> frame (normal/legacy tag, not EtherType tag) which is a TO_CPU frame
>> coming in from device 0, port 8, that frame will be dropped.
>>
>> It appears that the first two bytes of this particular DSA tag (which
>> overlap with what the FMan parser interprets as an EtherType/Length
>> field) look like one of the possible values below:
>>
>> 00 40
>> 00 44
>> 00 46
>>
>> Reported-by: Tobias Waldekranz <tobias@...dekranz.com>
>> Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
>> ---
>> .../net/ethernet/freescale/dpaa/dpaa_eth.c | 65 ++++++++++---------
>> .../net/ethernet/freescale/fman/fman_port.c | 8 ++-
>> .../net/ethernet/freescale/fman/fman_port.h | 2 +-
>> 3 files changed, 42 insertions(+), 33 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
>> index 720dc99bd1fc..069d38cd63c5 100644
>> --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
>> +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
>> @@ -55,6 +55,7 @@
>> #include <linux/phy_fixed.h>
>> #include <linux/bpf.h>
>> #include <linux/bpf_trace.h>
>> +#include <net/dsa.h>
>> #include <soc/fsl/bman.h>
>> #include <soc/fsl/qman.h>
>> #include "fman.h"
>> @@ -2447,34 +2448,6 @@ static inline int dpaa_eth_napi_schedule(struct dpaa_percpu_priv *percpu_priv,
>> return 0;
>> }
>>
>> -static enum qman_cb_dqrr_result rx_error_dqrr(struct qman_portal *portal,
>> - struct qman_fq *fq,
>> - const struct qm_dqrr_entry *dq,
>> - bool sched_napi)
>> -{
>> - struct dpaa_fq *dpaa_fq = container_of(fq, struct dpaa_fq, fq_base);
>> - struct dpaa_percpu_priv *percpu_priv;
>> - struct net_device *net_dev;
>> - struct dpaa_bp *dpaa_bp;
>> - struct dpaa_priv *priv;
>> -
>> - net_dev = dpaa_fq->net_dev;
>> - priv = netdev_priv(net_dev);
>> - dpaa_bp = dpaa_bpid2pool(dq->fd.bpid);
>> - if (!dpaa_bp)
>> - return qman_cb_dqrr_consume;
>> -
>> - percpu_priv = this_cpu_ptr(priv->percpu_priv);
>> -
>> - if (dpaa_eth_napi_schedule(percpu_priv, portal, sched_napi))
>> - return qman_cb_dqrr_stop;
>> -
>> - dpaa_eth_refill_bpools(priv);
>> - dpaa_rx_error(net_dev, priv, percpu_priv, &dq->fd, fq->fqid);
>> -
>> - return qman_cb_dqrr_consume;
>> -}
>> -
>> static int dpaa_xdp_xmit_frame(struct net_device *net_dev,
>> struct xdp_frame *xdpf)
>> {
>> @@ -2699,7 +2672,7 @@ static enum qman_cb_dqrr_result rx_default_dqrr(struct qman_portal *portal,
>> return qman_cb_dqrr_consume;
>> }
>>
>> - if (unlikely(fd_status & FM_FD_STAT_RX_ERRORS) != 0) {
>> + if (!netdev_uses_dsa(net_dev) && unlikely(fd_status & FM_FD_STAT_RX_ERRORS) != 0) {
>> if (net_ratelimit())
>> netif_warn(priv, hw, net_dev, "FD status = 0x%08x\n",
>> fd_status & FM_FD_STAT_RX_ERRORS);
>> @@ -2802,6 +2775,37 @@ static enum qman_cb_dqrr_result rx_default_dqrr(struct qman_portal *portal,
>> return qman_cb_dqrr_consume;
>> }
>>
>> +static enum qman_cb_dqrr_result rx_error_dqrr(struct qman_portal *portal,
>> + struct qman_fq *fq,
>> + const struct qm_dqrr_entry *dq,
>> + bool sched_napi)
>> +{
>> + struct dpaa_fq *dpaa_fq = container_of(fq, struct dpaa_fq, fq_base);
>> + struct dpaa_percpu_priv *percpu_priv;
>> + struct net_device *net_dev;
>> + struct dpaa_bp *dpaa_bp;
>> + struct dpaa_priv *priv;
>> +
>> + net_dev = dpaa_fq->net_dev;
>> + if (netdev_uses_dsa(net_dev))
>> + return rx_default_dqrr(portal, fq, dq, sched_napi);
>> +
>> + priv = netdev_priv(net_dev);
>> + dpaa_bp = dpaa_bpid2pool(dq->fd.bpid);
>> + if (!dpaa_bp)
>> + return qman_cb_dqrr_consume;
>> +
>> + percpu_priv = this_cpu_ptr(priv->percpu_priv);
>> +
>> + if (dpaa_eth_napi_schedule(percpu_priv, portal, sched_napi))
>> + return qman_cb_dqrr_stop;
>> +
>> + dpaa_eth_refill_bpools(priv);
>> + dpaa_rx_error(net_dev, priv, percpu_priv, &dq->fd, fq->fqid);
>> +
>> + return qman_cb_dqrr_consume;
>> +}
>> +
>> static enum qman_cb_dqrr_result conf_error_dqrr(struct qman_portal *portal,
>> struct qman_fq *fq,
>> const struct qm_dqrr_entry *dq,
>> @@ -2955,6 +2959,7 @@ static int dpaa_phy_init(struct net_device *net_dev)
>>
>> static int dpaa_open(struct net_device *net_dev)
>> {
>> + bool ignore_errors = netdev_uses_dsa(net_dev);
>> struct mac_device *mac_dev;
>> struct dpaa_priv *priv;
>> int err, i;
>> @@ -2968,7 +2973,7 @@ static int dpaa_open(struct net_device *net_dev)
>> goto phy_init_failed;
>>
>> for (i = 0; i < ARRAY_SIZE(mac_dev->port); i++) {
>> - err = fman_port_enable(mac_dev->port[i]);
>> + err = fman_port_enable(mac_dev->port[i], ignore_errors);
>> if (err)
>> goto mac_start_failed;
>> }
>> diff --git a/drivers/net/ethernet/freescale/fman/fman_port.c b/drivers/net/ethernet/freescale/fman/fman_port.c
>> index d9baac0dbc7d..763faec11f5c 100644
>> --- a/drivers/net/ethernet/freescale/fman/fman_port.c
>> +++ b/drivers/net/ethernet/freescale/fman/fman_port.c
>> @@ -106,6 +106,7 @@
>> #define BMI_EBD_EN 0x80000000
>>
>> #define BMI_PORT_CFG_EN 0x80000000
>> +#define BMI_PORT_CFG_FDOVR 0x02000000
>>
>> #define BMI_PORT_STATUS_BSY 0x80000000
>>
>> @@ -1629,7 +1630,7 @@ int fman_port_disable(struct fman_port *port)
>> }
>>
>> /* Disable BMI */
>> - tmp = ioread32be(bmi_cfg_reg) & ~BMI_PORT_CFG_EN;
>> + tmp = ioread32be(bmi_cfg_reg) & ~(BMI_PORT_CFG_EN | BMI_PORT_CFG_FDOVR);
>> iowrite32be(tmp, bmi_cfg_reg);
>>
>> /* Wait for graceful stop end */
>> @@ -1655,6 +1656,7 @@ EXPORT_SYMBOL(fman_port_disable);
>> /**
>> * fman_port_enable
>> * @port: A pointer to a FM Port module.
>> + * @ignore_errors: If set, do not discard frames received with errors.
>> *
>> * A runtime routine provided to allow disable/enable of port.
>> *
>> @@ -1662,7 +1664,7 @@ EXPORT_SYMBOL(fman_port_disable);
>> *
>> * Return: 0 on success; Error code otherwise.
>> */
>> -int fman_port_enable(struct fman_port *port)
>> +int fman_port_enable(struct fman_port *port, bool ignore_errors)
>> {
>> u32 __iomem *bmi_cfg_reg;
>> u32 tmp;
>> @@ -1692,6 +1694,8 @@ int fman_port_enable(struct fman_port *port)
>>
>> /* Enable BMI */
>> tmp = ioread32be(bmi_cfg_reg) | BMI_PORT_CFG_EN;
>> + if (ignore_errors)
>> + tmp |= BMI_PORT_CFG_FDOVR;
>> iowrite32be(tmp, bmi_cfg_reg);
>>
>> return 0;
>> diff --git a/drivers/net/ethernet/freescale/fman/fman_port.h b/drivers/net/ethernet/freescale/fman/fman_port.h
>> index 82f12661a46d..0928361b0e73 100644
>> --- a/drivers/net/ethernet/freescale/fman/fman_port.h
>> +++ b/drivers/net/ethernet/freescale/fman/fman_port.h
>> @@ -147,7 +147,7 @@ int fman_port_cfg_buf_prefix_content(struct fman_port *port,
>>
>> int fman_port_disable(struct fman_port *port);
>>
>> -int fman_port_enable(struct fman_port *port);
>> +int fman_port_enable(struct fman_port *port, bool ignore_errors);
>>
>> u32 fman_port_get_qman_channel_id(struct fman_port *port);
>>
>> -----------------------------[ cut here ]-----------------------------
>>
>> The netdev_uses_dsa thing is a bit trashy, I think that a more polished
>> version should rather set NETIF_F_RXALL for the DSA master, and have the
>> dpaa driver act upon that. But first I'm curious if it works.
>
> It does work. Thank you!
This is unfortunately not new, the stmmac choked on me with Broadcom
tags as well:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8cad443eacf661796a740903a75cb8944c675b4e
--
Florian
Powered by blists - more mailing lists