Message ID | 20240806094451.730622-1-dceara@redhat.com |
---|---|
Headers | show |
Series | Add ACL Sampling using per-flow IPFIX. | expand |
For patches 1-8: Acked-by: Mark Michelson <mmichels@redhat.com> I have a single small comment on patch 9. 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/20221018155936.1394396-1-amorenoz@redhat.com/ > > 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 >
On 8/6/24 20:44, Mark Michelson wrote: > For patches 1-8: > > Acked-by: Mark Michelson <mmichels@redhat.com> > 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/20221018155936.1394396-1-amorenoz@redhat.com/ >> >> 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 >> >