[ovs-dev] [PATCH v4] ovn-northd: Add logical flows to support native DHCPv4

[Numan Siddique]

On Tue, Jul 5, 2016 at 12:04 PM, Zong Kai LI <zealokii at gmail.com> wrote:

> +    /* 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.
>
>
​
Since DHCP options are associated with logical ports, I don't see any other
way. Also the IP address to be offered in the DHCP response comes from the
Logical_Switch_Port.addresses right ?

​


> Thanks and have a nice day.
> Zong Kai, LI
>