[ovs-dev,v3,1/9] ovs-vswitchd: Add Datapath, CT_Zone, and CT_Zone_Policy tables.
diff mbox series

Message ID 1565657498-62682-2-git-send-email-yihung.wei@gmail.com
State New
Headers show
Series
  • Support zone-based conntrack timeout policy
Related show

Commit Message

Yi-Hung Wei Aug. 13, 2019, 12:51 a.m. UTC
From: Justin Pettit <jpettit@ovn.org>

Signed-off-by: Justin Pettit <jpettit@ovn.org>
Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com>
Co-authored-by: Yi-Hung Wei <yihung.wei@gmail.com>
---
 vswitchd/vswitch.ovsschema |  51 ++++++++-
 vswitchd/vswitch.xml       | 275 +++++++++++++++++++++++++++++++++++++--------
 2 files changed, 277 insertions(+), 49 deletions(-)

Comments

Darrell Ball Aug. 13, 2019, 2:46 a.m. UTC | #1
Thanks for the patch

On Mon, Aug 12, 2019 at 5:52 PM Yi-Hung Wei <yihung.wei@gmail.com> wrote:

> From: Justin Pettit <jpettit@ovn.org>
>
> Signed-off-by: Justin Pettit <jpettit@ovn.org>
> Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com>
> Co-authored-by: Yi-Hung Wei <yihung.wei@gmail.com>
> ---
>  vswitchd/vswitch.ovsschema |  51 ++++++++-
>  vswitchd/vswitch.xml       | 275
> +++++++++++++++++++++++++++++++++++++--------
>  2 files changed, 277 insertions(+), 49 deletions(-)
>
> diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
> index f7c6eb8983cd..c0a2242ad345 100644
> --- a/vswitchd/vswitch.ovsschema
> +++ b/vswitchd/vswitch.ovsschema
> @@ -1,9 +1,14 @@
>  {"name": "Open_vSwitch",
> - "version": "8.0.0",
> - "cksum": "3962141869 23978",
> + "version": "8.1.0",
> + "cksum": "1635647160 26090",
>   "tables": {
>     "Open_vSwitch": {
>       "columns": {
> +       "datapaths": {
> +         "type": {"key": {"type": "string"},
>

I had a minor comment in V2 about using an enum here for key - 'system' or
'netdev'
Does it work or is there worry that other datapath types will likely develop
and we will need to update the enum ?


> +                  "value": {"type": "uuid",
> +                            "refTable": "Datapath"},
> +                  "min": 0, "max": "unlimited"}},

        "bridges": {
>           "type": {"key": {"type": "uuid",
>                            "refTable": "Bridge"},
> @@ -629,6 +634,48 @@
>                    "min": 0, "max": "unlimited"},
>           "ephemeral": true}},
>       "indexes": [["target"]]},
> +   "Datapath": {
> +     "columns": {
> +       "datapath_version": {
> +         "type": "string"},
> +       "ct_zones": {
> +         "type": {"key": {"type": "integer",
> +                          "minInteger": 0,
> +                          "maxInteger": 65535},
> +                  "value": {"type": "uuid",
> +                            "refTable": "CT_Zone"},
> +                  "min": 0, "max": "unlimited"}},
>

minor comment from V2; I think
+                  "min": 0, "max": "unlimited"}},
should be
+                  "min": 0, "max": "65536"}},



> +       "external_ids": {
> +         "type": {"key": "string", "value": "string",
> +                  "min": 0, "max": "unlimited"}}}},
> +   "CT_Zone": {
> +     "columns": {
> +       "timeout_policy": {
> +         "type": {"key": {"type": "uuid",
> +                          "refTable": "CT_Timeout_Policy"},
> +                  "min": 0, "max": 1}},
> +       "external_ids": {
> +         "type": {"key": "string", "value": "string",
> +                  "min": 0, "max": "unlimited"}}}},
> +   "CT_Timeout_Policy": {
> +     "columns": {
> +       "timeouts": {
> +         "type": {"key": {"type" : "string",
> +                          "enum": ["set", ["tcp_syn_sent", "tcp_syn_recv",
> +                                           "tcp_established",
> "tcp_fin_wait",
> +                                           "tcp_close_wait",
> "tcp_last_ack",
> +                                           "tcp_time_wait", "tcp_close",
> +                                           "tcp_syn_sent2",
> "tcp_retransmit",
> +                                           "tcp_unack", "udp_first",
> +                                           "udp_single", "udp_multiple",
> +                                           "icmp_first", "icmp_reply"]]},
> +                  "value": {"type" : "integer",
> +                            "minInteger" : 0,
> +                            "maxInteger" : 4294967295},
> +                  "min": 0, "max": "unlimited"}},
>

Should it be ?
+                  "min": 0, "max": "16"}},

or will this create upgrade issues ?



> +       "external_ids": {
> +         "type": {"key": "string", "value": "string",
> +                  "min": 0, "max": "unlimited"}}}},
>     "SSL": {
>       "columns": {
>         "private_key": {
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index 027aee2f523b..495f0acad842 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -52,6 +52,13 @@
>      one record in the <ref table="Open_vSwitch"/> table.
>
>      <group title="Configuration">
> +      <column name="datapaths">
> +        Map of datapath types to datapaths.  The
> +        <ref column="datapath_type"/> column of the <ref table="Bridge"/>
> +        table is used as a key for this map.  The value points to a row in
> +        the <ref table="Datapath"/> table.
> +      </column>
> +
>        <column name="bridges">
>          Set of bridges managed by the daemon.
>        </column>
> @@ -1192,53 +1199,11 @@
>        </column>
>
>        <column name="datapath_version">
> -        <p>
> -          Reports the version number of the Open vSwitch datapath in use.
> -          This allows management software to detect and report
> discrepancies
> -          between Open vSwitch userspace and datapath versions.  (The <ref
> -          column="ovs_version" table="Open_vSwitch"/> column in the <ref
> -          table="Open_vSwitch"/> reports the Open vSwitch userspace
> version.)
> -          The version reported depends on the datapath in use:
> -        </p>
> -
> -        <ul>
> -          <li>
> -            When the kernel module included in the Open vSwitch source
> tree is
> -            used, this column reports the Open vSwitch version from which
> the
> -            module was taken.
> -          </li>
> -
> -          <li>
> -            When the kernel module that is part of the upstream Linux
> kernel is
> -            used, this column reports <code>&lt;unknown&gt;</code>.
> -          </li>
> -
> -          <li>
> -            When the datapath is built into the <code>ovs-vswitchd</code>
> -            binary, this column reports <code>&lt;built-in&gt;</code>.  A
> -            built-in datapath is by definition the same version as the
> rest of
> -            the Open VSwitch userspace.
> -          </li>
> -
> -          <li>
> -            Other datapaths (such as the Hyper-V kernel datapath)
> currently
> -            report <code>&lt;unknown&gt;</code>.
> -          </li>
> -        </ul>
> -
> -        <p>
> -          A version discrepancy between <code>ovs-vswitchd</code> and the
> -          datapath in use is not normally cause for alarm.  The Open
> vSwitch
> -          kernel datapaths for Linux and Hyper-V, in particular, are
> designed
> -          for maximum inter-version compatibility: any userspace version
> works
> -          with with any kernel version.  Some reasons do exist to insist
> on
> -          particular user/kernel pairings.  First, newer kernel versions
> add
> -          new features, that can only be used by new-enough userspace,
> e.g.
> -          VXLAN tunneling requires certain minimal userspace and kernel
> -          versions.  Second, as an extension to the first reason, some
> newer
> -          kernel versions add new features for enhancing performance that
> only
> -          new-enough userspace versions can take advantage of.
> -        </p>
> +          Reports the datapath version.  This column is maintained for
> +          backwards compatibility.  The preferred locatation is the
> +          <ref column="datapath_id" table="Datapath"/> column of the
> +          <ref table="Datapath"/> table.  The full documentation for this
> +          column is there.
>        </column>
>
>        <column name="other_config" key="datapath-id">
> @@ -5560,6 +5525,222 @@ ovs-vsctl add-port br0 p0 -- set Interface p0
> type=patch options:peer=p1 \
>      </group>
>    </table>
>
> +  <table name="Datapath">
> +    <p>
> +      Configuration for a datapath within <ref table="Open_vSwitch"/>.
> +    </p>
> +    <p>
> +      A datapath is responsible for providing the packet handling in Open
> +      vSwitch.  There are two primary datapath implementations used by
> +      Open vSwitch: kernel and userspace.  Kernel datapath
> +      implementations are available for Linux and Hyper-V, and selected
> +      as <code>system</code> in the <ref column="datapath_type"/> column
> +      of the <ref table="Bridge"/> table.  The userspace datapath is used
> +      by DPDK and AF-XDP, and is selected as <code>netdev</code> in the
> +      <ref column="datapath_type"/> column of the <ref table="Bridge"/>
> +      table.
> +    </p>
> +    <p>
> +      A datapath of a particular type is shared by all the bridges that
> use
> +      that datapath.  Thus, configurations applied to this table affect
> +      all bridges that use this datapath.
> +    </p>
> +
> +    <column name="datapath_version">
> +      <p>
> +        Reports the version number of the Open vSwitch datapath in use.
> +        This allows management software to detect and report discrepancies
> +        between Open vSwitch userspace and datapath versions.  (The <ref
> +        column="ovs_version" table="Open_vSwitch"/> column in the <ref
> +        table="Open_vSwitch"/> reports the Open vSwitch userspace
> version.)
> +        The version reported depends on the datapath in use:
>

What is the recommended usage here for the userspace datapath.
Should we just say to refer to  'ovs_version' in table="Open_vSwitch"/>  for
the userspace datapath case or set the same value here ?



> +      </p>
> +
> +      <ul>
> +        <li>
> +          When the kernel module included in the Open vSwitch source tree
> is
> +          used, this column reports the Open vSwitch version from which
> the
> +          module was taken.
> +        </li>
> +
> +        <li>
> +          When the kernel module that is part of the upstream Linux
> kernel is
> +          used, this column reports <code>&lt;unknown&gt;</code>.
> +        </li>
> +
> +        <li>
> +          When the datapath is built into the <code>ovs-vswitchd</code>
> +          binary, this column reports <code>&lt;built-in&gt;</code>.  A
> +          built-in datapath is by definition the same version as the rest
> of
> +          the Open VSwitch userspace.
> +        </li>
> +
> +        <li>
> +          Other datapaths (such as the Hyper-V kernel datapath) currently
> +          report <code>&lt;unknown&gt;</code>.
> +        </li>
> +      </ul>
> +
> +      <p>
> +        A version discrepancy between <code>ovs-vswitchd</code> and the
> +        datapath in use is not normally cause for alarm.  The Open vSwitch
> +        kernel datapaths for Linux and Hyper-V, in particular, are
> designed
> +        for maximum inter-version compatibility: any userspace version
> works
> +        with with any kernel version.  Some reasons do exist to insist on
> +        particular user/kernel pairings.  First, newer kernel versions add
> +        new features, that can only be used by new-enough userspace, e.g.
> +        VXLAN tunneling requires certain minimal userspace and kernel
> +        versions.  Second, as an extension to the first reason, some newer
> +        kernel versions add new features for enhancing performance that
> only
> +        new-enough userspace versions can take advantage of.
> +      </p>
> +    </column>
> +
> +    <column name="ct_zones">
> +      Configuration for connection tracking zones.  Each pair maps from a
> +      zone id to a configuration for that zone.  Zone <code>0</code>
> applies
> +      to the default zone (ie, the one used if a zone is not specified in
> +      connection tracking-related OpenFlow matches and actions).
> +    </column>
> +
> +    <group title="Common Columns">
> +      The overall purpose of these columns is described under <code>Common
> +      Columns</code> at the beginning of this document.
> +
> +      <column name="external_ids"/>
> +    </group>
> +  </table>
> +
> +  <table name="CT_Zone">
> +    Connection tracking zone configuration
> +
> +    <column name="timeout_policy">
> +      Connection tracking timeout policy for this zone. If a timeout
> policy
> +      is not specified, it defaults to the timeout policy in the system.
> +    </column>
> +
> +    <group title="Common Columns">
> +      The overall purpose of these columns is described under <code>Common
> +      Columns</code> at the beginning of this document.
> +
> +      <column name="external_ids"/>
> +    </group>
> +  </table>
> +
> +  <table name="CT_Timeout_Policy">
> +    Connection tracking timeout policy configuration
> +
> +    <group title="Timeouts">
> +      <column name="timeouts">
> +        The <code>timeouts</code> column contains key-value pairs used
> +        to configure connection tracking timeouts in a datapath.
> +        Key-value pairs that are not supported by a datapath are
> +        ignored.  The timeout value is in seconds.
> +      </column>
> +
> +      <group title="TCP Timeouts">
> +        <column name="timeouts" key="tcp_syn_sent">
> +          The timeout for the connection after the first TCP SYN packet
> has
> +          been seen by conntrack.
> +        </column>
> +
> +        <column name="timeouts" key="tcp_syn_recv">
> +          The timeout of the connection after the first TCP SYN-ACK packet
> +          has been seen by conntrack.
> +        </column>
> +
> +        <column name="timeouts" key="tcp_established">
> +          The timeout of the connection after the connection has been
> fully
> +          established.
> +        </column>
> +
> +        <column name="timeouts" key="tcp_fin_wait">
> +          The timeout of the connection after the first TCP FIN packet
> +          has been seen by conntrack.
> +        </column>
> +
> +        <column name="timeouts" key="tcp_close_wait">
> +          The timeout of the connection after the first TCP ACK packet
> +          has been seen after it receives TCP FIN packet.  This timeout
> +          is only supported by the Linux kernel datapath.
> +        </column>
> +
> +        <column name="timeouts" key="tcp_last_ack">
> +          The timeout of the connection after TCP FIN packets have been
> +          seen by conntrack from both directions.  This timeout is only
> +          supported by the Linux kernel datapath.
> +        </column>
> +
> +        <column name="timeouts" key="tcp_time_wait">
> +          The timeout of the connection after conntrack has seen the
> +          TCP ACK packet for the second TCP FIN packet.
> +        </column>
> +
> +        <column name="timeouts" key="tcp_close">
> +          The timeout of the connection after the first TCP RST packet
> +          has been seen by conntrack.
> +        </column>
> +
> +        <column name="timeouts" key="tcp_syn_sent2">
> +          The timeout of the connection when only a TCP SYN packet has
> been
> +          seen by conntrack from both directions (simultaneous open).
> +          This timeout is only supported by the Linux kernel datapath.
> +        </column>
> +
> +        <column name="timeouts" key="tcp_retransmit">
> +          The timeout of the connection when it exceeds the maximum
> +          number of retransmissions.  This timeout is only supported by
> +          the Linux kernel datapath.
> +        </column>
> +
> +        <column name="timeouts" key="tcp_unack">
> +          The timeout of the connection when non-SYN packets create an
> +          established connection in TCP loose tracking mode.  This timeout
> +          is only supported by the Linux kernel datapath.
> +        </column>
> +      </group>
> +
> +      <group title="UDP Timeouts">
> +        <column name="timeouts" key="udp_first">
> +          The timeout of the connection after the first UDP packet has
> +          been seen by conntrack.  This timeout is only supported by the
> +          userspace datapath.
> +        </column>
> +
> +        <column name="timeouts" key="udp_single">
> +          The timeout of the connection when conntrack only seen UDP
> +          packet from the source host, but the destination host has never
> +          sent one back.
> +        </column>
> +
> +        <column name="timeouts" key="udp_multiple">
> +          The timeout of the connection when UDP packets have been seen in
> +          both directions.
> +        </column>
> +      </group>
> +
> +      <group title="ICMP Timeouts">
> +        <column name="timeouts" key="icmp_first">
> +          The timeout of the connection after the first ICMP packet has
> +          been seen by conntrack.
> +        </column>
> +
> +        <column name="timeouts" key="icmp_reply">
> +          The timeout of the connection after an ICMP error is replied in
> +          response to an ICMP packet.  This timeout is only supported by
> +          the userspace datapath.
> +        </column>
> +      </group>
> +    </group>
> +
> +    <group title="Common Columns">
> +      The overall purpose of these columns is described under <code>Common
> +      Columns</code> at the beginning of this document.
> +
> +      <column name="external_ids"/>
> +    </group>
> +  </table>
> +
>    <table name="SSL">
>      SSL configuration for an Open_vSwitch.
>
> --
> 2.7.4
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Yi-Hung Wei Aug. 14, 2019, 4:47 p.m. UTC | #2
On Mon, Aug 12, 2019 at 7:46 PM Darrell Ball <dlu998@gmail.com> wrote:
>> diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
>> index f7c6eb8983cd..c0a2242ad345 100644
>> --- a/vswitchd/vswitch.ovsschema
>> +++ b/vswitchd/vswitch.ovsschema
>> @@ -1,9 +1,14 @@
>>  {"name": "Open_vSwitch",
>> - "version": "8.0.0",
>> - "cksum": "3962141869 23978",
>> + "version": "8.1.0",
>> + "cksum": "1635647160 26090",
>>   "tables": {
>>     "Open_vSwitch": {
>>       "columns": {
>> +       "datapaths": {
>> +         "type": {"key": {"type": "string"},
>
>
> I had a minor comment in V2 about using an enum here for key - 'system' or 'netdev'
> Does it work or is there worry that other datapath types will likely develop
> and we will need to update the enum ?

Thanks for the review.

I discussed this with Justin about this.  We currently do not limit
the datapath type in the Bridge table in ovsdb schema. So it might
just keep it as is to be consistent with the the Bridge table.


>> +                  "value": {"type": "uuid",
>> +                            "refTable": "Datapath"},
>> +                  "min": 0, "max": "unlimited"}},
>>
>>         "bridges": {
>>           "type": {"key": {"type": "uuid",
>>                            "refTable": "Bridge"},
>> @@ -629,6 +634,48 @@
>>                    "min": 0, "max": "unlimited"},
>>           "ephemeral": true}},
>>       "indexes": [["target"]]},
>> +   "Datapath": {
>> +     "columns": {
>> +       "datapath_version": {
>> +         "type": "string"},
>> +       "ct_zones": {
>> +         "type": {"key": {"type": "integer",
>> +                          "minInteger": 0,
>> +                          "maxInteger": 65535},
>> +                  "value": {"type": "uuid",
>> +                            "refTable": "CT_Zone"},
>> +                  "min": 0, "max": "unlimited"}},
>
>
> minor comment from V2; I think
> +                  "min": 0, "max": "unlimited"}},
> should be
> +                  "min": 0, "max": "65536"}},

Since ct_zones is a map, so  the maximum size is already limited the key range.
+         "type": {"key": {"type": "integer",
+                          "minInteger": 0,
+                          "maxInteger": 65535},

Keep "max" as "unlimited" also has the benefit that we do not need to
update it when the range of value is changed.  There are other cases
in ovsdb schema that has similar behavior, for  example:

       "queues": {
         "type": {"key": {"type": "integer",
                          "minInteger": 0,
                          "maxInteger": 4294967295},
                  "value": {"type": "uuid",
                            "refTable": "Queue"},
                  "min": 0, "max": "unlimited"}},

       "mappings": {
         "type": {"key": {"type": "integer",
                          "minInteger": 0,
                          "maxInteger": 16777215},
                  "value": {"type": "integer",
                          "minInteger": 0,
                          "maxInteger": 4095},
                  "min": 0, "max": "unlimited"}}}}}}


