* [PATCH net 1/2] virtio-net: disable guest csum during XDP set
@ 2018-11-22 6:36 Jason Wang
2018-11-22 6:36 ` [PATCH net 2/2] virtio-net: fail XDP set if guest csum is negotiated Jason Wang
2018-11-23 20:01 ` [PATCH net 1/2] virtio-net: disable guest csum during XDP set David Miller
0 siblings, 2 replies; 5+ messages in thread
From: Jason Wang @ 2018-11-22 6:36 UTC (permalink / raw)
To: mst, jasowang, davem, virtualization, netdev, linux-kernel
Cc: Jesper Dangaard Brouer, Pavel Popa, David Ahern
We don't disable VIRTIO_NET_F_GUEST_CSUM if XDP was set. This means we
can receive partial csumed packets with metadata kept in the
vnet_hdr. This may have several side effects:
- It could be overridden by header adjustment, thus is might be not
correct after XDP processing.
- There's no way to pass such metadata information through
XDP_REDIRECT to another driver.
- XDP does not support checksum offload right now.
So simply disable guest csum if possible in this the case of XDP.
Fixes: 3f93522ffab2d ("virtio-net: switch off offloads on demand if possible on XDP set")
Reported-by: Jesper Dangaard Brouer <brouer@redhat.com>
Cc: Jesper Dangaard Brouer <brouer@redhat.com>
Cc: Pavel Popa <pashinho1990@gmail.com>
Cc: David Ahern <dsahern@gmail.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/net/virtio_net.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 3e2c041d76ac..9b5ace538824 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -70,7 +70,8 @@ static const unsigned long guest_offloads[] = {
VIRTIO_NET_F_GUEST_TSO4,
VIRTIO_NET_F_GUEST_TSO6,
VIRTIO_NET_F_GUEST_ECN,
- VIRTIO_NET_F_GUEST_UFO
+ VIRTIO_NET_F_GUEST_UFO,
+ VIRTIO_NET_F_GUEST_CSUM
};
struct virtnet_stat_desc {
@@ -2334,9 +2335,6 @@ static int virtnet_clear_guest_offloads(struct virtnet_info *vi)
if (!vi->guest_offloads)
return 0;
- if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_CSUM))
- offloads = 1ULL << VIRTIO_NET_F_GUEST_CSUM;
-
return virtnet_set_guest_offloads(vi, offloads);
}
@@ -2346,8 +2344,6 @@ static int virtnet_restore_guest_offloads(struct virtnet_info *vi)
if (!vi->guest_offloads)
return 0;
- if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_CSUM))
- offloads |= 1ULL << VIRTIO_NET_F_GUEST_CSUM;
return virtnet_set_guest_offloads(vi, offloads);
}
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH net 2/2] virtio-net: fail XDP set if guest csum is negotiated
2018-11-22 6:36 [PATCH net 1/2] virtio-net: disable guest csum during XDP set Jason Wang
@ 2018-11-22 6:36 ` Jason Wang
2018-11-23 20:01 ` David Miller
2018-11-23 20:01 ` [PATCH net 1/2] virtio-net: disable guest csum during XDP set David Miller
1 sibling, 1 reply; 5+ messages in thread
From: Jason Wang @ 2018-11-22 6:36 UTC (permalink / raw)
To: mst, jasowang, davem, virtualization, netdev, linux-kernel
Cc: Jesper Dangaard Brouer, Pavel Popa, David Ahern
We don't support partial csumed packet since its metadata will be lost
or incorrect during XDP processing. So fail the XDP set if guest_csum
feature is negotiated.
Fixes: f600b6905015 ("virtio_net: Add XDP support")
Reported-by: Jesper Dangaard Brouer <brouer@redhat.com>
Cc: Jesper Dangaard Brouer <brouer@redhat.com>
Cc: Pavel Popa <pashinho1990@gmail.com>
Cc: David Ahern <dsahern@gmail.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/net/virtio_net.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 9b5ace538824..cecfd77c9f3c 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2361,8 +2361,9 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
&& (virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_TSO4) ||
virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_TSO6) ||
virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_ECN) ||
- virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_UFO))) {
- NL_SET_ERR_MSG_MOD(extack, "Can't set XDP while host is implementing LRO, disable LRO first");
+ virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_UFO) ||
+ virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_CSUM))) {
+ NL_SET_ERR_MSG_MOD(extack, "Can't set XDP while host is implementing LRO/CSUM, disable LRO/CSUM first");
return -EOPNOTSUPP;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net 1/2] virtio-net: disable guest csum during XDP set
2018-11-22 6:36 [PATCH net 1/2] virtio-net: disable guest csum during XDP set Jason Wang
2018-11-22 6:36 ` [PATCH net 2/2] virtio-net: fail XDP set if guest csum is negotiated Jason Wang
@ 2018-11-23 20:01 ` David Miller
2018-11-26 4:03 ` Jason Wang
1 sibling, 1 reply; 5+ messages in thread
From: David Miller @ 2018-11-23 20:01 UTC (permalink / raw)
To: jasowang
Cc: mst, virtualization, netdev, linux-kernel, brouer, pashinho1990, dsahern
From: Jason Wang <jasowang@redhat.com>
Date: Thu, 22 Nov 2018 14:36:30 +0800
> We don't disable VIRTIO_NET_F_GUEST_CSUM if XDP was set. This means we
> can receive partial csumed packets with metadata kept in the
> vnet_hdr. This may have several side effects:
>
> - It could be overridden by header adjustment, thus is might be not
> correct after XDP processing.
> - There's no way to pass such metadata information through
> XDP_REDIRECT to another driver.
> - XDP does not support checksum offload right now.
>
> So simply disable guest csum if possible in this the case of XDP.
>
> Fixes: 3f93522ffab2d ("virtio-net: switch off offloads on demand if possible on XDP set")
> Reported-by: Jesper Dangaard Brouer <brouer@redhat.com>
> Cc: Jesper Dangaard Brouer <brouer@redhat.com>
> Cc: Pavel Popa <pashinho1990@gmail.com>
> Cc: David Ahern <dsahern@gmail.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Applied and queued up for -stable.
We really should have a way to use the checksum provided if the XDP
program returns XDP_PASS and does not modify the packet contents
or size.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net 2/2] virtio-net: fail XDP set if guest csum is negotiated
2018-11-22 6:36 ` [PATCH net 2/2] virtio-net: fail XDP set if guest csum is negotiated Jason Wang
@ 2018-11-23 20:01 ` David Miller
0 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2018-11-23 20:01 UTC (permalink / raw)
To: jasowang
Cc: mst, virtualization, netdev, linux-kernel, brouer, pashinho1990, dsahern
From: Jason Wang <jasowang@redhat.com>
Date: Thu, 22 Nov 2018 14:36:31 +0800
> We don't support partial csumed packet since its metadata will be lost
> or incorrect during XDP processing. So fail the XDP set if guest_csum
> feature is negotiated.
>
> Fixes: f600b6905015 ("virtio_net: Add XDP support")
> Reported-by: Jesper Dangaard Brouer <brouer@redhat.com>
> Cc: Jesper Dangaard Brouer <brouer@redhat.com>
> Cc: Pavel Popa <pashinho1990@gmail.com>
> Cc: David Ahern <dsahern@gmail.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Applied and queued up for -stable.
Same comments as for patch #1.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net 1/2] virtio-net: disable guest csum during XDP set
2018-11-23 20:01 ` [PATCH net 1/2] virtio-net: disable guest csum during XDP set David Miller
@ 2018-11-26 4:03 ` Jason Wang
0 siblings, 0 replies; 5+ messages in thread
From: Jason Wang @ 2018-11-26 4:03 UTC (permalink / raw)
To: David Miller
Cc: mst, virtualization, netdev, linux-kernel, brouer, pashinho1990, dsahern
On 2018/11/24 上午4:01, David Miller wrote:
> From: Jason Wang <jasowang@redhat.com>
> Date: Thu, 22 Nov 2018 14:36:30 +0800
>
>> We don't disable VIRTIO_NET_F_GUEST_CSUM if XDP was set. This means we
>> can receive partial csumed packets with metadata kept in the
>> vnet_hdr. This may have several side effects:
>>
>> - It could be overridden by header adjustment, thus is might be not
>> correct after XDP processing.
>> - There's no way to pass such metadata information through
>> XDP_REDIRECT to another driver.
>> - XDP does not support checksum offload right now.
>>
>> So simply disable guest csum if possible in this the case of XDP.
>>
>> Fixes: 3f93522ffab2d ("virtio-net: switch off offloads on demand if possible on XDP set")
>> Reported-by: Jesper Dangaard Brouer <brouer@redhat.com>
>> Cc: Jesper Dangaard Brouer <brouer@redhat.com>
>> Cc: Pavel Popa <pashinho1990@gmail.com>
>> Cc: David Ahern <dsahern@gmail.com>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
> Applied and queued up for -stable.
>
> We really should have a way to use the checksum provided if the XDP
> program returns XDP_PASS and does not modify the packet contents
> or size.
Yes, I think this may require the assistance of BPF verifier to set a
flag or other. Then we can assume the metadata is safe to use.
Thanks
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-11-26 4:03 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-22 6:36 [PATCH net 1/2] virtio-net: disable guest csum during XDP set Jason Wang
2018-11-22 6:36 ` [PATCH net 2/2] virtio-net: fail XDP set if guest csum is negotiated Jason Wang
2018-11-23 20:01 ` David Miller
2018-11-23 20:01 ` [PATCH net 1/2] virtio-net: disable guest csum during XDP set David Miller
2018-11-26 4:03 ` Jason Wang
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).