from:"Babu Shanmugam"

[ovs-dev] [PATCH v1] ovn: Cache mac bindings from broadcasted dynamic ARP responses

2016-12-28 Thread Babu Shanmugam

This patch attempts to avoid the usage of MAC_Binding table.

Dynamic ARP response originates from the logical ports with "unknown"
address. When the ARP resolution is requested via a logical
router datapath, ARP response will be delivered to the logical router
port of the switch. Since the logical router is distributed, the
response will never reach the other chassis from where the ARP resolution
is requested.

This is done using a OVN logical action bcast2lr() which adds an action
to send the packet to a specific multicast group for the logical router
datapath which will ultimately be broadcasted to all the other chassis.

Imagine two hypervisors hv0 and hv1, switches ls0 and ls1 are connected
to lr. ls1 has a logical port lspu with "unknown" destination and
bound to hv1. When a dynamic address is requested to be resolved from
ls0 in hv0, following happens;

1. The ARP request is routed to ls1 and will be sent from hv0 to hv1
   with the source mac address being that of ls1's endpoint to lr.
2. In hv1, the packet will be received by lspu and the ARP response
   will be sent to the source of the ARP request.
3. The ARP response will be processed by bcast2lr logical action which
   in hv1 will be converted to a flow, similar to

   priority=100,arp,reg14=0x1,metadata=0x2,dl_dst=f0:00:00:00:01:00,arp_op=2
   actions=push:OXM_OF_METADATA[],push:NXM_NX_REG15[],push:NXM_NX_REG14[],
   
load:0x3->OXM_OF_METADATA[],load:0x1->NXM_NX_REG14[],load:0xfffd->NXM_NX_REG15[],
   resubmit(,32),pop:NXM_NX_REG14[],pop:NXM_NX_REG15[],pop:OXM_OF_METADATA[],
   resubmit(,30)

   bcast2lr actions modifies the MFF_LOG_INPORT (REG14) to the port id of
   ls1's endpoint to lr and MFF_LOG_DATAPATH (METADATA) to the lr's datapath
   id and submits to table 32 which handles the multicast group for lr's
   datapath. It again reverts MFF_LOG_INPORT and MFF_LOG_DATAPATH values and
   submits it to table 30 which will enables the lr in hv1 to receive this ARP
   response.

4. The above flow will only be present in the chassis that has a logical
   port having "unknown" address. In hv0, bcast2lr() will have no actions,
   hence it will look like

   priority=100,arp,reg14=0x1,metadata=0x2,dl_dst=f0:00:00:00:01:00,arp_op=2
   actions=resubmit(,30)

5. In hv1, the packet is sent to a multicast group 0xfffd which will take care
   of the broadcast to all the chassis. The chassis which are supposed to 
receive
   the broadcasted ARP response from the multicast group 0xfffd (hv0),
   will have one additional flow in table 0 which looks like

   priority=150,tun_id=0x3,in_port=14
   actions=move:NXM_NX_TUN_ID[0..23]->OXM_OF_METADATA[0..23],
   move:NXM_NX_TUN_METADATA0[16..30]->NXM_NX_REG14[0..14],
   move:NXM_NX_TUN_METADATA0[0..15]->NXM_NX_REG15[0..15],resubmit(,16)

   The actions are all similar to the actions of the other flows in table0. This
   flow is at a higher priority and has a check for the tunnel id equivalent to
   lr's datapath id.

   This flow will not be present in hv1 which will not accept any message from
   this multicast group of lr's datapath.

6. Now that the packet has reached hv0, it will be processed by put_arp()
   logical action which caches the MAC binding in memory and updates the 
openflow
   table used for the dynamic MAC resolution.

Signed-off-by: Babu Shanmugam <bscha...@redhat.com>
---
 include/ovn/actions.h   |  23 ++-
 ovn/controller/lflow.c  | 118 +---
 ovn/controller/lflow.h  |   3 +-
 ovn/controller/ovn-controller.c |   8 +--
 ovn/controller/physical.c   |  74 +--
 ovn/controller/pinctrl.c|  94 +
 ovn/controller/pinctrl.h|  13 +++-
 ovn/lib/actions.c   | 130 
 ovn/northd/ovn-northd.c |  86 --
 ovn/ovn-architecture.7.xml  |   6 +-
 ovn/ovn-sb.ovsschema|  11 +---
 ovn/ovn-sb.xml  | 128 ++-
 ovn/utilities/ovn-sbctl.c   |   4 --
 ovn/utilities/ovn-trace.c   |  45 +-
 tests/ovn.at| 103 ---
 15 files changed, 494 insertions(+), 352 deletions(-)

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 0bf6145..9a35e90 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -68,7 +68,8 @@ struct simap;
 OVNACT(PUT_ND,ovnact_put_mac_bind)  \
 OVNACT(PUT_DHCPV4_OPTS, ovnact_put_dhcp_opts)   \
 OVNACT(PUT_DHCPV6_OPTS, ovnact_put_dhcp_opts)   \
-OVNACT(SET_QUEUE,   ovnact_set_queue)
+OVNACT(SET_QUEUE,   ovnact_set_queue)   \
+OVNACT(BCAST2LR,ovnact_bcast2lr)
 
 /* enum ovnact_type, with a member OVNACT_ for each action. */
 enum OVS_PACKED_ENUM ovnact_type {
@@ -234,6 +235,18 @@ struct ovnact_set_queue {
 uint16_t queue_id;
 };
 
+#defin

Re: [ovs-dev] [PATCH V2] Fix ovndb_servers master and VirtualIP are not on the same node.

2016-12-06 Thread Babu Shanmugam


+ Andrew.

Hi Andrew,

Please see below. Your comments are most welcome.

--

Babu

On Saturday 03 December 2016 01:08 PM, Guoshuai Li wrote:

PCS man page says role=Stopped/Started/Master/Slave.
A role can be master or slave (if no role is specified, it defaults to 
'started').

Command line "$pcs constraint colocation add ovndb_servers-master with master 
VirtualIP"
means that the Started role node of ovsdb follows the master node of VirtualIP.
But we actually want the ovsdb master node to follow the VirtualIP's Started 
node.
---
  IntegrationGuide.rst | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/IntegrationGuide.rst b/IntegrationGuide.rst
index 8c3cca7..1a8e82c 100644
--- a/IntegrationGuide.rst
+++ b/IntegrationGuide.rst
@@ -259,5 +259,5 @@ with the active server.
  
  pcs constraint order VirtualIP then ovndb_servers-master
  
-pcs constraint colocation add ovndb_servers-master with master VirtualIP \

+pcs constraint colocation add master ovndb_servers-master with VirtualIP \
  score=INFINITY


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

Re: [ovs-dev] [PATCH] utilities: Use FQDN for external_ids:hostname in Openvswitch table

2016-11-22 Thread Babu Shanmugam




On Wednesday 23 November 2016 04:01 AM, Ben Pfaff wrote:

On Tue, Nov 15, 2016 at 05:27:22PM +0530, bscha...@redhat.com wrote:

From: Babu Shanmugam <bscha...@redhat.com>

Openstack compute manager uses FQDN to check for the hypervisors to
which the ports are bound.

Without this fix, no instances can be launched as the hypervisor's hostname
mismatches.

Signed-off-by: Babu Shanmugam <bscha...@redhat.com>

Can you document in vswitch.xml that this is a fully qualified domain
name, and that before OVS 2.7 it was not fully qualified?  (Or, before
2.6.2, if you want it backported?)


Sure, Ben


Also, there is some possibility that some installations relied on this
being non-fully-qualified, and if we get bug reports about that change
then we'd probably have to revert it.


I understand.


Thanks,

Ben.


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

Re: [ovs-dev] [PATCH] rhel: fix ovn-common rpm installation failure

2016-11-21 Thread Babu Shanmugam




On Tuesday 22 November 2016 04:17 AM, Lance Richardson wrote:

The directory/usr/lib/ocf/  does not exist if the pacemaker
package has not been installed, which causes installation of the
ovn-common rpm to fail on "mkdir /usr/lib/ocf/resource.d/ovn".
Allow for the possibility that /usr/lib/ocf does not exist by
using "mkdir -p".

Fixes: a4245b7869c8 ("ovn: Add ovn db servers ocf script in fedora packager")
Signed-off-by: Lance Richardson<lrich...@redhat.com>
---
  rhel/openvswitch-fedora.spec.in | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


Acked-by: Babu Shanmugam <bscha...@redhat.com>
Tested-by: Babu Shanmugam <bscha...@redhat.com>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [RFC] ovn: Securing from compromised host - removal of MAC_Binding

2016-11-17 Thread Babu Shanmugam

We have been working on a series of changes to make ovn-northd the sole 
writer to the OVN southbound database and turn ovn-controller into a 
read-only client of this database.  One of the changes required is to 
drop the usage of the MAC_Bindings table.



Approaches:


1) Drop the use of the table and keep the MAC bindings cache local to 
each ovn-controller.



Unfortunately, this isn’t good enough.  This works only for the case 
where the chassis that sends the ARP request is the same one that 
processes the ARP response, which is true when the ARP request is sent 
from an L3 gateway to a physical network it is connected to.



We may also need to do ARP requests on virtual networks with an OVN 
distributed router.  In this case, the distributed router may generate 
the ARP request on one host, but the response will be processed by that 
same logical router on a different host.  When that happens, the result 
is not available on the hypervisor that initiated the request and needs 
the result.



2) When the destination for an ARP reply is an OVN distributed router, 
broadcast the response to all instances of the logical router so that 
the result is available in the local cache of each router instance.



Whenever the logical switch port of type ‘router’ receives an arp reply, 
it will broadcast it to its corresponding router’s datapath. By this way 
all the hypervisors will receive the ARP reply and can cache them.


Right now, the code is such that the arp reply on logical router 
datapath alone are processed. We need to add code to add a flow that 
process arp reply on the logical switch ports and broadcast them on the 
router datapath. Such flows need to be present only in hypervisors that 
has ports with “unknown” address and the switch to which such ports 
belong are connected to logical routers.



What do you all think of this approach?


Thanks,

Babu, Russell

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

[ovs-dev] [PATCH v6 4/4] ovn: Add ovndb servers ocf script in debian packager

2016-11-16 Thread Babu Shanmugam

The OCF script will be present in the ovn-common package and installed
in the openvswitch scripts folder and a symbolic link to this file will
be created in the OCF resources folder.

The OCF resource agent name for this resource is ocf:ovn:ovndb-servers

Signed-off-by: Babu Shanmugam <bscha...@redhat.com>
---
 debian/automake.mk |  2 ++
 debian/ovn-common.install  |  1 +
 debian/ovn-common.postinst | 24 
 debian/ovn-common.postrm   | 23 +++
 4 files changed, 50 insertions(+)
 create mode 100644 debian/ovn-common.postinst
 create mode 100644 debian/ovn-common.postrm

diff --git a/debian/automake.mk b/debian/automake.mk
index 2da7055..67b9c6b 100644
--- a/debian/automake.mk
+++ b/debian/automake.mk
@@ -60,6 +60,8 @@ EXTRA_DIST += \
debian/ovn-controller-vtep.manpages \
debian/ovn-common.install \
debian/ovn-common.manpages \
+   debian/ovn-common.postinst \
+   debian/ovn-common.postrm \
debian/ovn-docker.install \
debian/ovn-host.dirs \
debian/ovn-host.init \
diff --git a/debian/ovn-common.install b/debian/ovn-common.install
index ba9c343..99378af 100644
--- a/debian/ovn-common.install
+++ b/debian/ovn-common.install
@@ -2,3 +2,4 @@ usr/bin/ovn-nbctl
 usr/bin/ovn-sbctl
 usr/bin/ovn-trace
 usr/share/openvswitch/scripts/ovn-ctl
+usr/share/openvswitch/scripts/ovndb-servers.ocf
diff --git a/debian/ovn-common.postinst b/debian/ovn-common.postinst
new file mode 100644
index 000..588044f
--- /dev/null
+++ b/debian/ovn-common.postinst
@@ -0,0 +1,24 @@
+#!/bin/sh
+# postinst script for ovn-common
+#
+# see: dh_installdeb(1)
+
+set -e
+
+case "$1" in
+configure)
+mkdir -p /usr/lib/ocf/resource.d/ovn
+ln -sf /usr/share/openvswitch/scripts/ovndb-servers.ocf 
/usr/lib/ocf/resource.d/ovn/ovndb-servers
+;;
+abort-upgrade|abort-remove|abort-deconfigure)
+;;
+
+*)
+echo "postinst called with unknown argument \`$1'" >&2
+exit 1
+;;
+esac
+
+#DEBHELPER#
+
+exit 0
diff --git a/debian/ovn-common.postrm b/debian/ovn-common.postrm
new file mode 100644
index 000..9face72
--- /dev/null
+++ b/debian/ovn-common.postrm
@@ -0,0 +1,23 @@
+#!/bin/sh
+# postrm script for openvswitch-testcontroller
+#
+# see: dh_installdeb(1)
+
+set -e
+
+case "$1" in
+purge|remove)
+rm -rf /usr/lib/ocf/resource.d/ovn
+;;
+upgrade|failed-upgrade|abort-install|abort-upgrade|disappear)
+;;
+
+*)
+echo "postrm called with unknown argument \`$1'" >&2
+exit 1
+;;
+esac
+
+#DEBHELPER#
+
+exit 0
-- 
1.9.1

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

[ovs-dev] [PATCH v6 2/4] ovn: OCF script for OVN OVSDB servers

2016-11-16 Thread Babu Shanmugam

Co-authored-by: Numan Siddique <nusid...@redhat.com>
Signed-off-by: Numan Siddique <nusid...@redhat.com>
Co-authored-by: Andrew Beekhof <abeek...@redhat.com>
Signed-off-by: Andrew Beekhof <abeek...@redhat.com>
Signed-off-by: Babu Shanmugam <bscha...@redhat.com>
---
 IntegrationGuide.rst|  68 
 ovn/utilities/automake.mk   |   6 +-
 ovn/utilities/ovndb-servers.ocf | 356 
 3 files changed, 428 insertions(+), 2 deletions(-)
 create mode 100755 ovn/utilities/ovndb-servers.ocf

diff --git a/IntegrationGuide.rst b/IntegrationGuide.rst
index 4a591fa..c02bfbe 100644
--- a/IntegrationGuide.rst
+++ b/IntegrationGuide.rst
@@ -193,3 +193,71 @@ can be used:
 ::
 
 $ ovs-vsctl set Interface eth0 external-ids:iface-id='"${UUID}"'
+
+
+HA for OVN DB servers using pacemaker
+-
+
+The ovsdb servers can work in either active or backup mode. In backup mode, db
+server will be connected to an active server and replicate the active servers
+contents. At all times, the data can be transacted only from the active server.
+When the active server dies for some reason, entire OVN operations will be
+stalled.
+
+`Pacemaker <http://clusterlabs.org/pacemaker.html>`_ is a cluster resource
+manager which can manage a defined set of resource across a set of clustered
+nodes. Pacemaker manages the resource with the help of the resource agents.
+One among the resource agent is
+`OCF <http://www.linux-ha.org/wiki/OCF_Resource_Agents`_
+
+OCF is nothing but a shell script which accepts a set of actions and returns an
+appropriate status code.
+
+With the help of the OCF resource agent ovn/utilities/ovndb-servers.ocf, one
+can defined a resource for the pacemaker such that pacemaker will always
+maintain one running active server at any time.
+
+After creating a pacemaker cluster, use the following commands to create
+one active and multiple backup servers for OVN databases.
+
+::
+
+pcs resource create ovndb_servers ocf:ovn:ovndb-servers \
+ master_ip=x.x.x.x \
+ ovn_ctl= \
+ op monitor interval="10s"
+
+pcs resource master ovndb_servers-master ovndb_servers \
+meta notify="true"
+
+The `master_ip` and `ovn_ctl` are the parameters that will be used by the
+OCF script. `ovn_ctl` is optional, if not given, it assumes a default value of
+/usr/share/openvswitch/scripts/ovn-ctl. `master_ip` is the IP address on which
+the active database server is expected to be listening.
+
+Whenever the active server dies, pacemaker is responsible to promote one of
+the backup servers to be active. Both ovn-controller and ovn-northd needs the
+ip-address at which the active server is listening. With pacemaker changing the
+node at which the active server is run, it is not efficient to instruct all the
+ovn-controllers and the ovn-northd to listen to the latest active server's ip-
+address
+
+This problem can be solved by using a native ocf resource agent
+`ocf:heartbeat:IPaddr2`. The IPAddr2 resource agent is just a resource with an
+ip-address. When we colocate this resource with the active server, pacemaker
+will enable the active server to be connected with a single ip-address all the
+time. This is the ip-address that needs to be given as the parameter while
+creating the `ovndb_servers` resource.
+
+Use the following command to create the IPAddr2 resource and colocate it
+with the active server.
+
+::
+
+pcs resource create VirtualIP ocf:heartbeat:IPaddr2 ip=x.x.x.x \
+op monitor interval=30s
+
+pcs constraint order VirtualIP then ovndb_servers-master
+
+pcs constraint colocation add ovndb_servers-master with master VirtualIP \
+score=INFINITY
diff --git a/ovn/utilities/automake.mk b/ovn/utilities/automake.mk
index b03d125..164cdda 100644
--- a/ovn/utilities/automake.mk
+++ b/ovn/utilities/automake.mk
@@ -1,5 +1,6 @@
 scripts_SCRIPTS += \
-ovn/utilities/ovn-ctl
+ovn/utilities/ovn-ctl \
+ovn/utilities/ovndb-servers.ocf
 
 man_MANS += \
 ovn/utilities/ovn-ctl.8 \
@@ -20,7 +21,8 @@ EXTRA_DIST += \
 ovn/utilities/ovn-docker-overlay-driver \
 ovn/utilities/ovn-docker-underlay-driver \
 ovn/utilities/ovn-nbctl.8.xml \
-ovn/utilities/ovn-trace.8.xml
+ovn/utilities/ovn-trace.8.xml \
+ovn/utilities/ovndb-servers.ocf
 
 DISTCLEANFILES += \
 ovn/utilities/ovn-ctl.8 \
diff --git a/ovn/utilities/ovndb-servers.ocf b/ovn/utilities/ovndb-servers.ocf
new file mode 100755
index 000..1fc61a7
--- /dev/null
+++ b/ovn/utilities/ovndb-servers.ocf
@@ -0,0 +1,356 @@
+#!/bin/bash
+
+: ${OCF_FUNCTIONS_DIR=${OCF_ROOT}/lib/heartbeat}
+. ${OCF_FUNCTIONS_DIR}/ocf-shellfuncs
+: ${OVN_CTL_DEFAULT="/usr/share/openvswitch/scripts/ovn-ctl"}
+CRM_MASTER="${HA_SBIN_DIR}/crm_master -l reboot"
+CRM_ATTR_REPL_INFO="${HA_SBIN_DIR}/crm_attribute --type crm_config --name 
OVN_REPL_

[ovs-dev] [PATCH v6 0/4] High availability support for OVN DB servers using pacemaker

2016-11-16 Thread Babu Shanmugam

v5 -> v6:
  - Added more text to the man page of ovn-ctl and some more information to
the integration guide.

This patch contains changes required to run a pacemaker resource agent
to manage OVN db servers in active/standby mode in a HA cluster.

Babu Shanmugam (4):
  ovn: ovn-ctl support for HA ovn DB servers
  ovn: OCF script for OVN OVSDB servers
  ovn: Add ovn db servers ocf script in fedora packager
  ovn: Add ovndb servers ocf script in debian packager

 IntegrationGuide.rst|  68 
 debian/automake.mk  |   2 +
 debian/ovn-common.install   |   1 +
 debian/ovn-common.postinst  |  24 +++
 debian/ovn-common.postrm|  23 +++
 ovn/utilities/automake.mk   |   6 +-
 ovn/utilities/ovn-ctl   | 173 ---
 ovn/utilities/ovn-ctl.8.xml |  43 +
 ovn/utilities/ovndb-servers.ocf | 356 
 rhel/openvswitch-fedora.spec.in |   8 +
 10 files changed, 677 insertions(+), 27 deletions(-)
 create mode 100644 debian/ovn-common.postinst
 create mode 100644 debian/ovn-common.postrm
 create mode 100755 ovn/utilities/ovndb-servers.ocf

-- 
1.9.1

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

Re: [ovs-dev] [PATCH] rhel: Environment file option for northd service

2016-11-16 Thread Babu Shanmugam




On Wednesday 16 November 2016 11:45 AM, Babu Shanmugam wrote:




On Tuesday 15 November 2016 08:11 PM, Russell Bryant wrote:



On Tue, Nov 15, 2016 at 6:19 AM, <bscha...@redhat.com 
<mailto:bscha...@redhat.com>> wrote:


    From: Babu Shanmugam <bscha...@redhat.com
<mailto:bscha...@redhat.com>>

Since the northd service starts the DB servers as well, it will be
better to have an environment file options in the systemd unit
file for
northd service.
The environment file is expected to define NORTHD_OPTS
which will have additional parameters to be passed to ovn-ctl script
that starts ovn-northd.

Signed-off-by: Babu Shanmugam <bscha...@redhat.com
<mailto:bscha...@redhat.com>>
---
 rhel/usr_lib_systemd_system_ovn-northd.service | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/rhel/usr_lib_systemd_system_ovn-northd.service
b/rhel/usr_lib_systemd_system_ovn-northd.service
index 5b3b03a..12230e9 100644
--- a/rhel/usr_lib_systemd_system_ovn-northd.service
+++ b/rhel/usr_lib_systemd_system_ovn-northd.service
@@ -7,6 +7,7 @@ After=openvswitch.service
 [Service]
 Type=oneshot
 RemainAfterExit=yes
+EnvironmentFile=-/etc/sysconfig/ovn-northd
 Environment=OVS_RUNDIR=%t/openvswitch OVS_DBDIR=/var/lib/openvswitch
-ExecStart=/usr/share/openvswitch/scripts/ovn-ctl start_northd
+ExecStart=/usr/share/openvswitch/scripts/ovn-ctl start_northd
$NORTHD_OPTS
 ExecStop=/usr/share/openvswitch/scripts/ovn-ctl stop_northd


I'm not sure this is necessary.  I believe something close enough is 
possible with systemd already.


https://fedoraproject.org/wiki/Systemd#How_do_I_customize_a_unit_file.2F_add_a_custom_unit_file.3F

or:

"Example 2. Overriding vendor settings"
https://www.freedesktop.org/software/systemd/man/systemd.unit.html

For ovn-northd, you would create a directory and config file, 
/etc/systemd/system/ovn-northd.d/local.conf, with contents like:


[System]
Environment=MY_ENV_VAR=VALUE "MY_ENV_VAR2=VALUE 2"



Thanks for the information, Russell. This patch can be abandoned.


I was trying to use it like you said. But, I had to override the 
ExecStart as not all the options are read from the environment variable 
in the ovn-ctl script.
Do you think its better to add an EnvironmentFile option as proposed in 
this patch or to override ExecStart using the .conf file?





--
Russell Bryant




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

Re: [ovs-dev] [PATCH] rhel: Environment file option for northd service

2016-11-15 Thread Babu Shanmugam




On Tuesday 15 November 2016 08:11 PM, Russell Bryant wrote:



On Tue, Nov 15, 2016 at 6:19 AM, <bscha...@redhat.com 
<mailto:bscha...@redhat.com>> wrote:


    From: Babu Shanmugam <bscha...@redhat.com
<mailto:bscha...@redhat.com>>

Since the northd service starts the DB servers as well, it will be
better to have an environment file options in the systemd unit
file for
northd service.
The environment file is expected to define NORTHD_OPTS
which will have additional parameters to be passed to ovn-ctl script
that starts ovn-northd.

Signed-off-by: Babu Shanmugam <bscha...@redhat.com
<mailto:bscha...@redhat.com>>
---
 rhel/usr_lib_systemd_system_ovn-northd.service | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/rhel/usr_lib_systemd_system_ovn-northd.service
b/rhel/usr_lib_systemd_system_ovn-northd.service
index 5b3b03a..12230e9 100644
--- a/rhel/usr_lib_systemd_system_ovn-northd.service
+++ b/rhel/usr_lib_systemd_system_ovn-northd.service
@@ -7,6 +7,7 @@ After=openvswitch.service
 [Service]
 Type=oneshot
 RemainAfterExit=yes
+EnvironmentFile=-/etc/sysconfig/ovn-northd
 Environment=OVS_RUNDIR=%t/openvswitch OVS_DBDIR=/var/lib/openvswitch
-ExecStart=/usr/share/openvswitch/scripts/ovn-ctl start_northd
+ExecStart=/usr/share/openvswitch/scripts/ovn-ctl start_northd
$NORTHD_OPTS
 ExecStop=/usr/share/openvswitch/scripts/ovn-ctl stop_northd


I'm not sure this is necessary.  I believe something close enough is 
possible with systemd already.


https://fedoraproject.org/wiki/Systemd#How_do_I_customize_a_unit_file.2F_add_a_custom_unit_file.3F

or:

"Example 2. Overriding vendor settings"
https://www.freedesktop.org/software/systemd/man/systemd.unit.html

For ovn-northd, you would create a directory and config file, 
/etc/systemd/system/ovn-northd.d/local.conf, with contents like:


[System]
Environment=MY_ENV_VAR=VALUE "MY_ENV_VAR2=VALUE 2"



Thanks for the information, Russell. This patch can be abandoned.


--
Russell Bryant


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

Re: [ovs-dev] [PATCH v5 1/4] ovn: ovn-ctl support for HA ovn DB servers

2016-11-14 Thread Babu Shanmugam




On Friday 11 November 2016 02:18 PM, Andy Zhou wrote:



On Mon, Nov 7, 2016 at 11:55 PM, Babu Shanmugam <bscha...@redhat.com 
<mailto:bscha...@redhat.com>> wrote:




On Monday 07 November 2016 06:49 PM, Andy Zhou wrote:

This version is better, I am able to apply them. Thanks.

I got the system running, but managed to get system into a state
where both machines (centos and centos2)
are running the ovsdb in a backup mode. The output of "pcs
status" shows an error message, but the message is not
very helpful.  Any suggestion on how to debug this?

root@centos:/# pcs status
Cluster name: mycluster
Last updated: Mon Nov  7 05:12:06 2016Last change: Mon Nov  7
05:08:24 2016 by root via cibadmin on centos
Stack: corosync
Current DC: centos2 (version 1.1.13-10.el7_2.4-44eb2dd) -
partition with quorum
2 nodes and 3 resources configured

Node centos: standby
Online: [ centos2 ]

Full list of resources:

 virtip(ocf::heartbeat:IPaddr):Started centos2
 Master/Slave Set: ovndb_servers_master [ovndb_servers]
 Stopped: [ centos centos2 ]

Failed Actions:
* ovndb_servers_start_0 on centos2 'unknown error' (1): call=18,
status=Timed Out, exitreason='none',
last-rc-change='Mon Nov  7 02:28:07 2016', queued=0ms,
exec=30002ms


PCSD Status:
  centos: Online
  centos2: Online

Daemon Status:
  corosync: active/enabled
  pacemaker: active/enabled
  pcsd: active/enabled


root@centos:/# pcs config
Cluster Name: mycluster
Corosync Nodes:
 centos centos2
Pacemaker Nodes:
 centos centos2

Resources:
 Resource: virtip (class=ocf provider=heartbeat type=IPaddr)
  Attributes: ip=192.168.122.200 cidr_netmask=24
  Operations: start interval=0s timeout=20s
(virtip-start-interval-0s)
  stop interval=0s timeout=20s (virtip-stop-interval-0s)
  monitor interval=30s (virtip-monitor-interval-30s)
 Master: ovndb_servers_master
  Meta Attrs: notify=true
  Resource: ovndb_servers (class=ocf provider=ovn type=ovndb-servers)
   Attributes: master_ip=192.168.122.200


