| 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, 25 Jan 2022 19:48:27 -0800
From: Florian Fainelli <f.fainelli@...il.com>
To: Ansuel Smith <ansuelsmth@...il.com>, Andrew Lunn <andrew@...n.ch>,
Vivien Didelot <vivien.didelot@...il.com>,
Vladimir Oltean <olteanv@...il.com>,
"David S. Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>, linux-kernel@...r.kernel.org,
netdev@...r.kernel.org
Subject: Re: [RFC PATCH v7 15/16] net: da: qca8k: add support for larger
read/write size with mgmt Ethernet
On 1/22/2022 5:33 PM, Ansuel Smith wrote:
> mgmt Ethernet packet can read/write up to 16byte at times. The len reg
> is limited to 15 (0xf). The switch actually sends and accepts data in 4
> different steps of len values.
> Len steps:
> - 0: nothing
> - 1-4: first 4 byte
> - 5-6: first 12 byte
> - 7-15: all 16 byte
This is really odd, it almost felt like the length was a byte enable
bitmask, but it is not?
>
> In the allock skb function we check if the len is 16 and we fix it to a
> len of 15.
s/allock/alloc/
> It the read/write function interest to extract the real asked
> data. The tagger handler will always copy the fully 16byte with a READ
> command. This is useful for some big regs like the fdb reg that are
> more than 4byte of data. This permits to introduce a bulk function that
> will send and request the entire entry in one go.
> Write function is changed and it does now require to pass the pointer to
> val to also handle array val.
>
> Signed-off-by: Ansuel Smith <ansuelsmth@...il.com>
> ---
> drivers/net/dsa/qca8k.c | 56 ++++++++++++++++++++++++++++++-----------
> 1 file changed, 41 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
> index 2a43fb9aeef2..0183ce2d5b74 100644
> --- a/drivers/net/dsa/qca8k.c
> +++ b/drivers/net/dsa/qca8k.c
> @@ -219,7 +219,9 @@ static void qca8k_rw_reg_ack_handler(struct dsa_switch *ds, struct sk_buff *skb)
> if (cmd == MDIO_READ) {
> mgmt_hdr_data->data[0] = mgmt_ethhdr->mdio_data;
>
> - /* Get the rest of the 12 byte of data */
> + /* Get the rest of the 12 byte of data.
> + * The read/write function will extract the requested data.
> + */
> if (len > QCA_HDR_MGMT_DATA1_LEN)
> memcpy(mgmt_hdr_data->data + 1, skb->data,
> QCA_HDR_MGMT_DATA2_LEN);
> @@ -229,16 +231,30 @@ static void qca8k_rw_reg_ack_handler(struct dsa_switch *ds, struct sk_buff *skb)
> }
>
> static struct sk_buff *qca8k_alloc_mdio_header(enum mdio_cmd cmd, u32 reg, u32 *val,
> - int seq_num, int priority)
> + int seq_num, int priority, int len)
unsigned int len
> {
> struct mgmt_ethhdr *mgmt_ethhdr;
> struct sk_buff *skb;
> + int real_len;
Likewise.
> + u32 *data2;
> u16 hdr;
>
> skb = dev_alloc_skb(QCA_HDR_MGMT_PKG_LEN);
> if (!skb)
> return NULL;
>
> + /* Max value for len reg is 15 (0xf) but the switch actually return 16 byte
> + * Actually for some reason the steps are:
> + * 0: nothing
> + * 1-4: first 4 byte
> + * 5-6: first 12 byte
> + * 7-15: all 16 byte
> + */
> + if (len == 16)
> + real_len = 15;
> + else
> + real_len = len;
> +
> skb_reset_mac_header(skb);
> skb_set_network_header(skb, skb->len);
>
> @@ -253,7 +269,7 @@ static struct sk_buff *qca8k_alloc_mdio_header(enum mdio_cmd cmd, u32 reg, u32 *
> mgmt_ethhdr->seq = FIELD_PREP(QCA_HDR_MGMT_SEQ_NUM, seq_num);
>
> mgmt_ethhdr->command = FIELD_PREP(QCA_HDR_MGMT_ADDR, reg);
> - mgmt_ethhdr->command |= FIELD_PREP(QCA_HDR_MGMT_LENGTH, 4);
> + mgmt_ethhdr->command |= FIELD_PREP(QCA_HDR_MGMT_LENGTH, real_len);
> mgmt_ethhdr->command |= FIELD_PREP(QCA_HDR_MGMT_CMD, cmd);
> mgmt_ethhdr->command |= FIELD_PREP(QCA_HDR_MGMT_CHECK_CODE,
> QCA_HDR_MGMT_CHECK_CODE_VAL);
> @@ -263,19 +279,22 @@ static struct sk_buff *qca8k_alloc_mdio_header(enum mdio_cmd cmd, u32 reg, u32 *
>
> mgmt_ethhdr->hdr = htons(hdr);
>
> - skb_put_zero(skb, QCA_HDR_MGMT_DATA2_LEN + QCA_HDR_MGMT_PADDING_LEN);
> + data2 = skb_put_zero(skb, QCA_HDR_MGMT_DATA2_LEN + QCA_HDR_MGMT_PADDING_LEN);
> + if (cmd == MDIO_WRITE && len > QCA_HDR_MGMT_DATA1_LEN)
> + memcpy(data2, val + 1, len - QCA_HDR_MGMT_DATA1_LEN);
>
> return skb;
> }
>
> -static int qca8k_read_eth(struct qca8k_priv *priv, u32 reg, u32 *val)
> +static int qca8k_read_eth(struct qca8k_priv *priv, u32 reg, u32 *val, int len)
> {
> struct qca8k_mgmt_hdr_data *mgmt_hdr_data = &priv->mgmt_hdr_data;
> struct sk_buff *skb;
> bool ack;
> int ret;
>
> - skb = qca8k_alloc_mdio_header(MDIO_READ, reg, NULL, 200, QCA8K_ETHERNET_MDIO_PRIORITY);
> + skb = qca8k_alloc_mdio_header(MDIO_READ, reg, NULL, 200,
> + QCA8K_ETHERNET_MDIO_PRIORITY, len);
> if (!skb)
> return -ENOMEM;
>
> @@ -297,6 +316,9 @@ static int qca8k_read_eth(struct qca8k_priv *priv, u32 reg, u32 *val)
> msecs_to_jiffies(QCA8K_ETHERNET_TIMEOUT));
>
> *val = mgmt_hdr_data->data[0];
> + if (len > QCA_HDR_MGMT_DATA1_LEN)
> + memcpy(val + 1, mgmt_hdr_data->data + 1, len - QCA_HDR_MGMT_DATA1_LEN);
> +
> ack = mgmt_hdr_data->ack;
>
> mutex_unlock(&mgmt_hdr_data->mutex);
> @@ -310,14 +332,15 @@ static int qca8k_read_eth(struct qca8k_priv *priv, u32 reg, u32 *val)
> return 0;
> }
>
> -static int qca8k_write_eth(struct qca8k_priv *priv, u32 reg, u32 val)
> +static int qca8k_write_eth(struct qca8k_priv *priv, u32 reg, u32 *val, int len)
> {
> struct qca8k_mgmt_hdr_data *mgmt_hdr_data = &priv->mgmt_hdr_data;
> struct sk_buff *skb;
> bool ack;
> int ret;
>
> - skb = qca8k_alloc_mdio_header(MDIO_WRITE, reg, &val, 200, QCA8K_ETHERNET_MDIO_PRIORITY);
> + skb = qca8k_alloc_mdio_header(MDIO_WRITE, reg, val, 200,
> + QCA8K_ETHERNET_MDIO_PRIORITY, len);
> if (!skb)
> return -ENOMEM;
>
> @@ -357,14 +380,14 @@ qca8k_regmap_update_bits_eth(struct qca8k_priv *priv, u32 reg, u32 mask, u32 wri
> u32 val = 0;
> int ret;
>
> - ret = qca8k_read_eth(priv, reg, &val);
> + ret = qca8k_read_eth(priv, reg, &val, 4);
sizeof(val) instead of 4.
> if (ret)
> return ret;
>
> val &= ~mask;
> val |= write_val;
>
> - return qca8k_write_eth(priv, reg, val);
> + return qca8k_write_eth(priv, reg, &val, 4);
Likewise
> }
>
> static int
> @@ -376,7 +399,7 @@ qca8k_regmap_read(void *ctx, uint32_t reg, uint32_t *val)
> u16 r1, r2, page;
> int ret;
>
> - if (priv->mgmt_master && !qca8k_read_eth(priv, reg, val))
> + if (priv->mgmt_master && !qca8k_read_eth(priv, reg, val, 4))
Likewise and everywhere below as well.
--
Florian
Powered by blists - more mailing lists