[ovs-dev] [PATCH 1/5] lib/flow: Introduce miniflow_extract().

Joe Stringer joestringer at nicira.com
Wed May 7 03:49:57 UTC 2014


Hi Jarno,

Quick question..

On 18 April 2014 13:36, Jarno Rajahalme <jrajahalme at nicira.com> wrote:

> miniflow_extract() extracts packet headers directly to a miniflow,
> which is a compressed form of the struct flow.  This does not require
> a large struct to be cleared to begin with, and accesses less memory.
> These performance benefits should allow this to be used in the DPDK
> datapath.
>
> miniflow_extract() takes a miniflow as an input/output parameter.  On
> input the buffer for values to be extracted must be properly
> initialized.  On output the map contains ones for all the fields that
> have been extracted.
>
> Some struct flow fields are reordered to make miniflow_extract to
> progress in the logical order.
>
> Some explicit "inline" keywords are necessary for GCC to optimize this
> properly.  Also, macros are used for same reason instead of inline
> functions for pushing data to the miniflow.
>
> Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
> ---
>

...


> @@ -358,115 +344,283 @@ void
>  flow_extract(struct ofpbuf *packet, const struct pkt_metadata *md,
>               struct flow *flow)
>  {
> -    struct ofpbuf b = *packet;
> -    struct eth_header *eth;
> +    uint32_t buf[FLOW_U32S];
> +    struct miniflow mf;
>
>      COVERAGE_INC(flow_extract);
>
> -    memset(flow, 0, sizeof *flow);
> +    miniflow_initialize(&mf, buf);
> +    miniflow_extract(packet, md, &mf);
> +    miniflow_expand(&mf, flow);
> +}
>
> +/* Caller is responsible for initializing 'dst->values' with enough
> storage
> + * (FLOW_U64S * 8 bytes is enough). */
> +void
> +miniflow_extract(struct ofpbuf *packet, const struct pkt_metadata *md,
> +                 struct miniflow *dst)
> +{
> +    void *data = ofpbuf_data(packet);
> +    size_t size = ofpbuf_size(packet);
> +    char *l2;
> +    struct mf_ctx mf = { 0, dst->values, dst->values + FLOW_U32S };
> +    ovs_be16 dl_type;
> +    uint8_t nw_frag, nw_tos, nw_ttl, nw_proto;
> +
> +    /* Metadata. */
>      if (md) {
> -        flow->tunnel = md->tunnel;
> -        flow->in_port = md->in_port;
> -        flow->skb_priority = md->skb_priority;
> -        flow->pkt_mark = md->pkt_mark;
> -        flow->recirc_id = md->recirc_id;
> -        flow->dp_hash = md->dp_hash;
> +        if (md->tunnel.ip_dst) {
> +            miniflow_push_words(mf, tunnel, &md->tunnel,
> +                                sizeof md->tunnel / 4);
> +        }
> +        miniflow_push_uint32_check(mf, skb_priority, md->skb_priority);
> +        miniflow_push_uint32_check(mf, pkt_mark, md->pkt_mark);
> +        miniflow_push_uint32_check(mf, recirc_id, md->recirc_id);
> +        miniflow_push_uint32(mf, in_port,
> odp_to_u32(md->in_port.odp_port));
>      }


...

         }
>      }
> +    if (md) {
> +        miniflow_push_uint32_check(mf, dp_hash, md->dp_hash);
> +    }
> + out:
> +    dst->map = mf.map;
>  }
>

Was there a particular reason for shifting the dp_hash part to the end of
the function here, rather than having it grouped with the other metadata
fields?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20140507/29927574/attachment-0005.html>


More information about the dev mailing list