linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [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).