[ovs-dev] [PATCH v4 3/3] netdev-dpdk: Add TCP Segmentation Offload support

William Tu u9012063 at gmail.com
Fri Jan 17 20:37:56 UTC 2020


On Fri, Jan 17, 2020 at 04:58:57PM -0300, Flavio Leitner wrote:
> On Fri, Jan 17, 2020 at 06:58:56PM +0100, Ilya Maximets wrote:
> > On 16.01.2020 18:00, Flavio Leitner wrote:
> > > Abbreviated as TSO, TCP Segmentation Offload is a feature which enables
> > > the network stack to delegate the TCP segmentation to the NIC reducing
> > > the per packet CPU overhead.
> > > 
> > > A guest using vhostuser interface with TSO enabled can send TCP packets
> > > much bigger than the MTU, which saves CPU cycles normally used to break
> > > the packets down to MTU size and to calculate checksums.
> > > 
> > > It also saves CPU cycles used to parse multiple packets/headers during
> > > the packet processing inside virtual switch.
> > > 
> > > If the destination of the packet is another guest in the same host, then
> > > the same big packet can be sent through a vhostuser interface skipping
> > > the segmentation completely. However, if the destination is not local,
> > > the NIC hardware is instructed to do the TCP segmentation and checksum
> > > calculation.
> > > 
> > > It is recommended to check if NIC hardware supports TSO before enabling
> > > the feature, which is off by default. For additional information please
> > > check the tso.rst document.
> > > 
> > > Signed-off-by: Flavio Leitner <fbl at sysclose.org>
> > 
> > 
> > I still didn't check computation of offsets and device configuration.
> > Few comments inline.
> > 
> > In general, I'd like if Ben will take a look to this patch, especially
> > at extensive netdev-linux code changes.
> > 
> > Also, suggesting to rename the patch to:
> > 'userspace: Add TCP Segmentation Offload support.'
> 
> Will do a V5 with all the minor fixes listed below.
> 
> The ones that you suggested to do in another patch I also agree
> and left as is.
> 
> [...]
> > > @@ -1454,9 +1595,15 @@ netdev_linux_send(struct netdev *netdev_, int qid OVS_UNUSED,
> > >                    struct dp_packet_batch *batch,
> > >                    bool concurrent_txq OVS_UNUSED)
> > >  {
> > > +    bool tso = userspace_tso_enabled();
> > > +    int mtu = ETH_PAYLOAD_MAX;
> > >      int error = 0;
> > >      int sock = 0;
> > >  
> > > +    if (tso) {
> > > +        netdev_linux_get_mtu__(netdev_linux_cast(netdev_), &mtu);
> > 
> > netdev_linux_get_mtu__() could fail.  This needs to be handled.
> 
> If that fails, the mtu is not changed, so we use the ETH_PAYLOAD_MAX
> as a fall back.
> 
> [...]
> > > @@ -6234,3 +6393,136 @@ af_packet_sock(void)
> > >  
> > >      return sock;
> > >  }
> > > +
> > > +static int
> > > +netdev_linux_parse_l2(struct dp_packet *b, uint16_t *l4proto)
> > 
> > I'm not generally a fan of parsing packets here on receive especially
> > because we will parse it with miniflow_extract() later, but I'm not
> > sure how to avoid that.
> 
> Maybe miniflow_extract() could use hints provided by this parser.
> 
> [...] 
> > > diff --git a/lib/userspace-tso.c b/lib/userspace-tso.c
> > > new file mode 100644
> > > index 000000000..f843c2a76
> > > --- /dev/null
> > > +++ b/lib/userspace-tso.c
> > > @@ -0,0 +1,48 @@
> > > +/*
> > > + * Copyright (c) 2020 Red Hat, Inc.
> > > + *
> > > + * Licensed under the Apache License, Version 2.0 (the "License");
> > > + * you may not use this file except in compliance with the License.
> > > + * You may obtain a copy of the License at:
> > > + *
> > > + *     http://www.apache.org/licenses/LICENSE-2.0
> > > + *
> > > + * Unless required by applicable law or agreed to in writing, software
> > > + * distributed under the License is distributed on an "AS IS" BASIS,
> > > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> > > + * See the License for the specific language governing permissions and
> > > + * limitations under the License.
> > > + */
> > > +
> > > +#include <config.h>
> > > +
> > > +#include "smap.h"
> > > +#include "ovs-thread.h"
> > > +#include "openvswitch/vlog.h"
> > > +#include "dpdk.h"
> > > +#include "userspace-tso.h"
> > > +#include "vswitch-idl.h"
> > > +
> > > +VLOG_DEFINE_THIS_MODULE(userspace_tso);
> > > +
> > > +static bool userspace_tso = false;
> > > +
> > > +void
> > > +userspace_tso_init(const struct smap *ovs_other_config)
> > > +{
> > > +    if (smap_get_bool(ovs_other_config, "userspace-tso-enable", false)) {
> > > +        static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
> > > +
> > > +        if (ovsthread_once_start(&once)) {
> > > +            VLOG_INFO("Userspace TCP Segmentation Offloading support enabled");
> > > +            userspace_tso = true;
> > 
> > Since dp_packet functions has no implementation if OVS built without
> > DPDK support, I think, we need to restrict enabling the functionality.
> 
> I will add the restriction back.
> 
Hi Flavio,

I took a look at the netdev-linux.c and related parts.
I think this patchset also work for non-DPDK case if we
implement the dp_packet_hwol_*?

For af_packet, the change to netdev_linux_batch_rxq_recv_sock and 
netdev_linux_sock_batch_send using vnet header makes rx/tx GSO packet
from/to kernel possible, and similar case for tap interface.

Or am I missing something?

Thanks!
William

> > #ifdef DPDK_NETDEV
> >             VLOG_INFO("Userspace TCP Segmentation Offloading support enabled");
> >             userspace_tso = true;
> > #else
> >             VLOG_WARN("Userspace TCP Segmentation Offloading can not be enabled"
> >                       "since OVS built without DPDK support.");
> > #endif
> > 
> 
> Thanks,
> -- 
> fbl
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev


More information about the dev mailing list