[ovs-dev] [PATCH v3 ovn] northd: reduce number of nd_na lb logical flows
Numan Siddique
numans at ovn.org
Tue Sep 14 20:27:16 UTC 2021
On Tue, Sep 14, 2021 at 9:32 AM Mark Michelson <mmichels at redhat.com> wrote:
>
> It looks good to me, thanks Lorenzo.
>
> Acked-by: Mark Michelson <mmichels at redhat.com>
Thanks Lorenzo and Mark for the reviews. I applied this patch to the
main branch.
Numan
>
> On 9/9/21 8:50 AM, Lorenzo Bianconi wrote:
> > As it has been already done for IPv4, collapse IPv6 Neighbour
> > Advertisment flows for load balancer using address list and
> > reduce the number of logical flows from N*M to N where N is
> > the number of logical router port and M is the number of
> > Virtual IPs.
> >
> > https://bugzilla.redhat.com/show_bug.cgi?id=1970258
> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
> > ---
> > Changes since v2:
> > - remove open code and run build_lrouter_nd_flow() instead
> >
> > Changes since v1:
> > - rebase on top of ovn master
> > - add DDlog support
> > ---
> > northd/ovn-northd.c | 25 ++++++++++-------
> > northd/ovn_northd.dl | 66 ++++++++++++++++++++++++++++++++------------
> > tests/ovn-northd.at | 36 ++++++++----------------
> > 3 files changed, 75 insertions(+), 52 deletions(-)
> >
> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > index ee761cef0..fc623bcbe 100644
> > --- a/northd/ovn-northd.c
> > +++ b/northd/ovn-northd.c
> > @@ -9851,8 +9851,7 @@ build_lrouter_nd_flow(struct ovn_datapath *od, struct ovn_port *op,
> > ds_put_format(&actions,
> > "%s { "
> > "eth.src = %s; "
> > - "ip6.src = %s; "
> > - "nd.target = %s; "
> > + "ip6.src = nd.target; "
> > "nd.tll = %s; "
> > "outport = inport; "
> > "flags.loopback = 1; "
> > @@ -9860,8 +9859,6 @@ build_lrouter_nd_flow(struct ovn_datapath *od, struct ovn_port *op,
> > "};",
> > action,
> > eth_addr,
> > - ip_address,
> > - ip_address,
> > eth_addr);
> > ovn_lflow_add_with_hint__(lflows, od, S_ROUTER_IN_IP_INPUT, priority,
> > ds_cstr(&match), ds_cstr(&actions), NULL,
> > @@ -11899,7 +11896,6 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op,
> > &op->nbrp->header_, lflows);
> > }
> >
> > - const char *ip_address;
> > if (sset_count(&op->od->lb_ips_v4)) {
> > ds_clear(match);
> > if (is_l3dgw_port(op)) {
> > @@ -11920,17 +11916,26 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op,
> > ds_destroy(&load_balancer_ips_v4);
> > }
> >
> > - SSET_FOR_EACH (ip_address, &op->od->lb_ips_v6) {
> > + if (sset_count(&op->od->lb_ips_v6)) {
> > ds_clear(match);
> > + ds_clear(actions);
> > +
> > + struct ds load_balancer_ips_v6 = DS_EMPTY_INITIALIZER;
> > +
> > + ds_put_cstr(&load_balancer_ips_v6, "{ ");
> > + ds_put_and_free_cstr(&load_balancer_ips_v6,
> > + sset_join(&op->od->lb_ips_v6, ", ", " }"));
> > +
> > if (is_l3dgw_port(op)) {
> > ds_put_format(match, "is_chassis_resident(%s)",
> > op->cr_port->json_key);
> > }
> > -
> > build_lrouter_nd_flow(op->od, op, "nd_na",
> > - ip_address, NULL, REG_INPORT_ETH_ADDR,
> > - match, false, 90, NULL,
> > - lflows, meter_groups);
> > + ds_cstr(&load_balancer_ips_v6), NULL,
> > + REG_INPORT_ETH_ADDR, match, false, 90,
> > + NULL, lflows, meter_groups);
> > +
> > + ds_destroy(&load_balancer_ips_v6);
> > }
> >
> > if (!op->od->is_gw_router && !op->od->n_l3dgw_ports) {
> > diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
> > index ff92c989c..d91f8111f 100644
> > --- a/northd/ovn_northd.dl
> > +++ b/northd/ovn_northd.dl
> > @@ -5537,6 +5537,44 @@ LogicalRouterNdFlow(router, lrp, "nd_na", ipv6, true, mac, extra_match, drop, pr
> > LogicalRouterArpNdFlow(router, nat at NAT{.external_ip = IPv6{ipv6}}, lrp,
> > mac, extra_match, drop, priority).
> >
> > +relation LogicalRouterNdFlowLB(
> > + lr: Intern<Router>,
> > + lrp: Option<Intern<nb::Logical_Router_Port>>,
> > + ip: string,
> > + mac: string,
> > + extra_match: Option<string>,
> > + stage_hint: bit<32>)
> > +Flow(.logical_datapath = lr._uuid,
> > + .stage = s_ROUTER_IN_IP_INPUT(),
> > + .priority = 90,
> > + .__match = __match.intern(),
> > + .actions = actions,
> > + .stage_hint = stage_hint,
> > + .io_port = None,
> > + .controller_meter = lr.copp.get(cOPP_ND_NA())) :-
> > + LogicalRouterNdFlowLB(.lr = lr, .lrp = lrp, .ip = ip,
> > + .mac = mac, .extra_match = extra_match,
> > + .stage_hint = stage_hint),
> > + var __match = {
> > + var clauses = vec_with_capacity(4);
> > + match (lrp) {
> > + Some{p} -> clauses.push("inport == ${json_string_escape(p.name)}"),
> > + None -> ()
> > + };
> > + clauses.push("nd_ns && nd.target == ${ip}");
> > + clauses.append(extra_match.to_vec());
> > + clauses.join(" && ")
> > + },
> > + var actions =
> > + i"nd_na { "
> > + "eth.src = ${mac}; "
> > + "ip6.src = nd.target; "
> > + "nd.tll = ${mac}; "
> > + "outport = inport; "
> > + "flags.loopback = 1; "
> > + "output; "
> > + "};".
> > +
> > relation LogicalRouterArpFlow(
> > lr: Intern<Router>,
> > lrp: Option<Intern<nb::Logical_Router_Port>>,
> > @@ -5622,8 +5660,7 @@ Flow(.logical_datapath = lr._uuid,
> > } else {
> > (i"${action} { "
> > "eth.src = ${mac}; "
> > - "ip6.src = ${ip}; "
> > - "nd.target = ${ip}; "
> > + "ip6.src = nd.target; "
> > "nd.tll = ${mac}; "
> > "outport = inport; "
> > "flags.loopback = 1; "
> > @@ -5693,7 +5730,7 @@ var residence_check = match (is_redirect) {
> > true -> Some{"is_chassis_resident(${json_string_escape(chassis_redirect_name(lrp.name))})"},
> > false -> None
> > } in {
> > - (var all_ips_v4, _) = get_router_load_balancer_ips(router, false) in {
> > + (var all_ips_v4, var all_ips_v6) = get_router_load_balancer_ips(router, false) in {
> > if (not all_ips_v4.is_empty()) {
> > LogicalRouterArpFlow(.lr = router,
> > .lrp = Some{lrp},
> > @@ -5703,21 +5740,14 @@ var residence_check = match (is_redirect) {
> > .drop = false,
> > .priority = 90,
> > .stage_hint = 0)
> > - }
> > - };
> > - for (RouterLBVIP(.router = &Router{._uuid= lr_uuid}, .vip = vip)) {
> > - Some{(var ip_address, _)} = ip_address_and_port_from_lb_key(vip) in {
> > - IPv6{var ipv6} = ip_address in
> > - LogicalRouterNdFlow(.lr = router,
> > - .lrp = Some{lrp},
> > - .action = "nd_na",
> > - .ip = ipv6,
> > - .sn_ip = false,
> > - .mac = rEG_INPORT_ETH_ADDR(),
> > - .extra_match = residence_check,
> > - .drop = false,
> > - .priority = 90,
> > - .stage_hint = 0)
> > + };
> > + if (not all_ips_v6.is_empty()) {
> > + LogicalRouterNdFlowLB(.lr = router,
> > + .lrp = Some{lrp},
> > + .ip = "{ " ++ all_ips_v6.join(", ") ++ " }",
> > + .mac = rEG_INPORT_ETH_ADDR(),
> > + .extra_match = residence_check,
> > + .stage_hint = 0)
> > }
> > }
> > }
> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> > index 11886b94e..8200eb655 100644
> > --- a/tests/ovn-northd.at
> > +++ b/tests/ovn-northd.at
> > @@ -1707,13 +1707,10 @@ match=(inport == "lrp" && arp.op == 1 && arp.tpa == { 192.168.2.1, 192.168.2.4,
> > action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa <-> arp.spa; outport = inport; flags.loopback = 1; output;)
> > table=3 (lr_in_ip_input ), priority=90 , dnl
> > match=(inport == "lrp" && ip6.dst == {fe80::200:ff:fe00:1, ff02::1:ff00:1} && nd_ns && nd.target == fe80::200:ff:fe00:1), dnl
> > -action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = fe80::200:ff:fe00:1; nd.target = fe80::200:ff:fe00:1; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
> > +action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = nd.target; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
> > table=3 (lr_in_ip_input ), priority=90 , dnl
> > -match=(inport == "lrp" && nd_ns && nd.target == fe80::200:ff:fe00:101:8080), dnl
> > -action=(nd_na { eth.src = xreg0[[0..47]]; ip6.src = fe80::200:ff:fe00:101:8080; nd.target = fe80::200:ff:fe00:101:8080; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
> > - table=3 (lr_in_ip_input ), priority=90 , dnl
> > -match=(inport == "lrp" && nd_ns && nd.target == fe80::200:ff:fe00:102:8080), dnl
> > -action=(nd_na { eth.src = xreg0[[0..47]]; ip6.src = fe80::200:ff:fe00:102:8080; nd.target = fe80::200:ff:fe00:102:8080; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
> > +match=(inport == "lrp" && nd_ns && nd.target == { fe80::200:ff:fe00:101:8080, fe80::200:ff:fe00:102:8080 }), dnl
> > +action=(nd_na { eth.src = xreg0[[0..47]]; ip6.src = nd.target; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
> > table=3 (lr_in_ip_input ), priority=90 , dnl
> > match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.1 && arp.spa == 43.43.43.0/24), dnl
> > action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa <-> arp.spa; outport = inport; flags.loopback = 1; output;)
> > @@ -1722,13 +1719,10 @@ match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == { 192.168.2.1, 192.16
> > action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa <-> arp.spa; outport = inport; flags.loopback = 1; output;)
> > table=3 (lr_in_ip_input ), priority=90 , dnl
> > match=(inport == "lrp-public" && ip6.dst == {fe80::200:ff:fe00:100, ff02::1:ff00:100} && nd_ns && nd.target == fe80::200:ff:fe00:100), dnl
> > -action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = fe80::200:ff:fe00:100; nd.target = fe80::200:ff:fe00:100; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
> > - table=3 (lr_in_ip_input ), priority=90 , dnl
> > -match=(inport == "lrp-public" && nd_ns && nd.target == fe80::200:ff:fe00:101:8080), dnl
> > -action=(nd_na { eth.src = xreg0[[0..47]]; ip6.src = fe80::200:ff:fe00:101:8080; nd.target = fe80::200:ff:fe00:101:8080; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
> > +action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = nd.target; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
> > table=3 (lr_in_ip_input ), priority=90 , dnl
> > -match=(inport == "lrp-public" && nd_ns && nd.target == fe80::200:ff:fe00:102:8080), dnl
> > -action=(nd_na { eth.src = xreg0[[0..47]]; ip6.src = fe80::200:ff:fe00:102:8080; nd.target = fe80::200:ff:fe00:102:8080; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
> > +match=(inport == "lrp-public" && nd_ns && nd.target == { fe80::200:ff:fe00:101:8080, fe80::200:ff:fe00:102:8080 }), dnl
> > +action=(nd_na { eth.src = xreg0[[0..47]]; ip6.src = nd.target; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
> > ])
> >
> > # xreg0[0..47] isn't used anywhere else.
> > @@ -1782,13 +1776,10 @@ match=(inport == "lrp" && arp.op == 1 && arp.tpa == { 192.168.2.1, 192.168.2.4,
> > action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa <-> arp.spa; outport = inport; flags.loopback = 1; output;)
> > table=3 (lr_in_ip_input ), priority=90 , dnl
> > match=(inport == "lrp" && ip6.dst == {fe80::200:ff:fe00:1, ff02::1:ff00:1} && nd_ns && nd.target == fe80::200:ff:fe00:1), dnl
> > -action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = fe80::200:ff:fe00:1; nd.target = fe80::200:ff:fe00:1; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
> > +action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = nd.target; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
> > table=3 (lr_in_ip_input ), priority=90 , dnl
> > -match=(inport == "lrp" && nd_ns && nd.target == fe80::200:ff:fe00:101:8080), dnl
> > -action=(nd_na { eth.src = xreg0[[0..47]]; ip6.src = fe80::200:ff:fe00:101:8080; nd.target = fe80::200:ff:fe00:101:8080; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
> > - table=3 (lr_in_ip_input ), priority=90 , dnl
> > -match=(inport == "lrp" && nd_ns && nd.target == fe80::200:ff:fe00:102:8080), dnl
> > -action=(nd_na { eth.src = xreg0[[0..47]]; ip6.src = fe80::200:ff:fe00:102:8080; nd.target = fe80::200:ff:fe00:102:8080; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
> > +match=(inport == "lrp" && nd_ns && nd.target == { fe80::200:ff:fe00:101:8080, fe80::200:ff:fe00:102:8080 }), dnl
> > +action=(nd_na { eth.src = xreg0[[0..47]]; ip6.src = nd.target; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
> > table=3 (lr_in_ip_input ), priority=90 , dnl
> > match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.1 && arp.spa == 43.43.43.0/24), dnl
> > action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa <-> arp.spa; outport = inport; flags.loopback = 1; output;)
> > @@ -1797,13 +1788,10 @@ match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == { 192.168.2.1, 192.16
> > action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa <-> arp.spa; outport = inport; flags.loopback = 1; output;)
> > table=3 (lr_in_ip_input ), priority=90 , dnl
> > match=(inport == "lrp-public" && ip6.dst == {fe80::200:ff:fe00:100, ff02::1:ff00:100} && nd_ns && nd.target == fe80::200:ff:fe00:100 && is_chassis_resident("cr-lrp-public")), dnl
> > -action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = fe80::200:ff:fe00:100; nd.target = fe80::200:ff:fe00:100; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
> > - table=3 (lr_in_ip_input ), priority=90 , dnl
> > -match=(inport == "lrp-public" && nd_ns && nd.target == fe80::200:ff:fe00:101:8080 && is_chassis_resident("cr-lrp-public")), dnl
> > -action=(nd_na { eth.src = xreg0[[0..47]]; ip6.src = fe80::200:ff:fe00:101:8080; nd.target = fe80::200:ff:fe00:101:8080; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
> > +action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = nd.target; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
> > table=3 (lr_in_ip_input ), priority=90 , dnl
> > -match=(inport == "lrp-public" && nd_ns && nd.target == fe80::200:ff:fe00:102:8080 && is_chassis_resident("cr-lrp-public")), dnl
> > -action=(nd_na { eth.src = xreg0[[0..47]]; ip6.src = fe80::200:ff:fe00:102:8080; nd.target = fe80::200:ff:fe00:102:8080; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
> > +match=(inport == "lrp-public" && nd_ns && nd.target == { fe80::200:ff:fe00:101:8080, fe80::200:ff:fe00:102:8080 } && is_chassis_resident("cr-lrp-public")), dnl
> > +action=(nd_na { eth.src = xreg0[[0..47]]; ip6.src = nd.target; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
> > ])
> >
> > # Priority 91 drop flows (per distributed gw port), if port is not resident.
> >
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
More information about the dev
mailing list