* [PATCH v2 1/6] Revert "app/testpmd: modify mac in csum forwarding" [not found] <20220608124946.102623-1-maxime.coquelin@redhat.com> @ 2022-06-08 12:49 ` Maxime Coquelin 2022-06-13 12:24 ` Singh, Aman Deep 2022-06-08 12:49 ` [PATCH v2 2/6] vhost: fix missing enqueue pseudo-header calculation Maxime Coquelin 1 sibling, 1 reply; 4+ messages in thread From: Maxime Coquelin @ 2022-06-08 12:49 UTC (permalink / raw) To: dev, jasowang, chenbo.xia, david.marchand, olivier.matz, wenwux.ma, yuying.zhang, aman.deep.singh Cc: Maxime Coquelin, stable 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(-) 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], - ð_hdr->dst_addr); - rte_ether_addr_copy(&ports[fs->tx_port].eth_addr, - ð_hdr->src_addr); parse_ethernet(eth_hdr, &info); l3_hdr = (char *)eth_hdr + info.l2_len; -- 2.35.3 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 1/6] Revert "app/testpmd: modify mac in csum forwarding" 2022-06-08 12:49 ` [PATCH v2 1/6] Revert "app/testpmd: modify mac in csum forwarding" Maxime Coquelin @ 2022-06-13 12:24 ` Singh, Aman Deep 2022-06-17 12:42 ` Maxime Coquelin 0 siblings, 1 reply; 4+ messages in thread From: Singh, Aman Deep @ 2022-06-13 12:24 UTC (permalink / raw) To: Maxime Coquelin, dev, jasowang, chenbo.xia, david.marchand, olivier.matz, wenwux.ma, yuying.zhang Cc: stable 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], > - ð_hdr->dst_addr); > - rte_ether_addr_copy(&ports[fs->tx_port].eth_addr, > - ð_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. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 1/6] Revert "app/testpmd: modify mac in csum forwarding" 2022-06-13 12:24 ` Singh, Aman Deep @ 2022-06-17 12:42 ` Maxime Coquelin 0 siblings, 0 replies; 4+ messages in thread From: Maxime Coquelin @ 2022-06-17 12:42 UTC (permalink / raw) To: Singh, Aman Deep, dev, jasowang, chenbo.xia, david.marchand, olivier.matz, wenwux.ma, yuying.zhang Cc: stable 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], >> - ð_hdr->dst_addr); >> - rte_ether_addr_copy(&ports[fs->tx_port].eth_addr, >> - ð_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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2 2/6] vhost: fix missing enqueue pseudo-header calculation [not found] <20220608124946.102623-1-maxime.coquelin@redhat.com> 2022-06-08 12:49 ` [PATCH v2 1/6] Revert "app/testpmd: modify mac in csum forwarding" Maxime Coquelin @ 2022-06-08 12:49 ` Maxime Coquelin 1 sibling, 0 replies; 4+ messages in thread From: Maxime Coquelin @ 2022-06-08 12:49 UTC (permalink / raw) To: dev, jasowang, chenbo.xia, david.marchand, olivier.matz, wenwux.ma, yuying.zhang, aman.deep.singh Cc: Maxime Coquelin, stable The Virtio specification requires that in case of checksum offloading, the pseudo-header checksum must be set in the L4 header. When received from another Vhost-user port, the packet checksum might already contain the pseudo-header checksum but we have no way to know it. So we have no other choice than doing the pseudo-header checksum systematically. This patch handles this using the rte_net_intel_cksum_prepare() helper. Fixes: 859b480d5afd ("vhost: add guest offload setting") Cc: stable@dpdk.org Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> Reviewed-by: Chenbo Xia <chenbo.xia@intel.com> --- lib/vhost/virtio_net.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c index 68a26eb17d..ce22e3ac79 100644 --- a/lib/vhost/virtio_net.c +++ b/lib/vhost/virtio_net.c @@ -596,6 +596,16 @@ virtio_enqueue_offload(struct rte_mbuf *m_buf, struct virtio_net_hdr *net_hdr) csum_l4 |= RTE_MBUF_F_TX_TCP_CKSUM; if (csum_l4) { + /* + * Pseudo-header checksum must be set as per Virtio spec. + * + * Note: We don't propagate rte_net_intel_cksum_prepare() + * errors, as it would have an impact on performance, and an + * error would mean the packet is dropped by the guest instead + * of being dropped here. + */ + rte_net_intel_cksum_prepare(m_buf); + net_hdr->flags = VIRTIO_NET_HDR_F_NEEDS_CSUM; net_hdr->csum_start = m_buf->l2_len + m_buf->l3_len; -- 2.35.3 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-06-17 12:42 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20220608124946.102623-1-maxime.coquelin@redhat.com> 2022-06-08 12:49 ` [PATCH v2 1/6] Revert "app/testpmd: modify mac in csum forwarding" Maxime Coquelin 2022-06-13 12:24 ` Singh, Aman Deep 2022-06-17 12:42 ` Maxime Coquelin 2022-06-08 12:49 ` [PATCH v2 2/6] vhost: fix missing enqueue pseudo-header calculation Maxime Coquelin
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).