> -----Original Message----- > From: Jason Gunthorpe <jgg@xxxxxxxx> > Sent: Wednesday, December 5, 2018 5:13 PM > To: Hefty, Sean <sean.hefty@xxxxxxxxx> > Cc: Parav Pandit <parav@xxxxxxxxxxxx>; 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 10:37:08PM +0000, Hefty, Sean wrote: > > > 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. > > > > I haven't seen anything in the reviews that indicate that user space > > is able to create its own headers over Ethernet. It looks like it > > uses a proprietary addressing scheme with one address per device, and > > my guess is it relies on the SRD/UD QP to generate the headers. > > It has a 16 byte opaque blob for the address (see struct > efa_admin_create_ah_cmd) - who knows what is in there. > > If it includes a MAC address then this driver needs CAP_NET_RAW. > > If it includes an IP address, and the MAC address is shared with a netdev in > the system, then it also needs CAP_NET_RAW. > > Who knows what the format of the UD WR's are, maybe they wrongly > include headers to transmit. > efa_port_link_layer() need to return more appropriate link layer that reflects the addresses defined in ah. > And the same questions belatedly apply to usnic. > > > It supports PD calls, but I'm unsure if that's really a thing. On the > > surface it looks like EFA is a simplified verbs-like device that > > supports a proprietary transport, somewhat equivalent to an IB/RoCE > > device that only supports UD QPs. > > The admin mailboxes are passing around a PD #, so to me it looks like there > is no HW object backing the PD but the HW has all the information to do > required per-PD security enforcement.. > > > Adding a new QP type looks sensible. > > We are absolutely not adding new things to common verbs that are not well > specified to the level that someone else can implement them (let alone use > them). I've been adament about this point for a long, long time now. > > > I think the mistake in the tree is either having different protocols > > map to the same QP type value, or not having a separate protocol field > > provided as part of create QP. > > The QP type value specifies the API contract to the verbs consumer. UD, RC, > etc all have well defined semantics that must be adhered to. IIRC usnic broke > this too by using a UD QP type that did not meet all the requirements. > > Like usnic, I have no idea if EFA follows the contract for UD or not. There is > no documentation and no verbs provider to inspect. > > Yes, the wire protocol behind the API contract cannot be specified with > existing verbs, that need has yet to come up, and could be met someday > with a create_qp extension. > > At least in all present cases the various wire protocols are so different as to > need a different 'struct ib_device' anyhow. > > Jason
