diff mbox series

[ovs-dev] controller: Send RARP/GARP for VIF post link state is up.

Message ID 20240527182445.233431-1-shibir.basak@nutanix.com
State Accepted
Headers show
Series [ovs-dev] controller: Send RARP/GARP for VIF post link state is up. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/github-robot-_ovn-kubernetes fail github build: failed

Commit Message

Shibir Basak May 27, 2024, 6:24 p.m. UTC 
Currently, GARP/RARP broadcast is sent for VIFs (part of logical
switch with localnet port) after iface-id is set.
This fix is to avoid packet loss during migration if iface-id
is set even before the VM migration is completed.

Signed-off-by: Shibir Basak <shibir.basak@nutanix.com>
Acked-by: Naveen Yerramneni <naveen.yerramneni@nutanix.com>
---
 controller/ovn-controller.c | 1 +
 controller/pinctrl.c        | 4 ++++
 2 files changed, 5 insertions(+)

Comments

Shibir Basak June 3, 2024, 2:25 p.m. UTC | #1 
Hi team,

Can someone please review these changes?

Thanks,
Shibir Basak

From: Shibir Basak <shibir.basak@nutanix.com>
Date: Monday, 27 May 2024 at 11:55 PM
To: dev@openvswitch.org <dev@openvswitch.org>
Cc: Shibir Basak <shibir.basak@nutanix.com>, Naveen Yerramneni <naveen.yerramneni@nutanix.com>
Subject: [PATCH ovn] controller: Send RARP/GARP for VIF post link state is up.
Currently, GARP/RARP broadcast is sent for VIFs (part of logical
switch with localnet port) after iface-id is set.
This fix is to avoid packet loss during migration if iface-id
is set even before the VM migration is completed.

Signed-off-by: Shibir Basak <shibir.basak@nutanix.com>
Acked-by: Naveen Yerramneni <naveen.yerramneni@nutanix.com>
---
 controller/ovn-controller.c | 1 +
 controller/pinctrl.c        | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 6b38f113d..982378a50 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -1128,6 +1128,7 @@ ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl)
     ovsdb_idl_add_table(ovs_idl, &ovsrec_table_queue);
     ovsdb_idl_add_column(ovs_idl, &ovsrec_queue_col_other_config);
     ovsdb_idl_add_column(ovs_idl, &ovsrec_queue_col_external_ids);
+    ovsdb_idl_add_column(ovs_idl, &ovsrec_interface_col_link_state);

     chassis_register_ovs_idl(ovs_idl);
     encaps_register_ovs_idl(ovs_idl);
diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index 6a2c3dc68..b5d3162b8 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -6375,6 +6375,10 @@ get_localnet_vifs_l3gwports(
             if (!pb || pb->chassis != chassis) {
                 continue;
             }
+            if (!iface_rec->link_state ||
+                    strcmp(iface_rec->link_state, "up")) {
+                continue;
+            }
             struct local_datapath *ld
                 = get_local_datapath(local_datapaths,
                                      pb->datapath->tunnel_key);
--
2.22.3
Mark Michelson June 6, 2024, 8:24 p.m. UTC | #2 
Thank you for the patch Shibir. It looks good to me, so

Acked-by: Mark Michelson <mmichels@redhat.com>

On 5/27/24 14:24, Shibir Basak wrote:
> Currently, GARP/RARP broadcast is sent for VIFs (part of logical
> switch with localnet port) after iface-id is set.
> This fix is to avoid packet loss during migration if iface-id
> is set even before the VM migration is completed.
> 
> Signed-off-by: Shibir Basak <shibir.basak@nutanix.com>
> Acked-by: Naveen Yerramneni <naveen.yerramneni@nutanix.com>
> ---
>   controller/ovn-controller.c | 1 +
>   controller/pinctrl.c        | 4 ++++
>   2 files changed, 5 insertions(+)
> 
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 6b38f113d..982378a50 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -1128,6 +1128,7 @@ ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl)
>       ovsdb_idl_add_table(ovs_idl, &ovsrec_table_queue);
>       ovsdb_idl_add_column(ovs_idl, &ovsrec_queue_col_other_config);
>       ovsdb_idl_add_column(ovs_idl, &ovsrec_queue_col_external_ids);
> +    ovsdb_idl_add_column(ovs_idl, &ovsrec_interface_col_link_state);
>   
>       chassis_register_ovs_idl(ovs_idl);
>       encaps_register_ovs_idl(ovs_idl);
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index 6a2c3dc68..b5d3162b8 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -6375,6 +6375,10 @@ get_localnet_vifs_l3gwports(
>               if (!pb || pb->chassis != chassis) {
>                   continue;
>               }
> +            if (!iface_rec->link_state ||
> +                    strcmp(iface_rec->link_state, "up")) {
> +                continue;
> +            }
>               struct local_datapath *ld
>                   = get_local_datapath(local_datapaths,
>                                        pb->datapath->tunnel_key);
Shibir Basak June 17, 2024, 8:40 a.m. UTC | #3 
Hi Mark,

In case we are not waiting to batch any other patches, can we merge this?

Thanks,
Shibir

From: Mark Michelson <mmichels@redhat.com>
Date: Friday, 7 June 2024 at 1:54 AM
To: Shibir Basak <shibir.basak@nutanix.com>, dev@openvswitch.org <dev@openvswitch.org>
Subject: Re: [ovs-dev] [PATCH ovn] controller: Send RARP/GARP for VIF post link state is up.
!-------------------------------------------------------------------|
  CAUTION: External Email

|-------------------------------------------------------------------!

Thank you for the patch Shibir. It looks good to me, so

Acked-by: Mark Michelson <mmichels@redhat.com>

On 5/27/24 14:24, Shibir Basak wrote:
> Currently, GARP/RARP broadcast is sent for VIFs (part of logical
> switch with localnet port) after iface-id is set.
> This fix is to avoid packet loss during migration if iface-id
> is set even before the VM migration is completed.
>
> Signed-off-by: Shibir Basak <shibir.basak@nutanix.com>
> Acked-by: Naveen Yerramneni <naveen.yerramneni@nutanix.com>
> ---
>   controller/ovn-controller.c | 1 +
>   controller/pinctrl.c        | 4 ++++
>   2 files changed, 5 insertions(+)
>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 6b38f113d..982378a50 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -1128,6 +1128,7 @@ ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl)
>       ovsdb_idl_add_table(ovs_idl, &ovsrec_table_queue);
>       ovsdb_idl_add_column(ovs_idl, &ovsrec_queue_col_other_config);
>       ovsdb_idl_add_column(ovs_idl, &ovsrec_queue_col_external_ids);
> +    ovsdb_idl_add_column(ovs_idl, &ovsrec_interface_col_link_state);
>
>       chassis_register_ovs_idl(ovs_idl);
>       encaps_register_ovs_idl(ovs_idl);
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index 6a2c3dc68..b5d3162b8 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -6375,6 +6375,10 @@ get_localnet_vifs_l3gwports(
>               if (!pb || pb->chassis != chassis) {
>                   continue;
>               }
> +            if (!iface_rec->link_state ||
> +                    strcmp(iface_rec->link_state, "up")) {
> +                continue;
> +            }
>               struct local_datapath *ld
>                   = get_local_datapath(local_datapaths,
>                                        pb->datapath->tunnel_key);
Mark Michelson June 17, 2024, 6:36 p.m. UTC | #4 
Thanks Shibir,

I merged this to main, branch-24.03, and branch-23.09.

On 5/27/24 14:24, Shibir Basak wrote:
> Currently, GARP/RARP broadcast is sent for VIFs (part of logical
> switch with localnet port) after iface-id is set.
> This fix is to avoid packet loss during migration if iface-id
> is set even before the VM migration is completed.
> 
> Signed-off-by: Shibir Basak <shibir.basak@nutanix.com>
> Acked-by: Naveen Yerramneni <naveen.yerramneni@nutanix.com>
> ---
>   controller/ovn-controller.c | 1 +
>   controller/pinctrl.c        | 4 ++++
>   2 files changed, 5 insertions(+)
> 
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 6b38f113d..982378a50 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -1128,6 +1128,7 @@ ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl)
>       ovsdb_idl_add_table(ovs_idl, &ovsrec_table_queue);
>       ovsdb_idl_add_column(ovs_idl, &ovsrec_queue_col_other_config);
>       ovsdb_idl_add_column(ovs_idl, &ovsrec_queue_col_external_ids);
> +    ovsdb_idl_add_column(ovs_idl, &ovsrec_interface_col_link_state);
>   
>       chassis_register_ovs_idl(ovs_idl);
>       encaps_register_ovs_idl(ovs_idl);
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index 6a2c3dc68..b5d3162b8 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -6375,6 +6375,10 @@ get_localnet_vifs_l3gwports(
>               if (!pb || pb->chassis != chassis) {
>                   continue;
>               }
> +            if (!iface_rec->link_state ||
> +                    strcmp(iface_rec->link_state, "up")) {
> +                continue;
> +            }
>               struct local_datapath *ld
>                   = get_local_datapath(local_datapaths,
>                                        pb->datapath->tunnel_key);
diff mbox series

Patch

diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 6b38f113d..982378a50 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -1128,6 +1128,7 @@  ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl)
     ovsdb_idl_add_table(ovs_idl, &ovsrec_table_queue);
     ovsdb_idl_add_column(ovs_idl, &ovsrec_queue_col_other_config);
     ovsdb_idl_add_column(ovs_idl, &ovsrec_queue_col_external_ids);
+    ovsdb_idl_add_column(ovs_idl, &ovsrec_interface_col_link_state);
 
     chassis_register_ovs_idl(ovs_idl);
     encaps_register_ovs_idl(ovs_idl);
diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index 6a2c3dc68..b5d3162b8 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -6375,6 +6375,10 @@  get_localnet_vifs_l3gwports(
             if (!pb || pb->chassis != chassis) {
                 continue;
             }
+            if (!iface_rec->link_state ||
+                    strcmp(iface_rec->link_state, "up")) {
+                continue;
+            }
             struct local_datapath *ld
                 = get_local_datapath(local_datapaths,
                                      pb->datapath->tunnel_key);