On Mon, Jul 23, 2012 at 03:16:32PM +0900, Simon Horman wrote:
> In the case of Open Flow 1.2, which is currently the only
> time that OXM is be used, there is a 4 byte header before
> the match which needs to be taken into account when calculating
> the pad length. This complicates nx_match pull and put somewhat.
>
> This patch takes an approach suggested by Ben Pfaff to separate the
> encoding of the match and the adding of padding and, in the case of OXM,
> a header.
>
> Signed-off-by: Simon Horman <[email protected]>
>
> ---
>
> v7
> * Correct the parse-oxm sub-command of ovs-ofputil
> * Separate raw match encoding and header/pad encoding
> * Change subject from "nx-match: Take into account leading header when
> calculating pad"
>
> v6
> * No change
>
> v5
> * Move nx_padded_match_len into nx-match.h as it will
> be used by code outside of nx-match.c added by subsequent patches.
>
> v4
> * Pad is needed even if match is zero length
> (what was I thinking?!)
>
> v3
> * Initial post
>
> Conflicts:
> lib/nx-match.c
>
> wip
> ---
> lib/nx-match.c | 241
> ++++++++++++++++++++++++++++++++++++++++++++------
> lib/nx-match.h | 17 +++-
> lib/ofp-util.c | 30 +++----
> utilities/ovs-ofctl.c | 40 ++++++---
> 4 files changed, 272 insertions(+), 56 deletions(-)
>
> diff --git a/lib/nx-match.c b/lib/nx-match.c
> index 2498b9b..c49ebdb 100644
> --- a/lib/nx-match.c
> +++ b/lib/nx-match.c
> @@ -91,12 +91,11 @@ nx_entry_ok(const void *p, unsigned int match_len)
> }
>
> static enum ofperr
> -nx_pull_match__(struct ofpbuf *b, unsigned int match_len, bool strict,
> - uint16_t priority, struct cls_rule *rule,
> - ovs_be64 *cookie, ovs_be64 *cookie_mask)
> +nx_pull_raw(const uint8_t *p, unsigned int match_len, bool strict,
> + uint16_t priority, struct cls_rule *rule,
> + ovs_be64 *cookie, ovs_be64 *cookie_mask)
> {
> uint32_t header;
> - uint8_t *p;
>
> assert((cookie != NULL) == (cookie_mask != NULL));
>
> @@ -108,14 +107,6 @@ nx_pull_match__(struct ofpbuf *b, unsigned int
> match_len, bool strict,
> return 0;
> }
>
> - p = ofpbuf_try_pull(b, ROUND_UP(match_len, 8));
> - if (!p) {
> - VLOG_DBG_RL(&rl, "nx_match length %u, rounded up to a "
> - "multiple of 8, is longer than space in message (max "
> - "length %zu)", match_len, b->size);
> - return OFPERR_OFPBMC_BAD_LEN;
> - }
> -
> for (;
> (header = nx_entry_ok(p, match_len)) != 0;
> p += 4 + NXM_LENGTH(header), match_len -= 4 + NXM_LENGTH(header)) {
> @@ -198,6 +189,27 @@ nx_pull_match__(struct ofpbuf *b, unsigned int
> match_len, bool strict,
> return match_len ? OFPERR_OFPBMC_BAD_LEN : 0;
> }
>
> +static enum ofperr
> +nx_pull_match__(struct ofpbuf *b, unsigned int match_len, bool strict,
> + uint16_t priority, struct cls_rule *rule,
> + ovs_be64 *cookie, ovs_be64 *cookie_mask)
> +{
> + uint8_t *p = NULL;
> +
> + if (match_len) {
> + p = ofpbuf_try_pull(b, ROUND_UP(match_len, 8));
> + if (!p) {
> + VLOG_DBG_RL(&rl, "nx_match length %u, rounded up to a "
> + "multiple of 8, is longer than space in message (max
> "
> + "length %zu)", match_len, b->size);
> + return OFPERR_OFPBMC_BAD_LEN;
> + }
> + }
> +
> + return nx_pull_raw(p, match_len, strict, priority, rule,
> + cookie, cookie_mask);
> +}
> +
> /* Parses the nx_match formatted match description in 'b' with length
> * 'match_len'. The results are stored in 'rule', which is initialized with
> * 'priority'. If 'cookie' and 'cookie_mask' contain valid pointers, then
> the
> @@ -212,8 +224,8 @@ nx_pull_match(struct ofpbuf *b, unsigned int match_len,
> uint16_t priority, struct cls_rule *rule,
> ovs_be64 *cookie, ovs_be64 *cookie_mask)
> {
> - return nx_pull_match__(b, match_len, true, priority, rule, cookie,
> - cookie_mask);
> + return nx_pull_match__(b, match_len, true, priority, rule,
> + cookie, cookie_mask);
> }
>
> /* Behaves the same as nx_pull_match() with one exception. Skips over
> unknown
> @@ -223,8 +235,68 @@ nx_pull_match_loose(struct ofpbuf *b, unsigned int
> match_len,
> uint16_t priority, struct cls_rule *rule,
> ovs_be64 *cookie, ovs_be64 *cookie_mask)
> {
> - return nx_pull_match__(b, match_len, false, priority, rule, cookie,
> - cookie_mask);
> + return nx_pull_match__(b, match_len, false, priority, rule,
> + cookie, cookie_mask);
> +}
> +
> +static enum ofperr
> +oxm_pull_match__(struct ofpbuf *b, bool strict,
> + uint16_t priority, struct cls_rule *rule,
> + ovs_be64 *cookie, ovs_be64 *cookie_mask)
> +{
> + struct ofp11_match_header *omh = b->data;
> + uint8_t *p;
> + uint16_t match_len;
> +
> + if (b->size < sizeof *omh) {
> + return OFPERR_OFPBMC_BAD_LEN;
> + }
> +
> + match_len = ntohs(omh->length);
> + if (match_len < sizeof *omh) {
> + return OFPERR_OFPBMC_BAD_LEN;
> + }
> +
> + if (omh->type != htons(OFPMT_OXM)) {
> + return OFPERR_OFPBMC_BAD_TYPE;
> + }
> +
> + p = ofpbuf_try_pull(b, ROUND_UP(match_len, 8));
> + if (!p) {
> + VLOG_DBG_RL(&rl, "oxm length %u, rounded up to a "
> + "multiple of 8, is longer than space in message (max "
> + "length %zu)", match_len, b->size);
> + return OFPERR_OFPBMC_BAD_LEN;
> + }
> +
> + return nx_pull_raw(p + sizeof *omh, match_len - sizeof *omh,
> + strict, priority, rule, cookie, cookie_mask);
> +}
> +
> +/* Parses the oxm formatted match description preceeded by a struct
> + * ofp11_match in 'b' with length 'match_len'. The results are stored in
> + * 'rule', which is initialized with 'priority'. If 'cookie' and
> + * 'cookie_mask' contain valid pointers, then the cookie and mask will be
> + * stored in them if a "NXM_NX_COOKIE*" match is defined. Otherwise, 0 is
> + * stored in both.
> + *
> + * Fails with an error when encountering unknown OXM headers.
> + *
> + * Returns 0 if successful, otherwise an OpenFlow error code. */
> +enum ofperr
> +oxm_pull_match(struct ofpbuf *b, uint16_t priority, struct cls_rule *rule,
> + ovs_be64 *cookie, ovs_be64 *cookie_mask)
> +{
> + return oxm_pull_match__(b, true, priority, rule, cookie, cookie_mask);
> +}
> +
> +/* Behaves the same as oxm_pull_match() with one exception. Skips over
> unknown
> + * PXM headers instead of failing with an error when they are encountered. */
> +enum ofperr
> +oxm_pull_match_loose(struct ofpbuf *b, uint16_t priority, struct cls_rule
> *rule,
> + ovs_be64 *cookie, ovs_be64 *cookie_mask)
> +{
> + return oxm_pull_match__(b, false, priority, rule, cookie, cookie_mask);
> }
I wonder if the cookie and cookie_mack parameters should be removed
from oxm_pull_match(), oxm_pull_match_loose() and oxm_put_match() (below)
as cookie is an NXM match that is not present in OXM.
>
> /* nx_put_match() and helpers.
> @@ -472,10 +544,9 @@ nxm_put_ip(struct ofpbuf *b, const struct cls_rule *cr,
> }
>
> /* Appends to 'b' the nx_match format that expresses 'cr' (except for
> - * 'cr->priority', because priority is not part of nx_match), plus enough
> - * zero bytes to pad the nx_match out to a multiple of 8. For Flow Mod
> - * and Flow Stats Requests messages, a 'cookie' and 'cookie_mask' may be
> - * supplied. Otherwise, 'cookie_mask' should be zero.
> + * 'cr->priority', because priority is not part of nx_match).
> + * For Flow Mod * and Flow Stats Requests messages, a 'cookie' and
> + * 'cookie_mask' may be supplied. Otherwise, 'cookie_mask' should be zero.
> *
> * This function can cause 'b''s data to be reallocated.
> *
> @@ -483,9 +554,9 @@ nxm_put_ip(struct ofpbuf *b, const struct cls_rule *cr,
> *
> * If 'cr' is a catch-all rule that matches every packet, then this function
> * appends nothing to 'b' and returns 0. */
> -int
> -nx_put_match(struct ofpbuf *b, bool oxm, const struct cls_rule *cr,
> - ovs_be64 cookie, ovs_be64 cookie_mask)
> +static int
> +nx_put_raw(struct ofpbuf *b, bool oxm, const struct cls_rule *cr,
> + ovs_be64 cookie, ovs_be64 cookie_mask)
> {
> const flow_wildcards_t wc = cr->wc.wildcards;
> const struct flow *flow = &cr->flow;
> @@ -592,9 +663,65 @@ nx_put_match(struct ofpbuf *b, bool oxm, const struct
> cls_rule *cr,
> nxm_put_64m(b, NXM_NX_COOKIE, cookie, cookie_mask);
>
> match_len = b->size - start_len;
> + return match_len;
> +}
> +
> +
> +/* Appends to 'b' the nx_match format that expresses 'cr' (except for
> + * 'cr->priority', because priority is not part of nx_match), plus enough
> + * zero bytes to pad the nx_match out to a multiple of 8. For Flow Mod
> + * and Flow Stats Requests messages, a 'cookie' and 'cookie_mask' may be
> + * supplied. Otherwise, 'cookie_mask' should be zero.
> + *
> + * This function can cause 'b''s data to be reallocated.
> + *
> + * Returns the number of bytes appended to 'b', excluding padding.
> + *
> + * If 'cr' is a catch-all rule that matches every packet, then this function
> + * appends nothing to 'b' and returns 0. */
> +int
> +nx_put_match(struct ofpbuf *b, const struct cls_rule *cr,
> + ovs_be64 cookie, ovs_be64 cookie_mask)
> +{
> + int match_len = nx_put_raw(b, false, cr, cookie, cookie_mask);
> +
> + ofpbuf_put_zeros(b, ROUND_UP(match_len, 8) - match_len);
> + return match_len;
> +}
> +
> +
> +/* Appends to 'b' an struct ofp11_match_header followed by the oxm format
> that
> + * expresses 'cr' (except for 'cr->priority', because priority is not part
> + * of nx_match), plus enough zero bytes to pad the data appended out to a
> + * multiple of 8. For Flow Mod and Flow Stats Requests messages, a 'cookie'
> + * and 'cookie_mask' may be supplied. Otherwise, 'cookie_mask' should be
> zero.
> + *
> + * This function can cause 'b''s data to be reallocated.
> + *
> + * Returns the number of bytes appended to 'b', excluding the padding.
> + *
> + * If 'cr' is a catch-all rule that matches every packet, then the function
> + * appends a struct ofp11_match_header and padding to 'b' and returns
> + * the length of struct ofp11_match_header. */
> +int
> +oxm_put_match(struct ofpbuf *b, const struct cls_rule *cr,
> + ovs_be64 cookie, ovs_be64 cookie_mask)
> +{
> + int match_len;
> + struct ofp11_match_header *omh;
> + size_t start_len = b->size;
> +
> + ofpbuf_put_uninit(b, sizeof *omh);
> + match_len = nx_put_raw(b, true, cr, cookie, cookie_mask) + sizeof *omh;
> ofpbuf_put_zeros(b, ROUND_UP(match_len, 8) - match_len);
> +
> + omh = (struct ofp11_match_header *)((char *)b->data + start_len);
> + omh->type = htons(OFPMT_OXM);
> + omh->length = htons(match_len);
> +
> return match_len;
> }
> +
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
