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

Yi-Hung Wei yihung.wei at gmail.com
Wed Aug 14 16:47:25 UTC 2019


On Mon, Aug 12, 2019 at 7:46 PM Darrell Ball <dlu998 at 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


More information about the dev mailing list