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

2020-01-17 Thread Flavio Leitner
On Fri, Jan 17, 2020 at 06:58:56PM +0100, Ilya Maximets wrote:
> On 16.01.2020 18:00, Flavio Leitner wrote:
[...]
> > Signed-off-by: Flavio Leitner 
> 
> 
> 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.

Ben, v5 is posted here:
https://mail.openvswitch.org/pipermail/ovs-dev/2020-January/366950.html

fbl
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


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

2020-01-17 Thread William Tu
On Fri, Jan 17, 2020 at 1:41 PM Stokes, Ian  wrote:
>
>
>
> On 1/17/2020 9:37 PM, Flavio Leitner wrote:
> > On Fri, Jan 17, 2020 at 12:37:56PM -0800, William Tu wrote:
> >> 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:
> >   > [...]
> > diff --git a/lib/userspace-tso.c b/lib/userspace-tso.c
> > new file mode 100644
> > index 0..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 
> > +
> > +#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()) {
> > +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_*?
> >
> > That's correct.
> >
> >> 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?
> >
> > No, I think you're correct. However, I am adding the restriction
> > back because at this moment I am focusing on DPDK and I haven't
> > tested yet OvS without DPDK and TSO enabled.
> >
> > Once we know it's okay, we can remove that restriction.
>
> To be fair, this is an experimental function at the moment. Something we
> can fix up in the future.
>
Agree, thanks for the clarification.
William
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


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

2020-01-17 Thread Stokes, Ian




On 1/17/2020 9:37 PM, Flavio Leitner wrote:

On Fri, Jan 17, 2020 at 12:37:56PM -0800, William Tu wrote:

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:

  > [...]

diff --git a/lib/userspace-tso.c b/lib/userspace-tso.c
new file mode 100644
index 0..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 
+
+#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()) {
+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_*?


That's correct.


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?


No, I think you're correct. However, I am adding the restriction
back because at this moment I am focusing on DPDK and I haven't
tested yet OvS without DPDK and TSO enabled.

Once we know it's okay, we can remove that restriction.


To be fair, this is an experimental function at the moment. Something we 
can fix up in the future.


BR
Ian



___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


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

2020-01-17 Thread Flavio Leitner
On Fri, Jan 17, 2020 at 12:37:56PM -0800, William Tu wrote:
> 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:
 > [...] 
> > > > diff --git a/lib/userspace-tso.c b/lib/userspace-tso.c
> > > > new file mode 100644
> > > > index 0..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 
> > > > +
> > > > +#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()) {
> > > > +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_*?

That's correct. 

> 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?

No, I think you're correct. However, I am adding the restriction
back because at this moment I am focusing on DPDK and I haven't
tested yet OvS without DPDK and TSO enabled.

Once we know it's okay, we can remove that restriction.

-- 
fbl
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


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

2020-01-17 Thread William Tu
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 
> > 
> > 
> > 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_), );
> > 
> > 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 0..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 
> > > +
> > > +#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()) {
> > > +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 

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

2020-01-17 Thread Flavio Leitner
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 
> 
> 
> 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_), );
> 
> 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 0..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 
> > +
> > +#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()) {
> > +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.

> #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
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


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

2020-01-17 Thread Stokes, Ian




On 1/17/2020 5:58 PM, 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 



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.'

Best regards, Ilya maximets.


Thanks Ilya, your input is much appreciated.

@Ben, I agree, with Ilya WRT the netdev-linux side,  DO you think this 
is realistic for 2.13.? It's expermimental so at the very least I dont 
want to break anything existing.


BR
Ian




---
  Documentation/automake.mk  |   1 +
  Documentation/topics/index.rst |   1 +
  Documentation/topics/userspace-tso.rst |  98 +++
  NEWS   |   1 +
  lib/automake.mk|   2 +
  lib/conntrack.c|  29 +-
  lib/dp-packet.h| 186 +++-
  lib/ipf.c  |  32 +-
  lib/netdev-dpdk.c  | 349 +++---
  lib/netdev-linux-private.h |   5 +
  lib/netdev-linux.c | 386 ++---
  lib/netdev-provider.h  |   9 +
  lib/netdev.c   |  78 -
  lib/userspace-tso.c|  48 +++
  lib/userspace-tso.h|  23 ++
  vswitchd/bridge.c  |   2 +
  vswitchd/vswitch.xml   |  17 ++
  17 files changed, 1143 insertions(+), 124 deletions(-)
  create mode 100644 Documentation/topics/userspace-tso.rst
  create mode 100644 lib/userspace-tso.c
  create mode 100644 lib/userspace-tso.h

