On 8/30/17, 12:19 AM, "Finn Christensen" <[email protected]> wrote:

    Hi,
    
    Just wanted to add one comment to this correspondence. See below.
    
    
    
    Thanks,
    
    Finn Christensen
    
    
    
    -----Original Message-----
    
    From: [email protected] 
[mailto:[email protected]] On Behalf Of Yuanhan Liu
    
    Sent: 30. august 2017 04:25
    
    To: Darrell Ball <[email protected]>
    
    Cc: [email protected]
    
    Subject: Re: [ovs-dev] [PATCH 0/7 RFC] OVS-DPDK flow offload with rte_flow
    
    
    
    On Wed, Aug 30, 2017 at 01:32:10AM +0000, Darrell Ball wrote:
    
    >
    
    >     >         Though that being said, this patchset still has issues 
unresolved. The
    
    >     >         major issue is that maybe most NIC (for instance, Mellanox 
and Intel)
    
    >     >         can not support a pure MARK action. It has to be used 
together with a
    
    >     >         QUEUE action, which in turn needs a queue index. That comes 
to the issue:
    
    >     >         the queue index is not given in the flow context. To make 
it work, patch
    
    >     >         5 just set the queue index to 0, which is obviously wrong. 
One possible
    
    >     >         solution is to record the rxq and pass it down to the flow 
creation
    
    >     >         stage. It would be much better, but it's still far away 
from being perfect.
    
    >     >         Because it might have changed the steering rules 
stealthily, which may
    
    >     >         break the default RSS setup by OVS-DPDK.
    
    >     >
    
    >     > If this cannot be solved by removing this restriction, I guess 
another alternative is to actively
    
    >     > manage flow-queue associations.
    
    >
    
    >     do you mean let user provide the set_queue action?
    
    >
    
    > I mean in the worst case, if the restriction cannot be lifted, we
    
    > might to need to do flow distribution across queues with additional
    
    > added logic/tracking,
    
    
    
    Like the way mentioned above: record the rxq at recv stage and pass it down 
to flow_put?
    
    
    
    > because we would not really want to keep this restriction without a
    
    > workaround.
    
    
    
    For sure.
    
    
    
    > This would be a fair bit of work, however.
    
    
    
    Indeed. Another reason I don't really like above solution is it changes the 
core logic of OVS (just for OVS-DPDK use case). What's worse, it doesn't even 
solve the issue perfectly: it's still a workaround!
    
    
    
    That being said, if we have to provide a workaround, I think maybe a simple 
round-robin queue distribution is better, judging it's our first attempt to 
have hw offload? Later, we could either improve the workaround, or remove the 
restriction from DPDK side (if it's possible). Good to you?
    
    
    
    [Finn]
    
    I think we should not further intermix the rxqs distributed to different 
pmd's, other than initially configured, when setting up hw-offload. If we make 
a round-robin distribution of the rxqs, a different pmd will most likely 
receive the hw-offloaded packets - not the same pmd that ran the slow-path 
originally creating the flow.
    
    It is usual to optimize caches etc. per pmd and that would not work then. 
Maybe the further processing of the hw-offloaded packets does not need these 
optimizations at the moment, however, IMHO I think we would be better off using 
the first proposal above (use the same rxq as the one creating the flow).

[Darrell] Several ideas have some validity.
                 However, this sounds reasonable and simple and we could 
revisit as needed.
                 What do you think Yuanhan ?

    
    We will not be able to do RSS on a specific hw-offloaded flow anyway. That 
would require a larger rework of the RSS configuration in OVS, as Darell points 
out.
    
    
    
    
    
    > Alternatively, pushing this to the user in the general may be too much 
overhead; what do you think ?.
    
    > As a user specification, it could be optional, however ?
    
    
    
    Yes, that's what I was thinking. For the users that know their NIC supports 
MARK with QUEUE action, they could not add such option. For others, it's 
suggested to add it. Or, it's a must (if we do not do workaround like above). 
Makes sense?
    
    
    
    >     >
    
    >     >         The reason I still want to send it out is to get more 
comments/thoughts
    
    >     >         from community on this whole patchset. Meanwhile, I will 
try to resolve
    
    >     >         the QUEUE action issue.
    
    >     >
    
    >     >         Note that it's disabled by default, which can be enabled by:
    
    >     >
    
    >     >             $ ovs-vsctl set Open_vSwitch . 
other_config:hw-offload=true
    
    >     >
    
    >     > Maybe per in-port configuration would alleviate the issue to a 
certain degree.
    
    >
    
    >     Yes, it could be done. I choose it for following reasons:
    
    >
    
    >     - the option is already there, used by tc offloads.
    
    >     - it also simplifies the (first) patchset a bit, IMO.
    
    >
    
    > Of course, I understand.
    
    >
    
    >     However, I'm okay with making it per port. What's your suggestion for
    
    >     this? Making "hw-offload" be port, or introducing another one? If so,
    
    >     what's your suggestion on the naming?
    
    >
    
    > I am not suggesting to drop the global configuration Mainly we
    
    > ‘consider’ additional per interface configuration because of the
    
    > restriction with queue action we discuss. This reduces the scope of
    
    > the queue remapping from what RSS would yield with HWOL.
    
    > I would expect that when such configuration is done (if it were done),
    
    > that typically multiple ports would be configured, since traffic flows 
bi-directionally at least.
    
    >
    
    > If we were to do this, one of the possibilities would be something like:
    
    > ovs-vsctl set Interface dpdk0 other_config:hw-offload=true
    
    
    
    What's the scope between the global one and this one then? We simply ignore 
the globle one for OVS-DPDK?
    
    
    
    >     Thanks for the review. BTW, would you please add me in 'to' or 'cc'
    
    >     list while replying to me?  Otherwise, it's easy to get missed: too
    
    >     many emails :/
    
    >
    
    > of course
    
    
    
    Thank you!
    
    
    
    --yliu
    
    _______________________________________________
    
    dev mailing list
    
    [email protected]
    
    
https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwIGaQ&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=HdBRtuEEJHRlmUdaHqod0fVdcT_9ZlZVr04Fx7pL9y0&s=lwFJ5F0MHueaieQkaRRfn9ghhnBu2D_LMa6tOmmdEQI&e=
 
    
    Disclaimer: This email and any files transmitted with it may contain 
confidential information intended for the addressee(s) only. The information is 
not to be surrendered or copied to unauthorized persons. If you have received 
this communication in error, please notify the sender immediately and delete 
this e-mail from your system.
    
    

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

Reply via email to