* [PATCH] virtio: don't set VIRTIO_NET_HDR_F_DATA_VALID on xmit
@ 2017-01-17 18:13 Rolf Neugebauer
2017-01-17 18:27 ` Michael S. Tsirkin
2017-01-18 12:07 ` Sergei Shtylyov
0 siblings, 2 replies; 9+ messages in thread
From: Rolf Neugebauer @ 2017-01-17 18:13 UTC (permalink / raw)
To: netdev; +Cc: mst, davem, jasowang, rppt, Rolf Neugebauer
This patch part reverts fd2a0437dc33 and e858fae2b0b8 which introduced a
subtle change in how the virtio_net flags are derived from the SKBs
ip_summed field.
With the above commits, the flags are set to VIRTIO_NET_HDR_F_DATA_VALID
when ip_summed == CHECKSUM_UNNECESSARY, thus treating it differently to
ip_summed == CHECKSUM_NONE, which should be the same.
Further, the virtio spec 1.0 / CS04 explicitly says that
VIRTIO_NET_HDR_F_DATA_VALID must not be set by the driver.
Signed-off-by: Rolf Neugebauer <rolf.neugebauer@docker.com>
---
include/linux/virtio_net.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 66204007d7ac..56436472ccc7 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -91,8 +91,6 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
skb_checksum_start_offset(skb));
hdr->csum_offset = __cpu_to_virtio16(little_endian,
skb->csum_offset);
- } else if (skb->ip_summed == CHECKSUM_UNNECESSARY) {
- hdr->flags = VIRTIO_NET_HDR_F_DATA_VALID;
} /* else everything is zero */
return 0;
--
2.11.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] virtio: don't set VIRTIO_NET_HDR_F_DATA_VALID on xmit
2017-01-17 18:13 [PATCH] virtio: don't set VIRTIO_NET_HDR_F_DATA_VALID on xmit Rolf Neugebauer
@ 2017-01-17 18:27 ` Michael S. Tsirkin
2017-01-17 19:08 ` David Miller
2017-01-18 3:54 ` Jason Wang
2017-01-18 12:07 ` Sergei Shtylyov
1 sibling, 2 replies; 9+ messages in thread
From: Michael S. Tsirkin @ 2017-01-17 18:27 UTC (permalink / raw)
To: Rolf Neugebauer; +Cc: netdev, davem, jasowang, rppt
On Tue, Jan 17, 2017 at 06:13:51PM +0000, Rolf Neugebauer wrote:
> This patch part reverts fd2a0437dc33 and e858fae2b0b8 which introduced a
> subtle change in how the virtio_net flags are derived from the SKBs
> ip_summed field.
>
> With the above commits, the flags are set to VIRTIO_NET_HDR_F_DATA_VALID
> when ip_summed == CHECKSUM_UNNECESSARY, thus treating it differently to
> ip_summed == CHECKSUM_NONE, which should be the same.
>
> Further, the virtio spec 1.0 / CS04 explicitly says that
> VIRTIO_NET_HDR_F_DATA_VALID must not be set by the driver.
>
> Signed-off-by: Rolf Neugebauer <rolf.neugebauer@docker.com>
Fixes: fd2a0437dc33 ("virtio_net: introduce virtio_net_hdr_{from,to}_skb")
Fixes: e858fae2b0b8 (" virtio_net: use common code for virtio_net_hdr and skb GSO conversion")
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Should be backported into stable as well.
> ---
> include/linux/virtio_net.h | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index 66204007d7ac..56436472ccc7 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
> @@ -91,8 +91,6 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
> skb_checksum_start_offset(skb));
> hdr->csum_offset = __cpu_to_virtio16(little_endian,
> skb->csum_offset);
> - } else if (skb->ip_summed == CHECKSUM_UNNECESSARY) {
> - hdr->flags = VIRTIO_NET_HDR_F_DATA_VALID;
> } /* else everything is zero */
>
> return 0;
> --
> 2.11.0
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] virtio: don't set VIRTIO_NET_HDR_F_DATA_VALID on xmit
2017-01-17 18:27 ` Michael S. Tsirkin
@ 2017-01-17 19:08 ` David Miller
2017-01-18 3:54 ` Jason Wang
1 sibling, 0 replies; 9+ messages in thread
From: David Miller @ 2017-01-17 19:08 UTC (permalink / raw)
To: mst; +Cc: rolf.neugebauer, netdev, jasowang, rppt
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Tue, 17 Jan 2017 20:27:33 +0200
> On Tue, Jan 17, 2017 at 06:13:51PM +0000, Rolf Neugebauer wrote:
>> This patch part reverts fd2a0437dc33 and e858fae2b0b8 which introduced a
>> subtle change in how the virtio_net flags are derived from the SKBs
>> ip_summed field.
>>
>> With the above commits, the flags are set to VIRTIO_NET_HDR_F_DATA_VALID
>> when ip_summed == CHECKSUM_UNNECESSARY, thus treating it differently to
>> ip_summed == CHECKSUM_NONE, which should be the same.
>>
>> Further, the virtio spec 1.0 / CS04 explicitly says that
>> VIRTIO_NET_HDR_F_DATA_VALID must not be set by the driver.
>>
>> Signed-off-by: Rolf Neugebauer <rolf.neugebauer@docker.com>
>
> Fixes: fd2a0437dc33 ("virtio_net: introduce virtio_net_hdr_{from,to}_skb")
> Fixes: e858fae2b0b8 (" virtio_net: use common code for virtio_net_hdr and skb GSO conversion")
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
>
> Should be backported into stable as well.
Applied and queued up for -stable, thanks everyone.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] virtio: don't set VIRTIO_NET_HDR_F_DATA_VALID on xmit
2017-01-17 18:27 ` Michael S. Tsirkin
2017-01-17 19:08 ` David Miller
@ 2017-01-18 3:54 ` Jason Wang
2017-01-18 15:18 ` Michael S. Tsirkin
1 sibling, 1 reply; 9+ messages in thread
From: Jason Wang @ 2017-01-18 3:54 UTC (permalink / raw)
To: Michael S. Tsirkin, Rolf Neugebauer; +Cc: netdev, davem, rppt
On 2017年01月18日 02:27, Michael S. Tsirkin wrote:
> On Tue, Jan 17, 2017 at 06:13:51PM +0000, Rolf Neugebauer wrote:
>> This patch part reverts fd2a0437dc33 and e858fae2b0b8 which introduced a
>> subtle change in how the virtio_net flags are derived from the SKBs
>> ip_summed field.
>>
>> With the above commits, the flags are set to VIRTIO_NET_HDR_F_DATA_VALID
>> when ip_summed == CHECKSUM_UNNECESSARY, thus treating it differently to
>> ip_summed == CHECKSUM_NONE, which should be the same.
>>
>> Further, the virtio spec 1.0 / CS04 explicitly says that
>> VIRTIO_NET_HDR_F_DATA_VALID must not be set by the driver.
>>
>> Signed-off-by: Rolf Neugebauer <rolf.neugebauer@docker.com>
> Fixes: fd2a0437dc33 ("virtio_net: introduce virtio_net_hdr_{from,to}_skb")
> Fixes: e858fae2b0b8 (" virtio_net: use common code for virtio_net_hdr and skb GSO conversion")
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
>
> Should be backported into stable as well.
Looks like a side effect is that we will never see this on receive path?
We probably need a hint for virtio_net_hdr_from_skb().
Thanks
>
>
>> ---
>> include/linux/virtio_net.h | 2 --
>> 1 file changed, 2 deletions(-)
>>
>> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
>> index 66204007d7ac..56436472ccc7 100644
>> --- a/include/linux/virtio_net.h
>> +++ b/include/linux/virtio_net.h
>> @@ -91,8 +91,6 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
>> skb_checksum_start_offset(skb));
>> hdr->csum_offset = __cpu_to_virtio16(little_endian,
>> skb->csum_offset);
>> - } else if (skb->ip_summed == CHECKSUM_UNNECESSARY) {
>> - hdr->flags = VIRTIO_NET_HDR_F_DATA_VALID;
>> } /* else everything is zero */
>>
>> return 0;
>> --
>> 2.11.0
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] virtio: don't set VIRTIO_NET_HDR_F_DATA_VALID on xmit
2017-01-18 3:54 ` Jason Wang
@ 2017-01-18 15:18 ` Michael S. Tsirkin
2017-01-19 3:06 ` Jason Wang
0 siblings, 1 reply; 9+ messages in thread
From: Michael S. Tsirkin @ 2017-01-18 15:18 UTC (permalink / raw)
To: Jason Wang; +Cc: Rolf Neugebauer, netdev, davem, rppt
On Wed, Jan 18, 2017 at 11:54:21AM +0800, Jason Wang wrote:
>
>
> On 2017年01月18日 02:27, Michael S. Tsirkin wrote:
> > On Tue, Jan 17, 2017 at 06:13:51PM +0000, Rolf Neugebauer wrote:
> > > This patch part reverts fd2a0437dc33 and e858fae2b0b8 which introduced a
> > > subtle change in how the virtio_net flags are derived from the SKBs
> > > ip_summed field.
> > >
> > > With the above commits, the flags are set to VIRTIO_NET_HDR_F_DATA_VALID
> > > when ip_summed == CHECKSUM_UNNECESSARY, thus treating it differently to
> > > ip_summed == CHECKSUM_NONE, which should be the same.
> > >
> > > Further, the virtio spec 1.0 / CS04 explicitly says that
> > > VIRTIO_NET_HDR_F_DATA_VALID must not be set by the driver.
> > >
> > > Signed-off-by: Rolf Neugebauer <rolf.neugebauer@docker.com>
> > Fixes: fd2a0437dc33 ("virtio_net: introduce virtio_net_hdr_{from,to}_skb")
> > Fixes: e858fae2b0b8 (" virtio_net: use common code for virtio_net_hdr and skb GSO conversion")
> > Acked-by: Michael S. Tsirkin <mst@redhat.com>
> >
> > Should be backported into stable as well.
>
> Looks like a side effect is that we will never see this on receive path?
I thought receive path is using virtio_net_hdr_to_skb - isn't it?
> We
> probably need a hint for virtio_net_hdr_from_skb().
>
> Thanks
>
> >
> >
> > > ---
> > > include/linux/virtio_net.h | 2 --
> > > 1 file changed, 2 deletions(-)
> > >
> > > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> > > index 66204007d7ac..56436472ccc7 100644
> > > --- a/include/linux/virtio_net.h
> > > +++ b/include/linux/virtio_net.h
> > > @@ -91,8 +91,6 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
> > > skb_checksum_start_offset(skb));
> > > hdr->csum_offset = __cpu_to_virtio16(little_endian,
> > > skb->csum_offset);
> > > - } else if (skb->ip_summed == CHECKSUM_UNNECESSARY) {
> > > - hdr->flags = VIRTIO_NET_HDR_F_DATA_VALID;
> > > } /* else everything is zero */
> > > return 0;
> > > --
> > > 2.11.0
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] virtio: don't set VIRTIO_NET_HDR_F_DATA_VALID on xmit
2017-01-18 15:18 ` Michael S. Tsirkin
@ 2017-01-19 3:06 ` Jason Wang
2017-01-19 22:30 ` Michael S. Tsirkin
0 siblings, 1 reply; 9+ messages in thread
From: Jason Wang @ 2017-01-19 3:06 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Rolf Neugebauer, netdev, davem, rppt
On 2017年01月18日 23:18, Michael S. Tsirkin wrote:
> On Wed, Jan 18, 2017 at 11:54:21AM +0800, Jason Wang wrote:
>> On 2017年01月18日 02:27, Michael S. Tsirkin wrote:
>>> On Tue, Jan 17, 2017 at 06:13:51PM +0000, Rolf Neugebauer wrote:
>>>> This patch part reverts fd2a0437dc33 and e858fae2b0b8 which introduced a
>>>> subtle change in how the virtio_net flags are derived from the SKBs
>>>> ip_summed field.
>>>>
>>>> With the above commits, the flags are set to VIRTIO_NET_HDR_F_DATA_VALID
>>>> when ip_summed == CHECKSUM_UNNECESSARY, thus treating it differently to
>>>> ip_summed == CHECKSUM_NONE, which should be the same.
>>>>
>>>> Further, the virtio spec 1.0 / CS04 explicitly says that
>>>> VIRTIO_NET_HDR_F_DATA_VALID must not be set by the driver.
>>>>
>>>> Signed-off-by: Rolf Neugebauer<rolf.neugebauer@docker.com>
>>> Fixes: fd2a0437dc33 ("virtio_net: introduce virtio_net_hdr_{from,to}_skb")
>>> Fixes: e858fae2b0b8 (" virtio_net: use common code for virtio_net_hdr and skb GSO conversion")
>>> Acked-by: Michael S. Tsirkin<mst@redhat.com>
>>>
>>> Should be backported into stable as well.
>> Looks like a side effect is that we will never see this on receive path?
> I thought receive path is using virtio_net_hdr_to_skb - isn't it?
>
The problem is tun use this on xmit.
Thanks
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] virtio: don't set VIRTIO_NET_HDR_F_DATA_VALID on xmit
2017-01-19 3:06 ` Jason Wang
@ 2017-01-19 22:30 ` Michael S. Tsirkin
2017-01-20 3:29 ` Jason Wang
0 siblings, 1 reply; 9+ messages in thread
From: Michael S. Tsirkin @ 2017-01-19 22:30 UTC (permalink / raw)
To: Jason Wang; +Cc: Rolf Neugebauer, netdev, davem, rppt
On Thu, Jan 19, 2017 at 11:06:59AM +0800, Jason Wang wrote:
>
>
> On 2017年01月18日 23:18, Michael S. Tsirkin wrote:
> > On Wed, Jan 18, 2017 at 11:54:21AM +0800, Jason Wang wrote:
> > > On 2017年01月18日 02:27, Michael S. Tsirkin wrote:
> > > > On Tue, Jan 17, 2017 at 06:13:51PM +0000, Rolf Neugebauer wrote:
> > > > > This patch part reverts fd2a0437dc33 and e858fae2b0b8 which introduced a
> > > > > subtle change in how the virtio_net flags are derived from the SKBs
> > > > > ip_summed field.
> > > > >
> > > > > With the above commits, the flags are set to VIRTIO_NET_HDR_F_DATA_VALID
> > > > > when ip_summed == CHECKSUM_UNNECESSARY, thus treating it differently to
> > > > > ip_summed == CHECKSUM_NONE, which should be the same.
> > > > >
> > > > > Further, the virtio spec 1.0 / CS04 explicitly says that
> > > > > VIRTIO_NET_HDR_F_DATA_VALID must not be set by the driver.
> > > > >
> > > > > Signed-off-by: Rolf Neugebauer<rolf.neugebauer@docker.com>
> > > > Fixes: fd2a0437dc33 ("virtio_net: introduce virtio_net_hdr_{from,to}_skb")
> > > > Fixes: e858fae2b0b8 (" virtio_net: use common code for virtio_net_hdr and skb GSO conversion")
> > > > Acked-by: Michael S. Tsirkin<mst@redhat.com>
> > > >
> > > > Should be backported into stable as well.
> > > Looks like a side effect is that we will never see this on receive path?
> > I thought receive path is using virtio_net_hdr_to_skb - isn't it?
> >
>
> The problem is tun use this on xmit.
>
> Thanks
Oh I missed that. I agree, let's add a flag that allows this.
--
MST
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] virtio: don't set VIRTIO_NET_HDR_F_DATA_VALID on xmit
2017-01-19 22:30 ` Michael S. Tsirkin
@ 2017-01-20 3:29 ` Jason Wang
0 siblings, 0 replies; 9+ messages in thread
From: Jason Wang @ 2017-01-20 3:29 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Rolf Neugebauer, netdev, davem, rppt
On 2017年01月20日 06:30, Michael S. Tsirkin wrote:
> On Thu, Jan 19, 2017 at 11:06:59AM +0800, Jason Wang wrote:
>>
>> On 2017年01月18日 23:18, Michael S. Tsirkin wrote:
>>> On Wed, Jan 18, 2017 at 11:54:21AM +0800, Jason Wang wrote:
>>>> On 2017年01月18日 02:27, Michael S. Tsirkin wrote:
>>>>> On Tue, Jan 17, 2017 at 06:13:51PM +0000, Rolf Neugebauer wrote:
>>>>>> This patch part reverts fd2a0437dc33 and e858fae2b0b8 which introduced a
>>>>>> subtle change in how the virtio_net flags are derived from the SKBs
>>>>>> ip_summed field.
>>>>>>
>>>>>> With the above commits, the flags are set to VIRTIO_NET_HDR_F_DATA_VALID
>>>>>> when ip_summed == CHECKSUM_UNNECESSARY, thus treating it differently to
>>>>>> ip_summed == CHECKSUM_NONE, which should be the same.
>>>>>>
>>>>>> Further, the virtio spec 1.0 / CS04 explicitly says that
>>>>>> VIRTIO_NET_HDR_F_DATA_VALID must not be set by the driver.
>>>>>>
>>>>>> Signed-off-by: Rolf Neugebauer<rolf.neugebauer@docker.com>
>>>>> Fixes: fd2a0437dc33 ("virtio_net: introduce virtio_net_hdr_{from,to}_skb")
>>>>> Fixes: e858fae2b0b8 (" virtio_net: use common code for virtio_net_hdr and skb GSO conversion")
>>>>> Acked-by: Michael S. Tsirkin<mst@redhat.com>
>>>>>
>>>>> Should be backported into stable as well.
>>>> Looks like a side effect is that we will never see this on receive path?
>>> I thought receive path is using virtio_net_hdr_to_skb - isn't it?
>>>
>> The problem is tun use this on xmit.
>>
>> Thanks
> Oh I missed that. I agree, let's add a flag that allows this.
>
Will post a fix soon.
Thanks
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] virtio: don't set VIRTIO_NET_HDR_F_DATA_VALID on xmit
2017-01-17 18:13 [PATCH] virtio: don't set VIRTIO_NET_HDR_F_DATA_VALID on xmit Rolf Neugebauer
2017-01-17 18:27 ` Michael S. Tsirkin
@ 2017-01-18 12:07 ` Sergei Shtylyov
1 sibling, 0 replies; 9+ messages in thread
From: Sergei Shtylyov @ 2017-01-18 12:07 UTC (permalink / raw)
To: Rolf Neugebauer, netdev; +Cc: mst, davem, jasowang, rppt
Hello!
On 1/17/2017 9:13 PM, Rolf Neugebauer wrote:
> This patch part reverts fd2a0437dc33 and e858fae2b0b8 which introduced a
You also need to specify the patch summary lines enclosed in ("").
> subtle change in how the virtio_net flags are derived from the SKBs
> ip_summed field.
>
> With the above commits, the flags are set to VIRTIO_NET_HDR_F_DATA_VALID
> when ip_summed == CHECKSUM_UNNECESSARY, thus treating it differently to
> ip_summed == CHECKSUM_NONE, which should be the same.
>
> Further, the virtio spec 1.0 / CS04 explicitly says that
> VIRTIO_NET_HDR_F_DATA_VALID must not be set by the driver.
>
> Signed-off-by: Rolf Neugebauer <rolf.neugebauer@docker.com>
[...]
MBR, Sergei
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-01-20 3:37 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-17 18:13 [PATCH] virtio: don't set VIRTIO_NET_HDR_F_DATA_VALID on xmit Rolf Neugebauer
2017-01-17 18:27 ` Michael S. Tsirkin
2017-01-17 19:08 ` David Miller
2017-01-18 3:54 ` Jason Wang
2017-01-18 15:18 ` Michael S. Tsirkin
2017-01-19 3:06 ` Jason Wang
2017-01-19 22:30 ` Michael S. Tsirkin
2017-01-20 3:29 ` Jason Wang
2017-01-18 12:07 ` Sergei Shtylyov
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).