[ovs-dev] Mail System Error - Returned Mail

[jon]

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] Add some more flake8 types to ignore list to fix the compilation errors

[Numan Siddique]

with the flake8 check enabled, ovs compilation is failing. This
patch adds few more flake8 types to the igore list.

Signed-off-by: Numan Siddique 
---
 Makefile.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index 8b6ddb7..cb73ca6 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -349,7 +349,7 @@ ALL_LOCAL += flake8-check
 # E129 visually indented line with same indent as next logical line
 # E131 continuation line unaligned for hanging indent
 flake8-check: $(FLAKE8_PYFILES)
-   $(AM_V_GEN) if flake8 $^ --ignore=E123,E126,E127,E128,E129,E131 
${FLAKE8_FLAGS}; then touch $@; else exit 1; fi
+   $(AM_V_GEN) if flake8 $^ 
--ignore=E123,E126,E127,E128,E129,E131,D100,D101,D102,D103,D104,D105,D200,D202,D204,D205,D207,D208,D209,D210,D400,D401,H104,H201,H231,H232,H233,H238,H301,H306,H401,H403,H404,H405
 ${FLAKE8_FLAGS}; then touch $@; else exit 1; fi
 endif
 
 include $(srcdir)/manpages.mk
-- 
2.5.0

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] Add some more flake8 types to ignore list to fix the compilation errors

[Numan Siddique]

I guess this patch is not required, once Russel's patches on - Python style 
fixes and flake8 integration
are completely merged.


Thanks
Numan

On 01/06/2016 04:59 PM, Numan Siddique wrote:
> with the flake8 check enabled, ovs compilation is failing. This
> patch adds few more flake8 types to the igore list.
>
> Signed-off-by: Numan Siddique 
> ---
>  Makefile.am | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Makefile.am b/Makefile.am
> index 8b6ddb7..cb73ca6 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -349,7 +349,7 @@ ALL_LOCAL += flake8-check
>  # E129 visually indented line with same indent as next logical line
>  # E131 continuation line unaligned for hanging indent
>  flake8-check: $(FLAKE8_PYFILES)
> - $(AM_V_GEN) if flake8 $^ --ignore=E123,E126,E127,E128,E129,E131 
> ${FLAKE8_FLAGS}; then touch $@; else exit 1; fi
> + $(AM_V_GEN) if flake8 $^ 
> --ignore=E123,E126,E127,E128,E129,E131,D100,D101,D102,D103,D104,D105,D200,D202,D204,D205,D207,D208,D209,D210,D400,D401,H104,H201,H231,H232,H233,H238,H301,H306,H401,H403,H404,H405
>  ${FLAKE8_FLAGS}; then touch $@; else exit 1; fi
>  endif
>  
>  include $(srcdir)/manpages.mk

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] How is the label value, TTL, mpls_tc and bos get inserted into the first mpls label in openvSwitch?

[Pynbiang Hadem]

How is the label value, TTL, mpls_tc and bos get inserted into the first mpls 
label in openvSwitch?. I had looked into the OpenvSwitch flow_push_mpls (in 
openvswitch/lib/flow.c) class function but could not find the appropriate 
coding for generating the above mpls label fields.

Pls help, I need to find the correct source code for the above.

Thanks
Regards
Hadem.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH monitor_cond 11/12] python: move Python idl to work with monitor_cond

[Numan Siddique]

On Tue, Jan 5, 2016 at 6:44 PM, Liran Schour  wrote:

> Python idl works now with "monitor_cond" method. Add test
> for backward compatibility with old "monitor" method.
>
> Signed-off-by: Liran Schour 
> ---
>  python/ovs/db/data.py |  12 
>  python/ovs/db/idl.py  | 161
> +++---
>  tests/ovsdb-idl.at|  97 ++
>  3 files changed, 261 insertions(+), 9 deletions(-)
>
> diff --git a/python/ovs/db/data.py b/python/ovs/db/data.py
> index 6baff38..0d382e1 100644
> --- a/python/ovs/db/data.py
> +++ b/python/ovs/db/data.py
> @@ -386,6 +386,18 @@ class Datum(object):
>  s.append(tail)
>  return ''.join(s)
>
> +def diff(self, datum):
> +if self.type.n_max > 1 or len(self.values) == 0:
> +for k, v in datum.values.iteritems():
> +if k in self.values and v == self.values[k]:
> +del self.values[k]
> +else:
> +self.values[k] = v
> +else:
> +return datum
> +
> +return self
> +
>  def as_list(self):
>  if self.type.is_map():
>  return [[k.value, v.value] for k, v in
> self.values.iteritems()]
> diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py
> index c8990c7..2f0895c 100644
> --- a/python/ovs/db/idl.py
> +++ b/python/ovs/db/idl.py
> @@ -30,6 +30,9 @@ ROW_CREATE = "create"
>  ROW_UPDATE = "update"
>  ROW_DELETE = "delete"
>
> +class Update_version():
> +OVSDB_UPDATE =  0,
> +OVSDB_UPDATE2 = 1.
>
>  class Idl(object):
>  """Open vSwitch Database Interface Definition Language (OVSDB IDL).
> @@ -83,6 +86,10 @@ class Idl(object):
>currently being constructed, if there is one, or None otherwise.
>  """
>
> +IDL_S_INITIAL = 0
> +IDL_S_MONITOR_REQUESTED = 1
> +IDL_S_MONITOR_COND_REQUESTED = 2
> +
>  def __init__(self, remote, schema):
>  """Creates and returns a connection to the database named
> 'db_name' on
>  'remote', which should be in a form acceptable to
> @@ -113,6 +120,8 @@ class Idl(object):
>  self._monitor_request_id = None
>  self._last_seqno = None
>  self.change_seqno = 0
> +self.uuid = uuid.uuid1()
> +self.state = self.IDL_S_INITIAL
>
>  # Database locking.
>  self.lock_name = None  # Name of lock we need, None if
> none.
> @@ -131,6 +140,7 @@ class Idl(object):
>  table.need_table = False
>  table.rows = {}
>  table.idl = self
> +table.condition = []
>
>  def close(self):
>  """Closes the connection to the database.  The IDL will no longer
> @@ -177,11 +187,15 @@ class Idl(object):
>  if msg is None:
>  break
>  if (msg.type == ovs.jsonrpc.Message.T_NOTIFY
> +and msg.method == "update2"
> +and len(msg.params) == 2):
> +# Database contents changed.
> +self.__parse_update(msg.params[1],
> Update_version.OVSDB_UPDATE2)
> +elif (msg.type == ovs.jsonrpc.Message.T_NOTIFY
>  and msg.method == "update"
> -and len(msg.params) == 2
> -and msg.params[0] == None):
> +and len(msg.params) == 2):
>  # Database contents changed.
> -self.__parse_update(msg.params[1])
> +self.__parse_update(msg.params[1],
> Update_version.OVSDB_UPDATE)
>  elif (msg.type == ovs.jsonrpc.Message.T_REPLY
>and self._monitor_request_id is not None
>and self._monitor_request_id == msg.id):
> @@ -190,7 +204,12 @@ class Idl(object):
>  self.change_seqno += 1
>  self._monitor_request_id = None
>  self.__clear()
> -self.__parse_update(msg.result)
> +if self.state == self.IDL_S_MONITOR_COND_REQUESTED:
> +self.__parse_update(msg.result,
> Update_version.OVSDB_UPDATE2)
> +else:
> +assert self.state == self.IDL_S_MONITOR_REQUESTED
> +self.__parse_update(msg.result,
> Update_version.OVSDB_UPDATE)
> +
>  except error.Error, e:
>  vlog.err("%s: parse error in received schema: %s"
>% (self._session.get_name(), e))
> @@ -211,6 +230,11 @@ class Idl(object):
>  elif msg.type == ovs.jsonrpc.Message.T_NOTIFY and msg.id ==
> "echo":
>  # Reply to our echo request.  Ignore it.
>  pass
> +elif (msg.type == ovs.jsonrpc.Message.T_ERROR and
> +  self.state == self.IDL_S_MONITOR_COND_REQUESTED and
> +  self._monitor_request_id == msg.id):
> +  if msg.error == "unknown method":
> +  self.__send_mon

[ovs-dev] [PATCH net 0/2] vxlan: Set a large MTU on ovs-created vxlan devices

[David Wragg]

Prior to 4.3, openvswitch vxlan vports could transmit vxlan packets of
any size, constrained only by the ability to transmit the resulting
UDP packets.  4.3 introduced vxlan netdevs corresponding to vxlan
vports.  These netdevs have an MTU, which limits the size of a packet
that can be successfully vxlan-encapsulated.  The default value for
this MTU is 1500, which is awkwardly small, and leads to a conspicuous
change in behaviour for userspace.

These two patches set the MTU on openvswitch-crated vxlan devices to
be 65465 (the maximum IP packet size minus the vxlan-on-IPv6
overhead), effectively restoring the behaviour prior to 4.3.  In order
to accomplish this, the first patch removes the MTU constraint of 1500
for vxlan netdevs without an underlying device.

David Wragg (2):
  vxlan: Relax the MTU constraint on vxlan devices
  vxlan: Set a large MTU on ovs-created vxlan devices

 drivers/net/vxlan.c   | 38 +-
 net/openvswitch/vport-vxlan.c |  2 ++
 2 files changed, 27 insertions(+), 13 deletions(-)

-- 
2.5.0

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH net 1/2] vxlan: Relax the MTU constraint on vxlan devices

[David Wragg]

Allow the MTU of vxlan devices without an underlying device to be set to
larger values (up to a maximum based on IP packet limits and vxlan
overhead).

Previously, their MTUs could not be set to higher than the conventional
ethernet value of 1500.  This is a very arbitrary value in the context
of vxlan, and prevented such vxlan devices from being able to take
advantage of jumbo frames etc.

The default MTU remains 1500, for compatibility.

Signed-off-by: David Wragg 
---
 drivers/net/vxlan.c | 27 +--
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index ba363ce..96d1c55 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -2359,21 +2359,19 @@ static void vxlan_set_multicast_list(struct net_device 
*dev)
 {
 }
 
-static int vxlan_change_mtu(struct net_device *dev, int new_mtu)
+static int __vxlan_change_mtu(struct net_device *dev,
+ struct net_device *lowerdev,
+ struct vxlan_rdst *dst, int new_mtu)
 {
-   struct vxlan_dev *vxlan = netdev_priv(dev);
-   struct vxlan_rdst *dst = &vxlan->default_dst;
-   struct net_device *lowerdev;
-   int max_mtu;
+   int max_mtu = 65535;
 
-   lowerdev = __dev_get_by_index(vxlan->net, dst->remote_ifindex);
-   if (lowerdev == NULL)
-   return eth_change_mtu(dev, new_mtu);
+   if (lowerdev)
+   max_mtu = lowerdev->mtu;
 
if (dst->remote_ip.sa.sa_family == AF_INET6)
-   max_mtu = lowerdev->mtu - VXLAN6_HEADROOM;
+   max_mtu -= VXLAN6_HEADROOM;
else
-   max_mtu = lowerdev->mtu - VXLAN_HEADROOM;
+   max_mtu -= VXLAN_HEADROOM;
 
if (new_mtu < 68 || new_mtu > max_mtu)
return -EINVAL;
@@ -2382,6 +2380,15 @@ static int vxlan_change_mtu(struct net_device *dev, int 
new_mtu)
return 0;
 }
 
+static int vxlan_change_mtu(struct net_device *dev, int new_mtu)
+{
+   struct vxlan_dev *vxlan = netdev_priv(dev);
+   struct vxlan_rdst *dst = &vxlan->default_dst;
+   struct net_device *lowerdev = __dev_get_by_index(vxlan->net,
+dst->remote_ifindex);
+   return __vxlan_change_mtu(dev, lowerdev, dst, new_mtu);
+}
+
 static int egress_ipv4_tun_info(struct net_device *dev, struct sk_buff *skb,
struct ip_tunnel_info *info,
__be16 sport, __be16 dport)
-- 
2.5.0

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH net 2/2] vxlan: Set a large MTU on ovs-created vxlan devices

[David Wragg]

Prior to 4.3, vxlan vports could transmit vxlan packets of any size,
constrained only by the ability to transmit the resulting UDP packets.
4.3 introduced vxlan netdevs corresponding to vxlan vports.  These
netdevs have an MTU, which limits the size of a packet that can be
successfully vxlan-encapsulated.  The default value for this MTU is
1500, which is awkwardly small, and leads to a conspicuous change in
behaviour for userspace.

This sets the MTU on openvswitch-created vxlan devices to be 65465
(the maximum IP packet size minus the vxlan-on-IPv6 overhead),
effectively restoring the behaviour prior to 4.3.  Although the
vxlan_config struct already had a mtu field for this,
vxlan_dev_configure mostly ignored it; that is also addressed here.

Signed-off-by: David Wragg 
---
 drivers/net/vxlan.c   | 11 ---
 net/openvswitch/vport-vxlan.c |  2 ++
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 96d1c55..a15d300 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -2764,6 +2764,7 @@ static int vxlan_dev_configure(struct net *src_net, 
struct net_device *dev,
int err;
bool use_ipv6 = false;
__be16 default_port = vxlan->cfg.dst_port;
+   struct net_device *lowerdev = NULL;
 
vxlan->net = src_net;
 
@@ -2784,9 +2785,7 @@ static int vxlan_dev_configure(struct net *src_net, 
struct net_device *dev,
}
 
if (conf->remote_ifindex) {
-   struct net_device *lowerdev
-= __dev_get_by_index(src_net, conf->remote_ifindex);
-
+   lowerdev = __dev_get_by_index(src_net, conf->remote_ifindex);
dst->remote_ifindex = conf->remote_ifindex;
 
if (!lowerdev) {
@@ -2810,6 +2809,12 @@ static int vxlan_dev_configure(struct net *src_net, 
struct net_device *dev,
needed_headroom = lowerdev->hard_header_len;
}
 
+   if (conf->mtu) {
+   err = __vxlan_change_mtu(dev, lowerdev, dst, conf->mtu);
+   if (err)
+   return err;
+   }
+
if (use_ipv6 || conf->flags & VXLAN_F_COLLECT_METADATA)
needed_headroom += VXLAN6_HEADROOM;
else
diff --git a/net/openvswitch/vport-vxlan.c b/net/openvswitch/vport-vxlan.c
index 1605691..a97279f 100644
--- a/net/openvswitch/vport-vxlan.c
+++ b/net/openvswitch/vport-vxlan.c
@@ -91,6 +91,8 @@ static struct vport *vxlan_tnl_create(const struct 
vport_parms *parms)
struct vxlan_config conf = {
.no_share = true,
.flags = VXLAN_F_COLLECT_METADATA,
+   /* The maximum VXLAN payload to fit in an IPv6 packet */
+   .mtu = 65535 - VXLAN6_HEADROOM,
};
 
if (!options) {
-- 
2.5.0

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] Add some more flake8 types to ignore list to fix the compilation errors

[Russell Bryant]

On 01/06/2016 06:47 AM, Numan Siddique wrote:
> I guess this patch is not required, once Russel's patches on - Python style 
> fixes and flake8 integration
> are completely merged.

They are all merged now.  What version of flake8 do you have?

I had this originally:

$ flake8 --version
2.4.1 (pep8: 1.5.7, pyflakes: 0.8.1, mccabe: 0.3.1) CPython 2.7.10 on Linux

I just upgraded and still don't see any new warnings:

$ flake8 --version
2.5.1 (pep8: 1.5.7, pyflakes: 1.0.0, mccabe: 0.3.1) CPython 2.7.10 on Linux

I installed via pip.

$ sudo pip install flake8

or to upgrade via pip, run:

$ sudo pip install -U flake8


Assuming it's just a version issue, we have a couple of choices:

1) Update the configure script to ensure some minimum version of flake8.

2) Go back to how I originally implemented this, which was to run flake8
via 'tox', which ensures everyone uses the same version by automatically
creating/using a Python virtualenv and packages from PyPI.

I think we should just try #1 unless this turns out to be painful for
some reason.

-- 
Russell Bryant
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] Add some more flake8 types to ignore list to fix the compilation errors

