Re: [ovs-dev] [PATCH ovn 3/4] ovn-controller: Detect OVS datapath capabilities.

2021-06-04 Thread Dumitru Ceara 
On 6/4/21 5:55 PM, Mark Gray wrote:
> On 03/06/2021 16:05, Dumitru Ceara wrote:
>> Automatically create an OVS Datapath record if none exists for the
>> current br-int datapath type.
>>
>> Add a 'features' module to track which OVS features are available in
>> the datapath currently being used.  For now, only ct_zero_snat is
>> tracked, all other features are assumed to be on-par between all
>> datapaths.
>>
>> A future commit will make use of the 'features' module to conditionally
>> program openflows based on available datapath features.
>>
>> Signed-off-by: Dumitru Ceara 
>> ---
>>  controller/ovn-controller.c |  125 
>> ++-
>>  include/ovn/features.h  |   16 ++
>>  lib/automake.mk |1 
>>  lib/features.c  |   39 +
>>  tests/ovn-controller.at |   11 ++--
>>  5 files changed, 159 insertions(+), 33 deletions(-)
>>  create mode 100644 lib/features.c
>>
>> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
>> index d48ddc7a2..15af7a13c 100644
>> --- a/controller/ovn-controller.c
>> +++ b/controller/ovn-controller.c
>> @@ -46,6 +46,7 @@
>>  #include "openvswitch/vconn.h"
>>  #include "openvswitch/vlog.h"
>>  #include "ovn/actions.h"
>> +#include "ovn/features.h"
>>  #include "lib/chassis-index.h"
>>  #include "lib/extend-table.h"
>>  #include "lib/ip-mcast-index.h"
>> @@ -88,6 +89,7 @@ static unixctl_cb_func lflow_cache_show_stats_cmd;
>>  static unixctl_cb_func debug_delay_nb_cfg_report;
>>  
>>  #define DEFAULT_BRIDGE_NAME "br-int"
>> +#define DEFAULT_DATAPATH "system"
>>  #define DEFAULT_PROBE_INTERVAL_MSEC 5000
>>  #define OFCTRL_DEFAULT_PROBE_INTERVAL_SEC 0
>>  
>> @@ -95,6 +97,8 @@ static unixctl_cb_func debug_delay_nb_cfg_report;
>>  
>>  #define OVS_NB_CFG_NAME "ovn-nb-cfg"
>>  
>> +#define OVS_CT_ZERO_SNAT_FEATURE "ct_zero_snat"
>> +
>>  static char *parse_options(int argc, char *argv[]);
>>  OVS_NO_RETURN static void usage(void);
>>  
>> @@ -319,10 +323,6 @@ static const struct ovsrec_bridge *
>>  create_br_int(struct ovsdb_idl_txn *ovs_idl_txn,
>>const struct ovsrec_open_vswitch_table *ovs_table)
>>  {
>> -if (!ovs_idl_txn) {
>> -return NULL;
>> -}
>> -
>>  const struct ovsrec_open_vswitch *cfg;
>>  cfg = ovsrec_open_vswitch_table_first(ovs_table);
>>  if (!cfg) {
>> @@ -386,6 +386,21 @@ create_br_int(struct ovsdb_idl_txn *ovs_idl_txn,
>>  return bridge;
>>  }
>>  
>> +static const struct ovsrec_datapath *
>> +create_br_datapath(struct ovsdb_idl_txn *ovs_idl_txn,
>> +   const struct ovsrec_open_vswitch *cfg,
>> +   const char *datapath_type)
>> +{
>> +ovsdb_idl_txn_add_comment(ovs_idl_txn,
>> +  "ovn-controller: creating bridge datapath 
>> '%s'",
>> +  datapath_type);
>> +
>> +struct ovsrec_datapath *dp = ovsrec_datapath_insert(ovs_idl_txn);
>> +ovsrec_open_vswitch_verify_datapaths(cfg);
>> +ovsrec_open_vswitch_update_datapaths_setkey(cfg, datapath_type, dp);
>> +return dp;
>> +}
>> +
>>  static const struct ovsrec_bridge *
>>  get_br_int(const struct ovsrec_bridge_table *bridge_table,
>> const struct ovsrec_open_vswitch_table *ovs_table)
>> @@ -399,33 +414,78 @@ get_br_int(const struct ovsrec_bridge_table 
>> *bridge_table,
>>  return get_bridge(bridge_table, br_int_name(cfg));
>>  }
>>  
>> -static const struct ovsrec_bridge *
>> +static const struct ovsrec_datapath *
>> +get_br_datapath(const struct ovsrec_open_vswitch *cfg,
>> +const char *datapath_type)
>> +{
>> +for (size_t i = 0; i < cfg->n_datapaths; i++) {
>> +if (!strcmp(cfg->key_datapaths[i], datapath_type)) {
>> +return cfg->value_datapaths[i];
>> +}
>> +}
>> +return NULL;
>> +}
>> +
>> +static void
>>  process_br_int(struct ovsdb_idl_txn *ovs_idl_txn,
>> const struct ovsrec_bridge_table *bridge_table,
>> -   const struct ovsrec_open_vswitch_table *ovs_table)
>> +   const struct ovsrec_open_vswitch_table *ovs_table,
>> +   const struct ovsrec_bridge **br_int_,
>> +   const struct ovsrec_datapath **br_int_dp_)
>>  {
>> -const struct ovsrec_bridge *br_int = get_br_int(bridge_table,
>> -ovs_table);
>> -if (!br_int) {
>> -br_int = create_br_int(ovs_idl_txn, ovs_table);
>> -}
>> -if (br_int && ovs_idl_txn) {
>> -const struct ovsrec_open_vswitch *cfg;
>> -cfg = ovsrec_open_vswitch_table_first(ovs_table);
>> -ovs_assert(cfg);
>> -const char *datapath_type = smap_get(&cfg->external_ids,
>> - "ovn-bridge-datapath-type");
>> -/* Check for the datapath_type and set it only if it is defined in
>> - * cfg. */
>> -if (datapath_type && strcmp(br_int->datapath_type, datapath_type))

Re: [ovs-dev] [PATCH ovn 3/4] ovn-controller: Detect OVS datapath capabilities.

2021-06-04 Thread Mark Gray 
On 03/06/2021 16:05, Dumitru Ceara wrote:
> Automatically create an OVS Datapath record if none exists for the
> current br-int datapath type.
> 
> Add a 'features' module to track which OVS features are available in
> the datapath currently being used.  For now, only ct_zero_snat is
> tracked, all other features are assumed to be on-par between all
> datapaths.
> 
> A future commit will make use of the 'features' module to conditionally
> program openflows based on available datapath features.
> 
> Signed-off-by: Dumitru Ceara 
> ---
>  controller/ovn-controller.c |  125 
> ++-
>  include/ovn/features.h  |   16 ++
>  lib/automake.mk |1 
>  lib/features.c  |   39 +
>  tests/ovn-controller.at |   11 ++--
>  5 files changed, 159 insertions(+), 33 deletions(-)
>  create mode 100644 lib/features.c
> 
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index d48ddc7a2..15af7a13c 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -46,6 +46,7 @@
>  #include "openvswitch/vconn.h"
>  #include "openvswitch/vlog.h"
>  #include "ovn/actions.h"
> +#include "ovn/features.h"
>  #include "lib/chassis-index.h"
>  #include "lib/extend-table.h"
>  #include "lib/ip-mcast-index.h"
> @@ -88,6 +89,7 @@ static unixctl_cb_func lflow_cache_show_stats_cmd;
>  static unixctl_cb_func debug_delay_nb_cfg_report;
>  
>  #define DEFAULT_BRIDGE_NAME "br-int"
> +#define DEFAULT_DATAPATH "system"
>  #define DEFAULT_PROBE_INTERVAL_MSEC 5000
>  #define OFCTRL_DEFAULT_PROBE_INTERVAL_SEC 0
>  
> @@ -95,6 +97,8 @@ static unixctl_cb_func debug_delay_nb_cfg_report;
>  
>  #define OVS_NB_CFG_NAME "ovn-nb-cfg"
>  
> +#define OVS_CT_ZERO_SNAT_FEATURE "ct_zero_snat"
> +
>  static char *parse_options(int argc, char *argv[]);
>  OVS_NO_RETURN static void usage(void);
>  
> @@ -319,10 +323,6 @@ static const struct ovsrec_bridge *
>  create_br_int(struct ovsdb_idl_txn *ovs_idl_txn,
>const struct ovsrec_open_vswitch_table *ovs_table)
>  {
> -if (!ovs_idl_txn) {
> -return NULL;
> -}
> -
>  const struct ovsrec_open_vswitch *cfg;
>  cfg = ovsrec_open_vswitch_table_first(ovs_table);
>  if (!cfg) {
> @@ -386,6 +386,21 @@ create_br_int(struct ovsdb_idl_txn *ovs_idl_txn,
>  return bridge;
>  }
>  
> +static const struct ovsrec_datapath *
> +create_br_datapath(struct ovsdb_idl_txn *ovs_idl_txn,
> +   const struct ovsrec_open_vswitch *cfg,
> +   const char *datapath_type)
> +{
> +ovsdb_idl_txn_add_comment(ovs_idl_txn,
> +  "ovn-controller: creating bridge datapath 
> '%s'",
> +  datapath_type);
> +
> +struct ovsrec_datapath *dp = ovsrec_datapath_insert(ovs_idl_txn);
> +ovsrec_open_vswitch_verify_datapaths(cfg);
> +ovsrec_open_vswitch_update_datapaths_setkey(cfg, datapath_type, dp);
> +return dp;
> +}
> +
>  static const struct ovsrec_bridge *
>  get_br_int(const struct ovsrec_bridge_table *bridge_table,
> const struct ovsrec_open_vswitch_table *ovs_table)
> @@ -399,33 +414,78 @@ get_br_int(const struct ovsrec_bridge_table 
> *bridge_table,
>  return get_bridge(bridge_table, br_int_name(cfg));
>  }
>  
> -static const struct ovsrec_bridge *
> +static const struct ovsrec_datapath *
> +get_br_datapath(const struct ovsrec_open_vswitch *cfg,
> +const char *datapath_type)
> +{
> +for (size_t i = 0; i < cfg->n_datapaths; i++) {
> +if (!strcmp(cfg->key_datapaths[i], datapath_type)) {
> +return cfg->value_datapaths[i];
> +}
> +}
> +return NULL;
> +}
> +
> +static void
>  process_br_int(struct ovsdb_idl_txn *ovs_idl_txn,
> const struct ovsrec_bridge_table *bridge_table,
> -   const struct ovsrec_open_vswitch_table *ovs_table)
> +   const struct ovsrec_open_vswitch_table *ovs_table,
> +   const struct ovsrec_bridge **br_int_,
> +   const struct ovsrec_datapath **br_int_dp_)
>  {
> -const struct ovsrec_bridge *br_int = get_br_int(bridge_table,
> -ovs_table);
> -if (!br_int) {
> -br_int = create_br_int(ovs_idl_txn, ovs_table);
> -}
> -if (br_int && ovs_idl_txn) {
> -const struct ovsrec_open_vswitch *cfg;
> -cfg = ovsrec_open_vswitch_table_first(ovs_table);
> -ovs_assert(cfg);
> -const char *datapath_type = smap_get(&cfg->external_ids,
> - "ovn-bridge-datapath-type");
> -/* Check for the datapath_type and set it only if it is defined in
> - * cfg. */
> -if (datapath_type && strcmp(br_int->datapath_type, datapath_type)) {
> -ovsrec_bridge_set_datapath_type(br_int, datapath_type);
> +const struct ovsrec_bridge *br_int = get_br_int(bridge_table, ovs_table);
> +

[ovs-dev] [PATCH ovn 3/4] ovn-controller: Detect OVS datapath capabilities.

2021-06-03 Thread Dumitru Ceara 
Automatically create an OVS Datapath record if none exists for the
current br-int datapath type.

Add a 'features' module to track which OVS features are available in
the datapath currently being used.  For now, only ct_zero_snat is
tracked, all other features are assumed to be on-par between all
datapaths.

A future commit will make use of the 'features' module to conditionally
program openflows based on available datapath features.

Signed-off-by: Dumitru Ceara 
---
 controller/ovn-controller.c |  125 ++-
 include/ovn/features.h  |   16 ++
 lib/automake.mk |1 
 lib/features.c  |   39 +
 tests/ovn-controller.at |   11 ++--
 5 files changed, 159 insertions(+), 33 deletions(-)
 create mode 100644 lib/features.c

diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index d48ddc7a2..15af7a13c 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -46,6 +46,7 @@
 #include "openvswitch/vconn.h"
 #include "openvswitch/vlog.h"
 #include "ovn/actions.h"
+#include "ovn/features.h"
 #include "lib/chassis-index.h"
 #include "lib/extend-table.h"
 #include "lib/ip-mcast-index.h"
@@ -88,6 +89,7 @@ static unixctl_cb_func lflow_cache_show_stats_cmd;
 static unixctl_cb_func debug_delay_nb_cfg_report;
 
 #define DEFAULT_BRIDGE_NAME "br-int"
+#define DEFAULT_DATAPATH "system"
 #define DEFAULT_PROBE_INTERVAL_MSEC 5000
 #define OFCTRL_DEFAULT_PROBE_INTERVAL_SEC 0
 
@@ -95,6 +97,8 @@ static unixctl_cb_func debug_delay_nb_cfg_report;
 
 #define OVS_NB_CFG_NAME "ovn-nb-cfg"
 
+#define OVS_CT_ZERO_SNAT_FEATURE "ct_zero_snat"
+
 static char *parse_options(int argc, char *argv[]);
 OVS_NO_RETURN static void usage(void);
 
@@ -319,10 +323,6 @@ static const struct ovsrec_bridge *
 create_br_int(struct ovsdb_idl_txn *ovs_idl_txn,
   const struct ovsrec_open_vswitch_table *ovs_table)
 {
-if (!ovs_idl_txn) {
-return NULL;
-}
-
 const struct ovsrec_open_vswitch *cfg;
 cfg = ovsrec_open_vswitch_table_first(ovs_table);
 if (!cfg) {
@@ -386,6 +386,21 @@ create_br_int(struct ovsdb_idl_txn *ovs_idl_txn,
 return bridge;
 }
 
+static const struct ovsrec_datapath *
+create_br_datapath(struct ovsdb_idl_txn *ovs_idl_txn,
+   const struct ovsrec_open_vswitch *cfg,
+   const char *datapath_type)
+{
+ovsdb_idl_txn_add_comment(ovs_idl_txn,
+  "ovn-controller: creating bridge datapath '%s'",
+  datapath_type);
+
+struct ovsrec_datapath *dp = ovsrec_datapath_insert(ovs_idl_txn);
+ovsrec_open_vswitch_verify_datapaths(cfg);
+ovsrec_open_vswitch_update_datapaths_setkey(cfg, datapath_type, dp);
+return dp;
+}
+
 static const struct ovsrec_bridge *
 get_br_int(const struct ovsrec_bridge_table *bridge_table,
const struct ovsrec_open_vswitch_table *ovs_table)
@@ -399,33 +414,78 @@ get_br_int(const struct ovsrec_bridge_table *bridge_table,
 return get_bridge(bridge_table, br_int_name(cfg));
 }
 
-static const struct ovsrec_bridge *
+static const struct ovsrec_datapath *
+get_br_datapath(const struct ovsrec_open_vswitch *cfg,
+const char *datapath_type)
+{
+for (size_t i = 0; i < cfg->n_datapaths; i++) {
+if (!strcmp(cfg->key_datapaths[i], datapath_type)) {
+return cfg->value_datapaths[i];
+}
+}
+return NULL;
+}
+
+static void
 process_br_int(struct ovsdb_idl_txn *ovs_idl_txn,
const struct ovsrec_bridge_table *bridge_table,
-   const struct ovsrec_open_vswitch_table *ovs_table)
+   const struct ovsrec_open_vswitch_table *ovs_table,
+   const struct ovsrec_bridge **br_int_,
+   const struct ovsrec_datapath **br_int_dp_)
 {
-const struct ovsrec_bridge *br_int = get_br_int(bridge_table,
-ovs_table);
-if (!br_int) {
-br_int = create_br_int(ovs_idl_txn, ovs_table);
-}
-if (br_int && ovs_idl_txn) {
-const struct ovsrec_open_vswitch *cfg;
-cfg = ovsrec_open_vswitch_table_first(ovs_table);
-ovs_assert(cfg);
-const char *datapath_type = smap_get(&cfg->external_ids,
- "ovn-bridge-datapath-type");
-/* Check for the datapath_type and set it only if it is defined in
- * cfg. */
-if (datapath_type && strcmp(br_int->datapath_type, datapath_type)) {
-ovsrec_bridge_set_datapath_type(br_int, datapath_type);
+const struct ovsrec_bridge *br_int = get_br_int(bridge_table, ovs_table);
+const struct ovsrec_datapath *br_int_dp = NULL;
+if (ovs_idl_txn) {
+if (!br_int) {
+br_int = create_br_int(ovs_idl_txn, ovs_table);
 }
-if (!br_int->fail_mode || strcmp(br_int->fail_mode, "secure")) {
-ovsrec_bridge_set_fail_mode(br_int, "secure"