Re: [PATCH v2 3/6] net: ocelot: pre-compute injection frame header content

Le Wed, 3 Nov 2021 14:53:51 +0100,
Clément Léger [off-list ref] a écrit :

Le Wed, 3 Nov 2021 12:38:12 +0000,
Vladimir Oltean [off-list ref] a écrit :

quoted

On Wed, Nov 03, 2021 at 10:19:40AM +0100, Clément Léger wrote:  

quoted

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).

quoted

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.

I checked again my bandwith numbers (obtained with iperf3) with and
without the pre-computed header:

Test on standard packets with UDP (iperf3 -t 100 -l 1460 -u -b 0 -c *)
- With pre-computed header: UDP TX: 	33Mbit/s
- Without UDP TX: 			31Mbit/s
-> 6.5% improvement

Test on small packets with UDP (iperf3 -t 100 -l 700 -u -b 0 -c *)
- With pre-computed header: UDP TX: 	15.8Mbit/s
- Without UDP TX: 			16.4Mbit/s
-> 4.3% improvement

The improvement might not be huge but also not negligible at all.
Please tell me if you want me to drop it or not based on those numbers.

quoted

quoted

 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