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

["Ananyev, Konstantin" <konstantin.ananyev@intel.com>]

> > > 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)
> > >
> > > 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.
If that is not clearly stated in the doc - it might be just the gap in the documentation,
that needs to be fixed, not a mandate to break existing behavior.   

> 
> > 1. Have this proposed API to configure representor itself while
> >     setting config to representor and configuring VF if special
> >     bit enabled.
> > 2. Reverse the logic of current proposal, i.e. always apply
> >     configuration to VF while working with representor and apply
> >     configuration to representor itself if special bit is set.
> >
> > I'd probably prefer option #2, because:
> > - From the OVS and OpenStack point of view, I think, we don't
> >    really need to configure representor itself in most cases.
> >    And OVS really should not know if it works with representor
> >    or some real port.

+1 to keep existing behavior if possible.

 
> I don't thinks OVS can be really agnostic to the fact it runs on top of representors:
> 1. probing of representor has different command line -w <bdf>,representor=XXX
> 2. the whole acceleration framework based on insertion of flow rules for direct forward from the VF to target entity. Rules are applied on
> the representor and would not work if port is not such.
> 3. some multi-port devices cannot do direct fwd between its different port. This is why rep has switch_id and application should query it and
> act upon.
> 4. representor carry the VF port id. This is how application know to which VF (or vport) they associated with on their other side.
> 
> > - It seems that most of the existing code in DPDK already works
> >    like this, i.e. applying configs to VF itself.  Intel drivers
> >    works like this and  Mellanox drivers, as Thomas said, doesn't
> >    have this functionality at all.
> 
> As I said above, I don't think we need to refer to specific driver behavior, rather the API guidelines.
> To me, it is a bit strange and not natural that ethdev configuration is applied to different port w/o any explicit request from the application.
> This is why I would prefer #1 above.
> 
> >
> > >
> > >
> > >>>> The this is that this new API will produce conceptual fragmentation
> > >>>> between DPDK and the Linux kernel, because to do the same thing
> > >>>> you'll have to use different ways. I mean, to change mac of VF in
> > >>>> kernel you need to set mac to the representor, but in DPDK changing
> > >>>> setting mac to representor will lead to changing the mac of the
> > >>>> representor itself, not the VF. This will be really confusing for users.
> > >>>
> > >>> I am not responsible of the choices in Linux.
> > >>> But I agree it would be interesting to check the reason of such decision.
> > >>> Rony, please could you explain?
> > >
> > > I looked at few Linux drivers:
> > >
> > > 	bnxt_vf_rep_netdev_ops has no op to set MAC
> > > 	bnxt_netdev_ops.ndo_set_vf_mac = set VF MAC from PF
> > >
> > > 	lio_vf_rep_ndev_ops has no op to set MAC
> > > 	lionetdevops.ndo_set_vf_mac = set VF MAC from PF
> > >
> > > 	mlx5e_netdev_ops_rep has no op to set MAC
> > > 	mlx5e_netdev_ops.ndo_set_vf_mac = set VF MAC from PF
> > > 	mlx5e_netdev_ops_uplink_rep.ndo_set_vf_mac = set VF MAC from
> > PF
> > >
> > > 	nfp_repr_netdev_ops.ndo_set_mac_address = set representor
> > MAC
> > > 	nfp_repr_netdev_ops.ndo_set_vf_mac = set VF MAC from
> > representor
> > > 	nfp_net_netdev_ops.ndo_set_vf_mac = set VF MAC from PF
> > >
> > > There is a big chance that the behaviour is not standardized in Linux
> > > (as usual). So it is already confusing for users of Linux.
> > >
> > >