DPDK patches and discussions
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas@monjalon.net>
To: Andrey Vesnovaty <andrey.vesnovaty@gmail.com>
Cc: Ferruh Yigit <ferruh.yigit@intel.com>,
	Andrew Rybchenko <arybchenko@solarflare.com>,
	Ori Kam <orika@mellanox.com>,
	dev@dpdk.org
Subject: Re: [dpdk-dev] [RFC] add flow action context API
Date: Wed, 03 Jun 2020 12:02:30 +0200
Message-ID: <318236911.yWikKUQR1j@thomas> (raw)
In-Reply-To: <20200520091801.30163-1-andrey.vesnovaty@gmail.com>

20/05/2020 11:18, Andrey Vesnovaty:
> This commit introduces extension of DPDK flow action API enabling
> modification of single rte_flow_action.
> 
> Motivation and example
> ===
> Adding or removing one or more queues to RSS actions cloned in multiple
> flow rules imposes per rule toll for current DPDK flow API; the scenario
> requires for each flow sharing cloned RSS action:
> - call `rte_flow_destroy()`
> - call `rte_flow_create()` with modified RSS action
> 
> In order to prevent the overhead of multiple RSS flow rules reconfiguration
> API for in-place flow action modification introduced in this commit.

It seems there is an usability improvement with this new API.
If I understand well, the main motivation is to improve the performance?
The PMD implementation should try to keep a shared object
to benefit of the performance improvement, right?


The existing rte_flow API functions are:
	rte_flow_validate()
	rte_flow_create()
	rte_flow_destroy()
	rte_flow_flush()
	rte_flow_query()
	rte_flow_isolate()
	rte_flow_get_aged_flows()

> +	# added in 20.08
> +	rte_flow_action_ctx_create;
> +	rte_flow_action_ctx_destoy;
> +	rte_flow_action_ctx_modify;
> +	rte_flow_action_ctx_query;

We had "create", "destroy", "query", but no "modify" capability.
The new API is adding 2 things in my opinion:
	- shared action object
	- "modify" capability (is "update" a better wording?)

About the wording, do we need "ctx"?
I feel rte_flow_action is a good enough prefix for this API,
and should be documented as a shared action object.
I think the word "object" is more meaningful than "context".
Am I missing something?


> +	/**
> +	 * Describes action context.
> +	 *
> +	 * Enables multiple rules reference the same action by id/ctx.
> +	 *
> +	 * No action specific struct here (void*) since it can be any
> +	 * action type.
> +	 */
> +	RTE_FLOW_ACTION_TYPE_CTX,

Why do we need a new action type?


> @@ -101,6 +101,28 @@ struct rte_flow_ops {
> +	/** See rte_flow_action_ctx_destoy() */
> +	void *(*action_ctx_create)
> +		(struct rte_eth_dev *dev,
> +		const struct rte_flow_action *action,
> +		struct rte_flow_error *error);
> +	/** See rte_flow_action_ctx_create() */
> +	int (*action_ctx_destroy)
> +		(struct rte_eth_dev *dev,
> +		void *ctx,
> +		struct rte_flow_error *error);
> +	/** See rte_flow_action_ctx_modify() */
> +	int (*action_ctx_modify)
> +		(struct rte_eth_dev *dev,
> +		void *ctx,
> +		const void *action_conf,
> +		struct rte_flow_error *error);
> +	/** See rte_flow_action_ctx_query() */
> +	int (*action_ctx_query)
> +		(struct rte_eth_dev *dev,
> +		const void *ctx,
> +		void *data,
> +		struct rte_flow_error *error);

API functions are directly linked to PMD ops, it looks simple and good.



  reply	other threads:[~2020-06-03 10:02 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-20  9:18 Andrey Vesnovaty
2020-06-03 10:02 ` Thomas Monjalon [this message]
2020-06-04 11:12   ` Andrey Vesnovaty
2020-06-04 17:23     ` Thomas Monjalon
2020-06-05  8:30       ` Bruce Richardson
2020-06-05  8:33         ` Thomas Monjalon
2020-06-03 10:53 ` Jerin Jacob
2020-06-04 11:25   ` Andrey Vesnovaty
2020-06-04 12:36     ` Jerin Jacob
2020-06-04 15:57       ` Andrey Vesnovaty
2020-06-09 16:01         ` Jerin Jacob
2020-06-20 13:32           ` [dpdk-dev] [RFC v2 0/1] " Andrey Vesnovaty
2020-06-22 15:22             ` Thomas Monjalon
2020-06-22 17:09               ` Andrey Vesnovaty
2020-06-26 11:44             ` Jerin Jacob
2020-06-28  8:44               ` Andrey Vesnovaty
2020-06-28 13:42                 ` Jerin Jacob
2020-06-29 10:22                   ` Andrey Vesnovaty
2020-06-30  9:52                     ` Jerin Jacob
2020-07-01  9:24                       ` Andrey Vesnovaty
2020-07-01 10:34                         ` Jerin Jacob
2020-06-20 13:32           ` [dpdk-dev] [RFC v2 1/1] add flow shared action API Andrey Vesnovaty
2020-07-02  0:24             ` Stephen Hemminger
2020-07-02  7:20               ` Ori Kam
2020-07-02  8:06                 ` Andrey Vesnovaty

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=318236911.yWikKUQR1j@thomas \
    --to=thomas@monjalon.net \
    --cc=andrey.vesnovaty@gmail.com \
    --cc=arybchenko@solarflare.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=orika@mellanox.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

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git