diff mbox series

app/testpmd: improve sse based macswap

Message ID 20240716063724.850-1-vipin.varghese@amd.com (mailing list archive)
State Under Review
Delegated to: Ferruh Yigit
Headers
Series app/testpmd: improve sse based macswap |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/github-robot: build success github build: passed
ci/intel-Testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/intel-Functional success Functional PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-intel-Functional success Functional Testing PASS

Commit Message

Varghese, Vipin July 16, 2024, 6:37 a.m. UTC 
Goal of the patch is to improve SSE macswap on x86_64 by reducing
the stalls in backend engine. Original implementation of the SSE
macswap makes loop call to multiple load, shuffle & store. Using
SIMD ISA interleaving we can reduce the stalls for
 - load SSE token exhaustion
 - Shuffle and Load dependency

Also other changes which improves packet per second are
 - Filling access to MBUF for offload flags which is separate cacheline,
 - using register keyword

Build test using meson script:
``````````````````````````````

build-gcc-static
buildtools
build-gcc-shared
build-mini
build-clang-static
build-clang-shared
build-x86-generic

Test Results:
`````````````

Platform-1: AMD EPYC SIENA 8594P @2.3GHz, no boost

------------------------------------------------
TEST IO 64B: baseline <NIC : MPPs>
 - mellanox CX-7 2*200Gbps : 42.0
 - intel E810 1*100Gbps : 82.0
 - intel E810 2*200Gbps (2CQ-DA2): 82.45
------------------------------------------------
TEST MACSWAP 64B: <NIC : Before : After>
 - mellanox CX-7 2*200Gbps : 31.533 : 31.90
 - intel E810 1*100Gbps : 50.380 : 47.0
 - intel E810 2*200Gbps (2CQ-DA2): 48.840 : 49.827
------------------------------------------------
TEST MACSWAP 128B: <NIC : Before: After>
 - mellanox CX-7 2*200Gbps: 30.946 : 31.770
 - intel E810 1*100Gbps: 49.386 : 46.366
 - intel E810 2*200Gbps (2CQ-DA2): 47.979 : 49.503
------------------------------------------------
TEST MACSWAP 256B: <NIC: Before: After>
 - mellanox CX-7 2*200Gbps: 32.480 : 33.150
 - intel E810 1 * 100Gbps: 45.29 : 44.571
 - intel E810 2 * 200Gbps (2CQ-DA2): 45.033 : 45.117
------------------------------------------------

Platform-2: AMD EPYC 9554 @3.1GHz, no boost

------------------------------------------------
TEST IO 64B: baseline <NIC : MPPs>
 - intel E810 2*200Gbps (2CQ-DA2): 82.49
------------------------------------------------
<NIC intel E810 2*200Gbps (2CQ-DA2): Before : After>
TEST MACSWAP: 1Q 1C1T
 64B: : 45.0 : 45.54
128B: : 44.48 : 44.43
256B: : 42.0 : 41.99
+++++++++++++++++++++++++
TEST MACSWAP: 2Q 2C2T
 64B: : 59.5 : 60.55
128B: : 56.78 : 58.1
256B: : 41.85 : 41.99
------------------------------------------------

Signed-off-by: Vipin Varghese <vipin.varghese@amd.com>
---
 app/test-pmd/macswap_sse.h | 37 +++++++++++++++++--------------------
 1 file changed, 17 insertions(+), 20 deletions(-)

Comments

Ferruh Yigit July 23, 2024, 4:45 p.m. UTC | #1 
On 7/16/2024 7:37 AM, Vipin Varghese wrote:
> Goal of the patch is to improve SSE macswap on x86_64 by reducing
> the stalls in backend engine. Original implementation of the SSE
> macswap makes loop call to multiple load, shuffle & store. Using
> SIMD ISA interleaving we can reduce the stalls for
>  - load SSE token exhaustion
>  - Shuffle and Load dependency
> 
> Also other changes which improves packet per second are
>  - Filling access to MBUF for offload flags which is separate cacheline,
>  - using register keyword
> 
> Build test using meson script:
> ``````````````````````````````
> 
> build-gcc-static
> buildtools
> build-gcc-shared
> build-mini
> build-clang-static
> build-clang-shared
> build-x86-generic
> 
> Test Results:
> `````````````
> 
> Platform-1: AMD EPYC SIENA 8594P @2.3GHz, no boost
> 
> ------------------------------------------------
> TEST IO 64B: baseline <NIC : MPPs>
>  - mellanox CX-7 2*200Gbps : 42.0
>  - intel E810 1*100Gbps : 82.0
>  - intel E810 2*200Gbps (2CQ-DA2): 82.45
> ------------------------------------------------
> TEST MACSWAP 64B: <NIC : Before : After>
>  - mellanox CX-7 2*200Gbps : 31.533 : 31.90
>  - intel E810 1*100Gbps : 50.380 : 47.0
>  - intel E810 2*200Gbps (2CQ-DA2): 48.840 : 49.827
> ------------------------------------------------
> TEST MACSWAP 128B: <NIC : Before: After>
>  - mellanox CX-7 2*200Gbps: 30.946 : 31.770
>  - intel E810 1*100Gbps: 49.386 : 46.366
>  - intel E810 2*200Gbps (2CQ-DA2): 47.979 : 49.503
> ------------------------------------------------
> TEST MACSWAP 256B: <NIC: Before: After>
>  - mellanox CX-7 2*200Gbps: 32.480 : 33.150
>  - intel E810 1 * 100Gbps: 45.29 : 44.571
>  - intel E810 2 * 200Gbps (2CQ-DA2): 45.033 : 45.117
> ------------------------------------------------
> 
> Platform-2: AMD EPYC 9554 @3.1GHz, no boost
> 
> ------------------------------------------------
> TEST IO 64B: baseline <NIC : MPPs>
>  - intel E810 2*200Gbps (2CQ-DA2): 82.49
> ------------------------------------------------
> <NIC intel E810 2*200Gbps (2CQ-DA2): Before : After>
> TEST MACSWAP: 1Q 1C1T
>  64B: : 45.0 : 45.54
> 128B: : 44.48 : 44.43
> 256B: : 42.0 : 41.99
> +++++++++++++++++++++++++
> TEST MACSWAP: 2Q 2C2T
>  64B: : 59.5 : 60.55
> 128B: : 56.78 : 58.1
> 256B: : 41.85 : 41.99
> ------------------------------------------------
> 
> Signed-off-by: Vipin Varghese <vipin.varghese@amd.com>
>

