Re: [ovs-dev] [PATCH v6 1/1] acinclude: Also use LIBS from dpkg pkg-config

2019-02-12 Thread Christian Ehrhardt
On Tue, Feb 12, 2019 at 11:47 PM Ben Pfaff  wrote:
>
> On Tue, Feb 12, 2019 at 07:29:58AM +0100, Christian Ehrhardt wrote:
> > DPDK 18.11 builds using the more modern meson build system no more
> > provide the -ldpdk linker script. Instead it is expected to use
> > pkgconfig for linker options as well.
> >
> > This change will set DPDK_LIB from pkg-config (if pkg-config was
> > available) and since that already carries the whole-archive flags
> > around the PMDs skips the further wrapping in more whole-archive
> > if that is already part of DPDK_LIB.
> >
> > To work reliable in all environments this needs pkg-config 0.29.1.
> > We want to be able to use PKG_CHECK_MODULES_STATIC which
> > is not yet available in 0.24. Therefore update pkg.m4
> > to pkg-config 0.29.1.
> >
> > This should be backport-safe as these macro files are all versioned.
> > autoconf is smart enough to check the version if you have it locally,
> > and if the system's is higher, it will use that one instead.
> >
> > Acked-by: Luca Boccassi 
> > Acked-by: Aaron Conole 
> > Signed-off-by: Christian Ehrhardt 
>
> "Autoconf coding style" is basically an oxymoron--Autoconf looks ugly
> almost no matter what--but this look unusual to me:
>
> PKG_CHECK_MODULES_STATIC([DPDK], [libdpdk], [
> DPDK_INCLUDE="$DPDK_CFLAGS"
> DPDK_LIB="$DPDK_LIBS"
>  ], [
> DPDK_INCLUDE="-I/usr/local/include/dpdk -I/usr/include/dpdk"
> DPDK_LIB="-ldpdk"
>  ])
>
> I suggest the following:
>
> PKG_CHECK_MODULES_STATIC([DPDK], [libdpdk], [
> DPDK_INCLUDE="$DPDK_CFLAGS"
> DPDK_LIB="$DPDK_LIBS"], [
> DPDK_INCLUDE="-I/usr/local/include/dpdk -I/usr/include/dpdk"
> DPDK_LIB="-ldpdk"])
>
> or
>
> PKG_CHECK_MODULES_STATIC(
>[DPDK], [libdpdk],
>[DPDK_INCLUDE="$DPDK_CFLAGS"
> DPDK_LIB="$DPDK_LIBS"],
>[DPDK_INCLUDE="-I/usr/local/include/dpdk -I/usr/include/dpdk"
> DPDK_LIB="-ldpdk"])
>
> Beyond that, I will leave this patch to Ian.

Thanks Ben,
@Ian it seems we have the acks of all participants - is it ok for you
as well to push it now?
Feel free to use either coding style on commit or if you want I can
submit it as preferred - I can resend after you picked one :-)


-- 
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [patch v4 2/2] conntrack: Exclude l2 padding in 'conn_key_extract()'.

2019-02-12 Thread Nitin Katiyar
Hi,
We would like to get it backported till OVS 2.6 at least.

Regards,
Nitin

> -Original Message-
> From: Ben Pfaff [mailto:b...@ovn.org]
> Sent: Tuesday, February 12, 2019 9:13 AM
> To: Darrell Ball 
> Cc: d...@openvswitch.org; Nitin Katiyar 
> Subject: Re: [ovs-dev] [patch v4 2/2] conntrack: Exclude l2 padding in
> 'conn_key_extract()'.
> 
> On Mon, Feb 04, 2019 at 04:23:07PM -0800, Darrell Ball wrote:
> > 'conn_key_extract()' in userspace conntrack is including L2
> > (Ethernet) pad bytes for both L3 and L4 sizes. One problem is any
> > packet with non-zero L2 padding can incorrectly fail L4 checksum
> > validation.
> >
> > This patch fixes conn_key_extract() by ignoring L2 pad bytes.
> >
> > Fixes: a489b16854b5 ("conntrack: New userspace connection tracker.")
> > CC: Daniele Di Proietto 
> > Co-authored-by: Vishal Deep Ajmera 
> > Co-authored-by: Venkatesan Pradeep
> 
> > Co-authored-by: Nitin Katiyar 
> > Signed-off-by: Vishal Deep Ajmera 
> > Signed-off-by: Venkatesan Pradeep 
> > Signed-off-by: Nitin Katiyar 
> > Signed-off-by: Darrell Ball 
> 
> Thanks.
> 
> I applied this series to master.  Please let me know what branches I should
> backport it to.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [patch v12 8/8] Userspace datapath: Add fragmentation handling.

2019-02-12 Thread Darrell Ball
Thanks very much Ben

On Mon, Feb 11, 2019 at 5:33 PM Ben Pfaff  wrote:

> On Sun, Feb 10, 2019 at 06:56:30PM -0800, Darrell Ball wrote:
> > Fragmentation handling is added for supporting conntrack.
> > Both v4 and v6 are supported.
> >
> > After discussion with several people, I decided to not store
> > configuration state in the database to be more consistent with
> > the kernel in future, similarity with other conntrack configuration
> > which will not be in the database as well and overall simplicity.
> > Accordingly, fragmentation handling is enabled by default.
> >
> > This patch enables fragmentation tests for the userspace datapath.
> >
> > Signed-off-by: Darrell Ball 
>
> Thanks.
>
> It's not obvious to me why ipf_reassemble_v6_frags() accounts for pad
> size but ipf_reassemble_v4_frags() does not.  (Is there an inherent
> difference for padding between v4 and v6?  If so, I've forgotten about
> it.)
>

This is just prep for calling parse_ipv6_ext_hdrs() and 'pad' is not even
needed
here as payload length value would suffice at this point.

What happened here is ipf_reassemble_v6_frags() got changed a few times
and then I forgot to clean up afterwards. I now cleaned up more code on
revisit.


> I had some other comments, but I decided that it would be easier to
> present them as an incremental patch.  Please let me know what you think
> of folding in the following changes.  They are mainly stylistic one way
> or another, in the sense that I don't remember any of them actually
> fixing bugs.
>

I folded in all the suggestions (hopefully none missed); there were a few I
was not aware of.


