Hi Andrew, Ori,
On 04/10/2021 13:47, Andrew Rybchenko wrote:
On 10/4/21 3:00 AM, Ivan Malov wrote:
Hi Ori,
On 04/10/2021 00:09, Ori Kam wrote:
Hi Ivan,
-----Original Message-----
From: Ivan Malov <ivan.ma...@oktetlabs.ru>
Subject: Re: [PATCH v1 01/12] ethdev: add ethdev item to flow API
Hi Ori,
On 03/10/2021 14:52, Ori Kam wrote:
Hi Andrew and Ivan,
-----Original Message-----
From: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru>
Sent: Friday, October 1, 2021 4:47 PM
Subject: [PATCH v1 01/12] ethdev: add ethdev item to flow API
From: Ivan Malov <ivan.ma...@oktetlabs.ru>
For use with "transfer" flows. Supposed to match traffic transmitted
by the DPDK application via the specified ethdev, at e-switch level.
Must not be combined with attributes "ingress" / "egress".
Signed-off-by: Ivan Malov <ivan.ma...@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru>
---
[Snip]
/** Generate flow_action[] entry. */ diff --git
a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h index
7b1ed7f110..880502098e 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -574,6 +574,15 @@ enum rte_flow_item_type {
* @see struct rte_flow_item_conntrack.
*/
RTE_FLOW_ITEM_TYPE_CONNTRACK,
+
+ /**
+ * [META]
+ *
+ * Matches traffic at e-switch going from (sent by) the given
ethdev.
+ *
+ * @see struct rte_flow_item_ethdev
+ */
+ RTE_FLOW_ITEM_TYPE_ETHDEV,
};
/**
@@ -1799,6 +1808,24 @@ static const struct rte_flow_item_conntrack
rte_flow_item_conntrack_mask = { }; #endif
+/**
+ * @warning
+ * @b EXPERIMENTAL: this structure may change without prior notice
+ *
+ * Provides an ethdev ID for use with items which are as follows:
+ * RTE_FLOW_ITEM_TYPE_ETHDEV.
+ */
+struct rte_flow_item_ethdev {
+ uint16_t id; /**< Ethdev ID */
True for all above uses,
should this be renamed to port?
I'd not rename it to "port". The very idea of this series is to
disambiguate
things. This structure is shared between primitives ETHDEV and
ESWITCH_PORT. If we go for "port", then in conjunction with ESWITCH_PORT
the structure name may trick readers into thinking that the ID in
question is
the own ID of the e-switch port itself. But in fact this ID is an
ethdev ID which
is associated with the e-switch port.
Should you wish to elaborate on your concerns with regard to naming,
please
do so. I'm all ears.
Fully understand and agree that the idea is to clear the ambiguaty.
My concern is that we don't use ethdev id, from application ethdev has
only
ports, so what is the id? (if we keep this, we should document that
the id is the
port)
What about ETHDEV_PORT and ESWITCH_PORT?
I understand that, technically, the only ports which the application can
really interface with are ethdevs. So, terms "ethdev" and "port" may
appear synonymous to the application - you are right on that. But, given
the fact that we have some primitives like PHY_PORT and the likes, which
also have "PORT" in their names, I'd rather go for "ethdev" as more
precise term.
But let me assure you: I'm not saying that my opinion should prevail.
I'm giving more thoughts to this in the background. Maybe Andrew can
join this conversation as well.
As far as I can see ethdev API uses 'port_id' everywhere to
refer to ethdev port by its number. So, I suggest
struct rte_flow_item_ethdev {
uint16_t port_id; /**< ethdev port ID */
};
Basically I agree with Ori, that just "id" is a bit confusing
even when it is a member of the _ethdev structure, but I'd
prepend "port_" a field name to sync with ethdev API which
uses port_id. So, we have ethdev->port_id.
Ack
Andrew.
--
Ivan M
