Re: [ovs-dev] [PATCH v2 1/9] ovs-vswitchd: Add Datapath, CT_Zone, and CT_Zone_Policy tables.
One comment fix:
s/ "min": 0, "max": "65535"}},/ "min": 0, "max": "65536"}},/
On Mon, Aug 5, 2019 at 4:09 PM Darrell Ball wrote:
> Thanks for the patch
>
> I avoided duplicate comments from what Justin suggested
>
> comments inline
>
> On Thu, Aug 1, 2019 at 3:08 PM Yi-Hung Wei wrote:
>
>> From: Justin Pettit
>>
>> From: Justin Pettit
>>
>> Signed-off-by: Justin Pettit
>> ---
>> vswitchd/vswitch.ovsschema | 43 +++-
>> vswitchd/vswitch.xml | 252
>> -
>> 2 files changed, 246 insertions(+), 49 deletions(-)
>>
>> diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
>> index f7c6eb8983cd..d215f4edfefa 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": "1566974404 25483",
>> "tables": {
>> "Open_vSwitch": {
>> "columns": {
>> + "datapaths": {
>> + "type": {"key": {"type": "string"},
>>
>
> Should 'type' be an enum
> something like:
>
> "type": {"key": {"type": "string",
> "enum": ["set", ["system", "netdev"]]}},
>
> The schema can still be upgraded by adding new datapath types should more
> ever arise.
>
>
>
>> + "value": {"type": "uuid",
>> +"refTable": "Datapath"},
>> + "min": 0, "max": "unlimited"}},
>>
>
> accordingly:
>
> "min": 0, "max": "2"}},
>
>
>
>> "bridges": {
>> "type": {"key": {"type": "uuid",
>>"refTable": "Bridge"},
>> @@ -629,6 +634,40 @@
>>"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"}},
>>
>
>
> How about ?
>
> "min": 0, "max": "65535"}},
>
s/ "min": 0, "max": "65535"}},/ "min": 0, "max": "65536"}},/
>
> I don't think we can have multiple entries for the same zone and if we
> did, we don't
> handle it.
>
>
>
>> + "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": "string",
>> + "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..a0706c9c0fc1 100644
>> --- a/vswitchd/vswitch.xml
>> +++ b/vswitchd/vswitch.xml
>> @@ -52,6 +52,13 @@
>> one record in the table.
>>
>>
>> +
>> +Map of datapath types to datapaths. The
>> + column of the
>> +table is used as a key for this map. The value points to a row
>> in
>> +the table.
>> +
>> +
>>
>> Set of bridges managed by the daemon.
>>
>> @@ -1192,53 +1199,11 @@
>>
>>
>>
>> -
>> - 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
>> > - column="ovs_version" table="Open_vSwitch"/> column in the > - table="Open_vSwitch"/> reports the Open vSwitch userspace
>> version.)
>> - The version reported depends on the datapath in use:
>> -
>> -
>> -
>> -
>> -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.
>> -
>> -
>> -
>> -When the kernel module that is part of the upstream Linux
>> kernel is
>> -used, this column reports
Re: [ovs-dev] [PATCH v2 1/9] ovs-vswitchd: Add Datapath, CT_Zone, and CT_Zone_Policy tables.
Thanks for the patch
I avoided duplicate comments from what Justin suggested
comments inline
On Thu, Aug 1, 2019 at 3:08 PM Yi-Hung Wei wrote:
> From: Justin Pettit
>
> From: Justin Pettit
>
> Signed-off-by: Justin Pettit
> ---
> vswitchd/vswitch.ovsschema | 43 +++-
> vswitchd/vswitch.xml | 252
> -
> 2 files changed, 246 insertions(+), 49 deletions(-)
>
> diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
> index f7c6eb8983cd..d215f4edfefa 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": "1566974404 25483",
> "tables": {
> "Open_vSwitch": {
> "columns": {
> + "datapaths": {
> + "type": {"key": {"type": "string"},
>
Should 'type' be an enum
something like:
"type": {"key": {"type": "string",
"enum": ["set", ["system", "netdev"]]}},
The schema can still be upgraded by adding new datapath types should more
ever arise.
> + "value": {"type": "uuid",
> +"refTable": "Datapath"},
> + "min": 0, "max": "unlimited"}},
>
accordingly:
"min": 0, "max": "2"}},
> "bridges": {
> "type": {"key": {"type": "uuid",
>"refTable": "Bridge"},
> @@ -629,6 +634,40 @@
>"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"}},
>
How about ?
"min": 0, "max": "65535"}},
I don't think we can have multiple entries for the same zone and if we did,
we don't
handle it.
> + "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": "string",
> + "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..a0706c9c0fc1 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -52,6 +52,13 @@
> one record in the table.
>
>
> +
> +Map of datapath types to datapaths. The
> + column of the
> +table is used as a key for this map. The value points to a row in
> +the table.
> +
> +
>
> Set of bridges managed by the daemon.
>
> @@ -1192,53 +1199,11 @@
>
>
>
> -
> - 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 - column="ovs_version" table="Open_vSwitch"/> column in the - table="Open_vSwitch"/> reports the Open vSwitch userspace
> version.)
> - The version reported depends on the datapath in use:
> -
> -
> -
> -
> -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.
> -
> -
> -
> -When the kernel module that is part of the upstream Linux
> kernel is
> -used, this column reports unknown.
> -
> -
> -
> -When the datapath is built into the ovs-vswitchd
> -binary, this column reports built-in. A
> -built-in datapath is by definition the same version as the
> rest of
> -the Open VSwitch userspace.
> -
> -
> -
> -Other datapaths (such as the Hyper-V kernel datapath)
>
Re: [ovs-dev] [PATCH v2 1/9] ovs-vswitchd: Add Datapath, CT_Zone, and CT_Zone_Policy tables.
On Fri, Aug 2, 2019 at 11:15 AM Justin Pettit wrote: > > > > On Aug 1, 2019, at 3:07 PM, Yi-Hung Wei wrote: > > > > From: Justin Pettit > > > > From: Justin Pettit > > Can you drop one of these "From:" statements? Otherwise it appears in the > commit message. > > As we discussed off-line, can you apply the following diff, which we worked > on together along with your co-authored-by tag? > Thanks for review. I will add the diff into v3. Thanks, -Yi-Hung ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 1/9] ovs-vswitchd: Add Datapath, CT_Zone, and CT_Zone_Policy tables.
> On Aug 1, 2019, at 3:07 PM, Yi-Hung Wei wrote: > > From: Justin Pettit > > From: Justin Pettit Can you drop one of these "From:" statements? Otherwise it appears in the commit message. As we discussed off-line, can you apply the following diff, which we worked on together along with your co-authored-by tag? -=-=-=-=-=-=-=-=-=-=-=- diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml index a0706c9c0fc1..495f0acad842 100644 --- a/vswitchd/vswitch.xml +++ b/vswitchd/vswitch.xml @@ -5615,8 +5615,8 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \ Connection tracking zone configuration - Connection tracking timeout policy for this zone. If timeout policy is - not specified, defaults to the timeout policy in the system. + Connection tracking timeout policy for this zone. If a timeout policy + is not specified, it defaults to the timeout policy in the system. @@ -5632,80 +5632,103 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \ - The timeouts 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 timeouts 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. - TCP SYN sent timeout. + The timeout for the connection after the first TCP SYN packet has + been seen by conntrack. - TCP SYN receive timeout. + The timeout of the connection after the first TCP SYN-ACK packet + has been seen by conntrack. - TCP established timeout. + The timeout of the connection after the connection has been fully + established. - TCP FIN wait timeout. + The timeout of the connection after the first TCP FIN packet + has been seen by conntrack. - TCP close wait timeout. + 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. - TCP last ACK timeout. + 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. - TCP time wait timeout. + The timeout of the connection after conntrack has seen the + TCP ACK packet for the second TCP FIN packet. - TCP close timeout. + The timeout of the connection after the first TCP RST packet + has been seen by conntrack. - TCP syn sent2 timeout. + 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. - TCP retransmit timeout. + The timeout of the connection when it exceeds the maximum + number of retransmissions. This timeout is only supported by + the Linux kernel datapath. - TCP unacknowledgment timeout. + 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. - First UDP packet timeout. + The timeout of the connection after the first UDP packet has + been seen by conntrack. This timeout is only supported by the + userspace datapath. - The timeout in the state that source host sends more than one packet - but the destination host has never sent one backs. + The timeout of the connection when conntrack only seen UDP + packet from the source host, but the destination host has never + sent one back. - UDP packets seen in both directions timeout. + The timeout of the connection when UDP packets have been seen in + both directions. - First ICMP timeout. + The timeout of the connection after the first ICMP packet has + been seen by conntrack. - ICMP reply timeout. + The timeout of the connection after an ICMP error is replied in + response to an ICMP packet. This timeout is only supported by