Andy, you don't seem to have defined an attribute for ovn_ctl. It
means, the ovn-ctl script will be assumed to be present in
/usr/share/openvswitch/scripts/ovn-ctl. Can you check if you have
ovn-ctl at the correct location?

Yes, The script was installed there.--db-nb-sync-from-addr=

If not, please define an attribute similar to master_ip and name
it ovn_ctl and point that to the correct location of ovn-ctl ?

The document says "ovn-ctl" is optional. I now changed to have it 
fully specified, but makes no difference. There are some log 
information towards the end of email if they help. Overall, it could 
just be something weird about my system, I am not sure it will
be worth while to track it down.  On the other hand, I will be happy 
to provide more information about the my setup in case they

are useful.


Andy, can you please try to manually start the db servers using ovn-ctl 
and see if they are really started. I would use the following commands 
to learn that.


/usr/share/openvswitch/scripts/ovn-ctl 
--db-nb-sync-from-addr=192.0.2.254 --db-nb-sync-from-addr=192.0.2.254 
start_ovsdb

/usr/share/openvswitch/scripts/ovn-ctl status_ovnsb
/usr/share/openvswitch/scripts/ovn-ctl status_ovnnb

The last two commands should print 'running/backup'.

There are some log messages added in the ocf script in case the start 
funcitonality fails by timing out. You should be able to see some print 
starting with "ovndb_servers: After starting ovsdb, status is". These 
log messages should be present in the pacemaker logs.



Is the user expected to populate those files by hand?  If yes,
what IP address should be used? The floating IP?


This file will have to be populated by the user, only when the
user wants ovn-northd to connect to a different set of DB urls,
other than unix sockets in that same machine.
The IP address depends  on the setup. The pacemaker script uses
the master-ip address that you supply to the OCF resource as an
attribute.

Thanks. Should this be added to IntegrationGuide.rst?



I have added the need of IPAddr2 resource in IntegrationGuide.rst. The 
new options are in ovn-ctl man page. Is there something specific, that 
you feel missed out in the documentation?



More logs..

root@centos:~# ls -l /usr/share/openvswitch/scripts/ovn-ctl
-rwxr-xr-x. 1 root root 15539 Nov  7 02:12 
/usr/share/openvswitch/scripts/ovn-ctl


Resources:
 Resource: virtip (class=ocf provider=heartbeat type=IPaddr)
  Attributes: ip=192.168.122.200 cidr_netmask=24
  Operations: start interval=0s timeout=20s (virtip-start-interval-0s)
  stop interval=0s timeout=20s (virtip-stop-interval-0s)
  monitor

Re: [ovs-dev] [PATCH v5 1/4] ovn: ovn-ctl support for HA ovn DB servers

2016-11-07 Thread Babu Shanmugam




On Monday 07 November 2016 06:49 PM, Andy Zhou wrote:

This version is better, I am able to apply them. Thanks.

I got the system running, but managed to get system into a state where 
both machines (centos and centos2)
are running the ovsdb in a backup mode.   The output of "pcs status" 
shows an error message, but the message is not

very helpful.  Any suggestion on how to debug this?

root@centos:/# pcs status
Cluster name: mycluster
Last updated: Mon Nov  7 05:12:06 2016Last change: Mon Nov  7 05:08:24 
2016 by root via cibadmin on centos

Stack: corosync
Current DC: centos2 (version 1.1.13-10.el7_2.4-44eb2dd) - partition 
with quorum

2 nodes and 3 resources configured

Node centos: standby
Online: [ centos2 ]

Full list of resources:

 virtip(ocf::heartbeat:IPaddr):Started centos2
 Master/Slave Set: ovndb_servers_master [ovndb_servers]
 Stopped: [ centos centos2 ]

Failed Actions:
* ovndb_servers_start_0 on centos2 'unknown error' (1): call=18, 
status=Timed Out, exitreason='none',

last-rc-change='Mon Nov  7 02:28:07 2016', queued=0ms, exec=30002ms


PCSD Status:
  centos: Online
  centos2: Online

Daemon Status:
  corosync: active/enabled
  pacemaker: active/enabled
  pcsd: active/enabled


root@centos:/# pcs config
Cluster Name: mycluster
Corosync Nodes:
 centos centos2
Pacemaker Nodes:
 centos centos2

Resources:
 Resource: virtip (class=ocf provider=heartbeat type=IPaddr)
  Attributes: ip=192.168.122.200 cidr_netmask=24
  Operations: start interval=0s timeout=20s (virtip-start-interval-0s)
  stop interval=0s timeout=20s (virtip-stop-interval-0s)
  monitor interval=30s (virtip-monitor-interval-30s)
 Master: ovndb_servers_master
  Meta Attrs: notify=true
  Resource: ovndb_servers (class=ocf provider=ovn type=ovndb-servers)
   Attributes: master_ip=192.168.122.200


Andy, you don't seem to have defined an attribute for ovn_ctl. It means, 
the ovn-ctl script will be assumed to be present in 
/usr/share/openvswitch/scripts/ovn-ctl. Can you check if you have 
ovn-ctl at the correct location? If not, please define an attribute 
similar to master_ip and name it ovn_ctl and point that to the correct 
location of ovn-ctl ?



   Operations: start interval=0s timeout=30s 
(ovndb_servers-start-interval-0s)
   stop interval=0s timeout=20s 
(ovndb_servers-stop-interval-0s)
   promote interval=0s timeout=50s 
(ovndb_servers-promote-interval-0s)
   demote interval=0s timeout=50s 
(ovndb_servers-demote-interval-0s)

   monitor interval=10s (ovndb_servers-monitor-interval-10s)

Stonith Devices:
Fencing Levels:

Location Constraints:
Ordering Constraints:
  start virtip then start ovndb_servers_master (kind:Mandatory) 
(id:order-virtip-ovndb_servers_master-mandatory)

Colocation Constraints:
  ovndb_servers_master with virtip (score:INFINITY) (rsc-role:Started) 
(with-rsc-role:Master) 
(id:colocation-ovndb_servers_master-virtip-INFINITY)


Resources Defaults:
 No defaults set
Operations Defaults:
 No defaults set

Cluster Properties:
 OVN_REPL_INFO: centos
 cluster-infrastructure: corosync
 cluster-name: mycluster
 dc-version: 1.1.13-10.el7_2.4-44eb2dd
 default-resource-stickiness: INFINITY
 have-watchdog: false
 no-quorum-policy: ignore
 stonith-enabled: false
Node Attributes:
 centos: standby=on
root@centos:/#

--

I have one more question about the patch below.



On Sun, Nov 6, 2016 at 6:44 PM, Babu Shanmugam <bscha...@redhat.com 
<mailto:bscha...@redhat.com>> wrote:


This patch adds support to start_ovsdb() function in ovn-ctl to
start the
ovn db servers in backup mode. This can be done in the following ways
1. Use parameters --ovn-nb-sync-from-addr and
--ovn-sb-sync-from-addr to
   set the addresses of the active server.
2. Create files $etcdir/ovnnb-active.conf and
$etcdir/ovnsb-active.conf
   with the tcp url of the active servers.

Additional functions to promote a backup server to active and demote
active server to backup mode are also added in this patch

One can optionally set the DB parameters for northd in
$etcdir/ovn-northd-db-params.conf. For example,

--ovnnb-db=tcp:172.16.247.230:6641 <http://172.16.247.230:6641>
--ovnsb-db=tcp:172.16.247.230:6642 <http://172.16.247.230:6642>

The parameters will be used as is, by start_northd(). If this file
exists,
start_northd() will not start the DB servers even if
$OVN_MANAGE_OVSDB is
    'yes'.

Signed-off-by: Babu Shanmugam <bscha...@redhat.com
<mailto:bscha...@redhat.com>>
---
 ovn/utilities/ovn-ctl   | 173
+---
 ovn/utilities/ovn-ctl.8.xml |  18 +
 2 files changed, 166 insertions(+), 25 deletions(-)

diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl
index 8544d14..9f158e7 100755
--- a/ovn/uti

[ovs-dev] [PATCH v5 1/4] ovn: ovn-ctl support for HA ovn DB servers

2016-11-06 Thread Babu Shanmugam

This patch adds support to start_ovsdb() function in ovn-ctl to start the
ovn db servers in backup mode. This can be done in the following ways
1. Use parameters --ovn-nb-sync-from-addr and --ovn-sb-sync-from-addr to
   set the addresses of the active server.
2. Create files $etcdir/ovnnb-active.conf and $etcdir/ovnsb-active.conf
   with the tcp url of the active servers.

Additional functions to promote a backup server to active and demote
active server to backup mode are also added in this patch

One can optionally set the DB parameters for northd in
$etcdir/ovn-northd-db-params.conf. For example,

--ovnnb-db=tcp:172.16.247.230:6641 --ovnsb-db=tcp:172.16.247.230:6642

The parameters will be used as is, by start_northd(). If this file exists,
start_northd() will not start the DB servers even if $OVN_MANAGE_OVSDB is
'yes'.

Signed-off-by: Babu Shanmugam <bscha...@redhat.com>
---
 ovn/utilities/ovn-ctl   | 173 +---
 ovn/utilities/ovn-ctl.8.xml |  18 +
 2 files changed, 166 insertions(+), 25 deletions(-)

diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl
index 8544d14..9f158e7 100755
--- a/ovn/utilities/ovn-ctl
+++ b/ovn/utilities/ovn-ctl
@@ -26,6 +26,9 @@ for dir in "$sbindir" "$bindir" /sbin /bin /usr/sbin 
/usr/bin; do
 done
 
 
+ovnnb_active_conf_file="$etcdir/ovnnb-active.conf"
+ovnsb_active_conf_file="$etcdir/ovnsb-active.conf"
+ovn_northd_db_conf_file="$etcdir/ovn-northd-db-params.conf"
 ## - ##
 ## start ##
 ## - ##
@@ -45,6 +48,44 @@ stop_ovsdb () {
 fi
 }
 
+demote_ovnnb() {
+if test ! -z "$DB_NB_SYNC_FROM_ADDR"; then
+echo "tcp:$DB_NB_SYNC_FROM_ADDR:$DB_NB_SYNC_FROM_PORT" > 
$ovnnb_active_conf_file
+fi
+
+if test -e $ovnnb_active_conf_file; then
+ovs-appctl -t $rundir/ovnnb_db.ctl 
ovsdb-server/set-active-ovsdb-server `cat $ovnnb_active_conf_file`
+ovs-appctl -t $rundir/ovnnb_db.ctl 
ovsdb-server/connect-active-ovsdb-server
+else
+echo >&2 "$0: active server details not set"
+exit 1
+fi
+}
+
+demote_ovnsb() {
+if test ! -z "$DB_SB_SYNC_FROM_ADDR"; then
+echo "tcp:$DB_SB_SYNC_FROM_ADDR:$DB_SB_SYNC_FROM_PORT" > 
$ovnsb_active_conf_file
+fi
+
+if test -e $ovnsb_active_conf_file; then
+ovs-appctl -t $rundir/ovnsb_db.ctl 
ovsdb-server/set-active-ovsdb-server `cat $ovnsb_active_conf_file`
+ovs-appctl -t $rundir/ovnsb_db.ctl 
ovsdb-server/connect-active-ovsdb-server
+else
+echo >&2 "$0: active server details not set"
+exit 1
+fi
+}
+
+promote_ovnnb() {
+rm -f $ovnnb_active_conf_file
+ovs-appctl -t $rundir/ovnnb_db.ctl 
ovsdb-server/disconnect-active-ovsdb-server
+}
+
+promote_ovnsb() {
+rm -f $ovnsb_active_conf_file
+ovs-appctl -t $rundir/ovnsb_db.ctl 
ovsdb-server/disconnect-active-ovsdb-server
+}
+
 start_ovsdb () {
 # Check and eventually start ovsdb-server for Northbound DB
 if ! pidfile_is_running $DB_NB_PID; then
@@ -52,7 +93,20 @@ start_ovsdb () {
 
 set ovsdb-server
 
-set "$@" --detach --monitor $OVN_NB_LOG --log-file=$OVN_NB_LOGFILE 
--remote=punix:$DB_NB_SOCK --remote=ptcp:$DB_NB_PORT:$DB_NB_ADDR 
--pidfile=$DB_NB_PID --unixctl=ovnnb_db.ctl
+set "$@" --detach --monitor $OVN_NB_LOG \
+--log-file=$OVN_NB_LOGFILE \
+--remote=punix:$DB_NB_SOCK \
+--remote=ptcp:$DB_NB_PORT:$DB_NB_ADDR \
+--pidfile=$DB_NB_PID \
+--unixctl=ovnnb_db.ctl
+
+if test ! -z "$DB_NB_SYNC_FROM_ADDR"; then
+echo "tcp:$DB_NB_SYNC_FROM_ADDR:$DB_NB_SYNC_FROM_PORT" > 
$ovnnb_active_conf_file
+fi
+
+if test -e $ovnnb_active_conf_file; then
+set "$@" --sync-from=`cat $ovnnb_active_conf_file`
+fi
 
 $@ $DB_NB_FILE
 ovn-nbctl init
@@ -64,12 +118,46 @@ start_ovsdb () {
 
 set ovsdb-server
 
-set "$@" --detach --monitor $OVN_SB_LOG --log-file=$OVN_SB_LOGFILE 
--remote=punix:$DB_SB_SOCK --remote=ptcp:$DB_SB_PORT:$DB_SB_ADDR 
--pidfile=$DB_SB_PID --unixctl=ovnsb_db.ctl
+set "$@" --detach --monitor $OVN_SB_LOG \
+--log-file=$OVN_SB_LOGFILE \
+--remote=punix:$DB_SB_SOCK \
+--remote=ptcp:$DB_SB_PORT:$DB_SB_ADDR \
+--pidfile=$DB_SB_PID \
+--unixctl=ovnsb_db.ctl
+
+if test ! -z "$DB_SB_SYNC_FROM_ADDR"; then
+echo "tcp:$DB_SB_SYNC_FROM_ADDR:$DB_SB_SYNC_FROM_PORT" > 
$ovnsb_active_conf_file
+fi
+
+if test -e $ovnsb_active_conf_file; then
+set "$@" --sync-from=`cat $ovnsb_active_conf_file`
+fi
+
 $@ $DB_SB_FILE
 ovn-sbctl init
 fi
 }
 
+sync_status() {
+ovs-appctl -t $rundir/ovn

[ovs-dev] [PATCH v5 0/4] High availability support for OVN DB servers using pacemaker

2016-11-06 Thread Babu Shanmugam

v4 -> v5:
  - Rebased on top of master

This patch contains changes required to run a pacemaker resource agent
to manage OVN db servers in active/standby mode in a HA cluster.

Babu Shanmugam (4):
  ovn: ovn-ctl support for HA ovn DB servers
  ovn: OCF script for OVN OVSDB servers
  ovn: Add ovn db servers ocf script in fedora packager
  ovn: Add ovndb servers ocf script in debian packager

 IntegrationGuide.rst|  67 
 debian/automake.mk  |   2 +
 debian/ovn-common.install   |   1 +
 debian/ovn-common.postinst  |  24 +++
 debian/ovn-common.postrm|  23 +++
 ovn/utilities/automake.mk   |   6 +-
 ovn/utilities/ovn-ctl   | 173 ---
 ovn/utilities/ovn-ctl.8.xml |  18 ++
 ovn/utilities/ovndb-servers.ocf | 356 
 rhel/openvswitch-fedora.spec.in |   8 +
 10 files changed, 651 insertions(+), 27 deletions(-)
 create mode 100644 debian/ovn-common.postinst
 create mode 100644 debian/ovn-common.postrm
 create mode 100755 ovn/utilities/ovndb-servers.ocf

-- 
1.9.1

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

[ovs-dev] [PATCH] ovn: Fix QoS marking without match

2016-11-02 Thread Babu Shanmugam

When a Logical_Switch's qos_rule does not have a match set, the rule should
apply for all the logical ports in that switch.

Signed-off-by: Babu Shanmugam <bscha...@redhat.com>
---
 ovn/northd/ovn-northd.c | 8 +++-
 tests/ovn.at| 4 
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 91affe4..512abb7 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -2644,12 +2644,18 @@ build_qos(struct ovn_datapath *od, struct hmap *lflows) 
{
 
 if (!strcmp(qos->key_action, "dscp")) {
 struct ds dscp_action = DS_EMPTY_INITIALIZER;
+const char *match;
 
+if (!strcmp(qos->match, "")) {
+match = "1";
+} else {
+match = qos->match;
+}
 ds_put_format(_action, "ip.dscp = %d; next;",
   (uint8_t)qos->value_action);
 ovn_lflow_add(lflows, od, stage,
   qos->priority,
-  qos->match, ds_cstr(_action));
+  match, ds_cstr(_action));
 ds_destroy(_action);
 }
 }
diff --git a/tests/ovn.at b/tests/ovn.at
index cb3e7dd..24dd12f 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -5459,6 +5459,10 @@ check_tos 63
 ovn-nbctl --wait=hv clear Logical_Switch lsw0 qos_rules
 check_tos 0
 
+# without match, packets from all all logical port should be marked
+qos_id=$(ovn-nbctl --wait=hv -- --id=@lp1-qos create QoS priority=100 
action=dscp=48 direction="from-lport" -- set Logical_Switch lsw0 
qos_rules=@lp1-qos)
+check_tos 48
+
 OVN_CLEANUP([hv])
 AT_CLEANUP
 
-- 
1.9.1

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

[ovs-dev] [PATCH v4 4/4] ovn: Add ovndb servers ocf script in debian packager

2016-11-02 Thread Babu Shanmugam

The OCF script will be present in the ovn-common package and installed
in the openvswitch scripts folder and a symbolic link to this file will
be created in the OCF resources folder.

The OCF resource agent name for this resource is ocf:ovn:ovndb-servers

Signed-off-by: Babu Shanmugam <bscha...@redhat.com>
---
 debian/automake.mk |  2 ++
 debian/ovn-common.install  |  1 +
 debian/ovn-common.postinst | 24 
 debian/ovn-common.postrm   | 23 +++
 4 files changed, 50 insertions(+)
 create mode 100644 debian/ovn-common.postinst
 create mode 100644 debian/ovn-common.postrm

diff --git a/debian/automake.mk b/debian/automake.mk
index 73b4d00..f793d4f 100644
--- a/debian/automake.mk
+++ b/debian/automake.mk
@@ -63,6 +63,8 @@ EXTRA_DIST += \
debian/ovn-controller-vtep.manpages \
debian/ovn-common.install \
debian/ovn-common.manpages \
+   debian/ovn-common.postinst \
+   debian/ovn-common.postrm \
debian/ovn-docker.install \
debian/ovn-host.dirs \
debian/ovn-host.init \
diff --git a/debian/ovn-common.install b/debian/ovn-common.install
index acb1dc9..8b833dc 100644
--- a/debian/ovn-common.install
+++ b/debian/ovn-common.install
@@ -1,3 +1,4 @@
 usr/bin/ovn-nbctl
 usr/bin/ovn-sbctl
 usr/share/openvswitch/scripts/ovn-ctl
+usr/share/openvswitch/scripts/ovndb-servers.ocf
diff --git a/debian/ovn-common.postinst b/debian/ovn-common.postinst
new file mode 100644
index 000..588044f
--- /dev/null
+++ b/debian/ovn-common.postinst
@@ -0,0 +1,24 @@
+#!/bin/sh
+# postinst script for ovn-common
+#
+# see: dh_installdeb(1)
+
+set -e
+
+case "$1" in
+configure)
+mkdir -p /usr/lib/ocf/resource.d/ovn
+ln -sf /usr/share/openvswitch/scripts/ovndb-servers.ocf 
/usr/lib/ocf/resource.d/ovn/ovndb-servers
+;;
+abort-upgrade|abort-remove|abort-deconfigure)
+;;
+
+*)
+echo "postinst called with unknown argument \`$1'" >&2
+exit 1
+;;
+esac
+
+#DEBHELPER#
+
+exit 0
diff --git a/debian/ovn-common.postrm b/debian/ovn-common.postrm
new file mode 100644
index 000..9face72
--- /dev/null
+++ b/debian/ovn-common.postrm
@@ -0,0 +1,23 @@
+#!/bin/sh
+# postrm script for openvswitch-testcontroller
+#
+# see: dh_installdeb(1)
+
+set -e
+
+case "$1" in
+purge|remove)
+rm -rf /usr/lib/ocf/resource.d/ovn
+;;
+upgrade|failed-upgrade|abort-install|abort-upgrade|disappear)
+;;
+
+*)
+echo "postrm called with unknown argument \`$1'" >&2
+exit 1
+;;
+esac
+
+#DEBHELPER#
+
+exit 0
-- 
1.9.1

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

[ovs-dev] [PATCH v4 0/4] High availability support for OVN DB servers using pacemaker

2016-11-02 Thread Babu Shanmugam

v3 -> v4:
  Following changes were suggested by Andy <az...@ovn.org> on v3
  - Updated the ovn-ctl man page
  - Added information regarding the pacemaker integration in the 
IntegrationGuide.md
  - Added debug messages in the ocf script

This patch contains changes required to run a pacemaker resource agent
to manage OVN db servers in active/standby mode in a HA cluster.

Babu Shanmugam (4):
  ovn: ovn-ctl support for HA ovn DB servers
  ovn: OCF script for OVN OVSDB servers
  ovn: Add ovn db servers ocf script in fedora packager
  ovn: Add ovndb servers ocf script in debian packager

 IntegrationGuide.md |  63 +++
 debian/automake.mk  |   2 +
 debian/ovn-common.install   |   1 +
 debian/ovn-common.postinst  |  24 +++
 debian/ovn-common.postrm|  23 +++
 ovn/utilities/automake.mk   |   6 +-
 ovn/utilities/ovn-ctl   | 173 ---
 ovn/utilities/ovn-ctl.8.xml |  18 ++
 ovn/utilities/ovndb-servers.ocf | 356 
 rhel/openvswitch-fedora.spec.in |   8 +
 10 files changed, 647 insertions(+), 27 deletions(-)
 create mode 100644 debian/ovn-common.postinst
 create mode 100644 debian/ovn-common.postrm
 create mode 100755 ovn/utilities/ovndb-servers.ocf

-- 
1.9.1

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

[ovs-dev] [PATCH v4 3/4] ovn: Add ovn db servers ocf script in fedora packager

2016-11-02 Thread Babu Shanmugam

The OCF script will be present in the ovn-common package and installed
in the openvswitch scripts folder. A symbolic link to this file will
be created in the OCF resources folder.

The OCF resource agent name for this resource is ocf:ovn:ovndb-servers

Signed-off-by: Babu Shanmugam <bscha...@redhat.com>
---
 rhel/openvswitch-fedora.spec.in | 8 
 1 file changed, 8 insertions(+)

diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
index eda8767..fb4aecd 100644
--- a/rhel/openvswitch-fedora.spec.in
+++ b/rhel/openvswitch-fedora.spec.in
@@ -310,6 +310,10 @@ rm -rf $RPM_BUILD_ROOT
 fi
 %endif
 
+%post ovn-common
+mkdir /usr/lib/ocf/resource.d/ovn
+ln -sf %{_datadir}/openvswitch/scripts/ovndb-servers.ocf 
/usr/lib/ocf/resource.d/ovn/ovndb-servers
+
 %post ovn-central
 %if 0%{?systemd_post:1}
 %systemd_post ovn-northd.service
@@ -354,6 +358,9 @@ rm -rf $RPM_BUILD_ROOT
 fi
 %endif
 
+%postun ovn-common
+rm -rf /usr/lib/ocf/resource.d/ovn
+
 %postun ovn-central
 %if 0%{?systemd_postun_with_restart:1}
 %systemd_postun_with_restart ovn-northd.service
@@ -493,6 +500,7 @@ fi
 %{_bindir}/ovn-sbctl
 %{_bindir}/ovn-trace
 %{_datadir}/openvswitch/scripts/ovn-ctl
+%{_datadir}/openvswitch/scripts/ovndb-servers.ocf
 %{_datadir}/openvswitch/scripts/ovn-bugtool-nbctl-show
 %{_datadir}/openvswitch/scripts/ovn-bugtool-sbctl-lflow-list
 %{_datadir}/openvswitch/scripts/ovn-bugtool-sbctl-show
-- 
1.9.1

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

[ovs-dev] [PATCH v4 2/4] ovn: OCF script for OVN OVSDB servers

2016-11-02 Thread Babu Shanmugam

Co-authored-by: Numan Siddique <nusid...@redhat.com>
Signed-off-by: Numan Siddique <nusid...@redhat.com>
Co-authored-by: Andrew Beekhof <abeek...@redhat.com>
Signed-off-by: Andrew Beekhof <abeek...@redhat.com>
Signed-off-by: Babu Shanmugam <bscha...@redhat.com>
---
 IntegrationGuide.md |  63 +++
 ovn/utilities/automake.mk   |   6 +-
 ovn/utilities/ovndb-servers.ocf | 356 
 3 files changed, 423 insertions(+), 2 deletions(-)
 create mode 100755 ovn/utilities/ovndb-servers.ocf

diff --git a/IntegrationGuide.md b/IntegrationGuide.md
index 5d3e574..945ecfd 100644
--- a/IntegrationGuide.md
+++ b/IntegrationGuide.md
@@ -167,3 +167,66 @@ following command can be used:
 
 ovs-vsctl set Interface eth0 external-ids:iface-id='"${UUID}"'
 
+
+HA for OVN DB servers using pacemaker
+-
+
+The ovsdb servers can work in either active or backup mode. In backup mode, db
+server will be connected to an active server and replicate the active servers
+contents. At all times, the data can be transacted only from the active server.
+When the active server dies for some reason, entire OVN operations will be
+stalled.
+
+[Pacemaker][] is a cluster resource manager which can manage a defined set of
+resource across a set of clustered nodes. Pacemaker manages the resource with
+the help of the resource agents. One among the resource agent is [OCF][].
+
+OCF is nothing but a shell script which accepts a set of actions and returns an
+appropriate status code.
+
+With the help of the OCF resource agent ovn/utilities/ovndb-servers.ocf, one
+can defined a resource for the pacemaker such that pacemaker will always
+maintain one running active server at any time.
+
+After creating a pacemaker cluster, use the following commands to create
+one active and multiple backup servers for OVN databases.
+
+pcs resource create ovndb_servers ocf:ovn:ovndb-servers \
+ master_ip=x.x.x.x \
+ ovn_ctl= \
+ op monitor interval="10s"
+
+pcs resource master ovndb_servers-master ovndb_servers \
+meta notify="true"
+
+The `master_ip` and `ovn_ctl` are the parameters that will be used by the
+OCF script. `ovn_ctl` is optional, if not given, it assumes a default value of
+/usr/share/openvswitch/scripts/ovn-ctl.
+
+Whenever the active server dies, pacemaker is responsible to promote one of
+the backup servers to be active. Both ovn-controller and ovn-northd needs the
+ip-address at which the active server is listening. With pacemaker changing the
+node at which the active server is run, it is not efficient to instruct all the
+ovn-controllers and the ovn-northd to listen to the latest active server's ip-
+address
+
+This problem can be solved by using a native ocf resource agent
+`ocf:heartbeat:IPaddr2`. The IPAddr2 resource agent is just a resource with an
+ip-address. When we colocate this resource with the active server, pacemaker
+will enable the active server to be connected with a single ip-address all the
+time. This is the ip-address that needs to be given as the parameter while
+creating the `ovndb_servers` resource.
+
+Use the following command to create the IPAddr2 resource and colocate it
+with the active server.
+
+pcs resource create VirtualIP ocf:heartbeat:IPaddr2 ip=x.x.x.x \
+op monitor interval=30s
+
+pcs constraint order VirtualIP then ovndb_servers-master
+
+pcs constraint colocation add ovndb_servers-master with master VirtualIP \
+score=INFINITY
+
+[Pacemaker]: http://clusterlabs.org/pacemaker.html
+[OCF]: http://www.linux-ha.org/wiki/OCF_Resource_Agents
diff --git a/ovn/utilities/automake.mk b/ovn/utilities/automake.mk
index b03d125..164cdda 100644
--- a/ovn/utilities/automake.mk
+++ b/ovn/utilities/automake.mk
@@ -1,5 +1,6 @@
 scripts_SCRIPTS += \
