>
> +    /* Logical switch ingress table 10 and 11: DHCP options and response
> +         * priority 100 flows. */
> +    HMAP_FOR_EACH (op, key_node, ports) {
> +        if (!op->nbs) {
> +           continue;
> +        }
> +
> +        if (!lsp_is_enabled(op->nbs) || !strcmp(op->nbs->type, "router"))
> {
> +            /* Don't add the DHCP flows if the port is not enabled or if
> the
> +             * port is a router port. */
> +            continue;
> +        }
> +
> +        if (!op->nbs->dhcpv4_options) {
> +            /* CMS has disabled native DHCPv4 for this lport. */
> +            continue;
> +        }
> +
> +        for (size_t i = 0; i < op->nbs->n_addresses; i++) {
> +            struct lport_addresses laddrs;
> +            if (!extract_lsp_addresses(op->nbs->addresses[i], &laddrs,
> +                                       false)) {
> +                continue;
> +            }
> +
> +            for (size_t j = 0; j < laddrs.n_ipv4_addrs; j++) {
> +                struct ds options_action = DS_EMPTY_INITIALIZER;
> +                struct ds response_action = DS_EMPTY_INITIALIZER;
> +                if (build_dhcpv4_action(op, laddrs.ipv4_addrs[j].addr,
> +                                        &options_action,
> &response_action)) {
> +                    struct ds match = DS_EMPTY_INITIALIZER;
> +                    ds_put_format(
> +                        &match, "inport == %s && eth.src == "ETH_ADDR_FMT
> +                        " && ip4.src == 0.0.0.0 && "
> +                        "ip4.dst == 255.255.255.255 && udp.src == 68 && "
> +                        "udp.dst == 67", op->json_key,
> +                        ETH_ADDR_ARGS(laddrs.ea));
> +
> +                    ovn_lflow_add(lflows, op->od,
> S_SWITCH_IN_DHCP_OPTIONS,
> +                                  100, ds_cstr(&match),
> +                                  ds_cstr(&options_action));
> +                    /* If REGBIT_DHCP_OPTS_RESULT is set, it means the
> +                     * put_dhcp_opts action  is successful */
> +                    ds_put_cstr(&match, " && "REGBIT_DHCP_OPTS_RESULT);
> +                    ovn_lflow_add(lflows, op->od,
> S_SWITCH_IN_DHCP_RESPONSE,
> +                                  100, ds_cstr(&match),
> +                                  ds_cstr(&response_action));
> +                    ds_destroy(&match);
> +                    ds_destroy(&options_action);
> +                    ds_destroy(&response_action);
> +                    break;
> +                }
> +            }
> +            free(laddrs.ipv4_addrs);
> +        }
> +    }
> +
> +    /* Ingress table 10 and 11: DHCP options and response, by default
> goto next.
> +     * (priority 0). */
> +
> +    HMAP_FOR_EACH (od, key_node, datapaths) {
> +        if (!od->nbs) {
> +            continue;
> +        }
> +
> +        ovn_lflow_add(lflows, od, S_SWITCH_IN_DHCP_OPTIONS, 0, "1",
> "next;");
> +        ovn_lflow_add(lflows, od, S_SWITCH_IN_DHCP_RESPONSE, 0, "1",
> "next;");
> +    }
>

Great work, but only one thing makes me feel uncomfortable.
DHCP Options flows are generated per each port, but not per each switch
datapath.
Since the resumed packet copies L2-L4 fields from original packet, and DHCP
options fields are the same for each port in the same switch. It doesn't
make sense to build DHCP Options flows for each ports.
The inport and eth.src in DHCP Options flows, I think we need logical
switch metadata field indeed.

Thanks and have a nice day.
Zong Kai, LI
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to