[ovs-dev] [PATCH ovn v6 1/8] ovn-northd: reorganize processing of lflows

Numan Siddique numans at ovn.org
Thu Nov 12 10:27:25 UTC 2020


On Fri, Nov 6, 2020 at 9:03 PM <anton.ivanov at cambridgegreys.com> wrote:
>
> From: Anton Ivanov <anton.ivanov at cambridgegreys.com>
>
> 1. Merge lrouter and lswitch processing.
> 2. Move lrouter and lswitch lflow generation which uses the
> same iterator variables into common helpers
> 3. Set up structures to be used as "job descriptors" in
> order to allow parallel processing later.
>
> Signed-off-by: Anton Ivanov <anton.ivanov at cambridgegreys.com>

Hi Anton,

I applied the patches 1 to 6 of this series to the main branch. I
haven't reviewed patches 7 and 8 yet.
I'd also expect others to review before they are applied.

For some reason if patches 7 and 8 are not accepted, I think patches 1
to 6 are still good enough to be considered.
Hence I applied them.

Thanks
Numan

> ---
>  northd/ovn-northd.c | 191 ++++++++++++++++++++++++++------------------
>  1 file changed, 114 insertions(+), 77 deletions(-)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 684c2bd47..f3e34de28 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -8923,24 +8923,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
>      struct ds actions = DS_EMPTY_INITIALIZER;
>
>      struct ovn_datapath *od;
> -    HMAP_FOR_EACH (od, key_node, datapaths) {
> -        build_adm_ctrl_flows_for_lrouter(od, lflows);
> -    }
> -
>      struct ovn_port *op;
> -    HMAP_FOR_EACH (op, key_node, ports) {
> -        build_adm_ctrl_flows_for_lrouter_port(op, lflows, &match, &actions);
> -    }
> -
> -    HMAP_FOR_EACH (od, key_node, datapaths) {
> -        build_neigh_learning_flows_for_lrouter(
> -                od, lflows, &match, &actions);
> -    }
> -
> -    HMAP_FOR_EACH (op, key_node, ports) {
> -        build_neigh_learning_flows_for_lrouter_port(
> -                op, lflows, &match, &actions);
> -    }
>
>      HMAP_FOR_EACH (od, key_node, datapaths) {
>          if (!od->nbr) {
> @@ -9904,63 +9887,6 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
>          sset_destroy(&nat_entries);
>      }
>
> -    HMAP_FOR_EACH (op, key_node, ports) {
> -        build_ND_RA_flows_for_lrouter_port(op, lflows, &match, &actions);
> -    }
> -
> -    /* Logical router ingress table ND_RA_OPTIONS & ND_RA_RESPONSE: RS
> -     * responder, by default goto next. (priority 0). */
> -    HMAP_FOR_EACH (od, key_node, datapaths) {
> -        build_ND_RA_flows_for_lrouter(od, lflows);
> -    }
> -
> -    HMAP_FOR_EACH (op, key_node, ports) {
> -        build_ip_routing_flows_for_lrouter_port(op, lflows);
> -    }
> -
> -    /* Convert the static routes to flows. */
> -    HMAP_FOR_EACH (od, key_node, datapaths) {
> -        build_static_route_flows_for_lrouter(od, lflows, ports);
> -    }
> -
> -    HMAP_FOR_EACH (od, key_node, datapaths) {
> -        build_mcast_lookup_flows_for_lrouter(od, lflows, &match, &actions);
> -    }
> -
> -    HMAP_FOR_EACH (od, key_node, datapaths) {
> -        build_ingress_policy_flows_for_lrouter(od, lflows, ports);
> -    }
> -
> -    /* XXX destination unreachable */
> -
> -    HMAP_FOR_EACH (od, key_node, datapaths) {
> -        build_arp_resolve_flows_for_lrouter(od, lflows);
> -    }
> -
> -    HMAP_FOR_EACH (op, key_node, ports) {
> -        build_arp_resolve_flows_for_lrouter_port(
> -                op, lflows, ports, &match, &actions);
> -    }
> -
> -    HMAP_FOR_EACH (od, key_node, datapaths) {
> -        build_check_pkt_len_flows_for_lrouter(
> -                od, lflows, ports, &match, &actions);
> -    }
> -
> -    HMAP_FOR_EACH (od, key_node, datapaths) {
> -        build_gateway_redirect_flows_for_lrouter(
> -                od, lflows, &match, &actions);
> -    }
> -
> -    HMAP_FOR_EACH (od, key_node, datapaths) {
> -        build_arp_request_flows_for_lrouter(od, lflows, &match, &actions);
> -    }
> -
> -    HMAP_FOR_EACH (op, key_node, ports) {
> -        build_egress_delivery_flows_for_lrouter_port(
> -                op, lflows, &match, &actions);
> -    }
> -
>      ds_destroy(&match);
>      ds_destroy(&actions);
>  }
> @@ -11368,6 +11294,117 @@ build_ipv6_input_flows_for_lrouter_port(
>
>  }
>
> +struct lswitch_flow_build_info {
> +    struct hmap *datapaths;
> +    struct hmap *ports;
> +    struct hmap *port_groups;
> +    struct hmap *lflows;
> +    struct hmap *mcgroups;
> +    struct hmap *igmp_groups;
> +    struct shash *meter_groups;
> +    struct hmap *lbs;
> +    char *svc_check_match;
> +    struct ds match;
> +    struct ds actions;
> +};
> +
> +/* Helper function to combine all lflow generation which is iterated by
> + * datapath.
> + */
> +
> +static void
> +build_lswitch_and_lrouter_iterate_by_od(
> +            struct ovn_datapath *od, struct lswitch_flow_build_info *lsi)
> +{
> +
> +    /* Build Logical Router Flows. */
> +    build_adm_ctrl_flows_for_lrouter(od, lsi->lflows);
> +    build_neigh_learning_flows_for_lrouter(od, lsi->lflows, &lsi->match,
> +                                           &lsi->actions);
> +    build_ND_RA_flows_for_lrouter(od, lsi->lflows);
> +    build_static_route_flows_for_lrouter(od, lsi->lflows, lsi->ports);
> +    build_mcast_lookup_flows_for_lrouter(od, lsi->lflows, &lsi->match,
> +                                         &lsi->actions);
> +    build_ingress_policy_flows_for_lrouter(od, lsi->lflows, lsi->ports);
> +    build_arp_resolve_flows_for_lrouter(od, lsi->lflows);
> +    build_check_pkt_len_flows_for_lrouter(od, lsi->lflows, lsi->ports,
> +                                          &lsi->match, &lsi->actions);
> +    build_gateway_redirect_flows_for_lrouter(od, lsi->lflows, &lsi->match,
> +                                             &lsi->actions);
> +    build_arp_request_flows_for_lrouter(od, lsi->lflows, &lsi->match,
> +                                        &lsi->actions);
> +}
> +
> +/* Helper function to combine all lflow generation which is iterated by port.
> + */
> +
> +static void
> +build_lswitch_and_lrouter_iterate_by_op(
> +                    struct ovn_port *op,
> +                    struct lswitch_flow_build_info *lsi)
> +{
> +    /* Build Logical Router Flows. */
> +
> +    build_adm_ctrl_flows_for_lrouter_port(op, lsi->lflows, &lsi->match,
> +                                          &lsi->actions);
> +    build_neigh_learning_flows_for_lrouter_port(op, lsi->lflows, &lsi->match,
> +                                                &lsi->actions);
> +    build_ip_routing_flows_for_lrouter_port(op, lsi->lflows);
> +    build_ND_RA_flows_for_lrouter_port(op, lsi->lflows, &lsi->match,
> +                                       &lsi->actions);
> +    build_arp_resolve_flows_for_lrouter_port(op, lsi->lflows, lsi->ports,
> +                                             &lsi->match, &lsi->actions);
> +    build_egress_delivery_flows_for_lrouter_port(op, lsi->lflows, &lsi->match,
> +                                                 &lsi->actions);
> +}
> +
> +static void
> +build_lswitch_and_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
> +                    struct hmap *port_groups, struct hmap *lflows,
> +                    struct hmap *mcgroups, struct hmap *igmp_groups,
> +                    struct shash *meter_groups,
> +                    struct hmap *lbs)
> +{
> +
> +    struct ovn_datapath *od;
> +    struct ovn_port *op;
> +
> +    char *svc_check_match = xasprintf("eth.dst == %s", svc_monitor_mac);
> +
> +    struct lswitch_flow_build_info lsi = {
> +        .datapaths = datapaths,
> +        .ports = ports,
> +        .port_groups = port_groups,
> +        .lflows = lflows,
> +        .mcgroups = mcgroups,
> +        .igmp_groups = igmp_groups,
> +        .meter_groups = meter_groups,
> +        .lbs = lbs,
> +        .svc_check_match = svc_check_match,
> +        .match = DS_EMPTY_INITIALIZER,
> +        .actions = DS_EMPTY_INITIALIZER,
> +    };
> +
> +    /* Combined build - all lflow generation from lswitch and lrouter
> +     * will move here and will be reogranized by iterator type.
> +     */
> +    HMAP_FOR_EACH (od, key_node, datapaths) {
> +        build_lswitch_and_lrouter_iterate_by_od(od, &lsi);
> +    }
> +    HMAP_FOR_EACH (op, key_node, ports) {
> +        build_lswitch_and_lrouter_iterate_by_op(op, &lsi);
> +    }
> +    free(svc_check_match);
> +
> +    /* Legacy lswitch build - to be migrated. */
> +    build_lswitch_flows(datapaths, ports, port_groups, lflows, mcgroups,
> +                        igmp_groups, meter_groups, lbs);
> +
> +    /* Legacy lrouter build - to be migrated. */
> +    build_lrouter_flows(datapaths, ports, lflows, meter_groups, lbs);
> +}
> +
> +
>  /* Updates the Logical_Flow and Multicast_Group tables in the OVN_SB database,
>   * constructing their contents based on the OVN_NB database. */
>  static void
> @@ -11379,9 +11416,9 @@ build_lflows(struct northd_context *ctx, struct hmap *datapaths,
>  {
>      struct hmap lflows = HMAP_INITIALIZER(&lflows);
>
> -    build_lswitch_flows(datapaths, ports, port_groups, &lflows, mcgroups,
> -                        igmp_groups, meter_groups, lbs);
> -    build_lrouter_flows(datapaths, ports, &lflows, meter_groups, lbs);
> +    build_lswitch_and_lrouter_flows(datapaths, ports,
> +                                    port_groups, &lflows, mcgroups,
> +                                    igmp_groups, meter_groups, lbs);
>
>      /* Push changes to the Logical_Flow table to database. */
>      const struct sbrec_logical_flow *sbflow, *next_sbflow;
> --
> 2.20.1
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>


More information about the dev mailing list