On 06-Dec-18 04:37, Parav Pandit wrote: > > >> -----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. > We can add a new EFA link layer if it makes sense, the address format is proprietary (does not match existing link types). > >> 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
