DPDK patches and discussions
 help / color / mirror / Atom feed
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: Wed, 17 May 2017 16:37:10 +0100	[thread overview]
Message-ID: <23ee1b56-ded8-024d-9668-9a4b0b830130@intel.com> (raw)
In-Reply-To: <2601191342CEEE43887BDE71AB9772583FAF774A@IRSMSX109.ger.corp.intel.com>

On 5/17/2017 3:54 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?

> 
>> + *  - 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 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.
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?

  reply	other threads:[~2017-05-17 15:37 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 [this message]
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
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=23ee1b56-ded8-024d-9668-9a4b0b830130@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).