Re: [ovs-dev] [ovs-discuss] Flow Entry Statistics Trigger support

2017-05-12 Thread Ben Pfaff
I'm not aware of any.

On Fri, May 12, 2017 at 06:44:30AM +, Rohith Basavaraja wrote:
> Hi Ben,
> 
> Thanks for sharing the patch details. Looked at the patch diff and
> looks like these patches contains changes to add Support for Extensible Flow 
> Entry Statistics (EXT-334)
> and doesn’t contains changes to add support for Flow Entry Statistics Trigger 
> (EXT-335).
> 
> Is there any ongoing work to add support for Flow Entry Statistics Trigger 
> (EXT-335)?
> or this work is not started yet?
> 
> Thanks
> Rohith
> 
> 
> 
> 
> 
> -- Forwarded message --
> From: Ben Pfaff mailto:b...@ovn.org>>
> Date: Thu, May 11, 2017 at 7:12 PM
> Subject: Re: [ovs-discuss] Flow Entry Statistics Trigger support
> To: Rohith Basavaraja 
> mailto:rohith.basavar...@gmail.com>>
> Cc: ovs-disc...@openvswitch.org
> 
> 
> On Thu, May 11, 2017 at 12:08:25PM +0530, Rohith Basavaraja wrote:
> > Wanted to check whether anyone working on the Flow Entry Statistics Trigger
> > support in OVS or this work not started yet?.If anyone started working on
> > this what is targeted OVS release this feature will be supported.
> 
> There's some work toward this feature:
> https://patchwork.ozlabs.org/patch/761074/
> https://patchwork.ozlabs.org/patch/761075/
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v2 3/3] netdev-dpdk: Use uint8_t for port_id.

2017-05-12 Thread Darrell Ball
Have a look at the response to V1

Thanks
Darrell

On 5/12/17, 8:10 AM, "Ilya Maximets"  wrote:

Currently, signed integer is used for 'port_id' variable and
'-1' as identifier of bad or uninitialized 'port_id'.

This inconsistent with dpdk library and, also, in few cases,
leads to passing '-1' to dpdk functions where uint8_t expected.

Such behaviour doesn't produce any issues, but it's better to
use same type as in dpdk library for consistency.

Also, magic number '-1' replaced with DPDK_ETH_PORT_ID_INVALID
macro.

Signed-off-by: Ilya Maximets 
---
 lib/netdev-dpdk.c | 59 
