On 9/15/17, 1:18 AM, "Yuanhan Liu" <[email protected]> wrote:
On Wed, Sep 13, 2017 at 04:17:37PM +0000, Darrell Ball wrote:
>
>
> On 9/13/17, 2:57 AM, "Simon Horman" <[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.
>
> Agreed. Though I was more thinking of probing the hardware rather than
> having a capability API - I expect this would remove several of the
> dependencies you describe above.
>
>
> [Darrell] I have been pondering the probing option as well. It is
certainly a valid option;
After thinking twice, I do think the probing might be better than capability
feedback in some cases. For example, it makes more sense to probe the "mark
and queue" actions than asking DPDK to provide such ability. It's more like
a combination, which is hard to give from DPDK side.
[Darrell] Agreed, for this case, probing does seem better vs capability query in
the comparison of these 2 options.
But for some cases, like do the underlaying NIC support a (few) specifics
actions, or a (few) protocols, it's more clean and easier to provide cap
feedback API from DPDK, IMO.
[Darrell] For sure.
> we use it in other cases such as datapath probing. One of the aspects
that worries me here is
> maintaining the correct per interface (essentially; although the
attribute is per nic) state
> across various events such as new ports being added, vswitchd restarts,
races with flow
> creation. It would be non-trivial I guess and probably appropriate for
the next patch series, if done.
I would think so. I will think about it after this patchset, to see which
one
suits better for our case. Or maybe, we could have both: I do think it makes
sense to let DPDK to provide some basic capability feedbacks, for example,
the supported actions, protocols, etc.
[Darrell] Sounds good. If/when we have access to a capability query, we would
use it in these cases.
> In this case, we have what seems like a clear distinction b/w Napatech
which does not need the
> queue action workaround and everything else, which does.
> Besides the non-Napatech behavior, which is worrisome, maintaining the
difference for flow handling
> under the covers is concerning.
>
> I wonder if we should be upfront as possible here and just have a dpdk
interface configuration – maybe
> something like “supports native HWOL mark action” since the better
behavior is the exception?
> The interface config would be more robust than probing.
> This would need documentation, of course.
The thing is that the option is fixed, while the NIC driver may change.
Such option may apply to old versions may not apply to newer versions.
[Darrell] If the limitation is only at driver layer, then that would be good
and we should identify
that in discussion with Intel and Mellanox. In the case of a
‘good driver version’, the user would consult the
documentation and use the new driver version if possible and
configure as
“supports native HWOL mark action”.
At least in one case, my understanding was that there was no
plan to fix this, although that plan may
change of course.
Anyways, if you want to support probing in the next patchset,
then that is fine.
But, I think we still need verbose documentation about the nics
differences in OVS docs, including the impact
on receive queue, as used by OVS-DPDK.
> I think anyways we need documentation describing the difference b/w nics
in the dpdk documentation (howto part).
IIRC, there was already an ask from the DPDK mailing list.
[Darrell] I meant we need to document this in OVS since we have OVS code that
is exposed to these differences.
--yliu
> Assuming no such enhancement is appropriate at this time I would
> still like to ask if this is the best place for this
hardware-specific code?
>
> [Darrell]
>
> For OVS, the netdev-dpdk layer is the lowest layer.
> This kind of workaround is hard to hide, since we are messing with the
rxq, so I think OVS needs to know
> that it is in effect anyways. An alternative is to supply a mark and an
‘optional queue’ and let the driver decide if the queue is
> needed and report back whether it was. This would be hard to do across
various drivers. Supporting in the rte layer would require both
> rte and driver support, so even more support.
>
>
>
> > 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.
>
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
