Hello, Maxime, Ilya, your opinion will probably help.
On Wed, Jul 6, 2022 at 10:59 PM David Marchand <david.march...@redhat.com> wrote: > > @@ -5088,19 +5117,22 @@ netdev_dpdk_vhost_client_reconfigure(struct netdev > > *netdev) > > goto unlock; > > } > > > > + vhost_unsup_flags = 1ULL << VIRTIO_NET_F_HOST_ECN > > + | 1ULL << VIRTIO_NET_F_HOST_UFO; > > + > > + dev->hw_ol_features |= NETDEV_TX_IPV4_CKSUM_OFFLOAD; > > + dev->hw_ol_features |= NETDEV_TX_TCP_CKSUM_OFFLOAD; > > + dev->hw_ol_features |= NETDEV_TX_UDP_CKSUM_OFFLOAD; > > + dev->hw_ol_features |= NETDEV_TX_SCTP_CKSUM_OFFLOAD; > > I think we have problems with Tx vs Rx features. > > TX checksum offloads, from a vhost port point of view, should be > conditionned to what the guest announces it supports. > Afaiu, the guest accepting to receive partial tcp cheksums and other > is mapped to the VIRTIO_NET_F_GUEST_CSUM (1) feature. On this topic, let me add details on how I think we need to fix this. - First of all, exposing NETDEV_TX_IPV4_CKSUM_OFFLOAD is ok. virtio protocol does not support offloadnig ipv4 checksum. But, on the other hand, the DPDK API requests that TSO hw support comes with ipv4 hw checksum. For this reason, the vhost library has been handling the ipv4 checksum in sw: https://git.dpdk.org/dpdk-stable/tree/lib/vhost/virtio_net.c?h=v21.11.1#n441 And OVS can rely on this. - As for the rest of the capabilities, OVS must be able to support different scenarii. The virtio driver rx capabilities may change as a guest driver can be reinitialised or changed to a different driver that supports none of those offloads (like net/virtio + testpmd io mode). As a result, the vhost netdev checksum tx capabilities might change. So I think we need to update the netdev hw ol features (and rely on OVS sw fallback code) with a diff like: diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 3e7b86009..1c69150ea 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -4002,6 +4002,37 @@ new_device(int vid) dev->vhost_reconfigured = true; } + uint64_t features; + + if (rte_vhost_get_negotiated_features(vid, &features)) { + VLOG_INFO("Error checking guest features for " + "vHost Device '%s'", dev->vhost_id); + } else { + if (features & (1ULL << VIRTIO_NET_F_GUEST_CSUM)) { + dev->hw_ol_features |= NETDEV_TX_TCP_CKSUM_OFFLOAD; + dev->hw_ol_features |= NETDEV_TX_UDP_CKSUM_OFFLOAD; + dev->hw_ol_features |= NETDEV_TX_SCTP_CKSUM_OFFLOAD; + } + + if (userspace_tso_enabled()) { + if (features & (1ULL << VIRTIO_NET_F_GUEST_TSO4) + && features & (1ULL << VIRTIO_NET_F_GUEST_TSO6)) { + dev->hw_ol_features |= NETDEV_TX_TSO_OFFLOAD; + VLOG_DBG("%s: TSO enabled on vhost port", + netdev_get_name(&dev->up)); + } else { + VLOG_WARN("%s: Tx TSO offload is not supported.", + netdev_get_name(&dev->up)); + } + } + } + + /* There is no support in virtio net to offload IPv4 csum, + * but the vhost library handles IPv4 csum offloading fine. */ + dev->hw_ol_features |= NETDEV_TX_IPV4_CKSUM_OFFLOAD; + + netdev_dpdk_update_netdev_flags(dev); + ovsrcu_index_set(&dev->vid, vid); exists = true; @@ -4065,6 +4096,14 @@ destroy_device(int vid) dev->up.n_rxq * sizeof *dev->vhost_rxq_enabled); netdev_dpdk_txq_map_clear(dev); + /* Clear offload capabilities before next new_device. */ + dev->hw_ol_features &= ~NETDEV_TX_IPV4_CKSUM_OFFLOAD; + dev->hw_ol_features &= ~NETDEV_TX_TCP_CKSUM_OFFLOAD; + dev->hw_ol_features &= ~NETDEV_TX_UDP_CKSUM_OFFLOAD; + dev->hw_ol_features &= ~NETDEV_TX_SCTP_CKSUM_OFFLOAD; + dev->hw_ol_features &= ~NETDEV_TX_TSO_OFFLOAD; + netdev_dpdk_update_netdev_flags(dev); + netdev_change_seq_changed(&dev->up); ovs_mutex_unlock(&dev->mutex); exists = true; @@ -5045,11 +5084,6 @@ dpdk_vhost_reconfigure_helper(struct netdev_dpdk *dev) dev->tx_q[0].map = 0; } - if (userspace_tso_enabled()) { - dev->hw_ol_features |= NETDEV_TX_TSO_OFFLOAD; - VLOG_DBG("%s: TSO enabled on vhost port", netdev_get_name(&dev->up)); - } - netdev_dpdk_remap_txqs(dev); err = netdev_dpdk_mempool_configure(dev); -- David Marchand _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