[Numan Siddique]

On 01/06/2016 07:46 PM, Russell Bryant wrote:
> On 01/06/2016 06:47 AM, Numan Siddique wrote:
>> I guess this patch is not required, once Russel's patches on - Python style 
>> fixes and flake8 integration
>> are completely merged.
> They are all merged now.  What version of flake8 do you have?
>
> I had this originally:
>
> $ flake8 --version
> 2.4.1 (pep8: 1.5.7, pyflakes: 0.8.1, mccabe: 0.3.1) CPython 2.7.10 on Linux
>
> I just upgraded and still don't see any new warnings:
>
> $ flake8 --version
> 2.5.1 (pep8: 1.5.7, pyflakes: 1.0.0, mccabe: 0.3.1) CPython 2.7.10 on Linux
>
> I installed via pip.
>
> $ sudo pip install flake8
>
> or to upgrade via pip, run:
>
> $ sudo pip install -U flake8
>
>
> Assuming it's just a version issue, we have a couple of choices:
>
> 1) Update the configure script to ensure some minimum version of flake8.
>
> 2) Go back to how I originally implemented this, which was to run flake8
> via 'tox', which ensures everyone uses the same version by automatically
> creating/using a Python virtualenv and packages from PyPI.
>
> I think we should just try #1 unless this turns out to be painful for
> some reason.
>

I have 2.2.4 version. I will upgrade flake8 to latest version and test.
I also see the failures in the openstack CI for networking-ovn for one of the 
patch [1]

[1] - 
http://logs.openstack.org/26/178826/18/check/gate-tempest-dsvm-networking-ovn/cb05603/logs/devstacklog.txt.gz
  https://review.openstack.org/#/c/178826/

Thanks
Numan



___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] Add some more flake8 types to ignore list to fix the compilation errors

[Russell Bryant]

On 01/06/2016 09:23 AM, Numan Siddique wrote:
> On 01/06/2016 07:46 PM, Russell Bryant wrote:
>> On 01/06/2016 06:47 AM, Numan Siddique wrote:
>>> I guess this patch is not required, once Russel's patches on - Python style 
>>> fixes and flake8 integration
>>> are completely merged.
>> They are all merged now.  What version of flake8 do you have?
>>
>> I had this originally:
>>
>> $ flake8 --version
>> 2.4.1 (pep8: 1.5.7, pyflakes: 0.8.1, mccabe: 0.3.1) CPython 2.7.10 on Linux
>>
>> I just upgraded and still don't see any new warnings:
>>
>> $ flake8 --version
>> 2.5.1 (pep8: 1.5.7, pyflakes: 1.0.0, mccabe: 0.3.1) CPython 2.7.10 on Linux
>>
>> I installed via pip.
>>
>> $ sudo pip install flake8
>>
>> or to upgrade via pip, run:
>>
>> $ sudo pip install -U flake8
>>
>>
>> Assuming it's just a version issue, we have a couple of choices:
>>
>> 1) Update the configure script to ensure some minimum version of flake8.
>>
>> 2) Go back to how I originally implemented this, which was to run flake8
>> via 'tox', which ensures everyone uses the same version by automatically
>> creating/using a Python virtualenv and packages from PyPI.
>>
>> I think we should just try #1 unless this turns out to be painful for
>> some reason.
>>
> 
> I have 2.2.4 version. I will upgrade flake8 to latest version and test.
> I also see the failures in the openstack CI for networking-ovn for one of the 
> patch [1]
> 
> [1] - 
> http://logs.openstack.org/26/178826/18/check/gate-tempest-dsvm-networking-ovn/cb05603/logs/devstacklog.txt.gz
>   https://review.openstack.org/#/c/178826/

Yes, I just saw that too.  I'll go ahead and update the configure check
to ensure a newer version.

-- 
Russell Bryant
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] Add some more flake8 types to ignore list to fix the compilation errors

[Russell Bryant]

On 01/06/2016 06:29 AM, Numan Siddique wrote:
> with the flake8 check enabled, ovs compilation is failing. This
> patch adds few more flake8 types to the igore list.
> 
> Signed-off-by: Numan Siddique 

Thanks for the patch!  After some discussion on IRC and more testing, I
pushed this to master with some minor additions.

The issue turned out to be flake8 plugins.  My laptop did not have the
docstrings and hacking flake8 plugins, which generate these additional
errors.  Unfortunately, I don't see a way to configure flake8 with an
explicit list of plugins to use, so we just get whatever is installed.

If these system differences continue to be painful, the cleanest
solution would be to switch back to using 'tox', which will run flake8
in a consistently created virtual environment.  The biggest downside is
that it requires internet access to download packages when creating the
virtualenv, and I don't think that requirement exists for any other part
of the ovs build or test setup.

-- 
Russell Bryant
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH V3] Add Passive TCP connection to IDL

[Russell Bryant]

On 12/31/2015 06:55 AM, Ofer Ben Yacov wrote:
> Currently the IDL does not support passive TCP connection,
> i.e. when the OVSDB connects to its manager.
> 
> This patch enables IDL to use an already-open session
> (the one which was previously used for retrieving the db schema).
> In addition, it enables IDL to go back to "listen mode" in case the connection
> is lost.
> 
> LIMITATIONS:
> --
> 
> This patch enables a **SINGLE** TCP connection from an OVSDB server to an
> agent that uses IDL with {IP,PORT} pair. Therefore, the agent will support
> only **ONE** OVSDB server using {IP,PORT} pair.
> 
> Future development may add multi-session server capability that will allow
> an agent to use single {IP,PORT} pair to connect to multiple OVSDB servers.
> 
> 
> CAVEAT:
> --
> 
> When a database first connects to the agent, the agent gets the schema and
> data and builds its tables. If the session disconnects, the agent goes back
> to "listen mode" and accepts **ANY** TCP connection, which means that if
> another database will try to connect to the agent using the same {IP,PORT}
> pair, it will be connected to the IDL that has the schema and data from
> the first database.
> 
> A future patch can resolve this problem.
> 
> USAGE:
> -
> 
> To use IDL in passive mode, the following example code can be use:
> 
> (snippet)
> 
> from ovs.jsonrpc import Session
> ...
> 
> from neutron.agent.ovsdb.native import idlutils
> 
> ...
> 
> session = Session.open('ptcp:192.168.10.10:6640')
> 
> # first call to session.run creates the PassiveStream object and second one
> # accept incoming connection
> session.run()
> session.run()
> 
> # this static method is similar to the original neutron method but the
> # rpc.close() command that would result closing the socket.
> helper = idlutils.get_schema_helper_from_stream_no_close(session.stream,
> 'hardware_vtep')
> helper.register_all()
> self.idl = idl.Idl(self.connection, helper, session)
> idlutils.wait_for_change(self.idl, self.timeout)
> 
> self.poller = poller.Poller()
> self.thread = threading.Thread(target=self.run)
> self.thread.setDaemon(True)
> self.thread.start()
> 
> 
> TESTING:
> ---
> Added unit test for passive mode. See ovsdb-idl.at file.
> 
> 
> Signed-off-by: "Ofer Ben-Yacov" 
> 
> Tested-by: "Ofer Ben-Yacov" 

It looks like your email address got re-formatted here.

> Requested-by: Ben Pfaff , 
> "D M, Vikas" ,
> "Kamat, Maruti Haridas" ,
> "Sukhdev Kapur" ,
> "Migliaccio, Armando" 

Each email address should be on its own Requested-by line.

I'm not sure why, but I get an error trying to apply this patch even
though it looks like you sent it with git-send-email.

> Applying: Add Passive TCP connection to IDL
> /home/rbryant/src/ovs/.git/rebase-apply/patch:106: new blank line at EOF.
> +
> error: patch failed: python/ovs/stream.py:350
> error: python/ovs/stream.py: patch does not apply
> Patch failed at 0001 Add Passive TCP connection to IDL

Can you push the patch to github?  I can pull it down from there.

-- 
Russell Bryant
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] Add some more flake8 types to ignore list to fix the compilation errors

[Ben Pfaff]

On Wed, Jan 06, 2016 at 10:19:56AM -0500, Russell Bryant wrote:
> On 01/06/2016 06:29 AM, Numan Siddique wrote:
> > with the flake8 check enabled, ovs compilation is failing. This
> > patch adds few more flake8 types to the igore list.
> > 
> > Signed-off-by: Numan Siddique 
> 
> Thanks for the patch!  After some discussion on IRC and more testing, I
> pushed this to master with some minor additions.
> 
> The issue turned out to be flake8 plugins.  My laptop did not have the
> docstrings and hacking flake8 plugins, which generate these additional
> errors.  Unfortunately, I don't see a way to configure flake8 with an
> explicit list of plugins to use, so we just get whatever is installed.

Would it work better to whitelist the warnings we want, with --select,
instead of blacklisting the warnings we don't want with --ignore?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] Add some more flake8 types to ignore list to fix the compilation errors

[Russell Bryant]

On 01/06/2016 11:32 AM, Ben Pfaff wrote:
> On Wed, Jan 06, 2016 at 10:19:56AM -0500, Russell Bryant wrote:
>> On 01/06/2016 06:29 AM, Numan Siddique wrote:
>>> with the flake8 check enabled, ovs compilation is failing. This
>>> patch adds few more flake8 types to the igore list.
>>>
>>> Signed-off-by: Numan Siddique 
>>
>> Thanks for the patch!  After some discussion on IRC and more testing, I
>> pushed this to master with some minor additions.
>>
>> The issue turned out to be flake8 plugins.  My laptop did not have the
>> docstrings and hacking flake8 plugins, which generate these additional
>> errors.  Unfortunately, I don't see a way to configure flake8 with an
>> explicit list of plugins to use, so we just get whatever is installed.
> 
> Would it work better to whitelist the warnings we want, with --select,
> instead of blacklisting the warnings we don't want with --ignore?
> 

Maybe ... I'm not sure how to get a list.  It could be hundreds of
entries long.  We also wouldn't get new ones as they get added in upgrades.

I was able to come up with a minor improvement.

--ignore=E123,E126,E127,E128,E129,E131,W503,D,H

This ignores all of D*** and H***.

This would have been a little better, but the --select overrides the
--ignore here, so it didn't behave how I wanted.

--select=E,W,F --ignore=E123,E126,E127,E128,E129,E131,W503

I'll post a patch in a minute.

-- 
Russell Bryant
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] python: Ignore all D,H warnings from flake8.

[Russell Bryant]

A previous patch added the list of warnings emitted by the docstrings
and hacking plugins for flake8.  Switch to ignoring all warnings from
those plugins.  We can use --select to enable specific ones that we want
if needed later on.

Signed-off-by: Russell Bryant 
---
 Makefile.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index 976522d..ef921ee 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -353,7 +353,7 @@ ALL_LOCAL += flake8-check
 # D*** -- warnings from flake8-docstrings plugin
 # H*** -- warnings from flake8 hacking plugin (custom style checks beyond PEP8)
 flake8-check: $(FLAKE8_PYFILES)
-   $(AM_V_GEN) if flake8 $^ 
--ignore=E123,E126,E127,E128,E129,E131,W503,D100,D101,D102,D103,D104,D105,D200,D202,D203,D204,D205,D207,D208,D209,D210,D400,D401,H104,H201,H231,H232,H233,H238,H301,H306,H401,H403,H404,H405
 ${FLAKE8_FLAGS}; then touch $@; else exit 1; fi
+   $(AM_V_GEN) if flake8 $^ 
--ignore=E123,E126,E127,E128,E129,E131,W503,D,H ${FLAKE8_FLAGS}; then touch $@; 
else exit 1; fi
 endif
 
 include $(srcdir)/manpages.mk
-- 
2.5.0

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] Add some more flake8 types to ignore list to fix the compilation errors

[Ben Pfaff]

On Wed, Jan 06, 2016 at 11:50:15AM -0500, Russell Bryant wrote:
> On 01/06/2016 11:32 AM, Ben Pfaff wrote:
> > On Wed, Jan 06, 2016 at 10:19:56AM -0500, Russell Bryant wrote:
> >> On 01/06/2016 06:29 AM, Numan Siddique wrote:
> >>> with the flake8 check enabled, ovs compilation is failing. This
> >>> patch adds few more flake8 types to the igore list.
> >>>
> >>> Signed-off-by: Numan Siddique 
> >>
> >> Thanks for the patch!  After some discussion on IRC and more testing, I
> >> pushed this to master with some minor additions.
> >>
> >> The issue turned out to be flake8 plugins.  My laptop did not have the
> >> docstrings and hacking flake8 plugins, which generate these additional
> >> errors.  Unfortunately, I don't see a way to configure flake8 with an
> >> explicit list of plugins to use, so we just get whatever is installed.
> > 
> > Would it work better to whitelist the warnings we want, with --select,
> > instead of blacklisting the warnings we don't want with --ignore?
> 
> Maybe ... I'm not sure how to get a list.  It could be hundreds of
> entries long.  We also wouldn't get new ones as they get added in upgrades.
> 
> I was able to come up with a minor improvement.
> 
> --ignore=E123,E126,E127,E128,E129,E131,W503,D,H
> 
> This ignores all of D*** and H***.
> 
> This would have been a little better, but the --select overrides the
> --ignore here, so it didn't behave how I wanted.
> 
> --select=E,W,F --ignore=E123,E126,E127,E128,E129,E131,W503
> 
> I'll post a patch in a minute.

OK.

Thanks.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] python: Ignore all D,H warnings from flake8.

[Ben Pfaff]

On Wed, Jan 06, 2016 at 11:53:10AM -0500, Russell Bryant wrote:
> A previous patch added the list of warnings emitted by the docstrings
> and hacking plugins for flake8.  Switch to ignoring all warnings from
> those plugins.  We can use --select to enable specific ones that we want
> if needed later on.
> 
> Signed-off-by: Russell Bryant 

Thanks!

Acked-by: Ben Pfaff 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] python: Ignore all D,H warnings from flake8.

[Russell Bryant]

On 01/06/2016 01:08 PM, Ben Pfaff wrote:
> On Wed, Jan 06, 2016 at 11:53:10AM -0500, Russell Bryant wrote:
>> A previous patch added the list of warnings emitted by the docstrings
>> and hacking plugins for flake8.  Switch to ignoring all warnings from
>> those plugins.  We can use --select to enable specific ones that we want
>> if needed later on.
>>
>> Signed-off-by: Russell Bryant 
> 
> Thanks!
> 
> Acked-by: Ben Pfaff 
> 

Thanks, I pushed this to master.

-- 
Russell Bryant
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 1/4] test-netflow.c: Fix memory leak reported by valgrind

[William Tu]

Test case 890: ofproto-dpif - NetFlow flow expiration - IPv4 collector
Valgrind reports two leaks below:
unixctl_server_create (unixctl.c:250)
test_netflow_main (test-netflow.c:200)
ovstest_wrapper_test_netflow_main__ (test-netflow.c:301)
ovs_cmdl_run_command (command-line.c:121)
main (ovstest.c:132)
and
ofpbuf_init (ofpbuf.c:124)
test_netflow_main (test-netflow.c:208)
ovstest_wrapper_test_netflow_main__ (test-netflow.c:301)
ovs_cmdl_run_command (command-line.c:121)
main (ovstest.c:132)

Signed-off-by: William Tu 
Signed-off-by: Daniele Di Proietto 
Co-authored-by: Daniele Di Proietto http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 2/4] ovs-vsctl: fix memory leak reported by valgrind

[William Tu]

test case 1: appctl-bashcomp - basic verification
Reason: args used without being free
Call stacks:
ds_reserve (dynamic-string.c:63)
ds_put_uninit (dynamic-string.c:73)
ds_put_char__ (dynamic-string.c:82)
ds_put_char (dynamic-string.h:89)
process_escape_args (process.c:103)
main (ovs-vsctl.c:150)

Signed-off-by: William Tu 
---
 utilities/ovs-vsctl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index 36290db..8c2a252 100644
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -2488,6 +2488,7 @@ do_vsctl(const char *args, struct ctl_command *commands, 
size_t n_commands,
 }
 
 ovsdb_idl_txn_add_comment(txn, "ovs-vsctl: %s", args);
+free(args);
 
 ovs = ovsrec_open_vswitch_first(idl);
 if (!ovs) {
-- 
2.5.0

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 4/4] test-ovsdb.c: fix memory leak reported by valgrind

[William Tu]

Test case 1205: generate and apply diff -- set -- size (ovsdb-data.at:827)
Call stack:
ovsdb_error_valist (ovsdb-error.c:40)
ovsdb_error (ovsdb-error.c:55)
do_diff_data (test-ovsdb.c:427)
ovs_cmdl_run_command (command-line.c:121)
main (test-ovsdb.c:72)
Fix by calling ovsdb_error_destroy() before ovs_fatal()

Signed-off-by: William Tu 
---
 tests/test-ovsdb.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
index 0ce8f9d..dbd51f2 100644
--- a/tests/test-ovsdb.c
+++ b/tests/test-ovsdb.c
@@ -426,7 +426,9 @@ do_diff_data(struct ovs_cmdl_context *ctx)
 /* Apply diff to 'old' to create'reincarnation'. */
 error = ovsdb_datum_apply_diff(&reincarnation, &old, &diff, &type);
 if (error) {
-ovs_fatal(0, "%s", ovsdb_error_to_string(error));
+char *string = ovsdb_error_to_string(error);
+ovsdb_error_destroy(error);
+ovs_fatal(0, "%s", string);
 }
 
 /* Test to make sure 'new' equals 'reincarnation'.  */
-- 
2.5.0

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 3/4] ovsdb-client: fix memory leak reported by valgrind

[William Tu]

Test case 1508-1514: OVSDB -- ovsdb-server monitors, call stacks:
ovsdb_schema_create (ovsdb.c:34)
ovsdb_schema_from_json (ovsdb.c:196)
fetch_schema (ovsdb-client.c:375)
do_monitor__ (ovsdb-client.c:920)
main (ovsdb-client.c:152)
Fix by adding ovsdb_schema_destroy()

Signed-off-by: William Tu 
Signed-off-by: Daniele Di Proietto 
Co-authored-by: Daniele Di Proietto http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 0/9] Partial Python 3 compatibility.

[Russell Bryant]

I got the ovs Python code ported to Python 3 in a branch.  I've been submitting
the work in pieces to make it a bit easier to review, iterate on, and merge.

This series of patches addresses an assortment of Python 3 compatibility
issues. This first patch is one that was submitted to the list a few months ago.

One new dependency is introduced.  The 'six' Python library becomes a
requirement to aid in supporting both Python 2 and 3.  More information about
'six' can be found here: https://pythonhosted.org/six/

Russell Bryant (8):
  python: Stop use of tuple parameter unpacking
  python: Fix exception handler compatibility.
  python: Fix print function compatibility.
  python: Fix xmlrpclib imports.
  python: Stop using xrange().
  python: Convert dict iterators.
  python: Fix octal compatibility.
  python: Remove old style classes.

Terry Wilson (1):
  python: Start fixing some Python 3 issues.

 INSTALL.md |  2 +-
 Makefile.am| 24 ++
 debian/ovs-monitor-ipsec   | 22 +-
 m4/openvswitch.m4  |  6 +++
 ofproto/ipfix-gen-entities | 22 +-
 python/build/nroff.py  |  2 +-
 python/ovs/daemon.py   | 31 ++---
 python/ovs/db/data.py  | 18 
 python/ovs/db/idl.py   | 44 ++-
 python/ovs/db/schema.py| 18 
 python/ovs/fatal_signal.py |  2 +-
 python/ovs/json.py |  7 ++-
 python/ovs/ovsuuid.py  |  4 +-
 python/ovs/poller.py   |  4 +-
 python/ovs/socket_util.py  | 36 +++
 python/ovs/stream.py   | 16 ---
 python/ovs/unixctl/server.py   |  2 +
 python/ovs/vlog.py | 11 +++--
 python/ovstest/rpcserver.py|  9 ++--
 python/ovstest/tests.py| 34 ---
 python/ovstest/udp.py  |  4 +-
 python/ovstest/util.py |  7 ++-
 python/setup.py|  2 +-
 tests/test-json.py |  6 ++-
 tests/test-jsonrpc.py  |  4 +-
 tests/test-ovsdb.py| 51 --
 tests/test-reconnect.py| 22 +-
 tests/test-vlog.py |  3 +-
 utilities/ovs-pcap.in  | 14 +++---
 vtep/ovs-vtep  | 22 +-
 .../usr_share_openvswitch_scripts_ovs-xapi-sync| 11 ++---
 31 files changed, 262 insertions(+), 198 deletions(-)

-- 
2.5.0

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 1/9] python: Start fixing some Python 3 issues.

[Russell Bryant]

From: Terry Wilson 

This patch fixes just the Python 3 problems found by running:

  python3 setup.py install

There are still many other issues to be fixed, but this is a start.

Signed-off-by: Terry Wilson 
[russ...@ovn.org resolved conflicts with current master]
Signed-off-by: Russell Bryant 
---
 python/ovs/daemon.py   | 31 ---
 python/ovs/db/idl.py   | 10 +-
 python/ovs/fatal_signal.py |  2 +-
 python/ovs/json.py |  2 +-
 python/ovs/ovsuuid.py  |  2 +-
 python/ovs/poller.py   |  2 +-
 python/ovs/socket_util.py  | 32 
 python/ovs/stream.py   | 12 +++-
 python/setup.py|  2 +-
 9 files changed, 49 insertions(+), 46 deletions(-)

diff --git a/python/ovs/daemon.py b/python/ovs/daemon.py
index e5e8858..bd06195 100644
--- a/python/ovs/daemon.py
+++ b/python/ovs/daemon.py
@@ -132,30 +132,30 @@ def _make_pidfile():
 global file_handle
 
 file_handle = open(tmpfile, "w")
-except IOError, e:
+except IOError as e:
 _fatal("%s: create failed (%s)" % (tmpfile, e.strerror))
 
 try:
 s = os.fstat(file_handle.fileno())
-except IOError, e:
+except IOError as e:
 _fatal("%s: fstat failed (%s)" % (tmpfile, e.strerror))
 
 try:
 file_handle.write("%s\n" % pid)
 file_handle.flush()
-except OSError, e:
+except OSError as e:
 _fatal("%s: write failed: %s" % (tmpfile, e.strerror))
 
 try:
 fcntl.lockf(file_handle, fcntl.LOCK_EX | fcntl.LOCK_NB)
-except IOError, e:
+except IOError as e:
 _fatal("%s: fcntl failed: %s" % (tmpfile, e.strerror))
 
 # Rename or link it to the correct name.
 if _overwrite_pidfile:
 try:
 os.rename(tmpfile, _pidfile)
-except OSError, e:
+except OSError as e:
 _fatal("failed to rename \"%s\" to \"%s\" (%s)"
% (tmpfile, _pidfile, e.strerror))
 else:
@@ -163,7 +163,7 @@ def _make_pidfile():
 try:
 os.link(tmpfile, _pidfile)
 error = 0
-except OSError, e:
+except OSError as e:
 error = e.errno
 if error == errno.EEXIST:
 _check_already_running()
@@ -199,7 +199,7 @@ def _waitpid(pid, options):
 while True:
 try:
 return os.waitpid(pid, options)
-except OSError, e:
+except OSError as e:
 if e.errno == errno.EINTR:
 pass
 return -e.errno, 0
@@ -208,13 +208,13 @@ def _waitpid(pid, options):
 def _fork_and_wait_for_startup():
 try:
 rfd, wfd = os.pipe()
-except OSError, e:
+except OSError as e:
 sys.stderr.write("pipe failed: %s\n" % os.strerror(e.errno))
 sys.exit(1)
 
 try:
 pid = os.fork()
-except OSError, e:
+except OSError as e:
 sys.stderr.write("could not fork: %s\n" % os.strerror(e.errno))
 sys.exit(1)
 
@@ -226,7 +226,7 @@ def _fork_and_wait_for_startup():
 try:
 s = os.read(rfd, 1)
 error = 0
-except OSError, e:
+except OSError as e:
 s = ""
 error = e.errno
 if error != errno.EINTR:
@@ -313,7 +313,8 @@ def _monitor_daemon(daemon_pid):
 wakeup = last_restart + 1
 if now > wakeup:
 break
-print "sleep %f" % ((wakeup - now) / 1000.0)
+sys.stdout.write("sleep %f\n" % (
+(wakeup - now) / 1000.0))
 time.sleep((wakeup - now) / 1000.0)
 last_restart = ovs.timeval.msec()
 
@@ -404,7 +405,7 @@ def __read_pidfile(pidfile, delete_if_stale):
 
 try:
 file_handle = open(pidfile, "r+")
-except IOError, e:
+except IOError as e:
 if e.errno == errno.ENOENT and delete_if_stale:
 return 0
 vlog.warn("%s: open: %s" % (pidfile, e.strerror))
@@ -437,7 +438,7 @@ def __read_pidfile(pidfile, delete_if_stale):
 # We won the right to delete the stale pidfile.
 try:
 os.unlink(pidfile)
-except IOError, e:
+except IOError as e:
 vlog.warn("%s: failed to delete stale pidfile (%s)"
 % (pidfile, e.strerror))
 return -e.errno
@@ -445,7 +446,7 @@ def __read_pidfile(pidfile, delete_if_stale):
 vlog.dbg("%s: deleted stale pidfile" % pidfile)
 file_handle.close()
 return 0
-except IOError, e:
+except IOError as e:
 if e.errno not in [errno.EACCES, errno.EAGAIN]:
 vlog.warn("%s: fcntl: %s" % (pidfile, e.strerror))
 return -e.errno
@@ -454,7 +455,7 @@ def __read_pidfile(pidfile, delete_if_stale):
 try:
 try:
 error = int(

[ovs-dev] [PATCH 2/9] python: Stop use of tuple parameter unpacking

[Russell Bryant]

Python 3 removed support for tuple parameter unpacking.

https://www.python.org/dev/peps/pep-3113/

Instead of:

def foo((a, b)):
print(a)
print(b)

you should do:

def foo(a_b):
a, b = a_b
print(a)
print(b)

but in both uses here, the values were never used so the fix is even
simpler.

Signed-off-by: Russell Bryant 
---
 python/ovstest/udp.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/python/ovstest/udp.py b/python/ovstest/udp.py
index d6e6162..acd28d5 100644
--- a/python/ovstest/udp.py
+++ b/python/ovstest/udp.py
@@ -31,7 +31,7 @@ class UdpListener(DatagramProtocol):
 def __init__(self):
 self.stats = []
 
-def datagramReceived(self, data, (_1, _2)):
+def datagramReceived(self, data, _1_2):
 """This function is called each time datagram is received"""
 try:
 self.stats.append(struct.unpack_from("Q", data, 0))
@@ -67,7 +67,7 @@ class UdpSender(DatagramProtocol):
 self.looper.stop()
 self.looper = None
 
-def datagramReceived(self, data, (host, port)):
+def datagramReceived(self, data, host_port):
 pass
 
 def sendData(self):
-- 
2.5.0

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 7/9] python: Convert dict iterators.

[Russell Bryant]

In Python 2, dict.items(), dict.keys(), and dict.values() returned a
list.  dict.iteritems(), dict.iterkeys(), and dict.itervalues() returned
an iterator.

As of Python 3, dict.iteritems(), dict.itervalues(), and dict.iterkeys()
are gone.  items(), keys(), and values() now return an iterator.

In the case where we want an iterator, we now use the six.iter*()
helpers.  If we want a list, we explicitly create a list from the
iterator.

Signed-off-by: Russell Bryant 
---
 debian/ovs-monitor-ipsec   | 13 +
 python/build/nroff.py  |  2 +-
 python/ovs/db/data.py  | 18 +++-
 python/ovs/db/idl.py   | 34 --
 python/ovs/db/schema.py| 18 +++-
 python/ovs/json.py |  3 +-
 python/ovs/poller.py   |  2 +-
 python/ovs/stream.py   |  4 ++-
 python/ovs/vlog.py |  9 +++---
 tests/test-ovsdb.py| 15 +-
 vtep/ovs-vtep  | 21 ++---
 .../usr_share_openvswitch_scripts_ovs-xapi-sync|  9 +++---
 12 files changed, 81 insertions(+), 67 deletions(-)

diff --git a/debian/ovs-monitor-ipsec b/debian/ovs-monitor-ipsec
index 22883fc..09596b0 100755
--- a/debian/ovs-monitor-ipsec
+++ b/debian/ovs-monitor-ipsec
@@ -40,6 +40,7 @@ import ovs.unixctl
 import ovs.unixctl.server
 import ovs.vlog
 from six.moves import range
+import six
 
 vlog = ovs.vlog.Vlog("ovs-monitor-ipsec")
 root_prefix = ''# Prefix for absolute file names, for testing.
@@ -152,7 +153,7 @@ path certificate "%s";
 conf_file = open(root_prefix + self.conf_file, 'w')
 conf_file.write(Racoon.conf_header % (self.psk_file, self.cert_dir))
 
-for host, vals in self.cert_hosts.iteritems():
+for host, vals in six.iteritems(self.cert_hosts):
 conf_file.write(Racoon.cert_entry % (host, vals["certificate"],
 vals["private_key"], vals["peer_cert_file"]))
 
@@ -169,7 +170,7 @@ path certificate "%s";
 
 psk_file.write("# Generated by Open vSwitch...do not modify by hand!")
 psk_file.write("\n\n")
-for host, vals in self.psk_hosts.iteritems():
+for host, vals in six.iteritems(self.psk_hosts):
 psk_file.write("%s   %s\n" % (host, vals["psk"]))
 psk_file.close()
 
@@ -354,11 +355,11 @@ class IPsec:
 
 
 def update_ipsec(ipsec, interfaces, new_interfaces):
-for name, vals in interfaces.iteritems():
+for name, vals in six.iteritems(interfaces):
 if name not in new_interfaces:
 ipsec.del_entry(vals["local_ip"], vals["remote_ip"])
 
-for name, vals in new_interfaces.iteritems():
+for name, vals in six.iteritems(new_interfaces):
 orig_vals = interfaces.get(name)
 if orig_vals:
 # Configuration for this host already exists.  Check if it's
@@ -377,7 +378,7 @@ def update_ipsec(ipsec, interfaces, new_interfaces):
 
 
 def get_ssl_cert(data):
-for ovs_rec in data["Open_vSwitch"].rows.itervalues():
+for ovs_rec in six.itervalues(data["Open_vSwitch"].rows):
 if ovs_rec.ssl:
 ssl = ovs_rec.ssl[0]
 if ssl.certificate and ssl.private_key:
@@ -440,7 +441,7 @@ def main():
 ssl_cert = get_ssl_cert(idl.tables)
 
 new_interfaces = {}
-for rec in idl.tables["Interface"].rows.itervalues():
+for rec in six.itervalues(idl.tables["Interface"].rows):
 if rec.type == "ipsec_gre":
 name = rec.name
 options = rec.options
diff --git a/python/build/nroff.py b/python/build/nroff.py
index f0edd29..aed60eb 100644
--- a/python/build/nroff.py
+++ b/python/build/nroff.py
@@ -89,7 +89,7 @@ def inline_xml_to_nroff(node, font, to_upper=False, 
newline='\n'):
 s += node.attributes['db'].nodeValue
 else:
 raise error.Error("'ref' lacks required attributes: %s"
-  % node.attributes.keys())
+  % list(node.attributes.keys()))
 return s + font
 elif node.tagName in ['var', 'dfn', 'i']:
 s = r'\fI'
diff --git a/python/ovs/db/data.py b/python/ovs/db/data.py
index 6baff38..cd535d4 100644
--- a/python/ovs/db/data.py
+++ b/python/ovs/db/data.py
@@ -15,6 +15,8 @@
 import re
 import uuid
 
+import six
+
 import ovs.poller
 import ovs.socket_util
 import ovs.json
@@ -293,7 +295,7 @@ class Datum(object):
 This function is not commonly useful because the most ordinary way to
 obtain a datum is ultimately via Datum.from_json() or Atom.from_json(),
 which check constraints themselves."""
-for keyAtom, valueAtom in self.values.iteritems():
+for keyAtom, valueAtom in six.iteritems(self.val

[ovs-dev] [PATCH 6/9] python: Stop using xrange().

[Russell Bryant]

Python 2 had range() and xrange().  xrange() is more efficient, but
behaves differently so range() was retained for compatibility.  Python 3
only has range() and it behaves like Python 2's xrange().

Remove explicit use of xrange() and use six.moves.range() to
make sure we're using xrange() from Python 2 or range() from Python 3.

Signed-off-by: Russell Bryant 
---
 debian/ovs-monitor-ipsec | 1 +
 python/ovs/json.py   | 2 ++
 python/ovs/ovsuuid.py| 2 ++
 python/ovs/socket_util.py| 4 +++-
 python/ovs/unixctl/server.py | 2 ++
 python/ovs/vlog.py   | 2 ++
 python/ovstest/util.py   | 1 +
 tests/test-vlog.py   | 3 ++-
 vtep/ovs-vtep| 1 +
 9 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/debian/ovs-monitor-ipsec b/debian/ovs-monitor-ipsec
index efab91b..22883fc 100755
--- a/debian/ovs-monitor-ipsec
+++ b/debian/ovs-monitor-ipsec
@@ -39,6 +39,7 @@ import ovs.db.idl
 import ovs.unixctl
 import ovs.unixctl.server
 import ovs.vlog
+from six.moves import range
 
 vlog = ovs.vlog.Vlog("ovs-monitor-ipsec")
 root_prefix = ''# Prefix for absolute file names, for testing.
diff --git a/python/ovs/json.py b/python/ovs/json.py
index 4e87726..07fd9c1 100644
--- a/python/ovs/json.py
+++ b/python/ovs/json.py
@@ -16,6 +16,8 @@ import re
 import StringIO
 import sys
 
+from six.moves import range
+
 __pychecker__ = 'no-stringiter'
 
 escapes = {ord('"'): u"\\\"",
diff --git a/python/ovs/ovsuuid.py b/python/ovs/ovsuuid.py
index 5cc0e1d..f2d48ea 100644
--- a/python/ovs/ovsuuid.py
+++ b/python/ovs/ovsuuid.py
@@ -15,6 +15,8 @@
 import re
 import uuid
 
+from six.moves import range
+
 from ovs.db import error
 import ovs.db.parser
 
diff --git a/python/ovs/socket_util.py b/python/ovs/socket_util.py
index 586c23a..57b1992 100644
--- a/python/ovs/socket_util.py
+++ b/python/ovs/socket_util.py
@@ -19,6 +19,8 @@ import random
 import socket
 import sys
 
+from six.moves import range
+
 import ovs.fatal_signal
 import ovs.poller
 import ovs.vlog
@@ -32,7 +34,7 @@ def make_short_name(long_name):
 long_name = os.path.abspath(long_name)
 long_dirname = os.path.dirname(long_name)
 tmpdir = os.getenv('TMPDIR', '/tmp')
-for x in xrange(0, 1000):
+for x in range(0, 1000):
 link_name = \
 '%s/ovs-un-py-%d-%d' % (tmpdir, random.randint(0, 1), x)
 try:
diff --git a/python/ovs/unixctl/server.py b/python/ovs/unixctl/server.py
index c750fe9..9744cf2 100644
--- a/python/ovs/unixctl/server.py
+++ b/python/ovs/unixctl/server.py
@@ -17,6 +17,8 @@ import errno
 import os
 import types
 
+from six.moves import range
+
 import ovs.dirs
 import ovs.jsonrpc
 import ovs.stream
diff --git a/python/ovs/vlog.py b/python/ovs/vlog.py
index 0065020..6dcccbb 100644
--- a/python/ovs/vlog.py
+++ b/python/ovs/vlog.py
@@ -22,6 +22,8 @@ import socket
 import sys
 import threading
 
+from six.moves import range
+
 import ovs.dirs
 import ovs.unixctl
 import ovs.util
diff --git a/python/ovstest/util.py b/python/ovstest/util.py
index a522195..830feba 100644
--- a/python/ovstest/util.py
+++ b/python/ovstest/util.py
@@ -26,6 +26,7 @@ import signal
 import subprocess
 import re
 
+from six.moves import range
 import six.moves.xmlrpc_client
 
 
diff --git a/tests/test-vlog.py b/tests/test-vlog.py
index f6d0cec..ecfa26f 100644
--- a/tests/test-vlog.py
+++ b/tests/test-vlog.py
@@ -15,10 +15,11 @@
 import argparse
 
 import ovs.vlog
+from six.moves import range
 
 
 def main():
-modules = [ovs.vlog.Vlog("module_%d" % i) for i in xrange(3)]
+modules = [ovs.vlog.Vlog("module_%d" % i) for i in range(3)]
 
 parser = argparse.ArgumentParser(description="Vlog Module Tester")
 ovs.vlog.add_args(parser)
diff --git a/vtep/ovs-vtep b/vtep/ovs-vtep
index dd5b1a7..1db4e05 100755
--- a/vtep/ovs-vtep
+++ b/vtep/ovs-vtep
@@ -29,6 +29,7 @@ import ovs.util
 import ovs.daemon
 import ovs.unixctl.server
 import ovs.vlog
+from six.moves import range
 
 
 VERSION = "0.99"
-- 
2.5.0

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 4/9] python: Fix print function compatibility.

[Russell Bryant]

The print statement from Python 2 is a function in Python 3.  Enable
print function support for Python 2 and convert print statements to
function calls.

Enable the H233 flake8 warning.  If the hacking plugin is installed,
this will generate warnings for print statement usage not compatible
with Python 3.

  H233 Python 3.x incompatible use of print operator

Signed-off-by: Russell Bryant 
---
 Makefile.am |  3 ++-
 ofproto/ipfix-gen-entities  | 20 +++-
 python/ovstest/rpcserver.py |  4 +++-
 python/ovstest/tests.py | 34 ++
 tests/test-json.py  |  4 +++-
 tests/test-jsonrpc.py   |  4 +++-
 tests/test-ovsdb.py | 30 --
 tests/test-reconnect.py | 22 --
 utilities/ovs-pcap.in   | 10 ++
 9 files changed, 74 insertions(+), 57 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 05edc03..9228a08 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -357,8 +357,9 @@ ALL_LOCAL += flake8-check
 # D*** -- warnings from flake8-docstrings plugin
 # H*** -- warnings from flake8 hacking plugin (custom style checks beyond PEP8)
 #   H231 Python 3.x incompatible 'except x,y:' construct
+#   H233 Python 3.x incompatible use of print operator
 flake8-check: $(FLAKE8_PYFILES)
-   $(AM_V_GEN) if flake8 $^ --select=H231 
--ignore=E123,E126,E127,E128,E129,E131,W503,D,H ${FLAKE8_FLAGS}; then touch $@; 
else exit 1; fi
+   $(AM_V_GEN) if flake8 $^ --select=H231,H233 
--ignore=E123,E126,E127,E128,E129,E131,W503,D,H ${FLAKE8_FLAGS}; then touch $@; 
else exit 1; fi
 endif
 
 include $(srcdir)/manpages.mk
diff --git a/ofproto/ipfix-gen-entities b/ofproto/ipfix-gen-entities
index 54951b6..a603cd1 100755
--- a/ofproto/ipfix-gen-entities
+++ b/ofproto/ipfix-gen-entities
@@ -7,6 +7,8 @@
 # notice and this notice are preserved.  This file is offered as-is,
 # without warranty of any kind.
 
+from __future__ import print_function
+
 import getopt
 import re
 import sys
@@ -48,16 +50,16 @@ class IpfixEntityHandler(xml.sax.handler.ContentHandler):
 self.current_record = dict()
 
 def startDocument(self):
-print """\
+print("""\
 /* IPFIX entities. */
 #ifndef IPFIX_ENTITY
 #define IPFIX_ENTITY(ENUM, ID, SIZE, NAME)
 #endif
-"""
+""")
 
 def endDocument(self):
-print """
-#undef IPFIX_ENTITY"""
+print("""
+#undef IPFIX_ENTITY""")
 
 def startElement(self, name, attrs):
 if name in self.RECORD_FIELDS:
@@ -83,8 +85,8 @@ class IpfixEntityHandler(xml.sax.handler.ContentHandler):
 self.current_record['dataTypeSize'] = self.DATA_TYPE_SIZE.get(
 self.current_record['dataType'], 0)
 
-print 'IPFIX_ENTITY(%(enumName)s, %(elementId)s, ' \
-  '%(dataTypeSize)i, %(name)s)' % self.current_record
+print('IPFIX_ENTITY(%(enumName)s, %(elementId)s, '
+  '%(dataTypeSize)i, %(name)s)' % self.current_record)
 self.current_record.clear()
 
 def characters(self, content):
@@ -97,7 +99,7 @@ def print_ipfix_entity_macros(xml_file):
 
 
 def usage(name):
-print """\
+print("""\
 %(name)s: IPFIX entity definition generator
 Prints C macros defining IPFIX entities from the standard IANA file at
 
@@ -107,7 +109,7 @@ where XML is the standard IANA XML file defining IPFIX 
entities
 The following options are also available:
   -h, --help  display this help message
   -V, --version   display version information\
-""" % {'name': name}
+""" % {'name': name})
 sys.exit(0)
 
 if __name__ == '__main__':
@@ -122,7 +124,7 @@ if __name__ == '__main__':
 if key in ['-h', '--help']:
 usage()
 elif key in ['-V', '--version']:
-print 'ipfix-gen-entities (Open vSwitch)'
+print('ipfix-gen-entities (Open vSwitch)')
 else:
 sys.exit(0)
 
