DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Andrzej Ostruszka [C]" <aostruszka@marvell.com>
To: Thomas Monjalon <thomas@monjalon.net>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v2 0/4] Introduce IF proxy library
Date: Sat, 4 Apr 2020 18:07:12 +0000	[thread overview]
Message-ID: <9c033551-481e-d90e-9cc6-b10e627512c1@marvell.com> (raw)
In-Reply-To: <6610147.nAD6y4vbrC@xps>

First of all Thomas, thank you for taking time to look at this.

Please scroll below for my comments.  It looks like we are going to
detour a bit for a general config mechanism on top of which we can
"rebase" IF Proxy and other libs/apps.

Note - since I often like to think in concrete terms I might be
switching between general comments and specific examples.

On 4/3/20 11:42 PM, Thomas Monjalon wrote:
> Hi Andrzej,
[...]
> 10/03/2020 12:10, Andrzej Ostruszka:
[...]
>> The purpose of this library is to help with both of these tasks (as long
>> as they remain in domain of configuration available to the system).  In
>> other words, if DPDK application has some special needs, that cannot be
>> addressed by the normal system configuration utilities, then they need
>> to be solved by the application itself.
> 
> In any case, the application must be in the loop.
> The application should always remain in control.

OK - so let me try to understand what you mean here on the example of
this IF Proxy.  I wanted (and that is an explicit goal) to use iproute2
tools to configure DPDK ports.  In this context allowing application to
have control might mean two things:

- application can accept/ignore/deny change requested by the user from
the shell in a dynamic way (based on some state/event/...)

- application writer have a choice to bind or not, but once the proxy is
bound you simply accept the requests - just like you accept user
requests in e.g. testpmd shell.

So ...

> When querying some information, nothing need to be controlled I guess.
> But when adjusting some configuration, the application must be able
> to be notified and decide which change is allowed.
> Of course, the application might allow being bypassed.

... it looks like you are talking about the first option ("bypass" is I
guess the second option).  In the concrete example of IF Proxy that
might be a bit problematic.  User requests changes on proxy interface
kernel accepts them and the DPDK application is just notified about that
- has no chance to deny request (say "busy" or "not permitted" to the user).

Of course app can ignore it (do nothing in the callback or drop the
event) and have a mismatch between port and its proxy.  I'm not so sure
if this is what you had in mind.

> Currently this rule is not respected in the rte_mp IPC system.
> I think rte_mp and IF proxy should follow the same path,
> keeping the primary application process in control.
> 
> I would like not only secondary process and IF proxy be able to use
> this control path. It should be generic enough to allow any application
> (local or remote) be part of the control path, communicating with
> the DPDK application primary process.

That goal sounds indeed like ZMQ.  Is the consensus about that already
reached?  On a general level that sounds good to me - the devil might be
in details, like e.g. trying to be simple and generic enough.  I'd like
also to solicit here input from other members of the community.

> As a summary, I propose to target the following goal:
> implement a user configuration path as a DPDK standard
> that the application can enable.
> 
> Do we agree that the exception packet path is out of scope?

Could you rephrase this question?  I'm not sure I understand it.  If you
wanted to say that we should:

- implement general config/notification mechanism
- rebase IF Proxy upon it (so instead of deciding whether this should be
callback/queue it simply uses this new scheme to deliver the change to
application)

then I'm fine with this.  If you meant something else then please explain.

> [...]
>> We create two proxy interfaces (here based on Tap driver) and bind the
>> ports to their proxies.  When user issues a command changing MTU for
>> Tap1 interface the library notes this and calls "mtu_change" callback
>> for the Port1.  Similarly when user adds an IPv4 address to the Tap2
>> interface "addr_add" callback is called for the Port2 and the same
>> happens for configuration of routing rule pointing to Tap2.
> 
> Will it work as well with TC flow configuration converted to rte_flow?

