Thank you for the review.
Indeed, shortening the command from "flow actions_update" to "flow update" 
seems more natural.
However, note that the command updates only the actions of a flow rule and 
leaves all other parameters unchanged.
My concern is that in the future there can be some "flow pattern_update" 
command, thus making "flow update" command ambiguous.
Also, the name is consistent with the underlying rte_flow_actions_update() 
function.
With that in mind, please clarify if the name should still be changed.
________________________________
From: Dariusz Sosnowski <dsosnow...@nvidia.com>
Sent: Monday, February 5, 2024 9:18 PM
To: NBU-Contact-Thomas Monjalon (EXTERNAL) <tho...@monjalon.net>; Mykola 
Kostenok <mko-...@napatech.com>; Christian Koue Muf <c...@napatech.com>; 
Oleksandr Kolomeiets <okl-...@napatech.com>
Cc: aman.deep.si...@intel.com <aman.deep.si...@intel.com>; 
yuying.zh...@intel.com <yuying.zh...@intel.com>; dev@dpdk.org <dev@dpdk.org>; 
Ori Kam <or...@nvidia.com>
Subject: RE: [PATCH v2] app/testpmd: support updating flow rule actions

> -----Original Message-----
> From: Thomas Monjalon <tho...@monjalon.net>
> Sent: Friday, February 2, 2024 10:55
> To: mko-...@napatech.com; c...@napatech.com; Oleksandr Kolomeiets
> <okl-...@napatech.com>
> Cc: aman.deep.si...@intel.com; yuying.zh...@intel.com; dev@dpdk.org;
> Ori Kam <or...@nvidia.com>; Dariusz Sosnowski <dsosnow...@nvidia.com>
> Subject: Re: [PATCH v2] app/testpmd: support updating flow rule actions
>
> 01/02/2024 10:59, Oleksandr Kolomeiets:
> > "flow actions_update" updates a flow rule specified by a rule ID with
> > a new action list by making a call to "rte_flow_actions_update()":
> >
> >     flow actions_update {port_id} {rule_id}
> >         actions {action} [/ {action} [...]] / end [user_id]
> >
> > Creating, updating and destroying a flow rule:
> >
> >     testpmd> flow create 0 group 1 pattern eth / end actions drop / end
> >     Flow rule #0 created
> >     testpmd> flow actions_update 0 0 actions queue index 1 / end
> >     Flow rule #0 updated with new actions
> >     testpmd> flow destroy 0 rule 0
> >     Flow rule #0 destroyed
>
> Why not a simple "flow update" command name?

+1. This would also make it consistent with async version of this command - 
"flow queue {port_id} update ...".

Reply via email to