On 8/27/26 11:00 AM, Ales Musil via dev wrote: > When an external router sends an ICMP "Redirect to Host" > (IPv4 type 5) or ICMPv6 Redirect (type 137) to an OVN > Logical Router Port IP, the packet is not matched by any > specific handler in lr_in_ip_input. On gateway and > distributed-gateway routers this causes the packet to fall > through to conntrack stages where ct() fails, spamming OVS > logs. On plain routers the priority-80 ICMP-unreachable > catch-all handles them, but that is guarded by > !is_gw_router so gateway routers are left unprotected.
Hi Ales, Thanks for the fix! > > Per RFC 1812 section 5.2.7.2, a router SHOULD silently > discard any received ICMP Redirect. There is no reason for > an OVN logical router to route ICMP Redirect packets. > This is not true, the RFC doesn't say that. It does say a router MAY use ICMP Redirect to change its router decision. So that also means a router MAY choose to ignore the ICMP Redirect packets. There's no explicit mention about not routing ICMP redirect packets but I think you're right, there's no reason for an OVN logical router to route these. The topology is fixed. Moreover, the RFC _does_ say in section 5.2.7.2: The source address used in the ICMP Redirect MUST belong to the same logical (sub)net as the destination address. Which in my opinion is an indirect way to support the decision that OVN routers have no reason to route these. The IPv6 RFC OTOH does mention that redirect packets shouldn't be routed: https://datatracker.ietf.org/doc/html/rfc4861#section-8.1 8.1. Validation of Redirect Messages A host MUST silently discard any received Redirect message that does not satisfy all of the following validity checks: - IP Source Address is a link-local address. Routers must use their link-local address as the source for Router Advertisement and Redirect messages so that hosts can uniquely identify routers. But I think I'd just remove the RFC mention from the commit log. > Add priority-110 per-datapath flows in lr_in_ip_input that > unconditionally drop all ICMP Redirect packets (IPv4 type 5 > and ICMPv6 type 137) before the conntrack stages. This > covers all destination IPs including floating IPs and LB > VIPs, and gives uniform behavior across all router types. > > Reported-at: https://issues.redhat.com/browse/FDP-1936 > Assisted-by: Claude Opus 4.6, OpenCode > Signed-off-by: Ales Musil <[email protected]> > --- > Documentation/ref/ovn-logical-flows.7.rst | 6 ++ > northd/northd.c | 11 +++ > tests/ovn-northd.at | 4 + > tests/system-ovn.at | 110 ++++++++++++++++++++++ > 4 files changed, 131 insertions(+) > > diff --git a/Documentation/ref/ovn-logical-flows.7.rst > b/Documentation/ref/ovn-logical-flows.7.rst > index 1a9168ac8..a5bd6dd4b 100644 > --- a/Documentation/ref/ovn-logical-flows.7.rst > +++ b/Documentation/ref/ovn-logical-flows.7.rst > @@ -2377,6 +2377,12 @@ contains the following flows to implement very basic > IP host functionality. > flags.loopback = 1; > next; > > +- ICMP Redirect drop. Per RFC 1812 section 5.2.7.2, a router SHOULD This RFC mention is not correct, I think I'd rephrase it to: -- ICMP Redirect drop. Per RFC 1812 section 5.2.7.2, a router SHOULD - silently discard any received ICMP Redirect. A priority-110 flow matches - ``icmp4.type == 5`` and another matches ``icmp6.type == 137``, both with - action ``drop``. These flows are installed before the conntrack stages, - so Redirect packets are neither run through ``ct()`` nor forwarded. +- ICMP Redirect drop. OVN logical routers don't have a reason to forward + redirect packets; they also can't really use the redirect information + to change the routing decision so they just silently discard any + received ICMP Redirect. A priority-110 flow matches ``icmp4.type == 5`` + and another matches ``icmp6.type == 137``, both with action ``drop``. + These flows are installed before the conntrack stages, so Redirect + packets are neither run through ``ct()`` nor forwarded. The rest looks good to me. If you're ok with me mangling the commit message and with the small change in the docs above, I can squash that in and apply the patch myself. But, just to be sure, I'd give it some soak time on the main branch and not backport it unless someone requests it explicitly. What do you think? Regards, Dumitru > + silently discard any received ICMP Redirect. A priority-110 flow matches > + ``icmp4.type == 5`` and another matches ``icmp6.type == 137``, both with > + action ``drop``. These flows are installed before the conntrack stages, > + so Redirect packets are neither run through ``ct()`` nor forwarded. > + > - Reply to ARP requests. > > These flows reply to ARP requests for the router's own IP address. The ARP > diff --git a/northd/northd.c b/northd/northd.c > index 88e3ece88..c6665a1c0 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -17023,6 +17023,17 @@ build_misc_local_traffic_drop_flows_for_lrouter( > debug_drop_action(), > lflow_ref); > > + /* Drop ICMP Redirect packets (priority 110). > + * RFC 1812 s5.2.7.2: a router SHOULD silently discard any received > + * ICMP Redirect. Drop before conntrack stages so they don't cause > + * ct failures or get forwarded. */ > + ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 110, > + "icmp4.type == 5", debug_drop_action(), > + lflow_ref); > + ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 110, > + "icmp6.type == 137", debug_drop_action(), > + lflow_ref); > + > /* Drop ARP packets (priority 85). ARP request packets for router's own > * IPs are handled with priority-90 flows. > * Drop IPv6 ND packets (priority 85). ND NA packets for router's own > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 6d191c1a0..e99564035 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -14801,6 +14801,8 @@ AT_CHECK([grep "lr_in_ip_input" lr0flows | > ovn_strip_lflows], [0], [dnl > table=??(lr_in_ip_input ), priority=100 , match=(ip4.src == > {172.168.0.10, 172.168.0.255} && reg9[[0]] == 0), action=(drop;) > table=??(lr_in_ip_input ), priority=100 , match=(ip4.src == > {20.0.0.1, 20.0.0.255} && reg9[[0]] == 0), action=(drop;) > table=??(lr_in_ip_input ), priority=100 , match=(ip4.src_mcast > ||ip4.src == 255.255.255.255 || ip4.src == 127.0.0.0/8 || ip4.dst == > 127.0.0.0/8 || ip4.src == 0.0.0.0/8 || ip4.dst == 0.0.0.0/8), action=(drop;) > + table=??(lr_in_ip_input ), priority=110 , match=(icmp4.type == 5), > action=(drop;) > + table=??(lr_in_ip_input ), priority=110 , match=(icmp6.type == 137), > action=(drop;) > table=??(lr_in_ip_input ), priority=120 , match=(inport == > "lr0-public" && ip4.src == 172.168.0.100), action=(next;) > table=??(lr_in_ip_input ), priority=29 , match=(ip.ttl == {0, 1}), > action=(drop;) > table=??(lr_in_ip_input ), priority=30 , match=(inport == > "lr0-public" && ip4 && ip.ttl == {0, 1} && !ip.later_frag && > is_chassis_resident("cr-lr0-public")), action=(icmp4 {eth.dst = eth.src; > eth.src = 00:00:00:00:ff:02; icmp4.type = 11; /* Time exceeded */ icmp4.code > = 0; /* TTL exceeded in transit */ ip4.dst = ip4.src; ip4.src = 172.168.0.10; > ip.ttl = 254; outport = "lr0-public"; flags.loopback = 1; output; };) > @@ -14983,6 +14985,8 @@ AT_CHECK([grep "lr_in_ip_input" lr0flows | > ovn_strip_lflows], [0], [dnl > table=??(lr_in_ip_input ), priority=100 , match=(ip4.src == > {172.168.0.10, 172.168.0.255} && reg9[[0]] == 0), action=(drop;) > table=??(lr_in_ip_input ), priority=100 , match=(ip4.src == > {20.0.0.1, 20.0.0.255} && reg9[[0]] == 0), action=(drop;) > table=??(lr_in_ip_input ), priority=100 , match=(ip4.src_mcast > ||ip4.src == 255.255.255.255 || ip4.src == 127.0.0.0/8 || ip4.dst == > 127.0.0.0/8 || ip4.src == 0.0.0.0/8 || ip4.dst == 0.0.0.0/8), action=(drop;) > + table=??(lr_in_ip_input ), priority=110 , match=(icmp4.type == 5), > action=(drop;) > + table=??(lr_in_ip_input ), priority=110 , match=(icmp6.type == 137), > action=(drop;) > table=??(lr_in_ip_input ), priority=120 , match=(inport == > "lr0-public" && ip4.src == 172.168.0.100), action=(next;) > table=??(lr_in_ip_input ), priority=29 , match=(ip.ttl == {0, 1}), > action=(drop;) > table=??(lr_in_ip_input ), priority=30 , match=(inport == > "lr0-public" && ip4 && ip.ttl == {0, 1} && !ip.later_frag && > is_chassis_resident("cr-lr0-public")), action=(icmp4 {eth.dst = eth.src; > eth.src = 00:00:00:00:ff:02; icmp4.type = 11; /* Time exceeded */ icmp4.code > = 0; /* TTL exceeded in transit */ ip4.dst = ip4.src; ip4.src = 172.168.0.10; > ip.ttl = 254; outport = "lr0-public"; flags.loopback = 1; output; };) > diff --git a/tests/system-ovn.at b/tests/system-ovn.at > index 973c46728..a359b3c40 100644 > --- a/tests/system-ovn.at > +++ b/tests/system-ovn.at > @@ -22328,6 +22328,116 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port > patch-.*/d > AT_CLEANUP > ]) > > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([ICMP Redirect drop on logical router]) > +CHECK_SCAPY > + > +ovn_start > + > +OVS_TRAFFIC_VSWITCHD_START() > +ADD_BR([br-int]) > + > +check ovs-vsctl \ > + -- set Open_vSwitch . external-ids:system-id=hv1 \ > + -- set Open_vSwitch . > external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \ > + -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \ > + -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \ > + -- set bridge br-int fail-mode=secure > other-config:disable-in-band=true > + > +start_daemon ovn-controller > + > +dnl Topology: client (sw0) -- lr0 -- (sw1) server > +check ovn-nbctl lr-add lr0 > + > +check ovn-nbctl ls-add sw0 > +check ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:00:01 10.0.0.1/24 fd01::1/64 > +check ovn-nbctl lsp-add-router-port sw0 sw0-lr0 lr0-sw0 > + > +check ovn-nbctl ls-add sw1 > +check ovn-nbctl lrp-add lr0 lr0-sw1 00:00:00:00:00:02 20.0.0.1/24 fd02::1/64 > +check ovn-nbctl lsp-add-router-port sw1 sw1-lr0 lr0-sw1 > + > +check ovn-nbctl lsp-add sw0 client \ > + -- lsp-set-addresses client "f0:00:00:00:00:01 10.0.0.10 fd01::10" > +check ovn-nbctl lsp-add sw1 server \ > + -- lsp-set-addresses server "f0:00:00:00:00:02 20.0.0.10 fd02::10" > + > +ADD_NAMESPACES(client, server) > +ADD_VETH(client, client, br-int, "fd01::10/64", "f0:00:00:00:00:01", \ > + "fd01::1", "nodad", "10.0.0.10/24", "10.0.0.1") > +ADD_VETH(server, server, br-int, "fd02::10/64", "f0:00:00:00:00:02", \ > + "fd02::1", "nodad", "20.0.0.10/24", "20.0.0.1") > + > +dnl Capture any ICMP/ICMPv6 traffic arriving at the server. > +NETNS_START_TCPDUMP([server], [-nnne -i server icmp or icmp6], [server]) > + > +OVN_POPULATE_ARP > +wait_for_ports_up > +check ovn-nbctl --wait=hv sync > + > +dnl Verify the router is functional: ping the router port from the client. > +NS_CHECK_EXEC([client], [ping -q -c 1 -w 3 10.0.0.1], [0], [ignore]) > + > +dnl Send an ICMP Redirect (type 5, code 1 = redirect for host) from the > +dnl client toward the router's own IP. The router must silently drop it. > +ip netns exec client scapy -H <<-EOF > +p = Ether(dst='00:00:00:00:00:01', src='f0:00:00:00:00:01') / \ > + IP(src='10.0.0.10', dst='10.0.0.1', ttl=64) / \ > + ICMP(type=5, code=1, gw='10.0.0.254') / \ > + IP(src='10.0.0.1', dst='20.0.0.10', ttl=64) / \ > + Raw(load=b'\x00' * 8) > +sendp(p, iface='client', loop=0, verbose=0, count=3) > +EOF > + > +dnl Send another ICMP Redirect with a transit destination (20.0.0.10). > +dnl Even though the destination is not a router IP, the router must drop it. > +ip netns exec client scapy -H <<-EOF > +p = Ether(dst='00:00:00:00:00:01', src='f0:00:00:00:00:01') / \ > + IP(src='10.0.0.10', dst='20.0.0.10', ttl=64) / \ > + ICMP(type=5, code=1, gw='10.0.0.254') / \ > + IP(src='20.0.0.10', dst='10.0.0.10', ttl=64) / \ > + Raw(load=b'\x00' * 8) > +sendp(p, iface='client', loop=0, verbose=0, count=3) > +EOF > + > +dnl Send an ICMPv6 Redirect (type 137) toward the router's own IPv6. > +ip netns exec client scapy -H <<-EOF > +p = Ether(dst='00:00:00:00:00:01', src='f0:00:00:00:00:01') / \ > + IPv6(src='fd01::10', dst='fd01::1', hlim=64) / \ > + ICMPv6ND_Redirect(tgt='fd01::fe', dst='fd02::10') > +sendp(p, iface='client', loop=0, verbose=0, count=3) > +EOF > + > +dnl Send an ICMPv6 Redirect with a transit destination (fd02::10). > +ip netns exec client scapy -H <<-EOF > +p = Ether(dst='00:00:00:00:00:01', src='f0:00:00:00:00:01') / \ > + IPv6(src='fd01::10', dst='fd02::10', hlim=64) / \ > + ICMPv6ND_Redirect(tgt='fd01::fe', dst='fd01::10') > +sendp(p, iface='client', loop=0, verbose=0, count=3) > +EOF > + > +dnl Send regular pings (IPv4 + IPv6) from client to server as barriers. > +NS_CHECK_EXEC([client], [ping -q -c 1 -w 3 20.0.0.10], [0], [ignore]) > +NS_CHECK_EXEC([client], [ping -6 -q -c 1 -w 3 fd02::10], [0], [ignore]) > + > +dnl The barrier pings should arrive at the server. > +dnl No ICMP Redirect packets should have been forwarded. > +OVS_WAIT_UNTIL([ > + test "$(grep -c 'echo request' server.tcpdump)" -ge 2 > +]) > +AT_CHECK([grep -c 'redirect' server.tcpdump], [1], [0 > +]) > + > +OVN_CLEANUP_CONTROLLER([hv1]) > +OVN_CLEANUP_NORTHD > + > +as > +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d > +/connection dropped.*/d"]) > + > +AT_CLEANUP > +]) > + > OVN_FOR_EACH_NORTHD([ > AT_SETUP([DHCPv6 - Options heap overread]) > CHECK_SCAPY _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