Hi Bruce, John,

Can you please help testing macswap performance with this patch on Intel
platforms, to be sure it is not causing regression?

Other option is to get this patch for -rc3 and tested there, with the
condition to remove it in any regression, if this help testing the patch?

Thanks,
ferruh
Bruce Richardson July 23, 2024, 5:12 p.m. UTC | #2 
On Tue, Jul 23, 2024 at 05:45:57PM +0100, Ferruh Yigit wrote:
> On 7/16/2024 7:37 AM, Vipin Varghese wrote:
> > Goal of the patch is to improve SSE macswap on x86_64 by reducing
> > the stalls in backend engine. Original implementation of the SSE
> > macswap makes loop call to multiple load, shuffle & store. Using
> > SIMD ISA interleaving we can reduce the stalls for
> >  - load SSE token exhaustion
> >  - Shuffle and Load dependency
> > 
> > Also other changes which improves packet per second are
> >  - Filling access to MBUF for offload flags which is separate cacheline,
> >  - using register keyword
> > 
> > Build test using meson script:
> > ``````````````````````````````
> > 
> > build-gcc-static
> > buildtools
> > build-gcc-shared
> > build-mini
> > build-clang-static
> > build-clang-shared
> > build-x86-generic
> > 
> > Test Results:
> > `````````````
> > 
> > Platform-1: AMD EPYC SIENA 8594P @2.3GHz, no boost
> > 
> > ------------------------------------------------
> > TEST IO 64B: baseline <NIC : MPPs>
> >  - mellanox CX-7 2*200Gbps : 42.0
> >  - intel E810 1*100Gbps : 82.0
> >  - intel E810 2*200Gbps (2CQ-DA2): 82.45
> > ------------------------------------------------
> > TEST MACSWAP 64B: <NIC : Before : After>
> >  - mellanox CX-7 2*200Gbps : 31.533 : 31.90
> >  - intel E810 1*100Gbps : 50.380 : 47.0
> >  - intel E810 2*200Gbps (2CQ-DA2): 48.840 : 49.827
> > ------------------------------------------------
> > TEST MACSWAP 128B: <NIC : Before: After>
> >  - mellanox CX-7 2*200Gbps: 30.946 : 31.770
> >  - intel E810 1*100Gbps: 49.386 : 46.366
> >  - intel E810 2*200Gbps (2CQ-DA2): 47.979 : 49.503
> > ------------------------------------------------
> > TEST MACSWAP 256B: <NIC: Before: After>
> >  - mellanox CX-7 2*200Gbps: 32.480 : 33.150
> >  - intel E810 1 * 100Gbps: 45.29 : 44.571
> >  - intel E810 2 * 200Gbps (2CQ-DA2): 45.033 : 45.117
> > ------------------------------------------------
> > 
> > Platform-2: AMD EPYC 9554 @3.1GHz, no boost
> > 
> > ------------------------------------------------
> > TEST IO 64B: baseline <NIC : MPPs>
> >  - intel E810 2*200Gbps (2CQ-DA2): 82.49
> > ------------------------------------------------
> > <NIC intel E810 2*200Gbps (2CQ-DA2): Before : After>
> > TEST MACSWAP: 1Q 1C1T
> >  64B: : 45.0 : 45.54
> > 128B: : 44.48 : 44.43
> > 256B: : 42.0 : 41.99
> > +++++++++++++++++++++++++
> > TEST MACSWAP: 2Q 2C2T
> >  64B: : 59.5 : 60.55
> > 128B: : 56.78 : 58.1
> > 256B: : 41.85 : 41.99
> > ------------------------------------------------
> > 
> > Signed-off-by: Vipin Varghese <vipin.varghese@amd.com>
> >
> 
> Hi Bruce, John,
> 
> Can you please help testing macswap performance with this patch on Intel
> platforms, to be sure it is not causing regression?
> 
Hi Ferruh,

We can try and get some Intel numbers for you, but I think at this point it
is better deferred to 24.11 due to lack of discussion and analysis of the
numbers. This is because the numbers above already show that it is causing
regressions - in fact many of the regressions are larger than the benefits
shown. This may be acceptable, but it would imply that we shouldn't be too
hasty in applying the patch.

Regards,
/Bruce
Varghese, Vipin July 25, 2024, 12:47 p.m. UTC | #3
Unaddressed 
Hi Bruce,

Thanks for highlighting the variance. We found this was an internal test 
bed configuration issue. We are sharing the next version of the same 
patch with updated numbers.


On 7/23/2024 10:42 PM, Bruce Richardson wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> On Tue, Jul 23, 2024 at 05:45:57PM +0100, Ferruh Yigit wrote:
>> On 7/16/2024 7:37 AM, Vipin Varghese wrote:
>>> Goal of the patch is to improve SSE macswap on x86_64 by reducing
>>> the stalls in backend engine. Original implementation of the SSE
>>> macswap makes loop call to multiple load, shuffle & store. Using
>>> SIMD ISA interleaving we can reduce the stalls for
>>>   - load SSE token exhaustion
>>>   - Shuffle and Load dependency
>>>
>>> Also other changes which improves packet per second are
>>>   - Filling access to MBUF for offload flags which is separate cacheline,
>>>   - using register keyword
>>>
>>> Build test using meson script:
>>> ``````````````````````````````
>>>
>>> build-gcc-static
>>> buildtools
>>> build-gcc-shared
>>> build-mini
>>> build-clang-static
>>> build-clang-shared
>>> build-x86-generic
>>>
>>> Test Results:
>>> `````````````
>>>
>>> Platform-1: AMD EPYC SIENA 8594P @2.3GHz, no boost
>>>
>>> ------------------------------------------------
>>> TEST IO 64B: baseline <NIC : MPPs>
>>>   - mellanox CX-7 2*200Gbps : 42.0
>>>   - intel E810 1*100Gbps : 82.0
>>>   - intel E810 2*200Gbps (2CQ-DA2): 82.45
>>> ------------------------------------------------
>>> TEST MACSWAP 64B: <NIC : Before : After>
>>>   - mellanox CX-7 2*200Gbps : 31.533 : 31.90
>>>   - intel E810 1*100Gbps : 50.380 : 47.0
>>>   - intel E810 2*200Gbps (2CQ-DA2): 48.840 : 49.827
>>> ------------------------------------------------
>>> TEST MACSWAP 128B: <NIC : Before: After>
>>>   - mellanox CX-7 2*200Gbps: 30.946 : 31.770
>>>   - intel E810 1*100Gbps: 49.386 : 46.366
>>>   - intel E810 2*200Gbps (2CQ-DA2): 47.979 : 49.503
>>> ------------------------------------------------
>>> TEST MACSWAP 256B: <NIC: Before: After>
>>>   - mellanox CX-7 2*200Gbps: 32.480 : 33.150
>>>   - intel E810 1 * 100Gbps: 45.29 : 44.571
>>>   - intel E810 2 * 200Gbps (2CQ-DA2): 45.033 : 45.117
>>> ------------------------------------------------
>>>
>>> Platform-2: AMD EPYC 9554 @3.1GHz, no boost
>>>
>>> ------------------------------------------------
>>> TEST IO 64B: baseline <NIC : MPPs>
>>>   - intel E810 2*200Gbps (2CQ-DA2): 82.49
>>> ------------------------------------------------
>>> <NIC intel E810 2*200Gbps (2CQ-DA2): Before : After>
>>> TEST MACSWAP: 1Q 1C1T
>>>   64B: : 45.0 : 45.54
>>> 128B: : 44.48 : 44.43
>>> 256B: : 42.0 : 41.99
>>> +++++++++++++++++++++++++
>>> TEST MACSWAP: 2Q 2C2T
>>>   64B: : 59.5 : 60.55
>>> 128B: : 56.78 : 58.1
>>> 256B: : 41.85 : 41.99
>>> ------------------------------------------------
>>>
>>> Signed-off-by: Vipin Varghese<vipin.varghese@amd.com>
>>>
>> Hi Bruce, John,
>>
>> Can you please help testing macswap performance with this patch on Intel
>> platforms, to be sure it is not causing regression?
>>
> Hi Ferruh,
>
> We can try and get some Intel numbers for you, but I think at this point it
> is better deferred to 24.11 due to lack of discussion and analysis of the
> numbers. This is because the numbers above already show that it is causing
> regressions - in fact many of the regressions are larger than the benefits
> shown. This may be acceptable, but it would imply that we shouldn't be too
> hasty in applying the patch.
>
> Regards,
> /Bruce
Bruce Richardson July 25, 2024, 12:52 p.m. UTC | #4 
On Thu, Jul 25, 2024 at 06:17:35PM +0530, Varghese, Vipin wrote:
>    Hi Bruce,
>    Thanks for highlighting the variance. We found this was an internal
>    test bed configuration issue. We are sharing the next version of the
>    same patch with updated numbers.
> 

