diff mbox series

[ovs-dev,branch-23.03,2/5] Track interface MTU in if-status-mgr

Message ID 20230531200451.3541416-2-ihrachys@redhat.com
State Accepted
Headers show
Series [ovs-dev,branch-23.03,1/5] Track ip version of tunnel in chassis_tunnel struct | expand

Checks

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

Commit Message

Ihar Hrachyshka May 31, 2023, 8:04 p.m. UTC
This will be used in a later patch to calculate the effective interface
MTU after considering tunneling overhead.

NOTE: ideally, OVN would support Logical_Port MTU, in which case we
wouldn't have to track OVSDB for interfaces.

Signed-off-by: Ihar Hrachyshka <ihrachys@redhat.com>
Acked-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Mark Michelson <mmichels@redhat.com>
(cherry picked from commit 6562f50edf8e03f7018ae8decc442905ac7f6325)
---
 controller/binding.c        |   4 +-
 controller/if-status.c      |  40 ++++++++++--
 controller/if-status.h      |   5 ++
 controller/ovn-controller.c | 122 ++++++++++++++++++++++++++++++++++++
 controller/ovsport.c        |   9 +++
 controller/ovsport.h        |   2 +
 controller/physical.h       |   2 +
 7 files changed, 178 insertions(+), 6 deletions(-)

Comments

0-day Robot May 31, 2023, 8:15 p.m. UTC | #1
Bleep bloop.  Greetings Ihar Hrachyshka, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: Mark Michelson <mmichels@redhat.com>
Lines checked: 415, Warnings: 1, Errors: 0


Please check this out.  If you feel there has been an error, please email aconole@redhat.com

Thanks,
0-day Robot
diff mbox series

Patch

diff --git a/controller/binding.c b/controller/binding.c
index 248de5148..11e99911c 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -1273,7 +1273,7 @@  claim_lport(const struct sbrec_port_binding *pb,
                 }
                 set_pb_chassis_in_sbrec(pb, chassis_rec, true);
             } else {
-                if_status_mgr_claim_iface(if_mgr, pb, chassis_rec,
+                if_status_mgr_claim_iface(if_mgr, pb, chassis_rec, iface_rec,
                                           sb_readonly);
             }
             register_claim_timestamp(pb->logical_port, now);
@@ -1288,7 +1288,7 @@  claim_lport(const struct sbrec_port_binding *pb,
                     !smap_get_bool(&iface_rec->external_ids,
                                    OVN_INSTALLED_EXT_ID, false)) {
                     if_status_mgr_claim_iface(if_mgr, pb, chassis_rec,
-                                              sb_readonly);
+                                              iface_rec, sb_readonly);
                 }
             }
         }
diff --git a/controller/if-status.c b/controller/if-status.c
index 8503e5daa..2bff284d6 100644
--- a/controller/if-status.c
+++ b/controller/if-status.c
@@ -18,12 +18,14 @@ 
 #include "binding.h"
 #include "if-status.h"
 #include "ofctrl-seqno.h"
+#include "ovsport.h"
 #include "simap.h"
 
 #include "lib/hmapx.h"
 #include "lib/util.h"
 #include "timeval.h"
 #include "openvswitch/vlog.h"
+#include "lib/vswitch-idl.h"
 #include "lib/ovn-sb-idl.h"
 
 VLOG_DEFINE_THIS_MODULE(if_status);
@@ -181,6 +183,7 @@  struct ovs_iface {
                              * be fully programmed in OVS.  Only used in state
                              * OIF_INSTALL_FLOWS.
                              */
+    uint16_t mtu;           /* Extracted from OVS interface.mtu field. */
 };
 
 static uint64_t ifaces_usage;
@@ -205,9 +208,10 @@  struct if_status_mgr {
     uint32_t iface_seqno;
 };
 
-static struct ovs_iface *ovs_iface_create(struct if_status_mgr *,
-                                          const char *iface_id,
-                                          enum if_state );
+static struct ovs_iface *
+ovs_iface_create(struct if_status_mgr *, const char *iface_id,
+                 const struct ovsrec_interface *iface_rec,
+                 enum if_state);
 static void add_to_ovn_uninstall_hash(struct if_status_mgr *, const char *,
                                       const struct uuid *);
 static void ovs_iface_destroy(struct if_status_mgr *, struct ovs_iface *);
@@ -272,13 +276,14 @@  void
 if_status_mgr_claim_iface(struct if_status_mgr *mgr,
                           const struct sbrec_port_binding *pb,
                           const struct sbrec_chassis *chassis_rec,
+                          const struct ovsrec_interface *iface_rec,
                           bool sb_readonly)
 {
     const char *iface_id = pb->logical_port;
     struct ovs_iface *iface = shash_find_data(&mgr->ifaces, iface_id);
 
     if (!iface) {
-        iface = ovs_iface_create(mgr, iface_id, OIF_CLAIMED);
+        iface = ovs_iface_create(mgr, iface_id, iface_rec, OIF_CLAIMED);
     }
 
     memcpy(&iface->pb_uuid, &pb->header_.uuid, sizeof(iface->pb_uuid));
@@ -639,8 +644,34 @@  ovn_uninstall_hash_account_mem(const char *name, bool erase)
     }
 }
 
