[ovs-dev] Bug#863228: openvswtich: CVE-2017-9214

2017-05-23 Thread Salvatore Bonaccorso 
Package: openvswitch
Version: 2.6.2~pre+git20161223-3
Severity: important
Tags: patch upstream security

Hi

the following vulnerability was published for openvswitch.  


   



   
CVE-2017-9214[0]:   


   
| In Open vSwitch (OvS) 2.7.0, while parsing an 


   
| OFPT_QUEUE_GET_CONFIG_REPLY type OFP 1.0 message, there is a buffer   


   
| over-read that is caused by an unsigned integer underflow in the  


   
| function `ofputil_pull_queue_get_config_reply10` in `lib/ofp-util.c`. 


   



   
The code around the ofputil_pull_queue_get_config_reply* functions has  


   
changed quite a bit since the version in stable, so I'm unsure if the   


   
issue si there as well. Needs confirmation since similar checks are 


   
done.   


   



   
If you fix the vulnerability please also make sure to include the   


   
CVE (Common Vulnerabilities & Exposures) id in your changelog entry.


   



   
For further information see:


   

[0]

[ovs-dev] debian /etc/init.d/openvswitch status

2017-05-23 Thread Raymond Burkholder 
Using a Debian Stretch daily snapshot from a few days ago, systemd looks to be 
interfering with /etc/init.d/openvswitch.

These are the packages installed:

# dpkg -l | grep openvsw
ii  openvswitch-common2.6.2~pre+git20161223-3 amd64Open 
vSwitch common components
ii  openvswitch-switch2.6.2~pre+git20161223-3 amd64Open 
vSwitch switch implementations
ii  python-openvswitch2.6.2~pre+git20161223-3 all  
Python bindings for Open vSwitch

# uname -a
Linux server 4.9.0-3-amd64 #1 SMP Debian 4.9.25-1 (2017-05-02) x86_64 GNU/Linux

'/etc/init.d/openvswitch status’ will now generate a systemd based status 
instead of the ovs-ctl status:

 /etc/init.d/openvswitch-switch status
* openvswitch-switch.service - LSB: Open vSwitch switch
   Loaded: loaded (/etc/init.d/openvswitch-switch; generated; vendor preset: 
enabled)
   Active: active (running) since Wed 2017-05-24 00:52:45 UTC; 5min ago
 Docs: man:systemd-sysv-generator(8)
  Process: 584 ExecStart=/etc/init.d/openvswitch-switch start (code=exited, 
status=0/SUCCESS)
Tasks: 9 (limit: 4915)
   Memory: 53.4M
  CPU: 2.069s
   CGroup: /system.slice/openvswitch-switch.service

…… and etc.


This interferes with the status request in:

# grep -n  status /usr/share/openvswitch/scripts/ifupdown.sh

35:if /etc/init.d/openvswitch-switch status > /dev/null 2>&1; then :; else
36-/etc/init.d/openvswitch-switch start

As a quick and dirty work around, I changed it to:

# grep -n status /usr/share/openvswitch/scripts/ifupdown.sh

35:if /etc/init.d/openvswitch-switch ovsstatus > /dev/null 2>&1; then :; else
36-/etc/init.d/openvswitch-switch start

As a result, I had to perform a selective s/status/ovsstatus/ in:

the init file:
/etc/init.d/openvswitch

