* [dpdk-dev] [PATCH] vhost: fix IPv4 csum calculation
@ 2019-10-24 14:28 Flavio Leitner
2019-10-24 14:32 ` Maxime Coquelin
2019-10-24 16:29 ` [dpdk-dev] [PATCH v2] " Flavio Leitner
0 siblings, 2 replies; 6+ messages in thread
From: Flavio Leitner @ 2019-10-24 14:28 UTC (permalink / raw)
To: dev; +Cc: Maxime Coquelin, Tiwei Bie, Obrembski MichalX
Currently the IPv4 header checksum is calculated including its
current value, which can be a valid checksum or just garbage.
In any case, if the original value is not zero, then the result
is always wrong.
The IPv4 checksum is defined in RFC791, page 14 says:
Header Checksum: 16 bits
The checksum algorithm is:
The checksum field is the 16 bit one's complement of the one's
complement sum of all 16 bit words in the header. For purposes of
computing the checksum, the value of the checksum field is zero.
Thus force the csum field to always be zero.
Signed-off-by: Flavio Leitner <fbl@sysclose.org>
---
lib/librte_vhost/virtio_net.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c
index eae7825f04..cde7498c76 100644
--- a/lib/librte_vhost/virtio_net.c
+++ b/lib/librte_vhost/virtio_net.c
@@ -445,6 +445,7 @@ virtio_enqueue_offload(struct rte_mbuf *m_buf, struct virtio_net_hdr *net_hdr)
ipv4_hdr = rte_pktmbuf_mtod_offset(m_buf, struct rte_ipv4_hdr *,
m_buf->l2_len);
+ ipv4_hdr->hdr_checksum = 0;
ipv4_hdr->hdr_checksum = rte_ipv4_cksum(ipv4_hdr);
}
--
2.20.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH] vhost: fix IPv4 csum calculation
2019-10-24 14:28 [dpdk-dev] [PATCH] vhost: fix IPv4 csum calculation Flavio Leitner
@ 2019-10-24 14:32 ` Maxime Coquelin
2019-10-24 16:31 ` Flavio Leitner
2019-10-24 16:29 ` [dpdk-dev] [PATCH v2] " Flavio Leitner
1 sibling, 1 reply; 6+ messages in thread
From: Maxime Coquelin @ 2019-10-24 14:32 UTC (permalink / raw)
To: Flavio Leitner, dev; +Cc: Tiwei Bie, Obrembski MichalX, stable
Hi Flavio,
On 10/24/19 4:28 PM, Flavio Leitner wrote:
> Currently the IPv4 header checksum is calculated including its
> current value, which can be a valid checksum or just garbage.
> In any case, if the original value is not zero, then the result
> is always wrong.
>
> The IPv4 checksum is defined in RFC791, page 14 says:
> Header Checksum: 16 bits
>
> The checksum algorithm is:
> The checksum field is the 16 bit one's complement of the one's
> complement sum of all 16 bit words in the header. For purposes of
> computing the checksum, the value of the checksum field is zero.
>
> Thus force the csum field to always be zero.
I think we need Fixes tag:
Fixes: b08b8cfeb2ae ("vhost: fix IP checksum")
Cc: stable@dpdk.org
Also adding stable in Cc.
> Signed-off-by: Flavio Leitner <fbl@sysclose.org>
> ---
> lib/librte_vhost/virtio_net.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c
> index eae7825f04..cde7498c76 100644
> --- a/lib/librte_vhost/virtio_net.c
> +++ b/lib/librte_vhost/virtio_net.c
> @@ -445,6 +445,7 @@ virtio_enqueue_offload(struct rte_mbuf *m_buf, struct virtio_net_hdr *net_hdr)
>
> ipv4_hdr = rte_pktmbuf_mtod_offset(m_buf, struct rte_ipv4_hdr *,
> m_buf->l2_len);
> + ipv4_hdr->hdr_checksum = 0;
> ipv4_hdr->hdr_checksum = rte_ipv4_cksum(ipv4_hdr);
> }
>
>
Other than that:
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
I'll add the Fixes tag while applying.
Thanks!
Maxime
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [dpdk-dev] [PATCH] vhost: fix IPv4 csum calculation
2019-10-24 14:32 ` Maxime Coquelin
@ 2019-10-24 16:31 ` Flavio Leitner
2019-10-24 16:32 ` Maxime Coquelin
0 siblings, 1 reply; 6+ messages in thread
From: Flavio Leitner @ 2019-10-24 16:31 UTC (permalink / raw)
To: Maxime Coquelin; +Cc: dev, Tiwei Bie, Obrembski MichalX, stable
On Thu, 24 Oct 2019 16:32:43 +0200
Maxime Coquelin <maxime.coquelin@redhat.com> wrote:
> Hi Flavio,
>
> On 10/24/19 4:28 PM, Flavio Leitner wrote:
> > Currently the IPv4 header checksum is calculated including its
> > current value, which can be a valid checksum or just garbage.
> > In any case, if the original value is not zero, then the result
> > is always wrong.
> >
> > The IPv4 checksum is defined in RFC791, page 14 says:
> > Header Checksum: 16 bits
> >
> > The checksum algorithm is:
> > The checksum field is the 16 bit one's complement of the one's
> > complement sum of all 16 bit words in the header. For purposes of
> > computing the checksum, the value of the checksum field is zero.
> >
> > Thus force the csum field to always be zero.
>
> I think we need Fixes tag:
>
> Fixes: b08b8cfeb2ae ("vhost: fix IP checksum")
> Cc: stable@dpdk.org
>
> Also adding stable in Cc.
>
>
> > Signed-off-by: Flavio Leitner <fbl@sysclose.org>
> > ---
> > lib/librte_vhost/virtio_net.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/lib/librte_vhost/virtio_net.c
> > b/lib/librte_vhost/virtio_net.c index eae7825f04..cde7498c76 100644
> > --- a/lib/librte_vhost/virtio_net.c
> > +++ b/lib/librte_vhost/virtio_net.c
> > @@ -445,6 +445,7 @@ virtio_enqueue_offload(struct rte_mbuf *m_buf,
> > struct virtio_net_hdr *net_hdr)
> > ipv4_hdr = rte_pktmbuf_mtod_offset(m_buf, struct
> > rte_ipv4_hdr *, m_buf->l2_len);
> > + ipv4_hdr->hdr_checksum = 0;
> > ipv4_hdr->hdr_checksum = rte_ipv4_cksum(ipv4_hdr);
> > }
> >
> >
>
> Other than that:
>
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>
> I'll add the Fixes tag while applying.
Not sure if you will send to stable, so I did v2 just in case.
Feel free to drop either one if you took care of those items already.
Thanks!
fbl
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [dpdk-dev] [PATCH] vhost: fix IPv4 csum calculation
2019-10-24 16:31 ` Flavio Leitner
@ 2019-10-24 16:32 ` Maxime Coquelin
0 siblings, 0 replies; 6+ messages in thread
From: Maxime Coquelin @ 2019-10-24 16:32 UTC (permalink / raw)
To: Flavio Leitner; +Cc: dev, Tiwei Bie, Obrembski MichalX, stable
On 10/24/19 6:31 PM, Flavio Leitner wrote:
> On Thu, 24 Oct 2019 16:32:43 +0200
> Maxime Coquelin <maxime.coquelin@redhat.com> wrote:
>
>> Hi Flavio,
>>
>> On 10/24/19 4:28 PM, Flavio Leitner wrote:
>>> Currently the IPv4 header checksum is calculated including its
>>> current value, which can be a valid checksum or just garbage.
>>> In any case, if the original value is not zero, then the result
>>> is always wrong.
>>>
>>> The IPv4 checksum is defined in RFC791, page 14 says:
>>> Header Checksum: 16 bits
>>>
>>> The checksum algorithm is:
>>> The checksum field is the 16 bit one's complement of the one's
>>> complement sum of all 16 bit words in the header. For purposes of
>>> computing the checksum, the value of the checksum field is zero.
>>>
>>> Thus force the csum field to always be zero.
>>
>> I think we need Fixes tag:
>>
>> Fixes: b08b8cfeb2ae ("vhost: fix IP checksum")
>> Cc: stable@dpdk.org
>>
>> Also adding stable in Cc.
>>
>>
>>> Signed-off-by: Flavio Leitner <fbl@sysclose.org>
>>> ---
>>> lib/librte_vhost/virtio_net.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/lib/librte_vhost/virtio_net.c
>>> b/lib/librte_vhost/virtio_net.c index eae7825f04..cde7498c76 100644
>>> --- a/lib/librte_vhost/virtio_net.c
>>> +++ b/lib/librte_vhost/virtio_net.c
>>> @@ -445,6 +445,7 @@ virtio_enqueue_offload(struct rte_mbuf *m_buf,
>>> struct virtio_net_hdr *net_hdr)
>>> ipv4_hdr = rte_pktmbuf_mtod_offset(m_buf, struct
>>> rte_ipv4_hdr *, m_buf->l2_len);
>>> + ipv4_hdr->hdr_checksum = 0;
>>> ipv4_hdr->hdr_checksum = rte_ipv4_cksum(ipv4_hdr);
>>> }
>>>
>>>
>>
>> Other than that:
>>
>> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>>
>> I'll add the Fixes tag while applying.
>
> Not sure if you will send to stable, so I did v2 just in case.
> Feel free to drop either one if you took care of those items already.
> Thanks!
> fbl
>
>
I synced with Ferruh, and he proposed to pick it directly for -rc1.
Thanks!
Maxime
^ permalink raw reply [flat|nested] 6+ messages in thread
* [dpdk-dev] [PATCH v2] vhost: fix IPv4 csum calculation
2019-10-24 14:28 [dpdk-dev] [PATCH] vhost: fix IPv4 csum calculation Flavio Leitner
2019-10-24 14:32 ` Maxime Coquelin
@ 2019-10-24 16:29 ` Flavio Leitner
2019-10-24 17:40 ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
1 sibling, 1 reply; 6+ messages in thread
From: Flavio Leitner @ 2019-10-24 16:29 UTC (permalink / raw)
To: dev; +Cc: Maxime Coquelin, Tiwei Bie, Obrembski MichalX, stable
Currently the IPv4 header checksum is calculated including its
current value, which can be a valid checksum or just garbage.
In any case, if the original value is not zero, then the result
is always wrong.
The IPv4 checksum is defined in RFC791, page 14 says:
Header Checksum: 16 bits
The checksum algorithm is:
The checksum field is the 16 bit one's complement of the one's
complement sum of all 16 bit words in the header. For purposes of
computing the checksum, the value of the checksum field is zero.
Thus force the csum field to always be zero.
Fixes: b08b8cfeb2ae ("vhost: fix IP checksum")
Cc: stable@dpdk.org
Signed-off-by: Flavio Leitner <fbl@sysclose.org>
---
lib/librte_vhost/virtio_net.c | 1 +
1 file changed, 1 insertion(+)
v2:
CC stable
Added 'Fixes:' line.
diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c
index eae7825f04..cde7498c76 100644
--- a/lib/librte_vhost/virtio_net.c
+++ b/lib/librte_vhost/virtio_net.c
@@ -445,6 +445,7 @@ virtio_enqueue_offload(struct rte_mbuf *m_buf, struct virtio_net_hdr *net_hdr)
ipv4_hdr = rte_pktmbuf_mtod_offset(m_buf, struct rte_ipv4_hdr *,
m_buf->l2_len);
+ ipv4_hdr->hdr_checksum = 0;
ipv4_hdr->hdr_checksum = rte_ipv4_cksum(ipv4_hdr);
}
--
2.20.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [dpdk-dev] [dpdk-stable] [PATCH v2] vhost: fix IPv4 csum calculation
2019-10-24 16:29 ` [dpdk-dev] [PATCH v2] " Flavio Leitner
@ 2019-10-24 17:40 ` Ferruh Yigit
0 siblings, 0 replies; 6+ messages in thread
From: Ferruh Yigit @ 2019-10-24 17:40 UTC (permalink / raw)
To: Flavio Leitner, dev; +Cc: Maxime Coquelin, Tiwei Bie, Obrembski MichalX, stable
On 10/24/2019 5:29 PM, Flavio Leitner wrote:
> Currently the IPv4 header checksum is calculated including its
> current value, which can be a valid checksum or just garbage.
> In any case, if the original value is not zero, then the result
> is always wrong.
>
> The IPv4 checksum is defined in RFC791, page 14 says:
> Header Checksum: 16 bits
>
> The checksum algorithm is:
> The checksum field is the 16 bit one's complement of the one's
> complement sum of all 16 bit words in the header. For purposes of
> computing the checksum, the value of the checksum field is zero.
>
> Thus force the csum field to always be zero.
>
> Fixes: b08b8cfeb2ae ("vhost: fix IP checksum")
> Cc: stable@dpdk.org
>
> Signed-off-by: Flavio Leitner <fbl@sysclose.org>
Applied to dpdk-next-net/master, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-10-24 17:40 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-24 14:28 [dpdk-dev] [PATCH] vhost: fix IPv4 csum calculation Flavio Leitner
2019-10-24 14:32 ` Maxime Coquelin
2019-10-24 16:31 ` Flavio Leitner
2019-10-24 16:32 ` Maxime Coquelin
2019-10-24 16:29 ` [dpdk-dev] [PATCH v2] " Flavio Leitner
2019-10-24 17:40 ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
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).