On Thu, Feb 13, 2025 at 1:06 PM Vladislav Odintsov <[email protected]> wrote:
>
> Hi Dumitru, Ales,
>
> please, see inline.
>
> On 11.02.2025 19:58, Dumitru Ceara wrote:
> > 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).
>
> We've got two options here:
>
> 1. Re-write patch to use new "mirror" action, but this can take some
> additional time. I think a couple of days with testing. But will
> upstream hard-freeze wait us for this time?
> 2. Implement new action in a separate patch.
>
> Which way is preferred for you? I'd personally choose 1'st, so the
> feature to be holistic and not split by multiple patches. But we respect
> project's release schedule and okay with any way you choose.
I looked into the patch and I've few comments. I didn't do a thorough
review, but I tested the patch to understand it.
I'm fine with having this as an experimental feature. I like the idea
of reusing the parent port concept.
I guess with this, you can mirror to any logical port and it's fine
if the logical switch of this mirror port
is not connected to the source port via any router. Which I guess is
your goal too.
I don't think it is necessary for the cloned packet to go through all
the stages of the datapath pipeline once the packet is cloned.
I think it can be directly output to the destination mirror port.
I'm fine if we want to use a new action or not as a short term approach.
In this patch, what happens if the packet from a source port is
dropped in the ACL stage ?
Looks like we end up mirroring this packet. Is this fine ?
I also noticed that if a user creates a chain, like below, the
behavior is weird
and the packets are dropped. I didn't dig further.
ovn-nbctl mirror-add mirror0 both from-lport sw1-p1
ovn-nbctl lsp-attach-mirror sw0-p1 mirror0
ovn-nbctl mirror-add mirror1 both from-lport sw0-p1
ovn-nbctl lsp-attach-mirror sw1-p1 mirror1
In the long term (hopefully in 25.09), below is what I think we
should design this feature as :
Suppose if I create a mirror like this:
ovn-nbctl mirror-add mirror0 both from-lport sw1-p1
ovn-nbctl lsp-attach-mirror sw0-p1 mirror0
then
1 . Consider a mirror as a separate datapath. Hopefully we can make
use of composable services features.
2. Create an internal patch port connecting the logical switch sw0 to
the mirror datapath ( SB port binding)
3. If the packet needs to be mirrored, in the ingress or egress
pipeline we just do something like
table=2 (ls_in_mirror ), priority=100 , match=(inport ==
"sw0-p1"), action=(mirror(outport == sw0-mirror0); next;)
or
table=7 (ls_out_mirror ), priority=100 , match=(outport ==
"sw01-port1"), action=(mirror(outport == sw0-mirror0); ; next;)
In the mirror pipeline, we can add logical flows for the mirror rules
(or mirror ACLs). These mirror rules
can drop the packet if there is no match or output to the mirror output port.
3. We can consider that each mirror has one output port.
4. If we want we can enhance a mirror to have multiple output ports.
Depending on the match, the output port
can be chosen.
Thanks
Numan
>
> >>
> >>>
> >>>>>> 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.
>
> We'll add a NEWS item with a mark of this feature is an experimental,
> also add same note to the documentation.
>
> Out of curiosity, do you have a possible changes in mind, about fields
> naming, and Mirror_Rule table and mirror_rules field removal - how do
> you see they can be implemented if we have to provide mentioned
> functionality with traffic selection (explicit mirroring and explicit
> mirroring skip)? With this possible input we can consider possible
> changes now.
>
> >
> >> Thanks,
> >> Dumitru
>
> --
> Regards,
> Vladislav Odintsov
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
