From: Ferruh Yigit <ferruh.yigit@intel.com>
To: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
"dev@dpdk.org" <dev@dpdk.org>
Cc: "Mcnamara, John" <john.mcnamara@intel.com>,
"Tahhan, Maryam" <maryam.tahhan@intel.com>
Subject: Re: [dpdk-dev] [RFC 17.08] flow_classify: add librte_flow_classify library
Date: Thu, 18 May 2017 13:12:36 +0100 [thread overview]
Message-ID: <b7a8a427-d207-ee8c-5800-7bb73907a671@intel.com> (raw)
In-Reply-To: <2601191342CEEE43887BDE71AB9772583FAF77CA@IRSMSX109.ger.corp.intel.com>
On 5/17/2017 5:10 PM, Ananyev, Konstantin wrote:
>>> Hi Ferruh,
>>> Please see my comments/questions below.
>>> Thanks
>>> Konstantin
>>>
>>>> +
>>>> +/**
>>>> + * @file
>>>> + *
>>>> + * RTE Flow Classify Library
>>>> + *
>>>> + * This library provides flow record information with some measured properties.
>>>> + *
>>>> + * Application can select variety of flow types based on various flow keys.
>>>> + *
>>>> + * Library only maintains flow records between rte_flow_classify_stats_get()
>>>> + * calls and with a maximum limit.
>>>> + *
>>>> + * Provided flow record will be linked list rte_flow_classify_stat_xxx
>>>> + * structure.
>>>> + *
>>>> + * Library is responsible from allocating and freeing memory for flow record
>>>> + * table. Previous table freed with next rte_flow_classify_stats_get() call and
>>>> + * all tables are freed with rte_flow_classify_type_reset() or
>>>> + * rte_flow_classify_type_set(x, 0). Memory for table allocated on the fly while
>>>> + * creating records.
>>>> + *
>>>> + * A rte_flow_classify_type_set() with a valid type will register Rx/Tx
>>>> + * callbacks and start filling flow record table.
>>>> + * With rte_flow_classify_stats_get(), pointer sent to caller and meanwhile
>>>> + * library continues collecting records.
>>>> + *
>>>> + * Usage:
>>>> + * - application calls rte_flow_classify_type_set() for a device
>>>> + * - library creates Rx/Tx callbacks for packets and start filling flow table
>>>
>>> Does it necessary to use an RX callback here?
>>> Can library provide an API like collect(port_id, input_mbuf[], pkt_num) instead?
>>> So the user would have a choice either setup a callback or call collect() directly.
>>
>> This was also comment from Morten, I will update RFC to use direct API call.
>>
>>>
>>>> + * for that type of flow (currently only one flow type supported)
>>>> + * - application calls rte_flow_classify_stats_get() to get pointer to linked
>>>> + * listed flow table. Library assigns this pointer to another value and keeps
>>>> + * collecting flow data. In next rte_flow_classify_stats_get(), library first
>>>> + * free the previous table, and pass current table to the application, keep
>>>> + * collecting data.
>>>
>>> Ok, but that means that you can't use stats_get() for the same type
>>> from 2 different threads without explicit synchronization?
>>
>> Correct.
>> And multiple threads shouldn't be calling this API. It doesn't store
>> previous flow data, so multiple threads calling this only can have piece
>> of information. Do you see any use case that multiple threads can call
>> this API?
>
> One example would be when you have multiple queues per port,
> managed/monitored by different cores.
> BTW, how are you going to collect the stats in that way?
>
>>
>>>
>>>> + * - application calls rte_flow_classify_type_reset(), library unregisters the
>>>> + * callbacks and free all flow table data.
>>>> + *
>>>> + */
>>>> +
>>>> +enum rte_flow_classify_type {
>>>> + RTE_FLOW_CLASSIFY_TYPE_GENERIC = (1 << 0),
>>>> + RTE_FLOW_CLASSIFY_TYPE_MAX,
>>>> +};
>>>> +
>>>> +#define RTE_FLOW_CLASSIFY_TYPE_MASK = (((RTE_FLOW_CLASSIFY_TYPE_MAX - 1) << 1) - 1)
>>>> +
>>>> +/**
>>>> + * Global configuration struct
>>>> + */
>>>> +struct rte_flow_classify_config {
>>>> + uint32_t type; /* bitwise enum rte_flow_classify_type values */
>>>> + void *flow_table_prev;
>>>> + uint32_t flow_table_prev_item_count;
>>>> + void *flow_table_current;
>>>> + uint32_t flow_table_current_item_count;
>>>> +} rte_flow_classify_config[RTE_MAX_ETHPORTS];
>>>> +
>>>> +#define RTE_FLOW_CLASSIFY_STAT_MAX UINT16_MAX
>>>> +
>>>> +/**
>>>> + * Classification stats data struct
>>>> + */
>>>> +struct rte_flow_classify_stat_generic {
>>>> + struct rte_flow_classify_stat_generic *next;
>>>> + uint32_t id;
>>>> + uint64_t timestamp;
>>>> +
>>>> + struct ether_addr src_mac;
>>>> + struct ether_addr dst_mac;
>>>> + uint32_t src_ipv4;
>>>> + uint32_t dst_ipv4;
>>>> + uint8_t l3_protocol_id;
>>>> + uint16_t src_port;
>>>> + uint16_t dst_port;
>>>> +
>>>> + uint64_t packet_count;
>>>> + uint64_t packet_size; /* bytes */
>>>> +};
>>>
>>> Ok, so if I understood things right, for generic type it will always classify all incoming packets by:
>>> <src_mac, dst_mac, src_ipv4, dst_ipv4, l3_protocol_id, src_port, dst_port>
>>> all by absolute values, and represent results as a linked list.
>>> Is that correct, or I misunderstood your intentions here?
>>
>> Correct.
>>
>>> If so, then I see several disadvantages here:
>>> 1) It is really hard to predict what kind of stats is required for that particular cases.
>>> Let say some people would like to collect stat by <dst_mac,, vlan> ,
>>> another by <dst_ipv4,subnet_mask>, third ones by <l4 dst_port> and so on.
>>> Having just one hardcoded filter doesn't seem very felxable/usable.
>>> I think you need to find a way to allow user to define what type of filter they want to apply.
>>
>> The flow type should be provided by applications, according their needs,
>> and needs to be implemented in this library. The generic one will be the
>> only one implemented in first version:
>> enum rte_flow_classify_type {
>> RTE_FLOW_CLASSIFY_TYPE_GENERIC = (1 << 0),
>> RTE_FLOW_CLASSIFY_TYPE_MAX,
>> };
>>
>>
>> App should set the type first via the API:
>> rte_flow_classify_type_set(uint8_t port_id, uint32_t type);
>>
>>
>> And the stats for this type will be returned, because returned type can
>> be different type of struct, returned as void:
>> rte_flow_classify_stats_get(uint8_t port_id, void *stats);
>
> I understand that, but it means that for every different filter user wants to use,
> someone has to update the library: define a new type and write a new piece of code to handle it.
> That seems not flexible and totally un-extendable from user perspective.
> Even HW allows some flexibility with RX filters.
> Why not allow user to specify a classification filter he/she wants for that particular case?
> In a way both rte_flow and rte_acl work?
>
>>
>>> I think it was discussed already, but I still wonder why rte_flow_item can't be used for that approach?
>>
>>
>>> 2) Even one 10G port can produce you ~14M rte_flow_classify_stat_generic entries in one second
>>> (all packets have different ipv4/ports or so).
>>> Accessing/retrieving items over linked list with 14M entries - doesn't sound like a good idea.
>>> I'd say we need some better way to retrieve/present collected data.
>>
>> This is to keep flows, so I expect the numbers will be less comparing to
>> the packet numbers.
>
> That was an extreme example to show how bad the selected approach should behave.
> What I am trying to say: we need a way to collect and retrieve stats in a quick and easy way.
> Let say right now user invoked stats_get(port=0, type=generic).
> Now, he is interested to get stats for particular dst_ip only.
> The only way to get it: walk over whole list stats_get() returned and examine each entry one by one.
>
> I think would be much better to have something like:
>
> struct rte_flow_stats {timestamp; packet_count; packet_bytes; ..};
>
> <fill rte_flow_item (or something else) to define desired filter>
>
> filter_id = rte_flow_stats_register(.., &rte_flow_item);
> ....
> struct rte_flow_stats stats;
> rte_flow_stats_get(..., filter_id, &stats);
>
> That allows user to define flows to collect stats for.
> Again in that case you don't need to worry about when/where to destroy the previous
> version of your stats.
Except from using rte_flow, above suggest instead of:
- set key/filter
- poll collect()
- when ever app wants call stats_get()
using:
- poll stats_get(key/filter);
specially after switched from callbacks to polling, this makes sense
because application already will have to do to continuous calls to this
library. Merging set filter/collect/stats_get into same function saves
library from storing/deleting stats until app asks for them, as you
mentioned above.
So, I will update RFC according.
> Of course the open question is how to treat packets that would match more than one flow
> (priority/insertion order/something else?), but I suppose we'll need to deal with that question anyway.
>
> Konstantin
>
>> It is possible to use fixed size arrays for this. But I think it is easy
>> to make this switch later, I would like to see the performance effect
>> before doing this switch. Do you think is it OK to start like this and
>> give that decision during implementation?
next prev parent reply other threads:[~2017-05-18 12:12 UTC|newest]
Thread overview: 145+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-20 18:54 [dpdk-dev] [RFC 17.08] Flow classification library 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 [this message]
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
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=b7a8a427-d207-ee8c-5800-7bb73907a671@intel.com \
--to=ferruh.yigit@intel.com \
--cc=dev@dpdk.org \
--cc=john.mcnamara@intel.com \
--cc=konstantin.ananyev@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).