| Message ID | 20171009173315.6682-1-blp@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] faq: Better document how to add vendor extensions. | expand |
On Mon, Oct 09, 2017 at 10:33:15AM -0700, Ben Pfaff wrote:
> Signed-off-by: Ben Pfaff <blp@ovn.org>
I'd appreciate a review for this change. It should not be difficult
since it is entirely a documentation update.
On Mon, Oct 9, 2017 at 10:33 AM, Ben Pfaff <blp@ovn.org> wrote: > Signed-off-by: Ben Pfaff <blp@ovn.org> > --- > Documentation/faq/contributing.rst | 44 +++++++++++++++++++++++++++----------- > include/openvswitch/ofp-errors.h | 4 +++- > 2 files changed, 34 insertions(+), 14 deletions(-) > > diff --git a/Documentation/faq/contributing.rst b/Documentation/faq/contributing.rst > index 6dfc8bc4d436..d59376cd615c 100644 > --- a/Documentation/faq/contributing.rst > +++ b/Documentation/faq/contributing.rst > @@ -33,22 +33,28 @@ Q: How do I implement a new OpenFlow message? > as needed. (If you configure with ``--enable-Werror``, as described in > :doc:`/intro/install/general`, then it is impossible to miss any warnings.) > > - If you need to add an OpenFlow vendor extension message for a vendor that > - doesn't yet have any extension messages, then you will also need to edit > - ``build-aux/extract-ofp-msgs``. > + To add an OpenFlow vendor extension message (aka experimenter message) for > + a vendor that doesn't yet have any extension messages, you will also need > + to edit ``build-aux/extract-ofp-msgs`` and at least ``ofphdrs_decode()`` > + and ``ofpraw_put__()`` in ``lib/ofp-msgs.c``. OpenFlow doesn't standardize > + vendor extensions very well, so it's hard to make the process simpler than > + that. (If you have a choice of how to design your vendor extension > + messages, it will be easier if you make them resemble the ONF and OVS > + extension messages.) > > Q: How do I add support for a new field or header? > > A: Add new members for your field to ``struct flow`` in ``lib/flow.h``, and > add new enumerations for your new field to ``enum mf_field_id`` in > - ``lib/meta-flow.h``, following the existing pattern. Also, add support to Instead of ``lib/meta-flow.h``, maybe ``include/openvswitch/meta-flow.h``? Otherwise, looks good to me. Acked-by: Yi-Hung Wei <yihung.wei@gmail.com>
On Thu, Oct 26, 2017 at 03:08:00PM -0700, Yi-Hung Wei wrote: > On Mon, Oct 9, 2017 at 10:33 AM, Ben Pfaff <blp@ovn.org> wrote: > > Signed-off-by: Ben Pfaff <blp@ovn.org> > > --- > > Documentation/faq/contributing.rst | 44 +++++++++++++++++++++++++++----------- > > include/openvswitch/ofp-errors.h | 4 +++- > > 2 files changed, 34 insertions(+), 14 deletions(-) > > > > diff --git a/Documentation/faq/contributing.rst b/Documentation/faq/contributing.rst > > index 6dfc8bc4d436..d59376cd615c 100644 > > --- a/Documentation/faq/contributing.rst > > +++ b/Documentation/faq/contributing.rst > > @@ -33,22 +33,28 @@ Q: How do I implement a new OpenFlow message? > > as needed. (If you configure with ``--enable-Werror``, as described in > > :doc:`/intro/install/general`, then it is impossible to miss any warnings.) > > > > - If you need to add an OpenFlow vendor extension message for a vendor that > > - doesn't yet have any extension messages, then you will also need to edit > > - ``build-aux/extract-ofp-msgs``. > > + To add an OpenFlow vendor extension message (aka experimenter message) for > > + a vendor that doesn't yet have any extension messages, you will also need > > + to edit ``build-aux/extract-ofp-msgs`` and at least ``ofphdrs_decode()`` > > + and ``ofpraw_put__()`` in ``lib/ofp-msgs.c``. OpenFlow doesn't standardize > > + vendor extensions very well, so it's hard to make the process simpler than > > + that. (If you have a choice of how to design your vendor extension > > + messages, it will be easier if you make them resemble the ONF and OVS > > + extension messages.) > > > > Q: How do I add support for a new field or header? > > > > A: Add new members for your field to ``struct flow`` in ``lib/flow.h``, and > > add new enumerations for your new field to ``enum mf_field_id`` in > > - ``lib/meta-flow.h``, following the existing pattern. Also, add support to > Instead of ``lib/meta-flow.h``, maybe > ``include/openvswitch/meta-flow.h``? Otherwise, looks good to me. > > Acked-by: Yi-Hung Wei <yihung.wei@gmail.com> Thanks, I made that fix and applied this to master.
diff --git a/Documentation/faq/contributing.rst b/Documentation/faq/contributing.rst index 6dfc8bc4d436..d59376cd615c 100644 --- a/Documentation/faq/contributing.rst +++ b/Documentation/faq/contributing.rst @@ -33,22 +33,28 @@ Q: How do I implement a new OpenFlow message? as needed. (If you configure with ``--enable-Werror``, as described in :doc:`/intro/install/general`, then it is impossible to miss any warnings.) - If you need to add an OpenFlow vendor extension message for a vendor that - doesn't yet have any extension messages, then you will also need to edit - ``build-aux/extract-ofp-msgs``. + To add an OpenFlow vendor extension message (aka experimenter message) for + a vendor that doesn't yet have any extension messages, you will also need + to edit ``build-aux/extract-ofp-msgs`` and at least ``ofphdrs_decode()`` + and ``ofpraw_put__()`` in ``lib/ofp-msgs.c``. OpenFlow doesn't standardize + vendor extensions very well, so it's hard to make the process simpler than + that. (If you have a choice of how to design your vendor extension + messages, it will be easier if you make them resemble the ONF and OVS + extension messages.) Q: How do I add support for a new field or header? A: Add new members for your field to ``struct flow`` in ``lib/flow.h``, and add new enumerations for your new field to ``enum mf_field_id`` in - ``lib/meta-flow.h``, following the existing pattern. Also, add support to - ``miniflow_extract()`` in ``lib/flow.c`` for extracting your new field from - a packet into struct miniflow, and to ``nx_put_raw()`` in - ``lib/nx-match.c`` to output your new field in OXM matches. Then recompile - and fix all of the new warnings, implementing new functionality for the new - field or header as needed. (If you configure with ``--enable-Werror``, as - described in :doc:`/intro/install/general`, then it is impossible to miss - any warnings.) + ``lib/meta-flow.h``, following the existing pattern. If the field uses a + new OXM class, add it to OXM_CLASSES in ``build-aux/extract-ofp-fields``. + Also, add support to ``miniflow_extract()`` in ``lib/flow.c`` for + extracting your new field from a packet into struct miniflow, and to + ``nx_put_raw()`` in ``lib/nx-match.c`` to output your new field in OXM + matches. Then recompile and fix all of the new warnings, implementing new + functionality for the new field or header as needed. (If you configure + with ``--enable-Werror``, as described in :doc:`/intro/install/general`, + then it is impossible to miss any warnings.) If you want kernel datapath support for your new field, you also need to modify the kernel module for the operating systems you are interested in. @@ -71,5 +77,17 @@ Q: How do I add support for a new OpenFlow action? warnings.) If you need to add an OpenFlow vendor extension action for a vendor that - doesn't yet have any extension actions, then you will also need to edit - ``build-aux/extract-ofp-actions``. + doesn't yet have any extension actions, then you will also need to add the + vendor to ``vendor_map`` in ``build-aux/extract-ofp-actions``. Also, you + will need to add support for the vendor to ``ofpact_decode_raw()`` and + ``ofpact_put_raw()`` in ``lib/ofp-actions.c``. (If you have a choice of + how to design your vendor extension actions, it will be easier if you make + them resemble the ONF and OVS extension actions.) + +Q: How do I add support for a new OpenFlow error message? + + A: Add your new error to ``enum ofperr`` in + ``include/openvswitch/ofp-errors.h``. Read the large comment at the top of + the file for details. If you need to add an OpenFlow vendor extension + error for a vendor that doesn't yet have any, first add the vendor ID to + the ``<name>_VENDOR_ID`` list in ``include/openflow/openflow-common.h``. diff --git a/include/openvswitch/ofp-errors.h b/include/openvswitch/ofp-errors.h index 3d09be354af9..5e20e1adb7cd 100644 --- a/include/openvswitch/ofp-errors.h +++ b/include/openvswitch/ofp-errors.h @@ -60,7 +60,9 @@ struct ofpbuf; * type, and sometimes a code for each protocol that supports the error: * * # The vendor is OF for standard OpenFlow error codes. Otherwise it - * is one of the *_VENDOR_ID codes defined in openflow-common.h. + * is one of the *_VENDOR_ID codes defined in openflow-common.h. (To + * add support for a new vendor, add a VENDOR_ID code to that + * header.) * * # The version can specify a specific OpenFlow version, a version * range delimited by "-", or an open-ended range with "+".
Signed-off-by: Ben Pfaff <blp@ovn.org> --- Documentation/faq/contributing.rst | 44 +++++++++++++++++++++++++++----------- include/openvswitch/ofp-errors.h | 4 +++- 2 files changed, 34 insertions(+), 14 deletions(-)