Ben Pfaff | 29 Jul 00:22 2015

[PATCH] ofp-actions: OFPP_ANY (aka OFPP_NONE) is not a valid output port.

This is implied by the list of ports that are valid for output in the
various versions of the OpenFlow specification.

Found by OFTest.

Signed-off-by: Ben Pfaff <blp <at> nicira.com>
---
 lib/ofp-actions.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index eef3389..14a2802 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
 <at>  <at>  -5396,10 +5396,12  <at>  <at>  ofpact_check_output_port(ofp_port_t port, ofp_port_t max_ports)
     case OFPP_FLOOD:
     case OFPP_ALL:
     case OFPP_CONTROLLER:
-    case OFPP_NONE:
     case OFPP_LOCAL:
         return 0;

+    case OFPP_NONE:
+        return OFPERR_OFPBAC_BAD_OUT_PORT;
+
     default:
         if (ofp_to_u16(port) < ofp_to_u16(max_ports)) {
             return 0;
--

-- 
2.1.3
(Continue reading)

Ben Pfaff | 29 Jul 00:21 2015

[PATCH] ofproto: Ignore generation ID for role change to "equal".

The OpenFlow specification says that only role changes to slave or master
check the generation ID, so this is a bug fix.

Found by OFTest.

Signed-off-by: Ben Pfaff <blp <at> nicira.com>
---
 ofproto/ofproto.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index a724071..e9b1472 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
 <at>  <at>  -5315,7 +5315,8  <at>  <at>  handle_role_request(struct ofconn *ofconn, const struct ofp_header *oh)
     }

     if (request.role != OFPCR12_ROLE_NOCHANGE) {
-        if (request.have_generation_id
+        if (request.role != OFPCR12_ROLE_EQUAL
+            && request.have_generation_id
             && !ofconn_set_master_election_id(ofconn, request.generation_id)) {
                 return OFPERR_OFPRRFC_STALE;
         }
--

-- 
2.1.3

_______________________________________________
dev mailing list
dev <at> openvswitch.org
(Continue reading)

Ben Pfaff | 29 Jul 00:21 2015

[PATCH] ofp-util: Report error for weighted bucket in non-select group.

This complies with OpenFlow 1.1 and later.

Found by OFTest.

Signed-off-by: Ben Pfaff <blp <at> nicira.com>
---
 lib/ofp-util.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 9996e84..28f6d8f 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
 <at>  <at>  -8592,6 +8592,10  <at>  <at>  ofputil_decode_group_mod(const struct ofp_header *oh,
     }

     LIST_FOR_EACH (bucket, list_node, &gm->buckets) {
+        if (bucket->weight && gm->type != OFPGT11_SELECT) {
+            return OFPERR_OFPGMFC_INVALID_GROUP;
+        }
+
         switch (gm->type) {
         case OFPGT11_ALL:
         case OFPGT11_INDIRECT:
--

-- 
2.1.3

_______________________________________________
dev mailing list
dev <at> openvswitch.org
(Continue reading)

Jesse Gross | 29 Jul 00:20 2015

[PATCH] openflow: Add additional reserved space in struct nx_geneve_table_reply.

It's possible to imagine that a switch might want to report additional
capabilities related to Geneve beyond just the number of options and
how much space they can consume. Some examples include additional
restrictions on parsing (if this command is used for non-OVS implementations
or OVS changes how it works) and per-packet actions that can't be done
generically (such as checksums or encryption). It's not yet clear if
these will be necessary or if OpenFlow is the right place to expose
them. However, it's easy to do now and there is very little cost so
it seems like a good idea to leave some additional reserved space.

Signed-off-by: Jesse Gross <jesse <at> nicira.com>
---
 include/openflow/nicira-ext.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
index 465b19d..b56edb3 100644
--- a/include/openflow/nicira-ext.h
+++ b/include/openflow/nicira-ext.h
 <at>  <at>  -986,12 +986,12  <at>  <at>  OFP_ASSERT(sizeof(struct nx_geneve_table_mod) == 8);
 struct nx_geneve_table_reply {
     ovs_be32 max_option_space; /* Maximum total of option sizes supported. */
     ovs_be16 max_fields;       /* Maximum number of match fields supported. */
-    uint8_t pad[2];
+    uint8_t pad[10];
     /* struct nx_geneve_map[0]; Array of maps between indicies and Geneve
                                 options. The number of elements is
                                 inferred from the length field in the
                                 header. */
 };
(Continue reading)

Ben Pfaff | 28 Jul 22:41 2015

[PATCH] ovn-controller: Avoid overlooking changes that occur during commit.

A commit to the database takes multiple trips through the main loop.  In
that time, the database could change, but ovn-controller can't properly
react to the changes until the commit has succeeded or failed.  Since
commit f1fd765733 (ovn-controller: Avoid blocking to commit OVSDB
transactions), Open vSwitch has failed to properly re-check the contents
of the database following a successful commit.  That meant that it was
possible for ovn-controller to fail to react to a database change until
much later, if nothing else happened for some time.

Reported-by; Alex Wang <alexw <at> nicira.com>
Reported-at: http://openvswitch.org/pipermail/dev/2015-July/058176.html
Signed-off-by: Ben Pfaff <blp <at> nicira.com>
---
 ovn/controller/ovn-controller.c | 48 +++++++++++++++++++++++++----------------
 1 file changed, 30 insertions(+), 18 deletions(-)

diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 0657140..affc14b 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
 <at>  <at>  -189,28 +189,40  <at>  <at>  idl_loop_commit_and_wait(struct idl_loop *loop)
     struct ovsdb_idl_txn *txn = loop->committing_txn;
     if (txn) {
         enum ovsdb_idl_txn_status status = ovsdb_idl_txn_commit(txn);
-        switch (status) {
-        case TXN_INCOMPLETE:
-            break;
+        if (status != TXN_INCOMPLETE) {
+            switch (status) {
+            case TXN_TRY_AGAIN:
(Continue reading)

A.R. Karthick | 28 Jul 20:33 2015

[PATCH] netdev-dpdk: Increase limit for dpdk ring names

Hi,
 Currently, the ovs netdev-dpdk ring type dpdkr, restricts dpdk ring name
size to 10 bytes (includes the _rx/tx suffix).

 This only gives us space to accommodate 10 rings as truncation happens
with dpdkr10_rx/tx.

 The enclosed patch below ensures that we respect the ring name size
constraints imposed by the rte dpdk ring apis by allowing for a greater
size instead of using a hard-coded ring name size in ovs netdev-dpdk layer.

Regards,
-Karthick

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 5ae805e..00229c2 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
 <at>  <at>  -92,7 +92,7  <at>  <at>  BUILD_ASSERT_DECL((MAX_NB_MBUF /
ROUND_DOWN_POW2(MAX_NB_MBUF/MIN_NB_MBUF))

 #define NIC_PORT_RX_Q_SIZE 2048  /* Size of Physical NIC RX Queue, Max
(n+32<=4096)*/
 #define NIC_PORT_TX_Q_SIZE 2048  /* Size of Physical NIC TX Queue, Max
(n+32<=4096)*/
-
+#define RING_NAME_MAX (RTE_MEMZONE_NAMESIZE - sizeof(RTE_RING_MZ_PREFIX))
 static char *cuse_dev_name = NULL;    /* Character device cuse_dev_name. */
 static char *vhost_sock_dir = NULL;   /* Location of vhost-user sockets */

(Continue reading)

Ben Pfaff | 28 Jul 20:26 2015

[PATCH] ofctrl: Fix use of uninitialized hash value in ofctrl_add_flow().

When ofctrl_add_flow() called ovn_flow_lookup(), the latter used the hash
from the flow's hmap_node, but the former hadn't initialized it at that
point.  This commit fixes the problem.

Reported-by: Russell Bryant <rbryant <at> redhat.com>
Reported-at: http://openvswitch.org/pipermail/dev/2015-July/057851.html
Signed-off-by: Ben Pfaff <blp <at> nicira.com>
---
 ovn/controller/ofctrl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c
index c548645..e701f8b 100644
--- a/ovn/controller/ofctrl.c
+++ b/ovn/controller/ofctrl.c
 <at>  <at>  -268,6 +268,7  <at>  <at>  ofctrl_add_flow(struct hmap *desired_flows,
     f->match = *match;
     f->ofpacts = xmemdup(actions->data, actions->size);
     f->ofpacts_len = actions->size;
+    f->hmap_node.hash = ovn_flow_hash(f);

     if (ovn_flow_lookup(desired_flows, f)) {
         static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);
 <at>  <at>  -281,7 +282,7  <at>  <at>  ofctrl_add_flow(struct hmap *desired_flows,
         return;
     }

-    hmap_insert(desired_flows, &f->hmap_node, ovn_flow_hash(f));
+    hmap_insert(desired_flows, &f->hmap_node, f->hmap_node.hash);
 }
(Continue reading)

Alex Wang | 28 Jul 19:32 2015

[bracnh-2.3] vlan-splinter: Do not check for ipv6 address.

When ovs cleans up unused vlan sub interfaces created by vlan splinter
feature, it will not touch interface that has ip address assigned.
However, the get_in6() function implemented in netdev-linux module
is buggy and always returns 0 (found ipv6 address).  This causes the
vlan splinter created sub interfaces never being removed.

Since vlan splinter check is the only user of get_in6 function and
there is no known use case of vlan interface with ipv6 address, we can
simply remove the call of get_in6() function in vlan splinter check.

VMware-BZ: #1485521
Reported-by: Ronald Lee <ronaldlee <at> vmware.com>
Signed-off-by: Alex Wang <alexw <at> nicira.com>
---
 vswitchd/bridge.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 6cd30b8..5b13679 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
 <at>  <at>  -4155,8 +4155,7  <at>  <at>  collect_splinter_vlans(const struct ovsrec_open_vswitch *ovs_cfg)
                 struct netdev *netdev;

                 if (!netdev_open(vlan_dev->name, "system", &netdev)) {
-                    if (!netdev_get_in4(netdev, NULL, NULL) ||
-                        !netdev_get_in6(netdev, NULL)) {
+                    if (!netdev_get_in4(netdev, NULL, NULL)) {
                         /* It has an IP address configured, so we don't own
                          * it.  Don't delete it. */
(Continue reading)

Ben Pfaff | 28 Jul 17:44 2015

[PATCH v2 00/21] Change strategy for tunnel keys

This is mostly a rebase versus v1.  A few patches have been acked,
and patch 1 includes a bug fix.

Ben Pfaff (21):
  ovn-controller: Fix potential use-after-free in get_core_config().
  ovn-controller: Drop unnecessary checks for ovsdb_idl_is_alive().
  ovn-controller: Avoid blocking to commit OVSDB transactions.
  ovn-controller: Explicitly pass the flow table from function to
    function.
  ovn-controller: Pass 'br_int' explicitly to functions that need it.
  ovn-controller: Factor encapsulation code out of chassis code.
  ovn-controller: Pass 'chassis_id' explicitly to functions that need
    it.
  ovn-controller: Tolerate missing integration bridge.
  ovn-controller: Tolerate missing 'chassis_id'.
  ovn-controller: Rename init functions that just register IDL columns.
  ovn-controller: Honor external-ids:ovn-bridge changing.
  ovn-controller: Slightly adjust pipeline init and destroy for
    consistency.
  ovn-controller: Use controller_ctx just to pass around data.
  smap: New function smap_get_uuid().
  nroff: Add support for 'diagram' XML element for protocol headers.
  ovn: Rename Binding table to Port_Binding.
  ovn: Rename Pipeline table to Rule table.
  actions: Allow caller to specify output table.
  rule: Introduce MFF_LOG_DATAPATH macro for consistency.
  ofctrl: Negotiate OVN Geneve option.
  ovn: Change strategy for tunnel keys.

 lib/smap.c                             |   13 +-
(Continue reading)

saloni.jain12 | 28 Jul 14:09 2015
Picon

[PATCH v1 3/3] Implement OFPT_TABLE_STATUS Message.

From: Saloni Jain <saloni.jain <at> tcs.com>

On change in a table state, the controller needs to be informed with
the OFPT_TABLE_STATUS message. The message is sent with reason
OFPTR_VACANCY_DOWN or OFPTR_VACANCY_UP in case of change in remaining
space eventually crossing any one of the threshold.

Signed-off-by: Saloni Jain <saloni.jain <at> tcs.com>
---
 include/openflow/openflow-1.4.h |    8 ++++
 lib/learning-switch.c           |    1 +
 lib/ofp-msgs.h                  |    6 +++
 lib/ofp-print.c                 |   26 +++++++++++++
 lib/ofp-util.c                  |   82 +++++++++++++++++++++++++++++++++++++++
 lib/ofp-util.h                  |   12 ++++++
 lib/rconn.c                     |    1 +
 ofproto/connmgr.c               |   31 +++++++++++++++
 ofproto/connmgr.h               |    3 ++
 ofproto/ofproto.c               |   31 +++++++++++++++
 ovn/controller/ofctrl.c         |    1 +
 tests/ofproto.at                |   20 ++++++++--
 12 files changed, 219 insertions(+), 3 deletions(-)

diff --git a/include/openflow/openflow-1.4.h b/include/openflow/openflow-1.4.h
index 37eef4a..1c996f7 100644
--- a/include/openflow/openflow-1.4.h
+++ b/include/openflow/openflow-1.4.h
 <at>  <at>  -172,6 +172,14  <at>  <at>  struct ofp14_table_desc {
 };
 OFP_ASSERT(sizeof(struct ofp14_table_desc) == 8);
(Continue reading)

saloni.jain12 | 28 Jul 14:09 2015
Picon

[PATCH v1 2/3] Implement Vacancy Events for OFPMP_TABLE_DESC.

From: Saloni Jain <saloni.jain <at> tcs.com>

This patch adds support for vacancy events in table-desc.

ovs-ofctl -O OpenFlow14 dump-tables-desc <switch>
-This command is enhanced to display the Vacancy Event configuration
 of the tables on a <switch>, which is set using the mod-table command.

Signed-off-by: Saloni Jain <saloni.jain <at> tcs.com>
---
 lib/ofp-print.c   |   11 +++++++++++
 lib/ofp-util.c    |   31 +++++++++++++++++++++++++++++++
 ofproto/ofproto.c |   26 +++++++++++++++++++++-----
 tests/ofproto.at  |   10 +++++++++-
 4 files changed, 72 insertions(+), 6 deletions(-)

diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index cf35aa6..0c3b8a9 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
 <at>  <at>  -1057,6 +1057,17  <at>  <at>  ofp_print_table_desc(struct ds *string, const struct ofputil_table_desc *td)
                   ofputil_table_eviction_to_string(td->eviction));
     ofputil_put_eviction_flags(string, td->eviction_flags);
     ds_put_char(string, '\n');
+    ds_put_format(string, "   vacancy=%s",
+                  ofputil_table_vacancy_to_string(td->vacancy));
+    if (td->vacancy == OFPUTIL_TABLE_VACANCY_ON) {
+        ds_put_format(string, " vacancy_down=%"PRIu8"%%",
+                      td->table_vacancy.vacancy_down);
+        ds_put_format(string, " vacancy_up=%"PRIu8"%%",
(Continue reading)


Gmane