Re: [ovs-dev] [RFC] dpdk: support multiple queues in vhost

[Flavio Leitner]

On Fri, Aug 07, 2015 at 08:43:23AM -0400, Thomas F Herbert wrote:
 On 8/6/15 5:29 PM, Flavio Leitner wrote:
 On Thu, Aug 06, 2015 at 03:24:29PM -0400, Thomas F Herbert wrote:
 On 8/6/15 1:40 PM, Flavio Leitner wrote:
 On Thu, Aug 06, 2015 at 12:39:29PM -0400, Thomas F Herbert wrote:
 On 7/31/15 6:30 PM, Flavio Leitner wrote:
 This RFC is based on the vhost multiple queues work on
 dpdk-dev: http://dpdk.org/ml/archives/dev/2015-June/019345.html
 
 Signed-off-by: Flavio Leitner f...@redhat.com
 ---
   lib/netdev-dpdk.c | 61 
  ---
   1 file changed, 40 insertions(+), 21 deletions(-)
 
 diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
 index 5ae805e..493172c 100644
 --- a/lib/netdev-dpdk.c
 +++ b/lib/netdev-dpdk.c
 @@ -215,12 +215,9 @@ struct netdev_dpdk {
* If the numbers match, 'txq_needs_locking' is false, otherwise 
  it is
* true and we will take a spinlock on transmission */
   int real_n_txq;
 +int real_n_rxq;
   bool txq_needs_locking;
 
 -/* Spinlock for vhost transmission.  Other DPDK devices use 
 spinlocks in
 - * dpdk_tx_queue */
 -rte_spinlock_t vhost_tx_lock;
 -
   /* virtio-net structure for vhost device */
   OVSRCU_TYPE(struct virtio_net *) virtio_dev;
 
 @@ -602,13 +599,10 @@ dpdk_dev_parse_name(const char dev_name[], const 
 char prefix[],
   static int
   vhost_construct_helper(struct netdev *netdev_) OVS_REQUIRES(dpdk_mutex)
   {
 -struct netdev_dpdk *netdev = netdev_dpdk_cast(netdev_);
 -
   if (rte_eal_init_ret) {
   return rte_eal_init_ret;
   }
 
 -rte_spinlock_init(netdev-vhost_tx_lock);
   return netdev_dpdk_init(netdev_, -1, DPDK_DEV_VHOST);
   }
 
 @@ -791,9 +785,16 @@ netdev_dpdk_vhost_set_multiq(struct netdev 
 *netdev_, unsigned int n_txq,
   ovs_mutex_lock(dpdk_mutex);
   ovs_mutex_lock(netdev-mutex);
 
 +rte_free(netdev-tx_q);
 +/* FIXME: the number of vqueues needs to match */
 Do you still need this FIXME? Isn't the code you added below freeing and
 re-allocating the correct number of tx queues?
 
 Yes, because that is about virtual queues provided by qemu.
 Thanks,
 fbl
 I understand this is an RFC but I think your patch is in the right
 direction. I know the merging is complex and requires upstream changes to
 DPDK and Qemu. I ack this patch is an important step that moves the ball
 forward toward vhost user performance of DPDK accelerated OVS.

Thanks for your review
fbl

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

[ovs-dev] With our goods you'll increase drive tenfold!

[Elsa]

Bluep|lule is chaper than you can imagine. 




Feel the love energy! Buy at our shop. 

On our site all the necessary for you pharmaceuticals! 




 B

 E

 A

 C

 

upon, and there was that luxurious after-dinner atmosphere when


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

Re: [ovs-dev] [ovn-controller-vtep V5 09/12] ovn-controller-vtep: Add binding module.

[Russell Bryant]

On 08/07/2015 03:46 AM, Alex Wang wrote:
 This commit adds the binding module to ovn-controller-vtep.  The
 module will scan through the Port_Binding table in ovnsb.  If there is
 a port binding entry for a logical switch on the vtep gateway chassis's
 vtep_logical_switches, sets the port binding's chassis column to the
 vtep gateway chassis.
 
 Signed-off-by: Alex Wang al...@nicira.com

I'm getting a test failure on this one:

  2: ovn-controller-vtep - test binding 1FAILED
(ovn-controller-vtep.at:223)


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

[ovs-dev] [PATCH] ovn-controller: Fix flows between two local ports.

[Justin Pettit]

A flow was missing from the remote output table that causes local
packets to be resubmitted to the local ouptut table.

Reported-by: Russell Bryant rbry...@redhat.com
Signed-off-by: Justin Pettit jpet...@nicira.com
---
 ovn/controller/physical.c |   12 +++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index ff6cf57..2ec0ba9 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -483,13 +483,23 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id 
mff_ovn_geneve,
 ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 100, match, ofpacts);
 }
 
+/* Table 32, Priority 0.
+ * ===
+ *
+ * Resubmit packets that are not directed at tunnels or part of a
+ * multicast group to the local output table. */
+struct match match;
+match_init_catchall(match);
+ofpbuf_clear(ofpacts);
+put_resubmit(OFTABLE_LOCAL_OUTPUT, ofpacts);
+ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 0, match, ofpacts);
+
 /* Table 34, Priority 0.
  * ===
  *
  * Resubmit packets that don't output to the ingress port (already checked
  * in table 33) to the logical egress pipeline, clearing the logical
  * registers (for consistent behavior with packets that get tunneled). */
-struct match match;
 match_init_catchall(match);
 ofpbuf_clear(ofpacts);
 #define MFF_LOG_REG(ID) put_load(0, ID, 0, 32, ofpacts);
-- 
1.7.5.4

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

Re: [ovs-dev] [PATCH] ovn: Fix flows between 2 local ports.

[Justin Pettit]

Thanks for tracking down the issue and supplying a patch, Russell.  However, I 
think the core problem was that there was a missing flow.  Specifically, the 
one described in the last sentence from this part of the ovn-architecture man 
page:

Each  flow  in  table  32 matches on a logical output port for
unicast or multicast logical ports that include a logical port
on a remote hypervisor.  Each flow’s actions implement sending
a packet to the port it matches.  For unicast  logical  output
ports on remote hypervisors, the actions set the tunnel key to
the correct value, then send the packet on the tunnel port  to
the  correct hypervisor.  (When the remote hypervisor receives
the packet, table 0 there will  recognize  it  as  a  tunneled
packet  and pass it along to table 33.)  For multicast logical
output ports, the actions send one copy of the packet to  each
remote  hypervisor,  in  the  same way as for unicast destina‐
tions.  If a multicast group includes a logical port or  ports
on the local hypervisor, then its actions also resubmit to ta‐
ble 33.  Table 32 also includes a fallback flow that resubmits
to table 33 if there is no other match.

I've sent out a patch, which I think addresses the issue.  Would you be able to 
review the patch and make sure it solves the problem you were seeing?

I think we should work on some of the naming and descriptions of these flows, 
since it wasn't obvious to me that there should be a fallback flow for local 
traffic in the remote flow table!  I imagine this will improve over time.

Thanks!

