From: Andrew Rybchenko <arybchenko@solarflare.com>
To: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Cc: <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v3] ethdev: add isolated mode to flow API
Date: Wed, 14 Jun 2017 17:04:16 +0300 [thread overview]
Message-ID: <fb035f6a-3162-d6c5-e75c-b0412cd587c8@solarflare.com> (raw)
In-Reply-To: <20170614133529.GO1758@6wind.com>
On 06/14/2017 04:35 PM, Adrien Mazarguil wrote:
> On Wed, Jun 14, 2017 at 04:01:46PM +0300, Andrew Rybchenko wrote:
>> On 06/14/2017 03:45 PM, Adrien Mazarguil wrote:
>>> Isolated mode can be requested by applications on individual ports to avoid
>>> ingress traffic outside of the flow rules they define.
>>>
>>> Besides making ingress more deterministic, it allows PMDs to safely reuse
>>> resources otherwise assigned to handle the remaining traffic, such as
>>> global RSS configuration settings, VLAN filters, MAC address entries,
>>> legacy filter API rules and so on in order to expand the set of possible
>>> flow rule types.
>>>
>>> To minimize code complexity, PMDs implementing this mode may provide
>>> partial (or even no) support for flow rules when not enabled (e.g. no
>>> priorities, no RSS action). Applications written to use the flow API are
>>> therefore encouraged to enable it.
>>>
>>> Once effective, leaving isolated mode may not be possible depending on PMD
>>> implementation.
>>>
>>> Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
>>> Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
>>>
>>> ---
>>>
>>> v3:
>>> - Rebased on next-net/master. Note this patch depends on
>>> commit c0688ef1eded ("net/igb: parse flow API n-tuple filter") due to a
>>> necessary fix in igb's rte_flow_ops definition to avoid a compilation
>>> issue.
>>>
>>> v2:
>>> - Rebased on master.
>>> ---
>>> app/test-pmd/cmdline.c | 4 ++
>>> app/test-pmd/cmdline_flow.c | 49 ++++++++++++++++++++-
>>> app/test-pmd/config.c | 16 +++++++
>>> app/test-pmd/testpmd.h | 1 +
>>> doc/guides/prog_guide/rte_flow.rst | 56 ++++++++++++++++++++++++
>>> doc/guides/testpmd_app_ug/testpmd_funcs.rst | 48 +++++++++++++++++++-
>>> drivers/net/e1000/igb_flow.c | 9 ++--
>>> drivers/net/ixgbe/ixgbe_flow.c | 9 ++--
>>> lib/librte_ether/rte_ether_version.map | 7 +++
>>> lib/librte_ether/rte_flow.c | 18 ++++++++
>>> lib/librte_ether/rte_flow.h | 33 ++++++++++++++
>>> lib/librte_ether/rte_flow_driver.h | 5 +++
>>> 12 files changed, 242 insertions(+), 13 deletions(-)
>> <snip>
>>
>>> diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
>>> index b587ba9..699d2b2 100644
>>> --- a/doc/guides/prog_guide/rte_flow.rst
>>> +++ b/doc/guides/prog_guide/rte_flow.rst
>>> @@ -1517,6 +1517,62 @@ Return values:
>>> - 0 on success, a negative errno value otherwise and ``rte_errno`` is set.
>>> +Isolated mode
>>> +-------------
>>> +
>>> +The general expectation for ingress traffic is that flow rules process it
>>> +first; the remaining unmatched or pass-through traffic usually ends up in a
>>> +queue (with or without RSS, locally or in some sub-device instance)
>>> +depending on the global configuration settings of a port.
>>> +
>>> +While fine from a compatibility standpoint, this approach makes drivers more
>>> +complex as they have to check for possible side effects outside of this API
>>> +when creating or destroying flow rules. It results in a more limited set of
>>> +available rule types due to the way device resources are assigned (e.g. no
>>> +support for the RSS action even on capable hardware).
>>> +
>>> +Given that nonspecific traffic can be handled by flow rules as well,
>>> +isolated mode is a means for applications to tell a driver that ingress on
>>> +the underlying port must be injected from the defined flow rules only; that
>>> +no default traffic is expected outside those rules.
>>> +
>>> +This has the following benefits:
>>> +
>>> +- Applications get finer-grained control over the kind of traffic they want
>>> + to receive (no traffic by default).
>>> +
>>> +- More importantly they control at what point nonspecific traffic is handled
>>> + relative to other flow rules, by adjusting priority levels.
>>> +
>>> +- Drivers can assign more hardware resources to flow rules and expand the
>>> + set of supported rule types.
>>> +
>>> +Because toggling isolated mode may cause profound changes to the ingress
>>> +processing path of a driver, it may not be possible to leave it once
>>> +entered. Likewise, existing flow rules or global configuration settings may
>>> +prevent a driver from entering isolated mode.
>>> +
>>> +Applications relying on this mode are therefore encouraged to toggle it as
>>> +soon as possible after device initialization, ideally before the first call
>>> +to ``rte_eth_dev_configure()`` to avoid possible failures due to conflicting
>>> +settings.
>>> +
>> I think it would be useful to highlight how isolated mode coexists with
>> promiscuous
>> and all-multicast. What is the expected behaviour of the functions which
>> toggle
>> promiscuous and all-multicast mode if isolated mode is enabled? These
>> functions
>> return void right now, so it is impossible to return error. What should
>> rte_eth_promiscuous_get() and rte_eth_allmulticast_get() return?
> They can technically return nothing/anything as long as they have no effect
> on received traffic, as described.
I was just asking to highlight it in the documentation. Yes, idea of the
isolated
mode is clear and may be it is enough.
> Modifying existing wrappers that currently return void instead of an error
> is outside the scope of this patch and requires ABI breakage. This can be
> done later when the need arises.
It is perfectly clear.
> For mlx4/mlx5, we plan to expose a different set of rte_eth_dev_ops
> depending on whether isolated mode is toggled. When enabled, the
> allmulti/promisc/MAC/VLAN/etc callbacks would be NULL for instance, and the
> associated ethdev wrappers would automatically return an error where
> applicable.
Thanks for the idea. We'll consider it as well.
Andrew.
next prev parent reply other threads:[~2017-06-14 14:04 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-19 13:45 [dpdk-dev] [PATCH] " Adrien Mazarguil
2017-05-19 8:14 ` Adrien Mazarguil
2017-05-19 21:00 ` John Daley (johndale)
2017-05-22 10:17 ` Adrien Mazarguil
2017-05-24 13:53 ` Nélio Laranjeiro
2017-05-24 14:57 ` [dpdk-dev] [PATCH v2] " Adrien Mazarguil
2017-06-14 12:45 ` [dpdk-dev] [PATCH v3] " Adrien Mazarguil
2017-06-14 13:01 ` Andrew Rybchenko
2017-06-14 13:35 ` Adrien Mazarguil
2017-06-14 14:04 ` Andrew Rybchenko [this message]
2017-06-14 14:48 ` [dpdk-dev] [PATCH v4] " Adrien Mazarguil
2017-06-14 15:41 ` Andrew Rybchenko
2017-06-14 21:33 ` Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=fb035f6a-3162-d6c5-e75c-b0412cd587c8@solarflare.com \
--to=arybchenko@solarflare.com \
--cc=adrien.mazarguil@6wind.com \
--cc=dev@dpdk.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).