as well as:
 grep -n status /usr/share/openvswitch/scripts/*
/usr/share/openvswitch/scripts/ifupdown.sh:35:if /etc/init.d/openvswitch-switch 
ovsstatus > /dev/null 2>&1; then :; else
/usr/share/openvswitch/scripts/ovs-ctl:699:ovsstatus)
/usr/share/openvswitch/scripts/ovs-lib:52:"ovsstatus”)



As part of these changes for systemd related systems, it may also be suggested 
to add /lib/systemd/system/openvswitch-nonetwork.service as suggested at:

https://bugs.launchpad.net/ubuntu/+source/openvswitch/+bug/1448254/comments/5

=

All this then allows an /etc/network/interfaces file like the following to be 
properly processed:

source /etc/network/interfaces.d/*

# The loopback network interface
auto lo
iface lo inet loopback

allow-ovs ovsbr0
iface ovsbr0 inet static
address 10.1.1.10
netmask 255.255.255.0
gateway 10.1.1.1
ovs_type OVSBridge
ovs_ports eno1

allow-ovsbr0 eno1
iface eno1 inet manual
ovs_bridge ovsbr0
ovs_type OVSPort


-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.

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

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

2017-05-23 Thread Joe Stringer 
Many standard library functions are wrapped in OVS, so check for usage
of the original versions and suggest that authors replace them with the
OVS versions.

Signed-off-by: Joe Stringer 
---
v2: Drop checks for functions that don't replace library functions
Fix naming of functions xfoo() -> ovs_foo() where appropriate
Fix descriptions
---
 utilities/checkpatch.py | 28 
 1 file changed, 28 insertions(+)

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

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

[ovs-dev] [PATCH 5/5] test-hash: Reuse structs/functions in 256B check.

2017-05-23 Thread Joe Stringer 
The prior patch introduced a definition for the 32-bit offset u128, and
introduced a function for setting bits in this structure so refactor
the 256B hash test to reuse this code.

Signed-off-by: Joe Stringer 
---
 tests/test-hash.c | 25 +++--
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/tests/test-hash.c b/tests/test-hash.c
index 704e4bc72ff9..9e2b1d840763 100644
--- a/tests/test-hash.c
+++ b/tests/test-hash.c
@@ -28,7 +28,7 @@
 OVS_PACKED(struct offset_ovs_u128
 {
 uint32_t a;
-ovs_u128 b;
+ovs_u128 b[16];
 });
 
 static void
@@ -66,9 +66,9 @@ set_bit128_unaligned(struct offset_ovs_u128 *values, int bit, 
int n_bits)
 memset(values, 0, n_bits/8 + sizeof(values->a));
 
 if (b < 64) {
-values->b.u64.lo = UINT64_C(1) << (b % 64);
+values->b[bit / 128].u64.lo = UINT64_C(1) << (b % 64);
 } else {
-values->b.u64.hi = UINT64_C(1) << (b % 64);
+values->b[bit / 128].u64.hi = UINT64_C(1) << (b % 64);
 }
 }
 
@@ -175,10 +175,10 @@ check_hash_bytes128(void (*hash)(const void *, size_t, 
uint32_t, ovs_u128 *),
 uint32_t *in0p;
 ovs_u128 in1;
 
-in0p = [0];
+in0p = [0].u32[0];
 set_bit128_unaligned(, i, n_bits);
 set_bit128(, i, n_bits);
-assert(ovs_u128_equals(in0.b, in1));
+assert(ovs_u128_equals(in0.b[0], in1));
 hash(in0p, sizeof(ovs_u128), 0, );
 hash(, sizeof(ovs_u128), 0, );
 if (!ovs_u128_equals(out0, out1)) {
@@ -221,17 +221,14 @@ check_256byte_hash(void (*hash)(const void *, size_t, 
uint32_t, ovs_u128 *),
 int i, j;
 
 for (i = 0; i < n_bits; i++) {
-OVS_PACKED(struct offset_ovs_u128 {
-uint32_t a;
-ovs_u128 b[16];
-}) in0_data;
-ovs_u128 *in0, in1[16];
+struct offset_ovs_u128 in0;
+ovs_u128 in1[16];
 ovs_u128 out0, out1;
 
-in0 = in0_data.b;
-set_bit128(in0, i, n_bits);
-set_bit128(in1, i, n_bits);
-hash(in0, sizeof(ovs_u128) * 16, 0, );
+set_bit128_unaligned(, i, n_bits);
+set_bit128([0], i, n_bits);
+assert(ovs_u128_equals(in0.b[i / 128], in1[i / 128]));
+hash(, sizeof(ovs_u128) * 16, 0, );
 hash(in1, sizeof(ovs_u128) * 16, 0, );
 if (!ovs_u128_equals(out0, out1)) {
 printf("%s hash not the same for non-64 aligned data "
-- 
2.12.2

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

[ovs-dev] [PATCH 4/5] test-hash: Fix unaligned pointers.

2017-05-23 Thread Joe Stringer 
Clang 4.0 complains:

../tests/test-hash.c:160:16: error: taking address of packed member 'b' of
class or structure 'offset_ovs_u128' may result in an unaligned pointer value
  [-Werror,-Waddress-of-packed-member]
in0 = _data.b;

Rework the 128-bit hash test to have a separate function for setting
bits in the 32-bit offset u128 structure.

Signed-off-by: Joe Stringer 
---
 tests/test-hash.c | 36 
 1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/tests/test-hash.c b/tests/test-hash.c
index a20c87fad6a0..704e4bc72ff9 100644
--- a/tests/test-hash.c
+++ b/tests/test-hash.c
@@ -25,6 +25,12 @@
 #include "jhash.h"
 #include "ovstest.h"
 
+OVS_PACKED(struct offset_ovs_u128
+{
+uint32_t a;
+ovs_u128 b;
+});
+
 static void
 set_bit(uint32_t array[3], int bit)
 {
@@ -51,6 +57,21 @@ set_bit128(ovs_u128 *values, int bit, int n_bits)
 }
 }
 
+static void
+set_bit128_unaligned(struct offset_ovs_u128 *values, int bit, int n_bits)
+{
+int b = bit % 128;
+
+assert(bit >= 0 && bit < 2048);
+memset(values, 0, n_bits/8 + sizeof(values->a));
+
+if (b < 64) {
+values->b.u64.lo = UINT64_C(1) << (b % 64);
+} else {
+values->b.u64.hi = UINT64_C(1) << (b % 64);
+}
+}
+
 static uint64_t
 get_range128(ovs_u128 *value, int ofs, uint64_t mask)
 {
@@ -149,17 +170,16 @@ check_hash_bytes128(void (*hash)(const void *, size_t, 
uint32_t, ovs_u128 *),
 int i, j;
 
 for (i = 0; i < n_bits; i++) {
-OVS_PACKED(struct offset_ovs_u128 {
-uint32_t a;
-ovs_u128 b;
-}) in0_data;
-ovs_u128 *in0, in1;
+struct offset_ovs_u128 in0;
 ovs_u128 out0, out1;
+uint32_t *in0p;
+ovs_u128 in1;
 
-in0 = _data.b;
-set_bit128(in0, i, n_bits);
+in0p = [0];
+set_bit128_unaligned(, i, n_bits);
 set_bit128(, i, n_bits);
-hash(in0, sizeof(ovs_u128), 0, );
+assert(ovs_u128_equals(in0.b, in1));
+hash(in0p, sizeof(ovs_u128), 0, );
 hash(, sizeof(ovs_u128), 0, );
 if (!ovs_u128_equals(out0, out1)) {
 printf("%s hash not the same for non-64 aligned data "
-- 
2.12.2

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

[ovs-dev] [PATCH 3/5] test-hash: Don't check bit 2048.

2017-05-23 Thread Joe Stringer 
When running 256B hash check, we currently iterate from 0 up to and
including bit 2048, which is beyond the range of bits that 256B holds.
For bit 2048, set_bit128() doesn't set a bit due to the range check.
Simplify the code by dropping the handling of bit 2048.

Signed-off-by: Joe Stringer 
---
 tests/test-hash.c | 23 +++
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/tests/test-hash.c b/tests/test-hash.c
index d1beead36ed5..a20c87fad6a0 100644
--- a/tests/test-hash.c
+++ b/tests/test-hash.c
@@ -39,16 +39,15 @@ set_bit(uint32_t array[3], int bit)
 static void
 set_bit128(ovs_u128 *values, int bit, int n_bits)
 {
-assert(bit >= 0 && bit <= 2048);
+int b = bit % 128;
+
+assert(bit >= 0 && bit < 2048);
 memset(values, 0, n_bits/8);
-if (bit < n_bits) {
-int b = bit % 128;
 
-if (b < 64) {
-values[bit / 128].u64.lo = UINT64_C(1) << (b % 64);
-} else {
-values[bit / 128].u64.hi = UINT64_C(1) << (b % 64);
-}
+if (b < 64) {
+values[bit / 128].u64.lo = UINT64_C(1) << (b % 64);
+} else {
+values[bit / 128].u64.hi = UINT64_C(1) << (b % 64);
 }
 }
 
@@ -149,7 +148,7 @@ check_hash_bytes128(void (*hash)(const void *, size_t, 
uint32_t, ovs_u128 *),
 const int n_bits = sizeof(ovs_u128) * 8;
 int i, j;
 
-for (i = 0; i <= n_bits; i++) {
+for (i = 0; i < n_bits; i++) {
 OVS_PACKED(struct offset_ovs_u128 {
 uint32_t a;
 ovs_u128 b;
@@ -168,7 +167,7 @@ check_hash_bytes128(void (*hash)(const void *, size_t, 
uint32_t, ovs_u128 *),
name, out0.u64.lo, out0.u64.hi, out1.u64.lo, out1.u64.hi);
 }
 
-for (j = i + 1; j <= n_bits; j++) {
+for (j = i + 1; j < n_bits; j++) {
 ovs_u128 in2;
 ovs_u128 out2;
 int ofs;
@@ -201,7 +200,7 @@ check_256byte_hash(void (*hash)(const void *, size_t, 
uint32_t, ovs_u128 *),
 const int n_bits = sizeof(ovs_u128) * 8 * 16;
 int i, j;
 
-for (i = 0; i <= n_bits; i++) {
+for (i = 0; i < n_bits; i++) {
 OVS_PACKED(struct offset_ovs_u128 {
 uint32_t a;
 ovs_u128 b[16];
@@ -220,7 +219,7 @@ check_256byte_hash(void (*hash)(const void *, size_t, 
uint32_t, ovs_u128 *),
name, out0.u64.lo, out0.u64.hi, out1.u64.lo, out1.u64.hi);
 }
 
-for (j = i + 1; j <= n_bits; j++) {
+for (j = i + 1; j < n_bits; j++) {
 ovs_u128 in2[16];
 ovs_u128 out2;
 
-- 
2.12.2

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

[ovs-dev] [PATCH 1/5] odp-execute: Fix unaligned eth_addr pointers.

2017-05-23 Thread Joe Stringer 
Clang 4.0 complains:

../lib/odp-execute.c:61:37: error: taking address of packed member 'eth_dst' of
class or structure 'eth_header' may result in an unaligned pointer value
  [-Werror,-Waddress-of-packed-member]
ether_addr_copy_masked(>eth_src, key->eth_src, mask->eth_src);
^~~
../lib/odp-execute.c:62:37: error: taking address of packed member 'eth_dst' of
class or structure 'eth_header' may result in an unaligned pointer value
  [-Werror,-Waddress-of-packed-member]
ether_addr_copy_masked(>eth_dst, key->eth_dst, mask->eth_dst);

Ethernet source addresses are 48 bits offset into the Ethernet header,
so taking a pointer for this is not guaranteed to be valid on all
architectures. Fix this by referencing the memory direct from the
Ethernet header pointer.

Signed-off-by: Joe Stringer 
---
 lib/odp-execute.c | 19 +--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index 08bc50a46a2e..f0dadf56c422 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -48,6 +48,21 @@ ether_addr_copy_masked(struct eth_addr *dst, const struct 
eth_addr src,
 }
 
 static void
+ether_addrs_copy_masked(struct eth_header *hdr,
+const struct eth_addr src, const struct eth_addr smask,
+const struct eth_addr dst, const struct eth_addr dmask)
+{
+int i;
+
+for (i = 0; i < ARRAY_SIZE(src.be16); i++) {
+hdr->eth_src.be16[i] = src.be16[i]
+ | (hdr->eth_src.be16[i] & ~smask.be16[i]);
+hdr->eth_dst.be16[i] = dst.be16[i]
+ | (hdr->eth_dst.be16[i] & ~dmask.be16[i]);
+}
+}
+
+static void
 odp_eth_set_addrs(struct dp_packet *packet, const struct ovs_key_ethernet *key,
   const struct ovs_key_ethernet *mask)
 {
@@ -58,8 +73,8 @@ odp_eth_set_addrs(struct dp_packet *packet, const struct 
ovs_key_ethernet *key,
 eh->eth_src = key->eth_src;
 eh->eth_dst = key->eth_dst;
 } else {
-ether_addr_copy_masked(>eth_src, key->eth_src, mask->eth_src);
-ether_addr_copy_masked(>eth_dst, key->eth_dst, mask->eth_dst);
+ether_addrs_copy_masked(eh, key->eth_src, mask->eth_src,
+key->eth_dst, mask->eth_dst);
 }
 }
 }
-- 
2.12.2

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

[ovs-dev] [PATCH 2/5] ofproto-dpif: Fix unaligned eth_addr pointers.

2017-05-23 Thread Joe Stringer 
Clang 4.0 complains:

../ofproto/ofproto-dpif.c:2291:46: error: taking address of packed member
'eth_src' of class or structure 'eth_header' may result in an unaligned pointer
value
  [-Werror,-Waddress-of-packed-member]
netdev_get_etheraddr(ofport->up.netdev, >eth_src);
 ^~~~
../ofproto/ofproto-dpif.c:2316:50: error: taking address of packed member
'eth_src' of class or structure 'eth_header' may result in an unaligned pointer
value
  [-Werror,-Waddress-of-packed-member]
netdev_get_etheraddr(ofport->up.netdev, >eth_src);

Ethernet source addresses are 48 bits offset into the Ethernet header,
so taking a pointer for this is not guaranteed to be valid on all
architectures. Fix this by retrieving the mac address to the local stack
and copying it into the header.

Signed-off-by: Joe Stringer 
---
 ofproto/ofproto-dpif.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index dc5f004cd92d..5b866427f406 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -2287,13 +2287,15 @@ rstp_send_bpdu_cb(struct dp_packet *pkt, void *ofport_, 
void *ofproto_)
 struct ofproto_dpif *ofproto = ofproto_;
 struct ofport_dpif *ofport = ofport_;
 struct eth_header *eth = dp_packet_eth(pkt);
+struct eth_addr mac;
 
-netdev_get_etheraddr(ofport->up.netdev, >eth_src);
-if (eth_addr_is_zero(eth->eth_src)) {
+netdev_get_etheraddr(ofport->up.netdev, );
+if (eth_addr_is_zero(mac)) {
 VLOG_WARN_RL(, "%s port %d: cannot send RSTP BPDU on a port which "
  "does not have a configured source MAC address.",
  ofproto->up.name, ofp_to_u16(ofport->up.ofp_port));
 } else {
+eth->eth_src = mac;
 ofproto_dpif_send_packet(ofport, false, pkt);
 }
 dp_packet_delete(pkt);
@@ -2312,12 +2314,14 @@ send_bpdu_cb(struct dp_packet *pkt, int port_num, void 
*ofproto_)
  ofproto->up.name, port_num);
 } else {
 struct eth_header *eth = dp_packet_eth(pkt);
+struct eth_addr mac;
 
-netdev_get_etheraddr(ofport->up.netdev, >eth_src);
-if (eth_addr_is_zero(eth->eth_src)) {
+netdev_get_etheraddr(ofport->up.netdev, );
+if (eth_addr_is_zero(mac)) {
 VLOG_WARN_RL(, "%s: cannot send BPDU on port %d "
  "with unknown MAC", ofproto->up.name, port_num);
 } else {
+eth->eth_src = mac;
 ofproto_dpif_send_packet(ofport, false, pkt);
 }
 }
-- 
2.12.2

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

[ovs-dev] [PATCH 0/5] Fix unaligned pointer values reported by clang 4.0.

2017-05-23 Thread Joe Stringer 
Clang 4.0 has added some new warnings around taking the address of packed
members of structures which may result in unaligned pointer values. This series
addresses the resulting compilation failures (reported via -Werror).

Joe Stringer (5):
  odp-execute: Fix unaligned eth_addr access.
  ofproto-dpif: Fix unaligned eth_addr access.
  test-hash: Don't check bit 2048.
  test-hash: Fix unaligned memory access.
  test-hash: Reuse structs/functions in 256B check.

 lib/odp-execute.c  | 19 +++--
 ofproto/ofproto-dpif.c | 12 +---
 tests/test-hash.c  | 74 ++
 3 files changed, 70 insertions(+), 35 deletions(-)

-- 
2.12.2

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

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

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

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

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

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

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

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

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

Re: [ovs-dev] [PATCH] datapath-windows: Fix possible null dereference in ipfragment

2017-05-23 Thread Guru Shetty 
On 17 May 2017 at 06:54, Alin Serdean 
wrote:

> Found using static analysis tools.
>
> Signed-off-by: Alin Gabriel Serdean 
>
Applied, thanks!


> ---
>  datapath-windows/ovsext/IpFragment.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/datapath-windows/ovsext/IpFragment.c
> b/datapath-windows/ovsext/IpFragment.c
> index 675c32e..0874cb9 100644
> --- a/datapath-windows/ovsext/IpFragment.c
> +++ b/datapath-windows/ovsext/IpFragment.c
> @@ -343,7 +343,7 @@ OvsProcessIpv4Fragment(POVS_SWITCH_CONTEXT
> switchContext,
>  }
>  POVS_FRAGMENT_LIST next = entry->head;
>  POVS_FRAGMENT_LIST prev = entry->tail;
> -if (prev != NULL || prev->offset < offset) {
> +if (prev != NULL && prev->offset < offset) {
>  next = NULL;
>  goto found;
>  }
> --
> 2.10.2.windows.1
> ___
> 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] datapath-windows: Report success for conntrack actions over frags

2017-05-23 Thread Guru Shetty 
On 17 May 2017 at 06:43, Alin Serdean 
wrote:

> When a conntrack action is applied over an IP fragment we pend the fragment
> which will be consumed later. This should be transparent to the userspace.
>
> Report that the action was applied successfully so it does not spam
> the ovs-vswitchd log.
>
> Signed-off-by: Alin Gabriel Serdean 
>
Thanks, applied!


> ---
>  datapath-windows/ovsext/Actions.c | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/datapath-windows/ovsext/Actions.c b/datapath-windows/ovsext/
> Actions.c
> index ebfb8a3..31b4514 100644
> --- a/datapath-windows/ovsext/Actions.c
> +++ b/datapath-windows/ovsext/Actions.c
> @@ -2032,6 +2032,11 @@ OvsDoExecuteActions(POVS_SWITCH_CONTEXT
> switchContext,
>  if (status != NDIS_STATUS_PENDING) {
>  OVS_LOG_ERROR("CT Action failed");
>  dropReason = L"OVS-conntrack action failed";
> +} else {
> +/* We added a new pending NBL to be consumed later.
> + * Report to the userspace that the action applied
> + * successfully */
> +status = NDIS_STATUS_SUCCESS;
>  }
>  goto dropit;
>  } else if (oldNbl != ovsFwdCtx.curNbl) {
> --
> 2.10.2.windows.1
> ___
> 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] datapath-windows: Fix alignment in actions