>
> Thanks,
>
> Ben.
>
> --8<--cut here-->8--
>
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index 46b5dd570d53..6f2e788c6703 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2015, 2016, 2017 Nicira, Inc.
> + * Copyright (c) 2015, 2016, 2017, 2019 Nicira, Inc.
>   *
>   * Licensed under the Apache License, Version 2.0 (the "License");
>   * you may not use this file except in compliance with the License.
> @@ -341,7 +341,7 @@ conntrack_init(struct conntrack *ct)
>  atomic_init(&ct->n_conn_limit, DEFAULT_N_CONN_LIMIT);
>  latch_init(&ct->clean_thread_exit);
>  ct->clean_thread = ovs_thread_create("ct_clean", clean_thread_main,
> ct);
> -ipf_init(&ct->ipf);
> +ct->ipf = ipf_init();
>  }
>
>  /* Destroys the connection tracker 'ct' and frees all the allocated
> memory. */
> diff --git a/lib/ipf.c b/lib/ipf.c
> index bdfd85579165..8dc63ea99529 100644
> --- a/lib/ipf.c
> +++ b/lib/ipf.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2018 Nicira, Inc.
> + * Copyright (c) 2018, 2019 Nicira, Inc.
>   *
>   * Licensed under the Apache License, Version 2.0 (the "License");
>   * you may not use this file except in compliance with the License.
> @@ -84,10 +84,8 @@ enum ipf_counter_type {
>  };
>
>  union ipf_addr {
> -ovs_16aligned_be32 ipv4;
> -union ovs_16aligned_in6_addr ipv6;
> -ovs_be32 ipv4_aligned;
> -struct in6_addr ipv6_aligned;
> +ovs_be32 ipv4;
> +struct in6_addr ipv6;
>

ok
This technique is used very frequently in code everywhere, but I understand
the theoretical aspect here.


>  };
>
>  /* Represents a single fragment; part of a list of fragments. */
> @@ -101,6 +99,8 @@ struct ipf_frag {
>  /* The key for a collection of fragments potentially making up an
> unfragmented
>   * packet. */
>  struct ipf_list_key {
> +/* ipf_list_key_hash() requires 'src_addr' and 'dst_addr' to be the
> first
> + * two members. */
>  union ipf_addr src_addr;
>  union ipf_addr dst_addr;
>  uint32_t recirc_id;
> @@ -112,8 +112,9 @@ struct ipf_list_key {
>
>  /* A collection of fragments potentially making up an unfragmented
> packet. */
>  struct ipf_list {
> -struct hmap_node node;
> -struct ovs_list list_node;
> +struct hmap_node node; /* In struct ipf's 'frag_lists'. */
> +struct ovs_list list_node; /* In struct ipf's 'frag_exp_list' or
> +* 'frag_complete_list'. */
>  struct ipf_frag *frag_list;/* List of fragments for this list. */
>  struct ipf_list_key key;   /* The key for the fragemnt list. */
>  struct dp_packet *reass_execute_ctx; /* Reassembled packet. */
> @@ -127,15 +128,11 @@ struct ipf_list {
>  /* Represents a reassambled packet which typically is passed through
>   * conntrack. */
>  struct reassembled_pkt {
> -struct ovs_list rp_list_node;
> +struct ovs_list rp_list_node; /* In struct ipf's
> 'reassembled_pkt_list'. */
>  struct dp_packet *pkt;
>  struct ipf_list *list;
>  };
>
> -struct OVS_LOCKABLE ipf_lock {
> -struct ovs_mutex lock;
> -};
> -
>

 It looks like I cleaned up the unneeded wrapper lock apis, but forgot the
ipf_lock struct

This has some benefit to remove – thanks.


>  struct ipf {
>  /* The clean thread is used to clean up fragm

[ovs-dev] dringende Antwort erforderlich

2019-02-12 Thread Global Reliance Credit® via dev



Schöne Grüße
  Ich bin Mr.Patrick William von einem globalen 
Kreditkreditunternehmen, bekannt als Global Reliance Credit®. Wir bieten alle 
Arten von Darlehen zu einem Zinssatz von 1% an. Wenn Sie einen Kredit 
benötigen, kontaktieren Sie uns bitte mit den untenstehenden Informationen.

Bitte füllen Sie das untenstehende Formular aus und senden Sie es so schnell 
wie möglich zurück.

Vollständiger Name:
Geschlecht:
Benötigter Betrag:
Dauer:

Wir freuen uns, Sie zu unterstützen. Kontaktieren Sie uns per E-Mail: 
i...@globalreliancecredits.com
Verwaltung
  Mr. Patrick William
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] debian: Add libelf-dev dependency for dkms

2019-02-12 Thread Ben Pfaff
On Tue, Feb 12, 2019 at 02:49:40PM -0800, Gregory Rose wrote:
> On 2/12/2019 2:22 PM, Ben Pfaff wrote:
> > On Tue, Feb 12, 2019 at 12:37:03PM -0800, Greg Rose wrote:
> > > Newer kernels define CONFIG_UNWINDER_ORC for their kernel configurations
> > > and to build this the kernel compilation requires the libelf-dev
> > > package.  Add the dependency to the dkms build requirements.
> > > 
> > > VMware-BZ: #2287968
> > > Signed-off-by: Greg Rose 
> > Thanks, Greg (and Yifeng).  I applied this to master.
> 
> Thank you Ben!  Could we also get it backported to 2.11?

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


Re: [ovs-dev] [PATCH v1] doc: Add "Representors" topic document

2019-02-12 Thread Ophir Munk



> -Original Message-
> From: Ian Stokes 
> Sent: Tuesday, February 12, 2019 7:17 PM
> To: Ilya Maximets ; Ophir Munk
> ; ovs-dev@openvswitch.org
> Cc: Olga Shern ; Kevin Traynor
> ; Asaf Penso ; Thomas
> Monjalon 
> Subject: Re: [PATCH v1] doc: Add "Representors" topic document
> 
> On 2/12/2019 1:15 PM, Ilya Maximets wrote:
> > On 11.02.2019 3:01, Ophir Munk wrote:
> >> This details how to configure representors ports.
> >>
> >> Signed-off-by: Ophir Munk 
> >> ---
> >>   Documentation/topics/dpdk/phy.rst | 80
> +++
> >>   1 file changed, 80 insertions(+)
> >>
> >> diff --git a/Documentation/topics/dpdk/phy.rst
> >> b/Documentation/topics/dpdk/phy.rst
> >> index 1470623..3792fde 100644
> >> --- a/Documentation/topics/dpdk/phy.rst
> >> +++ b/Documentation/topics/dpdk/phy.rst
> >> @@ -219,6 +219,86 @@ For more information please refer to the `DPDK
> Port Hotplug Framework`__.
> >>
...
> >> +
> >> +Representors
> >> +
.
> >> +This syntax shows that a representor is an enumerated eth device
> >> +(with a representor ID) which uses the PF PCI address.
> >> +The following commands add representors 3 and 5 using PCI device
> >> +address
> >> +``:08:00.0``::
> >> +
> >> +$ ovs-vsctl add-port br0 dpdk-rep3 -- set Interface dpdk-rep3
> type=dpdk \
> >> +   options:dpdk-devargs=:08:00.0,representor=[3]
> >> +
> >> +$ ovs-vsctl add-port br0 dpdk-rep5 -- set Interface dpdk-rep5
> type=dpdk \
> >> +   options:dpdk-devargs=:08:00.0,representor=[5]
> >> +
> >> +.. important::
> >> +
> >> +   Representors ports are configured prior to OVS invocation and
> independently
> >> +   of it, or by other means as well. Please consult a NIC vendor
> instructions
> >> +   on how to establish representors.
> >
> > It'll be good to have configuration example for at least one commonly
> > used NIC (ixgbe/i40e ?). Or maybe a link to the docs where the process
> described.
> >
> > What do you think ?
> > Ian, maybe you could add some example, since you have already tried it in
> practice?
> >
> 
> Good call, I'll draw up an incremental and post here, if acceptable we can 
> roll
> it into the same patch.
> 

In addition to Ian drawing - please find a link which details how to create VFs 
for ConnectX-4 5 or 6 NICs:
https://docs.openstack.org/neutron/rocky/admin/config-ovs-offload.html section 
"Create Compute virtual functions".
I will send an updated v2 with this reference.

> Ian
> >> To verify their correct configuration,
> >> +   execute::
> >> +
> >> +$ ovs-vsctl show
> >> +
> >> +   and make sure no errors are indicated.
> >> +
> >> +.. _multi-dev-configuration:
> >> +
> >> +
> >> +Port representors are an example of multi devices. There are NICs
> >> +which support multi devices by other methods than representors for
> >> +which a generic devargs syntax is used. The generic syntax is based on
> the device mac address::
> >> +
> >> +class=eth,mac=
> >> +
> >> +For example, the following command adds a port to a bridge called
> >> +``br0`` using an eth device whose mac address is ``00:11:22:33:44:55``::
> >> +
> >> +$ ovs-vsctl add-port br0 dpdk-mac -- set Interface dpdk-mac
> type=dpdk \
> >> +   options:dpdk-devargs="class=eth,mac=00:11:22:33:44:55"
> >> +
> >>   Jumbo Frames
> >>   
> >>
> >>

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


Re: [ovs-dev] [PATCH] debian: Add libelf-dev dependency for dkms

2019-02-12 Thread Gregory Rose

On 2/12/2019 2:22 PM, Ben Pfaff wrote:

On Tue, Feb 12, 2019 at 12:37:03PM -0800, Greg Rose wrote:

Newer kernels define CONFIG_UNWINDER_ORC for their kernel configurations
and to build this the kernel compilation requires the libelf-dev
package.  Add the dependency to the dkms build requirements.

VMware-BZ: #2287968
Signed-off-by: Greg Rose 

Thanks, Greg (and Yifeng).  I applied this to master.


Thank you Ben!  Could we also get it backported to 2.11?

Thanks,

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


Re: [ovs-dev] [PATCH v6 1/1] acinclude: Also use LIBS from dpkg pkg-config

2019-02-12 Thread Ben Pfaff
On Tue, Feb 12, 2019 at 07:29:58AM +0100, Christian Ehrhardt wrote:
> DPDK 18.11 builds using the more modern meson build system no more
> provide the -ldpdk linker script. Instead it is expected to use
> pkgconfig for linker options as well.
> 
> This change will set DPDK_LIB from pkg-config (if pkg-config was
> available) and since that already carries the whole-archive flags
> around the PMDs skips the further wrapping in more whole-archive
> if that is already part of DPDK_LIB.
> 
> To work reliable in all environments this needs pkg-config 0.29.1.
> We want to be able to use PKG_CHECK_MODULES_STATIC which
> is not yet available in 0.24. Therefore update pkg.m4
> to pkg-config 0.29.1.
> 
> This should be backport-safe as these macro files are all versioned.
> autoconf is smart enough to check the version if you have it locally,
> and if the system's is higher, it will use that one instead.
> 
> Acked-by: Luca Boccassi 
> Acked-by: Aaron Conole 
> Signed-off-by: Christian Ehrhardt 

"Autoconf coding style" is basically an oxymoron--Autoconf looks ugly
almost no matter what--but this look unusual to me:

PKG_CHECK_MODULES_STATIC([DPDK], [libdpdk], [
DPDK_INCLUDE="$DPDK_CFLAGS"
DPDK_LIB="$DPDK_LIBS"
 ], [
DPDK_INCLUDE="-I/usr/local/include/dpdk -I/usr/include/dpdk"
DPDK_LIB="-ldpdk"
 ])

I suggest the following:

PKG_CHECK_MODULES_STATIC([DPDK], [libdpdk], [
DPDK_INCLUDE="$DPDK_CFLAGS"
DPDK_LIB="$DPDK_LIBS"], [
DPDK_INCLUDE="-I/usr/local/include/dpdk -I/usr/include/dpdk"
DPDK_LIB="-ldpdk"])

or

PKG_CHECK_MODULES_STATIC(
   [DPDK], [libdpdk],
   [DPDK_INCLUDE="$DPDK_CFLAGS"
DPDK_LIB="$DPDK_LIBS"],
   [DPDK_INCLUDE="-I/usr/local/include/dpdk -I/usr/include/dpdk"
DPDK_LIB="-ldpdk"])

Beyond that, I will leave this patch to Ian.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] python: Fix E117 over-indented.

2019-02-12 Thread Ben Pfaff
On Tue, Feb 12, 2019 at 06:34:02PM +0300, Ilya Maximets wrote:
> New check was added to recent pycodestyle-2.5.0 and flake8
> complains while building on Travis:
> 
>   ../utilities/bugtool/ovs-bugtool.in:767:17: E117 over-indented
>   ../utilities/bugtool/ovs-bugtool.in:771:17: E117 over-indented
>   ../utilities/bugtool/ovs-bugtool.in:774:17: E117 over-indented
>   ../utilities/bugtool/ovs-bugtool.in:778:17: E117 over-indented
>   ../python/ovs/db/error.py:33:17: E117 over-indented
>   ../python/ovs/poller.py:118:21: E117 over-indented
>   ../python/ovs/reconnect.py:244:17: E117 over-indented
> 
> Signed-off-by: Ilya Maximets 

Thanks, applied to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] dpif-netdev: Fix unsafe accesses to pmd polling lists.

2019-02-12 Thread Ben Pfaff
On Tue, Feb 12, 2019 at 10:40:30AM +0300, Ilya Maximets wrote:
> On 11.02.2019 22:15, Aaron Conole wrote:
> > Ilya Maximets  writes:
> > 
> >> All accesses to 'pmd->poll_list' should be guarded by
> >> 'pmd->port_mutex'. Additionally fixed inappropriate usage
> >> of 'HMAP_FOR_EACH_SAFE' (hmap doesn't change in a loop)
> >> and dropped not needed local variable 'proc_cycles'.
> >>
> >> CC: Nitin Katiyar 
> >> Fixes: 5bf84282482a ("Adding support for PMD auto load balancing")
> >> Signed-off-by: Ilya Maximets 
> >> ---
> >>  lib/dpif-netdev.c | 14 +-
> >>  1 file changed, 9 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> >> index 4d5bc576a..914b2bb8b 100644
> >> --- a/lib/dpif-netdev.c
> >> +++ b/lib/dpif-netdev.c
> >> @@ -3776,9 +3776,12 @@ set_pmd_auto_lb(struct dp_netdev *dp)
> >>  continue;
> >>  }
> >>  
> >> +ovs_mutex_lock(&pmd->port_mutex);
> >>  if (hmap_count(&pmd->poll_list) > 1) {
> >>  multi_rxq = true;
> >>  }
> >> +ovs_mutex_unlock(&pmd->port_mutex);
> > 
> > What does this mutex provide here?  I ask because as soon as we unlock,
> > the result is no longer assured, and we've used it to inform the
> > multi_rxq value.
> > 
> > Are you afraid that the read is non-atomic so you'll end up with the
> > multi_rxq condition true, even when it should not be?  That can happen
> > anyway - as soon as we unlock the count can change.
> > 
> > Maybe there's a better change to support that, where hmap_count does an
> > atomic read, or there's a version that can guarantee a full load with
> > no intervening writes.  But I don't see 
> > 
> > Maybe the pmd object lifetime goes away... but that wouldn't make sense,
> > because then the port_mutex itself would be invalid.
> > 
> > I'm confused what it does to add a lock here for either the safety, or
> > the logic.  I probably missed something.  Or maybe it's just trying to
> > be safe in case the hmap implementation would change in the future?  I
> > think if that's the case, there might be an alternate to implement?
> 
> The mutex here is mostly for a code consistency. We're marking the 'poll_list'
> as OVS_GUARDED. This means that we should guard it. Unfortunately, unlike С++,
> in C we can not use OVS_GUARDED_BY(port_mutex) inside the structure and
> clang could not track what metex should protect the variable. I agree that 
> we're
> not really need to protect this. Furthermore, the only writer for 'poll_list'
> is the main thread and 'set_pmd_auto_lb' is also called only in a main thread.
> So we don't need to protect any read accesses.
> However, for the safety, these details should not be taken into account to not
> provide code snippets that could be copied to the other (less safe) places and
> produce issues there. Especially because clang can't protect us from this kind
> of mistakes. Code architecture also could be changed in the future.
> As a developer I'd like to always assume that any access to hmap is not thread
> safe regardless of its internal implementation. Just like I'm assuming that
> cmap is safe for readers.
> This is affordable since we're not on a hot path.

Maybe a comment would be helpful.

/* We don't really need this lock but it suppresses a Clang warning. */
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] debian: Add libelf-dev dependency for dkms

2019-02-12 Thread Ben Pfaff
On Tue, Feb 12, 2019 at 12:37:03PM -0800, Greg Rose wrote:
> Newer kernels define CONFIG_UNWINDER_ORC for their kernel configurations
> and to build this the kernel compilation requires the libelf-dev
> package.  Add the dependency to the dkms build requirements.
> 
> VMware-BZ: #2287968
> Signed-off-by: Greg Rose 

Thanks, Greg (and Yifeng).  I applied this to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [RFC v3 1/5] datapath: Add a new action check_pkt_len

2019-02-12 Thread Gregory Rose



On 2/11/2019 6:27 PM, Ben Pfaff wrote:

Greg, I recommend that you take a look at this one.


My apologies but I haven't been feeling well the last few days. I'll 
have a look at this one.


Thanks,



(More commentary below.)

On Thu, Jan 10, 2019 at 11:29:48PM +0530, nusid...@redhat.com wrote:

From: Numan Siddique 

[Please note, this patch is submitted as RFC in ovs-dev ML to
get feedback before submitting to netdev ML]

This patch adds a new action which checks the packet length and
executes a set of actions if the packet length is greater than
the specified length or executes another set of actions if the
packet length is lesser or equal to.

This action takes below nlattrs
   * OVS_CHECK_PKT_LEN_ATTR_PKT_LEN - 'pkt_len' to check for

   * OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER (optional) - Nested actions
 to apply if the packet length is greater than the specified 'pkt_len'

   * OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL (optional) - Nested
 actions to apply if the packet length is lesser or equal to the
 specified 'pkt_len'.

The main use case for adding this action is to solve the packet
drops because of MTU mismatch in OVN virtual networking solution.
When a VM (which belongs to a logical switch of OVN) sends a packet
destined to go via the gateway router and if the nic which provides
external connectivity, has a lesser MTU, OVS drops the packet
if the packet length is greater than this MTU.

With the help of this action, OVN will check the packet length
and if it is greater than the MTU size, it will generate an
ICMP packet (type 3, code 4) and includes the next hop mtu in it
so that the sender can fragment the packets.

I'm not used to seeing series where different patches apply to different
repos!  It took me a while to figure out why "git am" didn't want to
work at all (but then I actually looked at the patch).

I didn't have a net-next tree handy so my comments are based on reading
the patch without applying or building it.

There is a special issue with this action, which is a lot like a special
issue with the "sample".  That is that the action has flow control that
might change the flow in different ways, and this can make an important
difference for actions that follow this one.  If one fork of the action
pops off a header, and the other one does not, then that makes
validating actions that come after it complicated.  I do not see
anything here that takes that into account.  I recommend reading the
validation code for the sample action for inspiration.

In validate_and_copy_check_pkt_len(), it might be better to use
nla_policy and nla_parse_nested().  Or maybe not.  I did not look
closely.

I don't think that execute_check_pkt_len() should need to check for
netlink format errors.  The validation code should take care of that.
It might also be able to assume a particular order for the attributes,
depending on how you implement validation.

I'm still not thrilled by the naming.  I don't have any wonderful names
though.


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


Re: [ovs-dev] debian: Add libelf-dev dependency for dkms

2019-02-12 Thread 0-day Robot
Bleep bloop.  Greetings Greg Rose, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line is 88 characters long (recommended limit is 79)
#26 FILE: debian/control:44:
Depends: dkms (>= 1.95), libc6-dev, libelf-dev, make, ${misc:Depends}, 
${python:Depends}

Lines checked: 33, Warnings: 1, Errors: 0


Please check this out.  If you feel there has been an error, please email 
acon...@bytheb.org

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] debian: Add libelf-dev dependency for dkms

2019-02-12 Thread Yifeng Sun
Looks good to me, thanks.

Reviewed-by: Yifeng Sun 

On Tue, Feb 12, 2019 at 12:37 PM Greg Rose  wrote:

> Newer kernels define CONFIG_UNWINDER_ORC for their kernel configurations
> and to build this the kernel compilation requires the libelf-dev
> package.  Add the dependency to the dkms build requirements.
>
> VMware-BZ: #2287968
> Signed-off-by: Greg Rose 
> ---
>  debian/control | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/debian/control b/debian/control
> index cde93f2..c70d2a6 100644
> --- a/debian/control
> +++ b/debian/control
> @@ -41,7 +41,7 @@ Description: Open vSwitch datapath module source -
> module-assistant version
>
>  Package: openvswitch-datapath-dkms
>  Architecture: all
> -Depends: dkms (>= 1.95), libc6-dev, make, ${misc:Depends},
> ${python:Depends}
> +Depends: dkms (>= 1.95), libc6-dev, libelf-dev, make, ${misc:Depends},
> ${python:Depends}
>  Description: Open vSwitch datapath module source - DKMS version
>   Open vSwitch is a production quality, multilayer, software-based,
>   Ethernet virtual switch. It is designed to enable massive network
> --
> 1.8.3.1
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] debian: Add libelf-dev dependency for dkms

2019-02-12 Thread Greg Rose
Newer kernels define CONFIG_UNWINDER_ORC for their kernel configurations
and to build this the kernel compilation requires the libelf-dev
package.  Add the dependency to the dkms build requirements.

VMware-BZ: #2287968
Signed-off-by: Greg Rose 
---
 debian/control | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/debian/control b/debian/control
index cde93f2..c70d2a6 100644
--- a/debian/control
+++ b/debian/control
@@ -41,7 +41,7 @@ Description: Open vSwitch datapath module source - 
module-assistant version
 
 Package: openvswitch-datapath-dkms
 Architecture: all
-Depends: dkms (>= 1.95), libc6-dev, make, ${misc:Depends}, ${python:Depends}
+Depends: dkms (>= 1.95), libc6-dev, libelf-dev, make, ${misc:Depends}, 
${python:Depends}
 Description: Open vSwitch datapath module source - DKMS version
  Open vSwitch is a production quality, multilayer, software-based,
  Ethernet virtual switch. It is designed to enable massive network
-- 
1.8.3.1

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


Re: [ovs-dev] Openvswitch| RSA Conference 2019 -Leads

2019-02-12 Thread Stacy Brown
Hi,

 

Any thoughts on below email ?

 

Thanks,

Stacy Brown

 

From: Stacy Brown [mailto:stacy.br...@epaxis.com] 
Sent: Thursday, February 7, 2019 11:15 PM
To: 'd...@openvswitch.org'
Subject: Openvswitch| RSA Conference 2019 -Leads

 

Hi,

 

Attendees-list of RSA Conference 2019 (March 4th to 8th  2019) is
available-for-purchase with contact name, company name, job title, business
email-address, phone number and mailing address etc...

 

Let me know if you'd be interested in hearing more about it.

 

Thanks,
Stacy Brown
Marketing Executive

PS : If you have any other list requirements that we can service may be
based on the target job titles, industries, countries, Event attendees list
, Technology install base, you sell into.

If you don't want to include yourself in our mailing list, please reply back
"No thanks" in a subject line.

 

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


Re: [ovs-dev] [PATCH 0/3] gcc-plugins: Introduce stackinit plugin

2019-02-12 Thread Kees Cook
On Mon, Jan 28, 2019 at 4:12 PM Alexander Popov  wrote:
>
> On 23.01.2019 14:03, Kees Cook wrote:
> > This adds a new plugin "stackinit" that attempts to perform unconditional
> > initialization of all stack variables
>
> Hello Kees! Hello everyone!
>
> I was curious about the performance impact of the initialization of all stack
> variables. So I did a very brief test with this plugin on top of 4.20.5.
>
> hackbench on Intel Core i7-4770 showed ~0.7% slowdown.
> hackbench on Kirin 620 (ARM Cortex-A53 Octa-core 1.2GHz) showed ~1.3% 
> slowdown.

Thanks for looking at this! I'll be including my hackbench
measurements for the v2 here in a moment.

> This test involves the kernel scheduler and allocator. I can't say whether 
> they
> use stack aggressively. Maybe performance tests of other subsystems (e.g.
> network subsystem) can show different numbers. Did you try?

I haven't found a stable network test yet. If someone can find a
reasonable workload, I'd love to hear about it.

> I've heard a hypothesis that the initialization of all stack variables would
> pollute CPU caches, which is critical for some types of computations. Maybe 
> some
> micro-benchmarks can disprove/confirm that?

I kind of think micro-benchmarks aren't so useful because they don't
represent a real-world workload. I've heard people talk about SAP-HANA
as a good test, but I can't get my hands on it. I wonder if anyone has
tried "mysqlslap"?

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


[ovs-dev] Capacitación a tu Medida Guía paso a paso

2019-02-12 Thread STPS - Trámites de capacitación
Cursos esenciales - Webinar Interactivo – Martes 05 de Marzo

Capacitación a tu Medida Guía paso a paso: 
Trámites de capacitación ante la S.T.P.S.

En este webinar te presentamos una guía práctica paso a paso del proceso que 
debemos seguir ante este organismo para 
registrar la capacitación de nuestro personal así como la documentación 
necesaria para cumplir ante una supervisión o auditoría. 

Ofreceremos una guía paso a paso de lo que la Secretaría del Trabajo y 
Previsión Social solicita a las empresas para registrar sus capacitaciones 
 

Objetivos Específicos:


• El participante revisará los conceptos básicos de la capacitación en las 
empresas.
• El participante conocerá el proceso que determina la S.T.P.S. para registrar 
capacitaciones así como los formatos requeridos.
• El participante recibirá recomendaciones para las supervisiones de la 
S.T.P.S. El participante podrá elaborar y aplicar con
éxito un programa de capacitación con la información proporcionada. 

Para mayor información, responder sobre este correo con la palabra STPS + los 
siguientes datos:


NOMBRE:

TELÉFONO:

EMPRESA: 


 

Llamanos al (045) 55 1554 6630
www.Innovalearn.mx 


 






 


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


[ovs-dev] Excel - webinar

2019-02-12 Thread Asegure su lugar / 3 lugares disponibles
Cursos escenciales - Webinar Interactivo – Viernes 22 de Febrero

Bases de datos y tablas dinámicas en Excel

Nuestro curso está diseñado para enseñarte a importar o construir bases de 
datos correctamente estructuradas para la creación
de tablas dinámicas y todas las herramientas que la herramienta nos ofrece para 
esta función.

Identificaremos y aplicaremos las funciones y utilidades del software para 
tareas propias del área financiera más comunes
en el ámbito empresarial. 

Ejes Temáticos:

• Estructura y composición de una base de datos.
• Asistente para creación de tablas dinámicas.
• Campos, filtros, etiquetas y valores.
• Diseño de tabla dinámica.
• Herramientas de tabla.

Para mayor información, responder sobre este correo con la palabra EXCEL + los 
siguientes datos:

NOMBRE:

TELÉFONO:

EMPRESA: 

Llamanos al (045) 55 1554 6630
www.Innovalearn.mx 


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


Re: [ovs-dev] [ovs-dev, v12, 8/8] Userspace datapath: Add fragmentation handling.

2019-02-12 Thread Darrell Ball
On Tue, Feb 12, 2019 at 7:50 AM Ilya Maximets 
wrote:

> Not a full review. Just a few notes about docs.
> See inline.
>
> Best regards, Ilya Maximets.
>
> On 11.02.2019 5:56, Darrell Ball wrote:
> > Fragmentation handling is added for supporting conntrack.
> > Both v4 and v6 are supported.
> >
> > After discussion with several people, I decided to not store
> > configuration state in the database to be more consistent with
> > the kernel in future, similarity with other conntrack configuration
> > which will not be in the database as well and overall simplicity.
> > Accordingly, fragmentation handling is enabled by default.
> >
> > This patch enables fragmentation tests for the userspace datapath.
> >
> > Signed-off-by: Darrell Ball 
> > ---
> >  Documentation/faq/releases.rst   |   49 +-
> >  NEWS |   10 +
> >  include/sparse/netinet/ip6.h |1 +
> >  lib/automake.mk  |4 +-
> >  lib/conntrack.c  |   20 +-
> >  lib/conntrack.h  |4 +
> >  lib/ct-dpif.c|   58 +-
> >  lib/ct-dpif.h|   12 +-
> >  lib/dpctl.c  |  215 +-
> >  lib/dpctl.man|   36 +
> >  lib/dpif-netdev.c|   65 +-
> >  lib/dpif-netlink.c   |9 +-
> >  lib/dpif-provider.h  |   53 +-
> >  lib/ipf.c| 1579
> ++
> >  lib/ipf.h|   60 ++
> >  tests/system-kmod-macros.at  |   46 +-
> >  tests/system-traffic.at  |   51 +-
> >  tests/system-userspace-macros.at |  186 -
> >  18 files changed, 2378 insertions(+), 80 deletions(-)
> >  create mode 100644 lib/ipf.c
> >  create mode 100644 lib/ipf.h
> >
> > diff --git a/Documentation/faq/releases.rst
> b/Documentation/faq/releases.rst
> > index 86f09e6..4c5ca51 100644
> > --- a/Documentation/faq/releases.rst
> > +++ b/Documentation/faq/releases.rst
> > @@ -105,31 +105,30 @@ Q: Are all features available with all datapaths?
> >  The following table lists the datapath supported features from an
> Open
> >  vSwitch user's perspective.
> >
> > -= == == =
> ===
> > -Feature   Linux upstream Linux OVS tree Userspace
> Hyper-V
> > -= == == =
> ===
> > -NAT   4.6YESYes   NO
> > -Connection tracking   4.3YESPARTIAL
>  PARTIAL
> > -Tunnel - LISP NO YESNONO
> > -Tunnel - STT  NO YESNOYES
> > -Tunnel - GRE  3.11   YESYES   YES
> > -Tunnel - VXLAN3.12   YESYES   YES
> > -Tunnel - Geneve   3.18   YESYES   YES
> > -Tunnel - GRE-IPv6 4.18   YESYES   NO
> > -Tunnel - VXLAN-IPv6   4.3YESYES   NO
> > -Tunnel - Geneve-IPv6  4.4YESYES   NO
> > -Tunnel - ERSPAN   4.18   YESYES   NO
> > -Tunnel - ERSPAN-IPv6  4.18   YESYES   NO
> > -QoS - PolicingYESYESYES   NO
> > -QoS - Shaping YESYESNONO
> > -sFlow YESYESYES   NO
> > -IPFIX 3.10   YESYES   NO
> > -Set actionYESYESYES
>  PARTIAL
> > -NIC Bonding   YESYESYES   YES
> > -Multiple VTEPsYESYESYES   YES
> > -Meters4.15   YESYES   NO
> > -Conntrack zone limit  4.18   YESNONO
> > -= == == =
> ===
> > +== == == =
> ===
> > +FeatureLinux upstream Linux OVS tree Userspace
> Hyper-V
> > +== == == =
> ===
> > +Connection tracking 4.3YES  YES
>   YES
> > +Conntrack Fragment Reass.   4.3YES  YES
>   YES
> > +NAT 4.6YES  YES
>   NO
> > +Conntrack zone limit4.18   YES  NO
>  NO
> > +Tunnel - LISP   NO YES  NO
>  NO
> > +Tunnel - STTNO YES  NO
>  YES
> > +Tunnel - GRE3.11   YES  YES
>   YES
> > +Tunnel - VXLAN  3.12   YES  YES
>   YES
> > +Tunnel - Genev

Re: [ovs-dev] [PATCH V3 0/2] Do not rewrite fields with the same values as matched

2019-02-12 Thread Eli Britstein
ping

On 2/7/2019 6:44 PM, Eli Britstein wrote:
> Hi
>
> This patch set avoids unnecessary rewrite actions to fields with the
> same values as matched on.
>
> Patch 1 is a pre-step of generating ovs key fields macros
>
> Patch 2 avoids the unnecessary rewrites and adapts the tests accordingly
>
>
> Travis link:
> https://travis-ci.org/roidayan/ovs/builds/490090058
> Appvoyer link:
> https://ci.appveyor.com/project/roidayan/ovs/builds/22198603
>
> Thanks,
> Eli
>
>
> Eli Britstein (2):
>Makefiles: Generate datapath ovs key fields macros
>odp-util: Do not rewrite fields with the same values as matched
>
>   .gitignore  |   1 +
>   build-aux/extract-odp-netlink-xmacros-h |  55 
>   include/automake.mk |  11 +++-
>   lib/odp-util.c  | 110 
> +---
>   tests/mpls-xlate.at |   2 +-
>   tests/ofproto-dpif.at   |  14 ++--
>   6 files changed, 172 insertions(+), 21 deletions(-)
>   create mode 100755 build-aux/extract-odp-netlink-xmacros-h
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v1] doc: Add "Representors" topic document

2019-02-12 Thread Ian Stokes

On 2/12/2019 1:15 PM, Ilya Maximets wrote:

On 11.02.2019 3:01, Ophir Munk wrote:

This details how to configure representors ports.

Signed-off-by: Ophir Munk 
---
  Documentation/topics/dpdk/phy.rst | 80 +++
  1 file changed, 80 insertions(+)

diff --git a/Documentation/topics/dpdk/phy.rst 
b/Documentation/topics/dpdk/phy.rst
index 1470623..3792fde 100644
--- a/Documentation/topics/dpdk/phy.rst
+++ b/Documentation/topics/dpdk/phy.rst
@@ -219,6 +219,86 @@ For more information please refer to the `DPDK Port 
Hotplug Framework`__.
  
  __ http://dpdk.org/doc/guides/prog_guide/port_hotplug_framework.html#hotplug
  
+.. _representors:

+
+Representors
+
+
+DPDK representors enable configuring a phy port to a guest (VM) machine.
+
+OVS resides in the hypervisor which has one or more physical interfaces also
+known as the physical functions (PFs). If a PF supports SR-IOV it can be used
+to enable communication with the VMs via Virtual Functions (VFs).
+The VFs are virtual PCIe devices created from the physical Ethernet controller.
+
+DPDK models a physical interface as a rte device on top of which an eth
+device is created.
+DPDK (version 18.xx) introduced the representors eth devices.
+A representor device represents the VF eth device (VM side) on the hypervisor
+side and operates on top of a PF.
+Representors are multi devices created on top of one PF.
+
+For more information, refer to the `DPDK documentation`__.
+
+__ https://doc.dpdk.org/guides-18.11/prog_guide/switch_representation.html
+
+Prior to port representors there was a one-to-one relationship between the PF
+and the eth device. With port representors the relationship becomes one PF to
+many eth devices.
+In case of two representors ports, when one of the ports is closed - the PCI
+bus cannot be detached until the second representor port is closed as well.
+
+.. _representors-configuration:
+
+When configuring a PF-based port, OVS traditionally assigns the device PCI
+address in devargs. For an existing bridge called ``br0`` and PCI address
+``:08:00.0`` an ``add-port`` command is written as::
+
+$ ovs-vsctl add-port br0 dpdk-pf -- set Interface dpdk-pf type=dpdk \
+   options:dpdk-devargs=:08:00.0
+
+When configuring a VF-based port, DPDK uses an extended devargs syntax which
+has the following format::
+
+BDBF,representor=[]
+
+This syntax shows that a representor is an enumerated eth device (with
+a representor ID) which uses the PF PCI address.
+The following commands add representors 3 and 5 using PCI device address
+``:08:00.0``::
+
+$ ovs-vsctl add-port br0 dpdk-rep3 -- set Interface dpdk-rep3 type=dpdk \
+   options:dpdk-devargs=:08:00.0,representor=[3]
+
+$ ovs-vsctl add-port br0 dpdk-rep5 -- set Interface dpdk-rep5 type=dpdk \
+   options:dpdk-devargs=:08:00.0,representor=[5]
+
+.. important::
+
+   Representors ports are configured prior to OVS invocation and independently
+   of it, or by other means as well. Please consult a NIC vendor instructions
+   on how to establish representors.


It'll be good to have configuration example for at least one commonly used NIC
(ixgbe/i40e ?). Or maybe a link to the docs where the process described.

What do you think ?
Ian, maybe you could add some example, since you have already tried it in 
practice?



Good call, I'll draw up an incremental and post here, if acceptable we 
can roll it into the same patch.


Ian

To verify their correct configuration,
+   execute::
+
+$ ovs-vsctl show
+
+   and make sure no errors are indicated.
+
+.. _multi-dev-configuration:
+
+
+Port representors are an example of multi devices. There are NICs which support
+multi devices by other methods than representors for which a generic devargs
+syntax is used. The generic syntax is based on the device mac address::
+
+class=eth,mac=
+
+For example, the following command adds a port to a bridge called ``br0`` using
+an eth device whose mac address is ``00:11:22:33:44:55``::
+
+$ ovs-vsctl add-port br0 dpdk-mac -- set Interface dpdk-mac type=dpdk \
+   options:dpdk-devargs="class=eth,mac=00:11:22:33:44:55"
+
  Jumbo Frames
  
  



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


Re: [ovs-dev] [PATCH] acinclude: Check for rte_config.h before checking dependencies.

2019-02-12 Thread Ian Stokes

On 2/12/2019 1:19 PM, Ilya Maximets wrote:

Current ./configure script shows misleading errors in case of wrong
DPDK path:

   # ./configure --with-dpdk=/wrong/path
   ...
   checking whether dpdk datapath is enabled... yes
   checking for library containing get_mempolicy... -lnuma
   checking for library containing pcap_dump... -lpcap
   checking for library containing mnl_attr_put... no
   configure: error: unable to find libmnl, install the dependency package

This happens because we're not checking for headers before checking
for dependencies. All the compile attempts fails and script thinks
that we need more dependencies.

With this change script will check for 'rte_config.h' availability
and produce sane error message:

   # ./configure --with-dpdk=/wrong/path
   ...
   checking for rte_config.h... no
   configure: error: unable to find rte_config.h in /wrong/path

'AC_INCLUDES_DEFAULT' passed explicitly to avoid preprocessor test.

Signed-off-by: Ilya Maximets 
---


Makes sense, thanks for this Ilya, applied to master.

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


Re: [ovs-dev] [ovs-dev, PATCHv8, 1/3] Improved Packet Drop Statistics in OVS

2019-02-12 Thread Aaron Conole
Ilya Maximets  writes:

> On 12.02.2019 18:55, Aaron Conole wrote:
>> Ilya Maximets  writes:
>> 
>>> Aaron, ovsrobot doesn't check this patch for some reason.
>>> I suspect, it's because "PATCH" and "v8" are glued together.
>>> Could you, please, take a look.
>> 
>> It won't.  But not for that reason.  The patch info says that this is
>> patch 1 of 3.  But 2/3 and 3/3 haven't been posted.  So doing a pull
>> from patchwork states:
>> 
>>"total":3,"received_total":1,"received_all":false
>> 
>> Because the patchwork thinks it hasn't received all the patches, it
>> defers trying to process the series.  In this case, the submitter may
>> have intended to post 2 more patches that never hit the list.  Maybe we
>> can revisit the way the robot works now that it creates a special branch
>> for each series that can be modified.
>
> Thanks for checking. Did you thought about checking patches in a series one
> by one ?

We do.  But we don't start processing the series until all the patches
are received.  The reason is for ordering.  It's possible that we get
1/3 and 3/3, and if we start processing we will apply 1/3, but then 3/3
won't apply.  There could be more logic added to address this, but then
we'll need to re-start the whole monitor mechanism and tell it to run
all over again - and what happens if the patches *never* arrive... the
system will be spinning in loops.

> I mean that we definitely want all the patches to not break the
> build. For this purpose ovsrobot needs to apply patches one by one and
> check.

For each patch, we only check that the build doesn't break.  The full
make-check does take a long time to run, so I pushed it until the end of
the series.

> This way, I guess, there could be possible to avoid this kind of
> misunderstanding with subject prefixes as patch 1/3 should be checked first
> anyway. OTOH, this will increase testing time significantly.

See above for why it's more complex to do partial series.  Maybe there's
a better design?  Pull requests to https://github.com/orgcandman/pw-ci
are always welcome :)

>> 
>> Perhaps this should have been posted without the '1/3' series counter?
>
> Yes. I guess, Anju wanted this patch to be treated as a single patch.

I can manually kick it off if one is ever missing.  OTOH, since you've
already noted lots of problems, maybe it's best that Anju submit a v9
with the correct metadata. :)
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [ovs-dev, PATCHv8, 1/3] Improved Packet Drop Statistics in OVS

