[ovs-dev] 答复: 答复: [bug] ovs crash when call xlate_normal_flood

2018-08-02 Thread Linhaifeng
Sorry . send wrong message. Pls igore.

The problem is ousr code had 'use-after-free'

-邮件原件-
发件人: Linhaifeng 
发送时间: 2018年8月3日 13:55
收件人: 'Ben Pfaff' 
抄送: d...@openvswitch.org
主题: 答复: [ovs-dev] 答复: [bug] ovs crash when call xlate_normal_flood

Some PMD add to ovs dpdk aslo have linux if interface . Linux if interface 
would notice ovs to reconfig and would fail too when failed to add.

-邮件原件-
发件人: Ben Pfaff [mailto:b...@ovn.org]
发送时间: 2018年7月11日 5:08
收件人: Linhaifeng 
抄送: d...@openvswitch.org
主题: Re: [ovs-dev] 答复: [bug] ovs crash when call xlate_normal_flood

Hmm, that's interesting.  What was the problem, then?

On Fri, Jun 29, 2018 at 01:14:38AM +, Linhaifeng wrote:
> Sorry.This is not ovs‘s bug.
> 
> 发件人: Linhaifeng
> 发送时间: 2018年6月21日 13:54
> 收件人: 'd...@openvswitch.org' 
> 主题: 答复: [bug] ovs crash when call xlate_normal_flood
> 
> Or stop upcall thread before xlate_remove_ofproto in destruct.
> 
> 发件人: Linhaifeng
> 发送时间: 2018年6月21日 13:49
> 收件人: 'd...@openvswitch.org' 
> mailto:d...@openvswitch.org>>
> 主题: [bug] ovs crash when call xlate_normal_flood
> 
> Should we use rwlock to use xbridge->xbundles? xbridge->xbundles may write in 
> main thread and read in upcall thread.
> 
> The crash stack as follow:
> #5  0x004a04e8 in xlate_normal_flood (ctx=ctx@entry=0x7faeee7b6f30, 
> in_xbundle=in_xbundle@entry=0x553d950, vlan=vlan@entry=0) at 
> ofproto/ofproto_dpif_xlate.c:2509
> 2509   in ofproto/ofproto_dpif_xlate.c
> (gdb) info locals
> xbundle = 0xffe8
> (gdb) p xbundle->ofbundle
> Cannot access memory at address 0xfff8
> (gdb) p in_xbundle->ofbundle
> $5 = (struct ofbundle *) 0x0
> (gdb) p in_xbundle
> $6 = (struct xbundle *) 0x553d950
> (gdb) p *in_xbundle
> $7 = {hmap_node = {hash = 0, next = 0x0}, ofbundle = 0x0, list_node = {prev = 
> 0x0, next = 0x0}, xbridge = 0x0, xports = {prev = 0x0, next = 0x0}, name = 
> 0x0, bond = 0x0, lacp = 0x0, vlan_mode = PORT_VLAN_ACCESS, vlan = 0,
>   trunks = 0x0, use_priority_tags = false, floodable = false, 
> protected = false, external_id = 0}
> 
> (gdb) bt
> #0  0x7fb05ea85197 in raise () from /usr/lib64/libc.so.6
> #1  0x7fb05ea86888 in abort () from /usr/lib64/libc.so.6
> #2  0x0065e1e9 in PAT_abort ()
> #3  0x0065b32d in patchIllInsHandler ()
> #4  
> #5  0x004a04e8 in xlate_normal_flood 
> (ctx=ctx@entry=0x7faeee7b6f30, in_xbundle=in_xbundle@entry=0x553d950, 
> vlan=vlan@entry=0) at ofproto/ofproto_dpif_xlate.c:2509
> #6  0x004a0e6e in xlate_normal (ctx=0x7faeee7b6f30) at 
> ofproto/ofproto_dpif_xlate.c:2736
> #7  xlate_output_action (ctx=ctx@entry=0x7faeee7b6f30, port= out>, max_len=, may_packet_in=may_packet_in@entry=true) 
> at ofproto/ofproto_dpif_xlate.c:4312
> #8  0x0049d240 in do_xlate_actions (ofpacts=, 
> ofpacts_len=, ctx=ctx@entry=0x7faeee7b6f30) at 
> ofproto/ofproto_dpif_xlate.c:5262
> #9  0x0049e410 in xlate_recursively (deepens=true, 
> rule=0x6a4be00, ctx=0x7faeee7b6f30) at 
> ofproto/ofproto_dpif_xlate.c:3587
> #10 xlate_table_action (ctx=0x7faeee7b6f30, in_port=, 
> table_id=, may_packet_in=, 
> honor_table_miss=) at ofproto/ofproto_dpif_xlate.c:3654
> #11 0x0049faf7 in compose_output_action__ 
> (ctx=ctx@entry=0x7faeee7b6f30, ofp_port=, xr= out>, check_stp=check_stp@entry=true) at 
> ofproto/ofproto_dpif_xlate.c:3317
> #12 0x004a0126 in compose_output_action (xr=, 
> ofp_port=, ctx=0x7faeee7b6f30) at 
> ofproto/ofproto_dpif_xlate.c:3567
> #13 output_normal (ctx=ctx@entry=0x7faeee7b6f30, 
> out_xbundle=out_xbundle@entry=0x3005770, vlan=vlan@entry=0) at 
> ofproto/ofproto_dpif_xlate.c:2113
> #14 0x004a0511 in xlate_normal_flood 
> (ctx=ctx@entry=0x7faeee7b6f30, in_xbundle=in_xbundle@entry=0x3008a40, 
> vlan=vlan@entry=0) at ofproto/ofproto_dpif_xlate.c:2517
> #15 0x004a0e6e in xlate_normal (ctx=0x7faeee7b6f30) at 
> ofproto/ofproto_dpif_xlate.c:2736
> #16 xlate_output_action (ctx=ctx@entry=0x7faeee7b6f30, port= out>, max_len=, may_packet_in=may_packet_in@entry=true) 
> at ofproto/ofproto_dpif_xlate.c:4312
> #17 0x0049d240 in do_xlate_actions (ofpacts=, 
> ofpacts_len=, ctx=ctx@entry=0x7faeee7b6f30) at 
> ofproto/ofproto_dpif_xlate.c:5262
> #18 0x0049e410 in xlate_recursively (deepens=true, 
> rule=0x6a32500, ctx=0x7faeee7b6f30) at 
> ofproto/ofproto_dpif_xlate.c:3587
> #19 xlate_table_action (ctx=0x7faeee7b6f30, in_port=, 
> table_id=, may_packet_in=, 
> honor_table_miss=) at ofproto/ofproto_dpif_xlate.c:3654
> #20 0x0049faf7 in compose_output_action__ 
> (ctx=ctx@entry=0x7faeee7b6f30, ofp_port=, xr= out>, check_stp=check_stp@entry=true) at 
> ofproto/ofproto_dpif_xlate.c:3317
> #21 0x004a0126 in compose_output_action (xr=, 
> ofp_port=, ctx=0x7faeee7b6f30) at 
> ofproto/ofproto_dpif_xlate.c:3567
> #22 output_normal (ctx=ctx@entry=0x7faeee7b6f30, 
> out_xbundle=out_xbundle@entry=0x93e3e10, vlan=vlan@entry=0) at 
> ofproto/ofproto_dpif_xlate.c:2113
> #23 0x004a0511 

[ovs-dev] 答复: 答复: [bug] ovs crash when call xlate_normal_flood

2018-08-02 Thread Linhaifeng
Some PMD add to ovs dpdk aslo have linux if interface . Linux if interface 
would notice ovs
to reconfig and would fail too when failed to add.

-邮件原件-
发件人: Ben Pfaff [mailto:b...@ovn.org] 
发送时间: 2018年7月11日 5:08
收件人: Linhaifeng 
抄送: d...@openvswitch.org
主题: Re: [ovs-dev] 答复: [bug] ovs crash when call xlate_normal_flood

Hmm, that's interesting.  What was the problem, then?

On Fri, Jun 29, 2018 at 01:14:38AM +, Linhaifeng wrote:
> Sorry.This is not ovs‘s bug.
> 
> 发件人: Linhaifeng
> 发送时间: 2018年6月21日 13:54
> 收件人: 'd...@openvswitch.org' 
> 主题: 答复: [bug] ovs crash when call xlate_normal_flood
> 
> Or stop upcall thread before xlate_remove_ofproto in destruct.
> 
> 发件人: Linhaifeng
> 发送时间: 2018年6月21日 13:49
> 收件人: 'd...@openvswitch.org' 
> mailto:d...@openvswitch.org>>
> 主题: [bug] ovs crash when call xlate_normal_flood
> 
> Should we use rwlock to use xbridge->xbundles? xbridge->xbundles may write in 
> main thread and read in upcall thread.
> 
> The crash stack as follow:
> #5  0x004a04e8 in xlate_normal_flood (ctx=ctx@entry=0x7faeee7b6f30, 
> in_xbundle=in_xbundle@entry=0x553d950, vlan=vlan@entry=0) at 
> ofproto/ofproto_dpif_xlate.c:2509
> 2509   in ofproto/ofproto_dpif_xlate.c
> (gdb) info locals
> xbundle = 0xffe8
> (gdb) p xbundle->ofbundle
> Cannot access memory at address 0xfff8
> (gdb) p in_xbundle->ofbundle
> $5 = (struct ofbundle *) 0x0
> (gdb) p in_xbundle
> $6 = (struct xbundle *) 0x553d950
> (gdb) p *in_xbundle
> $7 = {hmap_node = {hash = 0, next = 0x0}, ofbundle = 0x0, list_node = {prev = 
> 0x0, next = 0x0}, xbridge = 0x0, xports = {prev = 0x0, next = 0x0}, name = 
> 0x0, bond = 0x0, lacp = 0x0, vlan_mode = PORT_VLAN_ACCESS, vlan = 0,
>   trunks = 0x0, use_priority_tags = false, floodable = false, protected = 
> false, external_id = 0}
> 
> (gdb) bt
> #0  0x7fb05ea85197 in raise () from /usr/lib64/libc.so.6
> #1  0x7fb05ea86888 in abort () from /usr/lib64/libc.so.6
> #2  0x0065e1e9 in PAT_abort ()
> #3  0x0065b32d in patchIllInsHandler ()
> #4  
> #5  0x004a04e8 in xlate_normal_flood (ctx=ctx@entry=0x7faeee7b6f30, 
> in_xbundle=in_xbundle@entry=0x553d950, vlan=vlan@entry=0) at 
> ofproto/ofproto_dpif_xlate.c:2509
> #6  0x004a0e6e in xlate_normal (ctx=0x7faeee7b6f30) at 
> ofproto/ofproto_dpif_xlate.c:2736
> #7  xlate_output_action (ctx=ctx@entry=0x7faeee7b6f30, port=, 
> max_len=, may_packet_in=may_packet_in@entry=true) at 
> ofproto/ofproto_dpif_xlate.c:4312
> #8  0x0049d240 in do_xlate_actions (ofpacts=, 
> ofpacts_len=, ctx=ctx@entry=0x7faeee7b6f30) at 
> ofproto/ofproto_dpif_xlate.c:5262
> #9  0x0049e410 in xlate_recursively (deepens=true, rule=0x6a4be00, 
> ctx=0x7faeee7b6f30) at ofproto/ofproto_dpif_xlate.c:3587
> #10 xlate_table_action (ctx=0x7faeee7b6f30, in_port=, 
> table_id=, may_packet_in=, 
> honor_table_miss=) at ofproto/ofproto_dpif_xlate.c:3654
> #11 0x0049faf7 in compose_output_action__ 
> (ctx=ctx@entry=0x7faeee7b6f30, ofp_port=, xr=, 
> check_stp=check_stp@entry=true) at ofproto/ofproto_dpif_xlate.c:3317
> #12 0x004a0126 in compose_output_action (xr=, 
> ofp_port=, ctx=0x7faeee7b6f30) at 
> ofproto/ofproto_dpif_xlate.c:3567
> #13 output_normal (ctx=ctx@entry=0x7faeee7b6f30, 
> out_xbundle=out_xbundle@entry=0x3005770, vlan=vlan@entry=0) at 
> ofproto/ofproto_dpif_xlate.c:2113
> #14 0x004a0511 in xlate_normal_flood (ctx=ctx@entry=0x7faeee7b6f30, 
> in_xbundle=in_xbundle@entry=0x3008a40, vlan=vlan@entry=0) at 
> ofproto/ofproto_dpif_xlate.c:2517
> #15 0x004a0e6e in xlate_normal (ctx=0x7faeee7b6f30) at 
> ofproto/ofproto_dpif_xlate.c:2736
> #16 xlate_output_action (ctx=ctx@entry=0x7faeee7b6f30, port=, 
> max_len=, may_packet_in=may_packet_in@entry=true) at 
> ofproto/ofproto_dpif_xlate.c:4312
> #17 0x0049d240 in do_xlate_actions (ofpacts=, 
> ofpacts_len=, ctx=ctx@entry=0x7faeee7b6f30) at 
> ofproto/ofproto_dpif_xlate.c:5262
> #18 0x0049e410 in xlate_recursively (deepens=true, rule=0x6a32500, 
> ctx=0x7faeee7b6f30) at ofproto/ofproto_dpif_xlate.c:3587
> #19 xlate_table_action (ctx=0x7faeee7b6f30, in_port=, 
> table_id=, may_packet_in=, 
> honor_table_miss=) at ofproto/ofproto_dpif_xlate.c:3654
> #20 0x0049faf7 in compose_output_action__ 
> (ctx=ctx@entry=0x7faeee7b6f30, ofp_port=, xr=, 
> check_stp=check_stp@entry=true) at ofproto/ofproto_dpif_xlate.c:3317
> #21 0x004a0126 in compose_output_action (xr=, 
> ofp_port=, ctx=0x7faeee7b6f30) at 
> ofproto/ofproto_dpif_xlate.c:3567
> #22 output_normal (ctx=ctx@entry=0x7faeee7b6f30, 
> out_xbundle=out_xbundle@entry=0x93e3e10, vlan=vlan@entry=0) at 
> ofproto/ofproto_dpif_xlate.c:2113
> #23 0x004a0511 in xlate_normal_flood (ctx=ctx@entry=0x7faeee7b6f30, 
> in_xbundle=in_xbundle@entry=0x894de90, vlan=vlan@entry=0) at 
> ofproto/ofproto_dpif_xlate.c:2517
> #24 0x004a0e6e in xlate_normal (ctx=0x7faeee7b6f30) at 
> ofproto/ofproto_dpif_xlate.c:2736
> #25 

