From: "Clément Léger" <clement.leger@bootlin.com>
To: Vladimir Oltean <vladimir.oltean@nxp.com>
Cc: "David S. Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>,
Rob Herring <robh+dt@kernel.org>,
Claudiu Manoil <claudiu.manoil@nxp.com>,
Alexandre Belloni <alexandre.belloni@bootlin.com>,
"UNGLinuxDriver@microchip.com" <UNGLinuxDriver@microchip.com>,
Andrew Lunn <andrew@lunn.ch>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Subject: Re: [PATCH v2 3/6] net: ocelot: pre-compute injection frame header content
Date: Wed, 3 Nov 2021 14:53:51 +0100 [thread overview]
Message-ID: <20211103145351.793538c3@fixe.home> (raw)
In-Reply-To: <20211103123811.im5ua7kirogoltm7@skbuf>
Le Wed, 3 Nov 2021 12:38:12 +0000,
Vladimir Oltean <vladimir.oltean@nxp.com> a écrit :
> On Wed, Nov 03, 2021 at 10:19:40AM +0100, Clément Léger wrote:
> > IFH preparation can take quite some time on slow processors (up to
> > 5% in a iperf3 test for instance). In order to reduce the cost of
> > this preparation, pre-compute IFH since most of the parameters are
> > fixed per port. Only rew_op and vlan tag will be set when sending
> > if different than 0. This allows to remove entirely the calls to
> > packing() with basic usage. In the same time, export this function
> > that will be used by FDMA.
> >
> > Signed-off-by: Clément Léger <clement.leger@bootlin.com>
> > ---
>
> Honestly, this feels a bit cheap/gimmicky, and not really the
> fundamental thing to address. In my testing of a similar idea (see
> commits 67c2404922c2 ("net: dsa: felix: create a template for the DSA
> tags on xmit") and then 7c4bb540e917 ("net: dsa: tag_ocelot: create
> separate tagger for Seville"), the net difference is not that stark,
> considering that now you need to access one more memory region which
> you did not need before, do a memcpy, and then patch the IFH anyway
> for the non-constant stuff.
The memcpy is neglectable and the patching happens only in a few
cases (at least vs the packing function call). The VSC7514 CPU is really
slow and lead to 2.5% up to 5% time spent in packing() when using iperf3
and depending on the use case (according to ftrace).
>
> Certainly, for the calls to ocelot_port_inject_frame() from DSA, I
> would prefer not having this pre-computed IFH.
>
> Could you provide some before/after performance numbers and perf
> counters?
I will make another round of measure to confirm my previous number and
check the impact on the injection rate on ocelot.
>
> > drivers/net/ethernet/mscc/ocelot.c | 23 ++++++++++++++++++-----
> > include/soc/mscc/ocelot.h | 5 +++++
> > 2 files changed, 23 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/mscc/ocelot.c
> > b/drivers/net/ethernet/mscc/ocelot.c index
> > e6c18b598d5c..97693772595b 100644 ---
> > a/drivers/net/ethernet/mscc/ocelot.c +++
> > b/drivers/net/ethernet/mscc/ocelot.c @@ -1076,20 +1076,29 @@ bool
> > ocelot_can_inject(struct ocelot *ocelot, int grp) }
> > EXPORT_SYMBOL(ocelot_can_inject);
> >
> > +void ocelot_ifh_port_set(void *ifh, struct ocelot_port *port, u32
> > rew_op,
> > + u32 vlan_tag)
> > +{
> > + memcpy(ifh, port->ifh, OCELOT_TAG_LEN);
> > +
> > + if (vlan_tag)
> > + ocelot_ifh_set_vlan_tci(ifh, vlan_tag);
> > + if (rew_op)
> > + ocelot_ifh_set_rew_op(ifh, rew_op);
> > +}
> > +EXPORT_SYMBOL(ocelot_ifh_port_set);
> > +
> > void ocelot_port_inject_frame(struct ocelot *ocelot, int port, int
> > grp, u32 rew_op, struct sk_buff *skb)
> > {
> > + struct ocelot_port *port_s = ocelot->ports[port];
> > u32 ifh[OCELOT_TAG_LEN / 4] = {0};
> > unsigned int i, count, last;
> >
> > ocelot_write_rix(ocelot, QS_INJ_CTRL_GAP_SIZE(1) |
> > QS_INJ_CTRL_SOF, QS_INJ_CTRL, grp);
> >
> > - ocelot_ifh_set_bypass(ifh, 1);
> > - ocelot_ifh_set_dest(ifh, BIT_ULL(port));
> > - ocelot_ifh_set_tag_type(ifh, IFH_TAG_TYPE_C);
> > - ocelot_ifh_set_vlan_tci(ifh, skb_vlan_tag_get(skb));
> > - ocelot_ifh_set_rew_op(ifh, rew_op);
> > + ocelot_ifh_port_set(ifh, port_s, rew_op,
> > skb_vlan_tag_get(skb));
> > for (i = 0; i < OCELOT_TAG_LEN / 4; i++)
> > ocelot_write_rix(ocelot, ifh[i], QS_INJ_WR, grp);
> > @@ -2128,6 +2137,10 @@ void ocelot_init_port(struct ocelot *ocelot,
> > int port)
> > skb_queue_head_init(&ocelot_port->tx_skbs);
> >
> > + ocelot_ifh_set_bypass(ocelot_port->ifh, 1);
> > + ocelot_ifh_set_dest(ocelot_port->ifh, BIT_ULL(port));
> > + ocelot_ifh_set_tag_type(ocelot_port->ifh, IFH_TAG_TYPE_C);
> > +
> > /* Basic L2 initialization */
> >
> > /* Set MAC IFG Gaps
> > diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
> > index fef3a36b0210..b3381c90ff3e 100644
> > --- a/include/soc/mscc/ocelot.h
> > +++ b/include/soc/mscc/ocelot.h
> > @@ -6,6 +6,7 @@
> > #define _SOC_MSCC_OCELOT_H
> >
> > #include <linux/ptp_clock_kernel.h>
> > +#include <linux/dsa/ocelot.h>
> > #include <linux/net_tstamp.h>
> > #include <linux/if_vlan.h>
> > #include <linux/regmap.h>
> > @@ -623,6 +624,8 @@ struct ocelot_port {
> >
> > struct net_device *bridge;
> > u8 stp_state;
> > +
> > + u8 ifh[OCELOT_TAG_LEN];
> > };
> >
> > struct ocelot {
> > @@ -754,6 +757,8 @@ void __ocelot_target_write_ix(struct ocelot
> > *ocelot, enum ocelot_target target, bool ocelot_can_inject(struct
> > ocelot *ocelot, int grp); void ocelot_port_inject_frame(struct
> > ocelot *ocelot, int port, int grp, u32 rew_op, struct sk_buff *skb);
> > +void ocelot_ifh_port_set(void *ifh, struct ocelot_port *port, u32
> > rew_op,
> > + u32 vlan_tag);
> > int ocelot_xtr_poll_frame(struct ocelot *ocelot, int grp, struct
> > sk_buff **skb); void ocelot_drain_cpu_queue(struct ocelot *ocelot,
> > int grp);
> > --
> > 2.33.0
>
--
Clément Léger,
Embedded Linux and Kernel engineer at Bootlin
https://bootlin.com
next prev parent reply other threads:[~2021-11-03 13:54 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-03 9:19 [PATCH v2 0/6] Add FDMA support on ocelot switch driver Clément Léger
2021-11-03 9:19 ` [PATCH v2 1/6] net: ocelot: add support to get port mac from device-tree Clément Léger
2021-11-03 10:26 ` Vladimir Oltean
2021-11-15 11:19 ` Julian Wiedmann
2021-11-15 11:24 ` Clément Léger
2021-11-03 9:19 ` [PATCH v2 2/6] dt-bindings: net: convert mscc,vsc7514-switch bindings to yaml Clément Léger
2021-11-03 10:45 ` Vladimir Oltean
2021-11-08 11:13 ` Clément Léger
2021-11-12 20:06 ` Rob Herring
2021-11-03 9:19 ` [PATCH v2 3/6] net: ocelot: pre-compute injection frame header content Clément Léger
2021-11-03 12:38 ` Vladimir Oltean
2021-11-03 13:53 ` Clément Léger [this message]
2021-11-15 10:13 ` Clément Léger
2021-11-15 10:51 ` Vladimir Oltean
2021-11-15 10:58 ` Clément Léger
2021-11-15 14:08 ` Jakub Kicinski
2021-11-15 14:06 ` Clément Léger
2021-11-15 14:31 ` Vladimir Oltean
2021-11-15 16:03 ` Clément Léger
2021-11-03 9:19 ` [PATCH v2 4/6] net: ocelot: add support for ndo_change_mtu Clément Léger
2021-11-03 12:40 ` Vladimir Oltean
2021-11-03 13:07 ` Clément Léger
2021-11-03 9:19 ` [PATCH v2 5/6] net: ocelot: add FDMA support Clément Léger
2021-11-03 11:25 ` Denis Kirjanov
2021-11-03 12:31 ` Vladimir Oltean
2021-11-03 14:22 ` Clément Léger
2021-11-03 9:19 ` [PATCH v2 6/6] net: ocelot: add jumbo frame support for FDMA Clément Léger
2021-11-03 12:43 ` Vladimir Oltean
2021-11-03 14:30 ` Clément Léger
2021-11-03 10:46 ` [PATCH v2 0/6] Add FDMA support on ocelot switch driver Denis Kirjanov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20211103145351.793538c3@fixe.home \
--to=clement.leger@bootlin.com \
--cc=UNGLinuxDriver@microchip.com \
--cc=alexandre.belloni@bootlin.com \
--cc=andrew@lunn.ch \
--cc=claudiu.manoil@nxp.com \
--cc=davem@davemloft.net \
--cc=devicetree@vger.kernel.org \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=robh+dt@kernel.org \
--cc=thomas.petazzoni@bootlin.com \
--cc=vladimir.oltean@nxp.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).