Re: [ovs-dev] [PATCH v3 1/9] ovs-vswitchd: Add Datapath, CT_Zone, and CT_Zone_Policy tables.
On Wed, Aug 14, 2019 at 9:47 AM Yi-Hung Wei wrote:
> On Mon, Aug 12, 2019 at 7:46 PM Darrell Ball 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 ?
> > +
Re: [ovs-dev] [PATCH v3 1/9] ovs-vswitchd: Add Datapath, CT_Zone, and CT_Zone_Policy tables.
On Mon, Aug 12, 2019 at 7:46 PM Darrell Ball 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.
>> +
>> +
>> + Configuration for a datapath within .
>> +
>> +
>> + A datapath is responsible for providing the packet handling in Open
>> + vSwitch. There ar
Re: [ovs-dev] [PATCH v3 1/9] ovs-vswitchd: Add Datapath, CT_Zone, and CT_Zone_Policy tables.
Thanks for the patch
On Mon, Aug 12, 2019 at 5:52 PM Yi-Hung Wei wrote:
> From: Justin Pettit
>
> Signed-off-by: Justin Pettit
> Signed-off-by: Yi-Hung Wei
> Co-authored-by: Yi-Hung Wei
> ---
> 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 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