2017-05-23 Thread Guru Shetty 
On 17 May 2017 at 05:57, Alin Serdean 
wrote:

> Found by inspection.
>
> Signed-off-by: Alin Gabriel Serdean 
>

Thanks, applied!



> ---
>  datapath-windows/ovsext/Actions.c | 24 
>  1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/datapath-windows/ovsext/Actions.c b/datapath-windows/ovsext/
> Actions.c
> index e2eae9a..ebfb8a3 100644
> --- a/datapath-windows/ovsext/Actions.c
> +++ b/datapath-windows/ovsext/Actions.c
> @@ -2035,18 +2035,18 @@ OvsDoExecuteActions(POVS_SWITCH_CONTEXT
> switchContext,
>  }
>  goto dropit;
>  } else if (oldNbl != ovsFwdCtx.curNbl) {
> -   /*
> -* OvsIpv4Reassemble consumes the original NBL and creates
> a
> -* new one and assigns it to the curNbl of ovsFwdCtx.
> -*/
> -   OvsInitForwardingCtx(,
> -ovsFwdCtx.switchContext,
> -ovsFwdCtx.curNbl,
> -ovsFwdCtx.srcVportNo,
> -ovsFwdCtx.sendFlags,
> -NET_BUFFER_LIST_SWITCH_
> FORWARDING_DETAIL(ovsFwdCtx.curNbl),
> -ovsFwdCtx.completionList,
> -, FALSE);
> +/*
> + * OvsIpv4Reassemble consumes the original NBL and
> creates a
> + * new one and assigns it to the curNbl of ovsFwdCtx.
> + */
> +OvsInitForwardingCtx(,
> + ovsFwdCtx.switchContext,
> + ovsFwdCtx.curNbl,
> + ovsFwdCtx.srcVportNo,
> + ovsFwdCtx.sendFlags,
> + NET_BUFFER_LIST_SWITCH_
> FORWARDING_DETAIL(ovsFwdCtx.curNbl),
> + ovsFwdCtx.completionList,
> + , FALSE);
>  }
>  break;
>  }
> --
> 2.10.2.windows.1
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

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

Thanks Alin for the fix and Joe for the review. It looks good to me.

Acked-by: Andy Zhou 

I will push it in a few minutes.

> ---
> v2: move 'on_exit' after 'ovs-testcontroller' creation (Joe Stringer)
> ---
>  tests/bridge.at | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tests/bridge.at b/tests/bridge.at
> index cc7619d..f6c2327 100644
> --- a/tests/bridge.at
> +++ b/tests/bridge.at
> @@ -48,7 +48,8 @@ OVS_VSWITCHD_START(
>  set bridge br1 datapath-type=dummy other-config:datapath-id=1234 ])
>
>  dnl Start ovs-testcontroller
> -AT_CHECK([ovs-testcontroller --detach punix:controller], [0], [ignore])
> +AT_CHECK([ovs-testcontroller --detach punix:controller --pidfile], [0], 
> [ignore])
> +on_exit 'kill `cat ovs-testcontroller.pid`'
>  OVS_WAIT_UNTIL([test -e controller])
>
>  dnl Add the controller to both bridges, 5 seconds apart.
> --
> 2.10.2.windows.1
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] python ovs: Fix SSL exceptions with pyOpenSSL v0.13

