Re: [ovs-dev] gateway logic question (was: Re: [PATCH v2 1/5] ovsdb-idl: Redesign use of indexes.)

[Han Zhou]

I think it may be a corner case, since in most cases we just need one
gateway port per logical router. And for my understanding OpenStack doesn't
have the API to set more than one gateway ports for a logical router, so at
least it should be no issue in that set up.

On Tue, Jun 12, 2018 at 9:39 AM, Guru Shetty  wrote:
>
> There are 2 types of gateways in OVN - a "gateway router" and a
> "distributed gateway router port". The latter is where BFD is used and has
> been mostly been maintained by the OpenStack folks. I am adding the
> original authors for comment. I am not very familiar with the latter
> implementation.
>
> On 12 June 2018 at 08:23, Ben Pfaff  wrote:
>
> > Hi Guru, Han raised the following question while reading a patch of
> > mine.  Will you give your opinion?
> >
> > On Mon, Jun 11, 2018 at 07:17:13PM -0700, Han Zhou wrote:
> > > > +static struct ovs_list *
> > > > +bfd_find_ha_gateway_chassis(
> > > > +struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
> > > > +const struct chassis_index *chassis_index,
> > > > +const struct sbrec_datapath_binding *datapath)
> > > > +{
> > > > +struct sbrec_port_binding *target =
> > > sbrec_port_binding_index_init_row(
> > > > +sbrec_port_binding_by_datapath);
> > > > +sbrec_port_binding_index_set_datapath(target, datapath);
> > > > +
> > > > +struct ovs_list *ha_gateway_chassis = NULL;
> > > > +const struct sbrec_port_binding *pb;
> > > > +SBREC_PORT_BINDING_FOR_EACH_EQUAL (pb, target,
> > > > +
sbrec_port_binding_by_datapath)
> > {
> > > > +if (strcmp(pb->type, "chassisredirect")) {
> > > > +continue;
> > > > +}
> > > > +
> > > > +struct ovs_list *gateway_chassis =
> > gateway_chassis_get_ordered(
> > > > +pb, chassis_index);
> > > > +if (!gateway_chassis ||
ovs_list_is_short(gateway_chassis)) {
> > > > +/* We don't need BFD for non-HA chassisredirect. */
> > > > +gateway_chassis_destroy(gateway_chassis);
> > > > +continue;
> > > > +}
> > > > +
> > > > +ha_gateway_chassis = gateway_chassis;
> > > > +break;
> > > > +}
> > > > +sbrec_port_binding_index_destroy_row(target);
> > > > +return ha_gateway_chassis;
> > > > +}
> > >
> > > This is a good refactoring and it is functionally equal to the
original
> > > one, but I wonder if there is a problem even with the original logic.
It
> > > breaks out whenever the first logical router port is found with
multiple
> > > gateways chassises, but what if there are still some other logical
router
> > > ports on the same logical router are gateway ports and on different
> > > gateways, would those gateways be missed in the bfd sessions? (Of
course,
> > > it shouldn't belong to this patch even if it is a real problem.)
> >
> > Thanks,
> >
> > Ben.
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] [RFC] ovn-controller: Experiment with restricting access to columns.

[Han Zhou]

On Wed, Jun 13, 2018 at 3:37 PM, Ben Pfaff  wrote:
>
> To make ovn-controller recompute incrementally, we need accurate
> dependencies for each function that reads or writes a table.  It's
> currently difficult to be sure about these dependencies, and certainly
> difficult to maintain them over time, because there's no way to actually
> enforce them.
>
> This commit experiments with an approach that allows for fairly
> fine-grained access control within ovn-controller to tables and columns.
> It's based on generating a new version of the IDL data structures for each
> case where we want different access control.  All of these data structures
> have the same format, but the columns that a given piece of code is not
> supposed to touch are renamed to discourage programmers from using them,
> e.g. they're given names suffixed with "__accessdenied".  (This means
> that there is no runtime overhead to the access control since it only
> requires a cast to convert between the regular and restricted versions.)
> In addition, when a columns is supposed to be read-only, functions for
> modifying the column are not supplied.
>
> This commit only tries out this experiment for a single file within
> ovn-controller, the BFD implementation (mostly because that's
> alphabetically first, no other real reason).  It would require a little
> more work to apply it everywhere, but it's probably not a huge deal.
>
> Comments?
>
> CC: Han Zhou 
> Signed-off-by: Ben Pfaff 
> ---
>  ovn/controller/automake.mk |   5 +
>  ovn/controller/bfd-vswitch-idl.def |  21 
>  ovn/controller/bfd.c   |  20 ++--
>  ovn/controller/bfd.h   |   8 +-
>  ovn/controller/ovn-controller.c|  13 ++-
>  ovsdb/ovsdb-idlc.in| 223 ++
++-
>  6 files changed, 268 insertions(+), 22 deletions(-)
>  create mode 100644 ovn/controller/bfd-vswitch-idl.def
>

I wanted to have a quick test but it didn't pass the compile:
In file included from ovn/controller/bfd.c:17:0:
ovn/controller/bfd.h:19:44: fatal error: ovn/controller/bfd-vswitch-idl.h:
No such file or directory

I didn't debug, but by looking at the code I got the idea. It ensures the
purpose is declared through the generated data type and violations are
captured at compile time.
I think this is a brilliant way to achieve the check with such a small
change, however, I am not sure how is it supposed to help for generating
the dependency. I think instead of caring about whether it is read-only, we
should care about whether it is write-only.

For example, a engine node A reads on Table T1's column C1, reads & writes
on Table T2's column C2, and writes on T3's C3. In this case A depends on
T1 and T2, but not depends on T3.
So I think what matters to the dependency generation is whether it is
Write-Only. Read-Only is no different from ReadWrite.

If my understanding is correct (that we don't care about the difference
between RO and RW), for WO columns, we can simply just don't monitor its
change. Then we don't even need this feature from dependency generation
point of view.
Did I miss something?

Another thing, however, I think is really important for the dependency
generation is the columns that reference other tables, e.g. bfd_run()
doesn't has table "ovsrec_port" as input, but in fact it depends on this
table by referencing from ovsrec_bridge->ports. We need the table being
referenced appear in the input parameters.
The approach in the patch may not solve that problem directly, but
something similar may work: we could declare the usage for a referenced
column as "direct" or "indirect". "indirect" means the column can be
accessed only with "get functions" instead of pointers. The "get functions"
requires an additional parameter that is the instance of the table being
referenced. To call the "get function" to access the column, the caller
function needs to have the table being referenced passed in as parameter,
so we will be able to catch the dependency. I am not sure if this approach
is too heavy, or is tricky to implement.

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

Re: [ovs-dev] [PATCH] vxlan: Fix for the packet loop issue in vxlan

[Pravin Shelar]

On Wed, Jun 13, 2018 at 5:16 AM, Neelakantam Gaddam
 wrote:
> It should be Qdisc's busylock in dev_queue_xmit function. I have seen the
> issue in kernel version is 3.10.87 vanilla.
>

I see, This looks like general problem which exist in upstream kernel.
You would be able to reproduce it even with linux bridge and vxlan
device.
Proposed patch is specific solution. You can add another layer of
bridge and the patch would not handle same issue in that
configuration. Therefore I am bit hesitant to apply this patch.

