Fri, May 05, 2017 at 03:17:54PM CEST, leon@xxxxxxxxxx wrote: >On Fri, May 05, 2017 at 08:54:57AM +0200, Jiri Benc wrote: >> On Thu, 4 May 2017 21:02:08 +0300, Leon Romanovsky wrote: >> > In order to close object model, ensure reuse of existing code and make this >> > tool usable from day one, we decided to implement wrappers over legacy sysfs >> > prior to implementing netlink functionality. As a nice bonus, it will allow >> > to use this tool with old kernels too. >> >> This sounds wrong. We don't support legacy ioctl interface for the 'ip' >> command, either. I think rdma should be converted to netlink first and >> the new tool should only use netlink. > >RDMA in slightly different situation than "ip" tool was. "ip" was implemented >when tools like ifconfig existed. It allowed to old and new systems to be >configured to some degree. In RDMA community, there are no similar tools like >"ifconfig". Implementation in netlink-only interface will leave old systems without >common tool at all. > >As an upstream-oriented person, I personally fine with that, but anyway would >like to get wider agreement/disagreement on that, before removing sysfs >parsing logic from the rdmatool. I tend to agree with Jiri Benc. I fear that supporting sysfs + netlink api later on for the same things will make the code unnecessary complex. Also, the legacy sysfs will most likely stay there forever so there will be no actual motivation to port the existing things to the new netlink api. For the prototyping purposes, I belive that what you did makes perfect sense. But for the actual mergable version, my feeling is that we need to strictly stick with new netlink rdma interface and just forget about the old sysfs one. Distros would have to backport the new kernel rdma netlink api. Yes, this will be little bit more painful at the beginning, but in the long run, I believe it will save some severe headaches. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