2017-05-23 Thread Numan Siddique 
On May 23, 2017 11:04 PM, "Russell Bryant"  wrote:

On Mon, May 15, 2017 at 11:39 AM,   wrote:
> From: Numan Siddique 
>
> Centos provides pyOpenSSL version pyOpenSSL-0.13.1-3.el7.x86_64.
> There are 2 issues using this version, which this patch fixes
>
>  - The test case "simple idl verify notify - SSL" is skipped.
>This is because "python -m OpenSSL.SSL" is used to detect the
>presence of pyOpenSSL package. pyOpenSSL v0.13 has C python
>modules because of which the above command returns 1.
>So this patch fixes this by using 'python -c "import OpenSSL.SSL"'.
>
>  - The SSL.Context class do not have the function "set_session_cache_mode"
>defined. Setting the session cache mode has an effect for server-side
>sessions and doesn't make much sense for client-side sessions.
>Since python ovs doesn't support "pssl" connection mode, this patch
>deletes the reference to this function.

I'd like to tweak this wording a bit.  Setting the cache mode in
general *does* have an effect for client side connections, as there is
a client mode you can set it to.  However, the usage being removed was
only relevant for server side connections so it was a no-op for our
client-only usage.  I'll tweak the commit message because I'm sure you
won't mind.  :-)


Please do so :). Thanks for the proper commit message.

Numan


Otherwise, this looks good to me and I'll apply it to master and branch-2.7.

Thanks!

>
> I have not tested with older versions (< 0.13) of pyOpenSSL.
>
> Signed-off-by: Numan Siddique 
> ---
>  python/ovs/stream.py | 1 -
>  tests/ovsdb-idl.at   | 2 +-
>  2 files changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/python/ovs/stream.py b/python/ovs/stream.py
> index fc0368c..660d8bb 100644
> --- a/python/ovs/stream.py
> +++ b/python/ovs/stream.py
> @@ -767,7 +767,6 @@ class SSLStream(Stream):
>  ctx = SSL.Context(SSL.SSLv23_METHOD)
>  ctx.set_verify(SSL.VERIFY_PEER, SSLStream.verify_cb)
>  ctx.set_options(SSL.OP_NO_SSLv2 | SSL.OP_NO_SSLv3)
> -ctx.set_session_cache_mode(SSL.SESS_CACHE_OFF)
>  # If the client has not set the SSL configuration files
>  # exception would be raised.
>  ctx.use_privatekey_file(Stream._SSL_private_key_file)
> diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
> index d28dfc1..4eaf87f 100644
> --- a/tests/ovsdb-idl.at
> +++ b/tests/ovsdb-idl.at
> @@ -1185,7 +1185,7 @@ m4_define([OVSDB_CHECK_IDL_NOTIFY_SSL_PY],
>[AT_SETUP([$1 - SSL])
> AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
> AT_SKIP_IF([test $HAVE_PYTHON = no])
> -   $PYTHON -m OpenSSL.SSL
> +   $PYTHON -c "import OpenSSL.SSL"
> SSL_PRESENT=$?
> AT_SKIP_IF([test $SSL_PRESENT != 0])
> AT_KEYWORDS([ovsdb server idl Python notify - ssl socket])
> --
> 2.9.3
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev



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

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

2017-05-23 Thread Andy Zhou 
On Mon, May 22, 2017 at 9:12 PM, Numan Siddique  wrote:
>
>
> On Tue, May 23, 2017 at 5:21 AM, Andy Zhou  wrote:
>>
>> On Sun, May 21, 2017 at 6:35 PM,   wrote:
>> > From: Numan Siddique 
>> >
>> > start action returns OCF_RUNNING_MASTER in certain scenarios.
>> > But as per the OCF guidelines, status code OCF_RUNNING_MASTER shoud
>> > be returned only in monitor action [1].
>> >
>> > Whenever the start action returns OCF_RUNNING_MASTER, it is observed
>> > in the testing that, pacemaker stops the ovsdb-server ocf resource
>> > in that node. This patch fixes this issue by returning OCF_SUCESS in
>> > such cases.
>> >
>> > stop action returns OCF_RUNNING_MASTER if the ovsdb-servers are
>> > running as master. But as per the OCF guidelines [2], stop action
>> > should only return OCF_SUCCESS. If any other code is returned,
>> > pacemaker cluster would block that resource in that node.
>> >
>> > This patch fixes this issue by stopping the ovsdb-servers when they
>> > are running as masters (which is the expected case) and returns
>> > OCF_SUCCESS.
>> >
>> > [1] -
>> > http://www.linux-ha.org/doc/dev-guides/_literal_ocf_running_master_literal_8.html
>> > [2] -
>> > http://www.linux-ha.org/doc/dev-guides/_literal_stop_literal_action.html
>> >
>> > CC: Andy Zhou 
>> > Signed-off-by: Numan Siddique 
>>
>> Thanks for the fixes!  Both patches look reasonable to me. I pushed
>> them to master.
>
>
> Thanks Andy. Can these patches be back ported to  branch 2.7 ? It would be
> great since the tripleo patches for OVN needs these fixes
>
> Numan
Done. Thanks for the reminder.
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 2/3] datapath-windows: add two new build targets for code analysis

2017-05-23 Thread Alin Serdean 
Add two new build targets: 'Win8Analyze' and 'Win8.1Analyze'.
The new build targets have the static code analyzer (built in Visual
Studio feature).

This patch also introduces a new make target ('datapath_windows_analyze')
this can be added to the CI jobs to get a list warnings/errors issued
by the code analyzer.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/Package/package.VcxProj  | 28 
 datapath-windows/Package/package.VcxProj.user |  6 +++
 datapath-windows/automake.mk  |  4 ++
 datapath-windows/ovsext.sln   | 14 ++
 datapath-windows/ovsext/ovsext.vcxproj| 62 +++
 datapath-windows/ovsext/ovsext.vcxproj.user   |  6 +++
 6 files changed, 120 insertions(+)

diff --git a/datapath-windows/Package/package.VcxProj 
b/datapath-windows/Package/package.VcxProj
index 24bb914..4d6db59 100644
--- a/datapath-windows/Package/package.VcxProj
+++ b/datapath-windows/Package/package.VcxProj
@@ -25,6 +25,14 @@
   Win8 Release
   x64
 
+
+  Win8.1Analyze
+  x64
+
+
+  Win8Analyze
+  x64
+
   
   
 Utility
@@ -46,6 +54,11 @@
 true
 WindowsKernelModeDriver8.1
   
+  
+WindowsV6.3
+true
+WindowsKernelModeDriver8.1
+  
   
 
 
@@ -58,6 +71,11 @@
 true
 WindowsKernelModeDriver8.1
   
+  
+Windows8
+true
+WindowsKernelModeDriver8.1
+  
   
 
 
@@ -105,6 +123,11 @@
   true
 
   
+  
+
+  true
+
+  
   
 
   true
@@ -115,6 +138,11 @@
   true
 
   
+  
+
+  true
+
+  
   
 
   true
diff --git a/datapath-windows/Package/package.VcxProj.user 
b/datapath-windows/Package/package.VcxProj.user
index 75ea9d7..6231d93 100644
--- a/datapath-windows/Package/package.VcxProj.user
+++ b/datapath-windows/Package/package.VcxProj.user
@@ -3,9 +3,15 @@
   
 TestSign
   
+  
+TestSign
+  
   
 TestSign
   
+  
+TestSign
+  
   
 TestSign
   
diff --git a/datapath-windows/automake.mk b/datapath-windows/automake.mk
index 4f7b55a..49011f1 100644
--- a/datapath-windows/automake.mk
+++ b/datapath-windows/automake.mk
@@ -82,3 +82,7 @@ EXTRA_DIST += \
datapath-windows/ovsext/precomp.h \
datapath-windows/ovsext/precompsrc.c \
datapath-windows/ovsext/resource.h
+
+datapath_windows_analyze: all
+   MSBuild.exe //maxcpucount datapath-windows/ovsext.sln /target:Build 
/property:Configuration="Win8.1Analyze"
+   MSBuild.exe //maxcpucount datapath-windows/ovsext.sln /target:Build 
/property:Configuration="Win8Analyze"
diff --git a/datapath-windows/ovsext.sln b/datapath-windows/ovsext.sln
index 6cf441b..1000104 100644
--- a/datapath-windows/ovsext.sln
+++ b/datapath-windows/ovsext.sln
@@ -14,8 +14,10 @@ Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Win10Debug|x64 = Win10Debug|x64
Win10Release|x64 = Win10Release|x64
+   Win8.1Analyze|x64 = Win8.1Analyze|x64
Win8.1Debug|x64 = Win8.1Debug|x64
Win8.1Release|x64 = Win8.1Release|x64
+   Win8Analyze|x64 = Win8Analyze|x64
Win8Debug|x64 = Win8Debug|x64
Win8Release|x64 = Win8Release|x64
EndGlobalSection
@@ -26,10 +28,16 @@ Global

{911D7389-3E61-449F-B8F3-14AD7EE9A0F2}.Win10Release|x64.ActiveCfg = Win10 
Release|x64
{911D7389-3E61-449F-B8F3-14AD7EE9A0F2}.Win10Release|x64.Build.0 
= Win10 Release|x64

{911D7389-3E61-449F-B8F3-14AD7EE9A0F2}.Win10Release|x64.Deploy.0 = Win10 
Release|x64
+   
{911D7389-3E61-449F-B8F3-14AD7EE9A0F2}.Win8.1Analyze|x64.ActiveCfg = 
Win8.1Analyze|x64
+   
{911D7389-3E61-449F-B8F3-14AD7EE9A0F2}.Win8.1Analyze|x64.Build.0 = 
Win8.1Analyze|x64
+   
{911D7389-3E61-449F-B8F3-14AD7EE9A0F2}.Win8.1Analyze|x64.Deploy.0 = 
Win8.1Analyze|x64

{911D7389-3E61-449F-B8F3-14AD7EE9A0F2}.Win8.1Debug|x64.ActiveCfg = Win8.1 
Debug|x64
{911D7389-3E61-449F-B8F3-14AD7EE9A0F2}.Win8.1Debug|x64.Build.0 
= Win8.1 Debug|x64

{911D7389-3E61-449F-B8F3-14AD7EE9A0F2}.Win8.1Release|x64.ActiveCfg = Win8.1 
Release|x64

{911D7389-3E61-449F-B8F3-14AD7EE9A0F2}.Win8.1Release|x64.Build.0 = Win8.1 
Release|x64
+   
{911D7389-3E61-449F-B8F3-14AD7EE9A0F2}.Win8Analyze|x64.ActiveCfg = 
Win8Analyze|x64
+   {911D7389-3E61-449F-B8F3-14AD7EE9A0F2}.Win8Analyze|x64.Build.0 
= Win8Analyze|x64
+   {911D7389-3E61-449F-B8F3-14AD7EE9A0F2}.Win8Analyze|x64.Deploy.0 
= Win8Analyze|x64
{911D7389-3E61-449F-B8F3-14AD7EE9A0F2}.Win8Debug|x64.ActiveCfg 
= Win8 Debug|x64
{911D7389-3E61-449F-B8F3-14AD7EE9A0F2}.Win8Debug|x64.Build.0 = 
Win8 Debug|x64

{911D7389-3E61-449F-B8F3-14AD7EE9A0F2}.Win8Release|x64.ActiveCfg = Win8 
Release|x64
@@ -40,12

[ovs-dev] [PATCH 3/3] appveyor: Add new make target

2017-05-23 Thread Alin Serdean 
This patch adds the new make target 'datapath_windows_analyze' (static
analysis over the windows datapath code) to the appveyor build.

Signed-off-by: Alin Gabriel Serdean 
---
 appveyor.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/appveyor.yml b/appveyor.yml
index edcead6..6ca1479 100644
--- a/appveyor.yml
+++ b/appveyor.yml
@@ -43,3 +43,4 @@ build_script:
 - C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch && ./boot.sh"
 - C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch && ./configure 
CC=build-aux/cccl LD=\"`which link`\" LIBS=\"-lws2_32 -liphlpapi -lwbemuuid 
-lole32 -loleaut32\" --with-pthread=C:/pthreads-win32/Pre-built.2 
--with-openssl=C:/OpenSSL-Win32 --with-vstudiotarget=\"Debug\""
 - C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch && make"
+- C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch && make 
datapath_windows_analyze"
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 1/3] datapath-windows: Remove Strsafe usage from datapath

