Re: [ovs-dev] [PATCH] Supporting ovn-northd service HA depend on OVNDB-HA

[multi_t...@163.com]

Hi Numan,

Thank you very much,  I had pushed the patch new version  
https://github.com/openvswitch/ovs/pull/178

Thanks
Zhengwei



multi_t...@163.com
 
From: Numan Siddique
Date: 2017-05-22 15:33
To: 高正伟
CC: Ben Pfaff; ovs dev
Subject: Re: Re: [ovs-dev] [PATCH] Supporting ovn-northd service HA depend on 
OVNDB-HA


On Fri, May 19, 2017 at 7:21 AM, 高正伟  wrote:
Thanks Numan

I think that supporting  OVNDB and ovn-northd HA  depend on one pacemaker is an 
idea,   In case of thinking OVN-HA,  user will not individually care about 
OVNDB or ovn-northd HA.


Hi Zhangwei,

I do agree with you. But what I am trying to say is to make it configurable.

I tested with your patch again with tripleo and it is having few issues. 
1. Since you are not passing "--ovn-manage-ovsdb=no", when "ovn-ctl 
stop_northd" is called, it also stops ovsdb-servers. We probably don't want to 
stop ovsdb-servers unless, pacemaker calls "stop" action.

2. When a master node is demoted, ovn-northd is not stopped since "ovn-ctl 
stop_northd" code is not hit.

I tested with the below modifications and applying this patch - 
https://patchwork.ozlabs.org/patch/765224/. It is working as expected. Can you 
please test on your side and resubmit another version if it is fine with you.

If the user wants to manage ovn_northd using the ocf resources, he could pass 
manage_northd=yes while creating the resource.


---cut here -

diff --git a/ovn/utilities/ovndb-servers.ocf b/ovn/utilities/ovndb-servers.ocf
index 40c5541..018d904 100755
--- a/ovn/utilities/ovndb-servers.ocf
+++ b/ovn/utilities/ovndb-servers.ocf
@@ -7,6 +7,7 @@
 : ${NB_MASTER_PROTO_DEFAULT="tcp"}
 : ${SB_MASTER_PORT_DEFAULT="6642"}
 : ${SB_MASTER_PROTO_DEFAULT="tcp"}
+: ${MANAGE_NORTHD_DEFAULT="no"}
 CRM_MASTER="${HA_SBIN_DIR}/crm_master -l reboot"
 CRM_ATTR_REPL_INFO="${HA_SBIN_DIR}/crm_attribute --type crm_config --name 
OVN_REPL_INFO -s ovn_ovsdb_master_server"
 OVN_CTL=${OCF_RESKEY_ovn_ctl:-${OVN_CTL_DEFAULT}}
@@ -15,6 +16,7 @@ 
NB_MASTER_PORT=${OCF_RESKEY_nb_master_port:-${NB_MASTER_PORT_DEFAULT}}
 NB_MASTER_PROTO=${OCF_RESKEY_nb_master_protocol:-${NB_MASTER_PROTO_DEFAULT}}
 SB_MASTER_PORT=${OCF_RESKEY_sb_master_port:-${SB_MASTER_PORT_DEFAULT}}
 SB_MASTER_PROTO=${OCF_RESKEY_sb_master_protocol:-${SB_MASTER_PROTO_DEFAULT}}
+MANAGE_NORTHD=${OCF_RESKEY_manage_northd:-${MANAGE_NORTHD_DEFAULT}}
 
 # Invalid IP address is an address that can never exist in the network, as
 # mentioned in rfc-5737. The ovsdb servers connects to this IP address till
