Re: [PATCH v3 25/25] doc: describe testpmd flow command

[Adrien Mazarguil <adrien.mazarguil@6wind.com>]

On Mon, Dec 19, 2016 at 08:44:07PM +0000, Mcnamara, John wrote:
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Adrien Mazarguil
> > Sent: Monday, December 19, 2016 5:49 PM
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH v3 25/25] doc: describe testpmd flow command
> > 
> > Document syntax, interaction with rte_flow and provide usage examples.
> > 
> > Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> > 
> > ...
> >
> > +
> > +- Check whether a flow rule can be created::
> > +
> > +   flow validate {port_id}
> > +       [group {group_id}] [priority {level}] [ingress] [egress]
> > +       pattern {item} [/ {item} [...]] / end
> > +       actions {action} [/ {action} [...]] / end
> > +
> > +- Create a flow rule::
> > +
> > +   flow create {port_id}
> > +       [group {group_id}] [priority {level}] [ingress] [egress]
> > +       pattern {item} [/ {item} [...]] / end
> > +       actions {action} [/ {action} [...]] / end
> > +
> > +- Destroy specific flow rules::
> > +
> > +   flow destroy {port_id} rule {rule_id} [...]
> > +
> > +- Destroy all flow rules::
> > +
> > +   flow flush {port_id}
> > +
> 
> Just a note:
> 
> The verbs destroy and flush don't sound right here. Create/destroy are common
> verbs pairs for objects but these actions are more like add/remove. I guess the
> names come from the underlying APIs which possibly are creating/freeing
> objects/structures but maybe they should be called add/remove as well.
> 
> And flush generally applies to a pipeline or a queue. The action here is closer
> to "remove all".
> 
> Probably not worth reworking at this stage if it hasn't bothered anyone else.

Well, Kevin Traynor made a similar suggestion to which I replied that the
name would be modified if enough people complained [1].

I understand your point but for some reason I keep hearing a flushing noise
every time all rules are removed at once, hence the name.

Problem is also that we now have 3 PMD series floating on the ML that depend
on the current definition. If we decided to change it, I suggest doing so in
a separate fix. A few more complaints from developers are needed before it's
too late for 17.02.

> > +underlying device in its current state but stops short of creating it.
> > +It is bound to ``rte_flow_validate()``::
> > +
> > + flow validate {port_id}
> > +     [group {group_id}] [priority {level}] [ingress] [egress]
> > +     pattern {item} [/ {item} [...]] / end
> > +     actions {action} [/ {action} [...]] / end
> > +
> 
> Here and elsewhere the indentation should be the RST standard 3 spaces,
> similar to the rest of the doc. This is only worth changing if you
> do some other revision of this doc.
> 
> Otherwise very good documentation.
> 
> Acked-by: John McNamara <john.mcnamara@intel.com>

Thanks, I'll make those changes if anything else warrants a v4.

[1] http://dpdk.org/ml/archives/dev/2016-December/050973.html

-- 
Adrien Mazarguil
6WIND