Hi Aleksandra, Numan, On 2/18/25 4:42 PM, Numan Siddique wrote: > On Mon, Feb 17, 2025 at 6:36 AM Aleksandra Rukomoinikova > <[email protected]> wrote: >> >> Hi Ales, Numan, Dumitru, Mark, Han, >> >> I saw that the 25.03 branch was added. >> >> >> Is there a possibility with a patch for port mirroring >> >> yet get into version 25.03? >>
I'm really sorry for the lack of clarity on this topic. We're also kind of experimenting with "experimental features" in our release process so it's not a smooth ride. However, given that there was no agreement between maintainers until now, my opinion is that unfortunately this feature can't be added to 25.03. I know there was a lot of good discussion about the direction that should be chosen so that the implementation is cleaner and improved. Maybe it's a good opportunity to do that without rushing during the 25.09 development cycle. >> thank you very much for the answers! > > I'll take a look into the latest version. > > Thanks > Numan > Best regards, Dumitru >> >> On 14 Feb 2025, at 18:28, Aleksandra Rukomoinikova <[email protected]> >> wrote: >> >> Hi Dumitru >> >> yes, I will take care of it. >> In this case, I’m waiting for a response from the maintainers >> >> On 14 Feb 2025, at 17:55, Dumitru Ceara <[email protected]> wrote: >> >> Hi Numan, Mark, Han, Aleksandra, Vladislav, Ales, >> >> Today is the day we're supposed to branch for 25.03 and we still have >> this feature that is a candidate for 25.03. I'll try to summarize its >> status here (sorry for top posting). >> >> Based on the discussion between Numan and Aleksandra below it seems >> there's a rather clear plan about how this feature should be improved in >> the 25.09 cycle. >> >> However, as far as I understand, there's still no conclusion reached >> about accepting this feature in 25.03 as "experimental". I'm OK with >> that (if the conditions below are satisfied) and if I'm not wrong Numan >> also said he'd be OK with it. >> >> The reason for it to be marked "experimental" is that we expect with a >> high probability that the way this functionality is implemented will >> change fundamentally in 25.09 - potentially with user visible modifications. >> >> From my perspective, what I think this feature needs for it to be merged >> in 25.03 (as experimental) is: >> >> 1. A NEWS item: >> - the news item should mention the addition of the new feature and a >> brief description of what the feature does (this is a requirement for >> any new feature, in general). That item should also clearly indicate >> that the feature is experimental. >> >> 2. The ovn-nb.xml man page change for this functionality should contain >> a clear note saying that the new tables (and feature) are experimental. >> >> For (1), (2), please see 'Q: What does it mean when a feature is marked >> "experimental"?' and 'Q: How is a feature marked "experimental"?' in our >> documentation: >> https://github.com/ovn-org/ovn/blob/cd4ad2f561794a1f67427b2aa904f876a76b6e28/Documentation/faq/general.rst#L4 >> >> 3. At least one of the maintainers must sign-off on the patch - in this >> case I guess that would mean that the maintainer reviewed the code (to a >> reasonable extent) and is OK with accepting the experimental feature. >> >> Unfortunately, I'm not going to be able to do more reviews today. I did >> start looking and I pushed the rebased latest (v5) version of the patch >> here: >> >> https://github.com/dceara/ovn/tree/refs/heads/review-pws443995-port-mirroring-v5 >> >> v5: >> https://patchwork.ozlabs.org/project/ovn/patch/[email protected]/ >> >> FWIW, CI seems to be green with the patch applied. >> >> Aleksandra, Vladislav, would you be able of taking care of (1) and (2) >> above, that is, adding a NEWS item about this feature and the man page >> note about it being experimental? >> >> But for (3) we need maintainers to agree on what to do in 25.03. Numan, >> Mark, Han, what do you guys think? >> >> Thank you! >> >> Best regards, >> Dumitru >> >> On 2/14/25 2:50 PM, Aleksandra Rukomoinikova wrote: >> >> Hi Numan, >> >> On 14 Feb 2025, at 07:23, Numan Siddique <[email protected]> wrote: >> 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. >> >> Yes, we pursued this goal. >> >> >> 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. >> >> If I understood your point correctly, then logical flow is now being >> added to the egress pipeline of logical pipes containing the target ports >> in the pre_acl reset to the last table, which ensures this. >> >> >> I'm fine if we want to use a new action or not as a short term approach. >> >> >> I've seen your vision of the mirror action below, and I'll take it to work. >> >> 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 ? >> >> yes, we wanted to make the mirroring as close to the source port >> as possible. >> >> 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 >> >> Regarding the case you described, I tend to believe that the v5 >> patch fixes this situation. when pinging from port s1-p1 to >> s0-p1, I see a request on port s1-p1 and then 2 replays from port s0-p1. >> >> 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. >> >> Regarding your ideas on how it should be designed in 25.09: >> I understand your ideas, I will think in this way for version 25.09. >> >> Thank you for taking the time to test this patch, >> I am extremely grateful. >> >> >> >> >> > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
