[ovs-dev] [PATCH v2 1/8] dpif-netdev: associate flow with a mark id

[Simon Horman]

On Tue, Sep 05, 2017 at 05:22:54PM +0800, Yuanhan Liu wrote:
> This patch associate a flow with a mark id (a uint32_t number) by CMAP.
> 
> It re-uses the flow API (more precisely, the ->flow_put method) to setup
> the hw flow. The flow_put implementation then is supposed to create a
> flow with MARK action.
> 
> Co-authored-by: Finn Christensen <fc at napatech.com>
> Signed-off-by: Yuanhan Liu <yliu at fridaylinux.org>
> Signed-off-by: Finn Christensen <fc at napatech.com>
> ---
>  lib/dpif-netdev.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/netdev.h      |  6 ++++
>  2 files changed, 91 insertions(+)
> 
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 071ec14..f3b7f25 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -446,6 +446,12 @@ struct dp_netdev_flow {
>      const unsigned pmd_id;       /* The 'core_id' of pmd thread owning this */
>                                   /* flow. */
>  
> +    const struct cmap_node mark_node;   /* In owning dp_netdev_pmd_thread's */
> +                                        /* 'mark_to_flow' */
> +    bool has_mark;               /* A flag to tell whether this flow has a
> +                                    valid mark asscoiated with it. */
> +    uint32_t mark;               /* Unique flow mark assiged to a flow */
> +

I think the fields above could be placed so that the resulting structure
doesn't have so many holes due to alignment. In particular 'mark'
could be placed next to the existing 'dead' field.

...

> @@ -2235,6 +2249,38 @@ dp_netdev_pmd_find_flow(const struct dp_netdev_pmd_thread *pmd,
>      return NULL;
>  }
>  
> +static struct dp_netdev_flow *
> +dp_netdev_pmd_find_flow_by_mark(const struct dp_netdev_pmd_thread *pmd,
> +                                const uint32_t mark)
> +{
> +    struct dp_netdev_flow *netdev_flow;
> +
> +    CMAP_FOR_EACH_WITH_HASH (netdev_flow, mark_node, mark,
> +                             &pmd->mark_to_flow) {
> +        if (netdev_flow->has_mark && netdev_flow->mark == mark) {
> +            return netdev_flow;
> +        }
> +    }
> +
> +    return NULL;
> +}
> +
> +static bool
> +dp_netdev_alloc_flow_mark(const struct dp_netdev_pmd_thread *pmd,
> +                          uint32_t *mark)
> +{
> +    uint32_t i;
> +
> +    for (i = 0; i < UINT32_MAX; i++) {
> +        if (!dp_netdev_pmd_find_flow_by_mark(pmd, i)) {
> +            *mark = i;
> +            return true;
> +        }
> +    }
> +
> +    return false;
> +}
> +

Was consideration given to using id_pool_alloc_id() here?

...