2019-02-12 Thread Ilya Maximets
On 12.02.2019 18:55, Aaron Conole wrote:
> Ilya Maximets  writes:
> 
>> Aaron, ovsrobot doesn't check this patch for some reason.
>> I suspect, it's because "PATCH" and "v8" are glued together.
>> Could you, please, take a look.
> 
> It won't.  But not for that reason.  The patch info says that this is
> patch 1 of 3.  But 2/3 and 3/3 haven't been posted.  So doing a pull
> from patchwork states:
> 
>"total":3,"received_total":1,"received_all":false
> 
> Because the patchwork thinks it hasn't received all the patches, it
> defers trying to process the series.  In this case, the submitter may
> have intended to post 2 more patches that never hit the list.  Maybe we
> can revisit the way the robot works now that it creates a special branch
> for each series that can be modified.

Thanks for checking. Did you thought about checking patches in a series one
by one ? I mean that we definitely want all the patches to not break the
build. For this purpose ovsrobot needs to apply patches one by one and check.
This way, I guess, there could be possible to avoid this kind of
misunderstanding with subject prefixes as patch 1/3 should be checked first
anyway. OTOH, this will increase testing time significantly.

> 
> Perhaps this should have been posted without the '1/3' series counter?

Yes. I guess, Anju wanted this patch to be treated as a single patch.

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


