On 8/6/24 20:44, Mark Michelson wrote: > For patches 1-8: > > Acked-by: Mark Michelson <[email protected]> >
Thanks, Mark, I added your acks to patches 1-8. > I have a single small comment on patch 9. > Ales and I folded in a fix for your comment on patch 9. V7 posted for review: https://patchwork.ozlabs.org/project/ovn/list/?series=418313&state=* Regards, Dumitru > On 8/6/24 05:44, Dumitru Ceara wrote: >> This series adds support for sampling packets processed by ACLs by using >> per-flow IPFIX. This new feature allows users to configure >> (potentially) different sampling options for ACL matched traffic that >> creates new connections or that is forwarded on existing connections. >> >> This work is based on Adrian's original RFC: >> https://patchwork.ozlabs.org/project/ovn/cover/[email protected]/ >> >> In order for the whole feature to work properly some pre-requisite work >> is done: >> - patch 1: fixes the QoS logical flow documentation. This is needed >> because the sampling patches need to insert new tables and numbers >> were inconsistent. >> - patch 2: fixes a bug in the way ACLs with labels are processed when >> the switches also have load balancers configured >> >> The feature itself is implemented by the last 3 patches: >> - patch 3: adds support for users to configure different types of >> sampling applications (drop debug, acl-new-traffic, >> acl-established-traffic) >> - patch 4: combines the already existing drop debug sampling >> configuration with the new sampling application configuration (giving >> priority to the latter) >> - patch 5: adds sampling support to ACLs >> >> Patches 6-9 implement an optimization and reduce the number of logical >> and openflow rules for the case when sampling is enabled for ACLs with a >> single collector (the common case). This optimization requires the >> recently added OVS support for sampling with observation IDs passed >> directly from fields [0]. >> >> [0] >> https://github.com/openvswitch/ovs/commit/1aa9e137fe36a810271415d79735dedfedfc9f6e >> >> Changes in V6: >> - Addressed (some) review comments from Ilya (individual changes listed >> in each patch). >> Most important changes: >> - Changed sample_collector schema to add unique ID (4 bit): this fixes >> the case with multiple probabilities per set_id and reduces the >> number of register and ct-mark bits used. >> - Made Sample table non-root (this needs changes to ovn-nbctl acl-add >> command too). >> Not addressed review comments: >> - Didn't use the single collector per sample_config type suggestion >> because OVN-K8s needs the flexibility of using different collectors >> (or multiple collectors) per ACL. >> Fixed a bug with sampling on to-lport ACLs when they're hit in the >> egress pipeline towards logical routers. >> >> Changes in V5: >> - Addressed review comments from Numan and Ilya (individual changes >> listed in each patch). The most important change is the >> NB.Sampling_App 'name' column change to 'type' along with shortening >> of the strings representing allowed app types. >> >> Changes in V4: >> - Addressed review comments from Mark, Ales and Numan (individual >> changes listed in each patch). >> - Dropped first 4 patches of V3 because they were already accepted. >> - Added a first 1/5 patch to fix documentation that I needed to touch >> too. >> - Added Ales as co-author of patch 5, he provided most of the >> incremental changes that were added to that patch in v4. >> - Included Ales' patches (6-9) to reduce the number of sampling flows >> when the underlying OVS instance supports sampling with IDs taken from >> fields (or registers). >> >> Changes in V3: >> - Addressed Ilya's comment and bumped NB schema version on patch 8. >> I didn't bump it on patch 6 too because I don't think these two >> commits will ever be separated in different releases. >> >> Changes in V2: >> - Addressed Adrian's comments on patch 8. >> - Fixed unit test failure in patch 2. >> >> Adrian Moreno (1): >> northd: Add ACL Sampling. >> >> Ales Musil (4): >> features: Make querying of OpenFlow features more versatile. >> features: Add detection for sample with registers. >> actions: Add support for sample with register. >> northd: Allow flow simplification for ACL sampling. >> >> Dumitru Ceara (4): >> northd: Fix up logical flow documentation for QoS. >> northd: Commit from-lport ACL label (and state) when LBs are used. >> northd: Add Sampling_App table. >> northd: Override NB_Global drop sampling id with Sampling_App config. >> >> NEWS | 6 + >> controller/chassis.c | 15 + >> controller/lflow.h | 12 +- >> include/ovn/actions.h | 16 +- >> include/ovn/features.h | 5 + >> include/ovn/logical-fields.h | 2 + >> lib/actions.c | 12 +- >> lib/features.c | 360 ++++++++--- >> lib/logical-fields.c | 12 + >> lib/ovn-util.h | 2 +- >> northd/automake.mk | 2 + >> northd/debug.c | 12 +- >> northd/debug.h | 3 +- >> northd/en-global-config.c | 41 +- >> northd/en-global-config.h | 1 + >> northd/en-lflow.c | 5 + >> northd/en-sampling-app.c | 117 ++++ >> northd/en-sampling-app.h | 51 ++ >> northd/inc-proc-northd.c | 11 +- >> northd/northd.c | 635 ++++++++++++++++++-- >> northd/northd.h | 55 +- >> northd/ovn-northd.8.xml | 157 +++-- >> ovn-nb.ovsschema | 63 +- >> ovn-nb.xml | 96 +++ >> tests/atlocal.in | 6 + >> tests/ovn-controller.at | 168 +++--- >> tests/ovn-macros.at | 14 +- >> tests/ovn-nbctl.at | 36 ++ >> tests/ovn-northd.at | 795 +++++++++++++++++++++++-- >> tests/ovn.at | 88 +-- >> tests/system-common-macros.at | 11 + >> tests/system-ovn.at | 475 ++++++++++++++- >> utilities/containers/fedora/Dockerfile | 1 + >> utilities/containers/ubuntu/Dockerfile | 1 + >> utilities/ovn-nbctl.8.xml | 8 +- >> utilities/ovn-nbctl.c | 35 +- >> 36 files changed, 2904 insertions(+), 425 deletions(-) >> create mode 100644 northd/en-sampling-app.c >> create mode 100644 northd/en-sampling-app.h >> > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