@@ -90,6 +92,14 @@ ovsdb_server_metadata() {
   
   
 
+  
+  
+  If set to yes, manages ovn-northd service. ovn-northd will be started in
+  the master node.
+  
+  manage ovn-northd service
+  
+  
   
 
   
@@ -122,12 +132,17 @@ ovsdb_server_notify() {
 # the right thing at startup
 ocf_log debug "ovndb_server: $host_name is the master"
 ${CRM_ATTR_REPL_INFO} -v "$host_name"
-# Startup ovn-northd service
-${OVN_CTL} start_northd
+if [ "$MANAGE_NORTHD" = "yes" ]; then
+# Startup ovn-northd service
+${OVN_CTL} --ovn-manage-ovsdb=no start_northd
+fi
 
 else
-# Stop ovn-northd service
-${OVN_CTL} stop_northd
+if [ "$MANAGE_NORTHD" = "yes" ]; then
+# Stop ovn-northd service. Set --ovn-manage-ovsdb=no so that
+# ovn-ctl doesn't stop ovsdb-servers.
+${OVN_CTL} --ovn-manage-ovsdb=no stop_northd
+fi
 # Synchronize with the new master
 ocf_log debug "ovndb_server: Connecting to the new master 
${OCF_RESKEY_CRM_meta_notify_promote_uname}"
 ${OVN_CTL} demote_ovnnb --db-nb-sync-from-addr=${MASTER_IP} \
@@ -289,6 +304,13 @@ ovsdb_server_start() {
 }
 
 ovsdb_server_stop() {
+if [ "$MANAGE_NORTHD" = "yes" ]; then
+# Stop ovn-northd service in case it was running. This is required
+# when the master is demoted. For other cases, it would be a no-op.
+# Set --ovn-manage-ovsdb=no so that ovn-ctl doesn't stop ovsdb-servers.
+${OVN_CTL} --ovn-manage-ovsdb=no stop_northd
+fi
+
 ovsdb_server_check_status
 case $? in
 $OCF_NOT_RUNNING)return ${OCF_SUCCESS};;

--


Thanks 
Numan



 
Thanks.


在 2017-05-18 20:53:22，"Numan Siddique"  写道：


On Wed, May 17, 2017 at 9:15 PM, Ben Pfaff  wrote:
From: Zhengwei Gao 

As ovn-northd servcie  parse network element between ovnnb_db and
ovnsb_db, it ensures ovn-northd service connecting to ovnnb_db and
ovnsb_db. OVNDB-HA was supported with pacemaker, ovn-northd service
will be failover fllowing OVNDB-HA.
---
This was posted as a github pull request:
https://github.com/openvswitch/ovs/pull/176

 ovn/utilities/ovndb-servers.ocf | 4 
 1 file changed, 4 insertions(+)

diff --git 

Re: [ovs-dev] [PATCH 1/2] ovn pacemaker: Fix return code errors in start/stop action

[Numan Siddique]

On Tue, May 23, 2017 at 5:21 AM, Andy Zhou  wrote:

> On Sun, May 21, 2017 at 6:35 PM,   wrote:
> > From: Numan Siddique 
> >
> > start action returns OCF_RUNNING_MASTER in certain scenarios.
> > But as per the OCF guidelines, status code OCF_RUNNING_MASTER shoud
> > be returned only in monitor action [1].
> >
> > Whenever the start action returns OCF_RUNNING_MASTER, it is observed
> > in the testing that, pacemaker stops the ovsdb-server ocf resource
> > in that node. This patch fixes this issue by returning OCF_SUCESS in
> > such cases.
> >
> > stop action returns OCF_RUNNING_MASTER if the ovsdb-servers are
> > running as master. But as per the OCF guidelines [2], stop action
> > should only return OCF_SUCCESS. If any other code is returned,
> > pacemaker cluster would block that resource in that node.
> >
> > This patch fixes this issue by stopping the ovsdb-servers when they
> > are running as masters (which is the expected case) and returns
> > OCF_SUCCESS.
> >
> > [1] - http://www.linux-ha.org/doc/dev-guides/_literal_ocf_
> running_master_literal_8.html
> > [2] - http://www.linux-ha.org/doc/dev-guides/_literal_stop_
> literal_action.html
> >
> > CC: Andy Zhou 
> > Signed-off-by: Numan Siddique 
>
> Thanks for the fixes!  Both patches look reasonable to me. I pushed
> them to master.
>

Thanks Andy. Can these patches be back ported to  branch 2.7 ? It would be
great since the tripleo patches for OVN needs these fixes

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

Re: [ovs-dev] [PATCH] checkpatch: Check for stdlib usage.

[Joe Stringer]

On 22 May 2017 at 17:53, Ben Pfaff  wrote:
> On Mon, May 22, 2017 at 03:51:39PM -0700, Joe Stringer wrote:
>> Many standard library functions are wrapped in OVS, so check for usage
>> of the original versions and suggest that authors replace them with the
>> OVS versions.
>>
>> Signed-off-by: Joe Stringer 
>
> Thanks for working to improve the patch checks!
>
>> +def regex_function_factory(func_name):
>> +regex = re.compile('[^x]%s\([^)]*\)' % func_name)
>> +return lambda x: regex.search(x) is not None
>> +
>> +
>> +std_functions = [
>> +('malloc', 'Use xmalloc() in place of malloc()'),
>> +('calloc', 'Use xcmalloc() in place of calloc()'),
>
> xcalloc, not xcmalloc
>
>> +('zalloc', 'Use xzmalloc() in place of zalloc()'),
>
> I don't think there's a zalloc function.
>
>> +('realloc', 'Use xrealloc() in place of realloc()'),
>> +('memdup', 'Use xmemdup() in place of memdup()'),
>> +('memdup0', 'Use xmemdup0() in place of memdup0()'),
>
> I don't think there's a memdup0 function.
>
>> +('strdup', 'Use xstrdup() in place of strdup()'),
>> +('asprintf', 'Use xasprintf() in place of asprintf()'),
>> +('vasprintf', 'Use xvasprintf() in place of vasprintf()'),
>> +('2nrealloc', 'Use x2nrealloc() in place of 2nrealloc()'),
>
> I don't think there's a 2nrealloc function.
>
>> +('strlcpy', 'Use ovs_strlcpy() in place of strlcpy()'),
>> +('strzcpy', 'Use ovs_strzcpy() in place of strzcpy()'),
>
> I don't think there's a strzcpy function, but ovs_strzcpy() might be a
> substitute for strncpy.
>
> It can often be a good idea to use ovs_strlcpy() in place of plain
> strcpy().
>
>> +('strerror', 'Use ovs_strerror() in place of strerror()'),
>> +('sleep', 'Use xsleep() in place of sleep()'),
>> +('abort', 'Use xabort() in place of abort()'),
>
> I don't think we have an xabort function.
>
>> +('error', 'Use xerror() in place of error()'),
>
> I don't know of either error *or* xerror functions.

:-) Thanks for looking through. I started from lib/util.h and assumed
that anything prefaced with 'x' replaces a common library function.

For xabort/xerror I think I was looking at ovs_abort() and
ovs_error(). The former is probably a useful substitute but perhaps
not the latter.

I'll send a v2.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] checkpatch: Check for stdlib usage.

[Ben Pfaff]

On Mon, May 22, 2017 at 03:51:39PM -0700, Joe Stringer wrote:
> Many standard library functions are wrapped in OVS, so check for usage
> of the original versions and suggest that authors replace them with the
> OVS versions.
> 
> Signed-off-by: Joe Stringer 

Thanks for working to improve the patch checks!

> +def regex_function_factory(func_name):
> +regex = re.compile('[^x]%s\([^)]*\)' % func_name)
> +return lambda x: regex.search(x) is not None
> +
> +
> +std_functions = [
> +('malloc', 'Use xmalloc() in place of malloc()'),
> +('calloc', 'Use xcmalloc() in place of calloc()'),

xcalloc, not xcmalloc

> +('zalloc', 'Use xzmalloc() in place of zalloc()'),

I don't think there's a zalloc function.

> +('realloc', 'Use xrealloc() in place of realloc()'),
> +('memdup', 'Use xmemdup() in place of memdup()'),
> +('memdup0', 'Use xmemdup0() in place of memdup0()'),

I don't think there's a memdup0 function.

> +('strdup', 'Use xstrdup() in place of strdup()'),
> +('asprintf', 'Use xasprintf() in place of asprintf()'),
> +('vasprintf', 'Use xvasprintf() in place of vasprintf()'),
> +('2nrealloc', 'Use x2nrealloc() in place of 2nrealloc()'),

I don't think there's a 2nrealloc function.

> +('strlcpy', 'Use ovs_strlcpy() in place of strlcpy()'),
> +('strzcpy', 'Use ovs_strzcpy() in place of strzcpy()'),

I don't think there's a strzcpy function, but ovs_strzcpy() might be a
substitute for strncpy.

It can often be a good idea to use ovs_strlcpy() in place of plain
strcpy().

> +('strerror', 'Use ovs_strerror() in place of strerror()'),
> +('sleep', 'Use xsleep() in place of sleep()'),
> +('abort', 'Use xabort() in place of abort()'),

I don't think we have an xabort function.

> +('error', 'Use xerror() in place of error()'),

I don't know of either error *or* xerror functions.

Thanks,

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

Re: [ovs-dev] [PATCH] checkpatch: Check for stdlib usage.

[Greg Rose]

On Mon, 2017-05-22 at 15:51 -0700, Joe Stringer wrote:
> Many standard library functions are wrapped in OVS, so check for usage
> of the original versions and suggest that authors replace them with the
> OVS versions.
> 
> Signed-off-by: Joe Stringer 
> ---
>  utilities/checkpatch.py | 31 +++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
> index d486de81c8ff..4e2f5a42817f 100755
> --- a/utilities/checkpatch.py
> +++ b/utilities/checkpatch.py
> @@ -210,6 +210,37 @@ checks = [
>  ]
>  
> 
> +def regex_function_factory(func_name):
> +regex = re.compile('[^x]%s\([^)]*\)' % func_name)
> +return lambda x: regex.search(x) is not None
> +
> +
> +std_functions = [
> +('malloc', 'Use xmalloc() in place of malloc()'),
> +('calloc', 'Use xcmalloc() in place of calloc()'),
> +('zalloc', 'Use xzmalloc() in place of zalloc()'),
> +('realloc', 'Use xrealloc() in place of realloc()'),
> +('memdup', 'Use xmemdup() in place of memdup()'),
> +('memdup0', 'Use xmemdup0() in place of memdup0()'),
> +('strdup', 'Use xstrdup() in place of strdup()'),
> +('asprintf', 'Use xasprintf() in place of asprintf()'),
> +('vasprintf', 'Use xvasprintf() in place of vasprintf()'),
> +('2nrealloc', 'Use x2nrealloc() in place of 2nrealloc()'),
> +('strlcpy', 'Use ovs_strlcpy() in place of strlcpy()'),
> +('strzcpy', 'Use ovs_strzcpy() in place of strzcpy()'),
> +('strerror', 'Use ovs_strerror() in place of strerror()'),
> +('sleep', 'Use xsleep() in place of sleep()'),
> +('abort', 'Use xabort() in place of abort()'),
> +('error', 'Use xerror() in place of error()'),
> +]
> +checks += [
> +{'regex': '(.c|.h)(.in)?$',
> + 'match_name': None,
> + 'check': regex_function_factory(function_name),
> + 'print': lambda: print_error(description)}
> +for function_name, description in std_functions]
> +
> +
>  def get_file_type_checks(filename):
>  """Returns the list of checks for a file based on matching the filename
> against regex."""

Good idea.

thanks!

Acked-by: Greg Rose 

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

Re: [ovs-dev] [PATCH 1/2] ovn pacemaker: Fix return code errors in start/stop action

[Andy Zhou]

On Sun, May 21, 2017 at 6:35 PM,   wrote:
> From: Numan Siddique 
>
> start action returns OCF_RUNNING_MASTER in certain scenarios.
> But as per the OCF guidelines, status code OCF_RUNNING_MASTER shoud
> be returned only in monitor action [1].
>
> Whenever the start action returns OCF_RUNNING_MASTER, it is observed
> in the testing that, pacemaker stops the ovsdb-server ocf resource
> in that node. This patch fixes this issue by returning OCF_SUCESS in
> such cases.
>
> stop action returns OCF_RUNNING_MASTER if the ovsdb-servers are
> running as master. But as per the OCF guidelines [2], stop action
> should only return OCF_SUCCESS. If any other code is returned,
> pacemaker cluster would block that resource in that node.
>
> This patch fixes this issue by stopping the ovsdb-servers when they
> are running as masters (which is the expected case) and returns
> OCF_SUCCESS.
>
> [1] - 
> http://www.linux-ha.org/doc/dev-guides/_literal_ocf_running_master_literal_8.html
> [2] - http://www.linux-ha.org/doc/dev-guides/_literal_stop_literal_action.html
>
> CC: Andy Zhou 
> Signed-off-by: Numan Siddique 

Thanks for the fixes!  Both patches look reasonable to me. I pushed
them to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/3] dpif-netlink-rtnl: Tidy up some code.

[Joe Stringer]

On 19 May 2017 at 15:47, Greg Rose  wrote:
> On Fri, 2017-05-19 at 13:27 -0700, Joe Stringer wrote:
>> Simplify and refactor a couple of bits of code for improved readability.
>>
>> Signed-off-by: Joe Stringer 
>> ---
>>  lib/dpif-netlink-rtnl.c | 20 ++--
>>  1 file changed, 10 insertions(+), 10 deletions(-)
>>
>> diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
>> index 7faad5248037..0ca6529e9d82 100644
>> --- a/lib/dpif-netlink-rtnl.c
>> +++ b/lib/dpif-netlink-rtnl.c
>> @@ -1,5 +1,6 @@
>>  /*
>>   * Copyright (c) 2017 Red Hat, Inc.
>> + * Copyright (c) 2017 Nicira, Inc.
>
> ???
>
> The patch itself looks fine but changing a copyright seems strange.

Thanks for the reviews.

While reviewing the earlier revisions of the series, I collected a
bunch of changes, some that I submitted as suggestions which ended up
folded into the merged series, and some which I figured I could just
submit later so that we didn't have to keep going back and forth on
the series. Looks like this hunk was brought in when I rebased my
changes.

Fair enough though, there's nothing particularly interesting about
this particular patch that would warrant this. I can skip it.

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

Re: [ovs-dev] [patch_v1] flow.c: refactor ct_orig_tuple check in miniflow_extract.

[Joe Stringer]

On 20 May 2017 at 11:09, Darrell Ball  wrote:
> The checks to populate ct_orig_tuple in miniflow_extract
> includes recirc_id being non-zero.  This is changed here
> to populate the ct_orig_tuple fields based only on ct_state
> per the logic of the design.
>
> Signed-off-by: Darrell Ball 

I'm not exactly sure what "per the logic of the design" is supposed to
mean, and I suspect that someone reading through the git logs would be
just as lost. As far as I'm aware, any patch to OVS should be "per the
logic of the design", right?

Do you mean "The ct_orig_tuple fields are designed to be read only if
the ct_state is valid, so do not populate that field if the packet has
not gone through the connection tracker"? I think that this kind of
description would provide at least a little extra context to help
understand the motivation behind the change.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] tests: fix hanging test on windows

[Joe Stringer]

On 22 May 2017 at 05:18, Alin Serdean  wrote:
> 'multiple bridges share a controller' hangs on windows because it is
> lacking the exit information (it will hang when the test has finished)
>
> Introduce a pidfile to 'ovs-testcontroller' and end it on exit based on
> the pidfile.
>
> Signed-off-by: Alin Gabriel Serdean 

Hi Alin,

"on_exit" will queue up a command to be run at the end of the test
run, regardless of success or failure. As such, I think that this
should be run immediately after the launch of ovs-testcontroller.
Otherwise it's possible that something else in the test fails before
the end, and ovs-testcontroller is not cleaned up.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] checkpatch: Check for stdlib usage.

[Joe Stringer]

Many standard library functions are wrapped in OVS, so check for usage
of the original versions and suggest that authors replace them with the
OVS versions.

Signed-off-by: Joe Stringer 
---
 utilities/checkpatch.py | 31 +++
 1 file changed, 31 insertions(+)

diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
index d486de81c8ff..4e2f5a42817f 100755
--- a/utilities/checkpatch.py
+++ b/utilities/checkpatch.py
@@ -210,6 +210,37 @@ checks = [
 ]
 
 
+def regex_function_factory(func_name):
+regex = re.compile('[^x]%s\([^)]*\)' % func_name)
+return lambda x: regex.search(x) is not None
+
+
+std_functions = [
+('malloc', 'Use xmalloc() in place of malloc()'),
+('calloc', 'Use xcmalloc() in place of calloc()'),
+('zalloc', 'Use xzmalloc() in place of zalloc()'),
+('realloc', 'Use xrealloc() in place of realloc()'),
+('memdup', 'Use xmemdup() in place of memdup()'),
+('memdup0', 'Use xmemdup0() in place of memdup0()'),
+('strdup', 'Use xstrdup() in place of strdup()'),
+('asprintf', 'Use xasprintf() in place of asprintf()'),
+('vasprintf', 'Use xvasprintf() in place of vasprintf()'),
+('2nrealloc', 'Use x2nrealloc() in place of 2nrealloc()'),
+('strlcpy', 'Use ovs_strlcpy() in place of strlcpy()'),
+('strzcpy', 'Use ovs_strzcpy() in place of strzcpy()'),
+('strerror', 'Use ovs_strerror() in place of strerror()'),
+('sleep', 'Use xsleep() in place of sleep()'),
+('abort', 'Use xabort() in place of abort()'),
+('error', 'Use xerror() in place of error()'),
+]
+checks += [
+{'regex': '(.c|.h)(.in)?$',
+ 'match_name': None,
+ 'check': regex_function_factory(function_name),
+ 'print': lambda: print_error(description)}
+for function_name, description in std_functions]
+
+
 def get_file_type_checks(filename):
 """Returns the list of checks for a file based on matching the filename
against regex."""
-- 
2.12.2

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

[ovs-dev] Oakley Sunglass Warehouse

[Oakley Sunglass Warehouse]

From:   Oakley Sunglass Warehouse
Email:  ovs-dev@openvswitch.org

--

Hi

It's our birthday and we are celebrating 2 years of our outlet.  We'd like
to offer you only 19.95 USD for all styles of Oakley and Ray Ban Sunglasses.

Please visit our online store for more details:
http://www.queeniefashion.shop  


YOU WON'T WANT TO MISS THESE OFFERS


To your success!

Regards, 

Mila

Oakley Sunglass Warehouse

--


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

Re: [ovs-dev] [PATCH 3/3] dpif-netlink-rtnl: Use OVS_NOT_REACHED in verify.

[Eric Garver]

On Fri, May 19, 2017 at 01:27:36PM -0700, Joe Stringer wrote:
> The vport_type_to_kind() call at the top of dpif_netlink_rtnl_verify()
> ensures that these cases can never be hit, so use OVS_NOT_REACHED()
> instead of setting the err to EOPNOTSUPP.
> 
> Signed-off-by: Joe Stringer 
> ---
>  lib/dpif-netlink-rtnl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
> index 76ab0fe3fdec..c57923756f42 100644
> --- a/lib/dpif-netlink-rtnl.c
> +++ b/lib/dpif-netlink-rtnl.c
> @@ -256,7 +256,7 @@ dpif_netlink_rtnl_verify(const struct 
> netdev_tunnel_config *tnl_cfg,
>  case OVS_VPORT_TYPE_UNSPEC:
>  case __OVS_VPORT_TYPE_MAX:
>  default:
> -err = EOPNOTSUPP;
> +OVS_NOT_REACHED();
>  }
>  
>  ofpbuf_delete(reply);
> -- 
> 2.11.1
> 

Acked-by: Eric Garver 

Thanks for the clean ups, Joe.
Eric.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 2/3] dpif-netlink-rtnl: Use getlink() in common verify path.

[Eric Garver]

On Fri, May 19, 2017 at 01:27:35PM -0700, Joe Stringer wrote:
> The calls here were duplicated across each tunnel protocol.
> 
> Signed-off-by: Joe Stringer 
> ---
>  lib/dpif-netlink-rtnl.c | 100 
> +---
>  1 file changed, 43 insertions(+), 57 deletions(-)
> 
> diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
> index 0ca6529e9d82..76ab0fe3fdec 100644
> --- a/lib/dpif-netlink-rtnl.c
> +++ b/lib/dpif-netlink-rtnl.c
> @@ -160,34 +160,23 @@ rtnl_policy_parse(const char *kind, struct ofpbuf 
> *reply,
>  
>  static int
>  dpif_netlink_rtnl_vxlan_verify(const struct netdev_tunnel_config *tnl_cfg,
> -   const char *name, const char *kind)
> +   const char *kind, struct ofpbuf *reply)
>  {
> -struct ofpbuf *reply;
> +struct nlattr *vxlan[ARRAY_SIZE(vxlan_policy)];
>  int err;
>  
> -err = dpif_netlink_rtnl_getlink(name, );
> -
> +err = rtnl_policy_parse(kind, reply, vxlan_policy, vxlan,
> +ARRAY_SIZE(vxlan_policy));
>  if (!err) {
> -struct nlattr *vxlan[ARRAY_SIZE(vxlan_policy)];
> -
> -err = rtnl_policy_parse(kind, reply, vxlan_policy, vxlan,
> -ARRAY_SIZE(vxlan_policy));
> -if (!err) {
> -if (0 != nl_attr_get_u8(vxlan[IFLA_VXLAN_LEARNING])
> -|| 1 != nl_attr_get_u8(vxlan[IFLA_VXLAN_COLLECT_METADATA])
> -|| 1 != nl_attr_get_u8(vxlan[IFLA_VXLAN_UDP_ZERO_CSUM6_RX])
> -|| (tnl_cfg->dst_port
> -!= nl_attr_get_be16(vxlan[IFLA_VXLAN_PORT]))) {
> -err = EINVAL;
> -}
> -}
> -if (!err) {
> -if (tnl_cfg->exts & (1 << OVS_VXLAN_EXT_GBP)
> -&& !nl_attr_get_flag(vxlan[IFLA_VXLAN_GBP])) {
> -err = EINVAL;
> -}
> +if (0 != nl_attr_get_u8(vxlan[IFLA_VXLAN_LEARNING])
> +|| 1 != nl_attr_get_u8(vxlan[IFLA_VXLAN_COLLECT_METADATA])
> +|| 1 != nl_attr_get_u8(vxlan[IFLA_VXLAN_UDP_ZERO_CSUM6_RX])
> +|| (tnl_cfg->dst_port
> +!= nl_attr_get_be16(vxlan[IFLA_VXLAN_PORT]))
> +|| (tnl_cfg->exts & (1 << OVS_VXLAN_EXT_GBP)
> +&& !nl_attr_get_flag(vxlan[IFLA_VXLAN_GBP]))) {
> +err = EINVAL;
>  }
> -ofpbuf_delete(reply);
>  }
>  
>  return err;
> @@ -195,24 +184,17 @@ dpif_netlink_rtnl_vxlan_verify(const struct 
> netdev_tunnel_config *tnl_cfg,
>  
>  static int
>  dpif_netlink_rtnl_gre_verify(const struct netdev_tunnel_config OVS_UNUSED 
> *tnl,
> - const char *name, const char *kind)
> + const char *kind, struct ofpbuf *reply)
>  {
> -struct ofpbuf *reply;
> +struct nlattr *gre[ARRAY_SIZE(gre_policy)];
>  int err;
>  
> -err = dpif_netlink_rtnl_getlink(name, );
> -
> +err = rtnl_policy_parse(kind, reply, gre_policy, gre,
> +ARRAY_SIZE(gre_policy));
>  if (!err) {
> -struct nlattr *gre[ARRAY_SIZE(gre_policy)];
> -
> -err = rtnl_policy_parse(kind, reply, gre_policy, gre,
> -ARRAY_SIZE(gre_policy));
> -if (!err) {
> -if (!nl_attr_get_flag(gre[IFLA_GRE_COLLECT_METADATA])) {
> -err = EINVAL;
> -}
> +if (!nl_attr_get_flag(gre[IFLA_GRE_COLLECT_METADATA])) {
> +err = EINVAL;
>  }
> -ofpbuf_delete(reply);
>  }
>  
>  return err;
> @@ -220,27 +202,20 @@ dpif_netlink_rtnl_gre_verify(const struct 
> netdev_tunnel_config OVS_UNUSED *tnl,
>  
>  static int
>  dpif_netlink_rtnl_geneve_verify(const struct netdev_tunnel_config *tnl_cfg,
> -const char *name, const char *kind)
> +const char *kind, struct ofpbuf *reply)
>  {
> -struct ofpbuf *reply;
> +struct nlattr *geneve[ARRAY_SIZE(geneve_policy)];
>  int err;
>  
> -err = dpif_netlink_rtnl_getlink(name, );
> -
> +err = rtnl_policy_parse(kind, reply, geneve_policy, geneve,
> +ARRAY_SIZE(geneve_policy));
>  if (!err) {
> -struct nlattr *geneve[ARRAY_SIZE(geneve_policy)];
> -
> -err = rtnl_policy_parse(kind, reply, geneve_policy, geneve,
> -ARRAY_SIZE(geneve_policy));
> -if (!err) {
> -if (!nl_attr_get_flag(geneve[IFLA_GENEVE_COLLECT_METADATA])
> -|| 1 != nl_attr_get_u8(geneve[IFLA_GENEVE_UDP_ZERO_CSUM6_RX])
> -|| (tnl_cfg->dst_port
> -!= nl_attr_get_be16(geneve[IFLA_GENEVE_PORT]))) {
> -err = EINVAL;
> -}
> +if (!nl_attr_get_flag(geneve[IFLA_GENEVE_COLLECT_METADATA])
> +|| 1 != nl_attr_get_u8(geneve[IFLA_GENEVE_UDP_ZERO_CSUM6_RX])
> +|| 

Re: [ovs-dev] [PATCH 1/3] dpif-netlink-rtnl: Tidy up some code.

[Eric Garver]

On Fri, May 19, 2017 at 01:27:34PM -0700, Joe Stringer wrote:
> Simplify and refactor a couple of bits of code for improved readability.
> 
> Signed-off-by: Joe Stringer 
> ---
>  lib/dpif-netlink-rtnl.c | 20 ++--
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
> index 7faad5248037..0ca6529e9d82 100644
> --- a/lib/dpif-netlink-rtnl.c
> +++ b/lib/dpif-netlink-rtnl.c
> @@ -1,5 +1,6 @@
>  /*
>   * Copyright (c) 2017 Red Hat, Inc.
> + * Copyright (c) 2017 Nicira, Inc.
>   *
>   * Licensed under the Apache License, Version 2.0 (the "License");
>   * you may not use this file except in compliance with the License.
> @@ -369,17 +370,16 @@ dpif_netlink_rtnl_port_create(struct netdev *netdev)
>  err = dpif_netlink_rtnl_verify(tnl_cfg, type, name);
>  if (!err) {
>  return 0;
> -} else {
> -err = dpif_netlink_rtnl_destroy(name);
> -if (err) {
> -static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 
> 5);
> -
> -VLOG_WARN_RL(, "RTNL device %s exists and cannot be "
> - "deleted: %s", name, ovs_strerror(err));
> -return err;
> -}
> -err = dpif_netlink_rtnl_create(tnl_cfg, name, type, kind, flags);
>  }
> +err = dpif_netlink_rtnl_destroy(name);
> +if (err) {
> +static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
> +
> +VLOG_WARN_RL(, "RTNL device %s exists and cannot be "
> + "deleted: %s", name, ovs_strerror(err));
> +return err;
> +}
> +err = dpif_netlink_rtnl_create(tnl_cfg, name, type, kind, flags);
>  }
>  if (err) {
>  return err;
> -- 
> 2.11.1
> 

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

Re: [ovs-dev] [PATCH] Debian: Install libraries in Multi-Arch format

[Ben Warren via dev]

Pleas don’t apply this.  It turns out more work is required.

—Ben
> On May 18, 2017, at 4:49 PM, b...@skyportsystems.com wrote:
> 
> From: Ben Warren 
> 
> The Debian 'Multi-Arch' feature moves all library files (*.a, *.so) to
> an architecture-specific directory.  This allows libraries for multiple
> architectures to co-exist on a host and simplifies cross-compilation.
> 
> For example, issuing the following command:
> 
> apt-get install openvswitch-dev:amd64 openvswitch-dev:mips64el
> 
> will place libraries as follows:
> 
> $ find /usr/lib/ -name libopenvswitch*
> /usr/lib/mips64el-linux-gnuabi64/pkgconfig/libopenvswitch.pc
> /usr/lib/mips64el-linux-gnuabi64/libopenvswitch.a
> /usr/lib/mips64el-linux-gnuabi64/libopenvswitch.so
> /usr/lib/x86_64-linux-gnu/pkgconfig/libopenvswitch.pc
> /usr/lib/x86_64-linux-gnu/libopenvswitch.a
> /usr/lib/x86_64-linux-gnu/libopenvswitch.so
> 
> Signed-off-by: Ben Warren 
> ---
> debian/compat | 2 +-
> debian/control| 8 +---
> debian/openvswitch-common.install | 2 +-
> debian/openvswitch-dev.install| 6 +++---
> 4 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/debian/compat b/debian/compat
> index 45a4fb7..ec63514 100644
> --- a/debian/compat
> +++ b/debian/compat
> @@ -1 +1 @@
> -8
> +9
> diff --git a/debian/control b/debian/control
> index 0b75f2b..b8b14bc 100644
> --- a/debian/control
> +++ b/debian/control
> @@ -56,6 +56,7 @@ Description: Open vSwitch datapath module source - DKMS 
> version
> 
> Package: openvswitch-common
> Architecture: linux-any
> +Multi-Arch: foreign
> Depends: openssl,
>  python (>= 2.7),
>  python-six,
> @@ -285,9 +286,10 @@ Description: Open vSwitch VTEP utilities
> 
> Package: openvswitch-dev
> Architecture: linux-any
> -Depends:
> - openvswitch-common (>= ${binary:Version}),
> - ${misc:Depends}
> +Multi-Arch: same
> +Depends: openvswitch-common (= ${binary:Version}),
> + ${misc:Depends},
> + ${shlibs:Depends}
> Description: Open vSwitch development package
>  Open vSwitch is a production quality, multilayer, software-based, Ethernet
>  virtual switch. It is designed to enable massive network automation through
> diff --git a/debian/openvswitch-common.install 
> b/debian/openvswitch-common.install
> index ebb7d5c..aa41204 100644
> --- a/debian/openvswitch-common.install
> +++ b/debian/openvswitch-common.install
> @@ -9,4 +9,4 @@ usr/sbin/ovs-bugtool
> usr/share/openvswitch/bugtool-plugins
> usr/share/openvswitch/scripts/ovs-bugtool-*
> usr/share/openvswitch/scripts/ovs-lib
> -usr/lib/lib*.so.*
> +usr/lib/*/lib*.so.*
> diff --git a/debian/openvswitch-dev.install b/debian/openvswitch-dev.install
> index 11791e4..ca3d22c 100644
> --- a/debian/openvswitch-dev.install
> +++ b/debian/openvswitch-dev.install
> @@ -1,6 +1,6 @@
> -usr/lib/lib*.so
> -usr/lib/lib*.a
> -usr/lib/pkgconfig
> +usr/lib/*/lib*.so
> +usr/lib/*/lib*.a
> +usr/lib/*/pkgconfig
> include/*.h usr/include/openvswitch
> include/openflow/*.h usr/include/openvswitch/openflow
> include/openvswitch/*.h usr/include/openvswitch/openvswitch
> -- 
> 2.6.4
> 

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

Re: [ovs-dev] [RFC PATCH 0/6] Change dpdk rxq scheduling to incorporate rxq processing cycles.

[Bodireddy, Bhanuprakash]

>> Thanks for working on this. This work is important and would also solve OvS-
>DPDK scaling issue.
>> I have reviewed the patch series but haven't tested this yet with traffic. I
>would stick to comments on high level design at his stage.
>>
>> With this series, rxqs are sorted based on cycles consumed and are
>subsequently distributed across multiple PMDs in round robin fashion. While
>this approach is fine, I have some concerns w.r.t QoS.
>>
>> Now OvS-DPDK don't support QoS (i.e based on L2 - pcp, L3 - dscp bits). But
>when QoS is implemented in future based on Weighted Round Robin(WRR),
>Strict priority(SP) (or) hybrid models(WRR and SP) the proposed approach in
>this RFC may conflict or pose some limitations.
>>
>> I am aware that this RFC doesn't factor in QoS, but would like to know your
>thoughts and if you have some ideas to  make it less painful in future.
>>
>> 1)  In WRR/SP model, the rxq(s) shall be sorted according to queue
>credits/priorities.  PMD may have to poll rxq(s) based on this criteria?
>> 2) For example, signaling/control traffic which is high priority may be
>redirected to a specific rxq(based on pcp/dscp bits) in most of the scenarios.
>Also the control traffic can be <= 5% of total traffic and hence the cycles 
>spent
>on this queue can be insignificant and these rxq(s) shall be pushed to the end
>of the sorted list as per the proposed approach. Most importantly rxq
>receiving the *high volume lowest priority traffic* will top the list always 
>and
>shall be processed first by the respective PMD threads potentially tail
>dropping *low rate high priority traffic* during congestion.
>>
>> Also adding Billy from Intel who is working QoS for OvS-DPDK.
>>
>> - Bhanuprakash
>>
>
>Thanks for your comments. I can only speculate based on the high level
>description of QoS but I don't think it should be an issue. Think of this RFC 
>as a
>refinement of the default round robin queue to core assignment.
>
>At the moment, the rxq affinity pinning takes precedence over the default
>assignment. I would expect that a user defined QoS should also take
>precedence, but that there will be still be a need for code to perform the
>default assignment because QoS would be optional and/or queues may have
>equal priorities.

This makes sense. I will try to test this series with some traffic.

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

Re: [ovs-dev] [RFC PATCH 0/6] Change dpdk rxq scheduling to incorporate rxq processing cycles.

[Jan Scheurich]

Hi Kevin,

Thanks a lot for addressing this very important limitation of OVS-DPDK 
multi-core scalability in cloud contexts such as OpenStack. This is highly 
appreciated!

We have not started testing this, so for the time being just some high-level 
comments:

I would really like to see a new command ovs-appctl 
dpif-netdev/pmd-rxq-rebalance or similar to manually trigger redistribution for 
testing without having to reconfigure things.

Any redistribution of rx queues across PMDs under high load is a critical thing 
as the service interruption during the PMD reload can easily cause rx queue 
overruns and packet drop. Independently from this patch that optimizes the load 
balance of PMDs after redistribution, we should try to improve the actual 
reconfiguration to become hitless (i.e. not requiring a reload of PMDs).

In OpenStack context we really need an automatic re-balancing of rx queues over 
PMDs when the load balance of PMDs becomes so skewed that OVS unnecessarily 
drops packets due to overload of some PMDs while others are not fully loaded. 
Without such a function this patch does really not solve the scalability issue. 
Starting a new VM forces a re-balance, but that cannot take the load on the 
just added ports into account, so it will typically be sub-optimal. Also OVS 
would have no means to adapt to shifting load over time in a stable 
configuration.

If re-balance were hitless (see above) it could be triggered at any time. As 
long as it is not, it should probably only be triggered if a) there is overload 
on some PMD and b) a rebalancing would improve the situation such that there is 
zero (or less) loss. Due to a) the additional short service interruption should 
not matter.

A final note: when experimenting with a similar in-house prototype for rx queue 
rebalancing we have experienced strange effects with vhostuser tx queues 
locking up as a result of frequent reconfiguration. These might have been 
caused by internal vulnerabilities of the tested complex DPDK application in 
the guest, but I would suggest we pay very good attention to thread safety of 
shared DPDK and virtio data structures in host and guest when testing and 
reviewing this.

BR, Jan


> -Original Message-
> From: ovs-dev-boun...@openvswitch.org 
> [mailto:ovs-dev-boun...@openvswitch.org] On Behalf Of Kevin Traynor
> Sent: Friday, 05 May, 2017 18:34
> To: d...@openvswitch.org
> Subject: [ovs-dev] [RFC PATCH 0/6] Change dpdk rxq scheduling to incorporate 
> rxq processing cycles.
> 
> Rxqs are scheduled to be handled across available pmds in round robin
> order with no weight or priority.
> 
> It can happen that some very busy queues are handled by one pmd which
> does not have enough cycles to prevent packets being dropped on them.
> While at the same time another pmd which handles queues with no traffic
> on them, is essentially idling.
> 
> Rxq scheduling happens as a result of a number of events and when it does,
> the same unweighted round robin approach is applied each time.
> 
> This patchset proposes to augment the round robin nature of rxq scheduling
> by counting the processing cycles used by the rxqs during their operation
> and incorporate it into the rxq scheduling.
> 
> Before distributing in a round robin manner, the rxqs will be sorted in
> order of the processing cycles they have been consuming. Assuming multiple
> pmds, this ensures that the measured rxqs using most processing cycles will
> be distributed to different cores.
> 
> To try out:
> This patchset requires the updated pmd counting patch applied as a
> prerequisite. https://patchwork.ozlabs.org/patch/729970/
> 
> Alternatively the series with dependencies can be cloned from here:
> https://github.com/kevintraynor/ovs-rxq.git
> 
> Simple way to test is add some dpdk ports, add multiple pmds, vary traffic
> rates and rxqs on ports and trigger reschedules e.g. by changing rxqs or
> the pmd-cpu-mask.
> 
> Check rxq distribution with ovs-appctl dpif-netdev/pmd-rxq-show and see
> if it matches expected.
> 
> todo:
> -possibly add a dedicated reschedule trigger command
> -use consistent type names
> -update docs
> -more testing, especially for dual numa
> 
> thanks,
> Kevin.
> 
> Kevin Traynor (6):
>   dpif-netdev: Add rxq processing cycle counters.
>   dpif-netdev: Update rxq processing cycles from
> cycles_count_intermediate.
>   dpif-netdev: Change polled_queue to use dp_netdev_rxq.
>   dpif-netdev: Make dpcls optimization interval more generic.
>   dpif-netdev: Count the rxq processing cycles for an rxq.
>   dpif-netdev: Change rxq_scheduling to use rxq processing cycles.
> 
>  lib/dpif-netdev.c | 163 
> --
>  1 file changed, 133 insertions(+), 30 deletions(-)
> 
> --
> 1.8.3.1
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev 

Re: [ovs-dev] [PATCH] dpif-netdev: Fix insertion probability

[Jan Scheurich]

Thanks for the fix:
Acked-by: Jan Scheurich 

> -Original Message-
> From: ovs-dev-boun...@openvswitch.org 
> [mailto:ovs-dev-boun...@openvswitch.org] On Behalf Of Ciara Loftus
> Sent: Wednesday, 17 May, 2017 10:29
> To: d...@openvswitch.org
> Subject: [ovs-dev] [PATCH] dpif-netdev: Fix insertion probability
> 
> emc_conditional_insert uses pmd->last_cycles and the packet's RSS hash
> to generate a random number used to determine whether or not an emc
> entry should be inserted. This works for single-packet bursts as
> last_cycles is updated for each burst. However, for bursts > 1 packet,
> where the packets in the batch generate the same RSS hash,
> pmd->last_cycles remains constant for the entire burst also, and thus
> cannot be used as a random number for each packet in the burst.
> 
> This commit replaces the use of pmd->last_cycles with random_uint32()
> for this purpose and subsequently fixes the behavior of the
> emc_insert_inv_prob setting for high-throughput (large bursts)
> single-flow cases.
> 
> Fixes: 4c30b24602c3 ("dpif-netdev: Conditional EMC insert")
> Reported-by: Kevin Traynor 
> Signed-off-by: Ciara Loftus 
> ---
>  lib/dpif-netdev.c | 4 
>  1 file changed, 4 deletions(-)
> 
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index d215156..ab1e26e 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -2037,11 +2037,7 @@ emc_probabilistic_insert(struct dp_netdev_pmd_thread 
> *pmd,
>  uint32_t min;
>  atomic_read_relaxed(>dp->emc_insert_min, );
> 
> -#ifdef DPDK_NETDEV
> -if (min && (key->hash ^ (uint32_t) pmd->last_cycles) <= min) {
> -#else
>  if (min && (key->hash ^ random_uint32()) <= min) {
> -#endif
>  emc_insert(>flow_cache, key, flow);
>  }
>  }
> --
> 2.4.11
> 
> ___
> 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 v6 5/5] userspace: add vxlan gpe support to vport

[Jan Scheurich]

I had a similar comment to avoid bit-fields in the nsh_hdr struct in the NSH 
patch series.
@Yi: Have a look at the restructured nsh.h in my new branch.

I suggest you just add any required vxlan-gpe specific items to the VXLAN 
section in lib/packets.h rather than creating a new header file for vxlan-gpe.

/Jan

> -Original Message-
> From: ovs-dev-boun...@openvswitch.org 
> [mailto:ovs-dev-boun...@openvswitch.org] On Behalf Of Yang, Yi Y
> Sent: Friday, 19 May, 2017 07:18
> To: Ben Pfaff ; Zoltán Balogh 
> Cc: 'd...@openvswitch.org' 
> Subject: Re: [ovs-dev] [PATCH v6 5/5] userspace: add vxlan gpe support to 
> vport
> 
> Ben, vxlangpe.h is from Linux kernel header file, so style is Linux but not 
> ovs, keeping these intact is to make sure there are same pieces in
> both userspace and kernel, so code is also almost same. We'll change it to 
> ovs style per your comments. Zoltan, please let me know if you
> need any help from me.
> 
> -Original Message-
> From: ovs-dev-boun...@openvswitch.org 
> [mailto:ovs-dev-boun...@openvswitch.org] On Behalf Of Ben Pfaff
> Sent: Friday, May 19, 2017 12:54 PM
> To: Zoltán Balogh 
> Cc: 'd...@openvswitch.org' 
> Subject: Re: [ovs-dev] [PATCH v6 5/5] userspace: add vxlan gpe support to 
> vport
> 
> On Fri, May 12, 2017 at 11:07:46AM +, Zoltán Balogh wrote:
> > From: Georg Schmuecking 
> >
> > This patch is based on the "datapath: enable vxlangpe creation in compat 
> > mode"
> > from Yi Yang. It introduces an extension option "gpe" to the vxlan
> > port in the netdev-dpdk datapath. Furthermore it introduces a new
> > protocol header file vxlangpe.h in which the vxlan gpe protocoll is
> > described. In the vxlan specific methods the different packet are 
> > introduced and handled.
> >
> > Added VXLAN GPE tunnel push test.
> >
> > Signed-off-by: Yi Yang 
> > Signed-off-by: Georg Schmuecking 
> 
> checkpatch says:
> 
> ERROR: Inappropriate bracing around statement
> #138 FILE: lib/netdev-native-tnl.c:519:
> if (gpe->oam_flag)
> 
> WARNING: Line has non-spaces leading whitespace
> #153 FILE: lib/netdev-native-tnl.c:534:
> }
> 
> WARNING: Line length is >79-characters long
> #188 FILE: lib/netdev-native-tnl.c:578:
> put_16aligned_be32(>vx_vni, 
> htonl(ntohll(params->flow->tunnel.tun_id) << 8));
> 
> WARNING: Line length is >79-characters long
> #206 FILE: lib/netdev-native-tnl.c:596:
> put_16aligned_be32(>vx_vni, 
> htonl(ntohll(params->flow->tunnel.tun_id) << 8));
> 
> In netdev_vxlan_pop_header(), I don't see anything that verifies that the 
> packet is long enough when GPE is present.
> 
> The formatting of the switch statements look different from the usual OVS 
> style, which is like this:
> 
> switch (gpe->next_protocol) {
> case VXLAN_GPE_NP_IPV4:
> next_pt = PT_IPV4;
> break;
> case VXLAN_GPE_NP_IPV6:
> next_pt = PT_IPV6;
> break;
> case VXLAN_GPE_NP_ETHERNET:
> next_pt = PT_ETH;
> break;
> default:
> goto err;
> }
> 
> I suspect that struct vxlanhdr_gpe should use ovs_16aligned_be32 for the 
> vx_vni member.
> 
> I doubt that these #defines are a good idea, in vxlangpe.h:
> 
> #define u8 uint8_t
> #define u32 uint8_t
> #define __be32 ovs_be32
> 
> struct vxlanhdr in lib/packets.h is pretty different in philosophy from 
> struct vxlanhdr_gpe in vxlangpe.h.  I'd suggest making the new
> structure more like the existing one; OVS doesn't really use bit-fields much 
> and in my experience they do not make code much easier to
> deal with.
> 
> The new struct vxlan_metadata doesn't seem to be used anywhere and I wonder 
> whether it should be included.
> 
> 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 v6 4/5] ofproto-dpif-xlate: refactor compose_output_action__

[Jan Scheurich]

Thanks for merging this!

I do agree that we should continue to try to simplify this further. 

Perhaps we can have a look at it in the new attempt to upstream our previously 
reverted patch to avoid recirculation at TX to tunnel port, which refactors 
some of the patch-port ("output to peer") logic.

BR, Jan

> -Original Message-
> From: ovs-dev-boun...@openvswitch.org 
> [mailto:ovs-dev-boun...@openvswitch.org] On Behalf Of Ben Pfaff
> Sent: Friday, 19 May, 2017 06:42
> To: Zoltán Balogh 
> Cc: 'd...@openvswitch.org' 
> Subject: Re: [ovs-dev] [PATCH v6 4/5] ofproto-dpif-xlate: refactor 
> compose_output_action__
> 
> On Fri, May 12, 2017 at 11:07:43AM +, Zoltán Balogh wrote:
> > From: Jan Scheurich 
> >
> > The very long function compose_output_action__() has been re-factored to 
> > make
> > the different cases for output to patch-port, native tunnel port, kernel 
> > tunnel
> > port, recirculation, or termination of a native tunnel at output to LOCAL 
> > port
> > clearer. Larger, self-contained blocks have been split out into separate
> > functions.
> >
> > Signed-off-by: Jan Scheurich 
> > Co-authored-by: Zoltan Balogh 
> >
> > Conflicts:
> > ofproto/ofproto-dpif-xlate.c
> 
> I'm happy with this, it's a nice refactoring, although one wonders
> whether it can be taken even farther.
> 
> I folded in the following incremental and applied this to master.
> 
> --8<--cut here-->8--
> 
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 59ef77bf998a..f71a9db0a6b3 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -3303,11 +3303,9 @@ check_output_prerequisites(struct xlate_ctx *ctx,
>  return true;
>  }
> 
> -static inline bool
> -terminate_native_tunnel(struct xlate_ctx *ctx,
> -ofp_port_t ofp_port,
> -struct flow *flow,
> -struct flow_wildcards *wc,
> +static bool
> +terminate_native_tunnel(struct xlate_ctx *ctx, ofp_port_t ofp_port,
> +struct flow *flow, struct flow_wildcards *wc,
>  odp_port_t *tnl_port)
>  {
>  *tnl_port = ODPP_NONE;
> @@ -3319,7 +3317,7 @@ terminate_native_tunnel(struct xlate_ctx *ctx,
>  *tnl_port = tnl_port_map_lookup(flow, wc);
>  }
> 
> -return (*tnl_port != ODPP_NONE);
> +return *tnl_port != ODPP_NONE;
>  }
> 
>  static void
> @@ -3535,12 +3533,11 @@ compose_output_action__(struct xlate_ctx *ctx, 
> ofp_port_t ofp_port,
>  }
> 
>  if (out_port != ODPP_NONE) {
> -
>  /* Commit accumulated flow updates before output. */
>  xlate_commit_actions(ctx);
> 
>  if (xr) {
> -/* Recirculate the packet */
> +/* Recirculate the packet. */
>  struct ovs_action_hash *act_hash;
> 
>  /* Hash action. */
> @@ -3553,7 +3550,6 @@ compose_output_action__(struct xlate_ctx *ctx, 
> ofp_port_t ofp_port,
>  /* Recirc action. */
>  nl_msg_put_u32(ctx->odp_actions, OVS_ACTION_ATTR_RECIRC,
> xr->recirc_id);
> -
>  } else if (is_native_tunnel) {
>  /* Output to native tunnel port. */
>  build_tunnel_send(ctx, xport, flow, odp_port);
> @@ -3562,8 +3558,7 @@ compose_output_action__(struct xlate_ctx *ctx, 
> ofp_port_t ofp_port,
>  } else if (terminate_native_tunnel(ctx, ofp_port, flow, wc,
> _tnl_port)) {
>  /* Intercept packet to be received on native tunnel port. */
> -nl_msg_put_odp_port(ctx->odp_actions,
> -OVS_ACTION_ATTR_TUNNEL_POP,
> +nl_msg_put_odp_port(ctx->odp_actions, OVS_ACTION_ATTR_TUNNEL_POP,
>  odp_tnl_port);
> 
>  } else {
> ___
> 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 v2] netflow: Fix memory leak in netflow_unref.

[Greg Rose]

On Mon, 2017-05-22 at 12:55 +0800, Yunjian Wang wrote:
> The memory leak was triggered each time on calling netflow_unref() with
> containing netflow_flows. And flows need to be removed and destroyed.
> 
> Signed-off-by: Yunjian Wang 
> ---
>  ofproto/netflow.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/ofproto/netflow.c b/ofproto/netflow.c
> index 55f7814..29c5f3e 100644
> --- a/ofproto/netflow.c
> +++ b/ofproto/netflow.c
> @@ -409,10 +409,17 @@ netflow_ref(const struct netflow *nf_)
>  void
>  netflow_unref(struct netflow *nf)
>  {
> +struct netflow_flow *nf_flow, *next;
> +
>  if (nf && ovs_refcount_unref_relaxed(>ref_cnt) == 1) {
>  atomic_count_dec(_count);
>  collectors_destroy(nf->collectors);
>  ofpbuf_uninit(>packet);
> +HMAP_FOR_EACH_SAFE (nf_flow, next, hmap_node, >flows) {
> +hmap_remove(>flows, _flow->hmap_node);
> +free(nf_flow);
> +}
> +hmap_destroy(>flows);
>  free(nf);
>  }
>  }

This looks right to me.  The only other place I see the flows freed is
when they're detected as idle.  If the flow is never detected as idle
then I don't see anywhere else that they are freed up after the xzalloc
in netflow_flow_update().

Reviewed-by: Greg Rose 

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

Re: [ovs-dev] [PATCH v6 2/5] userspace: L3 tunnel support for GRE and LISP

[Jan Scheurich]

> I think that parse_gre_header() should perhaps be pickier about the
> Ethertypes it accepts, since values below 0x600 are not valid
> Ethertypes and sometimes they are used for special purposes, for example
> OpenFlow uses 0x5ff to mean that the frame lacks an Ethertype.

I agree. OVS could just drop packets from GRE tunnels with GRE protocols < 
0x600.

> 
> I recommend adding an item to NEWS to mention this new user-visible
> feature.

What about:

diff --git a/NEWS b/NEWS
index 25eb477..bbed787 100644
--- a/NEWS
+++ b/NEWS
@@ -45,6 +45,12 @@ Post-v2.7.0
- Fedora Packaging:
  * OVN services are no longer restarted automatically after upgrade.
- Add --cleanup option to command 'ovs-appctl exit' (see ovs-vswitchd(8)).
+   - L3 tunneling:
+ * Add "layer3" options for tunnel ports that support non-Ethernet (L3)
+   payload (GRE, VXLAN-GPE).
+ * New vxlan tunnel extension "gpe" to support VXLAN-GPE tunnels.
+ * Transparently pop and push Ethernet headers at transmit/reception
+   of packets to/from L3 tunnels.

 v2.7.0 - 21 Feb 2017
 -

> 
> Ideally, some new documentation would explain how layer 2 and 3 packets
> interact.

I am planning for proper documentation along the lines of the Google doc as 
part of the overall PTAP and generic Encap/Decap patch complex.

Can you recommend a good authoring tool for the .rst format used in OVS lately?

BR, Jan

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

Re: [ovs-dev] [RFC PATCH 0/6] Change dpdk rxq scheduling to incorporate rxq processing cycles.

[Kevin Traynor]

Hi Bhanu,

On 05/21/2017 10:25 PM, Bodireddy, Bhanuprakash wrote:
> Hello Kevin,
> 
>>
>> Rxqs are scheduled to be handled across available pmds in round robin order
>> with no weight or priority.
>>
>> It can happen that some very busy queues are handled by one pmd which
>> does not have enough cycles to prevent packets being dropped on them.
>> While at the same time another pmd which handles queues with no traffic on
>> them, is essentially idling.
>>
>> Rxq scheduling happens as a result of a number of events and when it does,
>> the same unweighted round robin approach is applied each time.
>>
>> This patchset proposes to augment the round robin nature of rxq scheduling
>> by counting the processing cycles used by the rxqs during their operation and
>> incorporate it into the rxq scheduling.
>>
>> Before distributing in a round robin manner, the rxqs will be sorted in 
>> order of
>> the processing cycles they have been consuming. Assuming multiple pmds,
>> this ensures that the measured rxqs using most processing cycles will be
>> distributed to different cores.
> 
> Thanks for working on this. This work is important and would also solve 
> OvS-DPDK scaling issue.  
> I have reviewed the patch series but haven't tested this yet with traffic. I 
> would stick to comments on high level design at his stage.
> 
> With this series, rxqs are sorted based on cycles consumed and are 
> subsequently distributed across multiple PMDs in round robin fashion. While 
> this approach is fine, I have some concerns w.r.t QoS.  
> 
> Now OvS-DPDK don't support QoS (i.e based on L2 - pcp, L3 - dscp bits). But 
> when QoS is implemented in future based on Weighted Round Robin(WRR), Strict 
> priority(SP) (or) hybrid models(WRR and SP) the proposed approach in this RFC 
> may conflict or pose some limitations.
> 
> I am aware that this RFC doesn't factor in QoS, but would like to know your 
> thoughts and if you have some ideas to  make it less painful in future.
> 
> 1)  In WRR/SP model, the rxq(s) shall be sorted according to queue 
> credits/priorities.  PMD may have to poll rxq(s) based on this criteria?
> 2) For example, signaling/control traffic which is high priority may be 
> redirected to a specific rxq(based on pcp/dscp bits) in most of the 
> scenarios. Also the control traffic can be <= 5% of total traffic and hence 
> the cycles spent on this queue can be insignificant and these rxq(s) shall be 
> pushed to the end of the sorted list as per the proposed approach. Most 
> importantly rxq receiving the *high volume lowest priority traffic* will top 
> the list always and shall be processed first by the respective PMD threads 
> potentially tail dropping *low rate high priority traffic* during congestion.
> 
> Also adding Billy from Intel who is working QoS for OvS-DPDK.
> 
> - Bhanuprakash
> 

Thanks for your comments. I can only speculate based on the high level
description of QoS but I don't think it should be an issue. Think of
this RFC as a refinement of the default round robin queue to core
assignment.

At the moment, the rxq affinity pinning takes precedence over the
default assignment. I would expect that a user defined QoS should also
take precedence, but that there will be still be a need for code to
perform the default assignment because QoS would be optional and/or
queues may have equal priorities.

thanks,
Kevin.

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

[ovs-dev] [PATCH] tests: fix hanging test on windows

[Alin Serdean]

'multiple bridges share a controller' hangs on windows because it is
lacking the exit information (it will hang when the test has finished)

Introduce a pidfile to 'ovs-testcontroller' and end it on exit based on
the pidfile.

Signed-off-by: Alin Gabriel Serdean 
---
 tests/bridge.at | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/bridge.at b/tests/bridge.at
index cc7619d..69bd80b 100644
--- a/tests/bridge.at
+++ b/tests/bridge.at
@@ -48,7 +48,7 @@ OVS_VSWITCHD_START(
 set bridge br1 datapath-type=dummy other-config:datapath-id=1234 ])
 
 dnl Start ovs-testcontroller
-AT_CHECK([ovs-testcontroller --detach punix:controller], [0], [ignore])
+AT_CHECK([ovs-testcontroller --detach punix:controller --pidfile], [0], 
[ignore])
 OVS_WAIT_UNTIL([test -e controller])
 
 dnl Add the controller to both bridges, 5 seconds apart.
@@ -77,4 +77,5 @@ AT_CHECK([ovs-vsctl --column=status list controller | dnl
 
 OVS_APP_EXIT_AND_WAIT([ovs-vswitchd])
 OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+on_exit 'kill `cat ovs-testcontroller.pid`'
 AT_CLEANUP
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v3 3/3] build-windows: cccl fail compilation on Wimplicit-function-declaration

[Alin Serdean]

From: Alin Serdean 

Gcc compiler argument -Wall contains -Wimplicit-function-declaration which
gives warnings when a function is used before declared.
Map VStudio compiler error C4013 to it.
More info on C4013:
https://msdn.microsoft.com/en-us/library/d3ct4kz9.aspx

At the moment we cannot switch to the equivalent -Werror because we need
to solve other warnings.

As a temporary solution issue an error when this warning is triggered.
This will help development on the Windows side.

Suggested-by: Ben Pfaff 
Signed-off-by: Alin Gabriel Serdean 
---
v3: no change
v2: no change
---
 build-aux/cccl | 8 
 1 file changed, 8 insertions(+)

diff --git a/build-aux/cccl b/build-aux/cccl
index 93f9c50..e2426fb 100644
--- a/build-aux/cccl
+++ b/build-aux/cccl
@@ -144,6 +144,14 @@ EOF
 #ignore pedantic
 ;;
 
+-Wall)
+# not all warnings are implemented
+# the following is equivalent to
+# Wimplicit-function-declaration but we will issue a compiler
+# error
+clopt="$clopt ${slash}we4013"
+;;
+
 -W*)
 #ignore warnings
 ;;
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v3 1/3] windows: add definition of getpid and getcwd