>
>
> On Wed, Jun 13, 2018 at 12:21 PM, Pravin Shelar  wrote:
>>
>> On Tue, Jun 12, 2018 at 10:58 PM, Neelakantam Gaddam
>>  wrote:
>> > Hi Pravin,
>> >
>> > I have seen the below crash.
>> >
>> > [] show_stack+0x6c/0xf8
>> >
>> > [] do_raw_spin_lock+0x168/0x170
>> >
>> > [] dev_queue_xmit+0x43c/0x470
>> >
>> > [] ip_finish_output+0x250/0x490
>> >
>> > [] rpl_iptunnel_xmit+0x134/0x218 [openvswitch]
>> >
>> > [] rpl_vxlan_xmit+0x430/0x538 [openvswitch]
>> >
>> > [] do_execute_actions+0x18f8/0x19e8 [openvswitch]
>> >
>> > [] ovs_execute_actions+0x90/0x208 [openvswitch]
>> >
>> > [] ovs_dp_process_packet+0xb0/0x1a8 [openvswitch]
>> >
>> > [] ovs_vport_receive+0x78/0x130 [openvswitch]
>> >
>> > [] internal_dev_xmit+0x34/0x98 [openvswitch]
>> >
>> > [] dev_hard_start_xmit+0x2e8/0x4f8
>> >
>> > [] sch_direct_xmit+0xf0/0x238
>> >
>> > [] dev_queue_xmit+0x1d8/0x470
>> >
>> > [] arp_process+0x614/0x628
>> >
>> > [] __netif_receive_skb_core+0x2e8/0x5d8
>> >
>> > [] process_backlog+0xc0/0x1b0
>> >
>> > [] net_rx_action+0x154/0x240
>> >
>> > [] __do_softirq+0x1d0/0x218
>> >
>> > [] do_softirq+0x68/0x70
>> >
>> > [] local_bh_enable+0xa8/0xb0
>> >
>> > [] netif_rx_ni+0x20/0x30
>> >
>> >
>> >
>> >
>> > I have spent some time in investigation and found that crash is because
>> > of
>> > spinlock recursion in dev_queue_xmit function.
>> > The packet path traced is : netif_rx->arp->dev_queue_xmit(internal
>> > port)->vxlan_xmit->dev_queue_xmit(internal port).
>> >
>>
>> Which spin-lock is it? I am surprised to see a lock taken in fast
>> path. Can you also share kernel version?
>>
>> > The macro (XMIT_RECURSION_LIMIT) is defined as 10. This limit wont
>> > prevent
>> > the crash since the recursion is 2 only for my configuration.
>>
>> right, The recursion limit is to avoid stack overflow.
>>
>> >
>> >
>> >
>> > On Wed, Jun 13, 2018 at 4:11 AM, Pravin Shelar  wrote:
>> >>
>> >> On Tue, Jun 12, 2018 at 10:11 AM, Neelakantam Gaddam
>> >>  wrote:
>> >> >
>> >> > Hi Pravin,
>> >> >
>> >> > The below configuration is causing the spinlock recursion issue.
>> >> >
>> >> > I am able to see the issue with below configuration.
>> >> >
>> >> >
>> >> >
>> >> > ovs-vsctl add-br br0
>> >> >
>> >> > ovs-vsctl add-bond br0 bond0 p1p1 p1p2
>> >> >
>> >> > ovs-vsctl set port bond0 lacp=active bond_mode=balance-tcp
>> >> >
>> >> > ifconfig br0 100.0.0.1 up
>> >> >
>> >> > ovs-vsctl add-port br0 veth0
>> >> >
>> >> > ovs-vsctl add-port br0 vx0 -- set interface vx0 type=vxlan
>> >> > options:local_ip=100.0.0.1 options:remote_ip=100.0.0.2
>> >> > option:key=flow
>> >> >
>> >> >
>> >> >
>> >> > ovs-ofctl add-flow br0 "table=0, priority=1, cookie=100, tun_id=100,
>> >> > in_port=4, action=output:3"
>> >> >
>> >> > ovs-ofctl add-flow br0 "table=0, priority=1, cookie=100, in_port=3,
>> >> > actions=set_field:100->tun_id output:4"
>> >> >
>> >> >
>> >> >
>> >> > The same bridge br0 is being used as the local interface for vxlan
>> >> > tunnel. Even though this configuration is invalid, we should not see
>> >> > the
>> >> > kernel crash.
>> >> >
>> >>
>> >> I agree this should not cause crash.
>> >> Can you post the crash or investigate why it is crashing I think such
>> >> configuration would hit the networking stack recursion limit
>> >> (XMIT_RECURSION_LIMIT) and then the packet would be dropped. I am not
>> >> sure which spinlock recursion issue you are referring to.
>> >>
>> >>
>> >> >
>> >> >
>> >> >
>> >> >
>> >> > On Tue, Jun 12, 2018 at 11:55 AM, Pravin Shelar 
>> >> > wrote:
>> >> >>
>> >> >>
>> >> >>
>> >> >> On Tue, May 22, 2018 at 10:16 PM, Neelakantam Gaddam
>> >> >>  wrote:
>> >> >>>
>> >> >>> This patch fixes the kernel soft lockup issue with vxlan
>> >> >>> configuration
>> >> >>> where the tunneled packet is sent on the same bridge where vxlan
>> >> >>> port
>> >> >>> is
>> >> >>> attched to. It detects the loop in vxlan xmit functionb and drops
>> >> >>> if
>> >> >>> loop is
>> >> >>> detected.
>> >> >>>
>> >> >>> Signed-off-by: Neelakantam Gaddam 
>> >> >>> ---
>> >> >>>  datapath/linux/compat/vxlan.c | 6 --
>> >> >>>  1 file changed, 4 insertions(+), 2 deletions(-)
>> >> >>>
>> >> >>> diff --git a/datapath/linux/compat/vxlan.c
>> >> >>> b/datapath/linux/compat/vxlan.c
>> >> >>> index 287dad2..00562fa 100644
>> >> >>> --- a/datapath/linux/compat/vxlan.c
>> >> >>> +++ b/datapath/linux/compat/vxlan.c
>> >> >>> @@ -1115,7 +1115,8 @@ static void vxlan_xmit_one(struct sk_buff
>> >> >>> *skb,
>> >> >>> struct net_device 

[ovs-dev] [PATCH v9 0/7] OVS-DPDK flow offload with rte_flow

[Koujalagi, MalleshX]

Hi Yuanhan/Finn/Shahaf,

I tried hw-offloading  patch on top OvS v2.9.0 and DPDK 17.11.0 with VXLAN 
encap/decap functionalities. VXLAN encap is working fine, however VXLAN decap 
functionality is broken, Please find HW/SW conf and Setup as below:

[HW/SW configuration]:
CPU:  Intel(R) Xeon(R) Platinum 8180 28C @ 
2.50GHz
RAM: 128GB DDR4 2666
Linux Kernel:  4.4.0
Linux Dist.   Ubuntu 16.04.4 LTS
DPDK:   v17.11.0 (Std configuration)
OVS:  v2.9.0
Gcc:   5.4.0
NIC:   2 x MCX416A-CCAT- ConnectX-4 network 
interface card 100GbE dual-port QSFP28; (version: 4.3-1.0.1 firmware-version: 
12.22.1002 (MT_2150110033)
Traffic Type:   64-byte Ethernet Frames with UDP/IP 
payload, Uniform Random Flows Sweeping Destination IP address
NUMA/Core setting:  on Socket #1

[VXLAN Setup]: Following section 8.5.1 VXLAN Test Methodology
https://download.01.org/packet-processing/ONPS2.0/Intel_ONP_Release_2.0_Performance_Test_Report_Rev1.0-1.pdf

[Logs]:
2018-06-13T16:02:51.712Z|00155|netdev_linux|INFO|ioctl(SIOCGIFINDEX) on 
vxlan_sys_4789 device failed: No such device
2018-06-13T16:02:51.723Z|00156|connmgr|INFO|br-int<->unix#2: 1 flow_mods in the 
last 0 s (1 adds)
2018-06-13T16:02:51.725Z|00157|connmgr|INFO|br-int<->unix#4: 1 flow_mods in the 
last 0 s (1 adds)
2018-06-13T16:03:00.354Z|1|netdev_linux(dp_netdev_flow_59)|INFO|ioctl(SIOCGIFINDEX)
 on vxlan_sys_4789 device failed: No such device
2018-06-13T16:03:00.354Z|2|netdev_tc_offloads(dp_netdev_flow_59)|ERR|flow_put:
 failed to get ifindex for vxlan_sys_4789: No such device
2018-06-13T16:08:40.767Z|3|netdev_dpdk(dp_netdev_flow_59)|ERR|cannot HW 
accelerate this flow due to unsupported protocols


After disabling HW-offload,  VXLAN ENCAP/DECAP functionalities are worked fine!!

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

[ovs-dev] Efectos de la Inflación

[Actualización 2018 en las NIF]

  InnovaGroup   Webinar Interactivo 19 y 20 de Junio  "Actualización 2018 en 
las Normas de Información Financiera. " 


¿Qué veré en este webinar? 

Las NIF (Normas de Información Financiera) comprenden un conjunto de conceptos 
generales y normas particulares que regulan la elaboración y presentación de la 
información contenida en los informes financieros y que son aceptadas de manera 
generalizada en lugar y a una fecha determinada. Este curso profundiza en los 
cambios que el CINIF (Consejo Mexicano para la Investigación y Desarrollo de 
Normas de Información Financiera) ha llevado a cabo y provee a los 
participantes de datos sobre las nuevas disposiciones para simplificar la 
elaboración de reportes e informes financieros. 
 
 ¿Cuáles son los temas que voy a ver en este webinar? 

Reglas en vigor 2018, NIF sobre Instrumentos Financieros, Inversión en 
Instrumentos Financieros, Instrumentos Financieros por Cobrar y por Pagar, 
Estados de flujo de Efectivo, Efectos de la Inflación, entre otros temas de 
suma importancia para el bienestar de la empresa. 

Temario e Inscripciones:

Respondiendo por este medio con la frase:

 "Normas"+ 

Nombre + Teléfono + Empresa
o marcando al:

045 + 5515546630   
 



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

[ovs-dev] [PATCH] [RFC] ovn-controller: Experiment with restricting access to columns.

[Ben Pfaff]

To make ovn-controller recompute incrementally, we need accurate
dependencies for each function that reads or writes a table.  It's
currently difficult to be sure about these dependencies, and certainly
difficult to maintain them over time, because there's no way to actually
enforce them.

This commit experiments with an approach that allows for fairly
fine-grained access control within ovn-controller to tables and columns.
It's based on generating a new version of the IDL data structures for each
case where we want different access control.  All of these data structures
have the same format, but the columns that a given piece of code is not
supposed to touch are renamed to discourage programmers from using them,
e.g. they're given names suffixed with "__accessdenied".  (This means
that there is no runtime overhead to the access control since it only
requires a cast to convert between the regular and restricted versions.)
In addition, when a columns is supposed to be read-only, functions for
modifying the column are not supplied.

This commit only tries out this experiment for a single file within
ovn-controller, the BFD implementation (mostly because that's
alphabetically first, no other real reason).  It would require a little
more work to apply it everywhere, but it's probably not a huge deal.

Comments?

CC: Han Zhou 
Signed-off-by: Ben Pfaff 
---
 ovn/controller/automake.mk |   5 +
 ovn/controller/bfd-vswitch-idl.def |  21 
 ovn/controller/bfd.c   |  20 ++--
 ovn/controller/bfd.h   |   8 +-
 ovn/controller/ovn-controller.c|  13 ++-
 ovsdb/ovsdb-idlc.in| 223 -
 6 files changed, 268 insertions(+), 22 deletions(-)
 create mode 100644 ovn/controller/bfd-vswitch-idl.def

diff --git a/ovn/controller/automake.mk b/ovn/controller/automake.mk
index cd5505ca6b7c..78c275b219f5 100644
--- a/ovn/controller/automake.mk
+++ b/ovn/controller/automake.mk
@@ -28,3 +28,8 @@ ovn_controller_ovn_controller_LDADD = ovn/lib/libovn.la 
lib/libopenvswitch.la
 man_MANS += ovn/controller/ovn-controller.8
 EXTRA_DIST += ovn/controller/ovn-controller.8.xml
 CLEANFILES += ovn/controller/ovn-controller.8
+
+$(ovn_controller_ovn_controller_SOURCES:.c=.$(OBJEXT)): \
+   ovn/controller/bfd-vswitch-idl.h
+ovn/controller/bfd-vswitch-idl.h: lib/vswitch-idl.ovsidl 
ovn/controller/bfd-vswitch-idl.def ovsdb/ovsdb-idlc.in
+   $(AM_V_GEN)$(OVSDB_IDLC) c-idl-subset lib/vswitch-idl.ovsidl 
$(srcdir)/ovn/controller/bfd-vswitch-idl.def > $@.tmp && mv $@.tmp $@
diff --git a/ovn/controller/bfd-vswitch-idl.def 
b/ovn/controller/bfd-vswitch-idl.def
new file mode 100644
index ..da60a4a40529
--- /dev/null
+++ b/ovn/controller/bfd-vswitch-idl.def
@@ -0,0 +1,21 @@
+# -*- python -*-
+
+{
+"prefix": "bfd_ovsrec_",
+"tables": {
+"Interface": {
+"name": RO,
+"bfd": RW,
+"bfd_status": RO,
+"options": RO},
+"Port": {
+"name": RO,
+"interfaces": RO,
+"external_ids": RO
+},
+"Bridge": {
+"name": RO,
+"ports": RO
+}
+}
+}
diff --git a/ovn/controller/bfd.c b/ovn/controller/bfd.c
index 051781f38ba8..50a092205a28 100644
--- a/ovn/controller/bfd.c
+++ b/ovn/controller/bfd.c
@@ -40,7 +40,7 @@ bfd_register_ovs_idl(struct ovsdb_idl *ovs_idl)
 
 
 static void
-interface_set_bfd(const struct ovsrec_interface *iface, bool bfd_setting)
+interface_set_bfd(const struct bfd_ovsrec_interface *iface, bool bfd_setting)
 {
 const char *new_setting = bfd_setting ? "true":"false";
 const char *current_setting = smap_get(>bfd, "enable");
@@ -50,14 +50,14 @@ interface_set_bfd(const struct ovsrec_interface *iface, 
bool bfd_setting)
 return;
 }
 const struct smap bfd = SMAP_CONST1(, "enable", new_setting);
-ovsrec_interface_verify_bfd(iface);
-ovsrec_interface_set_bfd(iface, );
+bfd_ovsrec_interface_verify_bfd(iface);
+bfd_ovsrec_interface_set_bfd(iface, );
 VLOG_INFO("%s BFD on interface %s", bfd_setting ? "Enabled" : "Disabled",
 iface->name);
 }
 
 void
-bfd_calculate_active_tunnels(const struct ovsrec_bridge *br_int,
+bfd_calculate_active_tunnels(const struct bfd_ovsrec_bridge *br_int,
  struct sset *active_tunnels)
 {
 int i;
@@ -68,7 +68,7 @@ bfd_calculate_active_tunnels(const struct ovsrec_bridge 
*br_int,
 }
 
 for (i = 0; i < br_int->n_ports; i++) {
-const struct ovsrec_port *port_rec = br_int->ports[i];
+const struct bfd_ovsrec_port *port_rec = br_int->ports[i];
 
 if (!strcmp(port_rec->name, br_int->name)) {
 continue;
@@ -76,7 +76,7 @@ bfd_calculate_active_tunnels(const struct ovsrec_bridge 
*br_int,
 
 int j;
 for (j = 0; j < port_rec->n_interfaces; j++) {
-const struct ovsrec_interface *iface_rec;
+const 

[ovs-dev] [PATCH 1/2] lldp: fix string warnings

[Aaron Conole]

lib/lldp/lldpd.c: In function :
lib/lldp/lldpd.c:520:17: warning:  output truncated before terminating nul 
copying as many bytes from a string as its length [-Wstringop-truncation]
strncat(buffer, cfg->g_protocols[i].name,
^
strlen(cfg->g_protocols[i].name));
~

lib/lldp/lldpd.c: In function :
lib/lldp/lldpd.c:519:17: warning:  specified bound 2 equals source length 
[-Wstringop-overflow=]
strncat(buffer, ", ", 2);
^~~~

Closer inspection shows that buffer is only used to output protocol names
when debug logging is enabled, so restructure the code a bit as well.

Signed-off-by: Aaron Conole 
---
 lib/lldp/lldpd.c | 40 +++-
 1 file changed, 19 insertions(+), 21 deletions(-)

diff --git a/lib/lldp/lldpd.c b/lib/lldp/lldpd.c
index ff5e62846..e9f3b6355 100644
--- a/lib/lldp/lldpd.c
+++ b/lib/lldp/lldpd.c
@@ -40,6 +40,7 @@
 #include 
 #endif
 #include "compiler.h"
+#include "openvswitch/dynamic-string.h"
 #include "openvswitch/list.h"
 #include "packets.h"
 #include "timeval.h"
@@ -417,7 +418,6 @@ lldpd_hide_ports(struct lldpd *cfg,
  int mask) {
 struct lldpd_port *port;
 int protocols[LLDPD_MODE_MAX + 1];
-char buffer[256];
 bool found = false;
 int i, j, k;
 unsigned int min;
@@ -505,29 +505,27 @@ lldpd_hide_ports(struct lldpd *cfg,
 j++;
 }
 
-buffer[0] = '\0';
-for (i = 0; cfg->g_protocols[i].mode != 0; i++) {
-if (cfg->g_protocols[i].enabled &&
-protocols[cfg->g_protocols[i].mode]) {
-if (strlen(buffer) +
-strlen(cfg->g_protocols[i].name) + 3 > sizeof(buffer)) {
-/* Unlikely, our buffer is too small */
-memcpy(buffer + sizeof(buffer) - 4, "...", 4);
-break;
-}
-if (buffer[0]) {
-strncat(buffer, ", ", 2);
-strncat(buffer, cfg->g_protocols[i].name,
-strlen(cfg->g_protocols[i].name));
+if (VLOG_IS_DBG_ENABLED()) {
+struct ds buffer;
+ds_init();
+for (i = 0; cfg->g_protocols[i].mode != 0; i++) {
+if (cfg->g_protocols[i].enabled &&
+protocols[cfg->g_protocols[i].mode]) {
+if (*ds_cstr()) {
+ds_put_cstr(, ", ");
+}
+ds_put_cstr(, cfg->g_protocols[i].name);
 }
 }
+VLOG_DBG("%s: %s: %d visible neighbors (out of %d)",
+ hw->h_ifname,
+ (mask == SMART_OUTGOING) ? "out filter" : "in filter",
+ k, j);
+VLOG_DBG("%s: protocols: %s",
+ hw->h_ifname,
+ *ds_cstr() ? ds_cstr() : "(none)");
+ds_destroy();
 }
-VLOG_DBG("%s: %s: %d visible neighbors (out of %d)",
- hw->h_ifname,
- (mask == SMART_OUTGOING) ? "out filter" : "in filter",
- k, j);
-VLOG_DBG("%s: protocols: %s",
- hw->h_ifname, buffer[0] ? buffer : "(none)");
 }
 
 /* Hide unwanted ports depending on smart mode set by the user */
-- 
2.14.3

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

[ovs-dev] [PATCH 2/2] netdev-dpdk: fix snprintf call

[Aaron Conole]

lib/netdev-dpdk.c: In function :
lib/netdev-dpdk.c:2865:49: warning:  output may be truncated before the last 
format character [-Wformat-truncation=]
snprintf(vhost_vring, 16, "vring_%d_size", i);
^
lib/netdev-dpdk.c:2865:9: note:  output between 13 and 17 bytes into a 
destination of size 16
snprintf(vhost_vring, 16, "vring_%d_size", i);
^

Since vring_num is 16 bits, the largest value ever would only be 17 bytes,
including the terminating nul.  Stretch it to 18 bytes (as a precaution
against a signed value, which again would never happen).

Signed-off-by: Aaron Conole 
---
 lib/netdev-dpdk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 2e2f568b8..e75943bb2 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2859,7 +2859,7 @@ netdev_dpdk_vhost_user_get_status(const struct netdev 
*netdev,
 
 for (int i = 0; i < vring_num; i++) {
 struct rte_vhost_vring vring;
-char vhost_vring[16];
+char vhost_vring[18];
 
 rte_vhost_get_vhost_vring(vid, i, );
 snprintf(vhost_vring, 16, "vring_%d_size", i);
-- 
2.14.3

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

Re: [ovs-dev] [PATCH] OVN: add ICMP time exceeded support to OVN logical router

[Mark Michelson]

Hi Lorenzo, the patch looks good to me. I have one comment on the test 
you added, though. See below.


On 06/05/2018 11:57 AM, Lorenzo Bianconi wrote:

Using icmp4 action, send an ICMP time exceeded frame whenever
an OVN logical router receives an IPv4 packets whose TTL has
expired (ip.ttl == {0, 1})

Signed-off-by: Lorenzo Bianconi 
---
  ovn/northd/ovn-northd.8.xml |  4 ---
  ovn/northd/ovn-northd.c | 26 +--
  tests/ovn.at| 77 +
  3 files changed, 100 insertions(+), 7 deletions(-)

diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
index 1d68f1aab..759d3dace 100644
--- a/ovn/northd/ovn-northd.8.xml
+++ b/ovn/northd/ovn-northd.8.xml
@@ -1401,10 +1401,6 @@ icmp4 {
  next;
  };
  
-
-
-  Not yet implemented.
-

  


diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 0e06776ad..60f43e2a6 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -4852,9 +4852,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
  ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 50,
"eth.bcast", "drop;");
  
-/* TTL discard.

- *
- * XXX Need to send ICMP time exceeded if !ip.later_frag. */
+/* TTL discard */
  ds_clear();
  ds_put_cstr(, "ip4 && ip.ttl == {0, 1}");
  ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 30,
@@ -4920,6 +4918,28 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
ds_cstr(), ds_cstr());
  }
  
+/* ICMP time exceeded */

+for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
+ds_clear();
+ds_clear();
+
+ds_put_format(,
+  "inport == %s && ip4 && "
+  "ip.ttl == {0, 1} && !ip.later_frag", op->json_key);
+ds_put_format(,
+  "icmp4 {"
+  "eth.dst <-> eth.src; "
+  "icmp4.type = 11; /* Time exceeded */ "
+  "icmp4.code = 0; /* TTL exceeded in transit */ "
+  "ip4.dst = ip4.src; "
+  "ip4.src = %s; "
+  "ip.ttl = 255; "
+  "next; };",
+  op->lrp_networks.ipv4_addrs[i].addr_s);
+ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 40,
+  ds_cstr(), ds_cstr());
+}
+
  /* ARP reply.  These flows reply to ARP requests for the router's own
   * IP address. */
  for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
diff --git a/tests/ovn.at b/tests/ovn.at
index f12c24c17..81c834276 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -10267,3 +10267,80 @@ $PYTHON "$top_srcdir/utilities/ovs-pcap.in" 
hv1/vif2-tx.pcap > 2.packets
  AT_CHECK([cat 2.packets], [0], [])
  
  AT_CLEANUP

+
+AT_SETUP([ovn -- TTL exceeded])
+AT_KEYWORDS([ttl-exceeded])
+AT_SKIP_IF([test $HAVE_PYTHON = no])
+ovn_start
+
+# test_ip_packet INPORT HV ETH_SRC ETH_DST IPV4_SRC IPV4_DST IPV4_ROUTER 
IP_CHKSUM EXP_IP_CHKSUM EXP_ICMP_CHKSUM
+#
+# Causes a packet to be received on INPORT of the hypervisor HV. The packet is 
an IPv4 packet with
+# ETH_SRC, ETH_DST, IPV4_SRC, IPV4_DST, IP_CHKSUM as specified and TTL set to 
1.
+# EXP_IP_CHKSUM and EXP_ICMP_CHKSUM are the ip and icmp checksums of the icmp 
time exceeded frame
+# generated by OVN logical router
+#
+# INPORT is a lport number, e.g. 11 for vif11.
+# HV is a hypervisor number
+# ETH_SRC and ETH_DST are each 12 hex digits.
+# IPV4_SRC, IPV4_DST and IPV4_ROUTER are each 8 hex digits.
+# IP_CHKSUM, EXP_IP_CHSUM and EXP_ICMP_CHKSUM are each 4 hex digits
+test_ip_packet() {
+local inport=$1 hv=$2 eth_src=$3 eth_dst=$4 ipv4_src=$5 ipv4_dst=$6 
ip_router=$7 ip_chksum=$8
+local exp_ip_chksum=$9 exp_icmp_chksum=${10}
+shift 10
+
+local ip_ttl=01
+local 
packet=${eth_dst}${eth_src}080045144000${ip_ttl}01${ip_chksum}${ipv4_src}${ipv4_dst}
+
+local reply_icmp_ttl=fe
+local icmp_type_code_response=0b00
+local icmp_data=
+local 
reply_icmp_payload=${icmp_type_code_response}${exp_icmp_chksum}${icmp_data}
+local 
reply=${eth_src}${eth_dst}0800451c4000${reply_icmp_ttl}01${exp_ip_chksum}${ip_router}${ipv4_src}${reply_icmp_payload}
+echo $reply >> vif$inport.expected
+
+as hv$hv ovs-appctl netdev-dummy/receive vif$inport $packet
+}
+
+ip_to_hex() {
+printf "%02x%02x%02x%02x" "$@"
+}
+
+for i in 1 2; do
+net_add n$i
+ovn-nbctl ls-add sw$i
+
+sim_add hv$i
+as hv$i
+ovs-vsctl add-br br-phys
+ovn_attach n$i br-phys 192.168.$i.1
+
+ovn-nbctl lsp-add sw$i sw$i-p${i}0 -- \
+lsp-set-addresses sw$i-p${i}0 "00:00:00:00:00:0$i 192.168.$i.1"
+
+ovs-vsctl -- add-port br-int vif$i -- \
+ 

Re: [ovs-dev] Issue with OVS 2.9.0, DPDK 18.02 and vhost-user

[Ian Stokes]

On 6/12/2018 9:50 PM, Ravi Kerur wrote:



On Tue, Jun 12, 2018 at 3:31 AM, Stokes, Ian > wrote:


> Hi,
> 
> I have used first link to install, compile and run OVS 2.9.0 and DPDK

> 18.02, second link to configure vhost-client ports. However, facing
> several issues when configured as per the documentation. Inputs
> appreciated.
> 
> http://docs.openvswitch.org/en/latest/intro/install/dpdk/


> 
> http://docs.openvswitch.org/en/latest/topics/dpdk/vhost-user/


> 


Is there a specific reason you require DPDK 18.02?

DPDK 17.11 is the latest officially support DPDK for OVS, I'd
recommend testing with this as I'm not sure how well validated DPDK
18.02 is.

In testing I also found that OVS will fail to compile with 18.02 due
to 'rte_eth_find_next_owned_by' not being part of the stable ABI.

The list of supported DPDK to OVS mappings can be found in the
release doc below.

http://docs.openvswitch.org/en/latest/faq/releases/



Thanks Ian. I am currently using OVS 2.9 and DPDK 18.02 seperately for 
different things, hence thought of trying with that first. I was able to 
compile and run as shown in the logs.  Issue was I didn't specify 
'datapath_type=netdev' when adding the bridge. Deleting and adding the 
bridge with 'datapath_type', I was able to add dpdkvhostclient ports on 
the bridge.


Having said that, I decided to try 'ovs-master' + dpdk-17.11 as 
mentioned in the link and was able to run everything with Tx/Rx packets. 
Some clarifications I need. My testbed has physical host + containers.


(1) tried with dpdkvhostuser (server port on OVS on host) and 
virtio-user-client (testpmd/dpdk on containers). Was able to configure 
and run traffic. I do see following messages in log file


2018-06-12T15:51:39.152Z|00080|netdev_dpdk|WARN|dpdkvhostuser ports are 
considered deprecated;  please migrate to dpdkvhostuserclient ports.


Any reason why dpdkvhostuser is not recommended or being deprecated?



Sure, the documentation explains it best:

"vHost User uses a client-server model. The server 
creates/manages/destroys the vHost User sockets, and the client connects 
to the server. Depending on which port type you use, dpdkvhostuser or 
dpdkvhostuserclient, a different configuration of the client-server 
model is used.


For vhost-user ports, Open vSwitch acts as the server and QEMU the 
client. This means if OVS dies, all VMs must be restarted. On the other 
hand, for vhost-user-client ports, OVS acts as the client and QEMU the 
server. This means OVS can die and be restarted without issue, and it is 
also possible to restart an instance itself. For this reason, 
vhost-user-client ports are the preferred type for all known use cases; 
the only limitation is that vhost-user client mode ports require QEMU 
version 2.7. Ports of type vhost-user are currently deprecated and will 
be removed in a future release."


http://docs.openvswitch.org/en/latest/topics/dpdk/vhost-user/#vhost-user-vs-vhost-user-client

(2) tried with dpdkvhostuserclient (client port OVS on host) and 
virtio-user-server (testpmd/dpdk on containers). Was unable to 
configure. Any examples available for this?


I haven't tried this setup myself to date.

Are you running DPDK 18.05 in the container? Just looking for references 
to virtio-server-mode and I only spotted it in the 18.05 release notes.


From the DPDK 18.05 release notes:

Added support for virtio-user server mode.

In a container environment if the vhost-user backend restarts, there’s 
no way for it to reconnect to virtio-user. To address this, support for 
server mode has been added. In this mode the socket file is created by 
virtio-user, which the backend connects to. This means that if the 
backend restarts, it can reconnect to virtio-user and continue 
communications.


https://dpdk.org/doc/guides/rel_notes/release_18_05.html

Note this isn't available in DPDK 17.11.

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

Re: [ovs-dev] [PATCH] OVN python IDL: avoid useless JSON conversion to enhance performance

[Miguel Angel Ajo Pelayo]

Thank you very much Ben :)

On Wed, Jun 13, 2018 at 5:20 PM Ben Pfaff  wrote:

> OK, I crossported to branch-2.9.
>
> On Wed, Jun 13, 2018 at 01:24:44PM +0200, Miguel Angel Ajo Pelayo wrote:
> > Ben, Russell, could we get this down to ovs 2.9?
> >
> > It's very important for scale.
> >
> > Best regards,
> > Miguel Ángel.
> >
> > On Wed, Jun 13, 2018 at 1:13 PM Daniel Alvarez Sanchez <
> dalva...@redhat.com>
> > wrote:
> >
> > > Hi,
> > > Bringing this up again.
> > > Can we please get this backported into 2.9 branch? The enhancement is
> very
> > > noticeable.
> > > Thanks a lot!
> > > Daniel
> > >
> > > On Tue, Mar 6, 2018 at 5:09 PM, Lucas Alvares Gomes <
> lucasago...@gmail.com
> > > > wrote:
> > >
> > >> Hi,
> > >>
> > >> Excellent investigative work here Daniel, thanks for that!
> > >>
> > >> On Wed, Feb 28, 2018 at 9:37 AM, Miguel Angel Ajo Pelayo
> > >>  wrote:
> > >> > And if we can get backports down the lane, that'd be great :)
> > >> >
> > >>
> > >> +1 for backporting it. The changes seems straight forward to do so.
> > >>
> > >> > On Wed, Feb 28, 2018 at 9:37 AM Miguel Angel Ajo Pelayo <
> > >> majop...@redhat.com>
> > >> > wrote:
> > >> >
> > >> >> Acked-by: Miguel Angel Ajo 
> > >> >>
> > >> >> On Wed, Feb 28, 2018 at 9:13 AM Daniel Alvarez Sanchez <
> > >> >> dalva...@redhat.com> wrote:
> > >> >>
> > >> >>> Thanks Terry and Han for the reviews!
> > >> >>> I removed the 'OVN' keyword from the title and submitted a v2:
> > >> >>> [PATCH v2] python: avoid useless JSON conversion to enhance
> > >> performance
> > >> >>>
> > >> >>> Thanks again.
> > >> >>> Daniel
> > >> >>>
> > >> >>> On Wed, Feb 28, 2018 at 12:41 AM, Han Zhou 
> wrote:
> > >> >>>
> > >> >>> > Great catch!
> > >> >>> > This patch is generic and not only for OVN, so I suggest to
> remove
> > >> the
> > >> >>> > "OVN" keyword in commit title.
> > >> >>> >
> > >> >>> > Acked-by: Han Zhou 
> > >> >>> >
> > >> >>> > On Tue, Feb 27, 2018 at 12:44 PM, Terry Wilson <
> twil...@redhat.com>
> > >> >>> wrote:
> > >> >>> >
> > >> >>> >> On Tue, Feb 27, 2018 at 9:25 AM, Daniel Alvarez <
> > >> dalva...@redhat.com>
> > >> >>> >> wrote:
> > >> >>> >> > This patch removes a useless conversion to/from JSON in the
> > >> >>> >> > processing of any 'modify' operations inside the
> process_update2
> > >> >>> >> > method in Python IDL implementation.
> > >> >>> >> >
> > >> >>> >> > Previous code will make resources creation take longer as the
> > >> number
> > >> >>> >> > of elements in the row grows because of that JSON conversion.
> > >> This
> > >> >>> >> > patch eliminates it and now the time remains consant
> regardless
> > >> >>> >> > of the database contents improving performance and scaling.
> > >> >>> >> >
> > >> >>> >> > Reported-by: Daniel Alvarez Sanchez 
> > >> >>> >> > Reported-at: https://mail.openvswitch.org/p
> > >> >>> >> ipermail/ovs-discuss/2018-February/046263.html
> > >> >>> >> > Signed-off-by: Daniel Alvarez 
> > >> >>> >> > ---
> > >> >>> >> >  python/ovs/db/idl.py | 12 +---
> > >> >>> >> >  1 file changed, 5 insertions(+), 7 deletions(-)
> > >> >>> >> >
> > >> >>> >> > diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py
> > >> >>> >> > index 60548bcf5..5a4d129c0 100644
> > >> >>> >> > --- a/python/ovs/db/idl.py
> > >> >>> >> > +++ b/python/ovs/db/idl.py
> > >> >>> >> > @@ -518,10 +518,8 @@ class Idl(object):
> > >> >>> >> >  if not row:
> > >> >>> >> >  raise error.Error('Modify non-existing row')
> > >> >>> >> >
> > >> >>> >> > -old_row_diff_json = self.__apply_diff(table,
> row,
> > >> >>> >> > -
> > >> >>> row_update['modify'])
> > >> >>> >> > -self.notify(ROW_UPDATE, row,
> > >> >>> >> > -Row.from_json(self, table, uuid,
> > >> >>> >> old_row_diff_json))
> > >> >>> >> > +old_row = self.__apply_diff(table, row,
> > >> >>> >> row_update['modify'])
> > >> >>> >> > +self.notify(ROW_UPDATE, row, Row(self, table,
> uuid,
> > >> >>> >> old_row))
> > >> >>> >> >  changed = True
> > >> >>> >> >  else:
> > >> >>> >> >  raise error.Error(' unknown
> operation',
> > >> >>> >> > @@ -584,7 +582,7 @@ class Idl(object):
> > >> >>> >> >  row_update[column.name] =
> > >> >>> >> self.__column_name(column)
> > >> >>> >> >
> > >> >>> >> >  def __apply_diff(self, table, row, row_diff):
> > >> >>> >> > -old_row_diff_json = {}
> > >> >>> >> > +old_row = {}
> > >> >>> >> >  for column_name, datum_diff_json in
> > >> six.iteritems(row_diff):
> > >> >>> >> >  column = table.columns.get(column_name)
> > >> >>> >> >  if not column:
> > >> >>> >> > @@ -601,12 +599,12 @@ class Idl(object):
> > >> >>> >> >% (column_name, table.name, e))
> > >> >>> >> >  continue
> > >> >>> >> >
> > >> >>> >> > -old_row_diff_json[column_name] =
> > >> >>> >> row._data[column_name].to_json()
> > >> 

Re: [ovs-dev] [PATCH] OVN python IDL: avoid useless JSON conversion to enhance performance

[Ben Pfaff]

OK, I crossported to branch-2.9.

On Wed, Jun 13, 2018 at 01:24:44PM +0200, Miguel Angel Ajo Pelayo wrote:
> Ben, Russell, could we get this down to ovs 2.9?
> 
> It's very important for scale.
> 
> Best regards,
> Miguel Ángel.
> 
> On Wed, Jun 13, 2018 at 1:13 PM Daniel Alvarez Sanchez 
> wrote:
> 
> > Hi,
> > Bringing this up again.
> > Can we please get this backported into 2.9 branch? The enhancement is very
> > noticeable.
> > Thanks a lot!
> > Daniel
> >
> > On Tue, Mar 6, 2018 at 5:09 PM, Lucas Alvares Gomes  > > wrote:
> >
> >> Hi,
> >>
> >> Excellent investigative work here Daniel, thanks for that!
> >>
> >> On Wed, Feb 28, 2018 at 9:37 AM, Miguel Angel Ajo Pelayo
> >>  wrote:
> >> > And if we can get backports down the lane, that'd be great :)
> >> >
> >>
> >> +1 for backporting it. The changes seems straight forward to do so.
> >>
> >> > On Wed, Feb 28, 2018 at 9:37 AM Miguel Angel Ajo Pelayo <
> >> majop...@redhat.com>
> >> > wrote:
> >> >
> >> >> Acked-by: Miguel Angel Ajo 
> >> >>
> >> >> On Wed, Feb 28, 2018 at 9:13 AM Daniel Alvarez Sanchez <
> >> >> dalva...@redhat.com> wrote:
> >> >>
> >> >>> Thanks Terry and Han for the reviews!
> >> >>> I removed the 'OVN' keyword from the title and submitted a v2:
> >> >>> [PATCH v2] python: avoid useless JSON conversion to enhance
> >> performance
> >> >>>
> >> >>> Thanks again.
> >> >>> Daniel
> >> >>>
> >> >>> On Wed, Feb 28, 2018 at 12:41 AM, Han Zhou  wrote:
> >> >>>
> >> >>> > Great catch!
> >> >>> > This patch is generic and not only for OVN, so I suggest to remove
> >> the
> >> >>> > "OVN" keyword in commit title.
> >> >>> >
> >> >>> > Acked-by: Han Zhou 
> >> >>> >
> >> >>> > On Tue, Feb 27, 2018 at 12:44 PM, Terry Wilson 
> >> >>> wrote:
> >> >>> >
> >> >>> >> On Tue, Feb 27, 2018 at 9:25 AM, Daniel Alvarez <
> >> dalva...@redhat.com>
> >> >>> >> wrote:
> >> >>> >> > This patch removes a useless conversion to/from JSON in the
> >> >>> >> > processing of any 'modify' operations inside the process_update2
> >> >>> >> > method in Python IDL implementation.
> >> >>> >> >
> >> >>> >> > Previous code will make resources creation take longer as the
> >> number
> >> >>> >> > of elements in the row grows because of that JSON conversion.
> >> This
> >> >>> >> > patch eliminates it and now the time remains consant regardless
> >> >>> >> > of the database contents improving performance and scaling.
> >> >>> >> >
> >> >>> >> > Reported-by: Daniel Alvarez Sanchez 
> >> >>> >> > Reported-at: https://mail.openvswitch.org/p
> >> >>> >> ipermail/ovs-discuss/2018-February/046263.html
> >> >>> >> > Signed-off-by: Daniel Alvarez 
> >> >>> >> > ---
> >> >>> >> >  python/ovs/db/idl.py | 12 +---
> >> >>> >> >  1 file changed, 5 insertions(+), 7 deletions(-)
> >> >>> >> >
> >> >>> >> > diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py
> >> >>> >> > index 60548bcf5..5a4d129c0 100644
> >> >>> >> > --- a/python/ovs/db/idl.py
> >> >>> >> > +++ b/python/ovs/db/idl.py
> >> >>> >> > @@ -518,10 +518,8 @@ class Idl(object):
> >> >>> >> >  if not row:
> >> >>> >> >  raise error.Error('Modify non-existing row')
> >> >>> >> >
> >> >>> >> > -old_row_diff_json = self.__apply_diff(table, row,
> >> >>> >> > -
> >> >>> row_update['modify'])
> >> >>> >> > -self.notify(ROW_UPDATE, row,
> >> >>> >> > -Row.from_json(self, table, uuid,
> >> >>> >> old_row_diff_json))
> >> >>> >> > +old_row = self.__apply_diff(table, row,
> >> >>> >> row_update['modify'])
> >> >>> >> > +self.notify(ROW_UPDATE, row, Row(self, table, uuid,
> >> >>> >> old_row))
> >> >>> >> >  changed = True
> >> >>> >> >  else:
> >> >>> >> >  raise error.Error(' unknown operation',
> >> >>> >> > @@ -584,7 +582,7 @@ class Idl(object):
> >> >>> >> >  row_update[column.name] =
> >> >>> >> self.__column_name(column)
> >> >>> >> >
> >> >>> >> >  def __apply_diff(self, table, row, row_diff):
> >> >>> >> > -old_row_diff_json = {}
> >> >>> >> > +old_row = {}
> >> >>> >> >  for column_name, datum_diff_json in
> >> six.iteritems(row_diff):
> >> >>> >> >  column = table.columns.get(column_name)
> >> >>> >> >  if not column:
> >> >>> >> > @@ -601,12 +599,12 @@ class Idl(object):
> >> >>> >> >% (column_name, table.name, e))
> >> >>> >> >  continue
> >> >>> >> >
> >> >>> >> > -old_row_diff_json[column_name] =
> >> >>> >> row._data[column_name].to_json()
> >> >>> >> > +old_row[column_name] = row._data[column_name].copy()
> >> >>> >> >  datum = row._data[column_name].diff(datum_diff)
> >> >>> >> >  if datum != row._data[column_name]:
> >> >>> >> >  row._data[column_name] = datum
> >> >>> >> >
> >> >>> >> > -return old_row_diff_json
> >> >>> >> > +return old_row
> >> >>> >> >
> >> >>> >> >  def 

Re: [ovs-dev] [PATCH] vxlan: Fix for the packet loop issue in vxlan

[Neelakantam Gaddam]

It should be Qdisc's busylock in dev_queue_xmit function. I have seen the
issue in kernel version is 3.10.87 vanilla.



On Wed, Jun 13, 2018 at 12:21 PM, Pravin Shelar  wrote:

> On Tue, Jun 12, 2018 at 10:58 PM, Neelakantam Gaddam
>  wrote:
> > Hi Pravin,
> >
> > I have seen the below crash.
> >
> > [] show_stack+0x6c/0xf8
> >
> > [] do_raw_spin_lock+0x168/0x170
> >
> > [] dev_queue_xmit+0x43c/0x470
> >
> > [] ip_finish_output+0x250/0x490
> >
> > [] rpl_iptunnel_xmit+0x134/0x218 [openvswitch]
> >
> > [] rpl_vxlan_xmit+0x430/0x538 [openvswitch]
> >
> > [] do_execute_actions+0x18f8/0x19e8 [openvswitch]
> >
> > [] ovs_execute_actions+0x90/0x208 [openvswitch]
> >
> > [] ovs_dp_process_packet+0xb0/0x1a8 [openvswitch]
> >
> > [] ovs_vport_receive+0x78/0x130 [openvswitch]
> >
> > [] internal_dev_xmit+0x34/0x98 [openvswitch]
> >
> > [] dev_hard_start_xmit+0x2e8/0x4f8
> >
> > [] sch_direct_xmit+0xf0/0x238
> >
> > [] dev_queue_xmit+0x1d8/0x470
> >
> > [] arp_process+0x614/0x628
> >
> > [] __netif_receive_skb_core+0x2e8/0x5d8
> >
> > [] process_backlog+0xc0/0x1b0
> >
> > [] net_rx_action+0x154/0x240
> >
> > [] __do_softirq+0x1d0/0x218
> >
> > [] do_softirq+0x68/0x70
> >
> > [] local_bh_enable+0xa8/0xb0
> >
> > [] netif_rx_ni+0x20/0x30
> >
> >
> >
> >
> > I have spent some time in investigation and found that crash is because
> of
> > spinlock recursion in dev_queue_xmit function.
> > The packet path traced is : netif_rx->arp->dev_queue_xmit(internal
> > port)->vxlan_xmit->dev_queue_xmit(internal port).
> >
>
> Which spin-lock is it? I am surprised to see a lock taken in fast
> path. Can you also share kernel version?
>
> > The macro (XMIT_RECURSION_LIMIT) is defined as 10. This limit wont
> prevent
> > the crash since the recursion is 2 only for my configuration.
>
> right, The recursion limit is to avoid stack overflow.
>
> >
> >
> >
> > On Wed, Jun 13, 2018 at 4:11 AM, Pravin Shelar  wrote:
> >>
> >> On Tue, Jun 12, 2018 at 10:11 AM, Neelakantam Gaddam
> >>  wrote:
> >> >
> >> > Hi Pravin,
> >> >
> >> > The below configuration is causing the spinlock recursion issue.
> >> >
> >> > I am able to see the issue with below configuration.
> >> >
> >> >
> >> >
> >> > ovs-vsctl add-br br0
> >> >
> >> > ovs-vsctl add-bond br0 bond0 p1p1 p1p2
> >> >
> >> > ovs-vsctl set port bond0 lacp=active bond_mode=balance-tcp
> >> >
> >> > ifconfig br0 100.0.0.1 up
> >> >
> >> > ovs-vsctl add-port br0 veth0
> >> >
> >> > ovs-vsctl add-port br0 vx0 -- set interface vx0 type=vxlan
> >> > options:local_ip=100.0.0.1 options:remote_ip=100.0.0.2 option:key=flow
> >> >
> >> >
> >> >
> >> > ovs-ofctl add-flow br0 "table=0, priority=1, cookie=100, tun_id=100,
> >> > in_port=4, action=output:3"
> >> >
> >> > ovs-ofctl add-flow br0 "table=0, priority=1, cookie=100, in_port=3,
> >> > actions=set_field:100->tun_id output:4"
> >> >
> >> >
> >> >
> >> > The same bridge br0 is being used as the local interface for vxlan
> >> > tunnel. Even though this configuration is invalid, we should not see
> the
> >> > kernel crash.
> >> >
> >>
> >> I agree this should not cause crash.
> >> Can you post the crash or investigate why it is crashing I think such
> >> configuration would hit the networking stack recursion limit
> >> (XMIT_RECURSION_LIMIT) and then the packet would be dropped. I am not
> >> sure which spinlock recursion issue you are referring to.
> >>
> >>
> >> >
> >> >
> >> >
> >> >
> >> > On Tue, Jun 12, 2018 at 11:55 AM, Pravin Shelar 
> wrote:
> >> >>
> >> >>
> >> >>
> >> >> On Tue, May 22, 2018 at 10:16 PM, Neelakantam Gaddam
> >> >>  wrote:
> >> >>>
> >> >>> This patch fixes the kernel soft lockup issue with vxlan
> configuration
> >> >>> where the tunneled packet is sent on the same bridge where vxlan
> port
> >> >>> is
> >> >>> attched to. It detects the loop in vxlan xmit functionb and drops if
> >> >>> loop is
> >> >>> detected.
> >> >>>
> >> >>> Signed-off-by: Neelakantam Gaddam 
> >> >>> ---
> >> >>>  datapath/linux/compat/vxlan.c | 6 --
> >> >>>  1 file changed, 4 insertions(+), 2 deletions(-)
> >> >>>
> >> >>> diff --git a/datapath/linux/compat/vxlan.c
> >> >>> b/datapath/linux/compat/vxlan.c
> >> >>> index 287dad2..00562fa 100644
> >> >>> --- a/datapath/linux/compat/vxlan.c
> >> >>> +++ b/datapath/linux/compat/vxlan.c
> >> >>> @@ -1115,7 +1115,8 @@ static void vxlan_xmit_one(struct sk_buff
> *skb,
> >> >>> struct net_device *dev,
> >> >>> goto tx_error;
> >> >>> }
> >> >>>
> >> >>> -   if (rt->dst.dev == dev) {
> >> >>> +   if ((rt->dst.dev == dev) ||
> >> >>> +   (OVS_CB(skb)->input_vport->dev ==
> >> >>> rt->dst.dev)) {
> >> >>
> >> >>
> >> >> I am not sure which case the  OVS_CB(skb)->input_vport->dev is not
> same
> >> >> as the dev when there is recursion. Can you explain how to reproduce
> it?
> >> >>
> >> >
> >> >
> >> >
> >> > --
> >> > Thanks & Regards
> >> > Neelakantam Gaddam
> >
> >
> >
> >
> > --
> > Thanks & Regards
> > Neelakantam 

Re: [ovs-dev] [PATCH] OVN python IDL: avoid useless JSON conversion to enhance performance

[Miguel Angel Ajo Pelayo]

Ben, Russell, could we get this down to ovs 2.9?

It's very important for scale.

Best regards,
Miguel Ángel.

On Wed, Jun 13, 2018 at 1:13 PM Daniel Alvarez Sanchez 
wrote:

> Hi,
> Bringing this up again.
> Can we please get this backported into 2.9 branch? The enhancement is very
> noticeable.
> Thanks a lot!
> Daniel
>
> On Tue, Mar 6, 2018 at 5:09 PM, Lucas Alvares Gomes  > wrote:
>
>> Hi,
>>
>> Excellent investigative work here Daniel, thanks for that!
>>
>> On Wed, Feb 28, 2018 at 9:37 AM, Miguel Angel Ajo Pelayo
>>  wrote:
>> > And if we can get backports down the lane, that'd be great :)
>> >
>>
>> +1 for backporting it. The changes seems straight forward to do so.
>>
>> > On Wed, Feb 28, 2018 at 9:37 AM Miguel Angel Ajo Pelayo <
>> majop...@redhat.com>
>> > wrote:
>> >
>> >> Acked-by: Miguel Angel Ajo 
>> >>
>> >> On Wed, Feb 28, 2018 at 9:13 AM Daniel Alvarez Sanchez <
>> >> dalva...@redhat.com> wrote:
>> >>
>> >>> Thanks Terry and Han for the reviews!
>> >>> I removed the 'OVN' keyword from the title and submitted a v2:
>> >>> [PATCH v2] python: avoid useless JSON conversion to enhance
>> performance
>> >>>
>> >>> Thanks again.
>> >>> Daniel
>> >>>
>> >>> On Wed, Feb 28, 2018 at 12:41 AM, Han Zhou  wrote:
>> >>>
>> >>> > Great catch!
>> >>> > This patch is generic and not only for OVN, so I suggest to remove
>> the
>> >>> > "OVN" keyword in commit title.
>> >>> >
>> >>> > Acked-by: Han Zhou 
>> >>> >
>> >>> > On Tue, Feb 27, 2018 at 12:44 PM, Terry Wilson 
>> >>> wrote:
>> >>> >
>> >>> >> On Tue, Feb 27, 2018 at 9:25 AM, Daniel Alvarez <
>> dalva...@redhat.com>
>> >>> >> wrote:
>> >>> >> > This patch removes a useless conversion to/from JSON in the
>> >>> >> > processing of any 'modify' operations inside the process_update2
>> >>> >> > method in Python IDL implementation.
>> >>> >> >
>> >>> >> > Previous code will make resources creation take longer as the
>> number
>> >>> >> > of elements in the row grows because of that JSON conversion.
>> This
>> >>> >> > patch eliminates it and now the time remains consant regardless
>> >>> >> > of the database contents improving performance and scaling.
>> >>> >> >
>> >>> >> > Reported-by: Daniel Alvarez Sanchez 
>> >>> >> > Reported-at: https://mail.openvswitch.org/p
>> >>> >> ipermail/ovs-discuss/2018-February/046263.html
>> >>> >> > Signed-off-by: Daniel Alvarez 
>> >>> >> > ---
>> >>> >> >  python/ovs/db/idl.py | 12 +---
>> >>> >> >  1 file changed, 5 insertions(+), 7 deletions(-)
>> >>> >> >
>> >>> >> > diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py
>> >>> >> > index 60548bcf5..5a4d129c0 100644
>> >>> >> > --- a/python/ovs/db/idl.py
>> >>> >> > +++ b/python/ovs/db/idl.py
>> >>> >> > @@ -518,10 +518,8 @@ class Idl(object):
>> >>> >> >  if not row:
>> >>> >> >  raise error.Error('Modify non-existing row')
>> >>> >> >
>> >>> >> > -old_row_diff_json = self.__apply_diff(table, row,
>> >>> >> > -
>> >>> row_update['modify'])
>> >>> >> > -self.notify(ROW_UPDATE, row,
>> >>> >> > -Row.from_json(self, table, uuid,
>> >>> >> old_row_diff_json))
>> >>> >> > +old_row = self.__apply_diff(table, row,
>> >>> >> row_update['modify'])
>> >>> >> > +self.notify(ROW_UPDATE, row, Row(self, table, uuid,
>> >>> >> old_row))
>> >>> >> >  changed = True
>> >>> >> >  else:
>> >>> >> >  raise error.Error(' unknown operation',
>> >>> >> > @@ -584,7 +582,7 @@ class Idl(object):
>> >>> >> >  row_update[column.name] =
>> >>> >> self.__column_name(column)
>> >>> >> >
>> >>> >> >  def __apply_diff(self, table, row, row_diff):
>> >>> >> > -old_row_diff_json = {}
>> >>> >> > +old_row = {}
>> >>> >> >  for column_name, datum_diff_json in
>> six.iteritems(row_diff):
>> >>> >> >  column = table.columns.get(column_name)
>> >>> >> >  if not column:
>> >>> >> > @@ -601,12 +599,12 @@ class Idl(object):
>> >>> >> >% (column_name, table.name, e))
>> >>> >> >  continue
>> >>> >> >
>> >>> >> > -old_row_diff_json[column_name] =
>> >>> >> row._data[column_name].to_json()
>> >>> >> > +old_row[column_name] = row._data[column_name].copy()
>> >>> >> >  datum = row._data[column_name].diff(datum_diff)
>> >>> >> >  if datum != row._data[column_name]:
>> >>> >> >  row._data[column_name] = datum
>> >>> >> >
>> >>> >> > -return old_row_diff_json
>> >>> >> > +return old_row
>> >>> >> >
>> >>> >> >  def __row_update(self, table, row, row_json):
>> >>> >> >  changed = False
>> >>> >> > --
>> >>> >> > 2.13.5
>> >>> >> >
>> >>> >> > ___
>> >>> >> > dev mailing list
>> >>> >> > d...@openvswitch.org
>> >>> >> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>> >>> >>
>> >>> >> +1
>> >>> >>
>> >>> >> This 

Re: [ovs-dev] [PATCH] OVN python IDL: avoid useless JSON conversion to enhance performance

[Daniel Alvarez Sanchez]

Hi,
Bringing this up again.
Can we please get this backported into 2.9 branch? The enhancement is very
noticeable.
Thanks a lot!
Daniel

On Tue, Mar 6, 2018 at 5:09 PM, Lucas Alvares Gomes 
wrote:

> Hi,
>
> Excellent investigative work here Daniel, thanks for that!
>
> On Wed, Feb 28, 2018 at 9:37 AM, Miguel Angel Ajo Pelayo
>  wrote:
> > And if we can get backports down the lane, that'd be great :)
> >
>
> +1 for backporting it. The changes seems straight forward to do so.
>
> > On Wed, Feb 28, 2018 at 9:37 AM Miguel Angel Ajo Pelayo <
> majop...@redhat.com>
> > wrote:
> >
> >> Acked-by: Miguel Angel Ajo 
> >>
> >> On Wed, Feb 28, 2018 at 9:13 AM Daniel Alvarez Sanchez <
> >> dalva...@redhat.com> wrote:
> >>
> >>> Thanks Terry and Han for the reviews!
> >>> I removed the 'OVN' keyword from the title and submitted a v2:
> >>> [PATCH v2] python: avoid useless JSON conversion to enhance performance
> >>>
> >>> Thanks again.
> >>> Daniel
> >>>
> >>> On Wed, Feb 28, 2018 at 12:41 AM, Han Zhou  wrote:
> >>>
> >>> > Great catch!
> >>> > This patch is generic and not only for OVN, so I suggest to remove
> the
> >>> > "OVN" keyword in commit title.
> >>> >
> >>> > Acked-by: Han Zhou 
> >>> >
> >>> > On Tue, Feb 27, 2018 at 12:44 PM, Terry Wilson 
> >>> wrote:
> >>> >
> >>> >> On Tue, Feb 27, 2018 at 9:25 AM, Daniel Alvarez <
> dalva...@redhat.com>
> >>> >> wrote:
> >>> >> > This patch removes a useless conversion to/from JSON in the
> >>> >> > processing of any 'modify' operations inside the process_update2
> >>> >> > method in Python IDL implementation.
> >>> >> >
> >>> >> > Previous code will make resources creation take longer as the
> number
> >>> >> > of elements in the row grows because of that JSON conversion. This
> >>> >> > patch eliminates it and now the time remains consant regardless
> >>> >> > of the database contents improving performance and scaling.
> >>> >> >
> >>> >> > Reported-by: Daniel Alvarez Sanchez 
> >>> >> > Reported-at: https://mail.openvswitch.org/p
> >>> >> ipermail/ovs-discuss/2018-February/046263.html
> >>> >> > Signed-off-by: Daniel Alvarez 
> >>> >> > ---
> >>> >> >  python/ovs/db/idl.py | 12 +---
> >>> >> >  1 file changed, 5 insertions(+), 7 deletions(-)
> >>> >> >
> >>> >> > diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py
> >>> >> > index 60548bcf5..5a4d129c0 100644
> >>> >> > --- a/python/ovs/db/idl.py
> >>> >> > +++ b/python/ovs/db/idl.py
> >>> >> > @@ -518,10 +518,8 @@ class Idl(object):
> >>> >> >  if not row:
> >>> >> >  raise error.Error('Modify non-existing row')
> >>> >> >
> >>> >> > -old_row_diff_json = self.__apply_diff(table, row,
> >>> >> > -
> >>> row_update['modify'])
> >>> >> > -self.notify(ROW_UPDATE, row,
> >>> >> > -Row.from_json(self, table, uuid,
> >>> >> old_row_diff_json))
> >>> >> > +old_row = self.__apply_diff(table, row,
> >>> >> row_update['modify'])
> >>> >> > +self.notify(ROW_UPDATE, row, Row(self, table, uuid,
> >>> >> old_row))
> >>> >> >  changed = True
> >>> >> >  else:
> >>> >> >  raise error.Error(' unknown operation',
> >>> >> > @@ -584,7 +582,7 @@ class Idl(object):
> >>> >> >  row_update[column.name] =
> >>> >> self.__column_name(column)
> >>> >> >
> >>> >> >  def __apply_diff(self, table, row, row_diff):
> >>> >> > -old_row_diff_json = {}
> >>> >> > +old_row = {}
> >>> >> >  for column_name, datum_diff_json in
> six.iteritems(row_diff):
> >>> >> >  column = table.columns.get(column_name)
> >>> >> >  if not column:
> >>> >> > @@ -601,12 +599,12 @@ class Idl(object):
> >>> >> >% (column_name, table.name, e))
> >>> >> >  continue
> >>> >> >
> >>> >> > -old_row_diff_json[column_name] =
> >>> >> row._data[column_name].to_json()
> >>> >> > +old_row[column_name] = row._data[column_name].copy()
> >>> >> >  datum = row._data[column_name].diff(datum_diff)
> >>> >> >  if datum != row._data[column_name]:
> >>> >> >  row._data[column_name] = datum
> >>> >> >
> >>> >> > -return old_row_diff_json
> >>> >> > +return old_row
> >>> >> >
> >>> >> >  def __row_update(self, table, row, row_json):
> >>> >> >  changed = False
> >>> >> > --
> >>> >> > 2.13.5
> >>> >> >
> >>> >> > ___
> >>> >> > dev mailing list
> >>> >> > d...@openvswitch.org
> >>> >> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >>> >>
> >>> >> +1
> >>> >>
> >>> >> This passes all of the functional tests in ovsdbapp when applied.
> Nice
> >>> >> find!
> >>> >>
> >>> >> Terry
> >>> >> ___
> >>> >> dev mailing list
> >>> >> d...@openvswitch.org
> >>> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >>> >>
> >>> >
> >>> >
> >>> 

Re: [ovs-dev] OVS DPDK: dpdk_merge pull request for master

[Stokes, Ian]

> Thanks for the pull requests.  I merged all of these (master, 2.9, and
> 2.8).

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

Re: [ovs-dev] [PATCH] vxlan: Fix for the packet loop issue in vxlan

[Pravin Shelar]

On Tue, Jun 12, 2018 at 10:58 PM, Neelakantam Gaddam
 wrote:
> Hi Pravin,
>
> I have seen the below crash.
>
> [] show_stack+0x6c/0xf8
>
> [] do_raw_spin_lock+0x168/0x170
>
> [] dev_queue_xmit+0x43c/0x470
>
> [] ip_finish_output+0x250/0x490
>
> [] rpl_iptunnel_xmit+0x134/0x218 [openvswitch]
>
> [] rpl_vxlan_xmit+0x430/0x538 [openvswitch]
>
> [] do_execute_actions+0x18f8/0x19e8 [openvswitch]
>
> [] ovs_execute_actions+0x90/0x208 [openvswitch]
>
> [] ovs_dp_process_packet+0xb0/0x1a8 [openvswitch]
>
> [] ovs_vport_receive+0x78/0x130 [openvswitch]
>
> [] internal_dev_xmit+0x34/0x98 [openvswitch]
>
> [] dev_hard_start_xmit+0x2e8/0x4f8
>
> [] sch_direct_xmit+0xf0/0x238
>
> [] dev_queue_xmit+0x1d8/0x470
>
> [] arp_process+0x614/0x628
>
> [] __netif_receive_skb_core+0x2e8/0x5d8
>
> [] process_backlog+0xc0/0x1b0
>
> [] net_rx_action+0x154/0x240
>
> [] __do_softirq+0x1d0/0x218
>
> [] do_softirq+0x68/0x70
>
> [] local_bh_enable+0xa8/0xb0
>
> [] netif_rx_ni+0x20/0x30
>
>
>
>
> I have spent some time in investigation and found that crash is because of
> spinlock recursion in dev_queue_xmit function.
> The packet path traced is : netif_rx->arp->dev_queue_xmit(internal
> port)->vxlan_xmit->dev_queue_xmit(internal port).
>

Which spin-lock is it? I am surprised to see a lock taken in fast
path. Can you also share kernel version?

> The macro (XMIT_RECURSION_LIMIT) is defined as 10. This limit wont prevent
> the crash since the recursion is 2 only for my configuration.

right, The recursion limit is to avoid stack overflow.

>
>
>
> On Wed, Jun 13, 2018 at 4:11 AM, Pravin Shelar  wrote:
>>
>> On Tue, Jun 12, 2018 at 10:11 AM, Neelakantam Gaddam
>>  wrote:
>> >
>> > Hi Pravin,
>> >
>> > The below configuration is causing the spinlock recursion issue.
>> >
>> > I am able to see the issue with below configuration.
>> >
>> >
>> >
>> > ovs-vsctl add-br br0
>> >
>> > ovs-vsctl add-bond br0 bond0 p1p1 p1p2
>> >
>> > ovs-vsctl set port bond0 lacp=active bond_mode=balance-tcp
>> >
>> > ifconfig br0 100.0.0.1 up
>> >
>> > ovs-vsctl add-port br0 veth0
>> >
>> > ovs-vsctl add-port br0 vx0 -- set interface vx0 type=vxlan
>> > options:local_ip=100.0.0.1 options:remote_ip=100.0.0.2 option:key=flow
>> >
>> >
>> >
>> > ovs-ofctl add-flow br0 "table=0, priority=1, cookie=100, tun_id=100,
>> > in_port=4, action=output:3"
>> >
>> > ovs-ofctl add-flow br0 "table=0, priority=1, cookie=100, in_port=3,
>> > actions=set_field:100->tun_id output:4"
>> >
>> >
>> >
>> > The same bridge br0 is being used as the local interface for vxlan
>> > tunnel. Even though this configuration is invalid, we should not see the
>> > kernel crash.
>> >
>>
>> I agree this should not cause crash.
>> Can you post the crash or investigate why it is crashing I think such
>> configuration would hit the networking stack recursion limit
>> (XMIT_RECURSION_LIMIT) and then the packet would be dropped. I am not
>> sure which spinlock recursion issue you are referring to.
>>
>>
>> >
>> >
>> >
>> >
>> > On Tue, Jun 12, 2018 at 11:55 AM, Pravin Shelar  wrote:
>> >>
>> >>
>> >>
>> >> On Tue, May 22, 2018 at 10:16 PM, Neelakantam Gaddam
>> >>  wrote:
>> >>>
>> >>> This patch fixes the kernel soft lockup issue with vxlan configuration
>> >>> where the tunneled packet is sent on the same bridge where vxlan port
>> >>> is
>> >>> attched to. It detects the loop in vxlan xmit functionb and drops if
>> >>> loop is
>> >>> detected.
>> >>>
>> >>> Signed-off-by: Neelakantam Gaddam 
>> >>> ---
>> >>>  datapath/linux/compat/vxlan.c | 6 --
>> >>>  1 file changed, 4 insertions(+), 2 deletions(-)
>> >>>
>> >>> diff --git a/datapath/linux/compat/vxlan.c
>> >>> b/datapath/linux/compat/vxlan.c
>> >>> index 287dad2..00562fa 100644
>> >>> --- a/datapath/linux/compat/vxlan.c
>> >>> +++ b/datapath/linux/compat/vxlan.c
>> >>> @@ -1115,7 +1115,8 @@ static void vxlan_xmit_one(struct sk_buff *skb,
>> >>> struct net_device *dev,
>> >>> goto tx_error;
>> >>> }
>> >>>
>> >>> -   if (rt->dst.dev == dev) {
>> >>> +   if ((rt->dst.dev == dev) ||
>> >>> +   (OVS_CB(skb)->input_vport->dev ==
>> >>> rt->dst.dev)) {
>> >>
>> >>
>> >> I am not sure which case the  OVS_CB(skb)->input_vport->dev is not same
>> >> as the dev when there is recursion. Can you explain how to reproduce it?
>> >>
>> >
>> >
>> >
>> > --
>> > Thanks & Regards
>> > Neelakantam Gaddam
>
>
>
>
> --
> Thanks & Regards
> Neelakantam Gaddam
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v9 0/7] OVS-DPDK flow offload with rte_flow

[Shahaf Shuler]

Hi Flavio,

I repeatedly tried to reproduce the results you have but failed. In fact I am 
now see better results for the hw-offload (thanks to optimization I learned 
from your script). 

w/o hw-offload I get fwd rate of ~7.76Mpps 

pmd thread numa_id 1 core_id 1:  
packets received: 475469632  
packet recirculations: 0 
avg. datapath passes per packet: 1.00
emc hits: 475466456  
megaflow hits: 3072  
avg. subtable lookups per megaflow hit: 1.00 
miss with success upcall: 1  
miss with failed upcall: 71  
avg. packets per output batch: 20.75 
idle cycles: 0 (0.00%)   
processing cycles: 135318738225 (100.00%)
avg cycles per packet: 284.60 (135318738225/475469632)   
avg processing cycles per packet: 284.60 (135318738225/475469632)

w/ hw-offload I get fwd rate of ~12Mpps. 

pmd thread numa_id 1 core_id 1:  
packets received: 213505047  
packet recirculations: 0 
avg. datapath passes per packet: 1.00
emc hits: 213502401  
megaflow hits: 2554  
avg. subtable lookups per megaflow hit: 1.00 
miss with success upcall: 1  
miss with failed upcall: 71  
avg. packets per output batch: 20.09 
idle cycles: 0 (0.00%)   
processing cycles: 40049794229 (100.00%) 
avg cycles per packet: 187.58 (40049794229/213505047)
avg processing cycles per packet: 187.58 (40049794229/213505047)

around 71% improvement and ~100 cycles of the datapath were saved by the HW 
offload.
Moreover, this is the most simple configuration. With more flows we can assume 
the gain will be much higher. 

Am wondering if a third person can check it to understand which system among 
the two is configured badly. 
Ian/Finn - can you please have a try? With Flavio scripts is really simple. 
BTW - you can use the latest stable tree of 17.11 (what will be 17.11.3), it 
includes the custom fixes I gave Flavio. 

More below 

Thursday, June 7, 2018 10:15 PM, Flavio Leitner:
> Subject: Re: [ovs-dev] [PATCH v9 0/7] OVS-DPDK flow offload with rte_flow
> 
> On Thu, Jun 07, 2018 at 07:24:25AM +, Shahaf Shuler wrote:
> > Thanks Flavio for the update,
> >
> > Wednesday, June 6, 2018 5:48 PM, Flavio Leitner:
> > > Subject: Re: [ovs-dev] [PATCH v9 0/7] OVS-DPDK flow offload with
> > > rte_flow
> > >
> > >
> > > (Looks like Intel doesn't allow shell scripts, so if you need I will
> > > put somewhere else to download)
> >
> > If you can pass me the scripts for your run it will be great, please include
> also packet gen pattern. I will try to reproduce it in house.
> > Looks like the flow is inserted correctly now.
> >
> > From my testing using single loopback rule, single core, single queue and
> single flow injected I can see ~20% improvement with the HW offload (from
> 5.2Mpps -> 6.2 Mpps).
> 
> Looking forward to get the same over here.
> 
> I added the Xena script to create the traffic and the script that brings up 
> the
> environment here:
> https://emea01.safelinks.protection.outlook.com/?url=http:%2F%2Fpeople.
> redhat.com%2F~fleitner%2Fhwol%2F=02%7C01%7Cshahafs%40mellan
> ox.com%7C0249e6ca39384c11979508d5ccab0556%7Ca652971c7d2e4d9ba6a4
> d149256f461b%7C0%7C1%7C636639957263712271=fWCBNjBGTpP1DS
> Nk6vHSebgyIrbkc1mnmvm%2BiLfJqd4%3D=0

Unfortunately I don't have Xena traffic generator. I used SW based traffic 
generator w/ 128 IP rules in round robin to reach my results. 
Is it possible some packets are dropped due to integrity reasons (CRC/checksum) 
? 

Can you attach the "ovs-appctl dpif-netdev/pmd-stats-show" output of each case?


> 
> Below is the packet headers I am providing to the traffic generator which will
> pad to 64 bytes, and increase the address from 10.0.0.1 to 10.0.0.128:
> 
>   1 0.00   10.0.0.1  10.0.0.2  IPv4 34
> 
> Frame 1: 34 bytes on wire (272 bits), 34 bytes captured (272 bits)
> Encapsulation type: Ethernet (1)
> Arrival Time: Jun  7, 2018 16:10:56.50865 -03
> [Time shift for this packet: 0.0 seconds]
> Epoch Time: