On Thu, Apr 13, 2023 at 12:44 PM Han Zhou <[email protected]> wrote: > > > > On Thu, Apr 13, 2023 at 12:35 PM Dumitru Ceara <[email protected]> wrote: > > > > On 4/13/23 21:21, Han Zhou wrote: > > > On Thu, Apr 13, 2023 at 8:37 AM Dumitru Ceara <[email protected]> wrote: > > >> > > >> On 4/13/23 17:34, Han Zhou wrote: > > >>> On Thu, Apr 13, 2023 at 12:54 AM Dumitru Ceara <[email protected]> > > > wrote: > > >>>> > > >>>> On 4/13/23 07:07, Han Zhou wrote: > > >>>>> In RFC1812 section 5.3.1, it is mentioned that: > > >>>>> > > >>>>> If the TTL is reduced to zero (or less), the packet MUST be > > >>>>> discarded, and if the destination is not a multicast address the > > >>>>> router MUST send an ICMP Time Exceeded message ... > > >>>>> > > >>>> > > >>>> The code itself looks OK but I wonder a bit about the rationale. Do > > > you > > >>>> have an example in which OVN replies with Time Exceeded for multicast > > >>>> destinations and that causes issues? > > >>>> > > >>>>> So if the destionation is a multicast address the route shouldn't send > > >>>>> ICMP Time Exceeded, but the current OVN implementation didn't check > > >>>>> multicast and tries to send ICMP regardless. This patch fixes it. > > >>>> > > >>>> The statement "if destination is not a multicast address the router > > > MUST > > >>>> send an ICMP Time Exceeded message" implies that "if destination is a > > >>>> multicast address the router MAY or MAY NOT send an ICMP Time Exceeded > > >>>> message". So the fact that OVN sends one is not necessarily wrong. > > >>> > > >>> For my limited understanding of multicast, sending ICMP time exceeded is > > >>> not a good idea. In multicast TTL has special meanings, for example: > > >>> TTL 0: Restricted to the same host, not transmitted by the router. > > >>> TTL 1: Restricted to the same subnet, not forwarded by the router. > > >>> If we send ICMP for such packets, it means for a very common use case of > > >>> multicast (ttl = 1, same subnet), we will end up sending ICMP for every > > >>> normal packet. > > >>> In production we saw this happening with a rate higher than 10k pps! > > >>> > > >> > > >> Makes sense, thanks for the details! > > >> > > >>> So I believe this is the reason behind the "if" statement in the RFC. > > > Maybe > > >>> I should add this rationale in the comment, too. > > >>> > > >> > > >> If you could add some of the details above to the commit log too then: > > >> > > >> Acked-by: Dumitru Ceara <[email protected]> > > > > > > Thanks Dumitru. I applied the series to main. > > > Han > > > > This sounds like a relevant bug fix. Should this get backported to > > stable branches too? > > > Sound good. I will do that.
I backported this series to the last release branch-23.03 and the LTS branch-22.03. The code base has changed a lot so was mostly manually backported instead of cherry-pick. Thanks, Han _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