-ovn/utilities/ovn-ctl
+ovn/utilities/ovn-ctl \
+ovn/utilities/ovndb-servers.ocf
 
 man_MANS += \
 ovn/utilities/ovn-ctl.8 \
@@ -20,7 +21,8 @@ EXTRA_DIST += \
 ovn/utilities/ovn-docker-overlay-driver \
 ovn/utilities/ovn-docker-underlay-driver \
 ovn/utilities/ovn-nbctl.8.xml \
-ovn/utilities/ovn-trace.8.xml
+ovn/utilities/ovn-trace.8.xml \
+ovn/utilities/ovndb-servers.ocf
 
 DISTCLEANFILES += \
 ovn/utilities/ovn-ctl.8 \
diff --git a/ovn/utilities/ovndb-servers.ocf b/ovn/utilities/ovndb-servers.ocf
new file mode 100755
index 000..1fc61a7
--- /dev/null
+++ b/ovn/utilities/ovndb-servers.ocf
@@ -0,0 +1,356 @@
+#!/bin/bash
+
+: ${OCF_FUNCTIONS_DIR=${OCF_ROOT}/lib/heartbeat}
+. ${OCF_FUNCTIONS_DIR}/ocf-shellfuncs
+: ${OVN_CTL_DEFAULT="/usr/share/openvswitch/scripts/ovn-ctl"}
+CRM_MASTER="${HA_SBIN_DIR}/crm_master -l reboot"
+CRM_ATTR_REPL_INFO="${HA_SBIN_DIR}/crm_attribute --type crm_config --name 
OVN_REPL_INFO -s ovn_ovsdb_master_server"
+OVN_CTL=${OCF_RESKEY_ovn_ctl:-${OVN_CTL_DEFAULT}}
+MA

[ovs-dev] [PATCH v4 1/4] ovn: ovn-ctl support for HA ovn DB servers

2016-11-02 Thread Babu Shanmugam

This patch adds support to start_ovsdb() function in ovn-ctl to start the
ovn db servers in backup mode. This can be done in the following ways
1. Use parameters --ovn-nb-sync-from-addr and --ovn-sb-sync-from-addr to
   set the addresses of the active server.
2. Create files $etcdir/ovnnb-active.conf and $etcdir/ovnsb-active.conf
   with the tcp url of the active servers.

Additional functions to promote a backup server to active and demote
active server to backup mode are also added in this patch

One can optionally set the DB parameters for northd in
$etcdir/ovn-northd-db-params.conf. For example,

--ovnnb-db=tcp:172.16.247.230:6641 --ovnsb-db=tcp:172.16.247.230:6642

The parameters will be used as is, by start_northd(). If this file exists,
start_northd() will not start the DB servers even if $OVN_MANAGE_OVSDB is
'yes'.

Signed-off-by: Babu Shanmugam <bscha...@redhat.com>
---
 ovn/utilities/ovn-ctl   | 173 +---
 ovn/utilities/ovn-ctl.8.xml |  18 +
 2 files changed, 166 insertions(+), 25 deletions(-)

diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl
index 07bff8a..1c1687f 100755
--- a/ovn/utilities/ovn-ctl
+++ b/ovn/utilities/ovn-ctl
@@ -26,6 +26,9 @@ for dir in "$sbindir" "$bindir" /sbin /bin /usr/sbin 
/usr/bin; do
 done
 
 
+ovnnb_active_conf_file="$etcdir/ovnnb-active.conf"
+ovnsb_active_conf_file="$etcdir/ovnsb-active.conf"
+ovn_northd_db_conf_file="$etcdir/ovn-northd-db-params.conf"
 ## - ##
 ## start ##
 ## - ##
@@ -45,6 +48,44 @@ stop_ovsdb () {
 fi
 }
 
+demote_ovnnb() {
+if test ! -z "$DB_NB_SYNC_FROM_ADDR"; then
+echo "tcp:$DB_NB_SYNC_FROM_ADDR:$DB_NB_SYNC_FROM_PORT" > 
$ovnnb_active_conf_file
+fi
+
+if test -e $ovnnb_active_conf_file; then
+ovs-appctl -t $rundir/ovnnb_db.ctl 
ovsdb-server/set-active-ovsdb-server `cat $ovnnb_active_conf_file`
+ovs-appctl -t $rundir/ovnnb_db.ctl 
ovsdb-server/connect-active-ovsdb-server
+else
+echo >&2 "$0: active server details not set"
+exit 1
+fi
+}
+
+demote_ovnsb() {
+if test ! -z "$DB_SB_SYNC_FROM_ADDR"; then
+echo "tcp:$DB_SB_SYNC_FROM_ADDR:$DB_SB_SYNC_FROM_PORT" > 
$ovnsb_active_conf_file
+fi
+
+if test -e $ovnsb_active_conf_file; then
+ovs-appctl -t $rundir/ovnsb_db.ctl 
ovsdb-server/set-active-ovsdb-server `cat $ovnsb_active_conf_file`
+ovs-appctl -t $rundir/ovnsb_db.ctl 
ovsdb-server/connect-active-ovsdb-server
+else
+echo >&2 "$0: active server details not set"
+exit 1
+fi
+}
+
+promote_ovnnb() {
+rm -f $ovnnb_active_conf_file
+ovs-appctl -t $rundir/ovnnb_db.ctl 
ovsdb-server/disconnect-active-ovsdb-server
+}
+
+promote_ovnsb() {
+rm -f $ovnsb_active_conf_file
+ovs-appctl -t $rundir/ovnsb_db.ctl 
ovsdb-server/disconnect-active-ovsdb-server
+}
+
 start_ovsdb () {
 # Check and eventually start ovsdb-server for Northbound DB
 if ! pidfile_is_running $DB_NB_PID; then
@@ -52,7 +93,20 @@ start_ovsdb () {
 
 set ovsdb-server
 
-set "$@" --detach --monitor $OVN_NB_LOG --log-file=$OVN_NB_LOGFILE 
--remote=punix:$DB_NB_SOCK --remote=ptcp:$DB_NB_PORT:$DB_NB_ADDR 
--pidfile=$DB_NB_PID --unixctl=ovnnb_db.ctl
+set "$@" --detach --monitor $OVN_NB_LOG \
+--log-file=$OVN_NB_LOGFILE \
+--remote=punix:$DB_NB_SOCK \
+--remote=ptcp:$DB_NB_PORT:$DB_NB_ADDR \
+--pidfile=$DB_NB_PID \
+--unixctl=ovnnb_db.ctl
+
+if test ! -z "$DB_NB_SYNC_FROM_ADDR"; then
+echo "tcp:$DB_NB_SYNC_FROM_ADDR:$DB_NB_SYNC_FROM_PORT" > 
$ovnnb_active_conf_file
+fi
+
+if test -e $ovnnb_active_conf_file; then
+set "$@" --sync-from=`cat $ovnnb_active_conf_file`
+fi
 
 $@ $DB_NB_FILE
 fi
@@ -63,11 +117,45 @@ start_ovsdb () {
 
 set ovsdb-server
 
-set "$@" --detach --monitor $OVN_SB_LOG --log-file=$OVN_SB_LOGFILE 
--remote=punix:$DB_SB_SOCK --remote=ptcp:$DB_SB_PORT:$DB_SB_ADDR 
--pidfile=$DB_SB_PID --unixctl=ovnsb_db.ctl
+set "$@" --detach --monitor $OVN_SB_LOG \
+--log-file=$OVN_SB_LOGFILE \
+--remote=punix:$DB_SB_SOCK \
+--remote=ptcp:$DB_SB_PORT:$DB_SB_ADDR \
+--pidfile=$DB_SB_PID \
+--unixctl=ovnsb_db.ctl
+
+if test ! -z "$DB_SB_SYNC_FROM_ADDR"; then
+echo "tcp:$DB_SB_SYNC_FROM_ADDR:$DB_SB_SYNC_FROM_PORT" > 
$ovnsb_active_conf_file
+fi
+
+if test -e $ovnsb_active_conf_file; then
+set "$@" --sync-from=`cat $ovnsb_active_conf_file`
+fi
+
 $@ $DB_SB_FILE
 fi
 }
 
+sync_status() {
+ovs-appctl -t $rundir/ovn${1}_db.ctl ovsdb-server/sync-status | awk

Re: [ovs-dev] [PATCH v3 1/4] ovn: ovn-ctl support for HA ovn DB servers

2016-10-14 Thread Babu Shanmugam




On Saturday 15 October 2016 02:25 AM, Andy Zhou wrote:



On Fri, Oct 14, 2016 at 2:52 AM, Babu Shanmugam <bscha...@redhat.com 
<mailto:bscha...@redhat.com>> wrote:




On Friday 14 October 2016 04:00 AM, Andy Zhou wrote:



Done. Now it shows the following.

[root@h2 ovs]# crm configure show

node1: h1 \

attributes

node2: h2

primitiveClusterIP IPaddr2 \

paramsip=10.33.75.200cidr_netmask=32\

opstart interval=0stimeout=20s\

opstop interval=0stimeout=20s\

opmonitor interval=30s

primitiveovndb ocf:ovn:ovndb-servers \

opstart interval=0stimeout=30s\

opstop interval=0stimeout=20s\

oppromote interval=0stimeout=50s\

opdemote interval=0stimeout=50s\

opmonitor interval=1min\

meta

msovndb-master ovndb\

metanotify=true

colocationcolocation-ovndb-master-ClusterIP-INFINITY inf:
ovndb-master:Started ClusterIP:Master

orderorder-ClusterIP-ovndb-master-mandatory inf:
ClusterIP:start ovndb-master:start

propertycib-bootstrap-options: \

have-watchdog=false\

dc-version=1.1.13-10.el7_2.4-44eb2dd\

cluster-infrastructure=corosync\

cluster-name=mycluster\

stonith-enabled=false

propertyovn_ovsdb_master_server: \




My installation does not have ocf-tester,  There is a program
called ocft with a test option. Not sure if this is a suitable
replacement. If not, how could I get
the ocf-tester program? I ran the ocft program and get the
following output. Not sure what it means.

 [root@h2 ovs]# ocft test -n test-ovndb -o master_ip 10.0.0.1
/usr/share/openvswitch/scripts/ovndb-servers.ocf

ERROR: cases directory not found.




I have attached ocf-tester with this mail. I guess it's a
standalone script. If it does not work, I think it's better not to
attempt anymore as we have another way to find out.




Alternately, you can check if the ovsdb servers are started
properly by running

/usr/share/openvswitch/scripts/ovn-ctl
--db-sb-sync-from=10.0.0.1 --db-nb-sync-from=10.0.0.1 start_ovsdb


The output are as follows. Should we use --db-sb-sync-from-addr
instead?
 [root@h2 ovs]# /usr/share/openvswitch/scripts/ovn-ctl
--db-sb-sync-from=10.0.0.1 --db-nb-sync-from=10.0.0.1 start_ovsdb

/usr/share/openvswitch/scripts/ovn-ctl: unknown option
"--db-sb-sync-from=10.0.0.1" (use --help for help)

/usr/share/openvswitch/scripts/ovn-ctl: unknown option
"--db-nb-sync-from=10.0.0.1" (use --help for help)

'ovn-ctl' runs without any error message after I fixed the
command line parameter.


I am sorry for the misinformation, Andy. What you ran is correct.
Could you check the status of the ovsdb servers in h2 after you
run the above command using the following commands.

ovn-ctl status_ovnnb
ovn-ctl status_ovnsb

Both the above commands should return "running/backup". If you see
in the OCF script in the function ovsdb_server_start(), we wait
indefinitely till  the DB servers are started. Since the 'start'
action on h2 times out, I doubt that the servers are not started
properly.

O.K. I was able to get both server up.  Mostly by try-and-error.


[root@h2 openvswitch]# crm status

Last updated: Fri Oct 14 09:34:50 2016Last change: Thu Oct 13
11:17:25 2016 by root via cibadmin on h2

Stack: corosync

Current DC: h1 (version 1.1.13-10.el7_2.4-44eb2dd) -
partition with quorum

2 nodes and 3 resources configured


*Online*: [ h1 h2 ]


Full list of resources:


*ClusterIP*(ocf::heartbeat:IPaddr2):*Started*h1

*Master*/*Slave*Set: ovndb-*master*[ovndb]

*Master*s: [ h1 ]

*Slave*s: [ h2 ]

At this point, I think the scripts can work and will probably work for 
the integration task you had in mind. However, when it does not work, 
debugging it may not be tribal. Not sure if this is a show stopper. I 
also don't know pacemaker well enough to compare OVN with other HA 
components.


From this email thread,  It should be clear the patch set can use more 
documentation,  1) Enhance ovn-ctl man page  2) Add an integration 
guide on integration with pacemaker, or add an 'HA' section in the 
IntegrationGuide.md. 3) consider adding logs in case of error to help 
trouble shooting, I don't have specific suggestions


Would you please make those changes and post a V2? Thanks.



I agree Andy. I will post a V2 soon.

Thank you,
Babu
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v3 1/4] ovn: ovn-ctl support for HA ovn DB servers

2016-10-14 Thread Babu Shanmugam




On Friday 14 October 2016 04:00 AM, Andy Zhou wrote:



Done. Now it shows the following.

[root@h2 ovs]# crm configure show

node1: h1 \

attributes

node2: h2

primitiveClusterIP IPaddr2 \

paramsip=10.33.75.200cidr_netmask=32\

opstart interval=0stimeout=20s\

opstop interval=0stimeout=20s\

opmonitor interval=30s

primitiveovndb ocf:ovn:ovndb-servers \

opstart interval=0stimeout=30s\

opstop interval=0stimeout=20s\

oppromote interval=0stimeout=50s\

opdemote interval=0stimeout=50s\

opmonitor interval=1min\

meta

msovndb-master ovndb\

metanotify=true

colocationcolocation-ovndb-master-ClusterIP-INFINITY inf:
ovndb-master:Started ClusterIP:Master

orderorder-ClusterIP-ovndb-master-mandatory inf: ClusterIP:start
ovndb-master:start

propertycib-bootstrap-options: \

have-watchdog=false\

dc-version=1.1.13-10.el7_2.4-44eb2dd\

cluster-infrastructure=corosync\

cluster-name=mycluster\

stonith-enabled=false

propertyovn_ovsdb_master_server: \




My installation does not have ocf-tester,  There is a program called 
ocft with a test option. Not sure if this is a suitable replacement. 
If not, how could I get
the ocf-tester program? I ran the ocft program and get the following 
output. Not sure what it means.


 [root@h2 ovs]# ocft test -n test-ovndb -o master_ip 10.0.0.1 
/usr/share/openvswitch/scripts/ovndb-servers.ocf


ERROR: cases directory not found.




I have attached ocf-tester with this mail. I guess it's a standalone 
script. If it does not work, I think it's better not to attempt anymore 
as we have another way to find out.





Alternately, you can check if the ovsdb servers are started
properly by running

/usr/share/openvswitch/scripts/ovn-ctl --db-sb-sync-from=10.0.0.1
--db-nb-sync-from=10.0.0.1 start_ovsdb


The output are as follows. Should we use --db-sb-sync-from-addr instead?
 [root@h2 ovs]# /usr/share/openvswitch/scripts/ovn-ctl 
--db-sb-sync-from=10.0.0.1 --db-nb-sync-from=10.0.0.1 start_ovsdb


/usr/share/openvswitch/scripts/ovn-ctl: unknown option 
"--db-sb-sync-from=10.0.0.1" (use --help for help)


/usr/share/openvswitch/scripts/ovn-ctl: unknown option 
"--db-nb-sync-from=10.0.0.1" (use --help for help)


'ovn-ctl' runs without any error message after I fixed the command 
line parameter.


I am sorry for the misinformation, Andy. What you ran is correct. Could 
you check the status of the ovsdb servers in h2 after you run the above 
command using the following commands.


ovn-ctl status_ovnnb
ovn-ctl status_ovnsb

Both the above commands should return "running/backup". If you see in 
the OCF script in the function ovsdb_server_start(), we wait 
indefinitely till  the DB servers are started. Since the 'start' action 
on h2 times out, I doubt that the servers are not started properly.




I think this is mostly due to the crm configuration. Once you add
the 'ms' and 'colocation' resources, you should be able to
overcome this problem.

No, ovndb still failed to launch on h2.

[root@h2 ovs]# crm status

Last updated: Thu Oct 13 11:27:42 2016Last change: Thu Oct 13 11:17:25 
2016 by root via cibadmin on h2


Stack: corosync

Current DC: h2 (version 1.1.13-10.el7_2.4-44eb2dd) - partition with quorum

2 nodes and 3 resources configured


*Online*: [ h1 h2 ]


Full list of resources:


*ClusterIP*(ocf::heartbeat:IPaddr2):*Started*h1

*Master*/*Slave*Set: ovndb-*master*[ovndb]

*Master*s: [ h1 ]

*Stopped*: [ h2 ]


Failed Actions:

* ovndb_start_0 on h2 '*unknown error*' (1): call=39, status=*Timed 
Out*, exitreason='none',


last-rc-change='Wed Oct 12 14:43:03 2016', queued=0ms, exec=30003

I have never tried colocating two resources with the ClusterIP
resource. Just for testing, is it possible to drop the WebServer
resource?

Done.  It did not make any difference that I can see.



#!/bin/sh
#
#   $Id: ocf-tester,v 1.2 2006/08/14 09:38:20 andrew Exp $
#
# Copyright (c) 2006 Novell Inc, Andrew Beekhof
#All Rights Reserved.
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of version 2 of the GNU General Public License as
# published by the Free Software Foundation.
#
# This program is distributed in the hope that it would be useful, but
# WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
#
# Further, this software is distributed without any warranty that it is
# free of the rightful claim of any third person regarding infringement
# or the like.  Any license provided herein, whether implied or
# otherwise, applies only to this software file.  Patent licenses, if
# any, provided herein do not apply to combinations of this program with
# other software, or any other product whatsoever.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, write

Re: [ovs-dev] [PATCH v3 1/4] ovn: ovn-ctl support for HA ovn DB servers

2016-10-12 Thread Babu Shanmugam




On Thursday 13 October 2016 07:26 AM, Andy Zhou wrote:



On Sun, Oct 9, 2016 at 12:02 AM, Babu Shanmugam <bscha...@redhat.com 
<mailto:bscha...@redhat.com>> wrote:




On Friday 07 October 2016 05:33 AM, Andy Zhou wrote:

Babu, Thank you for working on this.  At a high level, it is
not clear to me the boundary between ocf scripts and the
ovn-ctl script -- i.e. which aspect is managed by which
entity.  For example, 1) which scripts are responsible for
starting the ovsdb servers.

ovsdb servers are started by the pacemaker. It uses the OCF script
and the OCF script uses ovn-ctl.

2) Which script should manage the fail-over -- I tried to shut
down a cluster node using the "pcs" command, and fail-over did
not happen.

The OCF script for OVN DB servers is capable of understanding the
promote and demote calls. So, pacemaker will use this script to
run ovsdb server in all the nodes and promote one node as the
master(active server). If the node in which the master instance is
running fails, pacemaker automatically promotes another node as
the master. OCF script is an agent for the pacemaker for the OVN
db resource.
The above behavior depends on the way you are configuring the
resource that uses this OCF script. I am attaching a simple set of
commands to configure the ovsdb server. You can create the
resources after creating the cluster with the following command

crm configure < ovndb.pcmk

Please note, you have to replace the macros VM1_NAME, VM2_NAME,
VM3_NAME and MASTER_IP with the respective values before using
ovndb.pcmk. This script works with a 3 node cluster. I am assuming
the node ids as 101, 102, and 103. Please replace them as well to
work with your cluster.


--
Babu


Unfortunately, CRM is not distributed with pacemaker on centos 
anymore.  It took me some time to get it installed.  I think other may 
ran into similar issues, so
it may be worth while do document this, or change the script to use 
"pcs" which is part of the distribution.




I agree. Is INSTALL*.md good enough? In openstack, we are managing the 
resource through puppet manifests.




I adapted the script with my setup.  I have two nodes, 
"h1"(10.33.74.77) and "h2"(10.33.75.158), For Master_IP, I used 
10.33.75.220.


This is the output of crm configure show:

--

 [root@h2 azhou]# crm configure show

node1: h1 \

attributes

node2: h2

primitiveClusterIP IPaddr2 \

paramsip=10.33.75.200cidr_netmask=32\

opstart interval=0stimeout=20s\

opstop interval=0stimeout=20s\

opmonitor interval=30s

primitiveWebSite apache \

paramsconfigfile="/etc/httpd/conf/httpd.conf"statusurl="http://127.0.0.1/server-status"\

opstart interval=0stimeout=40s\

opstop interval=0stimeout=60s\

opmonitor interval=1min\

meta

primitiveovndb ocf:ovn:ovndb-servers \

opstart interval=0stimeout=30s\

opstop interval=0stimeout=20s\

oppromote interval=0stimeout=50s\

opdemote interval=0stimeout=50s\

opmonitor interval=1min\

meta

colocationcolocation-WebSite-ClusterIP-INFINITY inf: WebSiteClusterIP

orderorder-ClusterIP-WebSite-mandatory ClusterIP:start WebSite:start

propertycib-bootstrap-options: \

have-watchdog=false\

dc-version=1.1.13-10.el7_2.4-44eb2dd\

cluster-infrastructure=corosync\

cluster-name=mycluster\

stonith-enabled=false




You seem to have configured ovndb just as a primitive resource and not 
as a master slave resource. And there is no colocation resource 
configured for the ovndb with ClusterIP. Only with the colocation 
resource, ovndb server will be co-located with the ClusterIP resource.  
You will have to include the following lines for crm configure. You can 
configure the same with pcs as well.


ms ovndb-master ovndb meta notify="true"
colocation colocation-ovndb-master-ClusterIP-INFINITY inf: 
ovndb-master:Started ClusterIP:Master
order order-ClusterIP-ovndb-master-mandatory inf: ClusterIP:start 
ovndb-master:start





I have also added firewall rules to allow access to TCP port 6642 and 
port 6641.



At this stage, crm_mon shows:

Last updated: Wed Oct 12 14:49:07 2016  Last change: Wed Oct 
12 13:58:55


 2016 by root via crm_attributeon h2

Stack: corosync

Current DC: h2 (version 1.1.13-10.el7_2.4-44eb2dd) - partition with quorum

2 nodes and 3 resources configured


Online: [ h1 h2 ]


ClusterIP(ocf::heartbeat:IPaddr2):Started h2

WebSite (ocf::heartbeat:apache):Started h2

ovndb (ocf::ovn:ovndb-servers):Started h1


Failed Actions:

* ovndb_start_0 on h2 'unknown error' (1): call=39, status=Timed Out, 
exitreason


='none',

last-rc-change='Wed Oct 12 14:43:03 2016', queued=0ms, exec=30003ms


---

Not sure what the error message on h2 is about, Notice ovndb service 
is now running on h1, while the cluster IP is on h2.




Looks like, the OCF script is not a

Re: [ovs-dev] [PATCH v3 1/4] ovn: ovn-ctl support for HA ovn DB servers

2016-10-09 Thread Babu Shanmugam




On Friday 07 October 2016 05:33 AM, Andy Zhou wrote:
Babu,  Thank you for working on this.  At a high level, it is not 
clear to me the boundary between ocf scripts and the ovn-ctl script -- 
i.e. which aspect is managed by which entity.  For example, 
1) which scripts are responsible for starting the ovsdb servers.
ovsdb servers are started by the pacemaker. It uses the OCF script and 
the OCF script uses ovn-ctl.


2) Which script should manage the fail-over -- I tried to shut down a 
cluster node using the "pcs" command, and fail-over did not happen.
The OCF script for OVN DB servers is capable of understanding the 
promote and demote calls. So, pacemaker will use this script to run 
ovsdb server in all the nodes and promote one node as the master(active 
server). If the node in which the master instance is running fails, 
pacemaker automatically promotes another node as the master. OCF script 
is an agent for the pacemaker for the OVN db resource.
The above behavior depends on the way you are configuring the resource 
that uses this OCF script. I am attaching a simple set of commands to 
configure the ovsdb server. You can create the resources after creating 
the cluster with the following command


crm configure < ovndb.pcmk

Please note, you have to replace the macros VM1_NAME, VM2_NAME, VM3_NAME 
and MASTER_IP with the respective values before using ovndb.pcmk. This 
script works with a 3 node cluster. I am assuming the node ids as 101, 
102, and 103. Please replace them as well to work with your cluster.



--
Babu
node $id="101" VM1_NAME
node $id="102" VM2_NAME
node $id="103" VM3_NAME
primitive ovndb_servers ocf:ovn:ovndb-servers params master_ip="MASTER_IP" op 
start interval="0s" timeout="30s" op stop interval="0s" timeout="20s" op 
promote interval="0s" timeout="50s" op demote interval="0s" timeout="50s" op 
monitor interval="10s" timeout="20s"
primitive ovnip ocf:heartbeat:IPaddr2 params ip="MASTER_IP" cidr_netmask="24" 
op start interval="0s" timeout="20s" op stop interval="0s" timeout="20s" op 
monitor interval="10s" timeout="20s"
ms ovndb_servers-master ovndb_servers meta notify="true"
colocation colocation-ovndb_servers-master-ovnip-INFINITY inf: 
ovndb_servers-master:Started ovnip:Master
order order-ovnip-ovndb_servers-master-mandatory inf: ovnip:start 
ovndb_servers-master:start
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v13 2/2] DSCP marking on packets

2016-10-05 Thread Babu Shanmugam

On Wednesday 05 October 2016 12:05 AM, Ben Pfaff wrote:

On Wed, Sep 07, 2016 at 11:40:12AM +0530,bscha...@redhat.com  wrote:

>From: Babu Shanmugam<bscha...@redhat.com>
>
>This patch adds support for marking qos on IP packets based on arbitrary
>match criteria for a logical switch.
>
>Signed-off-by: Babu Shanmugam<bscha...@redhat.com>
>Suggested-by: Mickey Spiegel<mickeys@gmail.com>
>Acked-by: Mickey Spiegel<mickeys@gmail.com>

I spent a little time with this, but I couldn't make the test pass.  The
first issue seems to be that MFF_LOG_CT_ZONE (in reg13) is
nondeterministic: sometimes I see 0x1, sometimes 0x2.  That's not a
problem with this patch, so I folded in the appended incremental, which
also makes sure that the logical port numbers get assigned in the
expected order, by adding --wait and sync.  But even with that, I get
the following failure, so i think that some more work is needed here.

Thanks,

Ben.

Ben, I rebased the patch and applied your changes. I could not see any 
failures in any test case. I have published a v14 which is the rebased 
version with your changes. Kindly try it and let me know if you are 
still seeing failures.

Thank you,
Babu
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v13 2/2] DSCP marking on packets

2016-10-04 Thread Babu Shanmugam

On Wednesday 05 October 2016 12:05 AM, Ben Pfaff wrote:

On Wed, Sep 07, 2016 at 11:40:12AM +0530,bscha...@redhat.com  wrote:

>From: Babu Shanmugam<bscha...@redhat.com>
>
>This patch adds support for marking qos on IP packets based on arbitrary
>match criteria for a logical switch.
>
>Signed-off-by: Babu Shanmugam<bscha...@redhat.com>
>Suggested-by: Mickey Spiegel<mickeys@gmail.com>
>Acked-by: Mickey Spiegel<mickeys@gmail.com>

