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? thank you very much for the answers! 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
