DPDK patches and discussions
 help / color / mirror / Atom feed
From: Adrien Mazarguil <adrien.mazarguil@6wind.com>
To: Ferruh Yigit <ferruh.yigit@intel.com>
Cc: "Iremonger, Bernard" <bernard.iremonger@intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	"Mcnamara, John" <john.mcnamara@intel.com>,
	"Tahhan, Maryam" <maryam.tahhan@intel.com>
Subject: Re: [dpdk-dev] [RFC v2] Flow classification library
Date: Tue, 23 May 2017 15:30:57 +0200	[thread overview]
Message-ID: <20170523133057.GD1758@6wind.com> (raw)
In-Reply-To: <2742b205-fec9-3414-7693-e0c2369ae80d@intel.com>

On Tue, May 23, 2017 at 01:58:44PM +0100, Ferruh Yigit wrote:
> On 5/23/2017 1:26 PM, Adrien Mazarguil wrote:
> > On Mon, May 22, 2017 at 02:53:28PM +0100, Ferruh Yigit wrote:
> >> On 5/19/2017 5:30 PM, Iremonger, Bernard wrote:
> >>> Hi Ferruh,
> >>>
> >>>> -----Original Message-----
> >>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh Yigit
> >>>> Sent: Thursday, May 18, 2017 7:12 PM
> >>>> To: dev@dpdk.org
> >>>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Mcnamara, John
> >>>> <john.mcnamara@intel.com>; Tahhan, Maryam
> >>>> <maryam.tahhan@intel.com>
> >>>> Subject: [dpdk-dev] [RFC v2] Flow classification library
> >>>>
> >>>> DPDK works with packets, but some network administration tools works
> >>>> based on flow information.
> >>>>
> >>>> This library is suggested to provide helper API to convert packet based
> >>>> information to the flow records.
> >>>>
> >>>> Basically the library consist of a single API that gets packets, flow definition
> >>>> and action as parameter and provides flow stats based on action. Application
> >>>> should call the API for all received packets.
> >>>>
> >>>> Library header file has more comments on how library works and provided
> >>>> APIs.
> >>>>
> >>>> Packets to flow conversion will cause performance drop, that is why
> >>>> conversion done on demand by an API call provided by this library.
> >>>>
> >>>> Initial implementation in mind is to provide support for IPFIX metering
> >>>> process but library planned to be as generic as possible. And flow information
> >>>> provided by this library is missing to implement full IPFIX features, but this is
> >>>> planned to be initial step.
> >>>>
> >>>> Flows are defined using rte_flow, also measurements (actions) are provided
> >>>> by rte_flow. To support more IPFIX measurements, the implementation may
> >>>> require extending rte_flow addition to implementing this library.
> >>>
> >>> Do you know what extensions are needed to the rte_flow code?
> >>
> >> The extension may be required on two fields:
> >> 1- Defining the flow
> >> 2- Available actions
> >>
> >> For defining the flow, an update may not be required, specially at first
> >> version of the library.
> >>
> >> But for action, there may be some updates.
> >>
> >> IPFIX RFC defines Metering process as [1], (in [2]). This library should
> >> provide helper APIs to metering process.
> >>
> >> Currently only action can be used in rte_flow is COUNT, more actions can
> >> be added to help "packet header capturing, timestamping, sampling,
> >> classifying" tasks of the metering process.
> >>
> >> The exact list depends on the what will be implemented in this release.
> >>
> >>
> >> [1]
> >>    Metering Process
> >>
> >>       The Metering Process generates Flow Records.  Inputs to the
> >>       process are packet headers, characteristics, and Packet Treatment
> >>       observed at one or more Observation Points.
> >>
> >>       The Metering Process consists of a set of functions that includes
> >>       packet header capturing, timestamping, sampling, classifying, and
> >>       maintaining Flow Records.
> >>
> >>       The maintenance of Flow Records may include creating new records,
> >>       updating existing ones, computing Flow statistics, deriving
> >>       further Flow properties, detecting Flow expiration, passing Flow
> >>       Records to the Exporting Process, and deleting Flow Records.
> >>
> >> [2]
> >> https://tools.ietf.org/html/rfc7011
> > 
> > Since I did not take this into account in my previous answer [3], I now
> > understand several of these requirements cannot be met by hardware (at least
> > in the near future). Therefore I think it makes sense to leave IPFIX and
> > more generally the maintenance of software data associated with flows to
> > separate libraries, instead of adding many new rte_flow actions that can
> > only be implemented in software.
> > 
> > A hybrid solution as described in [3] is still needed regardless to offload
> > flow recognition, so that only flows of interest are reprocessed in software
> > to compute IPFIX and other data.
> > 
> > You suggested at one point to take flow rules in addition to mbufs as input
> > to handle that. Well, that's actually a nice approach.
> > 
> > For this to work, rte_flow_classify would have to use opaque handles like
> > rte_flow, provided back by the application when attempting to classify
> > traffic. If the handle is not known (e.g. MARK is unsupported), a separate
> > API function could take a mbuf as input and spit the related
> > rte_flow_classify object if any.
> > 
> > To be clear:
> > 
> > 1. Create classifier object:
> > 
> >    classify = rte_flow_classify_create([some rte_flow pattern],
> >       [classify-specific actions list, associated resources]);
> > 
> > 2. Create some flow rule with a MARK action to identify it uniquely. This
> >    step might fail and flow can be NULL, that's not an issue:
> > 
> >    flow = rte_flow_create([the same pattern], MARK with id 42)
> > 
> > 3. For each received packet:
> > 
> >    /*
> >     * Attempt HW and fall back on SW for flow identification in order to
> >     * update classifier flow-related data.
> >     */
> >    if (flow) {
> >       if (mbuf->ol_flags & PKT_RX_FDIR_ID && mbuf->hash.fdir.hi == 42)
> >          tmp_classify = classify;
> >    } else {
> >       tmp_classify = rte_flow_classify_lookup([classifier candidates], mbuf);
> >    }
> >    if (tmp_classify)
> >          rte_flow_classify_update(tmp_classify, mbuf);
> > 
> > 4. At some point, retrieve computed data from the classifier object itself:
> > 
> >    rte_flow_classify_stats_get(classify, [output buffer(s)])
> > 
> > On the RX path, the MARK action can be enough to implement the above. When
> > not supported, it could also be emulated through the "sw_fallback" bit
> > described in [3] however if the above approach is fine, no need for that.
> > 
> > It's a bit more complicated to benefit from rte_flow on the TX path since no
> > MARK data can be returned. There is currently no other solution than doing
> > it all in software anyway.
> > 
> > [3] http://dpdk.org/ml/archives/dev/2017-May/066177.html
> > 
> 
> Using MARK action is good idea indeed, but I believe the software
> version needs to be implemented anyway, so the MARK action can be next
> step optimization.

