Hi Mark,

Few comments below.


On Sat, Sep 7, 2019 at 2:38 AM Mark Michelson <mmich...@redhat.com> wrote:

> This document serves to provide an explanation for how OVN will remain
> compatible with OVS. It provides instructions for OVN contributors for
> how to maintain compatibility even across older versions of OVS when
> possible.
>
> Note that the document currently makes reference to some non-existent
> items. For instance, it refers to an ovs-compat directory in the OVN
>

I think you need to update the commit message as "ovs-compat" directory
approach is removed in RFC v2.


> project. Also, it refers to some macros for testing the OVS version.
> These will be added in a future commit if the process documented here is
> approved.
>
> I'm submitting this as an RFC, since I imagine there are some details I
> have not thought about, and there will also be some great suggestions
> from others on this matter.
>
> Signed-off-by: Mark Michelson <mmich...@redhat.com>
> ---
>  Documentation/automake.mk                          |   1 +
>  Documentation/internals/contributing/index.rst     |   1 +
>  .../contributing/ovs-ovn-compatibility.rst         | 129
> +++++++++++++++++++++
>  3 files changed, 131 insertions(+)
>  create mode 100644
> Documentation/internals/contributing/ovs-ovn-compatibility.rst
>
> diff --git a/Documentation/automake.mk b/Documentation/automake.mk
> index f7e1d2628..d9bd4be1f 100644
> --- a/Documentation/automake.mk
> +++ b/Documentation/automake.mk
> @@ -56,6 +56,7 @@ DOC_SOURCE = \
>         Documentation/internals/contributing/documentation-style.rst \
>         Documentation/internals/contributing/libopenvswitch-abi.rst \
>         Documentation/internals/contributing/submitting-patches.rst \
> +       Documentation/internals/contributing/ovs-ovn-compatibility.rst \
>         Documentation/requirements.txt \
>         $(addprefix Documentation/ref/,$(RST_MANPAGES)
> $(RST_MANPAGES_NOINST))
>  FLAKE8_PYFILES += Documentation/conf.py
> diff --git a/Documentation/internals/contributing/index.rst
> b/Documentation/internals/contributing/index.rst
> index a46cb046a..7ef57a1e2 100644
> --- a/Documentation/internals/contributing/index.rst
> +++ b/Documentation/internals/contributing/index.rst
> @@ -36,3 +36,4 @@ The below guides provide information on contributing to
> Open vSwitch itself.
>     coding-style-windows
>     documentation-style
>     libopenvswitch-abi
> +   ovs-ovn-compatibility
> diff --git
> a/Documentation/internals/contributing/ovs-ovn-compatibility.rst
> b/Documentation/internals/contributing/ovs-ovn-compatibility.rst
> new file mode 100644
> index 000000000..d40b58c32
> --- /dev/null
> +++ b/Documentation/internals/contributing/ovs-ovn-compatibility.rst
> @@ -0,0 +1,129 @@
> +..
> +      Copyright (c) 2019 Nicira, Inc.
> +
> +      Licensed under the Apache License, Version 2.0 (the "License"); you
> may
> +      not use this file except in compliance with the License. You may
> obtain
> +      a copy of the License at
> +
> +          http://www.apache.org/licenses/LICENSE-2.0
> +
> +      Unless required by applicable law or agreed to in writing, software
> +      distributed under the License is distributed on an "AS IS" BASIS,
> WITHOUT
> +      WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> See the
> +      License for the specific language governing permissions and
> limitations
> +      under the License.
> +
> +      Convention for heading levels in Open vSwitch documentation:
> +
> +      =======  Heading 0 (reserved for the title in a document)
> +      -------  Heading 1
> +      ~~~~~~~  Heading 2
> +      +++++++  Heading 3
> +      '''''''  Heading 4
> +
> +      Avoid deeper levels because they do not render well.
> +
> +===============================
> +Keeping OVN Compatible with OVS
> +===============================
> +
> +OVN has split from OVS. Prior to this split, there was no issue with
> +compatibility. All code changes happened at the same time and in the same
> repo.
> +New releases contained the latest OVS and OVN changes. But now these
> assumptions
> +are no longer valid, and so care must be taken to maintain compatibility.
> +
> +OVS and OVN versions
> +--------------------
> +
> +Prior to the split, the release schedule for OVN was bound to the release
> +schedule for OVS. OVS releases a new version approximately every 6
> months. OVN
> +has not yet determined a release schedule, but it is entirely possible
> that it
> +will be different from OVS. Eventually, this will lead to a situation
> where it
> +is very important that we publish which versions of OVN are compatible
> with
> +which versions of OVS. When incompatibilities are discovered, it is
> important to
> +ensure that these are clearly stated.
> +
> +The split of OVS and OVN happened in the run-up to the release of OVS
> 2.12. As a
> +result, all versions of OVN *must* be compiled against OVS version 2.12 or
> +later. Before going further into compatibility, let's explore the ways
> that OVN
> +and OVS can become incompatible.
> +
> +Compile-time Incompatibility
> +----------------------------
> +
> +The first way that the projects can become incompatible is if the C code
> for OVN
> +no longer can compile.
> +
> +The most likely case for this would be that an OVN change requires a
> parallel
> +change to OVS. Those keeping up to date with OVN but not OVS will find
> that OVN
> +will no longer compile since it refers to a nonexistent function or out
> of date
> +function in OVS.
> +
> +Most OVN users will consume OVN via package from their distribution of
> choice.
> +OVN consumes libopenvswitch statically, so even if the version of OVS
> installed
> +on a user's machine is incompatible at compile time, it will not matter.
> +
> +OVN developers are the only ones that would be inconvenienced by a
> compile-time
> +incompatibility. OVN developers will be expected to regularly update the
> version
> +of OVS they are using. If an OVN developer notices that OVN is not
> compiling,
> +then they should update their OVS code to the latest and try again.
> +
> +Developers who are making changes to both OVS and OVN at the same time
> *must*
> +contribute the OVS change first and ensure it is merged upstream before
> +submitting the OVN change. This way, OVN should never be in a state where
> it
> +will not compile.
> +
> +When compiling older releases of OVN, it should be able to compile
> against newer
> +versions of OVS due to API and ABI guarantees in OVS's libaries.
> +
> +Runtime Incompatibility
> +-----------------------
> +
> +The next way that the projects may become incompatible is at runtime. The
> most
> +common way this would happen is if new OpenFlow capabilities are added to
> OVS as
> +part of an OVN change. In this case, if someone updates OVN but does not
> also
> +updage OVS, then OVN will not be able to install the OpenFlow rules it
> wishes
> +to.
> +
> +Unlike with compile-time incompatibilities, we can't wallpaper over the
> fact
> +that the OVS installation is not up to date. The best we can do is make
> it very
> +clear at runtime that a certain feature is not present, and if the
> feature is
> +desired, OVS must be upgraded.
> +
> +The following is the process that OVN developers should use when making a
> +runtime compatibility change to OVS and OVN.
> +
> +1. Submit the change to OVS first. See the change through until it is
> merged.
> +2. Make the necessary changes to OVN.
> +
> +  a. At startup, probe OVS for the existence of the OpenFlow addition. If
> it
> +     is not present, then output an informational message that explains
> which
> +     OVN feature(s) cannot be used.
>

This could be tricky. I could not figure out a way at the startup to probe
and get a
list of supported actions and matches when I worked on using the
check_pkt_larger
action. One way could be to try to program the flow and detect if it was
successful.

It would be good if we can get some answers from Ben and other OVS
developers
if it is possible to do what is described here.


> +  b. If a user attempts to explicitly configure the feature that is not
> usable
> +     due to the incompatibility, then output a warning message.
> +  c. Ensure that the code that installs the OpenFlow will only do so if
> the new
> +     feature is present.
> +
> +Compatibility Statement
> +-----------------------
> +
> +Given the above, the OVN team will try its hardest to maintain any
> released
> +version of OVN with any released version of OVS after version 2.12.
> Versions of
> +OVS prior to 2.12 are not guaranteed to run properly since OVN does not
> have
> +appropriate OpenFlow feature probes in place.
> +
> +It may seem prudent to only guarantee compatibility with certain releases
> of
> +OVS (e.g. the current and previous versions of OVS). However, dropping
> +compatibility would involve actively removing code that ensures runtime
> safety.
> +It seems unwise to do so.
> +
> +This, however, is a "best effort" policy. The OVN project reserves the
> right to
> +withdraw compatibility support with a previous OVS version, for reasons
> such as:
> +
> +- Security risks.
> +- Earthshatteringly large changes in OVS (e.g. no longer using OpenFlow
> or the
> +  OVSDB).
> +- Difficulty in safely maintaining compatibility across versions.
> +
> +In the event that compatibility for a certain version or versions of OVS
> is
> +dropped, the OVN project will clearly document it.
>

Overall it looks good to me.

Thanks
Numan


> --
> 2.14.5
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to