diff --git a/python/ovstest/rpcserver.py b/python/ovstest/rpcserver.py
index 80b9c4e..80fc5dc 100644
--- a/python/ovstest/rpcserver.py
+++ b/python/ovstest/rpcserver.py
@@ -16,6 +16,8 @@
 rpcserver is an XML RPC server that allows RPC client to initiate tests
 """
 
+from __future__ import print_function
+
 import exceptions
 import sys
 import xmlrpclib
@@ -357,7 +359,7 @@ def start_rpc_server(port):
 rpc_server = TestArena()
 reactor.listenTCP(port, server.Site(rpc_server))
 try:
-print "Starting RPC server\n"
+print("Starting RPC server\n")
 sys.stdout.flush()
 # If this server was started from ovs-test client then we must flush
 # STDOUT so that client would know that server is ready to accept
diff --git a/python/ovstest/tests.py b/python/ovstest/tests.py
index 108cad4..e9e2936 100644
--- a/python/ovstest/tests.py
+++ b/python/ovstest/tests.py
@@ -10,6 +10,8 @@
 # See the License for the specific language governing permissions and

[ovs-dev] [PATCH 8/9] python: Fix octal compatibility.

[Russell Bryant]

Octal constants must be written as 0o077 instead of 0077 to be
compatible with both Python 2 and 3.

Signed-off-by: Russell Bryant 
---
 Makefile.am  | 3 ++-
 debian/ovs-monitor-ipsec | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 9228a08..71cd1b9 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -357,9 +357,10 @@ ALL_LOCAL += flake8-check
 # D*** -- warnings from flake8-docstrings plugin
 # H*** -- warnings from flake8 hacking plugin (custom style checks beyond PEP8)
 #   H231 Python 3.x incompatible 'except x,y:' construct
+#   H232 Python 3.x incompatible octal 077 should be written as 0o77
 #   H233 Python 3.x incompatible use of print operator
 flake8-check: $(FLAKE8_PYFILES)
-   $(AM_V_GEN) if flake8 $^ --select=H231,H233 
--ignore=E123,E126,E127,E128,E129,E131,W503,D,H ${FLAKE8_FLAGS}; then touch $@; 
else exit 1; fi
+   $(AM_V_GEN) if flake8 $^ --select=H231,H232,H233 
--ignore=E123,E126,E127,E128,E129,E131,W503,D,H ${FLAKE8_FLAGS}; then touch $@; 
else exit 1; fi
 endif
 
 include $(srcdir)/manpages.mk
diff --git a/debian/ovs-monitor-ipsec b/debian/ovs-monitor-ipsec
index 09596b0..2bccfb0 100755
--- a/debian/ovs-monitor-ipsec
+++ b/debian/ovs-monitor-ipsec
@@ -164,7 +164,7 @@ path certificate "%s";
 conf_file.close()
 
 # Rewrite the pre-shared keys file; it must only be readable by root.
-orig_umask = os.umask(0077)
+orig_umask = os.umask(0o077)
 psk_file = open(root_prefix + Racoon.psk_file, 'w')
 os.umask(orig_umask)
 
-- 
2.5.0

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 5/9] python: Fix xmlrpclib imports.

[Russell Bryant]

Fix imports of xmlrpclib to be compatible with Python 3.  Python 2 had
xmlrpclib (client) and SimpleXMLRPCServer (server).  In Python 3, these
have been renamed to xmlrpc.client and xmlrpc.server.

The solution implemented here is to use the six library.  It may seem
excessive for this particular issue, but the six library provides
helpers for Python 2 and 3 compatibility for many different issues.
This is just the first of many uses of the six library.

Signed-off-by: Russell Bryant 
---
 INSTALL.md  | 2 +-
 m4/openvswitch.m4   | 6 ++
 python/ovstest/rpcserver.py | 5 +++--
 python/ovstest/util.py  | 6 --
 4 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/INSTALL.md b/INSTALL.md
index bf756f2..4f619fa 100644
--- a/INSTALL.md
+++ b/INSTALL.md
@@ -48,7 +48,7 @@ you will need the following software:
 privileges.  If libcap-ng is installed, then Open vSwitch will
 automatically build with support for it.
 
-  - Python 2.7.
+  - Python 2.7. You must also have the Python six library.
 
 On Linux, you may choose to compile the kernel module that comes with
 the Open vSwitch distribution or to use the kernel module built into
diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4
index 6d4e5da..0149c30 100644
--- a/m4/openvswitch.m4
+++ b/m4/openvswitch.m4
@@ -341,6 +341,12 @@ else:
 fi
   done
 done
+if test $ovs_cv_python != no; then
+  if test -x "$ovs_cv_python" && ! "$ovs_cv_python" -c 'import six' 
>/dev/null 2>&1; then
+ovs_cv_python=no
+AC_MSG_WARN([Missing Python six library.])
+  fi
+fi
   fi])
AC_SUBST([HAVE_PYTHON])
AM_MISSING_PROG([PYTHON], [python])
diff --git a/python/ovstest/rpcserver.py b/python/ovstest/rpcserver.py
index 80fc5dc..7abf13c 100644
--- a/python/ovstest/rpcserver.py
+++ b/python/ovstest/rpcserver.py
@@ -20,8 +20,8 @@ from __future__ import print_function
 
 import exceptions
 import sys
-import xmlrpclib
 
+import six.moves.xmlrpc_client
 from twisted.internet import reactor
 from twisted.internet.error import CannotListenError
 from twisted.web import xmlrpc
@@ -108,7 +108,8 @@ class TestArena(xmlrpc.XMLRPC):
 Returns the ovs-test server IP address that the other ovs-test server
 with the given ip will see.
 """
-server1 = xmlrpclib.Server("http://%s:%u/"; % (his_ip, his_port))
+server1 = six.moves.xmlrpc_client.Server("http://%s:%u/"; %
+ (his_ip, his_port))
 return server1.get_my_address()
 
 def xmlrpc_create_udp_listener(self, port):
diff --git a/python/ovstest/util.py b/python/ovstest/util.py
index 16c012e..a522195 100644
--- a/python/ovstest/util.py
+++ b/python/ovstest/util.py
@@ -25,7 +25,8 @@ import struct
 import signal
 import subprocess
 import re
-import xmlrpclib
+
+import six.moves.xmlrpc_client
 
 
 def str_ip(ip_address):
@@ -167,7 +168,8 @@ def get_interface_from_routing_decision(ip):
 
 
 def rpc_client(ip, port):
-return xmlrpclib.Server("http://%s:%u/"; % (ip, port), allow_none=True)
+return six.moves.xmlrpc_client.Server("http://%s:%u/"; % (ip, port),
+  allow_none=True)
 
 
 def sigint_intercept():
-- 
2.5.0

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 3/9] python: Fix exception handler compatibility.

[Russell Bryant]

Python 3 dropped exception handlers of the deprecated form:

  except Exception, e:

You must use the newer syntax of:

  except Exception as e:

This patch also enables a flake8 warning for this.

  H231 Python 3.x incompatible 'except x,y:' construct

Signed-off-by: Russell Bryant 
---
 Makefile.am | 21 +
 debian/ovs-monitor-ipsec|  2 +-
 ofproto/ipfix-gen-entities  |  2 +-
 tests/test-json.py  |  2 +-
 tests/test-ovsdb.py |  6 +++---
 utilities/ovs-pcap.in   |  4 ++--
 .../usr_share_openvswitch_scripts_ovs-xapi-sync |  2 +-
 7 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index ef921ee..05edc03 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -343,17 +343,22 @@ endif
 if HAVE_FLAKE8
 ALL_LOCAL += flake8-check
 # http://flake8.readthedocs.org/en/latest/warnings.html
-# E123 closing bracket does not match indentation of opening bracket's line
-# E126 continuation line over-indented for hanging indent
-# E127 continuation line over-indented for visual indent
-# E128 continuation line under-indented for visual indent
-# E129 visually indented line with same indent as next logical line
-# E131 continuation line unaligned for hanging indent
-# W503 line break before binary operator
+# All warnings explicitly selected or ignored should be listed below.
+#
+# E***, W*** -- warnings from pep8
+#   E123 closing bracket does not match indentation of opening bracket's line
+#   E126 continuation line over-indented for hanging indent
+#   E127 continuation line over-indented for visual indent
+#   E128 continuation line under-indented for visual indent
+#   E129 visually indented line with same indent as next logical line
+#   E131 continuation line unaligned for hanging indent
+#   W503 line break before binary operator
+# F*** -- warnings native to flake8
 # D*** -- warnings from flake8-docstrings plugin
 # H*** -- warnings from flake8 hacking plugin (custom style checks beyond PEP8)
+#   H231 Python 3.x incompatible 'except x,y:' construct
 flake8-check: $(FLAKE8_PYFILES)
-   $(AM_V_GEN) if flake8 $^ 
--ignore=E123,E126,E127,E128,E129,E131,W503,D,H ${FLAKE8_FLAGS}; then touch $@; 
else exit 1; fi
+   $(AM_V_GEN) if flake8 $^ --select=H231 
--ignore=E123,E126,E127,E128,E129,E131,W503,D,H ${FLAKE8_FLAGS}; then touch $@; 
else exit 1; fi
 endif
 
 include $(srcdir)/manpages.mk
diff --git a/debian/ovs-monitor-ipsec b/debian/ovs-monitor-ipsec
index e79c755..efab91b 100755
--- a/debian/ovs-monitor-ipsec
+++ b/debian/ovs-monitor-ipsec
@@ -371,7 +371,7 @@ def update_ipsec(ipsec, interfaces, new_interfaces):
 
 try:
 ipsec.add_entry(vals["local_ip"], vals["remote_ip"], vals)
-except error.Error, msg:
+except error.Error as msg:
 vlog.warn("skipping ipsec config for %s: %s" % (name, msg))
 
 
diff --git a/ofproto/ipfix-gen-entities b/ofproto/ipfix-gen-entities
index 11d2aaf..54951b6 100755
--- a/ofproto/ipfix-gen-entities
+++ b/ofproto/ipfix-gen-entities
@@ -114,7 +114,7 @@ if __name__ == '__main__':
 try:
 options, args = getopt.gnu_getopt(sys.argv[1:], 'hV',
   ['help', 'version'])
-except getopt.GetoptError, geo:
+except getopt.GetoptError as geo:
 sys.stderr.write('%s: %s\n' % (sys.argv[0], geo.msg))
 sys.exit(1)
 
diff --git a/tests/test-json.py b/tests/test-json.py
index d9f0bfe..1356145 100644
--- a/tests/test-json.py
+++ b/tests/test-json.py
@@ -61,7 +61,7 @@ def main(argv):
 
 try:
 options, args = getopt.gnu_getopt(argv[1:], '', ['multiple'])
-except getopt.GetoptError, geo:
+except getopt.GetoptError as geo:
 sys.stderr.write("%s: %s\n" % (argv0, geo.msg))
 sys.exit(1)
 
diff --git a/tests/test-ovsdb.py b/tests/test-ovsdb.py
index e240208..5744fde 100644
--- a/tests/test-ovsdb.py
+++ b/tests/test-ovsdb.py
@@ -105,7 +105,7 @@ def do_parse_atoms(type_string, *atom_strings):
 try:
 atom = data.Atom.from_json(base, atom_json)
 print ovs.json.to_string(atom.to_json())
-except error.Error, e:
+except error.Error as e:
 print e.args[0].encode("utf8")
 
 
@@ -548,7 +548,7 @@ def main(argv):
 options, args = getopt.gnu_getopt(argv[1:], 't:h',
   ['timeout',
'help'])
-except getopt.GetoptError, geo:
+except getopt.GetoptError as geo:
 sys.stderr.write("%s: %s\n" % (ovs.util.PROGRAM_NAME, geo.msg))
 sys.exit(1)
 
@@ -617,6 +617,6 @@ def main(argv):
 if __name__ == '__main__':
 try:
 main(sys.argv)
-except error.Error, e:
+except error.Error as e:
 sys.stderr.write("%s\n" % e)
 sys.exit(1)
diff --git a/utili

[ovs-dev] [PATCH 9/9] python: Remove old style classes.

[Russell Bryant]

Python 3 removed support for "old-style classes".  Classes should always
inherit from object to get consistent behavior between Python 2 and 3.

Enable a flake8 warning to help prevent regressions in the future.

Signed-off-by: Russell Bryant 
---
 Makefile.am  | 3 ++-
 debian/ovs-monitor-ipsec | 4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 71cd1b9..a723c36 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -359,8 +359,9 @@ ALL_LOCAL += flake8-check
 #   H231 Python 3.x incompatible 'except x,y:' construct
 #   H232 Python 3.x incompatible octal 077 should be written as 0o77
 #   H233 Python 3.x incompatible use of print operator
+#   H238 old style class declaration, use new style (inherit from `object`)
 flake8-check: $(FLAKE8_PYFILES)
-   $(AM_V_GEN) if flake8 $^ --select=H231,H232,H233 
--ignore=E123,E126,E127,E128,E129,E131,W503,D,H ${FLAKE8_FLAGS}; then touch $@; 
else exit 1; fi
+   $(AM_V_GEN) if flake8 $^ --select=H231,H232,H233,H238 
--ignore=E123,E126,E127,E128,E129,E131,W503,D,H ${FLAKE8_FLAGS}; then touch $@; 
else exit 1; fi
 endif
 
 include $(srcdir)/manpages.mk
diff --git a/debian/ovs-monitor-ipsec b/debian/ovs-monitor-ipsec
index 2bccfb0..5d1cbf7 100755
--- a/debian/ovs-monitor-ipsec
+++ b/debian/ovs-monitor-ipsec
@@ -55,7 +55,7 @@ def unixctl_exit(conn, unused_argv, unused_aux):
 
 
 # Class to configure the racoon daemon, which handles IKE negotiation
-class Racoon:
+class Racoon(object):
 # Default locations for files
 conf_file = "/etc/racoon/racoon.conf"
 cert_dir = "/etc/racoon/certs"
@@ -259,7 +259,7 @@ path certificate "%s";
 # Class to configure IPsec on a system using racoon for IKE and setkey
 # for maintaining the Security Association Database (SAD) and Security
 # Policy Database (SPD).  Only policies for GRE are supported.
-class IPsec:
+class IPsec(object):
 def __init__(self):
 self.sad_flush()
 self.spd_flush()
-- 
2.5.0

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH net 0/2] vxlan: Set a large MTU on ovs-created vxlan devices

[David Miller]

From: David Wragg 
Date: Wed,  6 Jan 2016 13:33:04 +

> Prior to 4.3, openvswitch vxlan vports could transmit vxlan packets of
> any size, constrained only by the ability to transmit the resulting
> UDP packets.  4.3 introduced vxlan netdevs corresponding to vxlan
> vports.  These netdevs have an MTU, which limits the size of a packet
> that can be successfully vxlan-encapsulated.  The default value for
> this MTU is 1500, which is awkwardly small, and leads to a conspicuous
> change in behaviour for userspace.
> 
> These two patches set the MTU on openvswitch-crated vxlan devices to
> be 65465 (the maximum IP packet size minus the vxlan-on-IPv6
> overhead), effectively restoring the behaviour prior to 4.3.  In order
> to accomplish this, the first patch removes the MTU constraint of 1500
> for vxlan netdevs without an underlying device.

Is this really the right thing to do?  Won't we get a lot of fragmentation
by using such a large MTU, especially since you're making it the default
for OVS setups?

Things like path MTU discovery hinge strongly upon accurate MTU settings.
Otherwise they won't function properly.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] memory leak in recirc_state?

[Jarno Rajahalme]

> On Jan 5, 2016, at 8:24 PM, Ben Pfaff  wrote:
> 
> Thanks.  That was my experience also.
> 
> I applied this to master and branch-2.5.

Thanks for fixing this. I reviewed the patch and it seems correct to me as well.

>  I think that branch-2.4 has
> the same bug but the backport is not trivial and I do not know whether
> it is worthwhile.
> 

I’ll do the backport for branch-2.4, if that is fine by you.

  Jarno 

> On Tue, Jan 05, 2016 at 05:22:24PM -0800, William Tu wrote:
>> Hi Ben,
>> 
>> The patch works OK. It passes "make check" and "make check-valgrind"
>> without reporting memory leaks.
>> 
>> Thank you
>> William
>> 
>> On Tue, Jan 5, 2016 at 5:02 PM, William Tu  wrote:
>> 
>>> Hi Ben,
>>> 
>>> Sure, I will test this fix.
>>> 
>>> Regards,
>>> William
>>> 
>>> On Tue, Jan 5, 2016 at 4:55 PM, Ben Pfaff  wrote:
>>> 
 Thanks.  Would you mind testing this proposed fix?
http://openvswitch.org/pipermail/dev/2016-January/064070.html
 
 On Tue, Jan 05, 2016 at 11:32:48AM -0800, William Tu wrote:
> Hi Ben,
> 
> These two tests generate the leak:
> mpls_xlate
>381: MPLS xlate action
> ofproto-dpif
>852: ofproto-dpif - MPLS handling
> 
> ==65139==by 0x4E1C83: xmemdup (util.c:134)
> ==65139==by 0x431044: recirc_state_clone (ofproto-dpif-rid.c:221)
> ==65139==by 0x431044: recirc_alloc_id__ (ofproto-dpif-rid.c:238)
> ==65139==by 0x4315B8: recirc_alloc_id_ctx (ofproto-dpif-rid.c:281)
> ==65139==by 0x437C96: compose_recirculate_action__
> (ofproto-dpif-xlate.c:3643)
> ==65139==by 0x44095A: compose_recirculate_action
> (ofproto-dpif-xlate.c:3664)
> ==65139==by 0x44095A: xlate_actions (ofproto-dpif-xlate.c:5324)
> 
> Regards,
> William
> 
> 
> On Tue, Jan 5, 2016 at 11:22 AM, Ben Pfaff  wrote:
> 
>> I think that recirc_run needs to be modified so that
>> 
>> On Tue, Jan 05, 2016 at 06:40:23PM +, ChengChun Tu wrote:
>>> Hi Ben,
>>> 
>>> Yes, Valgrind testcase 381 reports leak and generates the call stack
>> below:
>>> I tried to debug it for a while but not able to understand it.
>> 
>> Thanks, what's the name of that test case?  The one I see as 381
 doesn't