+uint16_t
+if_status_mgr_iface_get_mtu(const struct if_status_mgr *mgr,
+                            const char *iface_id)
+{
+    const struct ovs_iface *iface = shash_find_data(&mgr->ifaces, iface_id);
+    return iface ? iface->mtu : 0;
+}
+
+bool
+if_status_mgr_iface_update(const struct if_status_mgr *mgr,
+                           const struct ovsrec_interface *iface_rec)
+{
+    const char *iface_id = smap_get(&iface_rec->external_ids, "iface-id");
+    if (!iface_id) {
+        return false;
+    }
+    uint16_t mtu = get_iface_mtu(iface_rec);
+    struct ovs_iface *iface = shash_find_data(&mgr->ifaces, iface_id);
+    if (iface && iface->mtu != mtu) {
+        iface->mtu = mtu;
+        return true;
+    }
+    return false;
+}
+
 static struct ovs_iface *
 ovs_iface_create(struct if_status_mgr *mgr, const char *iface_id,
+                 const struct ovsrec_interface *iface_rec,
                  enum if_state state)
 {
     struct ovs_iface *iface = xzalloc(sizeof *iface);
@@ -650,6 +681,7 @@  ovs_iface_create(struct if_status_mgr *mgr, const char *iface_id,
     shash_add_nocopy(&mgr->ifaces, iface->id, iface);
     ovs_iface_set_state(mgr, iface, state);
     ovs_iface_account_mem(iface_id, false);
+    if_status_mgr_iface_update(mgr, iface_rec);
     return iface;
 }
 
diff --git a/controller/if-status.h b/controller/if-status.h
index 8ba80acd9..55979ece4 100644
--- a/controller/if-status.h
+++ b/controller/if-status.h
@@ -30,6 +30,7 @@  void if_status_mgr_destroy(struct if_status_mgr *);
 void if_status_mgr_claim_iface(struct if_status_mgr *,
                                const struct sbrec_port_binding *pb,
                                const struct sbrec_chassis *chassis_rec,
+                               const struct ovsrec_interface *iface_rec,
                                bool sb_readonly);
 void if_status_mgr_release_iface(struct if_status_mgr *, const char *iface_id);
 void if_status_mgr_delete_iface(struct if_status_mgr *, const char *iface_id);
@@ -56,5 +57,9 @@  bool if_status_handle_claims(struct if_status_mgr *mgr,
 void if_status_mgr_remove_ovn_installed(struct if_status_mgr *mgr,
                                         const char *name,
                                         const struct uuid *uuid);
+uint16_t if_status_mgr_iface_get_mtu(const struct if_status_mgr *mgr,
+                                     const char *iface_id);
+bool if_status_mgr_iface_update(const struct if_status_mgr *mgr,
+                                const struct ovsrec_interface *iface_rec);
 
 # endif /* controller/if-status.h */
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 13a7d46ef..70c8623ce 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -60,6 +60,7 @@ 
 #include "lib/ovn-dirs.h"
 #include "lib/ovn-sb-idl.h"
 #include "lib/ovn-util.h"
+#include "ovsport.h"
 #include "patch.h"
 #include "vif-plug.h"
 #include "vif-plug-provider.h"
@@ -1060,6 +1061,7 @@  ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl)
     ovsdb_idl_track_add_column(ovs_idl, &ovsrec_interface_col_name);
     ovsdb_idl_track_add_column(ovs_idl, &ovsrec_interface_col_bfd);
     ovsdb_idl_track_add_column(ovs_idl, &ovsrec_interface_col_bfd_status);
+    ovsdb_idl_track_add_column(ovs_idl, &ovsrec_interface_col_mtu);
     ovsdb_idl_track_add_column(ovs_idl, &ovsrec_interface_col_type);
     ovsdb_idl_track_add_column(ovs_idl, &ovsrec_interface_col_options);
     ovsdb_idl_track_add_column(ovs_idl, &ovsrec_interface_col_ofport);
@@ -1158,6 +1160,56 @@  en_ofctrl_is_connected_run(struct engine_node *node, void *data)
     engine_set_node_state(node, EN_UNCHANGED);
 }
 