Re: [ovs-dev] [ovs-dev, PATCHv8, 1/3] Improved Packet Drop Statistics in OVS

2019-02-12 Thread Aaron Conole
Ilya Maximets  writes:

> Aaron, ovsrobot doesn't check this patch for some reason.
> I suspect, it's because "PATCH" and "v8" are glued together.
> Could you, please, take a look.

It won't.  But not for that reason.  The patch info says that this is
patch 1 of 3.  But 2/3 and 3/3 haven't been posted.  So doing a pull
from patchwork states:

   "total":3,"received_total":1,"received_all":false

Because the patchwork thinks it hasn't received all the patches, it
defers trying to process the series.  In this case, the submitter may
have intended to post 2 more patches that never hit the list.  Maybe we
can revisit the way the robot works now that it creates a special branch
for each series that can be modified.

Perhaps this should have been posted without the '1/3' series counter?

> Anju, patch breaks a lot of unit tests. ovs-vswitchd crashes with
> traces like this:
> ==21473== 
> ==21473== Thread 13 monitor11:
> ==21473== Invalid free() / delete / delete[] / realloc()
> ==21473==at 0x4C30D3B: free (in 
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==21473==by 0xA47F2F: dp_packet_delete (dp-packet.h:182)
> ==21473==by 0xA47F2F: dp_packet_delete_batch (dp-packet.h:852)
> ==21473==by 0xA47F2F: odp_execute_actions (odp-execute.c:1011)
> ==21473==by 0xA05857: dp_netdev_execute_actions (dpif-netdev.c:7311)
> ==21473==by 0xA05857: dpif_netdev_execute (dpif-netdev.c:3731)
> ==21473==by 0xA05857: dpif_netdev_operate (dpif-netdev.c:3762)
> ==21473==by 0xA166F5: dpif_operate (dpif.c:1363)
> ==21473==by 0xA16DC7: dpif_execute (dpif.c:1317)
> ==21473==by 0x9B9718: ofproto_dpif_execute_actions__ (ofproto-dpif.c:3990)
> ==21473==by 0x9B99E1: ofproto_dpif_execute_actions (ofproto-dpif.c:4007)
> ==21473==by 0x9DE9A1: xlate_send_packet (ofproto-dpif-xlate.c:7658)
> ==21473==by 0x9B9E44: ofproto_dpif_send_packet (ofproto-dpif.c:4980)
> ==21473==by 0x9CCF01: monitor_mport_run (ofproto-dpif-monitor.c:287)
> ==21473==by 0x9CCB3C: monitor_run (ofproto-dpif-monitor.c:227)
> ==21473==by 0x9CCB3C: monitor_main (ofproto-dpif-monitor.c:189)
> ==21473==by 0xA96201: ovsthread_wrapper (ovs-thread.c:353)
> ==21473==by 0x5B366DA: start_thread (pthread_create.c:463)
> ==21473==by 0x6A7E88E: clone (clone.S:95)
> ==21473==  Address 0xd21efc0 is on thread 13's stack
> ==21473==  in frame #10, created by monitor_main (ofproto-dpif-monitor.c:186)
>
> Before submitting patches to the list you may run 'make check' on your local
> machine to be sure that all the unit tests are OK.
>
> Some code related comments and build failure inline.
>
> Best regards, Ilya Maximets.
>
> On 08.02.2019 7:37, Anju Thomas wrote:
>> Currently OVS maintains explicit packet drop/error counters only on port
>> level. Packets that are dropped as part of normal OpenFlow processing are
>> counted in flow stats of “drop” flows or as table misses in table stats.
>> These can only be interpreted by controllers that know the semantics of
>> the configured OpenFlow pipeline. Without that knowledge, it is impossible
>> for an OVS user to obtain e.g. the total number of packets dropped due to
>> OpenFlow rules.
>> 
>> Furthermore, there are numerous other reasons for which packets can be
>> dropped by OVS slow path that are not related to the OpenFlow pipeline.
>> The generated datapath flow entries include a drop action to avoid further
>> expensive upcalls to the slow path, but subsequent packets dropped by the
>> datapath are not accounted anywhere.
>> 
>> Finally, the datapath itself drops packets in certain error situations.
>> Also, these drops are today not accounted for.
>> 
>> This makes it difficult for OVS users to monitor packet drop in an OVS
>> instance and to alert a management system in case of a unexpected increase
>> of such drops. Also OVS trouble-shooters face difficulties in analysing
>> packet drops.
>> 
>> With this patch we implement following changes to address the issues
>> mentioned above.
>> 
>> 1. Identify and account all the silent packet drop scenarios
>> 
>> 2. Display these drops in ovs-appctl coverage/show
>> 
>> A detailed presentation on this was presented at OvS conference 2017 and
>> link for the corresponding presentation is available at:
>> 
>> https://www.slideshare.net/LF_OpenvSwitch/lfovs17troubleshooting-the-data-plane-in-ovs-82280329
>> 
>> Co-authored-by: Rohith Basavaraja 
>> Co-authored-by: Keshav Gupta 
>> Signed-off-by: Anju Thomas 
>> Signed-off-by: Rohith Basavaraja 
>> Signed-off-by: Keshav Gupta 
>> ---
>>  datapath/linux/compat/include/linux/openvswitch.h |   1 +
>>  lib/dpif-netdev.c |  44 -
>>  lib/dpif.c|   7 +
>>  lib/dpif.h|   3 +
>>  lib/odp-execute.c |  81 +-
>>  lib/odp-util.c|   9 ++
>>  ofproto/ofproto-dpif-ipfix.c  | 

Re: [ovs-dev] [PATCH 1/8] travis: Fix building datapath instead of userspace with DPDK_SHARED.

2019-02-12 Thread Ian Stokes

On 2/8/2019 4:48 PM, Ilya Maximets wrote:

Current script does not check build of OVS with DPDK.
It builds datapath instead.

CC: Ian Stokes 
Fixes: edfe8d263d2e ("travis: Add dpdk shared library build.")
Signed-off-by: Ilya Maximets 
Acked-by: Aaron Conole 


Ben has applied this to master already but I've just backported this to 
OVS 2.11 also as it is of use there too.


Thanks
Ian

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


Re: [ovs-dev] [ovs-discuss] Geneve remote_ip as flow for OVN hosts

2019-02-12 Thread Venugopal Iyer
HI, Ben:


From: Ben Pfaff 
Sent: Monday, February 11, 2019 5:55 PM
To: Venugopal Iyer
Cc: Guru Shetty; Leonid Grossman; d...@openvswitch.org
Subject: Re: [ovs-discuss] [ovs-dev] Geneve remote_ip as flow for OVN hosts

On Mon, Feb 11, 2019 at 08:09:59PM +, Venugopal Iyer wrote:
> Of course we want users to upgrade the entire system.  We just need to
> make sure that it's possible to upgrade one piece at a time in an order
> that ensures that the system isn't broken by a partial upgrade.  The
> specified order for OVN is to upgrade the HVs first, then the central
> node.  (Although apparently some people want to do it in the other
> order, which is currently a problem.)
>
>  Thanks, I have updated the repo to squash all the commits and added a 
> high
>  level commit message. Please let me know if the message is helpful 
> and/or if
>  there are some best practices that I should follow. FYI, branch mvtep-br
>  @ https://github.com/iyervl/nv-ovs

Thanks for the revision.

It's not clear to me whether you believe that the upgrade compatibility
issue is fixed.  Is it?

 Sorry, it was not clear. Yes, it is fixed, specifically changes in
 ovn/controller/binding.c, L398-400, L413 and L547. Let me know
 if you have questions.

thanks,

-venu

Thanks,

Ben.

---
This email message is for the sole use of the intended recipient(s) and may 
contain
confidential information.  Any unauthorized review, use, disclosure or 
distribution
is prohibited.  If you are not the intended recipient, please contact the 
sender by
reply email and destroy all copies of the original message.
---
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [ovs-dev, v12, 8/8] Userspace datapath: Add fragmentation handling.

2019-02-12 Thread Ilya Maximets
Not a full review. Just a few notes about docs.
See inline.

Best regards, Ilya Maximets.

On 11.02.2019 5:56, Darrell Ball wrote:
> Fragmentation handling is added for supporting conntrack.
> Both v4 and v6 are supported.
> 
> After discussion with several people, I decided to not store
> configuration state in the database to be more consistent with
> the kernel in future, similarity with other conntrack configuration
> which will not be in the database as well and overall simplicity.
> Accordingly, fragmentation handling is enabled by default.
> 
> This patch enables fragmentation tests for the userspace datapath.
> 
> Signed-off-by: Darrell Ball 
> ---
>  Documentation/faq/releases.rst   |   49 +-
>  NEWS |   10 +
>  include/sparse/netinet/ip6.h |1 +
>  lib/automake.mk  |4 +-
>  lib/conntrack.c  |   20 +-
>  lib/conntrack.h  |4 +
>  lib/ct-dpif.c|   58 +-
>  lib/ct-dpif.h|   12 +-
>  lib/dpctl.c  |  215 +-
>  lib/dpctl.man|   36 +
>  lib/dpif-netdev.c|   65 +-
>  lib/dpif-netlink.c   |9 +-
>  lib/dpif-provider.h  |   53 +-
>  lib/ipf.c| 1579 
> ++
>  lib/ipf.h|   60 ++
>  tests/system-kmod-macros.at  |   46 +-
>  tests/system-traffic.at  |   51 +-
>  tests/system-userspace-macros.at |  186 -
>  18 files changed, 2378 insertions(+), 80 deletions(-)
>  create mode 100644 lib/ipf.c
>  create mode 100644 lib/ipf.h
> 
> diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst
> index 86f09e6..4c5ca51 100644
> --- a/Documentation/faq/releases.rst
> +++ b/Documentation/faq/releases.rst
> @@ -105,31 +105,30 @@ Q: Are all features available with all datapaths?
>  The following table lists the datapath supported features from an Open
>  vSwitch user's perspective.
>  
> -= == == = ===
> -Feature   Linux upstream Linux OVS tree Userspace Hyper-V
> -= == == = ===
> -NAT   4.6YESYes   NO
> -Connection tracking   4.3YESPARTIAL   PARTIAL
> -Tunnel - LISP NO YESNONO
> -Tunnel - STT  NO YESNOYES
> -Tunnel - GRE  3.11   YESYES   YES
> -Tunnel - VXLAN3.12   YESYES   YES
> -Tunnel - Geneve   3.18   YESYES   YES
> -Tunnel - GRE-IPv6 4.18   YESYES   NO
> -Tunnel - VXLAN-IPv6   4.3YESYES   NO
> -Tunnel - Geneve-IPv6  4.4YESYES   NO
> -Tunnel - ERSPAN   4.18   YESYES   NO
> -Tunnel - ERSPAN-IPv6  4.18   YESYES   NO
> -QoS - PolicingYESYESYES   NO
> -QoS - Shaping YESYESNONO
> -sFlow YESYESYES   NO
> -IPFIX 3.10   YESYES   NO
> -Set actionYESYESYES   PARTIAL
> -NIC Bonding   YESYESYES   YES
> -Multiple VTEPsYESYESYES   YES
> -Meters4.15   YESYES   NO
> -Conntrack zone limit  4.18   YESNONO
> -= == == = ===
> +== == == = 
> ===
> +FeatureLinux upstream Linux OVS tree Userspace 
> Hyper-V
> +== == == = 
> ===
> +Connection tracking 4.3YES  YES  YES
> +Conntrack Fragment Reass.   4.3YES  YES  YES
> +NAT 4.6YES  YES  NO
> +Conntrack zone limit4.18   YES  NO   NO
> +Tunnel - LISP   NO YES  NO   NO
> +Tunnel - STTNO YES  NO   YES
> +Tunnel - GRE3.11   YES  YES  YES
> +Tunnel - VXLAN  3.12   YES  YES  YES
> +Tunnel - Geneve 3.18   YES  YES  YES
> +Tunnel - GRE-IPv6   NO NO   YES  NO
> +Tunnel - VXLAN-IPv6 4.3YES  YES   

[ovs-dev] ABOUT YOUR PARCEL,,

2019-02-12 Thread DHL Regional headquarters
-- 
Attention:

We wish to draw your attention concerning your unclaimed parcel
registered and deposited with our regional headquarters in London.

You are to re-confirm your correct names,receiving address and a valid
telephone number to avoid sending the consignment to a wrong designate
because Our collecting data could not authenticate Your correct
details hence urgent reply is highly needed in order to facilitate and
expedite the process of shipment to you.

Your's in service,
Mr.Warren Blair
DHL Regional Headquarters
London,UK.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] python: Fix E117 over-indented.

