[PATCH v4] example: classifier: add queue and cos configurability#2339
Conversation
odpbuild
changed the title
| " <dst queue> Name of the destination queue (CoS).\n" | ||
| "\n" | ||
| " -c, --count <num> CPU count, 0=all available, default=1\n" | ||
| " -q, --cos_queue_param <name>:<queue prio>:<queue sync>:<cos enqueue profile type>:<optional custom value>:<optional tmo ns>:<optional size>\n" |
There was a problem hiding this comment.
I think the pre-existing implementation has some design issues that would be good to fix or improve before building more functionality on top.
The usage text and the command line parameters seem to conflate CoS objects and queues. They refer to source queues, although there are no such things, but source CoS:es. Then the "destination queue" term is used to refer both to the actual destination queue and the destination CoS. The implementation creates one CoS for each 'queue' and does not handle cleanly multiple policy rules with the same destination CoS/queue. The usage text does not really explain how the config model of this example app differs from the config model in ODP API, which is not helpful.
I think it would be good to improve the existing design before the new queue config things get added.
I suppose policy (or PMR rule) config could be separate from CoS config. CoS objects could be explicitly created (or on demand, as needed by PMR rules, if not existing already, i.e. not one-to-one with the policy rules. Similarly, queues could perhaps be separately created. Or if the queues would be implicitly created with 1:1 correspondence with CoS objects, that could be mentioned in the help text (and fixing the CoS / queue terminology would make things more clear too).
Then the queue config and CoS config for queue enqueuing that gets added in this commit could build on top of a cleaner base.
TuomasTaipale
force-pushed
the
dev/add_cls_ex_queue_conf
branch
from
5b8f22b to
bdfac68
Compare
odpbuild
changed the title
MatiasElo
left a comment
MatiasElo
left a comment
There was a problem hiding this comment.
Couple optional nits.
TuomasTaipale
force-pushed
the
dev/add_cls_ex_queue_conf
branch
from
bdfac68 to
619ee91
Compare
odpbuild
changed the title
Add new `-q`, `--cos_queue_param` option for configuring the CoS/queue that is defined with the policy destination queue parameter (`-p <dst queue>`). For a queue, priority, synchronization and a single packet aggregator with timeout and vector size can be configured. For the related CoS, aggregator enqueue profile and optional parameter for the custom profile can be configured. These now enable the testing of event aggregation in the context of classification with varying CoS aggregator enqueuing profiles. Signed-off-by: Tuomas Taipale <tuomas.taipale@nokia.com> Reviewed-by: Matias Elo <matias.elo@nokia.com>
TuomasTaipale
force-pushed
the
dev/add_cls_ex_queue_conf
branch
from
619ee91 to
6f4bbcd
Compare
odpbuild
changed the title
3 participants
Add new
-q,--cos_queue_paramoption for configuring the CoS/queue that is defined with the policy destination queue parameter (-p <dst queue>).For a queue, priority, synchronization and a single packet aggregator with timeout and vector size can be configured. For the related CoS, aggregator enqueue profile and optional parameter for the custom profile can be configured.
These now enable the testing of event aggregation in the context of classification with varying CoS aggregator enqueuing profiles.
v3:
v4: