On Wed, Feb 12, 2020 at 04:26:46PM +0100, Ilya Maximets wrote:
> On 2/10/20 8:18 PM, Flavio Leitner wrote:
> > Disable ECN and UFO since this is not supported yet. Also, disable
> > all other features when userspace_tso is not enabled.
>
> Disabling UFO saves us from receiving of a big unfragmented
> UDP packets, but UDP packets will still have bad checksums
> and will be dropped by the kernel on receive side.
> So, this is only part of a fix.
Yes, this patch is not intended to fix the UDP checksum problem.
Unless you see a way to do that with changing the flags below.
> > Fixes: 29cf9c1b3b9c ("userspace: Add TCP Segmentation Offload support")
> > Signed-off-by: Flavio Leitner <[email protected]>
> > ---
> > lib/netdev-dpdk.c | 25 ++++++++++++++++---------
> > 1 file changed, 16 insertions(+), 9 deletions(-)
> >
> >
> > This should go to branch-2.13 if passes the review.
> >
> >
> > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> > index 6187129c0..b3983b312 100644
> > --- a/lib/netdev-dpdk.c
> > +++ b/lib/netdev-dpdk.c
> > @@ -5186,6 +5186,7 @@ netdev_dpdk_vhost_client_reconfigure(struct netdev
> > *netdev)
> > struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
> > int err;
> > uint64_t vhost_flags = 0;
> > + uint64_t vhost_unsup_flags;
> > bool zc_enabled;
> >
> > ovs_mutex_lock(&dev->mutex);
> > @@ -5252,16 +5253,22 @@ netdev_dpdk_vhost_client_reconfigure(struct netdev
> > *netdev)
> > netdev->ol_flags |= NETDEV_TX_OFFLOAD_TCP_TSO;
> > netdev->ol_flags |= NETDEV_TX_OFFLOAD_TCP_CKSUM;
> > netdev->ol_flags |= NETDEV_TX_OFFLOAD_IPV4_CKSUM;
> > + vhost_unsup_flags = 1ULL << VIRTIO_NET_F_HOST_ECN
> > + | 1ULL << VIRTIO_NET_F_HOST_UFO;
> > } else {
> > - err = rte_vhost_driver_disable_features(dev->vhost_id,
> > - 1ULL << VIRTIO_NET_F_HOST_TSO4
> > - | 1ULL << VIRTIO_NET_F_HOST_TSO6
> > - | 1ULL << VIRTIO_NET_F_CSUM);
> > - if (err) {
> > - VLOG_ERR("rte_vhost_driver_disable_features failed for "
> > - "vhost user client port: %s\n", dev->up.name);
> > - goto unlock;
> > - }
> > + vhost_unsup_flags = 1ULL << VIRTIO_NET_F_CSUM
> > + | 1ULL << VIRTIO_NET_F_HOST_UFO
> > + | 1ULL << VIRTIO_NET_F_HOST_TSO4
> > + | 1ULL << VIRTIO_NET_F_HOST_TSO6
> > + | 1ULL << VIRTIO_NET_F_HOST_ECN;
>
> All above features depends on VIRTIO_NET_F_CSUM, so, I think, we only
> need to disable this one.
You right. I added the others flags anyways to make it explicit. I thought
either a comment or code would be needed. Since I see people grep'ing
for the flags, there you go.
fbl
>
> > + }
> > +
> > + err = rte_vhost_driver_disable_features(dev->vhost_id,
> > + vhost_unsup_flags);
> > + if (err) {
> > + VLOG_ERR("rte_vhost_driver_disable_features failed for "
> > + "vhost user client port: %s\n", dev->up.name);
> > + goto unlock;
> > }
> >
> > err = rte_vhost_driver_start(dev->vhost_id);
> >
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
--
fbl
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