2019-02-12 Thread Ilya Maximets
New check was added to recent pycodestyle-2.5.0 and flake8
complains while building on Travis:

  ../utilities/bugtool/ovs-bugtool.in:767:17: E117 over-indented
  ../utilities/bugtool/ovs-bugtool.in:771:17: E117 over-indented
  ../utilities/bugtool/ovs-bugtool.in:774:17: E117 over-indented
  ../utilities/bugtool/ovs-bugtool.in:778:17: E117 over-indented
  ../python/ovs/db/error.py:33:17: E117 over-indented
  ../python/ovs/poller.py:118:21: E117 over-indented
  ../python/ovs/reconnect.py:244:17: E117 over-indented

Signed-off-by: Ilya Maximets 
---
 python/ovs/db/error.py   |  2 +-
 python/ovs/poller.py |  2 +-
 python/ovs/reconnect.py  |  2 +-
 utilities/bugtool/ovs-bugtool.in | 16 
 4 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/python/ovs/db/error.py b/python/ovs/db/error.py
index d9217e411..4d192839b 100644
--- a/python/ovs/db/error.py
+++ b/python/ovs/db/error.py
@@ -30,5 +30,5 @@ class Error(Exception):
 # Compose message.
 syntax = ""
 if self.json is not None:
-syntax = 'syntax "%s": ' % ovs.json.to_string(self.json)
+syntax = 'syntax "%s": ' % ovs.json.to_string(self.json)
 Exception.__init__(self, "%s%s: %s" % (syntax, self.tag, self.msg))
