On 2/11/25 5:56 PM, Dumitru Ceara wrote:
> Hi Vladislav, Ales, Aleksandra,
> 
> On 2/11/25 8:52 AM, Ales Musil wrote:
>> On Mon, Feb 10, 2025 at 8:56 PM Vladislav Odintsov <[email protected]>
>> wrote:
>>
>>> Hi Ales,
>>>
>>> First of all, thank you for your time spent on digging into this change
>>> request!
>>>
>>
>>
>> Hi Vladislav,
>>
>> thank you for the response.
>>
>>
>>> While Aleksandra has already gave some comments regarding local/remote
>>> mirroring, let me give more description for full understanding of this
>>> feature.
>>>
>>> We've gonna integrate Network Traffic Analysis (NTA) solutions into our
>>> platform. This functionality requires next features to be supported by
>>> network virtualization platform:
>>>
>>> 1. Support mirroring (multiple) LSP(s) traffic to another one LSP
>>> regardless of physical location of source(s) and target (sink).
>>>
>>
>> So is my understanding right that you don't care about further
>> processing on the target (sink) switch, just to deliver to the LSP?
>>
>> 2. Support selection of traffic, which:
>>> - needs to be mirrored by direction (to-lport/from-lport);
>>> - needs to be mirrored by packet's 5-tuple (multiple rules per mirror
>>> should be allowed);
>>> - needs NOT to be mirrored by packet's 5-tuple (multiple rules per
>>> mirror should be allowed).
>>>
>>> All these requirements should be able to be mixed in the same mirror
>>> resource. This allows to make effective mirroring configurations for NTA
>>> - not to mirror elephant and costly for processing traffic (for example,
>>> backups) to NTA sensors.
>>>
>>> For example, we should be able to express next statement: "mirror all
>>> traffic except tcp/1111 and udp/2222 from LSPs lp1, lp2 connected to
>>> Logical Switch ls1, lp3 connected to ls2 to lp4 connected to ls2".
>>>
>>> If we're talking about our current implementation, we create one mirror
>>> object of type "lport" with sink "lp4". Then we attach this mirror to
>>> lp1, lp2, lp3 and add 2 rules to this mirror:
>>>
>>> priority=101, match="ip4 && udp && udp.dport === 2222", action=skip
>>> priority=100, match="ip4 && tcp && tcp.dport == 1111", action=skip
>>>
>>> If user wants more complex rules (mirror ONLY this traffic), rules
>>> should be:
>>>
>>> priority=101, match="ip4 && udp && udp.dport === 2222", action=mirror
>>> priority=100, match="ip4 && tcp && tcp.dport == 1111", action=mirror
>>> priority=1, match="1", action=skip.
>>>
>>
>> It is a bit unfortunate that negative conditions are a bit hard to
>> express, but it should still be doable.
>>
>>
>>>
>>> This is about current implementation.
>>>
>>> Please also see my answers inline.
>>>
>>> On 10.02.2025 19:45, Ales Musil wrote:
>>>> Hello Alexandra and Vladislav,
>>>>
>>>> as discussed during the community meeting I have in mind some ideas
>>>> how to change/simplify to some extent the feature, but I would like
>>>> to hear your opinion if my understanding for the requirements is
>>>> right.
>>>>
>>>> Also important thing, I'm not oposed to this feature being in 25.03
>>>> probably as "tech preview" so we have a chance to polish it for
>>>> the next release, but it is still usable.
>>>>
>>>> Going back to the beginning with requirements, I have 2 main
>>>> questions:
>>>>
>>>> 1) Do you need elaborate filtering for the mirrors, or is it enough
>>>> to have a single filter per mirror?
>>>>
>>>> If it's enough to have a single filter per mirror we can store it as
>>>> part of the mirror table in OVN NB itself, thus not requiring an
>>>> additional table to configure the feature. This could also be applied
>>>> to existing mirrors, because OvS supports filtering at the mirror
>>>> level.
>>>
>>> We do want to have an ability to configure multiple rules for mirroring
>>> (for cases like above, where we want to skip or explicitly mirror only
>>> part of traffic).
>>>
>>> We also thought about adoption of filtering to OVS filters and decided
>>> that if this is needed by someone, it can be implemented with just a
>>> limitation of one filter rule per mirror of type "local/erspan/gre".
>>> "match" can be used to express openflow mirror filter rule OR
>>> alternatively OVN lflow rule can be automatically expanded by
>>> ovn-controller into openflow syntax for consequent passing to OVS
>>> filter. So, the abstraction should work fine here.
>>>
>>
>> The problem with this approach is that it's not compatible, you
>> either have a filter per mirror or filtering in lflows, so for users
>> it might be a bit counterintuitive what to configure where. This RFE
>> actually allows users to attach the filter to "pure" ovs filter,
>> which the code doesn't prevent AFAICT. So the user won't get any
>> warning, but the filter won't work unless it's lport target.
>>
>>
>>>>
>>>>
>>>> 2) Is the main reason for OVN port being that you would like to
>>>> deliver the mirrored traffic locally and remotely without tunnel
>>>> setup etc. that is required for the OvS mirror to work locally?
>>>> Or is there a second reason being that you would like to process
>>>> the mirrored traffic further e.g. by sending it through the switch
>>>> and possibly being routed etc.? According to my understanding the
>>>> latter is not possible with the current approach and it would require
>>>> some changes. That's one of the reasons that makes me think it's
>>>> mainly about sending it to a remote chassis.
>>> We just want to deliver mirrored traffic to normal OVN logical switch
>>> port regardless of its location - on current chassis or remote. OVN-IC
>>> unfortunately is not supported here. Maybe later it could be possible to
>>> "output" traffic to lport of remote AZ.
>>>>
>>>> I'm asking this because I think we could make it work with pure OvS
>>>> mirrors even if we want to send it to a remote chassis. It would
>>>> require some work, that's why it's an IMO candidate for improvement
>>>> of the feature rather than redesign of the current state.
>>> We thought about such setup. It looks like we should send traffic from
>>> underlay (OVS mirror with type erspan) into overlay network. Without
>>> significant OVS changes I don't see an option here. VRFs, multitenancy
>>> should be somehow addressed in this case.
>>>>
>>>> The following hadn't be fully thought through, there might be some
>>>> missing pieces in the design and it makes assumptions about both
>>>> questions above:
>>>>
>>>> The mirror in OvS would cover filtering, we wouldn't have to install
>>>> logical flows in the pipeline for that. We would need the concept of
>>>> mirror sink, either as a separate entity or part of the mirror. You
>>>> could specify the chassis of the sink and connection to some form of
>>>> ovs interface (as we do for regular LSPs). OVN would setup a tunnel
>>>> for the mirrored traffic (separate one actually allows an additional
>>>> layer of security if needed). And simple ovs bridge setup that would
>>>> transfer the traffic between the tunnel and remote monitoring port.
>>>> For local delivery we could just connect the mirror to the port
>>>> directly as it is available out of the box in current OvS.
>>>
>>> OVN has already implemented all the tunnels configuration logic
>>> complexity. Extra-bridges seem to make things more complex at first
>>> glance. Current implementation reuses all this stuff from OVN with the
>>> cost of extra-flows.
>>>
>>
>> There is also extra cost for very careful configuration as the current
>> approach
>> is very fragile and can be broken accidentally by unrelated change.
>>
>>
>>>
>>> So, if my explanation gave more light on things which are happening,
>>> maybe you got new questions or comments? Or maybe you just agree with
>>> those points :)
>>>
>>
>> One question that wasn't answered is if you are ok with this being an
>> experimental feature in 25.03 with potential to polish it in 25.09?
>>
>>
>>>
>>>>
>>>> Please let me know if that would fit your use case or not.
>>>>
>>>> Thank you for your patience.
>>>> Regards,
>>>> Ales
>>>>
>>>> On Mon, Feb 10, 2025 at 4:12 PM Aleksandra Rukomoinikova
>>>> <[email protected]> wrote:
>>>>
>>>>     Hi, Ales!
>>>>
>>>>     We would like to avoid checking for ACL while passing the egress
>>>>     pipeline
>>>>     on logical switch the  where the target port (sink) locates for
>>>>     cloned packet.
>>>>
>>>>>     On 10 Feb 2025, at 17:04, Ales Musil <[email protected]> wrote:
>>>>>
>>>>>     That makes sense, with that you can probably scratch the
>>>>>     suggestion from
>>>>>     last paragraph.
>>>>>
>>>>>     However, it makes this approach very error prone. If OVN changes the
>>>>>     pipeline order, or adds another pipeline after "ls_in_l2_unknown",
>>>>>     we might break the mirroring. I think it would make it way clearer
>>>>>     and more robust if we would add a new action that would actually do
>>>>>     the whole "clone { outport = "...";
>>>>>     resubmit(,OFTABLE_OUTPUT_INIT) }".
>>>>
>>> It is an interesting tip, but I couldn't catch, how this can help not to
>>> be dependent on OVN logical pipeline changes? Could you please elaborate?
>>>
>>
>> Because this point on the pipeline is well defined, it's not part of the
>> lflow pipeline. In the code it can be defined to always correspond
>> to "output" resubmit for ingress instead of jumping to the last lflow
>> table.
>>
> 
> I agree with Ales on this one, it's a better abstraction if we have a
> "mirror-packet" action (or whatever name we agree on).  Which would
> translate to the correct thing (first table in the ingress/egress
> openflow pipeline based on where we're mirroring the packet from).
> 
> Otherwise we risk breaking this functionality if we change the order of
> stages in the logical pipeline (that does happen every now and then
> because it's an OVN-internal implementation detail).
> 
>>
>>
>>>>
>>>>>
>>>>>     I'm still a bit confused by the whole CT skip. So the flow matches
>>>>>     on the outport being the mirror port mp-*. Is it because the clone
>>>>>     will continue within the original switch in the egress so you want
>>>>>     to avoid ACLs?
>>>>
>>> Yes, this definitely solves the problem, that the target network
>>> interface can be a part of a port-group (security group) with configured
>>> stateless or stateful ACLs, so we do not want to process this traffic
>>> against ACLs and don't want to consult conntrack for the connection
>>> state, which is not interested in case of traffic mirroring.
>>>
>>> Again, many thanks for your attention to this change. Let me know if you
>>> need any description for things that are still not clear.
>>>
>>
>> I think the requirements and the steps taken are clear to me now.
>>
> 
> Ales, am I correct to understand that you'd be OK with the current
> implementation of the feature (in broad terms for now) if the feature is
> marked as "experimental" in 25.03.0?
> 
> I guess that means that the Mirror_Rule table and Mirror.mirror_rules
> column could potentially be removed (or maybe just ignored) from the OVN
> NB schema in releases >= 25.09.0 if we find a better mirroring solution
> by then, right?
> 

[I hit "send" too soon]

FWIW, from my perspective accepting the feature as experimental would be
OK.  CC-ing other maintainers too.

> Thanks,
> Dumitru

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

Reply via email to