+++
 1 file changed, 33 insertions(+), 26 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index baac7c3..1ed25b2 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -140,6 +140,8 @@ BUILD_ASSERT_DECL((MAX_NB_MBUF / 
ROUND_DOWN_POW2(MAX_NB_MBUF/MIN_NB_MBUF))
 #define OVS_VHOST_QUEUE_DISABLED(-2) /* Queue was disabled by guest 
and not
   * yet mapped to another queue. */
 
+#define DPDK_ETH_PORT_ID_INVALIDRTE_MAX_ETHPORTS
+
 #define VHOST_ENQ_RETRY_NUM 8
 #define IF_NAME_SZ (PATH_MAX > IFNAMSIZ ? PATH_MAX : IFNAMSIZ)
 
@@ -309,7 +311,7 @@ struct dpdk_ring {
 struct rte_ring *cring_tx;
 struct rte_ring *cring_rx;
 unsigned int user_port_id; /* User given port no, parsed from port 
name */
-int eth_port_id; /* ethernet device port id */
+uint8_t eth_port_id; /* ethernet device port id */
 struct ovs_list list_node OVS_GUARDED_BY(dpdk_mutex);
 };
 
@@ -325,7 +327,7 @@ enum dpdk_hw_ol_features {
 
 struct netdev_dpdk {
 struct netdev up;
-int port_id;
+uint8_t port_id;
 int max_packet_len;
 enum dpdk_dev_type type;
 
@@ -402,7 +404,7 @@ struct netdev_dpdk {
 
 struct netdev_rxq_dpdk {
 struct netdev_rxq up;
-int port_id;
+uint8_t port_id;
 };
 
 static int netdev_dpdk_class_init(void);
@@ -603,12 +605,12 @@ check_link_status(struct netdev_dpdk *dev)
 dev->link_reset_cnt++;
 dev->link = link;
 if (dev->link.link_status) {
-VLOG_DBG_RL(&rl, "Port %d Link Up - speed %u Mbps - %s",
+VLOG_DBG_RL(&rl, "Port %"PRIu8" Link Up - speed %u Mbps - %s",
 dev->port_id, (unsigned) dev->link.link_speed,
 (dev->link.link_duplex == ETH_LINK_FULL_DUPLEX) ?
  ("full-duplex") : ("half-duplex"));
 } else {
-VLOG_DBG_RL(&rl, "Port %d Link Down", dev->port_id);
+VLOG_DBG_RL(&rl, "Port %"PRIu8" Link Down", dev->port_id);
 }
 }
 }
@@ -726,8 +728,8 @@ dpdk_eth_checksum_offload_configure(struct netdev_dpdk 
*dev)
 if (rx_csum_ol_flag &&
 (info.rx_offload_capa & rx_chksm_offload_capa) !=
  rx_chksm_offload_capa) {
-VLOG_WARN_ONCE("Rx checksum offload is not supported on device %d",
-   dev->port_id);
+VLOG_WARN_ONCE("Rx checksum offload is not supported on device 
%"PRIu8,
+   dev->port_id);
 dev->hw_ol_features &= ~NETDEV_RX_CHECKSUM_OFFLOAD;
 return;
 }
@@ -738,7 +740,8 @@ static void
 dpdk_eth_flow_ctrl_setup(struct netdev_dpdk *dev) OVS_REQUIRES(dev->mutex)
 {
 if (rte_eth_dev_flow_ctrl_set(dev->port_id, &dev->fc_conf)) {
-VLOG_WARN("Failed to enable flow control on device %d", 
dev->port_id);
+VLOG_WARN("Failed to enable flow control on device %"PRIu8,
+  dev->port_id);
 }
 }
 
@@ -776,7 +779,7 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
 
 memset(ð_addr, 0x0, sizeof(eth_addr));
 rte_eth_macaddr_get(dev->port_id, ð_addr);
-VLOG_INFO_RL(&rl, "Port %d: "ETH_ADDR_FMT,
+VLOG_INFO_RL(&rl, "Port %"PRIu8": "ETH_ADDR_FMT,
 dev->port_id, 
ETH_ADDR_BYTES_ARGS(eth_addr.addr_bytes));
 
 memcpy(dev->hwaddr.ea, eth_addr.addr_bytes, ETH_ADDR_LEN);
@@ -788,7 +791,7 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
 /* Get the Flow control configuration for DPDK-ETH */
 diag = rte_eth_dev_flow_ctrl_get(dev->port_id, &dev->fc_conf);
 if (diag) {
-VLOG_DBG("cannot get flow control parameters on port=%d, err=%d",
+VLOG_DBG("cannot get flow control parameters on port=%"PRIu8", 
err=%d",
  dev->port_id, diag);
 }
 
@@ -833,7 +836,7 @@ netdev_dpdk_alloc_txq(unsigned int n_txqs)
 }
 
 static int
-common_construct(struct netdev *netdev, unsigned int port_no,
+common_construct(str

Re: [ovs-dev] [PATCH 3/3] netdev-dpdk: Use uint8_t for port_id.

2017-05-12 Thread Darrell Ball


On 5/12/17, 8:04 AM, "Ilya Maximets"  wrote:

On 05.04.2017 22:34, Darrell Ball wrote:
> 
> 
> On 4/3/17, 8:04 AM, "ovs-dev-boun...@openvswitch.org on behalf of Ilya 
Maximets"  
wrote:
> 
> Currently, signed integer is used for 'port_id' variable and
> '-1' as identifier of bad or uninitialized 'port_id'.
> 
> This inconsistent with dpdk library and, also, in few cases,
> leads to passing '-1' to dpdk functions where uint8_t expected.
> 
> Such behaviour doesn't produce any issues, but it's better to
> use same type as in dpdk library for consistency.
> 
> Also, magic number '-1' replaced with DPDK_ETH_PORT_ID_INVALID
> macro.
> 
> Signed-off-by: Ilya Maximets 
> ---
>  lib/netdev-dpdk.c | 61 
+++
>  1 file changed, 35 insertions(+), 26 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 658a454..216ced8 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -140,6 +140,8 @@ BUILD_ASSERT_DECL((MAX_NB_MBUF / 
ROUND_DOWN_POW2(MAX_NB_MBUF/MIN_NB_MBUF))
>  #define OVS_VHOST_QUEUE_DISABLED(-2) /* Queue was disabled by 
guest and not
>* yet mapped to another 
queue. */
>  
> +#define DPDK_ETH_PORT_ID_INVALIDRTE_MAX_ETHPORTS
> +
>  #define VHOST_ENQ_RETRY_NUM 8
>  #define IF_NAME_SZ (PATH_MAX > IFNAMSIZ ? PATH_MAX : IFNAMSIZ)
>  
> @@ -309,7 +311,7 @@ struct dpdk_ring {
>  struct rte_ring *cring_tx;
>  struct rte_ring *cring_rx;
>  unsigned int user_port_id; /* User given port no, parsed from 
port name */
> -int eth_port_id; /* ethernet device port id */
> +uint8_t eth_port_id; /* ethernet device port id */
> 
> The rte does not have a typedef for port_id.
> One optional change is for OVS to have a typedef for this external type.
> /* The dpdk rte uses uint8_t for port_id. */
> typedef uint8_t rte_port_id;

I don't think that it is a good change to do because we will have to
create additional typedef at least for PRIu8. This will look ugly.

No, see my following diff

Also, if DPDK someday will create typedef with different name
we will have typedef of the typedef?

I don’t follow this comment; see the diff below.

The reasons for the typedef here are:
1) Easier to maintain if the size of type changes in future
2) Serves as documentation that the origin of the type is the rte library and
and originates externally to ovs

Here is a few examples of typedefs in the OVS code base:

dpif-netdev.c:5587:typedef uint32_t map_type;
stp.h:41:typedef uint64_t stp_identifier;
timeval.c:45:typedef unsigned int clockid_t;


dball@ubuntu:~/ovs$ git diff
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 609b8da..6667274 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -143,6 +143,9 @@ BUILD_ASSERT_DECL((MAX_NB_MBUF / 
ROUND_DOWN_POW2(MAX_NB_MBUF/MIN_NB_MBUF))
 #define VHOST_ENQ_RETRY_NUM 8
 #define IF_NAME_SZ (PATH_MAX > IFNAMSIZ ? PATH_MAX : IFNAMSIZ)
 
+#define DPDK_ETH_PORT_ID_INVALIDRTE_MAX_ETHPORTS
+typedef uint8_t rte_port_id;
+
 static const struct rte_eth_conf port_conf = {
 .rxmode = {
 .mq_mode = ETH_MQ_RX_RSS,
@@ -309,7 +312,7 @@ struct dpdk_ring {
 struct rte_ring *cring_tx;
 struct rte_ring *cring_rx;
 unsigned int user_port_id; /* User given port no, parsed from port name */
-int eth_port_id; /* ethernet device port id */
+rte_port_id eth_port_id; /* ethernet device port id */
 struct ovs_list list_node OVS_GUARDED_BY(dpdk_mutex);
 };
 
@@ -325,7 +328,7 @@ enum dpdk_hw_ol_features {
 
 struct netdev_dpdk {
 struct netdev up;
-int port_id;
+rte_port_id port_id;
 int max_packet_len;
 enum dpdk_dev_type type;
 
@@ -399,7 +402,7 @@ struct netdev_dpdk {
 
 struct netdev_rxq_dpdk {
 struct netdev_rxq up;
-int port_id;
+rte_port_id port_id;
 };
 
 static int netdev_dpdk_class_init(void);
@@ -600,12 +603,12 @@ check_link_status(struct netdev_dpdk *dev)
 dev->link_reset_cnt++;
 dev->link = link;
 if (dev->link.link_status) {
-VLOG_DBG_RL(&rl, "Port %d Link Up - speed %u Mbps - %s",
+VLOG_DBG_RL(&rl, "Port %"PRIu8" Link Up - speed %u Mbps - %s",
 dev->port_id, (unsigned) dev->link.link_speed,
 (dev->link.link_duplex == ETH_LINK_FULL_DUPLEX) ?
  ("full-duplex") : ("half-duplex"));
 } else {
-VLOG_DBG_RL(&rl, "Port %d Link Down", dev->port_id);
+VLOG_DBG_RL(&rl, "Port %"PRIu8" Link Down", dev->port_id);
 }
 }
 }
@@ -723,7 +726,7 @@ dpdk_eth_checksum_offload_configure(struct n

[ovs-dev] [PATCH v2 3/3] ovn-sbctl: support setting rbac role for remote connections

2017-05-12 Thread Lance Richardson
Add support for specifying rbac "role" when setting remote
connection configuration in the southbound database.

Prior to this change, usage examples included:

ovn-sbctl set-connection ptcp:6642
ovn-sbctl set-connection pssl:6642 \
 read-only ptcp: \
 read-write punix:/tmp.foo

With this change, in addition to the above:

ovn-sbctl set-connection role=ovn-controller pssl:6642 \
 read-only role= ptcp: \
 read-write punix:/tmp/foo

As with the "read-only"/"read-write" attributes, the specified
role is applied to all subsequent connections until changed.

Signed-off-by: Lance Richardson 
---
v2: No changes

 ovn/utilities/ovn-sbctl.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/ovn/utilities/ovn-sbctl.c b/ovn/utilities/ovn-sbctl.c
index 0142062..1fd259d 100644
--- a/ovn/utilities/ovn-sbctl.c
+++ b/ovn/utilities/ovn-sbctl.c
@@ -943,6 +943,7 @@ pre_connection(struct ctl_context *ctx)
 ovsdb_idl_add_column(ctx->idl, &sbrec_sb_global_col_connections);
 ovsdb_idl_add_column(ctx->idl, &sbrec_connection_col_target);
 ovsdb_idl_add_column(ctx->idl, &sbrec_connection_col_read_only);
+ovsdb_idl_add_column(ctx->idl, &sbrec_connection_col_role);
 }
 
 static void
@@ -960,8 +961,10 @@ cmd_get_connection(struct ctl_context *ctx)
 SBREC_CONNECTION_FOR_EACH(conn, ctx->idl) {
 char *s;
 
-s = xasprintf("%s %s", conn->read_only ? "read-only" : "read-write",
-   conn->target);
+s = xasprintf("%s role=\"%s\" %s",
+  conn->read_only ? "read-only" : "read-write",
+  conn->role,
+  conn->target);
 svec_add(&targets, s);
 free(s);
 }
@@ -1002,6 +1005,7 @@ insert_connections(struct ctl_context *ctx, char 
*targets[], size_t n)
 struct sbrec_connection **connections;
 size_t i, conns=0;
 bool read_only = false;
+char *role = "";
 
 /* Insert each connection in a new row in Connection table. */
 connections = xmalloc(n * sizeof *connections);
@@ -1012,6 +1016,9 @@ insert_connections(struct ctl_context *ctx, char 
*targets[], size_t n)
 } else if (!strcmp(targets[i], "read-write")) {
 read_only = false;
 continue;
+} else if (!strncmp(targets[i], "role=", 5)) {
+role = targets[i] + 5;
+continue;
 } else if (stream_verify_name(targets[i]) &&
pstream_verify_name(targets[i])) {
 VLOG_WARN("target type \"%s\" is possibly erroneous", targets[i]);
@@ -1020,6 +1027,7 @@ insert_connections(struct ctl_context *ctx, char 
*targets[], size_t n)
 connections[conns] = sbrec_connection_insert(ctx->txn);
 sbrec_connection_set_target(connections[conns], targets[i]);
 sbrec_connection_set_read_only(connections[conns], read_only);
+sbrec_connection_set_role(connections[conns], role);
 conns++;
 }
 
-- 
2.9.3

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


Re: [ovs-dev] [PATCH] bridge: Fix controller status update

2017-05-12 Thread Andy Zhou
On Thu, May 11, 2017 at 8:35 PM, Greg Rose  wrote:
> On Thu, 2017-05-11 at 16:16 -0700, Andy Zhou wrote:
>> When multiple bridges connects to the same controller, the controller
>> status should be maintained separately for each bridge. Current
>> logic pushes status updates only based on the connection string,
>> which is the same across multiple bridges when connecting to the
>> same controller.
>>
>> Report-at: 
>> https://mail.openvswitch.org/pipermail/ovs-discuss/2017-May/044412.html
>> Reported-by: Tulio Ribeiro 
>> Signed-off-by: Andy Zhou 
>> ---
>>  AUTHORS.rst   |  1 +
>>  tests/bridge.at   | 33 +
>>  vswitchd/bridge.c | 33 +++--
>>  3 files changed, 49 insertions(+), 18 deletions(-)
>>
>> diff --git a/AUTHORS.rst b/AUTHORS.rst
>> index 59639756b09f..147ce48d15ca 100644
>> --- a/AUTHORS.rst
>> +++ b/AUTHORS.rst
>> @@ -530,6 +530,7 @@ Teemu Koponen   kopo...@nicira.com
>>  Thomas Morinthomas.mo...@orange.com
>>  Timothy Chentc...@nicira.com
>>  Torbjorn Tornkvist  kruska...@gmail.com
>> +Tulio Ribeiro   tribe...@lasige.di.fc.ul.pt
>>  Tytus Kurek tytus.ku...@pega.com
>>  Valentin Budvalen...@hackaserver.com
>>  Vasiliy Tolstov v.tols...@selfip.ru
>> diff --git a/tests/bridge.at b/tests/bridge.at
>> index 3dbabe511780..58b27d445062 100644
>> --- a/tests/bridge.at
>> +++ b/tests/bridge.at
>> @@ -38,3 +38,36 @@ dummy@ovs-dummy: hit:0 missed:0
>>  OVS_APP_EXIT_AND_WAIT([ovs-vswitchd])
>>  OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>>  AT_CLEANUP
>> +
>> +dnl When multiple bridges are connected to the same controller, make
>> +dnl sure their status are tracked independently.
>> +AT_SETUP([bridge - multiple bridges share a controller])
>> +OVS_VSWITCHD_START(
>> +   [add-br br1 -- \
>> +set bridge br1 other-config:hwaddr=aa:66:aa:66:00:00 -- \
>> +set bridge br1 datapath-type=dummy other-config:datapath-id=1234 ])
>> +
>> +dnl Start ovs-testcontroller
>> +AT_CHECK([ovs-testcontroller --detach punix:controller], [0], [ignore])
>> +
>> +dnl Add the controller to both bridges, 5 seconds apart.
>> +AT_CHECK([ovs-vsctl set-controller br0 unix:controller])
>> +AT_CHECK([ovs-appctl time/warp 5000], [0], [ignore])
>> +AT_CHECK([ovs-vsctl set-controller br1 unix:controller])
>> +
>> +dnl Wait for the controller connection to come up
>> +for i in `seq 0 7`
>> +do
>> +AT_CHECK([ovs-appctl time/warp 10], [0], [ignore])
>> +done
>> +
>> +dnl Make sure the connection status are different
>> +AT_CHECK([ovs-vsctl --columns=status list controller | sort], [0], [dnl
>> +
>> +status  : {sec_since_connect="0", state=ACTIVE}
>> +status  : {sec_since_connect="5", state=ACTIVE}
>> +])
>> +
>> +OVS_APP_EXIT_AND_WAIT([ovs-vswitchd])
>> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>> +AT_CLEANUP
>> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
>> index 31203d1ec232..972146e8da6d 100644
>> --- a/vswitchd/bridge.c
>> +++ b/vswitchd/bridge.c
>> @@ -2704,34 +2704,31 @@ static void
>>  refresh_controller_status(void)
>>  {
>>  struct bridge *br;
>> -struct shash info;
>> -const struct ovsrec_controller *cfg;
>> -
>> -shash_init(&info);
>>
>>  /* Accumulate status for controllers on all bridges. */
>>  HMAP_FOR_EACH (br, node, &all_bridges) {
>> +struct shash info = SHASH_INITIALIZER(&info);
>>  ofproto_get_ofproto_controller_info(br->ofproto, &info);
>> -}
>>
>> -/* Update each controller in the database with current status. */
>> -OVSREC_CONTROLLER_FOR_EACH(cfg, idl) {
>> -struct ofproto_controller_info *cinfo =
>> -shash_find_data(&info, cfg->target);
>> +/* Update each controller of the bridge in the database with
>> + * current status. */
>> +struct ovsrec_controller **controllers;
>> +size_t n_controllers = bridge_get_controllers(br, &controllers);
>> +size_t i;
>> +for (i = 0; i < n_controllers; i++) {
>> +struct ovsrec_controller *cfg = controllers[i];
>> +struct ofproto_controller_info *cinfo =
>> +shash_find_data(&info, cfg->target);
>>
>> -if (cinfo) {
>> +ovs_assert(cinfo);
>>  ovsrec_controller_set_is_connected(cfg, cinfo->is_connected);
>> -ovsrec_controller_set_role(cfg, ofp12_controller_role_to_str(
>> -   cinfo->role));
>> +const char *role = ofp12_controller_role_to_str(cinfo->role);
>> +ovsrec_controller_set_role(cfg, role);
>>  ovsrec_controller_set_status(cfg, &cinfo->pairs);
>> -} else {
>> -ovsrec_controller_set_is_connected(cfg, false);
>> -ovsrec_controller_set_role(cfg, NULL);
>> -ovsrec_controller_set_status(cfg, NULL);
>>  }
>> -}
>>
>> -ofprot

[ovs-dev] [PATCH v2 2/3] ovn: add rbac tables to ovn southbound schema

2017-05-12 Thread Lance Richardson
Add rbac "roles" and "permissions" tables to ovn southbound
database schema, add support to ovn-northd for managing these
tables.

Signed-off-by: Lance Richardson 
---
 v2: Rebased, added reference to ovsdb-server(1) to ovn-architecture(7).

 ovn/northd/ovn-northd.c| 190 +
 ovn/ovn-architecture.7.xml | 156 +
 ovn/ovn-sb.ovsschema   |  26 ++-
 ovn/ovn-sb.xml |  40 ++
 4 files changed, 410 insertions(+), 2 deletions(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 83db753..00f3731 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -5802,6 +5802,182 @@ check_and_add_supported_dhcpv6_opts_to_sb_db(struct 
northd_context *ctx)
 hmap_destroy(&dhcpv6_opts_to_add);
 }
 
+static const char *rbac_chassis_auth[] =
+{"name"};
+static const char *rbac_chassis_update[] =
+{"nb_cfg", "external_ids", "encaps", "vtep_logical_switches"};
+
+static const char *rbac_encap_auth[] =
+{""};
+static const char *rbac_encap_update[] =
+{"type", "options", "ip"};
+
+static const char *rbac_port_binding_auth[] =
+{""};
+static const char *rbac_port_binding_update[] =
+{"chassis"};
+
+static const char *rbac_mac_binding_auth[] =
+{""};
+static const char *rbac_mac_binding_update[] =
+{"logical_port", "ip", "mac", "datapath"};
+
+static struct rbac_perm_cfg {
+const char *table;
+const char **auth;
+int n_auth;
+bool insdel;
+const char **update;
+int n_update;
+const struct sbrec_rbac_permission *row;
+} rbac_perm_cfg[] = {
+{
+"Chassis",
+rbac_chassis_auth,
+ARRAY_SIZE(rbac_chassis_auth),
+true,
+rbac_chassis_update,
+ARRAY_SIZE(rbac_chassis_update),
+NULL
+},{
+"Encap",
+rbac_encap_auth,
+ARRAY_SIZE(rbac_encap_auth),
+true,
+rbac_encap_update,
+ARRAY_SIZE(rbac_encap_update),
+NULL
+},{
+"Port_Binding",
+rbac_port_binding_auth,
+ARRAY_SIZE(rbac_port_binding_auth),
+false,
+rbac_port_binding_update,
+ARRAY_SIZE(rbac_port_binding_update),
+NULL
+},{
+"MAC_Binding",
+rbac_mac_binding_auth,
+ARRAY_SIZE(rbac_mac_binding_auth),
+true,
+rbac_mac_binding_update,
+ARRAY_SIZE(rbac_mac_binding_update),
+NULL
+},
+{}
+};
+
+static bool
+ovn_rbac_validate_perm(const struct sbrec_rbac_permission *perm)
+{
+struct rbac_perm_cfg *pcfg;
+int i, j, n_found;
+
+for (pcfg = rbac_perm_cfg; pcfg->table; pcfg++) {
+if (!strcmp(perm->table, pcfg->table)) {
+break;
+}
+}
+if (!pcfg->table) {
+return false;
+}
+if (perm->n_authorization != pcfg->n_auth ||
+perm->n_update != pcfg->n_update) {
+return false;
+}
+if (perm->insert_delete != pcfg->insdel) {
+return false;
+}
+/* verify perm->authorization vs. pcfg->auth */
+n_found = 0;
+for (i = 0; i < pcfg->n_auth; i++) {
+for (j = 0; j < perm->n_authorization; j++) {
+if (!strcmp(pcfg->auth[i], perm->authorization[j])) {
+n_found++;
+break;
+}
+}
+}
+if (n_found != pcfg->n_auth) {
+return false;
+}
+
+/* verify perm->update vs. pcfg->update */
+n_found = 0;
+for (i = 0; i < pcfg->n_update; i++) {
+for (j = 0; j < perm->n_update; j++) {
+if (!strcmp(pcfg->update[i], perm->update[j])) {
+n_found++;
+break;
+}
+}
+}
+if (n_found != pcfg->n_update) {
+return false;
+}
+
+/* Success, db state matches expected state */
+pcfg->row = perm;
+return true;
+}
+
+static void
+ovn_rbac_create_perm(struct rbac_perm_cfg *pcfg,
+ struct northd_context *ctx,
+ const struct sbrec_rbac_role *rbac_role)
+{
+struct sbrec_rbac_permission *rbac_perm;
+
+rbac_perm = sbrec_rbac_permission_insert(ctx->ovnsb_txn);
+sbrec_rbac_permission_set_table(rbac_perm, pcfg->table);
+sbrec_rbac_permission_set_authorization(rbac_perm,
+pcfg->auth,
+pcfg->n_auth);
+sbrec_rbac_permission_set_insert_delete(rbac_perm, pcfg->insdel);
+sbrec_rbac_permission_set_update(rbac_perm,
+ pcfg->update,
+ pcfg->n_update);
+sbrec_rbac_role_update_permissions_setkey(rbac_role, pcfg->table,
+  rbac_perm);
+}
+
+static void
+check_and_update_rbac(struct northd_context *ctx)
+{
+const struct sbrec_rbac_role *rbac_role = NULL;
+const struct sbrec_rbac_permission *perm_row, *perm_next;
+const struct sbrec_rbac_role *role_

[ovs-dev] [PATCH v2 1/3] ovsdb: add support for role-based access controls

2017-05-12 Thread Lance Richardson
Add suport for ovsdb RBAC (role-based access control). This includes:

   - Support for "RBAC_Role" table. A db schema containing a table
 by this name will enable role-based access controls using
 this table for RBAC role configuration.

 The "RBAC_Role" table has one row per role, with each row having a
 "name" column (role name) and a "permissions" column (map of
 table name to UUID of row in separate permission table.) The
 permission table has one row per access control configuration,
 with the following columns:
  "name"  - name of table to which this row applies
  "authorization" - set of column names and column:key pairs
to be compared against client ID to
determine authorization status
  "insert_delete" - boolean, true if insertions and
authorized deletions are allowed.
  "update"- Set of columns and column:key pairs for
which authorized updates are allowed.
   - Support for a new "role" column in the remote configuration
 table.
   - Logic for applying the RBAC role and permission tables, in
 combination with session role from the remote connection table
 and client id, to determine whether operations modifying database
 contents should be permitted.
   - Support for specifying RBAC role string as a command-line option
 to ovsdb-tool (Ben Pfaff).

Signed-off-by: Lance Richardson 
---

Note to Ben: with your incremental patch, I think your co-authored-by and
signed-off-by are probably needed... if you agree, please add.

v2:
  - Folded in incremental patch from Ben Pfaff with style cleanup, a
bug fix, and addition of --rbac-role option to ovsdb-tool.
  - Documented differences with respect to RFC 7074 in ovsdb-server(1).

 lib/jsonrpc.c   |  10 ++
 lib/jsonrpc.h   |   1 +
 lib/ovsdb-error.c   |  13 ++
 lib/ovsdb-error.h   |   4 +
 lib/ovsdb-idl.c |   6 +
 ovsdb/automake.mk   |   2 +
 ovsdb/execution.c   |  43 -
 ovsdb/jsonrpc-server.c  |   6 +-
 ovsdb/jsonrpc-server.h  |   1 +
 ovsdb/ovsdb-server.1.in |  45 +
 ovsdb/ovsdb-server.c|   8 +-
 ovsdb/ovsdb-tool.1.in   |  10 +-
 ovsdb/ovsdb-tool.c  |  23 ++-
 ovsdb/ovsdb-util.c  |  31 
 ovsdb/ovsdb-util.h  |   4 +
 ovsdb/ovsdb.c   |   3 +
 ovsdb/ovsdb.h   |   3 +
 ovsdb/rbac.c| 444 
 ovsdb/rbac.h|  46 +
 ovsdb/trigger.c |   8 +-
 ovsdb/trigger.h |   5 +-
 tests/automake.mk   |   1 +
 tests/ovsdb-rbac.at | 358 ++
 tests/ovsdb.at  |   1 +
 tests/test-ovsdb.c  |   5 +-
 25 files changed, 1065 insertions(+), 16 deletions(-)
 create mode 100644 ovsdb/rbac.c
 create mode 100644 ovsdb/rbac.h
 create mode 100644 tests/ovsdb-rbac.at

diff --git a/lib/jsonrpc.c b/lib/jsonrpc.c
index a0ade9c..2fae057 100644
--- a/lib/jsonrpc.c
+++ b/lib/jsonrpc.c
@@ -1005,6 +1005,16 @@ jsonrpc_session_get_name(const struct jsonrpc_session *s)
 return reconnect_get_name(s->reconnect);
 }
 
+const char *
+jsonrpc_session_get_id(const struct jsonrpc_session *s)
+{
+if (s->rpc && s->rpc->stream) {
+return stream_get_peer_id(s->rpc->stream);
+} else {
+return NULL;
+}
+}
+
 /* Always takes ownership of 'msg', regardless of success. */
 int
 jsonrpc_session_send(struct jsonrpc_session *s, struct jsonrpc_msg *msg)
diff --git a/lib/jsonrpc.h b/lib/jsonrpc.h
index 982017a..9b4fb0e 100644
--- a/lib/jsonrpc.h
+++ b/lib/jsonrpc.h
@@ -130,5 +130,6 @@ void jsonrpc_session_set_probe_interval(struct 
jsonrpc_session *,
 int probe_interval);
 void jsonrpc_session_set_dscp(struct jsonrpc_session *,
   uint8_t dscp);
+const char *jsonrpc_session_get_id(const struct jsonrpc_session *);
 
 #endif /* jsonrpc.h */
diff --git a/lib/ovsdb-error.c b/lib/ovsdb-error.c
index dfa4249..d8161e6 100644
--- a/lib/ovsdb-error.c
+++ b/lib/ovsdb-error.c
@@ -167,6 +167,19 @@ ovsdb_internal_error(struct ovsdb_error *inner_error,
 return error;
 }
 
+struct ovsdb_error *
+ovsdb_perm_error(const char *details, ...)
+{
+struct ovsdb_error *error;
+va_list args;
+
+va_start(args, details);
+error = ovsdb_error_valist("permission error", details, args);
+va_end(args);
+
+return error;
+}
+
 void
 ovsdb_error_destroy(struct ovsdb_error *error)
 {
diff --git a/lib/ovsdb-error.h b/lib/ovsdb-error.h
index 2bc259a..da91b74 100644
--- a/lib/ovsdb-error.h
+++ b/lib/ovsdb-error.h
@@ -41,6 +41,10 @@ struct ovsdb_error *ovsdb_internal_error(struct ovsdb_error 
*error,
 OVS_PRINTF_FORMAT(4, 5)
 OVS_WARN_UNUSED_RESULT;
 
+struct ovsdb_error *ovsdb_perm_error(const char *details, ...)
+OVS_PRINTF_FORMAT(1, 2)
+OVS_WARN_UNUSED_RESULT;
+
 /* Returns a pointer t

[ovs-dev] [PATCH v2 0/3] role-based access controls for ovsdb-server, ovn-sb

2017-05-12 Thread Lance Richardson
This series implements role-based access control infrastructure for
ovsdb-server, and uses that infrastructure to apply role-based access
controls to the OVN_Southbound database. This implementation follows
the outline discussed at:

 https://mail.openvswitch.org/pipermail/ovs-dev/2017-March/329801.html

With this series applied, enabling role-based ACLs is a matter of:

- Configuring southbound ovsdb-server and ovn-controller to use SSL,
  configuring an ovn-controller "role" for SSL connections via e.g.:
 ovn-sbctl set-connection role=ovn-controller pssl:6642
- Using unique certificates for each ovn-controller with a unique
  CN for each chassis, generated e.g. via:
 ovs-pki -B 1024 -u req+sign chassis1 switch
 ovs-pki -B 1024 -u req+sign chassis2 switch
 ovs-pki -B 1024 -u req+sign chassis3 switch

Lance Richardson (3):
  ovsdb: add support for role-based access controls
  ovn: add rbac tables to ovn southbound schema
  ovn-sbctl: support setting rbac role for remote connections

 lib/jsonrpc.c  |  10 +
 lib/jsonrpc.h  |   1 +
 lib/ovsdb-error.c  |  13 ++
 lib/ovsdb-error.h  |   4 +
 lib/ovsdb-idl.c|   6 +
 ovn/northd/ovn-northd.c| 190 +++
 ovn/ovn-architecture.7.xml | 156 
 ovn/ovn-sb.ovsschema   |  26 ++-
 ovn/ovn-sb.xml |  40 
 ovn/utilities/ovn-sbctl.c  |  12 +-
 ovsdb/automake.mk  |   2 +
 ovsdb/execution.c  |  43 -
 ovsdb/jsonrpc-server.c |   6 +-
 ovsdb/jsonrpc-server.h |   1 +
 ovsdb/ovsdb-server.1.in|  45 +
 ovsdb/ovsdb-server.c   |   8 +-
 ovsdb/ovsdb-tool.1.in  |  10 +-
 ovsdb/ovsdb-tool.c |  23 ++-
 ovsdb/ovsdb-util.c |  31 
 ovsdb/ovsdb-util.h |   4 +
 ovsdb/ovsdb.c  |   3 +
 ovsdb/ovsdb.h  |   3 +
 ovsdb/rbac.c   | 444 +
 ovsdb/rbac.h   |  46 +
 ovsdb/trigger.c|   8 +-
 ovsdb/trigger.h|   5 +-
 tests/automake.mk  |   1 +
 tests/ovsdb-rbac.at| 358 
 tests/ovsdb.at |   1 +
 tests/test-ovsdb.c |   5 +-
 30 files changed, 1485 insertions(+), 20 deletions(-)
 create mode 100644 ovsdb/rbac.c
 create mode 100644 ovsdb/rbac.h
 create mode 100644 tests/ovsdb-rbac.at

-- 
2.9.3

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


[ovs-dev] AFFORDABLE LOAN AVAILABLE APPLY

2017-05-12 Thread Private Lender
DO YOU NEED ANY FINANCIAL ASSISTANCE? IF YES THEN EMAIL FOR DETAILS
-- 
Best Regards

Mr. Jason Drew

Multi Finance Loan Company
 
Address: 764 Fayetteville St, Durham, NC 27701, USA
Contact Telephone: (+1) 919-701-6288

=
Note:This message contains information which may be privileged. Unless you are 
the addressee (or are authorized to receive for the addressee), you may not 
use, copy, or disclose to anyone, the message or information contained therein. 
 If you have received this message in error, please advise the sender by reply 
email/fax and delete/discard the message.
Thank You.
-
 (c) 2017















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


[ovs-dev] ATTACHED IS YOUR LETTER.

2017-05-12 Thread © 2018 Russia


   








   

   

   

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


[ovs-dev] [PATCH v2 3/3] netdev-dpdk: Move Rx checksum config and make generic.

2017-05-12 Thread Kevin Traynor
Checking requested Rx checksum config and whether it should be
applied to the eth device was split partially between the higher
level set config function and a dedicated Rx checksum function.

Refactor that by moving all Rx checksum related config into one
function and make it generic so that it could easily be extended
for other Hw offloads that can be set during DPDK eth dev configure.

Cc: Sugesh Chandran 
Signed-off-by: Kevin Traynor 
---
 lib/netdev-dpdk.c | 53 -
 1 file changed, 32 insertions(+), 21 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 465bb82..76fb6f8 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -712,23 +712,43 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int 
n_rxq, int n_txq)
 
 static void
-dpdk_eth_checksum_offload_configure(struct netdev_dpdk *dev)
+dpdk_eth_hwol_offload_configure(struct netdev_dpdk *dev,
+const struct smap *args)
 OVS_REQUIRES(dev->mutex)
 {
 struct rte_eth_dev_info info;
-bool rx_csum_ol_flag = false;
-uint32_t rx_chksm_offload_capa = DEV_RX_OFFLOAD_UDP_CKSUM |
- DEV_RX_OFFLOAD_TCP_CKSUM |
- DEV_RX_OFFLOAD_IPV4_CKSUM;
+bool rx_csum_ol_req;
+uint32_t rx_csum_ol_mask = DEV_RX_OFFLOAD_UDP_CKSUM |
+   DEV_RX_OFFLOAD_TCP_CKSUM |
+   DEV_RX_OFFLOAD_IPV4_CKSUM;
+
+/* Check if requested rx-checksum is same as current rx-checksum config. */
+rx_csum_ol_req = smap_get_bool(args, "rx-checksum-offload", true);
+if (rx_csum_ol_req !=
+((dev->hw_ol_features & NETDEV_RX_CHECKSUM_OFFLOAD) != 0)) {
+
+dev->requested_hwol ^= NETDEV_RX_CHECKSUM_OFFLOAD;
+}
+
+/* Check if there is a requested change in hwol features. */
+if (dev->requested_hwol == dev->hw_ol_features) {
+return;
+}
+
+   /* Change the requested hwol features to only allow supported features. */
 rte_eth_dev_info_get(dev->port_id, &info);
-rx_csum_ol_flag = (dev->requested_hwol & NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
-
-if (rx_csum_ol_flag &&
-(info.rx_offload_capa & rx_chksm_offload_capa) !=
- rx_chksm_offload_capa) {
+/* rx-checksum offload. */
+if (rx_csum_ol_req &&
+(info.rx_offload_capa & rx_csum_ol_mask) !=
+ rx_csum_ol_mask) {
 VLOG_WARN_ONCE("Rx checksum offload is not supported on device %d",
dev->port_id);
 dev->requested_hwol &= ~NETDEV_RX_CHECKSUM_OFFLOAD;
+}
+
+/* Check if differences between requested and current hwol features. */
+if (dev->requested_hwol == dev->hw_ol_features) {
 return;
 }
+
 netdev_request_reconfigure(&dev->up);
 }
@@ -1180,6 +1200,4 @@ netdev_dpdk_set_config(struct netdev *netdev, const 
struct smap *args,
 {RTE_FC_RX_PAUSE, RTE_FC_FULL}
 };
-bool rx_chksm_ofld;
-bool temp_flag;
 const char *new_devargs;
 int err = 0;
@@ -1259,13 +1277,6 @@ netdev_dpdk_set_config(struct netdev *netdev, const 
struct smap *args,
 }
 
-/* Rx checksum offload configuration */
-/* By default the Rx checksum offload is ON */
-rx_chksm_ofld = smap_get_bool(args, "rx-checksum-offload", true);
-temp_flag = (dev->hw_ol_features & NETDEV_RX_CHECKSUM_OFFLOAD)
-!= 0;
-if (temp_flag != rx_chksm_ofld) {
-dev->requested_hwol ^= NETDEV_RX_CHECKSUM_OFFLOAD;
-dpdk_eth_checksum_offload_configure(dev);
-}
+/* Process any eth dev hwol configuration. */
+dpdk_eth_hwol_offload_configure(dev, args);
 
 out:
-- 
1.8.3.1

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


[ovs-dev] [PATCH v2 2/3] netdev-dpdk: Show Rx checksum status when false.

2017-05-12 Thread Kevin Traynor
Currently ovs-appctl dpctl/show only shows the Rx checksum offload
status when true. Change to also show the status when false.

Cc: Sugesh Chandran 
Signed-off-by: Kevin Traynor 
---
 lib/netdev-dpdk.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index d1688ce..465bb82 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1093,4 +1093,6 @@ netdev_dpdk_get_config(const struct netdev *netdev, 
struct smap *args)
 if (dev->hw_ol_features & NETDEV_RX_CHECKSUM_OFFLOAD) {
 smap_add(args, "rx_csum_offload", "true");
+} else {
+smap_add(args, "rx_csum_offload", "false");
 }
 }
-- 
1.8.3.1

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


[ovs-dev] [PATCH v2 1/3] netdev-dpdk: Fix Rx checksum reconfigure.

2017-05-12 Thread Kevin Traynor
Rx checksum offload is enabled by default where available, with
reconfiguration through OVSDB options:rx-checksum-offload.
However, setting rx-checksum-offload does not result in a
reconfiguration of the NIC.

Fix that by checking if the requested port config features
(e.g. rx checksum offload) are currently applied on the NIC and if
not, perform a reconfiguration.

Fixes: 1a2bb11817a4 ("netdev-dpdk: Enable Rx checksum offloading feature on 
DPDK physical ports.")
Cc: Sugesh Chandran 
Signed-off-by: Kevin Traynor 
---
 lib/netdev-dpdk.c | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 609b8da..d1688ce 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -374,4 +374,5 @@ struct netdev_dpdk {
 int requested_rxq_size;
 int requested_txq_size;
+uint32_t requested_hwol;
 
 /* Number of rx/tx descriptors for physical devices */
@@ -648,5 +649,5 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int 
n_rxq, int n_txq)
 conf.rxmode.max_rx_pkt_len = 0;
 }
-conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
+conf.rxmode.hw_ip_checksum = (dev->requested_hwol &
   NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
 /* A device may report more queues than it makes available (this has
@@ -702,4 +703,5 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int 
n_rxq, int n_txq)
 dev->up.n_rxq = n_rxq;
 dev->up.n_txq = n_txq;
+dev->hw_ol_features = dev->requested_hwol;
 
 return 0;
@@ -719,5 +721,5 @@ dpdk_eth_checksum_offload_configure(struct netdev_dpdk *dev)
  DEV_RX_OFFLOAD_IPV4_CKSUM;
 rte_eth_dev_info_get(dev->port_id, &info);
-rx_csum_ol_flag = (dev->hw_ol_features & NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
+rx_csum_ol_flag = (dev->requested_hwol & NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
 
 if (rx_csum_ol_flag &&
@@ -726,5 +728,5 @@ dpdk_eth_checksum_offload_configure(struct netdev_dpdk *dev)
 VLOG_WARN_ONCE("Rx checksum offload is not supported on device %d",
dev->port_id);
-dev->hw_ol_features &= ~NETDEV_RX_CHECKSUM_OFFLOAD;
+dev->requested_hwol &= ~NETDEV_RX_CHECKSUM_OFFLOAD;
 return;
 }
@@ -872,4 +874,5 @@ common_construct(struct netdev *netdev, unsigned int 
port_no,
 /* Initilize the hardware offload flags to 0 */
 dev->hw_ol_features = 0;
+dev->requested_hwol = 0;
 
 dev->flags = NETDEV_UP | NETDEV_PROMISC;
@@ -1260,5 +1263,5 @@ netdev_dpdk_set_config(struct netdev *netdev, const 
struct smap *args,
 != 0;
 if (temp_flag != rx_chksm_ofld) {
-dev->hw_ol_features ^= NETDEV_RX_CHECKSUM_OFFLOAD;
+dev->requested_hwol ^= NETDEV_RX_CHECKSUM_OFFLOAD;
 dpdk_eth_checksum_offload_configure(dev);
 }
@@ -3124,5 +3127,6 @@ netdev_dpdk_reconfigure(struct netdev *netdev)
 && dev->rxq_size == dev->requested_rxq_size
 && dev->txq_size == dev->requested_txq_size
-&& dev->socket_id == dev->requested_socket_id) {
+&& dev->socket_id == dev->requested_socket_id
+&& dev->hw_ol_features == dev->requested_hwol) {
 /* Reconfiguration is unnecessary */
 
-- 
1.8.3.1

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


[ovs-dev] [PATCH v2 0/3] Rx checksum config fix and refactor.

2017-05-12 Thread Kevin Traynor
V2: Added 3/3 to refactor Rx checksum config code and make generic.

Kevin Traynor (3):
  netdev-dpdk: Fix Rx checksum reconfigure.
  netdev-dpdk: Show Rx checksum status when false.
  netdev-dpdk: Move Rx checksum config and make generic.

 lib/netdev-dpdk.c | 65 +++
 1 file changed, 41 insertions(+), 24 deletions(-)

-- 
1.8.3.1

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


[ovs-dev] `ovs-ofctl replace-flows` unnecessarily replaces some flows

2017-05-12 Thread Kevin Lin
Hi,

We’re running OVS2.6.1. If we insert a flow with an action such as 
resubmit(,1), then call replace-flows with the same flow, it gets replaced. 
Specifically, the duration shown in `ovs-ofctl dump-flows` resets to zero.

I took a look around, and it looks like this is the same bug as one for 
`diff-flows` that Ben fixed awhile back 
(https://github.com/openvswitch/ovs/commit/98f7f427bf8bb339d4d1f1df1ff9b3310f8f5bc4
 
).

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


[ovs-dev] [PATCH v2 3/3] netdev-dpdk: Use uint8_t for port_id.

2017-05-12 Thread Ilya Maximets
Currently, signed integer is used for 'port_id' variable and
'-1' as identifier of bad or uninitialized 'port_id'.

This inconsistent with dpdk library and, also, in few cases,
leads to passing '-1' to dpdk functions where uint8_t expected.

Such behaviour doesn't produce any issues, but it's better to
use same type as in dpdk library for consistency.

Also, magic number '-1' replaced with DPDK_ETH_PORT_ID_INVALID
macro.

Signed-off-by: Ilya Maximets 
---
 lib/netdev-dpdk.c | 59 +++
 1 file changed, 33 insertions(+), 26 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index baac7c3..1ed25b2 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -140,6 +140,8 @@ BUILD_ASSERT_DECL((MAX_NB_MBUF / 
ROUND_DOWN_POW2(MAX_NB_MBUF/MIN_NB_MBUF))
 #define OVS_VHOST_QUEUE_DISABLED(-2) /* Queue was disabled by guest and not
   * yet mapped to another queue. */
 
+#define DPDK_ETH_PORT_ID_INVALIDRTE_MAX_ETHPORTS
+
 #define VHOST_ENQ_RETRY_NUM 8
 #define IF_NAME_SZ (PATH_MAX > IFNAMSIZ ? PATH_MAX : IFNAMSIZ)
 
@@ -309,7 +311,7 @@ struct dpdk_ring {
 struct rte_ring *cring_tx;
 struct rte_ring *cring_rx;
 unsigned int user_port_id; /* User given port no, parsed from port name */
-int eth_port_id; /* ethernet device port id */
+uint8_t eth_port_id; /* ethernet device port id */
 struct ovs_list list_node OVS_GUARDED_BY(dpdk_mutex);
 };
 
@@ -325,7 +327,7 @@ enum dpdk_hw_ol_features {
 
 struct netdev_dpdk {
 struct netdev up;
-int port_id;
+uint8_t port_id;
 int max_packet_len;
 enum dpdk_dev_type type;
 
@@ -402,7 +404,7 @@ struct netdev_dpdk {
 
 struct netdev_rxq_dpdk {
 struct netdev_rxq up;
-int port_id;
+uint8_t port_id;
 };
 
 static int netdev_dpdk_class_init(void);
@@ -603,12 +605,12 @@ check_link_status(struct netdev_dpdk *dev)
 dev->link_reset_cnt++;
 dev->link = link;
 if (dev->link.link_status) {
-VLOG_DBG_RL(&rl, "Port %d Link Up - speed %u Mbps - %s",
+VLOG_DBG_RL(&rl, "Port %"PRIu8" Link Up - speed %u Mbps - %s",
 dev->port_id, (unsigned) dev->link.link_speed,
 (dev->link.link_duplex == ETH_LINK_FULL_DUPLEX) ?
  ("full-duplex") : ("half-duplex"));
 } else {
-VLOG_DBG_RL(&rl, "Port %d Link Down", dev->port_id);
+VLOG_DBG_RL(&rl, "Port %"PRIu8" Link Down", dev->port_id);
 }
 }
 }
@@ -726,8 +728,8 @@ dpdk_eth_checksum_offload_configure(struct netdev_dpdk *dev)
 if (rx_csum_ol_flag &&
 (info.rx_offload_capa & rx_chksm_offload_capa) !=
  rx_chksm_offload_capa) {
-VLOG_WARN_ONCE("Rx checksum offload is not supported on device %d",
-   dev->port_id);
+VLOG_WARN_ONCE("Rx checksum offload is not supported on device %"PRIu8,
+   dev->port_id);
 dev->hw_ol_features &= ~NETDEV_RX_CHECKSUM_OFFLOAD;
 return;
 }
@@ -738,7 +740,8 @@ static void
 dpdk_eth_flow_ctrl_setup(struct netdev_dpdk *dev) OVS_REQUIRES(dev->mutex)
 {
 if (rte_eth_dev_flow_ctrl_set(dev->port_id, &dev->fc_conf)) {
-VLOG_WARN("Failed to enable flow control on device %d", dev->port_id);
+VLOG_WARN("Failed to enable flow control on device %"PRIu8,
+  dev->port_id);
 }
 }
 
@@ -776,7 +779,7 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
 
 memset(ð_addr, 0x0, sizeof(eth_addr));
 rte_eth_macaddr_get(dev->port_id, ð_addr);
-VLOG_INFO_RL(&rl, "Port %d: "ETH_ADDR_FMT,
+VLOG_INFO_RL(&rl, "Port %"PRIu8": "ETH_ADDR_FMT,
 dev->port_id, ETH_ADDR_BYTES_ARGS(eth_addr.addr_bytes));
 
 memcpy(dev->hwaddr.ea, eth_addr.addr_bytes, ETH_ADDR_LEN);
@@ -788,7 +791,7 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
 /* Get the Flow control configuration for DPDK-ETH */
 diag = rte_eth_dev_flow_ctrl_get(dev->port_id, &dev->fc_conf);
 if (diag) {
-VLOG_DBG("cannot get flow control parameters on port=%d, err=%d",
+VLOG_DBG("cannot get flow control parameters on port=%"PRIu8", err=%d",
  dev->port_id, diag);
 }
 
@@ -833,7 +836,7 @@ netdev_dpdk_alloc_txq(unsigned int n_txqs)
 }
 
 static int
-common_construct(struct netdev *netdev, unsigned int port_no,
+common_construct(struct netdev *netdev, uint8_t port_no,
  enum dpdk_dev_type type, int socket_id)
 OVS_REQUIRES(dpdk_mutex)
 {
@@ -918,7 +921,8 @@ vhost_common_construct(struct netdev *netdev)
 return ENOMEM;
 }
 
-return common_construct(netdev, -1, DPDK_DEV_VHOST, socket_id);
+return common_construct(netdev, DPDK_ETH_PORT_ID_INVALID,
+DPDK_DEV_VHOST, socket_id);
 }
 
 static int
@@ -978,7 +982,8 @@ netdev_dpdk_construct(struct netdev *netdev)
 int err;
 
 ovs_mutex_lock(&dpdk_mutex);
-err = common_constru

[ovs-dev] [PATCH v2 2/3] netdev-dpdk: Fix device leak on port deletion.

2017-05-12 Thread Ilya Maximets
Currently, once created device in dpdk will exist forever
even after del-port operation untill we manually call
'ovs-appctl netdev-dpdk/detach ', where  is not
the port's name but the name of dpdk eth device or pci address.

Few issues with current implementation:

1. Different API for usual (system) and DPDK devices.
   (We have to call 'ovs-appctl netdev-dpdk/detach' each
time after 'del-port' to actually free the device)
   This is a big issue mostly for virtual DPDK devices.

2. Follows from 1:
   For DPDK devices 'del-port' leads just to
   'rte_eth_dev_stop' and subsequent 'add-port' will
   just start the already existing device. Such behaviour
   will not reset the device to initial state as it could
   be expected. For example: virtual pcap pmd will continue
   reading input file instead of reading it from the beginning.

3. Follows from 2:
   After execution of the following commands 'port1' will be
   configured with the 'old-options' while 'ovs-vsctl show'
   will show us 'new-options' in dpdk-devargs field:

 ovs-vsctl add-port port1 -- set interface port1 type=dpdk \
   options:dpdk-devargs=,
 ovs-vsctl del-port port1
 ovs-vsctl add-port port1 -- set interface port1 type=dpdk \
   options:dpdk-devargs=,

4. Follows from 1:
   Not detached device consumes 'port_id'. Since we have very
   limited number of 'port_id's (32 in common case) this may
   lead to quick exhausting of id pool and inability to add any
   other port.

To avoid above issues we need to detach all the attached devices on
port destruction.
appctl 'netdev-dpdk/detach' removed because not needed anymore.

We need to use internal 'attached' variable to track ports on
which rte_eth_dev_attach() was called and returned successfully
to avoid closing and detaching devices that do not support hotplug or
by any other reason attached using the 'dpdk-extra' cmdline options.

CC: Ciara Loftus 
Fixes: 55e075e65ef9 ("netdev-dpdk: Arbitrary 'dpdk' port naming")
Fixes: 69876ed78611 ("netdev-dpdk: Add support for virtual DPDK PMDs (vdevs)")
Signed-off-by: Ilya Maximets 
---
 Documentation/howto/dpdk.rst |  5 ++-
 lib/netdev-dpdk.c| 72 
 2 files changed, 22 insertions(+), 55 deletions(-)

diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
index dc63f7d..20d8975 100644
--- a/Documentation/howto/dpdk.rst
+++ b/Documentation/howto/dpdk.rst
@@ -342,10 +342,9 @@ Then it can be attached to OVS::
 $ ovs-vsctl add-port br0 dpdkx -- set Interface dpdkx type=dpdk \
 options:dpdk-devargs=:01:00.0
 
-It is also possible to detach a port from ovs, the user has to remove the
-port using the del-port command, then it can be detached using::
+Detaching will be performed while processing del-port command::
 
-$ ovs-appctl netdev-dpdk/detach :01:00.0
+$ ovs-vsctl del-port dpdkx
 
 This feature is not supported with VFIO and does not work with some NICs.
 For more information please refer to the `DPDK Port Hotplug Framework
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 56b9d25..baac7c3 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -359,6 +359,9 @@ struct netdev_dpdk {
 /* Device arguments for dpdk ports */
 char *devargs;
 
+/* If true, device was attached by rte_eth_dev_attach(). */
+bool attached;
+
 /* In dpdk_list. */
 struct ovs_list list_node OVS_GUARDED_BY(dpdk_mutex);
 
@@ -852,6 +855,7 @@ common_construct(struct netdev *netdev, unsigned int 
port_no,
 dev->max_packet_len = MTU_TO_FRAME_LEN(dev->mtu);
 ovsrcu_index_init(&dev->vid, -1);
 dev->vhost_reconfigured = false;
+dev->attached = false;
 
 ovsrcu_init(&dev->qos_conf, NULL);
 
@@ -997,10 +1001,21 @@ static void
 netdev_dpdk_destruct(struct netdev *netdev)
 {
 struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
+char devname[RTE_ETH_NAME_MAX_LEN];
 
 ovs_mutex_lock(&dpdk_mutex);
 
 rte_eth_dev_stop(dev->port_id);
+
+if (dev->attached) {
+rte_eth_dev_close(dev->port_id);
+if (rte_eth_dev_detach(dev->port_id, devname) < 0) {
+VLOG_ERR("Device '%s' can not be detached", dev->devargs);
+} else {
+VLOG_INFO("Device '%s' detached", devname);
+}
+}
+
 free(dev->devargs);
 common_destruct(dev);
 
@@ -1112,7 +1127,8 @@ netdev_dpdk_lookup_by_port_id(int port_id)
 }
 
 static int
-netdev_dpdk_process_devargs(const char *devargs, char **errp)
+netdev_dpdk_process_devargs(struct netdev_dpdk *dev,
+const char *devargs, char **errp)
 {
 /* Get the name up to the first comma. */
 char *name = xmemdup0(devargs, strcspn(devargs, ","));
@@ -1124,6 +1140,7 @@ netdev_dpdk_process_devargs(const 

[ovs-dev] [PATCH v2 1/3] netdev-dpdk: Fix double attaching of virtual devices.

2017-05-12 Thread Ilya Maximets
'devargs' for virtual devices contains not only name but
also a list of arguments like this:

'net_pcap0,rx_pcap=file_rx.pcap,tx_pcap=file_tx.pcap'
or
'eth_af_packet0,iface=eth0'

We must cut off the arguments from this string before calling
'rte_eth_dev_get_port_by_name()' to avoid double attaching of
the same device.

CC: Ciara Loftus 
Fixes: 69876ed78611 ("netdev-dpdk: Add support for virtual DPDK PMDs (vdevs)")
Signed-off-by: Ilya Maximets 
---

 lib/netdev-dpdk.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 609b8da..56b9d25 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1114,10 +1114,12 @@ netdev_dpdk_lookup_by_port_id(int port_id)
 static int
 netdev_dpdk_process_devargs(const char *devargs, char **errp)
 {
+/* Get the name up to the first comma. */
+char *name = xmemdup0(devargs, strcspn(devargs, ","));
 uint8_t new_port_id = UINT8_MAX;
 
 if (!rte_eth_dev_count()
-|| rte_eth_dev_get_port_by_name(devargs, &new_port_id)
+|| rte_eth_dev_get_port_by_name(name, &new_port_id)
 || !rte_eth_dev_is_valid_port(new_port_id)) {
 /* Device not found in DPDK, attempt to attach it */
 if (!rte_eth_dev_attach(devargs, &new_port_id)) {
@@ -1130,6 +1132,7 @@ netdev_dpdk_process_devargs(const char *devargs, char 
**errp)
 }
 }
 
+free(name);
 return new_port_id;
 }
 
-- 
2.7.4

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


[ovs-dev] [PATCH v2 0/3] Hotplug fixes & port_id refactoring

2017-05-12 Thread Ilya Maximets
Version 2:
* 1st patch: 'index' --> 'strcspn' (Ben Pfaff)
* One change was moved from patch 3 to patch 2.
  They were splitted in a wrong way.
  (passing 'dev' to 'netdev_dpdk_process_devargs()')


Ilya Maximets (3):
  netdev-dpdk: Fix double attaching of virtual devices.
  netdev-dpdk: Fix device leak on port deletion.
  netdev-dpdk: Use uint8_t for port_id.

 Documentation/howto/dpdk.rst |   5 +-
 lib/netdev-dpdk.c| 132 ++-
 2 files changed, 57 insertions(+), 80 deletions(-)

-- 
2.7.4

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


Re: [ovs-dev] [PATCH 3/3] netdev-dpdk: Use uint8_t for port_id.

2017-05-12 Thread Ilya Maximets
On 05.04.2017 22:34, Darrell Ball wrote:
> 
> 
> On 4/3/17, 8:04 AM, "ovs-dev-boun...@openvswitch.org on behalf of Ilya 
> Maximets"  i.maxim...@samsung.com> wrote:
> 
> Currently, signed integer is used for 'port_id' variable and
> '-1' as identifier of bad or uninitialized 'port_id'.
> 
> This inconsistent with dpdk library and, also, in few cases,
> leads to passing '-1' to dpdk functions where uint8_t expected.
> 
> Such behaviour doesn't produce any issues, but it's better to
> use same type as in dpdk library for consistency.
> 
> Also, magic number '-1' replaced with DPDK_ETH_PORT_ID_INVALID
> macro.
> 
> Signed-off-by: Ilya Maximets 
> ---
>  lib/netdev-dpdk.c | 61 
> +++
>  1 file changed, 35 insertions(+), 26 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 658a454..216ced8 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -140,6 +140,8 @@ BUILD_ASSERT_DECL((MAX_NB_MBUF / 
> ROUND_DOWN_POW2(MAX_NB_MBUF/MIN_NB_MBUF))
>  #define OVS_VHOST_QUEUE_DISABLED(-2) /* Queue was disabled by guest 
> and not
>* yet mapped to another queue. 
> */
>  
> +#define DPDK_ETH_PORT_ID_INVALIDRTE_MAX_ETHPORTS
> +
>  #define VHOST_ENQ_RETRY_NUM 8
>  #define IF_NAME_SZ (PATH_MAX > IFNAMSIZ ? PATH_MAX : IFNAMSIZ)
>  
> @@ -309,7 +311,7 @@ struct dpdk_ring {
>  struct rte_ring *cring_tx;
>  struct rte_ring *cring_rx;
>  unsigned int user_port_id; /* User given port no, parsed from port 
> name */
> -int eth_port_id; /* ethernet device port id */
> +uint8_t eth_port_id; /* ethernet device port id */
> 
> The rte does not have a typedef for port_id.
> One optional change is for OVS to have a typedef for this external type.
> /* The dpdk rte uses uint8_t for port_id. */
> typedef uint8_t rte_port_id;

I don't think that it is a good change to do because we will have to
create additional typedef at least for PRIu8. This will look ugly.
Also, if DPDK someday will create typedef with different name
we will have typedef of the typedef?

> 
>  struct ovs_list list_node OVS_GUARDED_BY(dpdk_mutex);
>  };
>  
> @@ -325,7 +327,7 @@ enum dpdk_hw_ol_features {
>  
>  struct netdev_dpdk {
>  struct netdev up;
> -int port_id;
> +uint8_t port_id;
>  int max_packet_len;
>  enum dpdk_dev_type type;
>  
> @@ -402,7 +404,7 @@ struct netdev_dpdk {
>  
>  struct netdev_rxq_dpdk {
>  struct netdev_rxq up;
> -int port_id;
> +uint8_t port_id;
>  };
>  
>  static int netdev_dpdk_class_init(void);
> @@ -602,12 +604,12 @@ check_link_status(struct netdev_dpdk *dev)
>  dev->link_reset_cnt++;
>  dev->link = link;
>  if (dev->link.link_status) {
> -VLOG_DBG_RL(&rl, "Port %d Link Up - speed %u Mbps - %s",
> +VLOG_DBG_RL(&rl, "Port %"PRIu8" Link Up - speed %u Mbps - 
> %s",
>  dev->port_id, (unsigned) dev->link.link_speed,
>  (dev->link.link_duplex == ETH_LINK_FULL_DUPLEX) ?
>   ("full-duplex") : ("half-duplex"));
>  } else {
> -VLOG_DBG_RL(&rl, "Port %d Link Down", dev->port_id);
> +VLOG_DBG_RL(&rl, "Port %"PRIu8" Link Down", dev->port_id);
>  }
>  }
>  }
> @@ -725,8 +727,8 @@ dpdk_eth_checksum_offload_configure(struct 
> netdev_dpdk *dev)
>  if (rx_csum_ol_flag &&
>  (info.rx_offload_capa & rx_chksm_offload_capa) !=
>   rx_chksm_offload_capa) {
> -VLOG_WARN_ONCE("Rx checksum offload is not supported on device 
> %d",
> -   dev->port_id);
> +VLOG_WARN_ONCE("Rx checksum offload is not supported on device 
> %"PRIu8,
> +   dev->port_id);
>  dev->hw_ol_features &= ~NETDEV_RX_CHECKSUM_OFFLOAD;
>  return;
>  }
> @@ -737,7 +739,8 @@ static void
>  dpdk_eth_flow_ctrl_setup(struct netdev_dpdk *dev) 
> OVS_REQUIRES(dev->mutex)
>  {
>  if (rte_eth_dev_flow_ctrl_set(dev->port_id, &dev->fc_conf)) {
> -VLOG_WARN("Failed to enable flow control on device %d", 
> dev->port_id);
> +VLOG_WARN("Failed to enable flow control on device %"PRIu8,
> +  dev->port_id);
>  }
>  }
>  
> @@ -775,7 +778,7 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
>  
>  memset(ð_addr, 0x0, sizeof(eth_addr));
>  rte_eth_macaddr_get(dev->port_id, ð_addr);
> -VLOG_INFO_RL(&rl, "Port %d: "ETH_ADDR_FMT,
> +VLOG_INFO_RL(&rl, "Port %"PRIu8": "ETH_ADDR_FMT,
>  dev->port_id, 
> ETH_ADDR_BY

Re: [ovs-dev] [PATCH 2/3] netdev-dpdk: Fix device leak on port deletion.

2017-05-12 Thread Ilya Maximets
Hi Darrell.

Comments inline and I'm sorry for late responses once again.

Best regards, Ilya Maximets.

On 13.04.2017 00:28, Darrell Ball wrote:
> 
> 
> On 4/3/17, 8:04 AM, "ovs-dev-boun...@openvswitch.org on behalf of Ilya 
> Maximets"  i.maxim...@samsung.com> wrote:
> 
> Currently, once created device in dpdk will exist forever
> even after del-port operation untill we manually call
> 'ovs-appctl netdev-dpdk/detach ', where  is not
> the port's name but the name of dpdk eth device or pci address.
> 
> Few issues with current implementation:
> 
>   1. Different API for usual (system) and DPDK devices.
>  (We have to call 'ovs-appctl netdev-dpdk/detach' each
>   time after 'del-port' to actually free the device)
>  This is a big issue mostly for virtual DPDK devices.
> 
>   2. Follows from 1:
>  For DPDK devices 'del-port' leads just to
>  'rte_eth_dev_stop' and subsequent 'add-port' will
>  just start the already existing device. Such behaviour
>  will not reset the device to initial state as it could
>  be expected. For example: virtual pcap pmd will continue
>  reading input file instead of reading it from the beginning.
> 
>   3. Follows from 2:
>  After execution of the following commands 'port1' will be
>  configured with the 'old-options' while 'ovs-vsctl show'
>  will show us 'new-options' in dpdk-devargs field:
> 
>ovs-vsctl add-port port1 -- set interface port1 type=dpdk \
>  options:dpdk-devargs=,
>ovs-vsctl del-port port1
>ovs-vsctl add-port port1 -- set interface port1 type=dpdk \
>  options:dpdk-devargs=,
> 
>   4. Follows from 1:
>  Not detached device consumes 'port_id'. Since we have very
>  limited number of 'port_id's (32 in common case) this may
>  lead to quick exhausting of id pool and inability to add any
>  other port.
> 
> To avoid above issues we need to detach all the attached devices on
> port destruction.
> appctl 'netdev-dpdk/detach' removed because not needed anymore.
> 
> CC: Ciara Loftus 
> Fixes: 55e075e65ef9 ("netdev-dpdk: Arbitrary 'dpdk' port naming")
> Fixes: 69876ed78611 ("netdev-dpdk: Add support for virtual DPDK PMDs 
> (vdevs)")
> Signed-off-by: Ilya Maximets 
> ---
>  Documentation/howto/dpdk.rst |  5 ++--
>  lib/netdev-dpdk.c| 66 
> +++-
>  2 files changed, 18 insertions(+), 53 deletions(-)
> 
> diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
> index dc63f7d..20d8975 100644
> --- a/Documentation/howto/dpdk.rst
> +++ b/Documentation/howto/dpdk.rst
> @@ -342,10 +342,9 @@ Then it can be attached to OVS::
>  $ ovs-vsctl add-port br0 dpdkx -- set Interface dpdkx type=dpdk \
>  options:dpdk-devargs=:01:00.0
>  
> -It is also possible to detach a port from ovs, the user has to remove the
> -port using the del-port command, then it can be detached using::
> +Detaching will be performed while processing del-port command::
>  
> -$ ovs-appctl netdev-dpdk/detach :01:00.0
> +$ ovs-vsctl del-port dpdkx
>  
>  This feature is not supported with VFIO and does not work with some NICs.
>  For more information please refer to the `DPDK Port Hotplug Framework
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index c8d7108..658a454 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -359,6 +359,9 @@ struct netdev_dpdk {
>  /* Device arguments for dpdk ports */
>  char *devargs;
>  
> +/* If true, device was attached by rte_eth_dev_attach(). */
> +bool attached;
> +
> 
> OVS should not try to keep (replicate) internal state of the rte;
> to check “attached state” query the rte for valid port.

It's not about copying the dpdk internal 'attached' state. It's
about protecting statically attached devices.
We need to use internal 'attached' variable to track ports on
which rte_eth_dev_attach() was called and returned successfully
to avoid closing and detaching devices that do not support hotplug or
by any other reason attached using the 'dpdk-extra' cmdline options.

Closing not hotplugable devices may lead to inability to add them back.

I'll add this information to commit-message in v2.

>  /* In dpdk_list. */
>  struct ovs_list list_node OVS_GUARDED_BY(dpdk_mutex);
>  
> @@ -851,6 +854,7 @@ common_construct(struct netdev *netdev, unsigned int 
> port_no,
>  dev->max_packet_len = MTU_TO_FRAME_LEN(dev->mtu);
>  ovsrcu_index_init(&dev->vid, -1);
>  dev->vhost_reconfigured = false;
> +dev->attached = false;
>  
>  ovsrcu_init(&dev->qos_conf, NULL);
>  
> 

[ovs-dev] [PATCH RFC] Drop support for RHEL 5 and 6

2017-05-12 Thread Timothy Redaelli
RHEL 6 is not supported anymore since it uses Python 2.6 and GCC 4.4.x,
but Open vSwitch needs, at least, Python 2.7 and GCC 4.6 to build correctly.

http://docs.openvswitch.org/en/latest/intro/install/general/#build-requirements

Signed-off-by: Timothy Redaelli 
---
 Documentation/automake.mk |   1 -
 Documentation/howto/docker.rst|   2 +-
 Documentation/index.rst   |   1 -
 Documentation/intro/install/index.rst |   1 -
 Documentation/intro/install/rhel.rst  | 218 
 rhel/.gitignore   |   3 -
 rhel/automake.mk  |  11 --
 rhel/openvswitch-kmod-rhel6.spec.in   |  78 --
 rhel/openvswitch-kmod.files   |   3 -
 rhel/openvswitch.spec.in  | 264 --
 10 files changed, 1 insertion(+), 581 deletions(-)
 delete mode 100644 Documentation/intro/install/rhel.rst
 delete mode 100644 rhel/openvswitch-kmod-rhel6.spec.in
 delete mode 100644 rhel/openvswitch-kmod.files
 delete mode 100644 rhel/openvswitch.spec.in

diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index 3c75d906..a3912d90 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -18,7 +18,6 @@ DOC_SOURCE = \
Documentation/intro/install/general.rst \
Documentation/intro/install/netbsd.rst \
Documentation/intro/install/ovn-upgrades.rst \
-   Documentation/intro/install/rhel.rst \
Documentation/intro/install/userspace.rst \
Documentation/intro/install/windows.rst \
Documentation/intro/install/xenserver.rst \
diff --git a/Documentation/howto/docker.rst b/Documentation/howto/docker.rst
index ff8b708a..c3d4dbf2 100644
--- a/Documentation/howto/docker.rst
+++ b/Documentation/howto/docker.rst
@@ -296,7 +296,7 @@ The "underlay" mode
Depending on your VM, you can make the above step persistent across reboots.
For example, if your VM is Debian/Ubuntu-based, read
`openvswitch-switch.README.Debian` found in `debian` folder. If your VM is
-   RHEL-based, refer to :doc:`/intro/install/rhel`.
+   Fedora/RHEL7/CentOS7-based, refer to :doc:`/intro/install/fedora`.
 
 3. Start the Open vSwitch network driver
 
diff --git a/Documentation/index.rst b/Documentation/index.rst
index 836c37fc..43b599ed 100644
--- a/Documentation/index.rst
+++ b/Documentation/index.rst
@@ -85,7 +85,6 @@ Deeper Dive
 - **Testing** :doc:`topics/testing`
 
 - **Packaging:** :doc:`intro/install/debian` |
-  :doc:`intro/install/rhel` |
   :doc:`intro/install/fedora`
 
 The Open vSwitch Project
diff --git a/Documentation/intro/install/index.rst 
b/Documentation/intro/install/index.rst
index 3193c736..626b49f1 100644
--- a/Documentation/intro/install/index.rst
+++ b/Documentation/intro/install/index.rst
@@ -59,7 +59,6 @@ provided below.
distributions
debian
fedora
-   rhel
 
 Upgrades
 
diff --git a/Documentation/intro/install/rhel.rst 
b/Documentation/intro/install/rhel.rst
deleted file mode 100644
index 86c5cf32..
--- a/Documentation/intro/install/rhel.rst
+++ /dev/null
@@ -1,218 +0,0 @@
-..
-  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.
-
-  Convention for heading levels in Open vSwitch documentation:
-
-  ===  Heading 0 (reserved for the title in a document)
-  ---  Heading 1
-  ~~~  Heading 2
-  +++  Heading 3
-  '''  Heading 4
-
-  Avoid deeper levels because they do not render well.
-
-
-RHEL 5.6, 6.x Packaging for Open vSwitch
-
-
-This document describes how to build and install Open vSwitch on a Red Hat
-Enterprise Linux (RHEL) host.  If you want to install Open vSwitch on a generic
-Linux host, refer to :doc:`general` instead.
-
-We have tested these instructions with RHEL 5.6 and RHEL 6.0.
-
-For RHEL 7.x (or derivatives, such as CentOS 7.x), you should follow the
-instructions in the :doc:`fedora`.  The Fedora spec files are used for RHEL
-7.x.
-
-.. _rhel-prerequisites:
-
-Prerequisites
--
-
-You may build from an Open vSwitch distribution tarball or from an Open vSwitch
-Git tree.
-
-The default RPM build directory, ``_topdir``, has five directories in the
-top-level.
-
-BUILD/
-  where the software is unpacked and built
-RPMS/
-  where the newly created binary package files are written
-SOURCES/
-  contains the original sources, patches, and ico

Re: [ovs-dev] [PATCH 1/3] netdev-dpdk: Fix double attaching of virtual devices.

2017-05-12 Thread Ilya Maximets
Hi.

Sorry for late response.
Comments inline.

Best regards, Ilya Maximets.

On 22.04.2017 05:01, Ben Pfaff wrote:
> On Mon, Apr 03, 2017 at 06:04:16PM +0300, Ilya Maximets wrote:
>> 'devargs' for virtual devices contains not only name but
>> also a list of arguments like this:
>>
>>  'net_pcap0,rx_pcap=file_rx.pcap,tx_pcap=file_tx.pcap'
>>  or
>>  'eth_af_packet0,iface=eth0'
>>
>> We must cut off the arguments from this string before calling
>> 'rte_eth_dev_get_port_by_name()' to avoid double attaching of
>> the same device.
>>
>> CC: Ciara Loftus 
>> Fixes: 69876ed78611 ("netdev-dpdk: Add support for virtual DPDK PMDs 
>> (vdevs)")
>> Signed-off-by: Ilya Maximets 
>> ---
>>  lib/netdev-dpdk.c | 10 +-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> index ddc651b..c8d7108 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -1114,9 +1114,16 @@ static int
>>  netdev_dpdk_process_devargs(const char *devargs, char **errp)
>>  {
>>  uint8_t new_port_id = UINT8_MAX;
>> +char *ind, *name = xstrdup(devargs);
>> +
>> +/* Get the name from the comma separated list of arguments. */
>> +ind = index(name, ',');
>> +if (ind != NULL) {
>> +*ind = '\0';
>> +}
>>  
>>  if (!rte_eth_dev_count()
>> -|| rte_eth_dev_get_port_by_name(devargs, &new_port_id)
>> +|| rte_eth_dev_get_port_by_name(name, &new_port_id)
>>  || !rte_eth_dev_is_valid_port(new_port_id)) {
>>  /* Device not found in DPDK, attempt to attach it */
>>  if (!rte_eth_dev_attach(devargs, &new_port_id)) {
>> @@ -1129,6 +1136,7 @@ netdev_dpdk_process_devargs(const char *devargs, char 
>> **errp)
>>  }
>>  }
>>  
>> +free(name);
>>  return new_port_id;
>>  }
> 
> Maybe this is a simpler version?

Yes. I like it. Thanks.
Sometimes it's hard to find the best library function for some job.
Looks like 'strcspn' much better than 'index'.

I'll send v2 with this change soon.
 
> --8<--cut here-->8--
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index ddc651bf98a6..736b7908ee0e 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -1113,10 +1113,11 @@ netdev_dpdk_lookup_by_port_id(int port_id)
>  static int
>  netdev_dpdk_process_devargs(const char *devargs, char **errp)
>  {
> +/* Get the name up to the first comma. */
> +char *name = xmemdup0(devargs, strcspn(devargs, ","));
>  uint8_t new_port_id = UINT8_MAX;
> -
>  if (!rte_eth_dev_count()
> -|| rte_eth_dev_get_port_by_name(devargs, &new_port_id)
> +|| rte_eth_dev_get_port_by_name(name, &new_port_id)
>  || !rte_eth_dev_is_valid_port(new_port_id)) {
>  /* Device not found in DPDK, attempt to attach it */
>  if (!rte_eth_dev_attach(devargs, &new_port_id)) {
> @@ -1129,6 +1130,7 @@ netdev_dpdk_process_devargs(const char *devargs, char 
> **errp)
>  }
>  }
>  
> +free(name);
>  return new_port_id;
>  }
>  
> 
> 
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH 1/2] OF1.5/EXT-334 OXS/Individal Flow Statistics

2017-05-12 Thread Satyavalli Rama
Surely Ben.

Aaron thanks for the initial feedback will definitely address them.

Thanks & Regards
Satya Valli
Tata Consultancy Services
Mailto: satyavalli.r...@tcs.com
Website: http://www.tcs.com

Experience certainty.   IT Services
Business Solutions
Consulting



-Ben Pfaff  wrote: -
To: Satyavalli Rama 
From: Ben Pfaff 
Date: 05/12/2017 07:28PM
Cc: d...@openvswitch.org, Aaron Conole 
Subject: Re: [ovs-dev] [PATCH 1/2] OF1.5/EXT-334 OXS/Individal Flow Statistics

Even if you are sending fresh patches, I hope you will consider Aaron's 
comments.

On May 12, 2017 9:56 AM, Satyavalli Rama  wrote:
Hi Aaron

Please ignore the current patch.  
Due to some proxy issues we were unable to send the complete patches.   
Soon, we wil submitt fresh patches.

Thanks & Regards
Satya Valli
Tata Consultancy Services
Mailto: satyavalli.r...@tcs.com
Website: http://www.tcs.com

Experience certainty.   IT Services
Business Solutions
Consulting



-Aaron Conole  wrote: -
To: Ben Pfaff , Satya Valli 
From: Aaron Conole 
Date: 05/11/2017 11:23PM
Cc: d...@openvswitch.org,
Subject: Re: [ovs-dev] [PATCH 1/2] OF1.5/EXT-334 OXS/Individal Flow Statistics

Hi Satya,

I haven't checked the OF1.5 spec with this, yet.  Just saw a few things.

Thanks for the contribution!

Ben Pfaff  writes:

> From: Satya Valli 
>

Missing signed-off-by line.  Also, it would be good to describe exactly
which flow stat types are being provided.

> ---
>  include/openflow/openflow-1.5.h | 48 
> +
>  include/openvswitch/ofp-msgs.h  |  6 ++
>  lib/automake.mk |  2 ++
>  lib/ofp-parse.c | 45 +-
>  lib/ofp-print.c | 10 +
>  lib/rconn.c |  2 ++
>  ofproto/ofproto.c   |  4 
>  7 files changed, 116 insertions(+), 1 deletion(-)
>
> diff --git a/include/openflow/openflow-1.5.h b/include/openflow/openflow-1.5.h
> index 3649e6c29e63..ff5fa13fae4a 100644
> --- a/include/openflow/openflow-1.5.h
> +++ b/include/openflow/openflow-1.5.h
> @@ -150,4 +150,52 @@ struct ofp15_group_desc_stats {
>  };
>  OFP_ASSERT(sizeof(struct ofp15_group_desc_stats) == 16);
>  
> +struct ofp_oxs_stat {
> +ovs_be16 reserved;  /* One of OFPST_* */
> +ovs_be16 length;/* Stats Length */
> +};
> +OFP_ASSERT(sizeof(struct ofp_oxs_stat) == 4);
> +
> +/*Body for ofp_multipart_request of type
> +  OFPMP_FLOW_DESC & OFPMP_FLOW_STATS.*/
> +struct ofp15_oxs_flow_stats_request {
> +uint8_t table_id; /* ID of table to read
> + (from ofp_table_desc),
> + OFPTT_ALL for all tables. */
> +uint8_t pad[3];   /* Align to 32 bits. */
> +ovs_be32 out_port;/* Require matching entries to include
> + this as an output port. A value of
> + OFP_ANY indicates no restriction. */
> +ovs_be32 out_group;   /* Require matching entries to include
> + this as an output group. A value of
> + OFPG_ANY indicates no restriction. 
> */
> +uint8_t pad2[4];  /* Align to 64 bits. */
> +ovs_be64 cookie;  /* Require matching entries to contain
> + this cookie value */
> +ovs_be64 cookie_mask; /* Mask used to restrict the cookie 
> bits
> + that must match. A value of 0
> + indicates no restriction. */
> +};
> +OFP_ASSERT(sizeof(struct ofp15_oxs_flow_stats_request) == 32);
> +
> +/* Body of reply to OFPMP_FLOW_STATS request
> +* and body for OFPIT_STAT_TRIGGER generated status. */

Minor nit - please put a space at the beginning of the comment line
here.

> +struct ofp15_oxs_flow_stats_reply {
> +ovs_be16 length; /* Length of this entry.   */
> +uint8_t pad2[2]; /* Align to 64-bits.   */
> +uint8_t table_id;/* ID of table flow came from. */
> +uint8_t reason;  /* One of OFPFSR_*.*/
> +ovs_be16 priority;   /* Priority of the entry.  */
> +};
> +OFP_ASSERT(sizeof(struct ofp15_oxs_flow_stats_reply) == 8);
> +
> +/* OXS flow stat field types for OpenFlow basic class. */
> +enum oxs_ofb_stat_fields {
> +OFPXST_OFB_DURATION  = 0,   /* Time flow entry has been alive.*/
> +OFPXST_OFB_IDLE_TIME = 1,   /* Time flow entry has been idle. */
> +OFPXST_OFB_FLOW_COUNT= 2,   /* Number of aggregated flow entries. */
> +OFPXST_OFB_PACKET_COUNT  = 3,   /* Number of packets in flow

Re: [ovs-dev] [PATCH 1/2] OF1.5/EXT-334 OXS/Individal Flow Statistics

2017-05-12 Thread Satyavalli Rama
Hi Aaron

Please ignore the current patch.  
Due to some proxy issues we were unable to send the complete patches.   
Soon, we wil submitt fresh patches.

Thanks & Regards
Satya Valli
Tata Consultancy Services
Mailto: satyavalli.r...@tcs.com
Website: http://www.tcs.com

Experience certainty.   IT Services
Business Solutions
Consulting



-Aaron Conole  wrote: -
To: Ben Pfaff , Satya Valli 
From: Aaron Conole 
Date: 05/11/2017 11:23PM
Cc: d...@openvswitch.org,
Subject: Re: [ovs-dev] [PATCH 1/2] OF1.5/EXT-334 OXS/Individal Flow Statistics

Hi Satya,

I haven't checked the OF1.5 spec with this, yet.  Just saw a few things.

Thanks for the contribution!

Ben Pfaff  writes:

> From: Satya Valli 
>

Missing signed-off-by line.  Also, it would be good to describe exactly
which flow stat types are being provided.

> ---
>  include/openflow/openflow-1.5.h | 48 
> +
>  include/openvswitch/ofp-msgs.h  |  6 ++
>  lib/automake.mk |  2 ++
>  lib/ofp-parse.c | 45 +-
>  lib/ofp-print.c | 10 +
>  lib/rconn.c |  2 ++
>  ofproto/ofproto.c   |  4 
>  7 files changed, 116 insertions(+), 1 deletion(-)
>
> diff --git a/include/openflow/openflow-1.5.h b/include/openflow/openflow-1.5.h
> index 3649e6c29e63..ff5fa13fae4a 100644
> --- a/include/openflow/openflow-1.5.h
> +++ b/include/openflow/openflow-1.5.h
> @@ -150,4 +150,52 @@ struct ofp15_group_desc_stats {
>  };
>  OFP_ASSERT(sizeof(struct ofp15_group_desc_stats) == 16);
>  
> +struct ofp_oxs_stat {
> +ovs_be16 reserved;  /* One of OFPST_* */
> +ovs_be16 length;/* Stats Length */
> +};
> +OFP_ASSERT(sizeof(struct ofp_oxs_stat) == 4);
> +
> +/*Body for ofp_multipart_request of type
> +  OFPMP_FLOW_DESC & OFPMP_FLOW_STATS.*/
> +struct ofp15_oxs_flow_stats_request {
> +uint8_t table_id; /* ID of table to read
> + (from ofp_table_desc),
> + OFPTT_ALL for all tables. */
> +uint8_t pad[3];   /* Align to 32 bits. */
> +ovs_be32 out_port;/* Require matching entries to include
> + this as an output port. A value of
> + OFP_ANY indicates no restriction. */
> +ovs_be32 out_group;   /* Require matching entries to include
> + this as an output group. A value of
> + OFPG_ANY indicates no restriction. 
> */
> +uint8_t pad2[4];  /* Align to 64 bits. */
> +ovs_be64 cookie;  /* Require matching entries to contain
> + this cookie value */
> +ovs_be64 cookie_mask; /* Mask used to restrict the cookie 
> bits
> + that must match. A value of 0
> + indicates no restriction. */
> +};
> +OFP_ASSERT(sizeof(struct ofp15_oxs_flow_stats_request) == 32);
> +
> +/* Body of reply to OFPMP_FLOW_STATS request
> +* and body for OFPIT_STAT_TRIGGER generated status. */

Minor nit - please put a space at the beginning of the comment line
here.

> +struct ofp15_oxs_flow_stats_reply {
> +ovs_be16 length; /* Length of this entry.   */
> +uint8_t pad2[2]; /* Align to 64-bits.   */
> +uint8_t table_id;/* ID of table flow came from. */
> +uint8_t reason;  /* One of OFPFSR_*.*/
> +ovs_be16 priority;   /* Priority of the entry.  */
> +};
> +OFP_ASSERT(sizeof(struct ofp15_oxs_flow_stats_reply) == 8);
> +
> +/* OXS flow stat field types for OpenFlow basic class. */
> +enum oxs_ofb_stat_fields {
> +OFPXST_OFB_DURATION  = 0,   /* Time flow entry has been alive.*/
> +OFPXST_OFB_IDLE_TIME = 1,   /* Time flow entry has been idle. */
> +OFPXST_OFB_FLOW_COUNT= 2,   /* Number of aggregated flow entries. */
> +OFPXST_OFB_PACKET_COUNT  = 3,   /* Number of packets in flow entry.   */
> +OFPXST_OFB_BYTE_COUNT= 4,   /* Number of bytes in flow entry. */
> +};
> +
>  #endif /* openflow/openflow-1.5.h */
> diff --git a/include/openvswitch/ofp-msgs.h b/include/openvswitch/ofp-msgs.h
> index 34708f3bd846..49b0f7a2cfa8 100644
> --- a/include/openvswitch/ofp-msgs.h
> +++ b/include/openvswitch/ofp-msgs.h
> @@ -287,6 +287,8 @@ enum ofpraw {
>  OFPRAW_OFPST10_FLOW_REQUEST,
>  /* OFPST 1.1+ (1): struct ofp11_flow_stats_request, uint8_t[8][]. */
>  OFPRAW_OFPST11_FLOW_REQUEST,
> +/* OFPST 1.5+ (17): struct ofp15_oxs_flow_stats_request, uint8_t[8][]. */
> +OFPRAW_OFPST15_OXS_FLOW_REQUEST,
>  /* NXST 1.0 (0): struct nx_flow_stats_request,

[ovs-dev] ovn-nbctl and patch ports for logical routers

2017-05-12 Thread Lance Richardson
With branch-2.6, we've noticed a difference in behavior when configuring
logical switch ports via "ovn-nbctl lsp-set-*" vs. using "ovn-nbctl set
Logical_Switch_Port". The difference seems to be a matter of setting
individual columns for a logical switch port in multiple transactions
versus setting all of them in a single transaction.

For example (note "" string in patch port names):


ovn-nbctl ls-add s0
ovn-nbctl ls-add s1
ovn-nbctl lr-add r0

ovn-nbctl lrp-add r0 r0_s0 00:de:ad:ff:00:01 192.168.1.1/24
ovn-nbctl lrp-add r0 r0_s1 00:de:ad:ff:00:02 172.16.101.10/24

ovn-nbctl lsp-add s1 s1_r0
ovn-nbctl lsp-set-type s1_r0 router
ovn-nbctl lsp-set-addresses s1_r0 00:de:ad:ff:00:02
ovn-nbctl lsp-set-options s1_r0 router-port r0_s1


ovn-nbctl lsp-add s0 s0_r0
ovn-nbctl lsp-set-type s0_r0 router
ovn-nbctl lsp-set-addresses s0_r0 00:de:ad:ff:00:01
ovn-nbctl lsp-set-options s0_r0 router-port=r0_s0

$ ovs-vsctl show
74c40ced-675b-49df-a39c-c308681c54f4
Bridge br-int
fail_mode: secure
Port "patch-r0_s1-to-"
Interface "patch-r0_s1-to-"
type: patch
options: {peer="patch--to-r0_s1"}
Port "patch-s0_r0-to-r0_s0"
Interface "patch-s0_r0-to-r0_s0"
type: patch
options: {peer="patch-r0_s0-to-s0_r0"}
Port "patch-r0_s0-to-s0_r0"
Interface "patch-r0_s0-to-s0_r0"
type: patch
options: {peer="patch-s0_r0-to-r0_s0"}
Port "patch-s1_r0-to-"
Interface "patch-s1_r0-to-"
type: patch
options: {peer="patch--to-s1_r0"}
Port br-int
Interface br-int
type: internal
---

Equivalent (in theory) to the above, but setting multiple columns
in a single transaction:


ovn-nbctl ls-add s0
ovn-nbctl ls-add s1
ovn-nbctl lr-add r0

ovn-nbctl lrp-add r0 r0_s0 00:de:ad:ff:00:01 192.168.1.1/24
ovn-nbctl lrp-add r0 r0_s1 00:de:ad:ff:00:02 172.16.101.10/24

ovn-nbctl lsp-add s1 s1_r0 -- \
set Logical_Switch_Port s1_r0 type=router \
options:router-port=r0_s1 \
addresses='"00:de:ad:ff:00:02"'

ovn-nbctl lsp-add s0 s0_r0 -- \
set Logical_Switch_Port s0_r0 type=router \
options:router-port=r0_s0 \
addresses='"00:de:ad:ff:00:01"'

$ ovs-vsctl show
613524c4-77bb-4496-b1a4-a64129d8e6d8
Bridge br-int
fail_mode: secure
Port "patch-s1_r0-to-r0_s1"
Interface "patch-s1_r0-to-r0_s1"
type: patch
options: {peer="patch-r0_s1-to-s1_r0"}
Port "patch-r0_s1-to-s1_r0"
Interface "patch-r0_s1-to-s1_r0"
type: patch
options: {peer="patch-s1_r0-to-r0_s1"}
Port "patch-r0_s0-to-s0_r0"
Interface "patch-r0_s0-to-s0_r0"
type: patch
options: {peer="patch-s0_r0-to-r0_s0"}
Port "patch-s0_r0-to-r0_s0"
Interface "patch-s0_r0-to-r0_s0"
type: patch
options: {peer="patch-r0_s0-to-s0_r0"}
Port br-int
Interface br-int
type: internal



Is this a known issue in 2.6? I noticed that the unit tests involving
router ports use the second method.

Thanks,

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


[ovs-dev] [PATCH v6 5/5] userspace: add vxlan gpe support to vport

2017-05-12 Thread Zoltán Balogh
From: Georg Schmuecking 

This patch is based on the "datapath: enable vxlangpe creation in compat mode"
from Yi Yang. It introduces an extension option "gpe" to the vxlan port in the
netdev-dpdk datapath. Furthermore it introduces a new protocol header file
vxlangpe.h in which the vxlan gpe protocoll is described. In the vxlan specific
methods the different packet are introduced and handled.

Added VXLAN GPE tunnel push test.

Signed-off-by: Yi Yang 
Signed-off-by: Georg Schmuecking 
---
 datapath/linux/compat/include/linux/openvswitch.h |  1 +
 include/openvswitch/automake.mk   |  4 +-
 include/openvswitch/vxlangpe.h| 76 +++
 lib/netdev-native-tnl.c   | 61 --
 lib/netdev-vport.c| 18 +-
 tests/tunnel-push-pop.at  | 10 +++
 6 files changed, 160 insertions(+), 10 deletions(-)
 create mode 100755 include/openvswitch/vxlangpe.h

diff --git a/datapath/linux/compat/include/linux/openvswitch.h 
b/datapath/linux/compat/include/linux/openvswitch.h
index 7990638..2ae1797 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -291,6 +291,7 @@ enum ovs_vport_attr {
 enum {
OVS_VXLAN_EXT_UNSPEC,
OVS_VXLAN_EXT_GBP,  /* Flag or __u32 */
+   OVS_VXLAN_EXT_GPE = 8,  /* Flag or __u32 */
__OVS_VXLAN_EXT_MAX,
 };
 
diff --git a/include/openvswitch/automake.mk b/include/openvswitch/automake.mk
index c0e276f..c125f1e 100644
--- a/include/openvswitch/automake.mk
+++ b/include/openvswitch/automake.mk
@@ -29,5 +29,5 @@ openvswitchinclude_HEADERS = \
include/openvswitch/uuid.h \
include/openvswitch/version.h \
include/openvswitch/vconn.h \
-   include/openvswitch/vlog.h
-
+   include/openvswitch/vlog.h \
+   include/openvswitch/vxlangpe.h
diff --git a/include/openvswitch/vxlangpe.h b/include/openvswitch/vxlangpe.h
new file mode 100755
index 000..4c18d90
--- /dev/null
+++ b/include/openvswitch/vxlangpe.h
@@ -0,0 +1,76 @@
+#ifndef __OPENVSWITCH_VXLANGPE_H
+#define __OPENVSWITCH_VXLANGPE_H 1
+
+#include "openvswitch/types.h"
+
+#define u8 uint8_t
+#define u32 uint8_t
+#define __be32 ovs_be32
+
+/*
+ * VXLAN Generic Protocol Extension (VXLAN_F_GPE):
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |R|R|Ver|I|P|R|O|   Reserved|Next Protocol  |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |VXLAN Network Identifier (VNI) |   Reserved|
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ *
+ * Ver = Version. Indicates VXLAN GPE protocol version.
+ *
+ * P = Next Protocol Bit. The P bit is set to indicate that the
+ * Next Protocol field is present.
+ *
+ * O = OAM Flag Bit. The O bit is set to indicate that the packet
+ * is an OAM packet.
+ *
+ * Next Protocol = This 8 bit field indicates the protocol header
+ * immediately following the VXLAN GPE header.
+ *
+ * https://tools.ietf.org/html/draft-ietf-nvo3-vxlan-gpe-01
+ */
+
+struct vxlanhdr_gpe {
+#ifdef WORDS_BIGENDIAN
+   u8  reserved_flags2:2,
+   version:2,
+   instance_applied:1,
+   np_applied:1,
+   reserved_flags1:1,
+   oam_flag:1;
+#else
+   u8  oam_flag:1,
+   reserved_flags1:1,
+   np_applied:1,
+   instance_applied:1,
+   version:2,
+   reserved_flags2:2;
+#endif
+   u8  reserved_flags3;
+   u8  reserved_flags4;
+   u8  next_protocol;
+   __be32  vx_vni;
+};
+
+/* VXLAN-GPE header flags. */
+#define VXLAN_HF_VER   ((1U <<29) | (1U <<28))
+#define VXLAN_HF_NP(1U <<26)
+#define VXLAN_HF_OAM   (1U <<24)
+
+#define VXLAN_GPE_USED_BITS (VXLAN_HF_VER | VXLAN_HF_NP | VXLAN_HF_OAM | \
+0xff)
+
+/* VXLAN-GPE header Next Protocol. */
+#define VXLAN_GPE_NP_IPV4  0x01
+#define VXLAN_GPE_NP_IPV6  0x02
+#define VXLAN_GPE_NP_ETHERNET  0x03
+#define VXLAN_GPE_NP_NSH   0x04
+
+struct vxlan_metadata {
+   u32 gbp;
+   u32 gpe;
+};
+
+#define VXLAN_F_GPE0x4000
+#define VXLAN_HF_GPE 0x0400
+
+#endif /* __OPENVSWITCH_VXLANGPE_H */
diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c
index 4590e25..2d947c2 100644
--- a/lib/netdev-native-tnl.c
+++ b/lib/netdev-native-tnl.c
@@ -44,6 +44,7 @@
 #include "unaligned.h"
 #include "unixctl.h"
 #include "openvswitch/vlog.h"
+#include "openvswitch/vxlangpe.h"
 
 VLOG_DEFINE_THIS_MODULE(native_tnl);
 static struct vlog_rate_limit err_rl = VLOG_RATE_LIMIT_INIT(60, 5);
@@ -497,6 +498,8 @@ netdev_vxlan_pop_header(struct dp_packet *packet)
 struct flow_tnl *tnl = &md->tunnel;
 struct vxlanhdr *vxh;
 unsigned int hlen;
+ovs_be32 vx_flags;
+enum packet_type next_pt

[ovs-dev] [PATCH v6 4/5] ofproto-dpif-xlate: refactor compose_output_action__

2017-05-12 Thread Zoltán Balogh
From: Jan Scheurich 

The very long function compose_output_action__() has been re-factored to make
the different cases for output to patch-port, native tunnel port, kernel tunnel
port, recirculation, or termination of a native tunnel at output to LOCAL port
clearer. Larger, self-contained blocks have been split out into separate
functions.

Signed-off-by: Jan Scheurich 
Co-authored-by: Zoltan Balogh 

Conflicts:
ofproto/ofproto-dpif-xlate.c
---
 ofproto/ofproto-dpif-xlate.c | 151 +--
 1 file changed, 89 insertions(+), 62 deletions(-)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 993c2aa..bcee839 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3255,39 +3255,28 @@ xlate_flow_is_protected(const struct xlate_ctx *ctx, 
const struct flow *flow, co
 xport_in->xbundle->protected && xport_out->xbundle->protected);
 }
 
-static void
-compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
-const struct xlate_bond_recirc *xr, bool check_stp)
+static bool
+check_output_prerequisites(struct xlate_ctx *ctx,
+   const struct xport *xport,
+   struct flow *flow,
+   bool check_stp)
 {
-const struct xport *xport = get_ofp_port(ctx->xbridge, ofp_port);
 struct flow_wildcards *wc = ctx->wc;
-struct flow *flow = &ctx->xin->flow;
-struct flow_tnl flow_tnl;
-union flow_vlan_hdr flow_vlans[FLOW_MAX_VLAN_HEADERS];
-uint8_t flow_nw_tos;
-odp_port_t out_port, odp_port;
-bool tnl_push_pop_send = false;
-uint8_t dscp;
-
-/* If 'struct flow' gets additional metadata, we'll need to zero it out
- * before traversing a patch port. */
-BUILD_ASSERT_DECL(FLOW_WC_SEQ == 39);
-memset(&flow_tnl, 0, sizeof flow_tnl);
 
 if (!xport) {
 xlate_report(ctx, OFT_WARN, "Nonexistent output port");
-return;
+return false;
 } else if (xport->config & OFPUTIL_PC_NO_FWD) {
 xlate_report(ctx, OFT_DETAIL, "OFPPC_NO_FWD set, skipping output");
-return;
+return false;
 } else if (ctx->mirror_snaplen != 0 && xport->odp_port == ODPP_NONE) {
 xlate_report(ctx, OFT_WARN,
  "Mirror truncate to ODPP_NONE, skipping output");
-return;
+return false;
 } else if (xlate_flow_is_protected(ctx, flow, xport)) {
 xlate_report(ctx, OFT_WARN,
  "Flow is between protected ports, skipping output.");
-return;
+return false;
 } else if (check_stp) {
 if (is_stp(&ctx->base_flow)) {
 if (!xport_stp_should_forward_bpdu(xport) &&
@@ -3301,7 +3290,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t 
ofp_port,
  "RSTP not managing BPDU in this state, "
  "skipping bpdu output");
 }
-return;
+return false;
 }
 } else if ((xport->cfm && cfm_should_process_flow(xport->cfm, flow, 
wc))
|| (xport->bfd && bfd_should_process_flow(xport->bfd, flow,
@@ -3316,9 +3305,53 @@ compose_output_action__(struct xlate_ctx *ctx, 
ofp_port_t ofp_port,
 xlate_report(ctx, OFT_WARN,
  "RSTP not in forwarding state, skipping output");
 }
-return;
+return false;
 }
 }
+return true;
+}
+
+static inline bool
+terminate_native_tunnel(struct xlate_ctx *ctx,
+ofp_port_t ofp_port,
+struct flow *flow,
+struct flow_wildcards *wc,
+odp_port_t *tnl_port)
+{
+*tnl_port = ODPP_NONE;
+
+/* XXX: Write better Filter for tunnel port. We can use in_port
+ * in tunnel-port flow to avoid these checks completely. */
+if (ofp_port == OFPP_LOCAL &&
+ovs_native_tunneling_is_on(ctx->xbridge->ofproto)) {
+*tnl_port = tnl_port_map_lookup(flow, wc);
+}
+
+return (*tnl_port != ODPP_NONE);
+}
+
+static void
+compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
+const struct xlate_bond_recirc *xr, bool check_stp)
+{
+const struct xport *xport = get_ofp_port(ctx->xbridge, ofp_port);
+struct flow_wildcards *wc = ctx->wc;
+struct flow *flow = &ctx->xin->flow;
+struct flow_tnl flow_tnl;
+union flow_vlan_hdr flow_vlans[FLOW_MAX_VLAN_HEADERS];
+uint8_t flow_nw_tos;
+odp_port_t out_port, odp_port, odp_tnl_port;
+bool is_native_tunnel = false;
+uint8_t dscp;
+
+/* If 'struct flow' gets additional metadata, we'll need to zero it out
+ * before traversing a patch port. */
+BUILD_ASSERT_DECL(FLOW_WC_SEQ == 39);
+memset(&flow_tnl, 0, sizeof flow_tnl);
+
+if (!check_output_prerequisites(ctx, xport, flow, check_stp)) 

[ovs-dev] [PATCH v6 1/5] userspace: Switching of L3 packets in L2 pipeline

2017-05-12 Thread Zoltán Balogh
From: Jan Scheurich 

Ports have a new layer3 attribute if they send/receive L3 packets.

The packet_type included in structs dp_packet and flow is considered in
ofproto-dpif. The classical L2 match fields (dl_src, dl_dst, dl_type, and
vlan_tci, vlan_vid, vlan_pcp) now have Ethernet as pre-requisite.

A dummy ethernet header is pushed to L3 packets received from L3 ports
before the the pipeline processing starts. The ethernet header is popped
before sending a packet to a L3 port.

For datapath ports that can receive L2 or L3 packets, the packet_type
becomes part of the flow key for datapath flows and is handled
appropriately in dpif-netdev.

In the 'else' branch in flow_put_on_pmd() function, the additional check
flow_equal(&match.flow, &netdev_flow->flow) was removed, as a) the dpcls
lookup is sufficient to uniquely identify a flow and b) it caused false
negatives because the flow in netdev->flow may not properly masked.

In dpif_netdev_flow_put() we now use the same method for constructing the
netdev_flow_key as the one used when adding the flow to the dplcs to make sure
these always match. The function netdev_flow_key_from_flow() used so far was
not only inefficient but sometimes caused mismatches and subsequent flow
update failures.

Signed-off-by: Lorand Jakab 
Signed-off-by: Simon Horman 
Signed-off-by: Jiri Benc 
Signed-off-by: Yi Yang 
Signed-off-by: Jan Scheurich 
Co-authored-by: Zoltan Balogh 
---
 build-aux/extract-ofp-fields  |   1 +
 datapath/linux/compat/include/linux/openvswitch.h |   2 +
 include/openvswitch/match.h   |   1 +
 include/openvswitch/meta-flow.h   |  15 +-
 lib/dpif-netdev.c |  46 +++---
 lib/dpif-netlink.c|   2 +-
 lib/dpif.c|   2 +-
 lib/match.c   |  25 +++
 lib/meta-flow.c   |   2 +
 lib/netdev-vport.c|   8 +-
 lib/netdev.h  |   1 +
 lib/odp-execute.c |   2 +
 lib/odp-util.c| 184 ++
 lib/odp-util.h|   6 +-
 lib/packets.h |   1 -
 ofproto/ofproto-dpif-sflow.c  |   1 +
 ofproto/ofproto-dpif-upcall.c |   4 +-
 ofproto/ofproto-dpif-xlate.c  |  55 ++-
 ofproto/ofproto-dpif.c|   6 +-
 ofproto/tunnel.c  |   3 +
 tests/tunnel-push-pop-ipv6.at |  10 +-
 tests/tunnel-push-pop.at  |  12 +-
 tests/tunnel.at   |  28 ++--
 23 files changed, 304 insertions(+), 113 deletions(-)

diff --git a/build-aux/extract-ofp-fields b/build-aux/extract-ofp-fields
index af7c69b..d5b8a82 100755
--- a/build-aux/extract-ofp-fields
+++ b/build-aux/extract-ofp-fields
@@ -39,6 +39,7 @@ FORMATTING = {"decimal":("MFS_DECIMAL",  1,   
8),
   "TCP flags":  ("MFS_TCP_FLAGS",2,   2)}
 
 PREREQS = {"none": "MFP_NONE",
+   "Ethernet": "MFP_ETHERNET",
"ARP": "MFP_ARP",
"VLAN VID": "MFP_VLAN_VID",
"IPv4": "MFP_IPV4",
diff --git a/datapath/linux/compat/include/linux/openvswitch.h 
b/datapath/linux/compat/include/linux/openvswitch.h
index d22102e..7990638 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -363,6 +363,8 @@ enum ovs_key_attr {
/* Only used within kernel data path. */
OVS_KEY_ATTR_TUNNEL_INFO,  /* struct ovs_tunnel_info */
 #endif
+
+   OVS_KEY_ATTR_PACKET_TYPE,  /* be32 packet type */
__OVS_KEY_ATTR_MAX
 };
 
diff --git a/include/openvswitch/match.h b/include/openvswitch/match.h
index 06fa04c..ce06919 100644
--- a/include/openvswitch/match.h
+++ b/include/openvswitch/match.h
@@ -115,6 +115,7 @@ void match_set_ct_ipv6_dst(struct match *, const struct 
in6_addr *);
 void match_set_ct_ipv6_dst_masked(struct match *, const struct in6_addr *,
   const struct in6_addr *);
 
+void match_set_packet_type(struct match *, ovs_be32 packet_type);
 void match_set_skb_priority(struct match *, uint32_t skb_priority);
 void match_set_dl_type(struct match *, ovs_be16);
 void match_set_dl_src(struct match *, const struct eth_addr );
diff --git a/include/openvswitch/meta-flow.h b/include/openvswitch/meta-flow.h
index 11852d2..c284ec6 100644
--- a/include/openvswitch/meta-flow.h
+++ b/include/openvswitch/meta-flow.h
@@ -985,7 +985,7 @@ enum OVS_PACKED_ENUM mf_field_id {
  * Type: MAC.
  * Maskable: bitwise.
  * Formatting: Ethernet.
- * Prerequisites: none.
+ * Prerequisites: Ethernet.
  * Access: read/write.
  * NXM: NXM_OF_ETH_SRC(2) since v1.1

[ovs-dev] [PATCH v6 3/5] dpif-netlink: Don't send PACKET_TYPE to kernel

2017-05-12 Thread Zoltán Balogh
The kernel datapath does not support the packet_type match field.
Instead it encodes the packet type implictly by the presence or absence of
the Ethernet attribute in the flow key and mask.

This patch filters the PACKET_TYPE attribute out of netlink flow key and
mask to be sent to the kernel datapath.

Signed-off-by: Zoltan Balogh 
---
 lib/dpif-netlink.c | 37 -
 1 file changed, 32 insertions(+), 5 deletions(-)

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 5c3cebd..573e4a9 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -2917,6 +2917,34 @@ dpif_netlink_flow_from_ofpbuf(struct dpif_netlink_flow 
*flow,
 return 0;
 }
 
+static inline void
+nl_msg_put_exclude_packet_type(struct ofpbuf* buf, uint16_t type,
+   const struct nlattr *data,
+   uint16_t data_len)
+{
+const struct nlattr *packet_type = nl_attr_find__(data, data_len,
+  
OVS_KEY_ATTR_PACKET_TYPE);
+if (packet_type) {
+/* exclude PACKET_TYPE Netlink attribute. */
+ovs_assert(NLA_ALIGN(packet_type->nla_len) == NLA_HDRLEN + 
sizeof(uint32_t));
+
+size_t packet_type_len = NLA_HDRLEN + sizeof(uint32_t);
+size_t first_chunk_size =
+(size_t)((uint8_t *)packet_type - (uint8_t *)data);
+size_t second_chunk_size = data_len - first_chunk_size
+   - packet_type_len;
+uint8_t *first_attr = NULL;
+struct nlattr *next_attr = nl_attr_next(packet_type);
+
+first_attr = nl_msg_put_unspec_uninit(buf, type,
+  data_len - packet_type_len);
+memcpy(first_attr, data, first_chunk_size);
+memcpy(first_attr + first_chunk_size, next_attr, second_chunk_size);
+} else {
+nl_msg_put_unspec(buf, type, data, data_len);
+}
+}
+
 /* Appends to 'buf' (which must initially be empty) a "struct ovs_header"
  * followed by Netlink attributes corresponding to 'flow'. */
 static void
@@ -2943,13 +2971,12 @@ dpif_netlink_flow_to_ofpbuf(const struct 
dpif_netlink_flow *flow,
 }
 if (!flow->ufid_terse || !flow->ufid_present) {
 if (flow->key_len) {
-nl_msg_put_unspec(buf, OVS_FLOW_ATTR_KEY,
-  flow->key, flow->key_len);
+nl_msg_put_exclude_packet_type(buf, OVS_FLOW_ATTR_KEY, flow->key,
+   flow->key_len);
 }
-
 if (flow->mask_len) {
-nl_msg_put_unspec(buf, OVS_FLOW_ATTR_MASK,
-  flow->mask, flow->mask_len);
+nl_msg_put_exclude_packet_type(buf, OVS_FLOW_ATTR_MASK, flow->mask,
+   flow->mask_len);
 }
 if (flow->actions || flow->actions_len) {
 nl_msg_put_unspec(buf, OVS_FLOW_ATTR_ACTIONS,
-- 
1.9.1

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


[ovs-dev] [PATCH v6 2/5] userspace: L3 tunnel support for GRE and LISP

2017-05-12 Thread Zoltán Balogh
From: Jan Scheurich 

Add a boolean "layer3" configuration option for tunnel vports.
The layer3 option defaults to false for all ports except LISP.
GRE ports accept both true and false for "layer3".

A tunnel vport configured with layer3=true receives L3 packets.
which are then converted to Ethernet packets by pushing a dummy
Ethernet heder at the ingress of the OpenFlow pipeline. The
Ethernet header of a packet is stripped before sending to a
layer3 tunnel vport.

Presently a single GRE vport cannot carry both L2 and L3 packets.
But it is possible to create two GRE vports representing the same
GRE tunel, one with layer3=false, the other with layer3=true.
L2 packet from the tunnel are received on the first vport, L3
packets on the second. The controller must send packets to the
layer3 GRE vport to tunnel them without their Ethernet header.

Units tests have been added to check the L3 tunnel handling.

LISP tunnels are not yet supported by the netdev userspace datapath.

Signed-off-by: Simon Horman 
Signed-off-by: Jiri Benc 
Signed-off-by: Yi Yang 
Signed-off-by: Jan Scheurich 
Co-authored-by: Zoltan Balogh 
---
 lib/netdev-native-tnl.c   | 26 +-
 lib/netdev-vport.c| 14 --
 ofproto/tunnel.c  | 14 +++---
 tests/tunnel-push-pop-ipv6.at | 16 +++-
 tests/tunnel-push-pop.at  | 33 -
 vswitchd/vswitch.xml  | 13 +
 6 files changed, 96 insertions(+), 20 deletions(-)

diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c
index 2798324..4590e25 100644
--- a/lib/netdev-native-tnl.c
+++ b/lib/netdev-native-tnl.c
@@ -154,6 +154,10 @@ netdev_tnl_push_ip_header(struct dp_packet *packet,
 *ip_tot_size = dp_packet_size(packet) - sizeof (struct eth_header);
 
 memcpy(eth, header, size);
+/* The encapsulated packet has type Ethernet. Adjust dp_packet. */
+packet->packet_type = htonl(PT_ETH);
+dp_packet_reset_offsets(packet);
+packet->l3_ofs = sizeof (struct eth_header);
 
 if (netdev_tnl_is_header_ipv6(header)) {
 ip6 = netdev_tnl_ipv6_hdr(eth);
@@ -345,6 +349,7 @@ parse_gre_header(struct dp_packet *packet,
 ovs_16aligned_be32 *options;
 int hlen;
 unsigned int ulen;
+uint16_t greh_protocol;
 
 greh = netdev_tnl_ip_extract_tnl_md(packet, tnl, &ulen);
 if (!greh) {
@@ -355,10 +360,6 @@ parse_gre_header(struct dp_packet *packet,
 return -EINVAL;
 }
 
-if (greh->protocol != htons(ETH_TYPE_TEB)) {
-return -EINVAL;
-}
-
 hlen = ulen + gre_header_len(greh->flags);
 if (hlen > dp_packet_size(packet)) {
 return -EINVAL;
@@ -388,6 +389,15 @@ parse_gre_header(struct dp_packet *packet,
 options++;
 }
 
+/* Set the new packet type depending on the GRE protocol field. */
+greh_protocol = ntohs(greh->protocol);
+if (greh_protocol == ETH_TYPE_TEB) {
+packet->packet_type = htonl(PT_ETH);
+} else {
+/* Allow all GRE protocol values as Ethertypes */
+packet->packet_type = PACKET_TYPE_BE(OFPHTN_ETHERTYPE, greh_protocol);
+}
+
 return hlen;
 }
 
@@ -451,7 +461,11 @@ netdev_gre_build_header(const struct netdev *netdev,
 
 greh = netdev_tnl_ip_build_header(data, params, IPPROTO_GRE);
 
-greh->protocol = htons(ETH_TYPE_TEB);
+if (tnl_cfg->is_layer3) {
+greh->protocol = params->flow->dl_type;
+} else {
+greh->protocol = htons(ETH_TYPE_TEB);
+}
 greh->flags = 0;
 
 options = (ovs_16aligned_be32 *) (greh + 1);
@@ -504,6 +518,7 @@ netdev_vxlan_pop_header(struct dp_packet *packet)
 tnl->tun_id = htonll(ntohl(get_16aligned_be32(&vxh->vx_vni)) >> 8);
 tnl->flags |= FLOW_TNL_F_KEY;
 
+packet->packet_type = htonl(PT_ETH);
 dp_packet_reset_packet(packet, hlen + VXLAN_HLEN);
 
 return packet;
@@ -583,6 +598,7 @@ netdev_geneve_pop_header(struct dp_packet *packet)
 tnl->metadata.present.len = opts_len;
 tnl->flags |= FLOW_TNL_F_UDPIF;
 
+packet->packet_type = htonl(PT_ETH);
 dp_packet_reset_packet(packet, hlen);
 
 return packet;
diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index 84b9be3..ba69461 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -410,7 +410,7 @@ set_tunnel_config(struct netdev *dev_, const struct smap 
*args, char **errp)
 const char *name = netdev_get_name(dev_);
 const char *type = netdev_get_type(dev_);
 struct ds errors = DS_EMPTY_INITIALIZER;
-bool needs_dst_port, has_csum;
+bool needs_dst_port, has_csum, optional_layer3;
 uint16_t dst_proto = 0, src_proto = 0;
 struct netdev_tunnel_config tnl_cfg;
 struct smap_node *node;
@@ -418,6 +418,7 @@ set_tunnel_config(struct netdev *dev_, const struct smap 
*args, char **errp)
 
 has_csum = strstr(type, "gre") || strstr(type, "geneve") ||
strstr(type, "stt") || strstr(type, "vxlan");
+optional_layer3 = !strcmp(type, "gre");
 memset(

[ovs-dev] [PATCH v6 0/5] userspace: Support for L3 tunneling

2017-05-12 Thread Zoltán Balogh
From: Jan Scheurich 

This patch set is part of an initiative to deal with non-Ethernet packets in 
OVS for advanced use cases like L3 tunneling or NSH. The initiative is 
centering on the new OpenFlow concepts of "Packet type-aware pipeline" (PTAP) 
and "Generic encap/decap actions" (EXT-382). The overall design is documented 
in 
https://docs.google.com/document/d/1oWMYUH8sjZJzWa72o2q9kU0N6pNE-rwZcLH3-kbbDR8

The patches implement the user-space parts of the support for L3 tunnels 
connected to the legacy Ethernet-only pipeline in OVS. In large parts it is an 
adaptation of the earlier work on L3 tunneling by Lorand Jakab, Simon Horman, 
Jiri Benc and Yi Yang, adapted to the new design for packet type aware 
pipelines as prototyped by Jean Tourrilhes. 

Key changes compared to earlier patch series are the introduction of explicit 
packet_type members in the structs dp_packet and flow, as well as a simpler 
handling of L3 tunnels limited to the ofproto layer.

The present series v6 supersedes v5. The reason of creating v6 is that tunnel 
handling has been changed on master branch, so rebasing was necessary.
The present series v5 supersedes v4 
(https://mail.openvswitch.org/pipermail/ovs-dev/2017-April/331424.html), v4 
fixes struct ovs_action_push_eth, so kernel and userspace use same data 
structure and removes a patch from the series which was applied to master.
The series v4 supersedes v3 
(https://mail.openvswitch.org/pipermail/ovs-dev/2017-April/330488.html), v4 
fixes sparse warnings coding style and removes a patch from the series.
The series v3 superseded v2 
(https://mail.openvswitch.org/pipermail/ovs-dev/2017-April/330444.html) which 
added support for vxlan-gpe tunnels in the netdev-dpdk datapath based on 
earlier patches 14-16 out of a patch set by Yi Yang 
(https://mail.openvswitch.org/pipermail/ovs-dev/2017-February/328498.html). 

   userspace: Switching of L3 packets in L2 pipeline 
   userspace: L3 tunnel support for GRE and LISP 
   dpif-netlink: Don't send PACKET_TYPE to kernel 
   ofproto-dpif-xlate: refactor compose_output_action__
   userspace: add vxlan gpe support to vport

For native L3 tunneling with the userspace datapath these patches are complete 
and could be merged.

The necessary kernel module changes for L3 tunneling have already been 
upstreamed to the net-next kernel and back-ported to OVS. To apply L3 tunneling 
to the kernel datapath, one will need the following additional contributions:

* [PATCH v2 0/7] create tunnel devices using rtnetlink interface
   https://mail.openvswitch.org/pipermail/ovs-dev/2017-March/329893.html 
* An additional user-space patch based on the above to configure the layer3 
tunnel option in the kernel datapath

Changes v1 -> v2:
* Rebased to master (f40c558 ovn: Gratuitous ARP for distributed NAT rules)
* Moved new packet_type member to fill a 4-byte padding in struct flow
* Define the packet_type constants to be in host byte order
* Update the display format of packet_type in match and flow key printouts to a 
pair format "(ns, type)"
* Renamed dp_packet_l2() to dp_packet_eth()
* Removed unnecessary wrappers dp_packet_packet_type() and 
dp_packet_set_packet_type()
* Fixed "sparse" warnings
* Removed misleading or unnecessary comments
* Added support for vxlan-gpe tunnel vports for Ethernet, IPv4, IPv6 and NSH 
payload

Changes v2 -> v3:
* Removed unused function dp_packet_is_l3()
* Using VLOG_DROP_DBG and VLOG_DBG instead of VLOG_DBG_RL in 
dp_netdev_flow_add()
* Moved changes related comment from flow_put_on_md() into commit message
* PT_NS* macros were replaced with inline functions
* Unnecessary if statements removed from match_format()
* Using assignment operator instead of memset where it's possible
* Improved some commit messages

Changes v3 -> v4:
* Fixed "sparse" warnings
* Fixed coding style
* Removed patch "ofproto-dpif-upcall: Intialize dump-seq of new flow to zero"
* Rebased to origin/master (9a84f46)

Changes v4 -> v5:
* Eth_type member removed from struct ovs_action_push_eth.
* Patch "userspace: Add packet_type in dp_packet and flow" exluded.

Changes v5 ->v6:
* Rebased to origin/master (f5f6455)
* Patch "userspace: Support for push_eth and pop_eth actions" exluded.

 build-aux/extract-ofp-fields  |   1 +
 datapath/linux/compat/include/linux/openvswitch.h |   3 +
 include/openvswitch/automake.mk   |   4 +-
 include/openvswitch/match.h   |   1 +
 include/openvswitch/meta-flow.h   |  15 +-
 include/openvswitch/vxlangpe.h|  76 
 lib/dpif-netdev.c |  46 +++--
 lib/dpif-netlink.c|  39 +++-
 lib/dpif.c|   2 +-
 lib/match.c   |  25 +++
 lib/meta-flow.c   |   2 +
 lib/netdev-native-tnl.c   |  85 -
 lib/netdev-vport.c