Does it mean you are you fine with the separation of RFCv2's
rte_flow_classify_stats_get() into a sort of rte_flow_classify_lookup() and
rte_flow_classify_update() as described?

There is otherwise no change needed to benefit from MARK optimization
beside providing the ability to not perform the lookup step when not
necessary.

> And another thing is, having 3. and 4. as separate steps means
> rte_flow_classify to maintain the flow_records in the library for an
> unknown time.

I think it should be an application's problem. The library does not
necessarily allocate anything since the related temporary storage can be
provided as arguments to rte_flow_classify_create().

Expiration could be handled by making rte_flow_classify_lookup() return some
error code when the flow has expired. Application would then be free to call
rte_flow_classify_destroy() directly or retrieve stats one last time through
rte_flow_classify_stats_get() before that. 

> Application needs to explicitly do the call for 3., why not return the
> output buffers immediately so the application do the required
> association of data to the ports.

3. is typically done in the data path and must be as fast as possible,
right? The way I see it, in many cases if the flow is already identified
(classifier object provided) the mbuf does not even need to be parsed for
associated counters to be incremented.

Also because rte_flow_classify_stats_get() could perform extra computation
to convert internal data to an application-friendly format.

> Mainly, I would like to keep same API logic in RFC v2, but agree to add
> two new APIs:
> rte_flow_classify_create()
> rte_flow_classify_destroy()
> 
> Create will return and opaque rte_flow object (as done in rte_flow),
> meanwhile it can create some masks in rte_flow object to help parsing mbuf.

