On 9/15/17, 1:34 AM, "Yuanhan Liu" <[email protected]> wrote:

    On Tue, Sep 12, 2017 at 08:36:19AM +0000, Darrell Ball wrote:
    > 
    > On 9/10/17, 11:14 PM, "[email protected] on behalf of 
Yuanhan Liu" <[email protected] on behalf of 
[email protected]> wrote:
    > 
    >     On Fri, Sep 08, 2017 at 06:48:50PM +0200, Simon Horman wrote:
    >     > On Tue, Sep 05, 2017 at 05:22:59PM +0800, Yuanhan Liu wrote:
    >     > > From: Finn Christensen <[email protected]>
    >     > > 
    >     > > AFAIK, most (if not all) NICs (including Mellanox and Intel) do 
not
    >     > > support a pure MARK action.  It's required to be used together 
with
    >     > > some other actions, like QUEUE.
    >     > > 
    >     > > To workaround it, retry with a queue action when first try failed.
    >     > > 
    >     > > Moreover, some Intel's NIC (say XL710) needs the QUEUE action set
    >     > > before the MARK action.
    >     > > 
    >     > > Co-authored-by: Yuanhan Liu <[email protected]>
    >     > > Signed-off-by: Finn Christensen <[email protected]>
    >     > > Signed-off-by: Yuanhan Liu <[email protected]>
    >     > 
    >     > This feels a bit like the tail wagging the dog.
    >     > Is this the lowest level at which it makes sense to implement
    >     > this logic?
    >     > 
    >     > If so then I wonder if some sort of probing would be in order
    >     > to avoid the cost of trying to add the flow twice to hardware
    >     > where the queue is required.
    >     
    >     Do you mean something like rte_flow capability query, like whether
    >     a queue action is needed for a mark action? If so, yes, I do think
    >     we miss an interface like this.
    >     
    >     Note that even in this solution, the flow won't be created twice
    >     to the hardware, because the first try would be failed.
    > 
    > [Darrell]
    > 
    >               Having an api to quey capability and avoid the first try to 
HW would be nice, but there are dependencies
    >                on RTE, drivers etc and I don’t know definitive the api 
would be.
    > 
    >              Also, as nics are added this capability needs to be done and 
state needs to be kept in all cases.
    > 
    >            It is an enhancement and if done should be reliable.
    > 
    >            A separate comment is we need to document which nics need the 
queue action.
    > 
    >          Also, I think we should check errno in the present code.
    
    
    That was my first reaction to remove such blind re-try. It could have
    been a good option if all the PMD driver report the error consistenly.
    
    And unfortunately, it's not true. For example, for MARK without QUEUE
    action, i40e reports RTE_FLOW_ERROR_TYPE_ACTION, which, IMO, is nothing
    wrong. While for mlx5, it reports RTE_FLOW_ERROR_TYPE_HANDLE. I don't
    know why it was set like this, and we may could fix this. But my point
    was, it's not that reliable to use rte_errno, at least it's true for now.

[Darrell] I see what you mean looking at the drivers code and I imagine it 
makes you work difficult.
                It looks like only either RTE_FLOW_ERROR_TYPE_ACTION or 
RTE_FLOW_ERROR_TYPE_HANDLE
                is used though in these cases from what I saw; would it make 
sense to check for those specifically?

    
        --yliu
    

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to