[v2,1/6] Revert "app/testpmd: modify mac in csum forwarding"

  This patch reverts commit 10f4620f02e1 ("app/testpmd: modify mac in csum forwarding"),
as the checksum forwarding is expected to only perform
checksum and not also overwrites the source and destination
MAC addresses.

Doing so, we can test checksum offloading with real traffic
without breaking broadcast packets.

Fixes: 10f4620f02e1 ("app/testpmd: modify mac in csum forwarding")
Cc: stable@dpdk.org

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Acked-by: Chenbo Xia <chenbo.xia@intel.com>
---
 app/test-pmd/csumonly.c | 4 ----
 1 file changed, 4 deletions(-)
  

Hi Maxime,

On 6/8/2022 6:19 PM, Maxime Coquelin wrote:
> This patch reverts commit 10f4620f02e1 ("app/testpmd: modify mac in csum forwarding"),
> as the checksum forwarding is expected to only perform
> checksum and not also overwrites the source and destination
> MAC addresses.
>
> Doing so, we can test checksum offloading with real traffic
> without breaking broadcast packets.
>
> Fixes: 10f4620f02e1 ("app/testpmd: modify mac in csum forwarding")
> Cc: stable@dpdk.org
>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> Acked-by: Chenbo Xia <chenbo.xia@intel.com>

Acked-by: Aman Singh<aman.deep.singh@intel.com>

> ---
>   app/test-pmd/csumonly.c | 4 ----
>   1 file changed, 4 deletions(-)
>
> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
> index 7df201e047..1a3fd9ce8a 100644
> --- a/app/test-pmd/csumonly.c
> +++ b/app/test-pmd/csumonly.c
> @@ -916,10 +916,6 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
>   		 * and inner headers */
>   
>   		eth_hdr = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
> -		rte_ether_addr_copy(&peer_eth_addrs[fs->peer_addr],
> -				&eth_hdr->dst_addr);
> -		rte_ether_addr_copy(&ports[fs->tx_port].eth_addr,
> -				&eth_hdr->src_addr);
>   		parse_ethernet(eth_hdr, &info);
>   		l3_hdr = (char *)eth_hdr + info.l2_len;
>   

LGTM, In principle csum mode should not modify the mac addresses.
This code has been there from start, so might break some TC's.
  

On 6/13/22 14:24, Singh, Aman Deep wrote:
> Hi Maxime,
> 
> On 6/8/2022 6:19 PM, Maxime Coquelin wrote:
>> This patch reverts commit 10f4620f02e1 ("app/testpmd: modify mac in 
>> csum forwarding"),
>> as the checksum forwarding is expected to only perform
>> checksum and not also overwrites the source and destination
>> MAC addresses.
>>
>> Doing so, we can test checksum offloading with real traffic
>> without breaking broadcast packets.
>>
>> Fixes: 10f4620f02e1 ("app/testpmd: modify mac in csum forwarding")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Acked-by: Chenbo Xia <chenbo.xia@intel.com>
> 
> Acked-by: Aman Singh<aman.deep.singh@intel.com>
> 
>> ---
>>   app/test-pmd/csumonly.c | 4 ----
>>   1 file changed, 4 deletions(-)
>>
>> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
>> index 7df201e047..1a3fd9ce8a 100644
>> --- a/app/test-pmd/csumonly.c
>> +++ b/app/test-pmd/csumonly.c
>> @@ -916,10 +916,6 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
>>            * and inner headers */
>>           eth_hdr = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
>> -        rte_ether_addr_copy(&peer_eth_addrs[fs->peer_addr],
>> -                &eth_hdr->dst_addr);
>> -        rte_ether_addr_copy(&ports[fs->tx_port].eth_addr,
>> -                &eth_hdr->src_addr);
>>           parse_ethernet(eth_hdr, &info);
>>           l3_hdr = (char *)eth_hdr + info.l2_len;
> 
> LGTM, In principle csum mode should not modify the mac addresses.
> This code has been there from start, so might break some TC's.
> 
> 

Agree, some tests will need to be adapted.
David showed me some tests in DTS were removing the MAC rewriting.

Thanks,
Maxime