On Mon, Mar 08, 2021 at 08:33:03AM -0800, Alexander Duyck wrote: > On Sun, Mar 7, 2021 at 11:19 AM Leon Romanovsky <leon@xxxxxxxxxx> wrote: > > > > On Sun, Mar 07, 2021 at 10:55:24AM -0800, Alexander Duyck wrote: > > > On Sun, Feb 28, 2021 at 11:55 PM Leon Romanovsky <leon@xxxxxxxxxx> wrote: > > > > > > > > From: Leon Romanovsky <leonro@xxxxxxxxxx> > > > > > > > > @Alexander Duyck, please update me if I can add your ROB tag again > > > > to the series, because you liked v6 more. > > > > > > > > Thanks > > > > > > > > --------------------------------------------------------------------------------- > > > > Changelog > > > > v7: > > > > * Rebase on top v5.12-rc1 > > > > * More english fixes > > > > * Returned to static sysfs creation model as was implemented in v0/v1. > > > > > > Yeah, so I am not a fan of the series. The problem is there is only > > > one driver that supports this, all VFs are going to expose this sysfs, > > > and I don't know how likely it is that any others are going to > > > implement this functionality. I feel like you threw out all the > > > progress from v2-v6. > > > > I'm with you here and tried to present the rationale in v6 when had > > a discussion with Bjorn, so it is unfair to say "you threw out". > > > > Bjorn expressed his preference, and no one came forward to support v6. > > Sorry, it wasn't my intention to be accusatory. I'm just not a fan of > going back to where we were with v1. > > With that said, if it is what Bjorn wants then you are probably better > off going with that. However if that is the direction we are going in > then you should probably focus on getting his Reviewed-by or Ack since > he will ultimately be the maintainer for the code. I hope that he will do it soon. > > > > > > > I really feel like the big issue is that this model is broken as you > > > have the VFs exposing sysfs interfaces that make use of the PFs to > > > actually implement. Greg's complaint was the PF pushing sysfs onto the > > > VFs. My complaint is VFs sysfs files operating on the PF. The trick is > > > to find a way to address both issues. > > > > It is hard to say something meaningful about Greg's complain, he was > > added in the middle of the discussion without much chances to get full > > picture. > > Right, but what I am getting at is that the underlying problem is that > you either have sysfs being pushed onto a remote device, or sysfs that > is having to call into another device. It's not exactly something we > have had precedent for enabling before, and either perspective seems a > bit ugly. I don't agree with the word "ugly", but it is not the point. The point is that this interface is backed by the ecosystem and must-to-be for the right SR-IOV utilization. > > > > > > > Maybe the compromise is to reach down into the IOV code and have it > > > register the sysfs interface at device creation time in something like > > > pci_iov_sysfs_link if the PF has the functionality present to support > > > it. > > > > IMHO, it adds nothing. > > My thought was to reduce clutter. As I mentioned before with this > patch set we are enabling sysfs for functionality that is currently > only exposed by one device. I'm not sure it will be used by many > others or not. Having these sysfs interfaces instantiated at probe > time or at creation time in the case of VFs was preferable to me. I said that in v6 to Bjorn, that I expect up to 2-3 vendors to support this knob. There are not many devices in the market that are comparable to the mlx5 both in their complexity and adoption. Thanks