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 Numan > > > Numan > > > > > Numan > > > > > > > > Regards, > > > Dumitru > > > > > > _______________________________________________ > > > dev mailing list > > > [email protected] > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
