Hi Ori,
On 04/10/2021 14:37, Ori Kam wrote:
Hi Ivan,
-----Original Message-----
From: Ivan Malov <ivan.ma...@oktetlabs.ru>
Sent: Monday, October 4, 2021 2:06 PM
Cc: dev@dpdk.org
Subject: Re: [PATCH v1 02/12] ethdev: add eswitch port item to flow API
Hi Ori,
On 04/10/2021 08:45, Ori Kam wrote:
Hi Ivan,
-----Original Message-----
From: Ivan Malov <ivan.ma...@oktetlabs.ru>
Sent: Sunday, October 3, 2021 9:11 PM
Subject: Re: [PATCH v1 02/12] ethdev: add eswitch port item to flow
API
On 03/10/2021 15:40, 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 02/12] ethdev: add eswitch port item to flow API
From: Ivan Malov <ivan.ma...@oktetlabs.ru>
For use with "transfer" flows. Supposed to match traffic entering
the e-switch from the external world (network, guests) via the port
which is logically connected with the given ethdev.
Must not be combined with attributes "ingress" / "egress".
This item is meant to use the same structure as ethdev item.
In case the app is not working with representors, meaning each
switch port is mapped to ethdev.
both items (ethdev and eswitch port ) have the same meaning?
No. Ethdev means ethdev, and e-switch port is the point where this
ethdev is plugged to. For example, "transfer + ESWITCH_PORT" for a
regular PF ethdev typically means the network port (maybe you can
recall the idea that a PF ethdev "represents" the network port it's
associated with).
I believe, that diagrams which these patches add to
"doc/guides/prog_guide/rte_flow.rst" may come in handy to understand
the meaning. Also, you can take a look at our larger diagram from the
Sep 14 gathering.
Lets look at the following system:
E-Switch has 3 ports - PF, VF1, VF2
The ports are distributed as follows:
DPDK application:
ethdev(0) pf,
ethdev(1) representor to VF1
ethdev(2) representor to VF2
ethdev(3) VF1
VM:
VF2
As we know all representors are realy connected to the PF(at least in
this example)
This example tries to say that the e-switch has 3 ports in total, and, given
your explanation, one may indeed agree that *in this example* representors
re-use e-switch port of ethdev=0 (with some metadata to distinguish
packets, etc.). But one can hardly assume that *all* representors with any
vendor's NIC are connected to the e-switch the same way. It's vendor
specific. Well, at least, applications don't have this knowledge and don't need
to.
So matching on ethdev(3) means matching on traffic sent from DPDK port
3 right?
Item ETHDEV (ethdev_id=3) matches traffic sent by DPDK port 3. Looks like
we're on the same page here.
Good.
And matching on eswitch_port(3) means matching in traffic that goes
into VF1 which is the same traffic as ethdev(3) right?
I didn't catch the thought about "the same traffic". Direction is not the same.
Item ESWITCH_PORT (ethdev_id=3) matches traffic sent by DPDK port 1.
This is the critical part for my understanding.
Matching on ethdev_id(3) means matching on traffic that is coming from DPDK
port3.
Right.
So from E-Switch view point it is traffic that goes into VF1?
No. Above you clearly say "coming from DPDK port3". That is, from the
VF1. *Not* going into it. Port 3 (ethdev_id=3) *is* VF1.
While matching on E-Switch_port(3) means matching on traffic coming from VF1?
No. It means matching on traffic coming from ethdev 1. From the VF1's
representor.
And by the same logic matching on ethdev_id(1) means matching on taffic that
was sent
from DPDK port 1 and matching on E-Switch_port(1) means matching on traffic
coming from
VF1
In this case, you've got this right. But please see my above notes. By
the looks of it, you might have run into confusion over there.
So in this case eswitch_port(3) is equal ot eswitch_port(1) right?
While ethdev(1) is not equal to ethdev(3)
No.
Item ETHDEV (ethdev_id=1) equals item ESWITCH_PORT (ethdev_id=3).
Item ETHDEV (ethdev_id=3) equals item ESWITCH_PORT (ethdev_id=1).
And just to complete the picture, matching on ethdev(2) will result in traffic
coming from the dpdk port and matching on eswitch_port(2) will match
on traffic coming from VF2
Exactly.
But, Ori, let me draw your attention to the following issue. In order to
simplify understanding, I suggest that we refrain from saying "traffic
that GOES TO". Where it goes depends on default rules that are supposed
to be maintained by the PMD when ports get plugged / unplugged.
The flow items ETHDEV and ESWITH_PORT define the SOURCE of traffic.
That's it. They define where the traffic "goes FROM".
Say, the DPDK application sends a packet from ethdev 0. This packet
enters the e-switch. Match engine sits in the e-switch and intercepts
the packet. It doesn't care where the packet *would go* if it wasn't
intercepted. It cares about where the packet comes from. And it comes
from ethdev 0. So, in the focus, we have the SOURCE of the packet.
Yes, in this case neither of the ports (1, 3) is truly "external" (they both
interface the DPDK application), but, the thing is, they're "external" *to each
other* in the sense that they sit at the opposite ends of the wire.
Matching on ethdev(1) means matching on the PF port in the E-Switch but
with some
metadata that marks the traffic as coming from DPDK port 1 and not from
VF1 E-Switch
port right?
That's vendor specific. The application doesn't have to know how exactly
this particular ethdev is connected to the e-switch - whether it re-uses
the PF's e-switch port or has its own. The e-switch port that connects
the ethdev with the e-switch is just assumed to exist logically.
While matching on eswitch_port(2) means matching on traffic coming from
the VM right?
Right.
I think the my above question will clear everything for me.
Signed-off-by: Ivan Malov <ivan.ma...@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru>
---
app/test-pmd/cmdline_flow.c | 27
+++++++++++++++++++++
doc/guides/prog_guide/rte_flow.rst | 22 +++++++++++++++++
doc/guides/rel_notes/release_21_11.rst | 2 +-
doc/guides/testpmd_app_ug/testpmd_funcs.rst | 4 +++
lib/ethdev/rte_flow.c | 1 +
lib/ethdev/rte_flow.h | 12 ++++++++-
6 files changed, 66 insertions(+), 2 deletions(-)
diff --git a/app/test-pmd/cmdline_flow.c b/app/test-
pmd/cmdline_flow.c
index e05b0d83d2..188d0ee39d 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -308,6 +308,8 @@ enum index {
ITEM_POL_POLICY,
ITEM_ETHDEV,
ITEM_ETHDEV_ID,
+ ITEM_ESWITCH_PORT,
+ ITEM_ESWITCH_PORT_ETHDEV_ID,
Like my comment from previous patch, I'm not sure the correct
term for ETHDEV is ID is should be port.
Please see my reply in the previous thread. "ethdev" here is an
"anchor", a "beacon" of sorts which allows either to refer namely to
this ethdev or to the e-switch port associated with it.
/* Validate/create actions. */
ACTIONS,
@@ -1003,6 +1005,7 @@ static const enum index next_item[] = {
ITEM_INTEGRITY,
ITEM_CONNTRACK,
ITEM_ETHDEV,
+ ITEM_ESWITCH_PORT,
END_SET,
ZERO,
};
@@ -1377,6 +1380,12 @@ static const enum index item_ethdev[] = {
ZERO,
};
+static const enum index item_eswitch_port[] = {
+ ITEM_ESWITCH_PORT_ETHDEV_ID,
+ ITEM_NEXT,
+ ZERO,
+};
+
static const enum index next_action[] = {
ACTION_END,
ACTION_VOID,
@@ -3632,6 +3641,21 @@ static const struct token token_list[] = {
item_param),
.args = ARGS(ARGS_ENTRY(struct rte_flow_item_ethdev,
id)),
},
+ [ITEM_ESWITCH_PORT] = {
+ .name = "eswitch_port",
+ .help = "match traffic at e-switch going from the external port
associated with the given ethdev",
Missing the word logically since if we are talking about representor the
connected port
is the PF while we want to match traffic on one of the FVs.
Doesn't the word "external" say it all?
Representor Ethdev <--> Admin ethdev's PF <--> E-Switch <--> VF
(external / the most remote endpoint).
Until the last comment External had totally different meaning to me.
I think you should add some place the meaning of external or use
the most remote endpoint.
We'll keep this in mind. Given your above example (when both VF and its
representor) are plugged to the DPDK application, "external" may sound a
bit off, but maybe it can be retained and just clarified as the "most
remote endpoint" in the e-switch with respect to the ethdev in question.
+1 to the most remote or finding different wording.
+ .priv = PRIV_ITEM(ESWITCH_PORT,
+ sizeof(struct rte_flow_item_ethdev)),
+ .next = NEXT(item_eswitch_port),
+ .call = parse_vc,
+ },
+ [ITEM_ESWITCH_PORT_ETHDEV_ID] = {
+ .name = "ethdev_id",
+ .help = "ethdev ID",
+ .next = NEXT(item_eswitch_port,
NEXT_ENTRY(COMMON_UNSIGNED),
+ item_param),
+ .args = ARGS(ARGS_ENTRY(struct rte_flow_item_ethdev,
id)),
+ },
/* Validate/create actions. */
[ACTIONS] = {
.name = "actions",
@@ -8370,6 +8394,9 @@ flow_item_default_mask(const struct
rte_flow_item *item)
case RTE_FLOW_ITEM_TYPE_ETHDEV:
mask = &rte_flow_item_ethdev_mask;
break;
+ case RTE_FLOW_ITEM_TYPE_ESWITCH_PORT:
+ mask = &rte_flow_item_ethdev_mask;
+ break;
Not sure maybe merged the two cases?
A noble idea.
default:
break;
}
diff --git a/doc/guides/prog_guide/rte_flow.rst
b/doc/guides/prog_guide/rte_flow.rst
index ab628d9139..292bb42410 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -1460,6 +1460,28 @@ Use this with attribute **transfer**.
Attributes
**ingress** and
| ``mask`` | ``id`` | zeroed for wildcard match |
+----------+----------+---------------------------+
+Item: ``ESWITCH_PORT``
+^^^^^^^^^^^^^^^^^^^^^^
+
+Matches traffic at e-switch going from the external port associated
+with the given ethdev, for example, traffic from net. port or guest.
Maybe replace external with e-switch?
The word "e-switch" is already here. Basically, all "external" ports are
"e-switch external ports" = ports which connect the e-switch with
non-DPDK world: network ports, VFs passed to guests, etc.
Please see comment above, the word external is not clearly defined. >
+
+::
+
+ * (Ethdev) ~~~~~~~~~~~~ (Internal Port) ~~~~ [] <<<< (External
Port)
+ * : SW : Logical Net / Guest :
+ * : : :
+ * | ---- PMD Layer ---- | ------------ E-Switch Layer ------------ |
+ *
+ * [] shows the effective ("transfer") standpoint, the match engine;
+ * << shows the traffic flow in question hitting the match engine;
+ * ~~ shows logical interconnects between the endpoints.
+
I'm not sure I understand this diagram.
Thanks for the feedback. I have no way with drawing fancy diagrams, so
this one may not be ideal, yes. But could you please elaborate on your
concerns. Maybe you understand it the right way but just unsure. If you
describe what you see when looking at the diagram, I'll be able to
either confirm your vision or debunk any misunderstanding and possibly
improve the drawing.
I will try,
Ethdev is the port that the application sees in DPDK right?
Exactly.
Internal port is what the PMD sees, for example in case of representor it
will see the PF port.
Vendor-specific, but, yes, let's assume that.
External (also due to a drawing in one of your comments) is the E-Switch
end point.
So this drawing shows that the app select the external port that is
connected to the
internal port which is connected to the ethdev port?
So what's the problem?
Ethdev ----- E-switch port A -- ... --- E-switch port B------ VF
So, saying "transfer + ESWITCH_PORT" here implies the e-switch port B.
How do we name it? "External"? "The most remote"? Any other options?
Thanks,
Ori
+Use this with attribute **transfer**. Attributes **ingress** and
+**egress** (`Attribute: Traffic direction`_) must not be used.
+
+This item is meant to use the same structure as `Item: ETHDEV`_.
+
Actions
~~~~~~~
diff --git a/doc/guides/rel_notes/release_21_11.rst
b/doc/guides/rel_notes/release_21_11.rst
index 91631adb4e..b2b27de3f0 100644
--- a/doc/guides/rel_notes/release_21_11.rst
+++ b/doc/guides/rel_notes/release_21_11.rst
@@ -167,7 +167,7 @@ API Changes
Also, make sure to start the actual text at the margin.
=======================================================
-* ethdev: Added item ``ETHDEV`` to flow API.
+* ethdev: Added items ``ETHDEV``, ``ESWITCH_PORT`` to flow API.
* cryptodev: The API rte_cryptodev_pmd_is_valid_dev is modified to
rte_cryptodev_is_valid_dev as it can be used by the application as
diff --
git
a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 6d5de5457c..9a5c2a2d82 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -3824,6 +3824,10 @@ This section lists supported pattern items
and
their attributes, if any.
- ``id {unsigned}``: ethdev ID
+- ``eswitch_port``: match traffic at e-switch going from the external
+port associated with the given ethdev
+
+ - ``ethdev_id {unsigned}``: ethdev ID
+
Actions list
^^^^^^^^^^^^
diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c index
84eb61cb6c..c4aea5625f 100644
--- a/lib/ethdev/rte_flow.c
+++ b/lib/ethdev/rte_flow.c
@@ -101,6 +101,7 @@ static const struct rte_flow_desc_data
rte_flow_desc_item[] = {
MK_FLOW_ITEM(INTEGRITY, sizeof(struct rte_flow_item_integrity)),
MK_FLOW_ITEM(CONNTRACK, sizeof(uint32_t)),
MK_FLOW_ITEM(ETHDEV, sizeof(struct rte_flow_item_ethdev)),
+ MK_FLOW_ITEM(ESWITCH_PORT, sizeof(struct
rte_flow_item_ethdev)),
};
/** Generate flow_action[] entry. */
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h index
880502098e..1a7e4c2e3d 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -583,6 +583,16 @@ enum rte_flow_item_type {
* @see struct rte_flow_item_ethdev
*/
RTE_FLOW_ITEM_TYPE_ETHDEV,
+
+ /**
+ * [META]
+ *
+ * Matches traffic at e-switch going from the external port associated
+ * with the given ethdev, for example, traffic from net. port or guest.
+ *
+ * @see struct rte_flow_item_ethdev
+ */
+ RTE_FLOW_ITEM_TYPE_ESWITCH_PORT,
};
/**
@@ -1813,7 +1823,7 @@ static const struct rte_flow_item_conntrack
rte_flow_item_conntrack_mask = {
* @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.
+ * RTE_FLOW_ITEM_TYPE_ETHDEV,
RTE_FLOW_ITEM_TYPE_ESWITCH_PORT.
*/
struct rte_flow_item_ethdev {
uint16_t id; /**< Ethdev ID */
--
2.30.2
Best,
Ori
--
Ivan M
--
Ivan M
Thanks,
Ori
--
Ivan M
