All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ivan Malov <Ivan.Malov@oktetlabs.ru>
To: Eli Britstein <elibr@nvidia.com>,
	Ilya Maximets <i.maximets@ovn.org>,
	dev@dpdk.org
Cc: Smadar Fuks <smadarf@marvell.com>,
	Hyong Youb Kim <hyonkim@cisco.com>,
	Kishore Padmanabha <kishore.padmanabha@broadcom.com>,
	Ori Kam <orika@nvidia.com>,
	Ajit Khaparde <ajit.khaparde@broadcom.com>,
	Jerin Jacob <jerinj@marvell.com>, John Daley <johndale@cisco.com>,
	Thomas Monjalon <thomas@monjalon.net>,
	Ferruh Yigit <ferruh.yigit@intel.com>,
	Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Subject: Re: [dpdk-dev] [RFC PATCH] ethdev: clarify flow action PORT ID semantics
Date: Tue, 1 Jun 2021 17:49:25 +0300	[thread overview]
Message-ID: <82adfa25-f282-4072-7c68-ccf5f2d0ae57@oktetlabs.ru> (raw)
In-Reply-To: <9ed06b56-26e1-5812-e357-81147e699b3b@nvidia.com>

Hi Eli,

On 01/06/2021 16:24, Eli Britstein wrote:
> 
> On 6/1/2021 3:10 PM, Ilya Maximets wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> 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.
> 
> It doesn't make any sense to attach the VF itself to OVS, but only its 
> representor.

Sure. But that doesn't invalidate the idea of the patch.

> 
> For the PF, when in switchdev mode, it is the "uplink representor", so 
> it is also a representor.
> 

No. According to the existing "port representors" documentation, the 
admin PF port "represents itself", that is the PF, and by no means it 
represents the underlying upstream port. And this makes really big 
difference. One can indeed state that plugging VFs and not their 
reprsentors to DPDK/OvS is useless, but the same statement is not 
applicable to the admin's PF.

> That said, OVS does not care of the type of the port. It doesn't matter 
> if it's an "upstream" or not, or if it's a representor or not.
> 

 From the high-level standpoint, indeed, the port type is a don't care 
to OvS, but the truth is that DPDK offload path in OvS, being a 
lower-level component, must respect all underlying DPDK primitives' 
original meaning. Agreeing the top-level expectations (OvS) with the 
lower-level means (DPDK flow library) *is* effectively the proper job of 
app integration. And if for some reason the existing DPDK component 
misreads the lower-level action real semantics, it cannot be justified 
by high-level principles of OvS.

> 
>> 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.
> I am not sure how this is related.
>>
>> 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.
>> 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.
> Right. This is the way representors work. It is fully aligned with 
> configuration of OVS-kernel.
>>
>> If some specific use-case requires to distinguish VF from the 
>> representor,
>> there should probably be a separate special API/flag for that.
>>
>> Best regards, Ilya Maximets.

-- 
Ivan M

  parent reply	other threads:[~2021-06-01 14:49 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-01 11:14 [dpdk-dev] [RFC PATCH] ethdev: clarify flow action PORT ID semantics Ivan Malov
2021-06-01 12:10 ` Ilya Maximets
2021-06-01 13:24   ` Eli Britstein
2021-06-01 14:35     ` Andrew Rybchenko
2021-06-01 14:44       ` Eli Britstein
2021-06-01 14:50         ` Ivan Malov
2021-06-01 14:53         ` Andrew Rybchenko
2021-06-02  9:57           ` Eli Britstein
2021-06-02 10:50             ` Andrew Rybchenko
2021-06-02 11:21               ` Eli Britstein
2021-06-02 11:57                 ` Andrew Rybchenko
2021-06-02 12:36                 ` Ivan Malov
2021-06-03  9:18                   ` Ori Kam
2021-06-03  9:55                     ` Andrew Rybchenko
2021-06-07  8:28                       ` Thomas Monjalon
2021-06-07  9:42                         ` Andrew Rybchenko
2021-06-07 12:08                           ` Ori Kam
2021-06-07 13:21                             ` Ilya Maximets
2021-06-07 16:07                               ` Thomas Monjalon
2021-06-08 16:13                                 ` Thomas Monjalon
2021-06-08 16:32                                   ` Andrew Rybchenko
2021-06-08 18:49                                     ` Thomas Monjalon
2021-06-09 14:31                                       ` Andrew Rybchenko
2021-06-01 14:49     ` Ivan Malov [this message]
2021-06-01 14:28   ` Ivan Malov
2021-06-02 12:46     ` Ilya Maximets
2021-06-02 16:26       ` Andrew Rybchenko
2021-06-02 17:35         ` Ilya Maximets
2021-06-02 19:35           ` Ivan Malov
2021-06-03  9:29             ` Ilya Maximets
2021-06-03 10:33               ` Andrew Rybchenko
2021-06-03 11:05                 ` Ilya Maximets
2021-06-03 11:29               ` Ivan Malov
2021-06-07 19:27                 ` Ilya Maximets
2021-06-07 20:39                   ` Ivan Malov
2021-06-25 13:04       ` Ferruh Yigit
2021-06-02 12:16   ` Thomas Monjalon
2021-06-02 12:53     ` Ilya Maximets
2021-06-02 13:10     ` Andrew Rybchenko
2021-09-03  7:46 ` [dpdk-dev] [PATCH v1] " Andrew Rybchenko

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=82adfa25-f282-4072-7c68-ccf5f2d0ae57@oktetlabs.ru \
    --to=ivan.malov@oktetlabs.ru \
    --cc=ajit.khaparde@broadcom.com \
    --cc=andrew.rybchenko@oktetlabs.ru \
    --cc=dev@dpdk.org \
    --cc=elibr@nvidia.com \
    --cc=ferruh.yigit@intel.com \
    --cc=hyonkim@cisco.com \
    --cc=i.maximets@ovn.org \
    --cc=jerinj@marvell.com \
    --cc=johndale@cisco.com \
    --cc=kishore.padmanabha@broadcom.com \
    --cc=orika@nvidia.com \
    --cc=smadarf@marvell.com \
    --cc=thomas@monjalon.net \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.