2017-05-23 Thread Alin Serdean 
The removal is mandatory to use the VStudio 2013 static code analyzer.

The only function that was used from the include is: 'StringCbLengthA'.
We were not checking the result of that function, nor will the
'vportGet->name' exceed the 'OVS_MAX_PORT_NAME_LENGTH' limitation.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/Netlink/NetlinkBuf.c | 1 -
 datapath-windows/ovsext/Vport.c  | 2 --
 datapath-windows/ovsext/precomp.h| 1 -
 3 files changed, 4 deletions(-)

diff --git a/datapath-windows/ovsext/Netlink/NetlinkBuf.c 
b/datapath-windows/ovsext/Netlink/NetlinkBuf.c
index 0177e88..639b6e5 100644
--- a/datapath-windows/ovsext/Netlink/NetlinkBuf.c
+++ b/datapath-windows/ovsext/Netlink/NetlinkBuf.c
@@ -40,7 +40,6 @@
 #include 
 #include 
 #include 
-#include 
 
 #ifdef OVS_DBG_MOD
 #undef OVS_DBG_MOD
diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c
index 9142937..075f419 100644
--- a/datapath-windows/ovsext/Vport.c
+++ b/datapath-windows/ovsext/Vport.c
@@ -1618,7 +1618,6 @@ OvsGetExtInfoIoctl(POVS_VPORT_GET vportGet,
POVS_VPORT_EXT_INFO extInfo)
 {
 POVS_VPORT_ENTRY vport;
-size_t len;
 LOCK_STATE_EX lockState;
 NTSTATUS status = STATUS_SUCCESS;
 BOOLEAN doConvert = FALSE;
@@ -1626,7 +1625,6 @@ OvsGetExtInfoIoctl(POVS_VPORT_GET vportGet,
 RtlZeroMemory(extInfo, sizeof (POVS_VPORT_EXT_INFO));
 NdisAcquireRWLockRead(gOvsSwitchContext->dispatchLock, , 0);
 if (vportGet->portNo == 0) {
-StringCbLengthA(vportGet->name, OVS_MAX_PORT_NAME_LENGTH - 1, );
 vport = OvsFindVportByHvNameA(gOvsSwitchContext, vportGet->name);
 if (vport == NULL) {
 /* If the port is not a Hyper-V port and it has been added earlier,
diff --git a/datapath-windows/ovsext/precomp.h 
b/datapath-windows/ovsext/precomp.h
index a152582..14f6843 100644
--- a/datapath-windows/ovsext/precomp.h
+++ b/datapath-windows/ovsext/precomp.h
@@ -19,7 +19,6 @@
 #include 
 #include 
 #include 
-#include 
 
 #include "Types.h"
 
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

2017-05-23 Thread Russell Bryant 
On Tue, May 23, 2017 at 9:56 AM, Numan Siddique  wrote:
> On Tue, May 23, 2017 at 9:44 AM, multi_t...@163.com 
> wrote:
>
>> Hi Numan,
>>
>> Thank you very much,  I had pushed the patch new version
>> https://github.com/openvswitch/ovs/pull/178
>>
>
> Thanks. I tested it.
>
> Acked-by: Numan Siddique 
> Tested-by: Numan Siddique 
>
> I can post the patch to the mailing list if you want

Thanks both of you.  The patch is missing a Signed-off-by header from
the author.  Otherwise, I think it's ready.

>
> Thanks
> Numan
>
>
>> Thanks
>> Zhengwei
>>
>> --
>> multi_t...@163.com
>>
>>
>> *From:* Numan Siddique 
>> *Date:* 2017-05-22 15:33
>> *To:* 高正伟 
>> *CC:* Ben Pfaff ; ovs dev 
>> *Subject:* Re: Re: [ovs-dev] [PATCH] Supporting ovn-northd service HA
>> depend on OVNDB-HA
>>
>>
>> On Fri, May 19, 2017 at 7:21 AM, 高正伟  wrote:
>>
>>> Thanks Numan
>>>
>>> I think that supporting  OVNDB and ovn-northd HA  depend on one pacemaker
>>> is an idea,   In case of thinking OVN-HA,  user will not individually care
>>> about OVNDB or ovn-northd HA.
>>>
>>>
>> Hi Zhangwei,
>>
>> I do agree with you. But what I am trying to say is to make it
>> configurable.
>>
>> I tested with your patch again with tripleo and it is having few issues.
>> 1. Since you are not passing "--ovn-manage-ovsdb=no", when "ovn-ctl
>> stop_northd" is called, it also stops ovsdb-servers. We probably don't want
>> to stop ovsdb-servers unless, pacemaker calls "stop" action.
>>
>> 2. When a master node is demoted, ovn-northd is not stopped since "ovn-ctl
>> stop_northd" code is not hit.
>>
>> I tested with the below modifications and applying this patch -
>> https://patchwork.ozlabs.org/patch/765224/. It is working as expected.
>> Can you please test on your side and resubmit another version if it is fine
>> with you.
>>
>> If the user wants to manage ovn_northd using the ocf resources, he could
>> pass manage_northd=yes while creating the resource.
>>
>>
>> ---cut here -
>>
>> diff --git a/ovn/utilities/ovndb-servers.ocf
>> b/ovn/utilities/ovndb-servers.ocf
>> index 40c5541..018d904 100755
>> --- a/ovn/utilities/ovndb-servers.ocf
>> +++ b/ovn/utilities/ovndb-servers.ocf
>> @@ -7,6 +7,7 @@
>>  : ${NB_MASTER_PROTO_DEFAULT="tcp"}
>>  : ${SB_MASTER_PORT_DEFAULT="6642"}
>>  : ${SB_MASTER_PROTO_DEFAULT="tcp"}
>> +: ${MANAGE_NORTHD_DEFAULT="no"}
>>  CRM_MASTER="${HA_SBIN_DIR}/crm_master -l reboot"
>>  CRM_ATTR_REPL_INFO="${HA_SBIN_DIR}/crm_attribute --type crm_config
>> --name OVN_REPL_INFO -s ovn_ovsdb_master_server"
>>  OVN_CTL=${OCF_RESKEY_ovn_ctl:-${OVN_CTL_DEFAULT}}
>> @@ -15,6 +16,7 @@ NB_MASTER_PORT=${OCF_RESKEY_nb_master_port:-${NB_MASTER_
>> PORT_DEFAULT}}
>>  NB_MASTER_PROTO=${OCF_RESKEY_nb_master_protocol:-${NB_
>> MASTER_PROTO_DEFAULT}}
>>  SB_MASTER_PORT=${OCF_RESKEY_sb_master_port:-${SB_MASTER_PORT_DEFAULT}}
>>  SB_MASTER_PROTO=${OCF_RESKEY_sb_master_protocol:-${SB_
>> MASTER_PROTO_DEFAULT}}
>> +MANAGE_NORTHD=${OCF_RESKEY_manage_northd:-${MANAGE_NORTHD_DEFAULT}}
>>
>>  # Invalid IP address is an address that can never exist in the network, as
>>  # mentioned in rfc-5737. The ovsdb servers connects to this IP address
>> till
>> @@ -90,6 +92,14 @@ ovsdb_server_metadata() {
>>
>>
>>
>> +  
>> +  
>> +  If set to yes, manages ovn-northd service. ovn-northd will be started in
>> +  the master node.
>> +  
>> +  manage ovn-northd service
>> +  
>> +  
>>
>>
>>
>> @@ -122,12 +132,17 @@ ovsdb_server_notify() {
>>  # the right thing at startup
>>  ocf_log debug "ovndb_server: $host_name is the master"
>>  ${CRM_ATTR_REPL_INFO} -v "$host_name"
>> -# Startup ovn-northd service
>> -${OVN_CTL} start_northd
>> +if [ "$MANAGE_NORTHD" = "yes" ]; then
>> +# Startup ovn-northd service
>> +${OVN_CTL} --ovn-manage-ovsdb=no start_northd
>> +fi
>>
>>  else
>> -# Stop ovn-northd service
>> -${OVN_CTL} stop_northd
>> +if [ "$MANAGE_NORTHD" = "yes" ]; then
>> +# Stop ovn-northd service. Set --ovn-manage-ovsdb=no so that
>> +# ovn-ctl doesn't stop ovsdb-servers.
>> +${OVN_CTL} --ovn-manage-ovsdb=no stop_northd
>> +fi
>>  # Synchronize with the new master
>>  ocf_log debug "ovndb_server: Connecting to the new master
>> ${OCF_RESKEY_CRM_meta_notify_promote_uname}"
>>  ${OVN_CTL} demote_ovnnb --db-nb-sync-from-addr=${MASTER_IP} \
>> @@ -289,6 +304,13 @@ ovsdb_server_start() {
>>  }
>>
>>  ovsdb_server_stop() {
>> +if [ "$MANAGE_NORTHD" = "yes" ]; then
>> +# Stop ovn-northd service in case it was running. This is required
>> +# when the master is demoted. For other cases, it would be a
>> no-op.
>> +

Re: [ovs-dev] [PATCH v2] python ovs: Fix SSL exceptions with pyOpenSSL v0.13

2017-05-23 Thread Russell Bryant 
On Mon, May 15, 2017 at 11:39 AM,   wrote:
> From: Numan Siddique 
>
> Centos provides pyOpenSSL version pyOpenSSL-0.13.1-3.el7.x86_64.
> There are 2 issues using this version, which this patch fixes
>
>  - The test case "simple idl verify notify - SSL" is skipped.
>This is because "python -m OpenSSL.SSL" is used to detect the
>presence of pyOpenSSL package. pyOpenSSL v0.13 has C python
>modules because of which the above command returns 1.
>So this patch fixes this by using 'python -c "import OpenSSL.SSL"'.
>
>  - The SSL.Context class do not have the function "set_session_cache_mode"
>defined. Setting the session cache mode has an effect for server-side
>sessions and doesn't make much sense for client-side sessions.
>Since python ovs doesn't support "pssl" connection mode, this patch
>deletes the reference to this function.

I'd like to tweak this wording a bit.  Setting the cache mode in
general *does* have an effect for client side connections, as there is
a client mode you can set it to.  However, the usage being removed was
only relevant for server side connections so it was a no-op for our
client-only usage.  I'll tweak the commit message because I'm sure you
won't mind.  :-)

Otherwise, this looks good to me and I'll apply it to master and branch-2.7.

Thanks!

>
> I have not tested with older versions (< 0.13) of pyOpenSSL.
>
> Signed-off-by: Numan Siddique 
> ---
>  python/ovs/stream.py | 1 -
>  tests/ovsdb-idl.at   | 2 +-
>  2 files changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/python/ovs/stream.py b/python/ovs/stream.py
> index fc0368c..660d8bb 100644
> --- a/python/ovs/stream.py
> +++ b/python/ovs/stream.py
> @@ -767,7 +767,6 @@ class SSLStream(Stream):
>  ctx = SSL.Context(SSL.SSLv23_METHOD)
>  ctx.set_verify(SSL.VERIFY_PEER, SSLStream.verify_cb)
>  ctx.set_options(SSL.OP_NO_SSLv2 | SSL.OP_NO_SSLv3)
> -ctx.set_session_cache_mode(SSL.SESS_CACHE_OFF)
>  # If the client has not set the SSL configuration files
>  # exception would be raised.
>  ctx.use_privatekey_file(Stream._SSL_private_key_file)
> diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
> index d28dfc1..4eaf87f 100644
> --- a/tests/ovsdb-idl.at
> +++ b/tests/ovsdb-idl.at
> @@ -1185,7 +1185,7 @@ m4_define([OVSDB_CHECK_IDL_NOTIFY_SSL_PY],
>[AT_SETUP([$1 - SSL])
> AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
> AT_SKIP_IF([test $HAVE_PYTHON = no])
> -   $PYTHON -m OpenSSL.SSL
> +   $PYTHON -c "import OpenSSL.SSL"
> SSL_PRESENT=$?
> AT_SKIP_IF([test $SSL_PRESENT != 0])
> AT_KEYWORDS([ovsdb server idl Python notify - ssl socket])
> --
> 2.9.3
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev



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

Re: [ovs-dev] Nice To Meet you

2017-05-23 Thread Sara Patrick 
Hi, can we discuss something very important?
Yours Love
Sara Patrick





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

[ovs-dev] Netapp users' contact list

2017-05-23 Thread Celia Wolfe 
Hi,

 

Want to increase your sales? All you need is a reliable data source. Get
accurate data and reach your target easily. we have updated Netapp users'
contact list and thought you would be interested in it.

 

We can also provide you with:

* Seagate
Technology
* Western Digital
*
Brocade
* Imation
* Quantum
*   And many more.

 

We provide decision makers' contacts of companies using the above
technologies

Thank you for your time. Look forward to hear from you.

 

Regards,

Celia Wolfe 



To opt-out reply with "leave out" in the subject line.

 

 

 

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

Re: [ovs-dev] [PATCH v7 3/4] datapath-windows: NAT integration with conntrack

2017-05-23 Thread Alin Serdean 
Hi Yin,

Sorry it took a while to review the patch.

I just have a few inlined comments. I am stripping the code a bit to be more 
readable.

I will run some tests tonight over the current series to see that everything is 
ok from a functionality perspective.

Thanks,
Alin.

> 
> This patch integrates NAT module with existing conntrack module. NAT
> action is now supported.
> 
> Signed-off-by: Yin Lin 
> diff --git a/datapath-windows/automake.mk b/datapath-
> windows/automake.mk index 7177630..f1632cc 100644
> --- a/datapath-windows/automake.mk
> +++ b/datapath-windows/automake.mk
> @@ -16,9 +16,9 @@ EXTRA_DIST += \
>   datapath-windows/ovsext/Conntrack-icmp.c \
>   datapath-windows/ovsext/Conntrack-other.c \
>   datapath-windows/ovsext/Conntrack-related.c \
> -datapath-windows/ovsext/Conntrack-nat.c \
> + datapath-windows/ovsext/Conntrack-nat.c \
>   datapath-windows/ovsext/Conntrack-tcp.c \
> -datapath-windows/ovsext/Conntrack-nat.h \
> + datapath-windows/ovsext/Conntrack-nat.h \
[Alin Serdean] Just use tab instead of 4 spaces in patch 2/4
>   datapath-windows/ovsext/Conntrack.c \
>   datapath-windows/ovsext/Conntrack.h \
>   datapath-windows/ovsext/Datapath.c \
[Alin Serdean] <== cut >
> +
> +/* NatInfo is always initialized to be disabled, so that if NAT action
> + * fails, we will not end up deleting an non-existent NAT entry.
> + */
> +if (natInfo != NULL && OvsIsForwardNat(natInfo->natAction)) {
> +entry->natInfo = *natInfo;
> +if (!OvsNatCtEntry(entry)) {
> +return FALSE;
> +}
> +ctx->hash = OvsHashCtKey(>key);
> +} else {
> +entry->natInfo.natAction = natInfo->natAction;
[Alin Serdean] Shouldn't this be guarded for natInfo==NULL?
> +}
> +
[Alin Serdean] <== cut >
> -OvsCtUpdateFlowKey(key, state, ctx->key.zone, 0, NULL);
> -return entry;
> +break;
>  }
>  default:
>  goto invalid;
>  }
> 
> +if (commit && !entry) {
> +return NULL;
> +}
> +if (entry && !OvsCtAddEntry(entry, ctx, natInfo, currentTime)) {
[Alin Serdean] Shouldn't we delete the 'entry' here if OvsCtAddEntry failed?
> +return NULL;
> +}
> +OvsCtUpdateFlowKey(key, state, ctx->key.zone, 0, NULL);
> +return entry;
>  invalid:
>  state |= OVS_CS_F_INVALID;
>  OvsCtUpdateFlowKey(key, state, ctx->key.zone, 0, NULL); @@ -269,11
> +289,11 @@ invalid:
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v7 2/4] datapath-windows: Add NAT module in conntrack

2017-05-23 Thread Alin Serdean 
Just one small nit on this one
>   datapath-windows/ovsext/Conntrack-icmp.c \
>   datapath-windows/ovsext/Conntrack-other.c \
>   datapath-windows/ovsext/Conntrack-related.c \
> +datapath-windows/ovsext/Conntrack-nat.c \
[Alin Serdean] tab instead of 4 space
>   datapath-windows/ovsext/Conntrack-tcp.c \
> +datapath-windows/ovsext/Conntrack-nat.h \
[Alin Serdean] tab instead of 4 space
>   datapath-windows/ovsext/Conntrack.c \
>   datapath-windows/ovsext/Conntrack.h \
>   datapath-windows/ovsext/Datapath.c \
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

2017-05-23 Thread Alin Serdean 
From: Alin Serdean 

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

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

Signed-off-by: Alin Gabriel Serdean 
---
v2: move 'on_exit' after 'ovs-testcontroller' creation (Joe Stringer)
---
 tests/bridge.at | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

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

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

2017-05-23 Thread Alin Serdean 


> -Original Message-
> From: Joe Stringer [mailto:j...@ovn.org]
> Sent: Tuesday, May 23, 2017 2:39 AM
> To: Alin Serdean 
> Cc: d...@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH] tests: fix hanging test on windows
> 
> On 22 May 2017 at 05:18, Alin Serdean 
> wrote:
> > 'multiple bridges share a controller' hangs on windows because it is
> > lacking the exit information (it will hang when the test has finished)
> >
> > Introduce a pidfile to 'ovs-testcontroller' and end it on exit based
> > on the pidfile.
> >
> > Signed-off-by: Alin Gabriel Serdean 
> 
> Hi Alin,
> 
> "on_exit" will queue up a command to be run at the end of the test run,
> regardless of success or failure. As such, I think that this should be run
> immediately after the launch of ovs-testcontroller.
> Otherwise it's possible that something else in the test fails before the end,
> and ovs-testcontroller is not cleaned up.
[Alin Serdean] Thanks for the review Joe!
I was thinking about only the "happy path".
I will send a new revision in which I will address the comments.

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

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

