* [PATCH net-next 0/5] virtio-net: Add SCTP checksum offload support @ 2018-04-02 13:40 ` Vladislav Yasevich 0 siblings, 0 replies; 74+ messages in thread From: Vladislav Yasevich @ 2018-04-02 13:40 UTC (permalink / raw) To: netdev; +Cc: nhorman, mst, virtualization, linux-sctp Now that we have SCTP offload capabilities in the kernel, we can add them to virtio as well. First step is SCTP checksum. We need a new freature in virtio to negotiate this support since SCTP is excluded with the stardard checksum and requires a little bit extra. This series proposes VIRTIO_NET_F_SCTP_CSUM feature bit. As the "little bit extra", the kernel uses a new bit in the skb (skb->csum_not_inet) to determine whether to use standard inet checksum or the SCTP CRC32c checksum. This bit has to be communicated between the host and the guest. This bit is carried in the vnet header. Tap and macvtap support is added through an extra feature for the TUNSETOFFLOAD ioctl. Additionally macvtap will no correctly do sctp checksumming if the receive doesn't support SCTP offload. This also turns on sctp offloading for macvlan devices. As for the perf numbers, I am seeing about a 5% increase in vm-to-vm and vm-to-hos throughput which is the same as manually disabling sctp checksumming,since this is exactly what we are emulatting. Sending outside the host, the increase about 2.5-3%. As for GSO, the way sctp GSO is currently implemented buys us nothing in added support to virtio. To add true GSO, would require a lot of re-work inside of SCTP and would require extensions to the virtio net header to carry extra sctp data. Vladislav Yasevich (5): virtio: Add support for SCTP checksum offloading sctp: Handle sctp packets with CHECKSUM_PARTIAL sctp: Build sctp offload support into the base kernel tun: Add support for SCTP checksum offload macvlan/macvtap: Add support for SCTP checksum offload. drivers/net/macvlan.c | 5 +++-- drivers/net/tap.c | 8 +++++--- drivers/net/tun.c | 5 +++++ drivers/net/virtio_net.c | 10 +++++++--- include/linux/virtio_net.h | 6 ++++++ include/net/sctp/sctp.h | 5 ----- include/uapi/linux/if_tun.h | 1 + include/uapi/linux/virtio_net.h | 2 ++ net/Kconfig | 1 + net/sctp/Kconfig | 1 - net/sctp/Makefile | 3 ++- net/sctp/input.c | 11 ++++++++++- net/sctp/offload.c | 4 +++- net/sctp/protocol.c | 3 --- 14 files changed, 45 insertions(+), 20 deletions(-) -- 2.9.5 ^ permalink raw reply [flat|nested] 74+ messages in thread
* [PATCH net-next 0/5] virtio-net: Add SCTP checksum offload support @ 2018-04-02 13:40 ` Vladislav Yasevich 0 siblings, 0 replies; 74+ messages in thread From: Vladislav Yasevich @ 2018-04-02 13:40 UTC (permalink / raw) To: netdev; +Cc: nhorman, mst, virtualization, linux-sctp Now that we have SCTP offload capabilities in the kernel, we can add them to virtio as well. First step is SCTP checksum. We need a new freature in virtio to negotiate this support since SCTP is excluded with the stardard checksum and requires a little bit extra. This series proposes VIRTIO_NET_F_SCTP_CSUM feature bit. As the "little bit extra", the kernel uses a new bit in the skb (skb->csum_not_inet) to determine whether to use standard inet checksum or the SCTP CRC32c checksum. This bit has to be communicated between the host and the guest. This bit is carried in the vnet header. Tap and macvtap support is added through an extra feature for the TUNSETOFFLOAD ioctl. Additionally macvtap will no correctly do sctp checksumming if the receive doesn't support SCTP offload. This also turns on sctp offloading for macvlan devices. As for the perf numbers, I am seeing about a 5% increase in vm-to-vm and vm-to-hos throughput which is the same as manually disabling sctp checksumming,since this is exactly what we are emulatting. Sending outside the host, the increase about 2.5-3%. As for GSO, the way sctp GSO is currently implemented buys us nothing in added support to virtio. To add true GSO, would require a lot of re-work inside of SCTP and would require extensions to the virtio net header to carry extra sctp data. Vladislav Yasevich (5): virtio: Add support for SCTP checksum offloading sctp: Handle sctp packets with CHECKSUM_PARTIAL sctp: Build sctp offload support into the base kernel tun: Add support for SCTP checksum offload macvlan/macvtap: Add support for SCTP checksum offload. drivers/net/macvlan.c | 5 +++-- drivers/net/tap.c | 8 +++++--- drivers/net/tun.c | 5 +++++ drivers/net/virtio_net.c | 10 +++++++--- include/linux/virtio_net.h | 6 ++++++ include/net/sctp/sctp.h | 5 ----- include/uapi/linux/if_tun.h | 1 + include/uapi/linux/virtio_net.h | 2 ++ net/Kconfig | 1 + net/sctp/Kconfig | 1 - net/sctp/Makefile | 3 ++- net/sctp/input.c | 11 ++++++++++- net/sctp/offload.c | 4 +++- net/sctp/protocol.c | 3 --- 14 files changed, 45 insertions(+), 20 deletions(-) -- 2.9.5 ^ permalink raw reply [flat|nested] 74+ messages in thread
* [PATCH net-next 1/5] virtio: Add support for SCTP checksum offloading 2018-04-02 13:40 ` Vladislav Yasevich @ 2018-04-02 13:40 ` Vladislav Yasevich -1 siblings, 0 replies; 74+ messages in thread From: Vladislav Yasevich @ 2018-04-02 13:40 UTC (permalink / raw) To: netdev Cc: linux-sctp, virtualization, mst, jasowang, nhorman, Vladislav Yasevich To support SCTP checksum offloading, we need to add a new feature to virtio_net, so we can negotiate support between the hypervisor and the guest. The signalling to the guest that an alternate checksum needs to be used is done via a new flag in the virtio_net_hdr. If the flag is set, the host will know to perform an alternate checksum calculation, which right now is only CRC32c. Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> --- drivers/net/virtio_net.c | 11 ++++++++--- include/linux/virtio_net.h | 6 ++++++ include/uapi/linux/virtio_net.h | 2 ++ 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 7b187ec..b601294 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -2724,9 +2724,14 @@ static int virtnet_probe(struct virtio_device *vdev) /* Do we support "hardware" checksums? */ if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) { /* This opens up the world of extra features. */ - dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG; + netdev_features_t sctp = 0; + + if (virtio_has_feature(vdev, VIRTIO_NET_F_SCTP_CSUM)) + sctp |= NETIF_F_SCTP_CRC; + + dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; if (csum) - dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG; + dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) { dev->hw_features |= NETIF_F_TSO @@ -2952,7 +2957,7 @@ static struct virtio_device_id id_table[] = { }; #define VIRTNET_FEATURES \ - VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, \ + VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, VIRTIO_NET_F_SCTP_CSUM, \ VIRTIO_NET_F_MAC, \ VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6, \ VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6, \ diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h index f144216..2e7a64a 100644 --- a/include/linux/virtio_net.h +++ b/include/linux/virtio_net.h @@ -39,6 +39,9 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, if (!skb_partial_csum_set(skb, start, off)) return -EINVAL; + + if (hdr->flags & VIRTIO_NET_HDR_F_CSUM_NOT_INET) + skb->csum_not_inet = 1; } if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) { @@ -96,6 +99,9 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb, hdr->flags = VIRTIO_NET_HDR_F_DATA_VALID; } /* else everything is zero */ + if (skb->csum_not_inet) + hdr->flags &= VIRTIO_NET_HDR_F_CSUM_NOT_INET; + return 0; } diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h index 5de6ed3..3f279c8 100644 --- a/include/uapi/linux/virtio_net.h +++ b/include/uapi/linux/virtio_net.h @@ -36,6 +36,7 @@ #define VIRTIO_NET_F_GUEST_CSUM 1 /* Guest handles pkts w/ partial csum */ #define VIRTIO_NET_F_CTRL_GUEST_OFFLOADS 2 /* Dynamic offload configuration. */ #define VIRTIO_NET_F_MTU 3 /* Initial MTU advice */ +#define VIRTIO_NET_F_SCTP_CSUM 4 /* SCTP checksum offload support */ #define VIRTIO_NET_F_MAC 5 /* Host has given MAC address. */ #define VIRTIO_NET_F_GUEST_TSO4 7 /* Guest can handle TSOv4 in. */ #define VIRTIO_NET_F_GUEST_TSO6 8 /* Guest can handle TSOv6 in. */ @@ -101,6 +102,7 @@ struct virtio_net_config { struct virtio_net_hdr_v1 { #define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start, csum_offset */ #define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */ +#define VIRTIO_NET_HDR_F_CSUM_NOT_INET 4 /* Checksum is not inet */ __u8 flags; #define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame */ #define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP (TSO) */ -- 2.9.5 ^ permalink raw reply related [flat|nested] 74+ messages in thread
* [PATCH net-next 1/5] virtio: Add support for SCTP checksum offloading @ 2018-04-02 13:40 ` Vladislav Yasevich 0 siblings, 0 replies; 74+ messages in thread From: Vladislav Yasevich @ 2018-04-02 13:40 UTC (permalink / raw) To: netdev Cc: linux-sctp, virtualization, mst, jasowang, nhorman, Vladislav Yasevich To support SCTP checksum offloading, we need to add a new feature to virtio_net, so we can negotiate support between the hypervisor and the guest. The signalling to the guest that an alternate checksum needs to be used is done via a new flag in the virtio_net_hdr. If the flag is set, the host will know to perform an alternate checksum calculation, which right now is only CRC32c. Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> --- drivers/net/virtio_net.c | 11 ++++++++--- include/linux/virtio_net.h | 6 ++++++ include/uapi/linux/virtio_net.h | 2 ++ 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 7b187ec..b601294 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -2724,9 +2724,14 @@ static int virtnet_probe(struct virtio_device *vdev) /* Do we support "hardware" checksums? */ if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) { /* This opens up the world of extra features. */ - dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG; + netdev_features_t sctp = 0; + + if (virtio_has_feature(vdev, VIRTIO_NET_F_SCTP_CSUM)) + sctp |= NETIF_F_SCTP_CRC; + + dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; if (csum) - dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG; + dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) { dev->hw_features |= NETIF_F_TSO @@ -2952,7 +2957,7 @@ static struct virtio_device_id id_table[] = { }; #define VIRTNET_FEATURES \ - VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, \ + VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, VIRTIO_NET_F_SCTP_CSUM, \ VIRTIO_NET_F_MAC, \ VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6, \ VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6, \ diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h index f144216..2e7a64a 100644 --- a/include/linux/virtio_net.h +++ b/include/linux/virtio_net.h @@ -39,6 +39,9 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, if (!skb_partial_csum_set(skb, start, off)) return -EINVAL; + + if (hdr->flags & VIRTIO_NET_HDR_F_CSUM_NOT_INET) + skb->csum_not_inet = 1; } if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) { @@ -96,6 +99,9 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb, hdr->flags = VIRTIO_NET_HDR_F_DATA_VALID; } /* else everything is zero */ + if (skb->csum_not_inet) + hdr->flags &= VIRTIO_NET_HDR_F_CSUM_NOT_INET; + return 0; } diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h index 5de6ed3..3f279c8 100644 --- a/include/uapi/linux/virtio_net.h +++ b/include/uapi/linux/virtio_net.h @@ -36,6 +36,7 @@ #define VIRTIO_NET_F_GUEST_CSUM 1 /* Guest handles pkts w/ partial csum */ #define VIRTIO_NET_F_CTRL_GUEST_OFFLOADS 2 /* Dynamic offload configuration. */ #define VIRTIO_NET_F_MTU 3 /* Initial MTU advice */ +#define VIRTIO_NET_F_SCTP_CSUM 4 /* SCTP checksum offload support */ #define VIRTIO_NET_F_MAC 5 /* Host has given MAC address. */ #define VIRTIO_NET_F_GUEST_TSO4 7 /* Guest can handle TSOv4 in. */ #define VIRTIO_NET_F_GUEST_TSO6 8 /* Guest can handle TSOv6 in. */ @@ -101,6 +102,7 @@ struct virtio_net_config { struct virtio_net_hdr_v1 { #define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start, csum_offset */ #define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */ +#define VIRTIO_NET_HDR_F_CSUM_NOT_INET 4 /* Checksum is not inet */ __u8 flags; #define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame */ #define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP (TSO) */ -- 2.9.5 ^ permalink raw reply related [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 1/5] virtio: Add support for SCTP checksum offloading 2018-04-02 13:40 ` Vladislav Yasevich (?) @ 2018-04-10 3:17 ` Jason Wang -1 siblings, 0 replies; 74+ messages in thread From: Jason Wang @ 2018-04-10 3:17 UTC (permalink / raw) To: Vladislav Yasevich, netdev; +Cc: linux-sctp, mst, nhorman, virtualization On 2018年04月02日 21:40, Vladislav Yasevich wrote: > To support SCTP checksum offloading, we need to add a new feature > to virtio_net, so we can negotiate support between the hypervisor > and the guest. > > The signalling to the guest that an alternate checksum needs to > be used is done via a new flag in the virtio_net_hdr. If the > flag is set, the host will know to perform an alternate checksum > calculation, which right now is only CRC32c. > > Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> > --- > drivers/net/virtio_net.c | 11 ++++++++--- > include/linux/virtio_net.h | 6 ++++++ > include/uapi/linux/virtio_net.h | 2 ++ > 3 files changed, 16 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 7b187ec..b601294 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -2724,9 +2724,14 @@ static int virtnet_probe(struct virtio_device *vdev) > /* Do we support "hardware" checksums? */ > if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) { > /* This opens up the world of extra features. */ > - dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG; > + netdev_features_t sctp = 0; > + > + if (virtio_has_feature(vdev, VIRTIO_NET_F_SCTP_CSUM)) > + sctp |= NETIF_F_SCTP_CRC; > + > + dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; > if (csum) > - dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG; > + dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; > > if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) { > dev->hw_features |= NETIF_F_TSO > @@ -2952,7 +2957,7 @@ static struct virtio_device_id id_table[] = { > }; > > #define VIRTNET_FEATURES \ > - VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, \ > + VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, VIRTIO_NET_F_SCTP_CSUM, \ It looks to me _F_SCTP_CSUM implies the ability of both device and driver. Do we still need the flexibility like csum to differ guest/host ability like e.g _F_GUEST_SCTP_CSUM? Thanks > VIRTIO_NET_F_MAC, \ > VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6, \ > VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6, \ > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h > index f144216..2e7a64a 100644 > --- a/include/linux/virtio_net.h > +++ b/include/linux/virtio_net.h > @@ -39,6 +39,9 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, > > if (!skb_partial_csum_set(skb, start, off)) > return -EINVAL; > + > + if (hdr->flags & VIRTIO_NET_HDR_F_CSUM_NOT_INET) > + skb->csum_not_inet = 1; > } > > if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) { > @@ -96,6 +99,9 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb, > hdr->flags = VIRTIO_NET_HDR_F_DATA_VALID; > } /* else everything is zero */ > > + if (skb->csum_not_inet) > + hdr->flags &= VIRTIO_NET_HDR_F_CSUM_NOT_INET; > + > return 0; > } > > diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h > index 5de6ed3..3f279c8 100644 > --- a/include/uapi/linux/virtio_net.h > +++ b/include/uapi/linux/virtio_net.h > @@ -36,6 +36,7 @@ > #define VIRTIO_NET_F_GUEST_CSUM 1 /* Guest handles pkts w/ partial csum */ > #define VIRTIO_NET_F_CTRL_GUEST_OFFLOADS 2 /* Dynamic offload configuration. */ > #define VIRTIO_NET_F_MTU 3 /* Initial MTU advice */ > +#define VIRTIO_NET_F_SCTP_CSUM 4 /* SCTP checksum offload support */ > #define VIRTIO_NET_F_MAC 5 /* Host has given MAC address. */ > #define VIRTIO_NET_F_GUEST_TSO4 7 /* Guest can handle TSOv4 in. */ > #define VIRTIO_NET_F_GUEST_TSO6 8 /* Guest can handle TSOv6 in. */ > @@ -101,6 +102,7 @@ struct virtio_net_config { > struct virtio_net_hdr_v1 { > #define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start, csum_offset */ > #define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */ > +#define VIRTIO_NET_HDR_F_CSUM_NOT_INET 4 /* Checksum is not inet */ > __u8 flags; > #define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame */ > #define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP (TSO) */ _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 1/5] virtio: Add support for SCTP checksum offloading 2018-04-02 13:40 ` Vladislav Yasevich @ 2018-04-10 3:17 ` Jason Wang -1 siblings, 0 replies; 74+ messages in thread From: Jason Wang @ 2018-04-10 3:17 UTC (permalink / raw) To: Vladislav Yasevich, netdev Cc: linux-sctp, virtualization, mst, nhorman, Vladislav Yasevich On 2018年04月02日 21:40, Vladislav Yasevich wrote: > To support SCTP checksum offloading, we need to add a new feature > to virtio_net, so we can negotiate support between the hypervisor > and the guest. > > The signalling to the guest that an alternate checksum needs to > be used is done via a new flag in the virtio_net_hdr. If the > flag is set, the host will know to perform an alternate checksum > calculation, which right now is only CRC32c. > > Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> > --- > drivers/net/virtio_net.c | 11 ++++++++--- > include/linux/virtio_net.h | 6 ++++++ > include/uapi/linux/virtio_net.h | 2 ++ > 3 files changed, 16 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 7b187ec..b601294 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -2724,9 +2724,14 @@ static int virtnet_probe(struct virtio_device *vdev) > /* Do we support "hardware" checksums? */ > if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) { > /* This opens up the world of extra features. */ > - dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG; > + netdev_features_t sctp = 0; > + > + if (virtio_has_feature(vdev, VIRTIO_NET_F_SCTP_CSUM)) > + sctp |= NETIF_F_SCTP_CRC; > + > + dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; > if (csum) > - dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG; > + dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; > > if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) { > dev->hw_features |= NETIF_F_TSO > @@ -2952,7 +2957,7 @@ static struct virtio_device_id id_table[] = { > }; > > #define VIRTNET_FEATURES \ > - VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, \ > + VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, VIRTIO_NET_F_SCTP_CSUM, \ It looks to me _F_SCTP_CSUM implies the ability of both device and driver. Do we still need the flexibility like csum to differ guest/host ability like e.g _F_GUEST_SCTP_CSUM? Thanks > VIRTIO_NET_F_MAC, \ > VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6, \ > VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6, \ > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h > index f144216..2e7a64a 100644 > --- a/include/linux/virtio_net.h > +++ b/include/linux/virtio_net.h > @@ -39,6 +39,9 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, > > if (!skb_partial_csum_set(skb, start, off)) > return -EINVAL; > + > + if (hdr->flags & VIRTIO_NET_HDR_F_CSUM_NOT_INET) > + skb->csum_not_inet = 1; > } > > if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) { > @@ -96,6 +99,9 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb, > hdr->flags = VIRTIO_NET_HDR_F_DATA_VALID; > } /* else everything is zero */ > > + if (skb->csum_not_inet) > + hdr->flags &= VIRTIO_NET_HDR_F_CSUM_NOT_INET; > + > return 0; > } > > diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h > index 5de6ed3..3f279c8 100644 > --- a/include/uapi/linux/virtio_net.h > +++ b/include/uapi/linux/virtio_net.h > @@ -36,6 +36,7 @@ > #define VIRTIO_NET_F_GUEST_CSUM 1 /* Guest handles pkts w/ partial csum */ > #define VIRTIO_NET_F_CTRL_GUEST_OFFLOADS 2 /* Dynamic offload configuration. */ > #define VIRTIO_NET_F_MTU 3 /* Initial MTU advice */ > +#define VIRTIO_NET_F_SCTP_CSUM 4 /* SCTP checksum offload support */ > #define VIRTIO_NET_F_MAC 5 /* Host has given MAC address. */ > #define VIRTIO_NET_F_GUEST_TSO4 7 /* Guest can handle TSOv4 in. */ > #define VIRTIO_NET_F_GUEST_TSO6 8 /* Guest can handle TSOv6 in. */ > @@ -101,6 +102,7 @@ struct virtio_net_config { > struct virtio_net_hdr_v1 { > #define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start, csum_offset */ > #define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */ > +#define VIRTIO_NET_HDR_F_CSUM_NOT_INET 4 /* Checksum is not inet */ > __u8 flags; > #define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame */ > #define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP (TSO) */ ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 1/5] virtio: Add support for SCTP checksum offloading @ 2018-04-10 3:17 ` Jason Wang 0 siblings, 0 replies; 74+ messages in thread From: Jason Wang @ 2018-04-10 3:17 UTC (permalink / raw) To: Vladislav Yasevich, netdev Cc: linux-sctp, virtualization, mst, nhorman, Vladislav Yasevich On 2018年04月02日 21:40, Vladislav Yasevich wrote: > To support SCTP checksum offloading, we need to add a new feature > to virtio_net, so we can negotiate support between the hypervisor > and the guest. > > The signalling to the guest that an alternate checksum needs to > be used is done via a new flag in the virtio_net_hdr. If the > flag is set, the host will know to perform an alternate checksum > calculation, which right now is only CRC32c. > > Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> > --- > drivers/net/virtio_net.c | 11 ++++++++--- > include/linux/virtio_net.h | 6 ++++++ > include/uapi/linux/virtio_net.h | 2 ++ > 3 files changed, 16 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 7b187ec..b601294 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -2724,9 +2724,14 @@ static int virtnet_probe(struct virtio_device *vdev) > /* Do we support "hardware" checksums? */ > if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) { > /* This opens up the world of extra features. */ > - dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG; > + netdev_features_t sctp = 0; > + > + if (virtio_has_feature(vdev, VIRTIO_NET_F_SCTP_CSUM)) > + sctp |= NETIF_F_SCTP_CRC; > + > + dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; > if (csum) > - dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG; > + dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; > > if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) { > dev->hw_features |= NETIF_F_TSO > @@ -2952,7 +2957,7 @@ static struct virtio_device_id id_table[] = { > }; > > #define VIRTNET_FEATURES \ > - VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, \ > + VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, VIRTIO_NET_F_SCTP_CSUM, \ It looks to me _F_SCTP_CSUM implies the ability of both device and driver. Do we still need the flexibility like csum to differ guest/host ability like e.g _F_GUEST_SCTP_CSUM? Thanks > VIRTIO_NET_F_MAC, \ > VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6, \ > VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6, \ > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h > index f144216..2e7a64a 100644 > --- a/include/linux/virtio_net.h > +++ b/include/linux/virtio_net.h > @@ -39,6 +39,9 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, > > if (!skb_partial_csum_set(skb, start, off)) > return -EINVAL; > + > + if (hdr->flags & VIRTIO_NET_HDR_F_CSUM_NOT_INET) > + skb->csum_not_inet = 1; > } > > if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) { > @@ -96,6 +99,9 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb, > hdr->flags = VIRTIO_NET_HDR_F_DATA_VALID; > } /* else everything is zero */ > > + if (skb->csum_not_inet) > + hdr->flags &= VIRTIO_NET_HDR_F_CSUM_NOT_INET; > + > return 0; > } > > diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h > index 5de6ed3..3f279c8 100644 > --- a/include/uapi/linux/virtio_net.h > +++ b/include/uapi/linux/virtio_net.h > @@ -36,6 +36,7 @@ > #define VIRTIO_NET_F_GUEST_CSUM 1 /* Guest handles pkts w/ partial csum */ > #define VIRTIO_NET_F_CTRL_GUEST_OFFLOADS 2 /* Dynamic offload configuration. */ > #define VIRTIO_NET_F_MTU 3 /* Initial MTU advice */ > +#define VIRTIO_NET_F_SCTP_CSUM 4 /* SCTP checksum offload support */ > #define VIRTIO_NET_F_MAC 5 /* Host has given MAC address. */ > #define VIRTIO_NET_F_GUEST_TSO4 7 /* Guest can handle TSOv4 in. */ > #define VIRTIO_NET_F_GUEST_TSO6 8 /* Guest can handle TSOv6 in. */ > @@ -101,6 +102,7 @@ struct virtio_net_config { > struct virtio_net_hdr_v1 { > #define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start, csum_offset */ > #define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */ > +#define VIRTIO_NET_HDR_F_CSUM_NOT_INET 4 /* Checksum is not inet */ > __u8 flags; > #define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame */ > #define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP (TSO) */ ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 1/5] virtio: Add support for SCTP checksum offloading 2018-04-02 13:40 ` Vladislav Yasevich @ 2018-04-11 22:39 ` Marcelo Ricardo Leitner -1 siblings, 0 replies; 74+ messages in thread From: Marcelo Ricardo Leitner @ 2018-04-11 22:39 UTC (permalink / raw) To: Vladislav Yasevich Cc: netdev, linux-sctp, virtualization, mst, jasowang, nhorman, Vladislav Yasevich On Mon, Apr 02, 2018 at 09:40:02AM -0400, Vladislav Yasevich wrote: > To support SCTP checksum offloading, we need to add a new feature > to virtio_net, so we can negotiate support between the hypervisor > and the guest. > > The signalling to the guest that an alternate checksum needs to > be used is done via a new flag in the virtio_net_hdr. If the > flag is set, the host will know to perform an alternate checksum > calculation, which right now is only CRC32c. > > Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> > --- > drivers/net/virtio_net.c | 11 ++++++++--- > include/linux/virtio_net.h | 6 ++++++ > include/uapi/linux/virtio_net.h | 2 ++ > 3 files changed, 16 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 7b187ec..b601294 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -2724,9 +2724,14 @@ static int virtnet_probe(struct virtio_device *vdev) > /* Do we support "hardware" checksums? */ > if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) { > /* This opens up the world of extra features. */ > - dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG; > + netdev_features_t sctp = 0; > + > + if (virtio_has_feature(vdev, VIRTIO_NET_F_SCTP_CSUM)) > + sctp |= NETIF_F_SCTP_CRC; > + > + dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; > if (csum) > - dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG; > + dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; > > if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) { > dev->hw_features |= NETIF_F_TSO > @@ -2952,7 +2957,7 @@ static struct virtio_device_id id_table[] = { > }; > > #define VIRTNET_FEATURES \ > - VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, \ > + VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, VIRTIO_NET_F_SCTP_CSUM, \ > VIRTIO_NET_F_MAC, \ > VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6, \ > VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6, \ > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h > index f144216..2e7a64a 100644 > --- a/include/linux/virtio_net.h > +++ b/include/linux/virtio_net.h > @@ -39,6 +39,9 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, > > if (!skb_partial_csum_set(skb, start, off)) > return -EINVAL; > + > + if (hdr->flags & VIRTIO_NET_HDR_F_CSUM_NOT_INET) > + skb->csum_not_inet = 1; > } > > if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) { > @@ -96,6 +99,9 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb, > hdr->flags = VIRTIO_NET_HDR_F_DATA_VALID; > } /* else everything is zero */ > > + if (skb->csum_not_inet) > + hdr->flags &= VIRTIO_NET_HDR_F_CSUM_NOT_INET; Shouldn't this be |= instead? > + > return 0; > } > > diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h > index 5de6ed3..3f279c8 100644 > --- a/include/uapi/linux/virtio_net.h > +++ b/include/uapi/linux/virtio_net.h > @@ -36,6 +36,7 @@ > #define VIRTIO_NET_F_GUEST_CSUM 1 /* Guest handles pkts w/ partial csum */ > #define VIRTIO_NET_F_CTRL_GUEST_OFFLOADS 2 /* Dynamic offload configuration. */ > #define VIRTIO_NET_F_MTU 3 /* Initial MTU advice */ > +#define VIRTIO_NET_F_SCTP_CSUM 4 /* SCTP checksum offload support */ > #define VIRTIO_NET_F_MAC 5 /* Host has given MAC address. */ > #define VIRTIO_NET_F_GUEST_TSO4 7 /* Guest can handle TSOv4 in. */ > #define VIRTIO_NET_F_GUEST_TSO6 8 /* Guest can handle TSOv6 in. */ > @@ -101,6 +102,7 @@ struct virtio_net_config { > struct virtio_net_hdr_v1 { > #define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start, csum_offset */ > #define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */ > +#define VIRTIO_NET_HDR_F_CSUM_NOT_INET 4 /* Checksum is not inet */ > __u8 flags; > #define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame */ > #define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP (TSO) */ > -- > 2.9.5 > ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 1/5] virtio: Add support for SCTP checksum offloading @ 2018-04-11 22:39 ` Marcelo Ricardo Leitner 0 siblings, 0 replies; 74+ messages in thread From: Marcelo Ricardo Leitner @ 2018-04-11 22:39 UTC (permalink / raw) To: Vladislav Yasevich Cc: netdev, linux-sctp, virtualization, mst, jasowang, nhorman, Vladislav Yasevich On Mon, Apr 02, 2018 at 09:40:02AM -0400, Vladislav Yasevich wrote: > To support SCTP checksum offloading, we need to add a new feature > to virtio_net, so we can negotiate support between the hypervisor > and the guest. > > The signalling to the guest that an alternate checksum needs to > be used is done via a new flag in the virtio_net_hdr. If the > flag is set, the host will know to perform an alternate checksum > calculation, which right now is only CRC32c. > > Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> > --- > drivers/net/virtio_net.c | 11 ++++++++--- > include/linux/virtio_net.h | 6 ++++++ > include/uapi/linux/virtio_net.h | 2 ++ > 3 files changed, 16 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 7b187ec..b601294 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -2724,9 +2724,14 @@ static int virtnet_probe(struct virtio_device *vdev) > /* Do we support "hardware" checksums? */ > if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) { > /* This opens up the world of extra features. */ > - dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG; > + netdev_features_t sctp = 0; > + > + if (virtio_has_feature(vdev, VIRTIO_NET_F_SCTP_CSUM)) > + sctp |= NETIF_F_SCTP_CRC; > + > + dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; > if (csum) > - dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG; > + dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; > > if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) { > dev->hw_features |= NETIF_F_TSO > @@ -2952,7 +2957,7 @@ static struct virtio_device_id id_table[] = { > }; > > #define VIRTNET_FEATURES \ > - VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, \ > + VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, VIRTIO_NET_F_SCTP_CSUM, \ > VIRTIO_NET_F_MAC, \ > VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6, \ > VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6, \ > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h > index f144216..2e7a64a 100644 > --- a/include/linux/virtio_net.h > +++ b/include/linux/virtio_net.h > @@ -39,6 +39,9 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, > > if (!skb_partial_csum_set(skb, start, off)) > return -EINVAL; > + > + if (hdr->flags & VIRTIO_NET_HDR_F_CSUM_NOT_INET) > + skb->csum_not_inet = 1; > } > > if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) { > @@ -96,6 +99,9 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb, > hdr->flags = VIRTIO_NET_HDR_F_DATA_VALID; > } /* else everything is zero */ > > + if (skb->csum_not_inet) > + hdr->flags &= VIRTIO_NET_HDR_F_CSUM_NOT_INET; Shouldn't this be |= instead? > + > return 0; > } > > diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h > index 5de6ed3..3f279c8 100644 > --- a/include/uapi/linux/virtio_net.h > +++ b/include/uapi/linux/virtio_net.h > @@ -36,6 +36,7 @@ > #define VIRTIO_NET_F_GUEST_CSUM 1 /* Guest handles pkts w/ partial csum */ > #define VIRTIO_NET_F_CTRL_GUEST_OFFLOADS 2 /* Dynamic offload configuration. */ > #define VIRTIO_NET_F_MTU 3 /* Initial MTU advice */ > +#define VIRTIO_NET_F_SCTP_CSUM 4 /* SCTP checksum offload support */ > #define VIRTIO_NET_F_MAC 5 /* Host has given MAC address. */ > #define VIRTIO_NET_F_GUEST_TSO4 7 /* Guest can handle TSOv4 in. */ > #define VIRTIO_NET_F_GUEST_TSO6 8 /* Guest can handle TSOv6 in. */ > @@ -101,6 +102,7 @@ struct virtio_net_config { > struct virtio_net_hdr_v1 { > #define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start, csum_offset */ > #define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */ > +#define VIRTIO_NET_HDR_F_CSUM_NOT_INET 4 /* Checksum is not inet */ > __u8 flags; > #define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame */ > #define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP (TSO) */ > -- > 2.9.5 > ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 1/5] virtio: Add support for SCTP checksum offloading 2018-04-02 13:40 ` Vladislav Yasevich @ 2018-04-11 22:49 ` Michael S. Tsirkin -1 siblings, 0 replies; 74+ messages in thread From: Michael S. Tsirkin @ 2018-04-11 22:49 UTC (permalink / raw) To: Vladislav Yasevich Cc: netdev, linux-sctp, virtualization, jasowang, nhorman, Vladislav Yasevich On Mon, Apr 02, 2018 at 09:40:02AM -0400, Vladislav Yasevich wrote: > To support SCTP checksum offloading, we need to add a new feature > to virtio_net, so we can negotiate support between the hypervisor > and the guest. > > The signalling to the guest that an alternate checksum needs to > be used is done via a new flag in the virtio_net_hdr. If the > flag is set, the host will know to perform an alternate checksum > calculation, which right now is only CRC32c. > > Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> > --- > drivers/net/virtio_net.c | 11 ++++++++--- > include/linux/virtio_net.h | 6 ++++++ > include/uapi/linux/virtio_net.h | 2 ++ > 3 files changed, 16 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 7b187ec..b601294 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -2724,9 +2724,14 @@ static int virtnet_probe(struct virtio_device *vdev) > /* Do we support "hardware" checksums? */ > if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) { > /* This opens up the world of extra features. */ > - dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG; > + netdev_features_t sctp = 0; > + > + if (virtio_has_feature(vdev, VIRTIO_NET_F_SCTP_CSUM)) > + sctp |= NETIF_F_SCTP_CRC; > + > + dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; > if (csum) > - dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG; > + dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; > > if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) { > dev->hw_features |= NETIF_F_TSO > @@ -2952,7 +2957,7 @@ static struct virtio_device_id id_table[] = { > }; > > #define VIRTNET_FEATURES \ > - VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, \ > + VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, VIRTIO_NET_F_SCTP_CSUM, \ > VIRTIO_NET_F_MAC, \ > VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6, \ > VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6, \ > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h > index f144216..2e7a64a 100644 > --- a/include/linux/virtio_net.h > +++ b/include/linux/virtio_net.h > @@ -39,6 +39,9 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, > > if (!skb_partial_csum_set(skb, start, off)) > return -EINVAL; > + > + if (hdr->flags & VIRTIO_NET_HDR_F_CSUM_NOT_INET) > + skb->csum_not_inet = 1; > } > > if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) { > @@ -96,6 +99,9 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb, > hdr->flags = VIRTIO_NET_HDR_F_DATA_VALID; > } /* else everything is zero */ > > + if (skb->csum_not_inet) > + hdr->flags &= VIRTIO_NET_HDR_F_CSUM_NOT_INET; > + > return 0; > } > > diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h > index 5de6ed3..3f279c8 100644 > --- a/include/uapi/linux/virtio_net.h > +++ b/include/uapi/linux/virtio_net.h > @@ -36,6 +36,7 @@ > #define VIRTIO_NET_F_GUEST_CSUM 1 /* Guest handles pkts w/ partial csum */ > #define VIRTIO_NET_F_CTRL_GUEST_OFFLOADS 2 /* Dynamic offload configuration. */ > #define VIRTIO_NET_F_MTU 3 /* Initial MTU advice */ > +#define VIRTIO_NET_F_SCTP_CSUM 4 /* SCTP checksum offload support */ > #define VIRTIO_NET_F_MAC 5 /* Host has given MAC address. */ > #define VIRTIO_NET_F_GUEST_TSO4 7 /* Guest can handle TSOv4 in. */ > #define VIRTIO_NET_F_GUEST_TSO6 8 /* Guest can handle TSOv6 in. */ Is this a guest or a host checksum? We should differenciate between the two. > @@ -101,6 +102,7 @@ struct virtio_net_config { > struct virtio_net_hdr_v1 { > #define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start, csum_offset */ > #define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */ > +#define VIRTIO_NET_HDR_F_CSUM_NOT_INET 4 /* Checksum is not inet */ > __u8 flags; > #define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame */ > #define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP (TSO) */ > -- > 2.9.5 ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 1/5] virtio: Add support for SCTP checksum offloading @ 2018-04-11 22:49 ` Michael S. Tsirkin 0 siblings, 0 replies; 74+ messages in thread From: Michael S. Tsirkin @ 2018-04-11 22:49 UTC (permalink / raw) To: Vladislav Yasevich Cc: netdev, linux-sctp, virtualization, jasowang, nhorman, Vladislav Yasevich On Mon, Apr 02, 2018 at 09:40:02AM -0400, Vladislav Yasevich wrote: > To support SCTP checksum offloading, we need to add a new feature > to virtio_net, so we can negotiate support between the hypervisor > and the guest. > > The signalling to the guest that an alternate checksum needs to > be used is done via a new flag in the virtio_net_hdr. If the > flag is set, the host will know to perform an alternate checksum > calculation, which right now is only CRC32c. > > Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> > --- > drivers/net/virtio_net.c | 11 ++++++++--- > include/linux/virtio_net.h | 6 ++++++ > include/uapi/linux/virtio_net.h | 2 ++ > 3 files changed, 16 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 7b187ec..b601294 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -2724,9 +2724,14 @@ static int virtnet_probe(struct virtio_device *vdev) > /* Do we support "hardware" checksums? */ > if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) { > /* This opens up the world of extra features. */ > - dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG; > + netdev_features_t sctp = 0; > + > + if (virtio_has_feature(vdev, VIRTIO_NET_F_SCTP_CSUM)) > + sctp |= NETIF_F_SCTP_CRC; > + > + dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; > if (csum) > - dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG; > + dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; > > if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) { > dev->hw_features |= NETIF_F_TSO > @@ -2952,7 +2957,7 @@ static struct virtio_device_id id_table[] = { > }; > > #define VIRTNET_FEATURES \ > - VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, \ > + VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, VIRTIO_NET_F_SCTP_CSUM, \ > VIRTIO_NET_F_MAC, \ > VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6, \ > VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6, \ > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h > index f144216..2e7a64a 100644 > --- a/include/linux/virtio_net.h > +++ b/include/linux/virtio_net.h > @@ -39,6 +39,9 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, > > if (!skb_partial_csum_set(skb, start, off)) > return -EINVAL; > + > + if (hdr->flags & VIRTIO_NET_HDR_F_CSUM_NOT_INET) > + skb->csum_not_inet = 1; > } > > if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) { > @@ -96,6 +99,9 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb, > hdr->flags = VIRTIO_NET_HDR_F_DATA_VALID; > } /* else everything is zero */ > > + if (skb->csum_not_inet) > + hdr->flags &= VIRTIO_NET_HDR_F_CSUM_NOT_INET; > + > return 0; > } > > diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h > index 5de6ed3..3f279c8 100644 > --- a/include/uapi/linux/virtio_net.h > +++ b/include/uapi/linux/virtio_net.h > @@ -36,6 +36,7 @@ > #define VIRTIO_NET_F_GUEST_CSUM 1 /* Guest handles pkts w/ partial csum */ > #define VIRTIO_NET_F_CTRL_GUEST_OFFLOADS 2 /* Dynamic offload configuration. */ > #define VIRTIO_NET_F_MTU 3 /* Initial MTU advice */ > +#define VIRTIO_NET_F_SCTP_CSUM 4 /* SCTP checksum offload support */ > #define VIRTIO_NET_F_MAC 5 /* Host has given MAC address. */ > #define VIRTIO_NET_F_GUEST_TSO4 7 /* Guest can handle TSOv4 in. */ > #define VIRTIO_NET_F_GUEST_TSO6 8 /* Guest can handle TSOv6 in. */ Is this a guest or a host checksum? We should differenciate between the two. > @@ -101,6 +102,7 @@ struct virtio_net_config { > struct virtio_net_hdr_v1 { > #define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start, csum_offset */ > #define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */ > +#define VIRTIO_NET_HDR_F_CSUM_NOT_INET 4 /* Checksum is not inet */ > __u8 flags; > #define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame */ > #define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP (TSO) */ > -- > 2.9.5 ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 1/5] virtio: Add support for SCTP checksum offloading 2018-04-11 22:49 ` Michael S. Tsirkin @ 2018-04-16 13:45 ` Vlad Yasevich -1 siblings, 0 replies; 74+ messages in thread From: Vlad Yasevich @ 2018-04-16 13:45 UTC (permalink / raw) To: Michael S. Tsirkin, Vladislav Yasevich Cc: netdev, linux-sctp, virtualization, jasowang, nhorman On 04/11/2018 06:49 PM, Michael S. Tsirkin wrote: > On Mon, Apr 02, 2018 at 09:40:02AM -0400, Vladislav Yasevich wrote: >> To support SCTP checksum offloading, we need to add a new feature >> to virtio_net, so we can negotiate support between the hypervisor >> and the guest. >> >> The signalling to the guest that an alternate checksum needs to >> be used is done via a new flag in the virtio_net_hdr. If the >> flag is set, the host will know to perform an alternate checksum >> calculation, which right now is only CRC32c. >> >> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> >> --- >> drivers/net/virtio_net.c | 11 ++++++++--- >> include/linux/virtio_net.h | 6 ++++++ >> include/uapi/linux/virtio_net.h | 2 ++ >> 3 files changed, 16 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c >> index 7b187ec..b601294 100644 >> --- a/drivers/net/virtio_net.c >> +++ b/drivers/net/virtio_net.c >> @@ -2724,9 +2724,14 @@ static int virtnet_probe(struct virtio_device *vdev) >> /* Do we support "hardware" checksums? */ >> if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) { >> /* This opens up the world of extra features. */ >> - dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG; >> + netdev_features_t sctp = 0; >> + >> + if (virtio_has_feature(vdev, VIRTIO_NET_F_SCTP_CSUM)) >> + sctp |= NETIF_F_SCTP_CRC; >> + >> + dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; >> if (csum) >> - dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG; >> + dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; >> >> if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) { >> dev->hw_features |= NETIF_F_TSO >> @@ -2952,7 +2957,7 @@ static struct virtio_device_id id_table[] = { >> }; >> >> #define VIRTNET_FEATURES \ >> - VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, \ >> + VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, VIRTIO_NET_F_SCTP_CSUM, \ >> VIRTIO_NET_F_MAC, \ >> VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6, \ >> VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6, \ >> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h >> index f144216..2e7a64a 100644 >> --- a/include/linux/virtio_net.h >> +++ b/include/linux/virtio_net.h >> @@ -39,6 +39,9 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, >> >> if (!skb_partial_csum_set(skb, start, off)) >> return -EINVAL; >> + >> + if (hdr->flags & VIRTIO_NET_HDR_F_CSUM_NOT_INET) >> + skb->csum_not_inet = 1; >> } >> >> if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) { >> @@ -96,6 +99,9 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb, >> hdr->flags = VIRTIO_NET_HDR_F_DATA_VALID; >> } /* else everything is zero */ >> >> + if (skb->csum_not_inet) >> + hdr->flags &= VIRTIO_NET_HDR_F_CSUM_NOT_INET; >> + >> return 0; >> } >> >> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h >> index 5de6ed3..3f279c8 100644 >> --- a/include/uapi/linux/virtio_net.h >> +++ b/include/uapi/linux/virtio_net.h >> @@ -36,6 +36,7 @@ >> #define VIRTIO_NET_F_GUEST_CSUM 1 /* Guest handles pkts w/ partial csum */ >> #define VIRTIO_NET_F_CTRL_GUEST_OFFLOADS 2 /* Dynamic offload configuration. */ >> #define VIRTIO_NET_F_MTU 3 /* Initial MTU advice */ >> +#define VIRTIO_NET_F_SCTP_CSUM 4 /* SCTP checksum offload support */ >> #define VIRTIO_NET_F_MAC 5 /* Host has given MAC address. */ >> #define VIRTIO_NET_F_GUEST_TSO4 7 /* Guest can handle TSOv4 in. */ >> #define VIRTIO_NET_F_GUEST_TSO6 8 /* Guest can handle TSOv6 in. */ > > Is this a guest or a host checksum? We should differenciate between the > two. I suppose this is HOST checksum, since it behaves like VIRTIO_NET_F_CSUM only for SCTP. I couldn't find the use for the GUEST side flag, since there technically isn't any validations and there is no additional mappings from VIRTIO flag to a NETIF flag. If the feature is negotiated, the guest ends up generating partially check-summed packets, and the host turns on appropriate flags on it's side. The host will also make sure the checksum if fixed up if the guest doesn't support it. So 1 flag is currently all that is needed. -vlad > > >> @@ -101,6 +102,7 @@ struct virtio_net_config { >> struct virtio_net_hdr_v1 { >> #define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start, csum_offset */ >> #define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */ >> +#define VIRTIO_NET_HDR_F_CSUM_NOT_INET 4 /* Checksum is not inet */ >> __u8 flags; >> #define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame */ >> #define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP (TSO) */ >> -- >> 2.9.5 ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 1/5] virtio: Add support for SCTP checksum offloading @ 2018-04-16 13:45 ` Vlad Yasevich 0 siblings, 0 replies; 74+ messages in thread From: Vlad Yasevich @ 2018-04-16 13:45 UTC (permalink / raw) To: Michael S. Tsirkin, Vladislav Yasevich Cc: netdev, linux-sctp, virtualization, jasowang, nhorman On 04/11/2018 06:49 PM, Michael S. Tsirkin wrote: > On Mon, Apr 02, 2018 at 09:40:02AM -0400, Vladislav Yasevich wrote: >> To support SCTP checksum offloading, we need to add a new feature >> to virtio_net, so we can negotiate support between the hypervisor >> and the guest. >> >> The signalling to the guest that an alternate checksum needs to >> be used is done via a new flag in the virtio_net_hdr. If the >> flag is set, the host will know to perform an alternate checksum >> calculation, which right now is only CRC32c. >> >> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> >> --- >> drivers/net/virtio_net.c | 11 ++++++++--- >> include/linux/virtio_net.h | 6 ++++++ >> include/uapi/linux/virtio_net.h | 2 ++ >> 3 files changed, 16 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c >> index 7b187ec..b601294 100644 >> --- a/drivers/net/virtio_net.c >> +++ b/drivers/net/virtio_net.c >> @@ -2724,9 +2724,14 @@ static int virtnet_probe(struct virtio_device *vdev) >> /* Do we support "hardware" checksums? */ >> if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) { >> /* This opens up the world of extra features. */ >> - dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG; >> + netdev_features_t sctp = 0; >> + >> + if (virtio_has_feature(vdev, VIRTIO_NET_F_SCTP_CSUM)) >> + sctp |= NETIF_F_SCTP_CRC; >> + >> + dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; >> if (csum) >> - dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG; >> + dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; >> >> if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) { >> dev->hw_features |= NETIF_F_TSO >> @@ -2952,7 +2957,7 @@ static struct virtio_device_id id_table[] = { >> }; >> >> #define VIRTNET_FEATURES \ >> - VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, \ >> + VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, VIRTIO_NET_F_SCTP_CSUM, \ >> VIRTIO_NET_F_MAC, \ >> VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6, \ >> VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6, \ >> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h >> index f144216..2e7a64a 100644 >> --- a/include/linux/virtio_net.h >> +++ b/include/linux/virtio_net.h >> @@ -39,6 +39,9 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, >> >> if (!skb_partial_csum_set(skb, start, off)) >> return -EINVAL; >> + >> + if (hdr->flags & VIRTIO_NET_HDR_F_CSUM_NOT_INET) >> + skb->csum_not_inet = 1; >> } >> >> if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) { >> @@ -96,6 +99,9 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb, >> hdr->flags = VIRTIO_NET_HDR_F_DATA_VALID; >> } /* else everything is zero */ >> >> + if (skb->csum_not_inet) >> + hdr->flags &= VIRTIO_NET_HDR_F_CSUM_NOT_INET; >> + >> return 0; >> } >> >> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h >> index 5de6ed3..3f279c8 100644 >> --- a/include/uapi/linux/virtio_net.h >> +++ b/include/uapi/linux/virtio_net.h >> @@ -36,6 +36,7 @@ >> #define VIRTIO_NET_F_GUEST_CSUM 1 /* Guest handles pkts w/ partial csum */ >> #define VIRTIO_NET_F_CTRL_GUEST_OFFLOADS 2 /* Dynamic offload configuration. */ >> #define VIRTIO_NET_F_MTU 3 /* Initial MTU advice */ >> +#define VIRTIO_NET_F_SCTP_CSUM 4 /* SCTP checksum offload support */ >> #define VIRTIO_NET_F_MAC 5 /* Host has given MAC address. */ >> #define VIRTIO_NET_F_GUEST_TSO4 7 /* Guest can handle TSOv4 in. */ >> #define VIRTIO_NET_F_GUEST_TSO6 8 /* Guest can handle TSOv6 in. */ > > Is this a guest or a host checksum? We should differenciate between the > two. I suppose this is HOST checksum, since it behaves like VIRTIO_NET_F_CSUM only for SCTP. I couldn't find the use for the GUEST side flag, since there technically isn't any validations and there is no additional mappings from VIRTIO flag to a NETIF flag. If the feature is negotiated, the guest ends up generating partially check-summed packets, and the host turns on appropriate flags on it's side. The host will also make sure the checksum if fixed up if the guest doesn't support it. So 1 flag is currently all that is needed. -vlad > > >> @@ -101,6 +102,7 @@ struct virtio_net_config { >> struct virtio_net_hdr_v1 { >> #define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start, csum_offset */ >> #define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */ >> +#define VIRTIO_NET_HDR_F_CSUM_NOT_INET 4 /* Checksum is not inet */ >> __u8 flags; >> #define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame */ >> #define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP (TSO) */ >> -- >> 2.9.5 ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 1/5] virtio: Add support for SCTP checksum offloading 2018-04-16 13:45 ` Vlad Yasevich @ 2018-04-16 15:52 ` Michael S. Tsirkin -1 siblings, 0 replies; 74+ messages in thread From: Michael S. Tsirkin @ 2018-04-16 15:52 UTC (permalink / raw) To: Vlad Yasevich Cc: nhorman, netdev, virtualization, linux-sctp, Vladislav Yasevich On Mon, Apr 16, 2018 at 09:45:48AM -0400, Vlad Yasevich wrote: > On 04/11/2018 06:49 PM, Michael S. Tsirkin wrote: > > On Mon, Apr 02, 2018 at 09:40:02AM -0400, Vladislav Yasevich wrote: > >> To support SCTP checksum offloading, we need to add a new feature > >> to virtio_net, so we can negotiate support between the hypervisor > >> and the guest. > >> > >> The signalling to the guest that an alternate checksum needs to > >> be used is done via a new flag in the virtio_net_hdr. If the > >> flag is set, the host will know to perform an alternate checksum > >> calculation, which right now is only CRC32c. > >> > >> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> > >> --- > >> drivers/net/virtio_net.c | 11 ++++++++--- > >> include/linux/virtio_net.h | 6 ++++++ > >> include/uapi/linux/virtio_net.h | 2 ++ Could you pls include a virtio TC mailing list on uapi changes? > >> 3 files changed, 16 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > >> index 7b187ec..b601294 100644 > >> --- a/drivers/net/virtio_net.c > >> +++ b/drivers/net/virtio_net.c > >> @@ -2724,9 +2724,14 @@ static int virtnet_probe(struct virtio_device *vdev) > >> /* Do we support "hardware" checksums? */ > >> if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) { > >> /* This opens up the world of extra features. */ > >> - dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG; > >> + netdev_features_t sctp = 0; > >> + > >> + if (virtio_has_feature(vdev, VIRTIO_NET_F_SCTP_CSUM)) > >> + sctp |= NETIF_F_SCTP_CRC; > >> + > >> + dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; > >> if (csum) > >> - dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG; > >> + dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; > >> > >> if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) { > >> dev->hw_features |= NETIF_F_TSO > >> @@ -2952,7 +2957,7 @@ static struct virtio_device_id id_table[] = { > >> }; > >> > >> #define VIRTNET_FEATURES \ > >> - VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, \ > >> + VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, VIRTIO_NET_F_SCTP_CSUM, \ > >> VIRTIO_NET_F_MAC, \ > >> VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6, \ > >> VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6, \ > >> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h > >> index f144216..2e7a64a 100644 > >> --- a/include/linux/virtio_net.h > >> +++ b/include/linux/virtio_net.h > >> @@ -39,6 +39,9 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, > >> > >> if (!skb_partial_csum_set(skb, start, off)) > >> return -EINVAL; > >> + > >> + if (hdr->flags & VIRTIO_NET_HDR_F_CSUM_NOT_INET) > >> + skb->csum_not_inet = 1; > >> } > >> > >> if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) { > >> @@ -96,6 +99,9 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb, > >> hdr->flags = VIRTIO_NET_HDR_F_DATA_VALID; > >> } /* else everything is zero */ > >> > >> + if (skb->csum_not_inet) > >> + hdr->flags &= VIRTIO_NET_HDR_F_CSUM_NOT_INET; > >> + > >> return 0; > >> } > >> > >> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h > >> index 5de6ed3..3f279c8 100644 > >> --- a/include/uapi/linux/virtio_net.h > >> +++ b/include/uapi/linux/virtio_net.h > >> @@ -36,6 +36,7 @@ > >> #define VIRTIO_NET_F_GUEST_CSUM 1 /* Guest handles pkts w/ partial csum */ > >> #define VIRTIO_NET_F_CTRL_GUEST_OFFLOADS 2 /* Dynamic offload configuration. */ > >> #define VIRTIO_NET_F_MTU 3 /* Initial MTU advice */ > >> +#define VIRTIO_NET_F_SCTP_CSUM 4 /* SCTP checksum offload support */ > >> #define VIRTIO_NET_F_MAC 5 /* Host has given MAC address. */ > >> #define VIRTIO_NET_F_GUEST_TSO4 7 /* Guest can handle TSOv4 in. */ > >> #define VIRTIO_NET_F_GUEST_TSO6 8 /* Guest can handle TSOv6 in. */ > > > > Is this a guest or a host checksum? We should differenciate between the > > two. > > I suppose this is HOST checksum, since it behaves like VIRTIO_NET_F_CSUM only for > SCTP. I couldn't find the use for the GUEST side flag, since there technically > isn't any validations and there is no additional mappings from VIRTIO flag to a > NETIF flag. > > If the feature is negotiated, the guest ends up generating partially check-summed > packets, and the host turns on appropriate flags on it's side. The host will > also make sure the checksum if fixed up if the guest doesn't support it. > So 1 flag is currently all that is needed. > > -vlad Fine, but let's include HOST in there to make things clear. Also I think we should use a high flag, like 62. > > > > > >> @@ -101,6 +102,7 @@ struct virtio_net_config { > >> struct virtio_net_hdr_v1 { > >> #define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start, csum_offset */ > >> #define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */ > >> +#define VIRTIO_NET_HDR_F_CSUM_NOT_INET 4 /* Checksum is not inet */ > >> __u8 flags; > >> #define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame */ > >> #define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP (TSO) */ > >> -- > >> 2.9.5 ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 1/5] virtio: Add support for SCTP checksum offloading @ 2018-04-16 15:52 ` Michael S. Tsirkin 0 siblings, 0 replies; 74+ messages in thread From: Michael S. Tsirkin @ 2018-04-16 15:52 UTC (permalink / raw) To: Vlad Yasevich Cc: nhorman, netdev, virtualization, linux-sctp, Vladislav Yasevich On Mon, Apr 16, 2018 at 09:45:48AM -0400, Vlad Yasevich wrote: > On 04/11/2018 06:49 PM, Michael S. Tsirkin wrote: > > On Mon, Apr 02, 2018 at 09:40:02AM -0400, Vladislav Yasevich wrote: > >> To support SCTP checksum offloading, we need to add a new feature > >> to virtio_net, so we can negotiate support between the hypervisor > >> and the guest. > >> > >> The signalling to the guest that an alternate checksum needs to > >> be used is done via a new flag in the virtio_net_hdr. If the > >> flag is set, the host will know to perform an alternate checksum > >> calculation, which right now is only CRC32c. > >> > >> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> > >> --- > >> drivers/net/virtio_net.c | 11 ++++++++--- > >> include/linux/virtio_net.h | 6 ++++++ > >> include/uapi/linux/virtio_net.h | 2 ++ Could you pls include a virtio TC mailing list on uapi changes? > >> 3 files changed, 16 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > >> index 7b187ec..b601294 100644 > >> --- a/drivers/net/virtio_net.c > >> +++ b/drivers/net/virtio_net.c > >> @@ -2724,9 +2724,14 @@ static int virtnet_probe(struct virtio_device *vdev) > >> /* Do we support "hardware" checksums? */ > >> if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) { > >> /* This opens up the world of extra features. */ > >> - dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG; > >> + netdev_features_t sctp = 0; > >> + > >> + if (virtio_has_feature(vdev, VIRTIO_NET_F_SCTP_CSUM)) > >> + sctp |= NETIF_F_SCTP_CRC; > >> + > >> + dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; > >> if (csum) > >> - dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG; > >> + dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; > >> > >> if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) { > >> dev->hw_features |= NETIF_F_TSO > >> @@ -2952,7 +2957,7 @@ static struct virtio_device_id id_table[] = { > >> }; > >> > >> #define VIRTNET_FEATURES \ > >> - VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, \ > >> + VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, VIRTIO_NET_F_SCTP_CSUM, \ > >> VIRTIO_NET_F_MAC, \ > >> VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6, \ > >> VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6, \ > >> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h > >> index f144216..2e7a64a 100644 > >> --- a/include/linux/virtio_net.h > >> +++ b/include/linux/virtio_net.h > >> @@ -39,6 +39,9 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, > >> > >> if (!skb_partial_csum_set(skb, start, off)) > >> return -EINVAL; > >> + > >> + if (hdr->flags & VIRTIO_NET_HDR_F_CSUM_NOT_INET) > >> + skb->csum_not_inet = 1; > >> } > >> > >> if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) { > >> @@ -96,6 +99,9 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb, > >> hdr->flags = VIRTIO_NET_HDR_F_DATA_VALID; > >> } /* else everything is zero */ > >> > >> + if (skb->csum_not_inet) > >> + hdr->flags &= VIRTIO_NET_HDR_F_CSUM_NOT_INET; > >> + > >> return 0; > >> } > >> > >> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h > >> index 5de6ed3..3f279c8 100644 > >> --- a/include/uapi/linux/virtio_net.h > >> +++ b/include/uapi/linux/virtio_net.h > >> @@ -36,6 +36,7 @@ > >> #define VIRTIO_NET_F_GUEST_CSUM 1 /* Guest handles pkts w/ partial csum */ > >> #define VIRTIO_NET_F_CTRL_GUEST_OFFLOADS 2 /* Dynamic offload configuration. */ > >> #define VIRTIO_NET_F_MTU 3 /* Initial MTU advice */ > >> +#define VIRTIO_NET_F_SCTP_CSUM 4 /* SCTP checksum offload support */ > >> #define VIRTIO_NET_F_MAC 5 /* Host has given MAC address. */ > >> #define VIRTIO_NET_F_GUEST_TSO4 7 /* Guest can handle TSOv4 in. */ > >> #define VIRTIO_NET_F_GUEST_TSO6 8 /* Guest can handle TSOv6 in. */ > > > > Is this a guest or a host checksum? We should differenciate between the > > two. > > I suppose this is HOST checksum, since it behaves like VIRTIO_NET_F_CSUM only for > SCTP. I couldn't find the use for the GUEST side flag, since there technically > isn't any validations and there is no additional mappings from VIRTIO flag to a > NETIF flag. > > If the feature is negotiated, the guest ends up generating partially check-summed > packets, and the host turns on appropriate flags on it's side. The host will > also make sure the checksum if fixed up if the guest doesn't support it. > So 1 flag is currently all that is needed. > > -vlad Fine, but let's include HOST in there to make things clear. Also I think we should use a high flag, like 62. > > > > > >> @@ -101,6 +102,7 @@ struct virtio_net_config { > >> struct virtio_net_hdr_v1 { > >> #define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start, csum_offset */ > >> #define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */ > >> +#define VIRTIO_NET_HDR_F_CSUM_NOT_INET 4 /* Checksum is not inet */ > >> __u8 flags; > >> #define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame */ > >> #define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP (TSO) */ > >> -- > >> 2.9.5 ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 1/5] virtio: Add support for SCTP checksum offloading 2018-04-16 13:45 ` Vlad Yasevich @ 2018-04-16 17:09 ` Michael S. Tsirkin -1 siblings, 0 replies; 74+ messages in thread From: Michael S. Tsirkin @ 2018-04-16 17:09 UTC (permalink / raw) To: Vlad Yasevich Cc: Vladislav Yasevich, netdev, linux-sctp, virtualization, jasowang, nhorman On Mon, Apr 16, 2018 at 09:45:48AM -0400, Vlad Yasevich wrote: > On 04/11/2018 06:49 PM, Michael S. Tsirkin wrote: > > On Mon, Apr 02, 2018 at 09:40:02AM -0400, Vladislav Yasevich wrote: > >> To support SCTP checksum offloading, we need to add a new feature > >> to virtio_net, so we can negotiate support between the hypervisor > >> and the guest. > >> > >> The signalling to the guest that an alternate checksum needs to > >> be used is done via a new flag in the virtio_net_hdr. If the > >> flag is set, the host will know to perform an alternate checksum > >> calculation, which right now is only CRC32c. > >> > >> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> > >> --- > >> drivers/net/virtio_net.c | 11 ++++++++--- > >> include/linux/virtio_net.h | 6 ++++++ > >> include/uapi/linux/virtio_net.h | 2 ++ > >> 3 files changed, 16 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > >> index 7b187ec..b601294 100644 > >> --- a/drivers/net/virtio_net.c > >> +++ b/drivers/net/virtio_net.c > >> @@ -2724,9 +2724,14 @@ static int virtnet_probe(struct virtio_device *vdev) > >> /* Do we support "hardware" checksums? */ > >> if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) { > >> /* This opens up the world of extra features. */ > >> - dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG; > >> + netdev_features_t sctp = 0; > >> + > >> + if (virtio_has_feature(vdev, VIRTIO_NET_F_SCTP_CSUM)) > >> + sctp |= NETIF_F_SCTP_CRC; > >> + > >> + dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; > >> if (csum) > >> - dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG; > >> + dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; > >> > >> if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) { > >> dev->hw_features |= NETIF_F_TSO > >> @@ -2952,7 +2957,7 @@ static struct virtio_device_id id_table[] = { > >> }; > >> > >> #define VIRTNET_FEATURES \ > >> - VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, \ > >> + VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, VIRTIO_NET_F_SCTP_CSUM, \ > >> VIRTIO_NET_F_MAC, \ > >> VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6, \ > >> VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6, \ > >> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h > >> index f144216..2e7a64a 100644 > >> --- a/include/linux/virtio_net.h > >> +++ b/include/linux/virtio_net.h > >> @@ -39,6 +39,9 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, > >> > >> if (!skb_partial_csum_set(skb, start, off)) > >> return -EINVAL; > >> + > >> + if (hdr->flags & VIRTIO_NET_HDR_F_CSUM_NOT_INET) > >> + skb->csum_not_inet = 1; > >> } > >> > >> if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) { > >> @@ -96,6 +99,9 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb, > >> hdr->flags = VIRTIO_NET_HDR_F_DATA_VALID; > >> } /* else everything is zero */ > >> > >> + if (skb->csum_not_inet) > >> + hdr->flags &= VIRTIO_NET_HDR_F_CSUM_NOT_INET; > >> + > >> return 0; > >> } > >> > >> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h > >> index 5de6ed3..3f279c8 100644 > >> --- a/include/uapi/linux/virtio_net.h > >> +++ b/include/uapi/linux/virtio_net.h > >> @@ -36,6 +36,7 @@ > >> #define VIRTIO_NET_F_GUEST_CSUM 1 /* Guest handles pkts w/ partial csum */ > >> #define VIRTIO_NET_F_CTRL_GUEST_OFFLOADS 2 /* Dynamic offload configuration. */ > >> #define VIRTIO_NET_F_MTU 3 /* Initial MTU advice */ > >> +#define VIRTIO_NET_F_SCTP_CSUM 4 /* SCTP checksum offload support */ > >> #define VIRTIO_NET_F_MAC 5 /* Host has given MAC address. */ > >> #define VIRTIO_NET_F_GUEST_TSO4 7 /* Guest can handle TSOv4 in. */ > >> #define VIRTIO_NET_F_GUEST_TSO6 8 /* Guest can handle TSOv6 in. */ > > > > Is this a guest or a host checksum? We should differenciate between the > > two. > > I suppose this is HOST checksum, since it behaves like VIRTIO_NET_F_CSUM only for > SCTP. I couldn't find the use for the GUEST side flag, since there technically > isn't any validations and there is no additional mappings from VIRTIO flag to a > NETIF flag. > > If the feature is negotiated, the guest ends up generating partially check-summed > packets, and the host turns on appropriate flags on it's side. The host will > also make sure the checksum if fixed up if the guest doesn't support it. > So 1 flag is currently all that is needed. > > -vlad I see code handling VIRTIO_NET_HDR_F_CSUM_NOT_INET on RX side. Host needs to know whether it's ok/worth it to set this flag, too. > > > > > >> @@ -101,6 +102,7 @@ struct virtio_net_config { > >> struct virtio_net_hdr_v1 { > >> #define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start, csum_offset */ > >> #define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */ > >> +#define VIRTIO_NET_HDR_F_CSUM_NOT_INET 4 /* Checksum is not inet */ > >> __u8 flags; > >> #define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame */ > >> #define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP (TSO) */ > >> -- > >> 2.9.5 ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 1/5] virtio: Add support for SCTP checksum offloading @ 2018-04-16 17:09 ` Michael S. Tsirkin 0 siblings, 0 replies; 74+ messages in thread From: Michael S. Tsirkin @ 2018-04-16 17:09 UTC (permalink / raw) To: Vlad Yasevich Cc: Vladislav Yasevich, netdev, linux-sctp, virtualization, jasowang, nhorman On Mon, Apr 16, 2018 at 09:45:48AM -0400, Vlad Yasevich wrote: > On 04/11/2018 06:49 PM, Michael S. Tsirkin wrote: > > On Mon, Apr 02, 2018 at 09:40:02AM -0400, Vladislav Yasevich wrote: > >> To support SCTP checksum offloading, we need to add a new feature > >> to virtio_net, so we can negotiate support between the hypervisor > >> and the guest. > >> > >> The signalling to the guest that an alternate checksum needs to > >> be used is done via a new flag in the virtio_net_hdr. If the > >> flag is set, the host will know to perform an alternate checksum > >> calculation, which right now is only CRC32c. > >> > >> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> > >> --- > >> drivers/net/virtio_net.c | 11 ++++++++--- > >> include/linux/virtio_net.h | 6 ++++++ > >> include/uapi/linux/virtio_net.h | 2 ++ > >> 3 files changed, 16 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > >> index 7b187ec..b601294 100644 > >> --- a/drivers/net/virtio_net.c > >> +++ b/drivers/net/virtio_net.c > >> @@ -2724,9 +2724,14 @@ static int virtnet_probe(struct virtio_device *vdev) > >> /* Do we support "hardware" checksums? */ > >> if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) { > >> /* This opens up the world of extra features. */ > >> - dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG; > >> + netdev_features_t sctp = 0; > >> + > >> + if (virtio_has_feature(vdev, VIRTIO_NET_F_SCTP_CSUM)) > >> + sctp |= NETIF_F_SCTP_CRC; > >> + > >> + dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; > >> if (csum) > >> - dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG; > >> + dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; > >> > >> if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) { > >> dev->hw_features |= NETIF_F_TSO > >> @@ -2952,7 +2957,7 @@ static struct virtio_device_id id_table[] = { > >> }; > >> > >> #define VIRTNET_FEATURES \ > >> - VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, \ > >> + VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, VIRTIO_NET_F_SCTP_CSUM, \ > >> VIRTIO_NET_F_MAC, \ > >> VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6, \ > >> VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6, \ > >> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h > >> index f144216..2e7a64a 100644 > >> --- a/include/linux/virtio_net.h > >> +++ b/include/linux/virtio_net.h > >> @@ -39,6 +39,9 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, > >> > >> if (!skb_partial_csum_set(skb, start, off)) > >> return -EINVAL; > >> + > >> + if (hdr->flags & VIRTIO_NET_HDR_F_CSUM_NOT_INET) > >> + skb->csum_not_inet = 1; > >> } > >> > >> if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) { > >> @@ -96,6 +99,9 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb, > >> hdr->flags = VIRTIO_NET_HDR_F_DATA_VALID; > >> } /* else everything is zero */ > >> > >> + if (skb->csum_not_inet) > >> + hdr->flags &= VIRTIO_NET_HDR_F_CSUM_NOT_INET; > >> + > >> return 0; > >> } > >> > >> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h > >> index 5de6ed3..3f279c8 100644 > >> --- a/include/uapi/linux/virtio_net.h > >> +++ b/include/uapi/linux/virtio_net.h > >> @@ -36,6 +36,7 @@ > >> #define VIRTIO_NET_F_GUEST_CSUM 1 /* Guest handles pkts w/ partial csum */ > >> #define VIRTIO_NET_F_CTRL_GUEST_OFFLOADS 2 /* Dynamic offload configuration. */ > >> #define VIRTIO_NET_F_MTU 3 /* Initial MTU advice */ > >> +#define VIRTIO_NET_F_SCTP_CSUM 4 /* SCTP checksum offload support */ > >> #define VIRTIO_NET_F_MAC 5 /* Host has given MAC address. */ > >> #define VIRTIO_NET_F_GUEST_TSO4 7 /* Guest can handle TSOv4 in. */ > >> #define VIRTIO_NET_F_GUEST_TSO6 8 /* Guest can handle TSOv6 in. */ > > > > Is this a guest or a host checksum? We should differenciate between the > > two. > > I suppose this is HOST checksum, since it behaves like VIRTIO_NET_F_CSUM only for > SCTP. I couldn't find the use for the GUEST side flag, since there technically > isn't any validations and there is no additional mappings from VIRTIO flag to a > NETIF flag. > > If the feature is negotiated, the guest ends up generating partially check-summed > packets, and the host turns on appropriate flags on it's side. The host will > also make sure the checksum if fixed up if the guest doesn't support it. > So 1 flag is currently all that is needed. > > -vlad I see code handling VIRTIO_NET_HDR_F_CSUM_NOT_INET on RX side. Host needs to know whether it's ok/worth it to set this flag, too. > > > > > >> @@ -101,6 +102,7 @@ struct virtio_net_config { > >> struct virtio_net_hdr_v1 { > >> #define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start, csum_offset */ > >> #define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */ > >> +#define VIRTIO_NET_HDR_F_CSUM_NOT_INET 4 /* Checksum is not inet */ > >> __u8 flags; > >> #define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame */ > >> #define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP (TSO) */ > >> -- > >> 2.9.5 ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 1/5] virtio: Add support for SCTP checksum offloading 2018-04-16 17:09 ` Michael S. Tsirkin @ 2018-04-17 19:06 ` Vlad Yasevich -1 siblings, 0 replies; 74+ messages in thread From: Vlad Yasevich @ 2018-04-17 19:06 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Vladislav Yasevich, netdev, linux-sctp, virtualization, jasowang, nhorman On 04/16/2018 01:09 PM, Michael S. Tsirkin wrote: > On Mon, Apr 16, 2018 at 09:45:48AM -0400, Vlad Yasevich wrote: >> On 04/11/2018 06:49 PM, Michael S. Tsirkin wrote: >>> On Mon, Apr 02, 2018 at 09:40:02AM -0400, Vladislav Yasevich wrote: >>>> To support SCTP checksum offloading, we need to add a new feature >>>> to virtio_net, so we can negotiate support between the hypervisor >>>> and the guest. >>>> >>>> The signalling to the guest that an alternate checksum needs to >>>> be used is done via a new flag in the virtio_net_hdr. If the >>>> flag is set, the host will know to perform an alternate checksum >>>> calculation, which right now is only CRC32c. >>>> >>>> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> >>>> --- >>>> drivers/net/virtio_net.c | 11 ++++++++--- >>>> include/linux/virtio_net.h | 6 ++++++ >>>> include/uapi/linux/virtio_net.h | 2 ++ >>>> 3 files changed, 16 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c >>>> index 7b187ec..b601294 100644 >>>> --- a/drivers/net/virtio_net.c >>>> +++ b/drivers/net/virtio_net.c >>>> @@ -2724,9 +2724,14 @@ static int virtnet_probe(struct virtio_device *vdev) >>>> /* Do we support "hardware" checksums? */ >>>> if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) { >>>> /* This opens up the world of extra features. */ >>>> - dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG; >>>> + netdev_features_t sctp = 0; >>>> + >>>> + if (virtio_has_feature(vdev, VIRTIO_NET_F_SCTP_CSUM)) >>>> + sctp |= NETIF_F_SCTP_CRC; >>>> + >>>> + dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; >>>> if (csum) >>>> - dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG; >>>> + dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; >>>> >>>> if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) { >>>> dev->hw_features |= NETIF_F_TSO >>>> @@ -2952,7 +2957,7 @@ static struct virtio_device_id id_table[] = { >>>> }; >>>> >>>> #define VIRTNET_FEATURES \ >>>> - VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, \ >>>> + VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, VIRTIO_NET_F_SCTP_CSUM, \ >>>> VIRTIO_NET_F_MAC, \ >>>> VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6, \ >>>> VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6, \ >>>> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h >>>> index f144216..2e7a64a 100644 >>>> --- a/include/linux/virtio_net.h >>>> +++ b/include/linux/virtio_net.h >>>> @@ -39,6 +39,9 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, >>>> >>>> if (!skb_partial_csum_set(skb, start, off)) >>>> return -EINVAL; >>>> + >>>> + if (hdr->flags & VIRTIO_NET_HDR_F_CSUM_NOT_INET) >>>> + skb->csum_not_inet = 1; >>>> } >>>> >>>> if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) { >>>> @@ -96,6 +99,9 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb, >>>> hdr->flags = VIRTIO_NET_HDR_F_DATA_VALID; >>>> } /* else everything is zero */ >>>> >>>> + if (skb->csum_not_inet) >>>> + hdr->flags &= VIRTIO_NET_HDR_F_CSUM_NOT_INET; >>>> + >>>> return 0; >>>> } >>>> >>>> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h >>>> index 5de6ed3..3f279c8 100644 >>>> --- a/include/uapi/linux/virtio_net.h >>>> +++ b/include/uapi/linux/virtio_net.h >>>> @@ -36,6 +36,7 @@ >>>> #define VIRTIO_NET_F_GUEST_CSUM 1 /* Guest handles pkts w/ partial csum */ >>>> #define VIRTIO_NET_F_CTRL_GUEST_OFFLOADS 2 /* Dynamic offload configuration. */ >>>> #define VIRTIO_NET_F_MTU 3 /* Initial MTU advice */ >>>> +#define VIRTIO_NET_F_SCTP_CSUM 4 /* SCTP checksum offload support */ >>>> #define VIRTIO_NET_F_MAC 5 /* Host has given MAC address. */ >>>> #define VIRTIO_NET_F_GUEST_TSO4 7 /* Guest can handle TSOv4 in. */ >>>> #define VIRTIO_NET_F_GUEST_TSO6 8 /* Guest can handle TSOv6 in. */ >>> >>> Is this a guest or a host checksum? We should differenciate between the >>> two. >> >> I suppose this is HOST checksum, since it behaves like VIRTIO_NET_F_CSUM only for >> SCTP. I couldn't find the use for the GUEST side flag, since there technically >> isn't any validations and there is no additional mappings from VIRTIO flag to a >> NETIF flag. >> >> If the feature is negotiated, the guest ends up generating partially check-summed >> packets, and the host turns on appropriate flags on it's side. The host will >> also make sure the checksum if fixed up if the guest doesn't support it. >> So 1 flag is currently all that is needed. >> >> -vlad > > I see code handling VIRTIO_NET_HDR_F_CSUM_NOT_INET on RX side. Host > needs to know whether it's ok/worth it to set this flag, too. I think I understand. I have to re-consider outside of the context of Linux behavior. -vlad > >>> >>> >>>> @@ -101,6 +102,7 @@ struct virtio_net_config { >>>> struct virtio_net_hdr_v1 { >>>> #define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start, csum_offset */ >>>> #define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */ >>>> +#define VIRTIO_NET_HDR_F_CSUM_NOT_INET 4 /* Checksum is not inet */ >>>> __u8 flags; >>>> #define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame */ >>>> #define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP (TSO) */ >>>> -- >>>> 2.9.5 ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 1/5] virtio: Add support for SCTP checksum offloading @ 2018-04-17 19:06 ` Vlad Yasevich 0 siblings, 0 replies; 74+ messages in thread From: Vlad Yasevich @ 2018-04-17 19:06 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Vladislav Yasevich, netdev, linux-sctp, virtualization, jasowang, nhorman On 04/16/2018 01:09 PM, Michael S. Tsirkin wrote: > On Mon, Apr 16, 2018 at 09:45:48AM -0400, Vlad Yasevich wrote: >> On 04/11/2018 06:49 PM, Michael S. Tsirkin wrote: >>> On Mon, Apr 02, 2018 at 09:40:02AM -0400, Vladislav Yasevich wrote: >>>> To support SCTP checksum offloading, we need to add a new feature >>>> to virtio_net, so we can negotiate support between the hypervisor >>>> and the guest. >>>> >>>> The signalling to the guest that an alternate checksum needs to >>>> be used is done via a new flag in the virtio_net_hdr. If the >>>> flag is set, the host will know to perform an alternate checksum >>>> calculation, which right now is only CRC32c. >>>> >>>> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> >>>> --- >>>> drivers/net/virtio_net.c | 11 ++++++++--- >>>> include/linux/virtio_net.h | 6 ++++++ >>>> include/uapi/linux/virtio_net.h | 2 ++ >>>> 3 files changed, 16 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c >>>> index 7b187ec..b601294 100644 >>>> --- a/drivers/net/virtio_net.c >>>> +++ b/drivers/net/virtio_net.c >>>> @@ -2724,9 +2724,14 @@ static int virtnet_probe(struct virtio_device *vdev) >>>> /* Do we support "hardware" checksums? */ >>>> if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) { >>>> /* This opens up the world of extra features. */ >>>> - dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG; >>>> + netdev_features_t sctp = 0; >>>> + >>>> + if (virtio_has_feature(vdev, VIRTIO_NET_F_SCTP_CSUM)) >>>> + sctp |= NETIF_F_SCTP_CRC; >>>> + >>>> + dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; >>>> if (csum) >>>> - dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG; >>>> + dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; >>>> >>>> if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) { >>>> dev->hw_features |= NETIF_F_TSO >>>> @@ -2952,7 +2957,7 @@ static struct virtio_device_id id_table[] = { >>>> }; >>>> >>>> #define VIRTNET_FEATURES \ >>>> - VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, \ >>>> + VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, VIRTIO_NET_F_SCTP_CSUM, \ >>>> VIRTIO_NET_F_MAC, \ >>>> VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6, \ >>>> VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6, \ >>>> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h >>>> index f144216..2e7a64a 100644 >>>> --- a/include/linux/virtio_net.h >>>> +++ b/include/linux/virtio_net.h >>>> @@ -39,6 +39,9 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, >>>> >>>> if (!skb_partial_csum_set(skb, start, off)) >>>> return -EINVAL; >>>> + >>>> + if (hdr->flags & VIRTIO_NET_HDR_F_CSUM_NOT_INET) >>>> + skb->csum_not_inet = 1; >>>> } >>>> >>>> if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) { >>>> @@ -96,6 +99,9 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb, >>>> hdr->flags = VIRTIO_NET_HDR_F_DATA_VALID; >>>> } /* else everything is zero */ >>>> >>>> + if (skb->csum_not_inet) >>>> + hdr->flags &= VIRTIO_NET_HDR_F_CSUM_NOT_INET; >>>> + >>>> return 0; >>>> } >>>> >>>> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h >>>> index 5de6ed3..3f279c8 100644 >>>> --- a/include/uapi/linux/virtio_net.h >>>> +++ b/include/uapi/linux/virtio_net.h >>>> @@ -36,6 +36,7 @@ >>>> #define VIRTIO_NET_F_GUEST_CSUM 1 /* Guest handles pkts w/ partial csum */ >>>> #define VIRTIO_NET_F_CTRL_GUEST_OFFLOADS 2 /* Dynamic offload configuration. */ >>>> #define VIRTIO_NET_F_MTU 3 /* Initial MTU advice */ >>>> +#define VIRTIO_NET_F_SCTP_CSUM 4 /* SCTP checksum offload support */ >>>> #define VIRTIO_NET_F_MAC 5 /* Host has given MAC address. */ >>>> #define VIRTIO_NET_F_GUEST_TSO4 7 /* Guest can handle TSOv4 in. */ >>>> #define VIRTIO_NET_F_GUEST_TSO6 8 /* Guest can handle TSOv6 in. */ >>> >>> Is this a guest or a host checksum? We should differenciate between the >>> two. >> >> I suppose this is HOST checksum, since it behaves like VIRTIO_NET_F_CSUM only for >> SCTP. I couldn't find the use for the GUEST side flag, since there technically >> isn't any validations and there is no additional mappings from VIRTIO flag to a >> NETIF flag. >> >> If the feature is negotiated, the guest ends up generating partially check-summed >> packets, and the host turns on appropriate flags on it's side. The host will >> also make sure the checksum if fixed up if the guest doesn't support it. >> So 1 flag is currently all that is needed. >> >> -vlad > > I see code handling VIRTIO_NET_HDR_F_CSUM_NOT_INET on RX side. Host > needs to know whether it's ok/worth it to set this flag, too. I think I understand. I have to re-consider outside of the context of Linux behavior. -vlad > >>> >>> >>>> @@ -101,6 +102,7 @@ struct virtio_net_config { >>>> struct virtio_net_hdr_v1 { >>>> #define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start, csum_offset */ >>>> #define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */ >>>> +#define VIRTIO_NET_HDR_F_CSUM_NOT_INET 4 /* Checksum is not inet */ >>>> __u8 flags; >>>> #define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame */ >>>> #define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP (TSO) */ >>>> -- >>>> 2.9.5 ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 1/5] virtio: Add support for SCTP checksum offloading 2018-04-16 17:09 ` Michael S. Tsirkin (?) (?) @ 2018-04-17 19:06 ` Vlad Yasevich -1 siblings, 0 replies; 74+ messages in thread From: Vlad Yasevich @ 2018-04-17 19:06 UTC (permalink / raw) To: Michael S. Tsirkin Cc: nhorman, netdev, virtualization, linux-sctp, Vladislav Yasevich On 04/16/2018 01:09 PM, Michael S. Tsirkin wrote: > On Mon, Apr 16, 2018 at 09:45:48AM -0400, Vlad Yasevich wrote: >> On 04/11/2018 06:49 PM, Michael S. Tsirkin wrote: >>> On Mon, Apr 02, 2018 at 09:40:02AM -0400, Vladislav Yasevich wrote: >>>> To support SCTP checksum offloading, we need to add a new feature >>>> to virtio_net, so we can negotiate support between the hypervisor >>>> and the guest. >>>> >>>> The signalling to the guest that an alternate checksum needs to >>>> be used is done via a new flag in the virtio_net_hdr. If the >>>> flag is set, the host will know to perform an alternate checksum >>>> calculation, which right now is only CRC32c. >>>> >>>> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> >>>> --- >>>> drivers/net/virtio_net.c | 11 ++++++++--- >>>> include/linux/virtio_net.h | 6 ++++++ >>>> include/uapi/linux/virtio_net.h | 2 ++ >>>> 3 files changed, 16 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c >>>> index 7b187ec..b601294 100644 >>>> --- a/drivers/net/virtio_net.c >>>> +++ b/drivers/net/virtio_net.c >>>> @@ -2724,9 +2724,14 @@ static int virtnet_probe(struct virtio_device *vdev) >>>> /* Do we support "hardware" checksums? */ >>>> if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) { >>>> /* This opens up the world of extra features. */ >>>> - dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG; >>>> + netdev_features_t sctp = 0; >>>> + >>>> + if (virtio_has_feature(vdev, VIRTIO_NET_F_SCTP_CSUM)) >>>> + sctp |= NETIF_F_SCTP_CRC; >>>> + >>>> + dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; >>>> if (csum) >>>> - dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG; >>>> + dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; >>>> >>>> if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) { >>>> dev->hw_features |= NETIF_F_TSO >>>> @@ -2952,7 +2957,7 @@ static struct virtio_device_id id_table[] = { >>>> }; >>>> >>>> #define VIRTNET_FEATURES \ >>>> - VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, \ >>>> + VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, VIRTIO_NET_F_SCTP_CSUM, \ >>>> VIRTIO_NET_F_MAC, \ >>>> VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6, \ >>>> VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6, \ >>>> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h >>>> index f144216..2e7a64a 100644 >>>> --- a/include/linux/virtio_net.h >>>> +++ b/include/linux/virtio_net.h >>>> @@ -39,6 +39,9 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, >>>> >>>> if (!skb_partial_csum_set(skb, start, off)) >>>> return -EINVAL; >>>> + >>>> + if (hdr->flags & VIRTIO_NET_HDR_F_CSUM_NOT_INET) >>>> + skb->csum_not_inet = 1; >>>> } >>>> >>>> if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) { >>>> @@ -96,6 +99,9 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb, >>>> hdr->flags = VIRTIO_NET_HDR_F_DATA_VALID; >>>> } /* else everything is zero */ >>>> >>>> + if (skb->csum_not_inet) >>>> + hdr->flags &= VIRTIO_NET_HDR_F_CSUM_NOT_INET; >>>> + >>>> return 0; >>>> } >>>> >>>> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h >>>> index 5de6ed3..3f279c8 100644 >>>> --- a/include/uapi/linux/virtio_net.h >>>> +++ b/include/uapi/linux/virtio_net.h >>>> @@ -36,6 +36,7 @@ >>>> #define VIRTIO_NET_F_GUEST_CSUM 1 /* Guest handles pkts w/ partial csum */ >>>> #define VIRTIO_NET_F_CTRL_GUEST_OFFLOADS 2 /* Dynamic offload configuration. */ >>>> #define VIRTIO_NET_F_MTU 3 /* Initial MTU advice */ >>>> +#define VIRTIO_NET_F_SCTP_CSUM 4 /* SCTP checksum offload support */ >>>> #define VIRTIO_NET_F_MAC 5 /* Host has given MAC address. */ >>>> #define VIRTIO_NET_F_GUEST_TSO4 7 /* Guest can handle TSOv4 in. */ >>>> #define VIRTIO_NET_F_GUEST_TSO6 8 /* Guest can handle TSOv6 in. */ >>> >>> Is this a guest or a host checksum? We should differenciate between the >>> two. >> >> I suppose this is HOST checksum, since it behaves like VIRTIO_NET_F_CSUM only for >> SCTP. I couldn't find the use for the GUEST side flag, since there technically >> isn't any validations and there is no additional mappings from VIRTIO flag to a >> NETIF flag. >> >> If the feature is negotiated, the guest ends up generating partially check-summed >> packets, and the host turns on appropriate flags on it's side. The host will >> also make sure the checksum if fixed up if the guest doesn't support it. >> So 1 flag is currently all that is needed. >> >> -vlad > > I see code handling VIRTIO_NET_HDR_F_CSUM_NOT_INET on RX side. Host > needs to know whether it's ok/worth it to set this flag, too. I think I understand. I have to re-consider outside of the context of Linux behavior. -vlad > >>> >>> >>>> @@ -101,6 +102,7 @@ struct virtio_net_config { >>>> struct virtio_net_hdr_v1 { >>>> #define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start, csum_offset */ >>>> #define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */ >>>> +#define VIRTIO_NET_HDR_F_CSUM_NOT_INET 4 /* Checksum is not inet */ >>>> __u8 flags; >>>> #define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame */ >>>> #define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP (TSO) */ >>>> -- >>>> 2.9.5 ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 1/5] virtio: Add support for SCTP checksum offloading 2018-04-16 13:45 ` Vlad Yasevich ` (2 preceding siblings ...) (?) @ 2018-04-16 17:09 ` Michael S. Tsirkin -1 siblings, 0 replies; 74+ messages in thread From: Michael S. Tsirkin @ 2018-04-16 17:09 UTC (permalink / raw) To: Vlad Yasevich Cc: nhorman, netdev, virtualization, linux-sctp, Vladislav Yasevich On Mon, Apr 16, 2018 at 09:45:48AM -0400, Vlad Yasevich wrote: > On 04/11/2018 06:49 PM, Michael S. Tsirkin wrote: > > On Mon, Apr 02, 2018 at 09:40:02AM -0400, Vladislav Yasevich wrote: > >> To support SCTP checksum offloading, we need to add a new feature > >> to virtio_net, so we can negotiate support between the hypervisor > >> and the guest. > >> > >> The signalling to the guest that an alternate checksum needs to > >> be used is done via a new flag in the virtio_net_hdr. If the > >> flag is set, the host will know to perform an alternate checksum > >> calculation, which right now is only CRC32c. > >> > >> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> > >> --- > >> drivers/net/virtio_net.c | 11 ++++++++--- > >> include/linux/virtio_net.h | 6 ++++++ > >> include/uapi/linux/virtio_net.h | 2 ++ > >> 3 files changed, 16 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > >> index 7b187ec..b601294 100644 > >> --- a/drivers/net/virtio_net.c > >> +++ b/drivers/net/virtio_net.c > >> @@ -2724,9 +2724,14 @@ static int virtnet_probe(struct virtio_device *vdev) > >> /* Do we support "hardware" checksums? */ > >> if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) { > >> /* This opens up the world of extra features. */ > >> - dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG; > >> + netdev_features_t sctp = 0; > >> + > >> + if (virtio_has_feature(vdev, VIRTIO_NET_F_SCTP_CSUM)) > >> + sctp |= NETIF_F_SCTP_CRC; > >> + > >> + dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; > >> if (csum) > >> - dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG; > >> + dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; > >> > >> if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) { > >> dev->hw_features |= NETIF_F_TSO > >> @@ -2952,7 +2957,7 @@ static struct virtio_device_id id_table[] = { > >> }; > >> > >> #define VIRTNET_FEATURES \ > >> - VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, \ > >> + VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, VIRTIO_NET_F_SCTP_CSUM, \ > >> VIRTIO_NET_F_MAC, \ > >> VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6, \ > >> VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6, \ > >> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h > >> index f144216..2e7a64a 100644 > >> --- a/include/linux/virtio_net.h > >> +++ b/include/linux/virtio_net.h > >> @@ -39,6 +39,9 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, > >> > >> if (!skb_partial_csum_set(skb, start, off)) > >> return -EINVAL; > >> + > >> + if (hdr->flags & VIRTIO_NET_HDR_F_CSUM_NOT_INET) > >> + skb->csum_not_inet = 1; > >> } > >> > >> if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) { > >> @@ -96,6 +99,9 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb, > >> hdr->flags = VIRTIO_NET_HDR_F_DATA_VALID; > >> } /* else everything is zero */ > >> > >> + if (skb->csum_not_inet) > >> + hdr->flags &= VIRTIO_NET_HDR_F_CSUM_NOT_INET; > >> + > >> return 0; > >> } > >> > >> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h > >> index 5de6ed3..3f279c8 100644 > >> --- a/include/uapi/linux/virtio_net.h > >> +++ b/include/uapi/linux/virtio_net.h > >> @@ -36,6 +36,7 @@ > >> #define VIRTIO_NET_F_GUEST_CSUM 1 /* Guest handles pkts w/ partial csum */ > >> #define VIRTIO_NET_F_CTRL_GUEST_OFFLOADS 2 /* Dynamic offload configuration. */ > >> #define VIRTIO_NET_F_MTU 3 /* Initial MTU advice */ > >> +#define VIRTIO_NET_F_SCTP_CSUM 4 /* SCTP checksum offload support */ > >> #define VIRTIO_NET_F_MAC 5 /* Host has given MAC address. */ > >> #define VIRTIO_NET_F_GUEST_TSO4 7 /* Guest can handle TSOv4 in. */ > >> #define VIRTIO_NET_F_GUEST_TSO6 8 /* Guest can handle TSOv6 in. */ > > > > Is this a guest or a host checksum? We should differenciate between the > > two. > > I suppose this is HOST checksum, since it behaves like VIRTIO_NET_F_CSUM only for > SCTP. I couldn't find the use for the GUEST side flag, since there technically > isn't any validations and there is no additional mappings from VIRTIO flag to a > NETIF flag. > > If the feature is negotiated, the guest ends up generating partially check-summed > packets, and the host turns on appropriate flags on it's side. The host will > also make sure the checksum if fixed up if the guest doesn't support it. > So 1 flag is currently all that is needed. > > -vlad I see code handling VIRTIO_NET_HDR_F_CSUM_NOT_INET on RX side. Host needs to know whether it's ok/worth it to set this flag, too. > > > > > >> @@ -101,6 +102,7 @@ struct virtio_net_config { > >> struct virtio_net_hdr_v1 { > >> #define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start, csum_offset */ > >> #define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */ > >> +#define VIRTIO_NET_HDR_F_CSUM_NOT_INET 4 /* Checksum is not inet */ > >> __u8 flags; > >> #define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame */ > >> #define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP (TSO) */ > >> -- > >> 2.9.5 ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 1/5] virtio: Add support for SCTP checksum offloading 2018-04-11 22:49 ` Michael S. Tsirkin (?) (?) @ 2018-04-16 13:45 ` Vlad Yasevich -1 siblings, 0 replies; 74+ messages in thread From: Vlad Yasevich @ 2018-04-16 13:45 UTC (permalink / raw) To: Michael S. Tsirkin, Vladislav Yasevich Cc: netdev, linux-sctp, nhorman, virtualization On 04/11/2018 06:49 PM, Michael S. Tsirkin wrote: > On Mon, Apr 02, 2018 at 09:40:02AM -0400, Vladislav Yasevich wrote: >> To support SCTP checksum offloading, we need to add a new feature >> to virtio_net, so we can negotiate support between the hypervisor >> and the guest. >> >> The signalling to the guest that an alternate checksum needs to >> be used is done via a new flag in the virtio_net_hdr. If the >> flag is set, the host will know to perform an alternate checksum >> calculation, which right now is only CRC32c. >> >> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> >> --- >> drivers/net/virtio_net.c | 11 ++++++++--- >> include/linux/virtio_net.h | 6 ++++++ >> include/uapi/linux/virtio_net.h | 2 ++ >> 3 files changed, 16 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c >> index 7b187ec..b601294 100644 >> --- a/drivers/net/virtio_net.c >> +++ b/drivers/net/virtio_net.c >> @@ -2724,9 +2724,14 @@ static int virtnet_probe(struct virtio_device *vdev) >> /* Do we support "hardware" checksums? */ >> if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) { >> /* This opens up the world of extra features. */ >> - dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG; >> + netdev_features_t sctp = 0; >> + >> + if (virtio_has_feature(vdev, VIRTIO_NET_F_SCTP_CSUM)) >> + sctp |= NETIF_F_SCTP_CRC; >> + >> + dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; >> if (csum) >> - dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG; >> + dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; >> >> if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) { >> dev->hw_features |= NETIF_F_TSO >> @@ -2952,7 +2957,7 @@ static struct virtio_device_id id_table[] = { >> }; >> >> #define VIRTNET_FEATURES \ >> - VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, \ >> + VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, VIRTIO_NET_F_SCTP_CSUM, \ >> VIRTIO_NET_F_MAC, \ >> VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6, \ >> VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6, \ >> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h >> index f144216..2e7a64a 100644 >> --- a/include/linux/virtio_net.h >> +++ b/include/linux/virtio_net.h >> @@ -39,6 +39,9 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, >> >> if (!skb_partial_csum_set(skb, start, off)) >> return -EINVAL; >> + >> + if (hdr->flags & VIRTIO_NET_HDR_F_CSUM_NOT_INET) >> + skb->csum_not_inet = 1; >> } >> >> if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) { >> @@ -96,6 +99,9 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb, >> hdr->flags = VIRTIO_NET_HDR_F_DATA_VALID; >> } /* else everything is zero */ >> >> + if (skb->csum_not_inet) >> + hdr->flags &= VIRTIO_NET_HDR_F_CSUM_NOT_INET; >> + >> return 0; >> } >> >> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h >> index 5de6ed3..3f279c8 100644 >> --- a/include/uapi/linux/virtio_net.h >> +++ b/include/uapi/linux/virtio_net.h >> @@ -36,6 +36,7 @@ >> #define VIRTIO_NET_F_GUEST_CSUM 1 /* Guest handles pkts w/ partial csum */ >> #define VIRTIO_NET_F_CTRL_GUEST_OFFLOADS 2 /* Dynamic offload configuration. */ >> #define VIRTIO_NET_F_MTU 3 /* Initial MTU advice */ >> +#define VIRTIO_NET_F_SCTP_CSUM 4 /* SCTP checksum offload support */ >> #define VIRTIO_NET_F_MAC 5 /* Host has given MAC address. */ >> #define VIRTIO_NET_F_GUEST_TSO4 7 /* Guest can handle TSOv4 in. */ >> #define VIRTIO_NET_F_GUEST_TSO6 8 /* Guest can handle TSOv6 in. */ > > Is this a guest or a host checksum? We should differenciate between the > two. I suppose this is HOST checksum, since it behaves like VIRTIO_NET_F_CSUM only for SCTP. I couldn't find the use for the GUEST side flag, since there technically isn't any validations and there is no additional mappings from VIRTIO flag to a NETIF flag. If the feature is negotiated, the guest ends up generating partially check-summed packets, and the host turns on appropriate flags on it's side. The host will also make sure the checksum if fixed up if the guest doesn't support it. So 1 flag is currently all that is needed. -vlad > > >> @@ -101,6 +102,7 @@ struct virtio_net_config { >> struct virtio_net_hdr_v1 { >> #define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start, csum_offset */ >> #define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */ >> +#define VIRTIO_NET_HDR_F_CSUM_NOT_INET 4 /* Checksum is not inet */ >> __u8 flags; >> #define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame */ >> #define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP (TSO) */ >> -- >> 2.9.5 ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 1/5] virtio: Add support for SCTP checksum offloading 2018-04-02 13:40 ` Vladislav Yasevich ` (4 preceding siblings ...) (?) @ 2018-04-11 22:49 ` Michael S. Tsirkin -1 siblings, 0 replies; 74+ messages in thread From: Michael S. Tsirkin @ 2018-04-11 22:49 UTC (permalink / raw) To: Vladislav Yasevich; +Cc: nhorman, netdev, virtualization, linux-sctp On Mon, Apr 02, 2018 at 09:40:02AM -0400, Vladislav Yasevich wrote: > To support SCTP checksum offloading, we need to add a new feature > to virtio_net, so we can negotiate support between the hypervisor > and the guest. > > The signalling to the guest that an alternate checksum needs to > be used is done via a new flag in the virtio_net_hdr. If the > flag is set, the host will know to perform an alternate checksum > calculation, which right now is only CRC32c. > > Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> > --- > drivers/net/virtio_net.c | 11 ++++++++--- > include/linux/virtio_net.h | 6 ++++++ > include/uapi/linux/virtio_net.h | 2 ++ > 3 files changed, 16 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 7b187ec..b601294 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -2724,9 +2724,14 @@ static int virtnet_probe(struct virtio_device *vdev) > /* Do we support "hardware" checksums? */ > if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) { > /* This opens up the world of extra features. */ > - dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG; > + netdev_features_t sctp = 0; > + > + if (virtio_has_feature(vdev, VIRTIO_NET_F_SCTP_CSUM)) > + sctp |= NETIF_F_SCTP_CRC; > + > + dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; > if (csum) > - dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG; > + dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; > > if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) { > dev->hw_features |= NETIF_F_TSO > @@ -2952,7 +2957,7 @@ static struct virtio_device_id id_table[] = { > }; > > #define VIRTNET_FEATURES \ > - VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, \ > + VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, VIRTIO_NET_F_SCTP_CSUM, \ > VIRTIO_NET_F_MAC, \ > VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6, \ > VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6, \ > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h > index f144216..2e7a64a 100644 > --- a/include/linux/virtio_net.h > +++ b/include/linux/virtio_net.h > @@ -39,6 +39,9 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, > > if (!skb_partial_csum_set(skb, start, off)) > return -EINVAL; > + > + if (hdr->flags & VIRTIO_NET_HDR_F_CSUM_NOT_INET) > + skb->csum_not_inet = 1; > } > > if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) { > @@ -96,6 +99,9 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb, > hdr->flags = VIRTIO_NET_HDR_F_DATA_VALID; > } /* else everything is zero */ > > + if (skb->csum_not_inet) > + hdr->flags &= VIRTIO_NET_HDR_F_CSUM_NOT_INET; > + > return 0; > } > > diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h > index 5de6ed3..3f279c8 100644 > --- a/include/uapi/linux/virtio_net.h > +++ b/include/uapi/linux/virtio_net.h > @@ -36,6 +36,7 @@ > #define VIRTIO_NET_F_GUEST_CSUM 1 /* Guest handles pkts w/ partial csum */ > #define VIRTIO_NET_F_CTRL_GUEST_OFFLOADS 2 /* Dynamic offload configuration. */ > #define VIRTIO_NET_F_MTU 3 /* Initial MTU advice */ > +#define VIRTIO_NET_F_SCTP_CSUM 4 /* SCTP checksum offload support */ > #define VIRTIO_NET_F_MAC 5 /* Host has given MAC address. */ > #define VIRTIO_NET_F_GUEST_TSO4 7 /* Guest can handle TSOv4 in. */ > #define VIRTIO_NET_F_GUEST_TSO6 8 /* Guest can handle TSOv6 in. */ Is this a guest or a host checksum? We should differenciate between the two. > @@ -101,6 +102,7 @@ struct virtio_net_config { > struct virtio_net_hdr_v1 { > #define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start, csum_offset */ > #define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */ > +#define VIRTIO_NET_HDR_F_CSUM_NOT_INET 4 /* Checksum is not inet */ > __u8 flags; > #define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame */ > #define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP (TSO) */ > -- > 2.9.5 ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 1/5] virtio: Add support for SCTP checksum offloading 2018-04-02 13:40 ` Vladislav Yasevich ` (5 preceding siblings ...) (?) @ 2018-04-16 17:07 ` Michael S. Tsirkin -1 siblings, 0 replies; 74+ messages in thread From: Michael S. Tsirkin @ 2018-04-16 17:07 UTC (permalink / raw) To: Vladislav Yasevich; +Cc: nhorman, netdev, virtualization, linux-sctp On Mon, Apr 02, 2018 at 09:40:02AM -0400, Vladislav Yasevich wrote: > To support SCTP checksum offloading, we need to add a new feature > to virtio_net, so we can negotiate support between the hypervisor > and the guest. > > The signalling to the guest that an alternate checksum needs to > be used is done via a new flag in the virtio_net_hdr. If the > flag is set, the host will know to perform an alternate checksum > calculation, which right now is only CRC32c. > > Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> > --- > drivers/net/virtio_net.c | 11 ++++++++--- > include/linux/virtio_net.h | 6 ++++++ > include/uapi/linux/virtio_net.h | 2 ++ > 3 files changed, 16 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 7b187ec..b601294 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -2724,9 +2724,14 @@ static int virtnet_probe(struct virtio_device *vdev) > /* Do we support "hardware" checksums? */ > if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) { > /* This opens up the world of extra features. */ > - dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG; > + netdev_features_t sctp = 0; > + > + if (virtio_has_feature(vdev, VIRTIO_NET_F_SCTP_CSUM)) > + sctp |= NETIF_F_SCTP_CRC; > + > + dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; > if (csum) > - dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG; > + dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; > > if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) { > dev->hw_features |= NETIF_F_TSO > @@ -2952,7 +2957,7 @@ static struct virtio_device_id id_table[] = { > }; > > #define VIRTNET_FEATURES \ > - VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, \ > + VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, VIRTIO_NET_F_SCTP_CSUM, \ > VIRTIO_NET_F_MAC, \ > VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6, \ > VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6, \ > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h > index f144216..2e7a64a 100644 > --- a/include/linux/virtio_net.h > +++ b/include/linux/virtio_net.h > @@ -39,6 +39,9 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, > > if (!skb_partial_csum_set(skb, start, off)) > return -EINVAL; > + > + if (hdr->flags & VIRTIO_NET_HDR_F_CSUM_NOT_INET) > + skb->csum_not_inet = 1; > } > > if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) { Looks like we need a feature flag to tell host guest is able to handle this flag in incoming packets ... > @@ -96,6 +99,9 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb, > hdr->flags = VIRTIO_NET_HDR_F_DATA_VALID; > } /* else everything is zero */ Is comment above still correct? > > + if (skb->csum_not_inet) > + hdr->flags &= VIRTIO_NET_HDR_F_CSUM_NOT_INET; > + ... and a separate flag to tell guest it's ok to set this flag in outgoing packets. Also - this chunk looks like a hack fixing up a value we have set incorrectly. Specifically this clears all flags except VIRTIO_NET_HDR_F_CSUM_NOT_INET. Why do this at all? Do we really want to clear e.g. NEEDS_CSUM? I think we do not since csum_start, csum_offset are set. Also - what will ever set this flag in the header? > return 0; > } > > diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h > index 5de6ed3..3f279c8 100644 > --- a/include/uapi/linux/virtio_net.h > +++ b/include/uapi/linux/virtio_net.h > @@ -36,6 +36,7 @@ > #define VIRTIO_NET_F_GUEST_CSUM 1 /* Guest handles pkts w/ partial csum */ > #define VIRTIO_NET_F_CTRL_GUEST_OFFLOADS 2 /* Dynamic offload configuration. */ > #define VIRTIO_NET_F_MTU 3 /* Initial MTU advice */ > +#define VIRTIO_NET_F_SCTP_CSUM 4 /* SCTP checksum offload support */ > #define VIRTIO_NET_F_MAC 5 /* Host has given MAC address. */ > #define VIRTIO_NET_F_GUEST_TSO4 7 /* Guest can handle TSOv4 in. */ > #define VIRTIO_NET_F_GUEST_TSO6 8 /* Guest can handle TSOv6 in. */ > @@ -101,6 +102,7 @@ struct virtio_net_config { > struct virtio_net_hdr_v1 { > #define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start, csum_offset */ > #define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */ > +#define VIRTIO_NET_HDR_F_CSUM_NOT_INET 4 /* Checksum is not inet */ Not very informative. Instead, please specify what is it actually. > __u8 flags; > #define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame */ > #define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP (TSO) */ > -- > 2.9.5 ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 1/5] virtio: Add support for SCTP checksum offloading 2018-04-02 13:40 ` Vladislav Yasevich @ 2018-04-16 17:07 ` Michael S. Tsirkin -1 siblings, 0 replies; 74+ messages in thread From: Michael S. Tsirkin @ 2018-04-16 17:07 UTC (permalink / raw) To: Vladislav Yasevich Cc: netdev, linux-sctp, virtualization, jasowang, nhorman, Vladislav Yasevich On Mon, Apr 02, 2018 at 09:40:02AM -0400, Vladislav Yasevich wrote: > To support SCTP checksum offloading, we need to add a new feature > to virtio_net, so we can negotiate support between the hypervisor > and the guest. > > The signalling to the guest that an alternate checksum needs to > be used is done via a new flag in the virtio_net_hdr. If the > flag is set, the host will know to perform an alternate checksum > calculation, which right now is only CRC32c. > > Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> > --- > drivers/net/virtio_net.c | 11 ++++++++--- > include/linux/virtio_net.h | 6 ++++++ > include/uapi/linux/virtio_net.h | 2 ++ > 3 files changed, 16 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 7b187ec..b601294 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -2724,9 +2724,14 @@ static int virtnet_probe(struct virtio_device *vdev) > /* Do we support "hardware" checksums? */ > if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) { > /* This opens up the world of extra features. */ > - dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG; > + netdev_features_t sctp = 0; > + > + if (virtio_has_feature(vdev, VIRTIO_NET_F_SCTP_CSUM)) > + sctp |= NETIF_F_SCTP_CRC; > + > + dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; > if (csum) > - dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG; > + dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; > > if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) { > dev->hw_features |= NETIF_F_TSO > @@ -2952,7 +2957,7 @@ static struct virtio_device_id id_table[] = { > }; > > #define VIRTNET_FEATURES \ > - VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, \ > + VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, VIRTIO_NET_F_SCTP_CSUM, \ > VIRTIO_NET_F_MAC, \ > VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6, \ > VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6, \ > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h > index f144216..2e7a64a 100644 > --- a/include/linux/virtio_net.h > +++ b/include/linux/virtio_net.h > @@ -39,6 +39,9 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, > > if (!skb_partial_csum_set(skb, start, off)) > return -EINVAL; > + > + if (hdr->flags & VIRTIO_NET_HDR_F_CSUM_NOT_INET) > + skb->csum_not_inet = 1; > } > > if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) { Looks like we need a feature flag to tell host guest is able to handle this flag in incoming packets ... > @@ -96,6 +99,9 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb, > hdr->flags = VIRTIO_NET_HDR_F_DATA_VALID; > } /* else everything is zero */ Is comment above still correct? > > + if (skb->csum_not_inet) > + hdr->flags &= VIRTIO_NET_HDR_F_CSUM_NOT_INET; > + ... and a separate flag to tell guest it's ok to set this flag in outgoing packets. Also - this chunk looks like a hack fixing up a value we have set incorrectly. Specifically this clears all flags except VIRTIO_NET_HDR_F_CSUM_NOT_INET. Why do this at all? Do we really want to clear e.g. NEEDS_CSUM? I think we do not since csum_start, csum_offset are set. Also - what will ever set this flag in the header? > return 0; > } > > diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h > index 5de6ed3..3f279c8 100644 > --- a/include/uapi/linux/virtio_net.h > +++ b/include/uapi/linux/virtio_net.h > @@ -36,6 +36,7 @@ > #define VIRTIO_NET_F_GUEST_CSUM 1 /* Guest handles pkts w/ partial csum */ > #define VIRTIO_NET_F_CTRL_GUEST_OFFLOADS 2 /* Dynamic offload configuration. */ > #define VIRTIO_NET_F_MTU 3 /* Initial MTU advice */ > +#define VIRTIO_NET_F_SCTP_CSUM 4 /* SCTP checksum offload support */ > #define VIRTIO_NET_F_MAC 5 /* Host has given MAC address. */ > #define VIRTIO_NET_F_GUEST_TSO4 7 /* Guest can handle TSOv4 in. */ > #define VIRTIO_NET_F_GUEST_TSO6 8 /* Guest can handle TSOv6 in. */ > @@ -101,6 +102,7 @@ struct virtio_net_config { > struct virtio_net_hdr_v1 { > #define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start, csum_offset */ > #define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */ > +#define VIRTIO_NET_HDR_F_CSUM_NOT_INET 4 /* Checksum is not inet */ Not very informative. Instead, please specify what is it actually. > __u8 flags; > #define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame */ > #define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP (TSO) */ > -- > 2.9.5 ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 1/5] virtio: Add support for SCTP checksum offloading @ 2018-04-16 17:07 ` Michael S. Tsirkin 0 siblings, 0 replies; 74+ messages in thread From: Michael S. Tsirkin @ 2018-04-16 17:07 UTC (permalink / raw) To: Vladislav Yasevich Cc: netdev, linux-sctp, virtualization, jasowang, nhorman, Vladislav Yasevich On Mon, Apr 02, 2018 at 09:40:02AM -0400, Vladislav Yasevich wrote: > To support SCTP checksum offloading, we need to add a new feature > to virtio_net, so we can negotiate support between the hypervisor > and the guest. > > The signalling to the guest that an alternate checksum needs to > be used is done via a new flag in the virtio_net_hdr. If the > flag is set, the host will know to perform an alternate checksum > calculation, which right now is only CRC32c. > > Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> > --- > drivers/net/virtio_net.c | 11 ++++++++--- > include/linux/virtio_net.h | 6 ++++++ > include/uapi/linux/virtio_net.h | 2 ++ > 3 files changed, 16 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 7b187ec..b601294 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -2724,9 +2724,14 @@ static int virtnet_probe(struct virtio_device *vdev) > /* Do we support "hardware" checksums? */ > if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) { > /* This opens up the world of extra features. */ > - dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG; > + netdev_features_t sctp = 0; > + > + if (virtio_has_feature(vdev, VIRTIO_NET_F_SCTP_CSUM)) > + sctp |= NETIF_F_SCTP_CRC; > + > + dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; > if (csum) > - dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG; > + dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; > > if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) { > dev->hw_features |= NETIF_F_TSO > @@ -2952,7 +2957,7 @@ static struct virtio_device_id id_table[] = { > }; > > #define VIRTNET_FEATURES \ > - VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, \ > + VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, VIRTIO_NET_F_SCTP_CSUM, \ > VIRTIO_NET_F_MAC, \ > VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6, \ > VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6, \ > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h > index f144216..2e7a64a 100644 > --- a/include/linux/virtio_net.h > +++ b/include/linux/virtio_net.h > @@ -39,6 +39,9 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, > > if (!skb_partial_csum_set(skb, start, off)) > return -EINVAL; > + > + if (hdr->flags & VIRTIO_NET_HDR_F_CSUM_NOT_INET) > + skb->csum_not_inet = 1; > } > > if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) { Looks like we need a feature flag to tell host guest is able to handle this flag in incoming packets ... > @@ -96,6 +99,9 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb, > hdr->flags = VIRTIO_NET_HDR_F_DATA_VALID; > } /* else everything is zero */ Is comment above still correct? > > + if (skb->csum_not_inet) > + hdr->flags &= VIRTIO_NET_HDR_F_CSUM_NOT_INET; > + ... and a separate flag to tell guest it's ok to set this flag in outgoing packets. Also - this chunk looks like a hack fixing up a value we have set incorrectly. Specifically this clears all flags except VIRTIO_NET_HDR_F_CSUM_NOT_INET. Why do this at all? Do we really want to clear e.g. NEEDS_CSUM? I think we do not since csum_start, csum_offset are set. Also - what will ever set this flag in the header? > return 0; > } > > diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h > index 5de6ed3..3f279c8 100644 > --- a/include/uapi/linux/virtio_net.h > +++ b/include/uapi/linux/virtio_net.h > @@ -36,6 +36,7 @@ > #define VIRTIO_NET_F_GUEST_CSUM 1 /* Guest handles pkts w/ partial csum */ > #define VIRTIO_NET_F_CTRL_GUEST_OFFLOADS 2 /* Dynamic offload configuration. */ > #define VIRTIO_NET_F_MTU 3 /* Initial MTU advice */ > +#define VIRTIO_NET_F_SCTP_CSUM 4 /* SCTP checksum offload support */ > #define VIRTIO_NET_F_MAC 5 /* Host has given MAC address. */ > #define VIRTIO_NET_F_GUEST_TSO4 7 /* Guest can handle TSOv4 in. */ > #define VIRTIO_NET_F_GUEST_TSO6 8 /* Guest can handle TSOv6 in. */ > @@ -101,6 +102,7 @@ struct virtio_net_config { > struct virtio_net_hdr_v1 { > #define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start, csum_offset */ > #define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */ > +#define VIRTIO_NET_HDR_F_CSUM_NOT_INET 4 /* Checksum is not inet */ Not very informative. Instead, please specify what is it actually. > __u8 flags; > #define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame */ > #define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP (TSO) */ > -- > 2.9.5 ^ permalink raw reply [flat|nested] 74+ messages in thread
* [PATCH net-next 1/5] virtio: Add support for SCTP checksum offloading 2018-04-02 13:40 ` Vladislav Yasevich (?) (?) @ 2018-04-02 13:40 ` Vladislav Yasevich -1 siblings, 0 replies; 74+ messages in thread From: Vladislav Yasevich @ 2018-04-02 13:40 UTC (permalink / raw) To: netdev; +Cc: nhorman, mst, virtualization, linux-sctp To support SCTP checksum offloading, we need to add a new feature to virtio_net, so we can negotiate support between the hypervisor and the guest. The signalling to the guest that an alternate checksum needs to be used is done via a new flag in the virtio_net_hdr. If the flag is set, the host will know to perform an alternate checksum calculation, which right now is only CRC32c. Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> --- drivers/net/virtio_net.c | 11 ++++++++--- include/linux/virtio_net.h | 6 ++++++ include/uapi/linux/virtio_net.h | 2 ++ 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 7b187ec..b601294 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -2724,9 +2724,14 @@ static int virtnet_probe(struct virtio_device *vdev) /* Do we support "hardware" checksums? */ if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) { /* This opens up the world of extra features. */ - dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG; + netdev_features_t sctp = 0; + + if (virtio_has_feature(vdev, VIRTIO_NET_F_SCTP_CSUM)) + sctp |= NETIF_F_SCTP_CRC; + + dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; if (csum) - dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG; + dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG | sctp; if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) { dev->hw_features |= NETIF_F_TSO @@ -2952,7 +2957,7 @@ static struct virtio_device_id id_table[] = { }; #define VIRTNET_FEATURES \ - VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, \ + VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, VIRTIO_NET_F_SCTP_CSUM, \ VIRTIO_NET_F_MAC, \ VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6, \ VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6, \ diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h index f144216..2e7a64a 100644 --- a/include/linux/virtio_net.h +++ b/include/linux/virtio_net.h @@ -39,6 +39,9 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, if (!skb_partial_csum_set(skb, start, off)) return -EINVAL; + + if (hdr->flags & VIRTIO_NET_HDR_F_CSUM_NOT_INET) + skb->csum_not_inet = 1; } if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) { @@ -96,6 +99,9 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb, hdr->flags = VIRTIO_NET_HDR_F_DATA_VALID; } /* else everything is zero */ + if (skb->csum_not_inet) + hdr->flags &= VIRTIO_NET_HDR_F_CSUM_NOT_INET; + return 0; } diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h index 5de6ed3..3f279c8 100644 --- a/include/uapi/linux/virtio_net.h +++ b/include/uapi/linux/virtio_net.h @@ -36,6 +36,7 @@ #define VIRTIO_NET_F_GUEST_CSUM 1 /* Guest handles pkts w/ partial csum */ #define VIRTIO_NET_F_CTRL_GUEST_OFFLOADS 2 /* Dynamic offload configuration. */ #define VIRTIO_NET_F_MTU 3 /* Initial MTU advice */ +#define VIRTIO_NET_F_SCTP_CSUM 4 /* SCTP checksum offload support */ #define VIRTIO_NET_F_MAC 5 /* Host has given MAC address. */ #define VIRTIO_NET_F_GUEST_TSO4 7 /* Guest can handle TSOv4 in. */ #define VIRTIO_NET_F_GUEST_TSO6 8 /* Guest can handle TSOv6 in. */ @@ -101,6 +102,7 @@ struct virtio_net_config { struct virtio_net_hdr_v1 { #define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start, csum_offset */ #define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */ +#define VIRTIO_NET_HDR_F_CSUM_NOT_INET 4 /* Checksum is not inet */ __u8 flags; #define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame */ #define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP (TSO) */ -- 2.9.5 ^ permalink raw reply related [flat|nested] 74+ messages in thread
* [PATCH net-next 2/5] sctp: Handle sctp packets with CHECKSUM_PARTIAL 2018-04-02 13:40 ` Vladislav Yasevich ` (2 preceding siblings ...) (?) @ 2018-04-02 13:40 ` Vladislav Yasevich -1 siblings, 0 replies; 74+ messages in thread From: Vladislav Yasevich @ 2018-04-02 13:40 UTC (permalink / raw) To: netdev; +Cc: nhorman, mst, virtualization, linux-sctp With SCTP checksum offload available in virtio, it is now possible for virtio to receive a sctp packet with CHECKSUM_PARTIAL set (guest-to-guest traffic). SCTP doesn't really have a partial checksum like TCP does because CRC32c can't do partial additive checksumming. It's all or nothing. So an SCTP packet with CHECKSUM_PARTIAL will have checksum set to 0. Let SCTP treat this as a valid checksum if CHECKSUM_PARTIAL is set. Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> --- net/sctp/input.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/net/sctp/input.c b/net/sctp/input.c index ba8a6e6..055b8ffa 100644 --- a/net/sctp/input.c +++ b/net/sctp/input.c @@ -80,8 +80,17 @@ static inline int sctp_rcv_checksum(struct net *net, struct sk_buff *skb) { struct sctphdr *sh = sctp_hdr(skb); __le32 cmp = sh->checksum; - __le32 val = sctp_compute_cksum(skb, 0); + __le32 val = 0; + /* In sctp PARTIAL checksum is always 0. This is a case of + * a packet received from guest that supports checksum offload. + * Assume it's correct as there is really no way to verify, + * and we want to avaoid computing it unnecesarily. + */ + if (skb->ip_summed == CHECKSUM_PARTIAL) + return 0; + + val = sctp_compute_cksum(skb, 0); if (val != cmp) { /* CRC failure, dump it. */ __SCTP_INC_STATS(net, SCTP_MIB_CHECKSUMERRORS); -- 2.9.5 ^ permalink raw reply related [flat|nested] 74+ messages in thread
* [PATCH net-next 2/5] sctp: Handle sctp packets with CHECKSUM_PARTIAL 2018-04-02 13:40 ` Vladislav Yasevich @ 2018-04-02 13:40 ` Vladislav Yasevich -1 siblings, 0 replies; 74+ messages in thread From: Vladislav Yasevich @ 2018-04-02 13:40 UTC (permalink / raw) To: netdev Cc: linux-sctp, virtualization, mst, jasowang, nhorman, Vladislav Yasevich With SCTP checksum offload available in virtio, it is now possible for virtio to receive a sctp packet with CHECKSUM_PARTIAL set (guest-to-guest traffic). SCTP doesn't really have a partial checksum like TCP does because CRC32c can't do partial additive checksumming. It's all or nothing. So an SCTP packet with CHECKSUM_PARTIAL will have checksum set to 0. Let SCTP treat this as a valid checksum if CHECKSUM_PARTIAL is set. Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> --- net/sctp/input.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/net/sctp/input.c b/net/sctp/input.c index ba8a6e6..055b8ffa 100644 --- a/net/sctp/input.c +++ b/net/sctp/input.c @@ -80,8 +80,17 @@ static inline int sctp_rcv_checksum(struct net *net, struct sk_buff *skb) { struct sctphdr *sh = sctp_hdr(skb); __le32 cmp = sh->checksum; - __le32 val = sctp_compute_cksum(skb, 0); + __le32 val = 0; + /* In sctp PARTIAL checksum is always 0. This is a case of + * a packet received from guest that supports checksum offload. + * Assume it's correct as there is really no way to verify, + * and we want to avaoid computing it unnecesarily. + */ + if (skb->ip_summed == CHECKSUM_PARTIAL) + return 0; + + val = sctp_compute_cksum(skb, 0); if (val != cmp) { /* CRC failure, dump it. */ __SCTP_INC_STATS(net, SCTP_MIB_CHECKSUMERRORS); -- 2.9.5 ^ permalink raw reply related [flat|nested] 74+ messages in thread
* [PATCH net-next 2/5] sctp: Handle sctp packets with CHECKSUM_PARTIAL @ 2018-04-02 13:40 ` Vladislav Yasevich 0 siblings, 0 replies; 74+ messages in thread From: Vladislav Yasevich @ 2018-04-02 13:40 UTC (permalink / raw) To: netdev Cc: linux-sctp, virtualization, mst, jasowang, nhorman, Vladislav Yasevich With SCTP checksum offload available in virtio, it is now possible for virtio to receive a sctp packet with CHECKSUM_PARTIAL set (guest-to-guest traffic). SCTP doesn't really have a partial checksum like TCP does because CRC32c can't do partial additive checksumming. It's all or nothing. So an SCTP packet with CHECKSUM_PARTIAL will have checksum set to 0. Let SCTP treat this as a valid checksum if CHECKSUM_PARTIAL is set. Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> --- net/sctp/input.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/net/sctp/input.c b/net/sctp/input.c index ba8a6e6..055b8ffa 100644 --- a/net/sctp/input.c +++ b/net/sctp/input.c @@ -80,8 +80,17 @@ static inline int sctp_rcv_checksum(struct net *net, struct sk_buff *skb) { struct sctphdr *sh = sctp_hdr(skb); __le32 cmp = sh->checksum; - __le32 val = sctp_compute_cksum(skb, 0); + __le32 val = 0; + /* In sctp PARTIAL checksum is always 0. This is a case of + * a packet received from guest that supports checksum offload. + * Assume it's correct as there is really no way to verify, + * and we want to avaoid computing it unnecesarily. + */ + if (skb->ip_summed = CHECKSUM_PARTIAL) + return 0; + + val = sctp_compute_cksum(skb, 0); if (val != cmp) { /* CRC failure, dump it. */ __SCTP_INC_STATS(net, SCTP_MIB_CHECKSUMERRORS); -- 2.9.5 ^ permalink raw reply related [flat|nested] 74+ messages in thread
* [PATCH net-next 3/5] sctp: Build sctp offload support into the base kernel 2018-04-02 13:40 ` Vladislav Yasevich ` (4 preceding siblings ...) (?) @ 2018-04-02 13:40 ` Vladislav Yasevich -1 siblings, 0 replies; 74+ messages in thread From: Vladislav Yasevich @ 2018-04-02 13:40 UTC (permalink / raw) To: netdev; +Cc: nhorman, mst, virtualization, linux-sctp We need to take the sctp offload out of the sctp module and add it to the base kernel to support guests that can support it. This is similar to how IPv6 offloads are done and works around kernels that exclude sctp protocol support. Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> --- include/net/sctp/sctp.h | 5 ----- net/Kconfig | 1 + net/sctp/Kconfig | 1 - net/sctp/Makefile | 3 ++- net/sctp/offload.c | 4 +++- net/sctp/protocol.c | 3 --- 6 files changed, 6 insertions(+), 11 deletions(-) diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h index 72c5b8f..625b45f 100644 --- a/include/net/sctp/sctp.h +++ b/include/net/sctp/sctp.h @@ -183,11 +183,6 @@ struct sctp_transport *sctp_epaddr_lookup_transport( int __net_init sctp_proc_init(struct net *net); /* - * sctp/offload.c - */ -int sctp_offload_init(void); - -/* * sctp/stream_sched.c */ void sctp_sched_ops_init(void); diff --git a/net/Kconfig b/net/Kconfig index 0428f12..2773f98 100644 --- a/net/Kconfig +++ b/net/Kconfig @@ -64,6 +64,7 @@ config INET bool "TCP/IP networking" select CRYPTO select CRYPTO_AES + select LIBCRC32C ---help--- These are the protocols used on the Internet and on most local Ethernets. It is highly recommended to say Y here (this will enlarge diff --git a/net/sctp/Kconfig b/net/sctp/Kconfig index c740b18..d07477a 100644 --- a/net/sctp/Kconfig +++ b/net/sctp/Kconfig @@ -9,7 +9,6 @@ menuconfig IP_SCTP select CRYPTO select CRYPTO_HMAC select CRYPTO_SHA1 - select LIBCRC32C ---help--- Stream Control Transmission Protocol diff --git a/net/sctp/Makefile b/net/sctp/Makefile index e845e45..ee206ca 100644 --- a/net/sctp/Makefile +++ b/net/sctp/Makefile @@ -5,6 +5,7 @@ obj-$(CONFIG_IP_SCTP) += sctp.o obj-$(CONFIG_INET_SCTP_DIAG) += sctp_diag.o +obj-$(CONFIG_INET) += offload.o sctp-y := sm_statetable.o sm_statefuns.o sm_sideeffect.o \ protocol.o endpointola.o associola.o \ @@ -12,7 +13,7 @@ sctp-y := sm_statetable.o sm_statefuns.o sm_sideeffect.o \ inqueue.o outqueue.o ulpqueue.o \ tsnmap.o bind_addr.o socket.o primitive.o \ output.o input.o debug.o stream.o auth.o \ - offload.o stream_sched.o stream_sched_prio.o \ + stream_sched.o stream_sched_prio.o \ stream_sched_rr.o stream_interleave.o sctp_diag-y := diag.o diff --git a/net/sctp/offload.c b/net/sctp/offload.c index 123e9f2..c61cbde 100644 --- a/net/sctp/offload.c +++ b/net/sctp/offload.c @@ -107,7 +107,7 @@ static const struct skb_checksum_ops crc32c_csum_ops = { .combine = sctp_csum_combine, }; -int __init sctp_offload_init(void) +static int __init sctp_offload_init(void) { int ret; @@ -127,3 +127,5 @@ int __init sctp_offload_init(void) out: return ret; } + +fs_initcall(sctp_offload_init); diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c index a24cde2..46d2b63 100644 --- a/net/sctp/protocol.c +++ b/net/sctp/protocol.c @@ -1479,9 +1479,6 @@ static __init int sctp_init(void) if (status) goto err_v6_add_protocol; - if (sctp_offload_init() < 0) - pr_crit("%s: Cannot add SCTP protocol offload\n", __func__); - out: return status; err_v6_add_protocol: -- 2.9.5 ^ permalink raw reply related [flat|nested] 74+ messages in thread
* [PATCH net-next 3/5] sctp: Build sctp offload support into the base kernel 2018-04-02 13:40 ` Vladislav Yasevich @ 2018-04-02 13:40 ` Vladislav Yasevich -1 siblings, 0 replies; 74+ messages in thread From: Vladislav Yasevich @ 2018-04-02 13:40 UTC (permalink / raw) To: netdev Cc: linux-sctp, virtualization, mst, jasowang, nhorman, Vladislav Yasevich We need to take the sctp offload out of the sctp module and add it to the base kernel to support guests that can support it. This is similar to how IPv6 offloads are done and works around kernels that exclude sctp protocol support. Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> --- include/net/sctp/sctp.h | 5 ----- net/Kconfig | 1 + net/sctp/Kconfig | 1 - net/sctp/Makefile | 3 ++- net/sctp/offload.c | 4 +++- net/sctp/protocol.c | 3 --- 6 files changed, 6 insertions(+), 11 deletions(-) diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h index 72c5b8f..625b45f 100644 --- a/include/net/sctp/sctp.h +++ b/include/net/sctp/sctp.h @@ -183,11 +183,6 @@ struct sctp_transport *sctp_epaddr_lookup_transport( int __net_init sctp_proc_init(struct net *net); /* - * sctp/offload.c - */ -int sctp_offload_init(void); - -/* * sctp/stream_sched.c */ void sctp_sched_ops_init(void); diff --git a/net/Kconfig b/net/Kconfig index 0428f12..2773f98 100644 --- a/net/Kconfig +++ b/net/Kconfig @@ -64,6 +64,7 @@ config INET bool "TCP/IP networking" select CRYPTO select CRYPTO_AES + select LIBCRC32C ---help--- These are the protocols used on the Internet and on most local Ethernets. It is highly recommended to say Y here (this will enlarge diff --git a/net/sctp/Kconfig b/net/sctp/Kconfig index c740b18..d07477a 100644 --- a/net/sctp/Kconfig +++ b/net/sctp/Kconfig @@ -9,7 +9,6 @@ menuconfig IP_SCTP select CRYPTO select CRYPTO_HMAC select CRYPTO_SHA1 - select LIBCRC32C ---help--- Stream Control Transmission Protocol diff --git a/net/sctp/Makefile b/net/sctp/Makefile index e845e45..ee206ca 100644 --- a/net/sctp/Makefile +++ b/net/sctp/Makefile @@ -5,6 +5,7 @@ obj-$(CONFIG_IP_SCTP) += sctp.o obj-$(CONFIG_INET_SCTP_DIAG) += sctp_diag.o +obj-$(CONFIG_INET) += offload.o sctp-y := sm_statetable.o sm_statefuns.o sm_sideeffect.o \ protocol.o endpointola.o associola.o \ @@ -12,7 +13,7 @@ sctp-y := sm_statetable.o sm_statefuns.o sm_sideeffect.o \ inqueue.o outqueue.o ulpqueue.o \ tsnmap.o bind_addr.o socket.o primitive.o \ output.o input.o debug.o stream.o auth.o \ - offload.o stream_sched.o stream_sched_prio.o \ + stream_sched.o stream_sched_prio.o \ stream_sched_rr.o stream_interleave.o sctp_diag-y := diag.o diff --git a/net/sctp/offload.c b/net/sctp/offload.c index 123e9f2..c61cbde 100644 --- a/net/sctp/offload.c +++ b/net/sctp/offload.c @@ -107,7 +107,7 @@ static const struct skb_checksum_ops crc32c_csum_ops = { .combine = sctp_csum_combine, }; -int __init sctp_offload_init(void) +static int __init sctp_offload_init(void) { int ret; @@ -127,3 +127,5 @@ int __init sctp_offload_init(void) out: return ret; } + +fs_initcall(sctp_offload_init); diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c index a24cde2..46d2b63 100644 --- a/net/sctp/protocol.c +++ b/net/sctp/protocol.c @@ -1479,9 +1479,6 @@ static __init int sctp_init(void) if (status) goto err_v6_add_protocol; - if (sctp_offload_init() < 0) - pr_crit("%s: Cannot add SCTP protocol offload\n", __func__); - out: return status; err_v6_add_protocol: -- 2.9.5 ^ permalink raw reply related [flat|nested] 74+ messages in thread
* [PATCH net-next 3/5] sctp: Build sctp offload support into the base kernel @ 2018-04-02 13:40 ` Vladislav Yasevich 0 siblings, 0 replies; 74+ messages in thread From: Vladislav Yasevich @ 2018-04-02 13:40 UTC (permalink / raw) To: netdev Cc: linux-sctp, virtualization, mst, jasowang, nhorman, Vladislav Yasevich We need to take the sctp offload out of the sctp module and add it to the base kernel to support guests that can support it. This is similar to how IPv6 offloads are done and works around kernels that exclude sctp protocol support. Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> --- include/net/sctp/sctp.h | 5 ----- net/Kconfig | 1 + net/sctp/Kconfig | 1 - net/sctp/Makefile | 3 ++- net/sctp/offload.c | 4 +++- net/sctp/protocol.c | 3 --- 6 files changed, 6 insertions(+), 11 deletions(-) diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h index 72c5b8f..625b45f 100644 --- a/include/net/sctp/sctp.h +++ b/include/net/sctp/sctp.h @@ -183,11 +183,6 @@ struct sctp_transport *sctp_epaddr_lookup_transport( int __net_init sctp_proc_init(struct net *net); /* - * sctp/offload.c - */ -int sctp_offload_init(void); - -/* * sctp/stream_sched.c */ void sctp_sched_ops_init(void); diff --git a/net/Kconfig b/net/Kconfig index 0428f12..2773f98 100644 --- a/net/Kconfig +++ b/net/Kconfig @@ -64,6 +64,7 @@ config INET bool "TCP/IP networking" select CRYPTO select CRYPTO_AES + select LIBCRC32C ---help--- These are the protocols used on the Internet and on most local Ethernets. It is highly recommended to say Y here (this will enlarge diff --git a/net/sctp/Kconfig b/net/sctp/Kconfig index c740b18..d07477a 100644 --- a/net/sctp/Kconfig +++ b/net/sctp/Kconfig @@ -9,7 +9,6 @@ menuconfig IP_SCTP select CRYPTO select CRYPTO_HMAC select CRYPTO_SHA1 - select LIBCRC32C ---help--- Stream Control Transmission Protocol diff --git a/net/sctp/Makefile b/net/sctp/Makefile index e845e45..ee206ca 100644 --- a/net/sctp/Makefile +++ b/net/sctp/Makefile @@ -5,6 +5,7 @@ obj-$(CONFIG_IP_SCTP) += sctp.o obj-$(CONFIG_INET_SCTP_DIAG) += sctp_diag.o +obj-$(CONFIG_INET) += offload.o sctp-y := sm_statetable.o sm_statefuns.o sm_sideeffect.o \ protocol.o endpointola.o associola.o \ @@ -12,7 +13,7 @@ sctp-y := sm_statetable.o sm_statefuns.o sm_sideeffect.o \ inqueue.o outqueue.o ulpqueue.o \ tsnmap.o bind_addr.o socket.o primitive.o \ output.o input.o debug.o stream.o auth.o \ - offload.o stream_sched.o stream_sched_prio.o \ + stream_sched.o stream_sched_prio.o \ stream_sched_rr.o stream_interleave.o sctp_diag-y := diag.o diff --git a/net/sctp/offload.c b/net/sctp/offload.c index 123e9f2..c61cbde 100644 --- a/net/sctp/offload.c +++ b/net/sctp/offload.c @@ -107,7 +107,7 @@ static const struct skb_checksum_ops crc32c_csum_ops = { .combine = sctp_csum_combine, }; -int __init sctp_offload_init(void) +static int __init sctp_offload_init(void) { int ret; @@ -127,3 +127,5 @@ int __init sctp_offload_init(void) out: return ret; } + +fs_initcall(sctp_offload_init); diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c index a24cde2..46d2b63 100644 --- a/net/sctp/protocol.c +++ b/net/sctp/protocol.c @@ -1479,9 +1479,6 @@ static __init int sctp_init(void) if (status) goto err_v6_add_protocol; - if (sctp_offload_init() < 0) - pr_crit("%s: Cannot add SCTP protocol offload\n", __func__); - out: return status; err_v6_add_protocol: -- 2.9.5 ^ permalink raw reply related [flat|nested] 74+ messages in thread
* [PATCH net-next 4/5] tun: Add support for SCTP checksum offload 2018-04-02 13:40 ` Vladislav Yasevich ` (6 preceding siblings ...) (?) @ 2018-04-02 13:40 ` Vladislav Yasevich -1 siblings, 0 replies; 74+ messages in thread From: Vladislav Yasevich @ 2018-04-02 13:40 UTC (permalink / raw) To: netdev; +Cc: nhorman, mst, virtualization, linux-sctp Adds a new tun offload flag to allow for SCTP checksum offload. The flag has to be set by the user and defaults to "no offload". Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> --- drivers/net/tun.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/net/tun.c b/drivers/net/tun.c index a1ba262..263bcbe 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -2719,6 +2719,11 @@ static int set_offload(struct tun_struct *tun, unsigned long arg) arg &= ~TUN_F_UFO; } + if (arg & TUN_F_SCTP_CSUM) { + features |= NETIF_F_SCTP_CRC; + arg &= ~TUN_F_SCTP_CSUM; + } + /* This gives the user a way to test for new features in future by * trying to set them. */ if (arg) -- 2.9.5 ^ permalink raw reply related [flat|nested] 74+ messages in thread
* [PATCH net-next 4/5] tun: Add support for SCTP checksum offload 2018-04-02 13:40 ` Vladislav Yasevich @ 2018-04-02 13:40 ` Vladislav Yasevich -1 siblings, 0 replies; 74+ messages in thread From: Vladislav Yasevich @ 2018-04-02 13:40 UTC (permalink / raw) To: netdev Cc: linux-sctp, virtualization, mst, jasowang, nhorman, Vladislav Yasevich Adds a new tun offload flag to allow for SCTP checksum offload. The flag has to be set by the user and defaults to "no offload". Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> --- drivers/net/tun.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/net/tun.c b/drivers/net/tun.c index a1ba262..263bcbe 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -2719,6 +2719,11 @@ static int set_offload(struct tun_struct *tun, unsigned long arg) arg &= ~TUN_F_UFO; } + if (arg & TUN_F_SCTP_CSUM) { + features |= NETIF_F_SCTP_CRC; + arg &= ~TUN_F_SCTP_CSUM; + } + /* This gives the user a way to test for new features in future by * trying to set them. */ if (arg) -- 2.9.5 ^ permalink raw reply related [flat|nested] 74+ messages in thread
* [PATCH net-next 4/5] tun: Add support for SCTP checksum offload @ 2018-04-02 13:40 ` Vladislav Yasevich 0 siblings, 0 replies; 74+ messages in thread From: Vladislav Yasevich @ 2018-04-02 13:40 UTC (permalink / raw) To: netdev Cc: linux-sctp, virtualization, mst, jasowang, nhorman, Vladislav Yasevich Adds a new tun offload flag to allow for SCTP checksum offload. The flag has to be set by the user and defaults to "no offload". Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> --- drivers/net/tun.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/net/tun.c b/drivers/net/tun.c index a1ba262..263bcbe 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -2719,6 +2719,11 @@ static int set_offload(struct tun_struct *tun, unsigned long arg) arg &= ~TUN_F_UFO; } + if (arg & TUN_F_SCTP_CSUM) { + features |= NETIF_F_SCTP_CRC; + arg &= ~TUN_F_SCTP_CSUM; + } + /* This gives the user a way to test for new features in future by * trying to set them. */ if (arg) -- 2.9.5 ^ permalink raw reply related [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 4/5] tun: Add support for SCTP checksum offload 2018-04-02 13:40 ` Vladislav Yasevich @ 2018-04-03 0:49 ` kbuild test robot -1 siblings, 0 replies; 74+ messages in thread From: kbuild test robot @ 2018-04-03 0:49 UTC (permalink / raw) To: Vladislav Yasevich Cc: kbuild-all, netdev, linux-sctp, virtualization, mst, jasowang, nhorman, Vladislav Yasevich [-- Attachment #1: Type: text/plain, Size: 2668 bytes --] Hi Vladislav, Thank you for the patch! Yet something to improve: [auto build test ERROR on net-next/master] url: https://github.com/0day-ci/linux/commits/Vladislav-Yasevich/virtio-net-Add-SCTP-checksum-offload-support/20180402-221407 config: m68k-hp300_defconfig (attached as .config) compiler: m68k-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=m68k Note: the linux-review/Vladislav-Yasevich/virtio-net-Add-SCTP-checksum-offload-support/20180402-221407 HEAD 5e0497a085e70055a1981959802173f4ff05c86b builds fine. It only hurts bisectibility. All errors (new ones prefixed by >>): drivers/net/tun.c: In function 'set_offload': >> drivers/net/tun.c:2722:12: error: 'TUN_F_SCTP_CSUM' undeclared (first use in this function); did you mean 'TUN_F_CSUM'? if (arg & TUN_F_SCTP_CSUM) { ^~~~~~~~~~~~~~~ TUN_F_CSUM drivers/net/tun.c:2722:12: note: each undeclared identifier is reported only once for each function it appears in vim +2722 drivers/net/tun.c 2696 2697 /* This is like a cut-down ethtool ops, except done via tun fd so no 2698 * privs required. */ 2699 static int set_offload(struct tun_struct *tun, unsigned long arg) 2700 { 2701 netdev_features_t features = 0; 2702 2703 if (arg & TUN_F_CSUM) { 2704 features |= NETIF_F_HW_CSUM; 2705 arg &= ~TUN_F_CSUM; 2706 2707 if (arg & (TUN_F_TSO4|TUN_F_TSO6)) { 2708 if (arg & TUN_F_TSO_ECN) { 2709 features |= NETIF_F_TSO_ECN; 2710 arg &= ~TUN_F_TSO_ECN; 2711 } 2712 if (arg & TUN_F_TSO4) 2713 features |= NETIF_F_TSO; 2714 if (arg & TUN_F_TSO6) 2715 features |= NETIF_F_TSO6; 2716 arg &= ~(TUN_F_TSO4|TUN_F_TSO6); 2717 } 2718 2719 arg &= ~TUN_F_UFO; 2720 } 2721 > 2722 if (arg & TUN_F_SCTP_CSUM) { 2723 features |= NETIF_F_SCTP_CRC; 2724 arg &= ~TUN_F_SCTP_CSUM; 2725 } 2726 2727 /* This gives the user a way to test for new features in future by 2728 * trying to set them. */ 2729 if (arg) 2730 return -EINVAL; 2731 2732 tun->set_features = features; 2733 tun->dev->wanted_features &= ~TUN_USER_FEATURES; 2734 tun->dev->wanted_features |= features; 2735 netdev_update_features(tun->dev); 2736 2737 return 0; 2738 } 2739 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 12572 bytes --] ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 4/5] tun: Add support for SCTP checksum offload @ 2018-04-03 0:49 ` kbuild test robot 0 siblings, 0 replies; 74+ messages in thread From: kbuild test robot @ 2018-04-03 0:49 UTC (permalink / raw) To: linux-sctp [-- Attachment #1: Type: text/plain, Size: 2668 bytes --] Hi Vladislav, Thank you for the patch! Yet something to improve: [auto build test ERROR on net-next/master] url: https://github.com/0day-ci/linux/commits/Vladislav-Yasevich/virtio-net-Add-SCTP-checksum-offload-support/20180402-221407 config: m68k-hp300_defconfig (attached as .config) compiler: m68k-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=m68k Note: the linux-review/Vladislav-Yasevich/virtio-net-Add-SCTP-checksum-offload-support/20180402-221407 HEAD 5e0497a085e70055a1981959802173f4ff05c86b builds fine. It only hurts bisectibility. All errors (new ones prefixed by >>): drivers/net/tun.c: In function 'set_offload': >> drivers/net/tun.c:2722:12: error: 'TUN_F_SCTP_CSUM' undeclared (first use in this function); did you mean 'TUN_F_CSUM'? if (arg & TUN_F_SCTP_CSUM) { ^~~~~~~~~~~~~~~ TUN_F_CSUM drivers/net/tun.c:2722:12: note: each undeclared identifier is reported only once for each function it appears in vim +2722 drivers/net/tun.c 2696 2697 /* This is like a cut-down ethtool ops, except done via tun fd so no 2698 * privs required. */ 2699 static int set_offload(struct tun_struct *tun, unsigned long arg) 2700 { 2701 netdev_features_t features = 0; 2702 2703 if (arg & TUN_F_CSUM) { 2704 features |= NETIF_F_HW_CSUM; 2705 arg &= ~TUN_F_CSUM; 2706 2707 if (arg & (TUN_F_TSO4|TUN_F_TSO6)) { 2708 if (arg & TUN_F_TSO_ECN) { 2709 features |= NETIF_F_TSO_ECN; 2710 arg &= ~TUN_F_TSO_ECN; 2711 } 2712 if (arg & TUN_F_TSO4) 2713 features |= NETIF_F_TSO; 2714 if (arg & TUN_F_TSO6) 2715 features |= NETIF_F_TSO6; 2716 arg &= ~(TUN_F_TSO4|TUN_F_TSO6); 2717 } 2718 2719 arg &= ~TUN_F_UFO; 2720 } 2721 > 2722 if (arg & TUN_F_SCTP_CSUM) { 2723 features |= NETIF_F_SCTP_CRC; 2724 arg &= ~TUN_F_SCTP_CSUM; 2725 } 2726 2727 /* This gives the user a way to test for new features in future by 2728 * trying to set them. */ 2729 if (arg) 2730 return -EINVAL; 2731 2732 tun->set_features = features; 2733 tun->dev->wanted_features &= ~TUN_USER_FEATURES; 2734 tun->dev->wanted_features |= features; 2735 netdev_update_features(tun->dev); 2736 2737 return 0; 2738 } 2739 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 12572 bytes --] ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 4/5] tun: Add support for SCTP checksum offload 2018-04-02 13:40 ` Vladislav Yasevich (?) (?) @ 2018-04-03 0:49 ` kbuild test robot -1 siblings, 0 replies; 74+ messages in thread From: kbuild test robot @ 2018-04-03 0:49 UTC (permalink / raw) To: Vladislav Yasevich Cc: nhorman, mst, netdev, virtualization, linux-sctp, kbuild-all [-- Attachment #1: Type: text/plain, Size: 2668 bytes --] Hi Vladislav, Thank you for the patch! Yet something to improve: [auto build test ERROR on net-next/master] url: https://github.com/0day-ci/linux/commits/Vladislav-Yasevich/virtio-net-Add-SCTP-checksum-offload-support/20180402-221407 config: m68k-hp300_defconfig (attached as .config) compiler: m68k-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=m68k Note: the linux-review/Vladislav-Yasevich/virtio-net-Add-SCTP-checksum-offload-support/20180402-221407 HEAD 5e0497a085e70055a1981959802173f4ff05c86b builds fine. It only hurts bisectibility. All errors (new ones prefixed by >>): drivers/net/tun.c: In function 'set_offload': >> drivers/net/tun.c:2722:12: error: 'TUN_F_SCTP_CSUM' undeclared (first use in this function); did you mean 'TUN_F_CSUM'? if (arg & TUN_F_SCTP_CSUM) { ^~~~~~~~~~~~~~~ TUN_F_CSUM drivers/net/tun.c:2722:12: note: each undeclared identifier is reported only once for each function it appears in vim +2722 drivers/net/tun.c 2696 2697 /* This is like a cut-down ethtool ops, except done via tun fd so no 2698 * privs required. */ 2699 static int set_offload(struct tun_struct *tun, unsigned long arg) 2700 { 2701 netdev_features_t features = 0; 2702 2703 if (arg & TUN_F_CSUM) { 2704 features |= NETIF_F_HW_CSUM; 2705 arg &= ~TUN_F_CSUM; 2706 2707 if (arg & (TUN_F_TSO4|TUN_F_TSO6)) { 2708 if (arg & TUN_F_TSO_ECN) { 2709 features |= NETIF_F_TSO_ECN; 2710 arg &= ~TUN_F_TSO_ECN; 2711 } 2712 if (arg & TUN_F_TSO4) 2713 features |= NETIF_F_TSO; 2714 if (arg & TUN_F_TSO6) 2715 features |= NETIF_F_TSO6; 2716 arg &= ~(TUN_F_TSO4|TUN_F_TSO6); 2717 } 2718 2719 arg &= ~TUN_F_UFO; 2720 } 2721 > 2722 if (arg & TUN_F_SCTP_CSUM) { 2723 features |= NETIF_F_SCTP_CRC; 2724 arg &= ~TUN_F_SCTP_CSUM; 2725 } 2726 2727 /* This gives the user a way to test for new features in future by 2728 * trying to set them. */ 2729 if (arg) 2730 return -EINVAL; 2731 2732 tun->set_features = features; 2733 tun->dev->wanted_features &= ~TUN_USER_FEATURES; 2734 tun->dev->wanted_features |= features; 2735 netdev_update_features(tun->dev); 2736 2737 return 0; 2738 } 2739 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 12572 bytes --] [-- Attachment #3: Type: text/plain, Size: 183 bytes --] _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 4/5] tun: Add support for SCTP checksum offload 2018-04-02 13:40 ` Vladislav Yasevich @ 2018-04-16 17:12 ` Michael S. Tsirkin -1 siblings, 0 replies; 74+ messages in thread From: Michael S. Tsirkin @ 2018-04-16 17:12 UTC (permalink / raw) To: Vladislav Yasevich Cc: netdev, linux-sctp, virtualization, jasowang, nhorman, Vladislav Yasevich On Mon, Apr 02, 2018 at 09:40:05AM -0400, Vladislav Yasevich wrote: > Adds a new tun offload flag to allow for SCTP checksum offload. > The flag has to be set by the user and defaults to "no offload". > > Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> When would user set this flag? Wouldn't that be when userspace is ready to get SCTP packets without a checksum? Seems to be this is an indication that when userspace is qemu running a guest, said guest needs to communicate the new ability to qemu. > --- > drivers/net/tun.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/net/tun.c b/drivers/net/tun.c > index a1ba262..263bcbe 100644 > --- a/drivers/net/tun.c > +++ b/drivers/net/tun.c > @@ -2719,6 +2719,11 @@ static int set_offload(struct tun_struct *tun, unsigned long arg) > arg &= ~TUN_F_UFO; > } > > + if (arg & TUN_F_SCTP_CSUM) { > + features |= NETIF_F_SCTP_CRC; > + arg &= ~TUN_F_SCTP_CSUM; > + } > + > /* This gives the user a way to test for new features in future by > * trying to set them. */ > if (arg) > -- > 2.9.5 ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 4/5] tun: Add support for SCTP checksum offload @ 2018-04-16 17:12 ` Michael S. Tsirkin 0 siblings, 0 replies; 74+ messages in thread From: Michael S. Tsirkin @ 2018-04-16 17:12 UTC (permalink / raw) To: Vladislav Yasevich Cc: netdev, linux-sctp, virtualization, jasowang, nhorman, Vladislav Yasevich On Mon, Apr 02, 2018 at 09:40:05AM -0400, Vladislav Yasevich wrote: > Adds a new tun offload flag to allow for SCTP checksum offload. > The flag has to be set by the user and defaults to "no offload". > > Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> When would user set this flag? Wouldn't that be when userspace is ready to get SCTP packets without a checksum? Seems to be this is an indication that when userspace is qemu running a guest, said guest needs to communicate the new ability to qemu. > --- > drivers/net/tun.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/net/tun.c b/drivers/net/tun.c > index a1ba262..263bcbe 100644 > --- a/drivers/net/tun.c > +++ b/drivers/net/tun.c > @@ -2719,6 +2719,11 @@ static int set_offload(struct tun_struct *tun, unsigned long arg) > arg &= ~TUN_F_UFO; > } > > + if (arg & TUN_F_SCTP_CSUM) { > + features |= NETIF_F_SCTP_CRC; > + arg &= ~TUN_F_SCTP_CSUM; > + } > + > /* This gives the user a way to test for new features in future by > * trying to set them. */ > if (arg) > -- > 2.9.5 ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 4/5] tun: Add support for SCTP checksum offload 2018-04-02 13:40 ` Vladislav Yasevich ` (3 preceding siblings ...) (?) @ 2018-04-16 17:12 ` Michael S. Tsirkin -1 siblings, 0 replies; 74+ messages in thread From: Michael S. Tsirkin @ 2018-04-16 17:12 UTC (permalink / raw) To: Vladislav Yasevich; +Cc: nhorman, netdev, virtualization, linux-sctp On Mon, Apr 02, 2018 at 09:40:05AM -0400, Vladislav Yasevich wrote: > Adds a new tun offload flag to allow for SCTP checksum offload. > The flag has to be set by the user and defaults to "no offload". > > Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> When would user set this flag? Wouldn't that be when userspace is ready to get SCTP packets without a checksum? Seems to be this is an indication that when userspace is qemu running a guest, said guest needs to communicate the new ability to qemu. > --- > drivers/net/tun.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/net/tun.c b/drivers/net/tun.c > index a1ba262..263bcbe 100644 > --- a/drivers/net/tun.c > +++ b/drivers/net/tun.c > @@ -2719,6 +2719,11 @@ static int set_offload(struct tun_struct *tun, unsigned long arg) > arg &= ~TUN_F_UFO; > } > > + if (arg & TUN_F_SCTP_CSUM) { > + features |= NETIF_F_SCTP_CRC; > + arg &= ~TUN_F_SCTP_CSUM; > + } > + > /* This gives the user a way to test for new features in future by > * trying to set them. */ > if (arg) > -- > 2.9.5 ^ permalink raw reply [flat|nested] 74+ messages in thread
* [PATCH net-next 5/5] macvlan/macvtap: Add support for SCTP checksum offload. 2018-04-02 13:40 ` Vladislav Yasevich @ 2018-04-02 13:40 ` Vladislav Yasevich -1 siblings, 0 replies; 74+ messages in thread From: Vladislav Yasevich @ 2018-04-02 13:40 UTC (permalink / raw) To: netdev Cc: linux-sctp, virtualization, mst, jasowang, nhorman, Vladislav Yasevich Since we now have support for software CRC32c offload, turn it on for macvlan and macvtap devices so that guests can take advantage of offload SCTP checksums to the host or host hardware. Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> --- drivers/net/macvlan.c | 5 +++-- drivers/net/tap.c | 8 +++++--- include/uapi/linux/if_tun.h | 1 + 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c index 725f4b4..646b730 100644 --- a/drivers/net/macvlan.c +++ b/drivers/net/macvlan.c @@ -834,7 +834,7 @@ static struct lock_class_key macvlan_netdev_addr_lock_key; #define ALWAYS_ON_OFFLOADS \ (NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_GSO_SOFTWARE | \ - NETIF_F_GSO_ROBUST | NETIF_F_GSO_ENCAP_ALL) + NETIF_F_GSO_ROBUST | NETIF_F_GSO_ENCAP_ALL | NETIF_F_SCTP_CRC) #define ALWAYS_ON_FEATURES (ALWAYS_ON_OFFLOADS | NETIF_F_LLTX) @@ -842,7 +842,8 @@ static struct lock_class_key macvlan_netdev_addr_lock_key; (NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_HIGHDMA | NETIF_F_FRAGLIST | \ NETIF_F_GSO | NETIF_F_TSO | NETIF_F_LRO | \ NETIF_F_TSO_ECN | NETIF_F_TSO6 | NETIF_F_GRO | NETIF_F_RXCSUM | \ - NETIF_F_HW_VLAN_CTAG_FILTER | NETIF_F_HW_VLAN_STAG_FILTER) + NETIF_F_HW_VLAN_CTAG_FILTER | NETIF_F_HW_VLAN_STAG_FILTER | \ + NETIF_F_SCTP_CRC) #define MACVLAN_STATE_MASK \ ((1<<__LINK_STATE_NOCARRIER) | (1<<__LINK_STATE_DORMANT)) diff --git a/drivers/net/tap.c b/drivers/net/tap.c index 9b6cb78..2c8512b 100644 --- a/drivers/net/tap.c +++ b/drivers/net/tap.c @@ -369,8 +369,7 @@ rx_handler_result_t tap_handle_frame(struct sk_buff **pskb) * check, we either support them all or none. */ if (skb->ip_summed == CHECKSUM_PARTIAL && - !(features & NETIF_F_CSUM_MASK) && - skb_checksum_help(skb)) + skb_csum_hwoffload_help(skb, features)) goto drop; if (ptr_ring_produce(&q->ring, skb)) goto drop; @@ -945,6 +944,9 @@ static int set_offload(struct tap_queue *q, unsigned long arg) } } + if (arg & TUN_F_SCTP_CSUM) + feature_mask |= NETIF_F_SCTP_CRC; + /* tun/tap driver inverts the usage for TSO offloads, where * setting the TSO bit means that the userspace wants to * accept TSO frames and turning it off means that user space @@ -1077,7 +1079,7 @@ static long tap_ioctl(struct file *file, unsigned int cmd, case TUNSETOFFLOAD: /* let the user check for future flags */ if (arg & ~(TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 | - TUN_F_TSO_ECN | TUN_F_UFO)) + TUN_F_TSO_ECN | TUN_F_UFO | TUN_F_SCTP_CSUM)) return -EINVAL; rtnl_lock(); diff --git a/include/uapi/linux/if_tun.h b/include/uapi/linux/if_tun.h index ee432cd..c3bb282 100644 --- a/include/uapi/linux/if_tun.h +++ b/include/uapi/linux/if_tun.h @@ -86,6 +86,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_SCTP_CSUM 0x20 /* I can handle SCTP checksum offload */ /* Protocol info prepended to the packets (when IFF_NO_PI is not set) */ #define TUN_PKT_STRIP 0x0001 -- 2.9.5 ^ permalink raw reply related [flat|nested] 74+ messages in thread
* [PATCH net-next 5/5] macvlan/macvtap: Add support for SCTP checksum offload. @ 2018-04-02 13:40 ` Vladislav Yasevich 0 siblings, 0 replies; 74+ messages in thread From: Vladislav Yasevich @ 2018-04-02 13:40 UTC (permalink / raw) To: netdev Cc: linux-sctp, virtualization, mst, jasowang, nhorman, Vladislav Yasevich Since we now have support for software CRC32c offload, turn it on for macvlan and macvtap devices so that guests can take advantage of offload SCTP checksums to the host or host hardware. Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> --- drivers/net/macvlan.c | 5 +++-- drivers/net/tap.c | 8 +++++--- include/uapi/linux/if_tun.h | 1 + 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c index 725f4b4..646b730 100644 --- a/drivers/net/macvlan.c +++ b/drivers/net/macvlan.c @@ -834,7 +834,7 @@ static struct lock_class_key macvlan_netdev_addr_lock_key; #define ALWAYS_ON_OFFLOADS \ (NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_GSO_SOFTWARE | \ - NETIF_F_GSO_ROBUST | NETIF_F_GSO_ENCAP_ALL) + NETIF_F_GSO_ROBUST | NETIF_F_GSO_ENCAP_ALL | NETIF_F_SCTP_CRC) #define ALWAYS_ON_FEATURES (ALWAYS_ON_OFFLOADS | NETIF_F_LLTX) @@ -842,7 +842,8 @@ static struct lock_class_key macvlan_netdev_addr_lock_key; (NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_HIGHDMA | NETIF_F_FRAGLIST | \ NETIF_F_GSO | NETIF_F_TSO | NETIF_F_LRO | \ NETIF_F_TSO_ECN | NETIF_F_TSO6 | NETIF_F_GRO | NETIF_F_RXCSUM | \ - NETIF_F_HW_VLAN_CTAG_FILTER | NETIF_F_HW_VLAN_STAG_FILTER) + NETIF_F_HW_VLAN_CTAG_FILTER | NETIF_F_HW_VLAN_STAG_FILTER | \ + NETIF_F_SCTP_CRC) #define MACVLAN_STATE_MASK \ ((1<<__LINK_STATE_NOCARRIER) | (1<<__LINK_STATE_DORMANT)) diff --git a/drivers/net/tap.c b/drivers/net/tap.c index 9b6cb78..2c8512b 100644 --- a/drivers/net/tap.c +++ b/drivers/net/tap.c @@ -369,8 +369,7 @@ rx_handler_result_t tap_handle_frame(struct sk_buff **pskb) * check, we either support them all or none. */ if (skb->ip_summed = CHECKSUM_PARTIAL && - !(features & NETIF_F_CSUM_MASK) && - skb_checksum_help(skb)) + skb_csum_hwoffload_help(skb, features)) goto drop; if (ptr_ring_produce(&q->ring, skb)) goto drop; @@ -945,6 +944,9 @@ static int set_offload(struct tap_queue *q, unsigned long arg) } } + if (arg & TUN_F_SCTP_CSUM) + feature_mask |= NETIF_F_SCTP_CRC; + /* tun/tap driver inverts the usage for TSO offloads, where * setting the TSO bit means that the userspace wants to * accept TSO frames and turning it off means that user space @@ -1077,7 +1079,7 @@ static long tap_ioctl(struct file *file, unsigned int cmd, case TUNSETOFFLOAD: /* let the user check for future flags */ if (arg & ~(TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 | - TUN_F_TSO_ECN | TUN_F_UFO)) + TUN_F_TSO_ECN | TUN_F_UFO | TUN_F_SCTP_CSUM)) return -EINVAL; rtnl_lock(); diff --git a/include/uapi/linux/if_tun.h b/include/uapi/linux/if_tun.h index ee432cd..c3bb282 100644 --- a/include/uapi/linux/if_tun.h +++ b/include/uapi/linux/if_tun.h @@ -86,6 +86,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_SCTP_CSUM 0x20 /* I can handle SCTP checksum offload */ /* Protocol info prepended to the packets (when IFF_NO_PI is not set) */ #define TUN_PKT_STRIP 0x0001 -- 2.9.5 ^ permalink raw reply related [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 5/5] macvlan/macvtap: Add support for SCTP checksum offload. 2018-04-02 13:40 ` Vladislav Yasevich @ 2018-04-04 15:31 ` Davide Caratti -1 siblings, 0 replies; 74+ messages in thread From: Davide Caratti @ 2018-04-04 15:31 UTC (permalink / raw) To: Vladislav Yasevich, netdev Cc: linux-sctp, virtualization, mst, jasowang, nhorman, Vladislav Yasevich On Mon, 2018-04-02 at 09:40 -0400, Vladislav Yasevich wrote: > Since we now have support for software CRC32c offload, turn it on > for macvlan and macvtap devices so that guests can take advantage > of offload SCTP checksums to the host or host hardware. > > Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> > --- > drivers/net/macvlan.c | 5 +++-- > drivers/net/tap.c | 8 +++++--- > include/uapi/linux/if_tun.h | 1 + > 3 files changed, 9 insertions(+), 5 deletions(-) > I think also ipvlan can improve its SCTP performance with a similar patch. WDYT? thanks, -- davide ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 5/5] macvlan/macvtap: Add support for SCTP checksum offload. @ 2018-04-04 15:31 ` Davide Caratti 0 siblings, 0 replies; 74+ messages in thread From: Davide Caratti @ 2018-04-04 15:31 UTC (permalink / raw) To: Vladislav Yasevich, netdev Cc: linux-sctp, virtualization, mst, jasowang, nhorman, Vladislav Yasevich On Mon, 2018-04-02 at 09:40 -0400, Vladislav Yasevich wrote: > Since we now have support for software CRC32c offload, turn it on > for macvlan and macvtap devices so that guests can take advantage > of offload SCTP checksums to the host or host hardware. > > Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> > --- > drivers/net/macvlan.c | 5 +++-- > drivers/net/tap.c | 8 +++++--- > include/uapi/linux/if_tun.h | 1 + > 3 files changed, 9 insertions(+), 5 deletions(-) > I think also ipvlan can improve its SCTP performance with a similar patch. WDYT? thanks, -- davide ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 5/5] macvlan/macvtap: Add support for SCTP checksum offload. 2018-04-02 13:40 ` Vladislav Yasevich @ 2018-04-16 17:14 ` Michael S. Tsirkin -1 siblings, 0 replies; 74+ messages in thread From: Michael S. Tsirkin @ 2018-04-16 17:14 UTC (permalink / raw) To: Vladislav Yasevich Cc: netdev, linux-sctp, virtualization, jasowang, nhorman, Vladislav Yasevich On Mon, Apr 02, 2018 at 09:40:06AM -0400, Vladislav Yasevich wrote: > Since we now have support for software CRC32c offload, turn it on > for macvlan and macvtap devices so that guests can take advantage > of offload SCTP checksums to the host or host hardware. > > Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> > --- > drivers/net/macvlan.c | 5 +++-- > drivers/net/tap.c | 8 +++++--- > include/uapi/linux/if_tun.h | 1 + > 3 files changed, 9 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c > index 725f4b4..646b730 100644 > --- a/drivers/net/macvlan.c > +++ b/drivers/net/macvlan.c > @@ -834,7 +834,7 @@ static struct lock_class_key macvlan_netdev_addr_lock_key; > > #define ALWAYS_ON_OFFLOADS \ > (NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_GSO_SOFTWARE | \ > - NETIF_F_GSO_ROBUST | NETIF_F_GSO_ENCAP_ALL) > + NETIF_F_GSO_ROBUST | NETIF_F_GSO_ENCAP_ALL | NETIF_F_SCTP_CRC) > > #define ALWAYS_ON_FEATURES (ALWAYS_ON_OFFLOADS | NETIF_F_LLTX) > > @@ -842,7 +842,8 @@ static struct lock_class_key macvlan_netdev_addr_lock_key; > (NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_HIGHDMA | NETIF_F_FRAGLIST | \ > NETIF_F_GSO | NETIF_F_TSO | NETIF_F_LRO | \ > NETIF_F_TSO_ECN | NETIF_F_TSO6 | NETIF_F_GRO | NETIF_F_RXCSUM | \ > - NETIF_F_HW_VLAN_CTAG_FILTER | NETIF_F_HW_VLAN_STAG_FILTER) > + NETIF_F_HW_VLAN_CTAG_FILTER | NETIF_F_HW_VLAN_STAG_FILTER | \ > + NETIF_F_SCTP_CRC) > > #define MACVLAN_STATE_MASK \ > ((1<<__LINK_STATE_NOCARRIER) | (1<<__LINK_STATE_DORMANT)) > diff --git a/drivers/net/tap.c b/drivers/net/tap.c > index 9b6cb78..2c8512b 100644 > --- a/drivers/net/tap.c > +++ b/drivers/net/tap.c > @@ -369,8 +369,7 @@ rx_handler_result_t tap_handle_frame(struct sk_buff **pskb) > * check, we either support them all or none. > */ > if (skb->ip_summed == CHECKSUM_PARTIAL && > - !(features & NETIF_F_CSUM_MASK) && > - skb_checksum_help(skb)) > + skb_csum_hwoffload_help(skb, features)) > goto drop; > if (ptr_ring_produce(&q->ring, skb)) > goto drop; > @@ -945,6 +944,9 @@ static int set_offload(struct tap_queue *q, unsigned long arg) > } > } > > + if (arg & TUN_F_SCTP_CSUM) > + feature_mask |= NETIF_F_SCTP_CRC; > + > /* tun/tap driver inverts the usage for TSO offloads, where > * setting the TSO bit means that the userspace wants to > * accept TSO frames and turning it off means that user space Does not the above comment apply to the new flag too? It seems that it's value should be inverted for macvtap, and isn't, here. > @@ -1077,7 +1079,7 @@ static long tap_ioctl(struct file *file, unsigned int cmd, > case TUNSETOFFLOAD: > /* let the user check for future flags */ > if (arg & ~(TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 | > - TUN_F_TSO_ECN | TUN_F_UFO)) > + TUN_F_TSO_ECN | TUN_F_UFO | TUN_F_SCTP_CSUM)) > return -EINVAL; > > rtnl_lock(); > diff --git a/include/uapi/linux/if_tun.h b/include/uapi/linux/if_tun.h > index ee432cd..c3bb282 100644 > --- a/include/uapi/linux/if_tun.h > +++ b/include/uapi/linux/if_tun.h > @@ -86,6 +86,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_SCTP_CSUM 0x20 /* I can handle SCTP checksum offload */ > > /* Protocol info prepended to the packets (when IFF_NO_PI is not set) */ > #define TUN_PKT_STRIP 0x0001 Doesn't this belong in the previous patch? > 2.9.5 ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 5/5] macvlan/macvtap: Add support for SCTP checksum offload. @ 2018-04-16 17:14 ` Michael S. Tsirkin 0 siblings, 0 replies; 74+ messages in thread From: Michael S. Tsirkin @ 2018-04-16 17:14 UTC (permalink / raw) To: Vladislav Yasevich Cc: netdev, linux-sctp, virtualization, jasowang, nhorman, Vladislav Yasevich On Mon, Apr 02, 2018 at 09:40:06AM -0400, Vladislav Yasevich wrote: > Since we now have support for software CRC32c offload, turn it on > for macvlan and macvtap devices so that guests can take advantage > of offload SCTP checksums to the host or host hardware. > > Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> > --- > drivers/net/macvlan.c | 5 +++-- > drivers/net/tap.c | 8 +++++--- > include/uapi/linux/if_tun.h | 1 + > 3 files changed, 9 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c > index 725f4b4..646b730 100644 > --- a/drivers/net/macvlan.c > +++ b/drivers/net/macvlan.c > @@ -834,7 +834,7 @@ static struct lock_class_key macvlan_netdev_addr_lock_key; > > #define ALWAYS_ON_OFFLOADS \ > (NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_GSO_SOFTWARE | \ > - NETIF_F_GSO_ROBUST | NETIF_F_GSO_ENCAP_ALL) > + NETIF_F_GSO_ROBUST | NETIF_F_GSO_ENCAP_ALL | NETIF_F_SCTP_CRC) > > #define ALWAYS_ON_FEATURES (ALWAYS_ON_OFFLOADS | NETIF_F_LLTX) > > @@ -842,7 +842,8 @@ static struct lock_class_key macvlan_netdev_addr_lock_key; > (NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_HIGHDMA | NETIF_F_FRAGLIST | \ > NETIF_F_GSO | NETIF_F_TSO | NETIF_F_LRO | \ > NETIF_F_TSO_ECN | NETIF_F_TSO6 | NETIF_F_GRO | NETIF_F_RXCSUM | \ > - NETIF_F_HW_VLAN_CTAG_FILTER | NETIF_F_HW_VLAN_STAG_FILTER) > + NETIF_F_HW_VLAN_CTAG_FILTER | NETIF_F_HW_VLAN_STAG_FILTER | \ > + NETIF_F_SCTP_CRC) > > #define MACVLAN_STATE_MASK \ > ((1<<__LINK_STATE_NOCARRIER) | (1<<__LINK_STATE_DORMANT)) > diff --git a/drivers/net/tap.c b/drivers/net/tap.c > index 9b6cb78..2c8512b 100644 > --- a/drivers/net/tap.c > +++ b/drivers/net/tap.c > @@ -369,8 +369,7 @@ rx_handler_result_t tap_handle_frame(struct sk_buff **pskb) > * check, we either support them all or none. > */ > if (skb->ip_summed = CHECKSUM_PARTIAL && > - !(features & NETIF_F_CSUM_MASK) && > - skb_checksum_help(skb)) > + skb_csum_hwoffload_help(skb, features)) > goto drop; > if (ptr_ring_produce(&q->ring, skb)) > goto drop; > @@ -945,6 +944,9 @@ static int set_offload(struct tap_queue *q, unsigned long arg) > } > } > > + if (arg & TUN_F_SCTP_CSUM) > + feature_mask |= NETIF_F_SCTP_CRC; > + > /* tun/tap driver inverts the usage for TSO offloads, where > * setting the TSO bit means that the userspace wants to > * accept TSO frames and turning it off means that user space Does not the above comment apply to the new flag too? It seems that it's value should be inverted for macvtap, and isn't, here. > @@ -1077,7 +1079,7 @@ static long tap_ioctl(struct file *file, unsigned int cmd, > case TUNSETOFFLOAD: > /* let the user check for future flags */ > if (arg & ~(TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 | > - TUN_F_TSO_ECN | TUN_F_UFO)) > + TUN_F_TSO_ECN | TUN_F_UFO | TUN_F_SCTP_CSUM)) > return -EINVAL; > > rtnl_lock(); > diff --git a/include/uapi/linux/if_tun.h b/include/uapi/linux/if_tun.h > index ee432cd..c3bb282 100644 > --- a/include/uapi/linux/if_tun.h > +++ b/include/uapi/linux/if_tun.h > @@ -86,6 +86,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_SCTP_CSUM 0x20 /* I can handle SCTP checksum offload */ > > /* Protocol info prepended to the packets (when IFF_NO_PI is not set) */ > #define TUN_PKT_STRIP 0x0001 Doesn't this belong in the previous patch? > 2.9.5 ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 5/5] macvlan/macvtap: Add support for SCTP checksum offload. 2018-04-02 13:40 ` Vladislav Yasevich ` (2 preceding siblings ...) (?) @ 2018-04-16 17:14 ` Michael S. Tsirkin -1 siblings, 0 replies; 74+ messages in thread From: Michael S. Tsirkin @ 2018-04-16 17:14 UTC (permalink / raw) To: Vladislav Yasevich; +Cc: nhorman, netdev, virtualization, linux-sctp On Mon, Apr 02, 2018 at 09:40:06AM -0400, Vladislav Yasevich wrote: > Since we now have support for software CRC32c offload, turn it on > for macvlan and macvtap devices so that guests can take advantage > of offload SCTP checksums to the host or host hardware. > > Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> > --- > drivers/net/macvlan.c | 5 +++-- > drivers/net/tap.c | 8 +++++--- > include/uapi/linux/if_tun.h | 1 + > 3 files changed, 9 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c > index 725f4b4..646b730 100644 > --- a/drivers/net/macvlan.c > +++ b/drivers/net/macvlan.c > @@ -834,7 +834,7 @@ static struct lock_class_key macvlan_netdev_addr_lock_key; > > #define ALWAYS_ON_OFFLOADS \ > (NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_GSO_SOFTWARE | \ > - NETIF_F_GSO_ROBUST | NETIF_F_GSO_ENCAP_ALL) > + NETIF_F_GSO_ROBUST | NETIF_F_GSO_ENCAP_ALL | NETIF_F_SCTP_CRC) > > #define ALWAYS_ON_FEATURES (ALWAYS_ON_OFFLOADS | NETIF_F_LLTX) > > @@ -842,7 +842,8 @@ static struct lock_class_key macvlan_netdev_addr_lock_key; > (NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_HIGHDMA | NETIF_F_FRAGLIST | \ > NETIF_F_GSO | NETIF_F_TSO | NETIF_F_LRO | \ > NETIF_F_TSO_ECN | NETIF_F_TSO6 | NETIF_F_GRO | NETIF_F_RXCSUM | \ > - NETIF_F_HW_VLAN_CTAG_FILTER | NETIF_F_HW_VLAN_STAG_FILTER) > + NETIF_F_HW_VLAN_CTAG_FILTER | NETIF_F_HW_VLAN_STAG_FILTER | \ > + NETIF_F_SCTP_CRC) > > #define MACVLAN_STATE_MASK \ > ((1<<__LINK_STATE_NOCARRIER) | (1<<__LINK_STATE_DORMANT)) > diff --git a/drivers/net/tap.c b/drivers/net/tap.c > index 9b6cb78..2c8512b 100644 > --- a/drivers/net/tap.c > +++ b/drivers/net/tap.c > @@ -369,8 +369,7 @@ rx_handler_result_t tap_handle_frame(struct sk_buff **pskb) > * check, we either support them all or none. > */ > if (skb->ip_summed == CHECKSUM_PARTIAL && > - !(features & NETIF_F_CSUM_MASK) && > - skb_checksum_help(skb)) > + skb_csum_hwoffload_help(skb, features)) > goto drop; > if (ptr_ring_produce(&q->ring, skb)) > goto drop; > @@ -945,6 +944,9 @@ static int set_offload(struct tap_queue *q, unsigned long arg) > } > } > > + if (arg & TUN_F_SCTP_CSUM) > + feature_mask |= NETIF_F_SCTP_CRC; > + > /* tun/tap driver inverts the usage for TSO offloads, where > * setting the TSO bit means that the userspace wants to > * accept TSO frames and turning it off means that user space Does not the above comment apply to the new flag too? It seems that it's value should be inverted for macvtap, and isn't, here. > @@ -1077,7 +1079,7 @@ static long tap_ioctl(struct file *file, unsigned int cmd, > case TUNSETOFFLOAD: > /* let the user check for future flags */ > if (arg & ~(TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 | > - TUN_F_TSO_ECN | TUN_F_UFO)) > + TUN_F_TSO_ECN | TUN_F_UFO | TUN_F_SCTP_CSUM)) > return -EINVAL; > > rtnl_lock(); > diff --git a/include/uapi/linux/if_tun.h b/include/uapi/linux/if_tun.h > index ee432cd..c3bb282 100644 > --- a/include/uapi/linux/if_tun.h > +++ b/include/uapi/linux/if_tun.h > @@ -86,6 +86,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_SCTP_CSUM 0x20 /* I can handle SCTP checksum offload */ > > /* Protocol info prepended to the packets (when IFF_NO_PI is not set) */ > #define TUN_PKT_STRIP 0x0001 Doesn't this belong in the previous patch? > 2.9.5 ^ permalink raw reply [flat|nested] 74+ messages in thread
* [PATCH net-next 5/5] macvlan/macvtap: Add support for SCTP checksum offload. 2018-04-02 13:40 ` Vladislav Yasevich ` (9 preceding siblings ...) (?) @ 2018-04-02 13:40 ` Vladislav Yasevich -1 siblings, 0 replies; 74+ messages in thread From: Vladislav Yasevich @ 2018-04-02 13:40 UTC (permalink / raw) To: netdev; +Cc: nhorman, mst, virtualization, linux-sctp Since we now have support for software CRC32c offload, turn it on for macvlan and macvtap devices so that guests can take advantage of offload SCTP checksums to the host or host hardware. Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> --- drivers/net/macvlan.c | 5 +++-- drivers/net/tap.c | 8 +++++--- include/uapi/linux/if_tun.h | 1 + 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c index 725f4b4..646b730 100644 --- a/drivers/net/macvlan.c +++ b/drivers/net/macvlan.c @@ -834,7 +834,7 @@ static struct lock_class_key macvlan_netdev_addr_lock_key; #define ALWAYS_ON_OFFLOADS \ (NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_GSO_SOFTWARE | \ - NETIF_F_GSO_ROBUST | NETIF_F_GSO_ENCAP_ALL) + NETIF_F_GSO_ROBUST | NETIF_F_GSO_ENCAP_ALL | NETIF_F_SCTP_CRC) #define ALWAYS_ON_FEATURES (ALWAYS_ON_OFFLOADS | NETIF_F_LLTX) @@ -842,7 +842,8 @@ static struct lock_class_key macvlan_netdev_addr_lock_key; (NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_HIGHDMA | NETIF_F_FRAGLIST | \ NETIF_F_GSO | NETIF_F_TSO | NETIF_F_LRO | \ NETIF_F_TSO_ECN | NETIF_F_TSO6 | NETIF_F_GRO | NETIF_F_RXCSUM | \ - NETIF_F_HW_VLAN_CTAG_FILTER | NETIF_F_HW_VLAN_STAG_FILTER) + NETIF_F_HW_VLAN_CTAG_FILTER | NETIF_F_HW_VLAN_STAG_FILTER | \ + NETIF_F_SCTP_CRC) #define MACVLAN_STATE_MASK \ ((1<<__LINK_STATE_NOCARRIER) | (1<<__LINK_STATE_DORMANT)) diff --git a/drivers/net/tap.c b/drivers/net/tap.c index 9b6cb78..2c8512b 100644 --- a/drivers/net/tap.c +++ b/drivers/net/tap.c @@ -369,8 +369,7 @@ rx_handler_result_t tap_handle_frame(struct sk_buff **pskb) * check, we either support them all or none. */ if (skb->ip_summed == CHECKSUM_PARTIAL && - !(features & NETIF_F_CSUM_MASK) && - skb_checksum_help(skb)) + skb_csum_hwoffload_help(skb, features)) goto drop; if (ptr_ring_produce(&q->ring, skb)) goto drop; @@ -945,6 +944,9 @@ static int set_offload(struct tap_queue *q, unsigned long arg) } } + if (arg & TUN_F_SCTP_CSUM) + feature_mask |= NETIF_F_SCTP_CRC; + /* tun/tap driver inverts the usage for TSO offloads, where * setting the TSO bit means that the userspace wants to * accept TSO frames and turning it off means that user space @@ -1077,7 +1079,7 @@ static long tap_ioctl(struct file *file, unsigned int cmd, case TUNSETOFFLOAD: /* let the user check for future flags */ if (arg & ~(TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 | - TUN_F_TSO_ECN | TUN_F_UFO)) + TUN_F_TSO_ECN | TUN_F_UFO | TUN_F_SCTP_CSUM)) return -EINVAL; rtnl_lock(); diff --git a/include/uapi/linux/if_tun.h b/include/uapi/linux/if_tun.h index ee432cd..c3bb282 100644 --- a/include/uapi/linux/if_tun.h +++ b/include/uapi/linux/if_tun.h @@ -86,6 +86,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_SCTP_CSUM 0x20 /* I can handle SCTP checksum offload */ /* Protocol info prepended to the packets (when IFF_NO_PI is not set) */ #define TUN_PKT_STRIP 0x0001 -- 2.9.5 ^ permalink raw reply related [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 0/5] virtio-net: Add SCTP checksum offload support 2018-04-02 13:40 ` Vladislav Yasevich ` (10 preceding siblings ...) (?) @ 2018-04-02 14:16 ` David Miller -1 siblings, 0 replies; 74+ messages in thread From: David Miller @ 2018-04-02 14:16 UTC (permalink / raw) To: vyasevich; +Cc: nhorman, mst, netdev, virtualization, linux-sctp From: Vladislav Yasevich <vyasevich@gmail.com> Date: Mon, 2 Apr 2018 09:40:01 -0400 > Now that we have SCTP offload capabilities in the kernel, we can add > them to virtio as well. First step is SCTP checksum. Vlad, the net-next tree is closed, please resubmit this when the merge window is over and the net-next tree opens back up. Thank you. ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 0/5] virtio-net: Add SCTP checksum offload support 2018-04-02 13:40 ` Vladislav Yasevich @ 2018-04-02 14:16 ` David Miller -1 siblings, 0 replies; 74+ messages in thread From: David Miller @ 2018-04-02 14:16 UTC (permalink / raw) To: vyasevich Cc: netdev, linux-sctp, virtualization, mst, jasowang, nhorman, vyasevic From: Vladislav Yasevich <vyasevich@gmail.com> Date: Mon, 2 Apr 2018 09:40:01 -0400 > Now that we have SCTP offload capabilities in the kernel, we can add > them to virtio as well. First step is SCTP checksum. Vlad, the net-next tree is closed, please resubmit this when the merge window is over and the net-next tree opens back up. Thank you. ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 0/5] virtio-net: Add SCTP checksum offload support @ 2018-04-02 14:16 ` David Miller 0 siblings, 0 replies; 74+ messages in thread From: David Miller @ 2018-04-02 14:16 UTC (permalink / raw) To: vyasevich Cc: netdev, linux-sctp, virtualization, mst, jasowang, nhorman, vyasevic From: Vladislav Yasevich <vyasevich@gmail.com> Date: Mon, 2 Apr 2018 09:40:01 -0400 > Now that we have SCTP offload capabilities in the kernel, we can add > them to virtio as well. First step is SCTP checksum. Vlad, the net-next tree is closed, please resubmit this when the merge window is over and the net-next tree opens back up. Thank you. ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 0/5] virtio-net: Add SCTP checksum offload support 2018-04-02 13:40 ` Vladislav Yasevich @ 2018-04-02 14:47 ` Marcelo Ricardo Leitner -1 siblings, 0 replies; 74+ messages in thread From: Marcelo Ricardo Leitner @ 2018-04-02 14:47 UTC (permalink / raw) To: Vladislav Yasevich Cc: netdev, linux-sctp, virtualization, mst, jasowang, nhorman, Vladislav Yasevich On Mon, Apr 02, 2018 at 09:40:01AM -0400, Vladislav Yasevich wrote: > Now that we have SCTP offload capabilities in the kernel, we can add > them to virtio as well. First step is SCTP checksum. Thanks. > As for GSO, the way sctp GSO is currently implemented buys us nothing > in added support to virtio. To add true GSO, would require a lot of > re-work inside of SCTP and would require extensions to the virtio > net header to carry extra sctp data. Can you please elaborate more on this? Is this because SCTP GSO relies on the gso skb format for knowing how to segment it instead of having a list of sizes? Marcelo ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 0/5] virtio-net: Add SCTP checksum offload support @ 2018-04-02 14:47 ` Marcelo Ricardo Leitner 0 siblings, 0 replies; 74+ messages in thread From: Marcelo Ricardo Leitner @ 2018-04-02 14:47 UTC (permalink / raw) To: Vladislav Yasevich Cc: netdev, linux-sctp, virtualization, mst, jasowang, nhorman, Vladislav Yasevich On Mon, Apr 02, 2018 at 09:40:01AM -0400, Vladislav Yasevich wrote: > Now that we have SCTP offload capabilities in the kernel, we can add > them to virtio as well. First step is SCTP checksum. Thanks. > As for GSO, the way sctp GSO is currently implemented buys us nothing > in added support to virtio. To add true GSO, would require a lot of > re-work inside of SCTP and would require extensions to the virtio > net header to carry extra sctp data. Can you please elaborate more on this? Is this because SCTP GSO relies on the gso skb format for knowing how to segment it instead of having a list of sizes? Marcelo ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 0/5] virtio-net: Add SCTP checksum offload support 2018-04-02 14:47 ` Marcelo Ricardo Leitner @ 2018-04-17 20:35 ` Vlad Yasevich -1 siblings, 0 replies; 74+ messages in thread From: Vlad Yasevich @ 2018-04-17 20:35 UTC (permalink / raw) To: Marcelo Ricardo Leitner, Vladislav Yasevich Cc: netdev, linux-sctp, virtualization, mst, jasowang, nhorman On 04/02/2018 10:47 AM, Marcelo Ricardo Leitner wrote: > On Mon, Apr 02, 2018 at 09:40:01AM -0400, Vladislav Yasevich wrote: >> Now that we have SCTP offload capabilities in the kernel, we can add >> them to virtio as well. First step is SCTP checksum. > > Thanks. > >> As for GSO, the way sctp GSO is currently implemented buys us nothing >> in added support to virtio. To add true GSO, would require a lot of >> re-work inside of SCTP and would require extensions to the virtio >> net header to carry extra sctp data. > > Can you please elaborate more on this? Is this because SCTP GSO relies > on the gso skb format for knowing how to segment it instead of having > a list of sizes? > it's mainly because all the true segmentation, placing data into chunks, has already happened. All that GSO does is allow for higher bundling rate between VMs. If that is all SCTP GSO ever going to do, that fine, but the goal is to do real GSO eventually and potentially reduce the amount of memory copying we are doing. If we do that, any current attempt at GSO in virtio would have to be depricated and we'd need GSO2 or something like that. This is why, after doing the GSO support, I decided not to include it. -vlad > Marcelo > ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 0/5] virtio-net: Add SCTP checksum offload support @ 2018-04-17 20:35 ` Vlad Yasevich 0 siblings, 0 replies; 74+ messages in thread From: Vlad Yasevich @ 2018-04-17 20:35 UTC (permalink / raw) To: Marcelo Ricardo Leitner, Vladislav Yasevich Cc: netdev, linux-sctp, virtualization, mst, jasowang, nhorman On 04/02/2018 10:47 AM, Marcelo Ricardo Leitner wrote: > On Mon, Apr 02, 2018 at 09:40:01AM -0400, Vladislav Yasevich wrote: >> Now that we have SCTP offload capabilities in the kernel, we can add >> them to virtio as well. First step is SCTP checksum. > > Thanks. > >> As for GSO, the way sctp GSO is currently implemented buys us nothing >> in added support to virtio. To add true GSO, would require a lot of >> re-work inside of SCTP and would require extensions to the virtio >> net header to carry extra sctp data. > > Can you please elaborate more on this? Is this because SCTP GSO relies > on the gso skb format for knowing how to segment it instead of having > a list of sizes? > it's mainly because all the true segmentation, placing data into chunks, has already happened. All that GSO does is allow for higher bundling rate between VMs. If that is all SCTP GSO ever going to do, that fine, but the goal is to do real GSO eventually and potentially reduce the amount of memory copying we are doing. If we do that, any current attempt at GSO in virtio would have to be depricated and we'd need GSO2 or something like that. This is why, after doing the GSO support, I decided not to include it. -vlad > Marcelo > ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 0/5] virtio-net: Add SCTP checksum offload support 2018-04-17 20:35 ` Vlad Yasevich @ 2018-04-18 1:33 ` Marcelo Ricardo Leitner -1 siblings, 0 replies; 74+ messages in thread From: Marcelo Ricardo Leitner @ 2018-04-18 1:33 UTC (permalink / raw) To: Vlad Yasevich Cc: Vladislav Yasevich, netdev, linux-sctp, virtualization, mst, jasowang, nhorman, Xin Long On Tue, Apr 17, 2018 at 04:35:18PM -0400, Vlad Yasevich wrote: > On 04/02/2018 10:47 AM, Marcelo Ricardo Leitner wrote: > > On Mon, Apr 02, 2018 at 09:40:01AM -0400, Vladislav Yasevich wrote: > >> Now that we have SCTP offload capabilities in the kernel, we can add > >> them to virtio as well. First step is SCTP checksum. > > > > Thanks. > > > >> As for GSO, the way sctp GSO is currently implemented buys us nothing > >> in added support to virtio. To add true GSO, would require a lot of > >> re-work inside of SCTP and would require extensions to the virtio > >> net header to carry extra sctp data. > > > > Can you please elaborate more on this? Is this because SCTP GSO relies > > on the gso skb format for knowing how to segment it instead of having > > a list of sizes? > > > > it's mainly because all the true segmentation, placing data into chunks, > has already happened. All that GSO does is allow for higher bundling > rate between VMs. If that is all SCTP GSO ever going to do, that fine, > but the goal is to do real GSO eventually and potentially reduce the > amount of memory copying we are doing. > If we do that, any current attempt at GSO in virtio would have to be > depricated and we'd need GSO2 or something like that. > > This is why, after doing the GSO support, I decided not to include it. Gotcha. I don't think it will ever go further than what we have now. Placing data into chunks later is not really feasible/wanted, especially now with stream schedulers and idata chunks. Doesn't seem worth the hassle... we would have to support things like, "segment half of this message plus a third of this other one from that other stream." (in case it's round robin). Marcelo ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 0/5] virtio-net: Add SCTP checksum offload support @ 2018-04-18 1:33 ` Marcelo Ricardo Leitner 0 siblings, 0 replies; 74+ messages in thread From: Marcelo Ricardo Leitner @ 2018-04-18 1:33 UTC (permalink / raw) To: Vlad Yasevich Cc: Vladislav Yasevich, netdev, linux-sctp, virtualization, mst, jasowang, nhorman, Xin Long On Tue, Apr 17, 2018 at 04:35:18PM -0400, Vlad Yasevich wrote: > On 04/02/2018 10:47 AM, Marcelo Ricardo Leitner wrote: > > On Mon, Apr 02, 2018 at 09:40:01AM -0400, Vladislav Yasevich wrote: > >> Now that we have SCTP offload capabilities in the kernel, we can add > >> them to virtio as well. First step is SCTP checksum. > > > > Thanks. > > > >> As for GSO, the way sctp GSO is currently implemented buys us nothing > >> in added support to virtio. To add true GSO, would require a lot of > >> re-work inside of SCTP and would require extensions to the virtio > >> net header to carry extra sctp data. > > > > Can you please elaborate more on this? Is this because SCTP GSO relies > > on the gso skb format for knowing how to segment it instead of having > > a list of sizes? > > > > it's mainly because all the true segmentation, placing data into chunks, > has already happened. All that GSO does is allow for higher bundling > rate between VMs. If that is all SCTP GSO ever going to do, that fine, > but the goal is to do real GSO eventually and potentially reduce the > amount of memory copying we are doing. > If we do that, any current attempt at GSO in virtio would have to be > depricated and we'd need GSO2 or something like that. > > This is why, after doing the GSO support, I decided not to include it. Gotcha. I don't think it will ever go further than what we have now. Placing data into chunks later is not really feasible/wanted, especially now with stream schedulers and idata chunks. Doesn't seem worth the hassle... we would have to support things like, "segment half of this message plus a third of this other one from that other stream." (in case it's round robin). Marcelo ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 0/5] virtio-net: Add SCTP checksum offload support 2018-04-18 1:33 ` Marcelo Ricardo Leitner @ 2018-04-18 6:57 ` Xin Long -1 siblings, 0 replies; 74+ messages in thread From: Xin Long @ 2018-04-18 6:57 UTC (permalink / raw) To: Marcelo Ricardo Leitner Cc: Vlad Yasevich, Vladislav Yasevich, network dev, linux-sctp, virtualization, Michael S. Tsirkin, Jason Wang, Neil Horman On Wed, Apr 18, 2018 at 9:33 AM, Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> wrote: > On Tue, Apr 17, 2018 at 04:35:18PM -0400, Vlad Yasevich wrote: >> On 04/02/2018 10:47 AM, Marcelo Ricardo Leitner wrote: >> > On Mon, Apr 02, 2018 at 09:40:01AM -0400, Vladislav Yasevich wrote: >> >> Now that we have SCTP offload capabilities in the kernel, we can add >> >> them to virtio as well. First step is SCTP checksum. >> > >> > Thanks. >> > >> >> As for GSO, the way sctp GSO is currently implemented buys us nothing >> >> in added support to virtio. To add true GSO, would require a lot of >> >> re-work inside of SCTP and would require extensions to the virtio >> >> net header to carry extra sctp data. >> > >> > Can you please elaborate more on this? Is this because SCTP GSO relies >> > on the gso skb format for knowing how to segment it instead of having >> > a list of sizes? >> > >> >> it's mainly because all the true segmentation, placing data into chunks, >> has already happened. All that GSO does is allow for higher bundling >> rate between VMs. If that is all SCTP GSO ever going to do, that fine, >> but the goal is to do real GSO eventually and potentially reduce the >> amount of memory copying we are doing. The memory copying for bundling can't be avoided, as the chunks in one packet may be from some different messages, as Marcelo said below. >> If we do that, any current attempt at GSO in virtio would have to be >> depricated and we'd need GSO2 or something like that. Why would it be depreciated? virtio_net actually also supports frag_list, all we need to do is to enable it. I already have a patch for sctp gso over virtio_net in my local tree: patch on qemu-2.9.0(el7): https://paste.fedoraproject.org/paste/IgnSbW74L6P9aUZpnWHBsA patch on kernel-4.16(linus): https://paste.fedoraproject.org/paste/L7AhFIVVp3k8VfEV29QMiw The performance has been improved quite a lot with this: testing over virtio-net(guest): https://paste.fedoraproject.org/paste/cxQMnrDQDf9AoNgBpA-twA testing over tap(host): https://paste.fedoraproject.org/paste/QPdc22pgvF-x68oKDjWa9g >> >> This is why, after doing the GSO support, I decided not to include it. > > Gotcha. I don't think it will ever go further than what we have now. > Placing data into chunks later is not really feasible/wanted, > especially now with stream schedulers and idata chunks. Doesn't seem > worth the hassle... we would have to support things like, "segment > half of this message plus a third of this other one from that other > stream." (in case it's round robin). > > Marcelo ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 0/5] virtio-net: Add SCTP checksum offload support @ 2018-04-18 6:57 ` Xin Long 0 siblings, 0 replies; 74+ messages in thread From: Xin Long @ 2018-04-18 6:57 UTC (permalink / raw) To: Marcelo Ricardo Leitner Cc: Vlad Yasevich, Vladislav Yasevich, network dev, linux-sctp, virtualization, Michael S. Tsirkin, Jason Wang, Neil Horman On Wed, Apr 18, 2018 at 9:33 AM, Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> wrote: > On Tue, Apr 17, 2018 at 04:35:18PM -0400, Vlad Yasevich wrote: >> On 04/02/2018 10:47 AM, Marcelo Ricardo Leitner wrote: >> > On Mon, Apr 02, 2018 at 09:40:01AM -0400, Vladislav Yasevich wrote: >> >> Now that we have SCTP offload capabilities in the kernel, we can add >> >> them to virtio as well. First step is SCTP checksum. >> > >> > Thanks. >> > >> >> As for GSO, the way sctp GSO is currently implemented buys us nothing >> >> in added support to virtio. To add true GSO, would require a lot of >> >> re-work inside of SCTP and would require extensions to the virtio >> >> net header to carry extra sctp data. >> > >> > Can you please elaborate more on this? Is this because SCTP GSO relies >> > on the gso skb format for knowing how to segment it instead of having >> > a list of sizes? >> > >> >> it's mainly because all the true segmentation, placing data into chunks, >> has already happened. All that GSO does is allow for higher bundling >> rate between VMs. If that is all SCTP GSO ever going to do, that fine, >> but the goal is to do real GSO eventually and potentially reduce the >> amount of memory copying we are doing. The memory copying for bundling can't be avoided, as the chunks in one packet may be from some different messages, as Marcelo said below. >> If we do that, any current attempt at GSO in virtio would have to be >> depricated and we'd need GSO2 or something like that. Why would it be depreciated? virtio_net actually also supports frag_list, all we need to do is to enable it. I already have a patch for sctp gso over virtio_net in my local tree: patch on qemu-2.9.0(el7): https://paste.fedoraproject.org/paste/IgnSbW74L6P9aUZpnWHBsA patch on kernel-4.16(linus): https://paste.fedoraproject.org/paste/L7AhFIVVp3k8VfEV29QMiw The performance has been improved quite a lot with this: testing over virtio-net(guest): https://paste.fedoraproject.org/paste/cxQMnrDQDf9AoNgBpA-twA testing over tap(host): https://paste.fedoraproject.org/paste/QPdc22pgvF-x68oKDjWa9g >> >> This is why, after doing the GSO support, I decided not to include it. > > Gotcha. I don't think it will ever go further than what we have now. > Placing data into chunks later is not really feasible/wanted, > especially now with stream schedulers and idata chunks. Doesn't seem > worth the hassle... we would have to support things like, "segment > half of this message plus a third of this other one from that other > stream." (in case it's round robin). > > Marcelo ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 0/5] virtio-net: Add SCTP checksum offload support 2018-04-18 1:33 ` Marcelo Ricardo Leitner (?) (?) @ 2018-04-18 6:57 ` Xin Long -1 siblings, 0 replies; 74+ messages in thread From: Xin Long @ 2018-04-18 6:57 UTC (permalink / raw) To: Marcelo Ricardo Leitner Cc: Neil Horman, Michael S. Tsirkin, network dev, Vladislav Yasevich, virtualization, linux-sctp On Wed, Apr 18, 2018 at 9:33 AM, Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> wrote: > On Tue, Apr 17, 2018 at 04:35:18PM -0400, Vlad Yasevich wrote: >> On 04/02/2018 10:47 AM, Marcelo Ricardo Leitner wrote: >> > On Mon, Apr 02, 2018 at 09:40:01AM -0400, Vladislav Yasevich wrote: >> >> Now that we have SCTP offload capabilities in the kernel, we can add >> >> them to virtio as well. First step is SCTP checksum. >> > >> > Thanks. >> > >> >> As for GSO, the way sctp GSO is currently implemented buys us nothing >> >> in added support to virtio. To add true GSO, would require a lot of >> >> re-work inside of SCTP and would require extensions to the virtio >> >> net header to carry extra sctp data. >> > >> > Can you please elaborate more on this? Is this because SCTP GSO relies >> > on the gso skb format for knowing how to segment it instead of having >> > a list of sizes? >> > >> >> it's mainly because all the true segmentation, placing data into chunks, >> has already happened. All that GSO does is allow for higher bundling >> rate between VMs. If that is all SCTP GSO ever going to do, that fine, >> but the goal is to do real GSO eventually and potentially reduce the >> amount of memory copying we are doing. The memory copying for bundling can't be avoided, as the chunks in one packet may be from some different messages, as Marcelo said below. >> If we do that, any current attempt at GSO in virtio would have to be >> depricated and we'd need GSO2 or something like that. Why would it be depreciated? virtio_net actually also supports frag_list, all we need to do is to enable it. I already have a patch for sctp gso over virtio_net in my local tree: patch on qemu-2.9.0(el7): https://paste.fedoraproject.org/paste/IgnSbW74L6P9aUZpnWHBsA patch on kernel-4.16(linus): https://paste.fedoraproject.org/paste/L7AhFIVVp3k8VfEV29QMiw The performance has been improved quite a lot with this: testing over virtio-net(guest): https://paste.fedoraproject.org/paste/cxQMnrDQDf9AoNgBpA-twA testing over tap(host): https://paste.fedoraproject.org/paste/QPdc22pgvF-x68oKDjWa9g >> >> This is why, after doing the GSO support, I decided not to include it. > > Gotcha. I don't think it will ever go further than what we have now. > Placing data into chunks later is not really feasible/wanted, > especially now with stream schedulers and idata chunks. Doesn't seem > worth the hassle... we would have to support things like, "segment > half of this message plus a third of this other one from that other > stream." (in case it's round robin). > > Marcelo ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 0/5] virtio-net: Add SCTP checksum offload support 2018-04-17 20:35 ` Vlad Yasevich @ 2018-04-18 14:06 ` Michael S. Tsirkin -1 siblings, 0 replies; 74+ messages in thread From: Michael S. Tsirkin @ 2018-04-18 14:06 UTC (permalink / raw) To: Vlad Yasevich Cc: Marcelo Ricardo Leitner, Vladislav Yasevich, netdev, linux-sctp, virtualization, jasowang, nhorman On Tue, Apr 17, 2018 at 04:35:18PM -0400, Vlad Yasevich wrote: > On 04/02/2018 10:47 AM, Marcelo Ricardo Leitner wrote: > > On Mon, Apr 02, 2018 at 09:40:01AM -0400, Vladislav Yasevich wrote: > >> Now that we have SCTP offload capabilities in the kernel, we can add > >> them to virtio as well. First step is SCTP checksum. > > > > Thanks. > > > >> As for GSO, the way sctp GSO is currently implemented buys us nothing > >> in added support to virtio. To add true GSO, would require a lot of > >> re-work inside of SCTP and would require extensions to the virtio > >> net header to carry extra sctp data. > > > > Can you please elaborate more on this? Is this because SCTP GSO relies > > on the gso skb format for knowing how to segment it instead of having > > a list of sizes? > > > > it's mainly because all the true segmentation, placing data into chunks, > has already happened. All that GSO does is allow for higher bundling > rate between VMs. If that is all SCTP GSO ever going to do, that fine, > but the goal is to do real GSO eventually and potentially reduce the > amount of memory copying we are doing. > If we do that, any current attempt at GSO in virtio would have to be > depricated and we'd need GSO2 or something like that. Batching helps virtualization *a lot* though. Are there actual plans for GSO2? Is it just for SCTP? > > This is why, after doing the GSO support, I decided not to include it. > > -vlad > > Marcelo > > ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 0/5] virtio-net: Add SCTP checksum offload support @ 2018-04-18 14:06 ` Michael S. Tsirkin 0 siblings, 0 replies; 74+ messages in thread From: Michael S. Tsirkin @ 2018-04-18 14:06 UTC (permalink / raw) To: Vlad Yasevich Cc: Marcelo Ricardo Leitner, Vladislav Yasevich, netdev, linux-sctp, virtualization, jasowang, nhorman On Tue, Apr 17, 2018 at 04:35:18PM -0400, Vlad Yasevich wrote: > On 04/02/2018 10:47 AM, Marcelo Ricardo Leitner wrote: > > On Mon, Apr 02, 2018 at 09:40:01AM -0400, Vladislav Yasevich wrote: > >> Now that we have SCTP offload capabilities in the kernel, we can add > >> them to virtio as well. First step is SCTP checksum. > > > > Thanks. > > > >> As for GSO, the way sctp GSO is currently implemented buys us nothing > >> in added support to virtio. To add true GSO, would require a lot of > >> re-work inside of SCTP and would require extensions to the virtio > >> net header to carry extra sctp data. > > > > Can you please elaborate more on this? Is this because SCTP GSO relies > > on the gso skb format for knowing how to segment it instead of having > > a list of sizes? > > > > it's mainly because all the true segmentation, placing data into chunks, > has already happened. All that GSO does is allow for higher bundling > rate between VMs. If that is all SCTP GSO ever going to do, that fine, > but the goal is to do real GSO eventually and potentially reduce the > amount of memory copying we are doing. > If we do that, any current attempt at GSO in virtio would have to be > depricated and we'd need GSO2 or something like that. Batching helps virtualization *a lot* though. Are there actual plans for GSO2? Is it just for SCTP? > > This is why, after doing the GSO support, I decided not to include it. > > -vlad > > Marcelo > > ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 0/5] virtio-net: Add SCTP checksum offload support 2018-04-18 14:06 ` Michael S. Tsirkin @ 2018-04-20 17:22 ` Marcelo Ricardo Leitner -1 siblings, 0 replies; 74+ messages in thread From: Marcelo Ricardo Leitner @ 2018-04-20 17:22 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Vlad Yasevich, Vladislav Yasevich, netdev, linux-sctp, virtualization, jasowang, nhorman On Wed, Apr 18, 2018 at 05:06:46PM +0300, Michael S. Tsirkin wrote: > On Tue, Apr 17, 2018 at 04:35:18PM -0400, Vlad Yasevich wrote: > > On 04/02/2018 10:47 AM, Marcelo Ricardo Leitner wrote: > > > On Mon, Apr 02, 2018 at 09:40:01AM -0400, Vladislav Yasevich wrote: > > >> Now that we have SCTP offload capabilities in the kernel, we can add > > >> them to virtio as well. First step is SCTP checksum. > > > > > > Thanks. > > > > > >> As for GSO, the way sctp GSO is currently implemented buys us nothing > > >> in added support to virtio. To add true GSO, would require a lot of > > >> re-work inside of SCTP and would require extensions to the virtio > > >> net header to carry extra sctp data. > > > > > > Can you please elaborate more on this? Is this because SCTP GSO relies > > > on the gso skb format for knowing how to segment it instead of having > > > a list of sizes? > > > > > > > it's mainly because all the true segmentation, placing data into chunks, > > has already happened. All that GSO does is allow for higher bundling > > rate between VMs. If that is all SCTP GSO ever going to do, that fine, > > but the goal is to do real GSO eventually and potentially reduce the > > amount of memory copying we are doing. > > If we do that, any current attempt at GSO in virtio would have to be > > depricated and we'd need GSO2 or something like that. > > Batching helps virtualization *a lot* though. Yep. The results posted by Xin in the other email give good insights on it. > Are there actual plans for GSO2? Is it just for SCTP? No plans. In this context, at least, yes, just for SCTP. It was a supposition in case we start doing a different GSO for SCTP, one more like what we have for TCP. Currently, as the SCTP GSO code doesn't leave the system, we can update it if we want. But by the moment we add support for it in virtio, we will have to be backwards compatible if we end up doing SCTP GSO differently. But again, I don't think such approach for SCTP GSO would be neither feasible or worth. The complexity for it, to work across stream schedules and late TSN allocation, would do more harm then good IMO. > > > > > This is why, after doing the GSO support, I decided not to include it. > > > > -vlad > > > Marcelo > > > ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 0/5] virtio-net: Add SCTP checksum offload support @ 2018-04-20 17:22 ` Marcelo Ricardo Leitner 0 siblings, 0 replies; 74+ messages in thread From: Marcelo Ricardo Leitner @ 2018-04-20 17:22 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Vlad Yasevich, Vladislav Yasevich, netdev, linux-sctp, virtualization, jasowang, nhorman On Wed, Apr 18, 2018 at 05:06:46PM +0300, Michael S. Tsirkin wrote: > On Tue, Apr 17, 2018 at 04:35:18PM -0400, Vlad Yasevich wrote: > > On 04/02/2018 10:47 AM, Marcelo Ricardo Leitner wrote: > > > On Mon, Apr 02, 2018 at 09:40:01AM -0400, Vladislav Yasevich wrote: > > >> Now that we have SCTP offload capabilities in the kernel, we can add > > >> them to virtio as well. First step is SCTP checksum. > > > > > > Thanks. > > > > > >> As for GSO, the way sctp GSO is currently implemented buys us nothing > > >> in added support to virtio. To add true GSO, would require a lot of > > >> re-work inside of SCTP and would require extensions to the virtio > > >> net header to carry extra sctp data. > > > > > > Can you please elaborate more on this? Is this because SCTP GSO relies > > > on the gso skb format for knowing how to segment it instead of having > > > a list of sizes? > > > > > > > it's mainly because all the true segmentation, placing data into chunks, > > has already happened. All that GSO does is allow for higher bundling > > rate between VMs. If that is all SCTP GSO ever going to do, that fine, > > but the goal is to do real GSO eventually and potentially reduce the > > amount of memory copying we are doing. > > If we do that, any current attempt at GSO in virtio would have to be > > depricated and we'd need GSO2 or something like that. > > Batching helps virtualization *a lot* though. Yep. The results posted by Xin in the other email give good insights on it. > Are there actual plans for GSO2? Is it just for SCTP? No plans. In this context, at least, yes, just for SCTP. It was a supposition in case we start doing a different GSO for SCTP, one more like what we have for TCP. Currently, as the SCTP GSO code doesn't leave the system, we can update it if we want. But by the moment we add support for it in virtio, we will have to be backwards compatible if we end up doing SCTP GSO differently. But again, I don't think such approach for SCTP GSO would be neither feasible or worth. The complexity for it, to work across stream schedules and late TSN allocation, would do more harm then good IMO. > > > > > This is why, after doing the GSO support, I decided not to include it. > > > > -vlad > > > Marcelo > > > ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 0/5] virtio-net: Add SCTP checksum offload support 2018-04-20 17:22 ` Marcelo Ricardo Leitner (?) @ 2018-04-20 18:32 ` Michael S. Tsirkin -1 siblings, 0 replies; 74+ messages in thread From: Michael S. Tsirkin @ 2018-04-20 18:32 UTC (permalink / raw) To: Marcelo Ricardo Leitner Cc: nhorman, netdev, Vladislav Yasevich, virtualization, linux-sctp On Fri, Apr 20, 2018 at 02:22:19PM -0300, Marcelo Ricardo Leitner wrote: > On Wed, Apr 18, 2018 at 05:06:46PM +0300, Michael S. Tsirkin wrote: > > On Tue, Apr 17, 2018 at 04:35:18PM -0400, Vlad Yasevich wrote: > > > On 04/02/2018 10:47 AM, Marcelo Ricardo Leitner wrote: > > > > On Mon, Apr 02, 2018 at 09:40:01AM -0400, Vladislav Yasevich wrote: > > > >> Now that we have SCTP offload capabilities in the kernel, we can add > > > >> them to virtio as well. First step is SCTP checksum. > > > > > > > > Thanks. > > > > > > > >> As for GSO, the way sctp GSO is currently implemented buys us nothing > > > >> in added support to virtio. To add true GSO, would require a lot of > > > >> re-work inside of SCTP and would require extensions to the virtio > > > >> net header to carry extra sctp data. > > > > > > > > Can you please elaborate more on this? Is this because SCTP GSO relies > > > > on the gso skb format for knowing how to segment it instead of having > > > > a list of sizes? > > > > > > > > > > it's mainly because all the true segmentation, placing data into chunks, > > > has already happened. All that GSO does is allow for higher bundling > > > rate between VMs. If that is all SCTP GSO ever going to do, that fine, > > > but the goal is to do real GSO eventually and potentially reduce the > > > amount of memory copying we are doing. > > > If we do that, any current attempt at GSO in virtio would have to be > > > depricated and we'd need GSO2 or something like that. > > > > Batching helps virtualization *a lot* though. > > Yep. The results posted by Xin in the other email give good insights > on it. > > > Are there actual plans for GSO2? Is it just for SCTP? > > No plans. In this context, at least, yes, just for SCTP. > > It was a supposition in case we start doing a different GSO for SCTP, > one more like what we have for TCP. > > Currently, as the SCTP GSO code doesn't leave the system, we can > update it if we want. But by the moment we add support for it in > virtio, we will have to be backwards compatible if we end up doing > SCTP GSO differently. At least for TX you can always just disable the optimization. Won't be worse than what is there now. RX is trickier - but that's GRO not GSO. > But again, I don't think such approach for SCTP GSO would be neither > feasible or worth. The complexity for it, to work across stream > schedules and late TSN allocation, would do more harm then good IMO. > > > > > > > > > This is why, after doing the GSO support, I decided not to include it. > > > > > > -vlad > > > > Marcelo > > > > ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 0/5] virtio-net: Add SCTP checksum offload support 2018-04-20 17:22 ` Marcelo Ricardo Leitner @ 2018-04-20 18:32 ` Michael S. Tsirkin -1 siblings, 0 replies; 74+ messages in thread From: Michael S. Tsirkin @ 2018-04-20 18:32 UTC (permalink / raw) To: Marcelo Ricardo Leitner Cc: Vlad Yasevich, Vladislav Yasevich, netdev, linux-sctp, virtualization, jasowang, nhorman On Fri, Apr 20, 2018 at 02:22:19PM -0300, Marcelo Ricardo Leitner wrote: > On Wed, Apr 18, 2018 at 05:06:46PM +0300, Michael S. Tsirkin wrote: > > On Tue, Apr 17, 2018 at 04:35:18PM -0400, Vlad Yasevich wrote: > > > On 04/02/2018 10:47 AM, Marcelo Ricardo Leitner wrote: > > > > On Mon, Apr 02, 2018 at 09:40:01AM -0400, Vladislav Yasevich wrote: > > > >> Now that we have SCTP offload capabilities in the kernel, we can add > > > >> them to virtio as well. First step is SCTP checksum. > > > > > > > > Thanks. > > > > > > > >> As for GSO, the way sctp GSO is currently implemented buys us nothing > > > >> in added support to virtio. To add true GSO, would require a lot of > > > >> re-work inside of SCTP and would require extensions to the virtio > > > >> net header to carry extra sctp data. > > > > > > > > Can you please elaborate more on this? Is this because SCTP GSO relies > > > > on the gso skb format for knowing how to segment it instead of having > > > > a list of sizes? > > > > > > > > > > it's mainly because all the true segmentation, placing data into chunks, > > > has already happened. All that GSO does is allow for higher bundling > > > rate between VMs. If that is all SCTP GSO ever going to do, that fine, > > > but the goal is to do real GSO eventually and potentially reduce the > > > amount of memory copying we are doing. > > > If we do that, any current attempt at GSO in virtio would have to be > > > depricated and we'd need GSO2 or something like that. > > > > Batching helps virtualization *a lot* though. > > Yep. The results posted by Xin in the other email give good insights > on it. > > > Are there actual plans for GSO2? Is it just for SCTP? > > No plans. In this context, at least, yes, just for SCTP. > > It was a supposition in case we start doing a different GSO for SCTP, > one more like what we have for TCP. > > Currently, as the SCTP GSO code doesn't leave the system, we can > update it if we want. But by the moment we add support for it in > virtio, we will have to be backwards compatible if we end up doing > SCTP GSO differently. At least for TX you can always just disable the optimization. Won't be worse than what is there now. RX is trickier - but that's GRO not GSO. > But again, I don't think such approach for SCTP GSO would be neither > feasible or worth. The complexity for it, to work across stream > schedules and late TSN allocation, would do more harm then good IMO. > > > > > > > > > This is why, after doing the GSO support, I decided not to include it. > > > > > > -vlad > > > > Marcelo > > > > ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 0/5] virtio-net: Add SCTP checksum offload support @ 2018-04-20 18:32 ` Michael S. Tsirkin 0 siblings, 0 replies; 74+ messages in thread From: Michael S. Tsirkin @ 2018-04-20 18:32 UTC (permalink / raw) To: Marcelo Ricardo Leitner Cc: Vlad Yasevich, Vladislav Yasevich, netdev, linux-sctp, virtualization, jasowang, nhorman On Fri, Apr 20, 2018 at 02:22:19PM -0300, Marcelo Ricardo Leitner wrote: > On Wed, Apr 18, 2018 at 05:06:46PM +0300, Michael S. Tsirkin wrote: > > On Tue, Apr 17, 2018 at 04:35:18PM -0400, Vlad Yasevich wrote: > > > On 04/02/2018 10:47 AM, Marcelo Ricardo Leitner wrote: > > > > On Mon, Apr 02, 2018 at 09:40:01AM -0400, Vladislav Yasevich wrote: > > > >> Now that we have SCTP offload capabilities in the kernel, we can add > > > >> them to virtio as well. First step is SCTP checksum. > > > > > > > > Thanks. > > > > > > > >> As for GSO, the way sctp GSO is currently implemented buys us nothing > > > >> in added support to virtio. To add true GSO, would require a lot of > > > >> re-work inside of SCTP and would require extensions to the virtio > > > >> net header to carry extra sctp data. > > > > > > > > Can you please elaborate more on this? Is this because SCTP GSO relies > > > > on the gso skb format for knowing how to segment it instead of having > > > > a list of sizes? > > > > > > > > > > it's mainly because all the true segmentation, placing data into chunks, > > > has already happened. All that GSO does is allow for higher bundling > > > rate between VMs. If that is all SCTP GSO ever going to do, that fine, > > > but the goal is to do real GSO eventually and potentially reduce the > > > amount of memory copying we are doing. > > > If we do that, any current attempt at GSO in virtio would have to be > > > depricated and we'd need GSO2 or something like that. > > > > Batching helps virtualization *a lot* though. > > Yep. The results posted by Xin in the other email give good insights > on it. > > > Are there actual plans for GSO2? Is it just for SCTP? > > No plans. In this context, at least, yes, just for SCTP. > > It was a supposition in case we start doing a different GSO for SCTP, > one more like what we have for TCP. > > Currently, as the SCTP GSO code doesn't leave the system, we can > update it if we want. But by the moment we add support for it in > virtio, we will have to be backwards compatible if we end up doing > SCTP GSO differently. At least for TX you can always just disable the optimization. Won't be worse than what is there now. RX is trickier - but that's GRO not GSO. > But again, I don't think such approach for SCTP GSO would be neither > feasible or worth. The complexity for it, to work across stream > schedules and late TSN allocation, would do more harm then good IMO. > > > > > > > > > This is why, after doing the GSO support, I decided not to include it. > > > > > > -vlad > > > > Marcelo > > > > ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 0/5] virtio-net: Add SCTP checksum offload support 2018-04-20 17:22 ` Marcelo Ricardo Leitner @ 2018-04-23 13:17 ` Vlad Yasevich -1 siblings, 0 replies; 74+ messages in thread From: Vlad Yasevich @ 2018-04-23 13:17 UTC (permalink / raw) To: Marcelo Ricardo Leitner, Michael S. Tsirkin Cc: Vladislav Yasevich, netdev, linux-sctp, virtualization, jasowang, nhorman On 04/20/2018 01:22 PM, Marcelo Ricardo Leitner wrote: > On Wed, Apr 18, 2018 at 05:06:46PM +0300, Michael S. Tsirkin wrote: >> On Tue, Apr 17, 2018 at 04:35:18PM -0400, Vlad Yasevich wrote: >>> On 04/02/2018 10:47 AM, Marcelo Ricardo Leitner wrote: >>>> On Mon, Apr 02, 2018 at 09:40:01AM -0400, Vladislav Yasevich wrote: >>>>> Now that we have SCTP offload capabilities in the kernel, we can add >>>>> them to virtio as well. First step is SCTP checksum. >>>> >>>> Thanks. >>>> >>>>> As for GSO, the way sctp GSO is currently implemented buys us nothing >>>>> in added support to virtio. To add true GSO, would require a lot of >>>>> re-work inside of SCTP and would require extensions to the virtio >>>>> net header to carry extra sctp data. >>>> >>>> Can you please elaborate more on this? Is this because SCTP GSO relies >>>> on the gso skb format for knowing how to segment it instead of having >>>> a list of sizes? >>>> >>> >>> it's mainly because all the true segmentation, placing data into chunks, >>> has already happened. All that GSO does is allow for higher bundling >>> rate between VMs. If that is all SCTP GSO ever going to do, that fine, >>> but the goal is to do real GSO eventually and potentially reduce the >>> amount of memory copying we are doing. >>> If we do that, any current attempt at GSO in virtio would have to be >>> depricated and we'd need GSO2 or something like that. >> >> Batching helps virtualization *a lot* though. > > Yep. The results posted by Xin in the other email give good insights > on it. > >> Are there actual plans for GSO2? Is it just for SCTP? > > No plans. In this context, at least, yes, just for SCTP. > > It was a supposition in case we start doing a different GSO for SCTP, > one more like what we have for TCP. > > Currently, as the SCTP GSO code doesn't leave the system, we can > update it if we want. But by the moment we add support for it in > virtio, we will have to be backwards compatible if we end up doing > SCTP GSO differently. So, just because the linux code doesn't do it differently doesn't mean that someone else doesn't. Since the device has to work across different possible implementations, it needs to be generic enough. If we simply document the current linux practice, that may not be ideal on the future. I was hesitant to introduce this without studying the feasibility of doing late segmentation. -vlad > > But again, I don't think such approach for SCTP GSO would be neither > feasible or worth. The complexity for it, to work across stream > schedules and late TSN allocation, would do more harm then good IMO. > >> >>> >>> This is why, after doing the GSO support, I decided not to include it. >>> >>> -vlad >>>> Marcelo >>>> ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 0/5] virtio-net: Add SCTP checksum offload support @ 2018-04-23 13:17 ` Vlad Yasevich 0 siblings, 0 replies; 74+ messages in thread From: Vlad Yasevich @ 2018-04-23 13:17 UTC (permalink / raw) To: Marcelo Ricardo Leitner, Michael S. Tsirkin Cc: Vladislav Yasevich, netdev, linux-sctp, virtualization, jasowang, nhorman On 04/20/2018 01:22 PM, Marcelo Ricardo Leitner wrote: > On Wed, Apr 18, 2018 at 05:06:46PM +0300, Michael S. Tsirkin wrote: >> On Tue, Apr 17, 2018 at 04:35:18PM -0400, Vlad Yasevich wrote: >>> On 04/02/2018 10:47 AM, Marcelo Ricardo Leitner wrote: >>>> On Mon, Apr 02, 2018 at 09:40:01AM -0400, Vladislav Yasevich wrote: >>>>> Now that we have SCTP offload capabilities in the kernel, we can add >>>>> them to virtio as well. First step is SCTP checksum. >>>> >>>> Thanks. >>>> >>>>> As for GSO, the way sctp GSO is currently implemented buys us nothing >>>>> in added support to virtio. To add true GSO, would require a lot of >>>>> re-work inside of SCTP and would require extensions to the virtio >>>>> net header to carry extra sctp data. >>>> >>>> Can you please elaborate more on this? Is this because SCTP GSO relies >>>> on the gso skb format for knowing how to segment it instead of having >>>> a list of sizes? >>>> >>> >>> it's mainly because all the true segmentation, placing data into chunks, >>> has already happened. All that GSO does is allow for higher bundling >>> rate between VMs. If that is all SCTP GSO ever going to do, that fine, >>> but the goal is to do real GSO eventually and potentially reduce the >>> amount of memory copying we are doing. >>> If we do that, any current attempt at GSO in virtio would have to be >>> depricated and we'd need GSO2 or something like that. >> >> Batching helps virtualization *a lot* though. > > Yep. The results posted by Xin in the other email give good insights > on it. > >> Are there actual plans for GSO2? Is it just for SCTP? > > No plans. In this context, at least, yes, just for SCTP. > > It was a supposition in case we start doing a different GSO for SCTP, > one more like what we have for TCP. > > Currently, as the SCTP GSO code doesn't leave the system, we can > update it if we want. But by the moment we add support for it in > virtio, we will have to be backwards compatible if we end up doing > SCTP GSO differently. So, just because the linux code doesn't do it differently doesn't mean that someone else doesn't. Since the device has to work across different possible implementations, it needs to be generic enough. If we simply document the current linux practice, that may not be ideal on the future. I was hesitant to introduce this without studying the feasibility of doing late segmentation. -vlad > > But again, I don't think such approach for SCTP GSO would be neither > feasible or worth. The complexity for it, to work across stream > schedules and late TSN allocation, would do more harm then good IMO. > >> >>> >>> This is why, after doing the GSO support, I decided not to include it. >>> >>> -vlad >>>> Marcelo >>>> ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 0/5] virtio-net: Add SCTP checksum offload support 2018-04-20 17:22 ` Marcelo Ricardo Leitner ` (3 preceding siblings ...) (?) @ 2018-04-23 13:17 ` Vlad Yasevich -1 siblings, 0 replies; 74+ messages in thread From: Vlad Yasevich @ 2018-04-23 13:17 UTC (permalink / raw) To: Marcelo Ricardo Leitner, Michael S. Tsirkin Cc: nhorman, netdev, virtualization, linux-sctp, Vladislav Yasevich On 04/20/2018 01:22 PM, Marcelo Ricardo Leitner wrote: > On Wed, Apr 18, 2018 at 05:06:46PM +0300, Michael S. Tsirkin wrote: >> On Tue, Apr 17, 2018 at 04:35:18PM -0400, Vlad Yasevich wrote: >>> On 04/02/2018 10:47 AM, Marcelo Ricardo Leitner wrote: >>>> On Mon, Apr 02, 2018 at 09:40:01AM -0400, Vladislav Yasevich wrote: >>>>> Now that we have SCTP offload capabilities in the kernel, we can add >>>>> them to virtio as well. First step is SCTP checksum. >>>> >>>> Thanks. >>>> >>>>> As for GSO, the way sctp GSO is currently implemented buys us nothing >>>>> in added support to virtio. To add true GSO, would require a lot of >>>>> re-work inside of SCTP and would require extensions to the virtio >>>>> net header to carry extra sctp data. >>>> >>>> Can you please elaborate more on this? Is this because SCTP GSO relies >>>> on the gso skb format for knowing how to segment it instead of having >>>> a list of sizes? >>>> >>> >>> it's mainly because all the true segmentation, placing data into chunks, >>> has already happened. All that GSO does is allow for higher bundling >>> rate between VMs. If that is all SCTP GSO ever going to do, that fine, >>> but the goal is to do real GSO eventually and potentially reduce the >>> amount of memory copying we are doing. >>> If we do that, any current attempt at GSO in virtio would have to be >>> depricated and we'd need GSO2 or something like that. >> >> Batching helps virtualization *a lot* though. > > Yep. The results posted by Xin in the other email give good insights > on it. > >> Are there actual plans for GSO2? Is it just for SCTP? > > No plans. In this context, at least, yes, just for SCTP. > > It was a supposition in case we start doing a different GSO for SCTP, > one more like what we have for TCP. > > Currently, as the SCTP GSO code doesn't leave the system, we can > update it if we want. But by the moment we add support for it in > virtio, we will have to be backwards compatible if we end up doing > SCTP GSO differently. So, just because the linux code doesn't do it differently doesn't mean that someone else doesn't. Since the device has to work across different possible implementations, it needs to be generic enough. If we simply document the current linux practice, that may not be ideal on the future. I was hesitant to introduce this without studying the feasibility of doing late segmentation. -vlad > > But again, I don't think such approach for SCTP GSO would be neither > feasible or worth. The complexity for it, to work across stream > schedules and late TSN allocation, would do more harm then good IMO. > >> >>> >>> This is why, after doing the GSO support, I decided not to include it. >>> >>> -vlad >>>> Marcelo >>>> ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 0/5] virtio-net: Add SCTP checksum offload support 2018-04-17 20:35 ` Vlad Yasevich ` (2 preceding siblings ...) (?) @ 2018-04-18 14:06 ` Michael S. Tsirkin -1 siblings, 0 replies; 74+ messages in thread From: Michael S. Tsirkin @ 2018-04-18 14:06 UTC (permalink / raw) To: Vlad Yasevich Cc: Marcelo Ricardo Leitner, nhorman, netdev, virtualization, linux-sctp, Vladislav Yasevich On Tue, Apr 17, 2018 at 04:35:18PM -0400, Vlad Yasevich wrote: > On 04/02/2018 10:47 AM, Marcelo Ricardo Leitner wrote: > > On Mon, Apr 02, 2018 at 09:40:01AM -0400, Vladislav Yasevich wrote: > >> Now that we have SCTP offload capabilities in the kernel, we can add > >> them to virtio as well. First step is SCTP checksum. > > > > Thanks. > > > >> As for GSO, the way sctp GSO is currently implemented buys us nothing > >> in added support to virtio. To add true GSO, would require a lot of > >> re-work inside of SCTP and would require extensions to the virtio > >> net header to carry extra sctp data. > > > > Can you please elaborate more on this? Is this because SCTP GSO relies > > on the gso skb format for knowing how to segment it instead of having > > a list of sizes? > > > > it's mainly because all the true segmentation, placing data into chunks, > has already happened. All that GSO does is allow for higher bundling > rate between VMs. If that is all SCTP GSO ever going to do, that fine, > but the goal is to do real GSO eventually and potentially reduce the > amount of memory copying we are doing. > If we do that, any current attempt at GSO in virtio would have to be > depricated and we'd need GSO2 or something like that. Batching helps virtualization *a lot* though. Are there actual plans for GSO2? Is it just for SCTP? > > This is why, after doing the GSO support, I decided not to include it. > > -vlad > > Marcelo > > ^ permalink raw reply [flat|nested] 74+ messages in thread
* Re: [PATCH net-next 0/5] virtio-net: Add SCTP checksum offload support 2018-04-02 14:47 ` Marcelo Ricardo Leitner (?) (?) @ 2018-04-17 20:35 ` Vlad Yasevich -1 siblings, 0 replies; 74+ messages in thread From: Vlad Yasevich @ 2018-04-17 20:35 UTC (permalink / raw) To: Marcelo Ricardo Leitner, Vladislav Yasevich Cc: nhorman, mst, netdev, virtualization, linux-sctp On 04/02/2018 10:47 AM, Marcelo Ricardo Leitner wrote: > On Mon, Apr 02, 2018 at 09:40:01AM -0400, Vladislav Yasevich wrote: >> Now that we have SCTP offload capabilities in the kernel, we can add >> them to virtio as well. First step is SCTP checksum. > > Thanks. > >> As for GSO, the way sctp GSO is currently implemented buys us nothing >> in added support to virtio. To add true GSO, would require a lot of >> re-work inside of SCTP and would require extensions to the virtio >> net header to carry extra sctp data. > > Can you please elaborate more on this? Is this because SCTP GSO relies > on the gso skb format for knowing how to segment it instead of having > a list of sizes? > it's mainly because all the true segmentation, placing data into chunks, has already happened. All that GSO does is allow for higher bundling rate between VMs. If that is all SCTP GSO ever going to do, that fine, but the goal is to do real GSO eventually and potentially reduce the amount of memory copying we are doing. If we do that, any current attempt at GSO in virtio would have to be depricated and we'd need GSO2 or something like that. This is why, after doing the GSO support, I decided not to include it. -vlad > Marcelo > ^ permalink raw reply [flat|nested] 74+ messages in thread
end of thread, other threads:[~2018-04-23 13:17 UTC | newest] Thread overview: 74+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-04-02 13:40 [PATCH net-next 0/5] virtio-net: Add SCTP checksum offload support Vladislav Yasevich 2018-04-02 13:40 ` Vladislav Yasevich 2018-04-02 13:40 ` [PATCH net-next 1/5] virtio: Add support for SCTP checksum offloading Vladislav Yasevich 2018-04-02 13:40 ` Vladislav Yasevich 2018-04-10 3:17 ` Jason Wang 2018-04-10 3:17 ` Jason Wang 2018-04-10 3:17 ` Jason Wang 2018-04-11 22:39 ` Marcelo Ricardo Leitner 2018-04-11 22:39 ` Marcelo Ricardo Leitner 2018-04-11 22:49 ` Michael S. Tsirkin 2018-04-11 22:49 ` Michael S. Tsirkin 2018-04-16 13:45 ` Vlad Yasevich 2018-04-16 13:45 ` Vlad Yasevich 2018-04-16 15:52 ` Michael S. Tsirkin 2018-04-16 15:52 ` Michael S. Tsirkin 2018-04-16 17:09 ` Michael S. Tsirkin 2018-04-16 17:09 ` Michael S. Tsirkin 2018-04-17 19:06 ` Vlad Yasevich 2018-04-17 19:06 ` Vlad Yasevich 2018-04-17 19:06 ` Vlad Yasevich 2018-04-16 17:09 ` Michael S. Tsirkin 2018-04-16 13:45 ` Vlad Yasevich 2018-04-11 22:49 ` Michael S. Tsirkin 2018-04-16 17:07 ` Michael S. Tsirkin 2018-04-16 17:07 ` Michael S. Tsirkin 2018-04-16 17:07 ` Michael S. Tsirkin 2018-04-02 13:40 ` Vladislav Yasevich 2018-04-02 13:40 ` [PATCH net-next 2/5] sctp: Handle sctp packets with CHECKSUM_PARTIAL Vladislav Yasevich 2018-04-02 13:40 ` Vladislav Yasevich 2018-04-02 13:40 ` Vladislav Yasevich 2018-04-02 13:40 ` [PATCH net-next 3/5] sctp: Build sctp offload support into the base kernel Vladislav Yasevich 2018-04-02 13:40 ` Vladislav Yasevich 2018-04-02 13:40 ` Vladislav Yasevich 2018-04-02 13:40 ` [PATCH net-next 4/5] tun: Add support for SCTP checksum offload Vladislav Yasevich 2018-04-02 13:40 ` Vladislav Yasevich 2018-04-02 13:40 ` Vladislav Yasevich 2018-04-03 0:49 ` kbuild test robot 2018-04-03 0:49 ` kbuild test robot 2018-04-03 0:49 ` kbuild test robot 2018-04-16 17:12 ` Michael S. Tsirkin 2018-04-16 17:12 ` Michael S. Tsirkin 2018-04-16 17:12 ` Michael S. Tsirkin 2018-04-02 13:40 ` [PATCH net-next 5/5] macvlan/macvtap: " Vladislav Yasevich 2018-04-02 13:40 ` Vladislav Yasevich 2018-04-04 15:31 ` Davide Caratti 2018-04-04 15:31 ` Davide Caratti 2018-04-16 17:14 ` Michael S. Tsirkin 2018-04-16 17:14 ` Michael S. Tsirkin 2018-04-16 17:14 ` Michael S. Tsirkin 2018-04-02 13:40 ` Vladislav Yasevich 2018-04-02 14:16 ` [PATCH net-next 0/5] virtio-net: Add SCTP checksum offload support David Miller 2018-04-02 14:16 ` David Miller 2018-04-02 14:16 ` David Miller 2018-04-02 14:47 ` Marcelo Ricardo Leitner 2018-04-02 14:47 ` Marcelo Ricardo Leitner 2018-04-17 20:35 ` Vlad Yasevich 2018-04-17 20:35 ` Vlad Yasevich 2018-04-18 1:33 ` Marcelo Ricardo Leitner 2018-04-18 1:33 ` Marcelo Ricardo Leitner 2018-04-18 6:57 ` Xin Long 2018-04-18 6:57 ` Xin Long 2018-04-18 6:57 ` Xin Long 2018-04-18 14:06 ` Michael S. Tsirkin 2018-04-18 14:06 ` Michael S. Tsirkin 2018-04-20 17:22 ` Marcelo Ricardo Leitner 2018-04-20 17:22 ` Marcelo Ricardo Leitner 2018-04-20 18:32 ` Michael S. Tsirkin 2018-04-20 18:32 ` Michael S. Tsirkin 2018-04-20 18:32 ` Michael S. Tsirkin 2018-04-23 13:17 ` Vlad Yasevich 2018-04-23 13:17 ` Vlad Yasevich 2018-04-23 13:17 ` Vlad Yasevich 2018-04-18 14:06 ` Michael S. Tsirkin 2018-04-17 20:35 ` Vlad Yasevich
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.