Acked-by: Jarno Rajahalme <jrajaha...@nicira.com>
Two small nits and a comment error (?) spotted below,
Jarno
On Aug 7, 2014, at 4:13 PM, Ben Pfaff <b...@nicira.com> wrote:
> The ofproto implementation has had an abstraction layer on top of
> OFPTC11_TABLE_MISS for a while. This commit pushes that abstraction layer
> farther down, into ofp-util. This will be more useful in an upcoming
> commit.
>
> During the conversion I realized that the previous implementation was
> not entirely correct. In particular, the OpenFlow 1.3+ "table mod" was
> still being treated as if it had table miss configuration bits, even
> though it doesn't. This commit fixes that issue and updates the tests.
>
> OpenFlow 1.4 adds some more OFPTC_* flags that this new abstraction doesn't
> yet support, but OVS didn't support those flags any better before this
> commit, so abstracting those is left as future work.
>
> Signed-off-by: Ben Pfaff <b...@nicira.com>
> ---
> lib/ofp-parse.c | 9 +-
> lib/ofp-print.c | 408 ++++++++++--------------------
> lib/ofp-util.c | 589 +++++++++++++++++++++++++++++++++-----------
> lib/ofp-util.h | 67 +++--
> ofproto/connmgr.c | 13 +-
> ofproto/ofproto-dpif.c | 43 ++--
> ofproto/ofproto-provider.h | 72 ++----
> ofproto/ofproto.c | 216 ++++++++++------
> ofproto/ofproto.h | 20 +-
> tests/ofp-print.at | 107 +++++---
> tests/ofproto-dpif.at | 59 +++--
> tests/ofproto.at | 109 +++++---
> 12 files changed, 1011 insertions(+), 701 deletions(-)
>
(snip)
> +
> +static struct mf_bitmap
> +mf_bitmap_from_of10(ovs_be32 wc10)
> +{
> + struct mf_bitmap fields = MF_BITMAP_INITIALIZER;
> + const struct ofp10_wc_map *p;
> +
> + for (p = ofp10_wc_map; p < &ofp10_wc_map[ARRAY_SIZE(ofp10_wc_map)]; p++)
> {
> + if (wc10 & htonl(p->wc10)) {
Maybe ntohl(wc10) once outside the loop?
> + bitmap_set1(fields.bm, p->mf);
> + }
> + }
> + return fields;
> +}
> +
(snip)
> +static struct mf_bitmap
> +mf_bitmap_from_of11(ovs_be32 wc11)
> +{
> + struct mf_bitmap fields = MF_BITMAP_INITIALIZER;
> + const struct ofp11_wc_map *p;
> +
> + for (p = ofp11_wc_map; p < &ofp11_wc_map[ARRAY_SIZE(ofp11_wc_map)]; p++)
> {
> + if (wc11 & htonl(p->wc11)) {
Ditto.
> + bitmap_set1(fields.bm, p->mf);
> }
> }
> - return htonl(fmf11);
> + return fields;
> }
(snip)
> diff --git a/lib/ofp-util.h b/lib/ofp-util.h
> index 6c7559a..b9d2ae8 100644
> --- a/lib/ofp-util.h
> +++ b/lib/ofp-util.h
> @@ -578,10 +578,38 @@ enum ofperr ofputil_decode_port_mod(const struct
> ofp_header *,
> struct ofpbuf *ofputil_encode_port_mod(const struct ofputil_port_mod *,
> enum ofputil_protocol);
>
> +/* Abstract version of OFPTC11_TABLE_MISS_*.
> + *
> + * OpenFlow 1.0 always sends packets that miss to the controller.
> + *
> + * OpenFlow 1.1 and 1.2 can configure table miss behavior via a "table-mod"
> + * that specifies "send to controller", "miss", or "drop".
> + *
> + * OpenFlow 1.3 and later never sends packets that miss to the controller.
> + */
> +enum ofputil_table_miss {
> + /* Protocol-specific default behavior. On OpenFlow 1.0 through 1.2
> + * connections, the packet is sent to the controller, and on OpenFlow 1.3
> + * and later connections, the packet is dropped.
> + *
> + * This is also used as a result of decoding OpenFlow 1.3+ "config"
> values
> + * in table-mods, to indicate that no table-miss was specified. */
> + OFPUTIL_TABLE_MISS_DEFAULT, /* Protocol default behavior. */
> +
> + /* These constants have the same meanings as those in OpenFlow with the
> + * same names. */
> + OFPUTIL_TABLE_MISS_CONTROLLER, /* Send to controller. */
> + OFPUTIL_TABLE_MISS_CONTINUE, /* Go to next table, like OF1.0. */
The comment here does not seem to match the one above (“OpenFlow 1.0 always
sends packets that miss to the controller.”)
> + OFPUTIL_TABLE_MISS_DROP, /* Drop the packet. */
> +};
> +
> +ovs_be32 ofputil_table_miss_to_config(enum ofputil_table_miss,
> + enum ofp_version);
> +
(snip)
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
