Message-ID: <20161216023003.GA90951@knc-06.sc.intel.com>
Date:   Thu, 15 Dec 2016 18:30:04 -0800
From:   "Vishwanathapura, Niranjana" <niranjana.vishwanathapura@...el.com>
To:     "ira.weiny" <ira.weiny@...el.com>
Cc:     Jason Gunthorpe <jgunthorpe@...idianresearch.com>,
        Doug Ledford <dledford@...hat.com>,
        Leon Romanovsky <leon@...nel.org>,
        Jeff Kirsher <jeffrey.t.kirsher@...el.com>,
        "David S. Miller" <davem@...emloft.net>,
        linux-rdma@...r.kernel.org, netdev@...r.kernel.org,
        dennis.dalessandro@...el.com
Subject: Re: [RFC v2 00/10] HFI Virtual Network Interface Controller (VNIC)

On Thu, Dec 15, 2016 at 08:24:05PM -0500, ira.weiny wrote:
>On Thu, Dec 15, 2016 at 11:48:37AM -0700, Jason Gunthorpe wrote:
>> On Thu, Dec 15, 2016 at 01:19:18PM -0500, Doug Ledford wrote:
>> > On 12/15/2016 12:07 PM, Jason Gunthorpe wrote:
>> > > On Thu, Dec 15, 2016 at 11:28:06AM -0500, Doug Ledford wrote:
>> > >
>> > >> 1) Since your intent is to make this work with multiple versions of the
>> > >> hfi drivers, I disagree with Jason that just because there is only one
>> > >> driver today that we should keep it simple.  Design it right from the
>> > >> beginning of multi driver is your intent is, IMO, a better way to go.
>> > >> You'll work out the bugs in the initial implementation and when it comes
>> > >> time to add the second driver, things will go much more smoothly.
>> > >
>> > > If that is your position then this should be a straight up IB ULP that
>> > > works with any IB hardware.
>> >
>> > Yes, see my comments in point #3 of my previous email...
>>
>> Well, I'm not opposed to the vnic idea - Mellanox had (has?) a similar
>> IB driver. There are lots of good reasons to strictly maintain the
>> ethernet presentation.
>
>Agreed.  I'm pretty worried about the idea of putting VNIC into IPoIB.  It
>seems like a force fit at best.
>

Just to add what Jason, Ira already mentioned,
1) This isn't much common code between hfi_vnic and ipoib.
Besides we expect both ipoib and hfi_vnic to function parallely.
Registering with the network stack is also different.
hfi_vnic exchanges encapsulation information via IB MAD interface from OPA
EM which is not the case with ipoib.
We needed minimal set of interfaces (defined in include/rdma/opa_hfi.h in this 
path series) that represents HW.

2) The design is very different. There are no path record queries, QPs etc in 
hfi_vnic.

3) hfi_vnic also does the encapsulation with fabric (OPA) header, so bottom 
driver only puts it on the wire.
Whereas in ipoib, bottom ib device driver does the encapsulation for ipoib.

4) hfi_vnic do not need ib work request/completion structures.
hfi_vnic supports multiple TX/RX queues.

>>
>> There is much more going on here than just changing the LLADDR,
>> essentially everything MAD focused is different compared to ipoib, and
>> it looks like the required datastructures are different too. This is
>> more of a map a mac to a OPA_LRH approach with SA mediated discovery,
>> by my eye.
>>
>> The main share is the 'skb send' part, we've talked about hoisting
>> that out of ipoib in the past anyhow. A generic verb along those lines
>> would probably allow the sdma optimization for hfi for both this new
>> ulp and ipoib without creating such an ugly HFI1 specific interface.
>
>I'm not sure what you mean about "skb send" being used by ipoib.  Right now
>IPoIB already supplies a "generic skb send" for _Verbs_ in ipoib_send.
>
>I don't know what other devices would do to implement ipoib_send?  To me, it
>seems like the abstraction for IPoIB is at the proper layer now.
>
>For OPA, the hfi driver supports both IPoIB and VNIC.  So expecting IPoIB and
>VNIC to use a generic "skb send" in ib_device is going to make hfi1 do a lot of
>work to determine which ULP is calling it or make the interface kind of ugly.
>Either way I don't see how this is better than a separate set of functions.
>
>IMO the cleanest way to "clean up the ugly HFI1 interface" is to just  put the
>VNIC operations into ib_device similar to the iWarp specific structure
>"iw_cm_verbs" which is there today.
>
>If a device supports the VNIC operations then it can set the pointer and if not
>it will be NULL.  VNIC will look for that pointer for the support it needs.  If
>in the future other devices need modifications to that interface we can modify
>it then.
>
>Ira

Yes, I agree. The interface defined in include/rdma/opa_hfi.h in this patch 
series is pretty simple and generic interface that represents the HW.
If we include this file and put the hfi_vnic_ctrl_ops directly in ib_device 
structure, then it will simplify lot of stuff. We don't need to abstract
out hfi_ibdev and define any ib device capability flag for VNIC support.

>
>>
>> Jason
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>> the body of a message to majordomo@...r.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists