diff mbox series

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

Message ID 20220505102729.821075-2-maxime.coquelin@redhat.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers
Series Vhost checksum offload improvements |

Checks

Context Check Description
ci/checkpatch warning coding style issues

Commit Message

Maxime Coquelin May 5, 2022, 10:27 a.m. UTC 
  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 overwritte 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>
---
 app/test-pmd/csumonly.c | 4 ----
 1 file changed, 4 deletions(-)

Comments

Chenbo Xia May 16, 2022, 1:03 p.m. UTC | #1 
+ testpmd maintainers

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Thursday, May 5, 2022 6:27 PM
> To: dev@dpdk.org; jasowang@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>;
> david.marchand@redhat.com; olivier.matz@6wind.com
> Cc: stable@dpdk.org; Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [PATCH 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 overwritte the source and destination

overwrites

> 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

With above fixed:

Acked-by: Chenbo Xia <chenbo.xia@intel.com>

> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.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 cdb1920763..8b6665d6f3 100644
> --- a/app/test-pmd/csumonly.c
> +++ b/app/test-pmd/csumonly.c
> @@ -894,10 +894,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;
> 
> --
> 2.35.1
Zhang, Yuying May 17, 2022, 3:24 p.m. UTC | #2 
Hi Coquelin,

Please fix the code style issue of commit log.

> -----Original Message-----
> From: Xia, Chenbo <chenbo.xia@intel.com>
> Sent: Monday, May 16, 2022 9:04 PM
> To: Maxime Coquelin <maxime.coquelin@redhat.com>; dev@dpdk.org;
> jasowang@redhat.com; david.marchand@redhat.com;
> olivier.matz@6wind.com
> Cc: stable@dpdk.org; Li, Xiaoyun <xiaoyun.li@intel.com>; Zhang, Yuying
> <yuying.zhang@intel.com>; Singh, Aman Deep <aman.deep.singh@intel.com>
> Subject: RE: [PATCH 1/6] Revert "app/testpmd: modify mac in csum
> forwarding"
> 
> + testpmd maintainers
> 
> > -----Original Message-----
> > From: Maxime Coquelin <maxime.coquelin@redhat.com>
> > Sent: Thursday, May 5, 2022 6:27 PM
> > To: dev@dpdk.org; jasowang@redhat.com; Xia, Chenbo
> > <chenbo.xia@intel.com>; david.marchand@redhat.com;
> > olivier.matz@6wind.com
> > Cc: stable@dpdk.org; Maxime Coquelin <maxime.coquelin@redhat.com>
> > Subject: [PATCH 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 overwritte the source and destination
> 
> overwrites
> 
> > 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
> 
> With above fixed:
> 
> Acked-by: Chenbo Xia <chenbo.xia@intel.com>
> 
> >
> > Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.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
> > cdb1920763..8b6665d6f3 100644
> > --- a/app/test-pmd/csumonly.c
> > +++ b/app/test-pmd/csumonly.c
> > @@ -894,10 +894,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;
> >
> > --
> > 2.35.1
David Marchand May 19, 2022, 4:27 p.m. UTC | #3 
On Thu, May 5, 2022 at 12:27 PM Maxime Coquelin
<maxime.coquelin@redhat.com> 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 overwritte 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>

Thanks Maxime, this lgtm.

Cc: dts ml since dts was dropping this code for some tests.

origin/main:test_plans/dpdk_gro_lib_cbdma_test_plan.rst:
     ether_addr_copy(&peer_eth_addrs[fs->peer_addr],
origin/main:test_plans/dpdk_gro_lib_cbdma_test_plan.rst:
     ether_addr_copy(&ports[fs->tx_port].eth_addr,
origin/main:test_plans/dpdk_gro_lib_test_plan.rst:
ether_addr_copy(&peer_eth_addrs[fs->peer_addr],
origin/main:test_plans/dpdk_gro_lib_test_plan.rst:
ether_addr_copy(&ports[fs->tx_port].eth_addr,
origin/main:test_plans/dpdk_gso_lib_test_plan.rst:
ether_addr_copy(&peer_eth_addrs[fs->peer_addr],
origin/main:test_plans/dpdk_gso_lib_test_plan.rst:
ether_addr_copy(&ports[fs->tx_port].eth_addr,
origin/main:test_plans/virtio_user_as_exceptional_path_test_plan.rst:
                  ether_addr_copy(&peer_eth_addrs[fs->peer_addr],
origin/main:test_plans/virtio_user_as_exceptional_path_test_plan.rst:
                  ether_addr_copy(&ports[fs->tx_port].eth_addr,
origin/main:tests/TestSuite_dpdk_gro_lib.py:            "sed -i
'/ether_addr_copy(&peer_eth/i\#if 0' ./app/test-pmd/csumonly.c", "#"
origin/main:tests/TestSuite_dpdk_gro_lib_cbdma.py:            "sed -i
'/ether_addr_copy(&peer_eth/i\#if 0' ./app/test-pmd/csumonly.c", "#"
origin/main:tests/TestSuite_dpdk_gso_lib.py:            "sed -i
'/ether_addr_copy(&peer_eth/i\#if 0' ./app/test-pmd/csumonly.c", "#"
origin/main:tests/TestSuite_virtio_user_as_exceptional_path.py:
    "sed -i '/ether_addr_copy(&peer_eth/i\#if 0'
./app/test-pmd/csumonly.c", "#"
diff mbox series

Patch

diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index cdb1920763..8b6665d6f3 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -894,10 +894,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;