+struct ed_type_if_status_mgr {
+    const struct if_status_mgr *manager;
+    const struct ovsrec_interface_table *iface_table;
+};
+
+static void *
+en_if_status_mgr_init(struct engine_node *node OVS_UNUSED,
+                      struct engine_arg *arg OVS_UNUSED)
+{
+    struct ed_type_if_status_mgr *data = xzalloc(sizeof *data);
+    return data;
+}
+
+static void
+en_if_status_mgr_cleanup(void *data OVS_UNUSED)
+{
+}
+
+static void
+en_if_status_mgr_run(struct engine_node *node, void *data_)
+{
+    enum engine_node_state state = EN_UNCHANGED;
+    struct ed_type_if_status_mgr *data = data_;
+    struct controller_engine_ctx *ctrl_ctx = engine_get_context()->client_ctx;
+    data->manager = ctrl_ctx->if_mgr;
+    data->iface_table = EN_OVSDB_GET(engine_get_input("OVS_interface", node));
+
+    const struct ovsrec_interface *iface;
+    OVSREC_INTERFACE_TABLE_FOR_EACH (iface, data->iface_table) {
+        if (if_status_mgr_iface_update(data->manager, iface)) {
+            state = EN_UPDATED;
+        }
+    }
+    engine_set_node_state(node, state);
+}
+
+static bool
+if_status_mgr_ovs_interface_handler(struct engine_node *node, void *data)
+{
+    struct ed_type_if_status_mgr *data_ = data;
+
+    const struct ovsrec_interface *iface;
+    OVSREC_INTERFACE_TABLE_FOR_EACH_TRACKED (iface, data_->iface_table) {
+        if (if_status_mgr_iface_update(data_->manager, iface)) {
+            engine_set_node_state(node, EN_UPDATED);
+        }
+    }
+    return true;
+}
+
 /* This engine node is to wrap the OVS_interface input and maintain a copy of
  * the old version of data for the column external_ids.
  *
@@ -4056,6 +4108,9 @@  static void init_physical_ctx(struct engine_node *node,
     const struct ed_type_mff_ovn_geneve *ed_mff_ovn_geneve =
         engine_get_input_data("mff_ovn_geneve", node);
 
+    const struct ovsrec_interface_table *ovs_interface_table =
+        EN_OVSDB_GET(engine_get_input("if_status_mgr", node));
+
     const struct ovsrec_open_vswitch_table *ovs_table =
         EN_OVSDB_GET(engine_get_input("OVS_open_vswitch", node));
     const struct ovsrec_bridge_table *bridge_table =
@@ -4080,6 +4135,7 @@  static void init_physical_ctx(struct engine_node *node,
     p_ctx->sbrec_port_binding_by_name = sbrec_port_binding_by_name;
     p_ctx->sbrec_port_binding_by_datapath = sbrec_port_binding_by_datapath;
     p_ctx->port_binding_table = port_binding_table;
+    p_ctx->ovs_interface_table = ovs_interface_table;
     p_ctx->mc_group_table = multicast_group_table;
     p_ctx->br_int = br_int;
     p_ctx->chassis_table = chassis_table;
@@ -4093,6 +4149,9 @@  static void init_physical_ctx(struct engine_node *node,
     p_ctx->patch_ofports = &non_vif_data->patch_ofports;
     p_ctx->chassis_tunnels = &non_vif_data->chassis_tunnels;
 
+    struct controller_engine_ctx *ctrl_ctx = engine_get_context()->client_ctx;
+    p_ctx->if_mgr = ctrl_ctx->if_mgr;
+
     pflow_output_get_debug(node, &p_ctx->debug);
 }
 
@@ -4136,6 +4195,63 @@  en_pflow_output_run(struct engine_node *node, void *data)
     engine_set_node_state(node, EN_UPDATED);
 }
 
+static bool
+pflow_output_if_status_mgr_handler(struct engine_node *node,
+                                   void *data)
+{
+    struct ed_type_pflow_output *pfo = data;
+    struct ed_type_runtime_data *rt_data =
+        engine_get_input_data("runtime_data", node);
+    struct ed_type_non_vif_data *non_vif_data =
+        engine_get_input_data("non_vif_data", node);
+    struct ed_type_if_status_mgr *if_mgr_data =
+        engine_get_input_data("if_status_mgr", node);
+
+    struct physical_ctx p_ctx;
+    init_physical_ctx(node, rt_data, non_vif_data, &p_ctx);
+
+    const struct ovsrec_interface *iface;
+    OVSREC_INTERFACE_TABLE_FOR_EACH_TRACKED (iface, if_mgr_data->iface_table) {
+        const char *iface_id = smap_get(&iface->external_ids, "iface-id");
+        if (!iface_id) {
+            continue;
+        }
+
+        const struct sbrec_port_binding *pb = lport_lookup_by_name(
+            p_ctx.sbrec_port_binding_by_name, iface_id);
+        if (!pb) {
+            continue;
+        }
+        if (pb->n_additional_chassis) {
+            /* Update flows for all ports in datapath. */
+            struct sbrec_port_binding *target =
+                sbrec_port_binding_index_init_row(
+                    p_ctx.sbrec_port_binding_by_datapath);
+            sbrec_port_binding_index_set_datapath(target, pb->datapath);
+
+            const struct sbrec_port_binding *binding;
+            SBREC_PORT_BINDING_FOR_EACH_EQUAL (
+                    binding, target, p_ctx.sbrec_port_binding_by_datapath) {
+                bool removed = sbrec_port_binding_is_deleted(binding);
+                if (!physical_handle_flows_for_lport(binding, removed, &p_ctx,
+                                                     &pfo->flow_table)) {
+                    return false;
+                }
+            }
+            sbrec_port_binding_index_destroy_row(target);
+        } else {
+            /* If any multichassis ports, update flows for the port. */
+            bool removed = sbrec_port_binding_is_deleted(pb);
+            if (!physical_handle_flows_for_lport(pb, removed, &p_ctx,
+                                                 &pfo->flow_table)) {
+                return false;
+            }
+        }
+        engine_set_node_state(node, EN_UPDATED);
+    }
+    return true;
+}
+
 static bool
 pflow_output_sb_port_binding_handler(struct engine_node *node,
                                      void *data)
