On Wed, Aug 14, 2019 at 9:47 AM Yi-Hung Wei <yihung....@gmail.com> wrote:
> On Mon, Aug 12, 2019 at 7:46 PM Darrell Ball <dlu...@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
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
