diff mbox series

[ovs-dev,v1,16/20] ovn-sbctl: dump-flows and lflow-list should have the same options.

Message ID 20260901135638.290353-17-jtanenba@redhat.com
State Changes Requested
Headers show
Series ovn-sbctl: Review of ovn-sbctl documentation. | expand

Checks

Context Check Description
ovsrobot/apply-robot warning apply and check: warning

Commit Message

Jacob Tanenbaum Sept. 1, 2026, 1:56 p.m. UTC
dump-flows and lflow-list take the same arguments so the documentation
should reflect that.

Assisted-by: Claude Opus 4.6, Claude Code
Fixes: d25e286ddb5c ("ovn-controller: Tie OpenFlow and logical flows using OpenFlow cookie.")
Fixes: 6742717f04aa ("ovn-sbctl: Add --ovs option to "lflow-list", for listing OpenFlow flows.")
Fixes: eb25a7da639e ("Improve debuggability of OVN to OpenFlow translations.")
Signed-off-by: Jacob Tanenbaum <jtanenba@redhat.com>
---
 utilities/ovn-sbctl.8.xml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Mark Michelson Sept. 2, 2026, 5:28 p.m. UTC | #1
Hi Jacob.

This is one of those cases where the commit is technically correct:
dump-flows and lflow-list are aliases for the same operation. However,
I don't think it's right to mirror every option for both listings.
This means that every time an option is added or removed, we have to
document the change in two places. I think there are two possible
alternative ways to handle this.

1. Add a sentence to the end of the "lflow-list" section that states
that "dump-flows" can be used as an alias. This would be the way to go
if our intent is to prefer "lflow-list" over "dump-flows".
2. Have a barebones standalone entry for "dump-flows" that shows no
options at all. The text would say something like "This is an alias
for 'lflow-list' and accepts all of the same options." It would also
contain a reference to the "lflow-list" section so that HTML
documentation would generate a link to the "lflow-list" command. This
would be the way to go if our intent is to treat "lflow-list" and
"dump-flows" as equal citizens of ovn-sbctl, and we are fine with them
being used interchangeably.


On Tue, Sep 1, 2026 at 2:03 PM Jacob Tanenbaum via dev
<ovs-dev@openvswitch.org> wrote:
>
> dump-flows and lflow-list take the same arguments so the documentation
> should reflect that.
>
> Assisted-by: Claude Opus 4.6, Claude Code
> Fixes: d25e286ddb5c ("ovn-controller: Tie OpenFlow and logical flows using OpenFlow cookie.")
> Fixes: 6742717f04aa ("ovn-sbctl: Add --ovs option to "lflow-list", for listing OpenFlow flows.")
> Fixes: eb25a7da639e ("Improve debuggability of OVN to OpenFlow translations.")
> Signed-off-by: Jacob Tanenbaum <jtanenba@redhat.com>
> ---
>  utilities/ovn-sbctl.8.xml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/utilities/ovn-sbctl.8.xml b/utilities/ovn-sbctl.8.xml
> index a36ec6a0f..3afcf5dc3 100644
> --- a/utilities/ovn-sbctl.8.xml
> +++ b/utilities/ovn-sbctl.8.xml
> @@ -430,7 +430,7 @@
>          </p>
>        </dd>
>
> -      <dt>[<code>--uuid</code>] <code>dump-flows</code> [<var>logical-datapath</var>]</dt>
> +      <dt>[<code>--uuid</code>] [<code>--ovs</code>[<code>=<var>remote</var>]</code>] [<code>--stats</code>] [<code>--vflows</code>] <code>dump-flows</code> [<var>logical-datapath</var>] [<var>lflow</var>...]</dt>
>        <dd>Alias for <code>lflow-list</code>.</dd>
>
>        <dt><code>count-flows</code> [<var>logical-datapath</var>]</dt>
> --
> 2.55.0
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
diff mbox series

Patch

diff --git a/utilities/ovn-sbctl.8.xml b/utilities/ovn-sbctl.8.xml
index a36ec6a0f..3afcf5dc3 100644
--- a/utilities/ovn-sbctl.8.xml
+++ b/utilities/ovn-sbctl.8.xml
@@ -430,7 +430,7 @@ 
         </p>
       </dd>
 
-      <dt>[<code>--uuid</code>] <code>dump-flows</code> [<var>logical-datapath</var>]</dt>
+      <dt>[<code>--uuid</code>] [<code>--ovs</code>[<code>=<var>remote</var>]</code>] [<code>--stats</code>] [<code>--vflows</code>] <code>dump-flows</code> [<var>logical-datapath</var>] [<var>lflow</var>...]</dt>
       <dd>Alias for <code>lflow-list</code>.</dd>
 
       <dt><code>count-flows</code> [<var>logical-datapath</var>]</dt>