[ovs-dev] [patch v2] dpctl: Simplify dpctl_flush_conntrack.

2018-08-02 Thread Darrell Ball
The function dpctl_flush_conntrack() and other such functions with
multiple optional arguments can be simplified by introducing a new
function to check whether a valid datapath name is supplied as an
argument to the functions.

opt_dpif_open() can also make use of this new function to allow it
to handle callers with multiple optional arguments.

Signed-off-by: Darrell Ball 
---

v1->v2: Cleanup only.

 lib/dpctl.c | 62 +
 tests/system-traffic.at |  8 +++
 2 files changed, 36 insertions(+), 34 deletions(-)

diff --git a/lib/dpctl.c b/lib/dpctl.c
index 4f1e443..93ca3d2 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -187,18 +187,39 @@ parsed_dpif_open(const char *arg_, bool create, struct 
dpif **dpifp)
 return result;
 }
 
+static bool
+check_for_dpif_arg(int argc, const char *argv[])
+{
+if (argc > 1) {
+struct dpif *dpif;
+int error = parsed_dpif_open(argv[1], false, );
+if (!error) {
+dpif_close(dpif);
+return true;
+}
+}
+
+return false;
+}
+
 /* Open a dpif with an optional name argument.
  *
- * The datapath name is not a mandatory parameter for this command.  If
- * it is not specified -- so 'argc' < 'max_args' -- we retrieve it from
- * the current setup, assuming only one exists.  On success stores the
- * opened dpif in '*dpifp'. */
+ * The datapath name is not a mandatory parameter for this command.  If it is
+ * not specified, we retrieve it from the current setup, assuming only one
+ * exists.  On success stores the opened dpif in '*dpifp'. */
 static int
 opt_dpif_open(int argc, const char *argv[], struct dpctl_params *dpctl_p,
   uint8_t max_args, struct dpif **dpifp)
 {
+char *dpname = NULL;
 int error = 0;
-char *dpname = argc >= max_args ? xstrdup(argv[1]) : get_one_dp(dpctl_p);
+
+if (check_for_dpif_arg(argc, argv)) {
+dpname = xstrdup(argv[1]);
+} else if (argc != max_args) {
+dpname = get_one_dp(dpctl_p);
+}
+
 if (!dpname) {
 error = EINVAL;
 dpctl_error(dpctl_p, error, "datapath not found");
@@ -1313,34 +1334,15 @@ dpctl_flush_conntrack(int argc, const char *argv[],
 struct ct_dpif_tuple tuple, *ptuple = NULL;
 struct ds ds = DS_EMPTY_INITIALIZER;
 uint16_t zone, *pzone = NULL;
-char *name;
 int error, i = 1;
-bool got_dpif = false;
 
-/* Parse datapath name. It is not a mandatory parameter for this command.
- * If it is not specified, we retrieve it from the current setup,
- * assuming only one exists. */
-if (argc >= 2) {
-error = parsed_dpif_open(argv[i], false, );
-if (!error) {
-got_dpif = true;
-i++;
-} else if (argc == 4) {
-dpctl_error(dpctl_p, error, "invalid datapath");
-return error;
-}
+if (check_for_dpif_arg(argc, argv)) {
+i++;
 }
-if (!got_dpif) {
-name = get_one_dp(dpctl_p);
-if (!name) {
-return EINVAL;
-}
-error = parsed_dpif_open(name, false, );
-free(name);
-if (error) {
-dpctl_error(dpctl_p, error, "opening datapath");
-return error;
-}
+
+error = opt_dpif_open(argc, argv, dpctl_p, 4, );
+if (error) {
+return error;
 }
 
 /* Parse zone */
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index cbd9542..f53e51b 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -1124,17 +1124,17 @@ ovs-appctl: ovs-vswitchd: server returned an error
 ])
 
 AT_CHECK([ovs-appctl dpctl/ct-set-maxconns one-bad-dp 10], [2], [], [dnl
-ovs-vswitchd: opening datapath (Address family not supported by protocol)
+ovs-vswitchd: datapath not found (Invalid argument)
 ovs-appctl: ovs-vswitchd: server returned an error
 ])
 
 AT_CHECK([ovs-appctl dpctl/ct-get-maxconns one-bad-dp], [2], [], [dnl
-ovs-vswitchd: opening datapath (Address family not supported by protocol)
+ovs-vswitchd: datapath not found (Invalid argument)
 ovs-appctl: ovs-vswitchd: server returned an error
 ])
 
 AT_CHECK([ovs-appctl dpctl/ct-get-nconns one-bad-dp], [2], [], [dnl
-ovs-vswitchd: opening datapath (Address family not supported by protocol)
+ovs-vswitchd: datapath not found (Invalid argument)
 ovs-appctl: ovs-vswitchd: server returned an error
 ])
 
@@ -1164,7 +1164,7 @@ AT_CHECK([ovs-appctl dpctl/ct-get-maxconns], [], [dnl
 10
 ])
 
-OVS_TRAFFIC_VSWITCHD_STOP(["/could not create datapath one-bad-dp of unknown 
type system/d"])
+OVS_TRAFFIC_VSWITCHD_STOP(["/could not create datapath.*/d"])
 AT_CLEANUP
 
 AT_SETUP([conntrack - IPv6 ping])
-- 
1.9.1

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


Re: [ovs-dev] ofproto-dpif: Add logging when sending out an in_port

2018-08-02 Thread 0-day Robot
Bleep bloop.  Greetings Zak Whittington, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

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


git-am:
fatal: patch fragment without header at line 7: @@ -5092,6 +5094,8 @@ 
xlate_output_trunc_action(struct xlate_ctx *ctx,
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 ofproto-dpif: Add logging when sending out an in_port
The copy of the patch that failed is found in:
   
/var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch
When you have resolved this problem, run "git am --resolved".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


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

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


[ovs-dev] Problem in using ovs in raspberry pi - regd.

2018-08-02 Thread Viswanath Muthukumaraswamy Sathananth
Hi,

I am trying to implement a real test bed to check the interaction between 
switches and controllers. Raspberry Pi 3b+ act as switches with OVS version 
2.5.5 running on them. Two such  Raspberry Pis connected to a RYU controller 
running on Windows 10 Machine wirelessly. One Raspberry Pi which is a simple 
host is attached to every switch. When I try to ping from one host to another I 
get a few problems.

1. The host raspberry pi is treated as LOCAL. And the flow entries have in_port 
or out_port as LOCAL.
2. RYU controller is not able to indetify any links.
3. Ping request doesn't echo back.

What can be the possible errors in this case. Kindly suggest us a solution.

Thanks,

Viz

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


[ovs-dev] [PATCH] ofproto-dpif: Add logging when sending out an in_port

2018-08-02 Thread Zak Whittington
This patch adds a log message to warn when sending out an in_port.

VMware-BZ: #2158607
Signed-off-by: Zak Whittington 
---
 ofproto/ofproto-dpif-xlate.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 01f1faf..93fbee5 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -5007,6 +5007,8 @@ xlate_output_action(struct xlate_ctx *ctx, ofp_port_t
port,
 if (port != ctx->xin->flow.in_port.ofp_port) {
 compose_output_action(ctx, port, NULL, is_last_action,
truncate);
 } else {
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+VLOG_INFO_RL(, "skipping output to input port");
 xlate_report(ctx, OFT_WARN, "skipping output to input port");
 }
 break;
@@ -5092,6 +5094,8 @@ xlate_output_trunc_action(struct xlate_ctx *ctx,
 ctx->xout->slow |= SLOW_ACTION;
 }
 } else {
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+VLOG_INFO_RL(, "skipping output to input port");
 xlate_report(ctx, OFT_WARN, "skipping output to input port");
 }
 break;
-- 
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] json: Use unnamed embedded union.

2018-08-02 Thread Flavio Leitner
On Thu, Aug 02, 2018 at 06:44:10PM -0300, Flavio Leitner wrote:
> Otherwise the code does not build.
> 
> Fixes: fa37affad362 ("Embrace anonymous unions.")
> Signed-off-by: Flavio Leitner 

This needs to go in branch-2.10 as well.

-- 
Flavio

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


[ovs-dev] [PATCH] json: Use unnamed embedded union.

2018-08-02 Thread Flavio Leitner
Otherwise the code does not build.

Fixes: fa37affad362 ("Embrace anonymous unions.")
Signed-off-by: Flavio Leitner 
---
 python/ovs/_json.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/python/ovs/_json.c b/python/ovs/_json.c
index 7067ce26a..8b8402025 100644
--- a/python/ovs/_json.c
+++ b/python/ovs/_json.c
@@ -106,7 +106,7 @@ json_to_python(struct json *json)
 if (dict == NULL) {
 return PyErr_NoMemory();
 }
-SHASH_FOR_EACH(node, json->u.object) {
+SHASH_FOR_EACH (node, json->object) {
 PyObject *key = PyUnicode_FromString(node->name);
 PyObject *val = json_to_python(node->data);
 
@@ -124,13 +124,13 @@ json_to_python(struct json *json)
 }
 case JSON_ARRAY:{
 int i;
-PyObject *arr = PyList_New(json->u.array.n);
+PyObject *arr = PyList_New(json->array.n);
 
 if (arr == NULL) {
 return PyErr_NoMemory();
 }
-for (i = 0; i < json->u.array.n; i++) {
-PyObject *item = json_to_python(json->u.array.elems[i]);
+for (i = 0; i < json->array.n; i++) {
+PyObject *item = json_to_python(json->array.elems[i]);
 
 if (!item || PyList_SetItem(arr, i, item)) {
 Py_XDECREF(arr);
@@ -140,18 +140,18 @@ json_to_python(struct json *json)
 return arr;
 }
 case JSON_REAL:
-if (json->u.real != 0) {
-return PyFloat_FromDouble(json->u.real);
+if (json->real != 0) {
+return PyFloat_FromDouble(json->real);
 } /* fall through to treat 0 as int */
 case JSON_INTEGER:
 #ifdef IS_PY3K
-return PyLong_FromLong((long) json->u.integer);
+return PyLong_FromLong((long) json->integer);
 #else
-return PyInt_FromLong((long) json->u.integer);
+return PyInt_FromLong((long) json->integer);
 #endif
 
 case JSON_STRING:
-return PyUnicode_FromString(json->u.string);
+return PyUnicode_FromString(json->string);
 default:
 return NULL;
 }
-- 
2.14.4


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


Re: [ovs-dev] [PATCH 10/11] dpctl: Implement dpctl commands for conntrack per zone limit

2018-08-02 Thread Yi-Hung Wei
On Wed, Aug 1, 2018 at 10:37 PM, Darrell Ball  wrote:
> On Wed, Aug 1, 2018 at 3:46 PM, Yi-Hung Wei  wrote:
>
> Do these commands need a ‘zone’ keyword
>
> eg) ct-set-zone-limits
>
>>
>> $ ovs-appctl dpctl/ct-set-limits default=10 zone=0,limit=5 zone=1,limit=3

I think I am fine either way. I can change ct-set-zone-limits to
ct-set-limits in later version.

>
> I wonder if it makes sense to write
>
> ‘zone=default,limit=10’
>
> so that ‘default’ is treated like any zone?
That makes sense, and I think it does simplify the syntax.  Will make
this change in v3.

>>
>> $ ovs-appctl dpct/ct-del-limits zone=0
>
>
>
> I wonder if we set zone limit to zero (unlimited), it could be like deleting
> a zone limit.
>
> $ ovs-appctl dpctl/ct-set-limits zone=1,limit=0

The default limit may change from time to time, it is not necessarily
to be zero (unlimited).

On the other hand, we may set limit to 'default', such as having
commands like $ ovs-appctl dpctl/ct-set-limits zone=1,limit=default
zone=2,limit=default zone=3,limit=default

But it  seems to be easier to have syntax like $ ovs-appctl
dpctl/ct-del-lmits zone=1,2,3

Therefore, I prefer to keep the ct-del-limits command as is.

Thanks,

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


Re: [ovs-dev] [PATCH v2 03/11] datapath: compat: Introduce static key support

2018-08-02 Thread Yi-Hung Wei
On Wed, Aug 1, 2018 at 10:08 PM, Darrell Ball  wrote:
> Thanks for the patch Yi-hung
>
> On 8/1/18, 5:42 PM, "ovs-dev-boun...@openvswitch.org on behalf of Yi-Hung 
> Wei"  
> wrote:
>
> This is a feature that is needed for a follow up patch
> in ovs kernel datapath.
>
> It is usually implied that patch in a series is needed by a subsequent patch 
> in the same series.
> Would you mind expanding the commit message on the general utility?
> Also, the upstream commit id is embedded in the file, static_key.h
>
> + * This backport is based on upstream net-next commit 11276d5306b8
> + * ("locking/static_keys: Add a new static_key interface").
>
> Would it be possible to bring it into the commit message also?
Thanks for the feedback. I will update the commit message in v3 as following.

Static keys allow the inclusion of seldom used features in
performance-sensitive fast-path kernel code, via a GCC feature and a
code patching technique. For more information:
* https://www.kernel.org/doc/Documentation/static-keys.txt

Since upstream ovs kernel module now uses some static key API that was
introduced in v4.3 kernel, we shall backport them to the compat module
for older kernel supprots.

This backport is based on upstream net-next commit 11276d5306b8
("locking/static_keys: Add a new static_key interface").


Thanks,

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


Re: [ovs-dev] [PATCH v2 02/11] datapath: compat: Backports nf_conncount

2018-08-02 Thread Yi-Hung Wei
> On 8/1/18, 5:42 PM, "ovs-dev-boun...@openvswitch.org on behalf of Yi-Hung 
> Wei"  
> wrote:
>
> This patch backports the nf_conncount backend that counts the number
> of connections matching an arbitrary key.  The following patch will
> use the feature to support connection tracking zone limit in ovs
> kernel datapath.
>
> This backport is based on an upstream net-next commit 5c789e131cbb
> ("netfilter: nf_conncount: Add list lock and gc worker, and RCU for
> init tree search") that applies a couple of techniques to optimize
> nf_conncount performance.
>
> The upstream nf_conncount has a couple of export functions while
> this patch only export the ones that ovs kernel module needs.
>
>
> Could you specify what is not included in this patch from upstream?
> Is it just some parts were excluded or other logic changes as well?
>
The original nf_conncount backend exports the following functions
cat ./net-next/net/netfilter/nf_conncount.c | grep EXPORT
EXPORT_SYMBOL_GPL(nf_conncount_add);
EXPORT_SYMBOL_GPL(nf_conncount_lookup);
EXPORT_SYMBOL_GPL(nf_conncount_list_init);
EXPORT_SYMBOL_GPL(nf_conncount_gc_list);
EXPORT_SYMBOL_GPL(nf_conncount_count);
EXPORT_SYMBOL_GPL(nf_conncount_init);
EXPORT_SYMBOL_GPL(nf_conncount_cache_free);
EXPORT_SYMBOL_GPL(nf_conncount_destroy);

and only the following three of them are used by ovs
$ cat ./datapath/linux/compat/nf_conncount.c  | grep EXPORT
EXPORT_SYMBOL_GPL(rpl_nf_conncount_count);
EXPORT_SYMBOL_GPL(rpl_nf_conncount_init);
EXPORT_SYMBOL_GPL(rpl_nf_conncount_destroy);

Therefore, the rest of them are not exported on the ovs compat module.
There is no logic or functional changes on nf_conncount.

Also, the nf_conncount backports are based on the following 13
commits, I only listed the last commit ID in the commit message which
is a bit confusing. I will add the rest of commits to the commit
message on v3.

5c789e131cbb ("netfilter: nf_conncount: Add list lock and gc worker,
and RCU for init tree search")
34848d5c896e ("netfilter: nf_conncount: Split insert and traversal")
2ba39118c10a ("netfilter: nf_conncount: Move locking into count_tree()")
976afca1ceba ("netfilter: nf_conncount: Early exit in
nf_conncount_lookup() and cleanup")
cb2b36f5a97d ("netfilter: nf_conncount: Switch to plain list")
2a406e8ac7c3 ("netfilter: nf_conncount: Early exit for garbage collection")
b36e4523d4d5 ("netfilter: nf_conncount: fix garbage collection confirm race")
21ba8847f857 ("netfilter: nf_conncount: Fix garbage collection with zones")
5e5cbc7b23ea ("netfilter: nf_conncount: expose connection list interface")
35d8deb80c30 ("netfilter: conncount: Support count only use case")
6aec208786c2 ("netfilter: Refactor nf_conncount")
d384e65f1e75 ("netfilter: return booleans instead of integers")
625c556118f3 ("netfilter: connlimit: split xt_connlimit into front and backend")

Thanks,

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


Re: [ovs-dev] [PATCH v7 4/4] Replace router internal MAC with gateway MAC for reply packets

2018-08-02 Thread Mark Michelson

On 08/01/2018 08:16 AM, vkomm...@redhat.com wrote:

From: venkata anil 

Previous patches in the series doesn't address issue 1 explained in [1]
i.e
1) removal of router gateway port MAC address on external switches
after expiring of aging time.
2) then external switches unable to learn the gateway MAC as
reply packets carry router internal port MAC address as source

To fix this, router on gateway node will use router gateway MAC address
instead of router internal port MAC address as source for reply packets,
so that external switches can learn gateway MAC address.
This is done only for reply packets from router gateway to tenant VLAN
switch ports.
Later before delivering the packet to the port, ovn-controller will
replace the gateway MAC with router internal port MAC in table 33.

[1] //mail.openvswitch.org/pipermail/ovs-dev/2018-July/349803.html

Reported-by: Miguel Angel Ajo 
Reported-at: 
https://mail.openvswitch.org/pipermail/ovs-dev/2018-July/349803.html
Signed-off-by: Venkata Anil 
---

v6->v7:
* Added this patch


  ovn/controller/physical.c   | 60 ++---
  ovn/northd/ovn-northd.8.xml | 10 
  ovn/northd/ovn-northd.c | 29 ++
  ovn/ovn-architecture.7.xml  |  4 ++-
  4 files changed, 99 insertions(+), 4 deletions(-)

diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index f269a1d..1f41f59 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -190,7 +190,9 @@ get_zone_ids(const struct sbrec_port_binding *binding,
  static void
  put_local_common_flows(uint32_t dp_key, uint32_t port_key,
 bool nested_container, const struct zone_ids *zone_ids,
-   struct ofpbuf *ofpacts_p, struct hmap *flow_table)
+   struct ofpbuf *ofpacts_p, struct hmap *flow_table,
+   struct local_datapath *ld,
+   const struct hmap *local_datapaths)
  {
  struct match match;
  
@@ -221,11 +223,63 @@ put_local_common_flows(uint32_t dp_key, uint32_t port_key,

  }
  }
  
+struct ofpbuf *clone = NULL;

+clone = ofpbuf_clone(ofpacts_p);
+


I don't understand the use of the cloned ofpbuf here. You could just 
ofpbuf_clear(ofpacts_p) in each iteration of the for loop below and 
reuse ofpacts_p where you've used clone below.



  /* Resubmit to table 34. */
  put_resubmit(OFTABLE_CHECK_LOOPBACK, ofpacts_p);
  ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100, 0,
  , ofpacts_p);
  
+/* For a reply packet from gateway with VLAN switch port as destination

+ * (excluding localnet_port and external VLAN networks), gateway router
+ * will use gateway MAC address as source MAC instead of router internal
+ * port MAC, so that external switches can learn gateway MAC address.
+ * Here (before packet is given to the port) we replace router gateway
+ * MAC address with router internal port MAC. */
+if (ld->localnet_port && (port_key != ld->localnet_port->tunnel_key)) {
+for (int i = 0; i < ld->n_peer_dps; i++) {
+struct local_datapath *peer_ldp = get_local_datapath(
+local_datapaths, ld->peer_dps[i]->peer_dp->tunnel_key);
+const struct sbrec_port_binding *crp;
+crp = peer_ldp->chassisredirect_port;
+if (!crp) {
+continue;
+}
+
+if (strcmp(smap_get(>options, "distributed-port"),
+   ld->peer_dps[i]->peer->logical_port) &&
+(port_key != ld->peer_dps[i]->patch->tunnel_key)) {
+for (int j = 0; j < crp->n_mac; j++) {
+struct lport_addresses laddrs;
+if (!extract_lsp_addresses(crp->mac[j], )) {
+continue;
+}
+match_set_dl_src(, laddrs.ea);
+destroy_lport_addresses();
+break;
+}
+for (int j = 0; j < ld->peer_dps[i]->peer->n_mac; j++) {
+struct lport_addresses laddrs;
+uint64_t mac64;
+if (!extract_lsp_addresses(
+ld->peer_dps[i]->peer->mac[j], )) {
+continue;
+}
+mac64 = eth_addr_to_uint64(laddrs.ea);
+put_load(mac64,
+ MFF_ETH_SRC, 0, 48, clone);
+destroy_lport_addresses();
+break;
+}
+put_resubmit(OFTABLE_CHECK_LOOPBACK, clone);
+ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 150, 0,
+, clone);
+}
+}
+}
+ofpbuf_delete(clone);
+
  /* Table 34, Priority 100.
   * ===
   *
@@ -330,7 +384,7 @@ consider_port_binding(struct ovsdb_idl_index 

Re: [ovs-dev] [PATCH v4 0/9] IPsec support for tunneling

2018-08-02 Thread Qiuyu Xiao
Thanks for the review! I will start to work on the v5 patch.

-Qiuyu

On Thu, Aug 2, 2018 at 11:47 AM, Ben Pfaff  wrote:
> On Tue, Jul 31, 2018 at 02:08:45PM -0700, Qiuyu Xiao wrote:
>> This patch series reintroduce IPsec support for OVS tunneling and enable OVN 
>> to
>> use IPsec tunnels. GRE, VXLAN, GENEVE, and STT IPsec tunnels are supported.
>> StrongSwan and LibreSwan IKE daemons are supported.
>
> Thanks a lot for this.  I finished my review of v4, and applied some of
> the patches.  I hope that you can fold in my suggestions and your own
> comments and post v5.
>
> Thanks again,
>
> Ben.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v2 2/2] ovn: Modify restart_controller in ovn-ctl to use --restart

2018-08-02 Thread Ben Pfaff
On Tue, Jul 31, 2018 at 10:27:59AM +0200, Jakub Sitnicki wrote:
> On Mon, 30 Jul 2018 09:47:45 -0400
> Mark Michelson  wrote:
> 
> > The --restart flag allows for uninterrupted packet flowage when exiting
> > ovn-controller. This patch modifies the restart_controller argument to
> > ovn-ctl to use --restart.
> > 
> > Signed-off-by: Mark Michelson 
> > ---
> >  ovn/utilities/ovn-ctl | 4 ++--
> >  utilities/ovs-lib.in  | 2 +-
> >  2 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl
> > index 2fce47714..05feed2b6 100755
> > --- a/ovn/utilities/ovn-ctl
> > +++ b/ovn/utilities/ovn-ctl
> > @@ -350,7 +350,7 @@ stop_northd () {
> >  }
> >  
> >  stop_controller () {
> > -OVS_RUNDIR=${OVN_RUNDIR} stop_daemon ovn-controller
> > +OVS_RUNDIR=${OVN_RUNDIR} stop_daemon ovn-controller $@
> >  }
> 
> It needs to be "$@" or the arguments containing whitespace (or
> characters from IFS) will get split up :-C

I made that change and pushed this series to master and branch-2.10.
Thanks Mark and Jakub!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v4 8/9] OVN: native support for tunnel encryption

2018-08-02 Thread Qiuyu Xiao
Yes, it makes sense. I will add this to the next revision.

Thanks,
Qiuyu

On Thu, Aug 2, 2018 at 11:31 AM, Ben Pfaff  wrote:
> On Tue, Jul 31, 2018 at 02:08:53PM -0700, Qiuyu Xiao wrote:
>> This patch adds IPsec support for OVN tunnel. Basically, OVN offers a
>> binary option to its user for encryption configuration. If the IPsec
>> option is turned on, all tunnels will be encrypted. Otherwise, no tunnel
>> will be encrypted.
>>
>> The changes are summarized as below:
>> 1) Added a ipsec column on the NB_Global table and SB_Global table. The
>> value of ipsec column is propagated by ovn-northd from NB_Global to
>> SB_Global.
>>
>> 2) ovn-controller monitors the ipsec column in SB_Global. If the ipsec
>> value is true, ovn-controller sets options of the tunnel interface by
>> specifying "options:remote_name=". If the ipsec
>> value is false, ovn-controller removes these options.
>>
>> 3) ovs-monitor-ipsec daemon
>> (https://mail.openvswitch.org/pipermail/ovs-dev/2018-June/348701.html)
>> monitors the tunnel interface options and configures IKE daemon
>> accordingly for IPsec encryption.
>>
>> Signed-off-by: Qiuyu Xiao 
>
> It seems like, to be more secure, it would be wise for ovn-controller in
> ipsec mode to set ipsec_skb_mark to 1/1 and then add an OpenFlow flow
> that sets skb_mark to 1.  What do you think?
>
> Thanks,
>
> Ben.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] AUTHORS: Update email address for Jakub Sitnicki.

2018-08-02 Thread Ben Pfaff
On Thu, Aug 02, 2018 at 11:48:26AM -0400, Aaron Conole wrote:
> Jakub Sitnicki  writes:
> 
> > Signed-off-by: Jakub Sitnicki 
> > ---
> >
> > The j...@redhat.com address will be valid only until August 4th.
> 
> :'(
> 
> With sadness:
> 
> Acked-by: Aaron Conole 

Jakub, I'm sorry to see you go.

I applied this to all relevant branches.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v7] ovn: Allow for automatic dynamic updates of IPAM

2018-08-02 Thread Ben Pfaff
On Thu, Aug 02, 2018 at 08:52:56AM -0400, Mark Michelson wrote:
> OVN offers a method of IP address management that allows for an IPv4 subnet or
> IPv6 prefix to be specified on a logical switch. Then by specifying a
> switch port's address as "dynamic" or " dynamic", OVN will
> automatically assign addresses to the switch port.
> 
> While this works great for initial assignment of addresses, addresses do
> not automatically adjust when changes are made to the switch's
> configuration. For instance:
> * If the subnet, ipv6_prefix, or exclude_ips for a logical switch
> changes, the affected switch ports are not updated.
> * If a switch port with a static IP address is added to the switch, and
> that address conflicts with a dynamically assigned IP address, the
> dynamic address is not updated.
> * If a MAC address switched from being statically assigned to
> dynamically assigned, the MAC address would not be updated.
> * If a statically assigned MAC address changed, then the IPv6 address
> would not be updated.
> 
> This patch solves all of the above issues by changing the algorithm for
> IPAM assignment. There are essentially three steps.
> 1) While joining logical ports, all statically-assigned addresses (i.e.
> any ports without "dynamic" addresses) have their addresses registered
> to IPAM. This gives them top priority.
> 2) All logical ports with dynamic addresses are inspected. Any changes
> that must be made to the addresses are collected to be made later. Any
> addresses that do not require change are registered to IPAM. This allows
> for previously assigned dynamic addresses to be kept.
> 3) All gathered changes are enacted.

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


Re: [ovs-dev] [PATCH v4 0/9] IPsec support for tunneling

2018-08-02 Thread Ben Pfaff
On Tue, Jul 31, 2018 at 02:08:45PM -0700, Qiuyu Xiao wrote:
> This patch series reintroduce IPsec support for OVS tunneling and enable OVN 
> to
> use IPsec tunnels. GRE, VXLAN, GENEVE, and STT IPsec tunnels are supported.
> StrongSwan and LibreSwan IKE daemons are supported.

Thanks a lot for this.  I finished my review of v4, and applied some of
the patches.  I hope that you can fold in my suggestions and your own
comments and post v5.

Thanks again,

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


Re: [ovs-dev] [PATCH v4 9/9] Documentation: OVN RBAC and IPsec tutorial

2018-08-02 Thread Ben Pfaff
On Tue, Jul 31, 2018 at 02:08:54PM -0700, Qiuyu Xiao wrote:
> This patch adds step-by-step guide for configuring OVN Role-Based Access
> Control and IPsec.
> 
> Signed-off-by: Qiuyu Xiao 

Here are my suggestions for this patch (really for this one, this time).

I'll look forward to v5 of this series!

Thanks,

Ben.

--8<--cut here-->8--

diff --git a/Documentation/index.rst b/Documentation/index.rst
index bab5ba1f1a98..46261235c732 100644
--- a/Documentation/index.rst
+++ b/Documentation/index.rst
@@ -66,7 +66,9 @@ vSwitch? Start here.
   :doc:`tutorials/ovn-sandbox` |
   :doc:`tutorials/ovn-openstack` |
   :doc:`tutorials/ovs-conntrack` |
-  :doc:`tutorials/ipsec`
+  :doc:`tutorials/ipsec` |
+  :doc:`tutorials/ovn-ipsec` |
+  :doc:`tutorials/ovn-rbac`
 
 Deeper Dive
 ---
diff --git a/Documentation/tutorials/ovn-ipsec.rst 
b/Documentation/tutorials/ovn-ipsec.rst
index 76269c46a784..5a8701905fa1 100644
--- a/Documentation/tutorials/ovn-ipsec.rst
+++ b/Documentation/tutorials/ovn-ipsec.rst
@@ -43,15 +43,17 @@ Generating Certificates and Keys
 OVN chassis uses CA-signed certificate to authenticate peer chassis for
 building IPsec tunnel. If you have enabled Role-Based Access Control (RBAC) in
 OVN, you can use the RBAC SSL certificates and keys to set up OVN IPsec. Or you
-can generate seperate certificates and keys with ``ovs-pki`` (refer to
+can generate separate certificates and keys with ``ovs-pki`` (refer to
 :ref:`gen-certs-keys`).
 
 .. note::
 
OVN IPsec requires x.509 version 3 certificate with the subjectAltName DNS
field setting the same string as the common name (CN) field. CN should be
-   set as the chassis name.  Please generate compatible certificates if you use
-   another PKI tool to manage certificates.
+   set as the chassis name.  ``ovs-pki`` in Open vSwitch 2.10.90 and later
+   generates such certificates.  Please generate compatible certificates if you
+   use another PKI tool, or an older version of ``ovs-pki``, to manage
+   certificates.
 
 Configuring OVN IPsec
 -
@@ -67,27 +69,27 @@ each chassis. Use the following command::
 Enabling OVN IPsec
 --
 
-To enable OVN IPsec, set `ipsec` column in `NB_Global` table of the northbound
-database to be true::
+To enable OVN IPsec, set ``ipsec`` column in ``NB_Global`` table of the
+northbound database to true::
 
 $ ovn-nbctl set nb_global . ipsec=true
 
 With OVN IPsec enabled, all tunnel traffic in OVN will be encrypted with IPsec.
-To disable it, set `ipsec` column in `NB_Global` table of the northbound
-database to be false::
+To disable it, set ``ipsec`` column in ``NB_Global`` table of the northbound
+database to false::
 
 $ ovn-nbctl set nb_global . ipsec=false
 
 Troubleshooting
 ---
 
-ovs-monitor-ipsec daemon in each chassis manages and monitors the IPsec tunnel
-state. Use the following ovs-apptcl command to get ovs-monitor-ipsec internal
-representation of tunnel configuration::
+The ``ovs-monitor-ipsec`` daemon in each chassis manages and monitors the IPsec
+tunnel state. Use the following ``ovs-appctl`` command to view
+``ovs-monitor-ipsec`` internal representation of tunnel configuration::
 
 $ ovs-appctl -t ovs-monitor-ipsec tunnels/show
 
-If there is misconfiguration then ovs-appctl should indicate why.
+If there is a misconfiguration, then ``ovs-appctl`` should indicate why.
 For example::
 
Interface name: ovn-host_2-0 v1 (CONFIGURED) <--- Should be set to 
CONFIGURED.
@@ -119,13 +121,13 @@ For example::
  tunnel
 
 If you don't see any active connections, try to run the following command to
-refresh the ovs-monitor-ipsec daemon::
+refresh the ``ovs-monitor-ipsec`` daemon::
 
 $ ovs-appctl -t ovs-monitor-ipsec refresh
 
-You can also check the logs of the ovs-monitor-ipsec daemon and the IKE daemon
-to locate issues. The logs of the ovs-monitor-ipsec is in
-/var/log/openvswitch/ovs-monitor-ipsec.log.
+You can also check the logs of the ``ovs-monitor-ipsec`` daemon and the IKE
+daemon to locate issues.  ``ovs-monitor-ipsec`` outputs log messages to
+``/var/log/openvswitch/ovs-monitor-ipsec.log``.
 
 Bug Reporting
 -
diff --git a/Documentation/tutorials/ovn-rbac.rst 
b/Documentation/tutorials/ovn-rbac.rst
index ff93ba54bdc6..ec163e2df369 100644
--- a/Documentation/tutorials/ovn-rbac.rst
+++ b/Documentation/tutorials/ovn-rbac.rst
@@ -81,7 +81,7 @@ address `machine_3-ip`. `machine_3` also hosts public key 
infrastructure (PKI).
 
.. note::
 
- chassis_1 must be the same string as the external_ids:system-id in the
+ chassis_1 must be the same string as ``external_ids:system-id`` in the
  Open_vSwitch table (the chassis name) of machine_1. Same applies for
  chassis_2.
 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v4 9/9] Documentation: OVN RBAC and IPsec tutorial

2018-08-02 Thread Ben Pfaff
Thanks for the comments.  Can you integrate my suggestions and your
comments for v5?

Thanks,

Ben.

On Wed, Aug 01, 2018 at 05:28:02PM -0700, Qiuyu Xiao wrote:
> Thanks Ben! I made a few comments below. Other than that, all looks pretty 
> good!
> 
> -Qiuyu
> 
> On Wed, Aug 1, 2018 at 10:03 AM, Ben Pfaff  wrote:
> > On Tue, Jul 31, 2018 at 02:08:54PM -0700, Qiuyu Xiao wrote:
> >> This patch adds step-by-step guide for configuring OVN Role-Based Access
> >> Control and IPsec.
> >>
> >> Signed-off-by: Qiuyu Xiao 
> >
> > You wrote a lot of documentation, and it's really good!  Thank you.
> >
> > I spent some time working to make it even better.  I'm appending an
> > incremental that I'd suggest folding in.  Does it make sense to you?
> >
> > Thanks,
> >
> > Ben.
> >
> > --8<--cut here-->8--
> >
> > diff --git a/Documentation/howto/ipsec.rst b/Documentation/howto/ipsec.rst
> > index 17dead5010cf..32e55b5acd0d 100644
> > --- a/Documentation/howto/ipsec.rst
> > +++ b/Documentation/howto/ipsec.rst
> > @@ -48,7 +48,10 @@ OVS IPsec aims to provide a simple interface for user to 
> > add encryption on OVS
> >  tunnels. It supports GRE, GENEVE, VXLAN, and STT tunnel. The IPsec
> >  configuration is done by setting options of the tunnel interface and
> >  other_config of Open_vSwitch. You can choose different authentication 
> > methods
> > -and fowarding modes based on your system requirement.
> > +and forwarding modes based on your requirements.
> > +
> > +OVS does not currently provide any support for IPsec encryption for 
> > traffic not
> > +encapsulated in a tunnel.
> >
> >  Configuration
> >  -
> > @@ -59,7 +62,7 @@ Authentication Methods
> >  Hosts of the IPsec tunnel need to authenticate each other to build a secure
> >  channel. There are three authentication methods:
> >
> > -1) You can use pre-shared key (PSK) to do authentication. In both hosts, 
> > set
> > +1) You can use a pre-shared key (PSK) to do authentication. In both hosts, 
> > set
> > the same PSK value. This PSK is like your password. You should never 
> > reveal
> > it to untrusted parties. This method is easier to use but less secure 
> > than
> > the certificate-based methods::
> > @@ -72,9 +75,9 @@ channel. There are three authentication methods:
> >
> > .. note::
> >
> > -  The local_ip field is required for the IPsec tunnel.
> > +  The ``local_ip`` field is required for the IPsec tunnel.
> >
> > -2) You can use self-signed certificate to do authentication. In each host,
> > +2) You can use a self-signed certificate to do authentication. In each 
> > host,
> > generate a certificate and the paired private key. Copy the certificate 
> > of
> > the remote host to the local host and configure the OVS as following::
> >
> > @@ -98,6 +101,10 @@ channel. There are three authentication methods:
> >follow the tutorial in :doc:`/tutorials/ipsec` and use ovs-pki(8) to
> >generate compatible certificate and key.
> >
> > +  (Before OVS version 2.10.90, ovs-pki(8) did not generate x.509 v3
> > +  certificates, so if your existing PKI was generated by an older 
> > version,
> > +  it is not suitable for this purpose.)
> > +
> >  3) You can also use CA-signed certificate to do authentication. First, you 
> > need
> > to create a CA certificate and sign each host certificate with the CA 
> > key
> > (please see :doc:`/tutorials/ipsec`). Copy the CA certificate to each
> > @@ -133,8 +140,8 @@ actually taking affect to encrypt packets. To offset 
> > the risk of unencrypted
> >  packets leaking out during this period, you can choose a more secure 
> > forwarding
> >  mode.  There are three forwarding modes:
> >
> > -1) The default mode allows unencrypted packets being sent out before IPsec
> > -   taking effect::
> > +1) The default mode allows unencrypted packets to be sent before IPsec
> > +   completes negotiation::
> >
> >   $ ovs-vsctl add-port br0 ipsec_gre0 -- \
> >set interface ipsec_gre0 type=gre \
> > @@ -146,7 +153,7 @@ mode.  There are three forwarding modes:
> > and/or if there is firewall that can drop the plain packets that
> > occasionally leak the tunnel unencrypted on OVSDB (re)configuration 
> > events.
> >
> > -2) The ipsec_skb_mark mode filters unencrypted packets by using skb mark of
> > +2) The ipsec_skb_mark mode drops unencrypted packets by using skb_mark of
> > tunnel packets::
> >
> >   $ ovs-vsctl set Open_vSwitch . other_config:ipsec_skb_mark=0/1
> > @@ -156,15 +163,15 @@ mode.  There are three forwarding modes:
> >  options:remote_ip=2.2.2.2 \
> >  options:psk=swordfish
> >
> > -   OVS IPsec filters unencrypted packets which carry the same skb mark as
> > +   OVS IPsec drops unencrypted packets which carry the same skb_mark as
> > `ipsec_skb_mark`. By setting the ipsec_skb_mark as 

Re: [ovs-dev] [PATCH v4 8/9] OVN: native support for tunnel encryption

2018-08-02 Thread Ben Pfaff
On Tue, Jul 31, 2018 at 02:08:53PM -0700, Qiuyu Xiao wrote:
> This patch adds IPsec support for OVN tunnel. Basically, OVN offers a
> binary option to its user for encryption configuration. If the IPsec
> option is turned on, all tunnels will be encrypted. Otherwise, no tunnel
> will be encrypted.
> 
> The changes are summarized as below:
> 1) Added a ipsec column on the NB_Global table and SB_Global table. The
> value of ipsec column is propagated by ovn-northd from NB_Global to
> SB_Global.
> 
> 2) ovn-controller monitors the ipsec column in SB_Global. If the ipsec
> value is true, ovn-controller sets options of the tunnel interface by
> specifying "options:remote_name=". If the ipsec
> value is false, ovn-controller removes these options.
> 
> 3) ovs-monitor-ipsec daemon
> (https://mail.openvswitch.org/pipermail/ovs-dev/2018-June/348701.html)
> monitors the tunnel interface options and configures IKE daemon
> accordingly for IPsec encryption.
> 
> Signed-off-by: Qiuyu Xiao 

It seems like, to be more secure, it would be wise for ovn-controller in
ipsec mode to set ipsec_skb_mark to 1/1 and then add an OpenFlow flow
that sets skb_mark to 1.  What do you think?

Thanks,

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


Re: [ovs-dev] [PATCH] tests: Test for ovs-ofctl snoop command

2018-08-02 Thread Ashish Varma
Thanks for the review. I will try to use OVS_WAIT_UNTIL (or something
similar)

On Thu, Aug 2, 2018 at 7:33 AM, Ilya Maximets 
wrote:

> Hello. Thanks for the patch.
> I'm not much familiar with that functionality thus someone
> else should review the sanity of this patch, but I have few
> comments about testing itself. See inline.
>
> Best regard, Ilya Maximets.
>
> > Added test for snoop command to check for the initial handshake messages
> > when a bridge connects to a controller via 'unix' connection method.
> >
> > Signed-off-by: Ashish Varma 
> > ---
> >  tests/ovs-ofctl.at | 28 
> >  1 file changed, 28 insertions(+)
> >
> > diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
> > index 06597d7..794277b 100644
> > --- a/tests/ovs-ofctl.at
> > +++ b/tests/ovs-ofctl.at
> > @@ -3184,3 +3184,31 @@ AT_CHECK([grep -q "ct_dpif|DBG|.*ct_flush: zone
> 123" ovs-vswitchd.log])
> >
> >  OVS_VSWITCHD_STOP
> >  AT_CLEANUP
> > +
> > +
> > +AT_SETUP([ovs-ofctl snoop-unix-connection])
> > +OVS_VSWITCHD_START
> > +
> > +dnl setup controller for br0 before starting the controller
> > +AT_CHECK([ovs-vsctl set-controller br0 unix:testcontroller])
> > +
> > +dnl then start listening on the '.snoop' connection
> > +AT_CHECK([ovs-ofctl --detach --pidfile=ovsofctl_snoop.pid snoop br0 1>
> snoopbr0.txt 2>&1])
> > +on_exit 'kill `cat ovsofctl_snoop.pid`'
> > +on_exit 'unlink snoopbr0.txt'
> > +
> > +dnl finally start the controller
> > +AT_CHECK([ovs-testcontroller --detach --pidfile punix:testcontroller],
> [0], [ignore])
>
> Could you, please, add '-vsyslog:off' to this command?
>
> > +on_exit 'kill `cat ovs-testcontroller.pid`'
> > +OVS_WAIT_UNTIL([test -e testcontroller])
> > +
> > +dnl wait for 2 seconds for snoop to collect the messages from the bridge
> > +sleep 2
>
> Waiting few seconds is a bad style, IMHO. For example this could
> lead to test failures in parallel testing if the thread will have
> no enough time to work.
>
> Is it possible to replace this by something like OVS_WAIT_UNTIL ?
>
> > +
> > +dnl check some of the initial openflow setup messages
> > +AT_CHECK([egrep "OFPT_FEATURES_REQUEST" snoopbr0.txt 1> /dev/null 2>&1])
> > +AT_CHECK([egrep "OFPT_FEATURES_REPLY" snoopbr0.txt 1> /dev/null 2>&1])
> > +AT_CHECK([egrep "OFPT_SET_CONFIG" snoopbr0.txt 1> /dev/null 2>&1])
> > +
> > +OVS_VSWITCHD_STOP(["/connection failed (No such file or directory)/d"])
> > +AT_CLEANUP
> > --
> > 2.7.4
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] ofproto-dpif-upcall: Fix for flow limit issue in revalidator

2018-08-02 Thread Vishal Deep Ajmera
When the revalidator thread takes a long time to dump data path
flows (e.g. due to busy CPU), it reduces the maximum limit for
new flows that can be added. This results in more upcalls for
packets which do not find data path flows and temporarily reduces
overall throughput. When the situation improves and the revalidator
gets enough CPU cycles, it should increase the flow limit allowing
more flows to get inserted.

Currently the flow limit does not increase if the existing number of
flows is less than 2000 and does not allow any new flows due to
incorrect condition check. This results in a permanent drop in
performance in OVS with no automatic recovery.

This patch fixes the conditional check for increasing flow limit.

Signed-off-by: Vishal Deep Ajmera 
---
 ofproto/ofproto-dpif-upcall.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 85f5792..607 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -931,8 +931,8 @@ udpif_revalidator(void *arg)
 flow_limit /= duration / 1000;
 } else if (duration > 1300) {
 flow_limit = flow_limit * 3 / 4;
-} else if (duration < 1000 && n_flows > 2000
-   && flow_limit < n_flows * 1000 / duration) {
+} else if (duration < 1000 &&
+   flow_limit < n_flows * 1000 / duration) {
 flow_limit += 1000;
 }
 flow_limit = MIN(ofproto_flow_limit, MAX(flow_limit, 1000));
-- 
1.9.1

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


[ovs-dev] US $15.5million Cash Payment/Home Delivery. From Nat West Bank London UK.

2018-08-02 Thread ahmed idris via dev
Nat West Bank London,
United Kingdom.
(Offshore Office)

Attention:

I am contacting you by email from our offshore office, where a contract payment 
of $15.5million United States dollars only emerged on your name as a legitimate 
beneficiary of the fund.

I am the manager, over seeing the release and delivery of the said fund in a 
special method to you without any difficulty or problem once you follow my 
advice and directives.

It was resolved and agreed that your inheritance/contract payment sum of US 
$15.5 million should be release and send to you on a special method of payment 
tagged Cash Payment/Home Delivery in your country.

The cash in hand luggage will be send to you through a top safe and secured 
security company agent to deliver the package to you in your country home.
The delivery agent will contact you on telephone or email as soon as the 
luggage arrived your country.

Re-confirm to us the following information to proceed on the processing of the 
fund release on your name and delivery to your home address;

Your name,
Address,
Telephone,
Name the nearest Airport to you,
Attached copy of your international passport, driver's license or ID Card.

Waiting to hear from you.

Regards,
Ahmed Idris
Manager, Offshore Office,
Nat West Bank London.



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


Re: [ovs-dev] [PATCH] utilities: Run ovsdb-server pre-startup DB steps as root

2018-08-02 Thread Timothy Redaelli
On Thu, Aug 2, 2018 at 4:58 PM, Timothy Redaelli 
wrote:
[...]
>
> diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
> index c3b76ec94..33776aac7 100644
> --- a/utilities/ovs-lib.in
> +++ b/utilities/ovs-lib.in
> @@ -389,7 +389,10 @@ move_ip_routes () {
>
>  ovsdb_tool () {
>  if [ "$OVS_USER" != "" ]; then
> -runuser --user "${OVS_USER%:*}" -- ovsdb-tool -vconsole:off "$@"
> +local uid=$(id -u "${OVS_USER%:*}")
> +local gid=$(id -g "${OVS_USER%:*}")
> +local groups=$(id -G "${OVS_USER%:*}" | tr ' ' ',')
> +setpriv --reuid "$uid" --regid "$gid" --groups "$groups"
> ovsdb-tool -vconsole:off "$@"

^ I'm sorry, I had this long line wrapped.

>  else
>  ovsdb-tool -vconsole:off "$@"
>  fi

This is, hopefully, the correct git-diff:

diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
index c3b76ec94..33776aac7 100644
--- a/utilities/ovs-lib.in
+++ b/utilities/ovs-lib.in
@@ -389,7 +389,10 @@ move_ip_routes () {

 ovsdb_tool () {
 if [ "$OVS_USER" != "" ]; then
-runuser --user "${OVS_USER%:*}" -- ovsdb-tool -vconsole:off "$@"
+local uid=$(id -u "${OVS_USER%:*}")
+local gid=$(id -g "${OVS_USER%:*}")
+local groups=$(id -G "${OVS_USER%:*}" | tr ' ' ',')
+setpriv --reuid "$uid" --regid "$gid" --groups "$groups"
ovsdb-tool -vconsole:off "$@"
 else
 ovsdb-tool -vconsole:off "$@"
 fi
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] Word para Ejecutivos

2018-08-02 Thread Redacción de textos a nivel profesional
Correcta Utilización de WORD para ejecutivos
Agosto 23 
   

Introducción: 

Se conocerán distintas herramientas de edición de texto, gráficos, figuras, 
inserción de imágenes, tablas y utilización de plantillas. Se hará una revisión 
de todas las opciones que ofrece Microsoft Word para la redacción de textos a 
nivel profesional. 
  
Objetivos: 

El curso proporciona herramientas que permitan al participante desarrollar 
documentos de texto a partir de una idea o concepto general. 

  TEMARIO: 

1. Cuadros de Texto y Wordart.
2. Combinar Correspondencia. 
3. Numeración de Páginas. 
4. Tablas de Contenido. 
5. Mapas Conceptuales. 
6. Función RAND. 
7. Separar Correspondencia.   

Al solicitar información, recibirá de manera gratuita y sin compromiso: 
Temario, costos, reseña del instructor y otros datos de interés. 

Responda por este medio con la frase: "Word" + Empresa + Nombre + Número de 
Teléfono. 

o marcando al: 045 + 5515546630 
 
  


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


Re: [ovs-dev] [PATCH] utilities: Run ovsdb-server pre-startup DB steps as root

2018-08-02 Thread Timothy Redaelli
On Fri, Jul 27, 2018 at 8:16 PM, Aaron Conole  wrote:
> Markos Chandras  writes:
[...]
>
> Is it possible that the provided diff can fix most of the issue (rather
> than needing the corrective block in ovs-ctl)?  If so, I'd advocate for
> the smaller change instead.  I need to double check it on RHEL/Fedora.
>
> Flavio?  Timothy?
>
> diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
> index 92f98ad92..8db887ef6 100644
> --- a/utilities/ovs-lib.in
> +++ b/utilities/ovs-lib.in
> @@ -389,7 +389,7 @@ move_ip_routes () {
>
>  ovsdb_tool () {
>  if [ "$OVS_USER" != "" ]; then
> -runuser --user "${OVS_USER%:*}" -- ovsdb-tool -vconsole:off "$@"
> +setpriv --reuid "${OVS_USER%:*}" ovsdb-tool -vconsole:off "$@"
>  else
>  ovsdb-tool -vconsole:off "$@"
>  fi

Hi,
I tested your solution with SUSE (Vagrant), RHEL7 and Fedora 28.

Unfortunately, as-is, it doesn't work on RHEL7 since the old setpriv
version we use on RHEL7
doesn't support an username, but it wants the userid (the numeric one).
Moreover if you don't specify --regid setpriv maintains 0 (root) as
group id and this can be bad.

I created a variant of this implementation that works on SUSE, RHEL7
and Fedora 28
and that fixes the problem, by keeping the same uid/gid/groups used by runuser.

Is it ok?

diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
index c3b76ec94..33776aac7 100644
--- a/utilities/ovs-lib.in
+++ b/utilities/ovs-lib.in
@@ -389,7 +389,10 @@ move_ip_routes () {

 ovsdb_tool () {
 if [ "$OVS_USER" != "" ]; then
-runuser --user "${OVS_USER%:*}" -- ovsdb-tool -vconsole:off "$@"
+local uid=$(id -u "${OVS_USER%:*}")
+local gid=$(id -g "${OVS_USER%:*}")
+local groups=$(id -G "${OVS_USER%:*}" | tr ' ' ',')
+setpriv --reuid "$uid" --regid "$gid" --groups "$groups"
ovsdb-tool -vconsole:off "$@"
 else
 ovsdb-tool -vconsole:off "$@"
 fi
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] AUTHORS: Update email address for Jakub Sitnicki.

2018-08-02 Thread Aaron Conole
Jakub Sitnicki  writes:

> Signed-off-by: Jakub Sitnicki 
> ---
>
> The j...@redhat.com address will be valid only until August 4th.

:'(

With sadness:

Acked-by: Aaron Conole 

>  AUTHORS.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/AUTHORS.rst b/AUTHORS.rst
> index 25ee39d75..f3ac8e32b 100644
> --- a/AUTHORS.rst
> +++ b/AUTHORS.rst
> @@ -161,7 +161,7 @@ Isaku Yamahata yamah...@valinux.co.jp
>  Ivan Dyukovi.dyu...@samsung.com
>  IWASE Yusuke   iwase.yus...@gmail.com
>  Jakub Libosvar libos...@redhat.com
> -Jakub Sitnicki j...@redhat.com
> +Jakub Sitnicki jsitni...@gmail.com
>  James P.   roamp...@gmail.com
>  James Page james.p...@ubuntu.com
>  Jamie Lennox   jamielen...@gmail.com
> --
> 2.14.4
> ___
> 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] tests: Test for ovs-ofctl snoop command

2018-08-02 Thread Ilya Maximets
Hello. Thanks for the patch.
I'm not much familiar with that functionality thus someone
else should review the sanity of this patch, but I have few
comments about testing itself. See inline.

Best regard, Ilya Maximets.

> Added test for snoop command to check for the initial handshake messages
> when a bridge connects to a controller via 'unix' connection method.
> 
> Signed-off-by: Ashish Varma 
> ---
>  tests/ovs-ofctl.at | 28 
>  1 file changed, 28 insertions(+)
> 
> diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
> index 06597d7..794277b 100644
> --- a/tests/ovs-ofctl.at
> +++ b/tests/ovs-ofctl.at
> @@ -3184,3 +3184,31 @@ AT_CHECK([grep -q "ct_dpif|DBG|.*ct_flush: zone 123" 
> ovs-vswitchd.log])
>  
>  OVS_VSWITCHD_STOP
>  AT_CLEANUP
> +
> +
> +AT_SETUP([ovs-ofctl snoop-unix-connection])
> +OVS_VSWITCHD_START
> +
> +dnl setup controller for br0 before starting the controller
> +AT_CHECK([ovs-vsctl set-controller br0 unix:testcontroller])
> +
> +dnl then start listening on the '.snoop' connection
> +AT_CHECK([ovs-ofctl --detach --pidfile=ovsofctl_snoop.pid snoop br0 1> 
> snoopbr0.txt 2>&1])
> +on_exit 'kill `cat ovsofctl_snoop.pid`'
> +on_exit 'unlink snoopbr0.txt'
> +
> +dnl finally start the controller
> +AT_CHECK([ovs-testcontroller --detach --pidfile punix:testcontroller], [0], 
> [ignore])

Could you, please, add '-vsyslog:off' to this command?

> +on_exit 'kill `cat ovs-testcontroller.pid`'
> +OVS_WAIT_UNTIL([test -e testcontroller])
> +
> +dnl wait for 2 seconds for snoop to collect the messages from the bridge
> +sleep 2

Waiting few seconds is a bad style, IMHO. For example this could
lead to test failures in parallel testing if the thread will have
no enough time to work.

Is it possible to replace this by something like OVS_WAIT_UNTIL ?

> +
> +dnl check some of the initial openflow setup messages
> +AT_CHECK([egrep "OFPT_FEATURES_REQUEST" snoopbr0.txt 1> /dev/null 2>&1])
> +AT_CHECK([egrep "OFPT_FEATURES_REPLY" snoopbr0.txt 1> /dev/null 2>&1])
> +AT_CHECK([egrep "OFPT_SET_CONFIG" snoopbr0.txt 1> /dev/null 2>&1])
> +
> +OVS_VSWITCHD_STOP(["/connection failed (No such file or directory)/d"])
> +AT_CLEANUP
> -- 
> 2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] netlink-notifier: support blacklist

2018-08-02 Thread 0-day Robot
Bleep bloop.  Greetings Haifeng Lin, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

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


checkpatch:
ERROR: Remove Gerrit Change-Id's before submitting upstream.
9: Change-Id: I34121a4c44bfb2fcfe8799130762474bbfe5c015

WARNING: Line has non-spaces leading whitespace
#91 FILE: lib/rtnetlink.c:210:
rtnetlink_blacklist_init();

WARNING: Line has non-spaces leading whitespace
#100 FILE: lib/rtnetlink.c:220:
rtnetlink_blacklist_uninit();

ERROR: Improper whitespace around control block
#123 FILE: lib/rtnetlink.c:267:
CMAP_FOR_EACH(entry, cmap_node, _blacklist) {

ERROR: Improper whitespace around control block
#124 FILE: lib/rtnetlink.c:268:
if(entry) {

ERROR: Inappropriate spacing in pointer declaration
WARNING: Line lacks whitespace around operator
#135 FILE: lib/rtnetlink.c:279:
rtnetlink_blacklist_add(const char* name)

ERROR: Use xstrdup() in place of strdup()
#147 FILE: lib/rtnetlink.c:291:
entry->name = strdup(name);

ERROR: Inappropriate spacing in pointer declaration
WARNING: Line lacks whitespace around operator
#161 FILE: lib/rtnetlink.c:305:
rtnetlink_blacklist_del(const char* name)

ERROR: Improper whitespace around control block
#189 FILE: lib/rtnetlink.c:333:
CMAP_FOR_EACH(entry, cmap_node, _blacklist) {

ERROR: Improper whitespace around control block
#190 FILE: lib/rtnetlink.c:334:
if(entry) {

WARNING: Line is 81 characters long (recommended limit is 79)
#191 FILE: lib/rtnetlink.c:335:
if (!strncmp(entry->name, change->ifname, sizeof(*change->ifname))) 
{

ERROR: Inappropriate spacing in pointer declaration
WARNING: Line lacks whitespace around operator
#214 FILE: lib/rtnetlink.h:81:
rtnetlink_blacklist_add(const char* name);

ERROR: Inappropriate spacing in pointer declaration
WARNING: Line lacks whitespace around operator
#221 FILE: lib/rtnetlink.h:88:
rtnetlink_blacklist_del(const char* name);

Lines checked: 234, Warnings: 7, Errors: 10


build:
mv tests/system-dpdk-testsuite.tmp tests/system-dpdk-testsuite
\
{ sed -n -e '/%AUTHORS%/q' -e p < ./debian/copyright.in;   \
  sed '34,/^$/d' ./AUTHORS.rst |   \
sed -n -e '/^$/q' -e 's/^/  /p';   \
  sed -e '34,/%AUTHORS%/d' ./debian/copyright.in;  \
} > debian/copyright
(printf '\043 Generated automatically -- do not modify!-*- 
buffer-read-only: t -*-\n' && sed -e 's,[@]VERSION[@],2.10.90,g') < 
./rhel/openvswitch-dkms.spec.in > openvswitch-dkms.spec.tmp || exit 1; if cmp 
-s openvswitch-dkms.spec.tmp rhel/openvswitch-dkms.spec; then touch 
rhel/openvswitch-dkms.spec; rm openvswitch-dkms.spec.tmp; else mv 
openvswitch-dkms.spec.tmp rhel/openvswitch-dkms.spec; fi
(printf '\043 Generated automatically -- do not modify!-*- 
buffer-read-only: t -*-\n' && sed -e 's,[@]VERSION[@],2.10.90,g') < 
./rhel/kmod-openvswitch-rhel6.spec.in > kmod-openvswitch-rhel6.spec.tmp || exit 
1; if cmp -s kmod-openvswitch-rhel6.spec.tmp rhel/kmod-openvswitch-rhel6.spec; 
then touch rhel/kmod-openvswitch-rhel6.spec; rm 
kmod-openvswitch-rhel6.spec.tmp; else mv kmod-openvswitch-rhel6.spec.tmp 
rhel/kmod-openvswitch-rhel6.spec; fi
(printf '\043 Generated automatically -- do not modify!-*- 
buffer-read-only: t -*-\n' && sed -e 's,[@]VERSION[@],2.10.90,g') < 
./rhel/openvswitch-kmod-fedora.spec.in > openvswitch-kmod-fedora.spec.tmp || 
exit 1; if cmp -s openvswitch-kmod-fedora.spec.tmp 
rhel/openvswitch-kmod-fedora.spec; then touch 
rhel/openvswitch-kmod-fedora.spec; rm openvswitch-kmod-fedora.spec.tmp; else mv 
openvswitch-kmod-fedora.spec.tmp rhel/openvswitch-kmod-fedora.spec; fi
(printf '\043 Generated automatically -- do not modify!-*- 
buffer-read-only: t -*-\n' && sed -e 's,[@]VERSION[@],2.10.90,g') < 
./rhel/openvswitch.spec.in > openvswitch.spec.tmp || exit 1; if cmp -s 
openvswitch.spec.tmp rhel/openvswitch.spec; then touch rhel/openvswitch.spec; 
rm openvswitch.spec.tmp; else mv openvswitch.spec.tmp rhel/openvswitch.spec; fi
(printf '\043 Generated automatically -- do not modify!-*- 
buffer-read-only: t -*-\n' && sed -e 's,[@]VERSION[@],2.10.90,g') < 
./rhel/openvswitch-fedora.spec.in > openvswitch-fedora.spec.tmp || exit 1; if 
cmp -s openvswitch-fedora.spec.tmp rhel/openvswitch-fedora.spec; then touch 
rhel/openvswitch-fedora.spec; rm openvswitch-fedora.spec.tmp; else mv 
openvswitch-fedora.spec.tmp rhel/openvswitch-fedora.spec; fi
(printf '\043 Generated automatically -- do not modify!-*- 
buffer-read-only: t -*-\n' && sed -e 's,[@]VERSION[@],2.10.90,g') \
< ./xenserver/openvswitch-xen.spec.in > openvswitch-xen.spec.tmp || 
exit 1; \
if cmp -s openvswitch-xen.spec.tmp xenserver/openvswitch-xen.spec; then touch 
xenserver/openvswitch-xen.spec; rm openvswitch-xen.spec.tmp; else mv 
openvswitch-xen.spec.tmp xenserver/openvswitch-xen.spec; fi
make[3]: Entering directory 

[ovs-dev] [PATCH] netlink-notifier: support blacklist

2018-08-02 Thread Haifeng Lin
in dpdk ovs mode some ether not need rtnetlink notifier
so we can use blacklist remove ethernet from rtnetlink notifier

Change-Id: I34121a4c44bfb2fcfe8799130762474bbfe5c015
Signed-off-by: Haifeng Lin 
---
 lib/netdev-dpdk.c  |   3 ++
 lib/netlink-notifier.c |   5 ++-
 lib/rtnetlink.c| 109 +
 lib/rtnetlink.h|  22 ++
 4 files changed, 138 insertions(+), 1 deletion(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 9bf2185..2cfe5c8 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -65,6 +65,7 @@
 #include "timeval.h"
 #include "uuid.h"
 #include "unixctl.h"
+#include "rtnetlink.h"
 
 enum {VIRTIO_RXQ, VIRTIO_TXQ, VIRTIO_QNUM};
 
@@ -1015,6 +1016,8 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
 
 rte_eth_dev_info_get(dev->port_id, );
 
+rtnetlink_blacklist_add(dev->up.name);
+
 if (strstr(info.driver_name, "vf") != NULL) {
 VLOG_INFO("Virtual function detected, HW_CRC_STRIP will be enabled");
 dev->hw_ol_features |= NETDEV_RX_HW_CRC_STRIP;
diff --git a/lib/netlink-notifier.c b/lib/netlink-notifier.c
index dfecb97..430412d 100644
--- a/lib/netlink-notifier.c
+++ b/lib/netlink-notifier.c
@@ -27,6 +27,7 @@
 #include "netlink-socket.h"
 #include "openvswitch/ofpbuf.h"
 #include "openvswitch/vlog.h"
+#include "rtnetlink.h"
 
 VLOG_DEFINE_THIS_MODULE(netlink_notifier);
 
@@ -234,7 +235,9 @@ nln_report(const struct nln *nln, void *change, int group)
 
 LIST_FOR_EACH (notifier, node, >all_notifiers) {
 if (!change || group == notifier->multicast_group) {
-notifier->cb(change, notifier->aux);
+if (!rtnetlink_is_in_blacklist(change)) {
+notifier->cb(change, notifier->aux);
+}
 }
 }
 }
diff --git a/lib/rtnetlink.c b/lib/rtnetlink.c
index f822dff..c2e12e9 100644
--- a/lib/rtnetlink.c
+++ b/lib/rtnetlink.c
@@ -26,6 +26,20 @@
 #include "netlink-notifier.h"
 #include "openvswitch/ofpbuf.h"
 #include "packets.h"
+#include "rtnetlink.h"
+#include "cmap.h"
+
+struct if_entry{
+char *name;
+struct cmap_node cmap_node;
+};
+
+static struct cmap if_blacklist;
+
+static int
+rtnetlink_blacklist_init(void);
+static int
+rtnetlink_blacklist_uninit(void);
 
 #if IFLA_INFO_MAX < 5
 #define IFLA_INFO_SLAVE_KIND 4
@@ -193,6 +207,8 @@ rtnetlink_notifier_create(rtnetlink_notify_func *cb, void 
*aux)
 nln = nln_create(NETLINK_ROUTE, rtnetlink_parse_cb, _change);
 }
 
+   rtnetlink_blacklist_init();
+
 return nln_notifier_create(nln, RTNLGRP_LINK, (nln_notify_func *) cb, aux);
 }
 
@@ -201,6 +217,8 @@ rtnetlink_notifier_create(rtnetlink_notify_func *cb, void 
*aux)
 void
 rtnetlink_notifier_destroy(struct nln_notifier *notifier)
 {
+   rtnetlink_blacklist_uninit();
+
 nln_notifier_destroy(notifier);
 }
 
@@ -232,3 +250,94 @@ rtnetlink_report_link(void)
 nln_report(nln, NULL, RTNLGRP_LINK);
 }
 }
+
+static int
+rtnetlink_blacklist_init(void)
+{
+cmap_init(_blacklist);
+
+return 0;
+}
+
+static int
+rtnetlink_blacklist_uninit(void)
+{
+struct if_entry *entry;
+
+CMAP_FOR_EACH(entry, cmap_node, _blacklist) {
+if(entry) {
+free(entry->name);
+free(entry);
+}
+}
+
+cmap_destroy(_blacklist);
+return 0;
+}
+
+int
+rtnetlink_blacklist_add(const char* name)
+{
+struct if_entry *entry;
+
+CMAP_FOR_EACH_WITH_HASH (entry, cmap_node, hash_string(name, 0),
+ _blacklist) {
+if (!strncmp(name, entry->name, sizeof(*name))) {
+return 0;
+}
+}
+
+entry = xmalloc(sizeof *entry);
+entry->name = strdup(name);
+cmap_insert(_blacklist, >cmap_node,
+hash_string(entry->name, 0));
+return 0;
+}
+
+static void
+free_if_entry(struct if_entry *entry)
+{
+free(entry->name);
+free(entry);
+}
+
+int
+rtnetlink_blacklist_del(const char* name)
+{
+struct if_entry *entry;
+
+CMAP_FOR_EACH_WITH_HASH (entry, cmap_node, hash_string(name, 0),
+ _blacklist) {
+if (!strncmp(name, entry->name, sizeof(*name))) {
+break;
+}
+}
+
+cmap_remove(_blacklist, >cmap_node,
+hash_string(name, 0));
+ovsrcu_postpone(free_if_entry, entry);
+
+return 0;
+}
+
+bool
+rtnetlink_is_in_blacklist(void *c)
+{
+struct rtnetlink_change *change = c;
+struct if_entry *entry;
+
+if (!change) {
+return false;
+}
+
+CMAP_FOR_EACH(entry, cmap_node, _blacklist) {
+if(entry) {
+if (!strncmp(entry->name, change->ifname, 
sizeof(*change->ifname))) {
+return true;
+}
+}
+}
+
+return false;
+}
+
diff --git a/lib/rtnetlink.h b/lib/rtnetlink.h
index 422d1db..0fe7c5b 100755
--- a/lib/rtnetlink.h
+++ b/lib/rtnetlink.h
@@ -72,4 +72,26 @@ void rtnetlink_notifier_destroy(struct nln_notifier *);
 void rtnetlink_run(void);
 

Re: [ovs-dev] [PATCH v6] ovn: Allow for automatic dynamic updates of IPAM

2018-08-02 Thread Mark Michelson

On 08/02/2018 08:48 AM, Jakub Sitnicki wrote:

Hi Mark,

On Thu,  2 Aug 2018 08:18:12 -0400
Mark Michelson  wrote:


OVN offers a method of IP address management that allows for an IPv4
subnet or IPv6 prefix to be specified on a logical switch. Then by
specifying a switch port's address as "dynamic" or "
dynamic", OVN will automatically assign addresses to the switch port.

While this works great for initial assignment of addresses, addresses
do not automatically adjust when changes are made to the switch's
configuration. For instance:
* If the subnet, ipv6_prefix, or exclude_ips for a logical switch
changes, the affected switch ports are not updated.
* If a switch port with a static IP address is added to the switch,
and that address conflicts with a dynamically assigned IP address, the
dynamic address is not updated.
* If a MAC address switched from being statically assigned to
dynamically assigned, the MAC address would not be updated.
* If a statically assigned MAC address changed, then the IPv6 address
would not be updated.

This patch solves all of the above issues by changing the algorithm
for IPAM assignment. There are essentially three steps.
1) While joining logical ports, all statically-assigned addresses
(i.e. any ports without "dynamic" addresses) have their addresses
registered to IPAM. This gives them top priority.
2) All logical ports with dynamic addresses are inspected. Any changes
that must be made to the addresses are collected to be made later. Any
addresses that do not require change are registered to IPAM. This
allows for previously assigned dynamic addresses to be kept.
3) All gathered changes are enacted.

The change contains new tests that ensure that dynamic addresses are
updated when appropriate.

This patch also alters some existing IPAM tests. Those tests assumed
that dynamic addresses would not be updated automatically, so those
tests either had to be altered or removed.

Signed-off-by: Mark Michelson 
Acked-by: Jakub Sitnicki 
---
v5->v6:
  * Rebased

v4->v5:
  Cleanups suggested by Jakub Sitnicki + rebase
  * Add some convenience pointers for shortened code.
  * Separate checking of updates of dynamic addresses and registration
of unchanged addresses.
  * Use OVS_NOT_REACHED() instead of ovs_assert(0)

v3->v4:
  Print warning when multiple dynamic addresses are configured on a
  switch port. Ensure that dynamic addresses beyond the first on a
switch port are ignored. Found by Ben Pfaff.

v2->v3:
  Fixed a checkpatch problem (line too long)

v1->v2:
  Rebased
---


Recent fix pushed to master, 4d0214a365ae ("ovn: Fix typos in "ovn --
Address Set generation... test."), will cause the following test to
fail with this patch applied:

2578: ovn -- Address Set generation from Port Groups (dynamic addressing)

To fix the test you need to revert the mentioned commit and squash the
diff with your changes.

Thanks,
Jakub



Fixed in v7. Thanks!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH v7] ovn: Allow for automatic dynamic updates of IPAM

2018-08-02 Thread Mark Michelson
OVN offers a method of IP address management that allows for an IPv4 subnet or
IPv6 prefix to be specified on a logical switch. Then by specifying a
switch port's address as "dynamic" or " dynamic", OVN will
automatically assign addresses to the switch port.

While this works great for initial assignment of addresses, addresses do
not automatically adjust when changes are made to the switch's
configuration. For instance:
* If the subnet, ipv6_prefix, or exclude_ips for a logical switch
changes, the affected switch ports are not updated.
* If a switch port with a static IP address is added to the switch, and
that address conflicts with a dynamically assigned IP address, the
dynamic address is not updated.
* If a MAC address switched from being statically assigned to
dynamically assigned, the MAC address would not be updated.
* If a statically assigned MAC address changed, then the IPv6 address
would not be updated.

This patch solves all of the above issues by changing the algorithm for
IPAM assignment. There are essentially three steps.
1) While joining logical ports, all statically-assigned addresses (i.e.
any ports without "dynamic" addresses) have their addresses registered
to IPAM. This gives them top priority.
2) All logical ports with dynamic addresses are inspected. Any changes
that must be made to the addresses are collected to be made later. Any
addresses that do not require change are registered to IPAM. This allows
for previously assigned dynamic addresses to be kept.
3) All gathered changes are enacted.

The change contains new tests that ensure that dynamic addresses are
updated when appropriate.

This patch also alters some existing IPAM tests. Those tests assumed
that dynamic addresses would not be updated automatically, so those
tests either had to be altered or removed.

Signed-off-by: Mark Michelson 
Acked-by: Jakub Sitnicki 
---
v6->v7:
 * Alter port group address set generation test to pass

v5->v6:
 * Rebased

v4->v5:
 Cleanups suggested by Jakub Sitnicki + rebase
 * Add some convenience pointers for shortened code.
 * Separate checking of updates of dynamic addresses and registration
   of unchanged addresses.
 * Use OVS_NOT_REACHED() instead of ovs_assert(0)

v3->v4:
 Print warning when multiple dynamic addresses are configured on a
 switch port. Ensure that dynamic addresses beyond the first on a
switch port are ignored. Found by Ben Pfaff.

v2->v3:
 Fixed a checkpatch problem (line too long)

v1->v2:
 Rebased
---
 ovn/northd/ovn-northd.c | 398 +++-
 tests/ovn.at| 101 
 2 files changed, 364 insertions(+), 135 deletions(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 35baabcad..067d52d82 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -986,9 +986,6 @@ ipam_add_port_addresses(struct ovn_datapath *od, struct 
ovn_port *op)
 for (size_t i = 0; i < op->nbsp->n_addresses; i++) {
 ipam_insert_lsp_addresses(od, op, op->nbsp->addresses[i]);
 }
-if (op->nbsp->dynamic_addresses) {
-ipam_insert_lsp_addresses(od, op, op->nbsp->dynamic_addresses);
-}
 } else if (op->nbrp) {
 struct lport_addresses lrp_networks;
 if (!extract_lrp_networks(op->nbrp, _networks)) {
@@ -1061,64 +1058,263 @@ ipam_get_unused_ip(struct ovn_datapath *od)
 return od->ipam_info.start_ipv4 + new_ip_index;
 }
 
-static bool
-ipam_allocate_addresses(struct ovn_datapath *od, struct ovn_port *op,
-const char *addrspec)
+enum dynamic_update_type {
+NONE,/* No change to the address */
+REMOVE,  /* Address is no longer dynamic */
+STATIC,  /* Use static address (MAC only) */
+DYNAMIC, /* Assign a new dynamic address */
+};
+
+struct dynamic_address_update {
+struct ovs_list node;   /* In build_ipam()'s list of updates. */
+
+struct ovn_port *op;
+
+struct lport_addresses current_addresses;
+struct eth_addr static_mac;
+enum dynamic_update_type mac;
+enum dynamic_update_type ipv4;
+enum dynamic_update_type ipv6;
+};
+
+static enum dynamic_update_type
+dynamic_mac_changed(const char *lsp_addresses,
+struct dynamic_address_update *update)
+{
+   struct eth_addr ea;
+
+   if (ovs_scan(lsp_addresses, ETH_ADDR_SCAN_FMT, ETH_ADDR_SCAN_ARGS(ea))) {
+   if (eth_addr_equals(ea, update->current_addresses.ea)) {
+   return NONE;
+   } else {
+   /* MAC is still static, but it has changed */
+   update->static_mac = ea;
+   return STATIC;
+   }
+   }
+
+   uint64_t mac64 = eth_addr_to_uint64(update->current_addresses.ea);
+   if ((mac64 ^ MAC_ADDR_PREFIX) >> 24) {
+   return DYNAMIC;
+   } else {
+   return NONE;
+   }
+}
+
+static enum dynamic_update_type
+dynamic_ip4_changed(struct dynamic_address_update *update)
+{
+const struct ipam_info *ipam = >op->od->ipam_info;
+const struct lport_addresses 

Re: [ovs-dev] [PATCH v6] ovn: Allow for automatic dynamic updates of IPAM

2018-08-02 Thread Jakub Sitnicki
Hi Mark,

On Thu,  2 Aug 2018 08:18:12 -0400
Mark Michelson  wrote:

> OVN offers a method of IP address management that allows for an IPv4
> subnet or IPv6 prefix to be specified on a logical switch. Then by
> specifying a switch port's address as "dynamic" or "
> dynamic", OVN will automatically assign addresses to the switch port.
> 
> While this works great for initial assignment of addresses, addresses
> do not automatically adjust when changes are made to the switch's
> configuration. For instance:
> * If the subnet, ipv6_prefix, or exclude_ips for a logical switch
> changes, the affected switch ports are not updated.
> * If a switch port with a static IP address is added to the switch,
> and that address conflicts with a dynamically assigned IP address, the
> dynamic address is not updated.
> * If a MAC address switched from being statically assigned to
> dynamically assigned, the MAC address would not be updated.
> * If a statically assigned MAC address changed, then the IPv6 address
> would not be updated.
> 
> This patch solves all of the above issues by changing the algorithm
> for IPAM assignment. There are essentially three steps.
> 1) While joining logical ports, all statically-assigned addresses
> (i.e. any ports without "dynamic" addresses) have their addresses
> registered to IPAM. This gives them top priority.
> 2) All logical ports with dynamic addresses are inspected. Any changes
> that must be made to the addresses are collected to be made later. Any
> addresses that do not require change are registered to IPAM. This
> allows for previously assigned dynamic addresses to be kept.
> 3) All gathered changes are enacted.
> 
> The change contains new tests that ensure that dynamic addresses are
> updated when appropriate.
> 
> This patch also alters some existing IPAM tests. Those tests assumed
> that dynamic addresses would not be updated automatically, so those
> tests either had to be altered or removed.
> 
> Signed-off-by: Mark Michelson 
> Acked-by: Jakub Sitnicki 
> ---
> v5->v6:
>  * Rebased
> 
> v4->v5:
>  Cleanups suggested by Jakub Sitnicki + rebase
>  * Add some convenience pointers for shortened code.
>  * Separate checking of updates of dynamic addresses and registration
>of unchanged addresses.
>  * Use OVS_NOT_REACHED() instead of ovs_assert(0)
> 
> v3->v4:
>  Print warning when multiple dynamic addresses are configured on a
>  switch port. Ensure that dynamic addresses beyond the first on a
> switch port are ignored. Found by Ben Pfaff.
> 
> v2->v3:
>  Fixed a checkpatch problem (line too long)
> 
> v1->v2:
>  Rebased
> ---

Recent fix pushed to master, 4d0214a365ae ("ovn: Fix typos in "ovn --
Address Set generation... test."), will cause the following test to
fail with this patch applied:

2578: ovn -- Address Set generation from Port Groups (dynamic addressing)

To fix the test you need to revert the mentioned commit and squash the
diff with your changes.

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


[ovs-dev] [PATCH v6] ovn: Allow for automatic dynamic updates of IPAM

2018-08-02 Thread Mark Michelson
OVN offers a method of IP address management that allows for an IPv4 subnet or
IPv6 prefix to be specified on a logical switch. Then by specifying a
switch port's address as "dynamic" or " dynamic", OVN will
automatically assign addresses to the switch port.

While this works great for initial assignment of addresses, addresses do
not automatically adjust when changes are made to the switch's
configuration. For instance:
* If the subnet, ipv6_prefix, or exclude_ips for a logical switch
changes, the affected switch ports are not updated.
* If a switch port with a static IP address is added to the switch, and
that address conflicts with a dynamically assigned IP address, the
dynamic address is not updated.
* If a MAC address switched from being statically assigned to
dynamically assigned, the MAC address would not be updated.
* If a statically assigned MAC address changed, then the IPv6 address
would not be updated.

This patch solves all of the above issues by changing the algorithm for
IPAM assignment. There are essentially three steps.
1) While joining logical ports, all statically-assigned addresses (i.e.
any ports without "dynamic" addresses) have their addresses registered
to IPAM. This gives them top priority.
2) All logical ports with dynamic addresses are inspected. Any changes
that must be made to the addresses are collected to be made later. Any
addresses that do not require change are registered to IPAM. This allows
for previously assigned dynamic addresses to be kept.
3) All gathered changes are enacted.

The change contains new tests that ensure that dynamic addresses are
updated when appropriate.

This patch also alters some existing IPAM tests. Those tests assumed
that dynamic addresses would not be updated automatically, so those
tests either had to be altered or removed.

Signed-off-by: Mark Michelson 
Acked-by: Jakub Sitnicki 
---
v5->v6:
 * Rebased

v4->v5:
 Cleanups suggested by Jakub Sitnicki + rebase
 * Add some convenience pointers for shortened code.
 * Separate checking of updates of dynamic addresses and registration
   of unchanged addresses.
 * Use OVS_NOT_REACHED() instead of ovs_assert(0)

v3->v4:
 Print warning when multiple dynamic addresses are configured on a
 switch port. Ensure that dynamic addresses beyond the first on a switch
 port are ignored. Found by Ben Pfaff.

v2->v3:
 Fixed a checkpatch problem (line too long)

v1->v2:
 Rebased
---
 ovn/northd/ovn-northd.c | 398 +++-
 tests/ovn.at|  97 +---
 2 files changed, 362 insertions(+), 133 deletions(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 35baabcad..067d52d82 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -986,9 +986,6 @@ ipam_add_port_addresses(struct ovn_datapath *od, struct 
ovn_port *op)
 for (size_t i = 0; i < op->nbsp->n_addresses; i++) {
 ipam_insert_lsp_addresses(od, op, op->nbsp->addresses[i]);
 }
-if (op->nbsp->dynamic_addresses) {
-ipam_insert_lsp_addresses(od, op, op->nbsp->dynamic_addresses);
-}
 } else if (op->nbrp) {
 struct lport_addresses lrp_networks;
 if (!extract_lrp_networks(op->nbrp, _networks)) {
@@ -1061,64 +1058,263 @@ ipam_get_unused_ip(struct ovn_datapath *od)
 return od->ipam_info.start_ipv4 + new_ip_index;
 }
 
-static bool
-ipam_allocate_addresses(struct ovn_datapath *od, struct ovn_port *op,
-const char *addrspec)
+enum dynamic_update_type {
+NONE,/* No change to the address */
+REMOVE,  /* Address is no longer dynamic */
+STATIC,  /* Use static address (MAC only) */
+DYNAMIC, /* Assign a new dynamic address */
+};
+
+struct dynamic_address_update {
+struct ovs_list node;   /* In build_ipam()'s list of updates. */
+
+struct ovn_port *op;
+
+struct lport_addresses current_addresses;
+struct eth_addr static_mac;
+enum dynamic_update_type mac;
+enum dynamic_update_type ipv4;
+enum dynamic_update_type ipv6;
+};
+
+static enum dynamic_update_type
+dynamic_mac_changed(const char *lsp_addresses,
+struct dynamic_address_update *update)
+{
+   struct eth_addr ea;
+
+   if (ovs_scan(lsp_addresses, ETH_ADDR_SCAN_FMT, ETH_ADDR_SCAN_ARGS(ea))) {
+   if (eth_addr_equals(ea, update->current_addresses.ea)) {
+   return NONE;
+   } else {
+   /* MAC is still static, but it has changed */
+   update->static_mac = ea;
+   return STATIC;
+   }
+   }
+
+   uint64_t mac64 = eth_addr_to_uint64(update->current_addresses.ea);
+   if ((mac64 ^ MAC_ADDR_PREFIX) >> 24) {
+   return DYNAMIC;
+   } else {
+   return NONE;
+   }
+}
+
+static enum dynamic_update_type
+dynamic_ip4_changed(struct dynamic_address_update *update)
+{
+const struct ipam_info *ipam = >op->od->ipam_info;
+const struct lport_addresses *cur_addresses = >current_addresses;
+bool dynamic_ip4 = 

Re: [ovs-dev] [PATCH V2 0/4] Enable set/match of tos/ttl for IP tunnels on TC data-path

2018-08-02 Thread Simon Horman
On Wed, Aug 01, 2018 at 04:28:03PM +0300, Or Gerlitz wrote:
> On Wed, Aug 1, 2018 at 2:29 PM, Simon Horman  
> wrote:
> > Hi Or,
> >
> > On 1 August 2018 at 13:21, Or Gerlitz  wrote:
> >>
> >> On Wed, Aug 1, 2018 at 2:07 PM, Simon Horman 
> >> wrote:
> >> > On 1 August 2018 at 11:31, Simon Horman 
> >> > wrote:
> >> >>
> >> >> Thanks Or, Thanks Ben,
> >> >>
> >> >> On 1 August 2018 at 08:43, Or Gerlitz  wrote:
> >> >>>
> >> >>> On Tue, Jul 31, 2018 at 1:40 PM, Or Gerlitz 
> >> >>> wrote:
> >> >>> > This series comes to address the case to set (encap) and match
> >> >>> > (decap)
> >> >>> > also the tos and ttl fields of TC based IP tunnels. This happens e.g
> >> >>> > due to inherit setup of tunnel port for tos or due to specific OF
> >> >>> > rule.
> >> >>> >
> >> >>> > The series is rebased over Jianbo's patches for QinQ [1]
> >> >>>
> >> >>> FWIW - note that v2 was actually rebased to the master where Jianbo's
> >> >>> work
> >> >>> is already applied
> >> >>
> >> >>
> >> >> I have also reviewed these patches, tested that travis-ci is happy with
> >> >> everything when applied on top of
> >> >> 185b13f228ac ("ovn: Add Meter and Meter_Band tables to the NB and SB
> >> >> databases."), which was the most recent
> >> >> travis-ci-clean commit in the master branch yesterday, and Netronome
> >> >> has
> >> >> performed some testing in the lab.
> >> >>
> >> >> Overall I am happy with these patches and plan to apply them later
> >> >> today
> >> >> after one final run through travis-ci after rebasing onto the current
> >> >> master
> >> >> branch (which is not travis-ci-clean :( [See: "Re: [ovs-dev] [PATCH v2
> >> >> 3/3]
> >> >> ovn-northd: Propagate dynamic addresses to port group address sets."]).
> >>
> >> > Thanks again Or, I have applied this series to master.
> >>
> >>
> >> Thank you.
> >>
> >> So how is the stable process @ ovs goes? is that documented, where?
> >> e.g b4496fc "lib/tc: Handle ttl for ipv6 too" is a bug fix, should/who I
> >> ask
> >> for stable inclusion?
> 
> > The usual procedure, as I understand, is to ask if the maintainer doesn't 
> > apply
> > the fix to the desired stable branches. I'll take the above as a request to
> > apply the patch to branch-2.10.
> > Do you want it considered for any other stable branches?
> 
> Hi Simon,
> 
> Yes, please do apply the ttl fix to 2.10 and if possible, to 2.9 as well since
> the bug was introduced there.
> 
> Also, it would be good if dfa2ccd "lib/tc: Support matching on ip tos"
> would also go to 2.10.
> I realized that commit 8f283af "netdev-tc-offloads: Implement netdev
> flow put using tc interface"
> has blindly set the tos field @ the mask to zero (see mask->nw_tos = 0
> in netdev_tc_flow_put)
> as if we offloaded that to the TC DP, but we didn't..

Thanks, I backported and pushed:

* to branch-2.9
  - b4496fc "lib/tc: Handle ttl for ipv6 too"

* to branch-2.10
  - b4496fc "lib/tc: Handle ttl for ipv6 too"
  - dfa2ccd "lib/tc: Support matching on ip tos"
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] ovn: Fix typos in "ovn -- Address Set generation..." test.

2018-08-02 Thread Simon Horman
On Wed, Aug 01, 2018 at 11:56:09AM +0200, Simon Horman wrote:
> On Tue, Jul 31, 2018 at 12:45:41PM -0700, Ben Pfaff wrote:
> > These caused the test to fail.
> > 
> > CC: Jakub Sitnicki 
> > Fixes: 984c7d5ea8fe ("ovn-northd: Propagate dynamic addresses to port group 
> > address sets.")
> > Signed-off-by: Ben Pfaff 
> > ---
> >  tests/ovn.at | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tests/ovn.at b/tests/ovn.at
> > index 17c740aa2352..1b25b6e4d6b9 100644
> > --- a/tests/ovn.at
> > +++ b/tests/ovn.at
> > @@ -10361,10 +10361,10 @@ ovn-nbctl set Logical_Switch ls1 \
> >  
> >  dnl Check if updated address got propagated to the port group address sets
> >  AT_CHECK([ovn-sbctl get Address_Set pg1_ip4 addresses],
> > - [0], [[["10.11.0.2", "10.2.0.2"]]
> > + [0], [[["10.1.0.2", "10.2.0.2"]]
> >  ])
> >  AT_CHECK([ovn-sbctl get Address_Set pg1_ip6 addresses],
> > - [0], [[["2001:db8:11::ff:fe00:1", "2001:db8:2::ff:fe00:2"]]
> > + [0], [[["2001:db8:1::ff:fe00:1", "2001:db8:2::ff:fe00:2"]]
> >  ])
> >  
> >  AT_CLEANUP
> 
> Reviewed-by: Simon Horman 
> Tested-by: Simon Horman 

I took the liberty of pushing this to master and branch-2.10
as I believe it allows those branches to pass all tests
covered by travis-ci.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH 5/5] netdev: Clean up class initialization.

2018-08-02 Thread Chandran, Sugesh
Hi Ian/Ben ,

Please find my comments below.

Regards
_Sugesh

> -Original Message-
> From: Stokes, Ian
> Sent: Wednesday, August 1, 2018 3:24 PM
> To: Ben Pfaff ; d...@openvswitch.org
> Cc: Chandran, Sugesh 
> Subject: Re: [ovs-dev] [PATCH 5/5] netdev: Clean up class initialization.
> 
> On 7/12/2018 10:55 PM, Ben Pfaff wrote:
> > The macros are hard to read.  This makes it a little more readable.
> >
> 
> Thanks for this Ben, one minor comment below.
> 
> > Signed-off-by: Ben Pfaff 
> > ---
> >
> > -#define DPDK_FLOW_OFFLOAD_API \
> > -NULL,   /* flow_flush */  \
> > -NULL,   /* flow_dump_create */\
> > -NULL,   /* flow_dump_destroy */   \
> > -NULL,   /* flow_dump_next */  \
> > -netdev_dpdk_flow_put, \
> > -NULL,   /* flow_get */\
> > -netdev_dpdk_flow_del, \
> > -NULL/* init_flow_api */
> 
> Not sure if DPDK_FLOW_OFFLOAD_API should be completely removed, as I
> understand it the remaining offload functionality is currently being
> worked on with a view to enable full HW offload so they will be
> re-introduced in the future.
> 
> The macro could be moved from here to netdev-dpdk.h and then added to
> the NETDEV_DPDK_CLASS_BASE macro you introduce below (this would be
> similar to what is implemented for netdev-linux, and a more uniform
> approach across the netdevs).
> 
> Sugesh, you've been involved in the HW full offload work, do you have an
> opinion on this?
> 
> Ian
[Sugesh] I agree with Ian here, We are looking at full offload solutions in DPDK
Datapath which need to use the remaining flow related APIs for the flow 
management in the
hardware. So I think it make sense to keep it uniform across different netdevs.

> > -
> > -
> > -#define NETDEV_DPDK_CLASS(NAME, INIT, CONSTRUCT, DESTRUCT,\
> > -  SET_CONFIG, SET_TX_MULTIQ, SEND,\
> > -  GET_CARRIER, GET_STATS,\
> > -  GET_CUSTOM_STATS,
> >   \
> > -  GET_FEATURES, GET_STATUS,   \
> > -  RECONFIGURE, RXQ_RECV)  \
> > -{ \

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


[ovs-dev] 答复: [ovs-discuss] a question about ovs crash relationship with learn action

2018-08-02 Thread wangyunjian
This patch has problem.
But when the cache has XC_LEARN entry,the cache can't be executed multiple times
in xlate_push_stats. The ofm's match->flow in XC_LEARN entry may use-after-free.

>-邮件原件-
>发件人: Ben Pfaff [mailto:b...@ovn.org]
>发送时间: 2018年7月11日 5:00
>收件人: wangyunjian 
>抄送: d...@openvswitch.org; ovs-disc...@openvswitch.org; Lilijun (Jerry, Cloud
>Networking) 
>主题: Re: [ovs-discuss] a question about ovs crash relationship with learn action
>
>Can you explain further?  Throwing away the cache entries makes the cache
>less useful.
>
>Are you really using v2.7.0?  None of the line numbers match up, either in the
>backtrace or in the patch.
>
>On Tue, Jun 26, 2018 at 10:38:50AM +, wangyunjian wrote:
>> I think the function xlate_cache_clear() needs be called after
>> xlate_push_stats() to avoid xcache->entries use-after-free.
>>
>> diff --git a/ofproto/ofproto-dpif-upcall.c
>> b/ofproto/ofproto-dpif-upcall.c index 85f5792..71b846c 100644
>> --- a/ofproto/ofproto-dpif-upcall.c
>> +++ b/ofproto/ofproto-dpif-upcall.c
>> @@ -2252,6 +2252,7 @@ revalidate_ukey(struct udpif *udpif, struct
>udpif_key *ukey,
>>  /* Stats for deleted flows will be attributed upon flow deletion. Skip. 
>> */
>>  if (result != UKEY_DELETE) {
>>  xlate_push_stats(ukey->xcache, );
>> +xlate_cache_clear(ukey->xcache);
>>  ukey->stats = *stats;
>>  ukey->reval_seq = reval_seq;
>>  }
>>
>> > From: wangyunjian
>> > Sent: Monday, June 25, 2018 2:19 PM
>> > To: ovs-disc...@openvswitch.org
>> > Cc: 'b...@ovn.org' ; Lilijun (Jerry, Cloud Networking)
>> > 
>> > Subject: [ovs-discuss] a question about ovs crash relationship with
>> > learn action
>> >
>> > I'm running OVS 2.7.0 on a Linux 3.10.0 kernel. I found a ovs crash.
>> > I doubt it's caused by use-after-free set match->flow = NULL in
>> > minimatch_destroy function with following stack:
>> >
>> > (gdb) bt
>> > #0  0x7ff273b71197 in raise () from /usr/lib64/libc.so.6
>> > #1  0x7ff273b72888 in abort () from /usr/lib64/libc.so.6
>> > #2  0x00787289 in PAT_abort ()
>> > #3  0x007843cd in patchIllInsHandler ()
>> > #4  
>> > #5  0x004cbfae in miniflow_n_values (flow=0x0) at
>> > lib/flow.h:540
>> > #6  0x004cc95f in minimask_hash (mask=0x0, basis=0) at
>> > lib/classifier-private.h:321
>> > #7  0x004cf613 in find_subtable (cls=0x38ad6e8, mask=0x0) at
>> > lib/classifier.c:1406
>> > #8  0x004cefa7 in classifier_find_rule_exactly
>> > (cls=0x38ad6e8, target=0x7ff118025500, version=18446744073709551615)
>> > at lib/classifier.c:1178
>> > #9  0x0047bcaf in collect_rules_strict (ofproto=0x389bc30,
>> > criteria=0x7ff1180254f8, rules=0x7ff118025588) at
>> > ofproto/ofproto.c:4253
>> > #10 0x0047eba3 in modify_flow_start_strict
>> > (ofproto=0x389bc30, ofm=0x7ff1180254f0) at ofproto/ofproto.c:5492
>> > #11 0x00482c9f in ofproto_flow_mod_start (ofproto=0x389bc30,
>> > ofm=0x7ff1180254f0) at ofproto/ofproto.c:7506
>> > #12 0x0047dc01 in ofproto_flow_mod_learn_start
>> > (ofm=0x7ff1180254f0) at ofproto/ofproto.c:5088
>> > #13 0x0047dd4b in ofproto_flow_mod_learn
>> > (ofm=0x7ff1180254f0, keep_ref=true) at ofproto/ofproto.c:5140
>> > #14 0x004b55d4 in xlate_push_stats_entry
>> > (entry=0x7ff118015148, stats=0x7ff11d6675f0) at
>> > ofproto/ofproto-dpif-xlate-cache.c:130
>> > #15 0x004b57b6 in xlate_push_stats (xcache=0x7ff1180254a0,
>> > stats=0x7ff11d6675f0) at ofproto/ofproto-dpif-xlate-cache.c:183
>> > #16 0x004a312f in revalidate_ukey (udpif=0x38a5260,
>> > ukey=0x7ff0fc015910, stats=0x7ff11d668260,
>> > odp_actions=0x7ff11d66a3d0, reval_seq=25145760,
>> > recircs=0x7ff11d66a3b0) at ofproto/ofproto-dpif-upcall.c:2134
>> > #17 0x004a3d76 in revalidate (revalidator=0x4cdda08) at
>> > ofproto/ofproto-dpif-upcall.c:2428
>> > #18 0x004a0528 in udpif_revalidator (arg=0x4cdda08) at
>> > ofproto/ofproto-dpif-upcall.c:954
>> > #19 0x0058f811 in ovsthread_wrapper (aux_=0x55088a0) at
>> > lib/ovs-thread.c:682
>> > #20 0x7ff27549adc5 in start_thread () from
>> > /usr/lib64/libpthread.so.0
>> >
>> > Any idea about this?
>> > Thanks,
>> > Yunjian
>>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] AUTHORS: Update email address for Jakub Sitnicki.

2018-08-02 Thread Jakub Sitnicki
Signed-off-by: Jakub Sitnicki 
---

The j...@redhat.com address will be valid only until August 4th.

 AUTHORS.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/AUTHORS.rst b/AUTHORS.rst
index 25ee39d75..f3ac8e32b 100644
--- a/AUTHORS.rst
+++ b/AUTHORS.rst
@@ -161,7 +161,7 @@ Isaku Yamahata yamah...@valinux.co.jp
 Ivan Dyukovi.dyu...@samsung.com
 IWASE Yusuke   iwase.yus...@gmail.com
 Jakub Libosvar libos...@redhat.com
-Jakub Sitnicki j...@redhat.com
+Jakub Sitnicki jsitni...@gmail.com
 James P.   roamp...@gmail.com
 James Page james.p...@ubuntu.com
 Jamie Lennox   jamielen...@gmail.com
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev