[ovs-dev] [PATCH 1/8] bridge: Publish error count in database's rstp_statistics.

2015-02-19 Thread Ben Pfaff
The lower layers count errors but until now nothing actually reported them.

Found by inspection.

Signed-off-by: Ben Pfaff 
---
 vswitchd/bridge.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 297c0dd..a143be1 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
+/* Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -2489,8 +2489,8 @@ port_refresh_rstp_status(struct port *port)
 struct ofproto *ofproto = port->bridge->ofproto;
 struct iface *iface;
 struct ofproto_port_rstp_status status;
-char *keys[3];
-int64_t int_values[3];
+char *keys[4];
+int64_t int_values[4];
 struct smap smap;
 
 if (port_is_synthetic(port)) {
@@ -2539,6 +2539,8 @@ port_refresh_rstp_status(struct port *port)
 int_values[1] = status.rx_count;
 keys[2] = "rstp_uptime";
 int_values[2] = status.uptime;
+keys[3] = "rstp_error_count";
+int_values[3] = status.error_count;
 ovsrec_port_set_rstp_statistics(port->cfg, keys, int_values,
 ARRAY_SIZE(int_values));
 }
-- 
2.1.3

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


[ovs-dev] [PATCH 0/8] Initial OVN design documentation

2015-02-19 Thread Ben Pfaff
Most of this series is minor bug fixes and documentation improvements.
These are ready for review.  The part you actually care about, though, 
is the final patch that provides an initial stab at some preliminary design
documentation for the OVN project.  Feedback is honestly appreciated.

Ben Pfaff (8):
  bridge: Publish error count in database's rstp_statistics.
  vswitch: Document columns that had been previously overlooked.
  vtep: Document the ipaddr column in the Mcast_Macs_Local table.
  ovsdb-doc: Flag an error when a table or a column is left
undocumented.
  ovsdb-doc: Get manpage name from the XML file instead of command line.
  ovsdb-doc: Factor out nroff formatting into a separate Python module.
  xml2nroff: New program to generate a manpage from XML input.
  [RFC] ovn: Start work on design documentation.

 AUTHORS|   1 +
 Makefile.am|   2 +
 build-aux/xml2nroff| 110 
 configure.ac   |   3 +-
 ovn/automake.mk|  75 +
 ovn/ovn-architecture.7.xml | 331 ++
 ovn/ovn-controller.8.in|  41 +++
 ovn/ovn-nb.ovsschema   |  63 +
 ovn/ovn-nb.xml | 225 +++
 ovn/ovn.ovsschema  |  47 
 ovn/ovn.xml| 475 +++
 ovsdb/ovsdb-doc| 176 +++-
 python/automake.mk |   6 +
 python/build/__init__.py   |   0
 python/build/nroff.py  | 196 +
 vswitchd/automake.mk   |   1 -
 vswitchd/bridge.c  |   8 +-
 vswitchd/vswitch.xml   | 682 ++---
 vtep/automake.mk   |   1 -
 vtep/vtep.xml  |   6 +-
 20 files changed, 2008 insertions(+), 441 deletions(-)
 create mode 100755 build-aux/xml2nroff
 create mode 100644 ovn/automake.mk
 create mode 100644 ovn/ovn-architecture.7.xml
 create mode 100644 ovn/ovn-controller.8.in
 create mode 100644 ovn/ovn-nb.ovsschema
 create mode 100644 ovn/ovn-nb.xml
 create mode 100644 ovn/ovn.ovsschema
 create mode 100644 ovn/ovn.xml
 create mode 100644 python/build/__init__.py
 create mode 100644 python/build/nroff.py

-- 
2.1.3

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


[ovs-dev] [PATCH 2/8] vswitch: Document columns that had been previously overlooked.

2015-02-19 Thread Ben Pfaff
A fair number of columns had been overlooked.  This documents them.

The patch is smaller than it appears because this rearranges the STP and
RSTP documentation to group configuration, status, and statistics together
in the documentation for clarity.

Signed-off-by: Ben Pfaff 
---
 vswitchd/vswitch.xml | 680 +--
 1 file changed, 391 insertions(+), 289 deletions(-)

diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 932e4b2..04de3ca 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -681,80 +681,232 @@
 
 
 
-  The IEEE 802.1D Spanning Tree Protocol (STP) is a network protocol
-  that ensures loop-free topologies.  It allows redundant links to
-  be included in the network to provide automatic backup paths if
-  the active links fails.
+  
+The IEEE 802.1D Spanning Tree Protocol (STP) is a network protocol
+that ensures loop-free topologies.  It allows redundant links to
+be included in the network to provide automatic backup paths if
+the active links fails.
+  
 
-  
-Enable spanning tree on the bridge.  By default, STP is disabled
-on bridges.  Bond, internal, and mirror ports are not supported
-and will not participate in the spanning tree.
-  
+  
+These settings configure the slower-to-converge but still widely
+supported version of Spanning Tree Protocol, sometimes known as
+802.1D-1998.  Open vSwitch also supports the newer Rapid Spanning Tree
+Protocol (RSTP), documented later in the section titled Rapid
+Spanning Tree Configuration.
+  
 
-  
-The bridge's STP identifier (the lower 48 bits of the bridge-id)
-in the form
-
xx:xx:xx:xx:xx:xx.
-By default, the identifier is the MAC address of the bridge.
-  
+  
+
+  
+Enable spanning tree on the bridge.  By default, STP is disabled
+on bridges.  Bond, internal, and mirror ports are not supported
+and will not participate in the spanning tree.
+  
 
-  
-The bridge's relative priority value for determining the root
-bridge (the upper 16 bits of the bridge-id).  A bridge with the
-lowest bridge-id is elected the root.  By default, the priority
-is 0x8000.
-  
+  
+STP and RSTP are mutually exclusive.  If both are enabled, RSTP
+will be used.
+  
+
 
-  
-The interval between transmissions of hello messages by
-designated ports, in seconds.  By default the hello interval is
-2 seconds.
-  
+
+  The bridge's STP identifier (the lower 48 bits of the bridge-id)
+  in the form
+  
xx:xx:xx:xx:xx:xx.
+  By default, the identifier is the MAC address of the bridge.
+
 
-  
-The maximum age of the information transmitted by the bridge
-when it is the root bridge, in seconds.  By default, the maximum
-age is 20 seconds.
-  
+
+  The bridge's relative priority value for determining the root
+  bridge (the upper 16 bits of the bridge-id).  A bridge with the
+  lowest bridge-id is elected the root.  By default, the priority
+  is 0x8000.
+
 
-  
-The delay to wait between transitioning root and designated
-ports to forwarding, in seconds.  By default, the
-forwarding delay is 15 seconds.
-  
+
+  The interval between transmissions of hello messages by
+  designated ports, in seconds.  By default the hello interval is
+  2 seconds.
+
 
-  
-
-  The maximum number of seconds to retain a multicast snooping entry 
for
-  which no packets have been seen.  The default is currently 300
-  seconds (5 minutes).  The value, if specified, is forced into a
-  reasonable range, currently 15 to 3600 seconds.
-
-  
+
+  The maximum age of the information transmitted by the bridge
+  when it is the root bridge, in seconds.  By default, the maximum
+  age is 20 seconds.
+
 
-  
+
+  The delay to wait between transitioning root and designated
+  ports to forwarding, in seconds.  By default, the
+  forwarding delay is 15 seconds.
+
+
+
+  
+The maximum number of seconds to retain a multicast snooping entry 
for
+which no packets have been seen.  The default is currently 300
+seconds (5 minutes).  The value, if specified, is forced into a
+reasonable range, currently 15 to 3600 seconds.
+  
+
+
+
+  
+The maximum number of multicast snooping addresses to learn.  The
+default is currently 2048.  The v

[ovs-dev] [PATCH 3/8] vtep: Document the ipaddr column in the Mcast_Macs_Local table.

2015-02-19 Thread Ben Pfaff
This had been overlooked.

Signed-off-by: Ben Pfaff 
---
 vtep/vtep.xml | 4 
 1 file changed, 4 insertions(+)

diff --git a/vtep/vtep.xml b/vtep/vtep.xml
index 80fc99d..7ed7f43 100644
--- a/vtep/vtep.xml
+++ b/vtep/vtep.xml
@@ -776,6 +776,10 @@
   addresses of the appropriate VTEP(s).
 
 
+
+  The IP address to which this MAC corresponds. Optional field for
+  the purpose of ARP supression.
+
   
 
   
-- 
2.1.3

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


[ovs-dev] [PATCH 4/8] ovsdb-doc: Flag an error when a table or a column is left undocumented.

2015-02-19 Thread Ben Pfaff
This should make it harder to forget documentation.

Signed-off-by: Ben Pfaff 
---
 ovsdb/ovsdb-doc | 23 ---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/ovsdb/ovsdb-doc b/ovsdb/ovsdb-doc
index 6200915..b670452 100755
--- a/ovsdb/ovsdb-doc
+++ b/ovsdb/ovsdb-doc
@@ -152,7 +152,7 @@ def typeAndConstraintsToNroff(column):
 type += " (must be unique within table)"
 return type
 
-def columnGroupToNroff(table, groupXml):
+def columnGroupToNroff(table, groupXml, documented_columns):
 introNodes = []
 columnNodes = []
 for node in groupXml.childNodes:
@@ -172,6 +172,7 @@ def columnGroupToNroff(table, groupXml):
 for node in columnNodes:
 if node.tagName == 'column':
 name = node.attributes['name'].nodeValue
+documented_columns.add(name)
 column = table.columns[name]
 if node.hasAttribute('key'):
 key = node.attributes['key'].nodeValue
@@ -219,7 +220,8 @@ def columnGroupToNroff(table, groupXml):
 summary += [('column', nameNroff, typeNroff)]
 elif node.tagName == 'group':
 title = node.attributes["title"].nodeValue
-subSummary, subIntro, subBody = columnGroupToNroff(table, node)
+subSummary, subIntro, subBody = columnGroupToNroff(
+table, node, documented_columns)
 summary += [('group', title, subSummary)]
 body += '.ST "%s:"\n' % textToNroff(title)
 body += subIntro + subBody
@@ -242,15 +244,24 @@ def tableToNroff(schema, tableXml):
 tableName = tableXml.attributes['name'].nodeValue
 table = schema.tables[tableName]
 
+documented_columns = set()
 s = """.bp
 .SH "%s TABLE"
 """ % tableName
-summary, intro, body = columnGroupToNroff(table, tableXml)
+summary, intro, body = columnGroupToNroff(table, tableXml,
+  documented_columns)
 s += intro
 s += '.SS "Summary:\n'
 s += tableSummaryToNroff(summary)
 s += '.SS "Details:\n'
 s += body
+
+schema_columns = set(table.columns.keys())
+undocumented_columns = schema_columns - documented_columns
+for column in undocumented_columns:
+raise error.Error("table %s has undocumented column %s"
+  % (tableName, column))
+
 return s
 
 def docsToNroff(schemaFile, xmlFile, erFile, title=None, version=None):
@@ -306,6 +317,12 @@ def docsToNroff(schemaFile, xmlFile, erFile, title=None, 
version=None):
 else:
 introNodes += [dbNode]
 
+documented_tables = set((name for (name, title) in summary))
+schema_tables = set(schema.tables.keys())
+undocumented_tables = schema_tables - documented_tables
+for table in undocumented_tables:
+raise error.Error("undocumented table %s" % table)
+
 s += blockXmlToNroff(introNodes) + "\n"
 
 s += r"""
-- 
2.1.3

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


[ovs-dev] [PATCH 6/8] ovsdb-doc: Factor out nroff formatting into a separate Python module.

2015-02-19 Thread Ben Pfaff
This will make it cleaner to add another build-time program that generates
nroff from XML.

Signed-off-by: Ben Pfaff 
---
 ovsdb/ovsdb-doc  | 135 +---
 python/automake.mk   |   6 ++
 python/build/__init__.py |   0
 python/build/nroff.py| 196 +++
 4 files changed, 206 insertions(+), 131 deletions(-)
 create mode 100644 python/build/__init__.py
 create mode 100644 python/build/nroff.py

diff --git a/ovsdb/ovsdb-doc b/ovsdb/ovsdb-doc
index 15618ef..369efa9 100755
--- a/ovsdb/ovsdb-doc
+++ b/ovsdb/ovsdb-doc
@@ -3,7 +3,6 @@
 from datetime import date
 import getopt
 import os
-import re
 import sys
 import xml.dom.minidom
 
@@ -11,136 +10,9 @@ import ovs.json
 from ovs.db import error
 import ovs.db.schema
 
-argv0 = sys.argv[0]
-
-def textToNroff(s, font=r'\fR'):
-def escape(match):
-c = match.group(0)
-if c.startswith('-'):
-if c != '-' or font == r'\fB':
-return '\\' + c
-else:
-return '-'
-if c == '\\':
-return r'\e'
-elif c == '"':
-return r'\(dq'
-elif c == "'":
-return r'\(cq'
-else:
-raise error.Error("bad escape")
+from build.nroff import *
 
-# Escape - \ " ' as needed by nroff.
-s = re.sub('(-[0-9]|[-"\'])', escape, s)
-if s.startswith('.'):
-s = '\\' + s
-return s
-
-def escapeNroffLiteral(s):
-return r'\fB%s\fR' % textToNroff(s, r'\fB')
-
-def inlineXmlToNroff(node, font):
-if node.nodeType == node.TEXT_NODE:
-return textToNroff(node.data, font)
-elif node.nodeType == node.ELEMENT_NODE:
-if node.tagName in ['code', 'em', 'option']:
-s = r'\fB'
-for child in node.childNodes:
-s += inlineXmlToNroff(child, r'\fB')
-return s + font
-elif node.tagName == 'ref':
-s = r'\fB'
-if node.hasAttribute('column'):
-s += node.attributes['column'].nodeValue
-if node.hasAttribute('key'):
-s += ':' + node.attributes['key'].nodeValue
-elif node.hasAttribute('table'):
-s += node.attributes['table'].nodeValue
-elif node.hasAttribute('group'):
-s += node.attributes['group'].nodeValue
-else:
-raise error.Error("'ref' lacks required attributes: %s" % 
node.attributes.keys())
-return s + font
-elif node.tagName == 'var':
-s = r'\fI'
-for child in node.childNodes:
-s += inlineXmlToNroff(child, r'\fI')
-return s + font
-else:
-raise error.Error("element <%s> unknown or invalid here" % 
node.tagName)
-else:
-raise error.Error("unknown node %s in inline xml" % node)
-
-def blockXmlToNroff(nodes, para='.PP'):
-s = ''
-for node in nodes:
-if node.nodeType == node.TEXT_NODE:
-s += textToNroff(node.data)
-s = s.lstrip()
-elif node.nodeType == node.ELEMENT_NODE:
-if node.tagName in ['ul', 'ol']:
-if s != "":
-s += "\n"
-s += ".RS\n"
-i = 0
-for liNode in node.childNodes:
-if (liNode.nodeType == node.ELEMENT_NODE
-and liNode.tagName == 'li'):
-i += 1
-if node.tagName == 'ul':
-s += ".IP \\(bu\n"
-else:
-s += ".IP %d. .25in\n" % i
-s += blockXmlToNroff(liNode.childNodes, ".IP")
-elif (liNode.nodeType != node.TEXT_NODE
-  or not liNode.data.isspace()):
-raise error.Error("<%s> element may only have  
children" % node.tagName)
-s += ".RE\n"
-elif node.tagName == 'dl':
-if s != "":
-s += "\n"
-s += ".RS\n"
-prev = "dd"
-for liNode in node.childNodes:
-if (liNode.nodeType == node.ELEMENT_NODE
-and liNode.tagName == 'dt'):
-if prev == 'dd':
-s += '.TP\n'
-else:
-s += '.TQ\n'
-prev = 'dt'
-elif (liNode.nodeType == node.ELEMENT_NODE
-  and liNode.tagName == 'dd'):
-if prev == 'dd':
-s += '.IP\n'
-prev = 'dd'
-elif (liNode.nodeType != node.TEXT_NODE
-  or not liNode.data.isspace()):
-raise error.Error(" element may only have  and 
 children")
-s +

[ovs-dev] [PATCH 5/8] ovsdb-doc: Get manpage name from the XML file instead of command line.

2015-02-19 Thread Ben Pfaff
This seems like a better place for it.

Signed-off-by: Ben Pfaff 
---
 ovsdb/ovsdb-doc  | 18 --
 vswitchd/automake.mk |  1 -
 vswitchd/vswitch.xml |  2 +-
 vtep/automake.mk |  1 -
 vtep/vtep.xml|  2 +-
 5 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/ovsdb/ovsdb-doc b/ovsdb/ovsdb-doc
index b670452..15618ef 100755
--- a/ovsdb/ovsdb-doc
+++ b/ovsdb/ovsdb-doc
@@ -264,7 +264,7 @@ def tableToNroff(schema, tableXml):
 
 return s
 
-def docsToNroff(schemaFile, xmlFile, erFile, title=None, version=None):
+def docsToNroff(schemaFile, xmlFile, erFile, version=None):
 schema = ovs.db.schema.DbSchema.from_json(ovs.json.from_file(schemaFile))
 doc = xml.dom.minidom.parse(xmlFile).documentElement
 
@@ -272,8 +272,10 @@ def docsToNroff(schemaFile, xmlFile, erFile, title=None, 
version=None):
 xmlDate = os.stat(xmlFile).st_mtime
 d = date.fromtimestamp(max(schemaDate, xmlDate))
 
-if title == None:
-title = schema.name
+if doc.hasAttribute('name'):
+manpage = doc.attributes['name'].nodeValue
+else:
+manpage = schema.name
 
 if version == None:
 version = "UNKNOWN"
@@ -297,7 +299,7 @@ def docsToNroff(schemaFile, xmlFile, erFile, title=None, 
version=None):
 .SH NAME
 %s \- %s database schema
 .PP
-''' % (title, schema.version, version, textToNroff(schema.name), schema.name)
+''' % (manpage, schema.version, version, textToNroff(manpage), schema.name)
 
 tables = ""
 introNodes = []
@@ -378,7 +380,6 @@ where SCHEMA is an OVSDB schema in JSON format
 
 The following options are also available:
   --er-diagram=DIAGRAM.PICinclude E-R diagram from DIAGRAM.PIC
-  --title=TITLE   use TITLE as title instead of schema name
   --version=VERSION   use VERSION to display on document footer
   -h, --help  display this help message\
 """ % {'argv0': argv0}
@@ -388,20 +389,17 @@ if __name__ == "__main__":
 try:
 try:
 options, args = getopt.gnu_getopt(sys.argv[1:], 'hV',
-  ['er-diagram=', 'title=',
+  ['er-diagram=',
'version=', 'help'])
 except getopt.GetoptError, geo:
 sys.stderr.write("%s: %s\n" % (argv0, geo.msg))
 sys.exit(1)
 
 er_diagram = None
-title = None
 version = None
 for key, value in options:
 if key == '--er-diagram':
 er_diagram = value
-elif key == '--title':
-title = value
 elif key == '--version':
 version = value
 elif key in ['-h', '--help']:
@@ -415,7 +413,7 @@ if __name__ == "__main__":
 sys.exit(1)
 
 # XXX we should warn about undocumented tables or columns
-s = docsToNroff(args[0], args[1], er_diagram, title, version)
+s = docsToNroff(args[0], args[1], er_diagram, version)
 for line in s.split("\n"):
 line = line.strip()
 if len(line):
diff --git a/vswitchd/automake.mk b/vswitchd/automake.mk
index 80affe9..2f07c0f 100644
--- a/vswitchd/automake.mk
+++ b/vswitchd/automake.mk
@@ -48,7 +48,6 @@ vswitchd/ovs-vswitchd.conf.db.5: \
ovsdb/ovsdb-doc vswitchd/vswitch.xml vswitchd/vswitch.ovsschema \
$(VSWITCH_PIC)
$(AM_V_GEN)$(OVSDB_DOC) \
-   --title="ovs-vswitchd.conf.db" \
$(VSWITCH_DOT_DIAGRAM_ARG) \
--version=$(VERSION) \
$(srcdir)/vswitchd/vswitch.ovsschema \
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 04de3ca..c6516af 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -1,5 +1,5 @@
 
-
+
   
 A database with this schema holds the configuration for one Open
 vSwitch daemon.  The top-level configuration for the daemon is the
diff --git a/vtep/automake.mk b/vtep/automake.mk
index eac81d0..a204d0a 100644
--- a/vtep/automake.mk
+++ b/vtep/automake.mk
@@ -48,7 +48,6 @@ man_MANS += vtep/vtep.5
 vtep/vtep.5: \
ovsdb/ovsdb-doc vtep/vtep.xml vtep/vtep.ovsschema $(VTEP_PIC)
$(AM_V_GEN)$(OVSDB_DOC) \
-   --title="vtep" \
$(VTEP_DOT_DIAGRAM_ARG) \
--version=$(VERSION) \
$(srcdir)/vtep/vtep.ovsschema \
diff --git a/vtep/vtep.xml b/vtep/vtep.xml
index 7ed7f43..6199055 100644
--- a/vtep/vtep.xml
+++ b/vtep/vtep.xml
@@ -1,5 +1,5 @@
 
-
+
   
 This schema specifies relations that a VTEP can use to integrate
 physical ports into logical switches maintained by a network
-- 
2.1.3

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


[ovs-dev] [PATCH 7/8] xml2nroff: New program to generate a manpage from XML input.

2015-02-19 Thread Ben Pfaff
I really can't stand nroff syntax.  This makes it possible to install
nroff but write in a more sensible XML syntax.

The following commit adds the first user.

Signed-off-by: Ben Pfaff 
---
 Makefile.am |   1 +
 build-aux/xml2nroff | 110 
 2 files changed, 111 insertions(+)
 create mode 100755 build-aux/xml2nroff

diff --git a/Makefile.am b/Makefile.am
index 28496b3..0480d20 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -104,6 +104,7 @@ EXTRA_DIST = \
build-aux/dist-docs \
build-aux/sodepends.pl \
build-aux/soexpand.pl \
+   build-aux/xml2nroff \
$(MAN_FRAGMENTS) \
$(MAN_ROOTS) \
Vagrantfile
diff --git a/build-aux/xml2nroff b/build-aux/xml2nroff
new file mode 100755
index 000..817abc6
--- /dev/null
+++ b/build-aux/xml2nroff
@@ -0,0 +1,110 @@
+#! /usr/bin/python
+
+from datetime import date
+import getopt
+import os
+import sys
+import xml.dom.minidom
+
+from build.nroff import *
+
+argv0 = sys.argv[0]
+
+def usage():
+print """\
+%(argv0)s: XML to nroff converter
+Converts the XML format supplied as input into an nroff-formatted manpage.
+usage: %(argv0)s [OPTIONS] INPUT.XML
+where INPUT.XML is a manpage in an OVS-specific XML format.
+
+The following options are also available:
+  --version=VERSION   use VERSION to display on document footer
+  -h, --help  display this help message\
+""" % {'argv0': argv0}
+sys.exit(0)
+
+def manpage_to_nroff(xml_file, version=None):
+doc = xml.dom.minidom.parse(xml_file).documentElement
+d = date.fromtimestamp(os.stat(xml_file).st_mtime)
+
+if version == None:
+version = "UNKNOWN"
+program = doc.attributes['program'].nodeValue
+title = doc.attributes['title'].nodeValue
+section = doc.attributes['section'].nodeValue
+
+# Putting '\" p as the first line tells "man" that the manpage
+# needs to be preprocessed by "pic".
+s = r\" p
+.\" -*- nroff -*-
+.TH "%s" %s "%s" "Open vSwitch %s" "Open vSwitch Manual"
+.fp 5 L CR  \\" Make fixed-width font available as \\fL.
+.de TQ
+.  br
+.  ns
+.  TP "\\$1"
+..
+.de ST
+.  PP
+.  RS -0.15in
+.  I "\\$1"
+.  RE
+..
+''' % (textToNroff(program), textToNroff(section), textToNroff(title), 
textToNroff(version))
+
+s += blockXmlToNroff(doc.childNodes) + "\n"
+
+return s
+
+def usage():
+print """\
+%(argv0)s: ovsdb schema documentation generator
+Prints documentation for an OVSDB schema as an nroff-formatted manpage.
+usage: %(argv0)s [OPTIONS] SCHEMA XML
+where SCHEMA is an OVSDB schema in JSON format
+  and XML is OVSDB documentation in XML format.
+
+The following options are also available:
+  --version=VERSION   use VERSION to display on document footer
+  -h, --help  display this help message\
+""" % {'argv0': argv0}
+sys.exit(0)
+
+if __name__ == "__main__":
+try:
+try:
+options, args = getopt.gnu_getopt(sys.argv[1:], 'hV',
+  ['version=', 'help'])
+except getopt.GetoptError, geo:
+sys.stderr.write("%s: %s\n" % (argv0, geo.msg))
+sys.exit(1)
+
+er_diagram = None
+title = None
+version = None
+for key, value in options:
+if key == '--version':
+version = value
+elif key in ['-h', '--help']:
+usage()
+else:
+sys.exit(0)
+
+if len(args) != 1:
+sys.stderr.write("%s: exactly 1 non-option arguments required "
+ "(use --help for help)\n" % argv0)
+sys.exit(1)
+
+s = manpage_to_nroff(args[0], version)
+for line in s.split("\n"):
+line = line.strip()
+if len(line):
+print line
+
+except error.Error, e:
+sys.stderr.write("%s: %s\n" % (argv0, e.msg))
+sys.exit(1)
+
+# Local variables:
+# mode: python
+# End:
-- 
2.1.3

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


[ovs-dev] [PATCH 8/8] [RFC] ovn: Start work on design documentation.

2015-02-19 Thread Ben Pfaff
This commit adds preliminary design documentation for Open Virtual Network,
or OVN, a new OVS-based project to add support for virtual networking to
OVS, initially with OpenStack integration.

This initial design has been influenced by many people, including (in
alphabetical order) Aaron Rosen, Chris Wright, Jeremy Stribling,
Justin Pettit, Ken Duda, Madhu Venugopal, Martin Casado, Pankaj Thakkar,
Russell Bryant, and Teemu Koponen.  All blunders, however, are due to my
own hubris.

Signed-off-by: Ben Pfaff 
---
 AUTHORS|   1 +
 Makefile.am|   1 +
 configure.ac   |   3 +-
 ovn/automake.mk|  75 +++
 ovn/ovn-architecture.7.xml | 331 +++
 ovn/ovn-controller.8.in|  41 
 ovn/ovn-nb.ovsschema   |  63 ++
 ovn/ovn-nb.xml | 225 +
 ovn/ovn.ovsschema  |  47 +
 ovn/ovn.xml| 475 +
 10 files changed, 1261 insertions(+), 1 deletion(-)
 create mode 100644 ovn/automake.mk
 create mode 100644 ovn/ovn-architecture.7.xml
 create mode 100644 ovn/ovn-controller.8.in
 create mode 100644 ovn/ovn-nb.ovsschema
 create mode 100644 ovn/ovn-nb.xml
 create mode 100644 ovn/ovn.ovsschema
 create mode 100644 ovn/ovn.xml

diff --git a/AUTHORS b/AUTHORS
index 5fa9598..88655b5 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -312,6 +312,7 @@ Rogério Vinhal Nunes
 Roman Sokolkov  rsokol...@gmail.com
 Ronaldo A. Ferreira ronal...@cs.princeton.edu
 Ronny L. Bull   bul...@clarkson.edu
+Russell Bryant  rbry...@redhat.com
 Sander Eikelenboom  li...@eikelenboom.it
 Saul St. John   sstj...@cs.wisc.edu
 Scott Hendricks shendri...@nicira.com
diff --git a/Makefile.am b/Makefile.am
index 0480d20..699a580 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -370,3 +370,4 @@ include tutorial/automake.mk
 include vtep/automake.mk
 include datapath-windows/automake.mk
 include datapath-windows/include/automake.mk
+include ovn/automake.mk
diff --git a/configure.ac b/configure.ac
index d2d02ca..795f876 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1,4 +1,4 @@
-# Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
+# Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -182,6 +182,7 @@ dnl This makes sure that include/openflow gets created in 
the build directory.
 AC_CONFIG_COMMANDS([include/openflow/openflow.h.stamp])
 
 AC_CONFIG_COMMANDS([utilities/bugtool/dummy], [:])
+AC_CONFIG_COMMANDS([ovn/dummy], [:])
 
 m4_ifdef([AM_SILENT_RULES], [AM_SILENT_RULES])
 
diff --git a/ovn/automake.mk b/ovn/automake.mk
new file mode 100644
index 000..3889d56
--- /dev/null
+++ b/ovn/automake.mk
@@ -0,0 +1,75 @@
+# OVN schema and IDL
+EXTRA_DIST += ovn/ovn.ovsschema
+pkgdata_DATA += ovn/ovn.ovsschema
+
+# OVN E-R diagram
+#
+# If "python" or "dot" is not available, then we do not add graphical diagram
+# to the documentation.
+if HAVE_PYTHON
+if HAVE_DOT
+ovn/ovn.gv: ovsdb/ovsdb-dot.in ovn/ovn.ovsschema
+   $(AM_V_GEN)$(OVSDB_DOT) --no-arrows $(srcdir)/ovn/ovn.ovsschema > $@
+ovn/ovn.pic: ovn/ovn.gv ovsdb/dot2pic
+   $(AM_V_GEN)(dot -T plain < ovn/ovn.gv | $(PERL) $(srcdir)/ovsdb/dot2pic 
-f 3) > $@.tmp && \
+   mv $@.tmp $@
+OVN_PIC = ovn/ovn.pic
+OVN_DOT_DIAGRAM_ARG = --er-diagram=$(OVN_PIC)
+DISTCLEANFILES += ovn/ovn.gv ovn/ovn.pic
+endif
+endif
+
+# OVN schema documentation
+EXTRA_DIST += ovn/ovn.xml
+DISTCLEANFILES += ovn/ovn.5
+man_MANS += ovn/ovn.5
+ovn/ovn.5: \
+   ovsdb/ovsdb-doc ovn/ovn.xml ovn/ovn.ovsschema $(OVN_PIC)
+   $(AM_V_GEN)$(OVSDB_DOC) \
+   $(OVN_DOT_DIAGRAM_ARG) \
+   --version=$(VERSION) \
+   $(srcdir)/ovn/ovn.ovsschema \
+   $(srcdir)/ovn/ovn.xml > $@.tmp && \
+   mv $@.tmp $@
+
+# OVN northbound schema and IDL
+EXTRA_DIST += ovn/ovn-nb.ovsschema
+pkgdata_DATA += ovn/ovn-nb.ovsschema
+
+# OVN northbound E-R diagram
+#
+# If "python" or "dot" is not available, then we do not add graphical diagram
+# to the documentation.
+if HAVE_PYTHON
+if HAVE_DOT
+ovn/ovn-nb.gv: ovsdb/ovsdb-dot.in ovn/ovn-nb.ovsschema
+   $(AM_V_GEN)$(OVSDB_DOT) --no-arrows $(srcdir)/ovn/ovn-nb.ovsschema > $@
+ovn/ovn-nb.pic: ovn/ovn-nb.gv ovsdb/dot2pic
+   $(AM_V_GEN)(dot -T plain < ovn/ovn-nb.gv | $(PERL) 
$(srcdir)/ovsdb/dot2pic -f 3) > $@.tmp && \
+   mv $@.tmp $@
+OVN_NB_PIC = ovn/ovn-nb.pic
+OVN_NB_DOT_DIAGRAM_ARG = --er-diagram=$(OVN_NB_PIC)
+DISTCLEANFILES += ovn/ovn-nb.gv ovn/ovn-nb.pic
+endif
+endif
+
+# OVN northbound schema documentation
+EXTRA_DIST += ovn/ovn-nb.xml
+DISTCLEANFILES += ovn/ovn-nb.5
+man_MANS += ovn/ovn-nb.5
+ovn/ovn-nb.5: \
+   ovsdb/ovsdb-doc ovn/ovn-nb.xml ovn/ovn-nb.ovsschema $(OVN_NB_PIC)
+   $(AM_V_GEN)$(OVSDB_DOC) \
+   $(OVN

Re: [ovs-dev] [PATCH 8/8] [RFC] ovn: Start work on design documentation.

2015-02-19 Thread Ben Pfaff
On Thu, Feb 19, 2015 at 12:12:26AM -0800, Ben Pfaff wrote:
> This commit adds preliminary design documentation for Open Virtual Network,
> or OVN, a new OVS-based project to add support for virtual networking to
> OVS, initially with OpenStack integration.
> 
> This initial design has been influenced by many people, including (in
> alphabetical order) Aaron Rosen, Chris Wright, Jeremy Stribling,
> Justin Pettit, Ken Duda, Madhu Venugopal, Martin Casado, Pankaj Thakkar,
> Russell Bryant, and Teemu Koponen.  All blunders, however, are due to my
> own hubris.
> 
> Signed-off-by: Ben Pfaff 

I've posted the rendered version of the documentation following this
commit at http://benpfaff.org/~blp/dist-docs.  You probably want to look
at the ovn* manpages, especially ovn-architecture(7), ovn(5), and
ovn-nb(5).
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [neutron] [ovs-dev] [PATCH 8/8] [RFC] ovn: Start work on design Documentation.

2015-02-19 Thread Miguel Ángel Ajo
Thank you Ben!,  

Cross posting [1] to openstack list /neutron.


[1] http://benpfaff.org/~blp/dist-docs. 

On Thursday, 19 de February de 2015 at 09:13, Ben Pfaff wrote:

> On Thu, Feb 19, 2015 at 12:12:26AM -0800, Ben Pfaff wrote:
> > This commit adds preliminary design documentation for Open Virtual Network,
> > or OVN, a new OVS-based project to add support for virtual networking to
> > OVS, initially with OpenStack integration.
> > 
> > This initial design has been influenced by many people, including (in
> > alphabetical order) Aaron Rosen, Chris Wright, Jeremy Stribling,
> > Justin Pettit, Ken Duda, Madhu Venugopal, Martin Casado, Pankaj Thakkar,
> > Russell Bryant, and Teemu Koponen. All blunders, however, are due to my
> > own hubris.
> > 
> > Signed-off-by: Ben Pfaff mailto:b...@nicira.com)>
> 
> I've posted the rendered version of the documentation following this
> commit at http://benpfaff.org/~blp/dist-docs. You probably want to look
> at the ovn* manpages, especially ovn-architecture(7), ovn(5), and
> ovn-nb(5).
> ___
> dev mailing list
> dev@openvswitch.org (mailto:dev@openvswitch.org)
> http://openvswitch.org/mailman/listinfo/dev
> 
> 


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


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

2015-02-19 Thread m-mat
l]Cē
”¼¸'®/`mk¼›ìы©¨[ÜߌUh£7`×ño¦ÑìÒÍø9 ìƞeÇ üNH¥oÍH¡ãÒ
ö,0”¹VDšÊ¥jEöp±(W¡^¦|9izæÊþ/Œúý¨WH°oˆ>}¯Ã b–l[¥¤åënöt¯Pª ­jµG
ÁžCRÌjb±® ’'ÛG¥µˆâæ/ƅñ–We¯P³…±Á‘_–ÍÉ
†‘écfi 
’tz´>^NÅ`ž¸%d«i~˜~àÞsèq³LÛ"'‰iŒ¦ñw¾¯û>¸VRÌóÀX͘¢¤CârˆÌKÁnß[î¯&aUø—#_¾ƒÌ† 
a¨GcŽRïU4ڈΙÇ÷mÓ3$–÷46.í¡,Êàq


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


Re: [ovs-dev] [PATCH 8/8] [RFC] ovn: Start work on design ocumentation.

2015-02-19 Thread Miguel Ángel Ajo
Hi Ben,  

   I specially liked the VIF port lifecycle, looks good to me, Ionly miss some  
“port_security” concepts we have in neutron, which I guess could have been  
deliberately omitted for a start.

   In neutron we have something called security groups, and every port
belongs to 1 or more security groups.  Each security group has a list of
rules to control traffic at port level in a very fine grained fashion 
(ingress/egress
protocol/flags/etc…   remote_ip/mask or security_group ID)

I guess we could build  render security_group ID to multiple IPs for each port,
but then we will miss the ingress/egress and protocol flags (like type  of 
protocol,
ports, etc.. [1])

Also, be aware, that not having security group ID references from neutron,
when lot’s of ports go to the same security group we end up with an exponential
growth of rules / OF entries per port, we solved this in the server<->agent
communication for the reference OVS solution by keeping a lists of IPs  
belonging to security group IDs, and then, separately having the  
references from the rules.


[1] 
http://docs.openstack.org/admin-guide-cloud/content/securitygroup_api_abstractions.html
  

Miguel Ángel Ajo


On Thursday, 19 de February de 2015 at 09:13, Ben Pfaff wrote:

> On Thu, Feb 19, 2015 at 12:12:26AM -0800, Ben Pfaff wrote:
> > This commit adds preliminary design documentation for Open Virtual Network,
> > or OVN, a new OVS-based project to add support for virtual networking to
> > OVS, initially with OpenStack integration.
> >  
> > This initial design has been influenced by many people, including (in
> > alphabetical order) Aaron Rosen, Chris Wright, Jeremy Stribling,
> > Justin Pettit, Ken Duda, Madhu Venugopal, Martin Casado, Pankaj Thakkar,
> > Russell Bryant, and Teemu Koponen. All blunders, however, are due to my
> > own hubris.
> >  
> > Signed-off-by: Ben Pfaff mailto:b...@nicira.com)>
>  
> I've posted the rendered version of the documentation following this
> commit at http://benpfaff.org/~blp/dist-docs. You probably want to look
> at the ovn* manpages, especially ovn-architecture(7), ovn(5), and
> ovn-nb(5).
> ___
> dev mailing list
> dev@openvswitch.org (mailto: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/8] bridge: Publish error count in database's rstp_statistics.

2015-02-19 Thread Gurucharan Shetty
On Thu, Feb 19, 2015 at 12:12 AM, Ben Pfaff  wrote:
> The lower layers count errors but until now nothing actually reported them.
>
> Found by inspection.
>
> Signed-off-by: Ben Pfaff 
Acked-by: Gurucharan Shetty 
> ---
>  vswitchd/bridge.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index 297c0dd..a143be1 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -1,4 +1,4 @@
> -/* Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
> +/* Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
>   *
>   * Licensed under the Apache License, Version 2.0 (the "License");
>   * you may not use this file except in compliance with the License.
> @@ -2489,8 +2489,8 @@ port_refresh_rstp_status(struct port *port)
>  struct ofproto *ofproto = port->bridge->ofproto;
>  struct iface *iface;
>  struct ofproto_port_rstp_status status;
> -char *keys[3];
> -int64_t int_values[3];
> +char *keys[4];
> +int64_t int_values[4];
>  struct smap smap;
>
>  if (port_is_synthetic(port)) {
> @@ -2539,6 +2539,8 @@ port_refresh_rstp_status(struct port *port)
>  int_values[1] = status.rx_count;
>  keys[2] = "rstp_uptime";
>  int_values[2] = status.uptime;
> +keys[3] = "rstp_error_count";
> +int_values[3] = status.error_count;
>  ovsrec_port_set_rstp_statistics(port->cfg, keys, int_values,
>  ARRAY_SIZE(int_values));
>  }
> --
> 2.1.3
>
> ___
> 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 3/8] vtep: Document the ipaddr column in the Mcast_Macs_Local table.

2015-02-19 Thread Gurucharan Shetty
On Thu, Feb 19, 2015 at 12:12 AM, Ben Pfaff  wrote:
> This had been overlooked.
>
> Signed-off-by: Ben Pfaff 
I did not about this column.
Acked-by: Gurucharan Shetty 
> ---
>  vtep/vtep.xml | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/vtep/vtep.xml b/vtep/vtep.xml
> index 80fc99d..7ed7f43 100644
> --- a/vtep/vtep.xml
> +++ b/vtep/vtep.xml
> @@ -776,6 +776,10 @@
>addresses of the appropriate VTEP(s).
>  
>
> +
> +  The IP address to which this MAC corresponds. Optional field for
> +  the purpose of ARP supression.
> +
>
>
>
> --
> 2.1.3
>
> ___
> 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 4/8] ovsdb-doc: Flag an error when a table or a column is left undocumented.

2015-02-19 Thread Gurucharan Shetty
On Thu, Feb 19, 2015 at 12:12 AM, Ben Pfaff  wrote:
> This should make it harder to forget documentation.
>
> Signed-off-by: Ben Pfaff 
Nice!
Acked-by: Gurucharan Shetty 
> ---
>  ovsdb/ovsdb-doc | 23 ---
>  1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/ovsdb/ovsdb-doc b/ovsdb/ovsdb-doc
> index 6200915..b670452 100755
> --- a/ovsdb/ovsdb-doc
> +++ b/ovsdb/ovsdb-doc
> @@ -152,7 +152,7 @@ def typeAndConstraintsToNroff(column):
>  type += " (must be unique within table)"
>  return type
>
> -def columnGroupToNroff(table, groupXml):
> +def columnGroupToNroff(table, groupXml, documented_columns):
>  introNodes = []
>  columnNodes = []
>  for node in groupXml.childNodes:
> @@ -172,6 +172,7 @@ def columnGroupToNroff(table, groupXml):
>  for node in columnNodes:
>  if node.tagName == 'column':
>  name = node.attributes['name'].nodeValue
> +documented_columns.add(name)
>  column = table.columns[name]
>  if node.hasAttribute('key'):
>  key = node.attributes['key'].nodeValue
> @@ -219,7 +220,8 @@ def columnGroupToNroff(table, groupXml):
>  summary += [('column', nameNroff, typeNroff)]
>  elif node.tagName == 'group':
>  title = node.attributes["title"].nodeValue
> -subSummary, subIntro, subBody = columnGroupToNroff(table, node)
> +subSummary, subIntro, subBody = columnGroupToNroff(
> +table, node, documented_columns)
>  summary += [('group', title, subSummary)]
>  body += '.ST "%s:"\n' % textToNroff(title)
>  body += subIntro + subBody
> @@ -242,15 +244,24 @@ def tableToNroff(schema, tableXml):
>  tableName = tableXml.attributes['name'].nodeValue
>  table = schema.tables[tableName]
>
> +documented_columns = set()
>  s = """.bp
>  .SH "%s TABLE"
>  """ % tableName
> -summary, intro, body = columnGroupToNroff(table, tableXml)
> +summary, intro, body = columnGroupToNroff(table, tableXml,
> +  documented_columns)
>  s += intro
>  s += '.SS "Summary:\n'
>  s += tableSummaryToNroff(summary)
>  s += '.SS "Details:\n'
>  s += body
> +
> +schema_columns = set(table.columns.keys())
> +undocumented_columns = schema_columns - documented_columns
> +for column in undocumented_columns:
> +raise error.Error("table %s has undocumented column %s"
> +  % (tableName, column))
> +
>  return s
>
>  def docsToNroff(schemaFile, xmlFile, erFile, title=None, version=None):
> @@ -306,6 +317,12 @@ def docsToNroff(schemaFile, xmlFile, erFile, title=None, 
> version=None):
>  else:
>  introNodes += [dbNode]
>
> +documented_tables = set((name for (name, title) in summary))
> +schema_tables = set(schema.tables.keys())
> +undocumented_tables = schema_tables - documented_tables
> +for table in undocumented_tables:
> +raise error.Error("undocumented table %s" % table)
> +
>  s += blockXmlToNroff(introNodes) + "\n"
>
>  s += r"""
> --
> 2.1.3
>
> ___
> 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 5/8] ovsdb-doc: Get manpage name from the XML file instead of command line.

2015-02-19 Thread Gurucharan Shetty
On Thu, Feb 19, 2015 at 12:12 AM, Ben Pfaff  wrote:
> This seems like a better place for it.
>
> Signed-off-by: Ben Pfaff 
Acked-by: Gurucharan Shetty 
> ---
>  ovsdb/ovsdb-doc  | 18 --
>  vswitchd/automake.mk |  1 -
>  vswitchd/vswitch.xml |  2 +-
>  vtep/automake.mk |  1 -
>  vtep/vtep.xml|  2 +-
>  5 files changed, 10 insertions(+), 14 deletions(-)
>
> diff --git a/ovsdb/ovsdb-doc b/ovsdb/ovsdb-doc
> index b670452..15618ef 100755
> --- a/ovsdb/ovsdb-doc
> +++ b/ovsdb/ovsdb-doc
> @@ -264,7 +264,7 @@ def tableToNroff(schema, tableXml):
>
>  return s
>
> -def docsToNroff(schemaFile, xmlFile, erFile, title=None, version=None):
> +def docsToNroff(schemaFile, xmlFile, erFile, version=None):
>  schema = ovs.db.schema.DbSchema.from_json(ovs.json.from_file(schemaFile))
>  doc = xml.dom.minidom.parse(xmlFile).documentElement
>
> @@ -272,8 +272,10 @@ def docsToNroff(schemaFile, xmlFile, erFile, title=None, 
> version=None):
>  xmlDate = os.stat(xmlFile).st_mtime
>  d = date.fromtimestamp(max(schemaDate, xmlDate))
>
> -if title == None:
> -title = schema.name
> +if doc.hasAttribute('name'):
> +manpage = doc.attributes['name'].nodeValue
> +else:
> +manpage = schema.name
>
>  if version == None:
>  version = "UNKNOWN"
> @@ -297,7 +299,7 @@ def docsToNroff(schemaFile, xmlFile, erFile, title=None, 
> version=None):
>  .SH NAME
>  %s \- %s database schema
>  .PP
> -''' % (title, schema.version, version, textToNroff(schema.name), schema.name)
> +''' % (manpage, schema.version, version, textToNroff(manpage), schema.name)
>
>  tables = ""
>  introNodes = []
> @@ -378,7 +380,6 @@ where SCHEMA is an OVSDB schema in JSON format
>
>  The following options are also available:
>--er-diagram=DIAGRAM.PICinclude E-R diagram from DIAGRAM.PIC
> -  --title=TITLE   use TITLE as title instead of schema name
>--version=VERSION   use VERSION to display on document footer
>-h, --help  display this help message\
>  """ % {'argv0': argv0}
> @@ -388,20 +389,17 @@ if __name__ == "__main__":
>  try:
>  try:
>  options, args = getopt.gnu_getopt(sys.argv[1:], 'hV',
> -  ['er-diagram=', 'title=',
> +  ['er-diagram=',
> 'version=', 'help'])
>  except getopt.GetoptError, geo:
>  sys.stderr.write("%s: %s\n" % (argv0, geo.msg))
>  sys.exit(1)
>
>  er_diagram = None
> -title = None
>  version = None
>  for key, value in options:
>  if key == '--er-diagram':
>  er_diagram = value
> -elif key == '--title':
> -title = value
>  elif key == '--version':
>  version = value
>  elif key in ['-h', '--help']:
> @@ -415,7 +413,7 @@ if __name__ == "__main__":
>  sys.exit(1)
>
>  # XXX we should warn about undocumented tables or columns
> -s = docsToNroff(args[0], args[1], er_diagram, title, version)
> +s = docsToNroff(args[0], args[1], er_diagram, version)
>  for line in s.split("\n"):
>  line = line.strip()
>  if len(line):
> diff --git a/vswitchd/automake.mk b/vswitchd/automake.mk
> index 80affe9..2f07c0f 100644
> --- a/vswitchd/automake.mk
> +++ b/vswitchd/automake.mk
> @@ -48,7 +48,6 @@ vswitchd/ovs-vswitchd.conf.db.5: \
> ovsdb/ovsdb-doc vswitchd/vswitch.xml vswitchd/vswitch.ovsschema \
> $(VSWITCH_PIC)
> $(AM_V_GEN)$(OVSDB_DOC) \
> -   --title="ovs-vswitchd.conf.db" \
> $(VSWITCH_DOT_DIAGRAM_ARG) \
> --version=$(VERSION) \
> $(srcdir)/vswitchd/vswitch.ovsschema \
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index 04de3ca..c6516af 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -1,5 +1,5 @@
>  
> -
> +
>
>  A database with this schema holds the configuration for one Open
>  vSwitch daemon.  The top-level configuration for the daemon is the
> diff --git a/vtep/automake.mk b/vtep/automake.mk
> index eac81d0..a204d0a 100644
> --- a/vtep/automake.mk
> +++ b/vtep/automake.mk
> @@ -48,7 +48,6 @@ man_MANS += vtep/vtep.5
>  vtep/vtep.5: \
> ovsdb/ovsdb-doc vtep/vtep.xml vtep/vtep.ovsschema $(VTEP_PIC)
> $(AM_V_GEN)$(OVSDB_DOC) \
> -   --title="vtep" \
> $(VTEP_DOT_DIAGRAM_ARG) \
> --version=$(VERSION) \
> $(srcdir)/vtep/vtep.ovsschema \
> diff --git a/vtep/vtep.xml b/vtep/vtep.xml
> index 7ed7f43..6199055 100644
> --- a/vtep/vtep.xml
> +++ b/vtep/vtep.xml
> @@ -1,5 +1,5 @@
>  
> -
> +
>
>  This schema specifies relations that a VTEP can use to integrate
>  physical ports into logical switches maintained by a net

Re: [ovs-dev] [PATCH 2/8] vswitch: Document columns that had been previously overlooked.

2015-02-19 Thread Gurucharan Shetty
On Thu, Feb 19, 2015 at 12:12 AM, Ben Pfaff  wrote:
> A fair number of columns had been overlooked.  This documents them.
>
> The patch is smaller than it appears because this rearranges the STP and
> RSTP documentation to group configuration, status, and statistics together
> in the documentation for clarity.
>
> Signed-off-by: Ben Pfaff 
I do not know RSTP to review whether more clarity can be provided on
the newly added column documentation. If no one else reviews,
Acked-by: Gurucharan Shetty 
> ---
>  vswitchd/vswitch.xml | 680 
> +--
>  1 file changed, 391 insertions(+), 289 deletions(-)
>
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index 932e4b2..04de3ca 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -681,80 +681,232 @@
>  
>
>  
> -  The IEEE 802.1D Spanning Tree Protocol (STP) is a network protocol
> -  that ensures loop-free topologies.  It allows redundant links to
> -  be included in the network to provide automatic backup paths if
> -  the active links fails.
> +  
> +The IEEE 802.1D Spanning Tree Protocol (STP) is a network protocol
> +that ensures loop-free topologies.  It allows redundant links to
> +be included in the network to provide automatic backup paths if
> +the active links fails.
> +  
>
> -  
> -Enable spanning tree on the bridge.  By default, STP is disabled
> -on bridges.  Bond, internal, and mirror ports are not supported
> -and will not participate in the spanning tree.
> -  
> +  
> +These settings configure the slower-to-converge but still widely
> +supported version of Spanning Tree Protocol, sometimes known as
> +802.1D-1998.  Open vSwitch also supports the newer Rapid Spanning 
> Tree
> +Protocol (RSTP), documented later in the section titled Rapid
> +Spanning Tree Configuration.
> +  
>
> -  
> -The bridge's STP identifier (the lower 48 bits of the bridge-id)
> -in the form
> -
> xx:xx:xx:xx:xx:xx.
> -By default, the identifier is the MAC address of the bridge.
> -  
> +  
> +
> +  
> +Enable spanning tree on the bridge.  By default, STP is disabled
> +on bridges.  Bond, internal, and mirror ports are not supported
> +and will not participate in the spanning tree.
> +  
>
> -   -  type='{"type": "integer", "minInteger": 0, "maxInteger": 
> 65535}'>
> -The bridge's relative priority value for determining the root
> -bridge (the upper 16 bits of the bridge-id).  A bridge with the
> -lowest bridge-id is elected the root.  By default, the priority
> -is 0x8000.
> -  
> +  
> +STP and RSTP are mutually exclusive.  If both are enabled, RSTP
> +will be used.
> +  
> +
>
> -   -  type='{"type": "integer", "minInteger": 1, "maxInteger": 10}'>
> -The interval between transmissions of hello messages by
> -designated ports, in seconds.  By default the hello interval is
> -2 seconds.
> -  
> +
> +  The bridge's STP identifier (the lower 48 bits of the bridge-id)
> +  in the form
> +  
> xx:xx:xx:xx:xx:xx.
> +  By default, the identifier is the MAC address of the bridge.
> +
>
> -   -  type='{"type": "integer", "minInteger": 6, "maxInteger": 40}'>
> -The maximum age of the information transmitted by the bridge
> -when it is the root bridge, in seconds.  By default, the maximum
> -age is 20 seconds.
> -  
> + +type='{"type": "integer", "minInteger": 0, "maxInteger": 
> 65535}'>
> +  The bridge's relative priority value for determining the root
> +  bridge (the upper 16 bits of the bridge-id).  A bridge with the
> +  lowest bridge-id is elected the root.  By default, the priority
> +  is 0x8000.
> +
>
> -   -  type='{"type": "integer", "minInteger": 4, "maxInteger": 30}'>
> -The delay to wait between transitioning root and designated
> -ports to forwarding, in seconds.  By default, the
> -forwarding delay is 15 seconds.
> -  
> + +type='{"type": "integer", "minInteger": 1, "maxInteger": 
> 10}'>
> +  The interval between transmissions of hello messages by
> +  designated ports, in seconds.  By default the hello interval is
> +  2 seconds.
> +
>
> -   -  type='{"type": "integer", "minInteger": 1}'>
> -
> -  The maximum number of seconds to retain a multicast snooping entry 
> for
> -  which no packets have been seen.  The default is currently 300
> -  seconds (5 minutes).  The value, if specified, is forced into a
> - 

Re: [ovs-dev] [PATCH 6/8] ovsdb-doc: Factor out nroff formatting into a separate Python module.

2015-02-19 Thread Gurucharan Shetty
I don't understand nroff well enough to review patch 6 and patch 7. If
anyone else is up for it, please review.

On Thu, Feb 19, 2015 at 12:12 AM, Ben Pfaff  wrote:
> This will make it cleaner to add another build-time program that generates
> nroff from XML.
>
> Signed-off-by: Ben Pfaff 
> ---
>  ovsdb/ovsdb-doc  | 135 +---
>  python/automake.mk   |   6 ++
>  python/build/__init__.py |   0
>  python/build/nroff.py| 196 
> +++
>  4 files changed, 206 insertions(+), 131 deletions(-)
>  create mode 100644 python/build/__init__.py
>  create mode 100644 python/build/nroff.py
>
> diff --git a/ovsdb/ovsdb-doc b/ovsdb/ovsdb-doc
> index 15618ef..369efa9 100755
> --- a/ovsdb/ovsdb-doc
> +++ b/ovsdb/ovsdb-doc
> @@ -3,7 +3,6 @@
>  from datetime import date
>  import getopt
>  import os
> -import re
>  import sys
>  import xml.dom.minidom
>
> @@ -11,136 +10,9 @@ import ovs.json
>  from ovs.db import error
>  import ovs.db.schema
>
> -argv0 = sys.argv[0]
> -
> -def textToNroff(s, font=r'\fR'):
> -def escape(match):
> -c = match.group(0)
> -if c.startswith('-'):
> -if c != '-' or font == r'\fB':
> -return '\\' + c
> -else:
> -return '-'
> -if c == '\\':
> -return r'\e'
> -elif c == '"':
> -return r'\(dq'
> -elif c == "'":
> -return r'\(cq'
> -else:
> -raise error.Error("bad escape")
> +from build.nroff import *
>
> -# Escape - \ " ' as needed by nroff.
> -s = re.sub('(-[0-9]|[-"\'])', escape, s)
> -if s.startswith('.'):
> -s = '\\' + s
> -return s
> -
> -def escapeNroffLiteral(s):
> -return r'\fB%s\fR' % textToNroff(s, r'\fB')
> -
> -def inlineXmlToNroff(node, font):
> -if node.nodeType == node.TEXT_NODE:
> -return textToNroff(node.data, font)
> -elif node.nodeType == node.ELEMENT_NODE:
> -if node.tagName in ['code', 'em', 'option']:
> -s = r'\fB'
> -for child in node.childNodes:
> -s += inlineXmlToNroff(child, r'\fB')
> -return s + font
> -elif node.tagName == 'ref':
> -s = r'\fB'
> -if node.hasAttribute('column'):
> -s += node.attributes['column'].nodeValue
> -if node.hasAttribute('key'):
> -s += ':' + node.attributes['key'].nodeValue
> -elif node.hasAttribute('table'):
> -s += node.attributes['table'].nodeValue
> -elif node.hasAttribute('group'):
> -s += node.attributes['group'].nodeValue
> -else:
> -raise error.Error("'ref' lacks required attributes: %s" % 
> node.attributes.keys())
> -return s + font
> -elif node.tagName == 'var':
> -s = r'\fI'
> -for child in node.childNodes:
> -s += inlineXmlToNroff(child, r'\fI')
> -return s + font
> -else:
> -raise error.Error("element <%s> unknown or invalid here" % 
> node.tagName)
> -else:
> -raise error.Error("unknown node %s in inline xml" % node)
> -
> -def blockXmlToNroff(nodes, para='.PP'):
> -s = ''
> -for node in nodes:
> -if node.nodeType == node.TEXT_NODE:
> -s += textToNroff(node.data)
> -s = s.lstrip()
> -elif node.nodeType == node.ELEMENT_NODE:
> -if node.tagName in ['ul', 'ol']:
> -if s != "":
> -s += "\n"
> -s += ".RS\n"
> -i = 0
> -for liNode in node.childNodes:
> -if (liNode.nodeType == node.ELEMENT_NODE
> -and liNode.tagName == 'li'):
> -i += 1
> -if node.tagName == 'ul':
> -s += ".IP \\(bu\n"
> -else:
> -s += ".IP %d. .25in\n" % i
> -s += blockXmlToNroff(liNode.childNodes, ".IP")
> -elif (liNode.nodeType != node.TEXT_NODE
> -  or not liNode.data.isspace()):
> -raise error.Error("<%s> element may only have  
> children" % node.tagName)
> -s += ".RE\n"
> -elif node.tagName == 'dl':
> -if s != "":
> -s += "\n"
> -s += ".RS\n"
> -prev = "dd"
> -for liNode in node.childNodes:
> -if (liNode.nodeType == node.ELEMENT_NODE
> -and liNode.tagName == 'dt'):
> -if prev == 'dd':
> -s += '.TP\n'
> -else:
> -s += '.TQ\n'
> -prev = 'dt'
> -elif (liNode.nodeType ==

[ovs-dev] Mail System Error - Returned Mail

2015-02-19 Thread riesenf
Vc¹õ–¿ùªF¤j×]ÏcFòÛØ|uŽkâ¨cBƒê&>tJ¯›VJôù*cªù‘Öø
õÅ¢Ç.yHGnàȟðtGé NQ¯ÜÑ[¼¡wžš{FÛ²<6Ça²3±Ä_×ÓêSÛ#èz/;ÅWU¾Ë¤l7¾´•.2ŽfÙû±O^á’æV`
ëŒPв­xa›åŒ©,Øá§(í¦¦fYs VoÙïu‰0õ7ÙÚ`‘‡7,&Ѳåøx25²Ÿ½dØrii̺ƸTxàQ¦–¹ƒSwäc{c½ç
)”`ëq—0Xl›'
“J‹˜{áÙx±çøàDܤ,â“f†œŒê#ÛQÅ8üëE˜ä¼ù„Tžf¶IÆÎa]1ü£­áý9‰•ÒáÊs¢´°ì}'®ÅkF‡»‚ÞKÈÀ5±KŠŽÚ
Z®¨ÂVݯÄÀ$&T{º#N“’fyú¶Ö4V
sZµ¡Î¹
Ï
o"ŠÖ¹7É¢Òý›RÌׯ©HJY* _ª5æèn‹ø±Ñ",ÒºY«Ó^à88ód痸æ95:;¥0®:’*
µ`
»ŽAY/3Þ·¦X9hâ>ܑŽ{e®2·õ­¬¬m…³´ö“~ÍxJ$®×Xëèë{áÏïÎÍ¿[}¸æÛÒdþ/I)#b
Bd–Äv¾%:~#­Ÿ¡ÁµRÑI¸ÏI³Ú³Ø{¤šq“m,:Z|^ÉڃEhø²’þ&–ØrîPà‚d3uZá—÷j¿¶¤º„nI*¢ÕµuW‡îˆZø,Âá~XbƒuSÓSØ£x«¢›ÉÍHçKÑ-
…
B78ÞÖGK©A·o•v¥Y¦vze,#1pîu{ª§÷<àe8W§›¶Ï§7¼Ä¹¥gÝzeß¡Jý„ÛϪœrsqQäÍDNÆÈ/Fæ“N:R±BÒ݊¯Õê¾²l›ûj_øåyI`±°nªj9,œâñ?aëno2!¸?0Ò¢e¶²½VÛ·çâL8¬¶OBiÀzßÃSÊùq È.ÈȨôàÀa¤­ 
;s·ØàƒíòdEwgy6¶2÷!(ð6•t˜<ÒÙu¤LnD,›¹í͟øg‹ Œßš{‹ØÐGåÒÍ!Ÿ_ͨ®dðº!Ã/÷{å/(ҖJR…Ã«g¾ý," 
!}i­žš’ÄB¬Êé(µíf~…ØñðaTø
`õpçƒvÇá´àÂ|Fk‰GØa¼4#¥»‰Ÿh5'ÐÇ£A<~\~&“pG›§nOŽïPû‰%›]bËÒ%Ty*QhsM…
6wMQf/'‰l¸é¶;ßÐP¯Db»¹çU†ù5OÙ$` -ÀÀ·–ˆ[S\ï;:æ­É4Cm˜s®þ`†‰X
<çUHt_L$ö©‡ÇÁw‹çŸÎƒüº5;?":¦0KíX“c…Meø&6NŽô{S>(¿ìÁÙàìídãÔ9M
U–òPmÁƒî†à݌Mñä
¨,½¥½rSëèíéƒñ¬«Ûfe)ö3à¶2î/à\ŠÌš¾ƒ]Ѫ¹¶õS!{©ÎIôEH8½ÆäbVúº]0û;ÁŸì­dθ2?‰¦
ºô ‘ÊÔäv AŠ5µÇú_÷˜`øÉ0‹)í×¶ØTºå$…
«þN1LôÊ\D—œô&h°W×ez‰ncsÏgºº4"ǐhkÚ[ϊ¢†tŽ–£AáˆTыßot‡Öá/3«`P¸0™pþ1Œcéê)
­‰é`ÌR?Ç&‹L
e¸£tLֽįZ¹ÈUç2âí”PÎqþď±PíÛ☔ÖL>„Z˜¸ýn¹p8ñ3ŸÖ$Ɂ

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


[ovs-dev] RETURNED MAIL: DATA FORMAT ERROR

2015-02-19 Thread frank
Your message could not be delivered

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


Re: [ovs-dev] [PATCH 1/8] bridge: Publish error count in database's rstp_statistics.

2015-02-19 Thread Russell Bryant
On 02/19/2015 03:12 AM, Ben Pfaff wrote:
> The lower layers count errors but until now nothing actually reported them.
> 
> Found by inspection.
> 
> Signed-off-by: Ben Pfaff 
> ---
>  vswitchd/bridge.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index 297c0dd..a143be1 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -1,4 +1,4 @@
> -/* Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
> +/* Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
>   *
>   * Licensed under the Apache License, Version 2.0 (the "License");
>   * you may not use this file except in compliance with the License.
> @@ -2489,8 +2489,8 @@ port_refresh_rstp_status(struct port *port)
>  struct ofproto *ofproto = port->bridge->ofproto;
>  struct iface *iface;
>  struct ofproto_port_rstp_status status;
> -char *keys[3];
> -int64_t int_values[3];
> +char *keys[4];

Based on the code below, it looks like it would be nice to make this
"const char *keys[4];".

If that gets changed, it's passed to automatically generated code where
the parameter is not const.  It's the 2nd parameter here:

ovsrec_port_set_statistics(const struct ovsrec_port *row, char
**key_statistics, const int64_t *value_statistics, size_t n_statistics)

The second parameter is explicitly excluded from being made const, but
it's not clear why.  Is there some C detail I'm forgetting, or does this
seem safe?

The following change results in the 2nd parameter above being made const
and seems to still build without any warnings:

diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in
index 6c8aa43..2d32850 100755
--- a/ovsdb/ovsdb-idlc.in
+++ b/ovsdb/ovsdb-idlc.in
@@ -21,7 +21,7 @@ def annotateSchema(schemaFile, annotationFile):
 sys.stdout.write('\n')

 def constify(cType, const):
-if (const and cType.endswith('*') and not cType.endswith('**')):
+if const and cType.endswith('*'):
 return 'const %s' % cType
 else:
 return cType


> +int64_t int_values[4];
>  struct smap smap;
>  
>  if (port_is_synthetic(port)) {
> @@ -2539,6 +2539,8 @@ port_refresh_rstp_status(struct port *port)
>  int_values[1] = status.rx_count;
>  keys[2] = "rstp_uptime";
>  int_values[2] = status.uptime;
> +keys[3] = "rstp_error_count";
> +int_values[3] = status.error_count;
>  ovsrec_port_set_rstp_statistics(port->cfg, keys, int_values,
>  ARRAY_SIZE(int_values));
>  }
> 


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


Re: [ovs-dev] [PATCH 1/8] bridge: Publish error count in database's rstp_statistics.

2015-02-19 Thread Russell Bryant
On 02/19/2015 10:34 AM, Russell Bryant wrote:
> On 02/19/2015 03:12 AM, Ben Pfaff wrote:
>> The lower layers count errors but until now nothing actually reported them.
>>
>> Found by inspection.
>>
>> Signed-off-by: Ben Pfaff 
>> ---
>>  vswitchd/bridge.c | 8 +---
>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
>> index 297c0dd..a143be1 100644
>> --- a/vswitchd/bridge.c
>> +++ b/vswitchd/bridge.c
>> @@ -1,4 +1,4 @@
>> -/* Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
>> +/* Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
>>   *
>>   * Licensed under the Apache License, Version 2.0 (the "License");
>>   * you may not use this file except in compliance with the License.
>> @@ -2489,8 +2489,8 @@ port_refresh_rstp_status(struct port *port)
>>  struct ofproto *ofproto = port->bridge->ofproto;
>>  struct iface *iface;
>>  struct ofproto_port_rstp_status status;
>> -char *keys[3];
>> -int64_t int_values[3];
>> +char *keys[4];
> 
> Based on the code below, it looks like it would be nice to make this
> "const char *keys[4];".
> 
> If that gets changed, it's passed to automatically generated code where
> the parameter is not const.  It's the 2nd parameter here:
> 
> ovsrec_port_set_statistics(const struct ovsrec_port *row, char
> **key_statistics, const int64_t *value_statistics, size_t n_statistics)
> 
> The second parameter is explicitly excluded from being made const, but
> it's not clear why.  Is there some C detail I'm forgetting, or does this
> seem safe?
> 
> The following change results in the 2nd parameter above being made const
> and seems to still build without any warnings:

Nevermind ... of course there's a bunch of warnings.  I just didn't set
-Werror.

It looks like the below change wouldn't be desired, but maybe adding
const to just "char **" would be OK.  Of course, it's not important
anyway ...

> diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in
> index 6c8aa43..2d32850 100755
> --- a/ovsdb/ovsdb-idlc.in
> +++ b/ovsdb/ovsdb-idlc.in
> @@ -21,7 +21,7 @@ def annotateSchema(schemaFile, annotationFile):
>  sys.stdout.write('\n')
> 
>  def constify(cType, const):
> -if (const and cType.endswith('*') and not cType.endswith('**')):
> +if const and cType.endswith('*'):
>  return 'const %s' % cType
>  else:
>  return cType
> 
> 
>> +int64_t int_values[4];
>>  struct smap smap;
>>  
>>  if (port_is_synthetic(port)) {
>> @@ -2539,6 +2539,8 @@ port_refresh_rstp_status(struct port *port)
>>  int_values[1] = status.rx_count;
>>  keys[2] = "rstp_uptime";
>>  int_values[2] = status.uptime;
>> +keys[3] = "rstp_error_count";
>> +int_values[3] = status.error_count;
>>  ovsrec_port_set_rstp_statistics(port->cfg, keys, int_values,
>>  ARRAY_SIZE(int_values));
>>  }
>>
> 
> 


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


Re: [ovs-dev] [PATCH 6/8] ovsdb-doc: Factor out nroff formatting into a separate Python module.

2015-02-19 Thread Russell Bryant
On 02/19/2015 07:02 AM, Gurucharan Shetty wrote:
> I don't understand nroff well enough to review patch 6 and patch 7. If
> anyone else is up for it, please review.

I don't either, but this one looks like just a code move.  It looks good
to me.

My only thought was about adding something about trying to follow PEP8
for Python coding style to the coding style doc.  :-)

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


Re: [ovs-dev] [PATCH 6/8] ovsdb-doc: Factor out nroff formatting into a separate Python module.

2015-02-19 Thread Gurucharan Shetty
On Thu, Feb 19, 2015 at 9:09 AM, Ben Pfaff  wrote:
> On Thu, Feb 19, 2015 at 11:00:56AM -0500, Russell Bryant wrote:
>> On 02/19/2015 07:02 AM, Gurucharan Shetty wrote:
>> > I don't understand nroff well enough to review patch 6 and patch 7. If
>> > anyone else is up for it, please review.
>>
>> I don't either, but this one looks like just a code move.  It looks good
>> to me.
>
> That's right, this is just code movement.
It did look like you added an additional function while code
refactoring. Maybe I did not look carefully.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 6/8] ovsdb-doc: Factor out nroff formatting into a separate Python module.

2015-02-19 Thread Ben Pfaff
On Thu, Feb 19, 2015 at 11:00:56AM -0500, Russell Bryant wrote:
> On 02/19/2015 07:02 AM, Gurucharan Shetty wrote:
> > I don't understand nroff well enough to review patch 6 and patch 7. If
> > anyone else is up for it, please review.
> 
> I don't either, but this one looks like just a code move.  It looks good
> to me.

That's right, this is just code movement.

> My only thought was about adding something about trying to follow PEP8
> for Python coding style to the coding style doc.  :-)

We try to do something close to that for new Python code, but there's
a lot of Python in the tree that pre-dates that advice.

It's still reasonable to add that to CodingStyle.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 1/8] bridge: Publish error count in database's rstp_statistics.

2015-02-19 Thread Ben Pfaff
On Thu, Feb 19, 2015 at 10:45:21AM -0500, Russell Bryant wrote:
> On 02/19/2015 10:34 AM, Russell Bryant wrote:
> > On 02/19/2015 03:12 AM, Ben Pfaff wrote:
> >> The lower layers count errors but until now nothing actually reported them.
> >>
> >> Found by inspection.
> >>
> >> Signed-off-by: Ben Pfaff 
> >> ---
> >>  vswitchd/bridge.c | 8 +---
> >>  1 file changed, 5 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> >> index 297c0dd..a143be1 100644
> >> --- a/vswitchd/bridge.c
> >> +++ b/vswitchd/bridge.c
> >> @@ -1,4 +1,4 @@
> >> -/* Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
> >> +/* Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, 
> >> Inc.
> >>   *
> >>   * Licensed under the Apache License, Version 2.0 (the "License");
> >>   * you may not use this file except in compliance with the License.
> >> @@ -2489,8 +2489,8 @@ port_refresh_rstp_status(struct port *port)
> >>  struct ofproto *ofproto = port->bridge->ofproto;
> >>  struct iface *iface;
> >>  struct ofproto_port_rstp_status status;
> >> -char *keys[3];
> >> -int64_t int_values[3];
> >> +char *keys[4];
> > 
> > Based on the code below, it looks like it would be nice to make this
> > "const char *keys[4];".
> > 
> > If that gets changed, it's passed to automatically generated code where
> > the parameter is not const.  It's the 2nd parameter here:
> > 
> > ovsrec_port_set_statistics(const struct ovsrec_port *row, char
> > **key_statistics, const int64_t *value_statistics, size_t n_statistics)
> > 
> > The second parameter is explicitly excluded from being made const, but
> > it's not clear why.  Is there some C detail I'm forgetting, or does this
> > seem safe?
> > 
> > The following change results in the 2nd parameter above being made const
> > and seems to still build without any warnings:
> 
> Nevermind ... of course there's a bunch of warnings.  I just didn't set
> -Werror.
> 
> It looks like the below change wouldn't be desired, but maybe adding
> const to just "char **" would be OK.  Of course, it's not important
> anyway ...

"const" has really weird semantics in cases with double or multiple
pointers in C (C++ is actually more sane here) so I'm in the habit of
leaving off const in such cases.  Otherwise you end up with casts in
places where it seems like you shouldn't need them.  That's why I tend
not to use them.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 2/8] vswitch: Document columns that had been previously overlooked.

2015-02-19 Thread Ben Pfaff
On Thu, Feb 19, 2015 at 04:00:43AM -0800, Gurucharan Shetty wrote:
> On Thu, Feb 19, 2015 at 12:12 AM, Ben Pfaff  wrote:
> > A fair number of columns had been overlooked.  This documents them.
> >
> > The patch is smaller than it appears because this rearranges the STP and
> > RSTP documentation to group configuration, status, and statistics together
> > in the documentation for clarity.
> >
> > Signed-off-by: Ben Pfaff 
> I do not know RSTP to review whether more clarity can be provided on
> the newly added column documentation. If no one else reviews,
> Acked-by: Gurucharan Shetty 

Jarno, I think you know RSTP better than most here.  Can you provide
any better documentation for the newly documented columns here?


> > ---
> >  vswitchd/vswitch.xml | 680 
> > +--
> >  1 file changed, 391 insertions(+), 289 deletions(-)
> >
> > diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> > index 932e4b2..04de3ca 100644
> > --- a/vswitchd/vswitch.xml
> > +++ b/vswitchd/vswitch.xml
> > @@ -681,80 +681,232 @@
> >  
> >
> >  
> > -  The IEEE 802.1D Spanning Tree Protocol (STP) is a network protocol
> > -  that ensures loop-free topologies.  It allows redundant links to
> > -  be included in the network to provide automatic backup paths if
> > -  the active links fails.
> > +  
> > +The IEEE 802.1D Spanning Tree Protocol (STP) is a network protocol
> > +that ensures loop-free topologies.  It allows redundant links to
> > +be included in the network to provide automatic backup paths if
> > +the active links fails.
> > +  
> >
> > -  
> > -Enable spanning tree on the bridge.  By default, STP is disabled
> > -on bridges.  Bond, internal, and mirror ports are not supported
> > -and will not participate in the spanning tree.
> > -  
> > +  
> > +These settings configure the slower-to-converge but still widely
> > +supported version of Spanning Tree Protocol, sometimes known as
> > +802.1D-1998.  Open vSwitch also supports the newer Rapid Spanning 
> > Tree
> > +Protocol (RSTP), documented later in the section titled Rapid
> > +Spanning Tree Configuration.
> > +  
> >
> > -  
> > -The bridge's STP identifier (the lower 48 bits of the bridge-id)
> > -in the form
> > -
> > xx:xx:xx:xx:xx:xx.
> > -By default, the identifier is the MAC address of the bridge.
> > -  
> > +  
> > +
> > +  
> > +Enable spanning tree on the bridge.  By default, STP is 
> > disabled
> > +on bridges.  Bond, internal, and mirror ports are not supported
> > +and will not participate in the spanning tree.
> > +  
> >
> > -   > -  type='{"type": "integer", "minInteger": 0, "maxInteger": 
> > 65535}'>
> > -The bridge's relative priority value for determining the root
> > -bridge (the upper 16 bits of the bridge-id).  A bridge with the
> > -lowest bridge-id is elected the root.  By default, the priority
> > -is 0x8000.
> > -  
> > +  
> > +STP and RSTP are mutually exclusive.  If both are enabled, RSTP
> > +will be used.
> > +  
> > +
> >
> > -   > -  type='{"type": "integer", "minInteger": 1, "maxInteger": 
> > 10}'>
> > -The interval between transmissions of hello messages by
> > -designated ports, in seconds.  By default the hello interval is
> > -2 seconds.
> > -  
> > +
> > +  The bridge's STP identifier (the lower 48 bits of the bridge-id)
> > +  in the form
> > +  
> > xx:xx:xx:xx:xx:xx.
> > +  By default, the identifier is the MAC address of the bridge.
> > +
> >
> > -   > -  type='{"type": "integer", "minInteger": 6, "maxInteger": 
> > 40}'>
> > -The maximum age of the information transmitted by the bridge
> > -when it is the root bridge, in seconds.  By default, the maximum
> > -age is 20 seconds.
> > -  
> > + > +type='{"type": "integer", "minInteger": 0, "maxInteger": 
> > 65535}'>
> > +  The bridge's relative priority value for determining the root
> > +  bridge (the upper 16 bits of the bridge-id).  A bridge with the
> > +  lowest bridge-id is elected the root.  By default, the priority
> > +  is 0x8000.
> > +
> >
> > -   > -  type='{"type": "integer", "minInteger": 4, "maxInteger": 
> > 30}'>
> > -The delay to wait between transitioning root and designated
> > -ports to forwarding, in seconds.  By default, the
> > -forwarding delay is 15 seconds.
> > -  
> > + > +type='{"type": "integer", "minInteger": 1, "maxInteger": 
> > 10}'>
> > +  The interval between transmissions of he

Re: [ovs-dev] [PATCH 7/8] xml2nroff: New program to generate a manpage from XML input.

2015-02-19 Thread Russell Bryant
On 02/19/2015 03:12 AM, Ben Pfaff wrote:
> I really can't stand nroff syntax.  This makes it possible to install
> nroff but write in a more sensible XML syntax.
> 
> The following commit adds the first user.
> 
> Signed-off-by: Ben Pfaff 
> ---
>  Makefile.am |   1 +
>  build-aux/xml2nroff | 110 
> 
>  2 files changed, 111 insertions(+)
>  create mode 100755 build-aux/xml2nroff
> 
> diff --git a/Makefile.am b/Makefile.am
> index 28496b3..0480d20 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -104,6 +104,7 @@ EXTRA_DIST = \
>   build-aux/dist-docs \
>   build-aux/sodepends.pl \
>   build-aux/soexpand.pl \
> + build-aux/xml2nroff \
>   $(MAN_FRAGMENTS) \
>   $(MAN_ROOTS) \
>   Vagrantfile
> diff --git a/build-aux/xml2nroff b/build-aux/xml2nroff
> new file mode 100755
> index 000..817abc6
> --- /dev/null
> +++ b/build-aux/xml2nroff
> @@ -0,0 +1,110 @@
> +#! /usr/bin/python

A license header would be good here.

> +if __name__ == "__main__":
> +try:

Instead of the double nesting here, I would move the outer one to just
around the part that could raise error.Error, which appears to be only
the call to manpage_to_nroff().

> +try:
> +options, args = getopt.gnu_getopt(sys.argv[1:], 'hV',
> +  ['version=', 'help'])
> +except getopt.GetoptError, geo:
> +sys.stderr.write("%s: %s\n" % (argv0, geo.msg))
> +sys.exit(1)
> +
> +er_diagram = None
> +title = None
> +version = None
> +for key, value in options:
> +if key == '--version':
> +version = value
> +elif key in ['-h', '--help']:
> +usage()
> +else:
> +sys.exit(0)
> +
> +if len(args) != 1:
> +sys.stderr.write("%s: exactly 1 non-option arguments required "
> + "(use --help for help)\n" % argv0)
> +sys.exit(1)
> +
> +s = manpage_to_nroff(args[0], version)
> +for line in s.split("\n"):

can also use:

for line in s.splitlines():
...

> +line = line.strip()
> +if len(line):

also equivalent:

if line:

> +print line
> +
> +except error.Error, e:
> +sys.stderr.write("%s: %s\n" % (argv0, e.msg))
> +sys.exit(1)
> +
> +# Local variables:
> +# mode: python
> +# End:
> 

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


Re: [ovs-dev] [PATCH 6/8] ovsdb-doc: Factor out nroff formatting into a separate Python module.

2015-02-19 Thread Ben Pfaff
On Thu, Feb 19, 2015 at 09:12:07AM -0800, Gurucharan Shetty wrote:
> On Thu, Feb 19, 2015 at 9:09 AM, Ben Pfaff  wrote:
> > On Thu, Feb 19, 2015 at 11:00:56AM -0500, Russell Bryant wrote:
> >> On 02/19/2015 07:02 AM, Gurucharan Shetty wrote:
> >> > I don't understand nroff well enough to review patch 6 and patch 7. If
> >> > anyone else is up for it, please review.
> >>
> >> I don't either, but this one looks like just a code move.  It looks good
> >> to me.
> >
> > That's right, this is just code movement.
> It did look like you added an additional function while code
> refactoring. Maybe I did not look carefully.

Oh, looking again, I added support for a  element.  I didn't end
up using that later in the series, so I'm happy to drop it.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 2/8] vswitch: Document columns that had been previously overlooked.

2015-02-19 Thread Jarno Rajahalme
I’ll look in to this,

  Jarno

On Feb 19, 2015, at 9:22 AM, Ben Pfaff  wrote:

> On Thu, Feb 19, 2015 at 04:00:43AM -0800, Gurucharan Shetty wrote:
>> On Thu, Feb 19, 2015 at 12:12 AM, Ben Pfaff  wrote:
>>> A fair number of columns had been overlooked.  This documents them.
>>> 
>>> The patch is smaller than it appears because this rearranges the STP and
>>> RSTP documentation to group configuration, status, and statistics together
>>> in the documentation for clarity.
>>> 
>>> Signed-off-by: Ben Pfaff 
>> I do not know RSTP to review whether more clarity can be provided on
>> the newly added column documentation. If no one else reviews,
>> Acked-by: Gurucharan Shetty 
> 
> Jarno, I think you know RSTP better than most here.  Can you provide
> any better documentation for the newly documented columns here?
> 
> 
>>> ---
>>> vswitchd/vswitch.xml | 680 
>>> +--
>>> 1 file changed, 391 insertions(+), 289 deletions(-)
>>> 
>>> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
>>> index 932e4b2..04de3ca 100644
>>> --- a/vswitchd/vswitch.xml
>>> +++ b/vswitchd/vswitch.xml
>>> @@ -681,80 +681,232 @@
>>> 
>>> 
>>> 
>>> -  The IEEE 802.1D Spanning Tree Protocol (STP) is a network protocol
>>> -  that ensures loop-free topologies.  It allows redundant links to
>>> -  be included in the network to provide automatic backup paths if
>>> -  the active links fails.
>>> +  
>>> +The IEEE 802.1D Spanning Tree Protocol (STP) is a network protocol
>>> +that ensures loop-free topologies.  It allows redundant links to
>>> +be included in the network to provide automatic backup paths if
>>> +the active links fails.
>>> +  
>>> 
>>> -  
>>> -Enable spanning tree on the bridge.  By default, STP is disabled
>>> -on bridges.  Bond, internal, and mirror ports are not supported
>>> -and will not participate in the spanning tree.
>>> -  
>>> +  
>>> +These settings configure the slower-to-converge but still widely
>>> +supported version of Spanning Tree Protocol, sometimes known as
>>> +802.1D-1998.  Open vSwitch also supports the newer Rapid Spanning 
>>> Tree
>>> +Protocol (RSTP), documented later in the section titled Rapid
>>> +Spanning Tree Configuration.
>>> +  
>>> 
>>> -  
>>> -The bridge's STP identifier (the lower 48 bits of the bridge-id)
>>> -in the form
>>> -
>>> xx:xx:xx:xx:xx:xx.
>>> -By default, the identifier is the MAC address of the bridge.
>>> -  
>>> +  
>>> +
>>> +  
>>> +Enable spanning tree on the bridge.  By default, STP is 
>>> disabled
>>> +on bridges.  Bond, internal, and mirror ports are not supported
>>> +and will not participate in the spanning tree.
>>> +  
>>> 
>>> -  >> -  type='{"type": "integer", "minInteger": 0, "maxInteger": 
>>> 65535}'>
>>> -The bridge's relative priority value for determining the root
>>> -bridge (the upper 16 bits of the bridge-id).  A bridge with the
>>> -lowest bridge-id is elected the root.  By default, the priority
>>> -is 0x8000.
>>> -  
>>> +  
>>> +STP and RSTP are mutually exclusive.  If both are enabled, RSTP
>>> +will be used.
>>> +  
>>> +
>>> 
>>> -  >> -  type='{"type": "integer", "minInteger": 1, "maxInteger": 
>>> 10}'>
>>> -The interval between transmissions of hello messages by
>>> -designated ports, in seconds.  By default the hello interval is
>>> -2 seconds.
>>> -  
>>> +
>>> +  The bridge's STP identifier (the lower 48 bits of the bridge-id)
>>> +  in the form
>>> +  
>>> xx:xx:xx:xx:xx:xx.
>>> +  By default, the identifier is the MAC address of the bridge.
>>> +
>>> 
>>> -  >> -  type='{"type": "integer", "minInteger": 6, "maxInteger": 
>>> 40}'>
>>> -The maximum age of the information transmitted by the bridge
>>> -when it is the root bridge, in seconds.  By default, the maximum
>>> -age is 20 seconds.
>>> -  
>>> +>> +type='{"type": "integer", "minInteger": 0, "maxInteger": 
>>> 65535}'>
>>> +  The bridge's relative priority value for determining the root
>>> +  bridge (the upper 16 bits of the bridge-id).  A bridge with the
>>> +  lowest bridge-id is elected the root.  By default, the priority
>>> +  is 0x8000.
>>> +
>>> 
>>> -  >> -  type='{"type": "integer", "minInteger": 4, "maxInteger": 
>>> 30}'>
>>> -The delay to wait between transitioning root and designated
>>> -ports to forwarding, in seconds.  By default, the
>>> -forwarding delay is 15 seconds.
>>> -  
>>> +>> +type='{"type": "integ

Re: [ovs-dev] [PATCH 1/8] bridge: Publish error count in database's rstp_statistics.

2015-02-19 Thread Russell Bryant
On 02/19/2015 12:19 PM, Ben Pfaff wrote:
> On Thu, Feb 19, 2015 at 10:45:21AM -0500, Russell Bryant wrote:
>> On 02/19/2015 10:34 AM, Russell Bryant wrote:
>>> On 02/19/2015 03:12 AM, Ben Pfaff wrote:
 The lower layers count errors but until now nothing actually reported them.

 Found by inspection.

 Signed-off-by: Ben Pfaff 
 ---
  vswitchd/bridge.c | 8 +---
  1 file changed, 5 insertions(+), 3 deletions(-)

 diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
 index 297c0dd..a143be1 100644
 --- a/vswitchd/bridge.c
 +++ b/vswitchd/bridge.c
 @@ -1,4 +1,4 @@
 -/* Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
 +/* Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, 
 Inc.
   *
   * Licensed under the Apache License, Version 2.0 (the "License");
   * you may not use this file except in compliance with the License.
 @@ -2489,8 +2489,8 @@ port_refresh_rstp_status(struct port *port)
  struct ofproto *ofproto = port->bridge->ofproto;
  struct iface *iface;
  struct ofproto_port_rstp_status status;
 -char *keys[3];
 -int64_t int_values[3];
 +char *keys[4];
>>>
>>> Based on the code below, it looks like it would be nice to make this
>>> "const char *keys[4];".
>>>
>>> If that gets changed, it's passed to automatically generated code where
>>> the parameter is not const.  It's the 2nd parameter here:
>>>
>>> ovsrec_port_set_statistics(const struct ovsrec_port *row, char
>>> **key_statistics, const int64_t *value_statistics, size_t n_statistics)
>>>
>>> The second parameter is explicitly excluded from being made const, but
>>> it's not clear why.  Is there some C detail I'm forgetting, or does this
>>> seem safe?
>>>
>>> The following change results in the 2nd parameter above being made const
>>> and seems to still build without any warnings:
>>
>> Nevermind ... of course there's a bunch of warnings.  I just didn't set
>> -Werror.
>>
>> It looks like the below change wouldn't be desired, but maybe adding
>> const to just "char **" would be OK.  Of course, it's not important
>> anyway ...
> 
> "const" has really weird semantics in cases with double or multiple
> pointers in C (C++ is actually more sane here) so I'm in the habit of
> leaving off const in such cases.  Otherwise you end up with casts in
> places where it seems like you shouldn't need them.  That's why I tend
> not to use them.
> 

Fair enough.  FWIW, here's the impact to the code if it were added.
It's not bad and actually drops a cast.

diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in
index 6c8aa43..67e8a4e 100755
--- a/ovsdb/ovsdb-idlc.in
+++ b/ovsdb/ovsdb-idlc.in
@@ -21,7 +21,9 @@ def annotateSchema(schemaFile, annotationFile):
 sys.stdout.write('\n')

 def constify(cType, const):
-if (const and cType.endswith('*') and not cType.endswith('**')):
+if (const
+and cType.endswith('*') and
+(cType == 'char **' or not cType.endswith('**'))):
 return 'const %s' % cType
 else:
 return cType
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 297c0dd..7d9cf25 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -2262,7 +2262,7 @@ iface_refresh_cfm_stats(struct iface *iface)
 reasons[j++] = cfm_fault_reason_to_str(reason);
 }
 }
-ovsrec_interface_set_cfm_fault_status(cfg, (char **) reasons, j);
+ovsrec_interface_set_cfm_fault_status(cfg, reasons, j);

 ovsrec_interface_set_cfm_flap_count(cfg, &cfm_flap_count, 1);

@@ -2307,7 +2307,7 @@ iface_refresh_stats(struct iface *iface)
 enum { N_IFACE_STATS = IFACE_STATS };
 #undef IFACE_STAT
 int64_t values[N_IFACE_STATS];
-char *keys[N_IFACE_STATS];
+const char *keys[N_IFACE_STATS];
 int n;

 struct netdev_stats stats;
@@ -2419,7 +2419,7 @@ port_refresh_stp_stats(struct port *port)
 struct ofproto *ofproto = port->bridge->ofproto;
 struct iface *iface;
 struct ofproto_port_stp_stats stats;
-char *keys[3];
+const char *keys[3];
 int64_t int_values[3];

 if (port_is_synthetic(port)) {
@@ -2489,7 +2489,7 @@ port_refresh_rstp_status(struct port *port)
 struct ofproto *ofproto = port->bridge->ofproto;
 struct iface *iface;
 struct ofproto_port_rstp_status status;
-char *keys[3];
+const char *keys[3];
 int64_t int_values[3];
 struct smap smap;

@@ -4728,7 +4728,7 @@ mirror_refresh_stats(struct mirror *m)
 {
 struct ofproto *ofproto = m->bridge->ofproto;
 uint64_t tx_packets, tx_bytes;
-char *keys[2];
+const char *keys[2];
 int64_t values[2];
 size_t stat_cnt = 0;

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


Re: [ovs-dev] [PATCH 1/8] bridge: Publish error count in database's rstp_statistics.

2015-02-19 Thread Ben Pfaff
On Thu, Feb 19, 2015 at 12:41:48PM -0500, Russell Bryant wrote:
> On 02/19/2015 12:19 PM, Ben Pfaff wrote:
> > On Thu, Feb 19, 2015 at 10:45:21AM -0500, Russell Bryant wrote:
> >> On 02/19/2015 10:34 AM, Russell Bryant wrote:
> >>> On 02/19/2015 03:12 AM, Ben Pfaff wrote:
>  The lower layers count errors but until now nothing actually reported 
>  them.
> 
>  Found by inspection.
> 
>  Signed-off-by: Ben Pfaff 
>  ---
>   vswitchd/bridge.c | 8 +---
>   1 file changed, 5 insertions(+), 3 deletions(-)
> 
>  diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
>  index 297c0dd..a143be1 100644
>  --- a/vswitchd/bridge.c
>  +++ b/vswitchd/bridge.c
>  @@ -1,4 +1,4 @@
>  -/* Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
>  +/* Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, 
>  Inc.
>    *
>    * Licensed under the Apache License, Version 2.0 (the "License");
>    * you may not use this file except in compliance with the License.
>  @@ -2489,8 +2489,8 @@ port_refresh_rstp_status(struct port *port)
>   struct ofproto *ofproto = port->bridge->ofproto;
>   struct iface *iface;
>   struct ofproto_port_rstp_status status;
>  -char *keys[3];
>  -int64_t int_values[3];
>  +char *keys[4];
> >>>
> >>> Based on the code below, it looks like it would be nice to make this
> >>> "const char *keys[4];".
> >>>
> >>> If that gets changed, it's passed to automatically generated code where
> >>> the parameter is not const.  It's the 2nd parameter here:
> >>>
> >>> ovsrec_port_set_statistics(const struct ovsrec_port *row, char
> >>> **key_statistics, const int64_t *value_statistics, size_t n_statistics)
> >>>
> >>> The second parameter is explicitly excluded from being made const, but
> >>> it's not clear why.  Is there some C detail I'm forgetting, or does this
> >>> seem safe?
> >>>
> >>> The following change results in the 2nd parameter above being made const
> >>> and seems to still build without any warnings:
> >>
> >> Nevermind ... of course there's a bunch of warnings.  I just didn't set
> >> -Werror.
> >>
> >> It looks like the below change wouldn't be desired, but maybe adding
> >> const to just "char **" would be OK.  Of course, it's not important
> >> anyway ...
> > 
> > "const" has really weird semantics in cases with double or multiple
> > pointers in C (C++ is actually more sane here) so I'm in the habit of
> > leaving off const in such cases.  Otherwise you end up with casts in
> > places where it seems like you shouldn't need them.  That's why I tend
> > not to use them.
> > 
> 
> Fair enough.  FWIW, here's the impact to the code if it were added.
> It's not bad and actually drops a cast.

You're right, this is an improvement.  May I have a Signed-off-by to
commit this?  A Signed-off-by has the following form and meaning, as
described in CONTRIBUTING.md.  (This is the same form and meaning as
used in Linux kernel development.)

Signed-off-by: Author Name 

Informally, this indicates that Author Name is the author or
submitter of a patch and has the authority to submit it under
the terms of the license.  The formal meaning is to agree to
the Developer's Certificate of Origin (see below).

If the author and submitter are different, each must sign off.
If the patch has more than one author, all must sign off.

Signed-off-by: Author Name 
Signed-off-by: Submitter Name 

Developer's Certificate of Origin
-

To help track the author of a patch as well as the submission chain,
and be clear that the developer has authority to submit a patch for
inclusion in openvswitch please sign off your work.  The sign off
certifies the following:

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or

(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or

(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.

(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed c

[ovs-dev] [PATCH] CodingStyle: recommend PEP 8 for Python code

2015-02-19 Thread Russell Bryant
Add a new section about Python code to the coding style document.
Suggest that all new Python code should adhere to the PEP 8 standard.
Also include a reference to tools that can quickly check code for
style issues.

Signed-off-by: Russell Bryant 
---
 CodingStyle.md | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/CodingStyle.md b/CodingStyle.md
index 77b1572..0a441e0 100644
--- a/CodingStyle.md
+++ b/CodingStyle.md
@@ -569,3 +569,10 @@ for other compilers.  You can, however, use C99 features 
or GCC
 extensions also supported by Clang in code that compiles only on
 GNU/Linux (such as lib/netdev-linux.c), because GCC is the system
 compiler there.
+
+## PYTHON
+
+When introducing new Python code, try to follow Python's
+[PEP 8](http://www.python.org/dev/peps/pep-0008/) style.
+Consider running the `pep8` or `flake8` tool against your
+code to find issues.
-- 
2.1.0

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


Re: [ovs-dev] [PATCH 1/8] bridge: Publish error count in database's rstp_statistics.

2015-02-19 Thread Russell Bryant
On 02/19/2015 12:47 PM, Ben Pfaff wrote:
> On Thu, Feb 19, 2015 at 12:41:48PM -0500, Russell Bryant wrote:
>> On 02/19/2015 12:19 PM, Ben Pfaff wrote:
>>> On Thu, Feb 19, 2015 at 10:45:21AM -0500, Russell Bryant wrote:
 On 02/19/2015 10:34 AM, Russell Bryant wrote:
> On 02/19/2015 03:12 AM, Ben Pfaff wrote:
>> The lower layers count errors but until now nothing actually reported 
>> them.
>>
>> Found by inspection.
>>
>> Signed-off-by: Ben Pfaff 
>> ---
>>  vswitchd/bridge.c | 8 +---
>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
>> index 297c0dd..a143be1 100644
>> --- a/vswitchd/bridge.c
>> +++ b/vswitchd/bridge.c
>> @@ -1,4 +1,4 @@
>> -/* Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
>> +/* Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, 
>> Inc.
>>   *
>>   * Licensed under the Apache License, Version 2.0 (the "License");
>>   * you may not use this file except in compliance with the License.
>> @@ -2489,8 +2489,8 @@ port_refresh_rstp_status(struct port *port)
>>  struct ofproto *ofproto = port->bridge->ofproto;
>>  struct iface *iface;
>>  struct ofproto_port_rstp_status status;
>> -char *keys[3];
>> -int64_t int_values[3];
>> +char *keys[4];
>
> Based on the code below, it looks like it would be nice to make this
> "const char *keys[4];".
>
> If that gets changed, it's passed to automatically generated code where
> the parameter is not const.  It's the 2nd parameter here:
>
> ovsrec_port_set_statistics(const struct ovsrec_port *row, char
> **key_statistics, const int64_t *value_statistics, size_t n_statistics)
>
> The second parameter is explicitly excluded from being made const, but
> it's not clear why.  Is there some C detail I'm forgetting, or does this
> seem safe?
>
> The following change results in the 2nd parameter above being made const
> and seems to still build without any warnings:

 Nevermind ... of course there's a bunch of warnings.  I just didn't set
 -Werror.

 It looks like the below change wouldn't be desired, but maybe adding
 const to just "char **" would be OK.  Of course, it's not important
 anyway ...
>>>
>>> "const" has really weird semantics in cases with double or multiple
>>> pointers in C (C++ is actually more sane here) so I'm in the habit of
>>> leaving off const in such cases.  Otherwise you end up with casts in
>>> places where it seems like you shouldn't need them.  That's why I tend
>>> not to use them.
>>>
>>
>> Fair enough.  FWIW, here's the impact to the code if it were added.
>> It's not bad and actually drops a cast.
> 
> You're right, this is an improvement.  May I have a Signed-off-by to
> commit this?  A Signed-off-by has the following form and meaning, as
> described in CONTRIBUTING.md.  (This is the same form and meaning as
> used in Linux kernel development.)

Sure, I'll submit it to the list as a proper patch in a moment.

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


[ovs-dev] [PATCH] ovsdb-idlc: constify 'char **'

2015-02-19 Thread Russell Bryant
Update the logic used in constify() to add const to a 'char **' while
still excluding all other cases of more than one level of indirection.

This results in adding const to a parameter of a generated setter
function where we're generally passing in array of constant strings.
As a result, this patch includes the other necessary fixes to the code
base to reflect the const addition.

Signed-off-by: Russell Bryant 
---
 ovsdb/ovsdb-idlc.in |  4 +++-
 vswitchd/bridge.c   | 10 +-
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in
index 6c8aa43..67e8a4e 100755
--- a/ovsdb/ovsdb-idlc.in
+++ b/ovsdb/ovsdb-idlc.in
@@ -21,7 +21,9 @@ def annotateSchema(schemaFile, annotationFile):
 sys.stdout.write('\n')
 
 def constify(cType, const):
-if (const and cType.endswith('*') and not cType.endswith('**')):
+if (const
+and cType.endswith('*') and
+(cType == 'char **' or not cType.endswith('**'))):
 return 'const %s' % cType
 else:
 return cType
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 297c0dd..7d9cf25 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -2262,7 +2262,7 @@ iface_refresh_cfm_stats(struct iface *iface)
 reasons[j++] = cfm_fault_reason_to_str(reason);
 }
 }
-ovsrec_interface_set_cfm_fault_status(cfg, (char **) reasons, j);
+ovsrec_interface_set_cfm_fault_status(cfg, reasons, j);
 
 ovsrec_interface_set_cfm_flap_count(cfg, &cfm_flap_count, 1);
 
@@ -2307,7 +2307,7 @@ iface_refresh_stats(struct iface *iface)
 enum { N_IFACE_STATS = IFACE_STATS };
 #undef IFACE_STAT
 int64_t values[N_IFACE_STATS];
-char *keys[N_IFACE_STATS];
+const char *keys[N_IFACE_STATS];
 int n;
 
 struct netdev_stats stats;
@@ -2419,7 +2419,7 @@ port_refresh_stp_stats(struct port *port)
 struct ofproto *ofproto = port->bridge->ofproto;
 struct iface *iface;
 struct ofproto_port_stp_stats stats;
-char *keys[3];
+const char *keys[3];
 int64_t int_values[3];
 
 if (port_is_synthetic(port)) {
@@ -2489,7 +2489,7 @@ port_refresh_rstp_status(struct port *port)
 struct ofproto *ofproto = port->bridge->ofproto;
 struct iface *iface;
 struct ofproto_port_rstp_status status;
-char *keys[3];
+const char *keys[3];
 int64_t int_values[3];
 struct smap smap;
 
@@ -4728,7 +4728,7 @@ mirror_refresh_stats(struct mirror *m)
 {
 struct ofproto *ofproto = m->bridge->ofproto;
 uint64_t tx_packets, tx_bytes;
-char *keys[2];
+const char *keys[2];
 int64_t values[2];
 size_t stat_cnt = 0;
 
-- 
2.1.0

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


Re: [ovs-dev] [PATCH] CodingStyle: recommend PEP 8 for Python code

2015-02-19 Thread Ben Pfaff
On Thu, Feb 19, 2015 at 01:03:53PM -0500, Russell Bryant wrote:
> Add a new section about Python code to the coding style document.
> Suggest that all new Python code should adhere to the PEP 8 standard.
> Also include a reference to tools that can quickly check code for
> style issues.
> 
> Signed-off-by: Russell Bryant 

Applied, thanks!
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] ovsdb-idlc: constify 'char **'

2015-02-19 Thread Ben Pfaff
On Thu, Feb 19, 2015 at 01:10:51PM -0500, Russell Bryant wrote:
> Update the logic used in constify() to add const to a 'char **' while
> still excluding all other cases of more than one level of indirection.
> 
> This results in adding const to a parameter of a generated setter
> function where we're generally passing in array of constant strings.
> As a result, this patch includes the other necessary fixes to the code
> base to reflect the const addition.
> 
> Signed-off-by: Russell Bryant 

Applied, thanks!
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 1/8] bridge: Publish error count in database's rstp_statistics.

2015-02-19 Thread Ben Pfaff
On Thu, Feb 19, 2015 at 02:02:49AM -0800, Gurucharan Shetty wrote:
> On Thu, Feb 19, 2015 at 12:12 AM, Ben Pfaff  wrote:
> > The lower layers count errors but until now nothing actually reported them.
> >
> > Found by inspection.
> >
> > Signed-off-by: Ben Pfaff 
> Acked-by: Gurucharan Shetty 

Thanks for the review, I applied this to master.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 2/8] vswitch: Document columns that had been previously overlooked.

2015-02-19 Thread Ben Pfaff
I decided to apply this, but I'd still like to improve the
documentation further if possible, so when you get a chance please
review the RSTP documentation in vswitch.xml.

On Thu, Feb 19, 2015 at 09:48:15AM -0800, Jarno Rajahalme wrote:
> I?ll look in to this,
> 
>   Jarno
> 
> On Feb 19, 2015, at 9:22 AM, Ben Pfaff  wrote:
> 
> > On Thu, Feb 19, 2015 at 04:00:43AM -0800, Gurucharan Shetty wrote:
> >> On Thu, Feb 19, 2015 at 12:12 AM, Ben Pfaff  wrote:
> >>> A fair number of columns had been overlooked.  This documents them.
> >>> 
> >>> The patch is smaller than it appears because this rearranges the STP and
> >>> RSTP documentation to group configuration, status, and statistics together
> >>> in the documentation for clarity.
> >>> 
> >>> Signed-off-by: Ben Pfaff 
> >> I do not know RSTP to review whether more clarity can be provided on
> >> the newly added column documentation. If no one else reviews,
> >> Acked-by: Gurucharan Shetty 
> > 
> > Jarno, I think you know RSTP better than most here.  Can you provide
> > any better documentation for the newly documented columns here?
> > 
> > 
> >>> ---
> >>> vswitchd/vswitch.xml | 680 
> >>> +--
> >>> 1 file changed, 391 insertions(+), 289 deletions(-)
> >>> 
> >>> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> >>> index 932e4b2..04de3ca 100644
> >>> --- a/vswitchd/vswitch.xml
> >>> +++ b/vswitchd/vswitch.xml
> >>> @@ -681,80 +681,232 @@
> >>> 
> >>> 
> >>> 
> >>> -  The IEEE 802.1D Spanning Tree Protocol (STP) is a network protocol
> >>> -  that ensures loop-free topologies.  It allows redundant links to
> >>> -  be included in the network to provide automatic backup paths if
> >>> -  the active links fails.
> >>> +  
> >>> +The IEEE 802.1D Spanning Tree Protocol (STP) is a network 
> >>> protocol
> >>> +that ensures loop-free topologies.  It allows redundant links to
> >>> +be included in the network to provide automatic backup paths if
> >>> +the active links fails.
> >>> +  
> >>> 
> >>> -  
> >>> -Enable spanning tree on the bridge.  By default, STP is disabled
> >>> -on bridges.  Bond, internal, and mirror ports are not supported
> >>> -and will not participate in the spanning tree.
> >>> -  
> >>> +  
> >>> +These settings configure the slower-to-converge but still widely
> >>> +supported version of Spanning Tree Protocol, sometimes known as
> >>> +802.1D-1998.  Open vSwitch also supports the newer Rapid 
> >>> Spanning Tree
> >>> +Protocol (RSTP), documented later in the section titled 
> >>> Rapid
> >>> +Spanning Tree Configuration.
> >>> +  
> >>> 
> >>> -  
> >>> -The bridge's STP identifier (the lower 48 bits of the bridge-id)
> >>> -in the form
> >>> -
> >>> xx:xx:xx:xx:xx:xx.
> >>> -By default, the identifier is the MAC address of the bridge.
> >>> -  
> >>> +  
> >>> +
> >>> +  
> >>> +Enable spanning tree on the bridge.  By default, STP is 
> >>> disabled
> >>> +on bridges.  Bond, internal, and mirror ports are not 
> >>> supported
> >>> +and will not participate in the spanning tree.
> >>> +  
> >>> 
> >>> -   >>> -  type='{"type": "integer", "minInteger": 0, "maxInteger": 
> >>> 65535}'>
> >>> -The bridge's relative priority value for determining the root
> >>> -bridge (the upper 16 bits of the bridge-id).  A bridge with the
> >>> -lowest bridge-id is elected the root.  By default, the priority
> >>> -is 0x8000.
> >>> -  
> >>> +  
> >>> +STP and RSTP are mutually exclusive.  If both are enabled, 
> >>> RSTP
> >>> +will be used.
> >>> +  
> >>> +
> >>> 
> >>> -   >>> -  type='{"type": "integer", "minInteger": 1, "maxInteger": 
> >>> 10}'>
> >>> -The interval between transmissions of hello messages by
> >>> -designated ports, in seconds.  By default the hello interval is
> >>> -2 seconds.
> >>> -  
> >>> +
> >>> +  The bridge's STP identifier (the lower 48 bits of the 
> >>> bridge-id)
> >>> +  in the form
> >>> +  
> >>> xx:xx:xx:xx:xx:xx.
> >>> +  By default, the identifier is the MAC address of the bridge.
> >>> +
> >>> 
> >>> -   >>> -  type='{"type": "integer", "minInteger": 6, "maxInteger": 
> >>> 40}'>
> >>> -The maximum age of the information transmitted by the bridge
> >>> -when it is the root bridge, in seconds.  By default, the maximum
> >>> -age is 20 seconds.
> >>> -  
> >>> + >>> +type='{"type": "integer", "minInteger": 0, "maxInteger": 
> >>> 65535}'>
> >>> +  The bridge's relative priority value for determining the root
> >>> +  bridge (the upper 16 bi

Re: [ovs-dev] [PATCH 3/8] vtep: Document the ipaddr column in the Mcast_Macs_Local table.

2015-02-19 Thread Ben Pfaff
On Thu, Feb 19, 2015 at 02:57:44AM -0800, Gurucharan Shetty wrote:
> On Thu, Feb 19, 2015 at 12:12 AM, Ben Pfaff  wrote:
> > This had been overlooked.
> >
> > Signed-off-by: Ben Pfaff 
> I did not about this column.
> Acked-by: Gurucharan Shetty 

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


Re: [ovs-dev] [PATCH 4/8] ovsdb-doc: Flag an error when a table or a column is left undocumented.

2015-02-19 Thread Ben Pfaff
On Thu, Feb 19, 2015 at 03:14:25AM -0800, Gurucharan Shetty wrote:
> On Thu, Feb 19, 2015 at 12:12 AM, Ben Pfaff  wrote:
> > This should make it harder to forget documentation.
> >
> > Signed-off-by: Ben Pfaff 
> Nice!
> Acked-by: Gurucharan Shetty 

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


Re: [ovs-dev] [PATCH 6/8] ovsdb-doc: Factor out nroff formatting into a separate Python module.

2015-02-19 Thread Ben Pfaff
On Thu, Feb 19, 2015 at 09:23:55AM -0800, Ben Pfaff wrote:
> On Thu, Feb 19, 2015 at 09:12:07AM -0800, Gurucharan Shetty wrote:
> > On Thu, Feb 19, 2015 at 9:09 AM, Ben Pfaff  wrote:
> > > On Thu, Feb 19, 2015 at 11:00:56AM -0500, Russell Bryant wrote:
> > >> On 02/19/2015 07:02 AM, Gurucharan Shetty wrote:
> > >> > I don't understand nroff well enough to review patch 6 and patch 7. If
> > >> > anyone else is up for it, please review.
> > >>
> > >> I don't either, but this one looks like just a code move.  It looks good
> > >> to me.
> > >
> > > That's right, this is just code movement.
> > It did look like you added an additional function while code
> > refactoring. Maybe I did not look carefully.
> 
> Oh, looking again, I added support for a  element.  I didn't end
> up using that later in the series, so I'm happy to drop it.

Here it is without the added  element support:

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

From: Ben Pfaff 
Date: Thu, 19 Feb 2015 10:59:12 -0800
Subject: [PATCH] ovsdb-doc: Factor out nroff formatting into a separate
 Python module.

This will make it cleaner to add another build-time program that generates
nroff from XML.

Signed-off-by: Ben Pfaff 
---
 ovsdb/ovsdb-doc   |  135 ++--
 python/automake.mk|6 ++
 python/build/nroff.py |  185 +
 3 files changed, 195 insertions(+), 131 deletions(-)
 create mode 100644 python/build/__init__.py
 create mode 100644 python/build/nroff.py

diff --git a/ovsdb/ovsdb-doc b/ovsdb/ovsdb-doc
index 15618ef..369efa9 100755
--- a/ovsdb/ovsdb-doc
+++ b/ovsdb/ovsdb-doc
@@ -3,7 +3,6 @@
 from datetime import date
 import getopt
 import os
-import re
 import sys
 import xml.dom.minidom
 
@@ -11,136 +10,9 @@ import ovs.json
 from ovs.db import error
 import ovs.db.schema
 
-argv0 = sys.argv[0]
-
-def textToNroff(s, font=r'\fR'):
-def escape(match):
-c = match.group(0)
-if c.startswith('-'):
-if c != '-' or font == r'\fB':
-return '\\' + c
-else:
-return '-'
-if c == '\\':
-return r'\e'
-elif c == '"':
-return r'\(dq'
-elif c == "'":
-return r'\(cq'
-else:
-raise error.Error("bad escape")
+from build.nroff import *
 
-# Escape - \ " ' as needed by nroff.
-s = re.sub('(-[0-9]|[-"\'])', escape, s)
-if s.startswith('.'):
-s = '\\' + s
-return s
-
-def escapeNroffLiteral(s):
-return r'\fB%s\fR' % textToNroff(s, r'\fB')
-
-def inlineXmlToNroff(node, font):
-if node.nodeType == node.TEXT_NODE:
-return textToNroff(node.data, font)
-elif node.nodeType == node.ELEMENT_NODE:
-if node.tagName in ['code', 'em', 'option']:
-s = r'\fB'
-for child in node.childNodes:
-s += inlineXmlToNroff(child, r'\fB')
-return s + font
-elif node.tagName == 'ref':
-s = r'\fB'
-if node.hasAttribute('column'):
-s += node.attributes['column'].nodeValue
-if node.hasAttribute('key'):
-s += ':' + node.attributes['key'].nodeValue
-elif node.hasAttribute('table'):
-s += node.attributes['table'].nodeValue
-elif node.hasAttribute('group'):
-s += node.attributes['group'].nodeValue
-else:
-raise error.Error("'ref' lacks required attributes: %s" % 
node.attributes.keys())
-return s + font
-elif node.tagName == 'var':
-s = r'\fI'
-for child in node.childNodes:
-s += inlineXmlToNroff(child, r'\fI')
-return s + font
-else:
-raise error.Error("element <%s> unknown or invalid here" % 
node.tagName)
-else:
-raise error.Error("unknown node %s in inline xml" % node)
-
-def blockXmlToNroff(nodes, para='.PP'):
-s = ''
-for node in nodes:
-if node.nodeType == node.TEXT_NODE:
-s += textToNroff(node.data)
-s = s.lstrip()
-elif node.nodeType == node.ELEMENT_NODE:
-if node.tagName in ['ul', 'ol']:
-if s != "":
-s += "\n"
-s += ".RS\n"
-i = 0
-for liNode in node.childNodes:
-if (liNode.nodeType == node.ELEMENT_NODE
-and liNode.tagName == 'li'):
-i += 1
-if node.tagName == 'ul':
-s += ".IP \\(bu\n"
-else:
-s += ".IP %d. .25in\n" % i
-s += blockXmlToNroff(liNode.childNodes, ".IP")
-elif (liNode.nodeType != node.TEXT_NODE
-  or not liNode.data.isspace()):
-raise error.Error("

Re: [ovs-dev] [PATCH 5/8] ovsdb-doc: Get manpage name from the XML file instead of command line.

2015-02-19 Thread Ben Pfaff
On Thu, Feb 19, 2015 at 03:25:15AM -0800, Gurucharan Shetty wrote:
> On Thu, Feb 19, 2015 at 12:12 AM, Ben Pfaff  wrote:
> > This seems like a better place for it.
> >
> > Signed-off-by: Ben Pfaff 
> Acked-by: Gurucharan Shetty 

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


Re: [ovs-dev] [PATCH 6/8] ovsdb-doc: Factor out nroff formatting into a separate Python module.

2015-02-19 Thread Gurucharan Shetty
On Thu, Feb 19, 2015 at 9:23 AM, Ben Pfaff  wrote:
> On Thu, Feb 19, 2015 at 09:12:07AM -0800, Gurucharan Shetty wrote:
>> On Thu, Feb 19, 2015 at 9:09 AM, Ben Pfaff  wrote:
>> > On Thu, Feb 19, 2015 at 11:00:56AM -0500, Russell Bryant wrote:
>> >> On 02/19/2015 07:02 AM, Gurucharan Shetty wrote:
>> >> > I don't understand nroff well enough to review patch 6 and patch 7. If
>> >> > anyone else is up for it, please review.
>> >>
>> >> I don't either, but this one looks like just a code move.  It looks good
>> >> to me.
>> >
>> > That's right, this is just code movement.
>> It did look like you added an additional function while code
>> refactoring. Maybe I did not look carefully.
>
> Oh, looking again, I added support for a  element.  I didn't end
> up using that later in the series, so I'm happy to drop it.

I am fine with the change. Thanks!
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 7/8] xml2nroff: New program to generate a manpage from XML input.

2015-02-19 Thread Ben Pfaff
On Thu, Feb 19, 2015 at 12:24:42PM -0500, Russell Bryant wrote:
> On 02/19/2015 03:12 AM, Ben Pfaff wrote:
> > I really can't stand nroff syntax.  This makes it possible to install
> > nroff but write in a more sensible XML syntax.
> > 
> > The following commit adds the first user.
> > 
> > Signed-off-by: Ben Pfaff 
> > ---
> >  Makefile.am |   1 +
> >  build-aux/xml2nroff | 110 
> > 
> >  2 files changed, 111 insertions(+)
> >  create mode 100755 build-aux/xml2nroff
> > 
> > diff --git a/Makefile.am b/Makefile.am
> > index 28496b3..0480d20 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -104,6 +104,7 @@ EXTRA_DIST = \
> > build-aux/dist-docs \
> > build-aux/sodepends.pl \
> > build-aux/soexpand.pl \
> > +   build-aux/xml2nroff \
> > $(MAN_FRAGMENTS) \
> > $(MAN_ROOTS) \
> > Vagrantfile
> > diff --git a/build-aux/xml2nroff b/build-aux/xml2nroff
> > new file mode 100755
> > index 000..817abc6
> > --- /dev/null
> > +++ b/build-aux/xml2nroff
> > @@ -0,0 +1,110 @@
> > +#! /usr/bin/python
> 
> A license header would be good here.

Oops, thanks for pointing that out.  I started from ovsdb-doc, so I
grabbed copyright dates from ovsdb-doc's history (ovsdb-doc lacked a
license header; I'll fix that separately).

> > +if __name__ == "__main__":
> > +try:
> 
> Instead of the double nesting here, I would move the outer one to just
> around the part that could raise error.Error, which appears to be only
> the call to manpage_to_nroff().

Done, thanks.

> > +try:
> > +options, args = getopt.gnu_getopt(sys.argv[1:], 'hV',
> > +  ['version=', 'help'])
> > +except getopt.GetoptError, geo:
> > +sys.stderr.write("%s: %s\n" % (argv0, geo.msg))
> > +sys.exit(1)
> > +
> > +er_diagram = None
> > +title = None
> > +version = None
> > +for key, value in options:
> > +if key == '--version':
> > +version = value
> > +elif key in ['-h', '--help']:
> > +usage()
> > +else:
> > +sys.exit(0)
> > +
> > +if len(args) != 1:
> > +sys.stderr.write("%s: exactly 1 non-option arguments required "
> > + "(use --help for help)\n" % argv0)
> > +sys.exit(1)
> > +
> > +s = manpage_to_nroff(args[0], version)
> > +for line in s.split("\n"):
> 
> can also use:
> 
> for line in s.splitlines():
> ...

Thanks, changed.

> > +line = line.strip()
> > +if len(line):
> 
> also equivalent:
> 
> if line:

Thanks, changed.

> > +print line
> > +
> > +except error.Error, e:
> > +sys.stderr.write("%s: %s\n" % (argv0, e.msg))
> > +sys.exit(1)
> > +
> > +# Local variables:
> > +# mode: python
> > +# End:

I'll repost the patch separately; here's an incremental:

diff --git a/build-aux/xml2nroff b/build-aux/xml2nroff
index 817abc6..8dc9d4f 100755
--- a/build-aux/xml2nroff
+++ b/build-aux/xml2nroff
@@ -1,5 +1,19 @@
 #! /usr/bin/python
 
+# Copyright (c) 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at:
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
 from datetime import date
 import getopt
 import os
@@ -58,11 +72,10 @@ def manpage_to_nroff(xml_file, version=None):
 
 def usage():
 print """\
-%(argv0)s: ovsdb schema documentation generator
-Prints documentation for an OVSDB schema as an nroff-formatted manpage.
-usage: %(argv0)s [OPTIONS] SCHEMA XML
-where SCHEMA is an OVSDB schema in JSON format
-  and XML is OVSDB documentation in XML format.
+%(argv0)s: converts XML in a somewhat HTML-like format to nroff
+usage: %(argv0)s [OPTIONS] XML
+where XML is documentation in a somewhat HTML-like XML format.
+The manpage, in nroff "man" format, is output on stdout.
 
 The following options are also available:
   --version=VERSION   use VERSION to display on document footer
@@ -72,38 +85,38 @@ The following options are also available:
 
 if __name__ == "__main__":
 try:
-try:
-options, args = getopt.gnu_getopt(sys.argv[1:], 'hV',
-  ['version=', 'help'])
-except getopt.GetoptError, geo:
-sys.stderr.write("%s: %s\n" % (argv0, geo.msg))
-sys.exit(1)
-
-er_diagram = None
-title = None
-   

[ovs-dev] [PATCH] ovsdb-doc: Add license and copyright notice.

2015-02-19 Thread Ben Pfaff
The copyright dates are taken from "git log --follow ovsdb/ovsdb-doc",
considering only Nicira authors' changes.  (Only one change was from
a non-Nicira author anyhow.)

Signed-off-by: Ben Pfaff 
---
 ovsdb/ovsdb-doc |   14 ++
 1 file changed, 14 insertions(+)

diff --git a/ovsdb/ovsdb-doc b/ovsdb/ovsdb-doc
index 369efa9..51b022b 100755
--- a/ovsdb/ovsdb-doc
+++ b/ovsdb/ovsdb-doc
@@ -1,5 +1,19 @@
 #! /usr/bin/python
 
+# Copyright (c) 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at:
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
 from datetime import date
 import getopt
 import os
-- 
1.7.10.4

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


[ovs-dev] [PATCH 0/7] UFID feature backport from Linux.

2015-02-19 Thread Joe Stringer
This patch series backports the UFID changes from the Linux 3.19 tree to the
OVS tree.

Joe Stringer (5):
  datapath: Refactor ovs_nla_fill_match().
  datapath: Refactor ovs_flow_tbl_insert().
  datapath: Use sw_flow_key_range for key ranges.
  compat: Add genlmsg_parse() helper function.
  datapath: Add support for unique flow IDs.

Pravin B Shelar (2):
  datapath: Initialize unmasked key and uid len.
  datapath: Fix masked key serialization.

 acinclude.m4  |1 +
 datapath/datapath.c   |  226 ++---
 datapath/flow.h   |   28 ++-
 datapath/flow_netlink.c   |  102 +-
 datapath/flow_netlink.h   |   13 +-
 datapath/flow_table.c |  222 +++-
 datapath/flow_table.h |8 +-
 datapath/linux/compat/include/linux/openvswitch.h |   15 +-
 datapath/linux/compat/include/net/genetlink.h |   11 +
 9 files changed, 487 insertions(+), 139 deletions(-)

-- 
1.7.10.4

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


[ovs-dev] [PATCH 3/7] datapath: Use sw_flow_key_range for key ranges.

2015-02-19 Thread Joe Stringer
These minor tidyups make a future patch a little tidier.

Signed-off-by: Joe Stringer 
Acked-by: Pravin B Shelar 
Signed-off-by: David S. Miller 
---
 datapath/flow_table.c |   20 +---
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/datapath/flow_table.c b/datapath/flow_table.c
index a7157e5..a038077 100644
--- a/datapath/flow_table.c
+++ b/datapath/flow_table.c
@@ -439,9 +439,11 @@ int ovs_flow_tbl_flush(struct flow_table *flow_table)
return 0;
 }
 
-static u32 flow_hash(const struct sw_flow_key *key, int key_start,
-int key_end)
+static u32 flow_hash(const struct sw_flow_key *key,
+const struct sw_flow_key_range *range)
 {
+   int key_start = range->start;
+   int key_end = range->end;
const u32 *hash_key = (const u32 *)((const u8 *)key + key_start);
int hash_u32s = (key_end - key_start) >> 2;
 
@@ -477,9 +479,9 @@ static bool cmp_key(const struct sw_flow_key *key1,
 
 static bool flow_cmp_masked_key(const struct sw_flow *flow,
const struct sw_flow_key *key,
-   int key_start, int key_end)
+   const struct sw_flow_key_range *range)
 {
-   return cmp_key(&flow->key, key, key_start, key_end);
+   return cmp_key(&flow->key, key, range->key_start, range->key_end);
 }
 
 bool ovs_flow_cmp_unmasked_key(const struct sw_flow *flow,
@@ -499,19 +501,16 @@ static struct sw_flow *masked_flow_lookup(struct 
table_instance *ti,
 {
struct sw_flow *flow;
struct hlist_head *head;
-   int key_start = mask->range.start;
-   int key_end = mask->range.end;
u32 hash;
struct sw_flow_key masked_key;
 
ovs_flow_mask_key(&masked_key, unmasked, mask);
-   hash = flow_hash(&masked_key, key_start, key_end);
+   hash = flow_hash(&masked_key, &mask->range);
head = find_bucket(ti, hash);
(*n_mask_hit)++;
hlist_for_each_entry_rcu(flow, head, hash_node[ti->node_ver]) {
if (flow->mask == mask && flow->hash == hash &&
-   flow_cmp_masked_key(flow, &masked_key,
- key_start, key_end))
+   flow_cmp_masked_key(flow, &masked_key, &mask->range))
return flow;
}
return NULL;
@@ -827,8 +826,7 @@ static void flow_key_insert(struct flow_table *table, 
struct sw_flow *flow)
struct table_instance *new_ti = NULL;
struct table_instance *ti;
 
-   flow->hash = flow_hash(&flow->key, flow->mask->range.start,
-   flow->mask->range.end);
+   flow->hash = flow_hash(&flow->key, &flow->mask->range);
ti = ovsl_dereference(table->ti);
table_instance_insert(ti, flow);
table->count++;
-- 
1.7.10.4

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


[ovs-dev] [PATCH 1/7] datapath: Refactor ovs_nla_fill_match().

2015-02-19 Thread Joe Stringer
Refactor the ovs_nla_fill_match() function into separate netlink
serialization functions ovs_nla_put_{unmasked_key,mask}(). Modify
ovs_nla_put_flow() to handle attribute nesting and expose the 'is_mask'
parameter - all callers need to nest the flow, and callers have better
knowledge about whether it is serializing a mask or not.

Signed-off-by: Joe Stringer 
Acked-by: Pravin B Shelar 
Signed-off-by: David S. Miller 
---
 datapath/datapath.c |   41 ++---
 datapath/flow_netlink.c |   38 +++---
 datapath/flow_netlink.h |7 +--
 3 files changed, 46 insertions(+), 40 deletions(-)

diff --git a/datapath/datapath.c b/datapath/datapath.c
index c5c926b..9cf9c37 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -466,10 +466,8 @@ static int queue_userspace_packet(struct datapath *dp, 
struct sk_buff *skb,
 0, upcall_info->cmd);
upcall->dp_ifindex = dp_ifindex;
 
-   nla = nla_nest_start(user_skb, OVS_PACKET_ATTR_KEY);
-   err = ovs_nla_put_flow(key, key, user_skb);
+   err = ovs_nla_put_key(key, key, OVS_PACKET_ATTR_KEY, false, user_skb);
BUG_ON(err);
-   nla_nest_end(user_skb, nla);
 
if (upcall_info->userdata)
__nla_put(user_skb, OVS_PACKET_ATTR_USERDATA,
@@ -683,37 +681,6 @@ static size_t ovs_flow_cmd_msg_size(const struct 
sw_flow_actions *acts)
 }
 
 /* Called with ovs_mutex or RCU read lock. */
