On 9/1/26 1:42 PM, Ales Musil wrote:
> On Tue, Sep 1, 2026 at 1:41 PM Dumitru Ceara <[email protected]> wrote:
>
>> 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!
>>
>
> Hi Dumitru,
>
> thank you for the review.
>
>
>>
>>>
>>> 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?
>>
>
> All the suggestions are fine by me, I wouldn't backport that yet for sure.
>
Cool, thanks for the confirmation! I found another leftover RFC mention
so I also squashed this in:
diff --git a/northd/northd.c b/northd/northd.c
index 7eaa9f953e..ad6e97bcb4 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -17045,9 +17045,9 @@ build_misc_local_traffic_drop_flows_for_lrouter(
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 routers 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);
Then I applied the patch to the main branch.
Thanks,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
