On Sep 22, Jacob Tanenbaum wrote:
> When a router is created or destroyed add the ability to en-lflow
> engine to compute the new router lflows instead of recalculating the
> whole database.
>
> Acked-by: Lorenzo Bianconi <lorenzo.bianc...@redhat.com>
> Reported-by: Dumitru Ceara <dce...@redhat.com>
> Reported-at: https://issues.redhat.com/browse/FDP-757
> Signed-off-by: Jacob Tanenbaum <jtane...@redhat.com>
>
> ----
>
> v4: added ack by Lorenzo
>
> diff --git a/northd/en-lflow.c b/northd/en-lflow.c
> index 95c88ecea..e80fd9f9c 100644
> --- a/northd/en-lflow.c
> +++ b/northd/en-lflow.c
> @@ -146,16 +146,19 @@ lflow_northd_handler(struct engine_node *node,
> return EN_UNHANDLED;
> }
>
> - if (northd_has_lrouters_in_tracked_data(&northd_data->trk_data)) {
> - return EN_UNHANDLED;
> - }
> -
> const struct engine_context *eng_ctx = engine_get_context();
> struct lflow_data *lflow_data = data;
>
> struct lflow_input lflow_input;
> lflow_get_input_data(node, &lflow_input);
>
> + if (!lflow_handle_northd_lr_changes(eng_ctx->ovnsb_idl_txn,
> + &northd_data->trk_data.trk_routers,
> + &lflow_input,
> + lflow_data->lflow_table)) {
> + return EN_UNHANDLED;
> + }
> +
> if (!lflow_handle_northd_port_changes(eng_ctx->ovnsb_idl_txn,
> &northd_data->trk_data.trk_lsps,
> &lflow_input,
> diff --git a/northd/northd.c b/northd/northd.c
> index eaecb424b..e374db20f 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -540,6 +540,7 @@ ovn_datapath_create(struct hmap *datapaths, const struct
> uuid *key,
> od->ipam_info_initialized = false;
> od->tunnel_key = sdp->sb_dp->tunnel_key;
> init_mcast_info_for_datapath(od);
> + od->router_lflows = lflow_ref_create();
can you please rename it in something we can use even for logical_switches?
Regards,
Lorenzo
> return od;
> }
>
> @@ -568,6 +569,7 @@ ovn_datapath_destroy(struct ovn_datapath *od)
> destroy_mcast_info_for_datapath(od);
> destroy_ports_for_datapath(od);
> sset_destroy(&od->router_ips);
> + lflow_ref_destroy(od->router_lflows);
> free(od);
> }
> }
> @@ -13965,12 +13967,12 @@ build_default_route_flows_for_lrouter(
> struct simap *route_tables)
> {
> ovn_lflow_add_default_drop(lflows, od, S_ROUTER_IN_IP_ROUTING_ECMP,
> - NULL);
> + od->router_lflows);
> ovn_lflow_add_default_drop(lflows, od, S_ROUTER_IN_IP_ROUTING,
> - NULL);
> + od->router_lflows);
> ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_ROUTING_ECMP, 150,
> REG_ECMP_GROUP_ID" == 0", "next;",
> - NULL);
> + od->router_lflows);
>
> for (int i = 0; i < od->nbr->n_ports; i++) {
> build_route_table_lflow(od, lflows, od->nbr->ports[i],
> @@ -17779,40 +17781,50 @@ build_lswitch_and_lrouter_iterate_by_lr(struct
> ovn_datapath *od,
> struct lswitch_flow_build_info *lsi)
> {
> ovs_assert(od->nbr);
> - build_adm_ctrl_flows_for_lrouter(od, lsi->lflows, NULL);
> + build_adm_ctrl_flows_for_lrouter(od, lsi->lflows, od->router_lflows);
> build_neigh_learning_flows_for_lrouter(od, lsi->lflows, &lsi->match,
> &lsi->actions,
> - lsi->meter_groups, NULL);
> - build_ND_RA_flows_for_lrouter(od, lsi->lflows, NULL);
> - build_ip_routing_pre_flows_for_lrouter(od, lsi->lflows, NULL);
> + lsi->meter_groups,
> + od->router_lflows);
> + build_ND_RA_flows_for_lrouter(od, lsi->lflows, od->router_lflows);
> + build_ip_routing_pre_flows_for_lrouter(od, lsi->lflows,
> + od->router_lflows);
> build_route_flows_for_lrouter(od, lsi->lflows,
> lsi->route_data, lsi->route_tables,
> lsi->bfd_ports);
> - build_mcast_lookup_flows_for_lrouter(od, lsi->lflows, &lsi->match, NULL);
> + build_mcast_lookup_flows_for_lrouter(od, lsi->lflows, &lsi->match,
> + od->router_lflows);
> build_ingress_policy_flows_for_lrouter(od, lsi->lflows, lsi->lr_ports,
> - lsi->route_policies, NULL);
> - build_arp_resolve_flows_for_lrouter(od, lsi->lflows, NULL);
> + lsi->route_policies,
> + od->router_lflows);
> + build_arp_resolve_flows_for_lrouter(od, lsi->lflows, od->router_lflows);
> build_check_pkt_len_flows_for_lrouter(od, lsi->lflows, lsi->lr_ports,
> &lsi->match, &lsi->actions,
> - lsi->meter_groups, NULL,
> + lsi->meter_groups,
> + od->router_lflows,
> lsi->features);
> build_gateway_redirect_flows_for_lrouter(od, lsi->lflows, &lsi->match,
> - &lsi->actions, NULL);
> + &lsi->actions,
> + od->router_lflows);
> build_arp_request_flows_for_lrouter(od, lsi->lflows, &lsi->match,
> &lsi->actions,
> lsi->meter_groups,
> - NULL);
> + od->router_lflows);
> build_lrouter_network_id_flows(od, lsi->lflows, &lsi->match,
> - &lsi->actions, NULL);
> - build_misc_local_traffic_drop_flows_for_lrouter(od, lsi->lflows, NULL);
> + &lsi->actions, od->router_lflows);
> + build_misc_local_traffic_drop_flows_for_lrouter(od, lsi->lflows,
> + od->router_lflows);
>
> - build_lr_nat_defrag_and_lb_default_flows(od, lsi->lflows, NULL);
> - build_lrouter_lb_affinity_default_flows(od, lsi->lflows, NULL);
> + build_lr_nat_defrag_and_lb_default_flows(od, lsi->lflows,
> + od->router_lflows);
> + build_lrouter_lb_affinity_default_flows(od, lsi->lflows,
> + od->router_lflows);
>
> /* Default drop rule in lr_out_delivery stage. See
> * build_egress_delivery_flows_for_lrouter_port() which adds a rule
> * for each router port. */
> - ovn_lflow_add_default_drop(lsi->lflows, od, S_ROUTER_OUT_DELIVERY, NULL);
> + ovn_lflow_add_default_drop(lsi->lflows, od, S_ROUTER_OUT_DELIVERY,
> + od->router_lflows);
> }
>
> /* Helper function to combine all lflow generation which is iterated by
> logical
> @@ -18432,6 +18444,56 @@ lflow_reset_northd_refs(struct lflow_input
> *lflow_input)
> }
> }
>
> +bool
> +lflow_handle_northd_lr_changes(struct ovsdb_idl_txn *ovnsb_txn,
> + struct tracked_dps *tracked_lrs,
> + struct lflow_input *lflow_input,
> + struct lflow_table *lflows)
> +{
> + bool handled = true;
> + struct hmapx_node *hmapx_node;
> + HMAPX_FOR_EACH (hmapx_node, &tracked_lrs->deleted) {
> + struct ovn_datapath *od = hmapx_node->data;
> + lflow_ref_unlink_lflows(od->router_lflows);
> + handled = lflow_ref_sync_lflows(
> + od->router_lflows, lflows, ovnsb_txn, lflow_input->ls_datapaths,
> + lflow_input->lr_datapaths,
> + lflow_input->ovn_internal_version_changed,
> + lflow_input->sbrec_logical_flow_table,
> + lflow_input->sbrec_logical_dp_group_table);
> + if (!handled) {
> + return handled;
> + }
> + }
> +
> + struct lswitch_flow_build_info lsi = {
> + .lr_datapaths = lflow_input->lr_datapaths,
> + .lr_stateful_table = lflow_input->lr_stateful_table,
> + .lflows =lflows,
> + .route_data = lflow_input->route_data,
> + .route_tables = lflow_input->route_tables,
> + .route_policies = lflow_input->route_policies,
> + .match = DS_EMPTY_INITIALIZER,
> + .actions = DS_EMPTY_INITIALIZER,
> + };
> + HMAPX_FOR_EACH (hmapx_node, &tracked_lrs->crupdated) {
> + struct ovn_datapath *od = hmapx_node->data;
> + build_lswitch_and_lrouter_iterate_by_lr(od, &lsi);
> + handled = lflow_ref_sync_lflows(
> + od->router_lflows, lflows, ovnsb_txn, lflow_input->ls_datapaths,
> + lflow_input->lr_datapaths,
> + lflow_input->ovn_internal_version_changed,
> + lflow_input->sbrec_logical_flow_table,
> + lflow_input->sbrec_logical_dp_group_table);
> + if (!handled) {
> + break;
> + }
> + }
> + ds_destroy(&lsi.actions);
> + ds_destroy(&lsi.match);
> + return handled;
> +}
> +
> bool
> lflow_handle_northd_port_changes(struct ovsdb_idl_txn *ovnsb_txn,
> struct tracked_ovn_ports *trk_lsps,
> diff --git a/northd/northd.h b/northd/northd.h
> index 6836831d7..3507d864c 100644
> --- a/northd/northd.h
> +++ b/northd/northd.h
> @@ -461,6 +461,8 @@ struct ovn_datapath {
> /* Map of ovn_port objects belonging to this datapath.
> * This map doesn't include derived ports. */
> struct hmap ports;
> + /*reference to the router lflows beloining to this datapath */
> + struct lflow_ref *router_lflows;
> };
>
> const struct ovn_datapath *ovn_datapath_find(const struct hmap *datapaths,
> @@ -918,7 +920,10 @@ void build_route_data_flows_for_lrouter(
> const struct group_ecmp_datapath *route_node,
> const struct sset *bfd_ports);
>
> -
> +bool lflow_handle_northd_lr_changes(struct ovsdb_idl_txn *ovnsh_txn,
> + struct tracked_dps *,
> + struct lflow_input *,
> + struct lflow_table *lflows);
> bool lflow_handle_northd_port_changes(struct ovsdb_idl_txn *ovnsb_txn,
> struct tracked_ovn_ports *,
> struct lflow_input *,
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 4c8a6aca2..b53942faf 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -12525,18 +12525,17 @@ check_engine_stats lr_nat norecompute compute
> check_engine_stats lr_stateful norecompute compute
> check_engine_stats sync_to_sb_pb recompute nocompute
> check_engine_stats sync_to_sb_lb norecompute compute
> -check_engine_stats lflow recompute nocompute
> +check_engine_stats lflow norecompute compute
> CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
> check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> check ovn-nbctl --wait=sb lr-del lr0
> -
> check_engine_stats northd norecompute compute
> check_engine_stats lr_nat norecompute compute
> check_engine_stats lr_stateful norecompute compute
> check_engine_stats sync_to_sb_pb recompute nocompute
> check_engine_stats sync_to_sb_lb norecompute compute
> -check_engine_stats lflow recompute nocompute
> +check_engine_stats lflow norecompute compute
> CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
> check ovn-nbctl --wait=sb lr-add lr0
> --
> 2.51.0
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
