> Hi Lorenzo, Hi Mark,
> > Thanks for addressing my concerns so quickly. I see that you addressed my > concerns about load balancers with no ports in patch 3. However there are > still some lingering issues: thx for the second review. > > * In patch 1, the commit_lb_aff() action still expects all parsed IP > addresses to have a port. Do you mean lb_aff->backend_port in encode_COMMIT_LB_AFF()? I will fix it in v3. > > * In patch 1, the commit_lb_aff() action always expects a protocol to be > present when parsing. ack, I will fix it in v3. > > * Because of the above 2 issues, the resulting learn() action that we create > when encoding the commit_lb_aff() action will always specify a port number > and protocol. > > * In patch 3, northd always sets the protocol in the commit_lb_aff() action > it creates. It should only specify a protocol if a port is specified on > either the LB VIP or backend. I do not have any strong opinion here but there are multiple places in northd.c where we just use lb->proto without considering the vip port value: - build_empty_lb_event_flow() - ovn_lb_svc_create() - build_lrouter_nat_flows_for_lb() Anyway I will align it to build_lb_rules_pre_stateful() or build_lb_rules() in v3. Regards, Lorenzo > > On 10/13/22 11:51, Lorenzo Bianconi wrote: > > Introduce load-balancer affinity timeout in order to dnat connections > > received > > from the same client to a given load-balancer to the same backend if > > received > > in the affinity timeslot specified by the CMS. > > > > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2127959 > > > > Changes since v1: > > - rebase on top of ovn master > > - fix system-ovn.at > > - fix documentation > > - take into account the case the CMS does not provide vip/backends ports > > > > Lorenzo Bianconi (3): > > actions: introduce commit_lb_aff action > > actions: introduce chk_lb_aff action > > northd: rely on new actions for lb affinity > > > > controller/lflow.h | 1 + > > include/ovn/actions.h | 16 ++ > > include/ovn/logical-fields.h | 3 + > > lib/actions.c | 389 +++++++++++++++++++++++++++++++++++ > > lib/lb.c | 3 + > > lib/lb.h | 1 + > > northd/northd.c | 202 +++++++++++++++--- > > northd/ovn-northd.8.xml | 208 ++++++++++++++++--- > > ovn-nb.xml | 6 + > > ovn-sb.xml | 44 ++++ > > tests/ovn-northd.at | 346 +++++++++++++++++++------------ > > tests/ovn.at | 65 +++--- > > tests/system-ovn.at | 184 ++++++++++++++++- > > utilities/ovn-trace.c | 4 + > > 14 files changed, 1251 insertions(+), 221 deletions(-) > > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
