On 7/31/24 14:40, Dumitru Ceara wrote:
> 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?

Just means I didn't actually review the code.  I'm not sure I'll have time
for this, so don't wait for me.

Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to