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