On 8/9/24 12:13, Ilya Maximets wrote: > On 8/9/24 04:56, Numan Siddique wrote: >> On Thu, Aug 8, 2024 at 5:26 PM Numan Siddique <[email protected]> wrote: >>> >>> On Thu, Aug 8, 2024 at 5:22 PM Numan Siddique <[email protected]> wrote: >>>> >>>> On Thu, Aug 8, 2024 at 3:17 PM Dumitru Ceara <[email protected]> wrote: >>>>> >>>>> On 8/8/24 20:00, Ilya Maximets wrote: >>>>>> MLD flows are added to pipelines unconditionally in order to avoid >>>>>> sending such traffic through conntrack. The problem is that these >>>>>> matches turn into matches on ip6.dst that end up as exact matches in >>>>>> datapath flows. This means a separate datapath flow per destination >>>>>> IP address. This may cause significant performance issues in setups >>>>>> where traffic for many different IP addresses is passing through. >>>>>> Since network protocol is stored further in the packet, it is evaluated >>>>>> after checking the IP addresses, and so having a match on ip.proto >>>>>> doesn't save us in this scenario. >>>>>> >>>>>> MLD packets are all supposed to be multicast packets and so they all >>>>>> should have multicast destination ethernet addresses. Add the missing >>>>>> eth.mcast6 match to all such packets. This ensures that non-multicast >>>>>> traffic will quickly fail the OpenFlow lookup on such rules and the bits >>>>>> from higher layers will not be added to the match criteria in datapath >>>>>> flows. This also ensures that OVN doesn't handle incorrect MLD packets. >>>>>> >>>>>> There are still ND responder flows that can add extra matches for IPv6 >>>>>> addresses, but they can be disabled or handled by other means. >>>>>> >>>>>> IGMP did not check for IP address being multicast for some reason, so >>>>>> it didn't cause issues for IPv4 traffic. But let's fix it as well. >>>>>> >>>>>> Tests were using incorrect multicast addresses, fixed now. >>>>>> >>>>>> Fixes: 677a3ba4d66b ("ovn: Add MLD support.") >>>>>> Signed-off-by: Ilya Maximets <[email protected]> >>>>>> --- >>>>> >>>>> Thanks for the fix, Ilya! Looks good to me. >>>>> >>>>> Acked-by: Dumitru Ceara <[email protected]> >>>> >>>> Thanks for the fix. I applied this patch to the main. I'll backport >>>> to other branches after running the tests in my github CI. >>> >>> Regarding the matches for the ND responder flows, we already support an >>> option >>> to disable arp/nd responder flows [1]. The user can use this option for >>> now. >>> But I think we should address the datapath flow explosion due to these >>> ND responder flows (if the user doesn't >>> want to disable these flows). >>> >>> [1] - >>> https://github.com/ovn-org/ovn/commit/72fac9b51a5f5690ca37e83ead582aee3f94399e >> >> I forgot to mention one thing. I added Reported-at and Reported-by >> tags before applying the patch. >> >> The backport is not cleanly applying for the branch-23.09. @Ilya >> Maximets Can you please take a look at it >> and submit branch-23.09 only patch ? > > Thanks! I'll take a look at 23.09 version. > > Best regards, Ilya Maximets. >
I fixed the test conflict and posted a backport for 23.09 here: https://patchwork.ozlabs.org/project/ovn/patch/[email protected]/ Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