Changelog:
- v4
  * rebased on top of master (recvmmsg)
  * fixed URL in doc to point to 19.11
  * renamed tso to userspace-tso
  * renamed the option to userspace-tso-enable
  * removed prototype that left over from v2
  * fixed function style declaration
  * renamed dp_packet_hwol_tx_ip_checksum to dp_packet_hwol_tx_ipv4_checksum
  * dp_packet_hwol_tx_ipv4_checksum now checks for PKT_TX_IPV4.
  * account for drops while preping the batch for TX.
  * don't prep the batch for TX if TSO is disabled.
  * simplified setsockopt error checking
  * fixed af_packet_sock error checking to not call setsockopt on
   closed sockets.
  * fixed ol_flags comment.
  * used VLOG_ERR_BUF() to pass error messages.
  * fixed packet leak at netdev_send_prepare_batch()
  * added a coverage counter to account drops while preparing a batch
at netdev.c
  * fixed netdev_send() to not call ->send() if the batch is empty.
  * fixed packet leak at netdev_push_header and account for the drops.
  * removed DPDK requirement to enable userspace TSO support.
  * fixed parameter documentation in vswitch.xml.
  * renamed tso.rst to userspace-tso.rst and moved to topics/
  * added comments documeting the functions in dp-packet.h
  * fixed dp_packet_hwol_is_tso to check only PKT_TX_TCP_SEG

- v3
  * Improved the documentation.
  * Updated copyright year to 2020.
  * TSO offloaded msg now includes the netdev's name.
  * Added period at the end of all code comments.
  * Warn and drop encapsulation of TSO packets.
  * Fixed travis issue with restricted virtio types.
  * Fixed double headroom allocation in dpdk_copy_dp_packet_to_mbuf()
which caused packet corruption.
  * Fixed netdev_dpdk_prep_hwol_packet() to unconditionally set
PKT_TX_IP_CKSUM only for IPv4 packets.


diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index f2ca17bad..22976a3cd 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -57,6 +57,7 @@ DOC_SOURCE = \
Documentation/topics/ovsdb-replication.rst \
Documentation/topics/porting.rst \
Documentation/topics/tracing.rst \
+   Documentation/topics/userspace-tso.rst \
Documentation/topics/windows.rst \

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

2020-01-17 Thread Ilya Maximets
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 


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.'

Best regards, Ilya maximets. 


