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?
Best,
Ori
> >
> >> +};
> >> +
> >> +/** Default mask for RTE_FLOW_ITEM_TYPE_ETHDEV */ #ifndef
> >> __cplusplus
> >> +static const struct rte_flow_item_ethdev rte_flow_item_ethdev_mask
> = {
> >> + .id = 0xffff,
> >> +};
> >> +#endif
> >> +
> >> /**
> >> * Matching pattern item definition.
> >> *
> >> --
> >> 2.30.2
>
> --
> Ivan M