>> seem relevant.
>> ___
>> dev mailing list
>> dev@openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
>> 
 
>>> 
>>> 
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] memory leak in recirc_state?

[Ben Pfaff]

On Wed, Jan 06, 2016 at 01:14:23PM -0800, Jarno Rajahalme wrote:
> 
> > On Jan 5, 2016, at 8:24 PM, Ben Pfaff  wrote:
> > 
> > Thanks.  That was my experience also.
> > 
> > I applied this to master and branch-2.5.
> 
> Thanks for fixing this. I reviewed the patch and it seems correct to me as 
> well.
> 
> >  I think that branch-2.4 has
> > the same bug but the backport is not trivial and I do not know whether
> > it is worthwhile.
> > 
> 
> I’ll do the backport for branch-2.4, if that is fine by you.

That sounds great.  Thank you.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] ofproto-dpif-rid: Fix memory leak in recirc_state.

[Jarno Rajahalme]

From: Ben Pfaff 

recirc_alloc_id__() copies the stack nothing ever freed it.

This patch is adopted from the corresponding commit 85b9cb2 on master.

CC: Ben Pfaff 
CC: Andy Zhou 
Reported-by: William Tu 
Reported-at: http://openvswitch.org/pipermail/dev/2016-January/064040.html
Signed-off-by: Jarno Rajahalme 
---
 ofproto/ofproto-dpif-rid.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/ofproto/ofproto-dpif-rid.c b/ofproto/ofproto-dpif-rid.c
index f1b3bdc..b840ea1 100644
--- a/ofproto/ofproto-dpif-rid.c
+++ b/ofproto/ofproto-dpif-rid.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2015 Nicira, Inc.
+ * Copyright (c) 2014, 2015, 2016 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -36,6 +36,8 @@ static uint32_t next_id OVS_GUARDED_BY(mutex); /* Possible 
next free id. */
 
 #define RECIRC_POOL_STATIC_IDS 1024
 
+static void recirc_id_node_free(struct recirc_id_node *);
+
 void
 recirc_init(void)
 {
@@ -88,7 +90,7 @@ recirc_run(void)
  * finished. */
 LIST_FOR_EACH_POP (node, exp_node, &expired) {
 cmap_remove(&id_map, &node->id_node, node->id);
-ovsrcu_postpone(free, node);
+ovsrcu_postpone(recirc_id_node_free, node);
 }
 
 if (!list_is_empty(&expiring)) {
@@ -315,6 +317,13 @@ recirc_alloc_id(struct ofproto_dpif *ofproto)
 return node->id;
 }
 
+static void
+recirc_id_node_free(struct recirc_id_node *node)
+{
+ofpbuf_delete(node->stack);
+free(node);
+}
+
 void
 recirc_id_node_unref(const struct recirc_id_node *node_)
 OVS_EXCLUDED(mutex)
-- 
2.1.4

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] memory leak in recirc_state?

[Jarno Rajahalme]

> On Jan 6, 2016, at 1:21 PM, Ben Pfaff  wrote:
> 
> On Wed, Jan 06, 2016 at 01:14:23PM -0800, Jarno Rajahalme wrote:
>> 
>>> On Jan 5, 2016, at 8:24 PM, Ben Pfaff  wrote:
>>> 
>>> Thanks.  That was my experience also.
>>> 
>>> I applied this to master and branch-2.5.
>> 
>> Thanks for fixing this. I reviewed the patch and it seems correct to me as 
>> well.
>> 
>>> I think that branch-2.4 has
>>> the same bug but the backport is not trivial and I do not know whether
>>> it is worthwhile.
>>> 
>> 
>> I’ll do the backport for branch-2.4, if that is fine by you.
> 
> That sounds great.  Thank you.

Done (but forgot to edit the subject line):

http://openvswitch.org/pipermail/dev/2016-January/064115.html 


  Jarno

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] ofproto-dpif-upcall: Avoid double-delete of ukeys.

[Joe Stringer]

On 5 January 2016 at 16:24, Ben Pfaff  wrote:
> revalidate_sweep__() has two cases where it calls ukey_delete() to
> remove a ukey from the umap via cmap_remove().  The first case is a direct
> call to ukey_delete(), when !flow_exists.  The second case is an indirect
> call via push_ukey_ops(), when result != UKEY_KEEP.  If both of these
> conditions are simultaneously true, however, the code would call
> ukey_delete() twice, causing an assertion failure in the second call.  This
> commit fixes the problem by eliminating one of the calls.
>
> Reported-by: Keith Holleman 
> Reported-at: 
> http://openvswitch.org/pipermail/discuss/2015-December/019772.html
> CC: Joe Stringer 
> VMware-BZ: #1579057
> Signed-off-by: Ben Pfaff 
>
> ---
>  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 cd8a9f0..ca25701 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -1,4 +1,4 @@
> -/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
> +/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc.
>   *
>   * Licensed under the Apache License, Version 2.0 (the "License");
>   * you may not use this file except in compliance with the License.
> @@ -2274,7 +2274,7 @@ revalidator_sweep__(struct revalidator *revalidator, 
> bool purge)
>  n_ops = 0;
>  }
>
> -if (!flow_exists) {
> +if (result == UKEY_KEEP && !flow_exists) {
>  ovs_mutex_lock(&umap->mutex);
>  ukey_delete(umap, ukey);
>  ovs_mutex_unlock(&umap->mutex);

Thanks for finding this.

In the common case, I would expect the flow to be deleted during the
revalidator "dump" (mark) phase. With the current code, and with this
approach, we may end up attempting to delete the same flows multiple
times. I suggest the below instead:

@@ -2262,7 +2264,7 @@ revalidator_sweep__(struct revalidator
*revalidator, bool purge)
 result = revalidate_ukey(udpif, ukey, &stats, &odp_actions,
  reval_seq, &recircs);
 }
-if (result != UKEY_KEEP) {
+if (flow_exists && result != UKEY_KEEP) {
 /* Takes ownership of 'recircs'. */
 reval_op_init(&ops[n_ops++], result, udpif, ukey, &recircs,
   &odp_actions);


I notice a couple of other minor issues as well while looking at this code:
- We may currently revalidate ukeys in revalidator_sweep() whenever
the dump_seq/reval_seq mismatch, even if the ukey's corresponding flow
is already deleted. This is unnecessary: If we have previously deleted
the flow, we can just delete the ukey.
- If there is a seq mismatch in dump_seq or reval_seq, and
revalidate_ukey() returns UKEY_MODIFY, the flow will be modified and
the ukey deleted. I believe that this will not clear the datapath
stats, so next time the flow is dumped, all previous stats for the
flow will be double-attributed. This seems fairly unlikely as it
should only happen if the seqs change in between dump phase and
revalidation phase, or if the flow is not dumped by the kernel (as
well as only if revalidation requires modification of the flow - ie
actions changed but flows did not).

I will send patches for these latter issues.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] datapath-windows: Fix bug small bug in GRE

[Sairam Venugopal]

Thanks for fixing this. I held off on ACK¹ing it until the original GRE
patch was applied.

Acked-by: Sairam Venugopal 


On 12/11/15, 2:24 PM, "Alin Serdean" 
wrote:

>Allow GRE encapsulation to take place in the case we have a TCP payload
>without LSO.
>
>Signed-off-by: Alin Gabriel Serdean 
>---
> datapath-windows/ovsext/Gre.c | 1 -
> 1 file changed, 1 deletion(-)
>
>diff --git a/datapath-windows/ovsext/Gre.c b/datapath-windows/ovsext/Gre.c
>index 3ebfda3..5abd4a4 100644
>--- a/datapath-windows/ovsext/Gre.c
>+++ b/datapath-windows/ovsext/Gre.c
>@@ -161,7 +161,6 @@ OvsDoEncapGre(POVS_VPORT_ENTRY vport,
> default:
> OVS_LOG_ERROR("Unknown LSO transmit type:%d",
>   tsoInfo.Transmit.Type);
>-return NDIS_STATUS_FAILURE;
> }
> OVS_LOG_TRACE("MSS %u packet len %u", mss,
>   packetLength);
>-- 
>1.9.5.msysgit.0
>___
>dev mailing list
>dev@openvswitch.org
>https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailma
>n_listinfo_dev&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=Dc
>ruz40PROJ40ROzSpxyQSLw6fcrOWpJgEcEmNR3JEQ&m=U5BW5DDo0rxxq1UinKX4KnTOWqBtbw
>VxeAbXnlsuJAk&s=Nsy4NjPh6a4AZHJoghfQim_cPvsMkRzKg6-HZy2EK4c&e= 

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH net 0/2] vxlan: Set a large MTU on ovs-created vxlan devices

[Jesse Gross]

On Wed, Jan 6, 2016 at 12:59 PM, David Miller  wrote:
> From: David Wragg 
> Date: Wed,  6 Jan 2016 13:33:04 +
>
>> Prior to 4.3, openvswitch vxlan vports could transmit vxlan packets of
>> any size, constrained only by the ability to transmit the resulting
>> UDP packets.  4.3 introduced vxlan netdevs corresponding to vxlan
>> vports.  These netdevs have an MTU, which limits the size of a packet
>> that can be successfully vxlan-encapsulated.  The default value for
>> this MTU is 1500, which is awkwardly small, and leads to a conspicuous
>> change in behaviour for userspace.
>>
>> These two patches set the MTU on openvswitch-crated vxlan devices to
>> be 65465 (the maximum IP packet size minus the vxlan-on-IPv6
>> overhead), effectively restoring the behaviour prior to 4.3.  In order
>> to accomplish this, the first patch removes the MTU constraint of 1500
>> for vxlan netdevs without an underlying device.
>
> Is this really the right thing to do?  Won't we get a lot of fragmentation
> by using such a large MTU, especially since you're making it the default
> for OVS setups?
>
> Things like path MTU discovery hinge strongly upon accurate MTU settings.
> Otherwise they won't function properly.

At a minimum, I don't think this should be VXLAN specific. But I agree
that I'm not sure this is the right thing to do.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2] datapath-windows: Add LSOv2 support for VXLAN

[Sairam Venugopal]

Thanks for the patch. Just to clarify, I think we still don¹t support IPv6
in OvsTcpSegmentNBL.

Acked-by: Sairam Venugopal 



On 12/11/15, 2:29 PM, "Alin Serdean" 
wrote:

>This patch adds LSO version 2 support for the windows datapath.
>(https://urldefense.proofpoint.com/v2/url?u=https-3A__msdn.microsoft.com_e
>n-2Dus_library_windows_hardware_ff568840-2528v-3Dvs.85-2529.aspx&d=BQIGaQ&
>c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=Dcruz40PROJ40ROzSpxyQSLw6f
>crOWpJgEcEmNR3JEQ&m=ewMobSdivrA6XR-BJ8hbfBp5jsgbMzrEHybjAZ24RO4&s=Y9h65-o1
>FW1buPSLpV6zKv8ZTZtuEaFY245ELBMKd7w&e= )
>
>Tested using psping and iperf3.
>
>Signed-off-by: Alin Gabriel Serdean 
>---
>v2: Allow VXLAN encapsulation to take place if no LSO is available
>---
> datapath-windows/ovsext/Vxlan.c | 18 +++---
> 1 file changed, 15 insertions(+), 3 deletions(-)
>
>diff --git a/datapath-windows/ovsext/Vxlan.c
>b/datapath-windows/ovsext/Vxlan.c
>index c0611da..2516ece 100644
>--- a/datapath-windows/ovsext/Vxlan.c
>+++ b/datapath-windows/ovsext/Vxlan.c
>@@ -190,6 +190,7 @@ OvsDoEncapVxlan(POVS_VPORT_ENTRY vport,
> POVS_VXLAN_VPORT vportVxlan;
> UINT32 headRoom = OvsGetVxlanTunHdrSize();
> UINT32 packetLength;
>+ULONG mss = 0;
> 
> /*
>  * XXX: the assumption currently is that the NBL is owned by OVS, and
>@@ -204,12 +205,23 @@ OvsDoEncapVxlan(POVS_VPORT_ENTRY vport,
> 
> tsoInfo.Value = NET_BUFFER_LIST_INFO(curNbl,
>  
>TcpLargeSendNetBufferListInfo);
>-OVS_LOG_TRACE("MSS %u packet len %u", tsoInfo.LsoV1Transmit.MSS,
>+switch (tsoInfo.Transmit.Type) {
>+case NDIS_TCP_LARGE_SEND_OFFLOAD_V1_TYPE:
>+mss = tsoInfo.LsoV1Transmit.MSS;
>+break;
>+case NDIS_TCP_LARGE_SEND_OFFLOAD_V2_TYPE:
>+mss = tsoInfo.LsoV2Transmit.MSS;
>+break;
>+default:
>+OVS_LOG_ERROR("Unknown LSO transmit type:%d",
>+  tsoInfo.Transmit.Type);
>+}
>+OVS_LOG_TRACE("MSS %u packet len %u", mss,
>   packetLength);
>-if (tsoInfo.LsoV1Transmit.MSS) {
>+if (mss) {
> OVS_LOG_TRACE("l4Offset %d", layers->l4Offset);
> *newNbl = OvsTcpSegmentNBL(switchContext, curNbl, layers,
>-   tsoInfo.LsoV1Transmit.MSS,
>headRoom);
>+   mss, headRoom);
> if (*newNbl == NULL) {
> OVS_LOG_ERROR("Unable to segment NBL");
> return NDIS_STATUS_FAILURE;
>-- 
>1.9.5.msysgit.0
>___
>dev mailing list
>dev@openvswitch.org
>https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailma
>n_listinfo_dev&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=Dc
>ruz40PROJ40ROzSpxyQSLw6fcrOWpJgEcEmNR3JEQ&m=ewMobSdivrA6XR-BJ8hbfBp5jsgbMz
>rEHybjAZ24RO4&s=hCu09m9IAwqR2mS7U5CxPbo2fRMEBqPvfN1lTdJGgHw&e= 

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH monitor_cond 00/12] Implement conditional monitoring

[Andy Zhou]

I have some comments on the patch series.

Instead of "monitor_cond_change", why not just have a more generic
"monitor_update" message, so we can update
monitor columns as well as conditions.

Instead of reusing monitor id,  when "monitor_update" can also change to
use a new value.  Since update message
embeds the monitor_id, a client can tell the version of monitor  (and
conditions) applied for the update. Of course, monitor
cancellation needs to use the new id.

Should any columns within schema be allowed in condition? or only the ones
being monitored?
From protocol viewpoint, it would make sense to allow any column.

I see that conditions are applied when generating updates.  Could this lead
to inconsistency when fast and slow connections receives different content
when condition changes?

Noticed that tabs are used in some files, for example, condition.[ch]

It may make the code simpler if we keep the 3 term "condition" also for
true and false functions, but normalize the column and data to some known
values.

ovsdb-server man page section 4.1.13 says monitor_cond_change should use
.  Is this a typo? We should be using  instead.



On Tue, Jan 5, 2016 at 1:54 PM, Andy Zhou  wrote:

>
>
> On Tue, Jan 5, 2016 at 5:13 AM, Liran Schour  wrote:
>
>> This patch series implements conditional monitoring by introducing an
>> OVSDB
>> RFC extension with 2 new JSON-RPC methods: "monitor_cond" and
>> "monitor_cond_change". Specification of this extension is defined in the
>> ovsdb-server (1) man page.
>> Monitor2 is now merged into monitor_cond. A monitor_cond session with an
>> empty
>> condition, will behave exactly like monitor2 and will get update2
>> notifications.
>>
>> Thanks for posting. I will take a closer look. When applying patches,
> git-am complained about
> some white space issues:
>
> Description: [ovs-dev,monitor_cond,02/12] ovsdb: add conditions utilities
> to support monitor_cond
> Applying: ovsdb: add conditions utilities to support monitor_cond
> /home/azhou/projs/ovs-review/ovs/.git/rebase-apply/patch:141: trailing
> whitespace.
> field = !columns_index_map ?
> warning: 1 line adds whitespace errors.
>
> Description: [ovs-dev,monitor_cond,04/12] ovsdb-client: support
> monitor-cond
> Applying: ovsdb-client: support monitor-cond
> /home/azhou/projs/ovs-review/ovs/.git/rebase-apply/patch:53: trailing
> whitespace.
> all rows will be monitored. If at least one \fIcolumn\fR is specified,
> only those
> warning: 1 line adds whitespace errors.
>
> Description: [ovs-dev,monitor_cond,07/12] ovsdb: enable jsonrpc-server to
> service "monitor_cond_change" request
> Applying: ovsdb: enable jsonrpc-server to service "monitor_cond_change"
> request
> /home/azhou/projs/ovs-review/ovs/.git/rebase-apply/patch:78: trailing
> whitespace.
>
> /home/azhou/projs/ovs-review/ovs/.git/rebase-apply/patch:180: trailing
> whitespace.
>
> warning: 2 lines add whitespace errors.
>
>
>
>> Note: With this patch the json cache will be used only for monitor
>> sessions
>> with an empty condition. We can think on implementing a per row json
>> cache that
>> will be matched against condition clauses in the future.
>>
>>
>> Can we add conditions as part of cache selector?
>
>>
>> ___
>> dev mailing list
>> dev@openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
>>
>
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 1/2] Fix error log when subscribe/unsubscribe Windows

[Sairam Venugopal]

Acked-by: Sairam Venugopal 


On 1/4/16, 3:04 PM, "Alin Serdean"  wrote:

>The warning message was inverted on the performed operation.
>
>Also use the error returned by nl_sock_subscribe_packet__.
>
>Signed-off-by: Alin Gabriel Serdean 
>---
> lib/netlink-socket.c | 8 
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
>diff --git a/lib/netlink-socket.c b/lib/netlink-socket.c
>index 5cf1027..5ef4b15 100644
>--- a/lib/netlink-socket.c
>+++ b/lib/netlink-socket.c
>@@ -372,8 +372,8 @@ nl_sock_subscribe_packets(struct nl_sock *sock)
> 
> error = nl_sock_subscribe_packet__(sock, true);
> if (error) {
>-VLOG_WARN("could not unsubscribe packets (%s)",
>-  ovs_strerror(errno));
>+VLOG_WARN("could not subscribe packets (%s)",
>+  ovs_strerror(error));
> return error;
> }
> sock->read_ioctl = OVS_IOCTL_READ_PACKET;
>@@ -388,8 +388,8 @@ nl_sock_unsubscribe_packets(struct nl_sock *sock)
> 
> int error = nl_sock_subscribe_packet__(sock, false);
> if (error) {
>-VLOG_WARN("could not subscribe to packets (%s)",
>-  ovs_strerror(errno));
>+VLOG_WARN("could not unsubscribe to packets (%s)",
>+  ovs_strerror(error));
> return error;
> }
> 
>-- 
>1.9.5.msysgit.0
>___
>dev mailing list
>dev@openvswitch.org
>http://openvswitch.org/mailman/listinfo/dev

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 2/2] datapath-windows: Fix subscribe/unsubscribe packets

[Sairam Venugopal]

Acked-by: Sairam Venugopal 


On 1/4/16, 3:04 PM, "Alin Serdean"  wrote:

>The policy of the subscribe packets is defined by the following:
>const NL_POLICY policy[] =  {
>[OVS_NL_ATTR_PACKET_PID] = {.type = NL_A_U32 },
>[OVS_NL_ATTR_PACKET_SUBSCRIBE] = {.type = NL_A_U8 }
>};
>Switch the value of the join operation with the one from the policy.
>
>Signed-off-by: Alin Gabriel Serdean 
>---
> datapath-windows/ovsext/User.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/datapath-windows/ovsext/User.c
>b/datapath-windows/ovsext/User.c
>index 42af7f3..04d2294 100644
>--- a/datapath-windows/ovsext/User.c
>+++ b/datapath-windows/ovsext/User.c
>@@ -1109,7 +1109,7 @@ fail:
>  */
> NTSTATUS
> OvsSubscribePacketCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
>-UINT32 *replyLen)
>+ UINT32 *replyLen)
> {
> NDIS_STATUS status;
> BOOLEAN rc;
>@@ -1135,7 +1135,7 @@
>OvsSubscribePacketCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
> goto done;
> }
> 
>-join = NlAttrGetU8(attrs[OVS_NL_ATTR_PACKET_PID]);
>+join = NlAttrGetU8(attrs[OVS_NL_ATTR_PACKET_SUBSCRIBE]);
> pid = NlAttrGetU32(attrs[OVS_NL_ATTR_PACKET_PID]);
> 
> /* The socket subscribed with must be the same socket we perform
>receive*/
>-- 
>1.9.5.msysgit.0
>___
>dev mailing list
>dev@openvswitch.org
>https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailma
>n_listinfo_dev&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=Dc
>ruz40PROJ40ROzSpxyQSLw6fcrOWpJgEcEmNR3JEQ&m=7GSagh1_WqUyklVEEc6SDINKWiKU9_
>pz6YKdLrII7XE&s=6TUjz1NYcvTdEEPTn5ev_wQ1H4bkeeyfyBfw6sXKLhc&e= 

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH net 0/2] vxlan: Set a large MTU on ovs-created vxlan devices

