Re: [RFC 02/10] IB/hfi-vnic: Virtual Network Interface Controller (VNIC) Bus driver

[Jason Gunthorpe <jgunthorpe@obsidianresearch.com>]

On Mon, Nov 21, 2016 at 01:30:17PM -0800, Vishwanathapura, Niranjana wrote:
> On Sat, Nov 19, 2016 at 12:04:45PM -0700, Jason Gunthorpe wrote:
> >On Fri, Nov 18, 2016 at 02:42:10PM -0800, Vishwanathapura, Niranjana wrote:
> >>+HFI-VNIC DRIVER
> >>+M:	Dennis Dalessandro <dennis.dalessandro@intel.com>
> >>+M:	Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
> >>+L:	linux-rdma@vger.kernel.org
> >>+S:	Supported
> >>+F:	drivers/infiniband/sw/intel/vnic
> >
> >This is either a net driver or a ULP, no idea why it should go in this
> >directory!?
> >
> >It sounds like an ethernet driver, so you should probably put it
> >there...
> >
> 
> The hfi_vnic is an Ethernet driver. It is similar to ULP like ipoib, but
> instead it is Ethernet over Omni-path here.
> The VNIC Ethernet (hfi_vnic) driver encapsulates Ethernet packets in an
> Omni-path header.
> The hfi_vnic Ethernet driver do not access the HW. It interfaces with HFI1
> driver which sends/receives Omni-Path encapsulated Ethernet frames from HW.
> Also, the VNIC control path driver (VEMA) is an IB MAD agent which should be
> under drivers/infiniband/.. .
> Putting the VNIC Ethernet driver and the VNIC control driver together under
> a single module (hfi_vnic.ko) provided a simpler interface between them.
> 
> So, we have put the driver under drivers/infiniband/sw/intel for two reasons:
> a) We have VNIC control driver (VEMA) which is an IB mad agent.
> b) hfi_vnic Ethernet driver is dependent on HFI1 driver for sending/receving
> Omni-path encapsulated Ethernet packets from HW.

Sounds like this driver belongs under net/ someplace to me.

NAK on drivers/infiniband/sw/ at least - that dir is only for HCA
drivers.

> >>+/* hfi_vnic_bus_init - initialize the hfi vnic bus drvier */
> >>+static int hfi_vnic_bus_init(void)
> >>+{
> >>+	int rc;
> >>+
> >>+	ida_init(&hfi_vnic_ctrl_ida);
> >>+	idr_init(&hfi_vnic_idr);
> >>+
> >>+	rc = bus_register(&hfi_vnic_bus);
> >
> >Why on earth do we need this? Didn't I give you enough grief for the
> >psm stuff and now you want to create an entire subystem hidden away!?
> >
> >Use some netlink scheme to control your vnic like the rest of the net
> >stack..
> >
> 
> The hfi_vnic_bus is only abstracting the HW independent functionality (like
> Ethernet interface, encapsulation, IB MAD interface etc) with the HW
> dependent functionality (sending/receiving packets on the wire).
> Thus providing a cleaner interface between HW independent hfi_vnic Ethernet
> and Control drivers and the HW dependent HFI1 driver.

That doesn't explain anything, sound like you don't need it so get rid
of it.

> There is no other User interface here other than the standard Ethernet
> interface through network stack.

Good, then this isn't needed, because it doesn't provide a user interface.

> #ls /sys/bus/hfi_vnic_bus/devices/
>    hfi_vnic_ctrl_00         /* control device for HFI instance 0 */
>    hfi_vnic_00.01.00        /* first VNIC port on HFI instance 0, port 1 */

Jason