>> +       "external_ids": {
>> +         "type": {"key": "string", "value": "string",
>> +                  "min": 0, "max": "unlimited"}}}},
>> +   "CT_Zone": {
>> +     "columns": {
>> +       "timeout_policy": {
>> +         "type": {"key": {"type": "uuid",
>> +                          "refTable": "CT_Timeout_Policy"},
>> +                  "min": 0, "max": 1}},
>> +       "external_ids": {
>> +         "type": {"key": "string", "value": "string",
>> +                  "min": 0, "max": "unlimited"}}}},
>> +   "CT_Timeout_Policy": {
>> +     "columns": {
>> +       "timeouts": {
>> +         "type": {"key": {"type" : "string",
>> +                          "enum": ["set", ["tcp_syn_sent", "tcp_syn_recv",
>> +                                           "tcp_established", "tcp_fin_wait",
>> +                                           "tcp_close_wait", "tcp_last_ack",
>> +                                           "tcp_time_wait", "tcp_close",
>> +                                           "tcp_syn_sent2", "tcp_retransmit",
>> +                                           "tcp_unack", "udp_first",
>> +                                           "udp_single", "udp_multiple",
>> +                                           "icmp_first", "icmp_reply"]]},
>> +                  "value": {"type" : "integer",
>> +                            "minInteger" : 0,
>> +                            "maxInteger" : 4294967295},
>> +                  "min": 0, "max": "unlimited"}},
>
>
> Should it be ?
> +                  "min": 0, "max": "16"}},
>
> or will this create upgrade issues ?

