diff mbox series

[ovs-dev,v2,2/2] vswitch: Add missing documentation for "ct_flush" capability

Message ID 20230306133706.156615-2-amusil@redhat.com
State Changes Requested
Headers show
Series [ovs-dev,v2,1/2] dpctl: Fix flush-conntrack with datapath as argument | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/intel-ovs-compilation success test: success

Commit Message

Ales Musil March 6, 2023, 1:37 p.m. UTC
Signed-off-by: Ales Musil <amusil@redhat.com>
---
 vswitchd/vswitch.xml | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Ilya Maximets March 9, 2023, 12:15 p.m. UTC | #1
On 3/6/23 14:37, Ales Musil wrote:
> Signed-off-by: Ales Musil <amusil@redhat.com>

Needs a Fixes tag.

> ---
>  vswitchd/vswitch.xml | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index 12708a313..15e4f97b7 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -6301,6 +6301,11 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \
>          translated to an ephemeral port. If there is no collision, no SNAT
>          is performed.
>        </column>
> +      <column name="capabilities" key="ct_flush"
> +              type='{"type": "boolean"}'>
> +        True if the datapath supports CT flush extension. The extensions
> +        allows to flush CT entries based on specified parameters.

This doesn't really describe the capability.  It was possible to flush
conntrack based on the zone in the past.  That matches the description,
but doesn't match the intent behind the capability flag.  You probably
should mention how this capability relates to the new OpenFlow request.

Best regards, Ilya Maximets.
Ales Musil March 9, 2023, 1:58 p.m. UTC | #2
On Thu, Mar 9, 2023 at 1:15 PM Ilya Maximets <i.maximets@ovn.org> wrote:

> On 3/6/23 14:37, Ales Musil wrote:
> > Signed-off-by: Ales Musil <amusil@redhat.com>
>
> Needs a Fixes tag.
>
> > ---
> >  vswitchd/vswitch.xml | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> > index 12708a313..15e4f97b7 100644
> > --- a/vswitchd/vswitch.xml
> > +++ b/vswitchd/vswitch.xml
> > @@ -6301,6 +6301,11 @@ ovs-vsctl add-port br0 p0 -- set Interface p0
> type=patch options:peer=p1 \
> >          translated to an ephemeral port. If there is no collision, no
> SNAT
> >          is performed.
> >        </column>
> > +      <column name="capabilities" key="ct_flush"
> > +              type='{"type": "boolean"}'>
> > +        True if the datapath supports CT flush extension. The extensions
> > +        allows to flush CT entries based on specified parameters.
>
> This doesn't really describe the capability.  It was possible to flush
> conntrack based on the zone in the past.  That matches the description,
> but doesn't match the intent behind the capability flag.  You probably
> should mention how this capability relates to the new OpenFlow request.
>
> Best regards, Ilya Maximets.
>
>
I've made the relation with the extension more explicit in v3.

Thanks,
Ales
diff mbox series

Patch

diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 12708a313..15e4f97b7 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -6301,6 +6301,11 @@  ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \
         translated to an ephemeral port. If there is no collision, no SNAT
         is performed.
       </column>
+      <column name="capabilities" key="ct_flush"
+              type='{"type": "boolean"}'>
+        True if the datapath supports CT flush extension. The extensions
+        allows to flush CT entries based on specified parameters.
+      </column>
     </group>
 
     <group title="Common Columns">