[Alin Serdean]

From: Alin Serdean 

getcwd - is used in lib/util.c. getcwd is deprecated on Windows but has
_getcwd which is defined in :
https://msdn.microsoft.com/en-us/library/sf98bd4y(v=vs.120).aspx

getpid - is used in several files (i.e. lib/vlog.c). getpid
is also and deprecated and _getpid should be used:
https://msdn.microsoft.com/en-us/library/t2y34y40(v=vs.120).aspx
The problem using _getpid is that the definition is in .
A file called process.h also exists in the lib folder. This will mess up
includes.
An option would be to use a wrapper like we use for lib/string.h(.in) but
that would mean to also add it to the automake chain.
A simple solution would be to map it to GetCurrentProcessId
https://msdn.microsoft.com/en-us/library/windows/desktop/ms683180(v=vs.85).aspx

_getpid uses GetCurrentProcessId behind the scenes, casting the result
is not required.

Signed-off-by: Alin Gabriel Serdean 
Co-authored-by: Ben Pfaff 
---
v3: switch from  to 
v2: used an inline function for getpid to make it POSIX compatible as
suggested by Ben Pfaff
---
 include/windows/unistd.h | 12 
 1 file changed, 12 insertions(+)

diff --git a/include/windows/unistd.h b/include/windows/unistd.h
index 8629f7e..2e9f0ae 100644
--- a/include/windows/unistd.h
+++ b/include/windows/unistd.h
@@ -18,8 +18,11 @@
 
 #define WIN32_LEAN_AND_MEAN
 #include 