@@ -4619,6 +4735,7 @@  main(int argc, char *argv[])
     ENGINE_NODE_WITH_CLEAR_TRACK_DATA(port_groups, "port_groups");
     ENGINE_NODE(northd_options, "northd_options");
     ENGINE_NODE(dhcp_options, "dhcp_options");
+    ENGINE_NODE(if_status_mgr, "if_status_mgr");
     ENGINE_NODE_WITH_CLEAR_TRACK_DATA(lb_data, "lb_data");
 
 #define SB_NODE(NAME, NAME_STR) ENGINE_NODE_SB(NAME, NAME_STR);
@@ -4657,6 +4774,9 @@  main(int argc, char *argv[])
     engine_add_input(&en_non_vif_data, &en_ovs_interface,
                      non_vif_data_ovs_iface_handler);
 
+    engine_add_input(&en_if_status_mgr, &en_ovs_interface,
+                     if_status_mgr_ovs_interface_handler);
+
     /* Note: The order of inputs is important, all OVS interface changes must
      * be handled before any ct_zone changes.
      */
@@ -4667,6 +4787,8 @@  main(int argc, char *argv[])
     engine_add_input(&en_pflow_output, &en_sb_chassis,
                      pflow_lflow_output_sb_chassis_handler);
 
+    engine_add_input(&en_pflow_output, &en_if_status_mgr,
+                     pflow_output_if_status_mgr_handler);
     engine_add_input(&en_pflow_output, &en_sb_port_binding,
                      pflow_output_sb_port_binding_handler);
     engine_add_input(&en_pflow_output, &en_sb_multicast_group,
diff --git a/controller/ovsport.c b/controller/ovsport.c
index ec38c3fca..ebcb9cb6d 100644
--- a/controller/ovsport.c
+++ b/controller/ovsport.c
@@ -264,3 +264,12 @@  maintain_interface_smap_column(
         }
     }
 }
+
+uint16_t
+get_iface_mtu(const struct ovsrec_interface *iface)
+{
+    if (!iface || !iface->n_mtu || iface->mtu[0] <= 0) {
+        return 0;
+    }
+    return (uint16_t) iface->mtu[0];
+}
diff --git a/controller/ovsport.h b/controller/ovsport.h
index e355ff7ff..c40c1855a 100644
--- a/controller/ovsport.h
+++ b/controller/ovsport.h
@@ -57,4 +57,6 @@  const struct ovsrec_port * ovsport_lookup_by_interfaces(
 const struct ovsrec_port * ovsport_lookup_by_interface(
         struct ovsdb_idl_index *, struct ovsrec_interface *);
 
+uint16_t get_iface_mtu(const struct ovsrec_interface *);
+
 #endif /* lib/ovsport.h */
diff --git a/controller/physical.h b/controller/physical.h
index f450dca94..1f1ed55ef 100644
--- a/controller/physical.h
+++ b/controller/physical.h
@@ -52,11 +52,13 @@  struct physical_ctx {
     struct ovsdb_idl_index *sbrec_port_binding_by_name;
     struct ovsdb_idl_index *sbrec_port_binding_by_datapath;
     const struct sbrec_port_binding_table *port_binding_table;
+    const struct ovsrec_interface_table *ovs_interface_table;
     const struct sbrec_multicast_group_table *mc_group_table;
     const struct ovsrec_bridge *br_int;
     const struct sbrec_chassis_table *chassis_table;
     const struct sbrec_chassis *chassis;
     const struct sset *active_tunnels;
+    const struct if_status_mgr *if_mgr;
     struct hmap *local_datapaths;
     struct sset *local_lports;
     const struct simap *ct_zones;