| 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: <20230203225527.e3uyysmxmd463ani@skbuf>
Date: Sat, 4 Feb 2023 00:55:27 +0200
From: Vladimir Oltean <olteanv@...il.com>
To: Daniel Golle <daniel@...rotopia.org>
Cc: netdev@...r.kernel.org, linux-mediatek@...ts.infradead.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
Russell King <linux@...linux.org.uk>,
Heiner Kallweit <hkallweit1@...il.com>,
Lorenzo Bianconi <lorenzo@...nel.org>,
Mark Lee <Mark-MC.Lee@...iatek.com>,
John Crispin <john@...ozen.org>, Felix Fietkau <nbd@....name>,
AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>,
Matthias Brugger <matthias.bgg@...il.com>,
DENG Qingfang <dqfext@...il.com>,
Landen Chao <Landen.Chao@...iatek.com>,
Sean Wang <sean.wang@...iatek.com>,
Paolo Abeni <pabeni@...hat.com>,
Jakub Kicinski <kuba@...nel.org>,
Eric Dumazet <edumazet@...gle.com>,
"David S. Miller" <davem@...emloft.net>,
Florian Fainelli <f.fainelli@...il.com>,
Andrew Lunn <andrew@...n.ch>,
Jianhui Zhao <zhaojh329@...il.com>,
Bjørn Mork <bjorn@...k.no>
Subject: Re: [PATCH 6/9] net: ethernet: mtk_eth_soc: ppe: add support for
flow accounting
On Fri, Feb 03, 2023 at 07:05:08AM +0000, Daniel Golle wrote:
> The PPE units found in MT7622 and newer support packet and byte
> accounting of hw-offloaded flows. Add support for reading those counters
> as found in MediaTek's SDK[1], make them accessible via debugfs and add
> them to the flow offload stats.
>
> [1]: https://git01.mediatek.com/plugins/gitiles/openwrt/feeds/mtk-openwrt-feeds/+/bc6a6a375c800dc2b80e1a325a2c732d1737df92
> Signed-off-by: Daniel Golle <daniel@...rotopia.org>
> ---
> drivers/net/ethernet/mediatek/mtk_eth_soc.c | 8 +-
> drivers/net/ethernet/mediatek/mtk_eth_soc.h | 1 +
> drivers/net/ethernet/mediatek/mtk_ppe.c | 110 +++++++++++++++++-
> drivers/net/ethernet/mediatek/mtk_ppe.h | 24 +++-
> .../net/ethernet/mediatek/mtk_ppe_debugfs.c | 9 +-
> .../net/ethernet/mediatek/mtk_ppe_offload.c | 7 ++
> drivers/net/ethernet/mediatek/mtk_ppe_regs.h | 14 +++
> 7 files changed, 168 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> index f09cd6a132c9..d50dea1f20f3 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -4708,7 +4708,9 @@ static int mtk_probe(struct platform_device *pdev)
> u32 ppe_addr = eth->soc->reg_map->ppe_base + i * 0x400;
>
> eth->ppe[i] = mtk_ppe_init(eth, eth->base + ppe_addr,
> - eth->soc->offload_version, i);
> + eth->soc->offload_version, i,
> + eth->soc->has_accounting);
All arguments mtk_ppe_init() needs (this includes eth->soc->offload_version)
are already available to it. See, first line in mtk_ppe_init() is:
const struct mtk_soc_data *soc = eth->soc;
> +
> if (!eth->ppe[i]) {
> err = -ENOMEM;
> goto err_deinit_ppe;
> diff --git a/drivers/net/ethernet/mediatek/mtk_ppe.c b/drivers/net/ethernet/mediatek/mtk_ppe.c
> index 6883eb34cd8b..26fa89afc69a 100644
> --- a/drivers/net/ethernet/mediatek/mtk_ppe.c
> +++ b/drivers/net/ethernet/mediatek/mtk_ppe.c
> @@ -74,6 +74,46 @@ static int mtk_ppe_wait_busy(struct mtk_ppe *ppe)
> return ret;
> }
>
> +static int mtk_ppe_mib_wait_busy(struct mtk_ppe *ppe)
> +{
> + int ret;
> + u32 val;
> +
> + ret = readl_poll_timeout(ppe->base + MTK_PPE_MIB_SER_CR, val,
> + !(val & MTK_PPE_MIB_SER_CR_ST),
> + 20, MTK_PPE_WAIT_TIMEOUT_US);
> +
> + if (ret)
> + dev_err(ppe->dev, "MIB table busy");
> +
> + return ret;
> +}
> +
> +static int mtk_mib_entry_read(struct mtk_ppe *ppe, u16 index, u64 *bytes, u64 *packets)
> +{
> + u32 val, cnt_r0, cnt_r1, cnt_r2;
> + u32 byte_cnt_low, byte_cnt_high, pkt_cnt_low, pkt_cnt_high;
> +
> + val = FIELD_PREP(MTK_PPE_MIB_SER_CR_ADDR, index) | MTK_PPE_MIB_SER_CR_ST;
> + ppe_w32(ppe, MTK_PPE_MIB_SER_CR, val);
> +
> + if (mtk_ppe_mib_wait_busy(ppe))
> + return -ETIMEDOUT;
err = mtk_ppe_mib_wait_busy();
if (err)
return err;
> +
> + cnt_r0 = readl(ppe->base + MTK_PPE_MIB_SER_R0);
> + cnt_r1 = readl(ppe->base + MTK_PPE_MIB_SER_R1);
> + cnt_r2 = readl(ppe->base + MTK_PPE_MIB_SER_R2);
> +
> + byte_cnt_low = FIELD_GET(MTK_PPE_MIB_SER_R0_BYTE_CNT_LOW, cnt_r0);
> + byte_cnt_high = FIELD_GET(MTK_PPE_MIB_SER_R1_BYTE_CNT_HIGH, cnt_r1);
> + pkt_cnt_low = FIELD_GET(MTK_PPE_MIB_SER_R1_PKT_CNT_LOW, cnt_r1);
> + pkt_cnt_high = FIELD_GET(MTK_PPE_MIB_SER_R2_PKT_CNT_HIGH, cnt_r2);
> + *bytes = ((u64)byte_cnt_high << 32) | byte_cnt_low;
> + *packets = (pkt_cnt_high << 16) | pkt_cnt_low;
> +
> + return 0;
> +}
> +
> static void mtk_ppe_cache_clear(struct mtk_ppe *ppe)
> {
> ppe_set(ppe, MTK_PPE_CACHE_CTL, MTK_PPE_CACHE_CTL_CLEAR);
> struct mtk_ppe *mtk_ppe_init(struct mtk_eth *eth, void __iomem *base,
> - int version, int index)
> + int version, int index, bool accounting)
> {
> const struct mtk_soc_data *soc = eth->soc;
> struct device *dev = eth->dev;
> struct mtk_ppe *ppe;
> u32 foe_flow_size;
> void *foe;
> + struct mtk_mib_entry *mib;
> + struct mtk_foe_accounting *acct;
>
> ppe = devm_kzalloc(dev, sizeof(*ppe), GFP_KERNEL);
> if (!ppe)
> @@ -778,6 +856,7 @@ struct mtk_ppe *mtk_ppe_init(struct mtk_eth *eth, void __iomem *base,
> ppe->eth = eth;
> ppe->dev = dev;
> ppe->version = version;
> + ppe->accounting = accounting;
>
> foe = dmam_alloc_coherent(ppe->dev,
> MTK_PPE_ENTRIES * soc->foe_entry_size,
> @@ -793,6 +872,25 @@ struct mtk_ppe *mtk_ppe_init(struct mtk_eth *eth, void __iomem *base,
> if (!ppe->foe_flow)
> goto err_free_l2_flows;
>
> + if (accounting) {
> + mib = dmam_alloc_coherent(ppe->dev, MTK_PPE_ENTRIES * sizeof(*mib),
> + &ppe->mib_phys, GFP_KERNEL);
> + if (!mib)
> + return NULL;
> +
> + memset(mib, 0, MTK_PPE_ENTRIES * sizeof(*mib));
I remember Jakub pointing out in another email that consistent DMA
memory is already zero-initialized, and it appears in
scripts/coccinelle/api/alloc/zalloc-simple.cocci.
> +
> + ppe->mib_table = mib;
> +
> + acct = devm_kzalloc(dev, MTK_PPE_ENTRIES * sizeof(*acct),
> + GFP_KERNEL);
> +
> + if (!acct)
> + return NULL;
> +
> + ppe->acct_table = acct;
> + }
> +
> mtk_ppe_debugfs_init(ppe, index);
>
> return ppe;
> @@ -922,6 +1020,16 @@ void mtk_ppe_start(struct mtk_ppe *ppe)
> ppe_w32(ppe, MTK_PPE_DEFAULT_CPU_PORT1, 0xcb777);
> ppe_w32(ppe, MTK_PPE_SBW_CTRL, 0x7f);
> }
> +
> + if (ppe->accounting && ppe->mib_phys) {
> + ppe_w32(ppe, MTK_PPE_MIB_TB_BASE, ppe->mib_phys);
> + ppe_m32(ppe, MTK_PPE_MIB_CFG, MTK_PPE_MIB_CFG_EN,
> + MTK_PPE_MIB_CFG_EN);
> + ppe_m32(ppe, MTK_PPE_MIB_CFG, MTK_PPE_MIB_CFG_RD_CLR,
> + MTK_PPE_MIB_CFG_RD_CLR);
> + ppe_m32(ppe, MTK_PPE_MIB_CACHE_CTL, MTK_PPE_MIB_CACHE_CTL_EN,
> + MTK_PPE_MIB_CFG_RD_CLR);
> + }
> }
>
> int mtk_ppe_stop(struct mtk_ppe *ppe)
> diff --git a/drivers/net/ethernet/mediatek/mtk_ppe_debugfs.c b/drivers/net/ethernet/mediatek/mtk_ppe_debugfs.c
> index 391b071bcff3..39775740340b 100644
> --- a/drivers/net/ethernet/mediatek/mtk_ppe_debugfs.c
> +++ b/drivers/net/ethernet/mediatek/mtk_ppe_debugfs.c
> @@ -82,6 +82,7 @@ mtk_ppe_debugfs_foe_show(struct seq_file *m, void *private, bool bind)
> struct mtk_foe_entry *entry = mtk_foe_get_entry(ppe, i);
> struct mtk_foe_mac_info *l2;
> struct mtk_flow_addr_info ai = {};
> + struct mtk_foe_accounting *acct;
> unsigned char h_source[ETH_ALEN];
> unsigned char h_dest[ETH_ALEN];
> int type, state;
> @@ -95,6 +96,8 @@ mtk_ppe_debugfs_foe_show(struct seq_file *m, void *private, bool bind)
> if (bind && state != MTK_FOE_STATE_BIND)
> continue;
>
> + acct = mtk_foe_entry_get_mib(ppe, i, NULL);
might return NULL
> +
> type = FIELD_GET(MTK_FOE_IB1_PACKET_TYPE, entry->ib1);
> seq_printf(m, "%05x %s %7s", i,
> mtk_foe_entry_state_str(state),
> @@ -153,9 +156,11 @@ mtk_ppe_debugfs_foe_show(struct seq_file *m, void *private, bool bind)
> *((__be16 *)&h_dest[4]) = htons(l2->dest_mac_lo);
>
> seq_printf(m, " eth=%pM->%pM etype=%04x"
> - " vlan=%d,%d ib1=%08x ib2=%08x\n",
> + " vlan=%d,%d ib1=%08x ib2=%08x"
> + " packets=%lld bytes=%lld\n",
%llu
> h_source, h_dest, ntohs(l2->etype),
> - l2->vlan1, l2->vlan2, entry->ib1, ib2);
> + l2->vlan1, l2->vlan2, entry->ib1, ib2,
> + acct->packets, acct->bytes);
> }
>
> return 0;
> diff --git a/drivers/net/ethernet/mediatek/mtk_ppe_offload.c b/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
> index 81afd5ee3fbf..832e11ad9a16 100644
> --- a/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
> +++ b/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
> @@ -497,6 +497,7 @@ static int
> mtk_flow_offload_stats(struct mtk_eth *eth, struct flow_cls_offload *f)
> {
> struct mtk_flow_entry *entry;
> + struct mtk_foe_accounting diff;
> u32 idle;
>
> entry = rhashtable_lookup(ð->flow_table, &f->cookie,
> @@ -507,6 +508,12 @@ mtk_flow_offload_stats(struct mtk_eth *eth, struct flow_cls_offload *f)
> idle = mtk_foe_entry_idle_time(eth->ppe[entry->ppe_index], entry);
> f->stats.lastused = jiffies - idle * HZ;
>
> + if (entry->hash != 0xFFFF) {
> + mtk_foe_entry_get_mib(eth->ppe[entry->ppe_index], entry->hash, &diff);
If this returns NULL, you don't want to add diff.packets and diff.bytes
to f->stats, because no one bothers to initialize "diff" to all-zeroes.
So it contains junk from kernel stack memory. You might want to avoid this.
> + f->stats.pkts += diff.packets;
> + f->stats.bytes += diff.bytes;
> + }
> +
> return 0;
> }
>
Powered by blists - more mailing lists