Re: [ovs-dev] [PATCH v2 4/9] ovsdb: row: Add support for xor-based row updates.

2021-07-12 Thread Ilya Maximets 
On 6/19/21 8:01 PM, Mark Gray wrote:
> On 12/06/2021 03:00, Ilya Maximets wrote:
>> This will be used to apply update3 type updates to ovsdb tables
>> while processing updates for future ovsdb 'relay' service model.
>>
>> 'ovsdb_datum_apply_diff' is allowed to fail, so adding support
>> to return this error.
>>
>> Signed-off-by: Ilya Maximets 
>> ---
>>  ovsdb/execution.c   |  5 +++--
>>  ovsdb/replication.c |  2 +-
>>  ovsdb/row.c | 30 +-
>>  ovsdb/row.h |  6 --
>>  ovsdb/table.c   |  9 +
>>  ovsdb/table.h   |  2 +-
>>  6 files changed, 39 insertions(+), 15 deletions(-)
>>
>> diff --git a/ovsdb/execution.c b/ovsdb/execution.c
>> index 3a0dad5d0..f6150e944 100644
>> --- a/ovsdb/execution.c
>> +++ b/ovsdb/execution.c
>> @@ -483,8 +483,9 @@ update_row_cb(const struct ovsdb_row *row, void *ur_)
>>  
>>  ur->n_matches++;
>>  if (!ovsdb_row_equal_columns(row, ur->row, ur->columns)) {
>> -ovsdb_row_update_columns(ovsdb_txn_row_modify(ur->txn, row),
>> - ur->row, ur->columns);
>> +ovsdb_error_assert(ovsdb_row_update_columns(
>> +   ovsdb_txn_row_modify(ur->txn, row),
>> +   ur->row, ur->columns, false));
>>  }
>>  
>>  return true;
>> diff --git a/ovsdb/replication.c b/ovsdb/replication.c
>> index b755976b0..d8b56d813 100644
>> --- a/ovsdb/replication.c
>> +++ b/ovsdb/replication.c
>> @@ -677,7 +677,7 @@ process_table_update(struct json *table_update, const 
>> char *table_name,
>>  struct ovsdb_error *error;
>>  error = (!new ? ovsdb_table_execute_delete(txn, &uuid, table)
>>   : !old ? ovsdb_table_execute_insert(txn, &uuid, table, new)
>> - : ovsdb_table_execute_update(txn, &uuid, table, new));
>> + : ovsdb_table_execute_update(txn, &uuid, table, new, 
>> false));
>>  if (error) {
>>  if (!strcmp(ovsdb_error_get_tag(error), "consistency 
>> violation")) {
>>  ovsdb_error_assert(error);
>> diff --git a/ovsdb/row.c b/ovsdb/row.c
>> index 755ab91a8..65a054621 100644
>> --- a/ovsdb/row.c
>> +++ b/ovsdb/row.c
>> @@ -163,20 +163,40 @@ ovsdb_row_equal_columns(const struct ovsdb_row *a,
>>  return true;
>>  }
>>  
>> -void
>> +struct ovsdb_error *
>>  ovsdb_row_update_columns(struct ovsdb_row *dst,
>>   const struct ovsdb_row *src,
>> - const struct ovsdb_column_set *columns)
>> + const struct ovsdb_column_set *columns,
>> + bool xor)
>>  {
>>  size_t i;
>>  
>>  for (i = 0; i < columns->n_columns; i++) {
>>  const struct ovsdb_column *column = columns->columns[i];
>> +struct ovsdb_datum xor_datum;
>> +struct ovsdb_error *error;
>> +
>> +if (xor) {
>> +error = ovsdb_datum_apply_diff(&xor_datum,
>> +   &dst->fields[column->index],
>> +   &src->fields[column->index],
>> +   &column->type);
>> +if (error) {
>> +return error;
>> +}
>> +}
>> +
>>  ovsdb_datum_destroy(&dst->fields[column->index], &column->type);
>> -ovsdb_datum_clone(&dst->fields[column->index],
>> -  &src->fields[column->index],
>> -  &column->type);
> 
> Could you move ovsdb_datum_destroy(&dst->fields[column->index],
> &column->type) into the "else" clause below and then merge the "if"
> clause below into the "if" clause above?

We still need to destroy for both branches, so what I can do is
something like this:

@@ -184,13 +185,11 @@ ovsdb_row_update_columns(struct ovsdb_row *dst,
 if (error) {
 return error;
 }
-}
 
-ovsdb_datum_destroy(&dst->fields[column->index], &column->type);
-
-if (xor) {
+ovsdb_datum_destroy(&dst->fields[column->index], &column->type);
 ovsdb_datum_swap(&dst->fields[column->index], &xor_datum);
 } else {
+ovsdb_datum_destroy(&dst->fields[column->index], &column->type);
 ovsdb_datum_clone(&dst->fields[column->index],
   &src->fields[column->index],
   &column->type);
---

i.e. copy the ovsdb_datum_destroy() to both branches and merge the "if"s,
but I found this harder to read.

I'll keep as is for now, but if you think that above version will be better,
I can use it.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 4/9] ovsdb: row: Add support for xor-based row updates.

2021-06-25 Thread Dumitru Ceara 
On 6/12/21 4:00 AM, Ilya Maximets wrote:
> This will be used to apply update3 type updates to ovsdb tables
> while processing updates for future ovsdb 'relay' service model.
> 
> 'ovsdb_datum_apply_diff' is allowed to fail, so adding support
> to return this error.
> 
> Signed-off-by: Ilya Maximets 
> ---

Acked-by: Dumitru Ceara 

Thanks!

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

Re: [ovs-dev] [PATCH v2 4/9] ovsdb: row: Add support for xor-based row updates.

2021-06-19 Thread Mark Gray 
On 12/06/2021 03:00, Ilya Maximets wrote:
> This will be used to apply update3 type updates to ovsdb tables
> while processing updates for future ovsdb 'relay' service model.
> 
> 'ovsdb_datum_apply_diff' is allowed to fail, so adding support
> to return this error.
> 
> Signed-off-by: Ilya Maximets 
> ---
>  ovsdb/execution.c   |  5 +++--
>  ovsdb/replication.c |  2 +-
>  ovsdb/row.c | 30 +-
>  ovsdb/row.h |  6 --
>  ovsdb/table.c   |  9 +
>  ovsdb/table.h   |  2 +-
>  6 files changed, 39 insertions(+), 15 deletions(-)
> 
> diff --git a/ovsdb/execution.c b/ovsdb/execution.c
> index 3a0dad5d0..f6150e944 100644
> --- a/ovsdb/execution.c
> +++ b/ovsdb/execution.c
> @@ -483,8 +483,9 @@ update_row_cb(const struct ovsdb_row *row, void *ur_)
>  
>  ur->n_matches++;
>  if (!ovsdb_row_equal_columns(row, ur->row, ur->columns)) {
> -ovsdb_row_update_columns(ovsdb_txn_row_modify(ur->txn, row),
> - ur->row, ur->columns);
> +ovsdb_error_assert(ovsdb_row_update_columns(
> +   ovsdb_txn_row_modify(ur->txn, row),
> +   ur->row, ur->columns, false));
>  }
>  
>  return true;
> diff --git a/ovsdb/replication.c b/ovsdb/replication.c
> index b755976b0..d8b56d813 100644
> --- a/ovsdb/replication.c
> +++ b/ovsdb/replication.c
> @@ -677,7 +677,7 @@ process_table_update(struct json *table_update, const 
> char *table_name,
>  struct ovsdb_error *error;
>  error = (!new ? ovsdb_table_execute_delete(txn, &uuid, table)
>   : !old ? ovsdb_table_execute_insert(txn, &uuid, table, new)
> - : ovsdb_table_execute_update(txn, &uuid, table, new));
> + : ovsdb_table_execute_update(txn, &uuid, table, new, 
> false));
>  if (error) {
>  if (!strcmp(ovsdb_error_get_tag(error), "consistency 
> violation")) {
>  ovsdb_error_assert(error);
> diff --git a/ovsdb/row.c b/ovsdb/row.c
> index 755ab91a8..65a054621 100644
> --- a/ovsdb/row.c
> +++ b/ovsdb/row.c
> @@ -163,20 +163,40 @@ ovsdb_row_equal_columns(const struct ovsdb_row *a,
>  return true;
>  }
>  
> -void
> +struct ovsdb_error *
>  ovsdb_row_update_columns(struct ovsdb_row *dst,
>   const struct ovsdb_row *src,
> - const struct ovsdb_column_set *columns)
> + const struct ovsdb_column_set *columns,
> + bool xor)
>  {
>  size_t i;
>  
>  for (i = 0; i < columns->n_columns; i++) {
>  const struct ovsdb_column *column = columns->columns[i];
> +struct ovsdb_datum xor_datum;
> +struct ovsdb_error *error;
> +
> +if (xor) {
> +error = ovsdb_datum_apply_diff(&xor_datum,
> +   &dst->fields[column->index],
> +   &src->fields[column->index],
> +   &column->type);
> +if (error) {
> +return error;
> +}
> +}
> +
>  ovsdb_datum_destroy(&dst->fields[column->index], &column->type);
> -ovsdb_datum_clone(&dst->fields[column->index],
> -  &src->fields[column->index],
> -  &column->type);

Could you move ovsdb_datum_destroy(&dst->fields[column->index],
&column->type) into the "else" clause below and then merge the "if"
clause below into the "if" clause above?
> +
> +if (xor) {
> +ovsdb_datum_swap(&dst->fields[column->index], &xor_datum);
> +} else {
> +ovsdb_datum_clone(&dst->fields[column->index],
> +  &src->fields[column->index],
> +  &column->type);
> +}
>  }
> +return NULL;
>  }
>  
>  /* Appends the string form of the value in 'row' of each of the columns in
> diff --git a/ovsdb/row.h b/ovsdb/row.h
> index 2c441b5a4..394ac8eb4 100644
> --- a/ovsdb/row.h
> +++ b/ovsdb/row.h
> @@ -82,8 +82,10 @@ bool ovsdb_row_equal_columns(const struct ovsdb_row *,
>  int ovsdb_row_compare_columns_3way(const struct ovsdb_row *,
> const struct ovsdb_row *,
> const struct ovsdb_column_set *);
> -void ovsdb_row_update_columns(struct ovsdb_row *, const struct ovsdb_row *,
> -  const struct ovsdb_column_set *);
> +struct ovsdb_error *ovsdb_row_update_columns(struct ovsdb_row *,
> + const struct ovsdb_row *,
> + const struct ovsdb_column_set *,
> + bool xor);
>  void ovsdb_row_columns_to_string(const struct ovsdb_row *,
>   const struct ovsdb_column_set *, struct ds 
> *);
>  struct ovsdb_error *ovsdb_row_fro

[ovs-dev] [PATCH v2 4/9] ovsdb: row: Add support for xor-based row updates.

2021-06-11 Thread Ilya Maximets 
This will be used to apply update3 type updates to ovsdb tables
while processing updates for future ovsdb 'relay' service model.

'ovsdb_datum_apply_diff' is allowed to fail, so adding support
to return this error.

Signed-off-by: Ilya Maximets 
---
 ovsdb/execution.c   |  5 +++--
 ovsdb/replication.c |  2 +-
 ovsdb/row.c | 30 +-
 ovsdb/row.h |  6 --
 ovsdb/table.c   |  9 +
 ovsdb/table.h   |  2 +-
 6 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/ovsdb/execution.c b/ovsdb/execution.c
index 3a0dad5d0..f6150e944 100644
--- a/ovsdb/execution.c
+++ b/ovsdb/execution.c
@@ -483,8 +483,9 @@ update_row_cb(const struct ovsdb_row *row, void *ur_)
 
 ur->n_matches++;
 if (!ovsdb_row_equal_columns(row, ur->row, ur->columns)) {
-ovsdb_row_update_columns(ovsdb_txn_row_modify(ur->txn, row),
- ur->row, ur->columns);
+ovsdb_error_assert(ovsdb_row_update_columns(
+   ovsdb_txn_row_modify(ur->txn, row),
+   ur->row, ur->columns, false));
 }
 
 return true;
diff --git a/ovsdb/replication.c b/ovsdb/replication.c
index b755976b0..d8b56d813 100644
--- a/ovsdb/replication.c
+++ b/ovsdb/replication.c
@@ -677,7 +677,7 @@ process_table_update(struct json *table_update, const char 
*table_name,
 struct ovsdb_error *error;
 error = (!new ? ovsdb_table_execute_delete(txn, &uuid, table)
  : !old ? ovsdb_table_execute_insert(txn, &uuid, table, new)
- : ovsdb_table_execute_update(txn, &uuid, table, new));
+ : ovsdb_table_execute_update(txn, &uuid, table, new, false));
 if (error) {
 if (!strcmp(ovsdb_error_get_tag(error), "consistency violation")) {
 ovsdb_error_assert(error);
diff --git a/ovsdb/row.c b/ovsdb/row.c
index 755ab91a8..65a054621 100644
--- a/ovsdb/row.c
+++ b/ovsdb/row.c
@@ -163,20 +163,40 @@ ovsdb_row_equal_columns(const struct ovsdb_row *a,
 return true;
 }
 
-void
+struct ovsdb_error *
 ovsdb_row_update_columns(struct ovsdb_row *dst,
  const struct ovsdb_row *src,
- const struct ovsdb_column_set *columns)
+ const struct ovsdb_column_set *columns,
+ bool xor)
 {
 size_t i;
 
 for (i = 0; i < columns->n_columns; i++) {
 const struct ovsdb_column *column = columns->columns[i];
+struct ovsdb_datum xor_datum;
+struct ovsdb_error *error;
+
+if (xor) {
+error = ovsdb_datum_apply_diff(&xor_datum,
+   &dst->fields[column->index],
+   &src->fields[column->index],
+   &column->type);
+if (error) {
+return error;
+}
+}
+
 ovsdb_datum_destroy(&dst->fields[column->index], &column->type);
-ovsdb_datum_clone(&dst->fields[column->index],
-  &src->fields[column->index],
-  &column->type);
+
+if (xor) {
+ovsdb_datum_swap(&dst->fields[column->index], &xor_datum);
+} else {
+ovsdb_datum_clone(&dst->fields[column->index],
+  &src->fields[column->index],
+  &column->type);
+}
 }
+return NULL;
 }
 
 /* Appends the string form of the value in 'row' of each of the columns in
diff --git a/ovsdb/row.h b/ovsdb/row.h
index 2c441b5a4..394ac8eb4 100644
--- a/ovsdb/row.h
+++ b/ovsdb/row.h
@@ -82,8 +82,10 @@ bool ovsdb_row_equal_columns(const struct ovsdb_row *,
 int ovsdb_row_compare_columns_3way(const struct ovsdb_row *,
const struct ovsdb_row *,
const struct ovsdb_column_set *);
-void ovsdb_row_update_columns(struct ovsdb_row *, const struct ovsdb_row *,
-  const struct ovsdb_column_set *);
+struct ovsdb_error *ovsdb_row_update_columns(struct ovsdb_row *,
+ const struct ovsdb_row *,
+ const struct ovsdb_column_set *,
+ bool xor);
 void ovsdb_row_columns_to_string(const struct ovsdb_row *,
  const struct ovsdb_column_set *, struct ds *);
 struct ovsdb_error *ovsdb_row_from_json(struct ovsdb_row *,
diff --git a/ovsdb/table.c b/ovsdb/table.c
index 2935bd897..455a3663f 100644
--- a/ovsdb/table.c
+++ b/ovsdb/table.c
@@ -384,7 +384,8 @@ ovsdb_table_execute_delete(struct ovsdb_txn *txn, const 
struct uuid *row_uuid,
 
 struct ovsdb_error *
 ovsdb_table_execute_update(struct ovsdb_txn *txn, const struct uuid *row_uuid,
-   struct ovsdb_table *table, struct json *json_row)
+