| 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 >> >
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