On 7/31/24 14:31, Ilya Maximets wrote:
> On 7/31/24 11:05, Dumitru Ceara wrote:
>> From: Adrian Moreno <[email protected]>
>>
>> Introduce a new table called Sample where per-flow IPFIX configuration
>> can be specified.
>> Also, reference rows from such table from the ACL table to enable the
>> configuration of ACL sampling. If enabled, northd will add a sample
>> action to each ACL related logical flow.
>>
>> Packets that hit stateful ACLs are sampled in different ways depending
>> whether they are initiating a new session or are just forwarded on an
>> existing (already allowed) session.  Two new columns ("sample_new" and
>> "sample_est") are added to the ACL table to allow for potentially
>> different sampling rates for the two cases.
>>
>> Note: If an ACL has both sampling enabled and a label associated to it
>> then the label value overrides the observation point ID defined in the
>> sample configuration.  This is a side effect of the implementation
>> (observation point IDs are stored in conntrack in the same part of the
>> ct_label where ACL labels are also stored).  The two features
>> (sampling and ACL labels) serve however similar purposes so it's not
>> expected that they're both enabled together.
> 
> Not a full review, but this is a user-visible behavior, so it should be
> documented.  In the description of both features.  Note in a commit message
> is not enough.
> 

Fair enough.

Does "not a full review" mean that I should expect more comments from
you on this version of this patch or that it's fine to fix this and post
v5 when I think enough time has passed for others to have a chance to
review?

Regards,
Dumitru

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

Reply via email to