[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <Y2GYO9muWSmRypnF@makrotopia.org>
Date: Tue, 1 Nov 2022 22:05:47 +0000
From: Daniel Golle <daniel@...rotopia.org>
To: Felix Fietkau <nbd@....name>
Cc: John Crispin <john@...ozen.org>,
Sean Wang <sean.wang@...iatek.com>,
Mark Lee <Mark-MC.Lee@...iatek.com>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Matthias Brugger <matthias.bgg@...il.com>,
netdev@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] net: ethernet: mediatek: ppe: add support for flow
accounting
Hi Felix,
On Tue, Nov 01, 2022 at 08:38:59PM +0100, Felix Fietkau wrote:
> On 01.11.22 20:12, 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].
> >
> > [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 | 11 +-
> > drivers/net/ethernet/mediatek/mtk_eth_soc.h | 1 +
> > drivers/net/ethernet/mediatek/mtk_ppe.c | 122 +++++++++++++++++-
> > drivers/net/ethernet/mediatek/mtk_ppe.h | 23 +++-
> > .../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, 178 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > index 789268b15106ec..5fcd66fca7a089 100644
> > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > @@ -4221,6 +4223,7 @@ static const struct mtk_soc_data mt2701_data = {
> > .hw_features = MTK_HW_FEATURES,
> > .required_clks = MT7623_CLKS_BITMAP,
> > .required_pctl = true,
> > + .has_accounting = false,
> > .txrx = {
> > .txd_size = sizeof(struct mtk_tx_dma),
> > .rxd_size = sizeof(struct mtk_rx_dma),
> > @@ -4239,6 +4242,7 @@ static const struct mtk_soc_data mt7621_data = {
> > .required_pctl = false,
> > .offload_version = 2,
> > .hash_offset = 2,
> > + .has_accounting = false,
> > .foe_entry_size = sizeof(struct mtk_foe_entry) - 16,
> > .txrx = {
> > .txd_size = sizeof(struct mtk_tx_dma),
> > @@ -4259,6 +4263,7 @@ static const struct mtk_soc_data mt7622_data = {
> > .required_pctl = false,
> > .offload_version = 2,
> > .hash_offset = 2,
> > + .has_accounting = true,
> > .foe_entry_size = sizeof(struct mtk_foe_entry) - 16,
> > .txrx = {
> > .txd_size = sizeof(struct mtk_tx_dma),
> > @@ -4278,6 +4283,7 @@ static const struct mtk_soc_data mt7623_data = {
> > .required_pctl = true,
> > .offload_version = 2,
> > .hash_offset = 2,
> > + .has_accounting = false,
> > .foe_entry_size = sizeof(struct mtk_foe_entry) - 16,
> > .txrx = {
> > .txd_size = sizeof(struct mtk_tx_dma),
> > @@ -4296,6 +4302,7 @@ static const struct mtk_soc_data mt7629_data = {
> > .hw_features = MTK_HW_FEATURES,
> > .required_clks = MT7629_CLKS_BITMAP,
> > .required_pctl = false,
> > + .has_accounting = true,
> > .txrx = {
> > .txd_size = sizeof(struct mtk_tx_dma),
> > .rxd_size = sizeof(struct mtk_rx_dma),
> > @@ -4315,6 +4322,7 @@ static const struct mtk_soc_data mt7986_data = {
> > .required_pctl = false,
> > .hash_offset = 4,
> > .foe_entry_size = sizeof(struct mtk_foe_entry),
> > + .has_accounting = true,
> > .txrx = {
> > .txd_size = sizeof(struct mtk_tx_dma_v2),
> > .rxd_size = sizeof(struct mtk_rx_dma_v2),
> > @@ -4331,6 +4339,7 @@ static const struct mtk_soc_data rt5350_data = {
> > .hw_features = MTK_HW_FEATURES_MT7628,
> > .required_clks = MT7628_CLKS_BITMAP,
> > .required_pctl = false,
> > + .has_accounting = false,
> > .txrx = {
> > .txd_size = sizeof(struct mtk_tx_dma),
> > .rxd_size = sizeof(struct mtk_rx_dma),
> You can leave out the .has_accounting = false assignments.
Ack.
>
> > diff --git a/drivers/net/ethernet/mediatek/mtk_ppe.c b/drivers/net/ethernet/mediatek/mtk_ppe.c
> > index 2d8ca99f2467ff..bc4660da28451b 100644
> > --- a/drivers/net/ethernet/mediatek/mtk_ppe.c
> > +++ b/drivers/net/ethernet/mediatek/mtk_ppe.c
> > @@ -545,6 +592,16 @@ __mtk_foe_entry_commit(struct mtk_ppe *ppe, struct mtk_foe_entry *entry,
> > wmb();
> > hwe->ib1 = entry->ib1;
> > + if (ppe->accounting) {
> > + int type;
> > +
> > + type = FIELD_GET(MTK_FOE_IB1_PACKET_TYPE, entry->ib1);
> > + if (type >= MTK_PPE_PKT_TYPE_IPV4_DSLITE)
> > + hwe->ipv6.ib2 |= MTK_FOE_IB2_MIB_CNT;
> > + else
> > + hwe->ipv4.ib2 |= MTK_FOE_IB2_MIB_CNT;
> > + }
> Use mtk_foe_entry_ib2() here.
Ack, turns it into a one-liner ;)
>
> > @@ -654,11 +711,8 @@ void __mtk_ppe_check_skb(struct mtk_ppe *ppe, struct sk_buff *skb, u16 hash)
> > continue;
> > }
> > - if (found || !mtk_flow_entry_match(ppe->eth, entry, hwe)) {
> > - if (entry->hash != 0xffff)
> > - entry->hash = 0xffff;
> > + if (found || !mtk_flow_entry_match(ppe->eth, entry, hwe))
> > continue;
> > - }
> What is the reason for this change?
This has slipped in accidentally as it was part of the patch in
mtk-openwrt-feeds. I suppose it may be related to the rather mystical
2nd part of the commit message there:
Patches-4 fix __schedule_bug issue in the __mtk_foe_entry_clear().
However, as things seem to work just fine also without that I will send
v3 skipping this part.
Thanks for the review!
Daniel
Powered by blists - more mailing lists