-static int ovs_flow_cmd_fill_match(const struct sw_flow *flow,
-  struct sk_buff *skb)
-{
-   struct nlattr *nla;
-   int err;
-
-   /* Fill flow key. */
-   nla = nla_nest_start(skb, OVS_FLOW_ATTR_KEY);
-   if (!nla)
-   return -EMSGSIZE;
-
-   err = ovs_nla_put_flow(&flow->unmasked_key, &flow->unmasked_key, skb);
-   if (err)
-   return err;
-
-   nla_nest_end(skb, nla);
-
-   /* Fill flow mask. */
-   nla = nla_nest_start(skb, OVS_FLOW_ATTR_MASK);
-   if (!nla)
-   return -EMSGSIZE;
-
-   err = ovs_nla_put_flow(&flow->key, &flow->mask->key, skb);
-   if (err)
-   return err;
-
-   nla_nest_end(skb, nla);
-   return 0;
-}
-
-/* Called with ovs_mutex or RCU read lock. */
 static int ovs_flow_cmd_fill_stats(const struct sw_flow *flow,
   struct sk_buff *skb)
 {
@@ -794,7 +761,11 @@ static int ovs_flow_cmd_fill_info(const struct sw_flow 
*flow, int dp_ifindex,
 
ovs_header->dp_ifindex = dp_ifindex;
 
-   err = ovs_flow_cmd_fill_match(flow, skb);
+   err = ovs_nla_put_unmasked_key(flow, skb);
+   if (err)
+   goto error;
+
+   err = ovs_nla_put_mask(flow, skb);
if (err)
goto error;
 
diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
index 34edcfe..5f1b55e 100644
--- a/datapath/flow_netlink.c
+++ b/datapath/flow_netlink.c
@@ -1215,12 +1215,12 @@ int ovs_nla_get_flow_metadata(const struct nlattr *attr,
return metadata_from_nlattrs(&match, &attrs, a, false, log);
 }
 
-int ovs_nla_put_flow(const struct sw_flow_key *swkey,
-const struct sw_flow_key *output, struct sk_buff *skb)
+static int __ovs_nla_put_key(const struct sw_flow_key *swkey,
+const struct sw_flow_key *output, bool is_mask,
+struct sk_buff *skb)
 {
struct ovs_key_ethernet *eth_key;
struct nlattr *nla, *encap;
-   bool is_mask = (swkey != output);
 
if (nla_put_u32(skb, OVS_KEY_ATTR_RECIRC_ID, output->recirc_id))
goto nla_put_failure;
@@ -1430,6 +1430,38 @@ nla_put_failure:
return -EMSGSIZE;
 }
 
+int ovs_nla_put_key(const struct sw_flow_key *swkey,
+   const struct sw_flow_key *output, int attr, bool is_mask,
+   struct sk_buff *skb)
+{
+   int err;
+   struct nlattr *nla;
+
+   nla = nla_nest_start(skb, attr);
+   if (!nla)
+   return -EMSGSIZE;
+   err = __ovs_nla_put_key(swkey, output, is_mask, skb);
+   if (err)
+   return err;
+   nla_nest_end(skb, nla);
+
+   return 0;
+}
+
+/* Called with ovs_mutex or RCU read lock. */
+int ovs_nla_put_unmasked_key(const struct sw_flow *flow, struct sk_buff *skb)
+{
+   return ovs_nla_put_key(&flow->unmasked_key, &flow->unmasked_key,
+   OVS_FLOW_ATTR_KEY, false, skb);
+}
+
+/* Called with ovs_mutex or RCU read lock. */
+int ovs_nla_put_mask(const struct sw_flow *flow, struct sk_buff *skb)
+{
+   return ovs_nla_put_key(&flow->key, &flow->mask->key,
+   OVS_FLOW_ATTR_MASK, true, skb);
+}
+
 #define MAX_ACTIONS_BUFSIZE(32 * 1024)
 
 static struct sw_flow_actions *nla_alloc_flow_actions(int size, bool log)
diff --git a/datapath/flow_netlink.h b/datapath/flow_netlink.h
index 577f12b..9ed09e6 100644
--- a/datapath/flow_netlink.h

[ovs-dev] [PATCH 4/7] compat: Add genlmsg_parse() helper function.

2015-02-19 Thread Joe Stringer
The first user will be the next patch.

Signed-off-by: Joe Stringer 
Acked-by: Pravin B Shelar 
Signed-off-by: David S. Miller 
---
 acinclude.m4  |1 +
 datapath/linux/compat/include/net/genetlink.h |   11 +++
 2 files changed, 12 insertions(+)

diff --git a/acinclude.m4 b/acinclude.m4
index c2f45ce..88badf1 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -364,6 +364,7 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
   OVS_GREP_IFELSE([$KSRC/include/net/genetlink.h], [genlmsg_new_unicast])
   OVS_GREP_IFELSE([$KSRC/include/net/genetlink.h], 
[netlink_has_listeners(net->genl_sock],
   [OVS_DEFINE([HAVE_GENL_HAS_LISTENERS_TAKES_NET])])
+  OVS_GREP_IFELSE([$KSRC/include/net/genetlink.h], [genlmsg_parse])
   OVS_GREP_IFELSE([$KSRC/include/net/gre.h], [gre_cisco_register])
   OVS_GREP_IFELSE([$KSRC/include/net/gre.h], [gre_handle_offloads])
   OVS_GREP_IFELSE([$KSRC/include/net/ip_tunnels.h], [iptunnel_xmit.*net],
diff --git a/datapath/linux/compat/include/net/genetlink.h 
b/datapath/linux/compat/include/net/genetlink.h
index 8d1b89e..9edfd31 100644
--- a/datapath/linux/compat/include/net/genetlink.h
+++ b/datapath/linux/compat/include/net/genetlink.h
@@ -130,4 +130,15 @@ static inline int rpl_genl_has_listeners(struct 
genl_family *family,
 
 #endif /* HAVE_GENL_HAS_LISTENERS */
 
+#ifndef HAVE_GENLMSG_PARSE
+static inline int genlmsg_parse(const struct nlmsghdr *nlh,
+   const struct genl_family *family,
+   struct nlattr *tb[], int maxtype,
+   const struct nla_policy *policy)
+{
+   return nlmsg_parse(nlh, family->hdrsize + GENL_HDRLEN, tb, maxtype,
+  policy);
+}
+#endif
+
 #endif /* genetlink.h */
-- 
1.7.10.4

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


[ovs-dev] [PATCH 2/7] datapath: Refactor ovs_flow_tbl_insert().

2015-02-19 Thread Joe Stringer
Rework so that ovs_flow_tbl_insert() calls flow_{key,mask}_insert().
This tidies up a future patch.

Signed-off-by: Joe Stringer 
Acked-by: Pravin B Shelar 
Signed-off-by: David S. Miller 
---
 datapath/flow_table.c |   21 ++---
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/datapath/flow_table.c b/datapath/flow_table.c
index 8dad4c2..a7157e5 100644
--- a/datapath/flow_table.c
+++ b/datapath/flow_table.c
@@ -822,16 +822,10 @@ static int flow_mask_insert(struct flow_table *tbl, 
struct sw_flow *flow,
 }
 
 /* Must be called with OVS mutex held. */
-int ovs_flow_tbl_insert(struct flow_table *table, struct sw_flow *flow,
-   const struct sw_flow_mask *mask)
+static void flow_key_insert(struct flow_table *table, struct sw_flow *flow)
 {
struct table_instance *new_ti = NULL;
struct table_instance *ti;
-   int err;
-
-   err = flow_mask_insert(table, flow, mask);
-   if (err)
-   return err;
 
flow->hash = flow_hash(&flow->key, flow->mask->range.start,
flow->mask->range.end);
@@ -850,6 +844,19 @@ int ovs_flow_tbl_insert(struct flow_table *table, struct 
sw_flow *flow,
table_instance_destroy(ti, true);
table->last_rehash = jiffies;
}
+}
+
+/* Must be called with OVS mutex held. */
+int ovs_flow_tbl_insert(struct flow_table *table, struct sw_flow *flow,
+   const struct sw_flow_mask *mask)
+{
+   int err;
+
+   err = flow_mask_insert(table, flow, mask);
+   if (err)
+   return err;
+   flow_key_insert(table, flow);
+
return 0;
 }
 
-- 
1.7.10.4

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


[ovs-dev] [PATCH 7/7] datapath: Fix masked key serialization.

2015-02-19 Thread Joe Stringer
From: Pravin B Shelar 

Fix typo where mask is used rather than key.

Fixes: 74ed7ab9264("openvswitch: Add support for unique flow IDs.")
Reported-by: Joe Stringer 
Signed-off-by: Pravin B Shelar 
---
 datapath/flow_netlink.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
index 732434a..8772d04 100644
--- a/datapath/flow_netlink.c
+++ b/datapath/flow_netlink.c
@@ -1515,7 +1515,7 @@ int ovs_nla_put_identifier(const struct sw_flow *flow, 
struct sk_buff *skb)
 /* Called with ovs_mutex or RCU read lock. */
 int ovs_nla_put_masked_key(const struct sw_flow *flow, struct sk_buff *skb)
 {
-   return ovs_nla_put_key(&flow->mask->key, &flow->key,
+   return ovs_nla_put_key(&flow->key, &flow->key,
OVS_FLOW_ATTR_KEY, false, skb);
 }
 
-- 
1.7.10.4

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


[ovs-dev] [PATCH 5/7] datapath: Add support for unique flow IDs.

2015-02-19 Thread Joe Stringer
Previously, flows were manipulated by userspace specifying a full,
unmasked flow key. This adds significant burden onto flow
serialization/deserialization, particularly when dumping flows.

This patch adds an alternative way to refer to flows using a
variable-length "unique flow identifier" (UFID). At flow setup time,
userspace may specify a UFID for a flow, which is stored with the flow
and inserted into a separate table for lookup, in addition to the
standard flow table. Flows created using a UFID must be fetched or
deleted using the UFID.

All flow dump operations may now be made more terse with OVS_UFID_F_*
flags. For example, the OVS_UFID_F_OMIT_KEY flag allows responses to
omit the flow key from a datapath operation if the flow has a
corresponding UFID. This significantly reduces the time spent assembling
and transacting netlink messages. With all OVS_UFID_F_OMIT_* flags
enabled, the datapath only returns the UFID and statistics for each flow
during flow dump, increasing ovs-vswitchd revalidator performance by 40%
or more.

Signed-off-by: Joe Stringer 
Acked-by: Pravin B Shelar 
Signed-off-by: David S. Miller 
---
 datapath/datapath.c   |  207 -
 datapath/flow.h   |   28 ++-
 datapath/flow_netlink.c   |   68 ++-
 datapath/flow_netlink.h   |8 +-
 datapath/flow_table.c |  183 ++
 datapath/flow_table.h |8 +-
 datapath/linux/compat/include/linux/openvswitch.h |   15 +-
 7 files changed, 420 insertions(+), 97 deletions(-)

diff --git a/datapath/datapath.c b/datapath/datapath.c
index 9cf9c37..f386148 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -66,6 +66,8 @@ static struct genl_family dp_packet_genl_family;
 static struct genl_family dp_flow_genl_family;
 static struct genl_family dp_datapath_genl_family;
 
+static const struct nla_policy flow_policy[];
+
 static struct genl_multicast_group ovs_dp_flow_multicast_group = {
.name = OVS_FLOW_MCGROUP
 };
@@ -669,15 +671,48 @@ static void get_dp_stats(const struct datapath *dp, 
struct ovs_dp_stats *stats,
}
 }
 
-static size_t ovs_flow_cmd_msg_size(const struct sw_flow_actions *acts)
+static bool should_fill_key(const struct sw_flow_id *sfid, uint32_t ufid_flags)
+{
+   return ovs_identifier_is_ufid(sfid) &&
+  !(ufid_flags & OVS_UFID_F_OMIT_KEY);
+}
+
+static bool should_fill_mask(uint32_t ufid_flags)
+{
+   return !(ufid_flags & OVS_UFID_F_OMIT_MASK);
+}
+
+static bool should_fill_actions(uint32_t ufid_flags)
 {
-   return NLMSG_ALIGN(sizeof(struct ovs_header))
-   + nla_total_size(ovs_key_attr_size()) /* OVS_FLOW_ATTR_KEY */
-   + nla_total_size(ovs_key_attr_size()) /* OVS_FLOW_ATTR_MASK */
+   return !(ufid_flags & OVS_UFID_F_OMIT_ACTIONS);
+}
+
+static size_t ovs_flow_cmd_msg_size(const struct sw_flow_actions *acts,
+   const struct sw_flow_id *sfid,
+   uint32_t ufid_flags)
+{
+   size_t len = NLMSG_ALIGN(sizeof(struct ovs_header));
+
+   /* OVS_FLOW_ATTR_UFID */
+   if (sfid && ovs_identifier_is_ufid(sfid))
+   len += nla_total_size(sfid->ufid_len);
+
+   /* OVS_FLOW_ATTR_KEY */
+   if (!sfid || should_fill_key(sfid, ufid_flags))
+   len += nla_total_size(ovs_key_attr_size());
+
+   /* OVS_FLOW_ATTR_MASK */
+   if (should_fill_mask(ufid_flags))
+   len += nla_total_size(ovs_key_attr_size());
+
+   /* OVS_FLOW_ATTR_ACTIONS */
+   if (should_fill_actions(ufid_flags))
+   len += nla_total_size(acts->actions_len);
+
+   return len
+ nla_total_size(sizeof(struct ovs_flow_stats)) /* 
OVS_FLOW_ATTR_STATS */
+ nla_total_size(1) /* OVS_FLOW_ATTR_TCP_FLAGS */
-   + nla_total_size(8) /* OVS_FLOW_ATTR_USED */
-   + nla_total_size(acts->actions_len); /* OVS_FLOW_ATTR_ACTIONS */
+   + nla_total_size(8); /* OVS_FLOW_ATTR_USED */
 }
 
 /* Called with ovs_mutex or RCU read lock. */
@@ -748,7 +783,7 @@ static int ovs_flow_cmd_fill_actions(const struct sw_flow 
*flow,
 /* Called with ovs_mutex or RCU read lock. */
 static int ovs_flow_cmd_fill_info(const struct sw_flow *flow, int dp_ifindex,
  struct sk_buff *skb, u32 portid,
- u32 seq, u32 flags, u8 cmd)
+ u32 seq, u32 flags, u8 cmd, u32 ufid_flags)
 {
const int skb_orig_len = skb->len;
struct ovs_header *ovs_header;
@@ -761,21 +796,31 @@ static int ovs_flow_cmd_fill_info(const struct sw_flow 
*flow, int dp_ifindex,
 
ovs_header->dp_ifindex = dp_ifindex;
 
-   err = ovs_nla_put_unmasked_key(flow, skb);
+   err = ovs_nla_put_identifier(flow, skb);
if (err)
goto erro

[ovs-dev] [PATCH 6/7] datapath: Initialize unmasked key and uid len.

2015-02-19 Thread Joe Stringer
From: Pravin B Shelar 

Flow alloc needs to initialize unmasked key pointer. Otherwise
it can crash kernel trying to free random unmasked-key pointer.

general protection fault:  [#1] SMP
3.19.0-rc6-net-next+ #457
Hardware name: Supermicro X7DWU/X7DWU, BIOS  1.1 04/30/2008
RIP: 0010:[] [] kfree+0xac/0x196
Call Trace:
 [] flow_free+0x21/0x59 [openvswitch]
 [] ovs_flow_free+0x21/0x23 [openvswitch]
 [] ovs_packet_cmd_execute+0x2f3/0x35f [openvswitch]
 [] ? ovs_packet_cmd_execute+0x13e/0x35f [openvswitch]
 [] ? nla_parse+0x4f/0xec
 [] genl_family_rcv_msg+0x26d/0x2c9
 [] ? __lock_acquire+0x90e/0x9aa
 [] genl_rcv_msg+0x66/0x89
 [] ? genl_family_rcv_msg+0x2c9/0x2c9
 [] netlink_rcv_skb+0x3e/0x95
 [] ? genl_rcv+0x18/0x37
 [] genl_rcv+0x27/0x37
 [] netlink_unicast+0x103/0x191
 [] netlink_sendmsg+0x2c1/0x310
 [] ? might_fault+0x50/0xa0
 [] do_sock_sendmsg+0x5f/0x7a
 [] sock_sendmsg+0xb/0xd
 [] ___sys_sendmsg+0x1a3/0x218
 [] ? get_close_on_exec+0x86/0x86
 [] ? fsnotify+0x32c/0x348
 [] ? fsnotify+0x7c/0x348
 [] ? __fget+0xaa/0xbf
 [] ? get_close_on_exec+0x86/0x86
 [] __sys_sendmsg+0x3d/0x5e
 [] SyS_sendmsg+0x14/0x16
 [] system_call_fastpath+0x12/0x17

Reported-by: Or Gerlitz 
Signed-off-by: Pravin B Shelar 
---
 datapath/flow_table.c |2 ++
 1 file changed, 2 insertions(+)

diff --git a/datapath/flow_table.c b/datapath/flow_table.c
index 311e5ee..9a27bea 100644
--- a/datapath/flow_table.c
+++ b/datapath/flow_table.c
@@ -92,6 +92,8 @@ struct sw_flow *ovs_flow_alloc(void)
 
flow->sf_acts = NULL;
flow->mask = NULL;
+   flow->id.ufid_len = 0;
+   flow->id.unmasked_key = NULL;
flow->stats_last_writer = NUMA_NO_NODE;
 
/* Initialize the default stat node. */
-- 
1.7.10.4

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


Re: [ovs-dev] [PATCH 6/8] ovsdb-doc: Factor out nroff formatting into a separate Python module.

2015-02-19 Thread Ben Pfaff
On Thu, Feb 19, 2015 at 11:07:18AM -0800, Gurucharan Shetty wrote:
> On Thu, Feb 19, 2015 at 9:23 AM, Ben Pfaff  wrote:
> > On Thu, Feb 19, 2015 at 09:12:07AM -0800, Gurucharan Shetty wrote:
> >> On Thu, Feb 19, 2015 at 9:09 AM, Ben Pfaff  wrote:
> >> > On Thu, Feb 19, 2015 at 11:00:56AM -0500, Russell Bryant wrote:
> >> >> On 02/19/2015 07:02 AM, Gurucharan Shetty wrote:
> >> >> > I don't understand nroff well enough to review patch 6 and patch 7. If
> >> >> > anyone else is up for it, please review.
> >> >>
> >> >> I don't either, but this one looks like just a code move.  It looks good
> >> >> to me.
> >> >
> >> > That's right, this is just code movement.
> >> It did look like you added an additional function while code
> >> refactoring. Maybe I did not look carefully.
> >
> > Oh, looking again, I added support for a  element.  I didn't end
> > up using that later in the series, so I'm happy to drop it.
> 
> I am fine with the change. Thanks!

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


Re: [ovs-dev] [PATCH 7/8] xml2nroff: New program to generate a manpage from XML input.

2015-02-19 Thread Russell Bryant
On 02/19/2015 02:08 PM, Ben Pfaff wrote:
> I'll repost the patch separately; here's an incremental:

Incremental changes look good to me, thanks!

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


[ovs-dev] [PATCH v2 1/2] xml2nroff: New program to generate a manpage from XML input.

2015-02-19 Thread Ben Pfaff
I really can't stand nroff syntax.  This makes it possible to install
nroff but write in a more sensible XML syntax.

The following commit adds the first user.

Signed-off-by: Ben Pfaff 
---
 Makefile.am |1 +
 build-aux/xml2nroff |  123 +++
 2 files changed, 124 insertions(+)
 create mode 100755 build-aux/xml2nroff

diff --git a/Makefile.am b/Makefile.am
index 28496b3..0480d20 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -104,6 +104,7 @@ EXTRA_DIST = \
build-aux/dist-docs \
build-aux/sodepends.pl \
build-aux/soexpand.pl \
+   build-aux/xml2nroff \
$(MAN_FRAGMENTS) \
$(MAN_ROOTS) \
Vagrantfile
diff --git a/build-aux/xml2nroff b/build-aux/xml2nroff
new file mode 100755
index 000..8dc9d4f
--- /dev/null
+++ b/build-aux/xml2nroff
@@ -0,0 +1,123 @@
+#! /usr/bin/python
+
+# Copyright (c) 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at:
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from datetime import date
+import getopt
+import os
+import sys
+import xml.dom.minidom
+
+from build.nroff import *
+
+argv0 = sys.argv[0]
+
+def usage():
+print """\
+%(argv0)s: XML to nroff converter
+Converts the XML format supplied as input into an nroff-formatted manpage.
+usage: %(argv0)s [OPTIONS] INPUT.XML
+where INPUT.XML is a manpage in an OVS-specific XML format.
+
+The following options are also available:
+  --version=VERSION   use VERSION to display on document footer
+  -h, --help  display this help message\
+""" % {'argv0': argv0}
+sys.exit(0)
+
+def manpage_to_nroff(xml_file, version=None):
+doc = xml.dom.minidom.parse(xml_file).documentElement
+d = date.fromtimestamp(os.stat(xml_file).st_mtime)
+
+if version == None:
+version = "UNKNOWN"
+program = doc.attributes['program'].nodeValue
+title = doc.attributes['title'].nodeValue
+section = doc.attributes['section'].nodeValue
+
+# Putting '\" p as the first line tells "man" that the manpage
+# needs to be preprocessed by "pic".
+s = r\" p
+.\" -*- nroff -*-
+.TH "%s" %s "%s" "Open vSwitch %s" "Open vSwitch Manual"
+.fp 5 L CR  \\" Make fixed-width font available as \\fL.
+.de TQ
+.  br
+.  ns
+.  TP "\\$1"
+..
+.de ST
+.  PP
+.  RS -0.15in
+.  I "\\$1"
+.  RE
+..
+''' % (textToNroff(program), textToNroff(section), textToNroff(title), 
textToNroff(version))
+
+s += blockXmlToNroff(doc.childNodes) + "\n"
+
+return s
+
+def usage():
+print """\
+%(argv0)s: converts XML in a somewhat HTML-like format to nroff
+usage: %(argv0)s [OPTIONS] XML
+where XML is documentation in a somewhat HTML-like XML format.
+The manpage, in nroff "man" format, is output on stdout.
+
+The following options are also available:
+  --version=VERSION   use VERSION to display on document footer
+  -h, --help  display this help message\
+""" % {'argv0': argv0}
+sys.exit(0)
+
+if __name__ == "__main__":
+try:
+options, args = getopt.gnu_getopt(sys.argv[1:], 'hV',
+  ['version=', 'help'])
+except getopt.GetoptError, geo:
+sys.stderr.write("%s: %s\n" % (argv0, geo.msg))
+sys.exit(1)
+
+er_diagram = None
+title = None
+version = None
+for key, value in options:
+if key == '--version':
+version = value
+elif key in ['-h', '--help']:
+usage()
+else:
+sys.exit(0)
+
+if len(args) != 1:
+sys.stderr.write("%s: exactly 1 non-option arguments required "
+ "(use --help for help)\n" % argv0)
+sys.exit(1)
+
+try:
+s = manpage_to_nroff(args[0], version)
+except error.Error, e:
+sys.stderr.write("%s: %s\n" % (argv0, e.msg))
+sys.exit(1)
+for line in s.splitlines():
+line = line.strip()
+if line:
+print line
+
+
+# Local variables:
+# mode: python
+# End:
-- 
1.7.10.4

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


[ovs-dev] [PATCH v2 0/2] Initial OVN design documentation, take 2

2015-02-19 Thread Ben Pfaff
v1->v2:
  - Patches 1 through 6 applied and dropped.
  - Patch 7 revised according to Russell Bryant's suggestions.
  - Patch 8 is unchanged.

Ben Pfaff (2):
  xml2nroff: New program to generate a manpage from XML input.
  [RFC] ovn: Start work on design documentation.

 Makefile.am|2 +
 build-aux/xml2nroff|  123 
 configure.ac   |3 +-
 ovn/automake.mk|   75 +++
 ovn/ovn-architecture.7.xml |  331 ++
 ovn/ovn-controller.8.in|   41 
 ovn/ovn-nb.ovsschema   |   63 ++
 ovn/ovn-nb.xml |  225 +
 ovn/ovn.ovsschema  |   47 +
 ovn/ovn.xml|  475 
 10 files changed, 1384 insertions(+), 1 deletion(-)
 create mode 100755 build-aux/xml2nroff
 create mode 100644 ovn/automake.mk
 create mode 100644 ovn/ovn-architecture.7.xml
 create mode 100644 ovn/ovn-controller.8.in
 create mode 100644 ovn/ovn-nb.ovsschema
 create mode 100644 ovn/ovn-nb.xml
 create mode 100644 ovn/ovn.ovsschema
 create mode 100644 ovn/ovn.xml

-- 
1.7.10.4

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


[ovs-dev] [PATCH v2 2/2] [RFC] ovn: Start work on design documentation.

2015-02-19 Thread Ben Pfaff
This commit adds preliminary design documentation for Open Virtual Network,
or OVN, a new OVS-based project to add support for virtual networking to
OVS, initially with OpenStack integration.

This initial design has been influenced by many people, including (in
alphabetical order) Aaron Rosen, Chris Wright, Jeremy Stribling,
Justin Pettit, Ken Duda, Madhu Venugopal, Martin Casado, Pankaj Thakkar,
Russell Bryant, and Teemu Koponen.  All blunders, however, are due to my
own hubris.

Signed-off-by: Ben Pfaff 
---
 Makefile.am|1 +
 configure.ac   |3 +-
 ovn/automake.mk|   75 +++
 ovn/ovn-architecture.7.xml |  331 ++
 ovn/ovn-controller.8.in|   41 
 ovn/ovn-nb.ovsschema   |   63 ++
 ovn/ovn-nb.xml |  225 +
 ovn/ovn.ovsschema  |   47 +
 ovn/ovn.xml|  475 
 9 files changed, 1260 insertions(+), 1 deletion(-)
 create mode 100644 ovn/automake.mk
 create mode 100644 ovn/ovn-architecture.7.xml
 create mode 100644 ovn/ovn-controller.8.in
 create mode 100644 ovn/ovn-nb.ovsschema
 create mode 100644 ovn/ovn-nb.xml
 create mode 100644 ovn/ovn.ovsschema
 create mode 100644 ovn/ovn.xml

diff --git a/Makefile.am b/Makefile.am
index 0480d20..699a580 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -370,3 +370,4 @@ include tutorial/automake.mk
 include vtep/automake.mk
 include datapath-windows/automake.mk
 include datapath-windows/include/automake.mk
+include ovn/automake.mk
diff --git a/configure.ac b/configure.ac
index d2d02ca..795f876 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1,4 +1,4 @@
-# Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
+# Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -182,6 +182,7 @@ dnl This makes sure that include/openflow gets created in 
the build directory.
 AC_CONFIG_COMMANDS([include/openflow/openflow.h.stamp])
 
 AC_CONFIG_COMMANDS([utilities/bugtool/dummy], [:])
+AC_CONFIG_COMMANDS([ovn/dummy], [:])
 
 m4_ifdef([AM_SILENT_RULES], [AM_SILENT_RULES])
 
diff --git a/ovn/automake.mk b/ovn/automake.mk
new file mode 100644
index 000..3889d56
--- /dev/null
+++ b/ovn/automake.mk
@@ -0,0 +1,75 @@
+# OVN schema and IDL
+EXTRA_DIST += ovn/ovn.ovsschema
+pkgdata_DATA += ovn/ovn.ovsschema
+
+# OVN E-R diagram
+#
+# If "python" or "dot" is not available, then we do not add graphical diagram
+# to the documentation.
+if HAVE_PYTHON
+if HAVE_DOT
+ovn/ovn.gv: ovsdb/ovsdb-dot.in ovn/ovn.ovsschema
+   $(AM_V_GEN)$(OVSDB_DOT) --no-arrows $(srcdir)/ovn/ovn.ovsschema > $@
+ovn/ovn.pic: ovn/ovn.gv ovsdb/dot2pic
+   $(AM_V_GEN)(dot -T plain < ovn/ovn.gv | $(PERL) $(srcdir)/ovsdb/dot2pic 
-f 3) > $@.tmp && \
+   mv $@.tmp $@
+OVN_PIC = ovn/ovn.pic
+OVN_DOT_DIAGRAM_ARG = --er-diagram=$(OVN_PIC)
+DISTCLEANFILES += ovn/ovn.gv ovn/ovn.pic
+endif
+endif
+
+# OVN schema documentation
+EXTRA_DIST += ovn/ovn.xml
+DISTCLEANFILES += ovn/ovn.5
+man_MANS += ovn/ovn.5
+ovn/ovn.5: \
+   ovsdb/ovsdb-doc ovn/ovn.xml ovn/ovn.ovsschema $(OVN_PIC)
+   $(AM_V_GEN)$(OVSDB_DOC) \
+   $(OVN_DOT_DIAGRAM_ARG) \
+   --version=$(VERSION) \
+   $(srcdir)/ovn/ovn.ovsschema \
+   $(srcdir)/ovn/ovn.xml > $@.tmp && \
+   mv $@.tmp $@
+
+# OVN northbound schema and IDL
+EXTRA_DIST += ovn/ovn-nb.ovsschema
+pkgdata_DATA += ovn/ovn-nb.ovsschema
+
+# OVN northbound E-R diagram
+#
+# If "python" or "dot" is not available, then we do not add graphical diagram
+# to the documentation.
+if HAVE_PYTHON
+if HAVE_DOT
+ovn/ovn-nb.gv: ovsdb/ovsdb-dot.in ovn/ovn-nb.ovsschema
+   $(AM_V_GEN)$(OVSDB_DOT) --no-arrows $(srcdir)/ovn/ovn-nb.ovsschema > $@
+ovn/ovn-nb.pic: ovn/ovn-nb.gv ovsdb/dot2pic
+   $(AM_V_GEN)(dot -T plain < ovn/ovn-nb.gv | $(PERL) 
$(srcdir)/ovsdb/dot2pic -f 3) > $@.tmp && \
+   mv $@.tmp $@
+OVN_NB_PIC = ovn/ovn-nb.pic
+OVN_NB_DOT_DIAGRAM_ARG = --er-diagram=$(OVN_NB_PIC)
+DISTCLEANFILES += ovn/ovn-nb.gv ovn/ovn-nb.pic
+endif
+endif
+
+# OVN northbound schema documentation
+EXTRA_DIST += ovn/ovn-nb.xml
+DISTCLEANFILES += ovn/ovn-nb.5
+man_MANS += ovn/ovn-nb.5
+ovn/ovn-nb.5: \
+   ovsdb/ovsdb-doc ovn/ovn-nb.xml ovn/ovn-nb.ovsschema $(OVN_NB_PIC)
+   $(AM_V_GEN)$(OVSDB_DOC) \
+   $(OVN_NB_DOT_DIAGRAM_ARG) \
+   --version=$(VERSION) \
+   $(srcdir)/ovn/ovn-nb.ovsschema \
+   $(srcdir)/ovn/ovn-nb.xml > $@.tmp && \
+   mv $@.tmp $@
+
+man_MANS += ovn/ovn-controller.8 ovn/ovn-architecture.7
+EXTRA_DIST += ovn/ovn-controller.8.in ovn/ovn-architecture.7.xml
+
+SUFFIXES += .xml
+%: %.xml
+   $(AM_V_GEN)$(run_python) $(srcdir)/build-aux/xml2nroff \
+   --version=$(VERSION) $< > $@.tmp && mv $@.tmp $@
diff --git 

Re: [ovs-dev] [PATCH v2 1/2] xml2nroff: New program to generate a manpage from XML input.

2015-02-19 Thread Ben Pfaff
On Thu, Feb 19, 2015 at 11:16:12AM -0800, Ben Pfaff wrote:
> I really can't stand nroff syntax.  This makes it possible to install
> nroff but write in a more sensible XML syntax.
> 
> The following commit adds the first user.
> 
> Signed-off-by: Ben Pfaff 

Applied to master based on Russell Bryant's review in another thread.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 00/14] UDP tunneling backports

2015-02-19 Thread Pravin Shelar
On Wed, Feb 18, 2015 at 7:02 PM, Jesse Gross  wrote:
> This series backports much of the UDP tunnel infrastructure, ultimately
> in support of syncing Geneve with upstream and providing outer UDP
> checksum support.
>
> During the backporting process, I realized that there are several
> other lurking bugs that were fixed through a more complete UDP
> tunnel layer. The ones that I am aware of are:
>  * VXLAN TSO fails in some cases because the OVS handle_offload()
>function is used with the upstream GSO handler.
>  * Hardware VXLAN offload was disabled on kernels that is otherwise
>supported on since version checks were incremented by the GBP changes.
>  * LISP attempts to use hardware offload with NICs that only support
>VXLAN
>
> Jesse Gross (14):
>   datapath: Remove compat vxlan_src_port().
>   datapath: Account for "udp: Add udp_sock_create for UDP tunnels to
> open listener socket"
>   datapath: Account for "vxlan: Call udp_sock_create"
>   datapath: Use udp_sock_create() for LISP.
>   datapath: Account for "udp: Generic functions to set checksum"
>   datapath: Enable OVS GSO to be used up to 3.18 if necessary.
>   datapath: Account for "udp-tunnel: Add a few more UDP tunnel APIs"
>   datapath: Consistently set skb->inner_protocol for tunnels.
>   datapath: Account for "vxlan: Refactor vxlan driver to make use of the
> common UDP tunnel functions."
>   datapath: Use additional common UDP functions for LISP.
>   datapath: Account for "vlan: introduce *vlan_hwaccel_push_inside
> helpers"
>   datapath: Account for "udp: Do not require sock in
> udp_tunnel_xmit_skb"
>   datapath: Backport upstream Geneve implementation.
>   datapath: Account for "openvswitch: Add support for checksums on UDP
> tunnels."
>

I have not finished review yet, but wanted to let you know that this
series does not compile on 3.12, 3.17, so that you can fix it in
parallel.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [V2] ovs-sandbox: Add an option to allow running ovs-vswitchd under gdb

2015-02-19 Thread Andy Zhou
It is some times useful to leverage the sandbox facility to experiment
and explore the internals of ovs-vswitchd.  Since GDB requires console
access for user inputs, this patch launch an xterm for GDB, The main
terminal continue to run the sub-shell as before. Exiting the sub-shell
will also kill the ovs-vswitchd under GDB (but not GDB itself currently)

Signed-off-by: Andy Zhou 

I have been making similar changes in my private tree for a while.
It may be worth while to upstream it others can benefit as well.

Instead of launching xterm, we could probably leave the main console
for GDB, and allow user to start another sub-shell in a separate
terminal. However, this won't scale to multiple programs, in case
one wants to debug both ovs-vswitchd and ovsdb-server.
---
 tutorial/Tutorial.md | 21 +
 tutorial/automake.mk |  2 +-
 tutorial/ovs-sandbox | 30 +++---
 3 files changed, 49 insertions(+), 4 deletions(-)

diff --git a/tutorial/Tutorial.md b/tutorial/Tutorial.md
index d6a963b..0bc678f 100644
--- a/tutorial/Tutorial.md
+++ b/tutorial/Tutorial.md
@@ -104,6 +104,27 @@ The sandbox directory contains log files for the Open 
vSwitch dameons.
 You can examine them while you're running in the sandboxed environment
 or after you exit.
 
+Using GDB
+-
+
+GDB support is not required to go through the tutorial. It is added in case
+user wants to explore the internals of OVS programs.
+
+GDB can already be used to debug any running process, with the usual
+'gdb  ' command.
+
+'ovs-sandbox' also has a '-g' option for launching ovs-vswitchd under GDB.
+This option can be handy for setting break points before ovs-vswitchd runs,
+or for catching early segfaults.
+
+To avoid GDB mangling with the sandbox sub shell terminal, 'ovs-sandbox'
+starts a new xterm to run each GDB session.  For systems that do not support
+X windows, GDB support is effectively disabled.
+
+When launching sandbox through the build tree's make file, the '-g' option
+can be passed via the 'SANDBOXFLAGS' environment variable. 
+'make sandbox SANDBOXFLAGS=-g' will start the sandbox with ovs-vswitchd
+running under GDB in its own xterm if X is available.
 
 Motivation
 --
diff --git a/tutorial/automake.mk b/tutorial/automake.mk
index 3d22d7a..5af0aac 100644
--- a/tutorial/automake.mk
+++ b/tutorial/automake.mk
@@ -9,4 +9,4 @@ EXTRA_DIST += \
tutorial/t-stage4
 
 sandbox: all
-   cd $(srcdir)/tutorial && MAKE=$(MAKE) ./ovs-sandbox -b $(abs_builddir)
+   cd $(srcdir)/tutorial && MAKE=$(MAKE) ./ovs-sandbox -b $(abs_builddir) 
$(SANDBOXFLAGS)
diff --git a/tutorial/ovs-sandbox b/tutorial/ovs-sandbox
index 45bb234..5663acd 100755
--- a/tutorial/ovs-sandbox
+++ b/tutorial/ovs-sandbox
@@ -16,16 +16,36 @@
 
 set -e
 
-run () {
-echo "$@"
+run() {
 (cd "$sandbox" && "$@") || exit 1
 }
 
+run_xterm() {
+run xterm -e "$@" &
+}
+
+rungdb() {
+under_gdb=$1
+shift
+# Remove the --detach and to put the process under gdb control.
+# Also remove --vconsole:off to allow error message to show up
+# on the console.
+# Use "DISPLAY" variable to determine out if X is supported
+if $under_gdb && [ "$DISPLAY" ]; then
+args=`echo $@ |sed s/--detach//g | sed s/--vconsole:off//g`
+run_xterm gdb --args $args
+else
+run $@
+fi
+}
+
+gdb_vswitchd=false;
 builddir=
 srcdir=
 schema=
 installed=false
 built=false
+
 for option; do
 # This option-parsing mechanism borrowed from a Autoconf-generated
 # configure script under the following license:
@@ -63,6 +83,7 @@ These options force ovs-sandbox to use a particular OVS build:
   -s, --srcdir=DIR specify Open vSwitch source directory
 These options force ovs-sandbox to use an installed Open vSwitch:
   -i, --installed  use installed Open vSwitch
+  -g, --gdb-vswitchd   run ovs-vswitchd under gdb
   -S, --schema=FILEuse FILE as vswitch.ovsschema
 
 Other options:
@@ -98,6 +119,9 @@ EOF
 prev=schema
 installed=:
 ;;
+-g|--gdb-v*)
+gdb_vswitchd=true
+;;
 -*)
 echo "unrecognized option $option (use --help for help)" >&2
 exit 1
@@ -204,7 +228,7 @@ run ovsdb-server --detach --no-chdir --pidfile 
-vconsole:off --log-file \
 --remote=punix:"$sandbox"/db.sock
 
 # Start ovs-vswitchd.
-run ovs-vswitchd --detach --no-chdir --pidfile -vconsole:off --log-file \
+rungdb $gdb_vswitchd ovs-vswitchd --detach --no-chdir --pidfile -vconsole:off 
--log-file \
 --enable-dummy=override -vvconn -vnetdev_dummy
 
 cat 

Re: [ovs-dev] [PATCH 00/14] UDP tunneling backports

2015-02-19 Thread Jesse Gross
On Thu, Feb 19, 2015 at 11:29 AM, Pravin Shelar  wrote:
> On Wed, Feb 18, 2015 at 7:02 PM, Jesse Gross  wrote:
>> This series backports much of the UDP tunnel infrastructure, ultimately
>> in support of syncing Geneve with upstream and providing outer UDP
>> checksum support.
>>
>> During the backporting process, I realized that there are several
>> other lurking bugs that were fixed through a more complete UDP
>> tunnel layer. The ones that I am aware of are:
>>  * VXLAN TSO fails in some cases because the OVS handle_offload()
>>function is used with the upstream GSO handler.
>>  * Hardware VXLAN offload was disabled on kernels that is otherwise
>>supported on since version checks were incremented by the GBP changes.
>>  * LISP attempts to use hardware offload with NICs that only support
>>VXLAN
>>
>> Jesse Gross (14):
>>   datapath: Remove compat vxlan_src_port().
>>   datapath: Account for "udp: Add udp_sock_create for UDP tunnels to
>> open listener socket"
>>   datapath: Account for "vxlan: Call udp_sock_create"
>>   datapath: Use udp_sock_create() for LISP.
>>   datapath: Account for "udp: Generic functions to set checksum"
>>   datapath: Enable OVS GSO to be used up to 3.18 if necessary.
>>   datapath: Account for "udp-tunnel: Add a few more UDP tunnel APIs"
>>   datapath: Consistently set skb->inner_protocol for tunnels.
>>   datapath: Account for "vxlan: Refactor vxlan driver to make use of the
>> common UDP tunnel functions."
>>   datapath: Use additional common UDP functions for LISP.
>>   datapath: Account for "vlan: introduce *vlan_hwaccel_push_inside
>> helpers"
>>   datapath: Account for "udp: Do not require sock in
>> udp_tunnel_xmit_skb"
>>   datapath: Backport upstream Geneve implementation.
>>   datapath: Account for "openvswitch: Add support for checksums on UDP
>> tunnels."
>>
>
> I have not finished review yet, but wanted to let you know that this
> series does not compile on 3.12, 3.17, so that you can fix it in
> parallel.

Thanks, I'll take a look at that.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] ovsdb-doc: Add license and copyright notice.

2015-02-19 Thread Thomas Graf
On 02/19/15 at 11:10am, Ben Pfaff wrote:
> The copyright dates are taken from "git log --follow ovsdb/ovsdb-doc",
> considering only Nicira authors' changes.  (Only one change was from
> a non-Nicira author anyhow.)
> 
> Signed-off-by: Ben Pfaff 

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


Re: [ovs-dev] [PATCH v2 2/2] [RFC] ovn: Start work on design documentation.

2015-02-19 Thread Thomas Graf
This is a first high level review. I'll be digging into the schema
next.

On 02/19/15 at 11:16am, Ben Pfaff wrote:
> +  
> +
> +  The Cloud Management System, as defined above.
> +
> +
> +
> +  
> + The OVN/CMS Plugin is the component of the CMS that
> + interfaces to OVN.  In OpenStack, this is a Neutron plugin.
> + The plugin's main purpose is to translate the CMS's notion of logical
> + network configuration, stored in the CMS's configuration database in a
> + CMS-specific format, into an intermediate representation understood by
> + OVN.
> +  

Does it make sense to consider a multi writer CMS scenario where
multiple CMS managed a single physical network? An assumption could
be made that a particular hypervisor is only controlled through a
single CMS and that only the logical network and binding would require
some form of standardization.

I don't have a good practical example at this point so this is mostly
a brain exercise but I can't come up for a reason why this may not
come up as a future requirement.

> +
> +  Eventually, a user powers on the VM that owns the VIF.  On the 
> hypervisor
> +  where the VM is powered on, the integration between the hypervisor and
> +  Open vSwitch (described in IntegrationGuide.md) adds the 
> VIF
> +  to the OVN integration bridge and stores vif-id in
> +  external-ids:iface-id to indicate that the
> +  interface is an instantiation of the new VIF.  (None of this code is 
> new
> +  in OVN; this is pre-existing integration work that has already been 
> done
> +  on hypervisors that support OVS.)
> +

Is this piece CMS specific as well or can we leverage libvirt to
create some abstraction here and write the vif-id into OVSDB?

> +
> +  Eventually, a user powers off the VM that owns the VIF.  On the
> +  hypervisor where the VM was powered on, the VIF is deleted from the OVN
> +  integration bridge.
> +
>
I think it should also be noted who is responsible for cleaning up
this non-persistent data in case the power off does not occur, e.g.
system crash of he hypervisor. I assume it's the CMS integration plugin
running on the hypervisor. It could also be part of the ovn-controller
system bootup script.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] ovsdb-doc: Add license and copyright notice.

2015-02-19 Thread Ben Pfaff
On Thu, Feb 19, 2015 at 09:52:47PM +0100, Thomas Graf wrote:
> On 02/19/15 at 11:10am, Ben Pfaff wrote:
> > The copyright dates are taken from "git log --follow ovsdb/ovsdb-doc",
> > considering only Nicira authors' changes.  (Only one change was from
> > a non-Nicira author anyhow.)
> > 
> > Signed-off-by: Ben Pfaff 
> 
> Acked-by: Thomas Graf 

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


Re: [ovs-dev] [V2] ovs-sandbox: Add an option to allow running ovs-vswitchd under gdb

2015-02-19 Thread Ben Pfaff
On Thu, Feb 19, 2015 at 11:41:48AM -0800, Andy Zhou wrote:
> It is some times useful to leverage the sandbox facility to experiment
> and explore the internals of ovs-vswitchd.  Since GDB requires console
> access for user inputs, this patch launch an xterm for GDB, The main
> terminal continue to run the sub-shell as before. Exiting the sub-shell
> will also kill the ovs-vswitchd under GDB (but not GDB itself currently)
> 
> Signed-off-by: Andy Zhou 
> 
> I have been making similar changes in my private tree for a while.
> It may be worth while to upstream it others can benefit as well.
> 
> Instead of launching xterm, we could probably leave the main console
> for GDB, and allow user to start another sub-shell in a separate
> terminal. However, this won't scale to multiple programs, in case
> one wants to debug both ovs-vswitchd and ovsdb-server.

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


Re: [ovs-dev] [PATCH v2 2/2] [RFC] ovn: Start work on design documentation.

2015-02-19 Thread Russell Bryant
On 02/19/2015 04:20 PM, Thomas Graf wrote:
> This is a first high level review. I'll be digging into the schema
> next.
> 
> On 02/19/15 at 11:16am, Ben Pfaff wrote:
>> +  
>> +
>> +  The Cloud Management System, as defined above.
>> +
>> +
>> +
>> +  
>> +The OVN/CMS Plugin is the component of the CMS that
>> +interfaces to OVN.  In OpenStack, this is a Neutron plugin.
>> +The plugin's main purpose is to translate the CMS's notion of logical
>> +network configuration, stored in the CMS's configuration database in a
>> +CMS-specific format, into an intermediate representation understood by
>> +OVN.
>> +  
> 
> Does it make sense to consider a multi writer CMS scenario where
> multiple CMS managed a single physical network? An assumption could
> be made that a particular hypervisor is only controlled through a
> single CMS and that only the logical network and binding would require
> some form of standardization.
> 
> I don't have a good practical example at this point so this is mostly
> a brain exercise but I can't come up for a reason why this may not
> come up as a future requirement.

I think it makes sense to consider a multi-writer scenario.  I was
trying to see if I could come up with a reason that scenario would be a
problem.  Do you see any?

The only thing I came up with was the need for each CMS to note which
resources are theirs vs. managed by something else.  The inclusion of
"external_ids" in every table in the OVN northbound schema seems to
cover that bit.

>> +
>> +  Eventually, a user powers off the VM that owns the VIF.  On the
>> +  hypervisor where the VM was powered on, the VIF is deleted from the 
>> OVN
>> +  integration bridge.
>> +
>>
> I think it should also be noted who is responsible for cleaning up
> this non-persistent data in case the power off does not occur, e.g.
> system crash of he hypervisor. I assume it's the CMS integration plugin
> running on the hypervisor. It could also be part of the ovn-controller
> system bootup script.

I think making this the responsibility of the CMS integration makes
sense.  Taking OpenStack as an example, I would think OpenStack
Neutron's db is the source of truth of what the current state should be.
 It will have to include the ability to do a sync in response to any
event that could have put them out of sync and make sure the data in OVN
matches.  That would include this sort of cleanup.  In the case of a
hypervisor crash, something should still come around and tell Neutron
that the associated ports need to be deleted.

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


Re: [ovs-dev] [neutron]?? [PATCH 8/8] [RFC] ovn: Start work on design Documentation.

2015-02-19 Thread Ben Pfaff
Thanks.  It looks like openstack-dev only accepts posts from
subscribers, so I've followed up to your more detailed feedback only
on that list.  For anyone following along at home, here's an archives
link:
http://lists.openstack.org/pipermail/openstack-dev/2015-February/057376.html

On Thu, Feb 19, 2015 at 10:38:03AM +0100, Miguel ??ngel Ajo wrote:
> Thank you Ben!,  
> 
> Cross posting [1] to openstack list /neutron.
> 
> 
> [1] http://benpfaff.org/~blp/dist-docs. 
> 
> On Thursday, 19 de February de 2015 at 09:13, Ben Pfaff wrote:
> 
> > On Thu, Feb 19, 2015 at 12:12:26AM -0800, Ben Pfaff wrote:
> > > This commit adds preliminary design documentation for Open Virtual 
> > > Network,
> > > or OVN, a new OVS-based project to add support for virtual networking to
> > > OVS, initially with OpenStack integration.
> > > 
> > > This initial design has been influenced by many people, including (in
> > > alphabetical order) Aaron Rosen, Chris Wright, Jeremy Stribling,
> > > Justin Pettit, Ken Duda, Madhu Venugopal, Martin Casado, Pankaj Thakkar,
> > > Russell Bryant, and Teemu Koponen. All blunders, however, are due to my
> > > own hubris.
> > > 
> > > Signed-off-by: Ben Pfaff mailto:b...@nicira.com)>
> > 
> > I've posted the rendered version of the documentation following this
> > commit at http://benpfaff.org/~blp/dist-docs. You probably want to look
> > at the ovn* manpages, especially ovn-architecture(7), ovn(5), and
> > ovn-nb(5).
> > ___
> > dev mailing list
> > dev@openvswitch.org (mailto: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 v2 2/2] [RFC] ovn: Start work on design documentation.

2015-02-19 Thread Gurucharan Shetty
On Thu, Feb 19, 2015 at 11:16 AM, Ben Pfaff  wrote:
> This commit adds preliminary design documentation for Open Virtual Network,
> or OVN, a new OVS-based project to add support for virtual networking to
> OVS, initially with OpenStack integration.
>
> This initial design has been influenced by many people, including (in
> alphabetical order) Aaron Rosen, Chris Wright, Jeremy Stribling,
> Justin Pettit, Ken Duda, Madhu Venugopal, Martin Casado, Pankaj Thakkar,
> Russell Bryant, and Teemu Koponen.  All blunders, however, are due to my
> own hubris.
>
> Signed-off-by: Ben Pfaff 
Couple of whitespace errors.

...snip...
> +  
> +   Hypervisors and gateways are together called transport code
> +   or chassis.
s/code/node

...snip...
> +  
> +   The OVN Northbound Database received the intermediate
Did you mean s/received/receives ?

..snip...
> +
> +
> +  Some CMS systems, including OpenStack, fully start a VM only when its
> +  networking is ready.  To support this, ovn-nbd notices the
> +  new row in the Bindings table, and pushes this upward by
> +  updating the  column
> +  in the OVN Northbound database's  db="OVN_NB"/>
> +  table to indicate that the VIF is now up.  The CMS, if it uses this
> +  feature, can then react by allowing the VM's execution to proceed.
> +
> +
> +
> +  On every hypervisor but the one where the VIF resides,
> +  ovn-controller notices the new row in the
> +  Bindings table.  This provides ovn-controller
> +  the physical location of the logical port, so each instance updates the
> +  OpenFlow tables of its switch (based on logical datapath flows in the 
> OVN
> +  DB Pipeline table) so that packets to and from the VIF can
> +  be properly handled via tunnels.
> +
I wonder how much of a problem this delay in propagation of state to
other hypervisors will be.
At least with containers, where startup time is non-existent, it would
mean that every container application written should be smart enough
to retry connecting to another application in a different host.

...snip...
> +
> +  ovs-nbd receives the OVN Northbound update and in turn
> +  updates the OVN database accordingly, by the removing or updating the
s/the removing/removing?

---snip---

> +
> +
> +  The logical port name.  The name used here must match those used in the
> +   +  db="Open_vSwitch"/> in the  database's  +  table="Interface" db="Open_vSwitch"/> table, because hypervisors use 
>  +  key="iface-id" table="Interface" column="external_ids"
> +  db="Open_vSwitch"/> as a lookup key for logical ports.
> +
The above fits nicely if you want to run containers on hypervisors
too. Since the document clearly mentions that we only want to start by
integrating with CMS - openstack for VMs, that is alright. But, if we
want to run multiple containers inside each tenant VM, and then
provide individual IP addressability and policies on each of the
container interface, then we probably should update
IntegrationGuide.md to include the concept of "sub-interfaces" - where
each VM VIF can represent multiple container interfaces. Openstack
currently does not support such a concept, but it may make sense to
have the infrastructure in OVN for it while designing. This will be
useful in a world that contains containers, VMs and physical machines
interconnected with each other.


---snip---
> +  
> +
> +  Each row in this table represents one ACL rule for the logical switch 
> in
> +  its  column.  The  column 
> for
> +  the highest- matching row in this table
> +  determines a packet's treatment.  If no row matches, packets are 
> allowed
> +  by default.  (Default-deny treatment is possible: add a rule with  +  column="priority"/> 0, true as , and
> +  deny as .)
> +
> +
> +
> +  The switch to which the ACL rule applies.
> +
> +
Shouldn't a ACL rule apply to a logical port instead of a logical switch?

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


Re: [ovs-dev] [PATCH v2 2/2] [RFC] ovn: Start work on design documentation.

2015-02-19 Thread Ben Pfaff
On Thu, Feb 19, 2015 at 10:20:13PM +0100, Thomas Graf wrote:
> On 02/19/15 at 11:16am, Ben Pfaff wrote:
> > +  
> > +
> > +  The Cloud Management System, as defined above.
> > +
> > +
> > +
> > +  
> > +   The OVN/CMS Plugin is the component of the CMS that
> > +   interfaces to OVN.  In OpenStack, this is a Neutron plugin.
> > +   The plugin's main purpose is to translate the CMS's notion of logical
> > +   network configuration, stored in the CMS's configuration database in a
> > +   CMS-specific format, into an intermediate representation understood by
> > +   OVN.
> > +  
> 
> Does it make sense to consider a multi writer CMS scenario where
> multiple CMS managed a single physical network? An assumption could
> be made that a particular hypervisor is only controlled through a
> single CMS and that only the logical network and binding would require
> some form of standardization.

Do people do that?  (Do they want to do it?)  Those are serious
questions; I don't know the answers.

> I don't have a good practical example at this point so this is mostly
> a brain exercise but I can't come up for a reason why this may not
> come up as a future requirement.

Maybe that's my answer.

> > +
> > +  Eventually, a user powers on the VM that owns the VIF.  On the 
> > hypervisor
> > +  where the VM is powered on, the integration between the hypervisor 
> > and
> > +  Open vSwitch (described in IntegrationGuide.md) adds 
> > the VIF
> > +  to the OVN integration bridge and stores vif-id in
> > +  external-ids:iface-id to indicate that the
> > +  interface is an instantiation of the new VIF.  (None of this code is 
> > new
> > +  in OVN; this is pre-existing integration work that has already been 
> > done
> > +  on hypervisors that support OVS.)
> > +
> 
> Is this piece CMS specific as well or can we leverage libvirt to
> create some abstraction here and write the vif-id into OVSDB?

I believe that libvirt already does this; see
virNetDevOpenvswitchAddPort() in src/util/virnetdevopenvswitch.c.
(I'm looking at libvirt-0.9.12.3 with Debian patches applied, though I
doubt it matters.)

> > +
> > +  Eventually, a user powers off the VM that owns the VIF.  On the
> > +  hypervisor where the VM was powered on, the VIF is deleted from the 
> > OVN
> > +  integration bridge.
> > +
> >
> I think it should also be noted who is responsible for cleaning up
> this non-persistent data in case the power off does not occur, e.g.
> system crash of he hypervisor. I assume it's the CMS integration plugin
> running on the hypervisor. It could also be part of the ovn-controller
> system bootup script.

I thought about this a bit when I was working on the OVN database
schema, but I didn't bother to write it up.  Here's an incremental
diff against the OVN schema.  The first hunk is for the Chassis table,
the second for the Bindings table.  The Pipeline table doesn't need
special handling because ovs-nbd is fully in charge of it:

diff --git a/ovn/ovn.xml b/ovn/ovn.xml
index a0c3c82..69e429c 100644
--- a/ovn/ovn.xml
+++ b/ovn/ovn.xml
@@ -108,6 +108,15 @@
   copy of the remaining rows to determine how to reach other hypervisors.
 
 
+
+  When a chassis shuts down gracefully, it should remove its own row.
+  (This is not critical because resources hosted on the chassis are equally
+  unreachable regardless of whether the row is present.)  If a chassis
+  shuts down permanently without removing its row, some kind of manual or
+  automatic cleanup is eventually needed; we can devise a process for that
+  as necessary.
+
+
 
   A chassis name, taken from  in the Open_vSwitch
@@ -443,6 +452,16 @@
   via the conventions described in IntegrationGuide.md.
 
 
+
+  When a chassis shuts down gracefully, it should remove its bindings.
+  (This is not critical because resources hosted on the chassis are equally
+  unreachable regardless of whether their rows are present.)  To handle the
+  case where a VM is shut down abruptly on one chassis, then brought up
+  again on a different one, ovn-controller must delete any
+  existing  record for a logical port when it adds a
+  new one.
+
+
 
   A logical port, taken from  in the Open_vSwitch database's
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v2 2/2] [RFC] ovn: Start work on design documentation.

2015-02-19 Thread Thomas Graf
On 02/19/15 at 11:16am, Ben Pfaff wrote:
> diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
> new file mode 100644
> index 000..4c29562
> --- /dev/null
> +++ b/ovn/ovn-nb.ovsschema

The schema below has some whitespace / tab mixups.

[...]

> @@ -0,0 +1,63 @@
> +{
> +"name": "OVN_Northbound",
> +"tables": {
> +"Logical_Switch": {
> +"columns": {
> + "external_ids": {
> + "type": {"key": "string", "value": "string",
> +  "min": 0, "max": "unlimited",
> +"Logical_Port": {
> +"columns": {
> + "switch": {"type": {"key": {"type": "uuid",
> + "refTable": "Logical_Switch",
> + "refType": "strong"}}},
> + "router_port": {"type": {"key": {"type": "uuid",
> +  "refTable": 
> "Logical_Router_Port",
> +  "refType": "strong"},
> + "min": 0,
> + "max": 1}},

What is the reason to hook the Logical_Router_Port to a Logical_Port
and not to the Logical_Switch directly? It seems like the port_security
does not apply to routers and the state should also be "up" at all
times.

[...]

> +  
> +Following are not well thought out:
> +  
> +
> +  
> +  learn
> +
> +  conntrack
> +
> +  with(field=value) { 
> action, ... }
> +  execute actions with temporary changes to 
> fields
> +
> +  dec_ttl { action, ... } { 
> action; ...}
> +  
> +decrement TTL; execute first set of actions if
> +successful, second set if TTL decrement fails
> +  
> +
> +  icmp_reply { action, ... 
> }
> +  generate ICMP reply from packet, execute action

Possible useful addition could be arp_respond to implement the L2 pop
of OpenStack.

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


Re: [ovs-dev] [PATCH v2 2/2] [RFC] ovn: Start work on design documentation.

2015-02-19 Thread Thomas Graf
On 02/19/15 at 02:36pm, Ben Pfaff wrote:
> On Thu, Feb 19, 2015 at 10:20:13PM +0100, Thomas Graf wrote:
> > Is this piece CMS specific as well or can we leverage libvirt to
> > create some abstraction here and write the vif-id into OVSDB?
> 
> I believe that libvirt already does this; see
> virNetDevOpenvswitchAddPort() in src/util/virnetdevopenvswitch.c.
> (I'm looking at libvirt-0.9.12.3 with Debian patches applied, though I
> doubt it matters.)

Perfect ;-)

> I thought about this a bit when I was working on the OVN database
> schema, but I didn't bother to write it up.  Here's an incremental
> diff against the OVN schema.  The first hunk is for the Chassis table,
> the second for the Bindings table.  The Pipeline table doesn't need
> special handling because ovs-nbd is fully in charge of it:

Looks great. Eventually this cleanup process could also take care
of removing stale OVSDB ports after a reboot.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH net] openvswitch: Fix net exit.

2015-02-19 Thread Thomas Graf
On 02/17/15 at 11:23am, Pravin B Shelar wrote:
> Open vSwitch allows moving internal vport to different namespace
> while still connected to the bridge. But when namespace deleted
> OVS does not detach these vports, that results in dangling
> pointer to netdevice which causes kernel panic as follows.
> This issue is fixed by detaching all ovs ports from the deleted
> namespace at net-exit.

[...]

> Reported-by: Assaf Muller 
> Fixes: 46df7b81454("openvswitch: Add support for network namespaces.")
> Signed-off-by: Pravin B Shelar 

Reviewed-by: Thomas Graf 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH 06/14 v2] datapath: Enable OVS GSO to be used up to 3.18 if necessary.

2015-02-19 Thread Jesse Gross
There are two important GSO tunnel features that were introduced
after the 3.12 cutoff for our current out of tree GSO implementation:
 * 3.16 introduced support for outer UDP checksums.
 * 3.18 introduced support for verifying hardware support for protocols
   other than VXLAN.

In cases where these features are used, we should use OVS GSO to
ensure correct behavior. However, we also want to continue to use
kernel GSO or hardware TSO in existing situations. Therefore, this
extends the range of kernels where OVS GSO is available to 3.18 and
makes it easier to select which one to use.

Signed-off-by: Jesse Gross 
---
v2: Fix compilation on kernels 3.12 and 3.17.
---
 datapath/linux/compat/gso.c| 46 +-
 datapath/linux/compat/gso.h| 28 +
 datapath/linux/compat/include/net/ip_tunnels.h | 36 ++-
 datapath/linux/compat/ip_tunnels_core.c| 87 --
 4 files changed, 147 insertions(+), 50 deletions(-)

diff --git a/datapath/linux/compat/gso.c b/datapath/linux/compat/gso.c
index b20ad8a..fbc0356 100644
--- a/datapath/linux/compat/gso.c
+++ b/datapath/linux/compat/gso.c
@@ -167,7 +167,9 @@ drop:
kfree_skb(skb);
return err;
 }
+#endif /* 3.16 */
 
+#if LINUX_VERSION_CODE < KERNEL_VERSION(3,18,0)
 static __be16 __skb_network_protocol(struct sk_buff *skb)
 {
__be16 type = skb->protocol;
@@ -190,16 +192,6 @@ static __be16 __skb_network_protocol(struct sk_buff *skb)
return type;
 }
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(3,12,0)
-static void tnl_fix_segment(struct sk_buff *skb)
-{
-   if (OVS_GSO_CB(skb)->fix_segment)
-   OVS_GSO_CB(skb)->fix_segment(skb);
-}
-#else
-static void tnl_fix_segment(struct sk_buff *skb) { }
-#endif
-
 static struct sk_buff *tnl_skb_gso_segment(struct sk_buff *skb,
   netdev_features_t features,
   bool tx_path)
@@ -240,7 +232,9 @@ static struct sk_buff *tnl_skb_gso_segment(struct sk_buff 
*skb,
 
memcpy(ip_hdr(skb), iph, pkt_hlen);
memcpy(skb->cb, cb, sizeof(cb));
-   tnl_fix_segment(skb);
+
+   if (OVS_GSO_CB(skb)->fix_segment)
+   OVS_GSO_CB(skb)->fix_segment(skb);
 
skb->protocol = proto;
skb = skb->next;
@@ -293,10 +287,9 @@ int rpl_ip_local_out(struct sk_buff *skb)
}
return ret;
 }
-#endif /* 3.16 */
+#endif /* 3.18 */
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(3,12,0) || \
-   !defined USE_UPSTREAM_VXLAN
+#if LINUX_VERSION_CODE < KERNEL_VERSION(3,12,0)
 struct sk_buff *ovs_iptunnel_handle_offloads(struct sk_buff *skb,
  bool csum_help,
 void (*fix_segment)(struct sk_buff 
*))
@@ -345,4 +338,27 @@ error:
kfree_skb(skb);
return ERR_PTR(err);
 }
-#endif /* 3.12 || !USE_UPSTREAM_VXLAN */
+#elif LINUX_VERSION_CODE < KERNEL_VERSION(3,18,0)
+struct sk_buff *ovs_iptunnel_handle_offloads(struct sk_buff *skb,
+ bool csum_help,
+void (*fix_segment)(struct sk_buff 
*))
+{
+   int err;
+
+   if (skb_is_gso(skb)) {
+   if (skb_is_encapsulated(skb)) {
+   err = -ENOSYS;
+   goto error;
+   }
+   }
+
+   OVS_GSO_CB(skb)->fix_segment = fix_segment;
+
+   /* Pass in zero for the GSO type, it won't get used anyways. */ 
+   return iptunnel_handle_offloads(skb, csum_help, 0);
+
+error:
+   kfree_skb(skb);
+   return ERR_PTR(err);
+}
+#endif /* 3.18 */
diff --git a/datapath/linux/compat/gso.h b/datapath/linux/compat/gso.h
index 023d6d3..fe9ff78 100644
--- a/datapath/linux/compat/gso.h
+++ b/datapath/linux/compat/gso.h
@@ -2,8 +2,7 @@
 #define __LINUX_GSO_WRAPPER_H
 
 #include 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(3,12,0) || \
-   !defined USE_UPSTREAM_VXLAN
+#if LINUX_VERSION_CODE < KERNEL_VERSION(3,18,0)
 
 #include 
 #include 
@@ -15,15 +14,25 @@ typedef void (*gso_fix_segment_t)(struct sk_buff *);
 struct ovs_gso_cb {
struct ovs_skb_cb dp_cb;
gso_fix_segment_t fix_segment;
-   sk_buff_data_t  inner_mac_header;   /* Offset from skb->head */
 #if LINUX_VERSION_CODE < KERNEL_VERSION(3,11,0)
__be16  inner_protocol;
 #endif
+#if LINUX_VERSION_CODE < KERNEL_VERSION(3,12,0)
+   sk_buff_data_t  inner_mac_header;   /* Offset from skb->head */
u16 inner_network_header;   /* Offset from
 * inner_mac_header */
+#endif
 };
 #define OVS_GSO_CB(skb) ((struct ovs_gso_cb *)(skb)->cb)
 
+struct sk_buff *ovs_iptunnel_handle_offloads(struct sk_buff *skb,
+ bool csum_help,
+gso_fix_segm

Re: [ovs-dev] [PATCH 00/14] UDP tunneling backports

2015-02-19 Thread Jesse Gross
On Thu, Feb 19, 2015 at 12:02 PM, Jesse Gross  wrote:
> On Thu, Feb 19, 2015 at 11:29 AM, Pravin Shelar  wrote:
>> On Wed, Feb 18, 2015 at 7:02 PM, Jesse Gross  wrote:
>>> This series backports much of the UDP tunnel infrastructure, ultimately
>>> in support of syncing Geneve with upstream and providing outer UDP
>>> checksum support.
>>>
>>> During the backporting process, I realized that there are several
>>> other lurking bugs that were fixed through a more complete UDP
>>> tunnel layer. The ones that I am aware of are:
>>>  * VXLAN TSO fails in some cases because the OVS handle_offload()
>>>function is used with the upstream GSO handler.
>>>  * Hardware VXLAN offload was disabled on kernels that is otherwise
>>>supported on since version checks were incremented by the GBP changes.
>>>  * LISP attempts to use hardware offload with NICs that only support
>>>VXLAN
>>>
>>> Jesse Gross (14):
>>>   datapath: Remove compat vxlan_src_port().
>>>   datapath: Account for "udp: Add udp_sock_create for UDP tunnels to
>>> open listener socket"
>>>   datapath: Account for "vxlan: Call udp_sock_create"
>>>   datapath: Use udp_sock_create() for LISP.
>>>   datapath: Account for "udp: Generic functions to set checksum"
>>>   datapath: Enable OVS GSO to be used up to 3.18 if necessary.
>>>   datapath: Account for "udp-tunnel: Add a few more UDP tunnel APIs"
>>>   datapath: Consistently set skb->inner_protocol for tunnels.
>>>   datapath: Account for "vxlan: Refactor vxlan driver to make use of the
>>> common UDP tunnel functions."
>>>   datapath: Use additional common UDP functions for LISP.
>>>   datapath: Account for "vlan: introduce *vlan_hwaccel_push_inside
>>> helpers"
>>>   datapath: Account for "udp: Do not require sock in
>>> udp_tunnel_xmit_skb"
>>>   datapath: Backport upstream Geneve implementation.
>>>   datapath: Account for "openvswitch: Add support for checksums on UDP
>>> tunnels."
>>>
>>
>> I have not finished review yet, but wanted to let you know that this
>> series does not compile on 3.12, 3.17, so that you can fix it in
>> parallel.
>
> Thanks, I'll take a look at that.

I fixed the compilation problems on these kernels. The only affected
patch was #6 (enabling GSO on up to 3.18) so I resent that one only in
case you are still in the process of looking at the others.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 01/14] datapath: Remove compat vxlan_src_port().

2015-02-19 Thread Thomas Graf
On 02/18/15 at 07:02pm, Jesse Gross wrote:
> vxlan_src_port() has been replaced with the more generic
> udp_flow_src_port() upstream. We already have a backport for this and
> it is used everywhere where this is needed, so we can remove the
> dead vxlan_src_port() function.
> 
> Signed-off-by: Jesse Gross 

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


Re: [ovs-dev] [PATCH 02/14] datapath: Account for "udp: Add udp_sock_create for UDP tunnels to open listener socket"

2015-02-19 Thread Thomas Graf
On 02/18/15 at 07:02pm, Jesse Gross wrote:
> Upstream commit:
> udp: Add udp_sock_create for UDP tunnels to open listener socket
> 
> Added udp_tunnel.c which can contain some common functions for UDP
> tunnels. The first function in this is udp_sock_create which is used
> to open the listener port for a UDP tunnel.
> 
> Signed-off-by: Tom Herbert 
> Signed-off-by: David S. Miller 
> 
> Upstream: 8024e028 ("udp: Add udp_sock_create for UDP tunnels to open 
> listener socket")
> Signed-off-by: Jesse Gross 

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


Re: [ovs-dev] [PATCH 03/14] datapath: Account for "vxlan: Call udp_sock_create"

2015-02-19 Thread Thomas Graf
On 02/18/15 at 07:02pm, Jesse Gross wrote:
> + if (ipv6) {
> + udp_conf.family = AF_INET6;
> + } else {
> + udp_conf.family = AF_INET;
> + udp_conf.local_ip.s_addr = htonl(INADDR_ANY);
> + }

Maybe leave a comment in the code here that the checksum flags
are not supported in this backport?

Otherwise:

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


Re: [ovs-dev] [PATCH 04/14] datapath: Use udp_sock_create() for LISP.

2015-02-19 Thread Thomas Graf
On 02/18/15 at 07:02pm, Jesse Gross wrote:
> Use the common udp_sock_create() for LISP, similar to what was
> done for VXLAN.
> 
> Signed-off-by: Jesse Gross 

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


Re: [ovs-dev] [PATCH 03/14] datapath: Account for "vxlan: Call udp_sock_create"

2015-02-19 Thread Jesse Gross
On Thu, Feb 19, 2015 at 3:56 PM, Thomas Graf  wrote:
> On 02/18/15 at 07:02pm, Jesse Gross wrote:
>> + if (ipv6) {
>> + udp_conf.family = AF_INET6;
>> + } else {
>> + udp_conf.family = AF_INET;
>> + udp_conf.local_ip.s_addr = htonl(INADDR_ANY);
>> + }
>
> Maybe leave a comment in the code here that the checksum flags
> are not supported in this backport?

It's not so much that it's not supported as it doesn't make sense in
the OVS context since this is basically using the socket to store
configuration data and we do it on a much finer granularity. In any
case, I added to a comment to describe this.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 05/14] datapath: Account for "udp: Generic functions to set checksum"

2015-02-19 Thread Thomas Graf
On 02/18/15 at 07:02pm, Jesse Gross wrote:
> Upstream commit:
> udp: Generic functions to set checksum
> 
> Added udp_set_csum and udp6_set_csum functions to set UDP checksums
> in packets. These are for simple UDP packets such as those that might
> be created in UDP tunnels.
> 
> Signed-off-by: Tom Herbert 
> Signed-off-by: David S. Miller 
> 
> Upstream: af5fcba7 ("udp: Generic functions to set checksum")
> Signed-off-by: Jesse Gross 

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


Re: [ovs-dev] [PATCH 06/14 v2] datapath: Enable OVS GSO to be used up to 3.18 if necessary.

2015-02-19 Thread Pravin Shelar
On Thu, Feb 19, 2015 at 3:26 PM, Jesse Gross  wrote:
> There are two important GSO tunnel features that were introduced
> after the 3.12 cutoff for our current out of tree GSO implementation:
>  * 3.16 introduced support for outer UDP checksums.
>  * 3.18 introduced support for verifying hardware support for protocols
>other than VXLAN.
>
> In cases where these features are used, we should use OVS GSO to
> ensure correct behavior. However, we also want to continue to use
> kernel GSO or hardware TSO in existing situations. Therefore, this
> extends the range of kernels where OVS GSO is available to 3.18 and
> makes it easier to select which one to use.
>
> Signed-off-by: Jesse Gross 
> ---
> v2: Fix compilation on kernels 3.12 and 3.17.
> ---
>  datapath/linux/compat/gso.c| 46 +-
>  datapath/linux/compat/gso.h| 28 +
>  datapath/linux/compat/include/net/ip_tunnels.h | 36 ++-
>  datapath/linux/compat/ip_tunnels_core.c| 87 
> --
>  4 files changed, 147 insertions(+), 50 deletions(-)
>
> diff --git a/datapath/linux/compat/gso.c b/datapath/linux/compat/gso.c
> index b20ad8a..fbc0356 100644
> --- a/datapath/linux/compat/gso.c
> +++ b/datapath/linux/compat/gso.c
> @@ -167,7 +167,9 @@ drop:
> kfree_skb(skb);
> return err;
>  }
> +#endif /* 3.16 */
>
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(3,18,0)
>  static __be16 __skb_network_protocol(struct sk_buff *skb)
>  {
> __be16 type = skb->protocol;
> @@ -190,16 +192,6 @@ static __be16 __skb_network_protocol(struct sk_buff *skb)
> return type;
>  }
>
> -#if LINUX_VERSION_CODE < KERNEL_VERSION(3,12,0)
> -static void tnl_fix_segment(struct sk_buff *skb)
> -{
> -   if (OVS_GSO_CB(skb)->fix_segment)
> -   OVS_GSO_CB(skb)->fix_segment(skb);
> -}
> -#else
> -static void tnl_fix_segment(struct sk_buff *skb) { }
> -#endif
> -
>  static struct sk_buff *tnl_skb_gso_segment(struct sk_buff *skb,
>netdev_features_t features,
>bool tx_path)
> @@ -240,7 +232,9 @@ static struct sk_buff *tnl_skb_gso_segment(struct sk_buff 
> *skb,
>
> memcpy(ip_hdr(skb), iph, pkt_hlen);
> memcpy(skb->cb, cb, sizeof(cb));
> -   tnl_fix_segment(skb);
> +
> +   if (OVS_GSO_CB(skb)->fix_segment)
> +   OVS_GSO_CB(skb)->fix_segment(skb);
>
> skb->protocol = proto;
> skb = skb->next;
> @@ -293,10 +287,9 @@ int rpl_ip_local_out(struct sk_buff *skb)
> }
> return ret;
>  }
> -#endif /* 3.16 */
> +#endif /* 3.18 */
>
> -#if LINUX_VERSION_CODE < KERNEL_VERSION(3,12,0) || \
> -   !defined USE_UPSTREAM_VXLAN
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(3,12,0)
>  struct sk_buff *ovs_iptunnel_handle_offloads(struct sk_buff *skb,
>   bool csum_help,
>  void (*fix_segment)(struct 
> sk_buff *))
> @@ -345,4 +338,27 @@ error:
> kfree_skb(skb);
> return ERR_PTR(err);
>  }
> -#endif /* 3.12 || !USE_UPSTREAM_VXLAN */
> +#elif LINUX_VERSION_CODE < KERNEL_VERSION(3,18,0)
> +struct sk_buff *ovs_iptunnel_handle_offloads(struct sk_buff *skb,
> + bool csum_help,
> +void (*fix_segment)(struct 
> sk_buff *))
> +{
> +   int err;
> +
> +   if (skb_is_gso(skb)) {
> +   if (skb_is_encapsulated(skb)) {
> +   err = -ENOSYS;
> +   goto error;
> +   }
> +   }
> +
> +   OVS_GSO_CB(skb)->fix_segment = fix_segment;
> +
> +   /* Pass in zero for the GSO type, it won't get used anyways. */
> +   return iptunnel_handle_offloads(skb, csum_help, 0);
> +
> +error:
> +   kfree_skb(skb);
> +   return ERR_PTR(err);
> +}
> +#endif /* 3.18 */
> diff --git a/datapath/linux/compat/gso.h b/datapath/linux/compat/gso.h
> index 023d6d3..fe9ff78 100644
> --- a/datapath/linux/compat/gso.h
> +++ b/datapath/linux/compat/gso.h
> @@ -2,8 +2,7 @@
>  #define __LINUX_GSO_WRAPPER_H
>
>  #include 
> -#if LINUX_VERSION_CODE < KERNEL_VERSION(3,12,0) || \
> -   !defined USE_UPSTREAM_VXLAN
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(3,18,0)
>
>  #include 
>  #include 
> @@ -15,15 +14,25 @@ typedef void (*gso_fix_segment_t)(struct sk_buff *);
>  struct ovs_gso_cb {
> struct ovs_skb_cb dp_cb;
> gso_fix_segment_t fix_segment;
> -   sk_buff_data_t  inner_mac_header;   /* Offset from skb->head */
>  #if LINUX_VERSION_CODE < KERNEL_VERSION(3,11,0)
> __be16  inner_protocol;
>  #endif
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(3,12,0)
> +   sk_buff_data_t  inner_mac_header;   /* Offset from skb->head */
> u16 inner_network_header;   /* Offset from
>  

Re: [ovs-dev] [PATCH v2 2/2] [RFC] ovn: Start work on design documentation.

2015-02-19 Thread Ben Pfaff
On Thu, Feb 19, 2015 at 04:52:44PM -0500, Russell Bryant wrote:
> On 02/19/2015 04:20 PM, Thomas Graf wrote:
> > This is a first high level review. I'll be digging into the schema
> > next.
> > 
> > On 02/19/15 at 11:16am, Ben Pfaff wrote:
> >> +  
> >> +
> >> +  The Cloud Management System, as defined above.
> >> +
> >> +
> >> +
> >> +  
> >> +  The OVN/CMS Plugin is the component of the CMS that
> >> +  interfaces to OVN.  In OpenStack, this is a Neutron plugin.
> >> +  The plugin's main purpose is to translate the CMS's notion of logical
> >> +  network configuration, stored in the CMS's configuration database in a
> >> +  CMS-specific format, into an intermediate representation understood by
> >> +  OVN.
> >> +  
> > 
> > Does it make sense to consider a multi writer CMS scenario where
> > multiple CMS managed a single physical network? An assumption could
> > be made that a particular hypervisor is only controlled through a
> > single CMS and that only the logical network and binding would require
> > some form of standardization.
> > 
> > I don't have a good practical example at this point so this is mostly
> > a brain exercise but I can't come up for a reason why this may not
> > come up as a future requirement.
> 
> I think it makes sense to consider a multi-writer scenario.  I was
> trying to see if I could come up with a reason that scenario would be a
> problem.  Do you see any?
> 
> The only thing I came up with was the need for each CMS to note which
> resources are theirs vs. managed by something else.  The inclusion of
> "external_ids" in every table in the OVN northbound schema seems to
> cover that bit.

If you or Thomas have an idea of how this might work, then let's talk
a bit more about details.  Until then, how about the following
incremental diff to at least capture this thought?

diff --git a/ovn/ovn-architecture.7.xml b/ovn/ovn-architecture.7.xml
index 303865e..f2ca268 100644
--- a/ovn/ovn-architecture.7.xml
+++ b/ovn/ovn-architecture.7.xml
@@ -19,11 +19,18 @@
 
   
 
-  A Cloud Management System (CMS), which is
-  OVN's ultimate client (via its users and administrators).  OVN
-  integration requires installing a CMS-specific plugin and
-  related software (see below).  OVN initially targets OpenStack
-  as CMS.
+  
+A Cloud Management System (CMS), which is
+OVN's ultimate client (via its users and administrators).  OVN
+integration requires installing a CMS-specific plugin and
+related software (see below).  OVN initially targets OpenStack
+as CMS.
+  
+
+  
+We generally speak of ``the'' CMS, but one can imagine scenarios in
+which multiple CMSes manage different parts of an OVN deployment.
+  
 
 
 
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index 9a2423a..dcd9651 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -8,6 +8,11 @@
 db="OVN"/> database.
   
 
+  
+We generally speak of ``the'' CMS, but one can imagine scenarios in
+which multiple CMSes manage different parts of an OVN deployment.
+  
+
   External IDs
 
   

> >> +
> >> +  Eventually, a user powers off the VM that owns the VIF.  On the
> >> +  hypervisor where the VM was powered on, the VIF is deleted from the 
> >> OVN
> >> +  integration bridge.
> >> +
> >>
> > I think it should also be noted who is responsible for cleaning up
> > this non-persistent data in case the power off does not occur, e.g.
> > system crash of he hypervisor. I assume it's the CMS integration plugin
> > running on the hypervisor. It could also be part of the ovn-controller
> > system bootup script.
> 
> I think making this the responsibility of the CMS integration makes
> sense.  Taking OpenStack as an example, I would think OpenStack
> Neutron's db is the source of truth of what the current state should be.
>  It will have to include the ability to do a sync in response to any
> event that could have put them out of sync and make sure the data in OVN
> matches.  That would include this sort of cleanup.  In the case of a
> hypervisor crash, something should still come around and tell Neutron
> that the associated ports need to be deleted.

From OVN's point of view a crashed hypervisor is just unreachable, I
think.  The row for a permanently down hypervisor needs to be deleted
eventually; ovs-nbd could handle that, I guess, probably by adding an
occasionally updated "timestamp" or similar field.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v2 2/2] [RFC] ovn: Start work on design documentation.

2015-02-19 Thread Ben Pfaff
On Thu, Feb 19, 2015 at 11:52:51PM +0100, Thomas Graf wrote:
> On 02/19/15 at 02:36pm, Ben Pfaff wrote:
> > I thought about this a bit when I was working on the OVN database
> > schema, but I didn't bother to write it up.  Here's an incremental
> > diff against the OVN schema.  The first hunk is for the Chassis table,
> > the second for the Bindings table.  The Pipeline table doesn't need
> > special handling because ovs-nbd is fully in charge of it:
> 
> Looks great. Eventually this cleanup process could also take care
> of removing stale OVSDB ports after a reboot.

I tend to prefer the model where OVSDB is reconstructed essentially
from an empty DB at boot.  That's what happens on e.g. XenServer.  Is
the more long-term model used widely?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v2 2/2] [RFC] ovn: Start work on design documentation.

2015-02-19 Thread Ben Pfaff
On Thu, Feb 19, 2015 at 11:45:08PM +0100, Thomas Graf wrote:
> On 02/19/15 at 11:16am, Ben Pfaff wrote:
> > diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
> > new file mode 100644
> > index 000..4c29562
> > --- /dev/null
> > +++ b/ovn/ovn-nb.ovsschema
> 
> The schema below has some whitespace / tab mixups.

Oops.  I've run untabify now.

> > @@ -0,0 +1,63 @@
> > +{
> > +"name": "OVN_Northbound",
> > +"tables": {
> > +"Logical_Switch": {
> > +"columns": {
> > +   "external_ids": {
> > +   "type": {"key": "string", "value": "string",
> > +"min": 0, "max": "unlimited",
> > +"Logical_Port": {
> > +"columns": {
> > +   "switch": {"type": {"key": {"type": "uuid",
> > +   "refTable": "Logical_Switch",
> > +   "refType": "strong"}}},
> > +   "router_port": {"type": {"key": {"type": "uuid",
> > +"refTable": 
> > "Logical_Router_Port",
> > +"refType": "strong"},
> > +   "min": 0,
> > +   "max": 1}},
> 
> What is the reason to hook the Logical_Router_Port to a Logical_Port
> and not to the Logical_Switch directly? It seems like the port_security
> does not apply to routers and the state should also be "up" at all
> times.

I've switched this back and forth a couple of times.  Maybe I'll put
it back again.

The one feature that I want to make sure can apply to router ports is
ACLs.  That isn't contingent on a Logical_Router_Port being a
Logical_Port, though, so it's not a deal-breaker.

> [...]
> 
> > +  
> > +Following are not well thought out:
> > +  
> > +
> > +  
> > +  learn
> > +
> > +  conntrack
> > +
> > +  with(field=value) { 
> > action, ... }
> > +  execute actions with temporary changes to 
> > fields
> > +
> > +  dec_ttl { action, ... } { 
> > action; ...}
> > +  
> > +decrement TTL; execute first set of actions if
> > +successful, second set if TTL decrement fails
> > +  
> > +
> > +  icmp_reply { action, ... 
> > }
> > +  generate ICMP reply from packet, execute 
> > action
> 
> Possible useful addition could be arp_respond to implement the L2 pop
> of OpenStack.

I thought that was in there, but now I see that it isn't.  I guess
that never made it off my whiteboard.  I'll add it.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH net] openvswitch: Fix net exit.

2015-02-19 Thread Andy Zhou
On Tue, Feb 17, 2015 at 11:23 AM, Pravin B Shelar  wrote:
> Open vSwitch allows moving internal vport to different namespace
> while still connected to the bridge. But when namespace deleted
> OVS does not detach these vports, that results in dangling
> pointer to netdevice which causes kernel panic as follows.
> This issue is fixed by detaching all ovs ports from the deleted
> namespace at net-exit.
>
> BUG: unable to handle kernel NULL pointer dereference at 0028
> IP: [] ovs_vport_locate+0x35/0x80 [openvswitch]
> Oops:  [#1] SMP
> Call Trace:
>  [] lookup_vport+0x21/0xd0 [openvswitch]
>  [] ovs_vport_cmd_get+0x59/0xf0 [openvswitch]
>  [] genl_family_rcv_msg+0x1bc/0x3e0
>  [] genl_rcv_msg+0x79/0xc0
>  [] netlink_rcv_skb+0xb9/0xe0
>  [] genl_rcv+0x2c/0x40
>  [] netlink_unicast+0x12d/0x1c0
>  [] netlink_sendmsg+0x34a/0x6b0
>  [] sock_sendmsg+0xa0/0xe0
>  [] ___sys_sendmsg+0x408/0x420
>  [] __sys_sendmsg+0x51/0x90
>  [] SyS_sendmsg+0x12/0x20
>  [] system_call_fastpath+0x12/0x17
>
> Reported-by: Assaf Muller 
> Fixes: 46df7b81454("openvswitch: Add support for network namespaces.")
> Signed-off-by: Pravin B Shelar 

Would a  similar issue exist for veth type of device?
After cleaning up the vports, would user space still make reference to ports?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH net] openvswitch: Fix net exit.

2015-02-19 Thread Pravin Shelar
On Thu, Feb 19, 2015 at 5:40 PM, Andy Zhou  wrote:
> On Tue, Feb 17, 2015 at 11:23 AM, Pravin B Shelar  wrote:
>> Open vSwitch allows moving internal vport to different namespace
>> while still connected to the bridge. But when namespace deleted
>> OVS does not detach these vports, that results in dangling
>> pointer to netdevice which causes kernel panic as follows.
>> This issue is fixed by detaching all ovs ports from the deleted
>> namespace at net-exit.
>>
>> BUG: unable to handle kernel NULL pointer dereference at 0028
>> IP: [] ovs_vport_locate+0x35/0x80 [openvswitch]
>> Oops:  [#1] SMP
>> Call Trace:
>>  [] lookup_vport+0x21/0xd0 [openvswitch]
>>  [] ovs_vport_cmd_get+0x59/0xf0 [openvswitch]
>>  [] genl_family_rcv_msg+0x1bc/0x3e0
>>  [] genl_rcv_msg+0x79/0xc0
>>  [] netlink_rcv_skb+0xb9/0xe0
>>  [] genl_rcv+0x2c/0x40
>>  [] netlink_unicast+0x12d/0x1c0
>>  [] netlink_sendmsg+0x34a/0x6b0
>>  [] sock_sendmsg+0xa0/0xe0
>>  [] ___sys_sendmsg+0x408/0x420
>>  [] __sys_sendmsg+0x51/0x90
>>  [] SyS_sendmsg+0x12/0x20
>>  [] system_call_fastpath+0x12/0x17
>>
>> Reported-by: Assaf Muller 
>> Fixes: 46df7b81454("openvswitch: Add support for network namespaces.")
>> Signed-off-by: Pravin B Shelar 
>
> Would a  similar issue exist for veth type of device?
> After cleaning up the vports, would user space still make reference to ports?

veth devices are of netdev type and are handled correctly on net exit.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v2 2/2] [RFC] ovn: Start work on design documentation.

2015-02-19 Thread Ben Pfaff
On Thu, Feb 19, 2015 at 02:25:18PM -0800, Gurucharan Shetty wrote:
> On Thu, Feb 19, 2015 at 11:16 AM, Ben Pfaff  wrote:
> > This commit adds preliminary design documentation for Open Virtual Network,
> > or OVN, a new OVS-based project to add support for virtual networking to
> > OVS, initially with OpenStack integration.
> >
> > This initial design has been influenced by many people, including (in
> > alphabetical order) Aaron Rosen, Chris Wright, Jeremy Stribling,
> > Justin Pettit, Ken Duda, Madhu Venugopal, Martin Casado, Pankaj Thakkar,
> > Russell Bryant, and Teemu Koponen.  All blunders, however, are due to my
> > own hubris.
> >
> > Signed-off-by: Ben Pfaff 

Thanks for finding typos, I've now fixed those.

> > +
> > +  Some CMS systems, including OpenStack, fully start a VM only when its
> > +  networking is ready.  To support this, ovn-nbd notices 
> > the
> > +  new row in the Bindings table, and pushes this upward by
> > +  updating the  
> > column
> > +  in the OVN Northbound database's  > db="OVN_NB"/>
> > +  table to indicate that the VIF is now up.  The CMS, if it uses this
> > +  feature, can then react by allowing the VM's execution to proceed.
> > +
> > +
> > +
> > +  On every hypervisor but the one where the VIF resides,
> > +  ovn-controller notices the new row in the
> > +  Bindings table.  This provides 
> > ovn-controller
> > +  the physical location of the logical port, so each instance updates 
> > the
> > +  OpenFlow tables of its switch (based on logical datapath flows in 
> > the OVN
> > +  DB Pipeline table) so that packets to and from the VIF 
> > can
> > +  be properly handled via tunnels.
> > +
> I wonder how much of a problem this delay in propagation of state to
> other hypervisors will be.
> At least with containers, where startup time is non-existent, it would
> mean that every container application written should be smart enough
> to retry connecting to another application in a different host.

I don't know how much of a problem it will be.  Do you (or anyone) have
an idea about that?  I kind of feel like proceeding on the assumption
that it will be OK, and then invoking a backup plan if not.

But I'm actually much more concerned about what seems to me a bigger
problem with containers.  As I understand it, containers get created and
destroyed (not just booted and shut down) very frequently.  Creating and
destroying a VM and its VIFs takes a long round trip through the whole
CMS.  I'm worried that's going to be slow.

> > +
> > +  The logical port name.  The name used here must match those used in 
> > the
> > +   > +  db="Open_vSwitch"/> in the  database's  > +  table="Interface" db="Open_vSwitch"/> table, because hypervisors use 
> >  > +  key="iface-id" table="Interface" column="external_ids"
> > +  db="Open_vSwitch"/> as a lookup key for logical ports.
> > +
> The above fits nicely if you want to run containers on hypervisors
> too. Since the document clearly mentions that we only want to start by
> integrating with CMS - openstack for VMs, that is alright. But, if we
> want to run multiple containers inside each tenant VM, and then
> provide individual IP addressability and policies on each of the
> container interface, then we probably should update
> IntegrationGuide.md to include the concept of "sub-interfaces" - where
> each VM VIF can represent multiple container interfaces. Openstack
> currently does not support such a concept, but it may make sense to
> have the infrastructure in OVN for it while designing. This will be
> useful in a world that contains containers, VMs and physical machines
> interconnected with each other.

I'd really appreciate it if you'd propose something to start that off,
then we can workshop it on the list.  I don't feel comfortable with
container networking yet.

> > +  
> > +
> > +  Each row in this table represents one ACL rule for the logical 
> > switch in
> > +  its  column.  The  
> > column for
> > +  the highest- matching row in this table
> > +  determines a packet's treatment.  If no row matches, packets are 
> > allowed
> > +  by default.  (Default-deny treatment is possible: add a rule with 
> >  > +  column="priority"/> 0, true as , 
> > and
> > +  deny as .)
> > +
> > +
> > +
> > +  The switch to which the ACL rule applies.
> > +
> > +
> Shouldn't a ACL rule apply to a logical port instead of a logical switch?

It does, or rather, it can match on the logical ingress and egress ports
within a particular logical switch.

I can see how this looks a little confusing.  Does writing it this way
help?


  The switch to which the ACL rule applies.  The expression in the
   column may match against logical ports
  within this switch.


I've tentatively made that change.
___
dev mailing list
dev@openvswitch.org
http

Re: [ovs-dev] [PATCH v2 2/2] [RFC] ovn: Start work on design documentation.

2015-02-19 Thread Ben Pfaff
On Thu, Feb 19, 2015 at 05:15:53PM -0800, Ben Pfaff wrote:
> On Thu, Feb 19, 2015 at 11:45:08PM +0100, Thomas Graf wrote:
> > On 02/19/15 at 11:16am, Ben Pfaff wrote:
> > > diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
> > > new file mode 100644
> > > index 000..4c29562
> > > --- /dev/null
> > > +++ b/ovn/ovn-nb.ovsschema
> > 
> > The schema below has some whitespace / tab mixups.
> 
> Oops.  I've run untabify now.
> 
> > > @@ -0,0 +1,63 @@
> > > +{
> > > +"name": "OVN_Northbound",
> > > +"tables": {
> > > +"Logical_Switch": {
> > > +"columns": {
> > > + "external_ids": {
> > > + "type": {"key": "string", "value": "string",
> > > +  "min": 0, "max": "unlimited",
> > > +"Logical_Port": {
> > > +"columns": {
> > > + "switch": {"type": {"key": {"type": "uuid",
> > > + "refTable": "Logical_Switch",
> > > + "refType": "strong"}}},
> > > + "router_port": {"type": {"key": {"type": "uuid",
> > > +  "refTable": 
> > > "Logical_Router_Port",
> > > +  "refType": "strong"},
> > > + "min": 0,
> > > + "max": 1}},
> > 
> > What is the reason to hook the Logical_Router_Port to a Logical_Port
> > and not to the Logical_Switch directly? It seems like the port_security
> > does not apply to routers and the state should also be "up" at all
> > times.
> 
> I've switched this back and forth a couple of times.  Maybe I'll put
> it back again.
> 
> The one feature that I want to make sure can apply to router ports is
> ACLs.  That isn't contingent on a Logical_Router_Port being a
> Logical_Port, though, so it's not a deal-breaker.

I remember now why I had a bit of trouble with this: a logical router
port needs a name so that ACLs can be written across it.  But since
there's only one logical router port (at most) per logical switch, we
can just give it a fixed name.  So I applied this incremental:

diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
index 649a668..c75c632 100644
--- a/ovn/ovn-nb.ovsschema
+++ b/ovn/ovn-nb.ovsschema
@@ -3,6 +3,10 @@
 "tables": {
 "Logical_Switch": {
 "columns": {
+"router_port": {"type": {"key": {"type": "uuid",
+ "refTable": 
"Logical_Router_Port",
+ "refType": "strong"},
+ "min": 0, "max": 1}},
 "external_ids": {
 "type": {"key": "string", "value": "string",
  "min": 0, "max": "unlimited",
@@ -11,11 +15,6 @@
 "switch": {"type": {"key": {"type": "uuid",
 "refTable": "Logical_Switch",
 "refType": "strong"}}},
-"router_port": {"type": {"key": {"type": "uuid",
- "refTable": 
"Logical_Router_Port",
- "refType": "strong"},
-"min": 0,
-"max": 1}},
 "name": {"type": "string"},
 "macs": {"type": {"key": "string",
   "min": 0,
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index 5238296..9c9a619 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -37,10 +37,15 @@
   column="switch"/> column points to its row.
 
 
-
-  So far, there is no per-logical switch configuration, so this table
-  only has the commmon columns.
-
+
+  
+The router port to which this logical switch is connected, or empty if
+this logical switch is not connected to any router.  A switch may be
+connected to at most one logical router, but this is not a significant
+restriction because logical routers may be connected into arbitrary
+topologies.
+  
+
 
 
   
@@ -58,19 +63,6 @@
   The logical switch to which the logical port is connected.
 
 
-
-  
-The router port to which this logical port is connected, or empty if
-this switch port is connected to a host instead of a router.
-  
-
-  
-At most one logical port in a given logical switch may be connected to
-a logical router.  (This is not a significant restriction because
-logical routers may be connected into arbitrary toplogies.)
-  
-
-
 
   The logical port name.  The name used here must match those used in the
   
   The packets that the ACL should match, in the same expression language
   used for the  column in
-  the OVN database's  table.
+  the OVN database's  table.  Match
+  

[ovs-dev] [PATCH v3] [RFC] ovn: Start work on design documentation.

2015-02-19 Thread Ben Pfaff
This commit adds preliminary design documentation for Open Virtual Network,
or OVN, a new OVS-based project to add support for virtual networking to
OVS, initially with OpenStack integration.

This initial design has been influenced by many people, including (in
alphabetical order) Aaron Rosen, Chris Wright, Gurucharan Shetty, Jeremy
Stribling, Justin Pettit, Ken Duda, Kevin Benton, Madhu Venugopal, Martin
Casado, Natasha Gude, Pankaj Thakkar, Russell Bryant, Teemu Koponen, and
Thomas Graf.  All blunders, however, are due to my own hubris.

Signed-off-by: Ben Pfaff 
---
v1->v2: Rebase.
v2->v3:
  - Multiple CMSes are possible.
  - Whitespace and typo fixes.
  - ovn.ovsschema: Gateway table is not a root table, other tables are.
  - ovn.xml: Talk about deleting rows on HV shutdown.
  - ovn-nb.xml: Clarify 'switch' column in ACL table.
  - ovn-nb.ovssechma: A Logical_Router_Port is no longer a Logical_Port.
  - ovn.xml: Add action for generating ARP.
  - ovn-nb.xml: Add allow-related action for security group support.
---
 Makefile.am|   1 +
 configure.ac   |   3 +-
 ovn/automake.mk|  75 +++
 ovn/ovn-architecture.7.xml | 338 ++
 ovn/ovn-controller.8.in|  41 
 ovn/ovn-nb.ovsschema   |  62 ++
 ovn/ovn-nb.xml | 245 ++
 ovn/ovn.ovsschema  |  50 +
 ovn/ovn.xml| 497 +
 9 files changed, 1311 insertions(+), 1 deletion(-)
 create mode 100644 ovn/automake.mk
 create mode 100644 ovn/ovn-architecture.7.xml
 create mode 100644 ovn/ovn-controller.8.in
 create mode 100644 ovn/ovn-nb.ovsschema
 create mode 100644 ovn/ovn-nb.xml
 create mode 100644 ovn/ovn.ovsschema
 create mode 100644 ovn/ovn.xml

diff --git a/Makefile.am b/Makefile.am
index 0480d20..699a580 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -370,3 +370,4 @@ include tutorial/automake.mk
 include vtep/automake.mk
 include datapath-windows/automake.mk
 include datapath-windows/include/automake.mk
+include ovn/automake.mk
diff --git a/configure.ac b/configure.ac
index d2d02ca..795f876 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1,4 +1,4 @@
-# Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
+# Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -182,6 +182,7 @@ dnl This makes sure that include/openflow gets created in 
the build directory.
 AC_CONFIG_COMMANDS([include/openflow/openflow.h.stamp])
 
 AC_CONFIG_COMMANDS([utilities/bugtool/dummy], [:])
+AC_CONFIG_COMMANDS([ovn/dummy], [:])
 
 m4_ifdef([AM_SILENT_RULES], [AM_SILENT_RULES])
 
diff --git a/ovn/automake.mk b/ovn/automake.mk
new file mode 100644
index 000..3889d56
--- /dev/null
+++ b/ovn/automake.mk
@@ -0,0 +1,75 @@
+# OVN schema and IDL
+EXTRA_DIST += ovn/ovn.ovsschema
+pkgdata_DATA += ovn/ovn.ovsschema
+
+# OVN E-R diagram
+#
+# If "python" or "dot" is not available, then we do not add graphical diagram
+# to the documentation.
+if HAVE_PYTHON
+if HAVE_DOT
+ovn/ovn.gv: ovsdb/ovsdb-dot.in ovn/ovn.ovsschema
+   $(AM_V_GEN)$(OVSDB_DOT) --no-arrows $(srcdir)/ovn/ovn.ovsschema > $@
+ovn/ovn.pic: ovn/ovn.gv ovsdb/dot2pic
+   $(AM_V_GEN)(dot -T plain < ovn/ovn.gv | $(PERL) $(srcdir)/ovsdb/dot2pic 
-f 3) > $@.tmp && \
+   mv $@.tmp $@
+OVN_PIC = ovn/ovn.pic
+OVN_DOT_DIAGRAM_ARG = --er-diagram=$(OVN_PIC)
+DISTCLEANFILES += ovn/ovn.gv ovn/ovn.pic
+endif
+endif
+
+# OVN schema documentation
+EXTRA_DIST += ovn/ovn.xml
+DISTCLEANFILES += ovn/ovn.5
+man_MANS += ovn/ovn.5
+ovn/ovn.5: \
+   ovsdb/ovsdb-doc ovn/ovn.xml ovn/ovn.ovsschema $(OVN_PIC)
+   $(AM_V_GEN)$(OVSDB_DOC) \
+   $(OVN_DOT_DIAGRAM_ARG) \
+   --version=$(VERSION) \
+   $(srcdir)/ovn/ovn.ovsschema \
+   $(srcdir)/ovn/ovn.xml > $@.tmp && \
+   mv $@.tmp $@
+
+# OVN northbound schema and IDL
+EXTRA_DIST += ovn/ovn-nb.ovsschema
+pkgdata_DATA += ovn/ovn-nb.ovsschema
+
+# OVN northbound E-R diagram
+#
+# If "python" or "dot" is not available, then we do not add graphical diagram
+# to the documentation.
+if HAVE_PYTHON
+if HAVE_DOT
+ovn/ovn-nb.gv: ovsdb/ovsdb-dot.in ovn/ovn-nb.ovsschema
+   $(AM_V_GEN)$(OVSDB_DOT) --no-arrows $(srcdir)/ovn/ovn-nb.ovsschema > $@
+ovn/ovn-nb.pic: ovn/ovn-nb.gv ovsdb/dot2pic
+   $(AM_V_GEN)(dot -T plain < ovn/ovn-nb.gv | $(PERL) 
$(srcdir)/ovsdb/dot2pic -f 3) > $@.tmp && \
+   mv $@.tmp $@
+OVN_NB_PIC = ovn/ovn-nb.pic
+OVN_NB_DOT_DIAGRAM_ARG = --er-diagram=$(OVN_NB_PIC)
+DISTCLEANFILES += ovn/ovn-nb.gv ovn/ovn-nb.pic
+endif
+endif
+
+# OVN northbound schema documentation
+EXTRA_DIST += ovn/ovn-nb.xml
+DISTCLEANFILES += ovn/ovn-nb.5
+man_MANS += ovn/ovn-nb.5
+ovn/ovn-nb.5: \
+   ovsdb/ovsdb-doc ovn/ovn-nb.xml ovn/ovn-nb.ovsschema $(OVN_NB_PIC)
+   $(AM_V_GEN)$(OVSDB_