diff --git a/python/ovs/poller.py b/python/ovs/poller.py
index 9c6892d98..3624ec865 100644
--- a/python/ovs/poller.py
+++ b/python/ovs/poller.py
@@ -115,7 +115,7 @@ class _SelectSelect(object):
 False,  # Wait all
 timeout)
 except winutils.pywintypes.error:
-return [(0, POLLERR)]
+return [(0, POLLERR)]
 
 if retval == winutils.winerror.WAIT_TIMEOUT:
 return []
diff --git a/python/ovs/reconnect.py b/python/ovs/reconnect.py
index 34cc76987..4392bcea1 100644
--- a/python/ovs/reconnect.py
+++ b/python/ovs/reconnect.py
@@ -241,7 +241,7 @@ class Reconnect(object):
 
 if (self.state == Reconnect.Backoff and
 self.backoff > self.max_backoff):
-self.backoff = self.max_backoff
+self.backoff = self.max_backoff
 
 def set_backoff_free_tries(self, backoff_free_tries):
 """Sets the number of connection attempts that will be made without
diff --git a/utilities/bugtool/ovs-bugtool.in b/utilities/bugtool/ovs-bugtool.in
index 288c34f98..f3657dc96 100755
--- a/utilities/bugtool/ovs-bugtool.in
+++ b/utilities/bugtool/ovs-bugtool.in
@@ -764,18 +764,18 @@ def dump_scsi_hosts(cap):
 for h in scsi_list:
 procname = ''
 try:
-f = open('/sys/class/scsi_host/%s/proc_name' % h)
-procname = f.readline().strip("\n")
-f.close()
+f = open('/sys/class/scsi_host/%s/proc_name' % h)
+procname = f.readline().strip("\n")
+f.close()
 except:
-pass
+pass
 modelname = None
 try:
-f = open('/sys/class/scsi_host/%s/model_name' % h)
-modelname = f.readline().strip("\n")
-f.close()
+f = open('/sys/class/scsi_host/%s/model_name' % h)
+modelname = f.readline().strip("\n")
+f.close()
 except:
-pass
+pass
 
 output += "%s:\n" % h
 output += "%s%s\n" \
-- 
2.17.1

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


Re: [ovs-dev] [ovs-dev, PATCHv8, 1/3] Improved Packet Drop Statistics in OVS

2019-02-12 Thread Ilya Maximets
Aaron, ovsrobot doesn't check this patch for some reason.
I suspect, it's because "PATCH" and "v8" are glued together.
Could you, please, take a look.

Anju, patch breaks a lot of unit tests. ovs-vswitchd crashes with
traces like this:
==21473== 
==21473== Thread 13 monitor11:
==21473== Invalid free() / delete / delete[] / realloc()
==21473==at 0x4C30D3B: free (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==21473==by 0xA47F2F: dp_packet_delete (dp-packet.h:182)
==21473==by 0xA47F2F: dp_packet_delete_batch (dp-packet.h:852)
==21473==by 0xA47F2F: odp_execute_actions (odp-execute.c:1011)
==21473==by 0xA05857: dp_netdev_execute_actions (dpif-netdev.c:7311)
==21473==by 0xA05857: dpif_netdev_execute (dpif-netdev.c:3731)
==21473==by 0xA05857: dpif_netdev_operate (dpif-netdev.c:3762)
==21473==by 0xA166F5: dpif_operate (dpif.c:1363)
==21473==by 0xA16DC7: dpif_execute (dpif.c:1317)
==21473==by 0x9B9718: ofproto_dpif_execute_actions__ (ofproto-dpif.c:3990)
==21473==by 0x9B99E1: ofproto_dpif_execute_actions (ofproto-dpif.c:4007)
==21473==by 0x9DE9A1: xlate_send_packet (ofproto-dpif-xlate.c:7658)
==21473==by 0x9B9E44: ofproto_dpif_send_packet (ofproto-dpif.c:4980)
==21473==by 0x9CCF01: monitor_mport_run (ofproto-dpif-monitor.c:287)
==21473==by 0x9CCB3C: monitor_run (ofproto-dpif-monitor.c:227)
==21473==by 0x9CCB3C: monitor_main (ofproto-dpif-monitor.c:189)
==21473==by 0xA96201: ovsthread_wrapper (ovs-thread.c:353)
==21473==by 0x5B366DA: start_thread (pthread_create.c:463)
==21473==by 0x6A7E88E: clone (clone.S:95)
==21473==  Address 0xd21efc0 is on thread 13's stack
==21473==  in frame #10, created by monitor_main (ofproto-dpif-monitor.c:186)

Before submitting patches to the list you may run 'make check' on your local
machine to be sure that all the unit tests are OK.

Some code related comments and build failure inline.

Best regards, Ilya Maximets.

On 08.02.2019 7:37, Anju Thomas wrote:
> Currently OVS maintains explicit packet drop/error counters only on port
> level. Packets that are dropped as part of normal OpenFlow processing are
> counted in flow stats of “drop” flows or as table misses in table stats.
> These can only be interpreted by controllers that know the semantics of
> the configured OpenFlow pipeline. Without that knowledge, it is impossible
> for an OVS user to obtain e.g. the total number of packets dropped due to
> OpenFlow rules.
> 
> Furthermore, there are numerous other reasons for which packets can be
> dropped by OVS slow path that are not related to the OpenFlow pipeline.
> The generated datapath flow entries include a drop action to avoid further
> expensive upcalls to the slow path, but subsequent packets dropped by the
> datapath are not accounted anywhere.
> 
> Finally, the datapath itself drops packets in certain error situations.
> Also, these drops are today not accounted for.
> 
> This makes it difficult for OVS users to monitor packet drop in an OVS
> instance and to alert a management system in case of a unexpected increase
> of such drops. Also OVS trouble-shooters face difficulties in analysing
> packet drops.
> 
> With this patch we implement following changes to address the issues
> mentioned above.
> 
> 1. Identify and account all the silent packet drop scenarios
> 
> 2. Display these drops in ovs-appctl coverage/show
> 
> A detailed presentation on this was presented at OvS conference 2017 and
> link for the corresponding presentation is available at:
> 
> https://www.slideshare.net/LF_OpenvSwitch/lfovs17troubleshooting-the-data-plane-in-ovs-82280329
> 
> Co-authored-by: Rohith Basavaraja 
> Co-authored-by: Keshav Gupta 
> Signed-off-by: Anju Thomas 
> Signed-off-by: Rohith Basavaraja 
> Signed-off-by: Keshav Gupta 
> ---
>  datapath/linux/compat/include/linux/openvswitch.h |   1 +
>  lib/dpif-netdev.c |  44 -
>  lib/dpif.c|   7 +
>  lib/dpif.h|   3 +
>  lib/odp-execute.c |  81 +-
>  lib/odp-util.c|   9 ++
>  ofproto/ofproto-dpif-ipfix.c  |   1 +
>  ofproto/ofproto-dpif-sflow.c  |   1 +
>  ofproto/ofproto-dpif-xlate.c  | 101 +++-
>  ofproto/ofproto-dpif-xlate.h  |   3 +
>  ofproto/ofproto-dpif.c|   8 +
>  ofproto/ofproto-dpif.h|   7 +-
>  tests/automake.mk |   3 +-
>  tests/dpif-netdev.at  |   8 +
>  tests/drop-stats.at   | 189 
> ++
>  tests/ofproto-dpif.at |   2 +-
>  tests/testsuite.at|   1 +
>  tests/tunnel-push-pop.at  |  28 +++-
>  tests/tunnel.at

Re: [ovs-dev] [PATCH] dpif-netdev: Add thread safety annotation to sorted_poll_list.

2019-02-12 Thread Ian Stokes

On 2/11/2019 6:32 PM, Kevin Traynor wrote:

On 02/11/2019 05:35 PM, Ilya Maximets wrote:

'sorted_poll_list()' uses the 'pmd->poll_list' that should be
guarded by 'pmd->port_mutex'.

Signed-off-by: Ilya Maximets 
---


LGTM

Acked-by: Kevin Traynor 



Thanks, applied to master and backported back to OVS 2.7.

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


[ovs-dev] [PATCH] acinclude: Check for rte_config.h before checking dependencies.

2019-02-12 Thread Ilya Maximets
Current ./configure script shows misleading errors in case of wrong
DPDK path:

  # ./configure --with-dpdk=/wrong/path
  ...
  checking whether dpdk datapath is enabled... yes
  checking for library containing get_mempolicy... -lnuma
  checking for library containing pcap_dump... -lpcap
  checking for library containing mnl_attr_put... no
  configure: error: unable to find libmnl, install the dependency package

This happens because we're not checking for headers before checking
for dependencies. All the compile attempts fails and script thinks
that we need more dependencies.

With this change script will check for 'rte_config.h' availability
and produce sane error message:

  # ./configure --with-dpdk=/wrong/path
  ...
  checking for rte_config.h... no
  configure: error: unable to find rte_config.h in /wrong/path

'AC_INCLUDES_DEFAULT' passed explicitly to avoid preprocessor test.

Signed-off-by: Ilya Maximets 
---
 acinclude.m4 | 4 
 1 file changed, 4 insertions(+)

diff --git a/acinclude.m4 b/acinclude.m4
index e2af4ee16..88d80522c 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -250,6 +250,10 @@ AC_DEFUN([OVS_CHECK_DPDK], [
   LDFLAGS="$LDFLAGS -L${DPDK_LIB_DIR}"
 fi
 
+AC_CHECK_HEADERS([rte_config.h], [], [
+  AC_MSG_ERROR([unable to find rte_config.h in $with_dpdk])
+], [AC_INCLUDES_DEFAULT])
+
 AC_COMPILE_IFELSE([
   AC_LANG_PROGRAM(
 [
-- 
2.17.1

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


Re: [ovs-dev] [PATCH v1] doc: Add "Representors" topic document

2019-02-12 Thread Ilya Maximets
On 11.02.2019 3:01, Ophir Munk wrote:
> This details how to configure representors ports.
> 
> Signed-off-by: Ophir Munk 
> ---
>  Documentation/topics/dpdk/phy.rst | 80 
> +++
>  1 file changed, 80 insertions(+)
> 
> diff --git a/Documentation/topics/dpdk/phy.rst 
> b/Documentation/topics/dpdk/phy.rst
> index 1470623..3792fde 100644
> --- a/Documentation/topics/dpdk/phy.rst
> +++ b/Documentation/topics/dpdk/phy.rst
> @@ -219,6 +219,86 @@ For more information please refer to the `DPDK Port 
> Hotplug Framework`__.
>  
>  __ http://dpdk.org/doc/guides/prog_guide/port_hotplug_framework.html#hotplug
>  
> +.. _representors:
> +
> +Representors
> +
> +
> +DPDK representors enable configuring a phy port to a guest (VM) machine.
> +
> +OVS resides in the hypervisor which has one or more physical interfaces also
> +known as the physical functions (PFs). If a PF supports SR-IOV it can be used
> +to enable communication with the VMs via Virtual Functions (VFs).
> +The VFs are virtual PCIe devices created from the physical Ethernet 
> controller.
> +
> +DPDK models a physical interface as a rte device on top of which an eth
> +device is created.
> +DPDK (version 18.xx) introduced the representors eth devices.
> +A representor device represents the VF eth device (VM side) on the hypervisor
> +side and operates on top of a PF.
> +Representors are multi devices created on top of one PF.
> +
> +For more information, refer to the `DPDK documentation`__.
> +
> +__ https://doc.dpdk.org/guides-18.11/prog_guide/switch_representation.html
> +
> +Prior to port representors there was a one-to-one relationship between the PF
> +and the eth device. With port representors the relationship becomes one PF to
> +many eth devices.
> +In case of two representors ports, when one of the ports is closed - the PCI
> +bus cannot be detached until the second representor port is closed as well.
> +
> +.. _representors-configuration:
> +
> +When configuring a PF-based port, OVS traditionally assigns the device PCI
> +address in devargs. For an existing bridge called ``br0`` and PCI address
> +``:08:00.0`` an ``add-port`` command is written as::
> +
> +$ ovs-vsctl add-port br0 dpdk-pf -- set Interface dpdk-pf type=dpdk \
> +   options:dpdk-devargs=:08:00.0
> +
> +When configuring a VF-based port, DPDK uses an extended devargs syntax which
> +has the following format::
> +
> +BDBF,representor=[]
> +
> +This syntax shows that a representor is an enumerated eth device (with
> +a representor ID) which uses the PF PCI address.
> +The following commands add representors 3 and 5 using PCI device address
> +``:08:00.0``::
> +
> +$ ovs-vsctl add-port br0 dpdk-rep3 -- set Interface dpdk-rep3 type=dpdk \
> +   options:dpdk-devargs=:08:00.0,representor=[3]
> +
> +$ ovs-vsctl add-port br0 dpdk-rep5 -- set Interface dpdk-rep5 type=dpdk \
> +   options:dpdk-devargs=:08:00.0,representor=[5]
> +
> +.. important::
> +
> +   Representors ports are configured prior to OVS invocation and 
> independently
> +   of it, or by other means as well. Please consult a NIC vendor instructions
> +   on how to establish representors. 

It'll be good to have configuration example for at least one commonly used NIC
(ixgbe/i40e ?). Or maybe a link to the docs where the process described.

What do you think ?
Ian, maybe you could add some example, since you have already tried it in 
practice?

> To verify their correct configuration,
> +   execute::
> +
> +$ ovs-vsctl show
> +
> +   and make sure no errors are indicated.
> +
> +.. _multi-dev-configuration:
> +
> +
> +Port representors are an example of multi devices. There are NICs which 
> support
> +multi devices by other methods than representors for which a generic devargs
> +syntax is used. The generic syntax is based on the device mac address::
> +
> +class=eth,mac=
> +
> +For example, the following command adds a port to a bridge called ``br0`` 
> using
> +an eth device whose mac address is ``00:11:22:33:44:55``::
> +
> +$ ovs-vsctl add-port br0 dpdk-mac -- set Interface dpdk-mac type=dpdk \
> +   options:dpdk-devargs="class=eth,mac=00:11:22:33:44:55"
> +
>  Jumbo Frames
>  
>  
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] ORDER CONFIRMATION REF. N0. QP214.R8/PO 18-049 dated 12th march 2018

2019-02-12 Thread Asti Swastiani
Date : 12th Feb 2019 Ref. No.:   190159 / QP214.R8/PO /PO 12-03-2018
  

 
  Order Confirmation

   

 Dear Sir,

Good day from  PT Mignon Sista International
We refer to your email dated 12th oct, 2018.

 Please find attached our order confirmation for you. this is our 1st order (I 
hope there will be much more during our cooperation).
 
 
 Please provide us stamped and signed copy of proforma invoice for payment 
purposes.
 

 
  

  Best Regards,
  
 Asti Swastiani
 Sales International
  
  
 PT Mignon Sista International
 Jl. Raya GBHN No.120,Bojong Nangka
 Kab. Bogor 16963
 Phone : +62 21- 86863238
 Mobile: +62  82219148040
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] acinclude: Use NUMA_AWARE_HUGEPAGES too for libnuma check.

2019-02-12 Thread Ian Stokes

On 2/8/2019 2:28 PM, Ilya Maximets wrote:

On 07.02.2019 19:26, Ilya Maximets wrote:

On 07.02.2019 19:18, Ian Stokes wrote:

On 2/7/2019 1:00 PM, Ilya Maximets wrote:

This fixes build with NUMA_AWARE_HUGEPAGES enabled and VHOST_NUMA
disabled. This should not be a usual case. But it's possible to
configure DPDK this way.



Out of interest, with RTE_EAL_NUMA_AWARE_HUGEPAGES defined but not 
RTE_LIBRTE_VHOST_NUMA, does vhost numa still work as expected?


If RTE_LIBRTE_VHOST_NUMA disabled, all the numa related code from
librte_vhost will be compiled out. So, rte_vhost_get_numa_node()
will always return -1. i.e. we will not be able to reallocate
memory pools according to numa node where VM started.

At the same time eal_memory and eal_memalloc modules will work fine
allocating hugepages and memory from the requested numa nodes.



There are few realistic examples for that case:
   1. User builds dpdk without vhost library because there is no plan to
  run VMs on target platform. (Some HW switch or edge node).
   2. User builds dpdk without vhost library because there is a plan to
  use SR-IOV with full HW offloading and port representors. No
  vhost ports planned. (Not possible with upstream OVS, but patches
  are already on a list).
  


Thanks for the clarification.

I've pushed to master and backported to OVS 2.9.

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


Re: [ovs-dev] [RFC v3 1/5] datapath: Add a new action check_pkt_len

2019-02-12 Thread Numan Siddique
Thanks Ben and Lorenzo for the review and comments.

Few comments inline.


On Tue, Feb 12, 2019 at 7:57 AM Ben Pfaff  wrote:

> Greg, I recommend that you take a look at this one.
>
> (More commentary below.)
>
> On Thu, Jan 10, 2019 at 11:29:48PM +0530, nusid...@redhat.com wrote:
> > From: Numan Siddique 
> >
> > [Please note, this patch is submitted as RFC in ovs-dev ML to
> > get feedback before submitting to netdev ML]
> >
> > This patch adds a new action which checks the packet length and
> > executes a set of actions if the packet length is greater than
> > the specified length or executes another set of actions if the
> > packet length is lesser or equal to.
> >
> > This action takes below nlattrs
> >   * OVS_CHECK_PKT_LEN_ATTR_PKT_LEN - 'pkt_len' to check for
> >
> >   * OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER (optional) - Nested actions
> > to apply if the packet length is greater than the specified 'pkt_len'
> >
> >   * OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL (optional) - Nested
> > actions to apply if the packet length is lesser or equal to the
> > specified 'pkt_len'.
> >
> > The main use case for adding this action is to solve the packet
> > drops because of MTU mismatch in OVN virtual networking solution.
> > When a VM (which belongs to a logical switch of OVN) sends a packet
> > destined to go via the gateway router and if the nic which provides
> > external connectivity, has a lesser MTU, OVS drops the packet
> > if the packet length is greater than this MTU.
> >
> > With the help of this action, OVN will check the packet length
> > and if it is greater than the MTU size, it will generate an
> > ICMP packet (type 3, code 4) and includes the next hop mtu in it
> > so that the sender can fragment the packets.
>
> I'm not used to seeing series where different patches apply to different
> repos!  It took me a while to figure out why "git am" didn't want to
> work at all (but then I actually looked at the patch).
>

Thanks for looking at the patch. The main goal of including this datapath
patch
in this series is to get initial feedback about the overall approach and get
comments from the ovs datapath developers before submitting the patch to
net-next ML.
I mentioned this in RFC v3 0/5 patch. I think patchwork doesn't show  the
patch 0.



> I didn't have a net-next tree handy so my comments are based on reading
> the patch without applying or building it.
>
> There is a special issue with this action, which is a lot like a special
> issue with the "sample".  That is that the action has flow control that
> might change the flow in different ways, and this can make an important
> difference for actions that follow this one.  If one fork of the action
> pops off a header, and the other one does not, then that makes
> validating actions that come after it complicated.  I do not see
> anything here that takes that into account.  I recommend reading the
> validation code for the sample action for inspiration.
>
>
Sure. I will look into it.


> In validate_and_copy_check_pkt_len(), it might be better to use
> nla_policy and nla_parse_nested().  Or maybe not.  I did not look
> closely.
>

Ack.


>
> I don't think that execute_check_pkt_len() should need to check for
> netlink format errors.  The validation code should take care of that.
> It might also be able to assume a particular order for the attributes,
> depending on how you implement validation.
>

Ack.


>
> I'm still not thrilled by the naming.  I don't have any wonderful names
> though.
>

I am sorry. I couldn't think of any other name.

Does the action - "cpl_execute" or "cpl_exec" sounds a little bit better ?
cpl = check pkt len.

Thanks again for looking into the patch series providing the comments.

I will wait for Greg's comments. And then if it's fine, I will address the
commetns
and  will submit the patch to the net-next ML as a formal patch.

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