On Thu, Sep 14, 2023 at 3:43 PM Mark Michelson <[email protected]> wrote:
>
> Thanks for the fix, Numan.
>
> Acked-by: Mark Michelson <[email protected]>
Thanks. I applied this patch to main and branch-23.09.
One test case was failing due to the latest changes. I fixed that and applied.
----------------------------------------------------------------
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 7ebb115dc2..ef44e10c34 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -10069,7 +10069,7 @@ ovn-nbctl dhcp-options-set-options $CIDR_UUID
lease_time=3600 router=192.168.
check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
ovn-nbctl --wait=sb lsp-set-dhcpv4-options lsp0-2 $CIDR_UUID
-check_recompute_counter 0 0
+check_recompute_counter 0 0 0
CHECK_NO_CHANGE_AFTER_RECOMPUTE
@@ -10080,7 +10080,7 @@ options="\"server_id\"=\"00:00:00:10:00:01\"")"
check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
ovn-nbctl --wait=sb lsp-set-dhcpv6-options lsp0-2 ${d1}
-check_recompute_counter 0 0
+check_recompute_counter 0 0 0
CHECK_NO_CHANGE_AFTER_RECOMPUTE
----------------------------------------------------------------
Thanks
Numan
>
> On 9/14/23 09:51, [email protected] wrote:
> > From: Numan Siddique <[email protected]>
> >
> > The flows to allow DHCP response from ovn-controller were missing
> > if a logical VIF port had dhcp v4/v6 options set and were handled
> > incrementally.
> >
> > Fixes: 8bbd67891f68 ("northd: Incremental processing of VIF additions in
> > 'lflow' node.")
> > Signed-off-by: Numan Siddique <[email protected]>
> > ---
> > northd/northd.c | 122 ++++++++++++++++++++++----------------------
> > tests/ovn-northd.at | 25 +++++++++
> > 2 files changed, 85 insertions(+), 62 deletions(-)
> >
> > diff --git a/northd/northd.c b/northd/northd.c
> > index e9cb906e2a..8b0975b91c 100644
> > --- a/northd/northd.c
> > +++ b/northd/northd.c
> > @@ -7756,68 +7756,6 @@ build_acls(struct ovn_datapath *od, const struct
> > chassis_features *features,
> > }
> > }
> >
> > - /* Add 34000 priority flow to allow DHCP reply from ovn-controller to
> > all
> > - * logical ports of the datapath if the CMS has configured DHCPv4
> > options.
> > - * */
> > - for (size_t i = 0; i < od->nbs->n_ports; i++) {
> > - if (lsp_is_external(od->nbs->ports[i])) {
> > - continue;
> > - }
> > -
> > - if (od->nbs->ports[i]->dhcpv4_options) {
> > - const char *server_id = smap_get(
> > - &od->nbs->ports[i]->dhcpv4_options->options, "server_id");
> > - const char *server_mac = smap_get(
> > - &od->nbs->ports[i]->dhcpv4_options->options, "server_mac");
> > - const char *lease_time = smap_get(
> > - &od->nbs->ports[i]->dhcpv4_options->options, "lease_time");
> > - if (server_id && server_mac && lease_time) {
> > - const char *dhcp_actions =
> > - has_stateful ? REGBIT_ACL_VERDICT_ALLOW" = 1; "
> > - "ct_commit; next;"
> > - : REGBIT_ACL_VERDICT_ALLOW" = 1; next;";
> > - ds_clear(&match);
> > - ds_put_format(&match, "outport == \"%s\" && eth.src == %s "
> > - "&& ip4.src == %s && udp && udp.src == 67 "
> > - "&& udp.dst == 68", od->nbs->ports[i]->name,
> > - server_mac, server_id);
> > - ovn_lflow_add_with_lport_and_hint(
> > - lflows, od, S_SWITCH_OUT_ACL_EVAL, 34000,
> > ds_cstr(&match),
> > - dhcp_actions, od->nbs->ports[i]->name,
> > - &od->nbs->ports[i]->dhcpv4_options->header_);
> > - }
> > - }
> > -
> > - if (od->nbs->ports[i]->dhcpv6_options) {
> > - const char *server_mac = smap_get(
> > - &od->nbs->ports[i]->dhcpv6_options->options, "server_id");
> > - struct eth_addr ea;
> > - if (server_mac && eth_addr_from_string(server_mac, &ea)) {
> > - /* Get the link local IP of the DHCPv6 server from the
> > - * server MAC. */
> > - struct in6_addr lla;
> > - in6_generate_lla(ea, &lla);
> > -
> > - char server_ip[INET6_ADDRSTRLEN + 1];
> > - ipv6_string_mapped(server_ip, &lla);
> > -
> > - const char *dhcp6_actions =
> > - has_stateful ? REGBIT_ACL_VERDICT_ALLOW" = 1; "
> > - "ct_commit; next;"
> > - : REGBIT_ACL_VERDICT_ALLOW" = 1; next;";
> > - ds_clear(&match);
> > - ds_put_format(&match, "outport == \"%s\" && eth.src == %s "
> > - "&& ip6.src == %s && udp && udp.src == 547 "
> > - "&& udp.dst == 546", od->nbs->ports[i]->name,
> > - server_mac, server_ip);
> > - ovn_lflow_add_with_lport_and_hint(
> > - lflows, od, S_SWITCH_OUT_ACL_EVAL, 34000,
> > ds_cstr(&match),
> > - dhcp6_actions, od->nbs->ports[i]->name,
> > - &od->nbs->ports[i]->dhcpv6_options->header_);
> > - }
> > - }
> > - }
> > -
> > /* Add a 34000 priority flow to advance the DNS reply from
> > ovn-controller,
> > * if the CMS has configured DNS records for the datapath.
> > */
> > @@ -9114,6 +9052,33 @@ build_dhcpv4_options_flows(struct ovn_port *op,
> > ds_destroy(&options_action);
> > ds_destroy(&response_action);
> > ds_destroy(&ipv4_addr_match);
> > +
> > + /* Add 34000 priority flow to allow DHCP reply from
> > ovn-controller
> > + * to the ogical port of the datapath if the CMS has configured
> > + * DHCPv4 options.
> > + * */
> > + if (!is_external) {
> > + const char *server_id = smap_get(
> > + &op->nbsp->dhcpv4_options->options, "server_id");
> > + const char *server_mac = smap_get(
> > + &op->nbsp->dhcpv4_options->options, "server_mac");
> > + const char *lease_time = smap_get(
> > + &op->nbsp->dhcpv4_options->options, "lease_time");
> > + ovs_assert(server_id && server_mac && lease_time);
> > + const char *dhcp_actions =
> > + (op->od->has_stateful_acl || op->od->has_lb_vip)
> > + ? REGBIT_ACL_VERDICT_ALLOW" = 1; ct_commit; next;"
> > + : REGBIT_ACL_VERDICT_ALLOW" = 1; next;";
> > + ds_clear(&match);
> > + ds_put_format(&match, "outport == %s && eth.src == %s "
> > + "&& ip4.src == %s && udp && udp.src == 67 "
> > + "&& udp.dst == 68",op->json_key,
> > + server_mac, server_id);
> > + ovn_lflow_add_with_lport_and_hint(
> > + lflows, op->od, S_SWITCH_OUT_ACL_EVAL, 34000,
> > + ds_cstr(&match),dhcp_actions, op->key,
> > + &op->nbsp->dhcpv4_options->header_);
> > + }
> > break;
> > }
> > }
> > @@ -9166,6 +9131,39 @@ build_dhcpv6_options_flows(struct ovn_port *op,
> > &op->nbsp->dhcpv6_options->header_);
> > ds_destroy(&options_action);
> > ds_destroy(&response_action);
> > +
> > + /* Add 34000 priority flow to allow DHCP reply from
> > ovn-controller
> > + * to the ogical port of the datapath if the CMS has configured
> > + * DHCPv6 options.
> > + * */
> > + if (!is_external) {
> > + const char *server_mac = smap_get(
> > + &op->nbsp->dhcpv6_options->options, "server_id");
> > + struct eth_addr ea;
> > + ovs_assert(server_mac &&
> > + eth_addr_from_string(server_mac, &ea));
> > + /* Get the link local IP of the DHCPv6 server from the
> > + * server MAC. */
> > + struct in6_addr lla;
> > + in6_generate_lla(ea, &lla);
> > +
> > + char server_ip[INET6_ADDRSTRLEN + 1];
> > + ipv6_string_mapped(server_ip, &lla);
> > +
> > + const char *dhcp6_actions =
> > + (op->od->has_stateful_acl || op->od->has_lb_vip)
> > + ? REGBIT_ACL_VERDICT_ALLOW" = 1; ct_commit; next;"
> > + : REGBIT_ACL_VERDICT_ALLOW" = 1; next;";
> > + ds_clear(&match);
> > + ds_put_format(&match, "outport == %s && eth.src == %s "
> > + "&& ip6.src == %s && udp && udp.src == 547 "
> > + "&& udp.dst == 546", op->json_key,
> > + server_mac, server_ip);
> > + ovn_lflow_add_with_lport_and_hint(
> > + lflows, op->od, S_SWITCH_OUT_ACL_EVAL, 34000,
> > + ds_cstr(&match),dhcp6_actions, op->key,
> > + &op->nbsp->dhcpv6_options->header_);
> > + }
> > break;
> > }
> > }
> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> > index 5c9da811fa..689f70ebb9 100644
> > --- a/tests/ovn-northd.at
> > +++ b/tests/ovn-northd.at
> > @@ -10046,6 +10046,31 @@ check_recompute_counter 0 0
> >
> > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> >
> > +# Associate DHCP for lsp0-2
> > +ovn-nbctl dhcp-options-create 192.168.0.0/24
> > +
> > +CIDR_UUID=$(ovn-nbctl --bare --columns=_uuid find dhcp_options
> > cidr="192.168.0.0/24")
> > +ovn-nbctl dhcp-options-set-options $CIDR_UUID lease_time=3600
> > router=192.168.0.1 server_id=192.168.0.1 server_mac=c0:ff:ee:00:00:01
> > hostname="\"foo\""
> > +
> > +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
> > +ovn-nbctl --wait=sb lsp-set-dhcpv4-options lsp0-2 $CIDR_UUID
> > +check_recompute_counter 0 0
> > +
> > +CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > +
> > +# Add IPv6 address and associate DHCPv6 for lsp0-2
> > +check ovn-nbctl lsp-set-addresses lsp0-2 "aa:aa:aa:00:00:01 192.168.0.11
> > aef0::4"
> > +d1="$(ovn-nbctl create DHCP_Options cidr="aef0\:\:/64" \
> > +options="\"server_id\"=\"00:00:00:10:00:01\"")"
> > +
> > +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
> > +ovn-nbctl --wait=sb lsp-set-dhcpv6-options lsp0-2 ${d1}
> > +check_recompute_counter 0 0
> > +
> > +CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > +
> > +check ovn-nbctl --wait=hv ls-del ls0
> > +
> > OVN_CLEANUP([hv1])
> > AT_CLEANUP
> > ])
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
