On 05-Dec-18 22:45, Parav Pandit wrote: > > >> -----Original Message----- >> From: linux-rdma-owner@xxxxxxxxxxxxxxx <linux-rdma- >> owner@xxxxxxxxxxxxxxx> On Behalf Of Jason Gunthorpe >> Sent: Wednesday, December 5, 2018 2:03 PM >> To: Hefty, Sean <sean.hefty@xxxxxxxxx> >> Cc: Dalessandro, Dennis <dennis.dalessandro@xxxxxxxxx>; Gal Pressman >> <galpress@xxxxxxxxxx>; Doug Ledford <dledford@xxxxxxxxxx>; Alexander >> Matushevsky <matua@xxxxxxxxxx>; Yossi Leybovich >> <sleybo@xxxxxxxxxx>; linux-rdma@xxxxxxxxxxxxxxx; Tom Tucker >> <tom@xxxxxxxxxxxxxxxxxxxxx> >> Subject: Re: [PATCH rdma-next 00/13] Elastic Fabric Adapter (EFA) driver >> >> On Wed, Dec 05, 2018 at 07:14:28PM +0000, Hefty, Sean wrote: >>>>>> As far as usnic, it seems like its the driver that has been >>>> forgotten about. >>>>>> I see 3 commits in 2018. I'm not saying it's important or not, >>>> that's >>>>>> another debate for another thread. >>>>> >>>>> There are alot more than three, and all of them are from non-Cisco >>>>> people trying to maintain this driver. Many are mine. >>>>> >>>>> This is not a plus, it shows why this approach is a burden on the >>>> rest >>>>> of the community. >>>> >>>> That's the point I was beating around the bush about. As a community >>>> we don't need any fire and forget drivers where it gets upstreamed >>>> and abandoned. >>> >>> usNIC is a very simple device. I'm not sure you can assume that it >>> isn't used or was forgotten just because it's done. I do know that it >>> is actively maintained on the user space side, even if it doesn't need >>> anything more from the kernel. >> >> The driver still had security bugs I had to fix. >> >> I'm not sure it follows our security model (I have this guess it actually needs >> CAP_NET_RAW) >> >> This is all bad. > > I was about to reply same comment for 9th patch for function rdma_link_layer efa_port_link_layer() which says its Ethernet. > But I see your reply already, replying here. > Ethernet link layer has well defined L2 headers. > So there is intention to create such headers in user space without actually netdev->ifindex attached to UD or SRC QP, CAP_NET_RAW must be added. There is no intention to create headers in userspace, this is done by the device hence no need to add CAP_NET_RAW. The physical link layer is Ethernet, but for all practical purposes it shouldn't matter to the driver/userspace provider. > Kernel cannot depend on external network's robustness, even though it may be. > Same for the rx path too. > >> >>> I agree that exposing usNIC through verbs was wrong. However, as a >>> device, I don't see that it's that functionally different than QIB, >>> truescale, or hfi1. None of those are verbs devices, and throwing a >>> software verbs implementation over them doesn't really change that. >> >> The Linux standard that has evolved is that a common verbs driver can >> provide extensions beyond verbs through the driver private interface and >> still be 'drivers/infiniband' >> >> This makes sense in all cases so far as the extensions lean heavily on the >> core code to work properly, ie for addressing/security/etc. >> >> QIB/HFI are sort of marginal exceptions since their extension was not >> designed well as a proper extension, but that is more of a technical failing >> than a philosophical one. >> >> So at the userspace level psm/psm2/ucx may be distinct non-verbs things, >> but in the kernel we largely treat them as verbs extensions. >> >>>> Now if we do want to say a driver must support verbs it can't be >>>> wishy-washy. We should specify exactly the set of verbs that must be >>>> supported. The IBTA has the notion of some mandatory verbs right, so >>>> should that be the low bar? Should we whittle that down further? >>> >>> If you go this path, rename the subsystem to linux-verbs or >>> linux-ibverbs. >> >> It *is* the verbs subsystem - that is the only API it provides to the rest of the >> kernel today. >> >>> However, if the subsystem will support exposing non-verbs interfaces >>> (usnic, psm, psm2), then I disagree with placing a requirement that >>> the driver must also provide a software implementation of verbs over >>> it. >> >> This is what I mean when I talked about evolution earlier. If want to do non- >> verbs then do it right, not as a special QP hack like usnic did. >> >> Jason
