This series adds support for UDP segmentation offload feature
in TUN device according to the VIRTIO specification
Yuri Benditovich (4):
virtio-net: add definitions for host USO feature
virtio-net: add support of UDP segmentation (USO) on the host
tun: define feature bit for USO support
tun: indicate support for USO feature
drivers/net/tun.c | 2 +-
include/linux/virtio_net.h | 5 +++++
include/uapi/linux/if_tun.h | 1 +
include/uapi/linux/virtio_net.h | 2 ++
4 files changed, 9 insertions(+), 1 deletion(-)
--
2.26.3
Define feature bit and GSO type according to the VIRTIO
specification.
Signed-off-by: Yuri Benditovich <redacted>
---
include/uapi/linux/virtio_net.h | 2 ++
1 file changed, 2 insertions(+)
Large UDP packet provided by the guest with GSO type set to
VIRTIO_NET_HDR_GSO_UDP_L4 will be divided to several UDP
packets according to the gso_size field.
Signed-off-by: Yuri Benditovich <redacted>
---
include/linux/virtio_net.h | 5 +++++
1 file changed, 5 insertions(+)
User mode software can probe this bit to check whether the
USO feature is supported by TUN/TAP device.
Signed-off-by: Yuri Benditovich <redacted>
---
include/uapi/linux/if_tun.h | 1 +
1 file changed, 1 insertion(+)
@@ -88,6 +88,7 @@#define TUN_F_TSO6 0x04 /* I can handle TSO for IPv6 packets */#define TUN_F_TSO_ECN 0x08 /* I can handle TSO with ECN bits. */#define TUN_F_UFO 0x10 /* I can handle UFO packets */+#define TUN_F_USO 0x20 /* I can handle USO packets *//* Protocol info prepended to the packets (when IFF_NO_PI is not set) */#define TUN_PKT_STRIP 0x0001
@@ -2812,7 +2812,7 @@ static int set_offload(struct tun_struct *tun, unsigned long arg)arg&=~(TUN_F_TSO4|TUN_F_TSO6);}-arg&=~TUN_F_UFO;+arg&=~(TUN_F_UFO|TUN_F_USO);}/* This gives the user a way to test for new features in future by
From: Jason Wang <hidden> Date: 2021-05-11 06:47:17
在 2021/5/11 下午12:42, Yuri Benditovich 写道:
quoted hunk
Define feature bit and GSO type according to the VIRTIO
specification.
Signed-off-by: Yuri Benditovich <redacted>
---
include/uapi/linux/virtio_net.h | 2 ++
1 file changed, 2 insertions(+)
This is the gso_type not the feature actually.
I wonder what's the reason for not
1) introducing a dedicated virtio-net feature bit for this
(VIRTIO_NET_F_GUEST_GSO_UDP_L4.
2) toggle the NETIF_F_GSO_UDP_L4 feature for tuntap based on the
negotiated feature.
Thanks
#define VIRTIO_NET_HDR_GSO_ECN 0x80 /* TCP has ECN set */
__u8 gso_type;
__virtio16 hdr_len; /* Ethernet + IP + tcp/udp hdrs */
From: Jason Wang <hidden> Date: 2021-05-11 06:47:50
在 2021/5/11 下午12:42, Yuri Benditovich 写道:
quoted hunk
Large UDP packet provided by the guest with GSO type set to
VIRTIO_NET_HDR_GSO_UDP_L4 will be divided to several UDP
packets according to the gso_size field.
Signed-off-by: Yuri Benditovich <redacted>
---
include/linux/virtio_net.h | 5 +++++
1 file changed, 5 insertions(+)
@@ -2812,7 +2812,7 @@ static int set_offload(struct tun_struct *tun, unsigned long arg)arg&=~(TUN_F_TSO4|TUN_F_TSO6);}-arg&=~TUN_F_UFO;+arg&=~(TUN_F_UFO|TUN_F_USO);
It looks to me kernel doesn't use "USO", so TUN_F_UDP_GSO_L4 is a better
name for this and I guess we should toggle NETIF_F_UDP_GSO_l4 here?
And how about macvtap?
Thanks
}
/* This gives the user a way to test for new features in future by
On Tue, May 11, 2021 at 9:47 AM Jason Wang [off-list ref] wrote:
在 2021/5/11 下午12:42, Yuri Benditovich 写道:
quoted
Define feature bit and GSO type according to the VIRTIO
specification.
Signed-off-by: Yuri Benditovich <redacted>
---
include/uapi/linux/virtio_net.h | 2 ++
1 file changed, 2 insertions(+)
@@ -57,6 +57,7 @@*Steering*/#define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */+#define VIRTIO_NET_F_HOST_USO 56 /* Host can handle USO packets */
This is the gso_type not the feature actually.
I wonder what's the reason for not
1) introducing a dedicated virtio-net feature bit for this
(VIRTIO_NET_F_GUEST_GSO_UDP_L4.
This series is not for GUEST's feature, it is only for host feature.
2) toggle the NETIF_F_GSO_UDP_L4 feature for tuntap based on the
negotiated feature.
The NETIF_F_GSO_UDP_L4 would be required for the guest RX path.
The guest TX path does not require any flags to be propagated, it only
allows the guest to transmit large UDP packets and have them
automatically splitted.
(This is similar to HOST_UFO but does packet segmentation instead of
fragmentation. GUEST_UFO indeed requires a respective NETIF flag, as
it is unclear whether the guest is capable of receiving such packets).
Thanks
quoted
#define VIRTIO_NET_HDR_GSO_ECN 0x80 /* TCP has ECN set */
__u8 gso_type;
__virtio16 hdr_len; /* Ethernet + IP + tcp/udp hdrs */
On Tue, May 11, 2021 at 9:47 AM Jason Wang [off-list ref] wrote:
在 2021/5/11 下午12:42, Yuri Benditovich 写道:
quoted
Large UDP packet provided by the guest with GSO type set to
VIRTIO_NET_HDR_GSO_UDP_L4 will be divided to several UDP
packets according to the gso_size field.
Signed-off-by: Yuri Benditovich <redacted>
---
include/linux/virtio_net.h | 5 +++++
1 file changed, 5 insertions(+)
@@ -51,6 +51,11 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,ip_proto=IPPROTO_UDP;thlen=sizeof(structudphdr);break;+caseVIRTIO_NET_HDR_GSO_UDP_L4:+gso_type=SKB_GSO_UDP_L4;+ip_proto=IPPROTO_UDP;+thlen=sizeof(structudphdr);+break;
This is only for rx, how about tx?
In terms of the guest this is only for TX.
Guest RX is a different thing, this is actually coalescing of
segmented UDP packets into a large one.
This feature is not defined in the virtio spec yet and the support of
it first of all depends on the OS.
For example: TCP LSO (guest TX) is supported almost by all the
versions of Windows.
TCP RSC (coalescing of TCP segments) is supported by Win 8 / Server 2012 and up.
UDP segmentation is supported by Windows kernels 1903+
UDP coalescing is defined by Windows kernels 2004+ and not supported
by the driver yet.
From: Jason Wang <hidden> Date: 2021-05-11 08:26:49
On Tue, May 11, 2021 at 4:12 PM Yuri Benditovich
[off-list ref] wrote:
On Tue, May 11, 2021 at 9:47 AM Jason Wang [off-list ref] wrote:
quoted
在 2021/5/11 下午12:42, Yuri Benditovich 写道:
quoted
Define feature bit and GSO type according to the VIRTIO
specification.
Signed-off-by: Yuri Benditovich <redacted>
---
include/uapi/linux/virtio_net.h | 2 ++
1 file changed, 2 insertions(+)
@@ -57,6 +57,7 @@*Steering*/#define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */+#define VIRTIO_NET_F_HOST_USO 56 /* Host can handle USO packets */
This is the gso_type not the feature actually.
I wonder what's the reason for not
1) introducing a dedicated virtio-net feature bit for this
(VIRTIO_NET_F_GUEST_GSO_UDP_L4.
This series is not for GUEST's feature, it is only for host feature.
quoted
2) toggle the NETIF_F_GSO_UDP_L4 feature for tuntap based on the
negotiated feature.
The NETIF_F_GSO_UDP_L4 would be required for the guest RX path.
The guest TX path does not require any flags to be propagated, it only
allows the guest to transmit large UDP packets and have them
automatically splitted.
(This is similar to HOST_UFO but does packet segmentation instead of
fragmentation. GUEST_UFO indeed requires a respective NETIF flag, as
it is unclear whether the guest is capable of receiving such packets).
So I think it's better to implement TX/RX in the same series unless
there's something missed:
For Guest TX, NETIF_F_GSO_UDP_L4 needs to be enabled in the guest
virtio-net only when VIRTIO_NET_F_HOST_USO is negotiated.
For guest RX, NETIF_F_GSO_UDP_L4 needs to be enabled on the host
tuntap only when VIRTIO_NET_F_GUEST_USO is neogiated.
Thanks
quoted
Thanks
quoted
#define VIRTIO_NET_HDR_GSO_ECN 0x80 /* TCP has ECN set */
__u8 gso_type;
__virtio16 hdr_len; /* Ethernet + IP + tcp/udp hdrs */
From: Jason Wang <hidden> Date: 2021-05-11 08:31:34
On Tue, May 11, 2021 at 4:24 PM Yuri Benditovich
[off-list ref] wrote:
On Tue, May 11, 2021 at 9:47 AM Jason Wang [off-list ref] wrote:
quoted
在 2021/5/11 下午12:42, Yuri Benditovich 写道:
quoted
Large UDP packet provided by the guest with GSO type set to
VIRTIO_NET_HDR_GSO_UDP_L4 will be divided to several UDP
packets according to the gso_size field.
Signed-off-by: Yuri Benditovich <redacted>
---
include/linux/virtio_net.h | 5 +++++
1 file changed, 5 insertions(+)
@@ -51,6 +51,11 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,ip_proto=IPPROTO_UDP;thlen=sizeof(structudphdr);break;+caseVIRTIO_NET_HDR_GSO_UDP_L4:+gso_type=SKB_GSO_UDP_L4;+ip_proto=IPPROTO_UDP;+thlen=sizeof(structudphdr);+break;
This is only for rx, how about tx?
In terms of the guest this is only for TX.
So virtio_net_hdr_to_skb() can be called by all the followings:
1) receive_buf() which is guest RX.
2) tun_get_user() which is guest TX
3) tap_get_user() which is guest TX
4) {t}packet_send() which is userspace TX
So it touches for both RX and TX.
Guest RX is a different thing, this is actually coalescing of
segmented UDP packets into a large one.
Another case, the packet could be sent from another VM (like the UFO case).
Supporting that for both TX and RX and greatly improve the performance
of VM2VM traffic.
Thanks
This feature is not defined in the virtio spec yet and the support of
it first of all depends on the OS.
For example: TCP LSO (guest TX) is supported almost by all the
versions of Windows.
TCP RSC (coalescing of TCP segments) is supported by Win 8 / Server 2012 and up.
UDP segmentation is supported by Windows kernels 1903+
UDP coalescing is defined by Windows kernels 2004+ and not supported
by the driver yet.
@@ -2812,7 +2812,7 @@ static int set_offload(struct tun_struct *tun, unsigned long arg)arg&=~(TUN_F_TSO4|TUN_F_TSO6);}-arg&=~TUN_F_UFO;+arg&=~(TUN_F_UFO|TUN_F_USO);
It looks to me kernel doesn't use "USO", so TUN_F_UDP_GSO_L4 is a better
name for this
No problem, I can change it in v2
and I guess we should toggle NETIF_F_UDP_GSO_l4 here?
No, we do not, because this indicates only the fact that the guest can
send large UDP packets and have them splitted to UDP segments.
And how about macvtap?
We will check how to do that for macvtap. We will send a separate
patch for macvtap or ask for advice.
Thanks
quoted
}
/* This gives the user a way to test for new features in future by
On Tue, May 11, 2021 at 11:24 AM Jason Wang [off-list ref] wrote:
On Tue, May 11, 2021 at 4:12 PM Yuri Benditovich
[off-list ref] wrote:
quoted
On Tue, May 11, 2021 at 9:47 AM Jason Wang [off-list ref] wrote:
quoted
在 2021/5/11 下午12:42, Yuri Benditovich 写道:
quoted
Define feature bit and GSO type according to the VIRTIO
specification.
Signed-off-by: Yuri Benditovich <redacted>
---
include/uapi/linux/virtio_net.h | 2 ++
1 file changed, 2 insertions(+)
@@ -57,6 +57,7 @@*Steering*/#define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */+#define VIRTIO_NET_F_HOST_USO 56 /* Host can handle USO packets */
This is the gso_type not the feature actually.
I wonder what's the reason for not
1) introducing a dedicated virtio-net feature bit for this
(VIRTIO_NET_F_GUEST_GSO_UDP_L4.
This series is not for GUEST's feature, it is only for host feature.
quoted
2) toggle the NETIF_F_GSO_UDP_L4 feature for tuntap based on the
negotiated feature.
The NETIF_F_GSO_UDP_L4 would be required for the guest RX path.
The guest TX path does not require any flags to be propagated, it only
allows the guest to transmit large UDP packets and have them
automatically splitted.
(This is similar to HOST_UFO but does packet segmentation instead of
fragmentation. GUEST_UFO indeed requires a respective NETIF flag, as
it is unclear whether the guest is capable of receiving such packets).
So I think it's better to implement TX/RX in the same series unless
there's something missed:
For Guest TX, NETIF_F_GSO_UDP_L4 needs to be enabled in the guest
virtio-net only when VIRTIO_NET_F_HOST_USO is negotiated.
I understand that this is what should be done when this feature will
be added to Linux virtio-net driver.
But at the moment we do not have enough resources to work on it.
Currently we have a clear use case and ability to test in on Windows guest.
Respective QEMU changes are pending for kernel patches, current
reference is https://github.com/daynix/qemu/tree/uso
For guest RX, NETIF_F_GSO_UDP_L4 needs to be enabled on the host
tuntap only when VIRTIO_NET_F_GUEST_USO is neogiated.
Currently we are not able to use guest RX UDP GSO.
In order to do that we at least should be able to build our Windows
drivers with the most updated driver development kit (2004+).
At the moment we can't, this task is in a plan but can take several
months. So we do not have a test/use case with Windows VM.
Thanks
quoted
quoted
Thanks
quoted
#define VIRTIO_NET_HDR_GSO_ECN 0x80 /* TCP has ECN set */
__u8 gso_type;
__virtio16 hdr_len; /* Ethernet + IP + tcp/udp hdrs */
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com> Date: 2021-05-11 17:48:10
On Tue, May 11, 2021 at 12:43 AM Yuri Benditovich
[off-list ref] wrote:
quoted hunk
Large UDP packet provided by the guest with GSO type set to
VIRTIO_NET_HDR_GSO_UDP_L4 will be divided to several UDP
packets according to the gso_size field.
Signed-off-by: Yuri Benditovich <redacted>
---
include/linux/virtio_net.h | 5 +++++
1 file changed, 5 insertions(+)
@@ -51,6 +51,11 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,ip_proto=IPPROTO_UDP;thlen=sizeof(structudphdr);break;+caseVIRTIO_NET_HDR_GSO_UDP_L4:+gso_type=SKB_GSO_UDP_L4;+ip_proto=IPPROTO_UDP;+thlen=sizeof(structudphdr);+break;
If adding a new VIRTIO_NET_HDR type I suggest adding separate IPv4 and
IPv6 variants, analogous to VIRTIO_NET_HDR_GSO_TCPV[46]. To avoid
having to infer protocol again, as for UDP fragmentation offload (the
retry case below this code).
@@ -2812,7 +2812,7 @@ static int set_offload(struct tun_struct *tun, unsigned long arg)arg&=~(TUN_F_TSO4|TUN_F_TSO6);}-arg&=~TUN_F_UFO;+arg&=~(TUN_F_UFO|TUN_F_USO);
It looks to me kernel doesn't use "USO", so TUN_F_UDP_GSO_L4 is a better
name for this
No problem, I can change it in v2
and I guess we should toggle NETIF_F_UDP_GSO_l4 here?
No, we do not, because this indicates only the fact that the guest can
send large UDP packets and have them splitted to UDP segments.
quoted
And how about macvtap?
We will check how to do that for macvtap. We will send a separate
patch for macvtap or ask for advice.
I'll add this feature to the tap.c also (AFAIU this will enable the
USO for macvtap).
Please correct me if I'm mistaken.
quoted
Thanks
quoted
}
/* This gives the user a way to test for new features in future by
From: Jason Wang <hidden> Date: 2021-05-12 01:21:49
在 2021/5/11 下午5:21, Yuri Benditovich 写道:
On Tue, May 11, 2021 at 11:24 AM Jason Wang [off-list ref] wrote:
quoted
On Tue, May 11, 2021 at 4:12 PM Yuri Benditovich
[off-list ref] wrote:
quoted
On Tue, May 11, 2021 at 9:47 AM Jason Wang [off-list ref] wrote:
quoted
在 2021/5/11 下午12:42, Yuri Benditovich 写道:
quoted
Define feature bit and GSO type according to the VIRTIO
specification.
Signed-off-by: Yuri Benditovich <redacted>
---
include/uapi/linux/virtio_net.h | 2 ++
1 file changed, 2 insertions(+)
@@ -57,6 +57,7 @@*Steering*/#define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */+#define VIRTIO_NET_F_HOST_USO 56 /* Host can handle USO packets */
This is the gso_type not the feature actually.
I wonder what's the reason for not
1) introducing a dedicated virtio-net feature bit for this
(VIRTIO_NET_F_GUEST_GSO_UDP_L4.
This series is not for GUEST's feature, it is only for host feature.
quoted
2) toggle the NETIF_F_GSO_UDP_L4 feature for tuntap based on the
negotiated feature.
The NETIF_F_GSO_UDP_L4 would be required for the guest RX path.
The guest TX path does not require any flags to be propagated, it only
allows the guest to transmit large UDP packets and have them
automatically splitted.
(This is similar to HOST_UFO but does packet segmentation instead of
fragmentation. GUEST_UFO indeed requires a respective NETIF flag, as
it is unclear whether the guest is capable of receiving such packets).
So I think it's better to implement TX/RX in the same series unless
there's something missed:
For Guest TX, NETIF_F_GSO_UDP_L4 needs to be enabled in the guest
virtio-net only when VIRTIO_NET_F_HOST_USO is negotiated.
I understand that this is what should be done when this feature will
be added to Linux virtio-net driver.
But at the moment we do not have enough resources to work on it.
Currently we have a clear use case and ability to test in on Windows guest.
Respective QEMU changes are pending for kernel patches, current
reference is https://github.com/daynix/qemu/tree/uso
This looks fine but as replied in another thread.
We can test both TX and RX with Linux guests simply:
We can just use 2 VMs, and let one VM send GSO_UDP_L4 packet to another,
then both tx and rx in both guest (virtio-net) and host (virtio-net) are
tested?
Thanks
quoted
For guest RX, NETIF_F_GSO_UDP_L4 needs to be enabled on the host
tuntap only when VIRTIO_NET_F_GUEST_USO is neogiated.
Currently we are not able to use guest RX UDP GSO.
In order to do that we at least should be able to build our Windows
drivers with the most updated driver development kit (2004+).
At the moment we can't, this task is in a plan but can take several
months. So we do not have a test/use case with Windows VM.
quoted
Thanks
quoted
quoted
Thanks
quoted
#define VIRTIO_NET_HDR_GSO_ECN 0x80 /* TCP has ECN set */
__u8 gso_type;
__virtio16 hdr_len; /* Ethernet + IP + tcp/udp hdrs */
@@ -2812,7 +2812,7 @@ static int set_offload(struct tun_struct *tun, unsigned long arg)arg&=~(TUN_F_TSO4|TUN_F_TSO6);}-arg&=~TUN_F_UFO;+arg&=~(TUN_F_UFO|TUN_F_USO);
It looks to me kernel doesn't use "USO", so TUN_F_UDP_GSO_L4 is a better
name for this
No problem, I can change it in v2
and I guess we should toggle NETIF_F_UDP_GSO_l4 here?
No, we do not, because this indicates only the fact that the guest can
send large UDP packets and have them splitted to UDP segments.
Actually the reverse. The set_offload() controls the tuntap TX path
(guest RX path).
When VIRTIO_NET_F_GUEST_XXX was not negotiated, the corresponding netdev
features needs to be disabled. When host tries to send those packets to
guest, it needs to do software segmentation.
See virtio_net_apply_guest_offloads().
There's currently no way (or not need) to prevent tuntap from receiving
GSO packets.
Thanks
quoted
And how about macvtap?
We will check how to do that for macvtap. We will send a separate
patch for macvtap or ask for advice.
quoted
Thanks
quoted
}
/* This gives the user a way to test for new features in future by
@@ -2812,7 +2812,7 @@ static int set_offload(struct tun_struct *tun, unsigned long arg)arg&=~(TUN_F_TSO4|TUN_F_TSO6);}-arg&=~TUN_F_UFO;+arg&=~(TUN_F_UFO|TUN_F_USO);
It looks to me kernel doesn't use "USO", so TUN_F_UDP_GSO_L4 is a better
name for this
No problem, I can change it in v2
and I guess we should toggle NETIF_F_UDP_GSO_l4 here?
No, we do not, because this indicates only the fact that the guest can
send large UDP packets and have them splitted to UDP segments.
Actually the reverse. The set_offload() controls the tuntap TX path
(guest RX path).
The set_offloads does 2 things:
1. At the initialization time qemu probes set_offload(something) to
check which features are supported by TAP/TUN.
2. Later it configures the guest RX path according to guest's needs/capabilities
Typical initialization sequence is (in case the QEMU supports USO feature):
TAP/TUN set offload 11 (probe for UFO support)
TAP/TUN set offload 21 (probe for USO support)
TAP/TUN set offload 0
...
TAP/TUN set offload 7 (configuration of offloads according to GUEST features)
This series of patches is for VIRTIO_NET_F_HOST_USO only, virtio-net
features like VIRTIO_NET_F_GUEST_USO_(4/6/whatever) are not defined in
the spec yet.
When VIRTIO_NET_F_GUEST_XXX was not negotiated, the corresponding netdev
features needs to be disabled. When host tries to send those packets to
guest, it needs to do software segmentation.
See virtio_net_apply_guest_offloads().
There's currently no way (or not need) to prevent tuntap from receiving
GSO packets.
Thanks
quoted
quoted
And how about macvtap?
We will check how to do that for macvtap. We will send a separate
patch for macvtap or ask for advice.
quoted
Thanks
quoted
}
/* This gives the user a way to test for new features in future by
On Tue, May 11, 2021 at 8:48 PM Willem de Bruijn
[off-list ref] wrote:
On Tue, May 11, 2021 at 12:43 AM Yuri Benditovich
[off-list ref] wrote:
quoted
Large UDP packet provided by the guest with GSO type set to
VIRTIO_NET_HDR_GSO_UDP_L4 will be divided to several UDP
packets according to the gso_size field.
Signed-off-by: Yuri Benditovich <redacted>
---
include/linux/virtio_net.h | 5 +++++
1 file changed, 5 insertions(+)
@@ -51,6 +51,11 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,ip_proto=IPPROTO_UDP;thlen=sizeof(structudphdr);break;+caseVIRTIO_NET_HDR_GSO_UDP_L4:+gso_type=SKB_GSO_UDP_L4;+ip_proto=IPPROTO_UDP;+thlen=sizeof(structudphdr);+break;
If adding a new VIRTIO_NET_HDR type I suggest adding separate IPv4 and
IPv6 variants, analogous to VIRTIO_NET_HDR_GSO_TCPV[46]. To avoid
having to infer protocol again, as for UDP fragmentation offload (the
retry case below this code).
Thank you for denoting this important point of distinguishing between v4 and v6.
Let's try to take a deeper look to see what is the correct thing to do
and please correct me if I'm wrong:
1. For USO we do not need to guess the protocol as it is used with
VIRTIO_NET_HDR_F_NEEDS_CSUM (unlike UFO) and the USO packets
transmitted by the guest are under the same clause as both
VIRTIO_NET_HDR_GSO_TCP, i.e. under if (hdr->flags &
VIRTIO_NET_HDR_F_NEEDS_CSUM) {
2. If we even define VIRTIO_NET_HDR_GSO_UDPv4_L4 and
VIRTIO_NET_HDR_GSO_UDPv6_L4 - both will be translated to
SKB_GSO_UDP_L4, so this information is immediately lost (the code will
look like:
case VIRTIO_NET_HDR_GSO_UDP4_L4: case VIRTIO_NET_HDR_GSO_UDP6_L4
gso_type = SKB_GSO_UDP;
3. When we will define the respective guest features (like
VIRTIO_NET_F_HOST_USO4 VIRTIO_NET_F_HOST_USO6) we will need to
recreate the virtio_net header from the skb when both v4 and v6 have
the same SKB_GSO_UDP_L4, (see virtio_net_hdr_from_skb) and I'm not
sure whether somebody needs the exact v4 or v6 information on guest RX
path.
4. What is completely correct is that when we will start working with
the guest RX path we will need to define something like NETIF_F_USO4
and NETIF_F_USO6 and configure them according to exact guest offload
capabilities.
Do you agree?
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com> Date: 2021-05-12 14:33:24
On Wed, May 12, 2021 at 2:10 AM Yuri Benditovich
[off-list ref] wrote:
On Tue, May 11, 2021 at 8:48 PM Willem de Bruijn
[off-list ref] wrote:
quoted
On Tue, May 11, 2021 at 12:43 AM Yuri Benditovich
[off-list ref] wrote:
quoted
Large UDP packet provided by the guest with GSO type set to
VIRTIO_NET_HDR_GSO_UDP_L4 will be divided to several UDP
packets according to the gso_size field.
Signed-off-by: Yuri Benditovich <redacted>
---
include/linux/virtio_net.h | 5 +++++
1 file changed, 5 insertions(+)
@@ -51,6 +51,11 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,ip_proto=IPPROTO_UDP;thlen=sizeof(structudphdr);break;+caseVIRTIO_NET_HDR_GSO_UDP_L4:+gso_type=SKB_GSO_UDP_L4;+ip_proto=IPPROTO_UDP;+thlen=sizeof(structudphdr);+break;
If adding a new VIRTIO_NET_HDR type I suggest adding separate IPv4 and
IPv6 variants, analogous to VIRTIO_NET_HDR_GSO_TCPV[46]. To avoid
having to infer protocol again, as for UDP fragmentation offload (the
retry case below this code).
Thank you for denoting this important point of distinguishing between v4 and v6.
Let's try to take a deeper look to see what is the correct thing to do
and please correct me if I'm wrong:
1. For USO we do not need to guess the protocol as it is used with
VIRTIO_NET_HDR_F_NEEDS_CSUM (unlike UFO)
Enforcing that is a good start. We should also enforce that
skb->protocol is initialized to one of htons(ETH_P_IP) or
htons(ETH_P_IPV6), so that it does not have to be inferred by parsing.
These requirements were not enforced for previous values, and cannot
be introduced afterwards, which has led to have to add that extra code
to handle these obscure edge cases.
I agree that with well behaved configurations, the need for separate
_V4 and _V6 variants is not needed.
and the USO packets
transmitted by the guest are under the same clause as both
VIRTIO_NET_HDR_GSO_TCP, i.e. under if (hdr->flags &
VIRTIO_NET_HDR_F_NEEDS_CSUM) {
2. If we even define VIRTIO_NET_HDR_GSO_UDPv4_L4 and
VIRTIO_NET_HDR_GSO_UDPv6_L4 - both will be translated to
SKB_GSO_UDP_L4, so this information is immediately lost (the code will
look like:
case VIRTIO_NET_HDR_GSO_UDP4_L4: case VIRTIO_NET_HDR_GSO_UDP6_L4
gso_type = SKB_GSO_UDP;
3. When we will define the respective guest features (like
VIRTIO_NET_F_HOST_USO4 VIRTIO_NET_F_HOST_USO6) we will need to
recreate the virtio_net header from the skb when both v4 and v6 have
the same SKB_GSO_UDP_L4, (see virtio_net_hdr_from_skb) and I'm not
sure whether somebody needs the exact v4 or v6 information on guest RX
path.
FWIW, it is good to keep in mind that virtio_net_hdr is also used
outside virtio, in both ingress and egress paths.
4. What is completely correct is that when we will start working with
the guest RX path we will need to define something like NETIF_F_USO4
and NETIF_F_USO6 and configure them according to exact guest offload
capabilities.
Do you agree?
I don't immediately see the need for advertising this device feature
on a per-protocol basis. Can you elaborate?
On Wed, May 12, 2021 at 5:33 PM Willem de Bruijn
[off-list ref] wrote:
On Wed, May 12, 2021 at 2:10 AM Yuri Benditovich
[off-list ref] wrote:
quoted
On Tue, May 11, 2021 at 8:48 PM Willem de Bruijn
[off-list ref] wrote:
quoted
On Tue, May 11, 2021 at 12:43 AM Yuri Benditovich
[off-list ref] wrote:
quoted
Large UDP packet provided by the guest with GSO type set to
VIRTIO_NET_HDR_GSO_UDP_L4 will be divided to several UDP
packets according to the gso_size field.
Signed-off-by: Yuri Benditovich <redacted>
---
include/linux/virtio_net.h | 5 +++++
1 file changed, 5 insertions(+)
@@ -51,6 +51,11 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,ip_proto=IPPROTO_UDP;thlen=sizeof(structudphdr);break;+caseVIRTIO_NET_HDR_GSO_UDP_L4:+gso_type=SKB_GSO_UDP_L4;+ip_proto=IPPROTO_UDP;+thlen=sizeof(structudphdr);+break;
If adding a new VIRTIO_NET_HDR type I suggest adding separate IPv4 and
IPv6 variants, analogous to VIRTIO_NET_HDR_GSO_TCPV[46]. To avoid
having to infer protocol again, as for UDP fragmentation offload (the
retry case below this code).
Thank you for denoting this important point of distinguishing between v4 and v6.
Let's try to take a deeper look to see what is the correct thing to do
and please correct me if I'm wrong:
1. For USO we do not need to guess the protocol as it is used with
VIRTIO_NET_HDR_F_NEEDS_CSUM (unlike UFO)
Enforcing that is a good start. We should also enforce that
skb->protocol is initialized to one of htons(ETH_P_IP) or
htons(ETH_P_IPV6), so that it does not have to be inferred by parsing.
As this feature is new and is not used in any public release of any
misbehaving driver, probably it is enough to state in the spec that
VIRTIO_NET_HDR_F_NEEDS_CSUM is required for USO packets.
The spec states that the USO feature requires checksumming feature.
These requirements were not enforced for previous values, and cannot
be introduced afterwards, which has led to have to add that extra code
to handle these obscure edge cases.
I agree that with well behaved configurations, the need for separate
_V4 and _V6 variants is not needed.
quoted
and the USO packets
transmitted by the guest are under the same clause as both
VIRTIO_NET_HDR_GSO_TCP, i.e. under if (hdr->flags &
VIRTIO_NET_HDR_F_NEEDS_CSUM) {
2. If we even define VIRTIO_NET_HDR_GSO_UDPv4_L4 and
VIRTIO_NET_HDR_GSO_UDPv6_L4 - both will be translated to
SKB_GSO_UDP_L4, so this information is immediately lost (the code will
look like:
case VIRTIO_NET_HDR_GSO_UDP4_L4: case VIRTIO_NET_HDR_GSO_UDP6_L4
gso_type = SKB_GSO_UDP;
3. When we will define the respective guest features (like
VIRTIO_NET_F_HOST_USO4 VIRTIO_NET_F_HOST_USO6) we will need to
This is my typo: VIRTIO_NET_F_GUEST_USO4...
quoted
recreate the virtio_net header from the skb when both v4 and v6 have
the same SKB_GSO_UDP_L4, (see virtio_net_hdr_from_skb) and I'm not
sure whether somebody needs the exact v4 or v6 information on guest RX
path.
FWIW, it is good to keep in mind that virtio_net_hdr is also used
outside virtio, in both ingress and egress paths.
Can you please elaborate in which scenarios we do not have any virtio
device in path but need virtio_net_hdr?
quoted
4. What is completely correct is that when we will start working with
the guest RX path we will need to define something like NETIF_F_USO4
and NETIF_F_USO6 and configure them according to exact guest offload
capabilities.
Do you agree?
I don't immediately see the need for advertising this device feature
on a per-protocol basis. Can you elaborate?
Separate offload setting (controlled by the guest) for v4 and v6 in
guest RX path is mandatory, at least Windows always requires this for
any offload.
In this case it seems easy to have also virtio-net device features to
be indicated separately (the TAP/TUN should report its capabilities).
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com> Date: 2021-05-12 20:34:35
On Wed, May 12, 2021 at 2:56 PM Yuri Benditovich
[off-list ref] wrote:
On Wed, May 12, 2021 at 5:33 PM Willem de Bruijn
[off-list ref] wrote:
quoted
On Wed, May 12, 2021 at 2:10 AM Yuri Benditovich
[off-list ref] wrote:
quoted
On Tue, May 11, 2021 at 8:48 PM Willem de Bruijn
[off-list ref] wrote:
quoted
On Tue, May 11, 2021 at 12:43 AM Yuri Benditovich
[off-list ref] wrote:
quoted
Large UDP packet provided by the guest with GSO type set to
VIRTIO_NET_HDR_GSO_UDP_L4 will be divided to several UDP
packets according to the gso_size field.
Signed-off-by: Yuri Benditovich <redacted>
---
include/linux/virtio_net.h | 5 +++++
1 file changed, 5 insertions(+)
@@ -51,6 +51,11 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,ip_proto=IPPROTO_UDP;thlen=sizeof(structudphdr);break;+caseVIRTIO_NET_HDR_GSO_UDP_L4:+gso_type=SKB_GSO_UDP_L4;+ip_proto=IPPROTO_UDP;+thlen=sizeof(structudphdr);+break;
If adding a new VIRTIO_NET_HDR type I suggest adding separate IPv4 and
IPv6 variants, analogous to VIRTIO_NET_HDR_GSO_TCPV[46]. To avoid
having to infer protocol again, as for UDP fragmentation offload (the
retry case below this code).
Thank you for denoting this important point of distinguishing between v4 and v6.
Let's try to take a deeper look to see what is the correct thing to do
and please correct me if I'm wrong:
1. For USO we do not need to guess the protocol as it is used with
VIRTIO_NET_HDR_F_NEEDS_CSUM (unlike UFO)
Enforcing that is a good start. We should also enforce that
skb->protocol is initialized to one of htons(ETH_P_IP) or
htons(ETH_P_IPV6), so that it does not have to be inferred by parsing.
As this feature is new and is not used in any public release of any
misbehaving driver, probably it is enough to state in the spec that
VIRTIO_NET_HDR_F_NEEDS_CSUM is required for USO packets.
The spec states that the USO feature requires checksumming feature.
The spec is not sufficient. These rules need to be enforced in the
kernel code, too.
quoted
These requirements were not enforced for previous values, and cannot
be introduced afterwards, which has led to have to add that extra code
to handle these obscure edge cases.
I agree that with well behaved configurations, the need for separate
_V4 and _V6 variants is not needed.
quoted
and the USO packets
transmitted by the guest are under the same clause as both
VIRTIO_NET_HDR_GSO_TCP, i.e. under if (hdr->flags &
VIRTIO_NET_HDR_F_NEEDS_CSUM) {
2. If we even define VIRTIO_NET_HDR_GSO_UDPv4_L4 and
VIRTIO_NET_HDR_GSO_UDPv6_L4 - both will be translated to
SKB_GSO_UDP_L4, so this information is immediately lost (the code will
look like:
case VIRTIO_NET_HDR_GSO_UDP4_L4: case VIRTIO_NET_HDR_GSO_UDP6_L4
gso_type = SKB_GSO_UDP;
3. When we will define the respective guest features (like
VIRTIO_NET_F_HOST_USO4 VIRTIO_NET_F_HOST_USO6) we will need to
This is my typo: VIRTIO_NET_F_GUEST_USO4...
quoted
quoted
recreate the virtio_net header from the skb when both v4 and v6 have
the same SKB_GSO_UDP_L4, (see virtio_net_hdr_from_skb) and I'm not
sure whether somebody needs the exact v4 or v6 information on guest RX
path.
FWIW, it is good to keep in mind that virtio_net_hdr is also used
outside virtio, in both ingress and egress paths.
Can you please elaborate in which scenarios we do not have any virtio
device in path but need virtio_net_hdr?
Packet sockets, tuntap.
quoted
quoted
4. What is completely correct is that when we will start working with
the guest RX path we will need to define something like NETIF_F_USO4
and NETIF_F_USO6 and configure them according to exact guest offload
capabilities.
Do you agree?
I don't immediately see the need for advertising this device feature
on a per-protocol basis. Can you elaborate?
Separate offload setting (controlled by the guest) for v4 and v6 in
guest RX path is mandatory, at least Windows always requires this for
any offload.
In this case it seems easy to have also virtio-net device features to
be indicated separately (the TAP/TUN should report its capabilities).