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

[Darrell Ball]

On 9/15/17, 2:23 AM, "Yuanhan Liu" <yliu at fridaylinux.org> wrote:

    On Thu, Sep 14, 2017 at 11:42:38PM +0000, Darrell Ball wrote:
    > 
    > 
    > On 9/5/17, 2:23 AM, "Yuanhan Liu" <yliu at fridaylinux.org> 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 */
    > 
    > 
    > [Darrell] Since the PMD that receives a flow can change and multiple PMDs can allocate
    > the same mark value at the moment, it seems the mark allocation needs to include the PMD ID or
    > be from a global namespace to enforce uniqueness.
    > Pls verify.
    
    Yes, you are right. Let it be global then. 

[Darrell] One downside of global vs including PMD ID (maybe 10 bits) is 
more contention b/w pmds.

Talking about that, one of my
    colleague actually suggests to use an array to do the mapping. It's a
    "number -> flow" mapping after all: using a hash map looks like an overkill.
    Using array also would give us the best performance.
    
    The reason I chose cmap in the beginning is we don't know how big the
    array is. Also, it doesn't make sense to allocate a static array big
    enough to hold the maximum entires (it's uint32_t after all).
    
    So I'm now thinking:
    
    - allocate a small array first, say with 1024 entries
    - if there is no space, double the max (say, 2048) and re-allocate
    
    I don't know the performance difference yet. I will get back to you
    when I have the number. Besides that, what do you think.

[Darrell] Seems fine.
cmap has rcu build-in, but that can be replicated, if you find it is makes a significant difference.
    
    	--yliu