I spent a little time with this, but I couldn't make the test pass.  The
first issue seems to be that MFF_LOG_CT_ZONE (in reg13) is
nondeterministic: sometimes I see 0x1, sometimes 0x2.  That's not a
problem with this patch, so I folded in the appended incremental, which
also makes sure that the logical port numbers get assigned in the
expected order, by adding --wait and sync.  But even with that, I get
the following failure, so i think that some more work is needed here.

Thanks,

Ben.

Thank you for reviewing Ben. I will have a look at the test cases.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v3 2/4] ovn: OCF script for OVN OVSDB servers

2016-09-29 Thread Babu Shanmugam

Yes Andrew. The database is never reset while testing in all the 
scenarios that we discussed.



On Thursday 29 September 2016 09:14 AM, Andrew Beekhof wrote:

no problem. it went well in testing?

On Wed, Sep 28, 2016 at 2:32 PM, Babu Shanmugam <bscha...@redhat.com> wrote:

Hi Andrew,

I have added "Signed-off-by" on your behalf. Hope you don't mind.



On Wednesday 28 September 2016 10:00 AM, bscha...@redhat.com wrote:

From: Babu Shanmugam <bscha...@redhat.com>

Co-authored-by: Numan Siddique <nusid...@redhat.com>
Signed-off-by: Numan Siddique <nusid...@redhat.com>
Co-authored-by: Andrew Beekhof <abeek...@redhat.com>
Signed-off-by: Andrew Beekhof <abeek...@redhat.com>
Signed-off-by: Babu Shanmugam <bscha...@redhat.com>
---
   ovn/utilities/automake.mk   |   6 +-
   ovn/utilities/ovndb-servers.ocf | 349

   2 files changed, 353 insertions(+), 2 deletions(-)
   create mode 100755 ovn/utilities/ovndb-servers.ocf

diff --git a/ovn/utilities/automake.mk b/ovn/utilities/automake.mk
index b03d125..164cdda 100644
--- a/ovn/utilities/automake.mk
+++ b/ovn/utilities/automake.mk
@@ -1,5 +1,6 @@
   scripts_SCRIPTS += \
-ovn/utilities/ovn-ctl
+ovn/utilities/ovn-ctl \
+ovn/utilities/ovndb-servers.ocf
 man_MANS += \
   ovn/utilities/ovn-ctl.8 \
@@ -20,7 +21,8 @@ EXTRA_DIST += \
   ovn/utilities/ovn-docker-overlay-driver \
   ovn/utilities/ovn-docker-underlay-driver \
   ovn/utilities/ovn-nbctl.8.xml \
-ovn/utilities/ovn-trace.8.xml
+ovn/utilities/ovn-trace.8.xml \
+ovn/utilities/ovndb-servers.ocf
 DISTCLEANFILES += \
   ovn/utilities/ovn-ctl.8 \
diff --git a/ovn/utilities/ovndb-servers.ocf
b/ovn/utilities/ovndb-servers.ocf
new file mode 100755
index 000..bb6c9dc
--- /dev/null
+++ b/ovn/utilities/ovndb-servers.ocf
@@ -0,0 +1,349 @@
+#!/bin/bash
+
+: ${OCF_FUNCTIONS_DIR=${OCF_ROOT}/lib/heartbeat}
+. ${OCF_FUNCTIONS_DIR}/ocf-shellfuncs
+: ${OVN_CTL_DEFAULT="/usr/share/openvswitch/scripts/ovn-ctl"}
+CRM_MASTER="${HA_SBIN_DIR}/crm_master -l reboot"
+CRM_ATTR_REPL_INFO="${HA_SBIN_DIR}/crm_attribute --type crm_config --name
OVN_REPL_INFO -s ovn_ovsdb_master_server"
+OVN_CTL=${OCF_RESKEY_ovn_ctl:-${OVN_CTL_DEFAULT}}
+MASTER_IP=${OCF_RESKEY_master_ip}
+
+# Invalid IP address is an address that can never exist in the network,
as
+# mentioned in rfc-5737. The ovsdb servers connects to this IP address
till
+# a master is promoted and the IPAddr2 resource is started.
+INVALID_IP_ADDRESS=192.0.2.254
+
+host_name=$(ocf_local_nodename)
+: ${slave_score=5}
+: ${master_score=10}
+
+ovsdb_server_metadata() {
+cat <
+
+
+  1.0
+
+  
+This resource manages ovsdb-server.
+  
+
+  
+Manages ovsdb-server.
+  
+
+  
+
+  
+  
+  Location to the ovn-ctl script file
+  
+  ovn-ctl script
+  
+  
+
+  
+  
+  The IP address resource which will be available on the master ovsdb
server
+  
+  master ip address
+  
+  
+
+  
+
+  
+
+
+
+
+
+
+
+
+  
+
+END
+exit $OCF_SUCCESS
+}
+
+ovsdb_server_notify() {
+# requires the notify=true meta resource attribute
+local
type_op="${OCF_RESKEY_CRM_meta_notify_type}-${OCF_RESKEY_CRM_meta_notify_operation}"
+
+if [ "$type_op" != "post-promote" ]; then
+# We are only interested in specific events
+return $OCF_SUCCESS
+fi
+
+if [ "x${OCF_RESKEY_CRM_meta_notify_promote_uname}" = "x${host_name}"
]; then
+# Record ourselves so that the agent has a better chance of doing
+# the right thing at startup
+${CRM_ATTR_REPL_INFO} -v "$host_name"
+
+else
+# Synchronize with the new master
+${OVN_CTL} demote_ovnnb --db-nb-sync-from-addr=${MASTER_IP}
+${OVN_CTL} demote_ovnsb --db-sb-sync-from-addr=${MASTER_IP}
+fi
+}
+
+ovsdb_server_usage() {
+cat < Stop -> Start -> Promote
+# At the point this is run, the only active masters will be
+# previous masters minus any that were scheduled to be demoted
+
+for master in ${OCF_RESKEY_CRM_meta_notify_master_uname}; do
+found=0
+for old in ${OCF_RESKEY_CRM_meta_notify_demote_uname}; do
+if [ $master = $old ]; then
+found=1
+fi
+done
+if [ $found = 0 ]; then
+# Rely on master-max=1
+# Pacemaker will demote any additional ones it finds before
starting new copies
+echo "$master"
+return
+fi
+done
+
+local expected_master=$($CRM_ATTR_REPL_INFO --query  -q 2>/dev/null)
+case "x${OCF_RESKEY_CRM_meta_notify_start_uname}x" in
+*${expected_master}*) echo "${expected_master}";; # The previous
master is expected to start
+esac
+}
+
+ovsdb_server_find_active_peers() {
+# Do we have any peers that are n

Re: [ovs-dev] [PATCH v3 2/4] ovn: OCF script for OVN OVSDB servers

2016-09-27 Thread Babu Shanmugam


Hi Andrew,

I have added "Signed-off-by" on your behalf. Hope you don't mind.


On Wednesday 28 September 2016 10:00 AM, bscha...@redhat.com wrote:

From: Babu Shanmugam <bscha...@redhat.com>

Co-authored-by: Numan Siddique <nusid...@redhat.com>
Signed-off-by: Numan Siddique <nusid...@redhat.com>
Co-authored-by: Andrew Beekhof <abeek...@redhat.com>
Signed-off-by: Andrew Beekhof <abeek...@redhat.com>
Signed-off-by: Babu Shanmugam <bscha...@redhat.com>
---
  ovn/utilities/automake.mk   |   6 +-
  ovn/utilities/ovndb-servers.ocf | 349 
  2 files changed, 353 insertions(+), 2 deletions(-)
  create mode 100755 ovn/utilities/ovndb-servers.ocf

diff --git a/ovn/utilities/automake.mk b/ovn/utilities/automake.mk
index b03d125..164cdda 100644
--- a/ovn/utilities/automake.mk
+++ b/ovn/utilities/automake.mk
@@ -1,5 +1,6 @@
  scripts_SCRIPTS += \
-ovn/utilities/ovn-ctl
+ovn/utilities/ovn-ctl \
+ovn/utilities/ovndb-servers.ocf
  
  man_MANS += \

  ovn/utilities/ovn-ctl.8 \
@@ -20,7 +21,8 @@ EXTRA_DIST += \
  ovn/utilities/ovn-docker-overlay-driver \
  ovn/utilities/ovn-docker-underlay-driver \
  ovn/utilities/ovn-nbctl.8.xml \
-ovn/utilities/ovn-trace.8.xml
+ovn/utilities/ovn-trace.8.xml \
+ovn/utilities/ovndb-servers.ocf
  
  DISTCLEANFILES += \

  ovn/utilities/ovn-ctl.8 \
diff --git a/ovn/utilities/ovndb-servers.ocf b/ovn/utilities/ovndb-servers.ocf
new file mode 100755
index 000..bb6c9dc
--- /dev/null
+++ b/ovn/utilities/ovndb-servers.ocf
@@ -0,0 +1,349 @@
+#!/bin/bash
+
+: ${OCF_FUNCTIONS_DIR=${OCF_ROOT}/lib/heartbeat}
+. ${OCF_FUNCTIONS_DIR}/ocf-shellfuncs
+: ${OVN_CTL_DEFAULT="/usr/share/openvswitch/scripts/ovn-ctl"}
+CRM_MASTER="${HA_SBIN_DIR}/crm_master -l reboot"
+CRM_ATTR_REPL_INFO="${HA_SBIN_DIR}/crm_attribute --type crm_config --name 
OVN_REPL_INFO -s ovn_ovsdb_master_server"
+OVN_CTL=${OCF_RESKEY_ovn_ctl:-${OVN_CTL_DEFAULT}}
+MASTER_IP=${OCF_RESKEY_master_ip}
+
+# Invalid IP address is an address that can never exist in the network, as
+# mentioned in rfc-5737. The ovsdb servers connects to this IP address till
+# a master is promoted and the IPAddr2 resource is started.
+INVALID_IP_ADDRESS=192.0.2.254
+
+host_name=$(ocf_local_nodename)
+: ${slave_score=5}
+: ${master_score=10}
+
+ovsdb_server_metadata() {
+cat <
+
+
+  1.0
+
+  
+This resource manages ovsdb-server.
+  
+
+  
+Manages ovsdb-server.
+  
+
+  
+
+  
+  
+  Location to the ovn-ctl script file
+  
+  ovn-ctl script
+  
+  
+
+  
+  
+  The IP address resource which will be available on the master ovsdb server
+  
+  master ip address
+  
+  
+
+  
+
+  
+
+
+
+
+
+
+
+
+  
+
+END
+exit $OCF_SUCCESS
+}
+
+ovsdb_server_notify() {
+# requires the notify=true meta resource attribute
+local 
type_op="${OCF_RESKEY_CRM_meta_notify_type}-${OCF_RESKEY_CRM_meta_notify_operation}"
+
+if [ "$type_op" != "post-promote" ]; then
+# We are only interested in specific events
+return $OCF_SUCCESS
+fi
+
+if [ "x${OCF_RESKEY_CRM_meta_notify_promote_uname}" = "x${host_name}" ]; 
then
+# Record ourselves so that the agent has a better chance of doing
+# the right thing at startup
+${CRM_ATTR_REPL_INFO} -v "$host_name"
+
+else
+# Synchronize with the new master
+${OVN_CTL} demote_ovnnb --db-nb-sync-from-addr=${MASTER_IP}
+${OVN_CTL} demote_ovnsb --db-sb-sync-from-addr=${MASTER_IP}
+fi
+}
+
+ovsdb_server_usage() {
+cat < Stop -> Start -> Promote
+# At the point this is run, the only active masters will be
+# previous masters minus any that were scheduled to be demoted
+
+for master in ${OCF_RESKEY_CRM_meta_notify_master_uname}; do
+found=0
+for old in ${OCF_RESKEY_CRM_meta_notify_demote_uname}; do
+if [ $master = $old ]; then
+found=1
+fi
+done
+if [ $found = 0 ]; then
+# Rely on master-max=1
+# Pacemaker will demote any additional ones it finds before 
starting new copies
+echo "$master"
+return
+fi
+done
+
+local expected_master=$($CRM_ATTR_REPL_INFO --query  -q 2>/dev/null)
+case "x${OCF_RESKEY_CRM_meta_notify_start_uname}x" in
+*${expected_master}*) echo "${expected_master}";; # The previous 
master is expected to start
+esac
+}
+
+ovsdb_server_find_active_peers() {
+# Do we have any peers that are not stopping
+for peer in ${OCF_RESKEY_CRM_meta_notify_slave_uname}; do
+found=0
+for old in ${OCF_RESKEY_CRM_meta_notify_stop_uname}; do
+if [ $peer = $old ]; then
+found=1
+fi
+done
+if [ $found = 0 ]; then
+

Re: [ovs-dev] [PATCH v2 0/4] OCF script for OVN DB servers

2016-09-21 Thread Babu Shanmugam




On Thursday 22 September 2016 12:55 AM, Russell Bryant wrote:



On Fri, Sep 16, 2016 at 7:15 AM, <bscha...@redhat.com 
<mailto:bscha...@redhat.com>> wrote:


v1 -> v2:
 - Fixed a logical problem in the OCF script as suggested by
   Andrew Beekhof <abeek...@redhat.com <mailto:abeek...@redhat.com>>
 - Changed start_northd in ovn-ctl to look for DB parameters in a
   file and not start the DB server if present.

This patch contains changes required to run a pacemaker resource agent
to manage OVN db servers in active/standby mode in a HA cluster.

Babu Shanmugam (4):
  ovn: ovn-ctl support for HA ovn DB servers
  ovn: OCF script for OVN OVSDB servers
  ovn: Add ovn db servers ocf script in fedora packager
  ovn: Add ovndb servers ocf script in debian packager


Based on our discussion earlier today, I'm expecting a v3 before I 
should review this, right?




Yes Russell.


--
Russell Bryant


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

Re: [ovs-dev] [patch v11 2/2] DSCP marking on packets egressing VIF interface

2016-09-02 Thread Babu Shanmugam


Thank you for reviewing, Mickey. I completely agree with your proposal.

Do you think it will be better to have this patch for now till we work 
on the code for your proposal?


On Friday 02 September 2016 11:17 AM, Mickey Spiegel wrote:


On Wed, Aug 31, 2016 at 12:11 AM, <bscha...@redhat.com 
<mailto:bscha...@redhat.com>> wrote:


ovn-northd sets 'ip.dscp' to the DSCP value


If we were to go with DSCP based on port as the initial functionality, 
your changes look good. A couple of nits below, and the first patch 
(which I have not looked at) needs a rebase after the removal of 
incremental processing.


However, I think we should be more ambitious and support arbitrary 
match criteria. I am always worried about the migration impact when 
moving from one way of specifying a feature in NB schema (in this 
case, port options "qos_dscp") to something completely different (see 
below) later on.


During the OVN meeting this morning, there was a preference for 
creating separate tables for each feature such as ACLs, QoS marking, 
SFC insertion, rather than overloading ACLs. It was noted that tables 
are fairly cheap, and each one can be customized to the purpose.


So I am proposing my earlier suggestion for ovn/ovn-nb.ovsschema once 
again:


@@ -26,6 +26,11 @@
   "refType": "strong"},
   "min": 0,
   "max": "unlimited"}},
+  "qos_rules": {"type": {"key": {"type": "uuid",
+"refTable": "QoS",
+"refType": "strong"},
+"min": 0,
+"max": "unlimited"}},
 "load_balancer": {"type": {"key": {"type": "uuid",
   "refTable": "Load_Balancer",
   "refType": "strong"},
@@ -118,6 +123,23 @@
 "type": {"key": "string", "value": "string",
  "min": 0, "max": "unlimited"}}},
 "isRoot": false},
+"QoS": {
+  "columns": {
+  "priority": {"type": {"key": {"type": "integer",
+"minInteger": 0,
+"maxInteger": 32767}}},
+  "direction": {"type": {"key": {"type": "string",
+  "enum": ["set", ["from-lport", "to-lport"]]}}},
+  "match": {"type": "string"},
+  "action": {"type": {"key": {"type": "string",
+  "enum": ["set", ["dscp"]]},
+  "value": {"type": "integer",
+"minInteger": 0,
+"maxInteger": 63}}},
+  "external_ids": {
+  "type": {"key": "string", "value": "string",
+   "min": 0, "max": "unlimited"}}},
+  "isRoot": false},
 "Logical_Router": {
 "columns": {
 "name": {"type": "string"},

Any opinions from others whether we should stick with the patch as is 
or implement arbitrary match criteria?


I don't think arbitrary match criteria requires a lot of code, though 
it would benefit from new nbctl commands, based on the existing acl 
commands. I am willing to help out if you wish. Note that the 
ovn/ovn-nb.ovsschema proposal above is dependent on an IDL fix to 
overcome a build error. I have not submitted that fix yet but I will 
raise it very soon.



Signed-off-by: Babu Shanmugam <bscha...@redhat.com
<mailto:bscha...@redhat.com>>
---
 ovn/lib/logical-fields.c|  2 +-
 ovn/northd/ovn-northd.8.xml | 30 +++
 ovn/northd/ovn-northd.c | 69
++
 ovn/ovn-nb.xml  |  6 
 ovn/ovn-sb.xml  |  5 
 tests/ovn.at <http://ovn.at> | 73
+
 6 files changed, 152 insertions(+), 33 deletions(-)

diff --git a/ovn/lib/logical-fields.c b/ovn/lib/logical-fields.c
index 6dbb4ae..068c000 100644
--- a/ovn/lib/logical-fields.c
+++ b/ovn/lib/logical-fields.c
@@ -134,7 +134,7 @@ ovn_init_symtab(struct shash *symtab)
 expr_symtab_add_predicate(symtab, "ip6", "eth.type == 0x86dd");
 expr_symtab_add_predicate(symtab, "ip", "ip4 || ip6");
 ex

Re: [ovs-dev] [PATCH] ovn: ovn-ctl support to start ovn db servers in standby mode

2016-08-31 Thread Babu Shanmugam




On Wednesday 31 August 2016 02:42 AM, Andy Zhou wrote:



On Thu, Aug 25, 2016 at 6:48 AM, <bscha...@redhat.com 
<mailto:bscha...@redhat.com>> wrote:


This patch adds support to start_ovsdb() function in ovn-ctl to
start the
ovn db servers in standby mode. This can be done in the following ways
1. Use parameters --ovn-nb-sync-from-addr and
--ovn-sb-sync-from-addr to
   set the addresses of the master server.
2. Create files $etcdir/ovnnb-master.conf and
$etcdir/ovnsb-master.conf
   with the tcp url of the master servers.

If --ovn-nb-sync-from-addr and --ovn-sb-sync-from-addr is used, it
will
overwrite the contents in the $etcdir/*.conf and use that server
as the
master.

Additional functions to promote a standby server to master and demote
master server to standby mode are also added in this patch

Signed-off-by: Babu Shanmugam <bscha...@redhat.com
<mailto:bscha...@redhat.com>>


In the context of OVSDB HA, we have been calling the ovsdb-server pair 
as active/backup.
It would be nice if we can avoid using another term "master".  I have 
no strong preference

for "active", just want to avoid using another term.


I will change it in the next revision.

It seems that if demote_ovssb() is followed by restart_ovsdb(),  the 
role switching

won't be respected. true?

You are right Andy. I will fix this too.

Thank you

Thanks.


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

Re: [ovs-dev] [PATCH v9 2/2] DSCP marking on packets egressing VIF interface

2016-08-29 Thread Babu Shanmugam

I am sending out v10 shortly with the agreed changes. Thank you for the 
review, Mickey.


On Saturday 27 August 2016 01:00 AM, Mickey Spiegel wrote:



On Wed, Aug 17, 2016 at 6:39 AM, <bscha...@redhat.com 
<mailto:bscha...@redhat.com>> wrote:


    From: Babu Shanmugam <bscha...@redhat.com
<mailto:bscha...@redhat.com>>

ovn-northd sets 'ip.dscp' to the DSCP value

Signed-off-by: Babu Shanmugam <bscha...@redhat.com
<mailto:bscha...@redhat.com>>
---
 ovn/lib/logical-fields.c |  2 +-
 ovn/northd/ovn-northd.c | 13 +
 ovn/ovn-nb.xml   |  6 
 ovn/ovn-sb.xml   |  5 
 tests/ovn.at <http://ovn.at>| 73

 5 files changed, 98 insertions(+), 1 deletion(-)

diff --git a/ovn/lib/logical-fields.c b/ovn/lib/logical-fields.c
index 6dbb4ae..068c000 100644
--- a/ovn/lib/logical-fields.c
+++ b/ovn/lib/logical-fields.c
@@ -134,7 +134,7 @@ ovn_init_symtab(struct shash *symtab)
 expr_symtab_add_predicate(symtab, "ip6", "eth.type == 0x86dd");
 expr_symtab_add_predicate(symtab, "ip", "ip4 || ip6");
 expr_symtab_add_field(symtab, "ip.proto", MFF_IP_PROTO, "ip",
true);
-expr_symtab_add_field(symtab, "ip.dscp", MFF_IP_DSCP, "ip",
false);
+expr_symtab_add_field(symtab, "ip.dscp", MFF_IP_DSCP_SHIFTED,
"ip", false);
 expr_symtab_add_field(symtab, "ip.ecn", MFF_IP_ECN, "ip", false);
 expr_symtab_add_field(symtab, "ip.ttl", MFF_IP_TTL, "ip", false);


You removed the description of the flows that you are adding from 
ovn-northd.8.xml. You are still adding flows. They should be described 
in ovn-northd.8.xml.



I agree.


diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 91e1687..eed12c9 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -2631,6 +2631,19 @@ build_lswitch_flows(struct hmap *datapaths,
struct hmap *ports,
 ovn_lflow_add(lflows, op->od, S_SWITCH_IN_PORT_SEC_L2, 50,
   ds_cstr(), ds_cstr());

+const char *dscp = smap_get(>sb->options, "qos_dscp");
+if (dscp) {
+struct ds dscp_actions = DS_EMPTY_INITIALIZER;
+struct ds dscp_match = DS_EMPTY_INITIALIZER;
+
+ds_put_format(_match, "inport == %s", op->json_key);
+ds_put_format(_actions, "ip.dscp = %s; next;",
dscp);
+ovn_lflow_add(lflows, op->od,
S_SWITCH_IN_PORT_SEC_IP, 100,
+  ds_cstr(_match),
ds_cstr(_actions));


You are still adding this flow to stage S_SWITCH_IN_PORT_SEC_IP with 
priority 100.


+ ds_destroy(_match);
+ds_destroy(_actions);
+}
+
 if (op->nbsp->n_port_security) {
 build_port_security_ip(P_IN, op, lflows);


If you follow this code, it adds priority 90 and priority 80 flows to 
stage S_SWITCH_IN_PORT_SEC_IP with priority 90 and priority 80. The 
point is to drop traffic that does not match certain combinations of 
inport, source ethernet address, and source IP address.


With your code, if qos_dscp is specified for a port, a priority 100 
flow will be inserted that will override the priority 90 and priority 
80 flows, removing IP port security functionality from that port.


I agree.



IMO this should be in a new ingress pipeline stage in order to avoid 
interactions with any other OVN features.


 build_port_security_nd(op, lflows);
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index 76309f8..b56b304 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -291,6 +291,12 @@
   If set, indicates the maximum burst size for data sent
from this
   interface, in bits.
 
+
+
+  If set, indicates the DSCP code to be marked on the
packets ingressing
+  the switch from the VIF interface. Value should be in
the range of
+  0 to 63 (inclusive).
+
   
 

The proposal in this patch is only to support setting DSCP for all 
traffic on a certain inport to a single value.


While that is a common use case, it is far from the only use case. 
Another common use case is to limit the maximum DSCP value on an 
inport, still allowing a range of values to be used on that port. 
There are other use cases.


This makes me wonder whether port-based DSCP is the right place to 
start. I can think of two main alternatives:


1) Allow matching on arbitrary fields by adding a table that is 
similar to ACLs but with different actions, adding to ovn-nb.ovsschema

Re: [ovs-dev] [PATCH v8 2/2] DSCP marking on packets egressing VIF interface

2016-08-16 Thread Babu Shanmugam


Thank Mickey for your review. My comments are inlined.


On Tuesday 16 August 2016 09:59 PM, Mickey Spiegel wrote:
On Tue, Aug 16, 2016 at 3:55 AM, <bscha...@redhat.com 
<mailto:bscha...@redhat.com>> wrote:


    From: Babu Shanmugam <bscha...@redhat.com
<mailto:bscha...@redhat.com>>

ovn-northd sets 'ip.dscp' to the DSCP value

Signed-off-by: Babu Shanmugam <bscha...@redhat.com
<mailto:bscha...@redhat.com>>
---
 ovn/lib/logical-fields.c|  2 +-
 ovn/northd/ovn-northd.8.xml |  5 
 ovn/northd/ovn-northd.c | 13 
 ovn/ovn-nb.xml  |  6 
 ovn/ovn-sb.xml  |  5 
 tests/ovn.at <http://ovn.at>   | 73
+
 6 files changed, 103 insertions(+), 1 deletion(-)

diff --git a/ovn/lib/logical-fields.c b/ovn/lib/logical-fields.c
index 6dbb4ae..068c000 100644
--- a/ovn/lib/logical-fields.c
+++ b/ovn/lib/logical-fields.c
@@ -134,7 +134,7 @@ ovn_init_symtab(struct shash *symtab)
 expr_symtab_add_predicate(symtab, "ip6", "eth.type == 0x86dd");
 expr_symtab_add_predicate(symtab, "ip", "ip4 || ip6");
 expr_symtab_add_field(symtab, "ip.proto", MFF_IP_PROTO, "ip",
true);
-expr_symtab_add_field(symtab, "ip.dscp", MFF_IP_DSCP, "ip",
false);
+expr_symtab_add_field(symtab, "ip.dscp", MFF_IP_DSCP_SHIFTED,
"ip", false);
 expr_symtab_add_field(symtab, "ip.ecn", MFF_IP_ECN, "ip", false);
 expr_symtab_add_field(symtab, "ip.ttl", MFF_IP_TTL, "ip", false);

diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
index 1cf6b6e..8f203b3 100644
--- a/ovn/northd/ovn-northd.8.xml
+++ b/ovn/northd/ovn-northd.8.xml
@@ -155,6 +155,11 @@

 
   
+Priority 100 flow to mark DSCP, only when the port
has a valid DSCP
+setting
+  
+
+  


By adding a priority 100 flow that matches only on inport, you just 
clobbered
the main port security functionality at priority 90 which restricts 
traffic to certain

combinations of inport, MAC, and IP addresses. This really should be in a
separate pipeline stage where it does not affect other features.



DSCP marking is done at a different table. Priority 90 flows will be in 
the next table. I agree my documentation is not proper. I will correct it.


In the long run, this should be more like ACL, allowing matches on 
arbitrary
fields (not just inport), but with the action being set DSCP rather 
than allow or drop.


 Priority 90 flow to allow IPv4 traffic if it has IPv4 addresses
 which match the inport, valid
eth.src
 and valid ip4.src address(es).
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 91e1687..f6f9afe 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -1739,6 +1739,7 @@ build_port_security_nd(struct ovn_port *op,
struct hmap *lflows)
  *
  *   - If the port security has IPv4 addresses or IPv6 addresses
or both
  * - Priority 80 flow to drop all IPv4 and IPv6 traffic
+ * - Priority 100 flow to set dscp, if DSCP setting is in
port options
  */
 static void
 build_port_security_ip(enum ovn_pipeline pipeline, struct
ovn_port *op,
@@ -1748,6 +1749,18 @@ build_port_security_ip(enum ovn_pipeline
pipeline, struct ovn_port *op,
 enum ovn_stage stage;
 if (pipeline == P_IN) {
 port_direction = "inport";
+const char *dscp = smap_get(>sb->options, "qos_dscp");
+if (dscp) {
+struct ds dscp_actions = DS_EMPTY_INITIALIZER;
+struct ds dscp_match = DS_EMPTY_INITIALIZER;
+
+ds_put_format(_match, "inport == %s", op->json_key);
+ds_put_format(_actions, "ip.dscp = %s; next;",
dscp);
+ovn_lflow_add(lflows, op->od,
S_SWITCH_IN_PORT_SEC_IP, 100,
+  ds_cstr(_match),
ds_cstr(_actions));
+ds_destroy(_match);
+ds_destroy(_actions);
+}
 stage = S_SWITCH_IN_PORT_SEC_IP;
 } else {
 port_direction = "outport";
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index 76309f8..b56b304 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -291,6 +291,12 @@
   If set, indicates the maximum burst size for data sent
from this
   interface, in bits.
 
+
+
+  If set, indicates the DSCP code to be marked on the
packets ingressing
+  the switch from the V

Re: [ovs-dev] [PATCH v6 1/2] Check and allocate free qdisc queue id for ports with qos parameters

2016-08-02 Thread Babu Shanmugam




On Monday 01 August 2016 10:09 PM, Babu Shanmugam wrote:



On Monday 01 August 2016 09:14 PM, Ben Pfaff wrote:

On Mon, Aug 01, 2016 at 07:03:14PM +0530, Babu Shanmugam wrote:


On Thursday 28 July 2016 01:48 AM, Ben Pfaff wrote:

On Wed, Jul 20, 2016 at 08:02:03PM +0530,bscha...@redhat.com  wrote:
ovn-northd processes the list of Port_Bindings and hashes the 
list of
queues per chassis. When it finds a port with qos_parameters and 
without
a queue_id, it allocates a free queue for the chassis that this 
port belongs.
The queue_id information is stored in the options field of 
Port_binding table.

Adds an action set_queue to the ingress table 0 of the logical flows
which will be translated to openflow set_queue by ovn-controller

ovn-controller opens the netdev corresponding to the tunnel 
interface's
status:tunnel_egress_iface value and configures a HTB qdisc on 
it. Then for
each SB port_binding that has queue_id set, it allocates a queue 
with the

qos_parameters of that port. It also frees up unused queues.

This patch replaces the older approach of policing

Signed-off-by: Babu Shanmugam<bscha...@redhat.com>
I suggest folding in the following changes.  Notably, set_queue(0); 
was

documented but didn't work because QDISC_MIN_QUEUE_ID was 1.

This series has no tests.  I think that at least the DSCP marking
feature could be tested with the existing OVN testing infrastructure.
Will you work on that?

Ben,
I am trying to write a test case with the following sequence to test 
the

dscp marking.
- Add two ports (lp1, lp2) through ovn-nbctl and assign addresses
- Setup the vswitch db with external_ids:iface-id for the logical ports
created above
- Set options:qos_dscp on one logical port (lp1)  through ovn-nbctl
- ofproto/trace a simple ip packet originating from lp1, and check 
if the

final flow has the DSCP bit set or not

ofproto/trace is tracing out correctly only when qos_dscp is not 
set. When

qos_dscp is set, the controller adds includes an additional action
"0x22->OXM_OF_IP_DSCP[]" in table 16. With this, ofproto/trace says 
it drops

the packet after it passes through table 0.

Do you have any suggestion on how to overcome this?

Hmm, that's odd.  What's the whole trace output look like?

It looks like the following

$ ovs-appctl ofproto/trace br-int 
'in_port=1,dl_src=f0:00:00:00:00:02,dl_dst=f0:00:00:00:00:01' -generate

Bridge: br-int
Flow: 
in_port=1,vlan_tci=0x,dl_src=f0:00:00:00:00:02,dl_dst=f0:00:00:00:00:01,dl_type=0x


Rule: table=0 cookie=0 priority=100,in_port=1
OpenFlow 
actions=set_field:0x1->reg13,set_field:0x1->metadata,set_field:0x1->reg14,resubmit(,16)


Resubmitted flow: 
reg13=0x1,reg14=0x1,metadata=0x1,in_port=1,vlan_tci=0x,dl_src=f0:00:00:00:00:02,dl_dst=f0:00:00:00:00:01,dl_type=0x
Resubmitted regs: reg0=0x0 reg1=0x0 reg2=0x0 reg3=0x0 reg4=0x0 
reg5=0x0 reg6=0x0 reg7=0x0 reg8=0x0 reg9=0x0 reg10=0x0 reg11=0x0 
reg12=0x0 reg13=0x1 reg14=0x1 reg15=0x0

Resubmitted  odp: drop
Resubmitted megaflow: 
recirc_id=0,reg13=0,reg14=0,metadata=0,in_port=1,vlan_tci=0x/0x1000,dl_src=f0:00:00:00:00:02,dl_type=0x

Rule: table=254 cookie=0 priority=0,reg0=0x2
OpenFlow actions=drop

Final flow: 
reg13=0x1,reg14=0x1,metadata=0x1,in_port=1,vlan_tci=0x,dl_src=f0:00:00:00:00:02,dl_dst=f0:00:00:00:00:01,dl_type=0x
Megaflow: 
recirc_id=0,in_port=1,vlan_tci=0x/0x1000,dl_src=f0:00:00:00:00:02,dl_type=0x

Datapath actions: drop



Turns out that it was due to a bug in the DSCP marking code. The DSCP 
marking was done at L2 stage without any additional flows to handle the 
L2 packets itself. So any L3 packet gets dropped at this stage.
I moved the DSCP marking logic to L3 pipeline with additional tests and 
submitted the patches for review.


Sorry for the trouble,
Babu


--
Babu


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

Re: [ovs-dev] [PATCH v7 0/2] QOS updates with DSCP support

2016-08-02 Thread Babu Shanmugam




On Tuesday 02 August 2016 02:37 PM, bscha...@redhat.com wrote:

From: anbu <a...@anbu.dt>


Please ignore this email address. The email addresses in the patches are 
all correct.



v6 -> v7
  - Updated the DSCP marking part in ovn-northd. Moved the DSCP logical flow
to S_SWITCH_IN_PORT_SEC_IP pipeline.
  - Added test cases for the DSCP marking case.

Babu Shanmugam (2):
   Check and allocate free qdisc queue id for ports with qos parameters
   DSCP marking on packets egressing VIF interface

  include/ovn/actions.h|   7 ++
  ovn/controller/binding.c | 239 ++-
  ovn/controller/lflow.c   |   2 +-
  ovn/lib/actions.c|  29 ++
  ovn/northd/ovn-northd.c  | 144 ++--
  ovn/ovn-nb.xml   |  14 ++-
  ovn/ovn-sb.xml   |  42 -
  tests/ovn.at |  78 
  8 files changed, 515 insertions(+), 40 deletions(-)



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

Re: [ovs-dev] [PATCH v6 1/2] Check and allocate free qdisc queue id for ports with qos parameters

2016-08-01 Thread Babu Shanmugam




On Monday 01 August 2016 09:14 PM, Ben Pfaff wrote:

On Mon, Aug 01, 2016 at 07:03:14PM +0530, Babu Shanmugam wrote:


On Thursday 28 July 2016 01:48 AM, Ben Pfaff wrote:

On Wed, Jul 20, 2016 at 08:02:03PM +0530,bscha...@redhat.com  wrote:

ovn-northd processes the list of Port_Bindings and hashes the list of
queues per chassis. When it finds a port with qos_parameters and without
a queue_id, it allocates a free queue for the chassis that this port belongs.
The queue_id information is stored in the options field of Port_binding table.
Adds an action set_queue to the ingress table 0 of the logical flows
which will be translated to openflow set_queue by ovn-controller

ovn-controller opens the netdev corresponding to the tunnel interface's
status:tunnel_egress_iface value and configures a HTB qdisc on it. Then for
each SB port_binding that has queue_id set, it allocates a queue with the
qos_parameters of that port. It also frees up unused queues.

This patch replaces the older approach of policing

Signed-off-by: Babu Shanmugam<bscha...@redhat.com>

I suggest folding in the following changes.  Notably, set_queue(0); was
documented but didn't work because QDISC_MIN_QUEUE_ID was 1.

This series has no tests.  I think that at least the DSCP marking
feature could be tested with the existing OVN testing infrastructure.
Will you work on that?

Ben,
I am trying to write a test case with the following sequence to test the
dscp marking.
- Add two ports (lp1, lp2) through ovn-nbctl and assign addresses
- Setup the vswitch db with external_ids:iface-id for the logical ports
created above
- Set options:qos_dscp on one logical port (lp1)  through ovn-nbctl
- ofproto/trace a simple ip packet originating from lp1, and check if the
final flow has the DSCP bit set or not

ofproto/trace is tracing out correctly only when qos_dscp is not set. When
qos_dscp is set, the controller adds includes an additional action
"0x22->OXM_OF_IP_DSCP[]" in table 16. With this, ofproto/trace says it drops
the packet after it passes through table 0.

Do you have any suggestion on how to overcome this?

Hmm, that's odd.  What's the whole trace output look like?

It looks like the following

$ ovs-appctl ofproto/trace br-int 
'in_port=1,dl_src=f0:00:00:00:00:02,dl_dst=f0:00:00:00:00:01' -generate

Bridge: br-int
Flow: 
in_port=1,vlan_tci=0x,dl_src=f0:00:00:00:00:02,dl_dst=f0:00:00:00:00:01,dl_type=0x


Rule: table=0 cookie=0 priority=100,in_port=1
OpenFlow 
actions=set_field:0x1->reg13,set_field:0x1->metadata,set_field:0x1->reg14,resubmit(,16)


Resubmitted flow: 
reg13=0x1,reg14=0x1,metadata=0x1,in_port=1,vlan_tci=0x,dl_src=f0:00:00:00:00:02,dl_dst=f0:00:00:00:00:01,dl_type=0x
Resubmitted regs: reg0=0x0 reg1=0x0 reg2=0x0 reg3=0x0 reg4=0x0 
reg5=0x0 reg6=0x0 reg7=0x0 reg8=0x0 reg9=0x0 reg10=0x0 reg11=0x0 
reg12=0x0 reg13=0x1 reg14=0x1 reg15=0x0

Resubmitted  odp: drop
Resubmitted megaflow: 
recirc_id=0,reg13=0,reg14=0,metadata=0,in_port=1,vlan_tci=0x/0x1000,dl_src=f0:00:00:00:00:02,dl_type=0x

Rule: table=254 cookie=0 priority=0,reg0=0x2
OpenFlow actions=drop

Final flow: 
reg13=0x1,reg14=0x1,metadata=0x1,in_port=1,vlan_tci=0x,dl_src=f0:00:00:00:00:02,dl_dst=f0:00:00:00:00:01,dl_type=0x
Megaflow: 
recirc_id=0,in_port=1,vlan_tci=0x/0x1000,dl_src=f0:00:00:00:00:02,dl_type=0x

Datapath actions: drop

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

Re: [ovs-dev] [PATCH v6 1/2] Check and allocate free qdisc queue id for ports with qos parameters

2016-08-01 Thread Babu Shanmugam

On Thursday 28 July 2016 01:48 AM, Ben Pfaff wrote:

On Wed, Jul 20, 2016 at 08:02:03PM +0530,bscha...@redhat.com  wrote:

>ovn-northd processes the list of Port_Bindings and hashes the list of
>queues per chassis. When it finds a port with qos_parameters and without
>a queue_id, it allocates a free queue for the chassis that this port belongs.
>The queue_id information is stored in the options field of Port_binding table.
>Adds an action set_queue to the ingress table 0 of the logical flows
>which will be translated to openflow set_queue by ovn-controller
>
>ovn-controller opens the netdev corresponding to the tunnel interface's
>status:tunnel_egress_iface value and configures a HTB qdisc on it. Then for
>each SB port_binding that has queue_id set, it allocates a queue with the
>qos_parameters of that port. It also frees up unused queues.
>
>This patch replaces the older approach of policing
>
>Signed-off-by: Babu Shanmugam<bscha...@redhat.com>

I suggest folding in the following changes.  Notably, set_queue(0); was
documented but didn't work because QDISC_MIN_QUEUE_ID was 1.

This series has no tests.  I think that at least the DSCP marking
feature could be tested with the existing OVN testing infrastructure.
Will you work on that?

Ben,
I am trying to write a test case with the following sequence to test the 
dscp marking.

- Add two ports (lp1, lp2) through ovn-nbctl and assign addresses
- Setup the vswitch db with external_ids:iface-id for the logical ports 
created above

- Set options:qos_dscp on one logical port (lp1)  through ovn-nbctl
- ofproto/trace a simple ip packet originating from lp1, and check if 
the final flow has the DSCP bit set or not

ofproto/trace is tracing out correctly only when qos_dscp is not set. 
When qos_dscp is set, the controller adds includes an additional action 
"0x22->OXM_OF_IP_DSCP[]" in table 16. With this, ofproto/trace says it 
drops the packet after it passes through table 0.

Do you have any suggestion on how to overcome this?

Thank you,
Babu
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v6 1/2] Check and allocate free qdisc queue id for ports with qos parameters

2016-07-28 Thread Babu Shanmugam




On Thursday 28 July 2016 01:48 AM, Ben Pfaff wrote:

On Wed, Jul 20, 2016 at 08:02:03PM +0530,bscha...@redhat.com  wrote:

>ovn-northd processes the list of Port_Bindings and hashes the list of
>queues per chassis. When it finds a port with qos_parameters and without
>a queue_id, it allocates a free queue for the chassis that this port belongs.
>The queue_id information is stored in the options field of Port_binding table.
>Adds an action set_queue to the ingress table 0 of the logical flows
>which will be translated to openflow set_queue by ovn-controller
>
>ovn-controller opens the netdev corresponding to the tunnel interface's
>status:tunnel_egress_iface value and configures a HTB qdisc on it. Then for
>each SB port_binding that has queue_id set, it allocates a queue with the
>qos_parameters of that port. It also frees up unused queues.
>
>This patch replaces the older approach of policing
>
>Signed-off-by: Babu Shanmugam<bscha...@redhat.com>

I suggest folding in the following changes.  Notably, set_queue(0); was
documented but didn't work because QDISC_MIN_QUEUE_ID was 1.


Thanks for the review, Ben


This series has no tests.  I think that at least the DSCP marking
feature could be tested with the existing OVN testing infrastructure.
Will you work on that?


Sure. I will work on it.


Thanks,

Ben.


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

Re: [ovs-dev] [PATCH] ovn-controller: update_ct_zone operates always on empty set

2016-07-26 Thread Babu Shanmugam




On Wednesday 27 July 2016 06:43 AM, Russell Bryant wrote:
On Tue, Jul 26, 2016 at 6:46 AM, <bscha...@redhat.com 
<mailto:bscha...@redhat.com>> wrote:


    From: Babu Shanmugam <bscha...@redhat.com
<mailto:bscha...@redhat.com>>

Commit 263064a (Convert binding_run to incremental processing.)
removed the usage
of all_lports from binding_run, but it is infact used in the
context of the caller,
especially by update_ct_zones().

Without this change, update_ct_zones operates on an empty set always.

Signed-off-by: Babu Shanmugam <bscha...@redhat.com
<mailto:bscha...@redhat.com>>


Ouch. This is a really bad regression.  If I understand correctly, 
we're not setting a ct zone ID for any logical ports.  All are just 
using the default zone of 0.



Yes Russell, your understanding is correct.
We should think about a good way to test OVN's use of conntrack zones 
to ensure that entries end up in separate zones for separate ports.  A 
good test for that may require userspace conntrack support, though.
 Another test we could do now would be looking at the flows in table 0 
and ensuring that the input flow for each port has a different 
conntrack zone ID assigned.  That feels like kind of a hack, though.
I agree that we need more test cases. I could not spend much time to 
figure out a proper approach for a test case. I will have a look at it.


Thank you,
Babu
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v4 1/2] Check and allocate free qdisc queue id for ports with qos parameters

2016-07-03 Thread Babu Shanmugam

On Saturday 25 June 2016 01:40 AM, Ben Pfaff wrote:

On Fri, Jun 24, 2016 at 02:59:15PM +0530,bscha...@redhat.com  wrote:

>ovn-northd processes the list of Port_Bindings and hashes the list of
>queues per chassis. When it finds a port with qos_parameters and without
>a queue_id, it allocates a free queue for the chassis that this port belongs.
>The queue_id information is stored in the options field of Port_binding table.
>Adds an action set_queue to the ingress table 0 of the logical flows
>which will be translated to openflow set_queue by ovn-controller
>
>ovn-controller opens the netdev corresponding to the tunnel interface's
>status:tunnel_egress_iface value and configures a HTB qdisc on it. Then for
>each SB port_binding that has queue_id set, it allocates a queue with the
>qos_parameters of that port. It also frees up unused queues.
>
>This patch replaces the older approach of policing
>
>Signed-off-by: Babu Shanmugam<bscha...@redhat.com>

Thanks for the new version.

I'm passing along an incremental to fold in.  Some of the changes are
style.  Others:

* Put hmap_node at the start of structs because it makes valgrind's
  memory leak checker confident about pointers instead of calling
  them "possible leaks".

* Avoid trying to modify the OVS database when there's no transaction
  open.

* Log errors from netdev operations.

* Don't log a warning when setting up QoS.

* Fix a couple of memory leaks.

I was going to just apply this but there's an ongoing situation where we
might need to revert patches in this same area (see
http://openvswitch.org/pipermail/dev/2016-June/073608.html) and I don't
want to make that harder.  So please sit on this until that situation
resolves.

Hi Ben,
Should I rebase this patch publish it again?

Thank you,
Babu
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] The QoS feature minimum guaranteed bandwidth of OpenvSwitch not work

2016-07-01 Thread Babu Shanmugam


Hi Xiao,
My comments below;

On Friday 01 July 2016 09:10 AM, Xiao Ma (xima2) wrote:

Hi,All

I want to use the QoS feature of OpenvSwitch to control the bandwidth based on 
the vlan id(Scene 1) or port id(Scene 2).
So I deployed it as showed bellow,and configured the qos rules,the flows,and 
used iperf tool to test it.
But the result is disappointment.

Scene 1:


  ++
  ||
  |  switch|
  +-+---+--+
|   |
  bond0bond0
  +-|+
+-|-+
  | host1  +---+ || host2  
+---+  |
  ||   | |||
   |  |
  ||  ovsbr| |||  ovsbr 
   |  |
  |+---+ ||
+---+  |
  | |  ofport=1  ||  |  
  |
  |+---+ ||
+---+  |
  ||  br-int   | ||| br-int 
   |  |
  |+---+ ||
+---+  |
  |   tag=1002  |tag=1038|||
  | tag=1038  |
  | |   |||tag=1002|
  |   |
  | |   ||||
  |   |
  |   ++  +-+||   +---+ 
+---+ |
  |   |namespace   |  |namespace|||   |namespace  | 
|namespace  | |
  |   ||  | |||   |   | 
|   | |
  |   ||  | |||   |   | 
|   | |
  |   |10.10.12.2/24   |  |192.168.4.2/24   |||   |10.10.12.4/24  | 
|192.168.4.3/24 | |
  |   |iperf -c|  | |||   |iperf -s   | 
|   | |
  |   |10.10.12.4 -P 20|  | |||   |   | 
|   | |
  |   ||  | |||   |   | 
|   | |
  |   ||  | |||   |   | 
|   | |
  |   ||  | |||   |   | 
|   | |
  |   ||  | |||   |   | 
|   | |
  |   ||  | |||   |   | 
|   | |
  |   ++  +-+||   +---+ 
+---+ |
  |  || 
  |
  |  || 
  |
  +--+
+---+


1)Set QoS:

ovs-vsctl set port  bond0  qos=@qos -- --id=@qos create qos type=linux-htb\
queues:1=@queue_data \
queues:2=@queue_storage -- \
--id=@queue_data create queue other-config:min-rate=2 
other-config:max-rate=2-- \
--id=@queue_storage create queue other-config:min-rate=8 
other-config:max-rate=8


2)OVS in host1：

Flows of ovsbr:
priority=100,in_port=1,dl_vlan=1002 actions=set_queue:2,NORMAL
priority=50,in_port=1 actions=set_queue:1,NORMAL
priority=2,in_port=1 actions=drop
priority=1 actions=NORMAL

Flows of br-int:
priority=3,in_port=1,dl_vlan=1002 actions=mod_vlan_vid:1002,NORMAL
priority=2,in_port=1 actions=drop
priority=1 actions=NORMAL


Can you check "tc -s -d class show dev bond0" and see if the queues are 
set on bond0? If so, the queue_ids will mostly be equal to n+1, n being 
the value set in the queue table. From my understanding, 
https://github.com/openvswitch/ovs/blob/master/lib/netdev-linux.c#L3924 
does this.


What is the bandwidth that iperf shows when the packets are not shaped 
(without queues)?



3)iperf -c 10.10.12.4 -P 20,but the result is not 800M

[root@host1 ~]# iperf -c 10.10.12.4 -i 1 -t 100  -P 20 |grep SUM
[SUM]  0.0- 1.0 sec  53.2 MBytes446 Mbits/sec
[SUM]  1.0- 2.0 sec  51.0

Re: [ovs-dev] [PATCH v5 0/2] OVN address sets.

2016-06-28 Thread Babu Shanmugam


Kindly ignore this version. I sent it by mistake.

On Tuesday 28 June 2016 12:58 PM, bscha...@redhat.com wrote:

From: Babu Shanmugam <bscha...@redhat.com>

v4 -> v5
  - Added external_ids column to 'Address Set' table in NB db,
as suggested by Han Zhou
  - Some more correction as suggested by Zong Kai LI

Russel Bryant (1):
   Add address set support.

Russell Bryant (1):
   ovn: Add address_set() support for ACLs.

  ovn/controller/lflow.c| 155 +-
  ovn/lib/actions.c |   2 +-
  ovn/lib/expr.c| 126 +++--
  ovn/lib/expr.h|  17 +
  ovn/lib/lex.c |  16 +
  ovn/lib/lex.h |   1 +
  ovn/northd/ovn-northd.c   |  43 +
  ovn/ovn-nb.ovsschema  |  13 +++-
  ovn/ovn-nb.xml|  32 ++
  ovn/ovn-sb.ovsschema  |  12 +++-
  ovn/ovn-sb.xml|  19 ++
  ovn/utilities/ovn-nbctl.c |   4 ++
  ovn/utilities/ovn-sbctl.c |   4 ++
  tests/ovn.at  |  69 +
  tests/test-ovn.c  |  31 +-
  15 files changed, 530 insertions(+), 14 deletions(-)



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

Re: [ovs-dev] [PATCH v4 2/2] ovn: Add address_set() support for ACLs.

2016-06-28 Thread Babu Shanmugam


Hi Zong Kai LI,
My comments are in-lined below.

On Monday 27 June 2016 08:50 PM, Zong Kai LI wrote:


+static void
+update_address_sets(struct controller_ctx *ctx)
+{
+/* Remember the names of all address sets currently in
expr_address_sets
+ * so we can detect address sets that have been deleted. */
+struct sset cur_addr_set_names =
SSET_INITIALIZER(_addr_set_names);
+
+struct shash_node *node;
+SHASH_FOR_EACH (node, _address_sets) {
+sset_add(_addr_set_names, node->name);
+}
+
+/* Iterate address sets in the southbound database. Create
and update the
+ * corresponding symtab entries as necessary. */
+const struct sbrec_address_set *addr_set_rec;
+SBREC_ADDRESS_SET_FOR_EACH (addr_set_rec, ctx->ovnsb_idl) {
+node = shash_find(_address_sets, addr_set_rec->name);
+struct address_set *addr_set = node ? node->data : NULL;
+

How about using shash_find_data here?
struct address_set *addr_set = shash_find_data(_address_sets, 
addr_set_rec->name);


I agree.



+
+/* Anything remaining in cur_addr_set_names refers to an
address set that
+ * has been deleted from the southbound database.  We should
delete
+ * the corresponding symtab entry. */
+const char *cur_node, *next_node;
+SSET_FOR_EACH_SAFE (cur_node, next_node, _addr_set_names) {
+ expr_macros_remove(_address_sets, cur_node);
+
+struct address_set *addr_set
+= shash_find_and_delete(_address_sets, cur_node);
+address_set_destroy(addr_set);
+
+struct sset_node *sset_node = SSET_NODE_FROM_NAME(cur_node);
+sset_delete(_addr_set_names, sset_node);


Trivial: this seems unnecessary, sset_destroy will call sset_clear, 
and sset_clear will call sset_delete.


sset_destroy will have to loop the set from the beginning to delete the 
entries. Since we are anyway looping it before destroy(), it might save 
some minimal computing time. What do you think?



+   }
+
+sset_destroy(_addr_set_names);
+}

+static void
+sync_address_sets(struct northd_context *ctx)
+{
+struct shash sb_address_sets =
SHASH_INITIALIZER(_address_sets);
+
+const struct sbrec_address_set *sb_address_set;
+SBREC_ADDRESS_SET_FOR_EACH (sb_address_set, ctx->ovnsb_idl) {
+shash_add(_address_sets, sb_address_set->name,
sb_address_set);
+}
+
+const struct nbrec_address_set *nb_address_set;
+NBREC_ADDRESS_SET_FOR_EACH (nb_address_set, ctx->ovnnb_idl) {
+sb_address_set = shash_find_and_delete(_address_sets,
+  nb_address_set->name);
+if (!sb_address_set) {
+sb_address_set =
sbrec_address_set_insert(ctx->ovnsb_txn);
+sbrec_address_set_set_name(sb_address_set,
nb_address_set->name);
+}
+
+ sbrec_address_set_set_addresses(sb_address_set,
+/* "char **" is not compatible with "const char
**" */
+(const char **) nb_address_set->addresses,
+nb_address_set->n_addresses);
+}
+
+struct shash_node *node, *next;
+SHASH_FOR_EACH_SAFE (node, next, _address_sets) {
+sbrec_address_set_delete(node->data);


Trivial: this seems unnecessary, shash_destroy will delete and free 
node in sb_address_sets.




Same as the previous answer. Your suggestions are most welcome.


+shash_delete(_address_sets, node);
+}
+shash_destroy(_address_sets);
+}


+  
+
+  Each row in this table represents a named set of addresses.
+  An address set may contain MAC, IPv4, or IPv6 addresses and
cidrs.
+  The address set will ultimately be used in ACLs, where a
certain
+  type of field such as ip4.src or ip6.src will be compared
with the
+  address set. So, a single address set must contain
addresses of the
+  same type.
+
+


Thanks for updating this.

Thank you for reporting :)

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

Re: [ovs-dev] [PATCH v4 1/2] Add address set support.

2016-06-28 Thread Babu Shanmugam


Hi Zong Kai LI,
Thank you for pointing it out. I will take care of this in v5.

Babu

On Monday 27 June 2016 08:39 PM, Zong Kai LI wrote:



 static bool
+parse_macros(struct expr_context *ctx, struct expr_constant_set *cs,
+  size_t *allocated_values)
+{
+if (!ctx->macros) {
+expr_syntax_error(ctx, "No macros defined.");
+return false;
+}
+
+struct expr_constant_set *addr_set
+= shash_find_data(ctx->macros, ctx->lexer->token.s);
+if (!addr_set) {
+expr_syntax_error(ctx, "Unknown macro: '%s'",
ctx->lexer->token.s);
+return false;
+}
+
+cs->type = EXPR_C_INTEGER;


Hi, Babu.
It's odd you don't directly fix here with assign_constant_set_type, 
but in the following patch.

Hope it doesn't matter to others.

Thanks.
Zong Kai, LI


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

Re: [ovs-dev] [PATCH v3 2/2] ovn: Add address_set() support for ACLs.

2016-06-27 Thread Babu Shanmugam

Hi Han,
I am sorry, I must have missed it. I will include it in v5.

Thank you,
Babu

On Monday 27 June 2016 10:59 PM, Han Zhou wrote:

On Sun, Jun 26, 2016 at 11:29 PM, Babu Shanmugam <bscha...@redhat.com 
<mailto:bscha...@redhat.com>> wrote:

>
>
>
> On Thursday 23 June 2016 12:03 PM, Han Zhou wrote:
>>
>> It may be good to have column "external_ids", so that external 
names, such as security-group name in neutron, can be recognized easily.

>
> Han, do you want the external_ids to be present in the table in OVN 
SouthBound DB? I feel that it would be good for the table in 
NorthBound DB to have the external_ids.

>

Yes, I meant NorthBound DB. When I repost my comments I put it in 
wrong place, sorry. But I didn't find the change in your v4. Did I 
miss it somewhere?

Han

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

Re: [ovs-dev] [PATCH v3 2/2] ovn: Add address_set() support for ACLs.

2016-06-27 Thread Babu Shanmugam




On Thursday 23 June 2016 12:03 PM, Han Zhou wrote:
It may be good to have column "external_ids", so that external names, 
such as security-group name in neutron, can be recognized easily.
Han, do you want the external_ids to be present in the table in OVN 
SouthBound DB? I feel that it would be good for the table in NorthBound 
DB to have the external_ids.


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

Re: [ovs-dev] [PATCH v3 2/2] ovn: Add address_set() support for ACLs.

2016-06-24 Thread Babu Shanmugam



Hi Flavio,
Thanks for reviewing. My comments are below.

On Thursday 23 June 2016 10:33 PM, Flaviof wrote:



On Thu, Jun 23, 2016 at 1:05 AM, > wrote:


From: Russell Bryant >

+/* Return true if the address sets match, false otherwise. */
+static bool
+address_sets_match(struct address_set *addr_set,
+   const struct sbrec_address_set *addr_set_rec)


- make both parameters const  (const struct sbrec_address_set *)

- why not call these addr_set1 and addr_set2 ?


I agree. addr_set1 and addr_set2 is more readable.



+static void
+update_address_sets(struct controller_ctx *ctx)
+{
+/* Remember the names of all address sets currently in
expr_address_sets
+ * so we can detect address sets that have been deleted. */
+struct sset cur_address_sets =
SSET_INITIALIZER(_address_sets);


This sset is not an address_set, but address_set names (or keys).
How about renaming this variable to cur_address_set_names ?


I agree.


+
+
+/* OVN_Northbound and OVN_Southbound have an identical
Address_Set table.
+ * We always update OVN_Southbound to match the current data in
+ * OVN_Northbound. */
+static void
+sync_address_sets(struct northd_context *ctx)
+{
+struct shash sb_address_sets =
SHASH_INITIALIZER(_address_sets);
+
+const struct sbrec_address_set *sb_address_set;
+SBREC_ADDRESS_SET_FOR_EACH (sb_address_set, ctx->ovnsb_idl) {
+shash_add(_address_sets, sb_address_set->name,
sb_address_set);
+}
+
+const struct nbrec_address_set *nb_address_set;
+NBREC_ADDRESS_SET_FOR_EACH (nb_address_set, ctx->ovnnb_idl) {
+sb_address_set = shash_find_and_delete(_address_sets,
+  nb_address_set->name);
+if (!sb_address_set) {
+sb_address_set =
sbrec_address_set_insert(ctx->ovnsb_txn);
+ sbrec_address_set_set_name(sb_address_set, nb_address_set->name);
+}
+


I'm still wrapping my head around ctx and txn but I thought of asking 
a dumb question here:
In cases when hash_find_and_delete returns a non-null sb_address_set, 
is there a need
to clean anything up before calling sbrec_address_set_set_addresses? 
If so, this override may be causing a leak?
I think, shash_find_and_delete returns a db rec. It seems to me that 
there is no need for a cleanup. I will confirm it anyways. Thanks for 
pointing it out.





+  
+
+  Each row in this table represents a named set of addresses.
+  An address set may contain MAC, IPv4, or IPv6 addresses.


... and also IPv4 + IPv6 cidr (w.x.y.z/N), right?

Yes. I will update the documentation to include these as well. Thanks.


diff --git a/tests/ovn.at  b/tests/ovn.at

index 4f72107..59f9307 100644
--- a/tests/ovn.at 
+++ b/tests/ovn.at 
@@ -649,6 +649,8 @@ done
 ovn-nbctl acl-add lsw0 from-lport 1000 'eth.type == 0x1234' drop
 ovn-nbctl acl-add lsw0 from-lport 1000 'eth.type == 0x1235 &&
inport == "lp11"' drop
 ovn-nbctl acl-add lsw0 to-lport 1000 'eth.type == 0x1236 &&
outport == "lp33"' drop
+ovn-nbctl create Address_Set name=set1
addresses=\"f0:00:00:00:00:11\",\"f0:00:00:00:00:21\",\"f0:00:00:00:00:31\"
+ovn-nbctl acl-add lsw0 to-lport 1000 'eth.type == 0x1237 &&
eth.src == $set1 && outport == "lp33"' drop



it would be good to add some more tests to exercise the delete and 
update codepaths.

If you think it would help, I could take a stab at them.

I agree. I even saw your pull request. Thanks.

Thank you,
Babu
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v3 1/2] Add address set support.

2016-06-24 Thread Babu Shanmugam


Hi Flavio,
I agree with your suggestions and will include them in the next version.

Thank you,
Babu

On Thursday 23 June 2016 10:46 PM, Flaviof wrote:



On Thu, Jun 23, 2016 at 1:05 AM, <bscha...@redhat.com 
<mailto:bscha...@redhat.com>> wrote:


From: Russel Bryant <russ...@ovn.org <mailto:russ...@ovn.org>>

Update the OVN expression parser to support address sets. Previously,
you could have a set of IP or MAC addresses in this form:

{addr1, addr2, ..., addrN}

This patch adds support for a bit of indirection where we can define a
set of addresses and refer to them by name.

$name

This '$name' can be used in the expresssions like

{addr1, addr2, $name, ... }
{$name}
$name

A future patch will expose the ability to define address sets for use.

Signed-off-by: Russell Bryant <russ...@ovn.org
<mailto:russ...@ovn.org>>
Co-authored-by: Babu Shanmugam <bscha...@redhat.com
<mailto:bscha...@redhat.com>>
Signed-off-by: Babu Shanmugam <bscha...@redhat.com
<mailto:bscha...@redhat.com>>
---
 ovn/controller/lflow.c |   2 +-
 ovn/lib/actions.c  |   2 +-
 ovn/lib/expr.c | 121
+++--
 ovn/lib/expr.h |  17 +++
 ovn/lib/lex.c  |  16 +++
 ovn/lib/lex.h  |   1 +
 tests/ovn.at <http://ovn.at>   | 50 
 tests/test-ovn.c   |  31 +++--
 8 files changed, 230 insertions(+), 10 deletions(-)


Please see tests I suggest adding in this commit:

https://github.com/anbu-enovance/ovs/pull/2/commits/f463a43451b85b4896f1be78e1a2ee7066b3b465



diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index efc427d..263cba6 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -297,7 +297,7 @@ add_logical_flows(struct controller_ctx *ctx,
const struct lport_index *lports,
 struct hmap matches;
 struct expr *expr;

-expr = expr_parse_string(lflow->match, , );
+expr = expr_parse_string(lflow->match, , NULL,
);
 if (!error) {
 if (prereqs) {
 expr = expr_combine(EXPR_T_AND, expr, prereqs);
diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index 4a486a0..0771b61 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -180,7 +180,7 @@ add_prerequisite(struct action_context *ctx,
const char *prerequisite)
 struct expr *expr;
 char *error;

-expr = expr_parse_string(prerequisite, ctx->ap->symtab, );
+expr = expr_parse_string(prerequisite, ctx->ap->symtab, NULL,
);
 ovs_assert(!error);
 ctx->prereqs = expr_combine(EXPR_T_AND, ctx->prereqs, expr);
 }
diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c
index f274ab4..f87ece0 100644
--- a/ovn/lib/expr.c
+++ b/ovn/lib/expr.c
@@ -451,6 +451,7 @@ struct expr_field {
 struct expr_context {
 struct lexer *lexer;/* Lexer for pulling more tokens. */
 const struct shash *symtab; /* Symbol table. */
+const struct shash *macros; /* Table of macros. */
 char *error;/* Error, if any, otherwise NULL. */
 bool not;   /* True inside odd number of NOT
operators. */
 };
@@ -772,6 +773,41 @@ assign_constant_set_type(struct expr_context
*ctx,
 }

 static bool
+parse_macros(struct expr_context *ctx, struct expr_constant_set *cs,
+  size_t *allocated_values)
+{
+if (!ctx->macros) {
+expr_syntax_error(ctx, "No macros defined.");
+return false;
+}
+
+struct expr_constant_set *addr_set
+= shash_find_data(ctx->macros, ctx->lexer->token.s);
+if (!addr_set) {
+expr_syntax_error(ctx, "Unknown macro: '%s'",
ctx->lexer->token.s);
+return false;
+}
+



+   cs->type = EXPR_C_INTEGER;


// call use assign_constant_set_type() instead of blindly
// setting expr_constant_set's type. This way, we can stop
// cases when type is unxecpected by the macro parsing:
if (!assign_constant_set_type(ctx, cs, EXPR_C_INTEGER)) {

return false;

  }



+   size_t n_values = cs->n_values + addr_set->n_values;
+if (n_values >= *allocated_values) {
+cs->values = xrealloc(cs->values, n_values * sizeof
*cs->values);
+*allocated_values = n_values;
+}
+size_t i;
+for (i = 0; i < addr_set->n_values; i++) {
+union expr_constant *c1 = >values[cs->n_values++];
+union expr_constant *c2 = _set->values[i];
+c1->value = c2->value;
+

Re: [ovs-dev] [PATCH v3 3/3] DSCP marking on packets egressing VIF interface

2016-06-24 Thread Babu Shanmugam




On Friday 24 June 2016 09:52 AM, Ben Pfaff wrote:

On Mon, Jun 13, 2016 at 02:00:21PM +0530, bscha...@redhat.com wrote:

ovn-northd sets 'ip.dscp' to the DSCP value

Signed-off-by: Babu Shanmugam <bscha...@redhat.com>

The documentation says:
 Value should be in the range of 0 to 64.
in a couple of places.  I think that 64 is not a valid value so,
usually, I would write this as:
 Value should be in the range of 0 to 63.
or:
 Value should be in the range of 0 to 63 (inclusive).
if I wanted to be perfectly clear.

Acked-by: Ben Pfaff <b...@ovn.org>

I fixed this and pushed another revision.

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

Re: [ovs-dev] [PATCH v3 2/3] Check and allocate free qdisc queue id for ports with qos parameters

2016-06-24 Thread Babu Shanmugam


I rebased this patch and pushed for review.

Thank you,
Babu

On Friday 24 June 2016 09:50 AM, Ben Pfaff wrote:

On Mon, Jun 13, 2016 at 02:00:20PM +0530, bscha...@redhat.com wrote:

ovn-northd processes the list of Port_Bindings and hashes the list of
queues per chassis. When it finds a port with qos_parameters and without
a queue_id, it allocates a free queue for the chassis that this port belongs.
The queue_id information is stored in the options field of Port_binding table.
Adds an action set_queue to the ingress table 0 of the logical flows
which will be translated to openflow set_queue by ovn-controller

ovn-controller opens the netdev corresponding to the tunnel interface's
status:tunnel_egress_iface value and configures a HTB qdisc on it. Then for
each SB port_binding that has queue_id set, it allocates a queue with the
qos_parameters of that port. It also frees up unused queues.

This patch replaces the older approach of policing

Signed-off-by: Babu Shanmugam <bscha...@redhat.com>

My apologies, but this now has some patch rejects due to a patch I
applied earlier today.  Will you rebase it?

Thanks,

Ben.


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

Re: [ovs-dev] [PATCH v3 2/2] ovn: Add address_set() support for ACLs.

2016-06-23 Thread Babu Shanmugam


Hi Han,

On Thursday 23 June 2016 12:03 PM, Han Zhou wrote:

Thanks Babu for taking over this. I'd like to repost my comment here:

On Wed, Jun 22, 2016 at 10:05 PM, > wrote:


> diff --git a/ovn/ovn-sb.ovsschema b/ovn/ovn-sb.ovsschema
> index 06e8a07..22f7ad0 100644
> --- a/ovn/ovn-sb.ovsschema
> +++ b/ovn/ovn-sb.ovsschema
> @@ -1,7 +1,7 @@
>  {
>  "name": "OVN_Southbound",
> -"version": "1.3.0",
> -"cksum": "654726257 5528",
> +"version": "1.4.0",
> +"cksum": "2272541964 5853",
>  "tables": {
>  "Chassis": {
>  "columns": {
> @@ -28,6 +28,14 @@
>   "min": 0,
>   "max": "unlimited"}},
>  "ip": {"type": "string"}}},
> +"Address_Set": {
> +"columns": {
> +"name": {"type": "string"},
> +"addresses": {"type": {"key": "string",
> +   "min": 0,
> +   "max": "unlimited"}}},
> +"indexes": [["name"]],
> +"isRoot": true},

It may be good to have column "external_ids", so that external names, 
such as security-group name in neutron, can be recognized easily.



I think I missed it. Will add it in the next version.

Thanks,
Han



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

Re: [ovs-dev] [PATCH v3 2/2] ovn: Add address_set() support for ACLs.

2016-06-23 Thread Babu Shanmugam


Hi Zong Kai, Li,
My comments below.

On Thursday 23 June 2016 02:34 PM, Zong Kai YL Li wrote:

Hi, Babu.
I get a few questions about this patch:
 - 1, About SB DB. It says to support ACLs, so why do we need a 
identical table exists in both NB DB and NB DB? I can figure that out, 
please give some explanation.
When address sets are created in NB DB, it will be present in ACLs, but 
in SB DB it will be present in Logical_Flows. And the address-set 
replica in SB DB will be used by the controller to construct openflow 
flows from the Logical_Flows.
 - 2, About addresses format. Should the addresses in the same set 
keep the same format/type, all are MAC or IPv4 or IPv6 address format, 
not mix.

   if it should be, please update ovn-nb.xml for that.
Yes. It imposes the same restrictions as a list that is used in the 
expression language. It is also necessary that the address set is used 
in a list with the same format type. You can check that by

echo 'ip4.src == {1.2.0.0/20, ::1}' | ./tests/ovstest test-ovn expr-to-flows
which will fail.
 - 3, About utilities. Directly insert data into DB seems horrible, I 
prefer ovn-nbctl can support this.

   and by this, you can check addresses format to avoid inconsistent.
   and BTW, how about add a CLI to support extend addresses for a 
address-set.

I think thats a good suggestion.

Thanks and have a nice day! :)
Zong Kai, LI


I hope I answered your questions.
Thank you,
Babu
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2 1/2] Check and allocate free qdisc queue id for ports with qos parameters

2016-06-07 Thread Babu Shanmugam




On Monday 06 June 2016 08:30 PM, Ben Pfaff wrote:

On Mon, Jun 06, 2016 at 02:31:03PM +0530, Babu Shanmugam wrote:

On Saturday 04 June 2016 01:17 AM, Ben Pfaff wrote:

On Wed, May 25, 2016 at 03:35:44PM +0530, bscha...@redhat.com wrote:
I'm a little worried about the way that this will re-set all of the
queues on every iteration through the ovn-controller main loop.  That
could have some performance drawbacks.  However, maybe it's not worth
worrying about it until it's a real problem in practice.

I think setup_qos() does not touch the queues whose parameters are
consistent with the qos_settings on the logical_port.
It will modify the queues only when there is a change in the logical_ports.

You're right.

It does still go to the kernel to dump all the queues, which is the
expensive part in the common case.  I guess I won't worry about it until
it's a problem.


This patch makes ovn-controller configure queues directly on the egress
interface.  That is completely fine.  However, it means that if the
egress interface is on an OVS bridge (which is often reasonable,
especially if bonding is involved), then ovs-vswitchd will fight with
ovn-controller for control of the queues.  This is a long-time issue in
ovs-vswitchd, and there has been previous discussion:
 http://openvswitch.org/pipermail/discuss/2015-May/017687.html
(Currently, I favor the solution proposed there of adding a new QoS
type.)

I too feel that new Qos type is a better solution. I could not locate the
code which handles the new type, is it already available?

None of the solutions presented there has been implemented.
Ben, when I was trying to work on linux-noop QOS type and I noticed a 
qos type already in lib/netdev-linux.c named 'linux-default'. Can this 
qos type work here?

I don't think it's a good assumption that all the tunnels share the same
egress interface.

I agree to this. So, we now have to set queues on all possible egreess
interfaces. Is my understanding correct?

Yes.


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

Re: [ovs-dev] [PATCH v2 1/2] Check and allocate free qdisc queue id for ports with qos parameters

2016-06-06 Thread Babu Shanmugam




On Monday 06 June 2016 08:30 PM, Ben Pfaff wrote:

On Mon, Jun 06, 2016 at 02:31:03PM +0530, Babu Shanmugam wrote:

On Saturday 04 June 2016 01:17 AM, Ben Pfaff wrote:

On Wed, May 25, 2016 at 03:35:44PM +0530, bscha...@redhat.com wrote:
I'm a little worried about the way that this will re-set all of the
queues on every iteration through the ovn-controller main loop.  That
could have some performance drawbacks.  However, maybe it's not worth
worrying about it until it's a real problem in practice.

I think setup_qos() does not touch the queues whose parameters are
consistent with the qos_settings on the logical_port.
It will modify the queues only when there is a change in the logical_ports.

You're right.

It does still go to the kernel to dump all the queues, which is the
expensive part in the common case.  I guess I won't worry about it until
it's a problem.


This patch makes ovn-controller configure queues directly on the egress
interface.  That is completely fine.  However, it means that if the
egress interface is on an OVS bridge (which is often reasonable,
especially if bonding is involved), then ovs-vswitchd will fight with
ovn-controller for control of the queues.  This is a long-time issue in
ovs-vswitchd, and there has been previous discussion:
 http://openvswitch.org/pipermail/discuss/2015-May/017687.html
(Currently, I favor the solution proposed there of adding a new QoS
type.)

I too feel that new Qos type is a better solution. I could not locate the
code which handles the new type, is it already available?

None of the solutions presented there has been implemented.

I will include a new Qos type in the v3 version of this patch.

I don't think it's a good assumption that all the tunnels share the same
egress interface.

I agree to this. So, we now have to set queues on all possible egreess
interfaces. Is my understanding correct?

Yes.


Thank you,
Babu
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2 1/2] Check and allocate free qdisc queue id for ports with qos parameters

2016-06-06 Thread Babu Shanmugam




On Saturday 04 June 2016 01:17 AM, Ben Pfaff wrote:

On Wed, May 25, 2016 at 03:35:44PM +0530, bscha...@redhat.com wrote:

ovn-northd processes the list of Port_Bindings and hashes the list of
queues per chassis. When it finds a port with qos_parameters and without
a queue_id, it allocates a free queue for the chassis that this port belongs.
The queue_id information is stored in the options field of Port_binding table.
Adds an action set_queue to the ingress table 0 of the logical flows
which will be translated to openflow set_queue by ovn-controller

ovn-controller opens the netdev corresponding to the tunnel interface's
status:tunnel_egress_iface value and configures a HTB qdisc on it. Then for
each SB port_binding that has queue_id set, it allocates a queue with the
qos_parameters of that port. It also frees up unused queues.

This patch replaces the older approach of policing

Signed-off-by: Babu Shanmugam <bscha...@redhat.com>

This is starting to look really good!  I'm becoming enthusiastic about
it now.  Nevertheless, I have some comments.

Please write comments like
 /* This is a comment. */
to match the existing OVS style.

In parse_set_queue_action(), please make the error message more helpful
by adding details, maybe something like "Queue ID %d for set_queue is
not in valid range %d to %d."

QDISC_MIN_QUEUE_ID and QDISC_MAX_QUEUE_ID are declared in two different
places.  I'd suggest putting them into ovn/lib/actions.h.

In join_logical_ports(), the cast to uint32_t doesn't seem necessary.

In setup_qos(), this || should probably be &&:
 if (sb_info->max_rate == smap_get_int(_details, "max-rate", 
0) ||
 sb_info->burst == smap_get_int(_details, "burst", 0)) {

setup_qos() could use NETDEV_QUEUE_FOR_EACH for a small amount of
convenience.

I agree to all the above comments and will make sure v3 does all these.

I'm a little worried about the way that this will re-set all of the
queues on every iteration through the ovn-controller main loop.  That
could have some performance drawbacks.  However, maybe it's not worth
worrying about it until it's a real problem in practice.
I think setup_qos() does not touch the queues whose parameters are 
consistent with the qos_settings on the logical_port.

It will modify the queues only when there is a change in the logical_ports.

This patch makes ovn-controller configure queues directly on the egress
interface.  That is completely fine.  However, it means that if the
egress interface is on an OVS bridge (which is often reasonable,
especially if bonding is involved), then ovs-vswitchd will fight with
ovn-controller for control of the queues.  This is a long-time issue in
ovs-vswitchd, and there has been previous discussion:
 http://openvswitch.org/pipermail/discuss/2015-May/017687.html
(Currently, I favor the solution proposed there of adding a new QoS
type.)
I too feel that new Qos type is a better solution. I could not locate 
the code which handles the new type, is it already available?

I don't think it's a good assumption that all the tunnels share the same
egress interface.
I agree to this. So, we now have to set queues on all possible egreess 
interfaces. Is my understanding correct?

Thanks,

Ben.

Thanks for the review, Ben.
Babu
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [RFC] QOS using queues in OVN

2016-05-17 Thread Babu Shanmugam


Hi,
Following are my thoughts on how we can do egress shaping and dscp 
marking in OVN from what I understood from this 
 mail thread;


- Client sets the rate, burst and dscp parameters in 'options' field on 
the NB Logical_Port table.

- ovn-northd does the following
 - Figures out a free queue number and sets it in the 'options' 
field of SB Port_Binding table

 - Copies the rate and burst parameters from NB Logical_Port table.
 - Sets new actions 'set_queue(queue_number)' and 
'mark_dscp(dscp_value)'  in Logical_Flow that corresponds to the 
'inport' in the ls_in_port_sec_l2 stage.

- ovn-controller does the following
- Opens the netdev that has the 'ovn-encap-ip' address. Does 
'ovn-encap-ip' always belong to a physical device? Is there a better way 
in finding a physical interface?

- Checks and creates qdisc of type linux-htb/linux-hfsc
- Checks and creates queues for all the logical ports and sets 
rate, burst parameters
- Implements the necessary openflow actions for 'set_queue' and 
'mark_dscp' logical actions


Thank you,
Babu


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

Re: [ovs-dev] [PATCH 0/2] ovn: QOS updates with DSCP support

2016-05-17 Thread Babu Shanmugam


Thank you for your answers Ben.
I tried configuring a htb qdisc on a physical interface which is not 
attached to any bridge and found the egress shaping working on a tap 
device attached to br-int.


Babu

On Tuesday 17 May 2016 01:37 AM, Ben Pfaff wrote:

Hi Bryan, I think that you understand how QoS works in NVP.  We're
currently talking about how to implement QoS in OVN.  Can you help us
understand the issues?

...now back to the conversation already in progress:

On Tue, May 10, 2016 at 05:04:06PM +0530, Babu Shanmugam wrote:

On Friday 06 May 2016 10:33 PM, Ben Pfaff wrote:

But I'm still having trouble understanding the whole design here.
Without this patch, OVN applies ingress policing to packets received

>from (typically) a VM.  This limits the rate at which the VM can

introduce packets into the network, and thus acts as a direct (if
primitive) way to limit the VM's bandwidth resource usage on the
machine's physical interface.

With this patch, OVN applies shaping to packets *sent* to (typically) a
VM.  This limits the rate at which the VM can consume packets*from*  the
network.  This has no direct effect on the VM's consumption of bandwidth
resources on the network, because the packets that are discarded have
already consumed RX resources on the machine's physical interface and
there is in fact no direct way to prevent remote machines from sending
packets for the local machine to receive.  It might have an indirect
effect on the VM's bandwidth consumption, since remote senders using
(e.g.) TCP will notice that their packets were dropped and reduce their
sending rate, but it's far less efficient at it than shaping packets
going out to the network.

The design I expected to see in OVN, eventually, was this:

 - Each VM/VIF gets assigned a queue.  Packets received from the
   VM are tagged with the queue using an OpenFlow "set_queue"
   action (dunno if we have this as an OVN logical action yet but
   it's easily added).

 - OVN programs the machine's physical interface with a linux-htb
   or linux-hfsc qdisc that grants some min-rate to each
   queue.

 From what I understand, to setup egress shaping for a VIF interface
- We need a physical interface attached to br-int

It doesn't have to be attached to br-int, because queuing information is
preserved over a hop from bridge to bridge and through encapsulation in
tunnels, but OVN would have to configure queues on the interface
regardless of what bridge it was in.


- QOS and Queue tables has to be setup for the port entry that corresponds
to the physical interface
- Packets received from VIF are put in these queues using set_queue.
Is my understanding correct?

Yes, I believe so.


Is there any way that HTB/HFSC queues can work without a physical interface
attached to br-int? if not are we going to mandate in some way that a
physical interface has to be attached to br-int?

I don't think it's desirable or necessary to attach the physical
interface to br-int, only to ensure that the queues are configured on
it.

Bryan, what does NVP do?  In particular, does it configure queues on a
physical interface without caring what bridge it is attached to?

Thanks,

Ben.


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

Re: [ovs-dev] [PATCH 0/2] ovn: QOS updates with DSCP support

2016-05-10 Thread Babu Shanmugam




On Friday 06 May 2016 10:33 PM, Ben Pfaff wrote:

But I'm still having trouble understanding the whole design here.
Without this patch, OVN applies ingress policing to packets received
from (typically) a VM.  This limits the rate at which the VM can
introduce packets into the network, and thus acts as a direct (if
primitive) way to limit the VM's bandwidth resource usage on the
machine's physical interface.

With this patch, OVN applies shaping to packets*sent*  to (typically) a
VM.  This limits the rate at which the VM can consume packets*from*  the
network.  This has no direct effect on the VM's consumption of bandwidth
resources on the network, because the packets that are discarded have
already consumed RX resources on the machine's physical interface and
there is in fact no direct way to prevent remote machines from sending
packets for the local machine to receive.  It might have an indirect
effect on the VM's bandwidth consumption, since remote senders using
(e.g.) TCP will notice that their packets were dropped and reduce their
sending rate, but it's far less efficient at it than shaping packets
going out to the network.

Thank you for the information Ben.

The design I expected to see in OVN, eventually, was this:

 - Each VM/VIF gets assigned a queue.  Packets received from the
   VM are tagged with the queue using an OpenFlow "set_queue"
   action (dunno if we have this as an OVN logical action yet but
   it's easily added).

 - OVN programs the machine's physical interface with a linux-htb
   or linux-hfsc qdisc that grants some min-rate to each queue.

From what I understand, to setup egress shaping for a VIF interface
- We need a physical interface attached to br-int
- QOS and Queue tables has to be setup for the port entry that 
corresponds to the physical interface

- Packets received from VIF are put in these queues using set_queue.

Is my understanding correct?
Is there any way that HTB/HFSC queues can work without a physical 
interface attached to br-int? if not are we going to mandate in some way 
that a physical interface has to be attached to br-int?


Thank you,
Babu


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

Re: [ovs-dev] ovn: Issue in ACL stage when the dhcp reply packet is resumed

2016-05-05 Thread Babu Shanmugam


Numan,
It happens because flow chain is paused at the DHCP flow and while 
resuming, the inport is set 0. When the packet traverse down the 
pipeline, while processing an upcall for conntrack in the 49th table, a 
lookup for DP inport 0 is performed and could not be found. Hence, it 
was not able to proceed further.


I think using packet-out with necessary actions in the userdata for 
packet traversal would be better. We can use logical flow syntax such as 
"output(inport)" to imply that the response is a packet-out to the 
inport from which packet came. Suggestion are welcome.


Thank you,
Babu

On Wednesday 13 April 2016 09:23 PM, Numan Siddique wrote:

Hi,

I am seeing an issue while testing the ovn native dhcp implementation.

When the dhcp reply packet from ovn-controller is resumed by ovs-vswitchd,
the packet is getting dropped in the egress pipeline - ACL stage.

I see the below messages in the vswitchd.log

--
2016-04-13T14:14:03.813Z|2|ofproto_dpif_upcall(handler1)|INFO|received
packet on unassociated datapath port 0
2016-04-13T14:14:03.860Z|1|ofproto_dpif_upcall(revalidator2)|WARN|Failed
to acquire udpif_key corresponding to unexpected flow (Invalid argument):
ufid:a33e557f-369f-404e-aefb-2e3da678b09a



In the function upcall_receive(), I could see that xlate_lookup() is
returning error.

Also I could see that in the function odp_flow_key_to_flow__()
L5191,  nl_attr_get_odp_port(attrs[OVS_KEY_ATTR_IN_PORT]) is returning 0.


Please let me know if you need further information.

Thanks
Numan
___
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 0/2] ovn: QOS updates with DSCP support

2016-04-27 Thread Babu Shanmugam




On Tuesday 26 April 2016 08:43 PM, Ben Pfaff wrote:

On Mon, Apr 25, 2016 at 04:41:40PM +0530, Babu Shanmugam wrote:

On Friday 22 April 2016 10:51 PM, Ben Pfaff wrote:

On Fri, Apr 22, 2016 at 12:44:12PM +0530, bscha...@redhat.com wrote:

From: Babu Shanmugam <bscha...@redhat.com>

Following are done through this series
1. Changed the old approach of policing the packets. It is now shaped
with queues. Changed the Logical_Port options for NB db
2. Support of DSCP marking through options field in Logical_Port table

Babu Shanmugam (2):
   ovn: Replace the QOS policing parameters with the usage of QOS table
   ovn: QOS DSCP markings for ports

Have you tested this?  There are at least two aspects that seem relevant
to testing.  First, propagating queuing through tunnels is somewhat
indirect and one needs to make sure that the QoS configuration actually
makes it to the physical device.  Second, HTB has a reputation for poor
quality for links above about 1 Gbps, which isn't very fast
anymore--that's why we also support HFSC.

Ben, I have not tested these aspects. The reason I used HTB is mainly
because it supports
burst setting. From vswithc.conf.db man page, HFSC does not seem to have an
option
for burst setting.
I could not understand how "propagating queuing through tunnels is somewhat
indirect". I can test it if you
can give some more information on the problem.

Usually for shaping it only makes sense to configure it on the physical
NIC network device.  Does your series do that?  If you haven't tested
it, it's hard for me to imagine it working.

Why is burst setting valuable?
I tried testing the HTB rate params on a veth device. It seems to work. 
Thanks to iperf, we can see that it tries to control the traffic 
egressing the veth device. With out the HTB queue, it occupies the full 
bandwidth.


I faced some problems while testing. How much ever max-rate I try to set 
in the Queue table, I get a kernel warning "HTB: quantum of class 1FFFE 
is big. Consider r2q change". I see that OVS is using r2q = 10. I tried 
a rate as high as 2 to as low as 600, it still gives me that 
message. Because of this the kernel assigns a fixed quantum of 20 
always. I am still debugging this.


Openstack QOS policies demands burst setting as part of bandwidth rules. 
But, I am not sure how to get the burst working in HFSC.


Thank you,
Babu
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 0/2] ovn: QOS updates with DSCP support

2016-04-25 Thread Babu Shanmugam


On Friday 22 April 2016 10:51 PM, Ben Pfaff wrote:

On Fri, Apr 22, 2016 at 12:44:12PM +0530, bscha...@redhat.com wrote:

From: Babu Shanmugam <bscha...@redhat.com>

Following are done through this series
1. Changed the old approach of policing the packets. It is now shaped
with queues. Changed the Logical_Port options for NB db
2. Support of DSCP marking through options field in Logical_Port table

Babu Shanmugam (2):
   ovn: Replace the QOS policing parameters with the usage of QOS table
   ovn: QOS DSCP markings for ports

Have you tested this?  There are at least two aspects that seem relevant
to testing.  First, propagating queuing through tunnels is somewhat
indirect and one needs to make sure that the QoS configuration actually
makes it to the physical device.  Second, HTB has a reputation for poor
quality for links above about 1 Gbps, which isn't very fast
anymore--that's why we also support HFSC.
Ben, I have not tested these aspects. The reason I used HTB is mainly 
because it supports
burst setting. From vswithc.conf.db man page, HFSC does not seem to have 
an option

for burst setting.
I could not understand how "propagating queuing through tunnels is 
somewhat indirect". I can test it if you

can give some more information on the problem.


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

Re: [ovs-dev] [PATCH] Use 'RUNDIR' from make for rhel/ovn-controller.service

2016-03-06 Thread Babu Shanmugam




$ wget https://patchwork.ozlabs.org/patch/587935/mbox/ -O - | git am
...
Applying: Use 'RUNDIR' from make for rhel/ovn-controller.service
error: patch failed: rhel/automake.mk:50 
error: rhel/automake.mk : patch does not apply
Patch failed at 0001 Use 'RUNDIR' from make for 
rhel/ovn-controller.service

The copy of the patch that failed is found in:
   /home/rbryant/src/ovs/.git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


Babu, can you take a look?  Did you send this with "git send-email"?  
If not, can you try that?


rhel/automake.mk was changed after I published this patch for review. 
Hence this patch will not apply as it is. I will rebase and publish it soon.


Thank you,
Babu
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH v2] ovn: OVN changes for QOS settings

2016-02-09 Thread Babu Shanmugam

From: Ben Pfaff <b...@ovn.org>

Signed-off-by: Ben Pfaff <b...@ovn.org>
Signed-off-by: Babu Shanmugam <bscha...@redhat.com>
---
 ovn/controller/binding.c| 46 -
 ovn/controller/ovn-controller.c |  4 
 ovn/ovn-nb.xml  | 17 +++
 ovn/ovn-sb.xml  | 17 +++
 4 files changed, 70 insertions(+), 14 deletions(-)

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index ce9cccf..f295473 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2015 Nicira, Inc.
+/* Copyright (c) 2015, 2016 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -47,7 +47,7 @@ binding_register_ovs_idl(struct ovsdb_idl *ovs_idl)
 }
 
 static void
-get_local_iface_ids(const struct ovsrec_bridge *br_int, struct sset *lports)
+get_local_iface_ids(const struct ovsrec_bridge *br_int, struct shash *lports)
 {
 int i;
 
@@ -68,7 +68,7 @@ get_local_iface_ids(const struct ovsrec_bridge *br_int, 
struct sset *lports)
 if (!iface_id) {
 continue;
 }
-sset_add(lports, iface_id);
+shash_add(lports, iface_id, iface_rec);
 }
 }
 }
@@ -132,6 +132,17 @@ add_local_datapath(struct hmap *local_datapaths,
 }
 }
 
+static void
+update_qos(const struct ovsrec_interface *iface_rec,
+   const struct sbrec_port_binding *pb)
+{
+int rate = smap_get_int(>options, "policing_rate", 0);
+int burst = smap_get_int(>options, "policing_burst", 0);
+
+ovsrec_interface_set_ingress_policing_rate(iface_rec, MAX(0, rate));
+ovsrec_interface_set_ingress_policing_burst(iface_rec, MAX(0, burst));
+}
+
 void
 binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
 const char *chassis_id, struct simap *ct_zones,
@@ -139,8 +150,6 @@ binding_run(struct controller_ctx *ctx, const struct 
ovsrec_bridge *br_int,
 {
 const struct sbrec_chassis *chassis_rec;
 const struct sbrec_port_binding *binding_rec;
-struct sset lports, all_lports;
-const char *name;
 
 if (!ctx->ovnsb_idl_txn) {
 return;
@@ -151,15 +160,19 @@ binding_run(struct controller_ctx *ctx, const struct 
ovsrec_bridge *br_int,
 return;
 }
 
-sset_init();
-sset_init(_lports);
+struct shash lports = SHASH_INITIALIZER();
 if (br_int) {
 get_local_iface_ids(br_int, );
 } else {
 /* We have no integration bridge, therefore no local logical ports.
  * We'll remove our chassis from all port binding records below. */
 }
-sset_clone(_lports, );
+
+struct sset all_lports = SSET_INITIALIZER(_lports);
+struct shash_node *node;
+SHASH_FOR_EACH (node, ) {
+sset_add(_lports, node->name);
+}
 
 ovsdb_idl_txn_add_comment(
 ctx->ovnsb_idl_txn,"ovn-controller: updating port bindings for '%s'",
@@ -169,14 +182,19 @@ binding_run(struct controller_ctx *ctx, const struct 
ovsrec_bridge *br_int,
  * chassis and update the binding accordingly.  This includes both
  * directly connected logical ports and children of those ports. */
 SBREC_PORT_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) {
-if (sset_find_and_delete(, binding_rec->logical_port) ||
-(binding_rec->parent_port && binding_rec->parent_port[0] &&
- sset_contains(_lports, binding_rec->parent_port))) {
+const struct ovsrec_interface *iface_rec
+= shash_find_and_delete(, binding_rec->logical_port);
+if (iface_rec
+|| (binding_rec->parent_port && binding_rec->parent_port[0] &&
+sset_contains(_lports, binding_rec->parent_port))) {
 if (binding_rec->parent_port && binding_rec->parent_port[0]) {
 /* Add child logical port to the set of all local ports. */
 sset_add(_lports, binding_rec->logical_port);
 }
 add_local_datapath(local_datapaths, binding_rec);
+if (iface_rec) {
+update_qos(iface_rec, binding_rec);
+}
 if (binding_rec->chassis == chassis_rec) {
 continue;
 }
@@ -199,13 +217,13 @@ binding_run(struct controller_ctx *ctx, const struct 
ovsrec_bridge *br_int,
 }
 }
 
-SSET_FOR_EACH (name, ) {
-VLOG_DBG("No port binding record for lport %s", name);
+SHASH_FOR_EACH (node, ) {
+VLOG_DBG("No port binding record for lport %s", node->name);
 }
 
 update_ct_zones(_lports, ct_zones, ct_zone_bitmap);
 
-sset_destroy();
+shash_destroy();
 sset_destroy(_lports);
 }
 
diff --git a/ovn/controller

Re: [ovs-dev] [PATCH 1/3] ovn: ovn-controller changes for qos settings

2016-02-08 Thread Babu Shanmugam


The qos settings are managed using the 'options' fields in the
"Port_Binding" table.

Signed-off-by: Babu Shanmugam <bscha...@redhat.com>

This seems more complicated than necessary.  In the
SBREC_PORT_BINDING_FOR_EACH loop in binding_run(), I think that we only
need to be able to find the ovsrec_interface associated with the
binding_rec.  Then we can check and possibly update the
ovsrec_interface's ingress_policing_rate and ingress_policing_burst.
Can you explain why there's this more complicated data structure?
If not for this iface_qos hash map, we need to run a loop OVSREC 
Interface_table entries for every iteration of SBREC Port_Binding entry 
to check against its policing_values and update it if needed.
If we have an api to fetch the Interface_table entry with the interface 
id, this hash map wouldn't be needed. I was not able find such an api, 
but is there any such api?


Below is a summary on the use of this hash map;

 * At the start of binding_run(), policing values of all the interfaces
   are hashed inside this map with the interface_id as the key.
 * On each iteration of SBREC Port_Binding, this hash map is looked up
   and policing values of the SBREC port_binding and OVSREC
   Interface_table are compared and one of the following actions are taken
 o If both the values are equal, then the hash map entry
   corresponding to this interface id is removed
 o If there is a difference, the difference is noted in the hash
   map entry
 * After looping SBREC_Port_Binding, the OVSREC Inteface_table is
   looped for all the entries, and if the interface_id is present in
   the hash map, its policing values are updated.

Thank you,
Babu

Thanks,

Ben.


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

Re: [ovs-dev] [PATCH 1/3] ovn: ovn-controller changes for qos settings

2016-02-08 Thread Babu Shanmugam

Ben, I tested your solution and it works well. It is in fact simpler 
than my approach. I will squash the other two commits along with this 
for v2.



Thank you,
Babu

On Tuesday 09 February 2016 05:44 AM, Ben Pfaff wrote:

On Mon, Feb 08, 2016 at 03:20:11PM +0530, Babu Shanmugam wrote:

The qos settings are managed using the 'options' fields in the
"Port_Binding" table.

Signed-off-by: Babu Shanmugam <bscha...@redhat.com>

This seems more complicated than necessary.  In the
SBREC_PORT_BINDING_FOR_EACH loop in binding_run(), I think that we only
need to be able to find the ovsrec_interface associated with the
binding_rec.  Then we can check and possibly update the
ovsrec_interface's ingress_policing_rate and ingress_policing_burst.
Can you explain why there's this more complicated data structure?

If not for this iface_qos hash map, we need to run a loop OVSREC
Interface_table entries for every iteration of SBREC Port_Binding entry to
check against its policing_values and update it if needed.
If we have an api to fetch the Interface_table entry with the interface id,
this hash map wouldn't be needed. I was not able find such an api, but is
there any such api?

Below is a summary on the use of this hash map;

  * At the start of binding_run(), policing values of all the interfaces
are hashed inside this map with the interface_id as the key.
  * On each iteration of SBREC Port_Binding, this hash map is looked up
and policing values of the SBREC port_binding and OVSREC
Interface_table are compared and one of the following actions are taken
  o If both the values are equal, then the hash map entry
corresponding to this interface id is removed
  o If there is a difference, the difference is noted in the hash
map entry
  * After looping SBREC_Port_Binding, the OVSREC Inteface_table is
looped for all the entries, and if the interface_id is present in
the hash map, its policing values are updated.

Here's what I had in mind.  It's untested.

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

From: Ben Pfaff <b...@ovn.org>
Date: Mon, 8 Feb 2016 16:14:20 -0800
Subject: [PATCH] ovn-controller: Copy southbound database policing settings
  into OVS config.

Signed-off-by: Ben Pfaff <b...@ovn.org>
---
  ovn/controller/binding.c| 46 -
  ovn/controller/ovn-controller.c |  4 
  2 files changed, 36 insertions(+), 14 deletions(-)

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index ce9cccf..f295473 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2015 Nicira, Inc.
+/* Copyright (c) 2015, 2016 Nicira, Inc.
   *
   * Licensed under the Apache License, Version 2.0 (the "License");
   * you may not use this file except in compliance with the License.
@@ -47,7 +47,7 @@ binding_register_ovs_idl(struct ovsdb_idl *ovs_idl)
  }
  
  static void

-get_local_iface_ids(const struct ovsrec_bridge *br_int, struct sset *lports)
+get_local_iface_ids(const struct ovsrec_bridge *br_int, struct shash *lports)
  {
  int i;
  
@@ -68,7 +68,7 @@ get_local_iface_ids(const struct ovsrec_bridge *br_int, struct sset *lports)

  if (!iface_id) {
  continue;
  }
-sset_add(lports, iface_id);
+shash_add(lports, iface_id, iface_rec);
  }
  }
  }
@@ -132,6 +132,17 @@ add_local_datapath(struct hmap *local_datapaths,
  }
  }
  
+static void

+update_qos(const struct ovsrec_interface *iface_rec,
+   const struct sbrec_port_binding *pb)
+{
+int rate = smap_get_int(>options, "policing_rate", 0);
+int burst = smap_get_int(>options, "policing_burst", 0);
+
+ovsrec_interface_set_ingress_policing_rate(iface_rec, MAX(0, rate));
+ovsrec_interface_set_ingress_policing_burst(iface_rec, MAX(0, burst));
+}
+
  void
  binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
  const char *chassis_id, struct simap *ct_zones,
@@ -139,8 +150,6 @@ binding_run(struct controller_ctx *ctx, const struct 
ovsrec_bridge *br_int,
  {
  const struct sbrec_chassis *chassis_rec;
  const struct sbrec_port_binding *binding_rec;
-struct sset lports, all_lports;
-const char *name;
  
  if (!ctx->ovnsb_idl_txn) {

  return;
@@ -151,15 +160,19 @@ binding_run(struct controller_ctx *ctx, const struct 
ovsrec_bridge *br_int,
  return;
  }
  
-sset_init();

-sset_init(_lports);
+struct shash lports = SHASH_INITIALIZER();
  if (br_int) {
  get_local_iface_ids(br_int, );
  } else {
  /* We have no integration bridge, therefore no local logical ports.
   * We'll remove our chassis from all port binding records below. */
  }
-sset_clone(_lports, );
+
+struct sset all_lports = SSET_INITIALIZER(_lports);
+

Re: [ovs-dev] [PATCH 0/3] QOS support in OVN

2016-02-01 Thread Babu Shanmugam


Hi Ben,
Do you have more questions on this topic?

Thank you,
Babu

On Thursday 14 January 2016 06:46 PM, Miguel Angel Ajo wrote:


Hi!,

Russell Bryant wrote:

On 01/13/2016 11:30 AM, Russell Bryant wrote:

On 01/11/2016 08:19 PM, Ben Pfaff wrote:

On Tue, Jan 05, 2016 at 07:33:16PM +0530, bscha...@redhat.com wrote:

This patch series enables QOS support in OVN. Only two parameters
(policing_rate and policing_burst) are enabled through this patch 
series.


Babu Shanmugam (3):
   ovn: ovn-controller changes for qos settings
   ovn: Qos options for VMI updated in ovn-sb.xml
   ovn: Qos options for VMI updated in ovn-nb.xml
We found previously that ingress policing works particularly 
badly.  It

isn't accurate and some kind of traffic get handled in a really bad
way.  Have you have a different experience?

I suggested the QoS work item.  The background is just that I'm looking
to get parity with what OpenStack Neutron exposes for the current
integration.  Hopefully the limitations are adequately reflected in
docs.  Would you be OK including this support with appropriate caveats
in the docs?



I also asked the dev of the existing Neutron integration to weigh in
about their implementation and experience.

We're using ingress policing in neutron as a simple way to provide
VM/container - egress bandwidth policing.

It works well with TCP, but of course, UDP and other types of traffic
may need some sort of queuing, we didn't uses queues, because
neutron reference implementation makes a heavy use of NORMAL rules
and port tagging to direct traffic to the right place, that makes it 
impossible

to use queues.

Also, I found that to use queues (for VM-egress traffic) we need to 
have one
queue per traffic flow/port in the outgoing port, and as I understood 
it, we can
only create queues for linux kernel visible ports  (that excludes 
patch ports,
tunnel ports and OVS internal port bonds, but includes veth based 
patch ports,
and physical interfaces). Please correct me if I'm wrong here, 
probably there
is some sort of alternative here equivalent to IMQ's, I'm sadly not an 
expert in

this area.

So, even if it was a little bit limited seemed like reasonable initial 
step to

provide VM-egress bandwidth limits. (we left the limitations documented:
Application or protocol needs to expect packet drops).

VM-ingress/switch-egress are very easily implemented with queues.


Best regards,
Miguel Ángel.







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

Re: [ovs-dev] QOS in OVN

2015-12-30 Thread Babu Shanmugam

I think these options will apply only for the VIFs (logical_ports with 
an empty 'type').


--
Babu

On Thursday 31 December 2015 04:34 AM, Ben Pfaff wrote:
I was hoping to keep the options column for options that are specific 
to a particular type of logical port. Do you think that the QoS 
options will be type specific or generic?


On December 30, 2015 3:51:13 AM CST, Babu Shanmugam 
<bscha...@redhat.com> wrote:


I am trying to implement the QOS APIs of openstack neutron in the
networking-ovn plugin. I understand that I have to make the relevant
changes in OVN code as well.

I feel, 'options' field in Logical_Port table would be a decent
candidate to bring QoS support in OVN [1]. I could see from the
description that there are no 'options' mentioned for a VIF. Is there
any other way we could implement it through the northbound database?

[1].
Sample options - qos_(ingress | egress)_rate, qos_(ingress |
egress)_burst, qos_dscp.


--
Babu



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

Re: [ovs-dev] (no subject)

2015-12-30 Thread Babu Shanmugam


Kindly ignore this email. I sent it by mistake.

On Wednesday 30 December 2015 02:06 PM, Babu Shanmugam wrote:
I am trying to implement the QOS APIs of neutron in the networking-ovn 
plugin.


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

[ovs-dev] QOS in OVN

2015-12-30 Thread Babu Shanmugam

I am trying to implement the QOS APIs of openstack neutron in the 
networking-ovn plugin. I understand that I have to make the relevant 
changes in OVN code as well.


I feel, 'options' field in Logical_Port table would be a decent 
candidate to bring QoS support in OVN [1]. I could see from the 
description that there are no 'options' mentioned for a VIF. Is there 
any other way we could implement it through the northbound database?


[1].
Sample options - qos_(ingress | egress)_rate, qos_(ingress | 
egress)_burst, qos_dscp.



--
Babu

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

[ovs-dev] [PATCH v2 2/4] ovn: New flows for DHCP tranffic

2015-11-18 Thread Babu Shanmugam


The ovn-controller registers a flow in table 33 (LOCAL_OUTPUT) to
send a packet matching the DHCP discover/request structure to
a specific controller id, that the ovn pinctrl has registered on
it's ofconn.

Signed-off-by: Babu Shanmugam <bscha...@redhat.com>
Signed-off-by: Numan Siddique <nusid...@redhat.com>
Co-Authored-by: Numan Siddique <nusid...@redhat.com>
---
 lib/ofp-util.c| 14 ++
 lib/ofp-util.h|  3 +++
 ovn/controller/physical.c | 27 ---
 ovn/controller/pinctrl.c  |  6 ++
 ovn/controller/pinctrl.h  |  2 ++
 5 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 342be54..7cfa06b 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1544,6 +1544,20 @@ ofputil_make_set_packet_in_format(enum 
ofp_version ofp_version,

 return msg;
 }

+struct ofpbuf *
+ofputil_make_set_controller_id(enum ofp_version ofp_version,
+   uint16_t controller_id)
+{
+struct nx_controller_id *cid;
+struct ofpbuf *msg;
+
+msg = ofpraw_alloc(OFPRAW_NXT_SET_CONTROLLER_ID, ofp_version, 0);
+cid = ofpbuf_put_zeros(msg, sizeof *cid);
+cid->controller_id = htons(controller_id);
+
+return msg;
+}
+
 /* Returns an OpenFlow message that can be used to turn the 
flow_mod_table_id

  * extension on or off (according to 'flow_mod_table_id'). */
 struct ofpbuf *
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index 8914342..5373d86 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -237,6 +237,9 @@ int ofputil_packet_in_format_from_string(const char *);
 const char *ofputil_packet_in_format_to_string(enum nx_packet_in_format);
 struct ofpbuf *ofputil_make_set_packet_in_format(enum ofp_version,
  enum 
nx_packet_in_format);

+struct ofpbuf *
+ofputil_make_set_controller_id(enum ofp_version ofp_version,
+   uint16_t controller_id);

 /* NXT_FLOW_MOD_TABLE_ID extension. */
 struct ofpbuf *ofputil_make_flow_mod_table_id(bool flow_mod_table_id);
diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index 5821c11..44f72dc 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -18,6 +18,7 @@
 #include "lflow.h"
 #include "match.h"
 #include "ofctrl.h"
+#include "pinctrl.h"
 #include "ofp-actions.h"
 #include "ofpbuf.h"
 #include "ovn-controller.h"
@@ -396,17 +397,37 @@ physical_run(struct controller_ctx *ctx, enum 
mf_field_id mff_ovn_geneve,

 tag ? 150 : 100, , );
 }

-/* Table 33, priority 100.
- * ===
+/* Table 33, priority 100 and 150
+ * ==
  *
  * Implements output to local hypervisor.  Each flow matches a
  * logical output port on the local hypervisor, and 
resubmits to

- * table 34.
+ * table 34.
+ * Also, send the DHCP traffic to the controller as packet-in
  */

 match_init_catchall();
 ofpbuf_clear();

+match_set_metadata(, 
htonll(binding->datapath->tunnel_key));

+match_set_dl_type(, htons(ETH_TYPE_IP));
+match_set_nw_proto(, IPPROTO_UDP);
+match_set_nw_src(, htonl(INADDR_ANY));
+match_set_nw_dst(, htonl(INADDR_BROADCAST));
+match_set_tp_src(, htons(68));
+match_set_tp_dst(, htons(67));
+
+struct ofpact_controller *controller =
+ofpact_put_CONTROLLER();
+controller->max_len = UINT16_MAX;
+controller->controller_id = OVN_PACKET_IN_CONTROLLER_ID;
+controller->reason = OFPR_ACTION;
+ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 150, ,
+);
+
+match_init_catchall();
+ofpbuf_clear();
+
 /* Match MFF_LOG_DATAPATH, MFF_LOG_OUTPORT. */
 match_set_metadata(, 
htonll(binding->datapath->tunnel_key));

 match_set_reg(, MFF_LOG_OUTPORT - MFF_REG0,
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 2fb8ab3..1ea683b 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -98,6 +98,7 @@ pinctrl_recv(struct controller_ctx *ctx, const struct 
ofp_header *oh,

 } else if (type == OFPTYPE_GET_CONFIG_REPLY) {
 struct ofpbuf rq_buf;
 struct ofpbuf *spif;
+struct ofpbuf *scid;
 struct ofp_switch_config *config_, config;

 ofpbuf_use_const(_buf, oh, ntohs(oh->length));
@@ -105,9 +106,14 @@ pinctrl_recv(struct controller_ctx *ctx, const 
struct ofp_header *oh,

 config = *config_;
 config.miss_send_len = htons(UINT16_MAX);
 set_switch_config(swconn, );
+
 spif = 
ofputi

[ovs-dev] [PATCH v2 1/4] ovn: Dedicated connection handler for packet-ins

2015-11-18 Thread Babu Shanmugam


This patch opens and maintains a new connection that is dedicated
to monitor the packet-ins for br-int.

Signed-off-by: Babu Shanmugam <bscha...@redhat.com>
---
 ovn/controller/automake.mk  |   2 +
 ovn/controller/ovn-controller.c |   6 ++
 ovn/controller/pinctrl.c| 177 


 ovn/controller/pinctrl.h|  34 
 4 files changed, 219 insertions(+)
 create mode 100644 ovn/controller/pinctrl.c
 create mode 100644 ovn/controller/pinctrl.h

diff --git a/ovn/controller/automake.mk b/ovn/controller/automake.mk
index fec9bf1..cadfa9c 100644
--- a/ovn/controller/automake.mk
+++ b/ovn/controller/automake.mk
@@ -10,6 +10,8 @@ ovn_controller_ovn_controller_SOURCES = \
 ovn/controller/lflow.h \
 ovn/controller/ofctrl.c \
 ovn/controller/ofctrl.h \
+ovn/controller/pinctrl.c \
+ovn/controller/pinctrl.h \
 ovn/controller/patch.c \
 ovn/controller/patch.h \
 ovn/controller/ovn-controller.c \
diff --git a/ovn/controller/ovn-controller.c 
b/ovn/controller/ovn-controller.c

index 3f29b25..02ecb3e 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -41,6 +41,7 @@
 #include "util.h"

 #include "ofctrl.h"
+#include "pinctrl.h"
 #include "binding.h"
 #include "chassis.h"
 #include "encaps.h"
@@ -223,6 +224,7 @@ main(int argc, char *argv[])
 sbrec_init();

 ofctrl_init();
+pinctrl_init();
 lflow_init();

 /* Connect to OVS OVSDB instance.  We do not monitor all tables by
@@ -292,6 +294,8 @@ main(int argc, char *argv[])
 if (br_int) {
 enum mf_field_id mff_ovn_geneve = ofctrl_run(br_int);

+pinctrl_run(, br_int);
+
 struct hmap flow_table = HMAP_INITIALIZER(_table);
 lflow_run(, _table, _zones);
 if (chassis_id) {
@@ -314,6 +318,7 @@ main(int argc, char *argv[])

 if (br_int) {
 ofctrl_wait();
+pinctrl_wait();
 }
 poll_block();
 if (should_service_stop()) {
@@ -351,6 +356,7 @@ main(int argc, char *argv[])
 unixctl_server_destroy(unixctl);
 lflow_destroy();
 ofctrl_destroy();
+pinctrl_destroy();

 simap_destroy(_zones);

diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
new file mode 100644
index 000..2fb8ab3
--- /dev/null
+++ b/ovn/controller/pinctrl.c
@@ -0,0 +1,177 @@
+
+/* Copyright (c) 2015 Red Hat, 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.
+ */
+
+#include 
+#include "dirs.h"
+#include "pinctrl.h"
+#include "ofp-msgs.h"
+#include "ofp-print.h"
+#include "ofp-util.h"
+#include "rconn.h"
+#include "openvswitch/vlog.h"
+#include "socket-util.h"
+#include "vswitch-idl.h"
+
+VLOG_DEFINE_THIS_MODULE(dhcp);
+
+/* OpenFlow connection to the switch. */
+static struct rconn *swconn;
+
+/* Last seen sequence number for 'swconn'.  When this differs from
+ * rconn_get_connection_seqno(rconn), 'swconn' has reconnected. */
+static unsigned int conn_seq_no;
+
+void
+pinctrl_init(void)
+{
+swconn = rconn_create(5, 0, DSCP_DEFAULT, 0xF);
+conn_seq_no = 0;
+}
+
+static ovs_be32
+queue_msg(struct ofpbuf *msg)
+{
+const struct ofp_header *oh = msg->data;
+ovs_be32 xid = oh->xid;
+
+rconn_send(swconn, msg, NULL);
+return xid;
+}
+
+static void
+get_switch_config(struct rconn *swconn)
+{
+struct ofpbuf *request;
+
+request = ofpraw_alloc(OFPRAW_OFPT_GET_CONFIG_REQUEST,
+   rconn_get_version(swconn), 0);
+queue_msg(request);
+}
+
+static void
+set_switch_config(struct rconn *swconn, const struct ofp_switch_config 
*config)

+{
+struct ofpbuf *request;
+
+request =
+ofpraw_alloc(OFPRAW_OFPT_SET_CONFIG, rconn_get_version(swconn), 0);
+ofpbuf_put(request, config, sizeof *config);
+
+queue_msg(request);
+}
+
+static void
+process_packet_in(struct controller_ctx *ctx OVS_UNUSED,
+  const struct ofp_header *msg)
+{
+struct ofputil_packet_in pin;
+
+if (ofputil_decode_packet_in(, msg) != 0) {
+return;
+}
+if (pin.reason != OFPR_ACTION) {
+return;
+}
+
+/* XXX : process the received packet */
+}
+
+static void
+pinctrl_recv(struct controller_ctx *ctx, const struct ofp_header *oh,
+ enu

[ovs-dev] [PATCH v2 3/4] ovn: Process dhcp packet-ins and respond through packet-outs

2015-11-18 Thread Babu Shanmugam


The DHCP packets can be of two types
(1) DHCP Discover
(2) DHCP Request

For (1), the controller should respond with DHCP offer and for (2),
either DHCP Ack or DHCP Nack should be sent. In this patch, DHCP Nack
is never sent. In case of failures in validating the packet, the
controller does not respond at all.

For a DHCP packet, the IP address is read from the port binding table
with the source MAC as the search reference. The DHCP options for netmask
and router are expected with key values dhcp_opt_1 and
dhcp_opt_3 respectively in port binding table - options column.
In case of the absence of these options from
logical port entry, dhcp packet is flooded.

Signed-off-by: Babu Shanmugam <bscha...@redhat.com>
Signed-off-by: Numan Siddique <nusid...@redhat.com>
Co-Authored-by: Numan Siddique <nusid...@redhat.com>
---
 ovn/controller/automake.mk |   2 +
 ovn/controller/ovn-dhcp.c  | 493 
+

 ovn/controller/ovn-dhcp.h  |  34 
 ovn/controller/pinctrl.c   |  45 -
 ovn/ovn-nb.xml |  29 +++
 ovn/ovn-sb.xml |  29 +++
 6 files changed, 630 insertions(+), 2 deletions(-)
 create mode 100644 ovn/controller/ovn-dhcp.c
 create mode 100644 ovn/controller/ovn-dhcp.h

diff --git a/ovn/controller/automake.mk b/ovn/controller/automake.mk
index cadfa9c..fdd6788 100644
--- a/ovn/controller/automake.mk
+++ b/ovn/controller/automake.mk
@@ -16,6 +16,8 @@ ovn_controller_ovn_controller_SOURCES = \
ovn/controller/patch.h \
ovn/controller/ovn-controller.c \
ovn/controller/ovn-controller.h \
+   ovn/controller/ovn-dhcp.c \
+   ovn/controller/ovn-dhcp.h \
ovn/controller/physical.c \
ovn/controller/physical.h
 ovn_controller_ovn_controller_LDADD = ovn/lib/libovn.la 
lib/libopenvswitch.la

diff --git a/ovn/controller/ovn-dhcp.c b/ovn/controller/ovn-dhcp.c
new file mode 100644
index 000..db4442b
--- /dev/null
+++ b/ovn/controller/ovn-dhcp.c
@@ -0,0 +1,493 @@
+
+/* Copyright (c) 2015 Red Hat, 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.
+ */
+
+#include 
+#include "csum.h"
+#include "dp-packet.h"
+#include "dhcp.h"
+#include "lib/dhcp.h"
+#include "ofpbuf.h"
+#include "ofp-actions.h"
+#include "ofp-util.h"
+#include "ovn-controller.h"
+#include "ovn-dhcp.h"
+
+#define DHCP_SERVER_ID ((uint32_t)0x01010101)
+#define DHCP_LEASE_PERIOD  ((uint32_t)60*60*24) /* 1 day */
+
+#define DHCP_MAGIC_COOKIE (uint32_t)0x63825363
+
+#define DHCP_DEFAULT_NETMASK (uint32_t)0xFF00
+
+#define DHCP_DEFAULT_IFACE_MTU (uint16_t)1400
+
+#define DHCP_OP_REQUEST  ((uint8_t)1)
+#define DHCP_OP_REPLY((uint8_t)2)
+
+#define DHCP_MSG_DISCOVER ((uint8_t)1)
+#define DHCP_MSG_OFFER((uint8_t)2)
+#define DHCP_MSG_REQUEST  ((uint8_t)3)
+#define DHCP_MSG_ACK  ((uint8_t)5)
+#define DHCP_MSG_NACK ((uint8_t)6)
+
+/* DHCP options */
+#define DHCP_OPT_NETMASK  ((uint8_t)1)
+#define DHCP_OPT_ROUTER   ((uint8_t)3)
+#define DHCP_OPT_DNS_SERVER   ((uint8_t)6)
+#define DHCP_OPT_LOG_SERVER   ((uint8_t)7)
+#define DHCP_OPT_LPR_SERVER   ((uint8_t)9)
+#define DHCP_OPT_BOOT_FILE_SIZE   ((uint8_t)13)
+#define DHCP_OPT_DOMAIN_NAME  ((uint8_t)15)
+#define DHCP_OPT_SWAP_SERVER  ((uint8_t)16)
+#define DHCP_OPT_IP_FORWARD_ENABLE((uint8_t)19)
+#define DHCP_OPT_POLICY_FILTER((uint8_t)21)
+#define DHCP_OPT_DEFAULT_TTL  ((uint8_t)23)
+#define DHCP_OPT_IFACE_MTU((uint8_t)26)
+#define DHCP_OPT_RD   ((uint8_t)31)
+#define DHCP_OPT_ROUTER_SOLICITATION  ((uint8_t)32)
+#define DHCP_OPT_ARP_TIMEOUT  ((uint8_t)35)
+#define DHCP_OPT_ETH_ENCAP((uint8_t)36)
+#define DHCP_OPT_TCP_TTL  ((uint8_t)37)
+#define DHCP_OPT_TCP_KEEPALIVE((uint8_t)38)
+#define DHCP_OPT_NIS_DOMAIN   ((uint8_t)40)
+#define DHCP_OPT_NIS_SERVER   ((uint8_t)41)
+#define DHCP_OPT_NTP_SERVER   ((uint8_t)42)
+#define DHCP_OPT_ADDR_REQ ((uint8_t)50)
+#define DHCP_OPT_LEASE_TIME   ((uint8_t)51)
+#define DHCP_OPT_MSG_TYPE ((uint8_t)53)
+#define DHCP_OPT_SERVER_ID((uint8_t)54)
+#define DHCP_OPT_PARAMS   ((uint8_t)55)
+#define DHCP_OPT_TFTP_SERVER  ((uint8_t)66)
+#define DHCP_OPT_END  ((uint8_t)255

Re: [ovs-dev] [PATCH 1/4] ovn : Setup a controller for br-int

2015-11-12 Thread Babu Shanmugam


Thank you for the review Ben.

On Wednesday 11 November 2015 06:06 AM, Ben Pfaff wrote:

Sorry it's taken me so long to get back to this.

It works pretty much the same regardless of the direction of the
connection.  The only important differences are:

 * When you set up a Controller record, there can only be one
   outgoing connection to a given controller, which means that
   we'd be limited to a single OpenFlow channel for this purpose.
   That might be OK for now but I suspect that we'll want to have
   multiple connections eventually for different purposes or for
   fairness or load balancing.

I agree to this Ben, and I am working on it.

   On the other hand, ovn-controller can make as many connections
   to OVS as it likes.

 * If ovn-controller connects to OVS, it needs to send a specific
   message to turn on "packet-in"s, instead of getting them by
   default when the connection completes.

Done!


There's no real difference regarding keeping the connection up; in
either case that's easy to handle.

It seems a little cleaner to avoid setting up the Controller record,
since there's nothing to clean up on exit in that case and nothing that
an administrator can screw up by modifying the database.

I agree.


On Wed, Oct 28, 2015 at 05:32:27AM +0530, Babu Shanmugam wrote:

Hi Ben,
I understand that when an openflow controller is set on the switch, the
switch will try to connect to the controller target and keeps alive the
connection by a series of echo_requests and echo_replies. So, I thought, we
need a passive connection that listens on the target that is configured for
the controller and respond to the requests from the switch.
I also thought this will help in responding to the packet-in requests as
soon as it arrives. I hope my understanding is correct, if not, please
correct me.

Thank you,
Babu

On Sunday 25 October 2015 05:02 AM, Ben Pfaff wrote:

On Fri, Oct 23, 2015 at 01:12:50PM +0530, Babu Shanmugam wrote:

This patch configures the ovs records for a new controller to br-int
and an endpoint for the new controller

Signed-off-by: Babu Shanmugam <bscha...@redhat.com>

Doesn't it work to just start a second connection to the controller, in
the same way that ofctrl connects to the controller?


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

Re: [ovs-dev] [PATCH 1/4] ovn : Setup a controller for br-int

2015-10-27 Thread Babu Shanmugam


Hi Ben,
I understand that when an openflow controller is set on the switch, the 
switch will try to connect to the controller target and keeps alive the 
connection by a series of echo_requests and echo_replies. So, I thought, 
we need a passive connection that listens on the target that is 
configured for the controller and respond to the requests from the switch.
I also thought this will help in responding to the packet-in requests as 
soon as it arrives. I hope my understanding is correct, if not, please 
correct me.


Thank you,
Babu

On Sunday 25 October 2015 05:02 AM, Ben Pfaff wrote:

On Fri, Oct 23, 2015 at 01:12:50PM +0530, Babu Shanmugam wrote:

This patch configures the ovs records for a new controller to br-int
and an endpoint for the new controller

Signed-off-by: Babu Shanmugam <bscha...@redhat.com>

Doesn't it work to just start a second connection to the controller, in
the same way that ofctrl connects to the controller?


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

[ovs-dev] [PATCH 4/4] ovn: Process dhcp packet-ins and respond through packet-outs

2015-10-23 Thread Babu Shanmugam


The DHCP packets can be of two types
(1) DHCP Discover
(2) DHCP Request

For (1), the controller should respond with DHCP offer and for (2),
either DHCP Ack or DHCP Nack should be sent. In this patch, DHCP Nack
is never sent. In case of failures in validating the packet, the
controller does not respond at all.

For a DHCP packet, the IP address is read from the port binding table
with the source MAC as the search reference. The DHCP options for netmask
and router are expected with key values dhcp_opt_netmask and
dhcp_opt_router respectively. In case of the absence of these options from
logical port entry, default values of 255.255.255.0 and 0.0.0.0 is used
respectively.

Signed-off-by: Babu Shanmugam <bscha...@redhat.com>
Signed-off-by: Numan Siddique <nusid...@redhat.com>
Co-Authored-by: Numan Siddique <nusid...@redhat.com>
---
 ovn/controller/automake.mk|   2 +
 ovn/controller/ofcontroller.c |   9 +-
 ovn/controller/ovn-dhcp.c | 387 
++

 ovn/controller/ovn-dhcp.h |  25 +++
 ovn/ovn-nb.xml|  21 +++
 ovn/ovn-sb.xml|  21 +++
 6 files changed, 462 insertions(+), 3 deletions(-)
 create mode 100644 ovn/controller/ovn-dhcp.c
 create mode 100644 ovn/controller/ovn-dhcp.h

diff --git a/ovn/controller/automake.mk b/ovn/controller/automake.mk
index 9702b11..0d4b717 100644
--- a/ovn/controller/automake.mk
+++ b/ovn/controller/automake.mk
@@ -16,6 +16,8 @@ ovn_controller_ovn_controller_SOURCES = \
 ovn/controller/patch.h \
 ovn/controller/ovn-controller.c \
 ovn/controller/ovn-controller.h \
+ovn/controller/ovn-dhcp.c \
+ovn/controller/ovn-dhcp.h \
 ovn/controller/physical.c \
 ovn/controller/physical.h
 ovn_controller_ovn_controller_LDADD = ovn/lib/libovn.la 
lib/libopenvswitch.la

diff --git a/ovn/controller/ofcontroller.c b/ovn/controller/ofcontroller.c
index 8567925..7ba02d4 100644
--- a/ovn/controller/ofcontroller.c
+++ b/ovn/controller/ofcontroller.c
@@ -22,6 +22,7 @@
 #include "ofp-util.h"
 #include "ofp-actions.h"
 #include "ofp-version-opt.h"
+#include "ovn-dhcp.h"
 #include "openflow/openflow.h"
 #include "openvswitch/vconn.h"
 #include "openvswitch/vlog.h"
@@ -50,10 +51,10 @@ ofcontroller_init(char const *sock_path)
 }

 static void
-process_packet_in(struct controller_ctx *ctx OVS_UNUSED,
-  struct ofp_header *msg)
+process_packet_in(struct controller_ctx *ctx, struct ofp_header *msg)
 {
 struct ofputil_packet_in pin;
+struct ofpbuf *buf;

 if (ofputil_decode_packet_in(, msg) != 0) {
 return;
@@ -62,7 +63,9 @@ process_packet_in(struct controller_ctx *ctx OVS_UNUSED,
 return;
 }

-/* XXX : process the received packet */
+if (ovn_dhcp_process_packet(ctx, , ofcontroller_ofp_proto(), 
)) {

+rconn_send(rconn, buf, NULL);
+}
 }

 static void
diff --git a/ovn/controller/ovn-dhcp.c b/ovn/controller/ovn-dhcp.c
new file mode 100644
index 000..3068c3f
--- /dev/null
+++ b/ovn/controller/ovn-dhcp.c
@@ -0,0 +1,387 @@
+
+/* Copyright (c) 2015 Red Hat, 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.
+ */
+
+#include 
+#include "csum.h"
+#include "dp-packet.h"
+#include "dhcp.h"
+#include "ofpbuf.h"
+#include "ofp-actions.h"
+#include "ofp-util.h"
+#include "ovn-controller.h"
+#include "ovn-dhcp.h"
+
+#define DHCP_SERVER_ID ((uint32_t)0x01010101)
+#define DHCP_LEASE_PERIOD  ((uint32_t)60*60*24) /* 1 day */
+
+#define DHCP_CLIENT_PORT 68
+#define DHCP_SERVER_PORT 67
+
+#define DHCP_MAGIC_COOKIE (uint32_t)0x63825363
+
+#define DHCP_DEFAULT_NETMASK (uint32_t)0xFF00
+
+#define DHCP_OP_REQUEST  ((uint8_t)1)
+#define DHCP_OP_REPLY((uint8_t)2)
+
+#define DHCP_MSG_DISCOVER ((uint8_t)1)
+#define DHCP_MSG_OFFER((uint8_t)2)
+#define DHCP_MSG_REQUEST  ((uint8_t)3)
+#define DHCP_MSG_ACK  ((uint8_t)5)
+#define DHCP_MSG_NACK ((uint8_t)6)
+
+#define DHCP_OPT_NETMASK ((uint8_t)1)
+#define DHCP_OPT_ROUTER  ((uint8_t)3)
+#define DHCP_OPT_ADDR_REQ((uint8_t)50)
+#define DHCP_OPT_LEASE_TIME  ((uint8_t)51)
+#define DHCP_OPT_MSG_TYPE((uint8_t)53)
+#define DHCP_OPT_SERVER_ID   ((uint8_t)54)
+#define DHCP_OPT_PARAMS  ((uint8_t)55)
+#define DHCP_OPT_END ((uint8_t)255)
+
+#define OPTION_PAYLOAD(opt) ((char *)op

[ovs-dev] [PATCH 1/4] ovn : Setup a controller for br-int

2015-10-23 Thread Babu Shanmugam


This patch configures the ovs records for a new controller to br-int
and an endpoint for the new controller

Signed-off-by: Babu Shanmugam <bscha...@redhat.com>
---
 ovn/controller/ovn-controller.c | 62 
+++--

 1 file changed, 59 insertions(+), 3 deletions(-)

diff --git a/ovn/controller/ovn-controller.c 
b/ovn/controller/ovn-controller.c

index 3f29b25..343de28 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 

 #include "command-line.h"
@@ -56,6 +57,7 @@ static unixctl_cb_func ct_zone_list;
 #define DEFAULT_BRIDGE_NAME "br-int"

 static void parse_options(int argc, char *argv[]);
+static char const *get_switch_controller_path(void);
 OVS_NO_RETURN static void usage(void);

 static char *ovs_remote;
@@ -100,10 +102,58 @@ get_bridge(struct ovsdb_idl *ovs_idl, const char 
*br_name)

 return NULL;
 }

+static char const *
+get_switch_controller_path(void)
+{
+static char *path = NULL;
+
+if (!path) {
+path = xasprintf("%s/%s.%ld.sock",
+ ovs_rundir(), "ovn-controller", (long int) 
getpid());

+}
+return path;
+}
+
+static void
+ovs_bridge_set_controller(struct controller_ctx *ctx,
+  struct ovsrec_bridge const *br)
+{
+struct ovsrec_controller *controller;
+struct ovsdb_datum const *bctrl, *target;
+bool set = false;
+static char *proto = NULL;
+
+if (!proto) {
+proto = xasprintf("unix:%s", get_switch_controller_path());
+}
+
+if (!br || !ctx || !ctx->ovs_idl_txn || !ctx->ovs_idl) {
+return;
+}
+
+bctrl = ovsrec_bridge_get_controller(br, OVSDB_TYPE_UUID);
+if (bctrl && bctrl->n > 0) {
+struct ovsrec_controller const *ctrler;
+
+ctrler =
+ovsrec_controller_get_for_uuid(ctx->ovs_idl, 
>keys[0].uuid);

+target = ovsrec_controller_get_target(ctrler, OVSDB_TYPE_STRING);
+if (strcmp(target->keys[0].string, proto) != 0) {
+set = true;
+}
+} else {
+set = true;
+}
+if (set) {
+controller = ovsrec_controller_insert(ctx->ovs_idl_txn);
+ovsrec_controller_set_target(controller, proto);
+ovsrec_bridge_set_controller(br, , 1);
+}
+}
+
 static const struct ovsrec_bridge *
 create_br_int(struct controller_ctx *ctx,
-  const struct ovsrec_open_vswitch *cfg,
-  const char *bridge_name)
+  const struct ovsrec_open_vswitch *cfg, const char 
*bridge_name)

 {
 if (!ctx->ovs_idl_txn) {
 return NULL;
@@ -158,8 +208,9 @@ get_br_int(struct controller_ctx *ctx)
 const struct ovsrec_bridge *br;
 br = get_bridge(ctx->ovs_idl, br_int_name);
 if (!br) {
-return create_br_int(ctx, cfg, br_int_name);
+br = create_br_int(ctx, cfg, br_int_name);
 }
+ovs_bridge_set_controller(ctx, br);
 return br;
 }

@@ -246,6 +297,11 @@ main(int argc, char *argv[])
 ovsdb_idl_add_column(ovs_idl_loop.idl, _bridge_col_name);
 ovsdb_idl_add_column(ovs_idl_loop.idl, _bridge_col_fail_mode);
 ovsdb_idl_add_column(ovs_idl_loop.idl, 
_bridge_col_other_config);

+ovsdb_idl_add_column(ovs_idl_loop.idl, _bridge_col_controller);
+ovsdb_idl_add_table(ovs_idl_loop.idl, _table_controller);
+ovsdb_idl_add_column(ovs_idl_loop.idl, _controller_col_target);
+ovsdb_idl_add_column(ovs_idl_loop.idl,
+ _controller_col_is_connected);
 chassis_register_ovs_idl(ovs_idl_loop.idl);
 encaps_register_ovs_idl(ovs_idl_loop.idl);
 binding_register_ovs_idl(ovs_idl_loop.idl);
--
1.9.1

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

[ovs-dev] [PATCH 2/4] ovn: Passive connection handlers for the controller for br-int

2015-10-23 Thread Babu Shanmugam


This patch handles and maintains the passive connection for br-int

Signed-off-by: Babu Shanmugam <bscha...@redhat.com>
---
 ovn/controller/automake.mk  |   2 +
 ovn/controller/ofcontroller.c   | 231 


 ovn/controller/ofcontroller.h   |  33 ++
 ovn/controller/ovn-controller.c |   4 +
 4 files changed, 270 insertions(+)
 create mode 100644 ovn/controller/ofcontroller.c
 create mode 100644 ovn/controller/ofcontroller.h

diff --git a/ovn/controller/automake.mk b/ovn/controller/automake.mk
index fec9bf1..9702b11 100644
--- a/ovn/controller/automake.mk
+++ b/ovn/controller/automake.mk
@@ -10,6 +10,8 @@ ovn_controller_ovn_controller_SOURCES = \
 ovn/controller/lflow.h \
 ovn/controller/ofctrl.c \
 ovn/controller/ofctrl.h \
+ovn/controller/ofcontroller.c \
+ovn/controller/ofcontroller.h \
 ovn/controller/patch.c \
 ovn/controller/patch.h \
 ovn/controller/ovn-controller.c \
diff --git a/ovn/controller/ofcontroller.c b/ovn/controller/ofcontroller.c
new file mode 100644
index 000..8a6c124
--- /dev/null
+++ b/ovn/controller/ofcontroller.c
@@ -0,0 +1,231 @@
+
+/* Copyright (c) 2015 Red Hat, 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.
+ */
+
+#include 
+#include "dp-packet.h"
+#include "lflow.h"
+#include "ofctrl.h"
+#include "ofp-msgs.h"
+#include "ofp-util.h"
+#include "ofp-actions.h"
+#include "ofp-version-opt.h"
+#include "openflow/openflow.h"
+#include "openvswitch/vconn.h"
+#include "openvswitch/vlog.h"
+#include "ovn-controller.h"
+#include "physical.h"
+#include "rconn.h"
+#include "socket-util.h"
+#include "vswitch-idl.h"
+#include "ofcontroller.h"
+
+VLOG_DEFINE_THIS_MODULE(ofcontroller);
+
+/* Passive connection for the controller */
+struct pvconn *pvconn;
+
+/* Remote connection from the switch */
+struct rconn *rconn = NULL;
+
+void
+ofcontroller_init(char const *sock_path)
+{
+char *proto = xasprintf("punix:%s", sock_path);
+
+pvconn_open(proto, 0, 0, );
+free(proto);
+}
+
+static void
+process_packet_in(struct controller_ctx *ctx OVS_UNUSED,
+  struct ofp_header *msg)
+{
+struct ofputil_packet_in pin;
+
+if (ofputil_decode_packet_in(, msg) != 0) {
+return;
+}
+if (pin.reason != OFPR_ACTION) {
+return;
+}
+
+/* XXX : process the received packet */
+}
+
+static void
+process_packet(struct controller_ctx *ctx, struct ofpbuf *msg)
+{
+enum ofptype type;
+struct ofpbuf b;
+
+b = *msg;
+if (ofptype_pull(, )) {
+return;
+}
+switch (type) {
+case OFPTYPE_HELLO:
+{
+uint32_t allowed_versions;
+
+ofputil_decode_hello(msg->data, _versions);
+/* XXX: Negotiate */
+break;
+}
+case OFPTYPE_ECHO_REQUEST:
+{
+struct ofpbuf *r = make_echo_reply(msg->data);
+
+rconn_send(rconn, r, NULL);
+break;
+}
+case OFPTYPE_FEATURES_REPLY:
+/* XXX: Finish this */
+break;
+case OFPTYPE_PACKET_IN:
+process_packet_in(ctx, msg->data);
+break;
+case OFPTYPE_FLOW_REMOVED:
+case OFPTYPE_ERROR:
+case OFPTYPE_ECHO_REPLY:
+case OFPTYPE_FEATURES_REQUEST:
+case OFPTYPE_GET_CONFIG_REQUEST:
+case OFPTYPE_GET_CONFIG_REPLY:
+case OFPTYPE_SET_CONFIG:
+case OFPTYPE_PORT_STATUS:
+case OFPTYPE_PACKET_OUT:
+case OFPTYPE_FLOW_MOD:
+case OFPTYPE_GROUP_MOD:
+case OFPTYPE_PORT_MOD:
+case OFPTYPE_TABLE_MOD:
+case OFPTYPE_BARRIER_REQUEST:
+case OFPTYPE_BARRIER_REPLY:
+case OFPTYPE_QUEUE_GET_CONFIG_REQUEST:
+case OFPTYPE_QUEUE_GET_CONFIG_REPLY:
+case OFPTYPE_DESC_STATS_REQUEST:
+case OFPTYPE_DESC_STATS_REPLY:
+case OFPTYPE_FLOW_STATS_REQUEST:
+case OFPTYPE_FLOW_STATS_REPLY:
+case OFPTYPE_AGGREGATE_STATS_REQUEST:
+case OFPTYPE_AGGREGATE_STATS_REPLY:
+case OFPTYPE_TABLE_STATS_REQUEST:
+case OFPTYPE_TABLE_STATS_REPLY:
+case OFPTYPE_PORT_STATS_REQUEST:
+case OFPTYPE_PORT_STATS_REPLY:
+case OFPTYPE_QUEUE_STATS_REQUEST:
+case OFPTYPE_QUEUE_STATS_REPLY:
+case OFPTYPE_PORT_DESC_STATS_REQUEST:
+case OFPTYPE_PORT_DESC_STATS_REPLY:
+case OFPTYPE_ROLE_REQUEST:
+case O

[ovs-dev] DHCP responder in OVN

2015-10-13 Thread Babu Shanmugam


Hi,
Me and Numan are working to bring up the DHCP responder in OVN. We 
explored towards setting up flow entries to respond to DHCP discover and 
DHCP request messages. We end up learning that Openflow does not have 
provision to do that.
Looks like, the only way to do that is through packet-in. From our 
understanding the packet-in listener is not present in OVN controller. 
Please correct us if our understanding is wrong. If so, is anyone 
already working on the packet-in listener?


Thank you,
Babu
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

78 matches

Mail list logo