2017-05-23 Thread Numan Siddique 
On Tue, May 23, 2017 at 9:44 AM, multi_t...@163.com 
wrote:

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

Thanks. I tested it.

Acked-by: Numan Siddique 
Tested-by: Numan Siddique 

I can post the patch to the mailing list if you want

Thanks
Numan


> Thanks
> Zhengwei
>
> --
> multi_t...@163.com
>
>
> *From:* Numan Siddique 
> *Date:* 2017-05-22 15:33
> *To:* 高正伟 
> *CC:* Ben Pfaff ; ovs dev 
> *Subject:* Re: Re: [ovs-dev] [PATCH] Supporting ovn-northd service HA
> depend on OVNDB-HA
>
>
> On Fri, May 19, 2017 at 7:21 AM, 高正伟  wrote:
>
>> Thanks Numan
>>
>> I think that supporting  OVNDB and ovn-northd HA  depend on one pacemaker
>> is an idea,   In case of thinking OVN-HA,  user will not individually care
>> about OVNDB or ovn-northd HA.
>>
>>
> Hi Zhangwei,
>
> I do agree with you. But what I am trying to say is to make it
> configurable.
>
> I tested with your patch again with tripleo and it is having few issues.
> 1. Since you are not passing "--ovn-manage-ovsdb=no", when "ovn-ctl
> stop_northd" is called, it also stops ovsdb-servers. We probably don't want
> to stop ovsdb-servers unless, pacemaker calls "stop" action.
>
> 2. When a master node is demoted, ovn-northd is not stopped since "ovn-ctl
> stop_northd" code is not hit.
>
> I tested with the below modifications and applying this patch -
> https://patchwork.ozlabs.org/patch/765224/. It is working as expected.
> Can you please test on your side and resubmit another version if it is fine
> with you.
>
> If the user wants to manage ovn_northd using the ocf resources, he could
> pass manage_northd=yes while creating the resource.
>
>
> ---cut here -
>
> diff --git a/ovn/utilities/ovndb-servers.ocf
> b/ovn/utilities/ovndb-servers.ocf
> index 40c5541..018d904 100755
> --- a/ovn/utilities/ovndb-servers.ocf
> +++ b/ovn/utilities/ovndb-servers.ocf
> @@ -7,6 +7,7 @@
>  : ${NB_MASTER_PROTO_DEFAULT="tcp"}
>  : ${SB_MASTER_PORT_DEFAULT="6642"}
>  : ${SB_MASTER_PROTO_DEFAULT="tcp"}
> +: ${MANAGE_NORTHD_DEFAULT="no"}
>  CRM_MASTER="${HA_SBIN_DIR}/crm_master -l reboot"
>  CRM_ATTR_REPL_INFO="${HA_SBIN_DIR}/crm_attribute --type crm_config
> --name OVN_REPL_INFO -s ovn_ovsdb_master_server"
>  OVN_CTL=${OCF_RESKEY_ovn_ctl:-${OVN_CTL_DEFAULT}}
> @@ -15,6 +16,7 @@ NB_MASTER_PORT=${OCF_RESKEY_nb_master_port:-${NB_MASTER_
> PORT_DEFAULT}}
>  NB_MASTER_PROTO=${OCF_RESKEY_nb_master_protocol:-${NB_
> MASTER_PROTO_DEFAULT}}
>  SB_MASTER_PORT=${OCF_RESKEY_sb_master_port:-${SB_MASTER_PORT_DEFAULT}}
>  SB_MASTER_PROTO=${OCF_RESKEY_sb_master_protocol:-${SB_
> MASTER_PROTO_DEFAULT}}
> +MANAGE_NORTHD=${OCF_RESKEY_manage_northd:-${MANAGE_NORTHD_DEFAULT}}
>
>  # Invalid IP address is an address that can never exist in the network, as
>  # mentioned in rfc-5737. The ovsdb servers connects to this IP address
> till
> @@ -90,6 +92,14 @@ ovsdb_server_metadata() {
>
>
>
> +  
> +  
> +  If set to yes, manages ovn-northd service. ovn-northd will be started in
> +  the master node.
> +  
> +  manage ovn-northd service
> +  
> +  
>
>
>
> @@ -122,12 +132,17 @@ ovsdb_server_notify() {
>  # the right thing at startup
>  ocf_log debug "ovndb_server: $host_name is the master"
>  ${CRM_ATTR_REPL_INFO} -v "$host_name"
> -# Startup ovn-northd service
> -${OVN_CTL} start_northd
> +if [ "$MANAGE_NORTHD" = "yes" ]; then
> +# Startup ovn-northd service
> +${OVN_CTL} --ovn-manage-ovsdb=no start_northd
> +fi
>
>  else
> -# Stop ovn-northd service
> -${OVN_CTL} stop_northd
> +if [ "$MANAGE_NORTHD" = "yes" ]; then
> +# Stop ovn-northd service. Set --ovn-manage-ovsdb=no so that
> +# ovn-ctl doesn't stop ovsdb-servers.
> +${OVN_CTL} --ovn-manage-ovsdb=no stop_northd
> +fi
>  # Synchronize with the new master
>  ocf_log debug "ovndb_server: Connecting to the new master
> ${OCF_RESKEY_CRM_meta_notify_promote_uname}"
>  ${OVN_CTL} demote_ovnnb --db-nb-sync-from-addr=${MASTER_IP} \
> @@ -289,6 +304,13 @@ ovsdb_server_start() {
>  }
>
>  ovsdb_server_stop() {
> +if [ "$MANAGE_NORTHD" = "yes" ]; then
> +# Stop ovn-northd service in case it was running. This is required
> +# when the master is demoted. For other cases, it would be a
> no-op.
> +# Set --ovn-manage-ovsdb=no so that ovn-ctl doesn't stop
> ovsdb-servers.
> +${OVN_CTL} --ovn-manage-ovsdb=no stop_northd
> +fi
> +
>  ovsdb_server_check_status
>  case $? in
>  $OCF_NOT_RUNNING)return ${OCF_SUCCESS};;
>
> --
>
>
>

Re: [ovs-dev] [PATCH v4] OVN localport type support

2017-05-23 Thread Daniel Alvarez Sanchez 
On Tue, May 23, 2017 at 10:01 AM, Miguel Angel Ajo Pelayo <
majop...@redhat.com> wrote:

> If we forsee use cases with several local ports by logical switch/chassis
> could one option be to allocate a bit in REG10 to mark local ports,
> and then have a single rule that matches reg10 to drop output/forwarding
> of packets?
>
> I like the idea... let's see what others say about this, I don't know how
strict we want to be consuming
bits from registers.
Thanks Miguel for the suggestion :)



> Best,
> Miguel Ángel Ajo
>
>
>
> On Fri, May 19, 2017 at 4:26 PM, Daniel Alvarez Sanchez <
> dalva...@redhat.com> wrote:
>
>> Thanks a lot Ben for your review!
>>
>> On Fri, May 19, 2017 at 12:16 AM, Ben Pfaff  wrote:
>>
>> > On Wed, May 10, 2017 at 08:35:45AM +, Daniel Alvarez wrote:
>> > > This patch introduces a new type of OVN ports called "localport".
>> > > These ports will be present in every hypervisor and may have the
>> > > same IP/MAC addresses. They are not bound to any chassis and traffic
>> > > to these ports will never go through a tunnel.
>> > >
>> > > Its main use case is the OpenStack metadata API support which relies
>> > > on a local agent running on every hypervisor and serving metadata to
>> > > VM's locally. This service is described in detail at [0].
>> > >
>> > > An example to illustrate the purpose of this patch:
>> > >
>> > > - One logical switch sw0 with 2 ports (p1, p2) and 1 localport (lp)
>> > > - Two hypervisors: HV1 and HV2
>> > > - p1 in HV1 (OVS port with external-id:iface-id="p1")
>> > > - p2 in HV2 (OVS port with external-id:iface-id="p2")
>> > > - lp in both hypevisors (OVS port with external-id:iface-id="lp")
>> > > - p1 should be able to reach p2 and viceversa
>> > > - lp on HV1 should be able to reach p1 but not p2
>> > > - lp on HV2 should be able to reach p2 but not p1
>> > >
>> > > Explicit drop rules are inserted in table 32 with priority 150
>> > > in order to prevent traffic originated at a localport to go over
>> > > a tunnel.
>> > >
>> > > [0] https://review.openstack.org/#/c/452811/
>> > >
>> > > Signed-off-by: Daniel Alvarez 
>> >
>> > Thanks for working on this!
>> >
>> > This seems reasonable, but I'm torn about one aspect of it.  I'm not
>> > sure whether my concern is a kind of premature optimization or not, so
>> > let me just describe it and we can discuss.
>> >
>> > This adds code that iterates over every local lport (suppose there are N
>> > of them), which is nested inside a function that is executed for every
>> > port relevant to the local hypervisor (suppose there are M of them).
>> > That's O(N*M) time.  But the inner loop is only doing something useful
>> > for localport logical ports, and normally there would only be 1 or so of
>> > those; at any rate a constant number.  So in theory this could run in
>> > O(M) time.
>> >
>> > I see at least two ways to fix the problem, if it's a problem, but I
>> > don't know whether it's worth fixing.  Daniel?  Russell?  (Others?)
>> >
>> > Yes, I also thought about this but I don't know either if it's a problem
>> or not.
>> If we want to impose at most one logical localport per datapath then we
>> would have O(M) time (mimic localnet ports) but that's something I didn't
>> want to do unless you think it makes more sense.
>>
>>
>> > Thanks,
>> >
>> > Ben.
>> >
>> ___
>> dev mailing list
>> d...@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v4] OVN localport type support

