> > > 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. > > > > > >
