On Fri, May 16, 2025 at 3:03 PM Dumitru Ceara via dev < ovs-dev@openvswitch.org> wrote:
> These get matched against OVS Flow_Sample_Collector.ID which is a 32-bit > value. OVN doesn't store the set_id anywhere (in conntrack) so there's > no reason to artificially limit this. Also update the documentation of > the NB.Sample_Collector.set_id field. > > Reported-at: > https://mail.openvswitch.org/pipermail/ovs-discuss/2025-May/053640.html > Reported-by: Trọng Đạt Trần <trantrongdat7...@gmail.com> > Fixes: d15b12da6fe6 ("northd: Add ACL Sampling.") > Signed-off-by: Dumitru Ceara <dce...@redhat.com> > --- > northd/northd.c | 12 ++++++------ > ovn-nb.xml | 2 +- > tests/system-ovn.at | 16 ++++++++-------- > 3 files changed, 15 insertions(+), 15 deletions(-) > > diff --git a/northd/northd.c b/northd/northd.c > index 7b05147b41..486da8c3e1 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -6798,11 +6798,11 @@ build_acl_sample_action(struct ds *actions, const > struct nbrec_acl *acl, > > for (size_t i = 0; i < sample->n_collectors; i++) { > ds_put_format(actions, "sample(probability=%"PRIu16"," > - "collector_set=%"PRIu8"," > + "collector_set=%"PRIu32"," > "obs_domain=%"PRIu32"," > "obs_point=%"PRIu32");", > (uint16_t) > sample->collectors[i]->probability, > - (uint8_t) sample->collectors[i]->set_id, > + (uint32_t) sample->collectors[i]->set_id, > domain_id, point_id); > } > ds_put_cstr(actions, " next;"); > @@ -7066,12 +7066,12 @@ build_acl_sample_generic_new_flows(const struct > ovn_datapath *od, > (uint8_t) obs_stage); > > ds_put_format(actions, "sample(probability=%"PRIu16"," > - "collector_set=%"PRIu8"," > + "collector_set=%"PRIu32"," > "obs_domain=%"PRIu32"," > "obs_point="REG_OBS_POINT_ID_NEW");" > " next;", > (uint16_t) coll->probability, > - (uint8_t) coll->set_id, > + (uint32_t) coll->set_id, > sample_domain_id); > > ovn_lflow_add(lflows, od, stage, stateful ? 1000 : 900, > ds_cstr(match), > @@ -7105,12 +7105,12 @@ build_acl_sample_generic_est_flows(const struct > ovn_datapath *od, > (uint8_t) obs_stage); > > ds_put_format(actions, "sample(probability=%"PRIu16"," > - "collector_set=%"PRIu8"," > + "collector_set=%"PRIu32"," > "obs_domain=%"PRIu32"," > "obs_point=ct_label.obs_point_id);" > " next;", > (uint16_t) coll->probability, > - (uint8_t) coll->set_id, > + (uint32_t) coll->set_id, > sample_domain_id); > > ovn_lflow_add(lflows, od, stage, 1000, ds_cstr(match), > diff --git a/ovn-nb.xml b/ovn-nb.xml > index 4396023a4b..b846d21fbe 100644 > --- a/ovn-nb.xml > +++ b/ovn-nb.xml > @@ -553,7 +553,7 @@ > sampled while a value of 65535 corresponds to all packets being > sampled. > </column> > <column name="set_id"> > - The 8-bit integer identifier of the set of of collectors to send > + The 32-bit integer identifier of the set of of collectors to send > packets to. See Flow_Sample_Collector_Set Table in ovs-vswitchd's > database schema. > </column> > diff --git a/tests/system-ovn.at b/tests/system-ovn.at > index 1dccefb094..59f16ec22f 100644 > --- a/tests/system-ovn.at > +++ b/tests/system-ovn.at > @@ -13413,7 +13413,7 @@ ADD_VETH(vm1, vm1, br-int, "42.42.42.2/24", > "00:00:00:00:00:01", "42.42.42.1") > ADD_NAMESPACES(vm2) > ADD_VETH(vm2, vm2, br-int, "42.42.42.3/24", "00:00:00:00:00:02", > "42.42.42.1") > > -collector1=$(ovn-nbctl create Sample_Collector id=1 name=c1 > probability=65535 set_id=100) > +collector1=$(ovn-nbctl create Sample_Collector id=1 name=c1 > probability=65535 set_id=4294967295) > collector2=$(ovn-nbctl create Sample_Collector id=2 name=c2 > probability=65535 set_id=200) > check_row_count nb:Sample_Collector 2 > > @@ -13479,7 +13479,7 @@ OVS_WAIT_UNTIL([grep -q 'Startup nfcapd.' > collector.err]) > dnl Configure the OVS flow sample collector. > ovs-vsctl --id=@br get Bridge br-int \ > -- --id=@ipfix create IPFIX targets=\"127.0.0.1:4242\" > template_interval=1 \ > - -- --id=@cs create Flow_Sample_Collector_Set id=100 bridge=@br > ipfix=@ipfix > + -- --id=@cs create Flow_Sample_Collector_Set id=4294967295 bridge=@br > ipfix=@ipfix > > check ovn-nbctl --wait=hv sync > dnl And wait for it to be up and running. > @@ -13610,7 +13610,7 @@ ADD_VETH(vm1, vm1, br-int, "42.42.42.2/24", > "00:00:00:00:00:01", "42.42.42.1") > ADD_NAMESPACES(vm2) > ADD_VETH(vm2, vm2, br-int, "42.42.42.3/24", "00:00:00:00:00:02", > "42.42.42.1") > > -collector1=$(ovn-nbctl create Sample_Collector id=1 name=c1 > probability=65535 set_id=100) > +collector1=$(ovn-nbctl create Sample_Collector id=1 name=c1 > probability=65535 set_id=4294967295) > check_row_count nb:Sample_Collector 1 > > check_uuid ovn-nbctl create Sampling_App type="acl-new" id="42" > @@ -13648,7 +13648,7 @@ OVS_WAIT_UNTIL([grep -q 'Startup nfcapd.' > collector.err]) > dnl Configure the OVS flow sample collector. > ovs-vsctl --id=@br get Bridge br-int \ > -- --id=@ipfix create IPFIX targets=\"127.0.0.1:4242\" > template_interval=1 \ > - -- --id=@cs create Flow_Sample_Collector_Set id=100 bridge=@br > ipfix=@ipfix > + -- --id=@cs create Flow_Sample_Collector_Set id=4294967295 bridge=@br > ipfix=@ipfix > > check ovn-nbctl --wait=hv sync > dnl And wait for it to be up and running. > @@ -13737,7 +13737,7 @@ ADD_VETH(vm1, vm1, br-int, "42.42.42.2/24", > "00:00:00:00:00:01", "42.42.42.1") > ADD_NAMESPACES(vm2) > ADD_VETH(vm2, vm2, br-int, "42.42.42.3/24", "00:00:00:00:00:02", > "42.42.42.1") > > -collector1=$(ovn-nbctl create Sample_Collector id=1 name=c1 > probability=65535 set_id=100) > +collector1=$(ovn-nbctl create Sample_Collector id=1 name=c1 > probability=65535 set_id=4294967295) > check_row_count nb:Sample_Collector 1 > > check_uuid ovn-nbctl create Sampling_App type="acl-new" id="42" > @@ -13773,7 +13773,7 @@ OVS_WAIT_UNTIL([grep -q 'Startup nfcapd.' > collector.err]) > dnl Configure the OVS flow sample collector. > ovs-vsctl --id=@br get Bridge br-int \ > -- --id=@ipfix create IPFIX targets=\"127.0.0.1:4242\" > template_interval=1 \ > - -- --id=@cs create Flow_Sample_Collector_Set id=100 bridge=@br > ipfix=@ipfix > + -- --id=@cs create Flow_Sample_Collector_Set id=4294967295 bridge=@br > ipfix=@ipfix > > check ovn-nbctl --wait=hv sync > dnl And wait for it to be up and running. > @@ -13867,7 +13867,7 @@ check ovn-nbctl --wait=sb sync > ADD_NAMESPACES(vm1) > ADD_VETH(vm1, vm1, br-int, "42.42.42.2/24", "00:00:00:00:00:01", > "42.42.42.1") > > -collector1=$(ovn-nbctl create Sample_Collector id=1 name=c1 > probability=65535 set_id=100) > +collector1=$(ovn-nbctl create Sample_Collector id=1 name=c1 > probability=65535 set_id=4294967295) > collector2=$(ovn-nbctl create Sample_Collector id=2 name=c2 > probability=65535 set_id=200) > check_row_count nb:Sample_Collector 2 > > @@ -13902,7 +13902,7 @@ OVS_WAIT_UNTIL([grep -q 'Startup nfcapd.' > collector.err]) > dnl Configure the OVS flow sample collector. > ovs-vsctl --id=@br get Bridge br-int \ > -- --id=@ipfix create IPFIX targets=\"127.0.0.1:4242\" > template_interval=1 \ > - -- --id=@cs create Flow_Sample_Collector_Set id=100 bridge=@br > ipfix=@ipfix > + -- --id=@cs create Flow_Sample_Collector_Set id=4294967295 bridge=@br > ipfix=@ipfix > > check ovn-nbctl --wait=hv sync > dnl And wait for it to be up and running. > -- > 2.49.0 > > _______________________________________________ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev Thank you Dumitru, I went ahead and applied a patch to main and backported down to 24.09. Regards, Ales _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