Similarly, I would keep "max" as "unlimited" here, or we will need to
update "max" when more L4 protocol timeouts are supported.


>> +  <table name="Datapath">
>> +    <p>
>> +      Configuration for a datapath within <ref table="Open_vSwitch"/>.
>> +    </p>
>> +    <p>
>> +      A datapath is responsible for providing the packet handling in Open
>> +      vSwitch.  There are two primary datapath implementations used by
>> +      Open vSwitch: kernel and userspace.  Kernel datapath
>> +      implementations are available for Linux and Hyper-V, and selected
>> +      as <code>system</code> in the <ref column="datapath_type"/> column
>> +      of the <ref table="Bridge"/> table.  The userspace datapath is used
>> +      by DPDK and AF-XDP, and is selected as <code>netdev</code> in the
>> +      <ref column="datapath_type"/> column of the <ref table="Bridge"/>
>> +      table.
>> +    </p>
>> +    <p>
>> +      A datapath of a particular type is shared by all the bridges that use
>> +      that datapath.  Thus, configurations applied to this table affect
>> +      all bridges that use this datapath.
>> +    </p>
>> +
>> +    <column name="datapath_version">
>> +      <p>
>> +        Reports the version number of the Open vSwitch datapath in use.
>> +        This allows management software to detect and report discrepancies
>> +        between Open vSwitch userspace and datapath versions.  (The <ref
>> +        column="ovs_version" table="Open_vSwitch"/> column in the <ref
>> +        table="Open_vSwitch"/> reports the Open vSwitch userspace version.)
>> +        The version reported depends on the datapath in use:
>
>
> What is the recommended usage here for the userspace datapath.
> Should we just say to refer to  'ovs_version' in table="Open_vSwitch"/>  for
> the userspace datapath case or set the same value here ?

I think we will set the same value here for userspace datapath, so
that the management software can use the same logic to detect and
report discrepancies between vswitchd and datapath versions.

Thanks,

-YI-Hung
Darrell Ball Aug. 14, 2019, 8:25 p.m. UTC | #3
On Wed, Aug 14, 2019 at 9:47 AM Yi-Hung Wei <yihung.wei@gmail.com> wrote:

> On Mon, Aug 12, 2019 at 7:46 PM Darrell Ball <dlu998@gmail.com> wrote:
> >> diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
> >> index f7c6eb8983cd..c0a2242ad345 100644
> >> --- a/vswitchd/vswitch.ovsschema
> >> +++ b/vswitchd/vswitch.ovsschema
> >> @@ -1,9 +1,14 @@
> >>  {"name": "Open_vSwitch",
> >> - "version": "8.0.0",
> >> - "cksum": "3962141869 23978",
> >> + "version": "8.1.0",
> >> + "cksum": "1635647160 26090",
> >>   "tables": {
> >>     "Open_vSwitch": {
> >>       "columns": {
> >> +       "datapaths": {
> >> +         "type": {"key": {"type": "string"},
> >
> >
> > I had a minor comment in V2 about using an enum here for key - 'system'
> or 'netdev'
> > Does it work or is there worry that other datapath types will likely
> develop
> > and we will need to update the enum ?
>
> Thanks for the review.
>
> I discussed this with Justin about this.  We currently do not limit
> the datapath type in the Bridge table in ovsdb schema. So it might
> just keep it as is to be consistent with the the Bridge table.
>

Lets defer for now.
It can be treated as a separate issue and fixed later in one shot.


>
> >> +                  "value": {"type": "uuid",
> >> +                            "refTable": "Datapath"},
> >> +                  "min": 0, "max": "unlimited"}},
> >>
> >>         "bridges": {
> >>           "type": {"key": {"type": "uuid",
> >>                            "refTable": "Bridge"},
> >> @@ -629,6 +634,48 @@
> >>                    "min": 0, "max": "unlimited"},
> >>           "ephemeral": true}},
> >>       "indexes": [["target"]]},
> >> +   "Datapath": {
> >> +     "columns": {
> >> +       "datapath_version": {
> >> +         "type": "string"},
> >> +       "ct_zones": {
> >> +         "type": {"key": {"type": "integer",
> >> +                          "minInteger": 0,
> >> +                          "maxInteger": 65535},
> >> +                  "value": {"type": "uuid",
> >> +                            "refTable": "CT_Zone"},
> >> +                  "min": 0, "max": "unlimited"}},
> >
> >
> > minor comment from V2; I think
> > +                  "min": 0, "max": "unlimited"}},
> > should be
> > +                  "min": 0, "max": "65536"}},
>
> Since ct_zones is a map, so  the maximum size is already limited the key
> range.
> +         "type": {"key": {"type": "integer",
> +                          "minInteger": 0,
> +                          "maxInteger": 65535},
>
> Keep "max" as "unlimited" also has the benefit that we do not need to
> update it when the range of value is changed.  There are other cases
> in ovsdb schema that has similar behavior, for  example:


>        "queues": {
>          "type": {"key": {"type": "integer",
>                           "minInteger": 0,
>                           "maxInteger": 4294967295},
>                   "value": {"type": "uuid",
>                             "refTable": "Queue"},
>                   "min": 0, "max": "unlimited"}},
>
>        "mappings": {
>          "type": {"key": {"type": "integer",
>                           "minInteger": 0,
>                           "maxInteger": 16777215},
>                   "value": {"type": "integer",
>                           "minInteger": 0,
>                           "maxInteger": 4095},
>                   "min": 0, "max": "unlimited"}}}}}}
>

lets defer


>
> >> +       "external_ids": {
> >> +         "type": {"key": "string", "value": "string",
> >> +                  "min": 0, "max": "unlimited"}}}},
> >> +   "CT_Zone": {
> >> +     "columns": {
> >> +       "timeout_policy": {
> >> +         "type": {"key": {"type": "uuid",
> >> +                          "refTable": "CT_Timeout_Policy"},
> >> +                  "min": 0, "max": 1}},
> >> +       "external_ids": {
> >> +         "type": {"key": "string", "value": "string",
> >> +                  "min": 0, "max": "unlimited"}}}},
> >> +   "CT_Timeout_Policy": {
> >> +     "columns": {
> >> +       "timeouts": {
> >> +         "type": {"key": {"type" : "string",
> >> +                          "enum": ["set", ["tcp_syn_sent",
> "tcp_syn_recv",
> >> +                                           "tcp_established",
> "tcp_fin_wait",
> >> +                                           "tcp_close_wait",
> "tcp_last_ack",
> >> +                                           "tcp_time_wait",
> "tcp_close",
> >> +                                           "tcp_syn_sent2",
> "tcp_retransmit",
> >> +                                           "tcp_unack", "udp_first",
> >> +                                           "udp_single",
> "udp_multiple",
> >> +                                           "icmp_first",
> "icmp_reply"]]},
> >> +                  "value": {"type" : "integer",
> >> +                            "minInteger" : 0,
> >> +                            "maxInteger" : 4294967295},
> >> +                  "min": 0, "max": "unlimited"}},
> >
> >
> > Should it be ?
> > +                  "min": 0, "max": "16"}},
> >
> > or will this create upgrade issues ?
>
> Similarly, I would keep "max" as "unlimited" here, or we will need to
> update "max" when more L4 protocol timeouts are supported.
>

