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.

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to