> ---
>  Documentation/automake.mk  |   1 +
>  Documentation/topics/index.rst |   1 +
>  Documentation/topics/userspace-tso.rst |  98 +++
>  NEWS   |   1 +
>  lib/automake.mk|   2 +
>  lib/conntrack.c|  29 +-
>  lib/dp-packet.h| 186 +++-
>  lib/ipf.c  |  32 +-
>  lib/netdev-dpdk.c  | 349 +++---
>  lib/netdev-linux-private.h |   5 +
>  lib/netdev-linux.c | 386 ++---
>  lib/netdev-provider.h  |   9 +
>  lib/netdev.c   |  78 -
>  lib/userspace-tso.c|  48 +++
>  lib/userspace-tso.h|  23 ++
>  vswitchd/bridge.c  |   2 +
>  vswitchd/vswitch.xml   |  17 ++
>  17 files changed, 1143 insertions(+), 124 deletions(-)
>  create mode 100644 Documentation/topics/userspace-tso.rst
>  create mode 100644 lib/userspace-tso.c
>  create mode 100644 lib/userspace-tso.h
> 
> Changelog:
> - v4
>  * rebased on top of master (recvmmsg)
>  * fixed URL in doc to point to 19.11
>  * renamed tso to userspace-tso
>  * renamed the option to userspace-tso-enable
>  * removed prototype that left over from v2
>  * fixed function style declaration
>  * renamed dp_packet_hwol_tx_ip_checksum to dp_packet_hwol_tx_ipv4_checksum
>  * dp_packet_hwol_tx_ipv4_checksum now checks for PKT_TX_IPV4.
>  * account for drops while preping the batch for TX.
>  * don't prep the batch for TX if TSO is disabled.
>  * simplified setsockopt error checking
>  * fixed af_packet_sock error checking to not call setsockopt on
>   closed sockets.
>  * fixed ol_flags comment.
>  * used VLOG_ERR_BUF() to pass error messages.
>  * fixed packet leak at netdev_send_prepare_batch()
>  * added a coverage counter to account drops while preparing a batch
>at netdev.c
>  * fixed netdev_send() to not call ->send() if the batch is empty.
>  * fixed packet leak at netdev_push_header and account for the drops.
>  * removed DPDK requirement to enable userspace TSO support.
>  * fixed parameter documentation in vswitch.xml.
>  * renamed tso.rst to userspace-tso.rst and moved to topics/
>  * added comments documeting the functions in dp-packet.h
>  * fixed dp_packet_hwol_is_tso to check only PKT_TX_TCP_SEG
> 
> - v3
>  * Improved the documentation.
>  * Updated copyright year to 2020.
>  * TSO offloaded msg now includes the netdev's name.
>  * Added period at the end of all code comments.
>  * Warn and drop encapsulation of TSO packets.
>  * Fixed travis issue with restricted virtio types.
>  * Fixed double headroom allocation in dpdk_copy_dp_packet_to_mbuf()
>which caused packet corruption.
>  * Fixed netdev_dpdk_prep_hwol_packet() to unconditionally set
>PKT_TX_IP_CKSUM only for IPv4 packets.
> 
> 
> diff --git a/Documentation/automake.mk b/Documentation/automake.mk
> index f2ca17bad..22976a3cd 100644
> --- a/Documentation/automake.mk
> +++ b/Documentation/automake.mk
> @@ -57,6 +57,7 @@ DOC_SOURCE = \
>   Documentation/topics/ovsdb-replication.rst \
>   Documentation/topics/porting.rst \
>   Documentation/topics/tracing.rst \
> + Documentation/topics/userspace-tso.rst \
>   Documentation/topics/windows.rst \
>   Documentation/howto/index.rst \
>   Documentation/howto/dpdk.rst \
> diff --git a/Documentation/topics/index.rst b/Documentation/topics/index.rst
> index 34c4b10e0..08af3a24d 

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

2020-01-17 Thread Stokes, Ian




On 1/17/2020 4:10 PM, Ilya Maximets wrote:

On 17.01.2020 15:23, Stokes, Ian wrote:



On 1/16/2020 5:00 PM, 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.


Thanks for the patch Flavio,

Ciara has vaidated the series and no issues found on our side.

@Ilya, any concerns on your side? Any reason to blocking this to merge?


I'm looking at patches now.  Will reply soon with results.

One thing is that I'm asking to rename patches before pushing them
to master due to incorrect 'area':

vhost: Disable multi-segmented buffers
-->  netdev-dpdk: Disable multi-segmented buffers for vhost.

netdev-dpdk: Add TCP Segmentation Offload support
-->  userspace: Add TCP Segmentation Offload support.


Thanks Ilya, your feedback is much appreciated.

Regards
Ian


Best regards, Ilya Maximets.


___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


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

2020-01-17 Thread Ilya Maximets
On 17.01.2020 15:23, Stokes, Ian wrote:
> 
> 
> On 1/16/2020 5:00 PM, 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.
> 
> Thanks for the patch Flavio,
> 
> Ciara has vaidated the series and no issues found on our side.
> 
> @Ilya, any concerns on your side? Any reason to blocking this to merge?

I'm looking at patches now.  Will reply soon with results.

One thing is that I'm asking to rename patches before pushing them
to master due to incorrect 'area':

vhost: Disable multi-segmented buffers
-->  netdev-dpdk: Disable multi-segmented buffers for vhost.

netdev-dpdk: Add TCP Segmentation Offload support
-->  userspace: Add TCP Segmentation Offload support.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


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

2020-01-17 Thread Stokes, Ian




On 1/16/2020 5:00 PM, 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.


Thanks for the patch Flavio,

Ciara has vaidated the series and no issues found on our side.

@Ilya, any concerns on your side? Any reason to blocking this to merge?

Regards
Ian


Signed-off-by: Flavio Leitner 
---
  Documentation/automake.mk  |   1 +
  Documentation/topics/index.rst |   1 +
  Documentation/topics/userspace-tso.rst |  98 +++
  NEWS   |   1 +
  lib/automake.mk|   2 +
  lib/conntrack.c|  29 +-
  lib/dp-packet.h| 186 +++-
  lib/ipf.c  |  32 +-
  lib/netdev-dpdk.c  | 349 +++---
  lib/netdev-linux-private.h |   5 +
  lib/netdev-linux.c | 386 ++---
  lib/netdev-provider.h  |   9 +
  lib/netdev.c   |  78 -
  lib/userspace-tso.c|  48 +++
  lib/userspace-tso.h|  23 ++
  vswitchd/bridge.c  |   2 +
  vswitchd/vswitch.xml   |  17 ++
  17 files changed, 1143 insertions(+), 124 deletions(-)
  create mode 100644 Documentation/topics/userspace-tso.rst
  create mode 100644 lib/userspace-tso.c
  create mode 100644 lib/userspace-tso.h

Changelog:
- v4
  * rebased on top of master (recvmmsg)
  * fixed URL in doc to point to 19.11
  * renamed tso to userspace-tso
  * renamed the option to userspace-tso-enable
  * removed prototype that left over from v2
  * fixed function style declaration
  * renamed dp_packet_hwol_tx_ip_checksum to dp_packet_hwol_tx_ipv4_checksum
  * dp_packet_hwol_tx_ipv4_checksum now checks for PKT_TX_IPV4.
  * account for drops while preping the batch for TX.
  * don't prep the batch for TX if TSO is disabled.
  * simplified setsockopt error checking
  * fixed af_packet_sock error checking to not call setsockopt on
   closed sockets.
  * fixed ol_flags comment.
  * used VLOG_ERR_BUF() to pass error messages.
  * fixed packet leak at netdev_send_prepare_batch()
  * added a coverage counter to account drops while preparing a batch
at netdev.c
  * fixed netdev_send() to not call ->send() if the batch is empty.
  * fixed packet leak at netdev_push_header and account for the drops.
  * removed DPDK requirement to enable userspace TSO support.
  * fixed parameter documentation in vswitch.xml.
  * renamed tso.rst to userspace-tso.rst and moved to topics/
  * added comments documeting the functions in dp-packet.h
  * fixed dp_packet_hwol_is_tso to check only PKT_TX_TCP_SEG

- v3
  * Improved the documentation.
  * Updated copyright year to 2020.
  * TSO offloaded msg now includes the netdev's name.
  * Added period at the end of all code comments.
  * Warn and drop encapsulation of TSO packets.
  * Fixed travis issue with restricted virtio types.
  * Fixed double headroom allocation in dpdk_copy_dp_packet_to_mbuf()
which caused packet corruption.
  * Fixed netdev_dpdk_prep_hwol_packet() to unconditionally set
PKT_TX_IP_CKSUM only for IPv4 packets.


diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index f2ca17bad..22976a3cd 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -57,6 +57,7 @@ DOC_SOURCE = \
Documentation/topics/ovsdb-replication.rst \
Documentation/topics/porting.rst \
Documentation/topics/tracing.rst \
+   Documentation/topics/userspace-tso.rst \
Documentation/topics/windows.rst \
Documentation/howto/index.rst \
Documentation/howto/dpdk.rst \
diff --git a/Documentation/topics/index.rst b/Documentation/topics/index.rst
index 34c4b10e0..08af3a24d 100644
--- a/Documentation/topics/index.rst
+++ b/Documentation/topics/index.rst
@@ -50,5 +50,6 @@ OVS
 language-bindings
 testing
 tracing
+   userspace-tso
 idl-compound-indexes
 ovs-extensions
diff --git a/Documentation/topics/userspace-tso.rst 

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