If we ever supported L4 port number timers and we also used this table,
we would need to update the enum, anyways and also the ovs-vsctl command.

We already have protection in place thru. ovs-vsctl limiting the total
number of parameters
to 18, which includes 16 timeouts, so my suggestion is already in place if
the user is using
ovs-vsctl, which is likely.

{"add-zone-tp", 2, 18, "", pre_get_zone, cmd_add_zone_tp, NULL,
     "--may-exist", RW},


>
>
> >> +  <table name="Datapath">
> >> +    <p>
> >> +      Configuration for a datapath within <ref table="Open_vSwitch"/>.
> >> +    </p>
> >> +    <p>
> >> +      A datapath is responsible for providing the packet handling in
> Open
> >> +      vSwitch.  There are two primary datapath implementations used by
> >> +      Open vSwitch: kernel and userspace.  Kernel datapath
> >> +      implementations are available for Linux and Hyper-V, and selected
> >> +      as <code>system</code> in the <ref column="datapath_type"/>
> column
> >> +      of the <ref table="Bridge"/> table.  The userspace datapath is
> used
> >> +      by DPDK and AF-XDP, and is selected as <code>netdev</code> in the
> >> +      <ref column="datapath_type"/> column of the <ref table="Bridge"/>
> >> +      table.
> >> +    </p>
> >> +    <p>
> >> +      A datapath of a particular type is shared by all the bridges
> that use
> >> +      that datapath.  Thus, configurations applied to this table affect
> >> +      all bridges that use this datapath.
> >> +    </p>
> >> +
> >> +    <column name="datapath_version">
> >> +      <p>
> >> +        Reports the version number of the Open vSwitch datapath in use.
> >> +        This allows management software to detect and report
> discrepancies
> >> +        between Open vSwitch userspace and datapath versions.  (The
> <ref
> >> +        column="ovs_version" table="Open_vSwitch"/> column in the <ref
> >> +        table="Open_vSwitch"/> reports the Open vSwitch userspace
> version.)
> >> +        The version reported depends on the datapath in use:
> >
> >
> > What is the recommended usage here for the userspace datapath.
> > Should we just say to refer to  'ovs_version' in table="Open_vSwitch"/>
> for
> > the userspace datapath case or set the same value here ?
>
> I think we will set the same value here for userspace datapath, so
> that the management software can use the same logic to detect and
> report discrepancies between vswitchd and datapath versions.
>

As discussed, lets go with Option 1 as mentioned earler, which involves
just adding a comment to guide users.


>
> Thanks,
>
> -YI-Hung
>

Patch
diff mbox series

diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
index f7c6eb8983cd..c0a2242ad345 100644
--- a/vswitchd/vswitch.ovsschema
+++ b/vswitchd/vswitch.ovsschema
@@ -1,9 +1,14 @@ 
 {"name": "Open_vSwitch",
- "version": "8.0.0",
- "cksum": "3962141869 23978",
+ "version": "8.1.0",
+ "cksum": "1635647160 26090",
  "tables": {
    "Open_vSwitch": {
      "columns": {
+       "datapaths": {
+         "type": {"key": {"type": "string"},
+                  "value": {"type": "uuid",
+                            "refTable": "Datapath"},
+                  "min": 0, "max": "unlimited"}},
        "bridges": {
          "type": {"key": {"type": "uuid",
                           "refTable": "Bridge"},
@@ -629,6 +634,48 @@ 
                   "min": 0, "max": "unlimited"},
          "ephemeral": true}},
      "indexes": [["target"]]},
+   "Datapath": {
+     "columns": {
+       "datapath_version": {
+         "type": "string"},
+       "ct_zones": {
+         "type": {"key": {"type": "integer",
+                          "minInteger": 0,
+                          "maxInteger": 65535},
+                  "value": {"type": "uuid",
+                            "refTable": "CT_Zone"},
+                  "min": 0, "max": "unlimited"}},
+       "external_ids": {
+         "type": {"key": "string", "value": "string",
+                  "min": 0, "max": "unlimited"}}}},
+   "CT_Zone": {
+     "columns": {
+       "timeout_policy": {
+         "type": {"key": {"type": "uuid",
+                          "refTable": "CT_Timeout_Policy"},
+                  "min": 0, "max": 1}},
+       "external_ids": {
+         "type": {"key": "string", "value": "string",
+                  "min": 0, "max": "unlimited"}}}},
+   "CT_Timeout_Policy": {
+     "columns": {
+       "timeouts": {
+         "type": {"key": {"type" : "string",
+                          "enum": ["set", ["tcp_syn_sent", "tcp_syn_recv",
+                                           "tcp_established", "tcp_fin_wait",
+                                           "tcp_close_wait", "tcp_last_ack",
+                                           "tcp_time_wait", "tcp_close",
+                                           "tcp_syn_sent2", "tcp_retransmit",
+                                           "tcp_unack", "udp_first",
+                                           "udp_single", "udp_multiple",
+                                           "icmp_first", "icmp_reply"]]},
+                  "value": {"type" : "integer",
+                            "minInteger" : 0,
+                            "maxInteger" : 4294967295},
+                  "min": 0, "max": "unlimited"}},
+       "external_ids": {
+         "type": {"key": "string", "value": "string",
+                  "min": 0, "max": "unlimited"}}}},
    "SSL": {
      "columns": {
        "private_key": {
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 027aee2f523b..495f0acad842 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -52,6 +52,13 @@ 
     one record in the <ref table="Open_vSwitch"/> table.
 
     <group title="Configuration">
+      <column name="datapaths">
+        Map of datapath types to datapaths.  The
+        <ref column="datapath_type"/> column of the <ref table="Bridge"/>
+        table is used as a key for this map.  The value points to a row in
+        the <ref table="Datapath"/> table.
+      </column>
+
       <column name="bridges">
         Set of bridges managed by the daemon.
       </column>
@@ -1192,53 +1199,11 @@ 
       </column>
 
       <column name="datapath_version">
-        <p>
-          Reports the version number of the Open vSwitch datapath in use.
-          This allows management software to detect and report discrepancies
-          between Open vSwitch userspace and datapath versions.  (The <ref
-          column="ovs_version" table="Open_vSwitch"/> column in the <ref
-          table="Open_vSwitch"/> reports the Open vSwitch userspace version.)
-          The version reported depends on the datapath in use:
-        </p>
-
-        <ul>
-          <li>
-            When the kernel module included in the Open vSwitch source tree is
-            used, this column reports the Open vSwitch version from which the
-            module was taken.
-          </li>
-
-          <li>
-            When the kernel module that is part of the upstream Linux kernel is
-            used, this column reports <code>&lt;unknown&gt;</code>.
-          </li>
-
-          <li>
-            When the datapath is built into the <code>ovs-vswitchd</code>
-            binary, this column reports <code>&lt;built-in&gt;</code>.  A
-            built-in datapath is by definition the same version as the rest of
-            the Open VSwitch userspace.
-          </li>
-
-          <li>
-            Other datapaths (such as the Hyper-V kernel datapath) currently
-            report <code>&lt;unknown&gt;</code>.
-          </li>
-        </ul>
-
-        <p>
-          A version discrepancy between <code>ovs-vswitchd</code> and the
-          datapath in use is not normally cause for alarm.  The Open vSwitch
-          kernel datapaths for Linux and Hyper-V, in particular, are designed
-          for maximum inter-version compatibility: any userspace version works
-          with with any kernel version.  Some reasons do exist to insist on
-          particular user/kernel pairings.  First, newer kernel versions add
-          new features, that can only be used by new-enough userspace, e.g.
-          VXLAN tunneling requires certain minimal userspace and kernel
-          versions.  Second, as an extension to the first reason, some newer
-          kernel versions add new features for enhancing performance that only
-          new-enough userspace versions can take advantage of.
-        </p>
+          Reports the datapath version.  This column is maintained for
+          backwards compatibility.  The preferred locatation is the
+          <ref column="datapath_id" table="Datapath"/> column of the
+          <ref table="Datapath"/> table.  The full documentation for this
+          column is there.
       </column>
 
       <column name="other_config" key="datapath-id">
@@ -5560,6 +5525,222 @@  ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \
     </group>
   </table>
 
+  <table name="Datapath">
+    <p>
+      Configuration for a datapath within <ref table="Open_vSwitch"/>.
+    </p>
+    <p>
+      A datapath is responsible for providing the packet handling in Open
+      vSwitch.  There are two primary datapath implementations used by
+      Open vSwitch: kernel and userspace.  Kernel datapath
+      implementations are available for Linux and Hyper-V, and selected
+      as <code>system</code> in the <ref column="datapath_type"/> column
+      of the <ref table="Bridge"/> table.  The userspace datapath is used
+      by DPDK and AF-XDP, and is selected as <code>netdev</code> in the
+      <ref column="datapath_type"/> column of the <ref table="Bridge"/>
+      table.
+    </p>
+    <p>
+      A datapath of a particular type is shared by all the bridges that use
+      that datapath.  Thus, configurations applied to this table affect
+      all bridges that use this datapath.
+    </p>
+
+    <column name="datapath_version">
+      <p>
+        Reports the version number of the Open vSwitch datapath in use.
+        This allows management software to detect and report discrepancies
+        between Open vSwitch userspace and datapath versions.  (The <ref
+        column="ovs_version" table="Open_vSwitch"/> column in the <ref
+        table="Open_vSwitch"/> reports the Open vSwitch userspace version.)
+        The version reported depends on the datapath in use:
+      </p>
+
+      <ul>
+        <li>
+          When the kernel module included in the Open vSwitch source tree is
+          used, this column reports the Open vSwitch version from which the
+          module was taken.
+        </li>
+
+        <li>
+          When the kernel module that is part of the upstream Linux kernel is
+          used, this column reports <code>&lt;unknown&gt;</code>.
+        </li>
+
+        <li>
+          When the datapath is built into the <code>ovs-vswitchd</code>
+          binary, this column reports <code>&lt;built-in&gt;</code>.  A
+          built-in datapath is by definition the same version as the rest of
+          the Open VSwitch userspace.
+        </li>
+
+        <li>
+          Other datapaths (such as the Hyper-V kernel datapath) currently
+          report <code>&lt;unknown&gt;</code>.
+        </li>
+      </ul>
+
+      <p>
+        A version discrepancy between <code>ovs-vswitchd</code> and the
+        datapath in use is not normally cause for alarm.  The Open vSwitch
+        kernel datapaths for Linux and Hyper-V, in particular, are designed
+        for maximum inter-version compatibility: any userspace version works
+        with with any kernel version.  Some reasons do exist to insist on
+        particular user/kernel pairings.  First, newer kernel versions add
+        new features, that can only be used by new-enough userspace, e.g.
+        VXLAN tunneling requires certain minimal userspace and kernel
+        versions.  Second, as an extension to the first reason, some newer
+        kernel versions add new features for enhancing performance that only
+        new-enough userspace versions can take advantage of.
+      </p>
+    </column>
+
+    <column name="ct_zones">
+      Configuration for connection tracking zones.  Each pair maps from a
+      zone id to a configuration for that zone.  Zone <code>0</code> applies
+      to the default zone (ie, the one used if a zone is not specified in
+      connection tracking-related OpenFlow matches and actions).
+    </column>
+
+    <group title="Common Columns">
+      The overall purpose of these columns is described under <code>Common
+      Columns</code> at the beginning of this document.
+
+      <column name="external_ids"/>
+    </group>
+  </table>
+
+  <table name="CT_Zone">
+    Connection tracking zone configuration
+
+    <column name="timeout_policy">
+      Connection tracking timeout policy for this zone. If a timeout policy
+      is not specified, it defaults to the timeout policy in the system.
+    </column>
+
+    <group title="Common Columns">
+      The overall purpose of these columns is described under <code>Common
+      Columns</code> at the beginning of this document.
+
+      <column name="external_ids"/>
+    </group>
+  </table>
+
+  <table name="CT_Timeout_Policy">
+    Connection tracking timeout policy configuration
+
+    <group title="Timeouts">
+      <column name="timeouts">
+        The <code>timeouts</code> column contains key-value pairs used
+        to configure connection tracking timeouts in a datapath.
+        Key-value pairs that are not supported by a datapath are
+        ignored.  The timeout value is in seconds.
+      </column>
+
+      <group title="TCP Timeouts">
+        <column name="timeouts" key="tcp_syn_sent">
+          The timeout for the connection after the first TCP SYN packet has
+          been seen by conntrack.
+        </column>
+
+        <column name="timeouts" key="tcp_syn_recv">
+          The timeout of the connection after the first TCP SYN-ACK packet
+          has been seen by conntrack.
+        </column>
+
+        <column name="timeouts" key="tcp_established">
+          The timeout of the connection after the connection has been fully
+          established.
+        </column>
+
+        <column name="timeouts" key="tcp_fin_wait">
+          The timeout of the connection after the first TCP FIN packet
+          has been seen by conntrack.
+        </column>
+
+        <column name="timeouts" key="tcp_close_wait">
+          The timeout of the connection after the first TCP ACK packet
+          has been seen after it receives TCP FIN packet.  This timeout
+          is only supported by the Linux kernel datapath.
+        </column>
+
+        <column name="timeouts" key="tcp_last_ack">
+          The timeout of the connection after TCP FIN packets have been
+          seen by conntrack from both directions.  This timeout is only
+          supported by the Linux kernel datapath.
+        </column>
+
+        <column name="timeouts" key="tcp_time_wait">
+          The timeout of the connection after conntrack has seen the
+          TCP ACK packet for the second TCP FIN packet.
+        </column>
+
+        <column name="timeouts" key="tcp_close">
+          The timeout of the connection after the first TCP RST packet
+          has been seen by conntrack.
+        </column>
+
+        <column name="timeouts" key="tcp_syn_sent2">
+          The timeout of the connection when only a TCP SYN packet has been
+          seen by conntrack from both directions (simultaneous open).
+          This timeout is only supported by the Linux kernel datapath.
+        </column>
+
+        <column name="timeouts" key="tcp_retransmit">
+          The timeout of the connection when it exceeds the maximum
+          number of retransmissions.  This timeout is only supported by
+          the Linux kernel datapath.
+        </column>
+
+        <column name="timeouts" key="tcp_unack">
+          The timeout of the connection when non-SYN packets create an
+          established connection in TCP loose tracking mode.  This timeout
+          is only supported by the Linux kernel datapath.
+        </column>
+      </group>
+
+      <group title="UDP Timeouts">
+        <column name="timeouts" key="udp_first">
+          The timeout of the connection after the first UDP packet has
+          been seen by conntrack.  This timeout is only supported by the
+          userspace datapath.
+        </column>
+
+        <column name="timeouts" key="udp_single">
+          The timeout of the connection when conntrack only seen UDP
+          packet from the source host, but the destination host has never
+          sent one back.
+        </column>
+
+        <column name="timeouts" key="udp_multiple">
+          The timeout of the connection when UDP packets have been seen in
+          both directions.
+        </column>
+      </group>
+
+      <group title="ICMP Timeouts">
+        <column name="timeouts" key="icmp_first">
+          The timeout of the connection after the first ICMP packet has
+          been seen by conntrack.
+        </column>
+
+        <column name="timeouts" key="icmp_reply">
+          The timeout of the connection after an ICMP error is replied in
+          response to an ICMP packet.  This timeout is only supported by
+          the userspace datapath.
+        </column>
+      </group>
+    </group>
+
+    <group title="Common Columns">
+      The overall purpose of these columns is described under <code>Common
+      Columns</code> at the beginning of this document.
+
+      <column name="external_ids"/>
+    </group>
+  </table>
+
   <table name="SSL">
     SSL configuration for an Open_vSwitch.