Not at the moment.  But should be doable - as long as there is good
mapping between them (I haven't checked).

>> Apart from
>> callbacks this library can notify about changes via adding events to
>> notification queues.  See below for more inforamtion about that and
>> a complete list of available callbacks.
> 
> There is choice between callback in a random context,
> or a read from a message queue in a controlled context.
> Second option looks better.

Note that callback can be a simple enqueue to some ring.  From the IF
Proxy implementation point of view - this is not much of a difference.
I notice the change and in that place in code I can either call callback
or queue an event.  Since I expect queues to be a popular choice its
support is added but without this user could register callback that
would be enqueuing (one more indirection in slow path).

Having said that - the only reason callbacks are kept is that (as
mentioned in cover):

- I can easily implement global action (true - in random context), since
queues are "match all" each event will be added to all queues and cores
would have to decide which one of them performs the global action
- I can do single/global preparation before queueing event

But I guess this is rather a mute point since we are going in the
direction of general config which IF Proxy would be using.

>> Please note that nothing has been mentioned about forwarding of the
>> packets between system and DPDK.  Since the proxies are normal DPDK
>> ports you can receive/send to them via usual RX/TX burst API.  However
>> since the library is not aware of the structure of packet processing
>> used by the application it cannot automatically forward the packets - it
>> is responsibility of the application to include proxy ports into its
>> packet processing engine.
> 
> So IF proxy does nothing special with packets, right?

Correct.

>> Although the library only helps you to identify proxy for given port
>> (and vice versa) and calls appropriate callbacks it does open some
>> interesting possibilities.  For example you can use the proxy ports to
>> forward packets for protocols that you do not wish to handle in DPDK
>> application to the system protocol stack and just listen to the
>> configuration changes - so that way you can "offload" handling of those
>> protocols to the system.
> 
> Note that when using a bifurcated driver (af_xdp or mlx),
> the exception path in the kernel is not going through DPDK.
> Moreover, no proxy is needed for device configuration in such case.

True for the link level info.  But if application would like to have
also address/routing/neighbouring info then I guess proxy would be
needed.  As for the bifurcated drivers - in one version of the library I
had an option to bind port to itself.  The binding is there only to tell
library which if_index is interesting and how to report event (if_index
-> port_id)

[...]
>> This creates logical binding - as mentioned above there is no automatic
>> packet forwarding.  With this binding whenever user changes the state of
>> proxy interface in the system (link up/down, change mac/mtu, add/remove
>> IPv4/IPv6) you get appropriate notification for the bound port.
> 
> When configuring a port via DPDK API, is it mirrored automatically
> to the kernel device?

No it isn't.  It's one way at the moment.  If we wanted bidirectional
then I would have to plug in somewhere in eth_dev to monitor changes to
ports and request similar changes to the proxy.

[...]
>> and the actual logic used is: if there is callback registered then it is
>> called, if it returns non-zero then event is considered completed,
>> otherwise event is added to each configured notification queue.
>> That way application can update data structures that are safe to be
>> modified by single writer from within callback or do the common
>> preprocessing steps (if any needed) in callback and data that is
>> replicated can be updated during handling of queued events.
> 
> As explained above, the application must control every changes.
> 
> One issue is thread safety.
> The simplest model is to manage control path from a single thread
> in the primary process.
> 
> If we create an API to allow the application managing the control path
> from external requests, I think it should be a building block
> independent of IF proxy. Then IF proxy can plug into this subsystem.
> It would allow other control path mechanisms to co-exist.

I'm fine with this.

> [...]
>> It is worth to mention also that while typical case would be a 1-to-1
>> mapping between port and proxy, the 1-to-many mapping is also supported.
>> In that case related callbacks will be called for each port bound to
>> given proxy interface - it is application responsibility to define
>> semantic of such mapping (e.g. all changes apply to all ports, or link
>> changes apply to all but other are accepted in "round robin" fashion, or
>> some other logic).
> 
> I don't get the interest of one-to-many mapping.

That was a request during early version of library - with bridging in
mind.  However this is an "experimental" part of an "experimental"
library - I would not focus on this, as it might get removed if we don't
find a real use case for that.

> [...]
> 
> Thanks for the work.
> It seems there are some overlaps with telemetry and rte_mp channels.
> The same channel could be used also for dynamic tracing command
> or for remote control.
> Would you be OK to extend it to a global control subsystem,
> having IF proxy plugged in?

Yes.  I don't have a problem with that - however at the moment the
requirements/design are a bit vague, so let's discuss this more.

With regards
Andrzej Ostruszka

  reply	other threads:[~2020-04-04 18:07 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-06 16:41 [dpdk-dev] [PATCH " Andrzej Ostruszka
2020-03-06 16:41 ` [dpdk-dev] [PATCH 1/4] lib: introduce IF Proxy library Andrzej Ostruszka
2020-03-31 12:36   ` Harman Kalra
2020-03-31 15:37     ` Andrzej Ostruszka [C]
2020-04-01  5:29   ` Varghese, Vipin
2020-04-01 20:08     ` Andrzej Ostruszka [C]
2020-04-08  3:04       ` Varghese, Vipin
2020-04-08 18:13         ` Andrzej Ostruszka [C]
2020-03-06 16:41 ` [dpdk-dev] [PATCH 2/4] if_proxy: add library documentation Andrzej Ostruszka
2020-03-06 16:41 ` [dpdk-dev] [PATCH 3/4] if_proxy: add simple functionality test Andrzej Ostruszka
2020-03-06 16:41 ` [dpdk-dev] [PATCH 4/4] if_proxy: add example application Andrzej Ostruszka
2020-03-06 17:17 ` [dpdk-dev] [PATCH 0/4] Introduce IF proxy library Andrzej Ostruszka
2020-03-10 11:10 ` [dpdk-dev] [PATCH v2 " Andrzej Ostruszka
2020-03-10 11:10   ` [dpdk-dev] [PATCH v2 1/4] lib: introduce IF Proxy library Andrzej Ostruszka
2020-07-02  0:34     ` Stephen Hemminger
2020-07-07 20:13       ` Andrzej Ostruszka [C]
2020-07-08 16:07         ` Morten Brørup
2020-07-09  8:43           ` Andrzej Ostruszka [C]
2020-07-22  0:40             ` Thomas Monjalon
2020-07-22  8:45               ` Jerin Jacob
2020-07-22  8:56                 ` Thomas Monjalon
2020-07-22  9:09                   ` Jerin Jacob
2020-07-22  9:27                     ` Thomas Monjalon
2020-07-22  9:54                       ` Jerin Jacob
2020-07-23 14:09                         ` [dpdk-dev] [EXT] " Andrzej Ostruszka [C]
2020-03-10 11:10   ` [dpdk-dev] [PATCH v2 2/4] if_proxy: add library documentation Andrzej Ostruszka
2020-03-10 11:10   ` [dpdk-dev] [PATCH v2 3/4] if_proxy: add simple functionality test Andrzej Ostruszka
2020-03-10 11:10   ` [dpdk-dev] [PATCH v2 4/4] if_proxy: add example application Andrzej Ostruszka
2020-03-25  8:08   ` [dpdk-dev] [PATCH v2 0/4] Introduce IF proxy library David Marchand
2020-03-25 11:11     ` Morten Brørup
2020-03-26 17:42       ` Andrzej Ostruszka
2020-04-02 13:48         ` Andrzej Ostruszka [C]
2020-04-03 17:19           ` Thomas Monjalon
2020-04-03 19:09             ` Jerin Jacob
2020-04-03 21:18               ` Morten Brørup
2020-04-03 21:57                 ` Thomas Monjalon
2020-04-04 10:18                   ` Jerin Jacob
2020-04-10 10:41                     ` Morten Brørup
2020-04-04 18:30             ` Andrzej Ostruszka [C]
2020-04-04 19:58               ` Thomas Monjalon
2020-04-10 10:03                 ` Morten Brørup
2020-04-10 12:28                   ` Jerin Jacob
2020-03-26 12:41     ` Andrzej Ostruszka
2020-03-30 19:23       ` Andrzej Ostruszka
2020-04-03 21:42   ` Thomas Monjalon
2020-04-04 18:07     ` Andrzej Ostruszka [C] [this message]
2020-04-04 19:51       ` Thomas Monjalon
2020-04-16 16:11 ` [dpdk-dev] [PATCH " Stephen Hemminger
2020-04-16 16:49   ` Jerin Jacob
2020-04-16 17:04     ` Stephen Hemminger
2020-04-16 17:26       ` Andrzej Ostruszka [C]
2020-04-16 17:27       ` Jerin Jacob
2020-04-16 17:12     ` Andrzej Ostruszka [C]
2020-04-16 17:19       ` Stephen Hemminger
2020-05-04  8:53 ` [dpdk-dev] [PATCH v3 " Andrzej Ostruszka
2020-05-04  8:53   ` [dpdk-dev] [PATCH v3 1/4] lib: introduce IF Proxy library Andrzej Ostruszka
2020-05-04  8:53   ` [dpdk-dev] [PATCH v3 2/4] if_proxy: add library documentation Andrzej Ostruszka
2020-05-04  8:53   ` [dpdk-dev] [PATCH v3 3/4] if_proxy: add simple functionality test Andrzej Ostruszka
2020-05-04  8:53   ` [dpdk-dev] [PATCH v3 4/4] if_proxy: add example application Andrzej Ostruszka
2020-06-22  9:21 ` [dpdk-dev] [PATCH v4 0/4] Introduce IF proxy library Andrzej Ostruszka
2020-06-22  9:21   ` [dpdk-dev] [PATCH v4 1/4] lib: introduce IF Proxy library Andrzej Ostruszka
2020-06-22  9:21   ` [dpdk-dev] [PATCH v4 2/4] if_proxy: add library documentation Andrzej Ostruszka
2020-06-22  9:21   ` [dpdk-dev] [PATCH v4 3/4] if_proxy: add simple functionality test Andrzej Ostruszka
2020-06-22  9:21   ` [dpdk-dev] [PATCH v4 4/4] if_proxy: add example application Andrzej Ostruszka

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=9c033551-481e-d90e-9cc6-b10e627512c1@marvell.com \
    --to=aostruszka@marvell.com \
    --cc=dev@dpdk.org \
    --cc=thomas@monjalon.net \
    /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).