--Justin


 On Aug 6, 2015, at 12:23 PM, Russell Bryant rbry...@redhat.com wrote:
 
 Packets were always resubmmited to the remote output table, which
 handles output to a remote chassis.  Broadcast packets were only sent
 remotely and not to local ports on the same logical switch.  Unicast
 packets between 2 local ports also didn't work.
 
 This patch makes it so packets are sent to both remote and local
 output tables.  It would be a bit cleaner to only submit to the
 necessary table(s), but always resubmitting to both and letting the
 matching work it out seems to be fine and gets things working again.
 
 Signed-off-by: Russell Bryant rbry...@redhat.com
 ---
 ovn/controller/lflow.c | 14 ++
 ovn/lib/actions.c  | 20 +---
 ovn/lib/actions.h  |  9 +
 tests/test-ovn.c   |  5 +++--
 4 files changed, 31 insertions(+), 17 deletions(-)
 
 diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
 index 9246e61..efa0253 100644
 --- a/ovn/controller/lflow.c
 +++ b/ovn/controller/lflow.c
 @@ -271,9 +271,14 @@ lflow_run(struct controller_ctx *ctx, struct hmap 
 *flow_table)
 : 
 OFTABLE_LOG_EGRESS_PIPELINE);
 uint8_t next_phys_table
 = lflow-table_id + 1  LOG_PIPELINE_LEN ? phys_table + 1 : 0;
 -uint8_t output_phys_table = (ingress
 - ? OFTABLE_REMOTE_OUTPUT
 - : OFTABLE_LOG_TO_PHY);
 +uint8_t oftable_output[] = { OFTABLE_LOCAL_OUTPUT, 
 OFTABLE_REMOTE_OUTPUT };
 +uint8_t oftable_log_to_phy[] = { OFTABLE_LOG_TO_PHY };
 +uint8_t *output_phys_table = (ingress
 + ? oftable_output
 + : oftable_log_to_phy);
 +size_t n_output_tables = (ingress
 + ? ARRAY_SIZE(oftable_output)
 + : ARRAY_SIZE(oftable_log_to_phy));
 /* Translate OVN actions into OpenFlow actions.
  *
  * XXX Deny changes to 'outport' in egress pipeline. */
 @@ -284,7 +289,8 @@ lflow_run(struct controller_ctx *ctx, struct hmap 
 *flow_table)
 
 ofpbuf_use_stub(ofpacts, ofpacts_stub, sizeof ofpacts_stub);
 error = actions_parse_string(lflow-actions, symtab, ldp-ports,
 - next_phys_table, output_phys_table,
 + next_phys_table,
 + output_phys_table, n_output_tables,
  ofpacts, prereqs);
 if (error) {
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
 diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
 index 0a0158a..51b04ae 100644
 --- a/ovn/lib/actions.c
 +++ b/ovn/lib/actions.c
 @@ -31,7 +31,8 @@ struct action_context {
 struct lexer *lexer;/* Lexer for pulling more tokens. */
 const struct shash *symtab; /* Symbol table. */
 uint8_t next_table_id;  /* OpenFlow table for 'next' to resubmit. */
 -uint8_t output_table_id;/* OpenFlow table for 'output' to resubmit. 
 */
 +uint8_t 

Re: [ovs-dev] [ovn-controller-vtep V5 09/12] ovn-controller-vtep: Add binding module.

[Russell Bryant]

On 08/07/2015 03:46 AM, Alex Wang wrote:
 This commit adds the binding module to ovn-controller-vtep.  The
 module will scan through the Port_Binding table in ovnsb.  If there is
 a port binding entry for a logical switch on the vtep gateway chassis's
 vtep_logical_switches, sets the port binding's chassis column to the
 vtep gateway chassis.
 
 Signed-off-by: Alex Wang al...@nicira.com

I wanted to send the quick message about the test failures, but here are
a few inline comments and a question I wanted to ask ...

 
 ---
 V4-V5:
 - rebase on top of master.
 - change to use port binding type, and options when finding bindings
   for vtep gateway chassis.
 
 V3-V4:
 - rebase to master.
 
 V2-V3:
 - since ovn-sb schema changes (removal of Gateway table), the binding
   module code needs to be adapted.
 
 PATCH-V2:
 - split into separate commit.
 - disallow and warn if more than one logical port from one 'vlan_map'
   are attached to the same logical datapath.
 ---
  ovn/controller-vtep/automake.mk   |2 +
  ovn/controller-vtep/binding.c |  247 
 +
  ovn/controller-vtep/binding.h |   27 
  ovn/controller-vtep/ovn-controller-vtep.c |4 +
  tests/ovn-controller-vtep.at  |  112 +
  5 files changed, 392 insertions(+)
  create mode 100644 ovn/controller-vtep/binding.c
  create mode 100644 ovn/controller-vtep/binding.h
 
 diff --git a/ovn/controller-vtep/automake.mk b/ovn/controller-vtep/automake.mk
 index 514cafa..33f063f 100644
 --- a/ovn/controller-vtep/automake.mk
 +++ b/ovn/controller-vtep/automake.mk
 @@ -1,5 +1,7 @@
  bin_PROGRAMS += ovn/controller-vtep/ovn-controller-vtep
  ovn_controller_vtep_ovn_controller_vtep_SOURCES = \
 + ovn/controller-vtep/binding.c \
 + ovn/controller-vtep/binding.h \
   ovn/controller-vtep/gateway.c \
   ovn/controller-vtep/gateway.h \
   ovn/controller-vtep/ovn-controller-vtep.c \
 diff --git a/ovn/controller-vtep/binding.c b/ovn/controller-vtep/binding.c
 new file mode 100644
 index 000..fc12006
 --- /dev/null
 +++ b/ovn/controller-vtep/binding.c
 @@ -0,0 +1,247 @@
 +/* Copyright (c) 2015 Nicira, Inc.
 + *
 + * Licensed under the Apache License, Version 2.0 (the License);
 + * you may not use this file except in compliance with the License.
 + * You may obtain a copy of the License at:
 + *
 + * http://www.apache.org/licenses/LICENSE-2.0
 + *
 + * Unless required by applicable law or agreed to in writing, software
 + * distributed under the License is distributed on an AS IS BASIS,
 + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 + * See the License for the specific language governing permissions and
 + * limitations under the License.
 + */
 +
 +#include config.h
 +#include binding.h
 +
 +#include lib/shash.h
 +#include lib/smap.h
 +#include lib/util.h
 +#include openvswitch/vlog.h
 +#include ovn-controller-vtep.h
 +#include ovn/lib/ovn-sb-idl.h
 +#include vtep/vtep-idl.h
 +
 +VLOG_DEFINE_THIS_MODULE(binding);
 +
 +/*
 + * This module scans through the Port_Binding table in ovnsb.  If there is a
 + * logical port binding entry for logical switch in vtep gateway chassis's
 + * 'vtep_logical_switches' column, sets the binding's chassis column to the
 + * corresponding vtep gateway chassis.
 + *
 + */
 +
 +
 +/* Returns true if the vtep logical switch specified by 'port_binding_rec'
 + * has already been bound to another port binding entry, and resets
 + * 'port_binding_rec''s chassis column.  Otherwise, updates 'ls_to_pb'
 + * and returns false. */
 +static bool
 +check_pb_conflict(struct shash *ls_to_pb,
 +  const struct sbrec_port_binding *port_binding_rec,
 +  const struct sbrec_chassis *chassis_rec)
 +{
 +const char *vtep_lswitch =
 +smap_get(port_binding_rec-options, vtep-logical-switch);
 +const struct sbrec_port_binding *pb_conflict =
 +shash_find_data(ls_to_pb, vtep_lswitch);
 +
 +if (pb_conflict) {
 +VLOG_WARN(logical switch (%s), on vtep gateway chassis 
 +  (%s) has already been associated with logical 
 +  port (%s), ignore logical port (%s),
 +  vtep_lswitch, chassis_rec-name,
 +  pb_conflict-logical_port,
 +  port_binding_rec-logical_port);
 +sbrec_port_binding_set_chassis(port_binding_rec, NULL);
 +
 +return true;
 +}
 +
 +shash_replace(ls_to_pb, vtep_lswitch, port_binding_rec);
 +return false;
 +}
 +
 +/* Returns true if the vtep logical switch specified by 'port_binding_rec'
 + * has already been bound to a different datapath, and resets
 + * 'port_binding_rec''s chassis column.  Otherwise, updates 'ls_to_db' and
 + * returns false. */
 +static bool
 +check_db_conflict(struct shash *ls_to_db,
 +  const struct sbrec_port_binding *port_binding_rec,
 +  const struct sbrec_chassis *chassis_rec)
 +{
 +const 

Re: [ovs-dev] [PATCH] ovn: Fix flows between 2 local ports.

[Russell Bryant]

On 08/07/2015 06:11 PM, Justin Pettit wrote:
 Thanks for tracking down the issue and supplying a patch, Russell.
 However, I think the core problem was that there was a missing flow.
 Specifically, the one described in the last sentence from this part
 of the ovn-architecture man page:
 
 Each  flow  in  table  32 matches on a logical output port for
 unicast or multicast logical ports that include a logical port
 on a remote hypervisor.  Each flow’s actions implement sending
 a packet to the port it matches.  For unicast  logical  output
 ports on remote hypervisors, the actions set the tunnel key to
 the correct value, then send the packet on the tunnel port  to
 the  correct hypervisor.  (When the remote hypervisor receives
 the packet, table 0 there will  recognize  it  as  a  tunneled
 packet  and pass it along to table 33.)  For multicast logical
 output ports, the actions send one copy of the packet to  each
 remote  hypervisor,  in  the  same way as for unicast destina‐
 tions.  If a multicast group includes a logical port or  ports
 on the local hypervisor, then its actions also resubmit to ta‐
 ble 33.  Table 32 also includes a fallback flow that resubmits
 to table 33 if there is no other match.
 
 I've sent out a patch, which I think addresses the issue.  Would you
 be able to review the patch and make sure it solves the problem you
 were seeing?
 
 I think we should work on some of the naming and descriptions of
 these flows, since it wasn't obvious to me that there should be a
 fallback flow for local traffic in the remote flow table!  I imagine
 this will improve over time.

Thanks a lot for digging into this and pointing out the missing flow!
That makes sense to me.  I'll give your patch a try now and let you know
how it goes.

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

Re: [ovs-dev] [PATCH] ovn-controller: Fix flows between two local ports.

[Russell Bryant]

On 08/07/2015 06:05 PM, Justin Pettit wrote:
 A flow was missing from the remote output table that causes local
 packets to be resubmitted to the local ouptut table.
 
 Reported-by: Russell Bryant rbry...@redhat.com
 Signed-off-by: Justin Pettit jpet...@nicira.com

Works for me.  Thanks again for finding the root cause!

Acked-by: Russell Bryant rbry...@redhat.com

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

Re: [ovs-dev] [PATCH v2] dpif: allow adding ukeys for same flow by different pmds

[Alex Wang]

Thx for reporting this, I worry this may not be an ovs issue, but to do with
the dpdk driver...  Here is my analysis, please correct me if wrong,

When pmd threads are spawned, each of them will take ownership of port
queues in a round robin fashion...  In other words, each port queue will
be read by only one pmd thread.

Then based on my understanding the dpdk module will hash packets to one
particular queue on the receiving port.  Thusly packets from the same flow
(or stream) should only be handled by one pmd thread.

Next, the issue you reported will only happen when the packets from same
flow (or stream) are handled by multiple pmd threads.  [In that case, only
one
pmd thread will have the flow installed in its classifier (and exact match
cache)...  and all other pmd threads fail to install the flow due to
duplicate
ufid]

So, based on the analysis, this really should not happen, and if happens,
indicate a packet queueing issue in dpdk.

Ilya, could you reproduce this issue and provide the `ovs-appctl
coverage/show` output?  Want to confirm that the 'handler_duplicate_upcall'
counter is increasing when this issue happens.

Daniele, could you also provide thoughts on this?  Since I'm not sure if
the
ovs dpdk code changes (since i last touched it long time ago) affect the
issue
here.


Thanks,
Alex Wang,




On Fri, Aug 7, 2015 at 2:35 AM, Ilya Maximets i.maxim...@samsung.com
wrote:

 In multiqueue mode several pmd threads may process one
 port, but different queues. Flow doesn't depend on queue.

 So, while miss upcall processing, all threads (except first
 for that port) will receive error = ENOSPC due to
 ukey_install failure. Therefore they will not add the flow
 to flow_table and will not insert it to exact match cache.

 As a result all threads (except first for that port) will
 always execute a miss.

 Fix that by mixing pmd_id with the bits from the ufid
 for ukey-hash calculation.

 Signed-off-by: Ilya Maximets i.maxim...@samsung.com
 ---
  ofproto/ofproto-dpif-upcall.c | 22 --
  1 file changed, 12 insertions(+), 10 deletions(-)

 diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
 index 0f2e186..57a7f34 100644
 --- a/ofproto/ofproto-dpif-upcall.c
 +++ b/ofproto/ofproto-dpif-upcall.c
 @@ -291,7 +291,8 @@ static bool ukey_install_start(struct udpif *, struct
 udpif_key *ukey);
  static bool ukey_install_finish(struct udpif_key *ukey, int error);
  static bool ukey_install(struct udpif *udpif, struct udpif_key *ukey);
  static struct udpif_key *ukey_lookup(struct udpif *udpif,
 - const ovs_u128 *ufid);
 + const ovs_u128 *ufid,
 + const unsigned pmd_id);
  static int ukey_acquire(struct udpif *, const struct dpif_flow *,
  struct udpif_key **result, int *error);
  static void ukey_delete__(struct udpif_key *);
 @@ -1143,7 +1144,8 @@ process_upcall(struct udpif *udpif, struct upcall
 *upcall,
  }
  if (actions_len == 0) {
  /* Lookup actions in userspace cache. */
 -struct udpif_key *ukey = ukey_lookup(udpif, upcall-ufid);
 +struct udpif_key *ukey = ukey_lookup(udpif, upcall-ufid,
 + upcall-pmd_id);
  if (ukey) {
  actions = ukey-actions-data;
  actions_len = ukey-actions-size;
 @@ -1320,19 +1322,19 @@ handle_upcalls(struct udpif *udpif, struct upcall
 *upcalls,
  }

  static uint32_t
 -get_ufid_hash(const ovs_u128 *ufid)
 +get_ukey_hash(const ovs_u128 *ufid, const unsigned pmd_id)
  {
 -return ufid-u32[0];
 +return ufid-u32[0] + pmd_id;
  }

  static struct udpif_key *
 -ukey_lookup(struct udpif *udpif, const ovs_u128 *ufid)
 +ukey_lookup(struct udpif *udpif, const ovs_u128 *ufid, const unsigned
 pmd_id)
  {
  struct udpif_key *ukey;
 -int idx = get_ufid_hash(ufid) % N_UMAPS;
 +int idx = get_ukey_hash(ufid, pmd_id) % N_UMAPS;
  struct cmap *cmap = udpif-ukeys[idx].cmap;

 -CMAP_FOR_EACH_WITH_HASH (ukey, cmap_node, get_ufid_hash(ufid), cmap) {
 +CMAP_FOR_EACH_WITH_HASH (ukey, cmap_node, get_ukey_hash(ufid,
 pmd_id), cmap) {
  if (ovs_u128_equals(ukey-ufid, ufid)) {
  return ukey;
  }
 @@ -1362,7 +1364,7 @@ ukey_create__(const struct nlattr *key, size_t
 key_len,
  ukey-ufid_present = ufid_present;
  ukey-ufid = *ufid;
  ukey-pmd_id = pmd_id;
 -ukey-hash = get_ufid_hash(ukey-ufid);
 +ukey-hash = get_ukey_hash(ukey-ufid, pmd_id);
  ukey-actions = ofpbuf_clone(actions);

  ovs_mutex_init(ukey-mutex);
 @@ -1490,7 +1492,7 @@ ukey_install_start(struct udpif *udpif, struct
 udpif_key *new_ukey)
  idx = new_ukey-hash % N_UMAPS;
  umap = udpif-ukeys[idx];
  ovs_mutex_lock(umap-mutex);
 -old_ukey = ukey_lookup(udpif, new_ukey-ufid);
 +old_ukey = 

Re: [ovs-dev] [ovn-controller-vtep V5 01/12] ovn-sb: Remove the Gateway table from the ovn-sb schema.

[Alex Wang]

Thx so much for the quick review, Russell !!!

Please see my comments inline,

On Fri, Aug 7, 2015 at 10:13 AM, Russell Bryant rbry...@redhat.com wrote:

 On 08/07/2015 03:46 AM, Alex Wang wrote:
  In a gateway like the VTEP L2 gateway, physical vlans belonging to
  the same logical network form a logical switch.  Each logical switch
  has a dedicated tunnel key and will keep records of all MACs learned
  from the owned vlans.  So user can just send packet to a logical
  switch and the gateway will figure out the output port and vlan tag
  automatically.
 
  Therefore, it is really not necessary to keep record of the vlan map
  for each gateway physical port in the OVN_Southbound database using
  gateway_ports and to map each vlan to a unique ovn logical port.
  Instead, we should simply map each logical switch to a ovn logical
  port.
 
  Thusly, this commit removes the Gateway table from the OVN_Southbound
  database.  In the Chassis table, the gateway_ports column is replaced
  by vtep_logical_switches column which stores all vtep logical switch
  names.  Then, in the OVN_Northbound database, the CMS must specify the
  physical switch name and the logical switch name via
  options:vtep_physical_switch and options:vtep_logical_switch when
  creating logical port with type vtep.
 
  Signed-off-by: Alex Wang al...@nicira.com

 The change makes sense to me, but I had a couple of doc comments.

  ---
  V4-V5:
  - rebase on top of master.
  - change vtep_logical_switches to a set of strings storing all logical
switch names.
  - require user to specify vtep_physical_switch and
 vtep_logical_switch
in options.
 
  V3-V4:
  - change the column name to vtep_logical_switches.
  - adopt Ben's refinement of ovn-sb schema manual.
 
  -V3:
  - Realize that the Gateway table is not needed.
  ---
   ovn/ovn-nb.xml   |   31 +
   ovn/ovn-sb.ovsschema |   16 ++-
   ovn/ovn-sb.xml   |   75
 --
   3 files changed, 70 insertions(+), 52 deletions(-)
 
  diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
  index ade8164..77b559d 100644
  --- a/ovn/ovn-nb.xml
  +++ b/ovn/ovn-nb.xml
  @@ -111,18 +111,37 @@
   column name=type
 p
 Specify a type for this logical port.  Logical ports can be used
 to model
  -  other types of connectivity into an OVN logical switch.  Leaving
 this column
  -  blank maintains the default logical port behavior.
  +  other types of connectivity into an OVN logical switch.  Leaving
 this
  +  column blank maintains the default logical port behavior, which is
  +  for a VM (or VIF) interface.  Besides, the following types are
  +  defined:
 /p
 
  -  p
  -  There are no other logical port types implemented yet.
  -  /p
  +  dl
  +dtcodevtep/code/dt
  +ddA port to a logical switch on a VTEP gateway.  In order
  +to get this port correctly recognized by the ovn controller, the
  +ref column=options
 table=Logical_Port/:vtep_physical_switch
  +and ref column=options
 table=Logical_Port/:vtep_logical_switch
  +must also be defined./dd
  +  /dl
   /column
 
   column name=options
  +  p
   This column provides key/value settings specific to the logical
 port
  -ref column=type/.
  +ref column=type/.  The following options are defined:
  +  /p
  +
  +  dl
  +dtcodevtep_physical_switch/code/dt
  +ddThe ref column=name table=Chassis/ of the VTEP
 gateway./dd

 I'm not sure if this ref makes sense since it's a table in another
 schema.


Thx, will remove it.




  +  /dl
  +
  +  dl
  +dtcodevtep_logical_switch/code/dt
  +ddA logical switch name connected by the VTEP gateway./dd
  +  /dl
   /column

 All of these docs look good, but do you think it makes sense to add
 these docs later in the series once all of the code is in place to
 support the 'vtep' port type?



Okay, I'm writing up a description for ovn-controller-vtep in
ovn-architecture~
Maybe I should post the series after that, but I'm learning to use a new
keyboard, so, typing slow~

I'd like to fold this ovn-sb/ovn-nb.xml change in that patch~ what do you
think?


Thanks,
Alex Wang,


 --
 Russell Bryant

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

Re: [ovs-dev] [PATCH v2 0/4] Vagrant userspace testsuite

[Joe Stringer]

On 7 August 2015 at 11:40, Daniele Di Proietto diproiet...@vmware.com wrote:
 This series adds a new testsuite for the userspace datapath that runs
 the already written kmod-sanity tests.  The reason for this are
 explained in the 3rd commit message.

 The testsuite is called `system userspace-testsuite` and can be launched
 with `make check-system-userspace`.

 v1 - v2:
 * Dropped already applied commit.
 * Renamed 'userspace-testsuite' to 'system-userspace-testsuite',
   'kmod-testsuite' to 'system-kmod-testsuite' and
   'make check-userspace' to 'make check-system-userspace'.
 * Added new Vagrant provision target for running the system userspace
   testsuite.
 * Added ON_EXIT to remove port in ADD_VETH.
 * Introduced NS_EXEC and NS_CHECK_EXEC to abstract the namespace
   commands.
 * Added mention in NEWS.

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

Re: [ovs-dev] [ovn-controller-vtep V5 01/12] ovn-sb: Remove the Gateway table from the ovn-sb schema.

[Russell Bryant]

On 08/07/2015 06:43 PM, Alex Wang wrote:
 Thx so much for the quick review, Russell !!!
 
 Please see my comments inline,
 
 On Fri, Aug 7, 2015 at 10:13 AM, Russell Bryant rbry...@redhat.com
 mailto:rbry...@redhat.com wrote:
 
 On 08/07/2015 03:46 AM, Alex Wang wrote:
  In a gateway like the VTEP L2 gateway, physical vlans belonging to
  the same logical network form a logical switch.  Each logical switch
  has a dedicated tunnel key and will keep records of all MACs learned
  from the owned vlans.  So user can just send packet to a logical
  switch and the gateway will figure out the output port and vlan tag
  automatically.
 
  Therefore, it is really not necessary to keep record of the vlan map
  for each gateway physical port in the OVN_Southbound database using
  gateway_ports and to map each vlan to a unique ovn logical port.
  Instead, we should simply map each logical switch to a ovn logical
  port.
 
  Thusly, this commit removes the Gateway table from the OVN_Southbound
  database.  In the Chassis table, the gateway_ports column is 
 replaced
  by vtep_logical_switches column which stores all vtep logical switch
  names.  Then, in the OVN_Northbound database, the CMS must specify the
  physical switch name and the logical switch name via
  options:vtep_physical_switch and options:vtep_logical_switch when
  creating logical port with type vtep.
 
  Signed-off-by: Alex Wang al...@nicira.com mailto:al...@nicira.com
 
 The change makes sense to me, but I had a couple of doc comments.
 
  ---
  V4-V5:
  - rebase on top of master.
  - change vtep_logical_switches to a set of strings storing all
 logical
switch names.
  - require user to specify vtep_physical_switch and
 vtep_logical_switch
in options.
 
  V3-V4:
  - change the column name to vtep_logical_switches.
  - adopt Ben's refinement of ovn-sb schema manual.
 
  -V3:
  - Realize that the Gateway table is not needed.
  ---
   ovn/ovn-nb.xml   |   31 +
   ovn/ovn-sb.ovsschema |   16 ++-
   ovn/ovn-sb.xml   |   75
 --
   3 files changed, 70 insertions(+), 52 deletions(-)
 
  diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
  index ade8164..77b559d 100644
  --- a/ovn/ovn-nb.xml
  +++ b/ovn/ovn-nb.xml
  @@ -111,18 +111,37 @@
   column name=type
 p
 Specify a type for this logical port.  Logical ports can be
 used to model
  -  other types of connectivity into an OVN logical switch. 
 Leaving this column
  -  blank maintains the default logical port behavior.
  +  other types of connectivity into an OVN logical switch. 
 Leaving this
  +  column blank maintains the default logical port behavior,
 which is
  +  for a VM (or VIF) interface.  Besides, the following types are
  +  defined:
 /p
 
  -  p
  -  There are no other logical port types implemented yet.
  -  /p
  +  dl
  +dtcodevtep/code/dt
  +ddA port to a logical switch on a VTEP gateway.  In order
  +to get this port correctly recognized by the ovn
 controller, the
  +ref column=options
 table=Logical_Port/:vtep_physical_switch
  +and ref column=options
 table=Logical_Port/:vtep_logical_switch
  +must also be defined./dd
  +  /dl
   /column
 
   column name=options
  +  p
   This column provides key/value settings specific to the
 logical port
  -ref column=type/.
  +ref column=type/.  The following options are defined:
  +  /p
  +
  +  dl
  +dtcodevtep_physical_switch/code/dt
  +ddThe ref column=name table=Chassis/ of the VTEP
 gateway./dd
 
 I'm not sure if this ref makes sense since it's a table in another
 schema.
 
 
 Thx, will remove it.
 
 
  
 
  +  /dl
  +
  +  dl
  +dtcodevtep_logical_switch/code/dt
  +ddA logical switch name connected by the VTEP gateway./dd
  +  /dl
   /column
 
 All of these docs look good, but do you think it makes sense to add
 these docs later in the series once all of the code is in place to
 support the 'vtep' port type?
 
 
 
 Okay, I'm writing up a description for ovn-controller-vtep in
 ovn-architecture~
 Maybe I should post the series after that, but I'm learning to use a new 
 keyboard, so, typing slow~
 
 I'd like to fold this ovn-sb/ovn-nb.xml change in that patch~ what do you 
 think?

Sure, that sounds good to me.

-- 
Russell Bryant
___
dev mailing list
dev@openvswitch.org

Re: [ovs-dev] [PATCH] ovn-controller: Fix flows between two local ports.

[Justin Pettit]

 On Aug 7, 2015, at 3:40 PM, Russell Bryant rbry...@redhat.com wrote:
 
 On 08/07/2015 06:05 PM, Justin Pettit wrote:
 A flow was missing from the remote output table that causes local
 packets to be resubmitted to the local ouptut table.
 
 Reported-by: Russell Bryant rbry...@redhat.com
 Signed-off-by: Justin Pettit jpet...@nicira.com
 
 Works for me.  Thanks again for finding the root cause!
 
 Acked-by: Russell Bryant rbry...@redhat.com

Thanks for tracking down the issue and reviewing this patch.  I pushed it.

--Justin


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

[ovs-dev] [ovn-controller-vtep V5 03/12] ovn-sbctl: Add ovn-sbctl.

[Alex Wang]

This commit adds ovn-sbctl to ovn family by using the db-ctl-base
library.

Signed-off-by: Alex Wang al...@nicira.com
Acked-by: Ben Pfaff b...@nicira.com

---
V4-V5:
- rebase on top of master.
- change ovn-sbctl command format to target-action (e.g., chassis-add).

V3-V4:
- mention ovn-sbctl should never be used in normal operation.

V2-V3:
- rebase to master.
- adopt Russell's review comments.
- move ovn-nbctl/sbctl related files into ovn/utilities.

PATCH-V2:
- change command add/del-ch to add/del-chassis.
- refine the manual based on comments from Ben.
---
 manpages.mk  |   12 +
 ovn/utilities/.gitignore |3 +-
 ovn/utilities/automake.mk|   13 +-
 ovn/utilities/ovn-sbctl.8.in |  160 
 ovn/utilities/ovn-sbctl.c|  870 ++
 tests/automake.mk|5 +-
 tests/ovn-sbctl.at   |   61 +++
 tests/testsuite.at   |1 +
 8 files changed, 1120 insertions(+), 5 deletions(-)
 create mode 100644 ovn/utilities/ovn-sbctl.8.in
 create mode 100644 ovn/utilities/ovn-sbctl.c
 create mode 100644 tests/ovn-sbctl.at

diff --git a/manpages.mk b/manpages.mk
index 3cec260..6e2853b 100644
--- a/manpages.mk
+++ b/manpages.mk
@@ -1,5 +1,17 @@
 # Generated automatically -- do not modify!-*- buffer-read-only: t -*-
 
+ovn/utilities/ovn-sbctl.8: \
+   ovn/utilities/ovn-sbctl.8.in \
+   lib/db-ctl-base.man \
+   lib/table.man \
+   ovsdb/remote-active.man \
+   ovsdb/remote-passive.man
+ovn/utilities/ovn-sbctl.8.in:
+lib/db-ctl-base.man:
+lib/table.man:
+ovsdb/remote-active.man:
+ovsdb/remote-passive.man:
+
 ovsdb/ovsdb-client.1: \
ovsdb/ovsdb-client.1.in \
lib/common-syn.man \
diff --git a/ovn/utilities/.gitignore b/ovn/utilities/.gitignore
index 714aaab..5832b6c 100644
--- a/ovn/utilities/.gitignore
+++ b/ovn/utilities/.gitignore
@@ -1,4 +1,5 @@
 /ovn-ctl.8
 /ovn-nbctl
 /ovn-nbctl.8
-
+/ovn-sbctl
+/ovn-sbctl.8
diff --git a/ovn/utilities/automake.mk b/ovn/utilities/automake.mk
index 145ee44..b247a54 100644
--- a/ovn/utilities/automake.mk
+++ b/ovn/utilities/automake.mk
@@ -3,7 +3,10 @@ scripts_SCRIPTS += \
 
 man_MANS += \
 ovn/utilities/ovn-ctl.8 \
-ovn/utilities/ovn-nbctl.8
+ovn/utilities/ovn-nbctl.8 \
+ovn/utilities/ovn-sbctl.8
+
+MAN_ROOTS += ovn/utilities/ovn-sbctl.8.in
 
 EXTRA_DIST += \
 ovn/utilities/ovn-ctl \
@@ -12,9 +15,15 @@ EXTRA_DIST += \
 
 DISTCLEANFILES += \
 ovn/utilities/ovn-ctl.8 \
-ovn/utilities/ovn-nbctl.8
+ovn/utilities/ovn-nbctl.8 \
+ovn/utilities/ovn-sbctl.8
 
 # ovn-nbctl
 bin_PROGRAMS += ovn/utilities/ovn-nbctl
 ovn_utilities_ovn_nbctl_SOURCES = ovn/utilities/ovn-nbctl.c
 ovn_utilities_ovn_nbctl_LDADD = ovn/lib/libovn.la ovsdb/libovsdb.la 
lib/libopenvswitch.la
+
+# ovn-sbctl
+bin_PROGRAMS += ovn/utilities/ovn-sbctl
+ovn_utilities_ovn_sbctl_SOURCES = ovn/utilities/ovn-sbctl.c
+ovn_utilities_ovn_sbctl_LDADD = ovn/lib/libovn.la ovsdb/libovsdb.la 
lib/libopenvswitch.la
diff --git a/ovn/utilities/ovn-sbctl.8.in b/ovn/utilities/ovn-sbctl.8.in
new file mode 100644
index 000..b5c796e
--- /dev/null
+++ b/ovn/utilities/ovn-sbctl.8.in
@@ -0,0 +1,160 @@
+.\ -*- nroff -*-
+.de IQ
+.  br
+.  ns
+.  IP \\$1
+..
+.de ST
+.  PP
+.  RS -0.15in
+.  I \\$1
+.  RE
+..
+.TH ovn\-sbctl 8 @VERSION@ Open vSwitch Open vSwitch Manual
+.\ This program's name:
+.ds PN ovn\-sbctl
+.
+.SH NAME
+ovn\-sbctl \- utility for querying and configuring \fBOVN_Southbound\fR 
database
+.
+.SH SYNOPSIS
+\fBovn\-sbctl\fR [\fIoptions\fR] \fB\-\-\fR [\fIoptions\fR] \fIcommand
+\fR[\fIargs\fR] [\fB\-\-\fR [\fIoptions\fR] \fIcommand \fR[\fIargs\fR]]...
+.
+.SH DESCRIPTION
+The command should only be used for advanced debugging and troubleshooting
+of the \fBOVN_Southbound\fR database; and should never be used in normal
+operation.
+.PP
+The \fBovn\-sbctl\fR program configures the \fBOVN_Southbound\fR database
+by providing a high\-level interface to its configuration database.  See
+\fBovn\-sb\fR(5) for comprehensive documentation of the database schema.
+.PP
+\fBovn\-sbctl\fR connects to an \fBovsdb\-server\fR process that
+maintains an OVN_Southbound configuration database.  Using this
+connection, it queries and possibly applies changes to the database,
+depending on the supplied commands.
+.PP
+\fBovn\-sbctl\fR can perform any number of commands in a single run,
+implemented as a single atomic transaction against the database.
+.PP
+The \fBovn\-sbctl\fR command line begins with global options (see
+\fBOPTIONS\fR below for details).  The global options are followed by
+one or more commands.  Each command should begin with \fB\-\-\fR by
+itself as a command-line argument, to separate it from the following
+commands.  (The \fB\-\-\fR before the first command is optional.)  The
+command
+itself starts with command-specific options, if any, followed by the
+command name and any arguments.
+.
+.SH OPTIONS
+.
+The following options affect the behavior of \fBovn\-sbctl\fR 

[ovs-dev] [ovn-controller-vtep V5 12/12] ovn: Update TODOs related to vtep controller.

[Alex Wang]

Signed-off-by: Alex Wang al...@nicira.com
---
V5: new patch.
---
 ovn/TODO |   21 +
 1 file changed, 21 insertions(+)

diff --git a/ovn/TODO b/ovn/TODO
index 356b3ba..71c307c 100644
--- a/ovn/TODO
+++ b/ovn/TODO
@@ -80,3 +80,24 @@
So far, both ovn-controller and ovn-controller-vtep only allow
chassis to have one tunnel encapsulation entry.  We should extend
the implementation to support multiple tunnel encapsulations.
+
+** Update learned MAC addresses from VTEP to OVN
+
+   The VTEP gateway stores all MAC addresses learned from its
+   physical interfaces in the 'Ucast_Macs_Local' and the
+   'Mcast_Macs_Local' tables.  ovn-controller-vtep should be
+   able to update those information back to ovn-sb database,
+   so that other chassis knows where to send those packets to
+   instead of broadcasting.
+
+** Translate ovn-sb Multicast_Group table into VTEP config
+
+   The ovn-controller-vtep daemon should be able to translate
+   the Multicast_Group table entry in ovn-sb database into
+   Mcast_Macs_Remote table configuration in VTEP database.
+
+* Use BFD as tunnel monitor.
+
+   Both ovn-controller and ovn-contorller-vtep can use BFD to
+   monitor the tunnel liveness.  Both ovs-vswitchd schema and
+   VTEP schema supports BFD.
\ No newline at end of file
-- 
1.7.9.5

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

[ovs-dev] [ovn-controller-vtep V5 07/12] ovsdb-idl: Move get_initial_snapshot() to ovsdb-idl.

[Alex Wang]

The same function is defined in both ovn-controller.c and
ovn-controller-vtep.c, so worth librarizing.

Signed-off-by: Alex Wang al...@nicira.com
---
V5:
- new patch.
---
 lib/ovsdb-idl.c   |   15 +++
 lib/ovsdb-idl.h   |1 +
 ovn/controller-vtep/ovn-controller-vtep.c |   17 ++---
 ovn/controller/ovn-controller.c   |   17 ++---
 4 files changed, 20 insertions(+), 30 deletions(-)

diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index f0d5d9c..00b900d 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -2496,6 +2496,21 @@ ovsdb_idl_txn_get_idl (struct ovsdb_idl_txn *txn)
 {
 return txn-idl;
 }
+
+/* Blocks until 'idl' successfully connects to the remote database and
+ * retrieves its contents. */
+void
+ovsdb_idl_get_initial_snapshot(struct ovsdb_idl *idl)
+{
+while (1) {
+ovsdb_idl_run(idl);
+if (ovsdb_idl_has_ever_connected(idl)) {
+return;
+}
+ovsdb_idl_wait(idl);
+poll_block();
+}
+}
 
 /* If 'lock_name' is nonnull, configures 'idl' to obtain the named lock from
  * the database server and to avoid modifying the database when the lock cannot
diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h
index 28aa787..003ff60 100644
--- a/lib/ovsdb-idl.h
+++ b/lib/ovsdb-idl.h
@@ -222,6 +222,7 @@ const struct ovsdb_idl_row *ovsdb_idl_txn_insert(
 const struct uuid *);
 
 struct ovsdb_idl *ovsdb_idl_txn_get_idl (struct ovsdb_idl_txn *);
+void ovsdb_idl_get_initial_snapshot(struct ovsdb_idl *);
 
 
 /* ovsdb_idl_loop provides an easy way to manage the transactions related
diff --git a/ovn/controller-vtep/ovn-controller-vtep.c 
b/ovn/controller-vtep/ovn-controller-vtep.c
index e8bfcbd..2f9dc10 100644
--- a/ovn/controller-vtep/ovn-controller-vtep.c
+++ b/ovn/controller-vtep/ovn-controller-vtep.c
@@ -49,19 +49,6 @@ OVS_NO_RETURN static void usage(void);
 static char *vtep_remote;
 static char *ovnsb_remote;
 
-static void
-get_initial_snapshot(struct ovsdb_idl *idl)
-{
-while (1) {
-ovsdb_idl_run(idl);
-if (ovsdb_idl_has_ever_connected(idl)) {
-return;
-}
-ovsdb_idl_wait(idl);
-poll_block();
-}
-}
-
 int
 main(int argc, char *argv[])
 {
@@ -94,12 +81,12 @@ main(int argc, char *argv[])
 ovsdb_idl_add_table(vtep_idl_loop.idl, vteprec_table_global);
 ovsdb_idl_add_column(vtep_idl_loop.idl,
  vteprec_global_col_switches);
-get_initial_snapshot(vtep_idl_loop.idl);
+ovsdb_idl_get_initial_snapshot(vtep_idl_loop.idl);
 
 /* Connect to OVN SB database. */
 struct ovsdb_idl_loop ovnsb_idl_loop = OVSDB_IDL_LOOP_INITIALIZER(
 ovsdb_idl_create(ovnsb_remote, sbrec_idl_class, false, true));
-get_initial_snapshot(ovnsb_idl_loop.idl);
+ovsdb_idl_get_initial_snapshot(ovnsb_idl_loop.idl);
 
 /* Main loop. */
 exiting = false;
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index bb2b840..8e93a0f 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -57,19 +57,6 @@ OVS_NO_RETURN static void usage(void);
 
 static char *ovs_remote;
 
-static void
-get_initial_snapshot(struct ovsdb_idl *idl)
-{
-while (1) {
-ovsdb_idl_run(idl);
-if (ovsdb_idl_has_ever_connected(idl)) {
-return;
-}
-ovsdb_idl_wait(idl);
-poll_block();
-}
-}
-
 static const struct ovsrec_bridge *
 get_br_int(struct ovsdb_idl *ovs_idl)
 {
@@ -167,13 +154,13 @@ main(int argc, char *argv[])
 encaps_register_ovs_idl(ovs_idl_loop.idl);
 binding_register_ovs_idl(ovs_idl_loop.idl);
 physical_register_ovs_idl(ovs_idl_loop.idl);
-get_initial_snapshot(ovs_idl_loop.idl);
+ovsdb_idl_get_initial_snapshot(ovs_idl_loop.idl);
 
 /* Connect to OVN SB database. */
 char *ovnsb_remote = get_ovnsb_remote(ovs_idl_loop.idl);
 struct ovsdb_idl_loop ovnsb_idl_loop = OVSDB_IDL_LOOP_INITIALIZER(
 ovsdb_idl_create(ovnsb_remote, sbrec_idl_class, true, true));
-get_initial_snapshot(ovnsb_idl_loop.idl);
+ovsdb_idl_get_initial_snapshot(ovnsb_idl_loop.idl);
 
 /* Main loop. */
 exiting = false;
-- 
1.7.9.5

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

[ovs-dev] [ovn-controller-vtep V5 11/12] ovn-controller-vtep: Extend vtep module to install Ucast_Macs_Remote.

[Alex Wang]

This commit extends the vtep module to support creating the
'Ucast_Macs_Remote' table entries in the vtep database for
MAC addresses on the ovn logical ports.

Signed-off-by: Alex Wang al...@nicira.com
---
V4-V5:
- rebase on top of master.
- rewrite the feature since a lot have changed.

V3-V4:
- add logic to remove Ucast_Macs_Remote for non-existent MACs.

V2-V3:
- rebase to master.

PATCH-V2:
- split into separate commit.
- few optimizations.
---
 ovn/controller-vtep/vtep.c   |  324 --
 tests/ovn-controller-vtep.at |  136 ++
 2 files changed, 415 insertions(+), 45 deletions(-)

diff --git a/ovn/controller-vtep/vtep.c b/ovn/controller-vtep/vtep.c
index 55d2e0d..bdbc0bc 100644
--- a/ovn/controller-vtep/vtep.c
+++ b/ovn/controller-vtep/vtep.c
@@ -19,7 +19,8 @@
 
 #include lib/hash.h
 #include lib/hmap.h
-#include lib/smap.h
+#include lib/shash.h
+#include lib/sset.h
 #include lib/util.h
 #include ovn-controller-vtep.h
 #include openvswitch/vlog.h
@@ -34,64 +35,233 @@ VLOG_DEFINE_THIS_MODULE(vtep);
  *
  */
 
+/* Searches the 'chassis_rec-encaps' for the first vtep tunnel
+ * configuration, returns the 'ip'. */
+static const char *
+get_chassis_vtep_ip(const struct sbrec_chassis *chassis_rec)
+{
+if (chassis_rec) {
+size_t i;
+
+for (i = 0; i  chassis_rec-n_encaps; i++) {
+if (!strcmp(chassis_rec-encaps[i]-type, vxlan)) {
+return chassis_rec-encaps[i]-ip;
+}
+}
+}
+
+return NULL;
+}
+
+/* Creates a new 'Ucast_Macs_Remote'. */
+static struct vteprec_ucast_macs_remote *
+create_umr(struct ovsdb_idl_txn *vtep_idl_txn, const char *mac,
+   const struct vteprec_logical_switch *vtep_ls)
+{
+struct vteprec_ucast_macs_remote *new_umr;
+
+new_umr = vteprec_ucast_macs_remote_insert(vtep_idl_txn);
+vteprec_ucast_macs_remote_set_MAC(new_umr, mac);
+vteprec_ucast_macs_remote_set_logical_switch(new_umr, vtep_ls);
+
+return new_umr;
+}
+
+/* Creates a new 'Physical_Locator'. */
+static struct vteprec_physical_locator *
+create_pl(struct ovsdb_idl_txn *vtep_idl_txn, const char *chassis_ip)
+{
+struct vteprec_physical_locator *new_pl;
+
+new_pl = vteprec_physical_locator_insert(vtep_idl_txn);
+vteprec_physical_locator_set_dst_ip(new_pl, chassis_ip);
+vteprec_physical_locator_set_encapsulation_type(new_pl, VTEP_ENCAP_TYPE);
+
+return new_pl;
+}
+
+
 /* Updates the vtep Logical_Switch table entries' tunnel keys based
  * on the port bindings. */
 static void
-vtep_lswitch_run(struct controller_vtep_ctx *ctx)
+vtep_lswitch_run(struct shash *vtep_pbs, struct shash *vtep_lswitches)
 {
-struct shash vtep_lswitches = SHASH_INITIALIZER(vtep_lswitches);
-const struct sbrec_port_binding *port_binding_rec;
-const struct vteprec_logical_switch *vtep_ls;
-
-/* Stores all logical switches to 'vtep_lswitches' with name as key. */
-VTEPREC_LOGICAL_SWITCH_FOR_EACH (vtep_ls, ctx-vtep_idl) {
-shash_add(vtep_lswitches, vtep_ls-name, vtep_ls);
-}
+struct sset used_ls = SSET_INITIALIZER(used_ls);
+struct shash_node *node;
 
-ovsdb_idl_txn_add_comment(ctx-vtep_idl_txn,
-  ovn-controller-vtep: update logical switch 
-  tunnel keys);
 /* Collects the logical switch bindings from port binding entries.
  * Since the binding module has already guaranteed that each vtep
  * logical switch is bound only to one ovn-sb logical datapath,
  * we can just iterate and assign tunnel key to vtep logical switch. */
-SBREC_PORT_BINDING_FOR_EACH(port_binding_rec, ctx-ovnsb_idl) {
-if (!strcmp(port_binding_rec-type, vtep)
- port_binding_rec-chassis) {
-const char *lswitch_name = smap_get(port_binding_rec-options,
-vtep-logical-switch);
-
-/* If 'port_binding_rec-chassis' exists then 'lswitch_name'
- * must also exist. */
-ovs_assert(lswitch_name);
-vtep_ls = shash_find_and_delete(vtep_lswitches, lswitch_name);
-if (!vtep_ls) {
-VLOG_ERR(logical port (%s) bound to non-exist vtep logical 
- switch (%s), something is seriously wrong,
- port_binding_rec-logical_port, lswitch_name);
-} else {
-int64_t tnl_key;
-
-tnl_key = port_binding_rec-datapath-tunnel_key;
-if (vtep_ls-n_tunnel_key
- vtep_ls-tunnel_key[0] != tnl_key) {
-VLOG_DBG(set vtep logical switch (%s) tunnel key from 
- (%PRId64) to (%PRId64), vtep_ls-name,
- vtep_ls-tunnel_key[0], tnl_key);
-}
+SHASH_FOR_EACH (node, vtep_pbs) {
+const struct sbrec_port_binding *port_binding_rec = node-data;
+const char *lswitch_name = 

[ovs-dev] [ovn-controller-vtep V5 10/12] ovn-controller-vtep: Add vtep module.

[Alex Wang]

This commit adds the vtep module to ovn-controller-vtep.  The
module will scan through the Port_Binding table in OVN-SB database,
and update the vtep logcial switches tunnel keys.

Signed-off-by: Alex Wang al...@nicira.com
---
V5: new patch.
---
 ovn/controller-vtep/automake.mk   |4 +-
 ovn/controller-vtep/ovn-controller-vtep.c |3 +
 ovn/controller-vtep/vtep.c|  134 +
 ovn/controller-vtep/vtep.h|   27 ++
 tests/ovn-controller-vtep.at  |   32 +++
 5 files changed, 199 insertions(+), 1 deletion(-)
 create mode 100644 ovn/controller-vtep/vtep.c
 create mode 100644 ovn/controller-vtep/vtep.h

diff --git a/ovn/controller-vtep/automake.mk b/ovn/controller-vtep/automake.mk
index 33f063f..cacfae6 100644
--- a/ovn/controller-vtep/automake.mk
+++ b/ovn/controller-vtep/automake.mk
@@ -5,7 +5,9 @@ ovn_controller_vtep_ovn_controller_vtep_SOURCES = \
ovn/controller-vtep/gateway.c \
ovn/controller-vtep/gateway.h \
ovn/controller-vtep/ovn-controller-vtep.c \
-   ovn/controller-vtep/ovn-controller-vtep.h
+   ovn/controller-vtep/ovn-controller-vtep.h \
+   ovn/controller-vtep/vtep.c \
+   ovn/controller-vtep/vtep.h
 ovn_controller_vtep_ovn_controller_vtep_LDADD = ovn/lib/libovn.la 
lib/libopenvswitch.la vtep/libvtep.la
 man_MANS += ovn/controller-vtep/ovn-controller-vtep.8
 EXTRA_DIST += ovn/controller-vtep/ovn-controller-vtep.8.xml
diff --git a/ovn/controller-vtep/ovn-controller-vtep.c 
b/ovn/controller-vtep/ovn-controller-vtep.c
index 525cda6..3780f80 100644
--- a/ovn/controller-vtep/ovn-controller-vtep.c
+++ b/ovn/controller-vtep/ovn-controller-vtep.c
@@ -39,6 +39,7 @@
 
 #include binding.h
 #include gateway.h
+#include vtep.h
 #include ovn-controller-vtep.h
 
 VLOG_DEFINE_THIS_MODULE(ovn_vtep);
@@ -99,6 +100,7 @@ main(int argc, char *argv[])
 
 gateway_run(ctx);
 binding_run(ctx);
+vtep_run(ctx);
 unixctl_server_run(unixctl);
 
 unixctl_server_wait(unixctl);
@@ -124,6 +126,7 @@ main(int argc, char *argv[])
  * We're done if all of them return true. */
 done = gateway_cleanup(ctx);
 done = binding_cleanup(ctx)  done;
+done = vtep_cleanup(ctx)  done;
 if (done) {
 poll_immediate_wake();
 }
diff --git a/ovn/controller-vtep/vtep.c b/ovn/controller-vtep/vtep.c
new file mode 100644
index 000..55d2e0d
--- /dev/null
+++ b/ovn/controller-vtep/vtep.c
@@ -0,0 +1,134 @@
+/* Copyright (c) 2015 Nicira, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the License);
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an AS IS BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include config.h
+
+#include vtep.h
+
+#include lib/hash.h
+#include lib/hmap.h
+#include lib/smap.h
+#include lib/util.h
+#include ovn-controller-vtep.h
+#include openvswitch/vlog.h
+#include ovn/lib/ovn-sb-idl.h
+#include vtep/vtep-idl.h
+
+VLOG_DEFINE_THIS_MODULE(vtep);
+
+/*
+ * Scans through the Binding table in ovnsb and updates the vtep logical
+ * switch tunnel keys.
+ *
+ */
+
+/* Updates the vtep Logical_Switch table entries' tunnel keys based
+ * on the port bindings. */
+static void
+vtep_lswitch_run(struct controller_vtep_ctx *ctx)
+{
+struct shash vtep_lswitches = SHASH_INITIALIZER(vtep_lswitches);
+const struct sbrec_port_binding *port_binding_rec;
+const struct vteprec_logical_switch *vtep_ls;
+
+/* Stores all logical switches to 'vtep_lswitches' with name as key. */
+VTEPREC_LOGICAL_SWITCH_FOR_EACH (vtep_ls, ctx-vtep_idl) {
+shash_add(vtep_lswitches, vtep_ls-name, vtep_ls);
+}
+
+ovsdb_idl_txn_add_comment(ctx-vtep_idl_txn,
+  ovn-controller-vtep: update logical switch 
+  tunnel keys);
+/* Collects the logical switch bindings from port binding entries.
+ * Since the binding module has already guaranteed that each vtep
+ * logical switch is bound only to one ovn-sb logical datapath,
+ * we can just iterate and assign tunnel key to vtep logical switch. */
+SBREC_PORT_BINDING_FOR_EACH(port_binding_rec, ctx-ovnsb_idl) {
+if (!strcmp(port_binding_rec-type, vtep)
+ port_binding_rec-chassis) {
+const char *lswitch_name = smap_get(port_binding_rec-options,
+vtep-logical-switch);
+
+/* If 'port_binding_rec-chassis' exists then 'lswitch_name'
+ * must also exist. */
+ovs_assert(lswitch_name);
+ 

[ovs-dev] [ovn-controller-vtep V5 06/12] ovn: Add controller for VTEP gateway.

[Alex Wang]

This commit lays down the foundation for a new controller in OVN, the
ovn-controller-vtep, for controlling the vtep enabled gateways.

Signed-off-by: Alex Wang al...@nicira.com

---
V4-V5:
- rebase on top of master.
- use ovsdb_idl_loop to avoid block commit.

V3-V4:
- rebase to master.

V2-V3:
- rebase to master.

PATCH-V2:
- split into separate commit.
- add manpage for ovn-controller-vtep.
---
 ovn/automake.mk   |1 +
 ovn/controller-vtep/.gitignore|2 +
 ovn/controller-vtep/automake.mk   |8 +
 ovn/controller-vtep/ovn-controller-vtep.8.xml |   70 +++
 ovn/controller-vtep/ovn-controller-vtep.c |  245 +
 ovn/controller-vtep/ovn-controller-vtep.h |   31 
 tests/automake.mk |6 +-
 tests/ovn-controller-vtep.at  |1 +
 tests/testsuite.at|1 +
 9 files changed, 362 insertions(+), 3 deletions(-)
 create mode 100644 ovn/controller-vtep/.gitignore
 create mode 100644 ovn/controller-vtep/automake.mk
 create mode 100644 ovn/controller-vtep/ovn-controller-vtep.8.xml
 create mode 100644 ovn/controller-vtep/ovn-controller-vtep.c
 create mode 100644 ovn/controller-vtep/ovn-controller-vtep.h
 create mode 100644 tests/ovn-controller-vtep.at

diff --git a/ovn/automake.mk b/ovn/automake.mk
index 901e710..33bbd05 100644
--- a/ovn/automake.mk
+++ b/ovn/automake.mk
@@ -76,6 +76,7 @@ EXTRA_DIST += \
ovn/OVN-GW-HA.md
 
 include ovn/controller/automake.mk
+include ovn/controller-vtep/automake.mk
 include ovn/lib/automake.mk
 include ovn/northd/automake.mk
 include ovn/utilities/automake.mk
diff --git a/ovn/controller-vtep/.gitignore b/ovn/controller-vtep/.gitignore
new file mode 100644
index 000..3ec8072
--- /dev/null
+++ b/ovn/controller-vtep/.gitignore
@@ -0,0 +1,2 @@
+/ovn-controller-vtep
+/ovn-controller-vtep.8
diff --git a/ovn/controller-vtep/automake.mk b/ovn/controller-vtep/automake.mk
new file mode 100644
index 000..7adda15
--- /dev/null
+++ b/ovn/controller-vtep/automake.mk
@@ -0,0 +1,8 @@
+bin_PROGRAMS += ovn/controller-vtep/ovn-controller-vtep
+ovn_controller_vtep_ovn_controller_vtep_SOURCES = \
+   ovn/controller-vtep/ovn-controller-vtep.c \
+   ovn/controller-vtep/ovn-controller-vtep.h
+ovn_controller_vtep_ovn_controller_vtep_LDADD = ovn/lib/libovn.la 
lib/libopenvswitch.la vtep/libvtep.la
+man_MANS += ovn/controller-vtep/ovn-controller-vtep.8
+EXTRA_DIST += ovn/controller-vtep/ovn-controller-vtep.8.xml
+DISTCLEANFILES += ovn/controller-vtep/ovn-controller-vtep.8
diff --git a/ovn/controller-vtep/ovn-controller-vtep.8.xml 
b/ovn/controller-vtep/ovn-controller-vtep.8.xml
new file mode 100644
index 000..c924f9f
--- /dev/null
+++ b/ovn/controller-vtep/ovn-controller-vtep.8.xml
@@ -0,0 +1,70 @@
+?xml version=1.0 encoding=utf-8?
+manpage program=ovn-controller-vtep section=8 title=ovn-controller-vtep
+h1Name/h1
+povn-controller-vtep -- Open Virtual Network local controller for
+   vtep enabled physical switches.
+/p
+
+h1Synopsis/h1
+pcodeovn-controller-vtep/code [varoptions/var]
+[var--vtep-db=vtep-database/var] [var--ovnsb-db=ovnsb-database/var]
+/p
+
+h1Description/h1
+p
+  codeovn-controller-vtep/code is the local controller daemon in
+  OVN, the Open Virtual Network, for VTEP enabled physical switches.
+  It connects up to the OVN Southbound database (see
+  codeovn-sb/code(5)) over the OVSDB protocol, and down to the VTEP
+  database (see codevtep/code(5)) over the OVSDB protocol.
+/p
+
+h1Configuration/h1
+p
+  codeovn-controller-vtep/code retrieves its configuration
+  information from both the ovnsb and the vtep database.  If the
+  database locations are not given from command line, the default
+  is the codedb.sock/code in local OVSDB's 'run' directory.
+  The datapath location must take one of the following forms:
+/p
+ul
+  li
+p
+  codessl:varip/var:varport/var/code
+/p
+p
+  The specified SSL varport/var on the host at the given
+  varip/var, which must be expressed as an IP address (not a DNS
+  name) in IPv4 or IPv6 address format.  If varip/var is an IPv6
+  address, then wrap varip/var with square brackets, e.g.:
+  codessl:[::1]:6640/code.  The code--private-key/code,
+  code--certificate/code, and code--ca-cert/code options are
+  mandatory when this form is used.
+/p
+  /li
+  li
+p
+  codetcp:varip/var:varport/var/code
+/p
+p
+  Connect to the given TCP varport/var on varip/var, where
+  varip/var can be IPv4 or IPv6 address. If varip/var is an
+  IPv6 address, then wrap varip/var with square brackets, e.g.:
+  codetcp:[::1]:6640/code.
+/p
+  /li
+  li
+p
+  codeunix:varfile/var/code
+

[ovs-dev] [ovn-controller-vtep V5 08/12] ovn-controller-vtep: Add gateway module.

[Alex Wang]

This commit adds the gateway module to ovn-controller-vtep.  The
module will register the physical switches to ovnsb as chassis and
constantly update the vtep_logical_switches column in Chassis table.

Limitation (Recorded in TODO file):

- Do not support reading multiple tunnel ips of physical switch.

Signed-off-by: Alex Wang al...@nicira.com

---
V4-V5:
- rebase on top of master.
- adopt Russell's review suggestions, greatly simplify the code.

V3-V4:
- rebase to master.

V2-V3:
- since ovn-sb schema changes (removal of Gateway table), the gateway
  module code needs to be adapted.
- rebase to master.

PATCH-V2:
- split into separate commit.
- can register all physical switches controlled by vtep database.
---
 ovn/TODO   |6 +
 ovn/controller-vtep/automake.mk|2 +
 ovn/controller-vtep/gateway.c  |  224 
 .../{ovn-controller-vtep.h = gateway.h}   |   19 +-
 ovn/controller-vtep/ovn-controller-vtep.c  |   43 +++-
 ovn/controller-vtep/ovn-controller-vtep.h  |   20 ++
 tests/ovn-controller-vtep.at   |  151 +
 7 files changed, 445 insertions(+), 20 deletions(-)
 create mode 100644 ovn/controller-vtep/gateway.c
 copy ovn/controller-vtep/{ovn-controller-vtep.h = gateway.h} (65%)

diff --git a/ovn/TODO b/ovn/TODO
index 509e5d0..356b3ba 100644
--- a/ovn/TODO
+++ b/ovn/TODO
@@ -74,3 +74,9 @@
Epstein et al., What's the Difference? Efficient Set
Reconciliation Without Prior Context.  (I'm not yet aware of
previous non-academic use of this technique.)
+
+** Support multiple tunnel encapsulations in Chassis.
+
+   So far, both ovn-controller and ovn-controller-vtep only allow
+   chassis to have one tunnel encapsulation entry.  We should extend
+   the implementation to support multiple tunnel encapsulations.
diff --git a/ovn/controller-vtep/automake.mk b/ovn/controller-vtep/automake.mk
index 7adda15..514cafa 100644
--- a/ovn/controller-vtep/automake.mk
+++ b/ovn/controller-vtep/automake.mk
@@ -1,5 +1,7 @@
 bin_PROGRAMS += ovn/controller-vtep/ovn-controller-vtep
 ovn_controller_vtep_ovn_controller_vtep_SOURCES = \
+   ovn/controller-vtep/gateway.c \
+   ovn/controller-vtep/gateway.h \
ovn/controller-vtep/ovn-controller-vtep.c \
ovn/controller-vtep/ovn-controller-vtep.h
 ovn_controller_vtep_ovn_controller_vtep_LDADD = ovn/lib/libovn.la 
lib/libopenvswitch.la vtep/libvtep.la
diff --git a/ovn/controller-vtep/gateway.c b/ovn/controller-vtep/gateway.c
new file mode 100644
index 000..5f6da27
--- /dev/null
+++ b/ovn/controller-vtep/gateway.c
@@ -0,0 +1,224 @@
+/* Copyright (c) 2015 Nicira, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the License);
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an AS IS BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include config.h
+#include gateway.h
+
+#include lib/poll-loop.h
+#include lib/simap.h
+#include lib/sset.h
+#include lib/util.h
+#include openvswitch/vlog.h
+#include ovn/lib/ovn-sb-idl.h
+#include vtep/vtep-idl.h
+#include ovn-controller-vtep.h
+
+VLOG_DEFINE_THIS_MODULE(gateway);
+
+/*
+ * Registers the physical switches in vtep to ovnsb as chassis.  For each
+ * physical switch in the vtep database, finds all vtep logical switches that
+ * are associated with the physical switch, and updates the corresponding
+ * chassis's 'vtep_logical_switches' column.
+ *
+ */
+
+/* Global revalidation sequence number, incremented at each call to
+ * 'revalidate_gateway()'. */
+static unsigned int gw_reval_seq;
+
+/* Maps all chassis created by the gateway module to their own reval_seq. */
+static struct simap gw_chassis_map = SIMAP_INITIALIZER(gw_chassis_map);
+
+/* Creates and returns a new instance of 'struct sbrec_chassis'. */
+static const struct sbrec_chassis *
+create_chassis_rec(struct ovsdb_idl_txn *txn, const char *name,
+   const char *encap_ip)
+{
+const struct sbrec_chassis *chassis_rec;
+struct sbrec_encap *encap_rec;
+
+chassis_rec = sbrec_chassis_insert(txn);
+sbrec_chassis_set_name(chassis_rec, name);
+encap_rec = sbrec_encap_insert(txn);
+sbrec_encap_set_type(encap_rec, OVN_SB_ENCAP_TYPE);
+sbrec_encap_set_ip(encap_rec, encap_ip);
+sbrec_chassis_set_encaps(chassis_rec, encap_rec, 1);
+
+return chassis_rec;
+}
+
+/* Revalidates chassis in ovnsb against vtep database.  Creates chassis for
+ * new vtep physical switch.  And removes chassis which no longer have
+ * physical switch in vtep.
+ *
+ * xxx: Support 

[ovs-dev] [ovn-controller-vtep V5 09/12] ovn-controller-vtep: Add binding module.

[Alex Wang]

This commit adds the binding module to ovn-controller-vtep.  The
module will scan through the Port_Binding table in ovnsb.  If there is
a port binding entry for a logical switch on the vtep gateway chassis's
vtep_logical_switches, sets the port binding's chassis column to the
vtep gateway chassis.

Signed-off-by: Alex Wang al...@nicira.com

---
V4-V5:
- rebase on top of master.
- change to use port binding type, and options when finding bindings
  for vtep gateway chassis.

V3-V4:
- rebase to master.

V2-V3:
- since ovn-sb schema changes (removal of Gateway table), the binding
  module code needs to be adapted.

PATCH-V2:
- split into separate commit.
- disallow and warn if more than one logical port from one 'vlan_map'
  are attached to the same logical datapath.
---
 ovn/controller-vtep/automake.mk   |2 +
 ovn/controller-vtep/binding.c |  247 +
 ovn/controller-vtep/binding.h |   27 
 ovn/controller-vtep/ovn-controller-vtep.c |4 +
 tests/ovn-controller-vtep.at  |  112 +
 5 files changed, 392 insertions(+)
 create mode 100644 ovn/controller-vtep/binding.c
 create mode 100644 ovn/controller-vtep/binding.h

diff --git a/ovn/controller-vtep/automake.mk b/ovn/controller-vtep/automake.mk
index 514cafa..33f063f 100644
--- a/ovn/controller-vtep/automake.mk
+++ b/ovn/controller-vtep/automake.mk
@@ -1,5 +1,7 @@
 bin_PROGRAMS += ovn/controller-vtep/ovn-controller-vtep
 ovn_controller_vtep_ovn_controller_vtep_SOURCES = \
+   ovn/controller-vtep/binding.c \
+   ovn/controller-vtep/binding.h \
ovn/controller-vtep/gateway.c \
ovn/controller-vtep/gateway.h \
ovn/controller-vtep/ovn-controller-vtep.c \
diff --git a/ovn/controller-vtep/binding.c b/ovn/controller-vtep/binding.c
new file mode 100644
index 000..fc12006
--- /dev/null
+++ b/ovn/controller-vtep/binding.c
@@ -0,0 +1,247 @@
+/* Copyright (c) 2015 Nicira, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the License);
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an AS IS BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include config.h
+#include binding.h
+
+#include lib/shash.h
+#include lib/smap.h
+#include lib/util.h
+#include openvswitch/vlog.h
+#include ovn-controller-vtep.h
+#include ovn/lib/ovn-sb-idl.h
+#include vtep/vtep-idl.h
+
+VLOG_DEFINE_THIS_MODULE(binding);
+
+/*
+ * This module scans through the Port_Binding table in ovnsb.  If there is a
+ * logical port binding entry for logical switch in vtep gateway chassis's
+ * 'vtep_logical_switches' column, sets the binding's chassis column to the
+ * corresponding vtep gateway chassis.
+ *
+ */
+
+
+/* Returns true if the vtep logical switch specified by 'port_binding_rec'
+ * has already been bound to another port binding entry, and resets
+ * 'port_binding_rec''s chassis column.  Otherwise, updates 'ls_to_pb'
+ * and returns false. */
+static bool
+check_pb_conflict(struct shash *ls_to_pb,
+  const struct sbrec_port_binding *port_binding_rec,
+  const struct sbrec_chassis *chassis_rec)
+{
+const char *vtep_lswitch =
+smap_get(port_binding_rec-options, vtep-logical-switch);
+const struct sbrec_port_binding *pb_conflict =
+shash_find_data(ls_to_pb, vtep_lswitch);
+
+if (pb_conflict) {
+VLOG_WARN(logical switch (%s), on vtep gateway chassis 
+  (%s) has already been associated with logical 
+  port (%s), ignore logical port (%s),
+  vtep_lswitch, chassis_rec-name,
+  pb_conflict-logical_port,
+  port_binding_rec-logical_port);
+sbrec_port_binding_set_chassis(port_binding_rec, NULL);
+
+return true;
+}
+
+shash_replace(ls_to_pb, vtep_lswitch, port_binding_rec);
+return false;
+}
+
+/* Returns true if the vtep logical switch specified by 'port_binding_rec'
+ * has already been bound to a different datapath, and resets
+ * 'port_binding_rec''s chassis column.  Otherwise, updates 'ls_to_db' and
+ * returns false. */
+static bool
+check_db_conflict(struct shash *ls_to_db,
+  const struct sbrec_port_binding *port_binding_rec,
+  const struct sbrec_chassis *chassis_rec)
+{
+const char *vtep_lswitch =
+smap_get(port_binding_rec-options, vtep-logical-switch);
+const struct sbrec_datapath_binding *db_conflict =
+shash_find_data(ls_to_db, vtep_lswitch);
+
+if (db_conflict  db_conflict != port_binding_rec-datapath) {
+VLOG_WARN(logical switch 

[ovs-dev] [ovn-controller-vtep V5 04/12] ovn-northd: Pass logical port type and options to ovn-sb database.

[Alex Wang]

Signed-off-by: Alex Wang al...@nicira.com
---
V5:
- new patch.
---
 ovn/northd/ovn-northd.c |2 ++
 tests/ovn-sbctl.at  |   13 +
 2 files changed, 15 insertions(+)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index cf8e222..796070f 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -397,6 +397,8 @@ join_logical_ports(struct northd_context *ctx,
 static void
 ovn_port_update_sbrec(const struct ovn_port *op)
 {
+sbrec_port_binding_set_type(op-sb, op-nb-type);
+sbrec_port_binding_set_options(op-sb, op-nb-options);
 sbrec_port_binding_set_datapath(op-sb, op-od-sb);
 sbrec_port_binding_set_parent_port(op-sb, op-nb-parent_name);
 sbrec_port_binding_set_tag(op-sb, op-nb-tag, op-nb-n_tag);
diff --git a/tests/ovn-sbctl.at b/tests/ovn-sbctl.at
index a8d0fe8..6bda181 100644
--- a/tests/ovn-sbctl.at
+++ b/tests/ovn-sbctl.at
@@ -57,5 +57,18 @@ mac : [[f0:ab:cd:ef:01:02]]
 chassis : ${uuid}
 ])
 
+# test the passing down of logical port type and options.
+AT_CHECK([ovn-nbctl lport-add br-test vtep0])
+AT_CHECK([ovn-nbctl lport-set-type vtep0 vtep])
+AT_CHECK([ovn-nbctl lport-set-options vtep0 vtep_physical_switch=p0 
vtep_logical_switch=l0])
+
+OVS_WAIT_UNTIL([test -n `ovn-sbctl --columns=logical_port list Port_Binding | 
grep vtep0` ])
+AT_CHECK_UNQUOTED([ovn-sbctl --columns=logical_port,mac,type,options list 
Port_Binding vtep0], [0], [dnl
+logical_port: vtep0
+mac : [[]]
+type: vtep
+options : {vtep_logical_switch=l0, vtep_physical_switch=p0}
+])
+
 OVN_SBCTL_TEST_STOP
 AT_CLEANUP
-- 
1.7.9.5

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

[ovs-dev] [ovn-controller-vtep V5 05/12] idl-loop: Move idl-loop into ovsdb-idl library.

[Alex Wang]

idl-loop is needed in implementing other controller (i.e., vtep controller).
So, this commit moves the logic into ovsdb-idl library module.

Signed-off-by: Alex Wang al...@nicira.com
---
V5:
- new patch.
---
 lib/ovsdb-idl.c |   72 ++
 lib/ovsdb-idl.h |   26 ++
 ovn/controller/ovn-controller.c |  108 +--
 3 files changed, 110 insertions(+), 96 deletions(-)

diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 4ec7032..f0d5d9c 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -2632,3 +2632,75 @@ ovsdb_idl_parse_lock_notify(struct ovsdb_idl *idl,
 }
 }
 }
+
+void
+ovsdb_idl_loop_destroy(struct ovsdb_idl_loop *loop)
+{
+if (loop) {
+ovsdb_idl_destroy(loop-idl);
+}
+}
+
+struct ovsdb_idl_txn *
+ovsdb_idl_loop_run(struct ovsdb_idl_loop *loop)
+{
+ovsdb_idl_run(loop-idl);
+loop-open_txn = (loop-committing_txn
+  || ovsdb_idl_get_seqno(loop-idl) == loop-skip_seqno
+  ? NULL
+  : ovsdb_idl_txn_create(loop-idl));
+return loop-open_txn;
+}
+
+void
+ovsdb_idl_loop_commit_and_wait(struct ovsdb_idl_loop *loop)
+{
+if (loop-open_txn) {
+loop-committing_txn = loop-open_txn;
+loop-open_txn = NULL;
+
+loop-precommit_seqno = ovsdb_idl_get_seqno(loop-idl);
+}
+
+struct ovsdb_idl_txn *txn = loop-committing_txn;
+if (txn) {
+enum ovsdb_idl_txn_status status = ovsdb_idl_txn_commit(txn);
+if (status != TXN_INCOMPLETE) {
+switch (status) {
+case TXN_TRY_AGAIN:
+/* We want to re-evaluate the database when it's changed from
+ * the contents that it had when we started the commit.  (That
+ * might have already happened.) */
+loop-skip_seqno = loop-precommit_seqno;
+if (ovsdb_idl_get_seqno(loop-idl) != loop-skip_seqno) {
+poll_immediate_wake();
+}
+break;
+
+case TXN_SUCCESS:
+/* If the database has already changed since we started the
+ * commit, re-evaluate it immediately to avoid missing a change
+ * for a while. */
+if (ovsdb_idl_get_seqno(loop-idl) != loop-precommit_seqno) {
+poll_immediate_wake();
+}
+break;
+
+case TXN_UNCHANGED:
+case TXN_ABORTED:
+case TXN_NOT_LOCKED:
+case TXN_ERROR:
+break;
+
+case TXN_UNCOMMITTED:
+case TXN_INCOMPLETE:
+OVS_NOT_REACHED();
+
+}
+ovsdb_idl_txn_destroy(txn);
+loop-committing_txn = NULL;
+}
+}
+
+ovsdb_idl_wait(loop-idl);
+}
diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h
index a49f84f..28aa787 100644
--- a/lib/ovsdb-idl.h
+++ b/lib/ovsdb-idl.h
@@ -222,5 +222,31 @@ const struct ovsdb_idl_row *ovsdb_idl_txn_insert(
 const struct uuid *);
 
 struct ovsdb_idl *ovsdb_idl_txn_get_idl (struct ovsdb_idl_txn *);
+
+
+/* ovsdb_idl_loop provides an easy way to manage the transactions related
+ * to 'idl' and to cope with different status during transaction. */
+struct ovsdb_idl_loop {
+struct ovsdb_idl *idl;
+unsigned int skip_seqno;
+
+struct ovsdb_idl_txn *committing_txn;
+unsigned int precommit_seqno;
+
+struct ovsdb_idl_txn *open_txn;
+};
+
+#define OVSDB_IDL_LOOP_INITIALIZER(IDL) { .idl = (IDL) }
+
+static inline void
+ovsdb_idl_loop_init(struct ovsdb_idl_loop *loop, struct ovsdb_idl *idl)
+{
+memset(loop, 0, sizeof *loop);
+loop-idl = idl;
+}
+
+void ovsdb_idl_loop_destroy(struct ovsdb_idl_loop *);
+struct ovsdb_idl_txn *ovsdb_idl_loop_run(struct ovsdb_idl_loop *);
+void ovsdb_idl_loop_commit_and_wait(struct ovsdb_idl_loop *);
 
 #endif /* ovsdb-idl.h */
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 2c4248f..bb2b840 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -128,90 +128,6 @@ get_ovnsb_remote(struct ovsdb_idl *ovs_idl)
 }
 }
 
-struct idl_loop {
-struct ovsdb_idl *idl;
-unsigned int skip_seqno;
-
-struct ovsdb_idl_txn *committing_txn;
-unsigned int precommit_seqno;
-
-struct ovsdb_idl_txn *open_txn;
-};
-
-#define IDL_LOOP_INITIALIZER(IDL) { .idl = (IDL) }
-
-static void
-idl_loop_destroy(struct idl_loop *loop)
-{
-if (loop) {
-ovsdb_idl_destroy(loop-idl);
-}
-}
-
-static struct ovsdb_idl_txn *
-idl_loop_run(struct idl_loop *loop)
-{
-ovsdb_idl_run(loop-idl);
-loop-open_txn = (loop-committing_txn
-  || ovsdb_idl_get_seqno(loop-idl) == loop-skip_seqno
-  ? NULL
-  : ovsdb_idl_txn_create(loop-idl));
-return loop-open_txn;
-}
-
-static void
-idl_loop_commit_and_wait(struct idl_loop *loop)

[ovs-dev] [ovn-controller-vtep V5 01/12] ovn-sb: Remove the Gateway table from the ovn-sb schema.

[Alex Wang]

In a gateway like the VTEP L2 gateway, physical vlans belonging to
the same logical network form a logical switch.  Each logical switch
has a dedicated tunnel key and will keep records of all MACs learned
from the owned vlans.  So user can just send packet to a logical
switch and the gateway will figure out the output port and vlan tag
automatically.

Therefore, it is really not necessary to keep record of the vlan map
for each gateway physical port in the OVN_Southbound database using
gateway_ports and to map each vlan to a unique ovn logical port.
Instead, we should simply map each logical switch to a ovn logical
port.

Thusly, this commit removes the Gateway table from the OVN_Southbound
database.  In the Chassis table, the gateway_ports column is replaced
by vtep_logical_switches column which stores all vtep logical switch
names.  Then, in the OVN_Northbound database, the CMS must specify the
physical switch name and the logical switch name via
options:vtep_physical_switch and options:vtep_logical_switch when
creating logical port with type vtep.

Signed-off-by: Alex Wang al...@nicira.com

---
V4-V5:
- rebase on top of master.
- change vtep_logical_switches to a set of strings storing all logical
  switch names.
- require user to specify vtep_physical_switch and vtep_logical_switch
  in options.

V3-V4:
- change the column name to vtep_logical_switches.
- adopt Ben's refinement of ovn-sb schema manual.

-V3:
- Realize that the Gateway table is not needed.
---
 ovn/ovn-nb.xml   |   31 +
 ovn/ovn-sb.ovsschema |   16 ++-
 ovn/ovn-sb.xml   |   75 --
 3 files changed, 70 insertions(+), 52 deletions(-)

diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index ade8164..77b559d 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -111,18 +111,37 @@
 column name=type
   p
   Specify a type for this logical port.  Logical ports can be used to model
-  other types of connectivity into an OVN logical switch.  Leaving this 
column
-  blank maintains the default logical port behavior.
+  other types of connectivity into an OVN logical switch.  Leaving this
+  column blank maintains the default logical port behavior, which is
+  for a VM (or VIF) interface.  Besides, the following types are
+  defined:
   /p
 
-  p
-  There are no other logical port types implemented yet.
-  /p
+  dl
+dtcodevtep/code/dt
+ddA port to a logical switch on a VTEP gateway.  In order
+to get this port correctly recognized by the ovn controller, the
+ref column=options table=Logical_Port/:vtep_physical_switch
+and ref column=options table=Logical_Port/:vtep_logical_switch
+must also be defined./dd
+  /dl
 /column
 
 column name=options
+  p
 This column provides key/value settings specific to the logical port
-ref column=type/.
+ref column=type/.  The following options are defined:
+  /p
+
+  dl
+dtcodevtep_physical_switch/code/dt
+ddThe ref column=name table=Chassis/ of the VTEP gateway./dd
+  /dl
+
+  dl
+dtcodevtep_logical_switch/code/dt
+ddA logical switch name connected by the VTEP gateway./dd
+  /dl
 /column
 
 column name=parent_name
diff --git a/ovn/ovn-sb.ovsschema b/ovn/ovn-sb.ovsschema
index 40a29e9..9ee7431 100644
--- a/ovn/ovn-sb.ovsschema
+++ b/ovn/ovn-sb.ovsschema
@@ -7,12 +7,9 @@
 encaps: {type: {key: {type: uuid,
 refTable: Encap},
 min: 1, max: unlimited}},
-gateway_ports: {type: {key: string,
-   value: {type: uuid,
- refTable: Gateway,
- refType: strong},
-   min: 0,
-   max: unlimited}}},
+vtep_logical_switches : {type: {key: string,
+min: 0,
+max: unlimited}}},
 isRoot: true,
 indexes: [[name]]},
 Encap: {
@@ -25,13 +22,6 @@
  min: 0,
  max: unlimited}},
 ip: {type: string}}},
-Gateway: {
-columns: {vlan_map: {type: {key: {type: integer,
-  minInteger: 0,
-  maxInteger: 4095},
-  value: {type: string},
-  min: 0,
-  max: unlimited,
 Logical_Flow: {
 columns: {
 logical_datapath: {type: 

[ovs-dev] [ovn-controller-vtep V5 02/12] ovn-nbctl: Move ovn-nbctl to utilities directory.

[Alex Wang]

Signed-off-by: Alex Wang al...@nicira.com
---
V5:
- new patch.
---
 ovn/.gitignore  |2 --
 ovn/automake.mk |   11 +++
 ovn/utilities/.gitignore|3 +++
 ovn/utilities/automake.mk   |   14 +++---
 ovn/{ = utilities}/ovn-nbctl.8.xml |0
 ovn/{ = utilities}/ovn-nbctl.c |0
 6 files changed, 17 insertions(+), 13 deletions(-)
 rename ovn/{ = utilities}/ovn-nbctl.8.xml (100%)
 rename ovn/{ = utilities}/ovn-nbctl.c (100%)

diff --git a/ovn/.gitignore b/ovn/.gitignore
index 4c13616..5b3bc55 100644
--- a/ovn/.gitignore
+++ b/ovn/.gitignore
@@ -5,5 +5,3 @@
 /ovn-sb.5
 /ovn-sb.gv
 /ovn-sb.pic
-/ovn-nbctl
-/ovn-nbctl.8
diff --git a/ovn/automake.mk b/ovn/automake.mk
index 1170a0b..901e710 100644
--- a/ovn/automake.mk
+++ b/ovn/automake.mk
@@ -66,20 +66,15 @@ ovn/ovn-nb.5: \
$(srcdir)/ovn/ovn-nb.xml  $@.tmp  \
mv $@.tmp $@
 
-man_MANS += ovn/ovn-architecture.7 ovn/ovn-nbctl.8
-EXTRA_DIST += ovn/ovn-architecture.7.xml ovn/ovn-nbctl.8.xml
-DISTCLEANFILES += ovn/ovn-nbctl.8 ovn/ovn-architecture.7
+man_MANS += ovn/ovn-architecture.7
+EXTRA_DIST += ovn/ovn-architecture.7.xml
+DISTCLEANFILES += ovn/ovn-architecture.7
 
 EXTRA_DIST += \
ovn/TODO \
ovn/CONTAINERS.OpenStack.md \
ovn/OVN-GW-HA.md
 
-# ovn-nbctl
-bin_PROGRAMS += ovn/ovn-nbctl
-ovn_ovn_nbctl_SOURCES = ovn/ovn-nbctl.c
-ovn_ovn_nbctl_LDADD = ovn/lib/libovn.la ovsdb/libovsdb.la lib/libopenvswitch.la
-
 include ovn/controller/automake.mk
 include ovn/lib/automake.mk
 include ovn/northd/automake.mk
diff --git a/ovn/utilities/.gitignore b/ovn/utilities/.gitignore
index 1ddc63e..714aaab 100644
--- a/ovn/utilities/.gitignore
+++ b/ovn/utilities/.gitignore
@@ -1 +1,4 @@
 /ovn-ctl.8
+/ovn-nbctl
+/ovn-nbctl.8
+
diff --git a/ovn/utilities/automake.mk b/ovn/utilities/automake.mk
index 1373864..145ee44 100644
--- a/ovn/utilities/automake.mk
+++ b/ovn/utilities/automake.mk
@@ -2,11 +2,19 @@ scripts_SCRIPTS += \
 ovn/utilities/ovn-ctl
 
 man_MANS += \
-ovn/utilities/ovn-ctl.8
+ovn/utilities/ovn-ctl.8 \
+ovn/utilities/ovn-nbctl.8
 
 EXTRA_DIST += \
 ovn/utilities/ovn-ctl \
-ovn/utilities/ovn-ctl.8.xml
+ovn/utilities/ovn-ctl.8.xml \
+ovn/utilities/ovn-nbctl.8.xml
 
 DISTCLEANFILES += \
-ovn/utilities/ovn-ctl.8
+ovn/utilities/ovn-ctl.8 \
+ovn/utilities/ovn-nbctl.8
+
+# ovn-nbctl
+bin_PROGRAMS += ovn/utilities/ovn-nbctl
+ovn_utilities_ovn_nbctl_SOURCES = ovn/utilities/ovn-nbctl.c
+ovn_utilities_ovn_nbctl_LDADD = ovn/lib/libovn.la ovsdb/libovsdb.la 
lib/libopenvswitch.la
diff --git a/ovn/ovn-nbctl.8.xml b/ovn/utilities/ovn-nbctl.8.xml
similarity index 100%
rename from ovn/ovn-nbctl.8.xml
rename to ovn/utilities/ovn-nbctl.8.xml
diff --git a/ovn/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
similarity index 100%
rename from ovn/ovn-nbctl.c
rename to ovn/utilities/ovn-nbctl.c
-- 
1.7.9.5

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

[ovs-dev] [ovn-controller-vtep V5 00/12] Implement ovn-controller-vtep.

[Alex Wang]

This series adds a new ovn controller, ovn-controller-vtep, for VTEP
enabled physical switches.

This V5 is not simply a rebase on top of the current master.  With
the additon of logcial port 'type' and 'options' columns in ovn-nb
schema, and the new way of generating tunnel keys, this series
contains some series rework and optimization over the previous
verions.  Specifically:

1. ovn logical port to be bound to a VTEP gateway chassis must
   be configured with type vtep and options:vtep-physical-switch
   and vtep-logical-switch.

2. ovn-controller-vtep is responsible for setting the tunnel key of
   the VTEP logical switch.

3. more unittests are added for different corner cases. 

Any comments are welcomed~ 

Go OVN! ~

Alex Wang (12):
  ovn-sb: Remove the Gateway table from the ovn-sb schema.
  ovn-nbctl: Move ovn-nbctl to utilities directory.
  ovn-sbctl: Add ovn-sbctl.
  ovn-northd: Pass logical port type and options to ovn-sb database.
  idl-loop: Move idl-loop into ovsdb-idl library.
  ovn: Add controller for VTEP gateway.
  ovsdb-idl: Move get_initial_snapshot() to ovsdb-idl.
  ovn-controller-vtep: Add gateway module.
  ovn-controller-vtep: Add binding module.
  ovn-controller-vtep: Add vtep module.
  ovn-controller-vtep: Extend vtep module to install Ucast_Macs_Remote.
  ovn: Update TODOs related to vtep controller.

 lib/ovsdb-idl.c   |   87 +++
 lib/ovsdb-idl.h   |   27 +
 manpages.mk   |   12 +
 ovn/.gitignore|2 -
 ovn/TODO  |   27 +
 ovn/automake.mk   |   12 +-
 ovn/controller-vtep/.gitignore|2 +
 ovn/controller-vtep/automake.mk   |   14 +
 ovn/controller-vtep/binding.c |  247 +++
 ovn/controller-vtep/binding.h |   27 +
 ovn/controller-vtep/gateway.c |  224 +++
 ovn/controller-vtep/gateway.h |   26 +
 ovn/controller-vtep/ovn-controller-vtep.8.xml |   70 ++
 ovn/controller-vtep/ovn-controller-vtep.c |  266 
 ovn/controller-vtep/ovn-controller-vtep.h |   51 ++
 ovn/controller-vtep/vtep.c|  368 +++
 ovn/controller-vtep/vtep.h|   27 +
 ovn/controller/ovn-controller.c   |  125 +---
 ovn/northd/ovn-northd.c   |2 +
 ovn/ovn-nb.xml|   31 +-
 ovn/ovn-sb.ovsschema  |   16 +-
 ovn/ovn-sb.xml|   75 ++-
 ovn/utilities/.gitignore  |4 +
 ovn/utilities/automake.mk |   23 +-
 ovn/{ = utilities}/ovn-nbctl.8.xml   |0
 ovn/{ = utilities}/ovn-nbctl.c   |0
 ovn/utilities/ovn-sbctl.8.in  |  160 +
 ovn/utilities/ovn-sbctl.c |  870 +
 tests/automake.mk |7 +-
 tests/ovn-controller-vtep.at  |  432 
 tests/ovn-sbctl.at|   74 +++
 tests/testsuite.at|2 +
 32 files changed, 3131 insertions(+), 179 deletions(-)
 create mode 100644 ovn/controller-vtep/.gitignore
 create mode 100644 ovn/controller-vtep/automake.mk
 create mode 100644 ovn/controller-vtep/binding.c
 create mode 100644 ovn/controller-vtep/binding.h
 create mode 100644 ovn/controller-vtep/gateway.c
 create mode 100644 ovn/controller-vtep/gateway.h
 create mode 100644 ovn/controller-vtep/ovn-controller-vtep.8.xml
 create mode 100644 ovn/controller-vtep/ovn-controller-vtep.c
 create mode 100644 ovn/controller-vtep/ovn-controller-vtep.h
 create mode 100644 ovn/controller-vtep/vtep.c
 create mode 100644 ovn/controller-vtep/vtep.h
 rename ovn/{ = utilities}/ovn-nbctl.8.xml (100%)
 rename ovn/{ = utilities}/ovn-nbctl.c (100%)
 create mode 100644 ovn/utilities/ovn-sbctl.8.in
 create mode 100644 ovn/utilities/ovn-sbctl.c
 create mode 100644 tests/ovn-controller-vtep.at
 create mode 100644 tests/ovn-sbctl.at

-- 
1.7.9.5

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

Re: [ovs-dev] [PATCH] dpif: allow adding ukeys for same flow by different pmds

[Ilya Maximets]

Ok. I think, it will be better. I'll prepare new version.

On 06.08.2015 23:32, Joe Stringer wrote:
 On 6 August 2015 at 00:54, Ilya Maximets i.maxim...@samsung.com wrote:
 ping.

 On 30.07.2015 18:29, Ilya Maximets wrote:
 In multiqueue mode several pmd threads may process one
 port, but different queues. Flow doesn't depend on queue.

 So, while miss upcall processing, all threads (except first
 for that port) will receive error = ENOSPC due to
 ukey_install failure. Therefore they will not add the flow
 to flow_table and will not insert it to exact match cache.

 As a result all threads (except first for that port) will
 always execute a miss.

 Fix that by comparing ukeys not only by ufids but also
 by pmd_ids.

 Signed-off-by: Ilya Maximets i.maxim...@samsung.com
 
 Apologies for the delay. Did you consider amending get_ufid_hash() to
 take a pmd_id instead, mixing the id in with the bits from the ufid?
 This should decrease ukey iteration as there should be less ukeys with
 the same hash in the ukey_maps (same hash - map lookup for hash, then
 linked list N_PMD_THREADS long).
 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH v2] dpif: allow adding ukeys for same flow by different pmds

[Ilya Maximets]

In multiqueue mode several pmd threads may process one
port, but different queues. Flow doesn't depend on queue.

So, while miss upcall processing, all threads (except first
for that port) will receive error = ENOSPC due to
ukey_install failure. Therefore they will not add the flow
to flow_table and will not insert it to exact match cache.

As a result all threads (except first for that port) will
always execute a miss.

Fix that by mixing pmd_id with the bits from the ufid
for ukey-hash calculation.

Signed-off-by: Ilya Maximets i.maxim...@samsung.com
---
 ofproto/ofproto-dpif-upcall.c | 22 --
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 0f2e186..57a7f34 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -291,7 +291,8 @@ static bool ukey_install_start(struct udpif *, struct 
udpif_key *ukey);
 static bool ukey_install_finish(struct udpif_key *ukey, int error);
 static bool ukey_install(struct udpif *udpif, struct udpif_key *ukey);
 static struct udpif_key *ukey_lookup(struct udpif *udpif,
- const ovs_u128 *ufid);
+ const ovs_u128 *ufid,
+ const unsigned pmd_id);
 static int ukey_acquire(struct udpif *, const struct dpif_flow *,
 struct udpif_key **result, int *error);
 static void ukey_delete__(struct udpif_key *);
@@ -1143,7 +1144,8 @@ process_upcall(struct udpif *udpif, struct upcall *upcall,
 }
 if (actions_len == 0) {
 /* Lookup actions in userspace cache. */
-struct udpif_key *ukey = ukey_lookup(udpif, upcall-ufid);
+struct udpif_key *ukey = ukey_lookup(udpif, upcall-ufid,
+ upcall-pmd_id);
 if (ukey) {
 actions = ukey-actions-data;
 actions_len = ukey-actions-size;
@@ -1320,19 +1322,19 @@ handle_upcalls(struct udpif *udpif, struct upcall 
*upcalls,
 }
 
 static uint32_t
-get_ufid_hash(const ovs_u128 *ufid)
+get_ukey_hash(const ovs_u128 *ufid, const unsigned pmd_id)
 {
-return ufid-u32[0];
+return ufid-u32[0] + pmd_id;
 }
 
 static struct udpif_key *
-ukey_lookup(struct udpif *udpif, const ovs_u128 *ufid)
+ukey_lookup(struct udpif *udpif, const ovs_u128 *ufid, const unsigned pmd_id)
 {
 struct udpif_key *ukey;
-int idx = get_ufid_hash(ufid) % N_UMAPS;
+int idx = get_ukey_hash(ufid, pmd_id) % N_UMAPS;
 struct cmap *cmap = udpif-ukeys[idx].cmap;
 
-CMAP_FOR_EACH_WITH_HASH (ukey, cmap_node, get_ufid_hash(ufid), cmap) {
+CMAP_FOR_EACH_WITH_HASH (ukey, cmap_node, get_ukey_hash(ufid, pmd_id), 
cmap) {
 if (ovs_u128_equals(ukey-ufid, ufid)) {
 return ukey;
 }
@@ -1362,7 +1364,7 @@ ukey_create__(const struct nlattr *key, size_t key_len,
 ukey-ufid_present = ufid_present;
 ukey-ufid = *ufid;
 ukey-pmd_id = pmd_id;
-ukey-hash = get_ufid_hash(ukey-ufid);
+ukey-hash = get_ukey_hash(ukey-ufid, pmd_id);
 ukey-actions = ofpbuf_clone(actions);
 
 ovs_mutex_init(ukey-mutex);
@@ -1490,7 +1492,7 @@ ukey_install_start(struct udpif *udpif, struct udpif_key 
*new_ukey)
 idx = new_ukey-hash % N_UMAPS;
 umap = udpif-ukeys[idx];
 ovs_mutex_lock(umap-mutex);
-old_ukey = ukey_lookup(udpif, new_ukey-ufid);
+old_ukey = ukey_lookup(udpif, new_ukey-ufid, new_ukey-pmd_id);
 if (old_ukey) {
 /* Uncommon case: A ukey is already installed with the same UFID. */
 if (old_ukey-key_len == new_ukey-key_len
@@ -1572,7 +1574,7 @@ ukey_acquire(struct udpif *udpif, const struct dpif_flow 
*flow,
 struct udpif_key *ukey;
 int retval;
 
-ukey = ukey_lookup(udpif, flow-ufid);
+ukey = ukey_lookup(udpif, flow-ufid, flow-pmd_id);
 if (ukey) {
 retval = ovs_mutex_trylock(ukey-mutex);
 } else {
-- 
2.1.4

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

[ovs-dev] openvswitch inside a docker container

[Yoo, Jay]

I've created a ryu container and openvswitch container with ovs-vsctl bridge 
connecting the port to ryu container and host containers.

I ran simple_switch.py on ryu to learn l2 switching and still I can't ping 
among containers.  Any thoughts?

Best Regards,

Jay Yoo, Ph.D.

Johns Hopkins University - Applied Physics Laboratory
11100 Johns Hopkins Road, Laurel, MD 20723
240-228-6544
jay@jhuapl.edu
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] upcall processing in userspace

[ravulakollu.kumar]

Hi Team,

I am debugging upcall processing in ofproto-dpif-upcall.c  file . I am not able 
to visualize how opfbuf structure is being
filled with actions(Note: My bridge is configured without OF rules, meaning 
actions=NORMAL).
I could see packet is being flooded for unknown mac, vlan in the code. But 
somehow dpif_netlink_operate is receiving proper netlink action
attributes in sequence to be executed in DP.
Also I could see in gdb xout.odp_actions is only holding some addresses. How to 
know what actually resides in this address.
(gdb) p upcall-xout.odp_actions-data
$7 = (void *) 0x7f6275a53398

Could someone please , let me know  how this buffer is being filled, and what 
is the importance of ofpbuf.

Thanks in Advance,
Uday
The information contained in this electronic message and any attachments to 
this message are intended for the exclusive use of the addressee(s) and may 
contain proprietary, confidential or privileged information. If you are not the 
intended recipient, you should not disseminate, distribute or copy this e-mail. 
Please notify the sender immediately and destroy all copies of this message and 
any attachments. WARNING: Computer viruses can be transmitted via email. The 
recipient should check this email and any attachments for the presence of 
viruses. The company accepts no liability for any damage caused by any virus 
transmitted by this email. www.wipro.com
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] upcall processing in userspace

[Jarno Rajahalme]

Uday,

ofproto-dpif-upcall.c calls into ofproto-dpif-xlate.c that performs flow 
lookups and thereby find the actions that should be applied to the packet. For 
example, action types are discerned in do_xlate_actions(). The “normal” action 
is actually a special port number (OFPP_NORMAL) and is processed by 
late_output_action().

To see the flow lookups and action processing in action, you should get 
familiar with “ovs-appctl ofproto/trace”. You can find numerous examples by:

$ cd tests
$ git grep “ofproto/trace”

Regards,

  Jarno

 On Aug 7, 2015, at 7:57 AM, ravulakollu.ku...@wipro.com 
 ravulakollu.ku...@wipro.com wrote:
 
 Hi Team,
 
 I am debugging upcall processing in ofproto-dpif-upcall.c  file . I am not 
 able to visualize how opfbuf structure is being
 filled with actions(Note: My bridge is configured without OF rules, meaning 
 actions=NORMAL).
 I could see packet is being flooded for unknown mac, vlan in the code. But 
 somehow dpif_netlink_operate is receiving proper netlink action
 attributes in sequence to be executed in DP.
 Also I could see in gdb xout.odp_actions is only holding some addresses. How 
 to know what actually resides in this address.
 (gdb) p upcall-xout.odp_actions-data
 $7 = (void *) 0x7f6275a53398
 
 Could someone please , let me know  how this buffer is being filled, and what 
 is the importance of ofpbuf.
 
 Thanks in Advance,
 Uday
 The information contained in this electronic message and any attachments to 
 this message are intended for the exclusive use of the addressee(s) and may 
 contain proprietary, confidential or privileged information. If you are not 
 the intended recipient, you should not disseminate, distribute or copy this 
 e-mail. Please notify the sender immediately and destroy all copies of this 
 message and any attachments. WARNING: Computer viruses can be transmitted via 
 email. The recipient should check this email and any attachments for the 
 presence of viruses. The company accepts no liability for any damage caused 
 by any virus transmitted by this email. www.wipro.com
 ___
 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] datapath forwarding

[Raul Suarez Marin]

Hello,

Thank you for replying. I managed to see printk() on dmesg, I was debugging
wrong (I'm new in this kernel-space area).

I traced my mistake and I found it, thank you very much for your help

Kind regards,
Raúl

2015-08-06 20:14 GMT+02:00 Joe Stringer joestrin...@nicira.com:

 --verbose makes the ovs-ofctl process more verbose by allowing the
 VLOG_DBG() functions to print to the terminal.

 For kernel debugging, you want printk() or OVS_NLERR(), and these will
 print to the kernel log (which shows up in dmesg). If you're not able
 to see it there, then it may mean that your code isn't executing (are
 you sure that your own custom version of the module is inserted into
 the kernel?)

 On 6 August 2015 at 10:52, Raul Suarez Marin
 raul.suarez.ma...@gmail.com wrote:
  Is there a way of adding messages myself and see them thorugh --verbose?
 I
  was told that printk() did that but I'm not able to see it anywhere :(
 
  Thank you for your answer,
 
  Kind regards,
  Raúl
 
  2015-08-05 22:40 GMT+02:00 Joe Stringer joestrin...@nicira.com:
 
  On 5 August 2015 at 13:02, Raul Suarez Marin
  raul.suarez.ma...@gmail.com wrote:
   Hello everyone,
  
   I am developing a new match option in the datapath. I implemented
 almost
   everything already, but I am missing something and I don't know what
 nor
   what is next step.
  
   Adding the flow directly to the datapath gives the following error:
   ovs-dpctl: updating flow table (Invalid argument)
  
   The full stack
  
   sudo ovs-dpctl add-flow ovs-system
  
  
 in_port(3),eth(src=00:00:00:00:ca:fe,dst=05:05:05:05:05:05),eth_type(0x0800),ipv4(src=
  
  
 10.10.0.1/255.255.255.255,dst=10.10.0.2/255.255.255.255,proto=17/0xff,tos=0/0,ttl=64/0,frag=no/0xff
 ),udp(src=2152/0,dst=2152/0x),new_match_option(11259375/0x)
   1
  
   2015-08-05T19:58:58Z|1|dpif|WARN|system@ovs-system: failed to
   put[create] (Invalid argument)
  
  
 in_port(3),eth(src=00:00:00:00:ca:fe,dst=05:05:05:05:05:05),eth_type(0x0800),ipv4(src=
  
  
 10.10.0.1/255.255.255.255,dst=10.10.0.2/255.255.255.255,proto=17/0xff,tos=0/0,ttl=64/0,frag=no/0xff
  
  
 ),udp(src=2152/0,dst=2152/0x),new_match_option(id=11259375/0x)
   ovs-dpctl: updating flow table (Invalid argument)
  
  
   Any thoughts, hints or comments are appreciated.
 
  dmesg may also have some information.
 
  You can use --verbose to see more detail on what is actually
  happening from the ovs-dpctl perspective. But it looks like the kernel
  is rejecting your flow installation because you're providing an
  invalid argument (EINVAL), so you'll need to trace through the
  ovs_flow_cmd_new() code to see why that might be the case.
 
 

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

Re: [ovs-dev] [ovn-controller-vtep V5 04/12] ovn-northd: Pass logical port type and options to ovn-sb database.

[Russell Bryant]

On 08/07/2015 03:46 AM, Alex Wang wrote:
 Signed-off-by: Alex Wang al...@nicira.com

Nice catch, thanks!

It's not needed now, but at some point we might want to move this test
into a new file that tests the things ovn-northd is supposed to do.
Just a thought.  Thanks for adding tests.  It's helping me see how I
could add some other test cases!

Acked-by: Russell Bryant rbry...@redhat.com

 ---
 V5:
 - new patch.
 ---
  ovn/northd/ovn-northd.c |2 ++
  tests/ovn-sbctl.at  |   13 +
  2 files changed, 15 insertions(+)
 
 diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
 index cf8e222..796070f 100644
 --- a/ovn/northd/ovn-northd.c
 +++ b/ovn/northd/ovn-northd.c
 @@ -397,6 +397,8 @@ join_logical_ports(struct northd_context *ctx,
  static void
  ovn_port_update_sbrec(const struct ovn_port *op)
  {
 +sbrec_port_binding_set_type(op-sb, op-nb-type);
 +sbrec_port_binding_set_options(op-sb, op-nb-options);
  sbrec_port_binding_set_datapath(op-sb, op-od-sb);
  sbrec_port_binding_set_parent_port(op-sb, op-nb-parent_name);
  sbrec_port_binding_set_tag(op-sb, op-nb-tag, op-nb-n_tag);
 diff --git a/tests/ovn-sbctl.at b/tests/ovn-sbctl.at
 index a8d0fe8..6bda181 100644
 --- a/tests/ovn-sbctl.at
 +++ b/tests/ovn-sbctl.at
 @@ -57,5 +57,18 @@ mac : [[f0:ab:cd:ef:01:02]]
  chassis : ${uuid}
  ])
  
 +# test the passing down of logical port type and options.
 +AT_CHECK([ovn-nbctl lport-add br-test vtep0])
 +AT_CHECK([ovn-nbctl lport-set-type vtep0 vtep])
 +AT_CHECK([ovn-nbctl lport-set-options vtep0 vtep_physical_switch=p0 
 vtep_logical_switch=l0])
 +
 +OVS_WAIT_UNTIL([test -n `ovn-sbctl --columns=logical_port list Port_Binding 
 | grep vtep0` ])
 +AT_CHECK_UNQUOTED([ovn-sbctl --columns=logical_port,mac,type,options list 
 Port_Binding vtep0], [0], [dnl
 +logical_port: vtep0
 +mac : [[]]
 +type: vtep
 +options : {vtep_logical_switch=l0, vtep_physical_switch=p0}
 +])
 +
  OVN_SBCTL_TEST_STOP
  AT_CLEANUP
 


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

[ovs-dev] [PATCH v2 2/4] tests: Introduce NS_EXEC and NS_CHECK_EXEC for system tests.

[Daniele Di Proietto]

Instead of repeating every time ip netns exec ... it is better to
introduce some macros.

Signed-off-by: Daniele Di Proietto diproiet...@vmware.com
---
 tests/system-common-macros.at | 27 +--
 tests/system-traffic.at   | 24 
 2 files changed, 33 insertions(+), 18 deletions(-)

diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at
index bd622d2..ccdfdc6 100644
--- a/tests/system-common-macros.at
+++ b/tests/system-common-macros.at
@@ -7,7 +7,7 @@ m4_define([DEL_NAMESPACES],
 ])
]
 )
-#
+
 # ADD_NAMESPACES(ns [, ns ... ])
 #
 # Add new namespaces, if ns exists, the old one
@@ -21,6 +21,21 @@ m4_define([ADD_NAMESPACES],
]
 )
 
+# NS_EXEC([namespace], [command])
+#
+# Execute 'command' in 'namespace'
+m4_define([NS_EXEC],
+[ip netns exec $1 bash -c $2]
+)
+
+# NS_CHECK_EXEC([namespace], [command], other_params...)
+#
+# Wrapper for AT_CHECK that executes 'command' inside 'namespace'.
+# 'other_params' as passed as they are to AT_CHECK.
+m4_define([NS_CHECK_EXEC],
+[ AT_CHECK([NS_EXEC([$1], [$2])], m4_shift(m4_shift($@))) ]
+)
+
 # ADD_VETH([port], [namespace], [ovs-br], [ip_addr])
 #
 # Add a pair of veth ports. 'port' will be added to name space 'namespace',
@@ -36,8 +51,8 @@ m4_define([ADD_VETH],
   AT_CHECK([ip link set $1 netns $2])
   AT_CHECK([ovs-vsctl add-port $3 ovs-$1])
   AT_CHECK([ip link set dev ovs-$1 up])
-  AT_CHECK([ip netns exec $2 ip addr add $4 dev $1])
-  AT_CHECK([ip netns exec $2 ip link set dev $1 up])
+  NS_CHECK_EXEC([$2], [ip addr add $4 dev $1])
+  NS_CHECK_EXEC([$2], [ip link set dev $1 up])
 ]
 )
 
@@ -46,8 +61,8 @@ m4_define([ADD_VETH],
 # Add a VLAN device named 'port' within 'namespace'. It will be configured
 # with the ID 'vlan-id' and the address 'ip-addr'.
 m4_define([ADD_VLAN],
-[ AT_CHECK([ip netns exec $2 ip link add link $1 name $1.$3 type vlan id 
$3])
-  AT_CHECK([ip netns exec $2 ip link set dev $1.$3 up])
-  AT_CHECK([ip netns exec $2 ip addr add dev $1.$3 $4])
+[ NS_CHECK_EXEC([$2], [ip link add link $1 name $1.$3 type vlan id $3])
+  NS_CHECK_EXEC([$2], [ip link set dev $1.$3 up])
+  NS_CHECK_EXEC([$2], [ip addr add dev $1.$3 $4])
 ]
 )
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 9df8b62..34342d6 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -10,9 +10,9 @@ ADD_VETH(p0, at_ns0, br0, 10.1.1.1/24)
 ADD_VETH(p1, at_ns1, br0, 10.1.1.2/24)
 
 AT_CAPTURE_FILE([ping.output])
-AT_CHECK([ip netns exec at_ns0 bash -c ping -q -c 3 -i 0.3 -w 2 10.1.1.2  
ping.output])
-AT_CHECK([ip netns exec at_ns0 bash -c ping -s 1600 -q -c 3 -i 0.3 -w 2 
10.1.1.2  ping.output])
-AT_CHECK([ip netns exec at_ns0 bash -c ping -s 3200 -q -c 3 -i 0.3 -w 2 
10.1.1.2  ping.output])
+NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.2  ping.output])
+NS_CHECK_EXEC([at_ns0], [ping -s 1600 -q -c 3 -i 0.3 -w 2 10.1.1.2  
ping.output])
+NS_CHECK_EXEC([at_ns0], [ping -s 3200 -q -c 3 -i 0.3 -w 2 10.1.1.2  
ping.output])
 
 AT_CHECK([cat ping.output | grep transmitted | sed 's/time.*ms$/time 0ms/'], 
[0], [dnl
 3 packets transmitted, 3 received, 0% packet loss, time 0ms
@@ -36,9 +36,9 @@ ADD_VLAN(p0, at_ns0, 100, 10.2.2.1/24)
 ADD_VLAN(p1, at_ns1, 100, 10.2.2.2/24)
 
 AT_CAPTURE_FILE([ping.output])
-AT_CHECK([ip netns exec at_ns0 bash -c ping -q -c 3 -i 0.3 -w 2 10.2.2.2  
ping.output])
-AT_CHECK([ip netns exec at_ns0 bash -c ping -s 1600 -q -c 3 -i 0.3 -w 2 
10.2.2.2  ping.output])
-AT_CHECK([ip netns exec at_ns0 bash -c ping -s 3200 -q -c 3 -i 0.3 -w 2 
10.2.2.2  ping.output])
+NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.2.2.2  ping.output])
+NS_CHECK_EXEC([at_ns0], [ping -s 1600 -q -c 3 -i 0.3 -w 2 10.2.2.2  
ping.output])
+NS_CHECK_EXEC([at_ns0], [ping -s 3200 -q -c 3 -i 0.3 -w 2 10.2.2.2  
ping.output])
 
 AT_CHECK([cat ping.output | grep transmitted | sed 's/time.*ms$/time 0ms/'], 
[0], [dnl
 3 packets transmitted, 3 received, 0% packet loss, time 0ms
@@ -63,9 +63,9 @@ dnl connect: Cannot assign requested address
 sleep 2;
 
 AT_CAPTURE_FILE([ping.output])
-AT_CHECK([ip netns exec at_ns0 bash -c ping6 -q -c 3 -i 0.3 -w 2 fc00::2  
ping.output])
-AT_CHECK([ip netns exec at_ns0 bash -c ping6 -s 1600 -q -c 3 -i 0.3 -w 2 
fc00::2  ping.output])
-AT_CHECK([ip netns exec at_ns0 bash -c ping6 -s 3200 -q -c 3 -i 0.3 -w 2 
fc00::2  ping.output])
+NS_CHECK_EXEC([at_ns0], [ping6 -q -c 3 -i 0.3 -w 2 fc00::2  ping.output])
+NS_CHECK_EXEC([at_ns0], [ping6 -s 1600 -q -c 3 -i 0.3 -w 2 fc00::2  
ping.output])
+NS_CHECK_EXEC([at_ns0], [ping6 -s 3200 -q -c 3 -i 0.3 -w 2 fc00::2  
ping.output])
 
 AT_CHECK([cat ping.output | grep transmitted | sed 's/time.*ms$/time 0ms/'], 
[0], [dnl
 3 packets transmitted, 3 received, 0% packet loss, time 0ms
@@ -93,9 +93,9 @@ dnl connect: Cannot assign requested address
 sleep 2;
 
 AT_CAPTURE_FILE([ping.output])
-AT_CHECK([ip netns exec at_ns0 bash -c ping6 -q -c 3 -i 0.3 

[ovs-dev] [PATCH v2 4/4] Vagrantfile: Add test_ovs_system_userspace provision.

[Daniele Di Proietto]

Add 'test_ovs_system_userspace' provision.  Command:
# vagrant provision --provision-with=test_ovs_system_userspace

will run make check-system-userspace in the vagrant launched VM.

It may be more convenient to run this tests inside a vm rather than in
the host, because they interact with system networking.

Suggested-by: Joe Stringer joestrin...@nicira.com
Signed-off-by: Daniele Di Proietto diproiet...@vmware.com
---
 INSTALL.md  | 7 +++
 NEWS| 3 +++
 Vagrantfile | 6 ++
 3 files changed, 16 insertions(+)

diff --git a/INSTALL.md b/INSTALL.md
index 3dc16aa..9dac430 100644
--- a/INSTALL.md
+++ b/INSTALL.md
@@ -631,6 +631,13 @@ To recompile and reinstall OVS using RPM:
./boot.sh
vagrant provision --provision-with configure_ovs,install_rpm
 
+Two provisioners are included to run system tests with the OVS kernel
+module or with a userspace datapath.  This tests are different from
+the self-tests mentioned above.  To run them:
+
+   ./boot.sh
+   vagrant provision --provision-with 
configure_ovs,test_ovs_kmod,test_ovs_system_userspace
+
 Continuous Integration with Travis-CI
 -
 
diff --git a/NEWS b/NEWS
index f16df70..7b0a897 100644
--- a/NEWS
+++ b/NEWS
@@ -17,6 +17,9 @@ Post-v2.4.0
- Added OpenFlow 1.4+ OFPT_SET_ASYNC_CONFIG and OFPT_GET_ASYNC_CONFIG
  that allows controllers to have more precise control over OpenFlow1.4
  asynchronous messages.
+   - New 'check-system-userspace', 'check-kmod' and 'check-kernel' Makefile
+ targets to run a new system testsuite.  These tests can be run inside
+ a Vagrant box.  See INSTALL.md for details
 
 
 v2.4.0 - xx xxx 
diff --git a/Vagrantfile b/Vagrantfile
index 03e8693..e8c104d 100644
--- a/Vagrantfile
+++ b/Vagrantfile
@@ -47,6 +47,11 @@ systemctl start openvswitch
 systemctl status openvswitch
 SCRIPT
 
+$test_ovs_system_userspace = SCRIPT
+cd ~/build
+make check-system-userspace
+SCRIPT
+
 Vagrant.configure(VAGRANTFILE_API_VERSION) do |config|
   config.vm.define fedora-20 do |fedora|
fedora.vm.box = chef/fedora-20
@@ -54,6 +59,7 @@ Vagrant.configure(VAGRANTFILE_API_VERSION) do |config|
fedora.vm.provision configure_ovs, type: shell, inline: 
$configure_ovs
fedora.vm.provision build_ovs, type: shell, inline: $build_ovs
fedora.vm.provision test_ovs_kmod, type: shell, inline: $test_kmod
+   fedora.vm.provision test_ovs_system_userspace, type: shell, inline: 
$test_ovs_system_userspace
fedora.vm.provision install_rpm, type: shell, inline: $install_rpm
   end
 end
-- 
2.1.4

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

Re: [ovs-dev] [PATCH 0/3] Vagrant userspace testsuite

[Daniele Di Proietto]

On 07/08/2015 19:15, Joe Stringer joestrin...@nicira.com wrote:

On 4 August 2015 at 11:00, Daniele Di Proietto diproiet...@vmware.com
wrote:
 This series adds a new testsuite for the userspace datapath that runs
 the already written kmod-sanity tests.  The reason for this are
 explained in the 3rd commit message.

 The first two commits just move some code (without modifing it), while
 the 3rd introduces the new infrastructure.

 The testsuite is called `userspace-testsuite` and can be launched with
 `make check-userspace`.

Thanks, applied patches 1-2. Let me know if you're blocking on me for
moving forward with patch 3.

Thanks, I've decided to keep the make targets for the kernel as they are.
I've applied your suggestions and sent a v2 to the list.

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

[ovs-dev] [PATCH v2 3/4] tests: Add system-userspace-testsuite.

[Daniele Di Proietto]

The new system-userspace-testsuite, which can be launched via
`make check-system-userspace`, reuses the kmod tests on the userspace
datapath.

The userspace datapath is already tested by the main testsuite (and
that's not going to change), but having also the
system-userspace-testsuite has the following advantages:

* More complicated tests are possible: real client and server
  applications can be used.
* The same tests run on both kernel and userspace datapath: this gives
  us an easy way to make sure that the behaviour is consistent (e.g.
  with the upcoming connection tracker integration)

The userspace datapath is able to use system network interfaces via an
AF_PACKET socket.

Signed-off-by: Daniele Di Proietto diproiet...@vmware.com
---
 tests/.gitignore|  1 +
 tests/automake.mk   | 22 ++--
 tests/system-common-macros.at   |  3 ++-
 tests/system-kmod-macros.at | 19 +++---
 tests/system-traffic.at | 26 
 tests/system-userspace-macros.at| 40 +
 tests/system-userspace-testsuite.at | 23 +
 7 files changed, 111 insertions(+), 23 deletions(-)
 create mode 100644 tests/system-userspace-macros.at
 create mode 100644 tests/system-userspace-testsuite.at

diff --git a/tests/.gitignore b/tests/.gitignore
index 98a42f9..f4540a3 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -11,6 +11,7 @@
 /ovs-pki.log
 /pki/
 /system-kmod-testsuite
+/system-userspace-testsuite
 /test-aes128
 /test-atomic
 /test-bundle
diff --git a/tests/automake.mk b/tests/automake.mk
index 9e428ea..20bcb3f 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -1,9 +1,12 @@
 EXTRA_DIST += \
$(COMMON_MACROS_AT) \
$(TESTSUITE_AT) \
+   $(SYSTEM_TESTSUITE_AT) \
$(SYSTEM_KMOD_TESTSUITE_AT) \
+   $(SYSTEM_USERSPACE_TESTSUITE_AT) \
$(TESTSUITE) \
$(SYSTEM_KMOD_TESTSUITE) \
+   $(SYSTEM_USERSPACE_TESTSUITE) \
tests/atlocal.in \
$(srcdir)/package.m4 \
$(srcdir)/tests/testsuite \
@@ -88,12 +91,20 @@ TESTSUITE_AT = \
 SYSTEM_KMOD_TESTSUITE_AT = \
tests/system-common-macros.at \
tests/system-kmod-testsuite.at \
-   tests/system-kmod-macros.at \
+   tests/system-kmod-macros.at
+
+SYSTEM_USERSPACE_TESTSUITE_AT = \
+   tests/system-userspace-testsuite.at \
+   tests/system-userspace-macros.at
+
+SYSTEM_TESTSUITE_AT = \
+   tests/system-common-macros.at \
tests/system-traffic.at
 
 TESTSUITE = $(srcdir)/tests/testsuite
 TESTSUITE_PATCH = $(srcdir)/tests/testsuite.patch
 SYSTEM_KMOD_TESTSUITE = $(srcdir)/tests/system-kmod-testsuite
+SYSTEM_USERSPACE_TESTSUITE = $(srcdir)/tests/system-userspace-testsuite
 DISTCLEANFILES += tests/atconfig tests/atlocal
 
 AUTOTEST_PATH = utilities:vswitchd:ovsdb:vtep:tests:$(PTHREAD_WIN32_DIR_DLL)
@@ -199,6 +210,9 @@ check-kmod: all tests/atconfig tests/atlocal 
$(SYSTEM_KMOD_TESTSUITE)
modprobe -r openvswitch
$(MAKE) check-kernel
 
+check-system-userspace: all tests/atconfig tests/atlocal 
$(SYSTEM_USERSPACE_TESTSUITE)
+   $(SHELL) '$(SYSTEM_USERSPACE_TESTSUITE)' -C tests  
AUTOTEST_PATH='$(AUTOTEST_PATH)' $(TESTSUITEFLAGS)
+
 clean-local:
test ! -f '$(TESTSUITE)' || $(SHELL) '$(TESTSUITE)' -C tests --clean
 
@@ -215,7 +229,11 @@ $(TESTSUITE): package.m4 $(TESTSUITE_AT) 
$(COMMON_MACROS_AT)
$(AM_V_at)mv $@.tmp $@
 endif
 
-$(SYSTEM_KMOD_TESTSUITE): package.m4 $(SYSTEM_KMOD_TESTSUITE_AT) 
$(COMMON_MACROS_AT)
+$(SYSTEM_KMOD_TESTSUITE): package.m4 $(SYSTEM_TESTSUITE_AT) 
$(SYSTEM_KMOD_TESTSUITE_AT) $(COMMON_MACROS_AT)
+   $(AM_V_GEN)$(AUTOTEST) -I '$(srcdir)' -o $@.tmp $@.at
+   $(AM_V_at)mv $@.tmp $@
+
+$(SYSTEM_USERSPACE_TESTSUITE): package.m4 $(SYSTEM_TESTSUITE_AT) 
$(SYSTEM_USERSPACE_TESTSUITE_AT) $(COMMON_MACROS_AT)
$(AM_V_GEN)$(AUTOTEST) -I '$(srcdir)' -o $@.tmp $@.at
$(AM_V_at)mv $@.tmp $@
 
diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at
index ccdfdc6..1321e58 100644
--- a/tests/system-common-macros.at
+++ b/tests/system-common-macros.at
@@ -49,10 +49,11 @@ m4_define([NS_CHECK_EXEC],
 m4_define([ADD_VETH],
 [ AT_CHECK([ip link add $1 type veth peer name ovs-$1])
   AT_CHECK([ip link set $1 netns $2])
-  AT_CHECK([ovs-vsctl add-port $3 ovs-$1])
   AT_CHECK([ip link set dev ovs-$1 up])
+  AT_CHECK([ovs-vsctl add-port $3 ovs-$1])
   NS_CHECK_EXEC([$2], [ip addr add $4 dev $1])
   NS_CHECK_EXEC([$2], [ip link set dev $1 up])
+  ON_EXIT([ip link del ovs-$1])
 ]
 )
 
diff --git a/tests/system-kmod-macros.at b/tests/system-kmod-macros.at
index c74d348..eacdc5d 100644
--- a/tests/system-kmod-macros.at
+++ b/tests/system-kmod-macros.at
@@ -1,4 +1,10 @@
-# OVS_KMOD_VSWITCHD_START([vsctl-args], [vsctl-output], [=override])
+# ADD_BR([name])
+#
+# Expands into the proper ovs-vsctl commands to create a bridge with 

Re: [ovs-dev] [ovn-controller-vtep V5 02/12] ovn-nbctl: Move ovn-nbctl to utilities directory.

[Russell Bryant]

On 08/07/2015 01:18 PM, Russell Bryant wrote:
 On 08/07/2015 03:46 AM, Alex Wang wrote:
 Signed-off-by: Alex Wang al...@nicira.com
 ---
 V5:
 - new patch.
 ---
  ovn/.gitignore  |2 --
  ovn/automake.mk |   11 +++
  ovn/utilities/.gitignore|3 +++
  ovn/utilities/automake.mk   |   14 +++---
  ovn/{ = utilities}/ovn-nbctl.8.xml |0
  ovn/{ = utilities}/ovn-nbctl.c |0
  6 files changed, 17 insertions(+), 13 deletions(-)
  rename ovn/{ = utilities}/ovn-nbctl.8.xml (100%)
  rename ovn/{ = utilities}/ovn-nbctl.c (100%)
 
 diff --git a/ovn/utilities/.gitignore b/ovn/utilities/.gitignore
 index 1ddc63e..714aaab 100644
 --- a/ovn/utilities/.gitignore
 +++ b/ovn/utilities/.gitignore
 @@ -1 +1,4 @@
  /ovn-ctl.8
 +/ovn-nbctl
 +/ovn-nbctl.8
 +
 
 git complains that this patch introduces a whitespace error.  It's this
 new blank line at the end of .gitignore.
 
 Otherwise:
 
 Acked-by: Russell Bryant rbry...@redhat.com
 

One more thing ... this can be fixed later if needed, but ovn/utilities/
will need to be added to PATH in tutorial/ovs-sandbox.  After this
patch, ovn-nbctl (and later ovn-sbctl) aren't in $PATH when running the
sandbox.

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

Re: [ovs-dev] [ovn-controller-vtep V5 03/12] ovn-sbctl: Add ovn-sbctl.

[Russell Bryant]

On 08/07/2015 03:46 AM, Alex Wang wrote:
 This commit adds ovn-sbctl to ovn family by using the db-ctl-base
 library.
 
 Signed-off-by: Alex Wang al...@nicira.com
 Acked-by: Ben Pfaff b...@nicira.com

I verified that the tests pass and also played around with it manually.
 Everything worked as expected.  I only found one minor doc issue.  Nice
work!

Acked-by: Russell Bryant rbry...@redhat.com

 +static void
 +usage(void)
 +{
 +printf(\
 +%s: ovs-vswitchd management utility\n\
 +\n\
 +for debugging and testing only, never use it in production\n\
 +\n\
 +usage: %s [OPTIONS] COMMAND [ARG...]\n\
 +\n\
 +SouthBound DB commands:\n\
 +  showprint overview of database contents\n\
 +\n\
 +Chassis commands:\n\
 +  chassis-add CHASSIS create a new chassis named CHASSIS\n\

encap-type and encap-ip are required here, as well.  They're shown in
the man page but missing from this usage output.

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

Re: [ovs-dev] [PATCH 2/3] kmod-macros: Don't unload kmod in VSWITCHD_STOP.

[Joe Stringer]

On 6 August 2015 at 18:37, Andy Zhou az...@nicira.com wrote:
 On Thu, Aug 6, 2015 at 3:58 PM, Joe Stringer joestrin...@nicira.com wrote:
 On 30 July 2015 at 12:51, Joe Stringer joestrin...@nicira.com wrote:
 On 30 July 2015 at 11:57, Andy Zhou az...@nicira.com wrote:
 On Wed, Jul 29, 2015 at 4:52 PM, Joe Stringer joestrin...@nicira.com 
 wrote:
 We already queue the removal of the kernel module in OVS_VSWITCHD_START,
 via an ON_EXIT() call. This redundant removal also doesn't interact very
 well with usage of vports: If the datapath still exists in the kernel,
 we cannot remove a vport module; if we cannot remove a vport module, we
 cannot remove the openvswitch module. Having the call here prevents
 tunnel tests from manually cleaning up their datapath on exit, so this
 patch removes it.


 I don't understand this. If we consider each test as an independent
 sessions, should we be able
 to be add and remove kernel modules in between?  Some kernel bugs only
 shows up during module
 clean ups.

 The issue is that as long as the datapath has a tunnel port configured, you
 cannot remove the vport module (and you need to remove the vport
 module before removing openvswitch module). The solution I use here is
 to queue up three commands using ON_EXIT():
 - During OVS_VSWITCHD_START, ON_EXIT(modprobe -r openvswitch)
 - At the beginning of vport tests, ON_EXIT(modprobe -r vport_vxlan)
 - Following this, ON_EXIT(ovs-dpctl del-dp ovs-system).

 These should be executed in reverse order to remove tunnel ports,
 remove tunnel modules, and remove the ovs module.

 Andy, we spoke offline about this and as I understand your thought is
 that tests should begin with OVS_VSWITCHD_START and finish with
 OVS_VSWITCHD_STOP. I'm all for that, but I believe that the START
 operation (also, ADD_VETH, ADD_VLAN, etc.) should queue up cleanup of
 those devices where necessary so that whether it is a successful test
 run or a test failure, cleanup should occur. This will cause the
 minimum amount of disruption for subsequent tests. OVS_VSWITCHD_STOP
 still provides a way to safely cleanup the OVS processes and check
 logs for unexpected output, but it can leave out cleanup which is
 necessary to execute in both success and failure conditions.

 I am fine with the ON_EXIT() approach proposed here.  In terms of ovs modules,
 I wonder if it will be simpler if we simply load all modules for all
 kernel tests.
 OVS kmod used to be a single module.

Sounds reasonable.


 For cases where people want to debug a test in the middle, they can
 always clear the trap and nothing will be cleaned up, allowing
 debugging:
 trap : 0; AT_CHECK([fail])

 This is a good ideal. Providing an macro for it would be nice.  The cleanup
 file we generate can also be enhanced so that it can be invoked by hand.

I was thinking this as well.


 Given that the trap that's set up by ON_EXIT() is always run (even if
 OVS_VSWITCHD_STOP occurs first), I believe that this patch has the
 correct behaviour. Do you have any remaining reservations?
 No.

OK thanks, I've applied this patch to master. I plan to rebase and
improve the ping output checking for the remaining patch and resend
it.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH net-next v3] openvswitch: Make 100 percents packets sampled when sampling rate is 1.

[David Miller]

From: Wenyu Zhang wen...@vmware.com
Date: Wed, 5 Aug 2015 00:30:47 -0700

 When sampling rate is 1, the sampling probability is UINT32_MAX. The packet
 should be sampled even the prandom32() generate the number of UINT32_MAX.
 And none packet need be sampled when the probability is 0.
 
 Signed-off-by: Wenyu Zhang wen...@vmware.com

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

Re: [ovs-dev] [ovn-controller-vtep V5 01/12] ovn-sb: Remove the Gateway table from the ovn-sb schema.

[Russell Bryant]

On 08/07/2015 03:46 AM, Alex Wang wrote:
 In a gateway like the VTEP L2 gateway, physical vlans belonging to
 the same logical network form a logical switch.  Each logical switch
 has a dedicated tunnel key and will keep records of all MACs learned
 from the owned vlans.  So user can just send packet to a logical
 switch and the gateway will figure out the output port and vlan tag
 automatically.
 
 Therefore, it is really not necessary to keep record of the vlan map
 for each gateway physical port in the OVN_Southbound database using
 gateway_ports and to map each vlan to a unique ovn logical port.
 Instead, we should simply map each logical switch to a ovn logical
 port.
 
 Thusly, this commit removes the Gateway table from the OVN_Southbound
 database.  In the Chassis table, the gateway_ports column is replaced
 by vtep_logical_switches column which stores all vtep logical switch
 names.  Then, in the OVN_Northbound database, the CMS must specify the
 physical switch name and the logical switch name via
 options:vtep_physical_switch and options:vtep_logical_switch when
 creating logical port with type vtep.
 
 Signed-off-by: Alex Wang al...@nicira.com

The change makes sense to me, but I had a couple of doc comments.

 ---
 V4-V5:
 - rebase on top of master.
 - change vtep_logical_switches to a set of strings storing all logical
   switch names.
 - require user to specify vtep_physical_switch and vtep_logical_switch
   in options.
 
 V3-V4:
 - change the column name to vtep_logical_switches.
 - adopt Ben's refinement of ovn-sb schema manual.
 
 -V3:
 - Realize that the Gateway table is not needed.
 ---
  ovn/ovn-nb.xml   |   31 +
  ovn/ovn-sb.ovsschema |   16 ++-
  ovn/ovn-sb.xml   |   75 
 --
  3 files changed, 70 insertions(+), 52 deletions(-)
 
 diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
 index ade8164..77b559d 100644
 --- a/ovn/ovn-nb.xml
 +++ b/ovn/ovn-nb.xml
 @@ -111,18 +111,37 @@
  column name=type
p
Specify a type for this logical port.  Logical ports can be used to 
 model
 -  other types of connectivity into an OVN logical switch.  Leaving this 
 column
 -  blank maintains the default logical port behavior.
 +  other types of connectivity into an OVN logical switch.  Leaving this
 +  column blank maintains the default logical port behavior, which is
 +  for a VM (or VIF) interface.  Besides, the following types are
 +  defined:
/p
  
 -  p
 -  There are no other logical port types implemented yet.
 -  /p
 +  dl
 +dtcodevtep/code/dt
 +ddA port to a logical switch on a VTEP gateway.  In order
 +to get this port correctly recognized by the ovn controller, the
 +ref column=options table=Logical_Port/:vtep_physical_switch
 +and ref column=options table=Logical_Port/:vtep_logical_switch
 +must also be defined./dd
 +  /dl
  /column
  
  column name=options
 +  p
  This column provides key/value settings specific to the logical port
 -ref column=type/.
 +ref column=type/.  The following options are defined:
 +  /p
 +
 +  dl
 +dtcodevtep_physical_switch/code/dt
 +ddThe ref column=name table=Chassis/ of the VTEP 
 gateway./dd

I'm not sure if this ref makes sense since it's a table in another schema.

 +  /dl
 +
 +  dl
 +dtcodevtep_logical_switch/code/dt
 +ddA logical switch name connected by the VTEP gateway./dd
 +  /dl
  /column

All of these docs look good, but do you think it makes sense to add
these docs later in the series once all of the code is in place to
support the 'vtep' port type?

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

Re: [ovs-dev] [PATCH 0/3] Vagrant userspace testsuite

[Joe Stringer]

On 4 August 2015 at 11:00, Daniele Di Proietto diproiet...@vmware.com wrote:
 This series adds a new testsuite for the userspace datapath that runs
 the already written kmod-sanity tests.  The reason for this are
 explained in the 3rd commit message.

 The first two commits just move some code (without modifing it), while
 the 3rd introduces the new infrastructure.

 The testsuite is called `userspace-testsuite` and can be launched with
 `make check-userspace`.

Thanks, applied patches 1-2. Let me know if you're blocking on me for
moving forward with patch 3.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [ovn-controller-vtep V5 06/12] ovn: Add controller for VTEP gateway.

[Russell Bryant]

On 08/07/2015 03:46 AM, Alex Wang wrote:
 This commit lays down the foundation for a new controller in OVN, the
 ovn-controller-vtep, for controlling the vtep enabled gateways.
 
 Signed-off-by: Alex Wang al...@nicira.com

 diff --git a/ovn/controller-vtep/ovn-controller-vtep.c 
 b/ovn/controller-vtep/ovn-controller-vtep.c
 new file mode 100644
 index 000..e8bfcbd
 --- /dev/null
 +++ b/ovn/controller-vtep/ovn-controller-vtep.c
 @@ -0,0 +1,245 @@
 +/* Copyright (c) 2015 Nicira, Inc.
 + *
 + * Licensed under the Apache License, Version 2.0 (the License);
 + * you may not use this file except in compliance with the License.
 + * You may obtain a copy of the License at:
 + *
 + * http://www.apache.org/licenses/LICENSE-2.0
 + *
 + * Unless required by applicable law or agreed to in writing, software
 + * distributed under the License is distributed on an AS IS BASIS,
 + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 + * See the License for the specific language governing permissions and
 + * limitations under the License.
 + */
 +
 +#include config.h
 +
 +#include errno.h
 +#include getopt.h
 +#include signal.h
 +#include stdlib.h
 +#include string.h
 +
 +#include ovn-controller-vtep.h
 +
 +#include command-line.h
 +#include compiler.h
 +#include daemon.h
 +#include dirs.h
 +#include dynamic-string.h
 +#include fatal-signal.h
 +#include poll-loop.h
 +#include stream.h
 +#include stream-ssl.h
 +#include unixctl.h
 +#include util.h
 +#include openvswitch/vconn.h
 +#include openvswitch/vlog.h
 +#include ovn/lib/ovn-sb-idl.h
 +#include vtep/vtep-idl.h
 +
 +VLOG_DEFINE_THIS_MODULE(ovn_vtep);

The patch doesn't build because of this line:

ovn/controller-vtep/ovn-controller-vtep.c:42:1: error: unused variable
'THIS_MODULE' [-Werror,-Wunused-const-variable]
VLOG_DEFINE_THIS_MODULE(ovn_vtep);
^
./include/openvswitch/vlog.h:185:42: note: expanded from macro
'VLOG_DEFINE_THIS_MODULE'
static struct vlog_module *const THIS_MODULE = VLM_##MODULE

So, I guess it should be added later when you introduce some logging?

 +
 +static unixctl_cb_func ovn_controller_vtep_exit;
 +
 +static void parse_options(int argc, char *argv[]);
 +OVS_NO_RETURN static void usage(void);
 +
 +static char *vtep_remote;
 +static char *ovnsb_remote;
 +
 +static void
 +get_initial_snapshot(struct ovsdb_idl *idl)
 +{
 +while (1) {
 +ovsdb_idl_run(idl);
 +if (ovsdb_idl_has_ever_connected(idl)) {
 +return;
 +}
 +ovsdb_idl_wait(idl);
 +poll_block();
 +}
 +}
 +
 +int
 +main(int argc, char *argv[])
 +{
 +struct unixctl_server *unixctl;
 +bool exiting;
 +int retval;
 +
 +ovs_cmdl_proctitle_init(argc, argv);
 +set_program_name(argv[0]);
 +parse_options(argc, argv);
 +fatal_ignore_sigpipe();
 +
 +daemonize_start();
 +
 +retval = unixctl_server_create(NULL, unixctl);
 +if (retval) {
 +exit(EXIT_FAILURE);
 +}
 +unixctl_command_register(exit, , 0, 0, ovn_controller_vtep_exit,
 + exiting);
 +
 +daemonize_complete();
 +
 +vteprec_init();
 +sbrec_init();
 +
 +/* Connect to VTEP database. */
 +struct ovsdb_idl_loop vtep_idl_loop = OVSDB_IDL_LOOP_INITIALIZER(
 +ovsdb_idl_create(vtep_remote, vteprec_idl_class, false, true));
 +ovsdb_idl_add_table(vtep_idl_loop.idl, vteprec_table_global);
 +ovsdb_idl_add_column(vtep_idl_loop.idl,
 + vteprec_global_col_switches);
 +get_initial_snapshot(vtep_idl_loop.idl);
 +
 +/* Connect to OVN SB database. */
 +struct ovsdb_idl_loop ovnsb_idl_loop = OVSDB_IDL_LOOP_INITIALIZER(
 +ovsdb_idl_create(ovnsb_remote, sbrec_idl_class, false, true));
 +get_initial_snapshot(ovnsb_idl_loop.idl);
 +
 +/* Main loop. */
 +exiting = false;
 +while (!exiting) {

I expected ovsdb_idl_loop_run() in here.  I see you add it a couple of
patches later.

You could add it here and mark it as temporarily unused, though:

struct controller_vtep_ctx OVS_UNUSED ctx = {
.vtep_idl = vtep_idl_loop.idl,
.vtep_idl_txn = ovsdb_idl_loop_run(vtep_idl_loop),
.ovnsb_idl = ovnsb_idl_loop.idl,
.ovnsb_idl_txn = ovsdb_idl_loop_run(ovnsb_idl_loop),
};



 +unixctl_server_run(unixctl);
 +
 +unixctl_server_wait(unixctl);
 +if (exiting) {
 +poll_immediate_wake();
 +}
 +ovsdb_idl_loop_commit_and_wait(vtep_idl_loop);
 +ovsdb_idl_loop_commit_and_wait(ovnsb_idl_loop);
 +poll_block();
 +}
 +
 +unixctl_server_destroy(unixctl);
 +
 +ovsdb_idl_loop_destroy(vtep_idl_loop);
 +ovsdb_idl_loop_destroy(ovnsb_idl_loop);
 +
 +free(ovnsb_remote);
 +free(vtep_remote);
 +
 +exit(retval);
 +}

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

Re: [ovs-dev] [ovn-controller-vtep V5 02/12] ovn-nbctl: Move ovn-nbctl to utilities directory.

[Russell Bryant]

On 08/07/2015 03:46 AM, Alex Wang wrote:
 Signed-off-by: Alex Wang al...@nicira.com
 ---
 V5:
 - new patch.
 ---
  ovn/.gitignore  |2 --
  ovn/automake.mk |   11 +++
  ovn/utilities/.gitignore|3 +++
  ovn/utilities/automake.mk   |   14 +++---
  ovn/{ = utilities}/ovn-nbctl.8.xml |0
  ovn/{ = utilities}/ovn-nbctl.c |0
  6 files changed, 17 insertions(+), 13 deletions(-)
  rename ovn/{ = utilities}/ovn-nbctl.8.xml (100%)
  rename ovn/{ = utilities}/ovn-nbctl.c (100%)

 diff --git a/ovn/utilities/.gitignore b/ovn/utilities/.gitignore
 index 1ddc63e..714aaab 100644
 --- a/ovn/utilities/.gitignore
 +++ b/ovn/utilities/.gitignore
 @@ -1 +1,4 @@
  /ovn-ctl.8
 +/ovn-nbctl
 +/ovn-nbctl.8
 +

git complains that this patch introduces a whitespace error.  It's this
new blank line at the end of .gitignore.

Otherwise:

Acked-by: Russell Bryant rbry...@redhat.com

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

[ovs-dev] [PATCH v2 1/4] tests: Rename kmod-testsuite to system-kmod-testsuite.

[Daniele Di Proietto]

The name makes more sense, especially with the addition of a userspace
system testsuite.  No functional change in this commit.

Signed-off-by: Daniele Di Proietto diproiet...@vmware.com
---
 tests/.gitignore   |   2 +-
 tests/automake.mk  |  24 -
 tests/kmod-macros.at   |  35 --
 tests/kmod-testsuite.at|  23 -
 tests/system-common-macros.at  |  53 
 tests/system-kmod-macros.at|  35 ++
 tests/system-kmod-testsuite.at |  23 +
 tests/system-traffic.at| 107 +
 tests/traffic-common-macros.at |  53 
 tests/traffic.at   | 107 -
 10 files changed, 231 insertions(+), 231 deletions(-)
 delete mode 100644 tests/kmod-macros.at
 delete mode 100644 tests/kmod-testsuite.at
 create mode 100644 tests/system-common-macros.at
 create mode 100644 tests/system-kmod-macros.at
 create mode 100644 tests/system-kmod-testsuite.at
 create mode 100644 tests/system-traffic.at
 delete mode 100644 tests/traffic-common-macros.at
 delete mode 100644 tests/traffic.at

diff --git a/tests/.gitignore b/tests/.gitignore
index 9d0be33..98a42f9 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -6,11 +6,11 @@
 /idltest.c
 /idltest.h
 /idltest.ovsidl
-/kmod-testsuite
 /ovstest
 /test-dpdkr
 /ovs-pki.log
 /pki/
+/system-kmod-testsuite
 /test-aes128
 /test-atomic
 /test-bundle
diff --git a/tests/automake.mk b/tests/automake.mk
index 502bbd5..9e428ea 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -1,9 +1,9 @@
 EXTRA_DIST += \
$(COMMON_MACROS_AT) \
$(TESTSUITE_AT) \
-   $(KMOD_TESTSUITE_AT) \
+   $(SYSTEM_KMOD_TESTSUITE_AT) \
$(TESTSUITE) \
-   $(KMOD_TESTSUITE) \
+   $(SYSTEM_KMOD_TESTSUITE) \
tests/atlocal.in \
$(srcdir)/package.m4 \
$(srcdir)/tests/testsuite \
@@ -85,15 +85,15 @@ TESTSUITE_AT = \
tests/auto-attach.at \
tests/ovn.at
 
-KMOD_TESTSUITE_AT = \
-   tests/kmod-testsuite.at \
-   tests/kmod-macros.at \
-   tests/traffic-common-macros.at \
-   tests/traffic.at
+SYSTEM_KMOD_TESTSUITE_AT = \
+   tests/system-common-macros.at \
+   tests/system-kmod-testsuite.at \
+   tests/system-kmod-macros.at \
+   tests/system-traffic.at
 
 TESTSUITE = $(srcdir)/tests/testsuite
 TESTSUITE_PATCH = $(srcdir)/tests/testsuite.patch
-KMOD_TESTSUITE = $(srcdir)/tests/kmod-testsuite
+SYSTEM_KMOD_TESTSUITE = $(srcdir)/tests/system-kmod-testsuite
 DISTCLEANFILES += tests/atconfig tests/atlocal
 
 AUTOTEST_PATH = utilities:vswitchd:ovsdb:vtep:tests:$(PTHREAD_WIN32_DIR_DLL)
@@ -190,11 +190,11 @@ check-ryu: all
 EXTRA_DIST += tests/run-ryu
 
 # Run kmod tests. Assume kernel modules has been installed or linked into the 
kernel
-check-kernel: all tests/atconfig tests/atlocal $(KMOD_TESTSUITE)
-   $(SHELL) '$(KMOD_TESTSUITE)' -C tests  AUTOTEST_PATH='$(AUTOTEST_PATH)' 
-d $(TESTSUITEFLAGS)
+check-kernel: all tests/atconfig tests/atlocal $(SYSTEM_KMOD_TESTSUITE)
+   $(SHELL) '$(SYSTEM_KMOD_TESTSUITE)' -C tests  
AUTOTEST_PATH='$(AUTOTEST_PATH)' -d $(TESTSUITEFLAGS)
 
 # Testing the out of tree Kernel module
-check-kmod: all tests/atconfig tests/atlocal $(KMOD_TESTSUITE)
+check-kmod: all tests/atconfig tests/atlocal $(SYSTEM_KMOD_TESTSUITE)
$(MAKE) modules_install
modprobe -r openvswitch
$(MAKE) check-kernel
@@ -215,7 +215,7 @@ $(TESTSUITE): package.m4 $(TESTSUITE_AT) $(COMMON_MACROS_AT)
$(AM_V_at)mv $@.tmp $@
 endif
 
-$(KMOD_TESTSUITE): package.m4 $(KMOD_TESTSUITE_AT) $(COMMON_MACROS_AT)
+$(SYSTEM_KMOD_TESTSUITE): package.m4 $(SYSTEM_KMOD_TESTSUITE_AT) 
$(COMMON_MACROS_AT)
$(AM_V_GEN)$(AUTOTEST) -I '$(srcdir)' -o $@.tmp $@.at
$(AM_V_at)mv $@.tmp $@
 
diff --git a/tests/kmod-macros.at b/tests/kmod-macros.at
deleted file mode 100644
index c74d348..000
--- a/tests/kmod-macros.at
+++ /dev/null
@@ -1,35 +0,0 @@
-# OVS_KMOD_VSWITCHD_START([vsctl-args], [vsctl-output], [=override])
-#
-# Creates a database and starts ovsdb-server, starts ovs-vswitchd
-# connected to that database, calls ovs-vsctl to create a bridge named
-# br0 with predictable settings, passing 'vsctl-args' as additional
-# commands to ovs-vsctl.  If 'vsctl-args' causes ovs-vsctl to provide
-# output (e.g. because it includes create commands) then 'vsctl-output'
-# specifies the expected output after filtering through uuidfilt.pl.
-#
-m4_define([OVS_KMOD_VSWITCHD_START],
-  [ AT_CHECK([modprobe openvswitch])
-ON_EXIT([modprobe -r openvswitch])
-   _OVS_VSWITCHD_START([])
-   dnl Add bridges, ports, etc.
-   AT_CHECK([ovs-vsctl -- add-br br0 -- set bridge br0 
protocols=[[OpenFlow10,OpenFlow11,OpenFlow12,OpenFlow13,OpenFlow14,OpenFlow15]] 
fail-mode=secure -- $1 m4_if([$2], [], [], [| ${PERL} $srcdir/uuidfilt.pl])], 
[0], [$2])
-])
-#
-#
-# OVS_KMOD_VSWITCHD_STOP([WHITELIST], [extra_cmds])

[ovs-dev] [PATCH v2 0/4] Vagrant userspace testsuite

[Daniele Di Proietto]

This series adds a new testsuite for the userspace datapath that runs
the already written kmod-sanity tests.  The reason for this are
explained in the 3rd commit message.

The testsuite is called `system userspace-testsuite` and can be launched
with `make check-system-userspace`.

v1 - v2:
* Dropped already applied commit.
* Renamed 'userspace-testsuite' to 'system-userspace-testsuite',
  'kmod-testsuite' to 'system-kmod-testsuite' and
  'make check-userspace' to 'make check-system-userspace'.
* Added new Vagrant provision target for running the system userspace
  testsuite.
* Added ON_EXIT to remove port in ADD_VETH.
* Introduced NS_EXEC and NS_CHECK_EXEC to abstract the namespace
  commands.
* Added mention in NEWS.

Daniele Di Proietto (4):
  tests: Rename kmod-testsuite to system-kmod-testsuite.
  tests: Introduce NS_EXEC and NS_CHECK_EXEC for system tests.
  tests: Add system-userspace-testsuite.
  Vagrantfile: Add test_ovs_system_userspace provision.

 INSTALL.md  |   7 +++
 NEWS|   3 +
 Vagrantfile |   6 ++
 tests/.gitignore|   3 +-
 tests/automake.mk   |  42 ++
 tests/kmod-macros.at|  35 
 tests/kmod-testsuite.at |  23 
 tests/system-common-macros.at   |  69 +++
 tests/system-kmod-macros.at |  40 ++
 tests/system-kmod-testsuite.at  |  23 
 tests/system-traffic.at | 107 
 tests/system-userspace-macros.at|  40 ++
 tests/system-userspace-testsuite.at |  23 
 tests/traffic-common-macros.at  |  53 --
 tests/traffic.at| 107 
 15 files changed, 350 insertions(+), 231 deletions(-)
 delete mode 100644 tests/kmod-macros.at
 delete mode 100644 tests/kmod-testsuite.at
 create mode 100644 tests/system-common-macros.at
 create mode 100644 tests/system-kmod-macros.at
 create mode 100644 tests/system-kmod-testsuite.at
 create mode 100644 tests/system-traffic.at
 create mode 100644 tests/system-userspace-macros.at
 create mode 100644 tests/system-userspace-testsuite.at
 delete mode 100644 tests/traffic-common-macros.at
 delete mode 100644 tests/traffic.at

-- 
2.1.4

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

Re: [ovs-dev] [ovn-controller-vtep V5 05/12] idl-loop: Move idl-loop into ovsdb-idl library.

[Russell Bryant]

On 08/07/2015 03:46 AM, Alex Wang wrote:
 idl-loop is needed in implementing other controller (i.e., vtep controller).
 So, this commit moves the logic into ovsdb-idl library module.
 
 Signed-off-by: Alex Wang al...@nicira.com

This looks fine to me.  I have a comment but it's not a blocker.  It's
up to you if you want to change it or not.

Acked-by: Russell Bryant rbry...@redhat.com

 diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h
 index a49f84f..28aa787 100644
 --- a/lib/ovsdb-idl.h
 +++ b/lib/ovsdb-idl.h
 @@ -222,5 +222,31 @@ const struct ovsdb_idl_row *ovsdb_idl_txn_insert(
  const struct uuid *);
  
  struct ovsdb_idl *ovsdb_idl_txn_get_idl (struct ovsdb_idl_txn *);
 +
 +
 +/* ovsdb_idl_loop provides an easy way to manage the transactions related
 + * to 'idl' and to cope with different status during transaction. */
 +struct ovsdb_idl_loop {
 +struct ovsdb_idl *idl;
 +unsigned int skip_seqno;
 +
 +struct ovsdb_idl_txn *committing_txn;
 +unsigned int precommit_seqno;
 +
 +struct ovsdb_idl_txn *open_txn;
 +};
 +
 +#define OVSDB_IDL_LOOP_INITIALIZER(IDL) { .idl = (IDL) }
 +
 +static inline void
 +ovsdb_idl_loop_init(struct ovsdb_idl_loop *loop, struct ovsdb_idl *idl)
 +{
 +memset(loop, 0, sizeof *loop);
 +loop-idl = idl;

Most of the code looks like an unmodified code move, but this function
appears to be new.  I guess you're using it later in the series?

It seems you could also write this as:

*loop = OVSDB_IDL_LOOP_INITIALIZER(idl);

or you could drop this function completely since this one line does the
same thing, right?

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

Re: [ovs-dev] [ovn-controller-vtep V5 07/12] ovsdb-idl: Move get_initial_snapshot() to ovsdb-idl.

[Russell Bryant]

On 08/07/2015 03:46 AM, Alex Wang wrote:
 The same function is defined in both ovn-controller.c and
 ovn-controller-vtep.c, so worth librarizing.
 
 Signed-off-by: Alex Wang al...@nicira.com

Acked-by: Russell Bryant rbry...@redhat.com

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

Re: [ovs-dev] [ovn-controller-vtep V5 08/12] ovn-controller-vtep: Add gateway module.

[Russell Bryant]

On 08/07/2015 03:46 AM, Alex Wang wrote:
 This commit adds the gateway module to ovn-controller-vtep.  The
 module will register the physical switches to ovnsb as chassis and
 constantly update the vtep_logical_switches column in Chassis table.
 
 Limitation (Recorded in TODO file):
 
 - Do not support reading multiple tunnel ips of physical switch.
 
 Signed-off-by: Alex Wang al...@nicira.com
 
 ---
 V4-V5:
 - rebase on top of master.
 - adopt Russell's review suggestions, greatly simplify the code.
 
 V3-V4:
 - rebase to master.
 
 V2-V3:
 - since ovn-sb schema changes (removal of Gateway table), the gateway
   module code needs to be adapted.
 - rebase to master.
 
 PATCH-V2:
 - split into separate commit.
 - can register all physical switches controlled by vtep database.
 ---
  ovn/TODO   |6 +
  ovn/controller-vtep/automake.mk|2 +
  ovn/controller-vtep/gateway.c  |  224 
 
  .../{ovn-controller-vtep.h = gateway.h}   |   19 +-
  ovn/controller-vtep/ovn-controller-vtep.c  |   43 +++-
  ovn/controller-vtep/ovn-controller-vtep.h  |   20 ++
  tests/ovn-controller-vtep.at   |  151 +
  7 files changed, 445 insertions(+), 20 deletions(-)
  create mode 100644 ovn/controller-vtep/gateway.c
  copy ovn/controller-vtep/{ovn-controller-vtep.h = gateway.h} (65%)
 
 diff --git a/ovn/TODO b/ovn/TODO
 index 509e5d0..356b3ba 100644
 --- a/ovn/TODO
 +++ b/ovn/TODO
 @@ -74,3 +74,9 @@
 Epstein et al., What's the Difference? Efficient Set
 Reconciliation Without Prior Context.  (I'm not yet aware of
 previous non-academic use of this technique.)
 +
 +** Support multiple tunnel encapsulations in Chassis.
 +
 +   So far, both ovn-controller and ovn-controller-vtep only allow
 +   chassis to have one tunnel encapsulation entry.  We should extend
 +   the implementation to support multiple tunnel encapsulations.
 diff --git a/ovn/controller-vtep/automake.mk b/ovn/controller-vtep/automake.mk
 index 7adda15..514cafa 100644
 --- a/ovn/controller-vtep/automake.mk
 +++ b/ovn/controller-vtep/automake.mk
 @@ -1,5 +1,7 @@
  bin_PROGRAMS += ovn/controller-vtep/ovn-controller-vtep
  ovn_controller_vtep_ovn_controller_vtep_SOURCES = \
 + ovn/controller-vtep/gateway.c \
 + ovn/controller-vtep/gateway.h \
   ovn/controller-vtep/ovn-controller-vtep.c \
   ovn/controller-vtep/ovn-controller-vtep.h
  ovn_controller_vtep_ovn_controller_vtep_LDADD = ovn/lib/libovn.la 
 lib/libopenvswitch.la vtep/libvtep.la
 diff --git a/ovn/controller-vtep/gateway.c b/ovn/controller-vtep/gateway.c
 new file mode 100644
 index 000..5f6da27
 --- /dev/null
 +++ b/ovn/controller-vtep/gateway.c
 @@ -0,0 +1,224 @@
 +/* Copyright (c) 2015 Nicira, Inc.
 + *
 + * Licensed under the Apache License, Version 2.0 (the License);
 + * you may not use this file except in compliance with the License.
 + * You may obtain a copy of the License at:
 + *
 + * http://www.apache.org/licenses/LICENSE-2.0
 + *
 + * Unless required by applicable law or agreed to in writing, software
 + * distributed under the License is distributed on an AS IS BASIS,
 + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 + * See the License for the specific language governing permissions and
 + * limitations under the License.
 + */
 +
 +#include config.h
 +#include gateway.h
 +
 +#include lib/poll-loop.h
 +#include lib/simap.h
 +#include lib/sset.h
 +#include lib/util.h
 +#include openvswitch/vlog.h
 +#include ovn/lib/ovn-sb-idl.h
 +#include vtep/vtep-idl.h
 +#include ovn-controller-vtep.h
 +
 +VLOG_DEFINE_THIS_MODULE(gateway);
 +
 +/*
 + * Registers the physical switches in vtep to ovnsb as chassis.  For each
 + * physical switch in the vtep database, finds all vtep logical switches that
 + * are associated with the physical switch, and updates the corresponding
 + * chassis's 'vtep_logical_switches' column.
 + *
 + */
 +
 +/* Global revalidation sequence number, incremented at each call to
 + * 'revalidate_gateway()'. */
 +static unsigned int gw_reval_seq;
 +
 +/* Maps all chassis created by the gateway module to their own reval_seq. */
 +static struct simap gw_chassis_map = SIMAP_INITIALIZER(gw_chassis_map);
 +
 +/* Creates and returns a new instance of 'struct sbrec_chassis'. */
 +static const struct sbrec_chassis *
 +create_chassis_rec(struct ovsdb_idl_txn *txn, const char *name,
 +   const char *encap_ip)
 +{
 +const struct sbrec_chassis *chassis_rec;
 +struct sbrec_encap *encap_rec;
 +
 +chassis_rec = sbrec_chassis_insert(txn);
 +sbrec_chassis_set_name(chassis_rec, name);
 +encap_rec = sbrec_encap_insert(txn);
 +sbrec_encap_set_type(encap_rec, OVN_SB_ENCAP_TYPE);
 +sbrec_encap_set_ip(encap_rec, encap_ip);
 +sbrec_chassis_set_encaps(chassis_rec, encap_rec, 1);
 +
 +return chassis_rec;
 +}
 +
 +/* Revalidates chassis in ovnsb 

Re: [ovs-dev] [RFC] dpdk: support multiple queues in vhost

[Thomas F Herbert]

On 8/6/15 5:29 PM, Flavio Leitner wrote:

On Thu, Aug 06, 2015 at 03:24:29PM -0400, Thomas F Herbert wrote:

On 8/6/15 1:40 PM, Flavio Leitner wrote:

On Thu, Aug 06, 2015 at 12:39:29PM -0400, Thomas F Herbert wrote:

On 7/31/15 6:30 PM, Flavio Leitner wrote:

This RFC is based on the vhost multiple queues work on
dpdk-dev: http://dpdk.org/ml/archives/dev/2015-June/019345.html

Signed-off-by: Flavio Leitner f...@redhat.com
---
  lib/netdev-dpdk.c | 61 ---
  1 file changed, 40 insertions(+), 21 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 5ae805e..493172c 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -215,12 +215,9 @@ struct netdev_dpdk {
   * If the numbers match, 'txq_needs_locking' is false, otherwise it is
   * true and we will take a spinlock on transmission */
  int real_n_txq;
+int real_n_rxq;
  bool txq_needs_locking;

-/* Spinlock for vhost transmission.  Other DPDK devices use spinlocks in
- * dpdk_tx_queue */
-rte_spinlock_t vhost_tx_lock;
-
  /* virtio-net structure for vhost device */
  OVSRCU_TYPE(struct virtio_net *) virtio_dev;

@@ -602,13 +599,10 @@ dpdk_dev_parse_name(const char dev_name[], const char 
prefix[],
  static int
  vhost_construct_helper(struct netdev *netdev_) OVS_REQUIRES(dpdk_mutex)
  {
-struct netdev_dpdk *netdev = netdev_dpdk_cast(netdev_);
-
  if (rte_eal_init_ret) {
  return rte_eal_init_ret;
  }

-rte_spinlock_init(netdev-vhost_tx_lock);
  return netdev_dpdk_init(netdev_, -1, DPDK_DEV_VHOST);
  }

@@ -791,9 +785,16 @@ netdev_dpdk_vhost_set_multiq(struct netdev *netdev_, 
unsigned int n_txq,
  ovs_mutex_lock(dpdk_mutex);
  ovs_mutex_lock(netdev-mutex);

+rte_free(netdev-tx_q);
+/* FIXME: the number of vqueues needs to match */

Do you still need this FIXME? Isn't the code you added below freeing and
re-allocating the correct number of tx queues?


Yes, because that is about virtual queues provided by qemu.
Thanks,
fbl
I understand this is an RFC but I think your patch is in the right 
direction. I know the merging is complex and requires upstream changes 
to DPDK and Qemu. I ack this patch is an important step that moves the 
ball forward toward vhost user performance of DPDK accelerated OVS.


--TFH



  netdev-up.n_txq = n_txq;
-netdev-real_n_txq = 1;
-netdev-up.n_rxq = 1;
+netdev-up.n_rxq = n_rxq;
+
+/* vring has txq = rxq */
+netdev-real_n_txq = n_rxq;
+netdev-real_n_rxq = n_rxq;
+netdev-txq_needs_locking = netdev-real_n_txq != netdev-up.n_txq;
+netdev_dpdk_alloc_txq(netdev, netdev-up.n_txq);

  ovs_mutex_unlock(netdev-mutex);
  ovs_mutex_unlock(dpdk_mutex);
@@ -904,14 +905,14 @@ netdev_dpdk_vhost_rxq_recv(struct netdev_rxq *rxq_,
  struct netdev *netdev = rx-up.netdev;
  struct netdev_dpdk *vhost_dev = netdev_dpdk_cast(netdev);
  struct virtio_net *virtio_dev = netdev_dpdk_get_virtio(vhost_dev);
-int qid = 1;
+int qid = rxq_-queue_id;
  uint16_t nb_rx = 0;

  if (OVS_UNLIKELY(!is_vhost_running(virtio_dev))) {
  return EAGAIN;
  }

-nb_rx = rte_vhost_dequeue_burst(virtio_dev, qid,
+nb_rx = rte_vhost_dequeue_burst(virtio_dev, VIRTIO_TXQ + qid * 2,
  vhost_dev-dpdk_mp-mp,
  (struct rte_mbuf **)packets,
  NETDEV_MAX_BURST);
@@ -958,8 +959,9 @@ netdev_dpdk_rxq_recv(struct netdev_rxq *rxq_, struct 
dp_packet **packets,
  }

  static void
-__netdev_dpdk_vhost_send(struct netdev *netdev, struct dp_packet **pkts,
- int cnt, bool may_steal)
+__netdev_dpdk_vhost_send(struct netdev *netdev, int qid,
+ struct dp_packet **pkts, int cnt,
+ bool may_steal)
  {
  struct netdev_dpdk *vhost_dev = netdev_dpdk_cast(netdev);
  struct virtio_net *virtio_dev = netdev_dpdk_get_virtio(vhost_dev);
@@ -974,13 +976,16 @@ __netdev_dpdk_vhost_send(struct netdev *netdev, struct 
dp_packet **pkts,
  goto out;
  }

-/* There is vHost TX single queue, So we need to lock it for TX. */
-rte_spinlock_lock(vhost_dev-vhost_tx_lock);
+if (vhost_dev-txq_needs_locking) {
+qid = qid % vhost_dev-real_n_txq;
+rte_spinlock_lock(vhost_dev-tx_q[qid].tx_lock);
+}

  do {
+int vhost_qid = VIRTIO_RXQ + qid * VIRTIO_QNUM;
  unsigned int tx_pkts;

-tx_pkts = rte_vhost_enqueue_burst(virtio_dev, VIRTIO_RXQ,
+tx_pkts = rte_vhost_enqueue_burst(virtio_dev, vhost_qid,
cur_pkts, cnt);
  if (OVS_LIKELY(tx_pkts)) {
  /* Packets have been sent.*/
@@ -999,7 +1004,7 @@ __netdev_dpdk_vhost_send(struct netdev *netdev, struct 
dp_packet **pkts,
   * Unable to enqueue packets to vhost interface.
   * Check available 

[ovs-dev] [PATCH] ovn-architecture: Document the registers used for logical ports.

[Justin Pettit]

When reviewing the OpenFlow flows generated by ovn-controller, it's nice
to have this information.

Signed-off-by: Justin Pettit jpet...@nicira.com
---
 ovn/ovn-architecture.7.xml |   17 +
 1 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/ovn/ovn-architecture.7.xml b/ovn/ovn-architecture.7.xml
index 1b537f9..2424836 100644
--- a/ovn/ovn-architecture.7.xml
+++ b/ovn/ovn-architecture.7.xml
@@ -627,18 +627,19 @@
 
 dtlogical input port field/dt
 dd
-  A field that denotes the logical port from which the packet entered the
-  logical datapath.  OVN stores this in a Nicira extension register.  (This
-  field is passed across tunnels as part of the tunnel key.)
+  A field that denotes the logical port from which the packet
+  entered the logical datapath.  OVN stores this in Nicira extension
+  register number 6.  (This field is passed across tunnels as part
+  of the tunnel key.)
 /dd
 
 dtlogical output port field/dt
 dd
-  A field that denotes the logical port from which the packet will leave
-  the logical datapath.  This is initialized to 0 at the beginning of the
-  logical ingress pipeline.  OVN stores this in a Nicira extension
-  register.  (This field is passed across tunnels as part of the tunnel
-  key.)
+  A field that denotes the logical port from which the packet will
+  leave the logical datapath.  This is initialized to 0 at the
+  beginning of the logical ingress pipeline.  OVN stores this in
+  Nicira extension register number 7.  (This field is passed across
+  tunnels as part of the tunnel key.)
 /dd
 
 dtVLAN ID/dt
-- 
1.7.5.4

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

[ovs-dev] [PATCH 0/5] Extend system tests to include tunnel.

[Joe Stringer]

Various minor fixups and extensions to the system tests, followed by a basic
tunnel sanity test for vxlan.

Joe Stringer (5):
  system-kmod-macros: Fix VSWITCHD_STOP.
  system-common-macros: Don't use bash to exec in ns.
  system-traffic: Check ping-by-ping output.
  system-macros: Create ADD_BR variant.
  kmod-traffic: Add basic vxlan tunnel sanity test.

 tests/system-common-macros.at| 52 +-
 tests/system-kmod-macros.at  | 18 ++---
 tests/system-traffic.at  | 80 
 tests/system-userspace-macros.at |  6 +--
 4 files changed, 123 insertions(+), 33 deletions(-)

-- 
2.1.4

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

[ovs-dev] [PATCH 2/5] system-common-macros: Don't use bash to exec in ns.

[Joe Stringer]

ip netns exec $namespace $command doesn't need to use bash to execute
the command. Remove it.

Signed-off-by: Joe Stringer joestrin...@nicira.com
---
 tests/system-common-macros.at | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at
index 1321e58..fdc2bc7 100644
--- a/tests/system-common-macros.at
+++ b/tests/system-common-macros.at
@@ -25,7 +25,7 @@ m4_define([ADD_NAMESPACES],
 #
 # Execute 'command' in 'namespace'
 m4_define([NS_EXEC],
-[ip netns exec $1 bash -c $2]
+[ip netns exec $1 $2]
 )
 
 # NS_CHECK_EXEC([namespace], [command], other_params...)
-- 
2.1.4

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

[ovs-dev] [PATCH 3/5] system-traffic: Check ping-by-ping output.

[Joe Stringer]

Rather than saving all of the ping output to a file then checking at the
end, check each ping and fail as soon as there is a connectivity
failure.

Signed-off-by: Joe Stringer joestrin...@nicira.com
---
 tests/system-common-macros.at |  6 ++
 tests/system-traffic.at   | 44 ---
 2 files changed, 26 insertions(+), 24 deletions(-)

diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at
index fdc2bc7..7f243f7 100644
--- a/tests/system-common-macros.at
+++ b/tests/system-common-macros.at
@@ -67,3 +67,9 @@ m4_define([ADD_VLAN],
   NS_CHECK_EXEC([$2], [ip addr add dev $1.$3 $4])
 ]
 )
+
+# FORMAT_PING([])
+#
+# Strip variant pieces from ping output so the output can be reliably compared.
+#
+m4_define([FORMAT_PING], [grep transmitted | sed 's/time.*ms$/time 0ms/'])
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 1ff2286..8324480 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -9,14 +9,13 @@ ADD_NAMESPACES(at_ns0, at_ns1)
 ADD_VETH(p0, at_ns0, br0, 10.1.1.1/24)
 ADD_VETH(p1, at_ns1, br0, 10.1.1.2/24)
 
-AT_CAPTURE_FILE([ping.output])
-NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.2  ping.output])
-NS_CHECK_EXEC([at_ns0], [ping -s 1600 -q -c 3 -i 0.3 -w 2 10.1.1.2  
ping.output])
-NS_CHECK_EXEC([at_ns0], [ping -s 3200 -q -c 3 -i 0.3 -w 2 10.1.1.2  
ping.output])
-
-AT_CHECK([cat ping.output | grep transmitted | sed 's/time.*ms$/time 0ms/'], 
[0], [dnl
+NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.2 | FORMAT_PING], 
[0], [dnl
 3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+NS_CHECK_EXEC([at_ns0], [ping -s 1600 -q -c 3 -i 0.3 -w 2 10.1.1.2 | 
FORMAT_PING], [0], [dnl
 3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+NS_CHECK_EXEC([at_ns0], [ping -s 3200 -q -c 3 -i 0.3 -w 2 10.1.1.2 | 
FORMAT_PING], [0], [dnl
 3 packets transmitted, 3 received, 0% packet loss, time 0ms
 ])
 
@@ -35,14 +34,13 @@ ADD_VETH(p1, at_ns1, br0, 10.1.1.2/24)
 ADD_VLAN(p0, at_ns0, 100, 10.2.2.1/24)
 ADD_VLAN(p1, at_ns1, 100, 10.2.2.2/24)
 
-AT_CAPTURE_FILE([ping.output])
-NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.2.2.2  ping.output])
-NS_CHECK_EXEC([at_ns0], [ping -s 1600 -q -c 3 -i 0.3 -w 2 10.2.2.2  
ping.output])
-NS_CHECK_EXEC([at_ns0], [ping -s 3200 -q -c 3 -i 0.3 -w 2 10.2.2.2  
ping.output])
-
-AT_CHECK([cat ping.output | grep transmitted | sed 's/time.*ms$/time 0ms/'], 
[0], [dnl
+NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.2.2.2 | FORMAT_PING], 
[0], [dnl
 3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+NS_CHECK_EXEC([at_ns0], [ping -s 1600 -q -c 3 -i 0.3 -w 2 10.2.2.2 | 
FORMAT_PING], [0], [dnl
 3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+NS_CHECK_EXEC([at_ns0], [ping -s 3200 -q -c 3 -i 0.3 -w 2 10.2.2.2 | 
FORMAT_PING], [0], [dnl
 3 packets transmitted, 3 received, 0% packet loss, time 0ms
 ])
 
@@ -62,14 +60,13 @@ dnl Without this sleep, we get occasional failures due to 
the following error:
 dnl connect: Cannot assign requested address
 sleep 2;
 
-AT_CAPTURE_FILE([ping.output])
-NS_CHECK_EXEC([at_ns0], [ping6 -q -c 3 -i 0.3 -w 2 fc00::2  ping.output])
-NS_CHECK_EXEC([at_ns0], [ping6 -s 1600 -q -c 3 -i 0.3 -w 2 fc00::2  
ping.output])
-NS_CHECK_EXEC([at_ns0], [ping6 -s 3200 -q -c 3 -i 0.3 -w 2 fc00::2  
ping.output])
-
-AT_CHECK([cat ping.output | grep transmitted | sed 's/time.*ms$/time 0ms/'], 
[0], [dnl
+NS_CHECK_EXEC([at_ns0], [ping6 -q -c 3 -i 0.3 -w 2 fc00::2 | FORMAT_PING], 
[0], [dnl
 3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+NS_CHECK_EXEC([at_ns0], [ping6 -s 1600 -q -c 3 -i 0.3 -w 2 fc00::2 | 
FORMAT_PING], [0], [dnl
 3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+NS_CHECK_EXEC([at_ns0], [ping6 -s 3200 -q -c 3 -i 0.3 -w 2 fc00::2 | 
FORMAT_PING], [0], [dnl
 3 packets transmitted, 3 received, 0% packet loss, time 0ms
 ])
 
@@ -92,14 +89,13 @@ dnl Without this sleep, we get occasional failures due to 
the following error:
 dnl connect: Cannot assign requested address
 sleep 2;
 
-AT_CAPTURE_FILE([ping.output])
-NS_CHECK_EXEC([at_ns0], [ping6 -q -c 3 -i 0.3 -w 2 fc00:1::2  ping.output])
-NS_CHECK_EXEC([at_ns0], [ping6 -s 1600 -q -c 3 -i 0.3 -w 2 fc00:1::2  
ping.output])
-NS_CHECK_EXEC([at_ns0], [ping6 -s 3200 -q -c 3 -i 0.3 -w 2 fc00:1::2  
ping.output])
-
-AT_CHECK([cat ping.output | grep transmitted | sed 's/time.*ms$/time 0ms/'], 
[0], [dnl
+NS_CHECK_EXEC([at_ns0], [ping6 -q -c 3 -i 0.3 -w 2 fc00:1::2 | FORMAT_PING], 
[0], [dnl
 3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+NS_CHECK_EXEC([at_ns0], [ping6 -s 1600 -q -c 3 -i 0.3 -w 2 fc00:1::2 | 
FORMAT_PING], [0], [dnl
 3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+NS_CHECK_EXEC([at_ns0], [ping6 -s 3200 -q -c 3 -i 0.3 -w 2 fc00:1::2 | 
FORMAT_PING], [0], [dnl
 3 packets transmitted, 3 received, 0% packet loss, time 0ms
 ])
 
-- 
2.1.4

[ovs-dev] [PATCH 1/5] system-kmod-macros: Fix VSWITCHD_STOP.

[Joe Stringer]

This was renamed. Surprisingly, the tests still pass without this,
however the extra checks that this command performs were not executed.
Fix the macro definition.

Signed-off-by: Joe Stringer joestrin...@nicira.com
---
 tests/system-kmod-macros.at | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/system-kmod-macros.at b/tests/system-kmod-macros.at
index eacdc5d..3f94504 100644
--- a/tests/system-kmod-macros.at
+++ b/tests/system-kmod-macros.at
@@ -33,7 +33,7 @@ m4_define([OVS_TRAFFIC_VSWITCHD_START],
 # 'extra_cmds' are shell commands to be executed afte OVS_VSWITCHD_STOP() is
 # invoked. They can be used to perform additional cleanups such as name space
 # removal.
-m4_define([OVS_KMOD_VSWITCHD_STOP],
+m4_define([OVS_TRAFFIC_VSWITCHD_STOP],
   [AT_CHECK([ovs-vsctl del-br br0])
OVS_VSWITCHD_STOP([$1])
AT_CHECK([:; $2])
-- 
2.1.4

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

[ovs-dev] [PATCH 4/5] system-macros: Create ADD_BR variant.

[Joe Stringer]

This patch splits ADD_BR into two commands, so they can be used from
different contexts:

ADD_BR(...) is a standalone command to add a bridge to OVS, and allows
additional ovs-vsctl arguments to be passed. It uses _ADD_BR().
_ADD_BR(...) is the implementation-specific ovs-vsctl arguments to
set up the correct datapath type for userspace or kmod tests.

Signed-off-by: Joe Stringer joestrin...@nicira.com
---
 tests/system-common-macros.at| 6 ++
 tests/system-kmod-macros.at  | 6 +++---
 tests/system-userspace-macros.at | 6 +++---
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at
index 7f243f7..45e59e5 100644
--- a/tests/system-common-macros.at
+++ b/tests/system-common-macros.at
@@ -36,6 +36,12 @@ m4_define([NS_CHECK_EXEC],
 [ AT_CHECK([NS_EXEC([$1], [$2])], m4_shift(m4_shift($@))) ]
 )
 
+# ADD_BR([name], [vsctl-args])
+#
+# Expands into the proper ovs-vsctl commands to create a bridge with the
+# appropriate type, and allows additional arguments to be passed.
+m4_define([ADD_BR], [ovs-vsctl _ADD_BR([$1]) -- $2])
+
 # ADD_VETH([port], [namespace], [ovs-br], [ip_addr])
 #
 # Add a pair of veth ports. 'port' will be added to name space 'namespace',
diff --git a/tests/system-kmod-macros.at b/tests/system-kmod-macros.at
index 3f94504..a5aa5db 100644
--- a/tests/system-kmod-macros.at
+++ b/tests/system-kmod-macros.at
@@ -1,8 +1,8 @@
-# ADD_BR([name])
+# _ADD_BR([name])
 #
 # Expands into the proper ovs-vsctl commands to create a bridge with the
 # appropriate type
-m4_define([ADD_BR], [[add-br $1]])
+m4_define([_ADD_BR], [[add-br $1]])
 
 # OVS_TRAFFIC_VSWITCHD_START([vsctl-args], [vsctl-output], [=override])
 #
@@ -18,7 +18,7 @@ m4_define([OVS_TRAFFIC_VSWITCHD_START],
 ON_EXIT([modprobe -r openvswitch])
_OVS_VSWITCHD_START([])
dnl Add bridges, ports, etc.
-   AT_CHECK([ovs-vsctl -- ADD_BR([br0]) -- set bridge br0 
protocols=[[OpenFlow10,OpenFlow11,OpenFlow12,OpenFlow13,OpenFlow14,OpenFlow15]] 
fail-mode=secure -- $1 m4_if([$2], [], [], [| ${PERL} $srcdir/uuidfilt.pl])], 
[0], [$2])
+   AT_CHECK([ovs-vsctl -- _ADD_BR([br0]) -- set bridge br0 
protocols=[[OpenFlow10,OpenFlow11,OpenFlow12,OpenFlow13,OpenFlow14,OpenFlow15]] 
fail-mode=secure -- $1 m4_if([$2], [], [], [| ${PERL} $srcdir/uuidfilt.pl])], 
[0], [$2])
 ])
 
 # OVS_TRAFFIC_VSWITCHD_STOP([WHITELIST], [extra_cmds])
diff --git a/tests/system-userspace-macros.at b/tests/system-userspace-macros.at
index b273576..adc40c4 100644
--- a/tests/system-userspace-macros.at
+++ b/tests/system-userspace-macros.at
@@ -1,8 +1,8 @@
-# ADD_BR([name])
+# _ADD_BR([name])
 #
 # Expands into the proper ovs-vsctl commands to create a bridge with the
 # appropriate type
-m4_define([ADD_BR], [[add-br $1 -- set Bridge $1 datapath_type=netdev ]])
+m4_define([_ADD_BR], [[add-br $1 -- set Bridge $1 datapath_type=netdev ]])
 
 # OVS_TRAFFIC_VSWITCHD_START([vsctl-args], [vsctl-output], [=override])
 #
@@ -16,7 +16,7 @@ m4_define([OVS_TRAFFIC_VSWITCHD_START],
   [
_OVS_VSWITCHD_START([--disable-system])
dnl Add bridges, ports, etc.
-   AT_CHECK([ovs-vsctl -- ADD_BR([br0]) -- set bridge br0 
protocols=[[OpenFlow10,OpenFlow11,OpenFlow12,OpenFlow13,OpenFlow14,OpenFlow15]] 
fail-mode=secure -- $1 m4_if([$2], [], [], [| ${PERL} $srcdir/uuidfilt.pl])], 
[0], [$2])
+   AT_CHECK([ovs-vsctl -- _ADD_BR([br0]) -- set bridge br0 
protocols=[[OpenFlow10,OpenFlow11,OpenFlow12,OpenFlow13,OpenFlow14,OpenFlow15]] 
fail-mode=secure -- $1 m4_if([$2], [], [], [| ${PERL} $srcdir/uuidfilt.pl])], 
[0], [$2])
 ])
 
 # OVS_TRAFFIC_VSWITCHD_STOP([WHITELIST], [extra_cmds])
-- 
2.1.4

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

[ovs-dev] [PATCH 5/5] kmod-traffic: Add basic vxlan tunnel sanity test.

[Joe Stringer]

Signed-off-by: Joe Stringer joestrin...@nicira.com
---
 tests/system-common-macros.at | 38 ++
 tests/system-kmod-macros.at   | 10 --
 tests/system-traffic.at   | 38 ++
 3 files changed, 84 insertions(+), 2 deletions(-)

diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at
index 45e59e5..46629ac 100644
--- a/tests/system-common-macros.at
+++ b/tests/system-common-macros.at
@@ -74,6 +74,44 @@ m4_define([ADD_VLAN],
 ]
 )
 
+# ADD_OVS_TUNNEL([type], [bridge], [port], [remote-addr], [overlay-addr])
+#
+# Add an ovs-based tunnel device in the root namespace, with name 'port' and
+# type 'type'. The tunnel device will be configured as point-to-point with the
+# 'remote-addr' as the underlay address of the remote tunnel endpoint.
+#
+# 'port will be configured with the address 'overlay-addr'.
+#
+m4_define([ADD_OVS_TUNNEL],
+   [AT_CHECK([ovs-vsctl add-port $2 $3 -- \
+  set int $3 type=$1 options:remote_ip=$4])
+AT_CHECK([ip addr add dev $2 $5])
+AT_CHECK([ip link set dev $2 up])
+AT_CHECK([ip link set dev $2 mtu 1450])
+ON_EXIT([ip addr del dev $2 $5])
+   ]
+)
+
+# ADD_NATIVE_TUNNEL([type], [port], [namespace], [remote-addr], [overlay-addr],
+#   [link-args])
+#
+# Add a native tunnel device within 'namespace', with name 'port' and type
+# 'type'. The tunnel device will be configured as point-to-point with the
+# 'remote-addr' as the underlay address of the remote tunnel endpoint (as
+# viewed from the perspective of that namespace).
+#
+# 'port' will be configured with the address 'overlay-addr'. 'link-args' is
+# made available so that additional arguments can be passed to ip link,
+# for instance to configure the vxlan destination port.
+#
+m4_define([ADD_NATIVE_TUNNEL],
+   [NS_CHECK_EXEC([$3], [ip link add dev $2 type $1 remote $4 $6])
+NS_CHECK_EXEC([$3], [ip addr add dev $2 $5])
+NS_CHECK_EXEC([$3], [ip link set dev $2 up])
+NS_CHECK_EXEC([$3], [ip link set dev $2 mtu 1450])
+   ]
+)
+
 # FORMAT_PING([])
 #
 # Strip variant pieces from ping output so the output can be reliably compared.
diff --git a/tests/system-kmod-macros.at b/tests/system-kmod-macros.at
index a5aa5db..fbe9220 100644
--- a/tests/system-kmod-macros.at
+++ b/tests/system-kmod-macros.at
@@ -13,9 +13,15 @@ m4_define([_ADD_BR], [[add-br $1]])
 # output (e.g. because it includes create commands) then 'vsctl-output'
 # specifies the expected output after filtering through uuidfilt.pl.
 #
+# Best-effort loading of all available vport modules is performed.
+#
 m4_define([OVS_TRAFFIC_VSWITCHD_START],
-  [ AT_CHECK([modprobe openvswitch])
-ON_EXIT([modprobe -r openvswitch])
+  [AT_CHECK([modprobe openvswitch])
+   ON_EXIT([modprobe -r openvswitch])
+   m4_foreach([mod], [[vport_geneve], [vport_gre], [vport_lisp], [vport_stt], 
[vport_vxlan]],
+  [modprobe -q mod || echo Module mod not loaded.
+   ON_EXIT([modprobe -q -r mod])])
+   ON_EXIT([ovs-dpctl del-dp ovs-system])
_OVS_VSWITCHD_START([])
dnl Add bridges, ports, etc.
AT_CHECK([ovs-vsctl -- _ADD_BR([br0]) -- set bridge br0 
protocols=[[OpenFlow10,OpenFlow11,OpenFlow12,OpenFlow13,OpenFlow14,OpenFlow15]] 
fail-mode=secure -- $1 m4_if([$2], [], [], [| ${PERL} $srcdir/uuidfilt.pl])], 
[0], [$2])
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 8324480..e99cd80 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -101,3 +101,41 @@ NS_CHECK_EXEC([at_ns0], [ping6 -s 3200 -q -c 3 -i 0.3 -w 2 
fc00:1::2 | FORMAT_PI
 
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
+
+AT_SETUP([kmod - ping over vxlan tunnel])
+AT_SKIP_IF([! ip link help 21 | grep vxlan /dev/null])
+
+OVS_TRAFFIC_VSWITCHD_START(
+   [set-fail-mode br0 standalone -- ])
+ADD_BR([br-underlay], [set-fail-mode br-underlay standalone])
+ADD_NAMESPACES(at_ns0)
+
+dnl Set up underlay link from host into the namespace using veth pair.
+ADD_VETH(p0, at_ns0, br-underlay, 172.31.1.1/24)
+AT_CHECK([ip addr add dev br-underlay 172.31.1.100/24])
+AT_CHECK([ip link set dev br-underlay up])
+
+dnl Set up tunnel endpoints on OVS outside the namespace and with a native
+dnl linux device inside the namespace.
+ADD_OVS_TUNNEL([vxlan], [br0], [at_vxlan0], [172.31.1.1], [10.1.1.100/24])
+ADD_NATIVE_TUNNEL([vxlan], [at_vxlan1], [at_ns0], [172.31.1.100], 
[10.1.1.1/24],
+  [id 0 dstport 4789])
+
+dnl First, check the underlay
+NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 172.31.1.100 | FORMAT_PING], 
[0], [dnl
+3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+
+dnl Okay, now check the overlay with different packet sizes
+NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.100 | FORMAT_PING], 
[0], [dnl
+3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+NS_CHECK_EXEC([at_ns0], [ping -s 1600 -q -c 3 -i 0.3 -w 2 10.1.1.100 | 
FORMAT_PING], [0], [dnl
+3 packets transmitted, 3 

Re: [ovs-dev] [ovn-controller-vtep V5 02/12] ovn-nbctl: Move ovn-nbctl to utilities directory.

[Alex Wang]

On Fri, Aug 7, 2015 at 10:23 AM, Russell Bryant rbry...@redhat.com wrote:

 On 08/07/2015 01:18 PM, Russell Bryant wrote:
  On 08/07/2015 03:46 AM, Alex Wang wrote:
  Signed-off-by: Alex Wang al...@nicira.com
  ---
  V5:
  - new patch.
  ---
   ovn/.gitignore  |2 --
   ovn/automake.mk |   11 +++
   ovn/utilities/.gitignore|3 +++
   ovn/utilities/automake.mk   |   14 +++---
   ovn/{ = utilities}/ovn-nbctl.8.xml |0
   ovn/{ = utilities}/ovn-nbctl.c |0
   6 files changed, 17 insertions(+), 13 deletions(-)
   rename ovn/{ = utilities}/ovn-nbctl.8.xml (100%)
   rename ovn/{ = utilities}/ovn-nbctl.c (100%)
 
  diff --git a/ovn/utilities/.gitignore b/ovn/utilities/.gitignore
  index 1ddc63e..714aaab 100644
  --- a/ovn/utilities/.gitignore
  +++ b/ovn/utilities/.gitignore
  @@ -1 +1,4 @@
   /ovn-ctl.8
  +/ovn-nbctl
  +/ovn-nbctl.8
  +
 
  git complains that this patch introduces a whitespace error.  It's this
  new blank line at the end of .gitignore.
 
  Otherwise:
 
  Acked-by: Russell Bryant rbry...@redhat.com
 

 One more thing ... this can be fixed later if needed, but ovn/utilities/
 will need to be added to PATH in tutorial/ovs-sandbox.  After this
 patch, ovn-nbctl (and later ovn-sbctl) aren't in $PATH when running the
 sandbox.


Thx for pointing this out~!  Totally unaware,

Will also add it to utilities/ovs-sim.in



 --
 Russell Bryant

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

[ovs-dev] [PATCH v4 2/9] flow: Avoid compile errors.

[Jarno Rajahalme]

GCC (4.7) sees too wide shifts when there are none, refactor to
circumvent the false error.

Signed-off-by: Jarno Rajahalme jrajaha...@nicira.com
---
 lib/flow.c | 35 ++-
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/lib/flow.c b/lib/flow.c
index af51aac..61d9bdf 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -133,25 +133,34 @@ BUILD_MESSAGE(FLOW_WC_SEQ changed: miniflow_extract() 
will have runtime 
 #endif
 
 #define miniflow_set_map(MF, OFS)   \
-if ((OFS)  FLOW_TNL_U64S) {\
-MINIFLOW_ASSERT(!(MF.maps.tnl_map  (UINT64_MAX  (OFS)))  \
+{   \
+size_t ofs = (OFS); \
+\
+if (ofs  FLOW_TNL_U64S) {  \
+MINIFLOW_ASSERT(!(MF.maps.tnl_map  (UINT64_MAX  ofs))\
  !MF.maps.pkt_map);   \
-MF.maps.tnl_map |= UINT64_C(1)  (OFS);\
+MF.maps.tnl_map |= UINT64_C(1)  ofs;  \
 } else {\
-MINIFLOW_ASSERT(!(MF.maps.pkt_map   \
-   UINT64_MAX  ((OFS) - FLOW_TNL_U64S)));\
-MF.maps.pkt_map |= UINT64_C(1)  ((OFS) - FLOW_TNL_U64S);  \
-}
+ofs -= FLOW_TNL_U64S;   \
+MINIFLOW_ASSERT(!(MF.maps.pkt_map  (UINT64_MAX  ofs)));  \
+MF.maps.pkt_map |= UINT64_C(1)  ofs;  \
+}   \
+}
 
 #define miniflow_assert_in_map(MF, OFS) \
-if ((OFS)  FLOW_TNL_U64S) {\
-MINIFLOW_ASSERT(MF.maps.tnl_map  UINT64_C(1)  (OFS)  \
- !(MF.maps.tnl_map  UINT64_MAX  ((OFS) + 1)) \
+{   \
+size_t ofs = (OFS); \
+\
+if (ofs  FLOW_TNL_U64S) {  \
+MINIFLOW_ASSERT(MF.maps.tnl_map  UINT64_C(1)  ofs\
+ !(MF.maps.tnl_map  UINT64_MAX  (ofs + 1)) \
  !MF.maps.pkt_map);   \
 } else {\
-MINIFLOW_ASSERT(MF.maps.pkt_map  UINT64_C(1)  ((OFS) - 
FLOW_TNL_U64S) \
- !(MF.maps.pkt_map  UINT64_MAX  ((OFS) - 
FLOW_TNL_U64S + 1))); \
-}
+ofs -= FLOW_TNL_U64S;   \
+MINIFLOW_ASSERT(MF.maps.pkt_map  UINT64_C(1)  ofs\
+ !(MF.maps.pkt_map  UINT64_MAX  (ofs + 1))); \
+}   \
+}
 
 #define miniflow_push_uint64_(MF, OFS, VALUE)   \
 {   \
-- 
2.1.4

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

[ovs-dev] [PATCH v4 1/9] classifier: Fix comment.

[Jarno Rajahalme]

Signed-off-by: Jarno Rajahalme jrajaha...@nicira.com
---
 lib/classifier.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/lib/classifier.c b/lib/classifier.c
index fc6a1f5..0a6855e 100644
--- a/lib/classifier.c
+++ b/lib/classifier.c
@@ -283,11 +283,10 @@ cls_rule_is_catchall(const struct cls_rule *rule)
 return minimask_is_catchall(rule-match.mask);
 }
 
-/* Makes rule invisible after 'version'.  Once that version is made invisible
- * (by changing the version parameter used in lookups), the rule should be
- * actually removed via ovsrcu_postpone().
+/* Makes 'rule' invisible in 'remove_version'.  Once that version is used in
+ * lookups, the caller should remove 'rule' via ovsrcu_postpone().
  *
- * 'rule_' must be in a classifier. */
+ * 'rule' must be in a classifier. */
 void
 cls_rule_make_invisible_in_version(const struct cls_rule *rule,
cls_version_t remove_version)
-- 
2.1.4

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

[ovs-dev] [PATCH v4 4/9] classifier: Pre-compute stage masks.

[Jarno Rajahalme]

This makes stage mask computation happen only when a subtable is
inserted and allows simplification of the main lookup function.

Classifier benchmark shows that this speeds up the classification
(with wildcards) about 5%.

Signed-off-by: Jarno Rajahalme jrajaha...@nicira.com
---
 lib/classifier-private.h | 107 +-
 lib/classifier.c | 168 ---
 lib/flow.c   |   2 +-
 lib/flow.h   |  10 ++-
 4 files changed, 139 insertions(+), 148 deletions(-)

diff --git a/lib/classifier-private.h b/lib/classifier-private.h
index 3a150ab..cc255fe 100644
--- a/lib/classifier-private.h
+++ b/lib/classifier-private.h
@@ -42,7 +42,7 @@ struct cls_subtable {
 /* These fields are accessed by readers who care about wildcarding. */
 const tag_type tag;   /* Tag generated from mask for partitioning. */
 const uint8_t n_indices;   /* How many indices to use. */
-const uint8_t index_ofs[CLS_MAX_INDICES];  /* u64 segment boundaries. */
+const struct miniflow index_maps[CLS_MAX_INDICES + 1]; /* Stage maps. */
 unsigned int trie_plen[CLS_MAX_TRIES];  /* Trie prefix length in 'mask'
  * (runtime configurable). */
 const int ports_mask_len;
@@ -226,54 +226,6 @@ struct trie_node {
  * These are only used by the classifier, so place them here to allow
  * for better optimization. */
 
-/* Initializes 'map-tnl_map' and 'map-pkt_map' with a subset of 'miniflow'
- * that includes only the portions with u64-offset 'i' such that start = i 
- * end.  Does not copy any data from 'miniflow' to 'map'.
- *
- * TODO: Ensure that 'start' and 'end' are compile-time constants. */
-static inline unsigned int /* offset */
-miniflow_get_map_in_range(const struct miniflow *miniflow,
-  uint8_t start, uint8_t end, struct miniflow *map)
-{
-unsigned int offset = 0;
-
-map-tnl_map = miniflow-tnl_map;
-map-pkt_map = miniflow-pkt_map;
-
-if (start = FLOW_TNL_U64S) {
-offset += count_1bits(map-tnl_map);
-map-tnl_map = 0;
-if (start  FLOW_TNL_U64S) {
-/* Clear 'start - FLOW_TNL_U64S' LSBs from pkt_map. */
-start -= FLOW_TNL_U64S;
-uint64_t msk = (UINT64_C(1)  start) - 1;
-
-offset += count_1bits(map-pkt_map  msk);
-map-pkt_map = ~msk;
-}
-} else if (start  0) {
-/* Clear 'start' LSBs from tnl_map. */
-uint64_t msk = (UINT64_C(1)  start) - 1;
-
-offset += count_1bits(map-tnl_map  msk);
-map-tnl_map = ~msk;
-}
-
-if (end = FLOW_TNL_U64S) {
-map-pkt_map = 0;
-if (end  FLOW_TNL_U64S) {
-/* Keep 'end' LSBs in tnl_map. */
-map-tnl_map = (UINT64_C(1)  end) - 1;
-}
-} else {
-if (end  FLOW_U64S) {
-/* Keep 'end - FLOW_TNL_U64S' LSBs in pkt_map. */
-map-pkt_map = (UINT64_C(1)  (end - FLOW_TNL_U64S)) - 1;
-}
-}
-return offset;
-}
-
 /* Returns a hash value for the bits of 'flow' where there are 1-bits in
  * 'mask', given 'basis'.
  *
@@ -333,28 +285,29 @@ miniflow_hash_in_minimask(const struct miniflow *flow,
 static inline uint32_t
 flow_hash_in_minimask_range(const struct flow *flow,
 const struct minimask *mask,
-uint8_t start, uint8_t end, uint32_t *basis)
+const struct miniflow *maps,
+unsigned int *offset,
+uint32_t *basis)
 {
 const uint64_t *mask_values = miniflow_get_values(mask-masks);
+const uint64_t *p = mask_values + *offset;
 const uint64_t *flow_u64 = (const uint64_t *)flow;
-unsigned int offset;
-struct miniflow map;
-const uint64_t *p;
 uint32_t hash = *basis;
 size_t idx;
 
-offset = miniflow_get_map_in_range(mask-masks, start, end, map);
-p = mask_values + offset;
-MAP_FOR_EACH_INDEX(idx, map.tnl_map) {
-hash = hash_add64(hash, flow_u64[idx]  *p++);
+if (OVS_UNLIKELY(maps-tnl_map)) {
+MAP_FOR_EACH_INDEX(idx, maps-tnl_map) {
+hash = hash_add64(hash, flow_u64[idx]  *p++);
+}
 }
 flow_u64 += FLOW_TNL_U64S;
-MAP_FOR_EACH_INDEX(idx, map.pkt_map) {
+MAP_FOR_EACH_INDEX(idx, maps-pkt_map) {
 hash = hash_add64(hash, flow_u64[idx]  *p++);
 }
 
 *basis = hash; /* Allow continuation from the unfinished value. */
-return hash_finish(hash, (p - mask_values) * 8);
+*offset = p - mask_values;
+return hash_finish(hash, *offset * 8);
 }
 
 /* Fold minimask 'mask''s wildcard mask into 'wc's wildcard mask. */
@@ -365,24 +318,25 @@ flow_wildcards_fold_minimask(struct flow_wildcards *wc,
 flow_union_with_miniflow(wc-masks, mask-masks);
 }
 
-/* Fold minimask 'mask''s wildcard mask into 'wc's wildcard mask
- * in range [start, end). */
+/* Fold 

[ovs-dev] [PATCH v4 3/9] test-classifier: Add benchmark.

[Jarno Rajahalme]

Add a benchmark command for classifier lookup performance testing.

Usage:
ovstest test-classifier benchmark n_rules n_priorities n_subtables 
n_threads n_lookups

Where:

n_rules  - The number of rules to install for lookups.  More rules
 makes misses less likely.

n_priorities - How many different priorities to use.  Using only 1
 priority will force lookups to continue through all
 subtables.

n_subtables  - Number of subtables to use.  Normally a classifier has
 rules with different kinds of masks, resulting in
 multiple subtables (one per mask).  However, in some
 special cases a table may consist of only one kind of
 rules, so there will be only one subtable.

n_threads- How many lookup threads to use.  Using one thread should
 give less variance accross runs, but classifier
 scaling can be tested with multiple threads.

n_lookups- How many lookups each thread should perform.


For testing the classifier is filled with n_rules rules using
n_subtables different mask patterns and n_priorities different
priorities.  A random set of lookup flows are created, and n_threads
lookup threads are spawned to perform n_lookups lookups each.  The
count of hits and misses, as well as the overall execution time is
reported.

Example run:

$ tests/ovstest test-classifier benchmark 1000 30 30 1 380

Benchmarking with:
1000 rules with 30 prioritites in 30 tables, 1 threads doing 380 lookups 
each

Without wildcards:

hits: 442705, misses: 3357295
classifier lookups:408 ms, 9313725 lookups/sec

With wildcards:

hits: 442705, misses: 3357295
classifier lookups:995 ms, 3819095 lookups/sec
$

Signed-off-by: Jarno Rajahalme jrajaha...@nicira.com
---
 tests/test-classifier.c | 198 
 1 file changed, 198 insertions(+)

diff --git a/tests/test-classifier.c b/tests/test-classifier.c
index 6e5b36b..2462aed 100644
--- a/tests/test-classifier.c
+++ b/tests/test-classifier.c
@@ -34,11 +34,15 @@
 #include byte-order.h
 #include classifier-private.h
 #include command-line.h
+#include fatal-signal.h
 #include flow.h
 #include ofp-util.h
 #include ovstest.h
+#include ovs-atomic.h
+#include ovs-thread.h
 #include packets.h
 #include random.h
+#include timeval.h
 #include unaligned.h
 #include util.h
 
@@ -1269,6 +1273,199 @@ test_many_rules_in_five_tables(struct ovs_cmdl_context 
*ctx OVS_UNUSED)
 test_many_rules_in_n_tables(5);
 }
 
+/* Classifier benchmarks. */
+
+static int n_rules; /* Number of rules to insert. */
+static int n_priorities;/* Number of priorities to use. */
+static int n_tables;/* Number of subtables. */
+static int n_threads;   /* Number of threads to search and mutate. */
+static int n_lookups;   /* Number of lookups each thread performs. */
+
+static void benchmark(bool use_wc);
+
+static int
+elapsed(const struct timeval *start)
+{
+struct timeval end;
+
+xgettimeofday(end);
+return timeval_to_msec(end) - timeval_to_msec(start);
+}
+
+static void
+run_benchmarks(struct ovs_cmdl_context *ctx)
+{
+n_rules = strtol(ctx-argv[1], NULL, 10);
+n_priorities = strtol(ctx-argv[2], NULL, 10);
+n_tables = strtol(ctx-argv[3], NULL, 10);
+n_threads = strtol(ctx-argv[4], NULL, 10);
+n_lookups = strtol(ctx-argv[5], NULL, 10);
+
+printf(\nBenchmarking with:\n
+   %d rules with %d prioritites in %d tables, 
+   %d threads doing %d lookups each\n,
+   n_rules, n_priorities, n_tables, n_threads, n_lookups);
+
+puts(\nWithout wildcards: \n);
+benchmark(false);
+puts(\nWith wildcards: \n);
+benchmark(true);
+}
+
+struct cls_aux {
+const struct classifier *cls;
+size_t n_lookup_flows;
+struct flow *lookup_flows;
+bool use_wc;
+atomic_int hits;
+atomic_int misses;
+};
+
+static void *
+lookup_classifier(void *aux_)
+{
+struct cls_aux *aux = aux_;
+cls_version_t version = CLS_MIN_VERSION;
+int hits = 0, old_hits;
+int misses = 0, old_misses;
+size_t i;
+
+random_set_seed(1);
+
+for (i = 0; i  n_lookups; i++) {
+const struct cls_rule *cr;
+struct flow_wildcards wc;
+unsigned int x;
+
+x = random_range(aux-n_lookup_flows);
+
+if (aux-use_wc) {
+flow_wildcards_init_catchall(wc);
+cr = classifier_lookup(aux-cls, version, aux-lookup_flows[x],
+   wc);
+} else {
+cr = classifier_lookup(aux-cls, version, aux-lookup_flows[x],
+   NULL);
+}
+if (cr) {
+hits++;
+} else {
+misses++;
+}
+}
+atomic_add(aux-hits, hits, old_hits);
+atomic_add(aux-misses, misses, old_misses);
+return NULL;
+}
+
+/* Benchmark classification. */
+static void

[ovs-dev] [PATCH v4 9/9] classifier: Retire partitions.

[Jarno Rajahalme]

Classifier partitions allowed skipping subtables when if was known
from the flow's metadata field that the subtable cannot possibly
match.  This functionality was later implemented in a more general
fashion by staged lookup, where the first stage also covers the
metadata field, among the rest of the non-packet fields in the struct
flow.  While in theory skipping a subtable on the basis of the
metadata field alone could produce more effective wildcards, on the
basis of our testsuite coverage it does not seem to be the case, as
removing the partitioning feature did not result in any test failures.

Removing the partitioning feature makes classifier lookups roughly 20%
faster when a wildcard mask is not needed, and roughly 10% faster when
a wildcard mask is needed, as tested with the test-classifier
benchmark with one lookup thread.

Found by profiling with 'perf'.

Signed-off-by: Jarno Rajahalme jrajaha...@nicira.com
---
 lib/automake.mk  |   2 -
 lib/classifier-private.h |  15 ---
 lib/classifier.c | 107 ---
 lib/tag.c|  64 
 lib/tag.h| 100 ---
 5 files changed, 288 deletions(-)
 delete mode 100644 lib/tag.c
 delete mode 100644 lib/tag.h

diff --git a/lib/automake.mk b/lib/automake.mk
index 15a9373..c262af8 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -245,8 +245,6 @@ lib_libopenvswitch_la_SOURCES = \
lib/syslog-provider.h \
lib/table.c \
lib/table.h \
-   lib/tag.c \
-   lib/tag.h \
lib/timer.c \
lib/timer.h \
lib/timeval.c \
diff --git a/lib/classifier-private.h b/lib/classifier-private.h
index b315a4c..9258ca1 100644
--- a/lib/classifier-private.h
+++ b/lib/classifier-private.h
@@ -21,7 +21,6 @@
 #include flow.h
 #include hash.h
 #include rculist.h
-#include tag.h
 
 /* Classifier internal definitions, subject to change at any time. */
 
@@ -40,7 +39,6 @@ struct cls_subtable {
  * following data structures. */
 
 /* These fields are accessed by readers who care about wildcarding. */
-const tag_type tag;   /* Tag generated from mask for partitioning. */
 const uint8_t n_indices;   /* How many indices to use. */
 const struct flowmap index_maps[CLS_MAX_INDICES + 1]; /* Stage maps. */
 unsigned int trie_plen[CLS_MAX_TRIES];  /* Trie prefix length in 'mask'
@@ -55,16 +53,6 @@ struct cls_subtable {
 /* 'mask' must be the last field. */
 };
 
-/* Associates a metadata value (that is, a value of the OpenFlow 1.1+ metadata
- * field) with tags for the cls_subtables that contain rules that match that
- * metadata value.  */
-struct cls_partition {
-struct cmap_node cmap_node; /* In struct classifier's 'partitions' map. */
-ovs_be64 metadata;  /* metadata value for this partition. */
-tag_type tags;  /* OR of each flow's cls_subtable tag. */
-struct tag_tracker tracker; /* Tracks the bits in 'tags'. */
-};
-
 /* Internal representation of a rule in a struct cls_subtable.
  *
  * The 'next' member is an element in a singly linked, null-terminated list.
@@ -77,9 +65,6 @@ struct cls_match {
 OVSRCU_TYPE(struct cls_match *) next; /* Equal, lower-priority matches. */
 OVSRCU_TYPE(struct cls_conjunction_set *) conj_set;
 
-/* Accessed only by writers. */
-struct cls_partition *partition;
-
 /* Accessed by readers interested in wildcarding. */
 const int priority; /* Larger numbers are higher priorities. */
 struct cmap_node index_nodes[CLS_MAX_INDICES]; /* Within subtable's
diff --git a/lib/classifier.c b/lib/classifier.c
index 3866f54..53bcbe7 100644
--- a/lib/classifier.c
+++ b/lib/classifier.c
@@ -321,7 +321,6 @@ classifier_init(struct classifier *cls, const uint8_t 
*flow_segments)
 cls-n_rules = 0;
 cmap_init(cls-subtables_map);
 pvector_init(cls-subtables);
-cmap_init(cls-partitions);
 cls-n_flow_segments = 0;
 if (flow_segments) {
 while (cls-n_flow_segments  CLS_MAX_INDICES
@@ -343,7 +342,6 @@ void
 classifier_destroy(struct classifier *cls)
 {
 if (cls) {
-struct cls_partition *partition;
 struct cls_subtable *subtable;
 int i;
 
@@ -356,11 +354,6 @@ classifier_destroy(struct classifier *cls)
 }
 cmap_destroy(cls-subtables_map);
 
-CMAP_FOR_EACH (partition, cmap_node, cls-partitions) {
-ovsrcu_postpone(free, partition);
-}
-cmap_destroy(cls-partitions);
-
 pvector_destroy(cls-subtables);
 }
 }
@@ -493,43 +486,6 @@ classifier_count(const struct classifier *cls)
 return cls-n_rules;
 }
 
-static uint32_t
-hash_metadata(ovs_be64 metadata)
-{
-return hash_uint64((OVS_FORCE uint64_t) metadata);
-}
-
-static struct cls_partition *
-find_partition(const struct classifier *cls, ovs_be64 metadata, uint32_t hash)
-{
-struct cls_partition 

[ovs-dev] [PATCH v4 5/9] classifier: Remove unused hash functions.

[Jarno Rajahalme]

Remove unused cls_rule_hash() and minimatch_hash() functions.

Signed-off-by: Jarno Rajahalme jrajaha...@nicira.com
---
 lib/classifier-private.h | 7 ---
 lib/classifier.c | 7 ---
 lib/classifier.h | 1 -
 3 files changed, 15 deletions(-)

diff --git a/lib/classifier-private.h b/lib/classifier-private.h
index cc255fe..0fd58e4 100644
--- a/lib/classifier-private.h
+++ b/lib/classifier-private.h
@@ -378,13 +378,6 @@ minimask_hash(const struct minimask *mask, uint32_t basis)
 return miniflow_hash(mask-masks, basis);
 }
 
-/* Returns a hash value for 'match', given 'basis'. */
-static inline uint32_t
-minimatch_hash(const struct minimatch *match, uint32_t basis)
-{
-return miniflow_hash(match-flow, minimask_hash(match-mask, basis));
-}
-
 /* Returns a hash value for the bits of range [start, end) in 'minimatch',
  * given 'basis'.
  *
diff --git a/lib/classifier.c b/lib/classifier.c
index f30f1ab..4adee2d 100644
--- a/lib/classifier.c
+++ b/lib/classifier.c
@@ -262,13 +262,6 @@ cls_rule_equal(const struct cls_rule *a, const struct 
cls_rule *b)
 return a-priority == b-priority  minimatch_equal(a-match, b-match);
 }
 
-/* Returns a hash value for 'rule', folding in 'basis'. */
-uint32_t
-cls_rule_hash(const struct cls_rule *rule, uint32_t basis)
-{
-return minimatch_hash(rule-match, hash_int(rule-priority, basis));
-}
-
 /* Appends a string describing 'rule' to 's'. */
 void
 cls_rule_format(const struct cls_rule *rule, struct ds *s)
diff --git a/lib/classifier.h b/lib/classifier.h
index 5ffe756..889dc1e 100644
--- a/lib/classifier.h
+++ b/lib/classifier.h
@@ -372,7 +372,6 @@ void cls_rule_set_conjunctions(struct cls_rule *,
const struct cls_conjunction *, size_t n);
 
 bool cls_rule_equal(const struct cls_rule *, const struct cls_rule *);
-uint32_t cls_rule_hash(const struct cls_rule *, uint32_t basis);
 void cls_rule_format(const struct cls_rule *, struct ds *);
 bool cls_rule_is_catchall(const struct cls_rule *);
 bool cls_rule_is_loose_match(const struct cls_rule *rule,
-- 
2.1.4

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

[ovs-dev] [PATCH v4 6/9] classifier: Do not use mf_value.

[Jarno Rajahalme]

mf_value has grown bigger than needed for storing the biggest
supported prefix (IPv6 address length).  Define a new type to be used
instead of mf_value.

This makes classifier lookups a bit faster.

Signed-off-by: Jarno Rajahalme jrajaha...@nicira.com
---
 lib/classifier.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/lib/classifier.c b/lib/classifier.c
index 4adee2d..040d04f 100644
--- a/lib/classifier.c
+++ b/lib/classifier.c
@@ -138,12 +138,18 @@ next_visible_rule_in_list(const struct cls_match *rule, 
cls_version_t version)
 return rule;
 }
 
+/* Type with maximum supported prefix length. */
+typedef union {
+struct in6_addr ipv6;  /* For sizing. */
+ovs_be32 be32; /* For access. */
+} trie_prefix_t;
+
 static unsigned int minimask_get_prefix_len(const struct minimask *,
 const struct mf_field *);
 static void trie_init(struct classifier *cls, int trie_idx,
   const struct mf_field *);
 static unsigned int trie_lookup(const struct cls_trie *, const struct flow *,
-union mf_value *plens);
+trie_prefix_t *plens);
 static unsigned int trie_lookup_value(const rcu_trie_ptr *,
   const ovs_be32 value[], ovs_be32 plens[],
   unsigned int value_bits);
@@ -913,7 +919,7 @@ struct trie_ctx {
 bool lookup_done;/* Status of the lookup. */
 uint8_t be32ofs; /* U32 offset of the field in question. */
 unsigned int maskbits;   /* Prefix length needed to avoid false matches. */
-union mf_value match_plens; /* Bitmask of prefix lengths with possible
+trie_prefix_t match_plens;  /* Bitmask of prefix lengths with possible
  * matches. */
 };
 
@@ -2181,7 +2187,7 @@ trie_lookup_value(const rcu_trie_ptr *trie, const 
ovs_be32 value[],
 
 static unsigned int
 trie_lookup(const struct cls_trie *trie, const struct flow *flow,
-union mf_value *plens)
+trie_prefix_t *plens)
 {
 const struct mf_field *mf = trie-field;
 
@@ -2314,7 +2320,7 @@ static void
 trie_remove_prefix(rcu_trie_ptr *root, const ovs_be32 *prefix, int mlen)
 {
 struct trie_node *node;
-rcu_trie_ptr *edges[sizeof(union mf_value) * 8];
+rcu_trie_ptr *edges[sizeof(trie_prefix_t) * CHAR_BIT];
 int depth = 0, ofs = 0;
 
 /* Walk the tree. */
-- 
2.1.4

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

[ovs-dev] [PATCH v4 8/9] flow: Add struct flowmap.

[Jarno Rajahalme]

Struct miniflow is now sometimes used just as a map.  Define a new
struct flowmap for that purpose.  The flowmap is defined as an array of
maps, and it is automatically sized according to the size of struct
flow, so it will be easier to maintain in the future.

It would have been tempting to use the existing struct bitmap for this
purpose. The main reason this is not feasible at the moment is that
some flowmap algorithms are simpler when it can be assumed that no
struct flow member requires more bits than can fit to a single map
unit. The tunnel member already requires more than 32 bits, so the map
unit needs to be 64 bits wide.

Performance critical algorithms enumerate the flowmap array units
explicitly, as it is easier for the compiler to optimize, compared to
the normal iterator.  Without this optimization a classifier lookup
without wildcard masks would be about 25% slower.

With this more general (and maintainable) algorithm the classifier
lookups are about 5% slower, when the struct flow actually becomes big
enough to require a second map.  This negates the performance gained
in the Pre-compute stage masks patch earlier in the series.

Requested-by: Ben Pfaff b...@nicira.com
Signed-off-by: Jarno Rajahalme jrajaha...@nicira.com
---
 lib/classifier-private.h |  87 -
 lib/classifier.c | 228 ++-
 lib/dpif-netdev.c|  93 +++---
 lib/flow.c   | 356 +++
 lib/flow.h   | 469 +++
 lib/match.c  |   5 +-
 tests/test-classifier.c  |  32 ++--
 7 files changed, 646 insertions(+), 624 deletions(-)

diff --git a/lib/classifier-private.h b/lib/classifier-private.h
index a7a5132..b315a4c 100644
--- a/lib/classifier-private.h
+++ b/lib/classifier-private.h
@@ -42,7 +42,7 @@ struct cls_subtable {
 /* These fields are accessed by readers who care about wildcarding. */
 const tag_type tag;   /* Tag generated from mask for partitioning. */
 const uint8_t n_indices;   /* How many indices to use. */
-const struct miniflow index_maps[CLS_MAX_INDICES + 1]; /* Stage maps. */
+const struct flowmap index_maps[CLS_MAX_INDICES + 1]; /* Stage maps. */
 unsigned int trie_plen[CLS_MAX_TRIES];  /* Trie prefix length in 'mask'
  * (runtime configurable). */
 const int ports_mask_len;
@@ -238,16 +238,16 @@ flow_hash_in_minimask(const struct flow *flow, const 
struct minimask *mask,
 const uint64_t *mask_values = miniflow_get_values(mask-masks);
 const uint64_t *flow_u64 = (const uint64_t *)flow;
 const uint64_t *p = mask_values;
-uint32_t hash;
-size_t idx;
+uint32_t hash = basis;
+map_t map;
 
-hash = basis;
-MAP_FOR_EACH_INDEX(idx, mask-masks.tnl_map) {
-hash = hash_add64(hash, flow_u64[idx]  *p++);
-}
-flow_u64 += FLOW_TNL_U64S;
-MAP_FOR_EACH_INDEX(idx, mask-masks.pkt_map) {
-hash = hash_add64(hash, flow_u64[idx]  *p++);
+FLOWMAP_FOR_EACH_MAP (map, mask-masks.map) {
+size_t idx;
+
+MAP_FOR_EACH_INDEX (idx, map) {
+hash = hash_add64(hash, flow_u64[idx]  *p++);
+}
+flow_u64 += MAP_T_BITS;
 }
 
 return hash_finish(hash, (p - mask_values) * 8);
@@ -265,13 +265,10 @@ miniflow_hash_in_minimask(const struct miniflow *flow,
 const uint64_t *mask_values = miniflow_get_values(mask-masks);
 const uint64_t *p = mask_values;
 uint32_t hash = basis;
-uint64_t flow_u64;
+uint64_t value;
 
-MINIFLOW_FOR_EACH_IN_TNL_MAP(flow_u64, flow, mask-masks) {
-hash = hash_add64(hash, flow_u64  *p++);
-}
-MINIFLOW_FOR_EACH_IN_PKT_MAP(flow_u64, flow, mask-masks) {
-hash = hash_add64(hash, flow_u64  *p++);
+MINIFLOW_FOR_EACH_IN_FLOWMAP(value, flow, mask-masks.map) {
+hash = hash_add64(hash, value  *p++);
 }
 
 return hash_finish(hash, (p - mask_values) * 8);
@@ -285,24 +282,23 @@ miniflow_hash_in_minimask(const struct miniflow *flow,
 static inline uint32_t
 flow_hash_in_minimask_range(const struct flow *flow,
 const struct minimask *mask,
-const struct miniflow *maps,
+const struct flowmap fmap,
 unsigned int *offset,
 uint32_t *basis)
 {
 const uint64_t *mask_values = miniflow_get_values(mask-masks);
-const uint64_t *p = mask_values + *offset;
 const uint64_t *flow_u64 = (const uint64_t *)flow;
+const uint64_t *p = mask_values + *offset;
 uint32_t hash = *basis;
-size_t idx;
+map_t map;
 
-if (OVS_UNLIKELY(maps-tnl_map)) {
-MAP_FOR_EACH_INDEX(idx, maps-tnl_map) {
+FLOWMAP_FOR_EACH_MAP (map, fmap) {
+size_t idx;
+
+MAP_FOR_EACH_INDEX (idx, map) {
 hash = hash_add64(hash, flow_u64[idx]  *p++);
 }
-}
-flow_u64 

[ovs-dev] [PATCH v4 7/9] classifier: Simplify minimask_hash().

[Jarno Rajahalme]

minimask_hash() can be simplified as each value is known to be non-zero.

Move miniflow_hash() into test-classifier.c as miniflow_hash__() as it
is no longer needed elsewhere.

Signed-off-by: Jarno Rajahalme jrajaha...@nicira.com
---
 lib/classifier-private.h | 38 +-
 tests/test-classifier.c  | 34 +-
 2 files changed, 42 insertions(+), 30 deletions(-)

diff --git a/lib/classifier-private.h b/lib/classifier-private.h
index 0fd58e4..a7a5132 100644
--- a/lib/classifier-private.h
+++ b/lib/classifier-private.h
@@ -341,41 +341,21 @@ flow_wildcards_fold_minimask_in_map(struct flow_wildcards 
*wc,
 }
 }
 
-/* Returns a hash value for 'flow', given 'basis'. */
+/* Returns a hash value for 'mask', given 'basis'. */
 static inline uint32_t
-miniflow_hash(const struct miniflow *flow, uint32_t basis)
+minimask_hash(const struct minimask *mask, uint32_t basis)
 {
-const uint64_t *values = miniflow_get_values(flow);
-const uint64_t *p = values;
+const uint64_t *p = miniflow_get_values(mask-masks);
+size_t n_values = miniflow_n_values(mask-masks);
 uint32_t hash = basis;
-uint64_t hash_tnl_map = 0, hash_pkt_map = 0;
-uint64_t map;
 
-for (map = flow-tnl_map; map; map = zero_rightmost_1bit(map)) {
-if (*p) {
-hash = hash_add64(hash, *p);
-hash_tnl_map |= rightmost_1bit(map);
-}
-p++;
-}
-for (map = flow-pkt_map; map; map = zero_rightmost_1bit(map)) {
-if (*p) {
-hash = hash_add64(hash, *p);
-hash_pkt_map |= rightmost_1bit(map);
-}
-p++;
+for (size_t i = 0; i  n_values; i++) {
+hash = hash_add64(hash, *p++);
 }
-hash = hash_add64(hash, hash_tnl_map);
-hash = hash_add64(hash, hash_pkt_map);
+hash = hash_add64(hash, mask-masks.tnl_map);
+hash = hash_add64(hash, mask-masks.pkt_map);
 
-return hash_finish(hash, p - values);
-}
-
-/* Returns a hash value for 'mask', given 'basis'. */
-static inline uint32_t
-minimask_hash(const struct minimask *mask, uint32_t basis)
-{
-return miniflow_hash(mask-masks, basis);
+return hash_finish(hash, n_values);
 }
 
 /* Returns a hash value for the bits of range [start, end) in 'minimatch',
diff --git a/tests/test-classifier.c b/tests/test-classifier.c
index 2462aed..54b595f 100644
--- a/tests/test-classifier.c
+++ b/tests/test-classifier.c
@@ -1633,6 +1633,36 @@ miniflow_clone__(const struct miniflow *src)
 return dst;
 }
 
+/* Returns a hash value for 'flow', given 'basis'. */
+static inline uint32_t
+miniflow_hash__(const struct miniflow *flow, uint32_t basis)
+{
+const uint64_t *values = miniflow_get_values(flow);
+const uint64_t *p = values;
+uint32_t hash = basis;
+uint64_t hash_tnl_map = 0, hash_pkt_map = 0;
+uint64_t map;
+
+for (map = flow-tnl_map; map; map = zero_rightmost_1bit(map)) {
+if (*p) {
+hash = hash_add64(hash, *p);
+hash_tnl_map |= rightmost_1bit(map);
+}
+p++;
+}
+for (map = flow-pkt_map; map; map = zero_rightmost_1bit(map)) {
+if (*p) {
+hash = hash_add64(hash, *p);
+hash_pkt_map |= rightmost_1bit(map);
+}
+p++;
+}
+hash = hash_add64(hash, hash_tnl_map);
+hash = hash_add64(hash, hash_pkt_map);
+
+return hash_finish(hash, p - values);
+}
+
 static void
 test_miniflow(struct ovs_cmdl_context *ctx OVS_UNUSED)
 {
@@ -1667,7 +1697,7 @@ test_miniflow(struct ovs_cmdl_context *ctx OVS_UNUSED)
 /* Check that copying a miniflow works properly. */
 miniflow2 = miniflow_clone__(miniflow);
 assert(miniflow_equal(miniflow, miniflow2));
-assert(miniflow_hash(miniflow, 0) == miniflow_hash(miniflow2, 0));
+assert(miniflow_hash__(miniflow, 0) == miniflow_hash__(miniflow2, 0));
 miniflow_expand(miniflow2, flow3);
 assert(flow_equal(flow, flow3));
 
@@ -1683,6 +1713,8 @@ test_miniflow(struct ovs_cmdl_context *ctx OVS_UNUSED)
 assert(miniflow_equal_flow_in_minimask(miniflow, flow2, minimask));
 assert(miniflow_hash_in_minimask(miniflow, minimask, 0x12345678) ==
flow_hash_in_minimask(flow, minimask, 0x12345678));
+assert(minimask_hash(minimask, 0) ==
+   miniflow_hash__(minimask-masks, 0));
 
 /* Check that masked matches work as expected for differing flows and
  * miniflows. */
-- 
2.1.4

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

Re: [ovs-dev] [PATCH v2 7/7] flow: Add struct flowmap.

[Jarno Rajahalme]

 On Aug 6, 2015, at 5:39 PM, Jarno Rajahalme jrajaha...@nicira.com wrote:
 
 On Aug 6, 2015, at 16:29, Jesse Gross je...@nicira.com wrote:
 
 
 On Thu, Aug 6, 2015 at 4:17 PM, Jarno Rajahalme jrajaha...@nicira.com 
 wrote:
 Struct miniflow is now sometimes used just as a map.  Define a new
 struct flowmap for that purpose.  The flowmap is defined as an array of
 maps, and it is automatically sized according to the size of struct
 flow, so it will be easier to maintain in the future.
 
 It would have been tempting to use the existing struct bitmap for this
 purpose. The main reason this is not feasible at the moment is that
 some flowmap algorithms are simpler when it can be assumed that no
 struct flow member requires more bits than can fit to a single map
 unit. The tunnel member already requires more than 32 bits, so the map
 unit needs to be 64 bits wide.
 
 Performance critical algorithms enumerate the flowmap array units
 explicitly, as it is easier for the compiler to optimize, compared to
 the normal iterator.  Without this optimization a classifier lookup
 without wildcard masks would be about 25% slower.
 
 With this more general (and maintainable) algorithm the classifier
 lookups are about 5% slower, when the struct flow actually becomes big
 enough to require a second map.  This negates the performance gained
 in the Pre-compute stage masks patch earlier in the series.
 
 Requested-by: Ben Pfaff b...@nicira.com
 Signed-off-by: Jarno Rajahalme jrajaha...@nicira.com
 
 I think this patch may need a rebase after the tunnel metadata flow
 patches that I applied last night.
 
 Sorry about that.
 
 Thanks for the heads up! I just sent a v3.
 

I did some more testing and profiling and sent an improved version (v4). No 
changes in the first few patches.

  Jarno

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