Re: [ovs-dev] [PATCH ovn v5 5/7] northd: Create pb after tunnel id is allocated.

2024-04-11 Thread Ihar Hrachyshka
Sorry, this is incorrect, the order of steps here is important. Instead, we
should clean up the newly created record on failure. (Other patches in the
series are ok; will send an updated series.)

On Thu, Apr 11, 2024 at 6:59 PM Ihar Hrachyshka  wrote:

> This allows to avoid cleanup of the record in case tunnel id fails to
> allocate.
>
> Signed-off-by: Ihar Hrachyshka 
> ---
>  northd/northd.c | 15 ++-
>  1 file changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index 3d2715911..5a0225189 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -4305,6 +4305,10 @@ ls_port_init(struct ovn_port *op, struct
> ovsdb_idl_txn *ovnsb_txn,
>  if (!ovn_port_assign_requested_tnl_id(sbrec_chassis_table, op)) {
>  return false;
>  }
> +/* Assign new tunnel ids where needed. */
> +if (!ovn_port_allocate_key(sbrec_chassis_table, op)) {
> +return false;
> +}
>  if (sb) {
>  op->sb = sb;
>  /* Keep nonconflicting tunnel IDs that are already assigned. */
> @@ -4318,10 +4322,6 @@ ls_port_init(struct ovn_port *op, struct
> ovsdb_idl_txn *ovnsb_txn,
>  op->sb = sbrec_port_binding_insert(ovnsb_txn);
>  sbrec_port_binding_set_logical_port(op->sb, op->key);
>  }
> -/* Assign new tunnel ids where needed. */
> -if (!ovn_port_allocate_key(sbrec_chassis_table, op)) {
> -return false;
> -}
>  ovn_port_update_sbrec(ovnsb_txn, sbrec_chassis_by_name,
>sbrec_chassis_by_hostname, NULL,
> sbrec_mirror_table,
>op, NULL, NULL);
> @@ -4343,9 +4343,6 @@ ls_port_create(struct ovsdb_idl_txn *ovnsb_txn,
> struct hmap *ls_ports,
>  if (!ls_port_init(op, ovnsb_txn, od, sb,
>sbrec_mirror_table, sbrec_chassis_table,
>sbrec_chassis_by_name, sbrec_chassis_by_hostname)) {
> -if (op->sb) {
> -sbrec_port_binding_delete(op->sb);
> -}
>  ovn_port_destroy(ls_ports, op);
>  return NULL;
>  }
> @@ -4549,8 +4546,8 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
>  ni->sbrec_chassis_table,
>  ni->sbrec_chassis_by_name,
>  ni->sbrec_chassis_by_hostname)) {
> -if (op->sb) {
> -sbrec_port_binding_delete(op->sb);
> +if (sb) {
> +sbrec_port_binding_delete(sb);
>  }
>  ovs_list_remove(>list);
>  ovn_port_destroy(>ls_ports, op);
> --
> 2.41.0
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH ovn v5 5/7] northd: Create pb after tunnel id is allocated.

2024-04-11 Thread Ihar Hrachyshka
This allows to avoid cleanup of the record in case tunnel id fails to
allocate.

Signed-off-by: Ihar Hrachyshka 
---
 northd/northd.c | 15 ++-
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/northd/northd.c b/northd/northd.c
index 3d2715911..5a0225189 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -4305,6 +4305,10 @@ ls_port_init(struct ovn_port *op, struct ovsdb_idl_txn 
*ovnsb_txn,
 if (!ovn_port_assign_requested_tnl_id(sbrec_chassis_table, op)) {
 return false;
 }
+/* Assign new tunnel ids where needed. */
+if (!ovn_port_allocate_key(sbrec_chassis_table, op)) {
+return false;
+}
 if (sb) {
 op->sb = sb;
 /* Keep nonconflicting tunnel IDs that are already assigned. */
@@ -4318,10 +4322,6 @@ ls_port_init(struct ovn_port *op, struct ovsdb_idl_txn 
*ovnsb_txn,
 op->sb = sbrec_port_binding_insert(ovnsb_txn);
 sbrec_port_binding_set_logical_port(op->sb, op->key);
 }
-/* Assign new tunnel ids where needed. */
-if (!ovn_port_allocate_key(sbrec_chassis_table, op)) {
-return false;
-}
 ovn_port_update_sbrec(ovnsb_txn, sbrec_chassis_by_name,
   sbrec_chassis_by_hostname, NULL, sbrec_mirror_table,
   op, NULL, NULL);
@@ -4343,9 +4343,6 @@ ls_port_create(struct ovsdb_idl_txn *ovnsb_txn, struct 
hmap *ls_ports,
 if (!ls_port_init(op, ovnsb_txn, od, sb,
   sbrec_mirror_table, sbrec_chassis_table,
   sbrec_chassis_by_name, sbrec_chassis_by_hostname)) {
-if (op->sb) {
-sbrec_port_binding_delete(op->sb);
-}
 ovn_port_destroy(ls_ports, op);
 return NULL;
 }
@@ -4549,8 +4546,8 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn *ovnsb_idl_txn,
 ni->sbrec_chassis_table,
 ni->sbrec_chassis_by_name,
 ni->sbrec_chassis_by_hostname)) {
-if (op->sb) {
-sbrec_port_binding_delete(op->sb);
+if (sb) {
+sbrec_port_binding_delete(sb);
 }
 ovs_list_remove(>list);
 ovn_port_destroy(>ls_ports, op);
-- 
2.41.0

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