On 5/26/25 12:29 PM, Ales Musil wrote:
> On Fri, May 16, 2025 at 3:03 PM Dumitru Ceara via dev <
> [email protected]> 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 <[email protected]>
>> Fixes: d15b12da6fe6 ("northd: Add ACL Sampling.")
>> Signed-off-by: Dumitru Ceara <[email protected]>
>> ---
>>  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
>> [email protected]
>> 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.
> 

Cool, thanks a lot!

Regards,
Dumitru

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to