[David Wragg]

David Miller  writes:
>> Prior to 4.3, openvswitch vxlan vports could transmit vxlan packets of
>> any size, constrained only by the ability to transmit the resulting
>> UDP packets.  4.3 introduced vxlan netdevs corresponding to vxlan
>> vports.  These netdevs have an MTU, which limits the size of a packet
>> that can be successfully vxlan-encapsulated.  The default value for
>> this MTU is 1500, which is awkwardly small, and leads to a conspicuous
>> change in behaviour for userspace.
>> 
>> These two patches set the MTU on openvswitch-crated vxlan devices to
>> be 65465 (the maximum IP packet size minus the vxlan-on-IPv6
>> overhead), effectively restoring the behaviour prior to 4.3.  In order
>> to accomplish this, the first patch removes the MTU constraint of 1500
>> for vxlan netdevs without an underlying device.
>
> Is this really the right thing to do?

I'm certainly open to suggestions of better ways to solve the problem.

To be clear, the problem from our perspective is that a use of the
kernel openvswitch that worked fine in 4.2 and earlier is hobbled in
4.3.  Previously the MTU of an openvswitch-based vxlan overlay network
was constrained only by the MTU of the physical network.  In 4.3, we
can't take advantage of physical networks that support jumbo frames,
causing a huge hit to throughput across the overlay network.

The specific limit of 1500 seems very arbitrary.  For a vxlan overlay
network on top of a traditional ethernet network, the "correct" MTU for
the vxlan netdevs is 1450 rather than 1500.  And in general with
openvswitch, the destination for vxlan packets is determined on a
packet-by-packet basis, possibly involving different path MTUs of the
underlying network.  There is no single "correct" value.

> Won't we get a lot of fragmentation
> by using such a large MTU, especially since you're making it the default
> for OVS setups?

In the context of the openvswitch vxlan vport transmit path, I can't
find a place where the dev->mtu is used (and it would be surprising, on
the basis that the relevant parts of vxlan.c have not changed that much
since 4.2, when no netdev was involved in that path).

Considering non-openvswitch scenarios, when using vxlan netdevs
directly, a vxlan netdev locked to an underlying device supporting jumbo
frames can use a larger MTU.  It's only vxlan netdevs without an
underlying device that have the limit of 1500 imposed.  But why
shouldn't there be the same flexibility to select an MTU for best
performance in both cases?  Aren't the fragmentation concerns the same?

> Things like path MTU discovery hinge strongly upon accurate MTU settings.
> Otherwise they won't function properly.

True.  But in what sense is 1500 accurate?  Uses/users of the kernel
openvswitch code have always had to get this right, making sure that the
MTU set on a vxlan overlay network conforms to the underlying network
paths involved.

David
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] ofproto-dpif-rid: Fix memory leak in recirc_state.

[Ben Pfaff]

On Wed, Jan 06, 2016 at 01:58:28PM -0800, Jarno Rajahalme wrote:
> From: Ben Pfaff 
> 
> recirc_alloc_id__() copies the stack nothing ever freed it.
> 
> This patch is adopted from the corresponding commit 85b9cb2 on master.
> 
> CC: Ben Pfaff 
> CC: Andy Zhou 
> Reported-by: William Tu 
> Reported-at: http://openvswitch.org/pipermail/dev/2016-January/064040.html
> Signed-off-by: Jarno Rajahalme 

It looks good to me, but keep the original signoff in addition to adding
your own.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] ofproto-dpif-rid: Fix memory leak in recirc_state.

[Jarno Rajahalme]

> On Jan 6, 2016, at 3:27 PM, Ben Pfaff  wrote:
> 
> On Wed, Jan 06, 2016 at 01:58:28PM -0800, Jarno Rajahalme wrote:
>> From: Ben Pfaff 
>> 
>> recirc_alloc_id__() copies the stack nothing ever freed it.
>> 
>> This patch is adopted from the corresponding commit 85b9cb2 on master.
>> 
>> CC: Ben Pfaff 
>> CC: Andy Zhou 
>> Reported-by: William Tu 
>> Reported-at: http://openvswitch.org/pipermail/dev/2016-January/064040.html
>> Signed-off-by: Jarno Rajahalme 
> 
> It looks good to me, but keep the original signoff in addition to adding
> your own.

OK, pushed to branch-2.4.

  Jarno

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] ofproto-dpif-upcall: Avoid double-delete of ukeys.

[Ben Pfaff]

On Wed, Jan 06, 2016 at 02:50:36PM -0800, Joe Stringer wrote:
> On 5 January 2016 at 16:24, Ben Pfaff  wrote:
> > revalidate_sweep__() has two cases where it calls ukey_delete() to
> > remove a ukey from the umap via cmap_remove().  The first case is a direct
> > call to ukey_delete(), when !flow_exists.  The second case is an indirect
> > call via push_ukey_ops(), when result != UKEY_KEEP.  If both of these
> > conditions are simultaneously true, however, the code would call
> > ukey_delete() twice, causing an assertion failure in the second call.  This
> > commit fixes the problem by eliminating one of the calls.
> >
> > Reported-by: Keith Holleman 
> > Reported-at: 
> > http://openvswitch.org/pipermail/discuss/2015-December/019772.html
> > CC: Joe Stringer 
> > VMware-BZ: #1579057
> > Signed-off-by: Ben Pfaff 
> >
> > ---
> >  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 cd8a9f0..ca25701 100644
> > --- a/ofproto/ofproto-dpif-upcall.c
> > +++ b/ofproto/ofproto-dpif-upcall.c
> > @@ -1,4 +1,4 @@
> > -/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
> > +/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, 
> > Inc.
> >   *
> >   * Licensed under the Apache License, Version 2.0 (the "License");
> >   * you may not use this file except in compliance with the License.
> > @@ -2274,7 +2274,7 @@ revalidator_sweep__(struct revalidator *revalidator, 
> > bool purge)
> >  n_ops = 0;
> >  }
> >
> > -if (!flow_exists) {
> > +if (result == UKEY_KEEP && !flow_exists) {
> >  ovs_mutex_lock(&umap->mutex);
> >  ukey_delete(umap, ukey);
> >  ovs_mutex_unlock(&umap->mutex);
> 
> Thanks for finding this.
> 
> In the common case, I would expect the flow to be deleted during the
> revalidator "dump" (mark) phase. With the current code, and with this
> approach, we may end up attempting to delete the same flows multiple
> times. I suggest the below instead:
> 
> @@ -2262,7 +2264,7 @@ revalidator_sweep__(struct revalidator
> *revalidator, bool purge)
>  result = revalidate_ukey(udpif, ukey, &stats, &odp_actions,
>   reval_seq, &recircs);
>  }
> -if (result != UKEY_KEEP) {
> +if (flow_exists && result != UKEY_KEEP) {
>  /* Takes ownership of 'recircs'. */
>  reval_op_init(&ops[n_ops++], result, udpif, ukey, &recircs,
>&odp_actions);

Thanks!  I applied this change and pushed it to master and branch-2.5.

> I notice a couple of other minor issues as well while looking at this code:
> - We may currently revalidate ukeys in revalidator_sweep() whenever
> the dump_seq/reval_seq mismatch, even if the ukey's corresponding flow
> is already deleted. This is unnecessary: If we have previously deleted
> the flow, we can just delete the ukey.
> - If there is a seq mismatch in dump_seq or reval_seq, and
> revalidate_ukey() returns UKEY_MODIFY, the flow will be modified and
> the ukey deleted. I believe that this will not clear the datapath
> stats, so next time the flow is dumped, all previous stats for the
> flow will be double-attributed. This seems fairly unlikely as it
> should only happen if the seqs change in between dump phase and
> revalidation phase, or if the flow is not dumped by the kernel (as
> well as only if revalidation requires modification of the flow - ie
> actions changed but flows did not).
> 
> I will send patches for these latter issues.

Thanks!
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH net 0/2] vxlan: Set a large MTU on ovs-created vxlan devices

[Jesse Gross]

On Wed, Jan 6, 2016 at 3:25 PM, David Wragg  wrote:
> David Miller  writes:
>>> Prior to 4.3, openvswitch vxlan vports could transmit vxlan packets of
>>> any size, constrained only by the ability to transmit the resulting
>>> UDP packets.  4.3 introduced vxlan netdevs corresponding to vxlan
>>> vports.  These netdevs have an MTU, which limits the size of a packet
>>> that can be successfully vxlan-encapsulated.  The default value for
>>> this MTU is 1500, which is awkwardly small, and leads to a conspicuous
>>> change in behaviour for userspace.
>>>
>>> These two patches set the MTU on openvswitch-crated vxlan devices to
>>> be 65465 (the maximum IP packet size minus the vxlan-on-IPv6
>>> overhead), effectively restoring the behaviour prior to 4.3.  In order
>>> to accomplish this, the first patch removes the MTU constraint of 1500
>>> for vxlan netdevs without an underlying device.
>>
>> Is this really the right thing to do?
>
> I'm certainly open to suggestions of better ways to solve the problem.

One option is to simply set the MTU on the device from userspace.

The reality is that the code you're modifying is compatibility code.
Maybe we should make this change to preserve the old behavior for old
callers (although, again, it should not be just for VXLAN). But no new
features or tunnel types will be supported in this manner.

New or updated userspace programs should work by simply creating and
adding tunnel devices to OVS. That won't go through this path at all
so you're going to need to find another approach in the near future in
any case.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH net 0/2] vxlan: Set a large MTU on ovs-created vxlan devices

[Hannes Frederic Sowa]

Hi,

On 07.01.2016 00:57, Jesse Gross wrote:

On Wed, Jan 6, 2016 at 3:25 PM, David Wragg  wrote:

David Miller  writes:

Prior to 4.3, openvswitch vxlan vports could transmit vxlan packets of
any size, constrained only by the ability to transmit the resulting
UDP packets.  4.3 introduced vxlan netdevs corresponding to vxlan
vports.  These netdevs have an MTU, which limits the size of a packet
that can be successfully vxlan-encapsulated.  The default value for
this MTU is 1500, which is awkwardly small, and leads to a conspicuous
change in behaviour for userspace.

These two patches set the MTU on openvswitch-crated vxlan devices to
be 65465 (the maximum IP packet size minus the vxlan-on-IPv6
overhead), effectively restoring the behaviour prior to 4.3.  In order
to accomplish this, the first patch removes the MTU constraint of 1500
for vxlan netdevs without an underlying device.


Is this really the right thing to do?


I'm certainly open to suggestions of better ways to solve the problem.


One option is to simply set the MTU on the device from userspace.

The reality is that the code you're modifying is compatibility code.
Maybe we should make this change to preserve the old behavior for old
callers (although, again, it should not be just for VXLAN). But no new
features or tunnel types will be supported in this manner.

New or updated userspace programs should work by simply creating and
adding tunnel devices to OVS. That won't go through this path at all
so you're going to need to find another approach in the near future in
any case.


I don't see any other way as to make MTUs part of the flow if we want to 
have correct ip_local_error notifications. And those must also work 
across VMs, so openvswitch in quasi brouting mode would need to emit 
ICMP PtBs (hopefully with a correct source address, otherwise uRPF kills 
them before reaching the applications) or do error signaling via virtio_net.


Either the openvswitch user space can feed those information to the 
datapath or the ovs dataplane can do a lookup on the outer ip address 
while filling out the metadata_dst and caching it in the flow or we just 
keep the dst in the flow anyway. So a net_device used by ovs has no real 
mtu anymore.


Bye,
Hannes

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH net 0/2] vxlan: Set a large MTU on ovs-created vxlan devices

