On Mon, Oct 24, 2022 at 6:47 AM Felix Hüttner via dev
<ovs-dev@openvswitch.org> wrote:
>
> Previously graceful rarps sent from ovn-controller were handled as
> normal packets and flooded to other routers. As the other routers should
> already have that information, we can skip flooding (just like it is done
> for GARPs already) and thereby mitigate ovs refusing to send the packet
> because of too many resubmits.
>
> This change has been tested in combination with the previous one in the
> series and works well in environments which contain an external ipv6
> network with 600 ovn logical routers.
>
> Signed-off-by: Felix Huettner <felix.huettner@mail.schwarz>
Overall the patch looks good to me.
Can you please add the documentation for the flows modified in
northd/ovn-northd.8.xml ?
Also please drop the ovn_northd.dl changes as the northd ddlog code
is out of sync with the c version.
With these 2 changes done, you can add my Acked-by tag in the next version
Acked-by: Numan Siddique <num...@ovn.org>
Numan
> ---
> northd/northd.c | 9 +++++----
> northd/ovn_northd.dl | 2 +-
> tests/ovn-northd.at | 18 +++++++++---------
> tests/ovn.at | 8 +++++++-
> 4 files changed, 22 insertions(+), 15 deletions(-)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index 6771ccce5..63054a775 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -7345,7 +7345,7 @@ build_lrouter_groups(struct hmap *ports, struct
> ovs_list *lr_list)
> }
>
> /*
> - * Ingress table 24: Flows that flood self originated ARP/ND packets in the
> + * Ingress table 24: Flows that flood self originated ARP/RARP/ND packets in
> the
> * switching domain.
> */
> static void
> @@ -7378,7 +7378,7 @@ build_lswitch_rport_arp_req_self_orig_flow(struct
> ovn_port *op,
> sset_add(&all_eth_addrs, nat->external_mac);
> }
>
> - /* Self originated ARP requests/ND need to be flooded to the L2 domain
> + /* Self originated ARP requests/RARP/ND need to be flooded to the L2
> domain
> * (except on router ports). Determine that packets are self originated
> * by also matching on source MAC. Matching on ingress port is not
> * reliable in case this is a VLAN-backed network.
> @@ -7394,7 +7394,8 @@ build_lswitch_rport_arp_req_self_orig_flow(struct
> ovn_port *op,
> ds_chomp(ð_src, ',');
> ds_put_cstr(ð_src, "}");
>
> - ds_put_format(&match, "eth.src == %s && (arp.op == 1 || nd_ns)",
> + ds_put_format(&match,
> + "eth.src == %s && (arp.op == 1 || rarp.op == 3 || nd_ns)",
> ds_cstr(ð_src));
> ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_LKUP, priority, ds_cstr(&match),
> "outport = \""MC_FLOOD_L2"\"; output;");
> @@ -7590,7 +7591,7 @@ build_lswitch_rport_arp_req_flows(struct ovn_port *op,
> lflows, stage_hint);
> }
>
> - /* Self originated ARP requests/ND need to be flooded as usual.
> + /* Self originated ARP requests/RARP/ND need to be flooded as usual.
> *
> * However, if the switch doesn't have any non-router ports we shouldn't
> * even try to flood.
> diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
> index 2fe73959c..bdaa23d04 100644
> --- a/northd/ovn_northd.dl
> +++ b/northd/ovn_northd.dl
> @@ -4636,7 +4636,7 @@ Flow(.logical_datapath = sw._uuid,
> eth_src_set
> },
> var eth_src = "{" ++ eth_src_set.to_vec().join(", ") ++ "}",
> - var __match = i"eth.src == ${eth_src} && (arp.op == 1 || nd_ns)",
> + var __match = i"eth.src == ${eth_src} && (arp.op == 1 || rarp.op == 3 ||
> nd_ns)",
> var mc_flood_l2 = json_escape(mC_FLOOD_L2().0),
> var actions = i"outport = ${mc_flood_l2}; output;".
>
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 7d879b642..c6e269fba 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -4759,7 +4759,7 @@ AT_CHECK([grep "ls_in_l2_lkup" ls1_lflows | sed
> 's/table=../table=??/' | sort],
> table=??(ls_in_l2_lkup ), priority=50 , match=(eth.dst ==
> 00:00:00:00:01:01), action=(outport = "ls1-ro1"; output;)
> table=??(ls_in_l2_lkup ), priority=50 , match=(eth.dst ==
> 00:00:00:00:01:02), action=(outport = "vm1"; output;)
> table=??(ls_in_l2_lkup ), priority=70 , match=(eth.mcast),
> action=(outport = "_MC_flood"; output;)
> - table=??(ls_in_l2_lkup ), priority=75 , match=(eth.src ==
> {00:00:00:00:01:01} && (arp.op == 1 || nd_ns)), action=(outport =
> "_MC_flood_l2"; output;)
> + table=??(ls_in_l2_lkup ), priority=75 , match=(eth.src ==
> {00:00:00:00:01:01} && (arp.op == 1 || rarp.op == 3 || nd_ns)),
> action=(outport = "_MC_flood_l2"; output;)
> table=??(ls_in_l2_lkup ), priority=80 , match=(flags[[1]] == 0 &&
> arp.op == 1 && arp.tpa == 192.168.1.1), action=(clone {outport = "ls1-ro1";
> output; }; outport = "_MC_flood_l2"; output;)
> table=??(ls_in_l2_lkup ), priority=80 , match=(flags[[1]] == 0 &&
> nd_ns && nd.target == fe80::200:ff:fe00:101), action=(clone {outport =
> "ls1-ro1"; output; }; outport = "_MC_flood_l2"; output;)
> ])
> @@ -4771,7 +4771,7 @@ AT_CHECK([grep "ls_in_l2_lkup" ls2_lflows | sed
> 's/table=../table=??/' | sort],
> table=??(ls_in_l2_lkup ), priority=50 , match=(eth.dst ==
> 00:00:00:00:02:01), action=(outport = "ls2-ro2"; output;)
> table=??(ls_in_l2_lkup ), priority=50 , match=(eth.dst ==
> 00:00:00:00:02:02), action=(outport = "vm2"; output;)
> table=??(ls_in_l2_lkup ), priority=70 , match=(eth.mcast),
> action=(outport = "_MC_flood"; output;)
> - table=??(ls_in_l2_lkup ), priority=75 , match=(eth.src ==
> {00:00:00:00:02:01} && (arp.op == 1 || nd_ns)), action=(outport =
> "_MC_flood_l2"; output;)
> + table=??(ls_in_l2_lkup ), priority=75 , match=(eth.src ==
> {00:00:00:00:02:01} && (arp.op == 1 || rarp.op == 3 || nd_ns)),
> action=(outport = "_MC_flood_l2"; output;)
> table=??(ls_in_l2_lkup ), priority=80 , match=(flags[[1]] == 0 &&
> arp.op == 1 && arp.tpa == 192.168.2.1), action=(clone {outport = "ls2-ro2";
> output; }; outport = "_MC_flood_l2"; output;)
> table=??(ls_in_l2_lkup ), priority=80 , match=(flags[[1]] == 0 &&
> nd_ns && nd.target == fe80::200:ff:fe00:201), action=(clone {outport =
> "ls2-ro2"; output; }; outport = "_MC_flood_l2"; output;)
> ])
> @@ -4791,7 +4791,7 @@ AT_CHECK([grep "ls_in_l2_lkup" ls1_lflows | sed
> 's/table=../table=??/' | sort],
> table=??(ls_in_l2_lkup ), priority=50 , match=(eth.dst ==
> 00:00:00:00:01:01), action=(outport = "ls1-ro1"; output;)
> table=??(ls_in_l2_lkup ), priority=50 , match=(eth.dst ==
> 00:00:00:00:01:02), action=(outport = "vm1"; output;)
> table=??(ls_in_l2_lkup ), priority=70 , match=(eth.mcast),
> action=(outport = "_MC_flood"; output;)
> - table=??(ls_in_l2_lkup ), priority=75 , match=(eth.src ==
> {00:00:00:00:01:01} && (arp.op == 1 || nd_ns)), action=(outport =
> "_MC_flood_l2"; output;)
> + table=??(ls_in_l2_lkup ), priority=75 , match=(eth.src ==
> {00:00:00:00:01:01} && (arp.op == 1 || rarp.op == 3 || nd_ns)),
> action=(outport = "_MC_flood_l2"; output;)
> table=??(ls_in_l2_lkup ), priority=80 , match=(flags[[1]] == 0 &&
> arp.op == 1 && arp.tpa == 10.0.0.100), action=(clone {outport = "ls1-ro1";
> output; }; outport = "_MC_flood_l2"; output;)
> table=??(ls_in_l2_lkup ), priority=80 , match=(flags[[1]] == 0 &&
> arp.op == 1 && arp.tpa == 192.168.1.1), action=(clone {outport = "ls1-ro1";
> output; }; outport = "_MC_flood_l2"; output;)
> table=??(ls_in_l2_lkup ), priority=80 , match=(flags[[1]] == 0 &&
> nd_ns && nd.target == fe80::200:ff:fe00:101), action=(clone {outport =
> "ls1-ro1"; output; }; outport = "_MC_flood_l2"; output;)
> @@ -4804,7 +4804,7 @@ AT_CHECK([grep "ls_in_l2_lkup" ls2_lflows | sed
> 's/table=../table=??/' | sort],
> table=??(ls_in_l2_lkup ), priority=50 , match=(eth.dst ==
> 00:00:00:00:02:01), action=(outport = "ls2-ro2"; output;)
> table=??(ls_in_l2_lkup ), priority=50 , match=(eth.dst ==
> 00:00:00:00:02:02), action=(outport = "vm2"; output;)
> table=??(ls_in_l2_lkup ), priority=70 , match=(eth.mcast),
> action=(outport = "_MC_flood"; output;)
> - table=??(ls_in_l2_lkup ), priority=75 , match=(eth.src ==
> {00:00:00:00:02:01} && (arp.op == 1 || nd_ns)), action=(outport =
> "_MC_flood_l2"; output;)
> + table=??(ls_in_l2_lkup ), priority=75 , match=(eth.src ==
> {00:00:00:00:02:01} && (arp.op == 1 || rarp.op == 3 || nd_ns)),
> action=(outport = "_MC_flood_l2"; output;)
> table=??(ls_in_l2_lkup ), priority=80 , match=(flags[[1]] == 0 &&
> arp.op == 1 && arp.tpa == 192.168.2.1), action=(clone {outport = "ls2-ro2";
> output; }; outport = "_MC_flood_l2"; output;)
> table=??(ls_in_l2_lkup ), priority=80 , match=(flags[[1]] == 0 &&
> arp.op == 1 && arp.tpa == 20.0.0.100), action=(clone {outport = "ls2-ro2";
> output; }; outport = "_MC_flood_l2"; output;)
> table=??(ls_in_l2_lkup ), priority=80 , match=(flags[[1]] == 0 &&
> nd_ns && nd.target == fe80::200:ff:fe00:201), action=(clone {outport =
> "ls2-ro2"; output; }; outport = "_MC_flood_l2"; output;)
> @@ -4825,7 +4825,7 @@ AT_CHECK([grep "ls_in_l2_lkup" ls1_lflows | sed
> 's/table=../table=??/' | sort],
> table=??(ls_in_l2_lkup ), priority=50 , match=(eth.dst ==
> 00:00:00:00:01:01), action=(outport = "ls1-ro1"; output;)
> table=??(ls_in_l2_lkup ), priority=50 , match=(eth.dst ==
> 00:00:00:00:01:02), action=(outport = "vm1"; output;)
> table=??(ls_in_l2_lkup ), priority=70 , match=(eth.mcast),
> action=(outport = "_MC_flood"; output;)
> - table=??(ls_in_l2_lkup ), priority=75 , match=(eth.src ==
> {00:00:00:00:01:01} && (arp.op == 1 || nd_ns)), action=(outport =
> "_MC_flood_l2"; output;)
> + table=??(ls_in_l2_lkup ), priority=75 , match=(eth.src ==
> {00:00:00:00:01:01} && (arp.op == 1 || rarp.op == 3 || nd_ns)),
> action=(outport = "_MC_flood_l2"; output;)
> table=??(ls_in_l2_lkup ), priority=80 , match=(flags[[1]] == 0 &&
> arp.op == 1 && arp.tpa == 10.0.0.100), action=(clone {outport = "ls1-ro1";
> output; }; outport = "_MC_flood_l2"; output;)
> table=??(ls_in_l2_lkup ), priority=80 , match=(flags[[1]] == 0 &&
> arp.op == 1 && arp.tpa == 192.168.1.1), action=(clone {outport = "ls1-ro1";
> output; }; outport = "_MC_flood_l2"; output;)
> table=??(ls_in_l2_lkup ), priority=80 , match=(flags[[1]] == 0 &&
> arp.op == 1 && arp.tpa == 30.0.0.100), action=(clone {outport = "ls1-ro1";
> output; }; outport = "_MC_flood_l2"; output;)
> @@ -4839,7 +4839,7 @@ AT_CHECK([grep "ls_in_l2_lkup" ls2_lflows | sed
> 's/table=../table=??/' | sort],
> table=??(ls_in_l2_lkup ), priority=50 , match=(eth.dst ==
> 00:00:00:00:02:01), action=(outport = "ls2-ro2"; output;)
> table=??(ls_in_l2_lkup ), priority=50 , match=(eth.dst ==
> 00:00:00:00:02:02), action=(outport = "vm2"; output;)
> table=??(ls_in_l2_lkup ), priority=70 , match=(eth.mcast),
> action=(outport = "_MC_flood"; output;)
> - table=??(ls_in_l2_lkup ), priority=75 , match=(eth.src ==
> {00:00:00:00:02:01} && (arp.op == 1 || nd_ns)), action=(outport =
> "_MC_flood_l2"; output;)
> + table=??(ls_in_l2_lkup ), priority=75 , match=(eth.src ==
> {00:00:00:00:02:01} && (arp.op == 1 || rarp.op == 3 || nd_ns)),
> action=(outport = "_MC_flood_l2"; output;)
> table=??(ls_in_l2_lkup ), priority=80 , match=(flags[[1]] == 0 &&
> arp.op == 1 && arp.tpa == 192.168.2.1), action=(clone {outport = "ls2-ro2";
> output; }; outport = "_MC_flood_l2"; output;)
> table=??(ls_in_l2_lkup ), priority=80 , match=(flags[[1]] == 0 &&
> arp.op == 1 && arp.tpa == 20.0.0.100), action=(clone {outport = "ls2-ro2";
> output; }; outport = "_MC_flood_l2"; output;)
> table=??(ls_in_l2_lkup ), priority=80 , match=(flags[[1]] == 0 &&
> arp.op == 1 && arp.tpa == 40.0.0.100), action=(clone {outport = "ls2-ro2";
> output; }; outport = "_MC_flood_l2"; output;)
> @@ -4858,7 +4858,7 @@ AT_CHECK([grep "ls_in_l2_lkup" ls1_lflows | sed
> 's/table=../table=??/' | sort],
> table=??(ls_in_l2_lkup ), priority=50 , match=(eth.dst ==
> 00:00:00:00:01:01), action=(outport = "ls1-ro1"; output;)
> table=??(ls_in_l2_lkup ), priority=50 , match=(eth.dst ==
> 00:00:00:00:01:02), action=(outport = "vm1"; output;)
> table=??(ls_in_l2_lkup ), priority=70 , match=(eth.mcast),
> action=(outport = "_MC_flood"; output;)
> - table=??(ls_in_l2_lkup ), priority=75 , match=(eth.src ==
> {00:00:00:00:01:01} && (arp.op == 1 || nd_ns)), action=(outport =
> "_MC_flood_l2"; output;)
> + table=??(ls_in_l2_lkup ), priority=75 , match=(eth.src ==
> {00:00:00:00:01:01} && (arp.op == 1 || rarp.op == 3 || nd_ns)),
> action=(outport = "_MC_flood_l2"; output;)
> table=??(ls_in_l2_lkup ), priority=80 , match=(flags[[1]] == 0 &&
> arp.op == 1 && arp.tpa == 10.0.0.100), action=(clone {outport = "ls1-ro1";
> output; }; outport = "_MC_flood_l2"; output;)
> table=??(ls_in_l2_lkup ), priority=80 , match=(flags[[1]] == 0 &&
> arp.op == 1 && arp.tpa == 192.168.1.1), action=(clone {outport = "ls1-ro1";
> output; }; outport = "_MC_flood_l2"; output;)
> table=??(ls_in_l2_lkup ), priority=80 , match=(flags[[1]] == 0 &&
> arp.op == 1 && arp.tpa == 192.168.1.100), action=(clone {outport = "ls1-ro1";
> output; }; outport = "_MC_flood_l2"; output;)
> @@ -4876,7 +4876,7 @@ AT_CHECK([grep "ls_in_l2_lkup" ls1_lflows | sed
> 's/table=../table=??/' | sort],
> table=??(ls_in_l2_lkup ), priority=50 , match=(eth.dst ==
> 00:00:00:00:01:01), action=(outport = "ls1-ro1"; output;)
> table=??(ls_in_l2_lkup ), priority=50 , match=(eth.dst ==
> 00:00:00:00:01:02), action=(outport = "vm1"; output;)
> table=??(ls_in_l2_lkup ), priority=70 , match=(eth.mcast),
> action=(outport = "_MC_flood"; output;)
> - table=??(ls_in_l2_lkup ), priority=75 , match=(eth.src ==
> {00:00:00:00:01:01} && (arp.op == 1 || nd_ns)), action=(outport =
> "_MC_flood_l2"; output;)
> + table=??(ls_in_l2_lkup ), priority=75 , match=(eth.src ==
> {00:00:00:00:01:01} && (arp.op == 1 || rarp.op == 3 || nd_ns)),
> action=(outport = "_MC_flood_l2"; output;)
> table=??(ls_in_l2_lkup ), priority=80 , match=(flags[[1]] == 0 &&
> arp.op == 1 && arp.tpa == 10.0.0.100), action=(clone {outport = "ls1-ro1";
> output; }; outport = "_MC_flood_l2"; output;)
> table=??(ls_in_l2_lkup ), priority=80 , match=(flags[[1]] == 0 &&
> arp.op == 1 && arp.tpa == 192.168.1.1), action=(clone {outport = "ls1-ro1";
> output; }; outport = "_MC_flood_l2"; output;)
> table=??(ls_in_l2_lkup ), priority=80 , match=(flags[[1]] == 0 &&
> arp.op == 1 && arp.tpa == 192.168.1.100), action=(clone {outport = "ls1-ro1";
> output; }; outport = "_MC_flood_l2"; output;)
> @@ -4900,7 +4900,7 @@ AT_CHECK([grep "ls_in_l2_lkup" ls1_lflows | sed
> 's/table=../table=??/' | sort],
> table=??(ls_in_l2_lkup ), priority=50 , match=(eth.dst ==
> 00:00:00:00:01:01), action=(outport = "ls1-ro1"; output;)
> table=??(ls_in_l2_lkup ), priority=50 , match=(eth.dst ==
> 00:00:00:00:01:02), action=(outport = "vm1"; output;)
> table=??(ls_in_l2_lkup ), priority=70 , match=(eth.mcast),
> action=(outport = "_MC_flood"; output;)
> - table=??(ls_in_l2_lkup ), priority=75 , match=(eth.src ==
> {00:00:00:00:01:01} && (arp.op == 1 || nd_ns)), action=(outport =
> "_MC_flood_l2"; output;)
> + table=??(ls_in_l2_lkup ), priority=75 , match=(eth.src ==
> {00:00:00:00:01:01} && (arp.op == 1 || rarp.op == 3 || nd_ns)),
> action=(outport = "_MC_flood_l2"; output;)
> table=??(ls_in_l2_lkup ), priority=80 , match=(flags[[1]] == 0 &&
> arp.op == 1 && arp.tpa == 10.0.0.100), action=(clone {outport = "ls1-ro1";
> output; }; outport = "_MC_flood_l2"; output;)
> table=??(ls_in_l2_lkup ), priority=80 , match=(flags[[1]] == 0 &&
> arp.op == 1 && arp.tpa == 192.168.1.1), action=(clone {outport = "ls1-ro1";
> output; }; outport = "_MC_flood_l2"; output;)
> table=??(ls_in_l2_lkup ), priority=80 , match=(flags[[1]] == 0 &&
> arp.op == 1 && arp.tpa == 192.168.1.100), action=(clone {outport = "ls1-ro1";
> output; }; outport = "_MC_flood_l2"; output;)
> diff --git a/tests/ovn.at b/tests/ovn.at
> index f8b8db4df..184fc0fdd 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -24176,7 +24176,13 @@ nd_target=fe80::200:1ff:fe00:0
> # - 00:00:00:01:00:00 - dnat_and_snat external MAC.
> # - 00:00:00:02:00:00 - dnat_and_snat external MAC.
> as hv1
> -AT_CHECK([ovs-ofctl dump-flows br-int | grep -E
> "priority=75,.*${match_sw_metadata}.*arp_op=1" | grep -oE
> "dl_src=[[0-9a-f:]]+" | sort], [0], [dnl
> +AT_CHECK([ovs-ofctl dump-flows br-int | grep -E
> "priority=75,arp.*${match_sw_metadata}.*arp_op=1" | grep -oE
> "dl_src=[[0-9a-f:]]+" | sort], [0], [dnl
> +dl_src=00:00:00:00:01:00
> +dl_src=00:00:00:00:02:00
> +dl_src=00:00:00:01:00:00
> +dl_src=00:00:00:02:00:00
> +])
> +AT_CHECK([ovs-ofctl dump-flows br-int | grep -E
> "priority=75,rarp.*${match_sw_metadata}.*arp_op=3" | grep -oE
> "dl_src=[[0-9a-f:]]+" | sort], [0], [dnl
> dl_src=00:00:00:00:01:00
> dl_src=00:00:00:00:02:00
> dl_src=00:00:00:01:00:00
> --
> 2.38.0
> Diese E Mail enthält möglicherweise vertrauliche Inhalte und ist nur für die
> Verwertung durch den vorgesehenen Empfänger bestimmt. Sollten Sie nicht der
> vorgesehene Empfänger sein, setzen Sie den Absender bitte unverzüglich in
> Kenntnis und löschen diese E Mail. Hinweise zum Datenschutz finden Sie
> hier<https://www.datenschutz.schwarz>.
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