+#include 
+#include 
 
 #define fsync _commit
+#define getcwd _getcwd
 
 /* Standard file descriptors.  */
 #define STDIN_FILENO0   /* Standard input.  */
@@ -33,6 +36,15 @@
 #define _SC_NPROCESSORS_ONLN0x2
 #define _SC_PHYS_PAGES  0x4
 
+
+static __inline pid_t getpid(void)
+{
+/* Since _getpid: https://msdn.microsoft.com/en-us/library/t2y34y40.aspx
+ * uses GetCurrentProcessId behind the scenes it is safe to assume no
+ * casting is required */
+return GetCurrentProcessId();
+}
+
 __inline int GetNumLogicalProcessors(void)
 {
 SYSTEM_INFO info_temp;
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v3 2/3] windows: add includes to daemon-windows

[Alin Serdean]

From: Alin Serdean 

Add fatal-signal.h include since it uses: fatal_signal_atexit_handler
and fatal_signal_add_hook

Use the defined getpid() function and also include  since
it is defined in include/windows/unistd.h .

Signed-off-by: Alin Gabriel Serdean 
---
v3: no change
v2: change fprintf %d to %ld (Ben Pfaff)
---
 lib/daemon-windows.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/daemon-windows.c b/lib/daemon-windows.c
index 4fc97a8..55545f8 100644
--- a/lib/daemon-windows.c
+++ b/lib/daemon-windows.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014 Nicira, Inc.
+ * Copyright (c) 2014, 2017 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -20,7 +20,9 @@
 #include 
 #include 
 #include 
+#include 
 #include "dirs.h"
+#include "fatal-signal.h"
 #include "ovs-thread.h"
 #include "poll-loop.h"
 #include "openvswitch/vlog.h"
@@ -475,7 +477,7 @@ make_pidfile(void)
 
 fatal_signal_add_hook(unlink_pidfile, NULL, NULL, true);
 
-fprintf(filep_pidfile, "%d\n", _getpid());
+fprintf(filep_pidfile, "%ld\n", (long int) getpid());
 if (fflush(filep_pidfile) == EOF) {
 VLOG_FATAL("Failed to write into the pidfile %s", pidfile);
 }
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Fwd: [PATCH] bfd: Detect Multiplier configuration

[Gabor Szucs]

Hi,
hereby I'm sending an implementation of configurable BFD Detect Multiplier.

Mult value (bfd.DetectMult in RFC5880) is hard-coded and equal to 3 in
current openvswitch. As a consequence remote and local mult is the same.

In this commit the mult (Detect Multiplier/bfd.DetectMult/Detect Mult)
can be set on each interface setting the mult= in bfd Column
in Interface table of ovsdb database.
Example:
ovs-vsctl set Interface p1 bfd:mult=4
sets mult=4 on p1 interface

The modification based on RFC5880 June 2010.
The relevant paragraphs are:
4.1. Generic BFD Control Packet Format
6.8.4. Calculating the Detection Time
6.8.7. Transmitting BFD Control Packets
6.8.12. Detect Multiplier Change

