Re: [dpdk-dev] [RFC PATCH] ethdev: clarify flow action PORT ID semantics

[Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>]

On 6/2/21 3:16 PM, Thomas Monjalon wrote:
> 01/06/2021 14:10, Ilya Maximets:
>> On 6/1/21 1:14 PM, Ivan Malov wrote:
>>> By its very name, action PORT_ID means that packets hit an ethdev with the
>>> given DPDK port ID. At least the current comments don't state the opposite.
>>> That said, since port representors had been adopted, applications like OvS
>>> have been misusing the action. They misread its purpose as sending packets
>>> to the opposite end of the "wire" plugged to the given ethdev, for example,
>>> redirecting packets to the VF itself rather than to its representor ethdev.
>>> Another example: OvS relies on this action with the admin PF's ethdev port
>>> ID specified in it in order to send offloaded packets to the physical port.
>>>
>>> Since there might be applications which use this action in its valid sense,
>>> one can't just change the documentation to greenlight the opposite meaning.
>>> This patch adds an explicit bit to the action configuration which will let
>>> applications, depending on their needs, leverage the two meanings properly.
>>> Applications like OvS, as well as PMDs, will have to be corrected when the
>>> patch has been applied. But the improved clarity of the action is worth it.
>>>
>>> The proposed change is not the only option. One could avoid changes in OvS
>>> and PMDs if the new configuration field had the opposite meaning, with the
>>> action itself meaning delivery to the represented port and not to DPDK one.
>>> Alternatively, one could define a brand new action with the said behaviour.
>>
>> We had already very similar discussions regarding the understanding of what
>> the representor really is from the DPDK API's point of view, and the last
>> time, IIUC, it was concluded by a tech. board that representor should be
>> a "ghost of a VF", i.e. DPDK APIs should apply configuration by default to
>> VF and not to the representor device:
>>   https://patches.dpdk.org/project/dpdk/cover/20191029185051.32203-1-thomas@monjalon.net/#104376
>> This wasn't enforced though, IIUC, for existing code and semantics is still mixed.
> 
> Quoting myself from above link:
> "the representor port must be a real DPDK port, not a ghost."

That days I had no opinion on the topic. Now I tend to agree
with it, but I'm not sure that I understand all implications.
I'm afraid it is more complex:
 - it is a real DPDK port since application can send
   traffic to it, and receive traffic from it
 - however, in our case, it is basically just a direct pipe:
    - packets sent to representor bypass transfer and go
      directly to represented function (VF or PF)
    - packets sent by represented function go to representor
      by default, but transfer rules (HW offload) may change it
   Of course, it is just a vendor implementation detail.
 - I doubt that all ethdev operation makes sense for
   representor itself, but some, for example stats, definitely
   makes sense (representor and represented entity stats could
   differ a lot because of HW offload). So, if operation does
   not make sense or simply not supported, it should return an
   error and that's it.

In fact, I see nothing bad in attaching both representor and
represented entity (VF, PF or sub-function) to the same DPDK
application, for example, for testing purposes. So, it should
behave consistently.

> and
> "During the Technical Board yesterday, it was decided to go with Intel
> understanding of what is a representor, i.e. a ghost of the VF."
> and
> "we will continue to mix VF and representor operations
> with the same port ID. For the record, I believe it is very bad."
> 
>> I still think that configuration should be applied to VF, and the same applies
>> to rte_flow API.  IMHO, average application should not care if device is
>> a VF itself or its representor.  Everything should work exactly the same.
> 
> What means "work exactly the same"?
> Is it considering what is behind the representor silently,
> or considering the representor as a real port?
> 
> There is a need to really consider representor port as any other port,
> and stop this ugly mix. I want to propose such change again for DPDK 21.11.
> To me the real solution is to use a bit in the port id of a representor
> for explicitly identifying the port behind the representor.
> This bit could be translated as a flag or a sign in testpmd text grammar.

+1, if so, it will allow to use it in PORT_ID action and item
without any changes in flow API. Just clarification in
documentation what it means for various cases.

However, I'd like to draw attention to network port <-> PF
ethdev port case. Strictly speaking it is not a representor
(as I tried to proove in other mail) and requires to be a
part of solution.

>> I think this matches with the original idea/design of the switchdev functionality
>> in the linux kernel and also matches with how the average user thinks about
>> representor devices.
> 
> There is no "average" user or application, just right and wrong.
> In the switchdev model, a representor is a port of a switch like
> any other port, not a ghost of its peer.
> 
>> If some specific use-case requires to distinguish VF from the representor,
>> there should probably be a separate special API/flag for that.
> 
> Yes, port ID of a representor must be the representor itself,
> and a bit can help reaching the port behind the representor.

+1 with clarification of network port <-> PF ethdev case
semantics which is not an easy task: for example,
when we configure ethdev port and specify speed capabilities,
it is really configuration of the associated upstream network
port, but we still apply it to ethdev port.