2020-01-16 Thread Flavio Leitner
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 
---
 Documentation/automake.mk  |   1 +
 Documentation/topics/index.rst |   1 +
 Documentation/topics/userspace-tso.rst |  98 +++
 NEWS   |   1 +
 lib/automake.mk|   2 +
 lib/conntrack.c|  29 +-
 lib/dp-packet.h| 186 +++-
 lib/ipf.c  |  32 +-
 lib/netdev-dpdk.c  | 349 +++---
 lib/netdev-linux-private.h |   5 +
 lib/netdev-linux.c | 386 ++---
 lib/netdev-provider.h  |   9 +
 lib/netdev.c   |  78 -
 lib/userspace-tso.c|  48 +++
 lib/userspace-tso.h|  23 ++
 vswitchd/bridge.c  |   2 +
 vswitchd/vswitch.xml   |  17 ++
 17 files changed, 1143 insertions(+), 124 deletions(-)
 create mode 100644 Documentation/topics/userspace-tso.rst
 create mode 100644 lib/userspace-tso.c
 create mode 100644 lib/userspace-tso.h

Changelog:
- v4
 * rebased on top of master (recvmmsg)
 * fixed URL in doc to point to 19.11
 * renamed tso to userspace-tso
 * renamed the option to userspace-tso-enable
 * removed prototype that left over from v2
 * fixed function style declaration
 * renamed dp_packet_hwol_tx_ip_checksum to dp_packet_hwol_tx_ipv4_checksum
 * dp_packet_hwol_tx_ipv4_checksum now checks for PKT_TX_IPV4.
 * account for drops while preping the batch for TX.
 * don't prep the batch for TX if TSO is disabled.
 * simplified setsockopt error checking
 * fixed af_packet_sock error checking to not call setsockopt on
  closed sockets.
 * fixed ol_flags comment.
 * used VLOG_ERR_BUF() to pass error messages.
 * fixed packet leak at netdev_send_prepare_batch()
 * added a coverage counter to account drops while preparing a batch
   at netdev.c
 * fixed netdev_send() to not call ->send() if the batch is empty.
 * fixed packet leak at netdev_push_header and account for the drops.
 * removed DPDK requirement to enable userspace TSO support.
 * fixed parameter documentation in vswitch.xml.
 * renamed tso.rst to userspace-tso.rst and moved to topics/
 * added comments documeting the functions in dp-packet.h
 * fixed dp_packet_hwol_is_tso to check only PKT_TX_TCP_SEG

- v3
 * Improved the documentation.
 * Updated copyright year to 2020.
 * TSO offloaded msg now includes the netdev's name.
 * Added period at the end of all code comments.
 * Warn and drop encapsulation of TSO packets.
 * Fixed travis issue with restricted virtio types.
 * Fixed double headroom allocation in dpdk_copy_dp_packet_to_mbuf()
   which caused packet corruption.
 * Fixed netdev_dpdk_prep_hwol_packet() to unconditionally set
   PKT_TX_IP_CKSUM only for IPv4 packets.


diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index f2ca17bad..22976a3cd 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -57,6 +57,7 @@ DOC_SOURCE = \
Documentation/topics/ovsdb-replication.rst \
Documentation/topics/porting.rst \
Documentation/topics/tracing.rst \
+   Documentation/topics/userspace-tso.rst \
Documentation/topics/windows.rst \
Documentation/howto/index.rst \
Documentation/howto/dpdk.rst \
diff --git a/Documentation/topics/index.rst b/Documentation/topics/index.rst
index 34c4b10e0..08af3a24d 100644
--- a/Documentation/topics/index.rst
+++ b/Documentation/topics/index.rst
@@ -50,5 +50,6 @@ OVS
language-bindings
testing
tracing
+   userspace-tso
idl-compound-indexes
ovs-extensions
diff --git a/Documentation/topics/userspace-tso.rst 
b/Documentation/topics/userspace-tso.rst
new file mode 100644
index 0..893c64839
--- /dev/null
+++ b/Documentation/topics/userspace-tso.rst
@@ -0,0 +1,98 @@
+..
+  Copyright 2020, Red Hat, Inc.
+
+  Licensed under the Apache License, Version 2.0 (the "License"); you may
+  not