The mult value is set to default 3 if it is not set in ovsdb. This
provides backward compatibility to previous openvswitch behaviour.
The RFC5880 says in 6.8.1 that DetectMult shall be a non-zero integer.
In RFC5880 4.1. "Detect Mult" has 8 bit length and is declared
as a 8 bit unsigned integer in bfd.c.
Consequently mult value shall be greater than 0 and less then 256.
In case of incorrect mult value is given in ovsdb the default value (3)
will be set and a message is logged into ovs-vswitchd.log on that.
Local or remote mult value change is also logged into ovs-vswitchd.log.

Since remote and local mult is not the same calculation of detect time
has been changed. Due to RFC5880 6.8.4 Detection Time is calculated using
mult value of the remote system.
Detection time is recalculated due to remote mult change.

The BFD packet transmission jitter is different in case of mult=1
due to RFC5880 6.8.7. The maximum interval of the transmitted bfd packet
is 90% of the transmission interval.

The value of remote mult is printed in the last line of the output of
ovs-appctl bfd/show command with label: Remote Detect Mult.

There is a feature in openvswitch connected with forwarding_if_rx that
is not the part of RFC5880. This feature also uses mult value but it is
not specified if local or remote since it was the
same in original code. The relevant description in code:
   /* When 'bfd->forwarding_if_rx' is set, at least one bfd control packet
 * is required to be received every 100 * bfd->cfg_min_rx.  If bfd
 * control packet is not received within this interval, even if data
 * packets are received, the bfd->forwarding will still be false. */