2017-05-23 Thread Miguel Angel Ajo Pelayo 
If we forsee use cases with several local ports by logical switch/chassis
could one option be to allocate a bit in REG10 to mark local ports,
and then have a single rule that matches reg10 to drop output/forwarding
of packets?

Best,
Miguel Ángel Ajo



On Fri, May 19, 2017 at 4:26 PM, Daniel Alvarez Sanchez  wrote:

> Thanks a lot Ben for your review!
>
> On Fri, May 19, 2017 at 12:16 AM, Ben Pfaff  wrote:
>
> > On Wed, May 10, 2017 at 08:35:45AM +, Daniel Alvarez wrote:
> > > This patch introduces a new type of OVN ports called "localport".
> > > These ports will be present in every hypervisor and may have the
> > > same IP/MAC addresses. They are not bound to any chassis and traffic
> > > to these ports will never go through a tunnel.
> > >
> > > Its main use case is the OpenStack metadata API support which relies
> > > on a local agent running on every hypervisor and serving metadata to
> > > VM's locally. This service is described in detail at [0].
> > >
> > > An example to illustrate the purpose of this patch:
> > >
> > > - One logical switch sw0 with 2 ports (p1, p2) and 1 localport (lp)
> > > - Two hypervisors: HV1 and HV2
> > > - p1 in HV1 (OVS port with external-id:iface-id="p1")
> > > - p2 in HV2 (OVS port with external-id:iface-id="p2")
> > > - lp in both hypevisors (OVS port with external-id:iface-id="lp")
> > > - p1 should be able to reach p2 and viceversa
> > > - lp on HV1 should be able to reach p1 but not p2
> > > - lp on HV2 should be able to reach p2 but not p1
> > >
> > > Explicit drop rules are inserted in table 32 with priority 150
> > > in order to prevent traffic originated at a localport to go over
> > > a tunnel.
> > >
> > > [0] https://review.openstack.org/#/c/452811/
> > >
> > > Signed-off-by: Daniel Alvarez 
> >
> > Thanks for working on this!
> >
> > This seems reasonable, but I'm torn about one aspect of it.  I'm not
> > sure whether my concern is a kind of premature optimization or not, so
> > let me just describe it and we can discuss.
> >
> > This adds code that iterates over every local lport (suppose there are N
> > of them), which is nested inside a function that is executed for every
> > port relevant to the local hypervisor (suppose there are M of them).
> > That's O(N*M) time.  But the inner loop is only doing something useful
> > for localport logical ports, and normally there would only be 1 or so of
> > those; at any rate a constant number.  So in theory this could run in
> > O(M) time.
> >
> > I see at least two ways to fix the problem, if it's a problem, but I
> > don't know whether it's worth fixing.  Daniel?  Russell?  (Others?)
> >
> > Yes, I also thought about this but I don't know either if it's a problem
> or not.
> If we want to impose at most one logical localport per datapath then we
> would have O(M) time (mimic localnet ports) but that's something I didn't
> want to do unless you think it makes more sense.
>
>
> > Thanks,
> >
> > Ben.
> >
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev