Re: [ovs-dev] [PATCH] dpdk: Fix abort on double free.

2016-11-28 Thread Ilya Maximets 
On 28.11.2016 21:55, Aaron Conole wrote:
> Ilya Maximets  writes:
> 
>> According to DPDK API (lib/librte_eal/common/include/rte_eal.h):
>>
>>  "After the call to rte_eal_init(), all arguments argv[x]
>>   with x < ret may be modified and should not be accessed
>>   by the application."
>>
>> This means, that OVS must not free the arguments passed to DPDK.
>> In real world, 'rte_eal_init()' replaces the last argument in
>> 'dpdk_argv' with the first one by doing this:
> 
> Thanks for spotting this error, Ilya.
> 
>>  # eal_parse_args() from lib/librte_eal/linuxapp/eal/eal.c
>>
>>  char *prgname = argv[0];
>>  ...
>>  if (optind >= 0)
>>  argv[optind-1] = prgname;
>>
>> This leads to double free inside 'deferred_argv_release()' and
>> possible ABORT at exit:
> 
> I haven't seen this, which is both shocking and scary - the commit which
> does this copy is almost 4 years old;  did you have to do anything
> specific for this behavior to occur?  Did something change in DPDK
> recently that exposed this behavior?  Just wondering how you reproduced
> it.

Abort was caught up accidentally. I'm able to reproduce it on my a
little unusual testing system (ARMv8 + Fedora 21 + clang 3.5) without
any specific manipulations. The bug exists always but it's hard
for libc to detect double free here because there are many other
frees/allocations at exit time. I've used following patch to confirm
the issue if it wasn't detected by libc:

diff --git a/lib/dpdk.c b/lib/dpdk.c
index 49a589a..65d2d28 100644
--- a/lib/dpdk.c
+++ b/lib/dpdk.c
@@ -258,6 +258,8 @@ deferred_argv_release(void)
 {
 int result;
 for (result = 0; result < dpdk_argc; ++result) {
+VLOG_INFO("DPDK ARGV release: %2d: 0x%" PRIx64 " (%s)",
+  result, (intptr_t)dpdk_argv[result], dpdk_argv[result]);
 free(dpdk_argv[result]);
 }
 


> 
>> *** Error in `ovs-vswitchd': double free or corruption (fasttop) <...> ***
>>  Program received signal SIGABRT, Aborted.
>>
>>  #0  raise () from /lib64/libc.so.6
>>  #1  abort () from /lib64/libc.so.6
>>  #2  __libc_message () from /lib64/libc.so.6
>>  #3  free () from /lib64/libc.so.6
>>  #4  deferred_argv_release () at lib/dpdk.c:261
>>  #5  __run_exit_handlers () from /lib64/libc.so.6
>>  #6  exit () from /lib64/libc.so.6
>>  #7  __libc_start_main () from /lib64/libc.so.6
>>  #8  _start ()
>>
>> Fix that by not calling free for the memory passed to DPDK.
>>
>> CC: Aaron Conole 
>> Fixes: bab694097133 ("netdev-dpdk: Convert initialization from cmdline to 
>> db")
>> Signed-off-by: Ilya Maximets 
>> ---
> 
> We need to free the memory - I think that is not a question;

Actually, it is. According to DPDK API (see above) 'rte_eal_init()'
takes the ownership of 'argv'. This means that we must not free
or use this memory.

Some thoughts:
DPDK internally doesn't free this memory, but it's not the reason to
touch it from the outside. Actually, DPDK API change required here to
support freeing of this resources if needed. But until there is no
'rte_eal_uninit()' such API change isn't actually useful.

Also, I forget to remove the variables. So, the following incremental
to my original patch required:


diff --git a/lib/dpdk.c b/lib/dpdk.c
index 2014946..4201149 100644
--- a/lib/dpdk.c
+++ b/lib/dpdk.c
@@ -250,9 +250,6 @@ get_dpdk_args(const struct smap *ovs_other_config, char 
***argv,
 return i + extra_argc;
 }
 
-static char **dpdk_argv;
-static int dpdk_argc;
-
 static void
 dpdk_init__(const struct smap *ovs_other_config)
 {
@@ -370,9 +367,6 @@ dpdk_init__(const struct smap *ovs_other_config)
 }
 }
 
-dpdk_argv = argv;
-dpdk_argc = argc;
-
 rte_memzone_dump(stdout);
 
 /* We are called from the main thread here */


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

Re: [ovs-dev] [PATCH v2] datapath: compat: tunnel: Check if device is UP.

2016-11-28 Thread Ben Pfaff 
On Mon, Nov 28, 2016 at 05:37:48PM -0800, Joe Stringer wrote:
> On 28 November 2016 at 17:00, Ben Pfaff  wrote:
> > On Fri, Oct 21, 2016 at 11:47:54AM -0700, Pravin Shelar wrote:
> >> On Fri, Oct 21, 2016 at 11:39 AM, Joe Stringer  wrote:
> >> > On 21 October 2016 at 11:37, Joe Stringer  wrote:
> >> >> On 21 October 2016 at 10:55, Pravin B Shelar  wrote:
> >> >>> On upstream kernel datapath OVS make use of networking devices
> >> >>> where networking stack does check if device is UP. following
> >> >>> patch adds same check in case of compat tunneling implementation.
> >> >>> This check also fixes kernel crash in case vxlan device was
> >> >>> brought down by user.
> >> >>>
> >> >>> CPU: 4 PID: 12988 Comm: handler903 Tainted:
> >> >>> RIP: 0010:[] vxlan_xmit_one.constprop.50+0x47/0x1210 
> >> >>> [openvswitch]
> >> >>> Call Trace:
> >> >>>  [] rpl_vxlan_xmit+0x55/0x80 [openvswitch]
> >> >>>  [] ovs_vport_send+0x44/0xb0 [openvswitch]
> >> >>>  [] do_output+0x65/0x180 [openvswitch]
> >> >>>  [] do_execute_actions+0x10c/0x860 [openvswitch]
> >> >>>  [] ovs_execute_actions+0x40/0x130 [openvswitch]
> >> >>>  [] ovs_packet_cmd_execute+0x2c9/0x2f0 [openvswitch]
> >> >>>  [] genl_family_rcv_msg+0x1cd/0x400
> >> >>>  [] genl_rcv_msg+0x91/0xd0
> >> >>>  [] netlink_rcv_skb+0xa9/0xc0
> >> >>>  [] genl_rcv+0x28/0x40
> >> >>>  [] netlink_unicast+0x16a/0x210
> >> >>>  [] netlink_sendmsg+0x317/0x430
> >> >>>  [] sock_sendmsg+0xb0/0xf0
> >> >>>  [] ___sys_sendmsg+0x3a9/0x3c0
> >> >>>  [] __sys_sendmsg+0x51/0x90
> >> >>>  [] SyS_sendmsg+0x12/0x20
> >> >>>  [] system_call_fastpath+0x16/0x1b
> >> >>>
> >> >>> Reported-by: Huanglili (lee) 
> >> >>> Signed-off-by: Pravin B Shelar 
> >> >>
> >> >> I'm not 100% convinced that this solves the issue (or whether this
> >> >> affects upstream as well), but it seems like this at least makes it
> >> >> significantly less likely to crash - If I understand correctly,
> >> >> setting the vxlan port down currently guarantees a crash, whereas this
> >> >> patch would mean it either doesn't crash, or it only crashes based on
> >> >> a race condition. Specifically, one where the reconfiguration gets
> >> >> past the synchronize_net() into ndo_stop() (before clearing IFF_UP)
> >> >> while OVS manages to execute all of a flow up until output and
> >> >> proceeds past this check before the reconfiguration thread clears it.
> >> >>
> >> >> Probably the type of test to check this would be running heavy traffic
> >> >> through a vxlan port, then toggle the port up and down.
> >> >>
> >> >> Will you also check upstream?
> >> >>
> >> >> Anyway, this LGTM so:
> >> >> Acked-by: Joe Stringer 
> >> >
> >> > Hmm, the original reporter said that the issue is not fixed. Perhaps
> >> > we should hold off on this?
> >>
> >> Yes, I would like to see another solution before merging this patch.
> >
> > Joe, do you know whether this needs further action?  It's not merged and
> > I don't know whether it should be.
> 
> No, it looks like commit 76e671eade40 ("datapath: backport: vxlan:
> avoid using stale vxlan socket.") in the OVS tree fixes this issue,
> rendering this version of the fix unnecessary.

Thanks, I marked it in patchwork as "not applicable".
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v4] ovn-northd: Force SNAT for multiple gateway routers.

2016-11-28 Thread Mickey Spiegel 
Acked-by: Mickey Spiegel 

A few comments and nits below.

On Thu, Nov 17, 2016 at 10:17 PM, Gurucharan Shetty  wrote:

> When multiple gateway routers exist, a packet can
> enter any gateway router. Once the packet reaches its
> destination, its reverse direction should be via the
> same gateway router.  This is achieved by doing a SNAT
> of the packet in the inward direction (towards logical space)
> with a IP address of the gateway router such that packet travels back
> to the same gateway router.
>
> To do the above, we introduce two new options in the logical router.
>
> options:dnat_force_snat_ip=$IP will force SNAT any packet to $IP if
> it has been previously DNATted.
>
> options:lb_force_snat_ip=$IP will force SNAT any packet to $IP if
> it has been previously load-balanced.
>
> Signed-off-by: Gurucharan Shetty 
> ---
> v3->v4:
>   1. Use an additional bit to hint that force SNAT has to be done for LB.
>   2. The established traffic of LB gets its own flows just like new
> connection.
> ---
>  ovn/lib/logical-fields.c|   8 +
>  ovn/lib/logical-fields.h|  10 ++
>  ovn/northd/ovn-northd.8.xml |  62 ++-
>  ovn/northd/ovn-northd.c | 155 --
>  ovn/ovn-nb.xml  |  25 +++
>  tests/system-ovn.at | 386 ++
> ++
>  6 files changed, 628 insertions(+), 18 deletions(-)
>
> diff --git a/ovn/lib/logical-fields.c b/ovn/lib/logical-fields.c
> index d4578c3..fa134d6 100644
> --- a/ovn/lib/logical-fields.c
> +++ b/ovn/lib/logical-fields.c
> @@ -88,6 +88,14 @@ ovn_init_symtab(struct shash *symtab)
>  char flags_str[16];
>  snprintf(flags_str, sizeof flags_str, "flags[%d]",
> MLF_ALLOW_LOOPBACK_BIT);
>  expr_symtab_add_subfield(symtab, "flags.loopback", NULL, flags_str);
> +snprintf(flags_str, sizeof flags_str, "flags[%d]",
> + MLF_FORCE_SNAT_FOR_DNAT_BIT);
> +expr_symtab_add_subfield(symtab, "flags.force_snat_for_dnat", NULL,
> + flags_str);
> +snprintf(flags_str, sizeof flags_str, "flags[%d]",
> + MLF_FORCE_SNAT_FOR_LB_BIT);
> +expr_symtab_add_subfield(symtab, "flags.force_snat_for_lb", NULL,
> + flags_str);
>
>  /* Connection tracking state. */
>  expr_symtab_add_field(symtab, "ct_mark", MFF_CT_MARK, NULL, false);
> diff --git a/ovn/lib/logical-fields.h b/ovn/lib/logical-fields.h
> index a1f1da6..696c529 100644
> --- a/ovn/lib/logical-fields.h
> +++ b/ovn/lib/logical-fields.h
> @@ -47,6 +47,8 @@ void ovn_init_symtab(struct shash *symtab);
>  enum mff_log_flags_bits {
>  MLF_ALLOW_LOOPBACK_BIT = 0,
>  MLF_RCV_FROM_VXLAN_BIT = 1,
> +MLF_FORCE_SNAT_FOR_DNAT_BIT = 2,
> +MLF_FORCE_SNAT_FOR_LB_BIT = 3,
>  };
>
>  /* MFF_LOG_FLAGS_REG flag assignments */
> @@ -59,6 +61,14 @@ enum mff_log_flags {
>   * VXLAN encapsulation.  Egress port information is available for
>   * Geneve and STT tunnel types. */
>  MLF_RCV_FROM_VXLAN = (1 << MLF_RCV_FROM_VXLAN_BIT),
> +
> +/* Indicate that a packet needs a force SNAT in the gateway router
> when
> + * DNAT has taken place. */
> +MLF_FORCE_SNAT_FOR_DNAT = (1 << MLF_FORCE_SNAT_FOR_DNAT_BIT),
> +
> +/* Indicate that a packet needs a force SNAT in the gateway router
> when
> + * load-balancing has taken place. */
> +MLF_FORCE_SNAT_FOR_LB = (1 << MLF_FORCE_SNAT_FOR_LB_BIT),
>  };
>
>  #endif /* ovn/lib/logical-fields.h */
> diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
> index df53d4c..11245c6 100644
> --- a/ovn/northd/ovn-northd.8.xml
> +++ b/ovn/northd/ovn-northd.8.xml
> @@ -1153,6 +1153,14 @@ icmp4 {
>  
>
>  
> +  If the Gateway router has been configured to force SNAT (any
> +  previously DNATted or Load-balanced packets) to B,
> +  a priority-100 flow matches ip 
> +  ip4.dst == B with an action ct_snat;
> +  next;.
> +
> +
> +
>A priority-0 logical flow with match 1 has actions
>next;.
>  
> @@ -1176,7 +1184,23 @@ icmp4 {
>   P  P.dst == PORT
>   with an action of ct_lb(args) code>,
>  where args contains comma separated IPv4 addresses (and
> -optional port numbers) to load balance to.
> +optional port numbers) to load balance to.  If the Gateway router
> +is configured to force SNAT any load-balanced packets, the above
> +action will be replaced by flags.force_snat_for_lb = 1;
> +ct_lb(args);.
> +  
> +
> +  
> +For all the configured load balancing rules for Gateway router in
> +OVN_Northbound database that includes a L4 port
> +PORT of protocol P and IPv4 address
> +VIP, a priority-120 flow that matches on
> +ct.est  ip  ip4.dst == VIP
> + P  P.dst == PORT
> + with an action of ct_dnat;.
> +If the Gateway router is

[ovs-dev] Administración de Sueldos - En Línea

2016-11-28 Thread prestaciones y beneficios 
 

En línea y en Vivo / Para todo su Equipo con una sola Conexión 

Administración de Sueldos y 
Programas de Compensación
14 de diciembre - Online en Vivo - 10:00 a 13:00 y de 15:00 a 18:00 Hrs   
 
Mantenga salarios competitivos sin dañar la economía de su empresa. Las 
presiones económicas y las regulaciones del gobierno pueden convertir su 
política de sueldos y salarios en una maraña de contradicciones. Analice la 
situación actual.
 

"Pregunte Por Nuestra Promoción Navideña" 




Temario: 

1. Anatomía de los Sueldos y los programas de Compensación.


2. Problemática actual de las prácticas de compensación.

3. Análisis, descripción y valuación de puestos.

4. Administración de prestaciones y beneficios.




...¡Y mucho más!


 
¿Requiere la información a la Brevedad?
responda este email con la palabra: 
Info - Sueldos.
centro telefónico: 018002129393
 

Lic. Ángel Hernández
Coordinador de Evento


 
¿Demasiados mensajes en su cuenta? Responda este mensaje indicando que solo 
desea recibir CALENDARIO y sólo recibirá un correo al mes. Si desea cancelar la 
suscripción, solicite su BAJA. 
 

 

 

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

Re: [ovs-dev] [PATCH 2/2] ovn-trace: Fix implementation of get_arp and get_nd logical actions.

2016-11-28 Thread Ben Pfaff 
On Mon, Nov 28, 2016 at 02:27:11PM -0500, Aaron Conole wrote:
> Ben Pfaff  writes:
> 
> > These actions looked up the MAC binding but failed to update eth.dst with
> > the result.
> >
> > Signed-off-by: Ben Pfaff 
> > ---
> 
> This looks safe to me.
> 
> Reviewed-by: Aaron Conole 

Thanks Aaron.  I applied these to master and branch-2.6.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 7/7] checkpatch: Add file-parsing mode

2016-11-28 Thread Ben Pfaff 
On Fri, Oct 21, 2016 at 02:49:09PM -0400, Aaron Conole wrote:
> This adds a new argument and feature, 'check-file', which will allow
> checkpatch to run against files instead of only against patches.
> 
> Signed-off-by: Aaron Conole 

Thanks for the patches.  I applied all of these to master.  I didn't
test them; I hope that you did.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 1/7] checkpatch: Announce the file where errors occur

2016-11-28 Thread Ben Pfaff 
On Fri, Oct 21, 2016 at 02:49:03PM -0400, Aaron Conole wrote:
> This makes finding the warning and error marks much easier.
> 
> Signed-off-by: Aaron Conole 

Thanks, I applied this to master, it's certainly going to be a help
sometimes.

However, I'm used to error message in a format like
filename:lineno: message
which a lot of editors can use to jump directly to the file and line
number.  I don't know whether they support this kind of format; if they
tend to, it's also fine.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] datapath: compat: tunnel: Check if device is UP.

2016-11-28 Thread Ben Pfaff 
On Fri, Oct 21, 2016 at 11:47:54AM -0700, Pravin Shelar wrote:
> On Fri, Oct 21, 2016 at 11:39 AM, Joe Stringer  wrote:
> > On 21 October 2016 at 11:37, Joe Stringer  wrote:
> >> On 21 October 2016 at 10:55, Pravin B Shelar  wrote:
> >>> On upstream kernel datapath OVS make use of networking devices
> >>> where networking stack does check if device is UP. following
> >>> patch adds same check in case of compat tunneling implementation.
> >>> This check also fixes kernel crash in case vxlan device was
> >>> brought down by user.
> >>>
> >>> CPU: 4 PID: 12988 Comm: handler903 Tainted:
> >>> RIP: 0010:[] vxlan_xmit_one.constprop.50+0x47/0x1210 
> >>> [openvswitch]
> >>> Call Trace:
> >>>  [] rpl_vxlan_xmit+0x55/0x80 [openvswitch]
> >>>  [] ovs_vport_send+0x44/0xb0 [openvswitch]
> >>>  [] do_output+0x65/0x180 [openvswitch]
> >>>  [] do_execute_actions+0x10c/0x860 [openvswitch]
> >>>  [] ovs_execute_actions+0x40/0x130 [openvswitch]
> >>>  [] ovs_packet_cmd_execute+0x2c9/0x2f0 [openvswitch]
> >>>  [] genl_family_rcv_msg+0x1cd/0x400
> >>>  [] genl_rcv_msg+0x91/0xd0
> >>>  [] netlink_rcv_skb+0xa9/0xc0
> >>>  [] genl_rcv+0x28/0x40
> >>>  [] netlink_unicast+0x16a/0x210
> >>>  [] netlink_sendmsg+0x317/0x430
> >>>  [] sock_sendmsg+0xb0/0xf0
> >>>  [] ___sys_sendmsg+0x3a9/0x3c0
> >>>  [] __sys_sendmsg+0x51/0x90
> >>>  [] SyS_sendmsg+0x12/0x20
> >>>  [] system_call_fastpath+0x16/0x1b
> >>>
> >>> Reported-by: Huanglili (lee) 
> >>> Signed-off-by: Pravin B Shelar 
> >>
> >> I'm not 100% convinced that this solves the issue (or whether this
> >> affects upstream as well), but it seems like this at least makes it
> >> significantly less likely to crash - If I understand correctly,
> >> setting the vxlan port down currently guarantees a crash, whereas this
> >> patch would mean it either doesn't crash, or it only crashes based on
> >> a race condition. Specifically, one where the reconfiguration gets
> >> past the synchronize_net() into ndo_stop() (before clearing IFF_UP)
> >> while OVS manages to execute all of a flow up until output and
> >> proceeds past this check before the reconfiguration thread clears it.
> >>
> >> Probably the type of test to check this would be running heavy traffic
> >> through a vxlan port, then toggle the port up and down.
> >>
> >> Will you also check upstream?
> >>
> >> Anyway, this LGTM so:
> >> Acked-by: Joe Stringer 
> >
> > Hmm, the original reporter said that the issue is not fixed. Perhaps
> > we should hold off on this?
> 
> Yes, I would like to see another solution before merging this patch.

Joe, do you know whether this needs further action?  It's not merged and
I don't know whether it should be.

Thanks,

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

Re: [ovs-dev] [PATCH v12 rebased 1/3] userspace: add support for pop_eth and push_eth actions

2016-11-28 Thread Ben Pfaff 
On Wed, Oct 19, 2016 at 07:21:17PM +0200, Jiri Benc wrote:
> From: Lorand Jakab 
> 
> These actions will allow L2->L3 and L3->L2 switching, and are supposed
> to be added to flows installed in the datapath transparently by
> ovs-vswitchd.
> 
> Signed-off-by: Lorand Jakab 
> Signed-off-by: Simon Horman 
> Signed-off-by: Jiri Benc 

I know that this isn't intended to be applied yet, according to the
cover letter, but it doesn't build:

../lib/odp-util.c:5504:15: error: no member named 'base_layer' in 'struct 
flow'
../lib/odp-util.c:5504:40: error: no member named 'base_layer' in 'struct 
flow'
../lib/odp-util.c:5505:19: error: no member named 'base_layer' in 'struct 
flow'
../lib/odp-util.c:5505:33: error: use of undeclared identifier 'LAYER_2'
../lib/odp-util.c:5512:20: error: no member named 'base_layer' in 'struct 
flow'
../lib/odp-util.c:5512:40: error: no member named 'base_layer' in 'struct 
flow'

and furthermore I can't any mention of base_layer in the OVS history so
I'm not sure what's going on here.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] tests: Test reset_counts in flow_mod message

2016-11-28 Thread Tony van der Peet 
commit 23c753793204df86aa6eedd00ec62911a1ef9559
Author: Tony van der Peet 
Date:   Tue Nov 29 13:16:17 2016 +1300

tests: Test reset_counts in flow_mod message

Setting the reset_counts flag in a flow_mod message should cause the
flow counters to be reset. Test this by:

- creating a flow
- sending a single packet
- modifying the flow with reset_counts flag set
- dumping the flow and expecting n_packets=0, n_bytes=1

This is assertion 140.80 of the ONF conformance test specification for
v1.3.4.

diff --git a/tests/ofproto.at b/tests/ofproto.at
index d360b7a..1723e77 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -1648,6 +1648,19 @@ OFPST_FLOW reply (OF1.2):
 OVS_VSWITCHD_STOP
 AT_CLEANUP

+AT_SETUP([ofproto - mod flow with reset_counts flag - OF1.3])
+OVS_VSWITCHD_START
+add_of_ports br0 1 2 3
+AT_CHECK([ovs-ofctl -O OpenFlow13 add-flow br0 in_port=1,actions=2])
+ovs-appctl netdev-dummy/receive p1 
'in_port(1),eth(src=50:54:00:00:00:07,dst=50:54:00:00:00:05),eth_type(0x0800),ipv4(src=192.168.0.2,dst=192.168.0.1,proto=6,tos=0,ttl=64,frag=no)'
+AT_CHECK([ovs-ofctl -O openflow13 mod-flows br0 
reset_counts,in_port=1,actions=3])
+AT_CHECK([ovs-ofctl -O openflow13 dump-flows br0 | ofctl_strip | sort], [0], 
[dnl
+ in_port=1 actions=output:3
+OFPST_FLOW reply (OF1.3):
+])
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_SETUP([ofproto - del flows with cookies])
 OVS_VSWITCHD_START
 AT_CHECK([ovs-ofctl add-flow br0 cookie=0x1,in_port=1,actions=1])

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

Re: [ovs-dev] [PATCH] tests: Test reset_counts in flow_mod message

2016-11-28 Thread Tony van der Peet 
Actually, I have a split personality wrt Open vSwitch. I should
probably post this from my work email account. See if that works any
better.

Tony

On Tue, Nov 29, 2016 at 1:36 PM, Ben Pfaff  wrote:
> Hi, thanks for the patch.  It came across word-wrapped, e.g. the
> following should be only a single line:
>
> On Tue, Nov 29, 2016 at 01:22:41PM +1300, Tony van der Peet wrote:
>> +ovs-appctl netdev-dummy/receive p1
>> 'in_port(1),eth(src=50:54:00:00:00:07,dst=50:54:00:00:00:05),eth_type(0x0800),ipv4(src=192.168.0.2,dst=192.168.0.1,proto=6,tos=0,ttl=64,frag=no)'
>
> I see that you're posting from gmail.  It's easy to set up "git
> send-email" with gmail, and once you have it set up it always works
> properly.  You can find instructions in various places, including the
> git-send-email manpage.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] tests: Test reset_counts in flow_mod message

2016-11-28 Thread Ben Pfaff 
Hi, thanks for the patch.  It came across word-wrapped, e.g. the
following should be only a single line:

On Tue, Nov 29, 2016 at 01:22:41PM +1300, Tony van der Peet wrote:
> +ovs-appctl netdev-dummy/receive p1
> 'in_port(1),eth(src=50:54:00:00:00:07,dst=50:54:00:00:00:05),eth_type(0x0800),ipv4(src=192.168.0.2,dst=192.168.0.1,proto=6,tos=0,ttl=64,frag=no)'

I see that you're posting from gmail.  It's easy to set up "git
send-email" with gmail, and once you have it set up it always works
properly.  You can find instructions in various places, including the
git-send-email manpage.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] tests: Test reset_counts in flow_mod message

2016-11-28 Thread Tony van der Peet 
commit 23c753793204df86aa6eedd00ec62911a1ef9559
Author: Tony van der Peet 
Date:   Tue Nov 29 13:16:17 2016 +1300

tests: Test reset_counts in flow_mod message

Setting the reset_counts flag in a flow_mod message should cause the
flow counters to be reset. Test this by:

- creating a flow
- sending a single packet
- modifying the flow with reset_counts flag set
- dumping the flow and expecting n_packets=0, n_bytes=1

This is assertion 140.80 of the ONF conformance test specification for
v1.3.4.

diff --git a/tests/ofproto.at b/tests/ofproto.at
index d360b7a..1723e77 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -1648,6 +1648,19 @@ OFPST_FLOW reply (OF1.2):
 OVS_VSWITCHD_STOP
 AT_CLEANUP

+AT_SETUP([ofproto - mod flow with reset_counts flag - OF1.3])
+OVS_VSWITCHD_START
+add_of_ports br0 1 2 3
+AT_CHECK([ovs-ofctl -O OpenFlow13 add-flow br0 in_port=1,actions=2])
+ovs-appctl netdev-dummy/receive p1
'in_port(1),eth(src=50:54:00:00:00:07,dst=50:54:00:00:00:05),eth_type(0x0800),ipv4(src=192.168.0.2,dst=192.168.0.1,proto=6,tos=0,ttl=64,frag=no)'
+AT_CHECK([ovs-ofctl -O openflow13 mod-flows br0
reset_counts,in_port=1,actions=3])
+AT_CHECK([ovs-ofctl -O openflow13 dump-flows br0 | ofctl_strip |
sort], [0], [dnl
+ in_port=1 actions=output:3
+OFPST_FLOW reply (OF1.3):
+])
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_SETUP([ofproto - del flows with cookies])
 OVS_VSWITCHD_START
 AT_CHECK([ovs-ofctl add-flow br0 cookie=0x1,in_port=1,actions=1])
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC] [PATCH] ovn: Support sample action in logical datapath

2016-11-28 Thread Ben Pfaff 
On Fri, Oct 14, 2016 at 04:35:46PM +0500, Valentine Sinitsyn wrote:
> This is a quick attempt to implement sample action at logical port level.The
> goal is to export IPFIX flows for logical ports, yet it is easy to extend
> this approach to logical switches as well.
> 
> Nothing is done to provision OVS instances with required
> Flow_Sample_Collector_Set and IPFIX entries at this point.
> 
> Does the approach I take looks sensible? If so, I can add tests and re-send
> this patch for in-depth review.
> 
> Many thanks.
> 
> Signed-off-by: Valentine Sinitsyn 

Sorry about the long delay in review.  It's been a difficult month.

This is pretty cool!  The integration among OVS and OVN and IPFIX is
graceful.

The part that worries me is the CMS integration.  Have you actually
built that integration already (for which CMS)?  I have two concerns.
First, I'd prefer to see at least one CMS (probably OpenStack) support
this at or around the time that it goes into OVN.  Second, I have some
skepticism around the idea that the CMS should configure the
Flow_Sample_Collector_Set, etc., because OVN doesn't currently require
the CMS to have any connectivity to OVSDB on each of the hypervisors and
this would require the CMS to add that support.

Do you have any thoughts about supporting other monitoring technology
that OVS supports (e.g. sFlow) using similar techniques?

I hope that the long delay does not discourage you from following up.  I
hope to be more responsive now.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] OVS-Hyper-V-Discovery-Agent Design Document

2016-11-28 Thread Nithin Raju 
Ben,
The current proposal that Yin has put out calls for a new daemon that monitors 
VIFs that show up on the native HyperV swtitch, gives them a name and adds it 
to OVSDB. It does  it change the existing workflow. You can think of it as how 
libvirt integrates with OVS in terms of adding ports to OVSDB.

Alin's argument is to integrate this new daemon into ovs-vSwitchd and have 
ovs-vswitchd add ports to OVSDB when ports show up on the HyperV switch. Is 
there any precedence to ovs-vswitchd adding ports to OVSDB? Would this be a 
divergence?

Thanks,
-- Nithin




On Mon, Nov 28, 2016 at 4:00 PM -0800, "Ben Pfaff" 
> wrote:

Does this actually call for vswitchd to add ports?  Reading it, it looks
like the new daemon does that.  But I didn't read it carefully enough to
understand it in full.

On Mon, Nov 28, 2016 at 11:13:21PM +, Nithin Raju wrote:
> hi Alin,
> Thanks for the comments.
>
> There has not been a case so far (AFAIK) where vswitchd added ports to OVSDB. 
> This would be a diversion from that. We’ll have to take this up with Ben or 
> Justin to see how they feel.
>
> Thanks,
> -- Nithin
>
> On Nov 28, 2016, at 11:18 AM, Alin Serdean 
> > 
> wrote:
>
> Hi Yin,
>
> Thanks a lot for the patch and the document!
>
> There are a few concerns that I have with this implementation:
> - the need for a new daemon,
> - use of cmd calls to add/delete ports,
> - addition of .NET to compile the binaries.
>
> As previously discussed in the meetings, we could just use 'ovs-vswitchd' 
> with an argument i.e. '--integration_bridge=' to add the ports. We 
> see the port creations in the kernel and we could issue an event to the 
> userspace to add ports under that bridge. We could add a new type of 'action' 
> or try to reuse the events that are already present in the implementation.
> Another advantage to use ovs-vswitchd is that we could talk directly to 
> ovsdb. Also, no additional dependency needed.
>
> We could add the argument '--integration_bridge=' when creating the 
> service via the installer with a nice text to inform the user about what it 
> does. Changing the service properties is normal activity for a system 
> administrator, so he could easily remove it afterwards.
>
> Thanks,
> Alin.
>
> -Original Message-
> From: ovs-dev-boun...@openvswitch.org 
> [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Yin Lin
> Sent: Monday, November 21, 2016 10:07 PM
> To: d...@openvswitch.org
> Subject: [ovs-dev] OVS-Hyper-V-Discovery-Agent Design Document
>
> Hi,
>
> Below is a document that describes the design and implementation of VIF
> discovery agent for Hyper-V that I have been working on. The coding has
> already been completed and will be sent out for review in a follow up patch.
> The document describes the effort and the choices made. Thanks Sairam
> Venugopal for the initial review and edit of the document
>
>
>
> Please have a look, and get in touch for any questions or comments.
>
>
>
> Thanks!
>
> -- Yin
> ___
> dev mailing list
> d...@openvswitch.org
> https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev=DgICAg=uilaK90D4TOVoH58JNXRgQ=-xl6DPE_Y3uQD-mpZD7osBo2iL4s3jwdmSjTlGgjlsQ=GMr-AZul4UsNIDzghSvQVWBWWf9ZXBYbRf5TFXxEtFg=tAcMGwbQ11t617kkksED_MalGT4y_bWtSDojOFp58vc=
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] OVS-Hyper-V-Discovery-Agent Design Document

2016-11-28 Thread Ben Pfaff 
Does this actually call for vswitchd to add ports?  Reading it, it looks
like the new daemon does that.  But I didn't read it carefully enough to
understand it in full.

On Mon, Nov 28, 2016 at 11:13:21PM +, Nithin Raju wrote:
> hi Alin,
> Thanks for the comments.
> 
> There has not been a case so far (AFAIK) where vswitchd added ports to OVSDB. 
> This would be a diversion from that. We’ll have to take this up with Ben or 
> Justin to see how they feel.
> 
> Thanks,
> -- Nithin
> 
> On Nov 28, 2016, at 11:18 AM, Alin Serdean 
> > 
> wrote:
> 
> Hi Yin,
> 
> Thanks a lot for the patch and the document!
> 
> There are a few concerns that I have with this implementation:
> - the need for a new daemon,
> - use of cmd calls to add/delete ports,
> - addition of .NET to compile the binaries.
> 
> As previously discussed in the meetings, we could just use 'ovs-vswitchd' 
> with an argument i.e. '--integration_bridge=' to add the ports. We 
> see the port creations in the kernel and we could issue an event to the 
> userspace to add ports under that bridge. We could add a new type of 'action' 
> or try to reuse the events that are already present in the implementation.
> Another advantage to use ovs-vswitchd is that we could talk directly to 
> ovsdb. Also, no additional dependency needed.
> 
> We could add the argument '--integration_bridge=' when creating the 
> service via the installer with a nice text to inform the user about what it 
> does. Changing the service properties is normal activity for a system 
> administrator, so he could easily remove it afterwards.
> 
> Thanks,
> Alin.
> 
> -Original Message-
> From: ovs-dev-boun...@openvswitch.org 
> [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Yin Lin
> Sent: Monday, November 21, 2016 10:07 PM
> To: d...@openvswitch.org
> Subject: [ovs-dev] OVS-Hyper-V-Discovery-Agent Design Document
> 
> Hi,
> 
> Below is a document that describes the design and implementation of VIF
> discovery agent for Hyper-V that I have been working on. The coding has
> already been completed and will be sent out for review in a follow up patch.
> The document describes the effort and the choices made. Thanks Sairam
> Venugopal for the initial review and edit of the document
> 
> 
> 
> Please have a look, and get in touch for any questions or comments.
> 
> 
> 
> Thanks!
> 
> -- Yin
> ___
> dev mailing list
> d...@openvswitch.org
> https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev=DgICAg=uilaK90D4TOVoH58JNXRgQ=-xl6DPE_Y3uQD-mpZD7osBo2iL4s3jwdmSjTlGgjlsQ=GMr-AZul4UsNIDzghSvQVWBWWf9ZXBYbRf5TFXxEtFg=tAcMGwbQ11t617kkksED_MalGT4y_bWtSDojOFp58vc=
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] OVS-Hyper-V-Discovery-Agent Design Document

2016-11-28 Thread Nithin Raju 
hi Alin,
Thanks for the comments.

There has not been a case so far (AFAIK) where vswitchd added ports to OVSDB. 
This would be a diversion from that. We’ll have to take this up with Ben or 
Justin to see how they feel.

Thanks,
-- Nithin

On Nov 28, 2016, at 11:18 AM, Alin Serdean 
> wrote:

Hi Yin,

Thanks a lot for the patch and the document!

There are a few concerns that I have with this implementation:
- the need for a new daemon,
- use of cmd calls to add/delete ports,
- addition of .NET to compile the binaries.

As previously discussed in the meetings, we could just use 'ovs-vswitchd' with 
an argument i.e. '--integration_bridge=' to add the ports. We see the 
port creations in the kernel and we could issue an event to the userspace to 
add ports under that bridge. We could add a new type of 'action' or try to 
reuse the events that are already present in the implementation.
Another advantage to use ovs-vswitchd is that we could talk directly to ovsdb. 
Also, no additional dependency needed.

We could add the argument '--integration_bridge=' when creating the 
service via the installer with a nice text to inform the user about what it 
does. Changing the service properties is normal activity for a system 
administrator, so he could easily remove it afterwards.

Thanks,
Alin.

-Original Message-
From: ovs-dev-boun...@openvswitch.org 
[mailto:ovs-dev-
boun...@openvswitch.org] On Behalf Of Yin Lin
Sent: Monday, November 21, 2016 10:07 PM
To: d...@openvswitch.org
Subject: [ovs-dev] OVS-Hyper-V-Discovery-Agent Design Document

Hi,

Below is a document that describes the design and implementation of VIF
discovery agent for Hyper-V that I have been working on. The coding has
already been completed and will be sent out for review in a follow up patch.
The document describes the effort and the choices made. Thanks Sairam
Venugopal for the initial review and edit of the document



Please have a look, and get in touch for any questions or comments.



Thanks!

-- Yin
___
dev mailing list
d...@openvswitch.org
https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev=DgICAg=uilaK90D4TOVoH58JNXRgQ=-xl6DPE_Y3uQD-mpZD7osBo2iL4s3jwdmSjTlGgjlsQ=GMr-AZul4UsNIDzghSvQVWBWWf9ZXBYbRf5TFXxEtFg=tAcMGwbQ11t617kkksED_MalGT4y_bWtSDojOFp58vc=

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

Re: [ovs-dev] [PATCH] netdev-dpdk: Assign value '0' to unsupported netdev features

2016-11-28 Thread Ben Pfaff 
On Thu, Oct 13, 2016 at 11:18:20PM +0800, Binbin Xu wrote:
> When OVS is used, DPDK doesn't support features 'advertised',
> 'supported' and 'peer'. If a physical port added to bridge, features
> descirbed above can't be assigned, and the values are random.
> 
> Signed-off-by: Binbin Xu 

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

Re: [ovs-dev] [patch_v8] ovn: Add datapaths of interest filtering.

2016-11-28 Thread Ben Pfaff 
I hope that this version is ready to go in.  Liran, are you happy with
this version?

On Sun, Nov 27, 2016 at 01:08:59PM -0800, Darrell Ball wrote:
> This patch adds datapaths of interest support where only datapaths of
> local interest are monitored by the ovn-controller ovsdb client.  The
> idea is to do a flood fill in ovn-controller of datapath associations
> calculated by northd. A new column is added to the SB database
> datapath_binding table - related_datapaths to facilitate this so all
> datapaths associations are known quickly in ovn-controller.  This
> allows monitoring to adapt quickly with a single new monitor setting
> for all datapaths of interest locally.
> 
> To reduce risk and minimize latency on network churn, only logical
> flows are conditionally monitored for now.  This should provide
> the bulk of the benefit.  This will be re-evaluated later to
> possibly add additional conditional monitoring such as for
> logical ports.
> 
> Liran Schour contributed enhancements to the conditional
> monitoring granularity in ovs and also submitted patches
> for ovn usage of conditional monitoring which aided this patch
> though sharing of concepts through code review work.
> 
> Ben Pfaff suggested that northd could be used to pre-populate
> related datapaths for ovn-controller to use.  That idea is
> used as part of this patch.
> 
> CC: Ben Pfaff 
> CC: Liran Schour 
> Signed-off-by: Darrell Ball 
> ---
> 
> v7->v8: Start wth logical flow conditional monitoring off.
> Remove deprecated code.
> Recover SB DB version number change.
> Change test to be more definitive. 
> 
> v6->v7: Rebase
> 
> v5->v6: Rebase; fix stale handling.
> 
> v4->v5: Correct cleanup of monitors.
> Fix warning.
> 
> v3->v4: Refactor after incremental processing backout.
> Limit filtering to logical flows to limit risk.
> 
> v2->v3: Line length violation fixups :/
> 
> v1->v2: Added logical port removal monitoring handling, factoring
> in recent changes for incremental processing.  Added some
> comments in the code regarding initial conditions for
> database conditional monitoring.
> 
>  ovn/controller/binding.c| 10 +++--
>  ovn/controller/lflow.c  |  5 +++
>  ovn/controller/ovn-controller.c | 92 
> +
>  ovn/controller/ovn-controller.h |  3 ++
>  ovn/controller/patch.c  |  6 ++-
>  ovn/northd/ovn-northd.c | 76 ++
>  ovn/ovn-sb.ovsschema| 11 +++--
>  ovn/ovn-sb.xml  |  9 
>  tests/ovn.at|  8 
>  9 files changed, 211 insertions(+), 9 deletions(-)
> 
> diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
> index d7b175e..2aff69a 100644
> --- a/ovn/controller/binding.c
> +++ b/ovn/controller/binding.c
> @@ -106,7 +106,8 @@ get_local_iface_ids(const struct ovsrec_bridge *br_int,
>  
>  static void
>  add_local_datapath(struct hmap *local_datapaths,
> -const struct sbrec_port_binding *binding_rec)
> +const struct sbrec_port_binding *binding_rec,
> +const struct controller_ctx *ctx)
>  {
>  if (get_local_datapath(local_datapaths,
> binding_rec->datapath->tunnel_key)) {
> @@ -118,6 +119,7 @@ add_local_datapath(struct hmap *local_datapaths,
>  memcpy(>uuid, _rec->header_.uuid, sizeof ld->uuid);
>  hmap_insert(local_datapaths, >hmap_node,
>  binding_rec->datapath->tunnel_key);
> +do_datapath_flood_fill(ctx, binding_rec->datapath);
>  }
>  
>  static void
> @@ -295,7 +297,7 @@ consider_local_datapath(struct controller_ctx *ctx,
>  /* Add child logical port to the set of all local ports. */
>  sset_add(all_lports, binding_rec->logical_port);
>  }
> -add_local_datapath(local_datapaths, binding_rec);
> +add_local_datapath(local_datapaths, binding_rec, ctx);
>  if (iface_rec && qos_map && ctx->ovs_idl_txn) {
>  get_qos_params(binding_rec, qos_map);
>  }
> @@ -330,7 +332,7 @@ consider_local_datapath(struct controller_ctx *ctx,
>  }
>  
>  sset_add(all_lports, binding_rec->logical_port);
> -add_local_datapath(local_datapaths, binding_rec);
> +add_local_datapath(local_datapaths, binding_rec, ctx);
>  if (binding_rec->chassis == chassis_rec) {
>  return;
>  }
> @@ -344,7 +346,7 @@ consider_local_datapath(struct controller_ctx *ctx,
>  const char *chassis = smap_get(_rec->options,
> "l3gateway-chassis");
>  if (!strcmp(chassis, chassis_rec->name) && ctx->ovnsb_idl_txn) {
> -add_local_datapath(local_datapaths, binding_rec);
> +add_local_datapath(local_datapaths, binding_rec, ctx);
>  }
>  } else if (chassis_rec && binding_rec->chassis == chassis_rec) {
>

Re: [ovs-dev] [patch_v6 2/3] ovn: Add additional comments regarding arp responders.

2016-11-28 Thread Ben Pfaff 
On Fri, Nov 04, 2016 at 10:06:17AM -0700, Darrell Ball wrote:
> There has been enough confusion regarding logical switch datapath
> arp responders in ovn to warrant some additional comments;
> hence add a general description regarding why they exist and
> document the special cases.
> 
> Signed-off-by: Darrell Ball 
> Signed-off-by: Ramu Ramamurthy 
> Co-authored-by: Ramu Ramamurthy 
> Acked-by: Han Zhou 
> ---
> 
> v5->v6: Rewording based on review feedback including designating
> peer logical router port correctly.
> 
> v4->v5: Splice in some rewording from review from multiple sources.
> 
> v3->v4: Capitalization fixes.
> Reinstate comment regarding L2 learning confusion.
> 
> v2->v3: Reword and further elaborate.
> 
> v1->v2: Dropped RFC code change for logical switch router
> type ports.
> 
>  ovn/northd/ovn-northd.8.xml | 67 
> +

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

Re: [ovs-dev] [PATCH] dist-docs: typo in IntegrationGuide.rst

2016-11-28 Thread Russell Bryant 
On Mon, Nov 28, 2016 at 3:50 PM, Lance Richardson 
wrote:

> Unbalanced angle bracket was causing 'Unknown target name: "ocf
>  when building dist-docs.
>
> Signed-off-by: Lance Richardson 
> ---
>  IntegrationGuide.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/IntegrationGuide.rst b/IntegrationGuide.rst
> index c02bfbe..8c3cca7 100644
> --- a/IntegrationGuide.rst
> +++ b/IntegrationGuide.rst
> @@ -208,7 +208,7 @@ stalled.
>  manager which can manage a defined set of resource across a set of
> clustered
>  nodes. Pacemaker manages the resource with the help of the resource
> agents.
>  One among the resource agent is
> -`OCF  +`OCF `_
>
>  OCF is nothing but a shell script which accepts a set of actions and
> returns an
>  appropriate status code.


Thanks, applied to master.

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

Re: [ovs-dev] [PATCH v2 2/2] rhel: Add EnvironmentFile for ovn services.

2016-11-28 Thread Russell Bryant 
On Fri, Nov 25, 2016 at 9:16 AM, Simon Horman 
wrote:

> On Wed, Nov 23, 2016 at 03:01:24PM -0500, Russell Bryant wrote:
> > A previous commit documented how you do this using systemd
> > native interfaces for customizing services.  However, it seems
> > that some people still would rather see environment variables
> > specified through an old-style sysconfig file.  It seems harmless
> > enough to provide, so this patch adds it.
> >
> > The "-" prefix to the filename means that systemd will ignore this
> > setting if the file does not exist and will not report any error.
> >
> > Signed-off-by: Russell Bryant 
>
> Acked-by: Simon Horman 
>

Thanks!  I applied this to master.

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

Re: [ovs-dev] [PATCH v2 1/2] rhel: Fix Environment for ovn-controller-vtep.

2016-11-28 Thread Russell Bryant 
On Fri, Nov 25, 2016 at 9:14 AM, Simon Horman 
wrote:

> Hi Russell,
>
> On Wed, Nov 23, 2016 at 03:01:23PM -0500, Russell Bryant wrote:
> > The systemd unit for ovn-controller-vtep incorrectly specified
> > Environment multiple times.  Multiple environment variables must
> > be specified separated by a space to a single Environment option.
>
> I'm trying to verify if that is the case. However, according to the
> following seems to indicate that the current setup is acceptable.
> Perhaps this documentation isn't compatibile with some systemd versions?
>
> https://www.freedesktop.org/software/systemd/man/systemd.
> exec.html#Environment=
>
>
I think you're right and I had just read the documentation wrong when I was
looking at this.  Consider this patch dropped.


> >
> > Signed-off-by: Russell Bryant 
> > ---
> >  rhel/usr_lib_systemd_system_ovn-controller-vtep.service | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > v1->v2:
> >  - no changes to patch 1
> >
> > diff --git a/rhel/usr_lib_systemd_system_ovn-controller-vtep.service
> b/rhel/usr_lib_systemd_system_ovn-controller-vtep.service
> > index 717ff12..26d4d9a 100644
> > --- a/rhel/usr_lib_systemd_system_ovn-controller-vtep.service
> > +++ b/rhel/usr_lib_systemd_system_ovn-controller-vtep.service
> > @@ -29,9 +29,7 @@ After=openvswitch.service
> >
> >  [Service]
> >  Type=simple
> > -Environment=OVS_RUNDIR=%t/openvswitch
> > -Environment=OVN_DB=unix:%t/openvswitch/db.sock
> > -Environment=VTEP_DB=unix:%t/openvswitch/db.sock
> > +Environment=OVS_RUNDIR=%t/openvswitch OVN_DB=unix:%t/openvswitch/db.sock
> VTEP_DB=unix:%t/openvswitch/db.sock
> >  ExecStart=/usr/bin/ovn-controller-vtep -vconsole:emer -vsyslog:err
> -vfile:info \
> >--log-file=/var/log/openvswitch/ovn-controller-vtep.log \
> >--no-chdir --pidfile=${OVS_RUNDIR}/ovn-controller-vtep.pid \
> > --
> > 2.9.3
> >
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
>



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

[ovs-dev] Guía para Elaborar un Mapeo de Procesos

2016-11-28 Thread Estrategia, procesos, actividades 
 

En línea y en Vivo / Para todo su Equipo con una sola Conexión 

Guía para Elaborar un Mapeo de Procesos
13 de diciembre - Online en Vivo - 10:00 a 13:00 y de 15:00 a 18:00 Hrs   
 
Este seminario está diseñado, en teoría y práctica, para aprender a mapear los 
procesos con un enfoque analítico que identifique relaciones, dependencias y 
secuencias en las actividades de la organización, desde los macro procesos 
empresariales hasta su desdoblamiento en los subprocesos y actividades 
productivas. 
TEMARIO: 


1. Estrategia, procesos, actividades y su interrelación.

2. Preparación del mapeo.

3. Realización del mapeo.

4. Resultado del mapeo y entrega de resultados.




...¡Y mucho más!


 
¿Requiere la información a la Brevedad?
responda este email con la palabra: 
Info - Mapeo.
centro telefónico: 018002129393
 

Lic. Pamela Rangel
Coordinador de Evento


 
¿Demasiados mensajes en su cuenta? Responda este mensaje indicando que solo 
desea recibir CALENDARIO y sólo recibirá un correo al mes. Si desea cancelar la 
suscripción, solicite su BAJA. 
 

 

 

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

Re: [ovs-dev] [PATCH] dpdk: Fix abort on double free.

2016-11-28 Thread Aaron Conole 
Ilya Maximets  writes:

> According to DPDK API (lib/librte_eal/common/include/rte_eal.h):
>
>   "After the call to rte_eal_init(), all arguments argv[x]
>with x < ret may be modified and should not be accessed
>by the application."
>
> This means, that OVS must not free the arguments passed to DPDK.
> In real world, 'rte_eal_init()' replaces the last argument in
> 'dpdk_argv' with the first one by doing this:

Thanks for spotting this error, Ilya.

>   # eal_parse_args() from lib/librte_eal/linuxapp/eal/eal.c
>
>   char *prgname = argv[0];
>   ...
>   if (optind >= 0)
>   argv[optind-1] = prgname;
>
> This leads to double free inside 'deferred_argv_release()' and
> possible ABORT at exit:

I haven't seen this, which is both shocking and scary - the commit which
does this copy is almost 4 years old;  did you have to do anything
specific for this behavior to occur?  Did something change in DPDK
recently that exposed this behavior?  Just wondering how you reproduced
it.

> *** Error in `ovs-vswitchd': double free or corruption (fasttop) <...> ***
>   Program received signal SIGABRT, Aborted.
>
>   #0  raise () from /lib64/libc.so.6
>   #1  abort () from /lib64/libc.so.6
>   #2  __libc_message () from /lib64/libc.so.6
>   #3  free () from /lib64/libc.so.6
>   #4  deferred_argv_release () at lib/dpdk.c:261
>   #5  __run_exit_handlers () from /lib64/libc.so.6
>   #6  exit () from /lib64/libc.so.6
>   #7  __libc_start_main () from /lib64/libc.so.6
>   #8  _start ()
>
> Fix that by not calling free for the memory passed to DPDK.
>
> CC: Aaron Conole 
> Fixes: bab694097133 ("netdev-dpdk: Convert initialization from cmdline to db")
> Signed-off-by: Ilya Maximets 
> ---

We need to free the memory - I think that is not a question; but we
should probably keep a separate copy of the elements to release, so that
the area is still free for dpdk to play with (complying with c99 5.1.2.2.1
item 2, bullet 4), but that we can release the memory (since 5.1.2.2.2
makes no guarantee that memory will be released if I'm reading it
correctly).

Is this (completely untested) patch acceptable?

---

diff --git a/lib/dpdk.c b/lib/dpdk.c
index 49a589a..2343588 100644
--- a/lib/dpdk.c
+++ b/lib/dpdk.c
@@ -250,7 +250,7 @@ get_dpdk_args(const struct smap *ovs_other_config, char 
***argv,
 return i + extra_argc;
 }
 
-static char **dpdk_argv;
+static char **dpdk_argv, **dpdk_argv_release;
 static int dpdk_argc;
 
 static void
@@ -258,10 +258,11 @@ deferred_argv_release(void)
 {
 int result;
 for (result = 0; result < dpdk_argc; ++result) {
-free(dpdk_argv[result]);
+free(dpdk_argv_release[result]);
 }
 
 free(dpdk_argv);
+free(dpdk_argv_release);
 }
 
 static void
@@ -366,6 +367,11 @@ dpdk_init__(const struct smap *ovs_other_config)
 ds_destroy(_args);
 }
 
+dpdk_argv_release = grow_argv(_argv_release, 0, argc);
+for (argc_tmp = 0; argc_tmp < argc; ++argc_tmp) {
+dpdk_argv_release[argc_tmp] = argv[argc_tmp];
+}
+
 /* Make sure things are initialized ... */
 result = rte_eal_init(argc, argv);
 if (result < 0) {
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] Numa: Allow leading 0x on pmd-cpu-mask.

2016-11-28 Thread Ben Pfaff 
On Mon, Nov 28, 2016 at 05:06:53PM +, billy.o.mah...@intel.com wrote:
> From: billyom 
> 
> pmd-cpu-mask is interpreted as a hex bit mask. So it should be written
> with a leading 0x to indicate this. But if this is done, while the value
> is interpreted correctly and the PMDs pinned as expected, a confusing
> warning message is also issued.
> 
> This patch allows but does not require a leading 0x or 0X to be used
> without a warning. Existing functionality is not affected. Relevant DPDK
> docs also updated.
> 
> Signed-off-by: Billy O'Mahony 

This is a whitespace-only change to the C code in OVS, so it's hard to
believe that it does what it says.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] Numa: Allow leading 0x on pmd-cpu-mask.

2016-11-28 Thread billy . o . mahony 
From: billyom 

pmd-cpu-mask is interpreted as a hex bit mask. So it should be written
with a leading 0x to indicate this. But if this is done, while the value
is interpreted correctly and the PMDs pinned as expected, a confusing
warning message is also issued.

This patch allows but does not require a leading 0x or 0X to be used
without a warning. Existing functionality is not affected. Relevant DPDK
docs also updated.

Signed-off-by: Billy O'Mahony 
---
 INSTALL.DPDK-ADVANCED.rst |   13 +++--
 INSTALL.DPDK.rst  |6 +++---
 lib/ovs-numa.c|2 +-
 3 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/INSTALL.DPDK-ADVANCED.rst b/INSTALL.DPDK-ADVANCED.rst
index 9b2895b..1c2437d 100644
--- a/INSTALL.DPDK-ADVANCED.rst
+++ b/INSTALL.DPDK-ADVANCED.rst
@@ -175,10 +175,11 @@ core shared by two logical cores, run::
 where ``N`` is the logical core number.
 
 In this example, it would show that cores ``1`` and ``21`` share the same
-physical core., thus, the ``pmd-cpu-mask`` can be used to enable these two pmd
-threads running on these two logical cores (one physical core) is::
+physical core. As cores are counted from 0, the ``pmd-cpu-mask`` can be used
+to enable these two pmd threads running on these two logical cores (one
+physical core) is::
 
-$ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=12
+$ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0x22
 
 Isolate Cores
 ~
@@ -239,7 +240,7 @@ affinitized accordingly.
   By setting a bit in the mask, a pmd thread is created and pinned to the
   corresponding CPU core. e.g. to run a pmd thread on core 2::
 
-  $ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=4
+  $ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0x4
 
   .. note::
 pmd thread on a NUMA node is only created if there is at least one DPDK
@@ -269,7 +270,7 @@ is done automatically.
 A set bit in the mask means a pmd thread is created and pinned to the
 corresponding CPU core. For example, to run pmd threads on core 1 and 2::
 
-$ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=6
+$ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0x6
 
 When using dpdk and dpdkvhostuser ports in a bi-directional VM loopback as
 shown below, spreading the workload over 2 or 4 pmd threads shows significant
@@ -436,7 +437,7 @@ guide`_ to create and initialize the database, start 
ovs-vswitchd and add
of rx queues at vhost-user interface gets automatically configured after
virtio device connection and doesn't need manual configuration::
 
-   $ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=c
+   $ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0xC
$ ovs-vsctl set Interface dpdk0 options:n_rxq=2
$ ovs-vsctl set Interface dpdk1 options:n_rxq=2
 
diff --git a/INSTALL.DPDK.rst b/INSTALL.DPDK.rst
index a078093..c50b7a3 100644
--- a/INSTALL.DPDK.rst
+++ b/INSTALL.DPDK.rst
@@ -223,10 +223,10 @@ NUMA node 0, run::
 
 Similarly, if you wish to better scale the workloads across cores, then
 multiple pmd threads can be created and pinned to CPU cores by explicity
-specifying ``pmd-cpu-mask``. For example, to spawn two pmd threads and pin
-them to cores 1,2, run::
+specifying ``pmd-cpu-mask``. Cores are numbered from 0, so to spawn two pmd
+threads and pin them to cores 1,2, run::
 
-$ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=6
+$ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0x6
 
 For details on using ivshmem with DPDK, refer to `the advanced installation
 guide `__.
diff --git a/lib/ovs-numa.c b/lib/ovs-numa.c
index c8173e0..0f2ef7c 100644
--- a/lib/ovs-numa.c
+++ b/lib/ovs-numa.c
@@ -575,7 +575,7 @@ ovs_numa_set_cpu_mask(const char *cmask)
 if (core_id >= hmap_count(_cpu_cores)) {
 return;
 }
-   }
+}
 }
 
 /* For unspecified cores, sets 'available' to false.  */
-- 
1.7.0.7

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

[ovs-dev] [PATCH v6] netdev-dpdk: Enable Rx checksum offloading feature on DPDK physical ports.

2016-11-28 Thread Sugesh Chandran 
Add Rx checksum offloading feature support on DPDK physical ports. By default,
the Rx checksum offloading is enabled if NIC supports. However,
the checksum offloading can be turned OFF either while adding a new DPDK
physical port to OVS or at runtime.

The rx checksum offloading can be turned off by setting the parameter to
'false'. For eg: To disable the rx checksum offloading when adding a port,

 'ovs-vsctl add-port br0 dpdk0 -- \
  set Interface dpdk0 type=dpdk options:rx-checksum-offload=false'

OR (to disable at run time after port is being added to OVS)

'ovs-vsctl set Interface dpdk0 options:rx-checksum-offload=false'

Similarly to turn ON rx checksum offloading at run time,

'ovs-vsctl set Interface dpdk0 options:rx-checksum-offload=true'

The Tx checksum offloading support is not implemented due to the following
reasons.

1) Checksum offloading and vectorization are mutually exclusive in DPDK poll
mode driver. Vector packet processing is turned OFF when checksum offloading
is enabled which causes significant performance drop at Tx side.

2) Normally, OVS generates checksum for tunnel packets in software at the
'tunnel push' operation, where the tunnel headers are created. However
enabling Tx checksum offloading involves,

  *) Mark every packets for tx checksum offloading at 'tunnel_push' and
  recirculate.
  *) At the time of xmit, validate the same flag and instruct the NIC to do the
  checksum calculation.  In case NIC doesnt support Tx checksum offloading,
  the checksum calculation has to be done in software before sending out the
  packets.

No significant performance improvement noticed with Tx checksum offloading
due to the e overhead of additional validations + non vector packet processing.
In some test scenarios, it introduces performance drop too.

Rx checksum offloading still offers 8-9% of improvement on VxLAN tunneling
decapsulation even though the SSE vector Rx function is disabled in DPDK poll
mode driver.

Signed-off-by: Sugesh Chandran 
Acked-by: Jesse Gross 

---
v6
- Avoid unnecessary reconfiguration of DPDK ports when Rx checksum offload
  support is unavailable.
- Limit the 'Rx checksum offload is unsupported' message in logging.
- Merge to latest OVS for DPDK 16.11 support. The DPDK checksum flags that used
- in the patch are only introduced in 16.11.

v5
- Reset the checksum flag in common tunnel pop function than in
  'udp_extract_tnl_md' function.

v4
- Unconditonally clear off the checksum flag one time in pop operation than
  doing separately in IP and UDP layers.

v3
- Reset the checksum offload flags in tunnel pop operation after the validation.
- Reconfigure the dpdk port with rx checksum offload only if new configuration
  is different than current one.

v2
- Set Rx checksum enabled by default.
- Modified commit message, explaining the tradeoff with tx checksum offloading.
- Use dpdk mbuf checksum offload flags  instead of defining new metadata field
  in OVS dp_packet.
- validate udp checksum mbuf flag only if the checksum present in the packet.
- Doc update with Rx checksum offloading feature.
---
---
 INSTALL.DPDK-ADVANCED.rst | 16 
 lib/dp-packet.h   | 29 +
 lib/netdev-dpdk.c | 47 +++
 lib/netdev-native-tnl.c   | 35 ---
 lib/netdev.c  |  4 
 vswitchd/vswitch.xml  | 14 ++
 6 files changed, 130 insertions(+), 15 deletions(-)

diff --git a/INSTALL.DPDK-ADVANCED.rst b/INSTALL.DPDK-ADVANCED.rst
index 9b2895b..e209ae4 100644
--- a/INSTALL.DPDK-ADVANCED.rst
+++ b/INSTALL.DPDK-ADVANCED.rst
@@ -922,6 +922,22 @@ largest frame size supported by Fortville NIC using the 
DPDK i40e driver, but
 larger frames and other DPDK NIC drivers may be supported. These cases are
 common for use cases involving East-West traffic only.
 
+Rx Checksum Offload
+---
+
+By default, DPDK physical ports are enabled with Rx checksum offload. Rx
+checksum offload can be configured on a DPDK physical port either when adding
+or at run time.
+
+To disable Rx checksum offload when adding a DPDK port dpdk0::
+
+$ ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk \
+  options:rx-checksum-offload=false
+
+Similarly to disable the Rx checksum offloading on a existing DPDK port dpdk0::
+
+$ ovs-vsctl set Interface dpdk0 type=dpdk options:rx-checksum-offload=false
+
 vsperf
 --
 
diff --git a/lib/dp-packet.h b/lib/dp-packet.h
index 1469864..d61ec83 100644
--- a/lib/dp-packet.h
+++ b/lib/dp-packet.h
@@ -598,6 +598,35 @@ dp_packet_rss_invalidate(struct dp_packet *p)
 #endif
 }
 
+static inline bool
+dp_packet_ip_checksum_valid(struct dp_packet *p)
+{
+#ifdef DPDK_NETDEV
+return p->mbuf.ol_flags & PKT_RX_IP_CKSUM_GOOD;
+#else
+return 0;
+#endif
+}
+
+static inline bool
+dp_packet_l4_checksum_valid(struct dp_packet *p)
+{
+#ifdef DPDK_NETDEV
+

[ovs-dev] [PATCH] dpdk: Fix abort on double free.

2016-11-28 Thread Ilya Maximets 
According to DPDK API (lib/librte_eal/common/include/rte_eal.h):

"After the call to rte_eal_init(), all arguments argv[x]
 with x < ret may be modified and should not be accessed
 by the application."

This means, that OVS must not free the arguments passed to DPDK.
In real world, 'rte_eal_init()' replaces the last argument in
'dpdk_argv' with the first one by doing this:

# eal_parse_args() from lib/librte_eal/linuxapp/eal/eal.c

char *prgname = argv[0];
...
if (optind >= 0)
argv[optind-1] = prgname;

This leads to double free inside 'deferred_argv_release()' and
possible ABORT at exit:

*** Error in `ovs-vswitchd': double free or corruption (fasttop) <...> ***
Program received signal SIGABRT, Aborted.

#0  raise () from /lib64/libc.so.6
#1  abort () from /lib64/libc.so.6
#2  __libc_message () from /lib64/libc.so.6
#3  free () from /lib64/libc.so.6
#4  deferred_argv_release () at lib/dpdk.c:261
#5  __run_exit_handlers () from /lib64/libc.so.6
#6  exit () from /lib64/libc.so.6
#7  __libc_start_main () from /lib64/libc.so.6
#8  _start ()

Fix that by not calling free for the memory passed to DPDK.

CC: Aaron Conole 
Fixes: bab694097133 ("netdev-dpdk: Convert initialization from cmdline to db")
Signed-off-by: Ilya Maximets 
---
 lib/dpdk.c | 13 -
 1 file changed, 13 deletions(-)

diff --git a/lib/dpdk.c b/lib/dpdk.c
index 49a589a..2014946 100644
--- a/lib/dpdk.c
+++ b/lib/dpdk.c
@@ -254,17 +254,6 @@ static char **dpdk_argv;
 static int dpdk_argc;
 
 static void
-deferred_argv_release(void)
-{
-int result;
-for (result = 0; result < dpdk_argc; ++result) {
-free(dpdk_argv[result]);
-}
-
-free(dpdk_argv);
-}
-
-static void
 dpdk_init__(const struct smap *ovs_other_config)
 {
 char **argv = NULL;
@@ -384,8 +373,6 @@ dpdk_init__(const struct smap *ovs_other_config)
 dpdk_argv = argv;
 dpdk_argc = argc;
 
-atexit(deferred_argv_release);
-
 rte_memzone_dump(stdout);
 
 /* We are called from the main thread here */
-- 
2.7.4

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

[ovs-dev] [PATCH] datapath-windows: fix return value in conntrack

2016-11-28 Thread Alin Serdean 
'status' is of type 'NTSTATUS' and NlFillOvsMsgForNfGenMsg is of type bool.

Signed-off-by: Alin Gabriel Serdean 
Acked-by: Sairam Venugopal 
---
 datapath-windows/ovsext/Conntrack.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index b0846f6..e663c3b 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -802,12 +802,14 @@ OvsCtDeleteCmdHandler(POVS_USER_PARAMS_CONTEXT 
usrParamsCtx,
 NlBufInit(,
   usrParamsCtx->outputBuffer,
   usrParamsCtx->outputLength);
-status = NlFillOvsMsgForNfGenMsg(, nlmsgType, NLM_F_CREATE,
- msgIn->nlMsg.nlmsgSeq,
- msgIn->nlMsg.nlmsgPid,
- AF_UNSPEC,
- msgIn->nfGenMsg.version,
- 0);
+if (!NlFillOvsMsgForNfGenMsg(, nlmsgType, NLM_F_CREATE,
+ msgIn->nlMsg.nlmsgSeq,
+ msgIn->nlMsg.nlmsgPid,
+ AF_UNSPEC,
+ msgIn->nfGenMsg.version,
+ 0)) {
+status = STATUS_INVALID_PARAMETER;
+}
 nlMsg = (PNL_MSG_HDR)NlBufAt(, 0, 0);
 nlMsg->nlmsgLen = NlBufSize();
 *replyLen = msgOut->nlMsg.nlmsgLen;
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] rhel: Support building python ovs package with native json impl

2016-11-28 Thread Numan Siddique 
Since building python-openvswitch _json.so requires libopenvswitch.so
a separate spec file is added which configures Open vSwitch with
--enable-shared flag. The resulting RPM also includes the Open vSwitch
shared library.

$ rpm -qlp python-openvswitch-2.6.90-1.fc25.x86_64.rpm
/usr/lib64/libopenvswitch.so
/usr/lib64/libopenvswitch.so.1
/usr/lib64/libopenvswitch.so.1.0.0
/usr/lib64/python2.7/site-packages/ovs
/usr/lib64/python2.7/site-packages/ovs-2.6.90-py2.7.egg-info
...
/usr/lib64/python2.7/site-packages/ovs/_json.so
...

CC: Terry Wilson 
Signed-off-by: Numan Siddique 
---
 INSTALL.Fedora.rst |  13 +
 python/setup.py|   9 ++-
 rhel/automake.mk   |   9 +++
 rhel/python-openvswitch-fedora.spec.in | 103 +
 4 files changed, 133 insertions(+), 1 deletion(-)
 create mode 100644 rhel/python-openvswitch-fedora.spec.in

diff --git a/INSTALL.Fedora.rst b/INSTALL.Fedora.rst
index b9be0ed..40eacfc 100644
--- a/INSTALL.Fedora.rst
+++ b/INSTALL.Fedora.rst
@@ -83,6 +83,8 @@ This will create the RPMs `openvswitch`, `python-openvswitch`,
 `openvswitch-ovn-central`, `openvswitch-ovn-host`, `openvswitch-ovn-vtep`,
 `openvswitch-ovn-docker`, and `openvswitch-debuginfo`.
 
+`python-openvswitch` RPM doesn't include the native json library.
+
 To enable DPDK support in the openvswitch package, the ``--with dpdk`` option
 can be added:
 
@@ -98,6 +100,17 @@ these tests, the ``--without check`` option can be added:
 
 $ make rpm-fedora RPMBUILD_OPT="--without check"
 
+Open vSwitch python binding RPM with native json library
+
+
+To build the `python-openvswitch` RPM with native json library, run:
+
+
+::
+$ make rpm-fedora-python-ovs
+
+This also builds the Open vSwitch shared library and includes it in the RPM.
+
 Kernel OVS Tree Datapath RPM
 
 
diff --git a/python/setup.py b/python/setup.py
index 19c1f18..5070c9b 100644
--- a/python/setup.py
+++ b/python/setup.py
@@ -12,6 +12,7 @@
 
 from __future__ import print_function
 import sys
+import os
 
 from distutils.command.build_ext import build_ext
 from distutils.errors import CCompilerError, DistutilsExecError, \
@@ -33,6 +34,10 @@ ext_errors = (CCompilerError, DistutilsExecError, 
DistutilsPlatformError)
 if sys.platform == 'win32':
 ext_errors += (IOError, ValueError)
 
+# Get the include path if defined
+include_dirs = os.environ.get('OVS_INCLUDE_DIR', '.')
+library_dirs = os.environ.get('OVS_LIB_DIR', '.')
+
 
 class BuildFailed(Exception):
 pass
@@ -77,7 +82,9 @@ setup_args = dict(
 'Programming Language :: Python :: 3.4',
 ],
 ext_modules=[setuptools.Extension("ovs._json", sources=["ovs/_json.c"],
-  libraries=['openvswitch'])],
+  libraries=['openvswitch'],
+  include_dirs=[include_dirs],
+  library_dirs=[library_dirs])],
 cmdclass={'build_ext': try_build_ext},
 )
 
diff --git a/rhel/automake.mk b/rhel/automake.mk
index 45aa9b1..1113fd8 100644
--- a/rhel/automake.mk
+++ b/rhel/automake.mk
@@ -23,6 +23,7 @@ EXTRA_DIST += \
rhel/openvswitch.spec.in \
rhel/openvswitch-fedora.spec \
rhel/openvswitch-fedora.spec.in \
+   rhel/python-openvswitch-fedora.spec.in \
rhel/usr_share_openvswitch_scripts_sysconfig.template \
rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template \
rhel/usr_lib_systemd_system_openvswitch.service \
@@ -62,6 +63,14 @@ rpm-fedora: dist $(srcdir)/rhel/openvswitch-fedora.spec
  -D "_topdir ${RPMBUILD_TOP}" \
  -ba $(srcdir)/rhel/openvswitch-fedora.spec
 
+# Build Python binding RPM with native json
+rpm-fedora-python-ovs: dist $(srcdir)/rhel/python-openvswitch-fedora.spec
+   ${MKDIR_P} ${RPMBUILD_TOP}/SOURCES
+   cp ${DIST_ARCHIVES} ${RPMBUILD_TOP}/SOURCES
+   rpmbuild ${RPMBUILD_OPT} \
+ -D "_topdir ${RPMBUILD_TOP}" \
+ -ba $(srcdir)/rhel/python-openvswitch-fedora.spec
+
 # Build kernel datapath RPM
 rpm-fedora-kmod: dist $(srcdir)/rhel/openvswitch-kmod-fedora.spec
${MKDIR_P} ${RPMBUILD_TOP}/SOURCES
diff --git a/rhel/python-openvswitch-fedora.spec.in 
b/rhel/python-openvswitch-fedora.spec.in
new file mode 100644
index 000..cd863d8
--- /dev/null
+++ b/rhel/python-openvswitch-fedora.spec.in
@@ -0,0 +1,103 @@
+# Spec file for Python Open vSwitch with native json library
+
+# Copyright (C) 2016 Red Hat, Inc.
+#
+# Copying and distribution of this file, with or without modification,
+# are permitted in any medium without royalty provided the copyright
+# notice and this notice are preserved.  This file is offered as-is,
+# without warranty of any kind.
+#
+
+# If libcap-ng isn't available and there is no need for