Due to lack of specification local mult value is used for calculation of
forwarding_if_rx_detect_time. This detect time is recalculated at mult
change if forwarding_if_rx is true and bfd is in UP state.

A new unit test has been added: "bfd - Edit the Detect Mult values"
The following cases are tested:
- Without setting mult the mult will be the default value (3).
- The setting of the lowest (1) and highest (255) valid mult value
  and the detection of remote mult value.
- The setting of out of range mult value (0, 256) in ovsdb results
  sets default value in ovs-vswitchd
- Clearing non default mult value from ovsdb results sets default
  value in ovs-vswitchd.

Signed-off-by: Gábor Szűcs 
---
diff --git a/lib/bfd.c b/lib/bfd.c
index 383be20..cc3adc7 100644
--- a/lib/bfd.c
+++ b/lib/bfd.c
@@ -146,6 +146,8 @@ BUILD_ASSERT_DECL(BFD_PACKET_LEN == sizeof(struct msg));
 #define VERS_SHIFT 5
 #define STATE_MASK 0xC0
 #define FLAGS_MASK 0x3f
+#define DEFAULT_MULT 3
+

 struct bfd {
 struct hmap_node node;/* In 'all_bfds'. */
@@ -155,6 +157,7 @@ struct bfd {

 bool cpath_down;  /* Concatenated Path Down. */
 uint8_t mult; /* bfd.DetectMult. */
+uint8_t rmt_mult; /* Remote bfd.DetectMult. */

 struct netdev *netdev;
 uint64_t rx_packets;  /* Packets received by 'netdev'. */
@@ -354,6 +357,8 @@ bfd_configure(struct bfd *bfd, const char *name, const 
struct smap *cfg,
 bool need_poll = false;
 bool cfg_min_rx_changed = false;
 bool cpath_down, forwarding_if_rx;
+int new_mult;
+int old_mult;

 if (!cfg || !smap_get_bool(cfg, "enable", false)) {
 bfd_unref(bfd);
@@ -370,7 +375,8 @@ bfd_configure(struct bfd *bfd, const char *name, const 
struct smap *cfg,

 bfd->diag = DIAG_NONE;
 bfd->min_tx = 1000;
-bfd->mult = 3;
+bfd->rmt_mult = 0;
+bfd->mult = DEFAULT_MULT;
 ovs_refcount_init(>ref_cnt);
 bfd->netdev = netdev_ref(netdev);
 bfd->rx_packets = bfd_rx_packets(bfd);
@@ -389,6 +395,21 @@ bfd_configure(struct bfd *bfd, const char *name, const 
struct smap *cfg,
 bfd_status_changed(bfd);
 }

+old_mult = bfd->mult;
+
+new_mult = smap_get_int(cfg, "mult", DEFAULT_MULT);
+if (new_mult < 1 || new_mult > 255) {
+VLOG_INFO("Interface %s mult value %d out of range 1-255 changed to 
%d",
+   name, new_mult, DEFAULT_MULT);
+new_mult = DEFAULT_MULT;
+}
+
+bfd->mult = (uint8_t) new_mult;
+if (new_mult != old_mult) {
+

[ovs-dev] [PATCH] ovn-northd: Fix ping failure of vlan networks.

[wang . qianyu]

There are two computer node, each have one vm. And the two vms in 
indifferent vlan networks. The ping between the vms is not success.

The reason is that, acl of to-localnet port or from-localnet port is
signed to contrack. So the pair of icmp request and reply have different
zone id in one ovs node. This makes the ct state not correct.

This patch do the modification that localnet port do not use ct.

Signed-off-by: wangqianyu 
---
 ovn/northd/ovn-northd.c | 22 +-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 83db753..5d1587e 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -1,4 +1,4 @@
-/*
+/*
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at:
@@ -416,6 +416,7 @@ struct ovn_datapath {
 /* The "derived" OVN port representing the instance of l3dgw_port on
  * the "redirect-chassis". */
 struct ovn_port *l3redirect_port;
+struct ovn_port *localnet_port;
 };
 
 struct macam_node {
@@ -1351,6 +1352,10 @@ join_logical_ports(struct northd_context *ctx,
 ovs_list_push_back(nb_only, >list);
 }
 
+if(!strcmp(nbsp->type, "localnet")) {
+   od->localnet_port = op;
+}
+
 op->lsp_addrs
 = xmalloc(sizeof *op->lsp_addrs * nbsp->n_addresses);
 for (size_t j = 0; j < nbsp->n_addresses; j++) {
@@ -2629,6 +2634,21 @@ build_pre_acls(struct ovn_datapath *od, struct hmap 
*lflows)
 ds_destroy(_in);
 ds_destroy(_out);
 }
+if(od->localnet_port) {
+struct ds match_in = DS_EMPTY_INITIALIZER;
+struct ds match_out = DS_EMPTY_INITIALIZER;
+
+ds_put_format(_in, "ip && inport == %s", 
od->localnet_port->json_key);
+ds_put_format(_out, "ip && outport == %s", 
od->localnet_port->json_key);
+ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110,
+  ds_cstr(_in), "next;");
+ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110,
+  ds_cstr(_out), "next;");
+
+ds_destroy(_in);
+ds_destroy(_out);
+}
+
 /* Ingress and Egress Pre-ACL Table (Priority 110).
  *
  * Not to do conntrack on ND packets. */
-- 
2.7.2.windows.1



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

[ovs-dev] Viable partnership request.

[Mr Albert Yang]

Compliment of the day,


I have access to very vital information that can be used to move a huge amount 
of money. 
I have done my homework very well and i have the machineries in place to get it 
done. 
Ultimately I need an honest person to play an important role in the completion 
of this business transaction.


Regards,
Albert.

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

Re: [ovs-dev] [PATCH] Supporting ovn-northd service HA depend on OVNDB-HA

[Numan Siddique]

On Fri, May 19, 2017 at 7:21 AM, 高正伟  wrote:

> Thanks Numan
>
> I think that supporting  OVNDB and ovn-northd HA  depend on one pacemaker
> is an idea,   In case of thinking OVN-HA,  user will not individually care
> about OVNDB or ovn-northd HA.
>
>
Hi Zhangwei,

I do agree with you. But what I am trying to say is to make it configurable.

I tested with your patch again with tripleo and it is having few issues.
1. Since you are not passing "--ovn-manage-ovsdb=no", when "ovn-ctl
stop_northd" is called, it also stops ovsdb-servers. We probably don't want
to stop ovsdb-servers unless, pacemaker calls "stop" action.

2. When a master node is demoted, ovn-northd is not stopped since "ovn-ctl
stop_northd" code is not hit.

I tested with the below modifications and applying this patch -
https://patchwork.ozlabs.org/patch/765224/. It is working as expected. Can
you please test on your side and resubmit another version if it is fine
with you.

If the user wants to manage ovn_northd using the ocf resources, he could
pass manage_northd=yes while creating the resource.


---cut here -

diff --git a/ovn/utilities/ovndb-servers.ocf
b/ovn/utilities/ovndb-servers.ocf
index 40c5541..018d904 100755
--- a/ovn/utilities/ovndb-servers.ocf
+++ b/ovn/utilities/ovndb-servers.ocf
@@ -7,6 +7,7 @@
 : ${NB_MASTER_PROTO_DEFAULT="tcp"}
 : ${SB_MASTER_PORT_DEFAULT="6642"}
 : ${SB_MASTER_PROTO_DEFAULT="tcp"}
+: ${MANAGE_NORTHD_DEFAULT="no"}
 CRM_MASTER="${HA_SBIN_DIR}/crm_master -l reboot"
 CRM_ATTR_REPL_INFO="${HA_SBIN_DIR}/crm_attribute --type crm_config --name
OVN_REPL_INFO -s ovn_ovsdb_master_server"
 OVN_CTL=${OCF_RESKEY_ovn_ctl:-${OVN_CTL_DEFAULT}}
@@ -15,6 +16,7 @@
NB_MASTER_PORT=${OCF_RESKEY_nb_master_port:-${NB_MASTER_PORT_DEFAULT}}
 NB_MASTER_PROTO=${OCF_RESKEY_nb_master_protocol:-${NB_MASTER_PROTO_DEFAULT}}
 SB_MASTER_PORT=${OCF_RESKEY_sb_master_port:-${SB_MASTER_PORT_DEFAULT}}
 SB_MASTER_PROTO=${OCF_RESKEY_sb_master_protocol:-${SB_MASTER_PROTO_DEFAULT}}
+MANAGE_NORTHD=${OCF_RESKEY_manage_northd:-${MANAGE_NORTHD_DEFAULT}}

 # Invalid IP address is an address that can never exist in the network, as
 # mentioned in rfc-5737. The ovsdb servers connects to this IP address till
@@ -90,6 +92,14 @@ ovsdb_server_metadata() {
   
   

+  
+  
+  If set to yes, manages ovn-northd service. ovn-northd will be started in
+  the master node.
+  
+  manage ovn-northd service
+  
+  
   

   
@@ -122,12 +132,17 @@ ovsdb_server_notify() {
 # the right thing at startup
 ocf_log debug "ovndb_server: $host_name is the master"
 ${CRM_ATTR_REPL_INFO} -v "$host_name"
-# Startup ovn-northd service
-${OVN_CTL} start_northd
+if [ "$MANAGE_NORTHD" = "yes" ]; then
+# Startup ovn-northd service
+${OVN_CTL} --ovn-manage-ovsdb=no start_northd
+fi

 else
-# Stop ovn-northd service
-${OVN_CTL} stop_northd
+if [ "$MANAGE_NORTHD" = "yes" ]; then
+# Stop ovn-northd service. Set --ovn-manage-ovsdb=no so that
+# ovn-ctl doesn't stop ovsdb-servers.
+${OVN_CTL} --ovn-manage-ovsdb=no stop_northd
+fi
 # Synchronize with the new master
 ocf_log debug "ovndb_server: Connecting to the new master
${OCF_RESKEY_CRM_meta_notify_promote_uname}"
 ${OVN_CTL} demote_ovnnb --db-nb-sync-from-addr=${MASTER_IP} \
@@ -289,6 +304,13 @@ ovsdb_server_start() {
 }

 ovsdb_server_stop() {
+if [ "$MANAGE_NORTHD" = "yes" ]; then
+# Stop ovn-northd service in case it was running. This is required
+# when the master is demoted. For other cases, it would be a no-op.
+# Set --ovn-manage-ovsdb=no so that ovn-ctl doesn't stop
ovsdb-servers.
+${OVN_CTL} --ovn-manage-ovsdb=no stop_northd
+fi
+
 ovsdb_server_check_status
 case $? in
 $OCF_NOT_RUNNING)return ${OCF_SUCCESS};;

--


Thanks
Numan





> Thanks.
>
>
> 在 2017-05-18 20:53:22，"Numan Siddique"  写道：
>
>
>
> On Wed, May 17, 2017 at 9:15 PM, Ben Pfaff  wrote:
>
>> From: Zhengwei Gao 
>>
>> As ovn-northd servcie  parse network element between ovnnb_db and
>> ovnsb_db, it ensures ovn-northd service connecting to ovnnb_db and
>> ovnsb_db. OVNDB-HA was supported with pacemaker, ovn-northd service
>> will be failover fllowing OVNDB-HA.
>> ---
>> This was posted as a github pull request:
>> https://github.com/openvswitch/ovs/pull/176
>>
>>  ovn/utilities/ovndb-servers.ocf | 4 
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/ovn/utilities/ovndb-servers.ocf
>> b/ovn/utilities/ovndb-servers.ocf
>> index 908cb3c17d84..40c55411e828 100755
>> --- a/ovn/utilities/ovndb-servers.ocf
>> +++ b/ovn/utilities/ovndb-servers.ocf
>> @@ -122,8 +122,12 @@ ovsdb_server_notify() {
>>  # the right thing at startup
>>