OK, I suggest naming that object "struct rte_flow_classify" to avoid
confusion though.

> And rte_flow_classify_stats_get() will get rte_flow object and stats as
> parameter. Also instead of getting an array of filters, to simplify the
> logic, it will get a rte_flow object at a time.

Perfect.

> I am planning to send an updated RFC (v3) as above, if there is no major
> objection, we can continue to discuss on that RFC.

Except for my above comments, looks like we're converging.

-- 
Adrien Mazarguil
6WIND

  reply	other threads:[~2017-05-23 13:31 UTC|newest]

Thread overview: 145+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-20 18:54 [dpdk-dev] [RFC 17.08] " Ferruh Yigit
2017-04-20 18:54 ` [dpdk-dev] [RFC 17.08] flow_classify: add librte_flow_classify library Ferruh Yigit
2017-05-04 11:35   ` Mcnamara, John
2017-05-16 22:19     ` Thomas Monjalon
2017-05-17 14:54   ` Ananyev, Konstantin
2017-05-17 15:37     ` Ferruh Yigit
2017-05-17 16:10       ` Ananyev, Konstantin
2017-05-18 12:12         ` Ferruh Yigit
2017-05-17 16:02     ` Ferruh Yigit
2017-05-17 16:18       ` Ananyev, Konstantin
2017-05-17 16:38       ` Gaëtan Rivet
2017-05-18 11:33         ` Ferruh Yigit
2017-05-18 20:31           ` Thomas Monjalon
2017-05-19  8:57             ` Ananyev, Konstantin
2017-05-19  9:11               ` Gaëtan Rivet
2017-05-19  9:40                 ` Ananyev, Konstantin
2017-05-19 10:11                 ` Thomas Monjalon
2017-05-22  9:13                   ` Adrien Mazarguil
2017-04-21 10:38 ` [dpdk-dev] [RFC 17.08] Flow classification library Gaëtan Rivet
2017-05-03  9:15   ` Mcnamara, John
2017-05-06 14:04     ` Morten Brørup
2017-05-09 13:37       ` Ferruh Yigit
2017-05-09 19:24         ` Morten Brørup
2017-05-17 11:26           ` Ferruh Yigit
2017-05-09 13:26   ` Ferruh Yigit
2017-05-18 18:12 ` [dpdk-dev] [RFC v2] " Ferruh Yigit
2017-05-18 18:12   ` [dpdk-dev] [RFC v2] flow_classify: add librte_flow_classify library Ferruh Yigit
2017-05-19 16:30   ` [dpdk-dev] [RFC v2] Flow classification library Iremonger, Bernard
2017-05-22 13:53     ` Ferruh Yigit
2017-05-23 12:26       ` Adrien Mazarguil
2017-05-23 12:58         ` Ferruh Yigit
2017-05-23 13:30           ` Adrien Mazarguil [this message]
2017-05-23 16:42             ` Ferruh Yigit
2017-05-25 15:46   ` [dpdk-dev] [RFC v3] " Ferruh Yigit
2017-05-25 15:46     ` [dpdk-dev] [RFC v3] flow_classify: add librte_flow_classify library Ferruh Yigit
2017-05-30 12:59       ` Iremonger, Bernard
2017-08-23 13:51     ` [dpdk-dev] [PATCH v1 0/6] Flow classification library Bernard Iremonger
2017-08-25 16:10       ` [dpdk-dev] [PATCH v2 0/6] flow " Bernard Iremonger
2017-08-31 14:54         ` [dpdk-dev] [PATCH v3 0/5] " Bernard Iremonger
2017-09-06 10:27           ` [dpdk-dev] [PATCH v4 " Bernard Iremonger
2017-09-07 16:43             ` [dpdk-dev] [PATCH v5 0/6] " Bernard Iremonger
2017-09-29  9:18               ` [dpdk-dev] [PATCH v6 0/4] " Bernard Iremonger
2017-10-02  9:31                 ` [dpdk-dev] [PATCH v7 " Bernard Iremonger
2017-10-17 20:26                   ` [dpdk-dev] [PATCH v8 " Bernard Iremonger
2017-10-22 13:32                     ` [dpdk-dev] [PATCH v9 " Bernard Iremonger
2017-10-23 15:16                       ` [dpdk-dev] [PATCH v10 " Bernard Iremonger
2017-10-23 20:59                         ` Thomas Monjalon
2017-10-24  8:40                           ` Iremonger, Bernard
2017-10-24  9:23                             ` Mcnamara, John
2017-10-24  9:38                               ` Thomas Monjalon
2017-10-24  9:53                                 ` Iremonger, Bernard
2017-10-24 10:25                                   ` Thomas Monjalon
2017-10-24 17:27                         ` [dpdk-dev] [PATCH v11 " Bernard Iremonger
2017-10-24 20:33                           ` Thomas Monjalon
2017-10-25  8:47                             ` Iremonger, Bernard
2017-10-25  8:56                               ` Thomas Monjalon
2017-10-24 17:28                         ` [dpdk-dev] [PATCH v11 1/4] flow_classify: add flow classify library Bernard Iremonger
2017-10-24 19:39                           ` Thomas Monjalon
2017-10-25 11:10                             ` Iremonger, Bernard
2017-10-25 12:13                               ` Thomas Monjalon
2017-10-24 19:41                           ` Thomas Monjalon
2017-10-24 19:43                           ` Thomas Monjalon
2017-10-24 20:05                           ` Thomas Monjalon
2017-10-24 20:16                           ` Thomas Monjalon
2017-10-24 20:18                           ` Thomas Monjalon
2017-10-24 17:28                         ` [dpdk-dev] [PATCH v11 2/4] examples/flow_classify: flow classify sample application Bernard Iremonger
2017-10-24 20:13                           ` Thomas Monjalon
2017-10-24 17:28                         ` [dpdk-dev] [PATCH v11 3/4] test: add packet burst generator functions Bernard Iremonger
2017-10-24 17:28                         ` [dpdk-dev] [PATCH v11 4/4] test: flow classify library unit tests Bernard Iremonger
2017-10-23 15:16                       ` [dpdk-dev] [PATCH v10 1/4] librte_flow_classify: add flow classify library Bernard Iremonger
2017-10-23 16:03                         ` Singh, Jasvinder
2017-10-24  9:50                         ` Thomas Monjalon
2017-10-24 10:09                           ` Iremonger, Bernard
2017-10-23 15:16                       ` [dpdk-dev] [PATCH v10 2/4] examples/flow_classify: flow classify sample application Bernard Iremonger
2017-10-23 16:04                         ` Singh, Jasvinder
2017-10-23 15:16                       ` [dpdk-dev] [PATCH v10 3/4] test: add packet burst generator functions Bernard Iremonger
2017-10-23 16:05                         ` Singh, Jasvinder
2017-10-23 15:16                       ` [dpdk-dev] [PATCH v10 4/4] test: flow classify library unit tests Bernard Iremonger
2017-10-23 16:06                         ` Singh, Jasvinder
2017-10-22 13:32                     ` [dpdk-dev] [PATCH v9 1/4] librte_flow_classify: add flow classify library Bernard Iremonger
2017-10-23 13:21                       ` Singh, Jasvinder
2017-10-23 13:37                         ` Iremonger, Bernard
2017-10-22 13:32                     ` [dpdk-dev] [PATCH v9 2/4] examples/flow_classify: flow classify sample application Bernard Iremonger
2017-10-22 13:32                     ` [dpdk-dev] [PATCH v9 3/4] test: add packet burst generator functions Bernard Iremonger
2017-10-22 13:32                     ` [dpdk-dev] [PATCH v9 4/4] test: flow classify library unit tests Bernard Iremonger
2017-10-17 20:26                   ` [dpdk-dev] [PATCH v8 1/4] librte_flow_classify: add flow classify library Bernard Iremonger
2017-10-19 14:22                     ` Singh, Jasvinder
2017-10-20 16:59                       ` Iremonger, Bernard
2017-10-21 12:07                         ` Iremonger, Bernard
2017-10-17 20:26                   ` [dpdk-dev] [PATCH v8 2/4] examples/flow_classify: flow classify sample application Bernard Iremonger
2017-10-17 20:26                   ` [dpdk-dev] [PATCH v8 3/4] test: add packet burst generator functions Bernard Iremonger
2017-10-17 20:26                   ` [dpdk-dev] [PATCH v8 4/4] test: flow classify library unit tests Bernard Iremonger
2017-10-02  9:31                 ` [dpdk-dev] [PATCH v7 1/4] librte_flow_classify: add librte_flow_classify library Bernard Iremonger
2017-10-06 15:00                   ` Singh, Jasvinder
2017-10-09  9:28                     ` Mcnamara, John
2017-10-13 15:39                     ` Iremonger, Bernard
2017-10-02  9:31                 ` [dpdk-dev] [PATCH v7 2/4] examples/flow_classify: flow classify sample application Bernard Iremonger
2017-10-02  9:31                 ` [dpdk-dev] [PATCH v7 3/4] test: add packet burst generator functions Bernard Iremonger
2017-10-02  9:31                 ` [dpdk-dev] [PATCH v7 4/4] test: flow classify library unit tests Bernard Iremonger
2017-09-29  9:18               ` [dpdk-dev] [PATCH v6 1/4] librte_flow_classify: add librte_flow_classify library Bernard Iremonger
2017-09-29  9:18               ` [dpdk-dev] [PATCH v6 2/4] examples/flow_classify: flow classify sample application Bernard Iremonger
2017-09-29  9:18               ` [dpdk-dev] [PATCH v6 3/4] test: add packet burst generator functions Bernard Iremonger
2017-09-29  9:18               ` [dpdk-dev] [PATCH v6 4/4] test: flow classify library unit tests Bernard Iremonger
2017-09-07 16:43             ` [dpdk-dev] [PATCH v5 1/6] librte_table: fix acl entry add and delete functions Bernard Iremonger
2017-09-18 15:29               ` Singh, Jasvinder
2017-09-20 12:21                 ` Dumitrescu, Cristian
2017-09-29  8:25                   ` Iremonger, Bernard
2017-09-07 16:43             ` [dpdk-dev] [PATCH v5 2/6] librte_table: fix acl lookup function Bernard Iremonger
2017-09-20 12:24               ` Dumitrescu, Cristian
2017-09-29  8:27                 ` Iremonger, Bernard
2017-09-07 16:43             ` [dpdk-dev] [PATCH v5 3/6] librte_flow_classify: add librte_flow_classify library Bernard Iremonger
2017-09-07 16:43             ` [dpdk-dev] [PATCH v5 4/6] examples/flow_classify: flow classify sample application Bernard Iremonger
2017-09-07 16:43             ` [dpdk-dev] [PATCH v5 5/6] test: add packet burst generator functions Bernard Iremonger
2017-09-07 16:43             ` [dpdk-dev] [PATCH v5 6/6] test: flow classify library unit tests Bernard Iremonger
2017-09-06 10:27           ` [dpdk-dev] [PATCH v4 1/5] librte_table: fix acl entry add and delete functions Bernard Iremonger
2017-09-06 10:27           ` [dpdk-dev] [PATCH v4 2/5] librte_table: fix acl lookup function Bernard Iremonger
2017-09-06 10:27           ` [dpdk-dev] [PATCH v4 3/5] librte_flow_classify: add librte_flow_classify library Bernard Iremonger
2017-09-06 10:27           ` [dpdk-dev] [PATCH v4 4/5] examples/flow_classify: flow classify sample application Bernard Iremonger
2017-09-06 10:27           ` [dpdk-dev] [PATCH v4 5/5] test: flow classify library unit tests Bernard Iremonger
2017-08-31 14:54         ` [dpdk-dev] [PATCH v3 1/5] librte_table: fix acl entry add and delete functions Bernard Iremonger
2017-08-31 15:09           ` Pavan Nikhilesh Bhagavatula
2017-08-31 14:54         ` [dpdk-dev] [PATCH v3 2/5] librte_table: fix acl lookup function Bernard Iremonger
2017-08-31 14:54         ` [dpdk-dev] [PATCH v3 3/5] librte_flow_classify: add librte_flow_classify library Bernard Iremonger
2017-08-31 15:18           ` Pavan Nikhilesh Bhagavatula
2017-08-31 14:54         ` [dpdk-dev] [PATCH v3 4/5] examples/flow_classify: flow classify sample application Bernard Iremonger
2017-08-31 14:54         ` [dpdk-dev] [PATCH v3 5/5] test: flow classify library unit tests Bernard Iremonger
2017-08-25 16:10       ` [dpdk-dev] [PATCH v2 1/6] librte_table: fix acl entry add and delete functions Bernard Iremonger
2017-08-25 16:10       ` [dpdk-dev] [PATCH v2 2/6] librte_table: fix acl lookup function Bernard Iremonger
2017-08-25 16:10       ` [dpdk-dev] [PATCH v2 3/6] librte_ether: initialise IPv4 protocol mask for rte_flow Bernard Iremonger
2017-08-30 12:39         ` Adrien Mazarguil
2017-08-30 13:28           ` Iremonger, Bernard
2017-08-30 14:39             ` Adrien Mazarguil
2017-08-30 15:12               ` Iremonger, Bernard
2017-08-25 16:10       ` [dpdk-dev] [PATCH v2 4/6] librte_flow_classify: add librte_flow_classify library Bernard Iremonger
2017-08-25 16:10       ` [dpdk-dev] [PATCH v2 5/6] examples/flow_classify: flow classify sample application Bernard Iremonger
2017-08-25 16:10       ` [dpdk-dev] [PATCH v2 6/6] test: flow classify library unit tests Bernard Iremonger
2017-08-23 13:51     ` [dpdk-dev] [PATCH v1 1/6] librte_table: move structure to header file Bernard Iremonger
2017-08-23 14:13       ` Dumitrescu, Cristian
2017-08-23 14:32         ` Iremonger, Bernard
2017-08-28  8:48           ` Iremonger, Bernard
2017-08-23 13:51     ` [dpdk-dev] [PATCH v1 2/6] librte_table: fix acl entry add and delete functions Bernard Iremonger
2017-08-23 13:51     ` [dpdk-dev] [PATCH v1 3/6] librte_ether: initialise IPv4 protocol mask for rte_flow Bernard Iremonger
2017-08-23 13:51     ` [dpdk-dev] [PATCH v1 4/6] librte_flow_classify: add librte_flow_classify library Bernard Iremonger
2017-08-23 13:51     ` [dpdk-dev] [PATCH v1 5/6] examples/flow_classify: flow classify sample application Bernard Iremonger
2017-08-23 13:51     ` [dpdk-dev] [PATCH v1 6/6] test: flow classify library unit tests Bernard Iremonger

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=20170523133057.GD1758@6wind.com \
    --to=adrien.mazarguil@6wind.com \
    --cc=bernard.iremonger@intel.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=john.mcnamara@intel.com \
    --cc=maryam.tahhan@intel.com \
    /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).