Great, thanks for the update.
diff mbox series

Patch

diff --git a/app/test-pmd/macswap_sse.h b/app/test-pmd/macswap_sse.h
index 223f87a539..6e4ed21924 100644
--- a/app/test-pmd/macswap_sse.h
+++ b/app/test-pmd/macswap_sse.h
@@ -16,16 +16,16 @@  do_macswap(struct rte_mbuf *pkts[], uint16_t nb,
 	uint64_t ol_flags;
 	int i;
 	int r;
-	__m128i addr0, addr1, addr2, addr3;
+	register __m128i addr0, addr1, addr2, addr3;
 	/**
 	 * shuffle mask be used to shuffle the 16 bytes.
 	 * byte 0-5 wills be swapped with byte 6-11.
 	 * byte 12-15 will keep unchanged.
 	 */
-	__m128i shfl_msk = _mm_set_epi8(15, 14, 13, 12,
-					5, 4, 3, 2,
-					1, 0, 11, 10,
-					9, 8, 7, 6);
+	register const __m128i shfl_msk = _mm_set_epi8(15, 14, 13, 12,
+							5, 4, 3, 2,
+							1, 0, 11, 10,
+							9, 8, 7, 6);
 
 	ol_flags = ol_flags_init(txp->dev_conf.txmode.offloads);
 	vlan_qinq_set(pkts, nb, ol_flags,
@@ -44,23 +44,24 @@  do_macswap(struct rte_mbuf *pkts[], uint16_t nb,
 
 		mb[0] = pkts[i++];
 		eth_hdr[0] = rte_pktmbuf_mtod(mb[0], struct rte_ether_hdr *);
-		addr0 = _mm_loadu_si128((__m128i *)eth_hdr[0]);
-
 		mb[1] = pkts[i++];
 		eth_hdr[1] = rte_pktmbuf_mtod(mb[1], struct rte_ether_hdr *);
-		addr1 = _mm_loadu_si128((__m128i *)eth_hdr[1]);
-
-
 		mb[2] = pkts[i++];
 		eth_hdr[2] = rte_pktmbuf_mtod(mb[2], struct rte_ether_hdr *);
-		addr2 = _mm_loadu_si128((__m128i *)eth_hdr[2]);
-
 		mb[3] = pkts[i++];
 		eth_hdr[3] = rte_pktmbuf_mtod(mb[3], struct rte_ether_hdr *);
-		addr3 = _mm_loadu_si128((__m128i *)eth_hdr[3]);
 
+		/* Interleave loads and shuffle with field set */
+		addr0 = _mm_loadu_si128((__m128i *)eth_hdr[0]);
+		mbuf_field_set(mb[0], ol_flags);
+		addr1 = _mm_loadu_si128((__m128i *)eth_hdr[1]);
+		mbuf_field_set(mb[1], ol_flags);
 		addr0 = _mm_shuffle_epi8(addr0, shfl_msk);
+		addr2 = _mm_loadu_si128((__m128i *)eth_hdr[2]);
+		mbuf_field_set(mb[2], ol_flags);
 		addr1 = _mm_shuffle_epi8(addr1, shfl_msk);
+		addr3 = _mm_loadu_si128((__m128i *)eth_hdr[3]);
+		mbuf_field_set(mb[3], ol_flags);
 		addr2 = _mm_shuffle_epi8(addr2, shfl_msk);
 		addr3 = _mm_shuffle_epi8(addr3, shfl_msk);
 
@@ -69,25 +70,21 @@  do_macswap(struct rte_mbuf *pkts[], uint16_t nb,
 		_mm_storeu_si128((__m128i *)eth_hdr[2], addr2);
 		_mm_storeu_si128((__m128i *)eth_hdr[3], addr3);
 
-		mbuf_field_set(mb[0], ol_flags);
-		mbuf_field_set(mb[1], ol_flags);
-		mbuf_field_set(mb[2], ol_flags);
-		mbuf_field_set(mb[3], ol_flags);
 		r -= 4;
 	}
 
 	for ( ; i < nb; i++) {
-		if (i < nb - 1)
+		if (i < (nb - 1))
 			rte_prefetch0(rte_pktmbuf_mtod(pkts[i+1], void *));
 		mb[0] = pkts[i];
 		eth_hdr[0] = rte_pktmbuf_mtod(mb[0], struct rte_ether_hdr *);
 
 		/* Swap dest and src mac addresses. */
 		addr0 = _mm_loadu_si128((__m128i *)eth_hdr[0]);
+		/* invoke field_set as it is on separate cacheline */
+		mbuf_field_set(mb[0], ol_flags);
 		addr0 = _mm_shuffle_epi8(addr0, shfl_msk);
 		_mm_storeu_si128((__m128i *)eth_hdr[0], addr0);
-
-		mbuf_field_set(mb[0], ol_flags);
 	}
 }