On Thu, Nov 23, 2023 at 4:26 PM Dumitru Ceara <[email protected]> wrote: > > On 11/15/23 07:45, Han Zhou wrote: > > On Thu, Oct 26, 2023 at 11:16 AM <[email protected]> wrote: > >> > >> From: Numan Siddique <[email protected]> > >> > >> This is not required. > >> > > Thanks Numan for the fix. Could you provide a little more detail why this > > is not required: > > - Is it a bug fix? What's the impact? > > - Shall we update the ovn-northd documentation for the related lflow > > changes? > > I agree with Han, the commit log could be improved. I think the > ovn-northd documentation was already behind wrt this flow. There was no > explicit mention of it anywhere. > > I'm pretty sure it's safe to not commit dhcp responses in conntrack, we > bypass conntrack for the requests anyway: > > https://github.com/ovn-org/ovn/blob/5ef2a08ade01d698f84e197987ea679d8978d2b9/northd/northd.c#L7215-L7224 > > > - Is there a reason this is in the I-P patch series? > > Guessing again, I'm still reviewing the rest of the series, but probably > to avoid an unnecessary dependency on logical switch data (if switches > have stateful ACLs applied).
Correct. I've squashed this commit with the next one in v3. so that there is no confusion. And updated the commit message accordingly. Numan > > Regards, > Dumitru > > > > > Thanks, > > Han > > > >> Signed-off-by: Numan Siddique <[email protected]> > >> --- > >> northd/northd.c | 8 ++------ > >> 1 file changed, 2 insertions(+), 6 deletions(-) > >> > >> diff --git a/northd/northd.c b/northd/northd.c > >> index 1877cbc7df..c8a224d3cd 100644 > >> --- a/northd/northd.c > >> +++ b/northd/northd.c > >> @@ -9223,9 +9223,7 @@ build_dhcpv4_options_flows(struct ovn_port *op, > >> &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;"; > >> + 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 " > >> @@ -9308,9 +9306,7 @@ build_dhcpv6_options_flows(struct ovn_port *op, > >> 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;"; > >> + 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 > > " > >> -- > >> 2.41.0 > >> > >> _______________________________________________ > >> 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 > > _______________________________________________ > 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
