Re: [dpdk-dev] [PATCH v2 0/3] ethdev: configure SR-IOV VF from host

[Thomas Monjalon <thomas@monjalon.net>]

Bad news for the DPDK API below:

03/11/2019 23:09, Thomas Monjalon:
> 03/11/2019 16:27, Ananyev, Konstantin:
> > 
> > > > > If we consider what Intel did, i.e. configure VF in place of
> > > > > representor for some operations, there are two drawbacks:
> > > > > - confusing that some ops apply to representor, others apply to VF
> > > > > - some ops are not possible on representor (because targetted to VF)

It seems this confusion and limitation is not scary enough to convince
of the right approach.

Most probably this figure was not well understood:

VF1 ------ VF1 rep /--------\
                   | switch | uplink rep ------ uplink ------ wire
VF2 ------ VF2 rep \--------/    (PF)

Please understand that a packet going from the VF rep to the VF,
is Tx from the representor point of view,
while it is Rx from the VF point of view.
It should explain why VF and rep are different.

One more explanation: doing Rx/Tx from the VF rep is the way,
for a vSwitch, to manage the forwarding in CPU,
before offloading it in the HW switch with flow rules.

After reading how the representor is implemented in Intel PMDs,
I understand there is no intent to do some vSwitch offload with SR-IOV.
The Mellanox PMD is capable of such full offload, that's why
the representor port must be a real DPDK port, not a ghost.


> > > > >
> > > > > I still feel that the addition of one single bit in the port ID is an
> > > > > elegant solution to target either the VF or its representor.
> > > >
> > > > Since we already have a confusion about what is configured when operations
> > > > are performed on a representor port we have 2 options:
> > > 
> > > I don't agree we have. I don't think there is any design note or API doc that says the ethdev configuration on representor should be applied
> > > on VF (please share if I missed it).
> > > The fact that there are some drivers that implemented it doesn't mean it is correct.
> > 
> > Well, it means that at least authors and reviewers of these patches,
> > plus probably next-net maintainers believe that it is correct.
> > At least they did - when patch was applied.

For information, a Mellanox paper of a netdevconf talk in 2016,
which is giving more information about the ideas behind the switchdev model:
https://netdevconf.info/1.2/papers/efraim-gerlitz-sriov-ovs-final.pdf


> > If that is not clearly stated in the doc - it might be just the gap in the documentation,
> 
> Gap in the documentation? We should state that the config should be applied
> to the port specified with port_id and no other one? Funny
> 
> > that needs to be fixed, not a mandate to break existing behavior.
> 
> So because you managed to have a wrong patch applied in your PMD,
> you want to make it the generic API in ethdev?
> What a process!
> 
> Hey guys, if you want to change an API behaviour in a way others don't,
> you just have to implement what you want in your PMD silently,
> then you will be able to change the API to comply with your behaviour.
> Wonderful.
> If we allow such practice, DPDK is dead.

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.
It was asked to implement the representor operations on the representor
port with a special flag to make explicit a non-VF operation.
I really tried to implement this reversed logic.
One blocker is to check the port ID flag in every functions,
including the inline Rx/Tx. So I came to the conclusion that the
reverse logic is really ugly and it has no short-term benefit
compared to the mix we have currently.
After long hours thinking in the night, I prefer focusing on the release.
It means we will continue to mix VF and representor operations
with the same port ID. For the record, I believe it is very bad.