[David Wragg]

Jesse Gross  writes:
> On Wed, Jan 6, 2016 at 3:25 PM, David Wragg  wrote:
>> I'm certainly open to suggestions of better ways to solve the problem.
>
> One option is to simply set the MTU on the device from userspace.

If that worked I wouldn't be submitting a patch.

The MTU value of 1500 is not merely the default.  It is also the maximum
allowed for a vxlan netdev not associated with an underlying netdev.  If
you do e.g. "ip link set dev vxlan-6784 mtu 8950", where vxlan-6784
was created by an ovs vport, it fails with EINVAL.

The first patch of the two submitted removes that limit.

> The reality is that the code you're modifying is compatibility code.
> Maybe we should make this change to preserve the old behavior or old
> callers (although, again, it should not be just for VXLAN). But no new
> features or tunnel types will be supported in this manner.

That's fine.  Naturally, the ideal from our point of view is if the
compatibility code is fully compatible, so we don't have to make changes
on our side that involve different code paths for different kernel
versions.  That's what my patches are intended to achieve.

But we can live with such changes on our side, as long as there is some
reasonable way to do so.  In the case of this vxlan MTU issue, there
doesn't seem to be one.

> New or updated userspace programs should work by simply creating and
> adding tunnel devices to OVS. That won't go through this path at all
> so you're going to need to find another approach in the near future in
> any case.

Ok.  But please try to be gentle on the poor souls who have to come up
with a single codebase that works on a range of kernel versions going
back a few years.

David
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH net 0/2] vxlan: Set a large MTU on ovs-created vxlan devices

[Jesse Gross]

On Wed, Jan 6, 2016 at 4:14 PM, Hannes Frederic Sowa
 wrote:
> Hi,
>
>
> On 07.01.2016 00:57, Jesse Gross wrote:
>>
>> On Wed, Jan 6, 2016 at 3:25 PM, David Wragg  wrote:
>>>
>>> David Miller  writes:
>
> Prior to 4.3, openvswitch vxlan vports could transmit vxlan packets of
> any size, constrained only by the ability to transmit the resulting
> UDP packets.  4.3 introduced vxlan netdevs corresponding to vxlan
> vports.  These netdevs have an MTU, which limits the size of a packet
> that can be successfully vxlan-encapsulated.  The default value for
> this MTU is 1500, which is awkwardly small, and leads to a conspicuous
> change in behaviour for userspace.
>
> These two patches set the MTU on openvswitch-crated vxlan devices to
> be 65465 (the maximum IP packet size minus the vxlan-on-IPv6
> overhead), effectively restoring the behaviour prior to 4.3.  In order
> to accomplish this, the first patch removes the MTU constraint of 1500
> for vxlan netdevs without an underlying device.


 Is this really the right thing to do?
>>>
>>>
>>> I'm certainly open to suggestions of better ways to solve the problem.
>>
>>
>> One option is to simply set the MTU on the device from userspace.
>>
>> The reality is that the code you're modifying is compatibility code.
>> Maybe we should make this change to preserve the old behavior for old
>> callers (although, again, it should not be just for VXLAN). But no new
>> features or tunnel types will be supported in this manner.
>>
>> New or updated userspace programs should work by simply creating and
>> adding tunnel devices to OVS. That won't go through this path at all
>> so you're going to need to find another approach in the near future in
>> any case.
>
>
> I don't see any other way as to make MTUs part of the flow if we want to
> have correct ip_local_error notifications. And those must also work across
> VMs, so openvswitch in quasi brouting mode would need to emit ICMP PtBs
> (hopefully with a correct source address, otherwise uRPF kills them before
> reaching the applications) or do error signaling via virtio_net.

I actually implemented this a long time ago and then there was some
additional discussion on this about a year ago. I agree it's the right
solution overall but it's not entirely clearly to me how to get the
details correct.

> Either the openvswitch user space can feed those information to the datapath
> or the ovs dataplane can do a lookup on the outer ip address while filling
> out the metadata_dst and caching it in the flow or we just keep the dst in
> the flow anyway. So a net_device used by ovs has no real mtu anymore.

I agree that the concept of MTU is much more complicated than a single
number on a device, we just have to find the right way to model it.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH net 0/2] vxlan: Set a large MTU on ovs-created vxlan devices

[Jesse Gross]

On Wed, Jan 6, 2016 at 4:29 PM, David Wragg  wrote:
> Jesse Gross  writes:
>> On Wed, Jan 6, 2016 at 3:25 PM, David Wragg  wrote:
>>> I'm certainly open to suggestions of better ways to solve the problem.
>>
>> One option is to simply set the MTU on the device from userspace.
>
> If that worked I wouldn't be submitting a patch.
>
> The MTU value of 1500 is not merely the default.  It is also the maximum
> allowed for a vxlan netdev not associated with an underlying netdev.  If
> you do e.g. "ip link set dev vxlan-6784 mtu 8950", where vxlan-6784
> was created by an ovs vport, it fails with EINVAL.
>
> The first patch of the two submitted removes that limit.

I saw your first patch and I agree that it fixes a problem. I was
referring to the second patch.

>> The reality is that the code you're modifying is compatibility code.
>> Maybe we should make this change to preserve the old behavior or old
>> callers (although, again, it should not be just for VXLAN). But no new
>> features or tunnel types will be supported in this manner.
>
> That's fine.  Naturally, the ideal from our point of view is if the
> compatibility code is fully compatible, so we don't have to make changes
> on our side that involve different code paths for different kernel
> versions.  That's what my patches are intended to achieve.

The intention is to be fully backwards compatible with existing
software. If you want to take advantage of future functionality then,
yes, you will need to change to the new model.

I agree that behavior changed with existing compatibility code. I'm
fine with your series as long as you generalize it to all tunnel types
and not just VXLAN. Just be aware that you're going to have to find
another solution long term.

> Ok.  But please try to be gentle on the poor souls who have to come up
> with a single codebase that works on a range of kernel versions going
> back a few years.

I maintain a large program that needs to do this myself, so I am aware
that it can be challenging.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH repost 1/2] ovs-ofctl: Generalize code for finding ports into general-purpose iterator.

[Andy Zhou]

On Mon, Dec 21, 2015 at 3:26 PM, Ben Pfaff  wrote:

> From: Ben Pfaff 
>
> The port_iterator will acquire another user in an upcoming commit.
>
> Signed-off-by: Ben Pfaff 
> ---
>
Acked-by: Andy Zhou 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH repost] Better abstract OFPT_SET_CONFIG and OFPT_GET_CONFIG_REPLY, make stricter.

[Andy Zhou]

On Mon, Dec 21, 2015 at 3:38 PM, Ben Pfaff  wrote:

> From: Ben Pfaff 
>
> The OFPT_SET_CONFIG and OFPT_GET_CONFIG_REPLY messages, which have the
> same format, have a 'flags' field in which OpenFlow defines some bits,
> which change somewhat from one version to another, and does not define
> others.  Until now, Open vSwitch has not abstracted these messages at all
> and has ignored the bits that OpenFlow leaves undefined.  This commit
> abstracts the messages in the same way as other OpenFlow messages and
> validates in OFPT_SET_CONFIG messages that the undefined bits are set to
> zero.
>
> OpenFlow 1.1 and 1.2, but not OpenFlow 1.0, define a flag named
> OFPC_INVALID_TTL_TO_CONTROLLER.  Open vSwitch has until now also
> implemented this as an extension to OpenFlow 1.0, and this commit retains
> that extension.
>
> Reported-by: Manpreet Singh 
> Signed-off-by: Ben Pfaff 
> ---
>
Acked-by: Andy Zhou 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] test-aa: fix memory leak reported by valgrind

[William Tu]

test case 1698: auto-attach - packet tests
Report several leaks at lldp_create_dummy(), the patch fixes the
following 3 leaks:
{lldp_send (lldp.c:334), lldp_decode (lldp.c:374),
 lldp_create_dummy (ovs-lldp.c:890)}
test_aa_send (test-aa.c:252)
test_aa_main (test-aa.c:281)
Comments:
1. Create a new function "lldp_destroy_dummy()" because
   many structures and its elements, ex: lldp_hardware and lldp_chassis,
   are from stack not heap (see test_aa_send). As a result, calling
   lldpd_cleanup() is incorrect.
2. Remove lchassis->c_id = xmalloc(ETH_ADDR_LEN);
   because it is overwritten at test_aa_send()
3. remove memcpy(&hw->h_lport.p_element.system_id.system_mac,
   lchassis->c_id, lchassis->c_id_len);
   because the source buf is empty

Signed-off-by: William Tu 
---
 lib/ovs-lldp.c  | 34 ++
 lib/ovs-lldp.h  |  1 +
 tests/test-aa.c |  5 +
 3 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/lib/ovs-lldp.c b/lib/ovs-lldp.c
index bd1729c..16225b5 100644
--- a/lib/ovs-lldp.c
+++ b/lib/ovs-lldp.c
@@ -887,7 +887,6 @@ lldp_create_dummy(void)
 lchassis->c_cap_enabled = LLDP_CAP_BRIDGE;
 lchassis->c_id_subtype = LLDP_CHASSISID_SUBTYPE_LLADDR;
 lchassis->c_id_len = ETH_ADDR_LEN;
-lchassis->c_id = xmalloc(ETH_ADDR_LEN);
 
 list_init(&lchassis->c_mgmt);
 lchassis->c_ttl = LLDP_CHASSIS_TTL;
@@ -911,8 +910,6 @@ lldp_create_dummy(void)
 /* Auto Attach element tlv */
 hw->h_lport.p_element.type = LLDP_TLV_AA_ELEM_TYPE_CLIENT_VIRTUAL_SWITCH;
 hw->h_lport.p_element.mgmt_vlan = 0;
-memcpy(&hw->h_lport.p_element.system_id.system_mac,
-   lchassis->c_id, lchassis->c_id_len);
 hw->h_lport.p_element.system_id.conn_type =
 LLDP_TLV_AA_ELEM_CONN_TYPE_SINGLE;
 hw->h_lport.p_element.system_id.rsvd = 0;
@@ -950,7 +947,7 @@ lldp_unref(struct lldp *lldp)
 free(lldp);
 }
 
-/* Unreference a specific LLDP instance.
+/* Reference a specific LLDP instance.
  */
 struct lldp *
 lldp_ref(const struct lldp *lldp_)
@@ -961,3 +958,32 @@ lldp_ref(const struct lldp *lldp_)
 }
 return lldp;
 }
+
+void
+lldp_destroy_dummy(struct lldp *lldp)
+{
+struct lldpd_hardware *hw, *hw_next;
+struct lldpd_chassis *chassis, *chassis_next;
+struct lldpd *cfg;
+
+if (!lldp) {
+return;
+}
+
+cfg = lldp->lldpd;
+
+LIST_FOR_EACH_SAFE (hw, hw_next, h_entries, &cfg->g_hardware) {
+list_remove(&hw->h_entries);
+free(hw->h_lport.p_lastframe);
+free(hw);
+}
+
+LIST_FOR_EACH_SAFE (chassis, chassis_next, list, &cfg->g_chassis) {
+list_remove(&chassis->list);
+free(chassis);
+}
+
+free(lldp->lldpd);
+free(lldp);
+}
+
diff --git a/lib/ovs-lldp.h b/lib/ovs-lldp.h
index e780e5b..71dff44 100644
--- a/lib/ovs-lldp.h
+++ b/lib/ovs-lldp.h
@@ -103,5 +103,6 @@ int aa_mapping_unregister(void *aux);
 
 /* Used by unit tests */
 struct lldp * lldp_create_dummy(void);
+void lldp_destroy_dummy(struct lldp *);
 
 #endif /* OVS_LLDP_H */
diff --git a/tests/test-aa.c b/tests/test-aa.c
index 2da572d..eedefeb 100644
--- a/tests/test-aa.c
+++ b/tests/test-aa.c
@@ -267,6 +267,11 @@ test_aa_send(void)
 /* Verify auto attach values */
 check_received_aa(&hardware.h_lport, nport, map_init);
 
+lldpd_chassis_cleanup(nchassis, true);
+lldpd_port_cleanup(nport, true);
+free(nport);
+lldp_destroy_dummy(lldp);
+
 return 0;
 }
 
-- 
2.5.0

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] Returned mail: see transcript for details

[The Post Office]

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev