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? > > thank you very much for the answers!
I'll take a look into the latest version. Thanks Numan > > 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
