Message ID | 1479508938-63799-3-git-send-email-niranjana.vishwanathapura@intel.com (mailing list archive) |
---|---|
State | RFC |
Headers | show |
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... > +/* 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.. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. >> +/* 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. There is no other User interface here other than the standard Ethernet interface through network stack. HFI1 driver creates VNIC devices on the hfi_vnic_bus as below and the hfi_vnic Ethernet and Control drivers drive them. #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 */ hfi_vnic_00.01.01 /* second VNIC port on HFI instance 0, port 1 */ The design is as shown in the below diagram. +-------------------+ +----------------------+ | | | Linux | | IB MAD | | Network | | | | Stack | +-------------------+ +----------------------+ | | | | +--------------------------------------------+ | | | HFI VNIC Module | | (HFI VNIC Netdev and EMA drivers) | | (HW independent) | +--------------------------------------------+ | | +--------------------------------------------+ | HFI VNIC Bus | +--------------------------------------------+ | | +--------------------------------------------+ | | | HFI1 Driver with VNIC support | | (HW dependent) | +--------------------------------------------+ Niranjana >Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Nov 21, 2016 at 02:39:30PM -0700, Jason Gunthorpe wrote: >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. > I did not see any example IB mad agent outside drivers/inifiniband folder. I did see some netdev drivers outside the net/ folder (like ipoib and drivers/infiniband/hw/nes/). The hfi_vnic Ethernet driver is entirely a soft driver without any HW access. Also, any interface changes between hfi_vnic and the HFI driver makes it difficult to manage if they are under two subsystems/maintainers. So drivers/infiniband is probably more approriate for this driver. We have 'rdmavt' and 'soft-roce' drivers under drivers/infiniband/sw/ folder which are soft drivers similar to hfi_vnic. So we decided to put 'hif_vnic' under there. Other places under drivers/infiniband where we can put this driver are, drivers/infiniband/ulp/hfi_vnic drivers/infiniband/hw/hfi_vnic >> >>+/* 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. > Can you explain what exactly you are asking to get rid of here and why? >> #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 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Nov 21, 2016 at 03:26:29PM -0800, Vishwanathapura, Niranjana wrote: > I did not see any example IB mad agent outside drivers/inifiniband > folder. You can be the first. > I did see some netdev drivers outside the net/ folder (like ipoib and > drivers/infiniband/hw/nes/). It is very rare, and arguably ipoib was a mistake.. > The hfi_vnic Ethernet driver is entirely a soft driver without any HW access. > Also, any interface changes between hfi_vnic and the HFI driver makes it > difficult to manage if they are under two subsystems/maintainers. We manage this already for all the rocee drivers that are based on netdev drivers, you will be fine. > We have 'rdmavt' and 'soft-roce' drivers under drivers/infiniband/sw/ folder > which are soft drivers similar to hfi_vnic. So we decided to put > 'hif_vnic' As I said those are HCA drivers & support, not just random 'software'. > >>>>+ 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. > > > > Can you explain what exactly you are asking to get rid of here and why? Get rid of the bus_register/etc as drivers do not get to call this. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Nov 21, 2016 at 04:31:18PM -0700, Jason Gunthorpe wrote: >> >>>>+ 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. >> > >> >> Can you explain what exactly you are asking to get rid of here and why? > >Get rid of the bus_register/etc as drivers do not get to call this. > There are many example drivers in kernel which are using bus_register() in an initcall. We could add a custom Interface between HFI1 driver and hfi_vnic drivers without involving a bus. But using the existing bus model gave a lot of in-built flexibility in decoupling devices from the drivers. Niranjana >Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Nov 21, 2016 at 05:53:04PM -0800, Vishwanathapura, Niranjana wrote: > There are many example drivers in kernel which are using bus_register() in > an initcall. There really are not, certainly not in major subsystems. > We could add a custom Interface between HFI1 driver and hfi_vnic drivers > without involving a bus. hfi is already registering on the infiniband class, just use that. > But using the existing bus model gave a lot of in-built flexibility in > decoupling devices from the drivers. If you want to have your own bus then you need your own hfi subsystem. drivers/infiniband is not a dumping ground.. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Ok, I do understand Jason's point that we should probably not put this driver under drivers/infiniband/sw/.., as this driver is not a HCA. It is an ULP similar to ipoib, built on top of Omni-path irrespective of whether we register a hfi_vnic_bus or a direct custom interface with HFI1. This ULP will transmit and recieve Omni-path packets over the fabric, and is dependent on IB MAD interface and the HFI1 driver. Doug, Will it be acceptable if we put it under 'drivers/infiniband/ulp/hfi_vnic'? Niranjana -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 22 Nov 2016, Vishwanathapura, Niranjana wrote: > Ok, I do understand Jason's point that we should probably not put this driver > under drivers/infiniband/sw/.., as this driver is not a HCA. > It is an ULP similar to ipoib, built on top of Omni-path irrespective of > whether we register a hfi_vnic_bus or a direct custom interface with HFI1. > This ULP will transmit and recieve Omni-path packets over the fabric, and is > dependent on IB MAD interface and the HFI1 driver. This is something that encapsulates IP (v4 right?) in something else. Would belong into linux/net/ipv4 You already have similar implementations there See f.e. ipip.c, ip_tunnel.c and lots more (try ls linux/net/ipv4/*tunnel* ) If this is more like a device then it would belong into linux/drivers/net/hfi or so (see also linux/drivers/net/ppp, plip, loopback, etc etc) -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Nov 22, 2016 at 10:04:07AM -0700, Jason Gunthorpe wrote: > On Mon, Nov 21, 2016 at 05:53:04PM -0800, Vishwanathapura, Niranjana wrote: > > There are many example drivers in kernel which are using bus_register() in > > an initcall. > > There really are not, certainly not in major subsystems. I see 2 drivers in the Block subsystem which do this: 19 5354 /nfs/site/home/iweiny/linux-stable/drivers/block/cciss.c <<cciss_init>> err = bus_register(&cciss_bus_type); 20 6447 /nfs/site/home/iweiny/linux-stable/drivers/block/rbd.c <<rbd_sysfs_init>> ret = bus_register(&rbd_bus_type); 2 drivers in the drm subsystem which do this: 29 1155 /nfs/site/home/iweiny/linux-stable/drivers/gpu/drm/drm_mipi_dsi.c <<mipi_dsi_bus_init>> return bus_register(&mipi_dsi_bus_type); 30 242 /nfs/site/home/iweiny/linux-stable/drivers/gpu/host1x/dev.c <<tegra_host1x_init>> err = bus_register(&host1x_bus_type); And I think there are a couple others. I'm not sure what these devices/buses do but they are registering their own bus while being in another major subsystem. Is what we are doing really so crazy/wrong? > > > We could add a custom Interface between HFI1 driver and hfi_vnic drivers > > without involving a bus. > > hfi is already registering on the infiniband class, just use that. > I don't understand what you mean here? The bus_register provides a really clean way for the hfi1 driver and hfi_vnic driver to find each other. This includes being able to support hfi1 with or without hfi_vnic being loaded. Note that without configuration from the "EM" Ethernet Manager the hfi_vnic does not export a net device. Why wouldn't we use this core kernel support?[*] > > But using the existing bus model gave a lot of in-built flexibility in > > decoupling devices from the drivers. > > If you want to have your own bus then you need your own hfi > subsystem. drivers/infiniband is not a dumping ground.. > We don't consider drivers/infiniband a "dumping ground". There is a requirement on ib_mad from the hfi_vnic driver. Ira [*] As an aside why does the ib_core not use this methodology? It dawned on me that this may be a better way to fix our module load problems. However, I have not looked into details. > Jason > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Nov 22, 2016 at 11:49:18AM -0800, Vishwanathapura, Niranjana wrote: > Ok, I do understand Jason's point that we should probably not put > this driver under drivers/infiniband/sw/.., as this driver is not a > HCA. > It is an ULP similar to ipoib, built on top of Omni-path > irrespective of whether we register a hfi_vnic_bus or a direct > custom interface with HFI1. > This ULP will transmit and recieve Omni-path packets over the > fabric, and is dependent on IB MAD interface and the HFI1 driver. > > Doug, > Will it be acceptable if we put it under 'drivers/infiniband/ulp/hfi_vnic'? How about turning this whole discussion around. This is a network driver. So ask the network Maintainers where he wants it. Send the patch to David Miller <davem@davemloft.net> and netdev with the question, where does this code belong? Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Nov 22, 2016 at 07:05:05PM -0500, ira.weiny wrote: > On Tue, Nov 22, 2016 at 10:04:07AM -0700, Jason Gunthorpe wrote: > > On Mon, Nov 21, 2016 at 05:53:04PM -0800, Vishwanathapura, Niranjana wrote: > > > There are many example drivers in kernel which are using bus_register() in > > > an initcall. > > > > There really are not, certainly not in major subsystems. > > I see 2 drivers in the Block subsystem which do this: > > > 19 5354 /nfs/site/home/iweiny/linux-stable/drivers/block/cciss.c <<cciss_init>> > err = bus_register(&cciss_bus_type); > 20 6447 /nfs/site/home/iweiny/linux-stable/drivers/block/rbd.c <<rbd_sysfs_init>> > ret = bus_register(&rbd_bus_type); > > 2 drivers in the drm subsystem which do this: > > > 29 1155 /nfs/site/home/iweiny/linux-stable/drivers/gpu/drm/drm_mipi_dsi.c <<mipi_dsi_bus_init>> > return bus_register(&mipi_dsi_bus_type); > 30 242 /nfs/site/home/iweiny/linux-stable/drivers/gpu/host1x/dev.c <<tegra_host1x_init>> > err = bus_register(&host1x_bus_type); IMHO this is all obscure or legacy stuff (eg ccsiss lost its bus when it was reworked into hpsa). Who knows about that SOC stuff, maybe there really is a special on-chip bus under those drivers. The point is using a bus as a generic interconnect between two driver modules seems very rare, and is not what we have historically ever done in drivers/infiniband - all our split drivers use a trivial register scheme. eg see cxgb4_register_uld/mlx4_register_interface/etc. Should a multi-function driver use a bus or class to connect its parts? Who knows. Maybe Greg KH/etc has an opinion. But that is not what we have been doing, it doesn't seem very simplifying, and this series doesn't even make module auto-loading work... Since doing this creates a bunch of uapis (again, from a driver, ugh) it seems like a bad idea without more support as 'the right way' .. and yes, it would be nice to have a lightweight mechanism to replace those register functions that could handle module auto loading too, and maybe that is a 'multi part driver bus/class' or somesuch ... This is really a topic for the device core maintainers, IMHO. > > > We could add a custom Interface between HFI1 driver and hfi_vnic drivers > > > without involving a bus. > > > > hfi is already registering on the infiniband class, just use that. > > I don't understand what you mean here? Get the struct ib_device for the hfi and then do something to get hfi specific function calls. Or work it backwards with a _register function.. > [*] As an aside why does the ib_core not use this methodology? It dawned on > me that this may be a better way to fix our module load problems. However, I > have not looked into details. ib_core is a class, which is appropriate. RDMA devices are not busses. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Nov 22, 2016 at 05:04:37PM -0600, Christoph Lameter wrote: >On Tue, 22 Nov 2016, Vishwanathapura, Niranjana wrote: > >> Ok, I do understand Jason's point that we should probably not put this driver >> under drivers/infiniband/sw/.., as this driver is not a HCA. >> It is an ULP similar to ipoib, built on top of Omni-path irrespective of >> whether we register a hfi_vnic_bus or a direct custom interface with HFI1. >> This ULP will transmit and recieve Omni-path packets over the fabric, and is >> dependent on IB MAD interface and the HFI1 driver. > >This is something that encapsulates IP (v4 right?) in something else. >Would belong into > > linux/net/ipv4 > >You already have similar implementations there > >See f.e. ipip.c, ip_tunnel.c and lots more (try > ls linux/net/ipv4/*tunnel* > >) > >If this is more like a device then it would belong into > >linux/drivers/net/hfi or so (see also linux/drivers/net/ppp, plip, >loopback, etc etc) > It is Ethernet packet encapsulated in Omni-path header by hfi_vnic driver. The packets are sent and received over the wire by the HFI1 device driven by HFI1 driver. The encapsulation information is obtained via IB MAD control interface. Niranjana > > -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Nov 22, 2016 at 05:49:32PM -0700, Jason Gunthorpe wrote: >> > > We could add a custom Interface between HFI1 driver and hfi_vnic drivers >> > > without involving a bus. >> > >> > hfi is already registering on the infiniband class, just use that. >> >> I don't understand what you mean here? > >Get the struct ib_device for the hfi and then do something to get hfi >specific function calls. > >Or work it backwards with a _register function.. > OK, thanks for your feedback. We can make the hfi_vnic module as an ib client (which it is) like other ULPs, and do not have an in-built or custom bus for binding. Then the hfi_vnic ULP by some mechanism will identify the device as hfi1 device and will only serve that device. In order to pass the hfi function pointers to the hfi_vnic ULP, I can, a) Have hfi_vnic ULP define an interface API for hfi1 driver to call to register its callback (as you pointed). Unfortunately there will be a module dependency here. Or, b) Add a new member ‘struct vnic_ops’ either to the ib_device structure or ib_port_immutable structure. As it is hfi1 specific, only hfi1 driver will set it. No module dependency here. And will move the hfi_vnic module under ‘drivers/infiniband/ulp/hfi_vnic’. All these will remove undue complexity and fit the driver in current design framework as per your suggestion. Let me know your comments. Niranjana > >Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Nov 23, 2016 at 04:08:25PM -0800, Vishwanathapura, Niranjana wrote: > In order to pass the hfi function pointers to the hfi_vnic ULP, I can, > a) Have hfi_vnic ULP define an interface API for hfi1 driver to call to > register its callback (as you pointed). Unfortunately there will be a module > dependency here. > Or, That is probably backwards > b) Add a new member ‘struct vnic_ops’ either to the ib_device structure or > ib_port_immutable structure. As it is hfi1 specific, only hfi1 driver will > set it. No module dependency here. You can add a hfi1_get_vnic_ops(struct ib_device *) and implement it in your module.. > And will move the hfi_vnic module under > ‘drivers/infiniband/ulp/hfi_vnic’. I would prefer drivers/net/ethernet This is clearly not a ULP since it doesn't use verbs. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Nov 24, 2016 at 09:15:45AM -0700, Jason Gunthorpe wrote: >On Wed, Nov 23, 2016 at 04:08:25PM -0800, Vishwanathapura, Niranjana wrote: > >> In order to pass the hfi function pointers to the hfi_vnic ULP, I can, >> a) Have hfi_vnic ULP define an interface API for hfi1 driver to call to >> register its callback (as you pointed). Unfortunately there will be a module >> dependency here. >> Or, > >That is probably backwards > >> b) Add a new member ‘struct vnic_ops’ either to the ib_device structure or >> ib_port_immutable structure. As it is hfi1 specific, only hfi1 driver will >> set it. No module dependency here. > >You can add a hfi1_get_vnic_ops(struct ib_device *) and implement it >in your module.. > In order to be truely device independent the hfi_vnic ULP should not depend on a device exported symbol. Instead device should register its functions with the ULP. Hence the approaches a) and b). Niranjana >Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Nov 24, 2016 at 06:13:50PM -0800, Vishwanathapura, Niranjana wrote: > In order to be truely device independent the hfi_vnic ULP should not depend > on a device exported symbol. Instead device should register its functions > with the ULP. Hence the approaches a) and b). It is not device independent, it is hard linked to hfi1, just like our other multi-component drivers.. So don't worry about that. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Nov 24, 2016 at 09:15:45AM -0700, Jason Gunthorpe wrote: >> And will move the hfi_vnic module under >> ‘drivers/infiniband/ulp/hfi_vnic’. > >I would prefer drivers/net/ethernet > >This is clearly not a ULP since it doesn't use verbs. > I understand it is not using verbs, but the control path (ib_device client) is using verbs (IB MAD). Our prefernce is to keep it somewhere under drivers/infiniband. Summarizing reasons again here, - VNIC control driver (ib_device client) is an IB MAD agent. - It is purly a software construct, encapsualtes ethernet packets in Omni-path packet and depends on hfi1 driver here for HW access. Doug, Any comments? >Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Nov 25, 2016 at 12:05:09PM -0700, Jason Gunthorpe wrote: >On Thu, Nov 24, 2016 at 06:13:50PM -0800, Vishwanathapura, Niranjana wrote: > >> In order to be truely device independent the hfi_vnic ULP should not depend >> on a device exported symbol. Instead device should register its functions >> with the ULP. Hence the approaches a) and b). > >It is not device independent, it is hard linked to hfi1, just like our >other multi-component drivers.. So don't worry about that. > We would like to keep the design clean and avoid any tight coupling here (our original design in this series tackled these). Any strong reason not to go with a) or b) ? Niranjana >Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Nov 28, 2016 at 10:31:06PM -0800, Vishwanathapura, Niranjana wrote: > On Fri, Nov 25, 2016 at 12:05:09PM -0700, Jason Gunthorpe wrote: > >On Thu, Nov 24, 2016 at 06:13:50PM -0800, Vishwanathapura, Niranjana wrote: > > > >>In order to be truely device independent the hfi_vnic ULP should not depend > >>on a device exported symbol. Instead device should register its functions > >>with the ULP. Hence the approaches a) and b). > > > >It is not device independent, it is hard linked to hfi1, just like our > >other multi-component drivers.. So don't worry about that. > > > > We would like to keep the design clean and avoid any tight coupling here > (our original design in this series tackled these). > Any strong reason not to go with a) or b) ? You are not making a subsystem. Don't overcomplicate things. A multi-part device device can just directly link. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Nov 28, 2016 at 10:29:38PM -0800, Vishwanathapura, Niranjana wrote: > On Thu, Nov 24, 2016 at 09:15:45AM -0700, Jason Gunthorpe wrote: > >>And will move the hfi_vnic module under > >>???drivers/infiniband/ulp/hfi_vnic???. > > > >I would prefer drivers/net/ethernet > > > >This is clearly not a ULP since it doesn't use verbs. > > > > I understand it is not using verbs, but the control path (ib_device client) > is using verbs (IB MAD). > Our prefernce is to keep it somewhere under drivers/infiniband. Summarizing > reasons again here, > > - VNIC control driver (ib_device client) is an IB MAD agent. > - It is purly a software construct, encapsualtes ethernet packets in > Omni-path packet and depends on hfi1 driver here for HW access. Is the majority of the code MAD focused or net stack focused? I'm not sure it matters, it isn't like we can review Intel's proprietary mad stuff anyhow. :\ Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> You are not making a subsystem. Don't overcomplicate things. A > multi-part device device can just directly link. The VNIC may be usable over multiple generations of HFIs, but I don't know if that is the intent. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Nov 29, 2016 at 04:44:37PM +0000, Hefty, Sean wrote: > > You are not making a subsystem. Don't overcomplicate things. A > > multi-part device device can just directly link. > > The VNIC may be usable over multiple generations of HFIs, but I > don't know if that is the intent. If Intel wants to build a HFI subystem within RDMA with multiple drivers then sure, but they are not there yet, and we don't even know what that could look like. So it is better to leave it simple for now. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Nov 29, 2016 at 09:50:09AM -0700, Jason Gunthorpe wrote: >On Tue, Nov 29, 2016 at 04:44:37PM +0000, Hefty, Sean wrote: >> > You are not making a subsystem. Don't overcomplicate things. A >> > multi-part device device can just directly link. >> >> The VNIC may be usable over multiple generations of HFIs, but I >> don't know if that is the intent. > >If Intel wants to build a HFI subystem within RDMA with multiple >drivers then sure, but they are not there yet, and we don't even know >what that could look like. So it is better to leave it simple for now. > >Jason Sorry for the delay, I was weighing in couple options. We envisioned vnic as a pure software construct and hence should be independent (like ipoib). ie., both hfi_vnic and hfi1 should be independently loadable (like ipoib) despite hfi_vnic being dependent on hfi1 here for HW access. There doesn't seem to be much value of hfi_vnic being a 'ib client', if it still has compilation and module dependency on hfi1 module. The more I think of it, having vnic ops added to ib device structure (option (b)) makes it cleaner (no dependency). We can probably consider extending 'ib device' in hfi1 in order for hfi_vnic to get to the vnic ops. But (b) makes it simpler. Though Jason's suggestion could be a temporary measure for this patch series, the above approach is what I would like to target here. Niranjana -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Nov 29, 2016 at 09:21:13AM -0700, Jason Gunthorpe wrote: >On Mon, Nov 28, 2016 at 10:29:38PM -0800, Vishwanathapura, Niranjana wrote: >> On Thu, Nov 24, 2016 at 09:15:45AM -0700, Jason Gunthorpe wrote: >> >>And will move the hfi_vnic module under >> >>???drivers/infiniband/ulp/hfi_vnic???. >> > >> >I would prefer drivers/net/ethernet >> > >> >This is clearly not a ULP since it doesn't use verbs. >> > >> >> I understand it is not using verbs, but the control path (ib_device client) >> is using verbs (IB MAD). >> Our prefernce is to keep it somewhere under drivers/infiniband. Summarizing >> reasons again here, >> >> - VNIC control driver (ib_device client) is an IB MAD agent. >> - It is purly a software construct, encapsualtes ethernet packets in >> Omni-path packet and depends on hfi1 driver here for HW access. > >Is the majority of the code MAD focused or net stack focused? > >I'm not sure it matters, it isn't like we can review Intel's >proprietary mad stuff anyhow. :\ > >Jason That is an intersting measure. In hfi_vnic driver, I would say, >60% of the code is MAD focused, mainly interfacing with the IB MAD agent. It also includes populating/parsing those MAD packets. At the least it is not supporting the driver to be put under net folder. Even in the remaining <40%, half of it is involved with encapsulating ethernet frames with Omni-path header (does this makes it belong under drivers/infiniband/hw?). The net stack interface part is pretty standard, hence is not much of code. I do see the reason to put it under net folder, but I am seeing more reason for it to be somewhere under drivers/infiniband. Niranjana -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/MAINTAINERS b/MAINTAINERS index 3d838cf..8c37878 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5628,6 +5628,13 @@ F: drivers/block/cciss* F: include/linux/cciss_ioctl.h F: include/uapi/linux/cciss_ioctl.h +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 + HFI1 DRIVER M: Mike Marciniszyn <mike.marciniszyn@intel.com> M: Dennis Dalessandro <dennis.dalessandro@intel.com> diff --git a/drivers/infiniband/Kconfig b/drivers/infiniband/Kconfig index fb3fb89..7fe9095 100644 --- a/drivers/infiniband/Kconfig +++ b/drivers/infiniband/Kconfig @@ -84,6 +84,7 @@ source "drivers/infiniband/ulp/srpt/Kconfig" source "drivers/infiniband/ulp/iser/Kconfig" source "drivers/infiniband/ulp/isert/Kconfig" +source "drivers/infiniband/sw/intel/vnic/hfi_vnic_bus/Kconfig" source "drivers/infiniband/sw/rdmavt/Kconfig" source "drivers/infiniband/sw/rxe/Kconfig" diff --git a/drivers/infiniband/sw/Makefile b/drivers/infiniband/sw/Makefile index 8b095b2..4fa6058 100644 --- a/drivers/infiniband/sw/Makefile +++ b/drivers/infiniband/sw/Makefile @@ -1,2 +1,3 @@ obj-$(CONFIG_INFINIBAND_RDMAVT) += rdmavt/ obj-$(CONFIG_RDMA_RXE) += rxe/ +obj-$(CONFIG_INFINIBAND) += intel/vnic/ diff --git a/drivers/infiniband/sw/intel/vnic/Makefile b/drivers/infiniband/sw/intel/vnic/Makefile new file mode 100644 index 0000000..083e55b --- /dev/null +++ b/drivers/infiniband/sw/intel/vnic/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_HFI_VNIC_BUS) += hfi_vnic_bus/ diff --git a/drivers/infiniband/sw/intel/vnic/hfi_vnic_bus/Kconfig b/drivers/infiniband/sw/intel/vnic/hfi_vnic_bus/Kconfig new file mode 100644 index 0000000..85952d6 --- /dev/null +++ b/drivers/infiniband/sw/intel/vnic/hfi_vnic_bus/Kconfig @@ -0,0 +1,8 @@ +config HFI_VNIC_BUS + tristate "Intel HFI VNIC bus support" + depends on X86_64 + ---help--- + This is HFI Virtual Network Interface Controller (VNIC) Bus driver + for binding Intel HFI VNIC devices and drivers. It separates the + hardware independent VNIC functionaity from the hw dependent. It + provides APIs to register and unregister VNIC devices and drivers. diff --git a/drivers/infiniband/sw/intel/vnic/hfi_vnic_bus/Makefile b/drivers/infiniband/sw/intel/vnic/hfi_vnic_bus/Makefile new file mode 100644 index 0000000..5fac098 --- /dev/null +++ b/drivers/infiniband/sw/intel/vnic/hfi_vnic_bus/Makefile @@ -0,0 +1,5 @@ +# Makefile - Intel HFI Virtual Network Controller bus driver +# Copyright(c) 2016, Intel Corporation. +# +ccflags-y += -I$(src)/../include +obj-$(CONFIG_HFI_VNIC_BUS) += hfi_vnic_bus.o diff --git a/drivers/infiniband/sw/intel/vnic/hfi_vnic_bus/hfi_vnic_bus.c b/drivers/infiniband/sw/intel/vnic/hfi_vnic_bus/hfi_vnic_bus.c new file mode 100644 index 0000000..5455fc7 --- /dev/null +++ b/drivers/infiniband/sw/intel/vnic/hfi_vnic_bus/hfi_vnic_bus.c @@ -0,0 +1,366 @@ +/* + * Copyright(c) 2016 Intel Corporation. + * + * This file is provided under a dual BSD/GPLv2 license. When using or + * redistributing this file, you may do so under either license. + * + * GPL LICENSE SUMMARY + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of version 2 of the GNU General Public License as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * BSD LICENSE + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * - Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * - Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * - Neither the name of Intel Corporation nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +/* + * This file contains HFI Virtual Network Controller bus driver + */ +#include <linux/idr.h> +#include <linux/module.h> + +#include "hfi_vnic.h" + +#define HFI_VNIC_ID_HFI_SHFT 16 +#define HFI_VNIC_ID_PORT_SHFT 8 +#define HFI_VNIC_GET_ID(hfi, port, vport) (((hfi) << HFI_VNIC_ID_HFI_SHFT) | \ + ((port) << HFI_VNIC_ID_PORT_SHFT) | (vport)) + +/* Unique numbering for vnic control devices */ +static struct ida hfi_vnic_ctrl_ida; + +/* Unique numbering for vnic devices */ +static struct idr hfi_vnic_idr; + +/* hfi vnic device type */ +static const struct device_type hfi_vnic_dev_type = { + .name = "hfi_vnic_dev", +}; + +/* hfi vnic control device type */ +static const struct device_type hfi_vnic_ctrl_dev_type = { + .name = "hfi_vnic_ctrl_dev", +}; + +static inline enum hfi_vnic_drv_type +hfi_vnic_get_drv_type(struct device_driver *drv) +{ + return container_of(drv, struct hfi_vnic_drvwrap, driver)->type; +} + +/* hfi_vnic_match_device - device, driver match function */ +static int hfi_vnic_match_device(struct device *dev, struct device_driver *drv) +{ + enum hfi_vnic_drv_type drv_type = hfi_vnic_get_drv_type(drv); + + if ((dev->type == &hfi_vnic_dev_type) && (drv_type == HFI_VNIC_DRV)) + return 1; + else if ((dev->type == &hfi_vnic_ctrl_dev_type) && + (drv_type == HFI_VNIC_CTRL_DRV)) + return 1; + + return 0; +} + +/* hfi vnic bus structure */ +static struct bus_type hfi_vnic_bus = { + .name = "hfi_vnic_bus", + .match = hfi_vnic_match_device, +}; + +static void hfi_vnic_dev_release(struct device *dev) +{ + struct hfi_vnic_device *vdev = container_of(dev, + struct hfi_vnic_device, dev); + kfree(vdev); +} + +/** + * hfi_vnic_get_dev - return hfi vnic device + * @cdev: pointer to the control device + * @port_num: hfi port number + * @vport_num: vnic port number + * + * Return pointer to the vnic device on the given hfi instance + * (control device) and hfi port, with specified vnic port number. + * + */ +struct hfi_vnic_device *hfi_vnic_get_dev(struct hfi_vnic_ctrl_device *cdev, + u8 port_num, u8 vport_num) +{ + int id; + + if (vport_num == HFI_MAX_NUM_VNICS) + return NULL; + + id = HFI_VNIC_GET_ID(cdev->id, port_num, vport_num); + return idr_find(&hfi_vnic_idr, id); +} +EXPORT_SYMBOL(hfi_vnic_get_dev); + +/** + * hfi_vnic_device_register - register hfi vnic device on the hfi vnic bus + * @cdev: pointer to the control device + * @port_num: hfi port number + * @vport_num: vnic port number + * @priv: pointer to the device private data + * @ops: pointer to the bus operations + * @hfi_info: hfi device hw specific information + * + * A hfi vnic device is created and registered on the hfi vnic bus. + * The device is assigned a unique id based on the hfi instance (id of the + * control device associated with it), hfi port number and the vnic port + * number on the given hfi port. + */ +struct hfi_vnic_device *hfi_vnic_device_register( + struct hfi_vnic_ctrl_device *cdev, + u8 port_num, u8 vport_num, void *priv, + struct hfi_vnic_ops *ops, + struct hfi_vnic_info hfi_info) +{ + struct hfi_vnic_device *vdev; + int id, rc; + + if (vport_num == HFI_MAX_NUM_VNICS) + return ERR_PTR(-EINVAL); + + id = HFI_VNIC_GET_ID(cdev->id, port_num, vport_num); + vdev = kzalloc(sizeof(*vdev), GFP_KERNEL); + if (!vdev) + return ERR_PTR(-ENOMEM); + + rc = idr_alloc(&hfi_vnic_idr, vdev, id, (id + 1), GFP_NOWAIT); + if (rc < 0) { + kfree(vdev); + goto idr_err; + } + + vdev->dev.release = hfi_vnic_dev_release; + vdev->dev.bus = &hfi_vnic_bus; + + vdev->id = id; + vdev->cdev = cdev; + vdev->bus_ops = ops; + vdev->hfi_priv = priv; + vdev->hfi_info = hfi_info; + vdev->port_num = port_num; + vdev->vport_num = vport_num; + vdev->dev.parent = cdev->dev.parent; + + vdev->dev.type = &hfi_vnic_dev_type; + dev_set_name(&vdev->dev, "hfi_vnic_%02x.%02x.%02x", + cdev->id, port_num, vport_num); + + rc = device_register(&vdev->dev); + if (rc) { + put_device(&vdev->dev); + goto dev_err; + } + + dev_info(&vdev->dev, "added vport\n"); + return vdev; + +dev_err: + idr_remove(&hfi_vnic_idr, id); +idr_err: + return ERR_PTR(rc); +} +EXPORT_SYMBOL(hfi_vnic_device_register); + +/** + * hfi_vnic_device_unregister - remove hfi vnic device from the hfi vnic bus + * @vdev: pointer to hfi vnic device + */ +void hfi_vnic_device_unregister(struct hfi_vnic_device *vdev) +{ + int id = vdev->id; + + dev_info(&vdev->dev, "removing vport\n"); + device_unregister(&vdev->dev); + idr_remove(&hfi_vnic_idr, id); +} +EXPORT_SYMBOL(hfi_vnic_device_unregister); + +/** + * hfi_vnic_driver_register - register hfi vnic driver on the hfi vnic bus + * @drv: pointer to hfi vnic driver + */ +int hfi_vnic_driver_register(struct hfi_vnic_driver *drv) +{ + drv->drvwrap.driver.bus = &hfi_vnic_bus; + return driver_register(&drv->drvwrap.driver); +} +EXPORT_SYMBOL(hfi_vnic_driver_register); + +/** + * hfi_vnic_driver_unregister - remove hfi vnic driver from the hfi vnic bus + * @drv: pointer to hfi vnic driver + */ +void hfi_vnic_driver_unregister(struct hfi_vnic_driver *drv) +{ + driver_unregister(&drv->drvwrap.driver); +} +EXPORT_SYMBOL(hfi_vnic_driver_unregister); + +static void hfi_vnic_ctrl_dev_release(struct device *dev) +{ + struct hfi_vnic_ctrl_device *cdev = container_of(dev, + struct hfi_vnic_ctrl_device, dev); + kfree(cdev); +} + +/** + * hfi_vnic_ctrl_device_register - register hfi vnic control device + * @parent: pointer to the parent device + * @ibdev: pointer to the ib device + * @num_ports: number of hfi ports + * @priv: pointer to the device private data + * @ops: pointer to the bus operations + * + * A hfi vnic control device is created and registered on the hfi vnic bus. + * The device is assigned a unique id. + */ +struct hfi_vnic_ctrl_device *hfi_vnic_ctrl_device_register( + struct device *parent, + struct ib_device *ibdev, + u8 num_ports, void *priv, + struct hfi_vnic_ctrl_ops *ops) +{ + struct hfi_vnic_ctrl_device *cdev; + int rc; + + cdev = kzalloc(sizeof(*cdev), GFP_KERNEL); + if (!cdev) + return ERR_PTR(-ENOMEM); + + rc = ida_simple_get(&hfi_vnic_ctrl_ida, 0, 0, GFP_KERNEL); + if (rc < 0) { + kfree(cdev); + goto ida_err; + } + + cdev->id = rc; + cdev->dev.release = hfi_vnic_ctrl_dev_release; + cdev->dev.bus = &hfi_vnic_bus; + cdev->dev.parent = parent; + + cdev->ibdev = ibdev; + cdev->num_ports = num_ports; + cdev->ctrl_ops = ops; + cdev->hfi_priv = priv; + dev_set_name(&cdev->dev, "hfi_vnic_ctrl_%02x", cdev->id); + cdev->dev.type = &hfi_vnic_ctrl_dev_type; + rc = device_register(&cdev->dev); + if (rc) { + put_device(&cdev->dev); + goto dev_err; + } + + dev_info(&cdev->dev, "added vnic control port\n"); + return cdev; + +dev_err: + ida_simple_remove(&hfi_vnic_ctrl_ida, cdev->id); +ida_err: + return ERR_PTR(rc); +} +EXPORT_SYMBOL(hfi_vnic_ctrl_device_register); + +/** + * hfi_vnic_ctrl_device_unregister - remove hfi vnic control device + * @cdev: pointer to hfi vnic control device + */ +void hfi_vnic_ctrl_device_unregister(struct hfi_vnic_ctrl_device *cdev) +{ + int id = cdev->id; + + dev_info(&cdev->dev, "removing vnic control port\n"); + device_unregister(&cdev->dev); + ida_simple_remove(&hfi_vnic_ctrl_ida, id); +} +EXPORT_SYMBOL(hfi_vnic_ctrl_device_unregister); + +/** + * hfi_vnic_ctrl_driver_register - register hfi vnic control driver + * @drv: pointer to hfi vnic control driver + */ +int hfi_vnic_ctrl_driver_register(struct hfi_vnic_ctrl_driver *drv) +{ + drv->drvwrap.driver.bus = &hfi_vnic_bus; + return driver_register(&drv->drvwrap.driver); +} +EXPORT_SYMBOL(hfi_vnic_ctrl_driver_register); + +/** + * hfi_vnic_ctrl_driver_unregister - remove hfi vnic control driver + * @drv: pointer to hfi vnic control driver + */ +void hfi_vnic_ctrl_driver_unregister(struct hfi_vnic_ctrl_driver *drv) +{ + driver_unregister(&drv->drvwrap.driver); +} +EXPORT_SYMBOL(hfi_vnic_ctrl_driver_unregister); + +/* 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); + if (rc) { + pr_err("hfi vnic bus init failed %d\n", rc); + idr_destroy(&hfi_vnic_idr); + ida_destroy(&hfi_vnic_ctrl_ida); + } + + return rc; +} +postcore_initcall(hfi_vnic_bus_init); + +/* hfi_vnic_bus_deinit - remove the hfi vnic bus drvier */ +static void hfi_vnic_bus_deinit(void) +{ + bus_unregister(&hfi_vnic_bus); + idr_destroy(&hfi_vnic_idr); + ida_destroy(&hfi_vnic_ctrl_ida); +} +module_exit(hfi_vnic_bus_deinit); + +MODULE_LICENSE("Dual BSD/GPL"); +MODULE_AUTHOR("Intel Corporation"); +MODULE_DESCRIPTION("Intel HFI Virtual Network Controller bus driver"); diff --git a/drivers/infiniband/sw/intel/vnic/include/hfi_vnic.h b/drivers/infiniband/sw/intel/vnic/include/hfi_vnic.h new file mode 100644 index 0000000..a5a723b --- /dev/null +++ b/drivers/infiniband/sw/intel/vnic/include/hfi_vnic.h @@ -0,0 +1,282 @@ +#ifndef _HFI_VNIC_H +#define _HFI_VNIC_H +/* + * Copyright(c) 2016 Intel Corporation. + * + * This file is provided under a dual BSD/GPLv2 license. When using or + * redistributing this file, you may do so under either license. + * + * GPL LICENSE SUMMARY + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of version 2 of the GNU General Public License as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * BSD LICENSE + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * - Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * - Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * - Neither the name of Intel Corporation nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +/* + * This file contains HFI Virtual Network Interface Controller (VNIC) + * driver interfaces + */ + +#include <linux/etherdevice.h> +#include <linux/module.h> + +/* Maximum possible number of VNICs */ +#define HFI_MAX_NUM_VNICS 255 + +#define HFI_VNIC_MAX_QUEUE 16 + +#define HFI_VNIC_CAP_SG BIT(0) + +enum hfi_vnic_drv_type { + HFI_VNIC_DRV, /* VNIC NETDEV driver */ + HFI_VNIC_CTRL_DRV /* VNIC control driver */ +}; + +enum { + /* Packet received on queue 0 */ + HFI_VNIC_EVT_RX0, + /* Tx wakeup notification on queue 0 */ + HFI_VNIC_EVT_TX0 + = HFI_VNIC_EVT_RX0 + HFI_VNIC_MAX_QUEUE, + HFI_VNIC_NUM_EVTS + = HFI_VNIC_EVT_TX0 + HFI_VNIC_MAX_QUEUE, +}; + +struct hfi_vnic_device; +struct hfi_vnic_ctrl_device; + +typedef void (*hfi_vnic_evt_cb_fn)(struct hfi_vnic_device *vdev, u8 evt); + +/** + * struct hfi_vnic_ops - HFI HW specific VNIC functions + * @init: Initialize the hfi device + * @deinit: De-initialize the hfi device + * @open: Open vnic hfi device for vnic traffic + * @close: Close vnic hfi device for vnic traffic + * @put_skb: transmit an skb + * @get_skb: receive an skb + * @get_read_avail: return number of available to read + * @get_write_avail: return whether write space is available or not + * @select_queue: select tx queue + * @config_notify: enable/disable notification + */ +struct hfi_vnic_ops { + int (*init)(struct hfi_vnic_device *vdev); + void (*deinit)(struct hfi_vnic_device *vdev); + int (*open)(struct hfi_vnic_device *vdev, + hfi_vnic_evt_cb_fn cb); + void (*close)(struct hfi_vnic_device *vdev); + int (*put_skb)(struct hfi_vnic_device *vdev, + u8 q_idx, struct sk_buff *skb); + struct sk_buff *(*get_skb)(struct hfi_vnic_device *vdev, u8 q_idx); + u16 (*get_read_avail)(struct hfi_vnic_device *vdev, u8 q_idx); + bool (*get_write_avail)(struct hfi_vnic_device *vdev, u8 q_idx); + u8 (*select_queue)(struct hfi_vnic_device *vdev, u8 vl, u8 entropy); + void (*config_notify)(struct hfi_vnic_device *vdev, + u8 evt, bool enable); +}; + +/** + * struct hfi_vnic_ctrl_ops - HFI HW specific VNIC control functions + * @add_vport: add a vnic port device + * @rem_vport: remove a vnic port device + */ +struct hfi_vnic_ctrl_ops { + int (*add_vport)(struct hfi_vnic_ctrl_device *cdev, + u8 port_num, u8 vport_num); + void (*rem_vport)(struct hfi_vnic_ctrl_device *cdev, + u8 port_num, u8 vport_num); +}; + +/** + * struct hfi_vnic_stats - HFI HW specific statistics + * @rx_fifo_errors: receive packets dropped due to fifo full + * @tx_fifo_errors: transmit packets dropped due to fifo full + * @rx_missed_errors: receive packets missed due to no memory + * @tx_carrier_errors: packet transmits when STL link is down + * @rx_bad_veswid: receive packets with invalid vesw id + * @rx_logic_errors: receive packets dropped due to other errors + * @tx_logic_errors: transmit packets dropped due to other errors + * + * This structure holds any statistics information that is + * collected by HW specific driver layer. + */ +struct hfi_vnic_stats { + u64 rx_fifo_errors; + u64 tx_fifo_errors; + u64 rx_missed_errors; + u64 tx_carrier_errors; + u64 rx_bad_veswid; + u64 rx_logic_errors; + u64 tx_logic_errors; +}; + +/** + * struct hfi_vnic_info - HFI HW specific VNIC information + * @cap: capabilities + * @num_rx_q: number of receive queues + * @num_tx_q: number of transmit queues + */ +struct hfi_vnic_info { + u32 cap; + u8 num_rx_q; + u8 num_tx_q; +}; + +/** + * struct hfi_vnic_device - HFI virtual NIC device + * @dev: device + * @id: unique hfi vnic device instance + * @vesw_id: virtual ethernet switch id + * @netdev: pointer to associated netdev + * @port_num: hfi port instance + * @vport_num: vnic port instance on the hfi port + * @cdev: vnic control device pointer + * @bus_ops: hfi vnic bus operations + * @hfi_priv: hfi private data pointer + * @hfi_info: hfi information + * @hfi_stats: per queue hfi statistics + */ +struct hfi_vnic_device { + struct device dev; + int id; + u16 vesw_id; + struct net_device *netdev; + u8 port_num; + u8 vport_num; + + struct hfi_vnic_ctrl_device *cdev; + struct hfi_vnic_ops *bus_ops; + void *hfi_priv; + struct hfi_vnic_info hfi_info; + struct hfi_vnic_stats hfi_stats[HFI_VNIC_MAX_QUEUE]; +}; + +/** + * struct hfi_vnic_ctrl_device - HFI virtual NIC control device + * @dev: device + * @id: unique hfi vnic control device instance + * @ibdev: pointer to ib device + * @num_ports: number of hfi ports + * @ctrl_ops: hfi vnic control operations + * @hfi_priv: hfi private data pointer + */ +struct hfi_vnic_ctrl_device { + struct device dev; + int id; + + struct ib_device *ibdev; + u8 num_ports; + + struct hfi_vnic_ctrl_ops *ctrl_ops; + void *hfi_priv; +}; + +/** + * struct hfi_vnic_drvwrap - HFI vnic driver wrapper + * @type: driver type + * @driver: device driver + */ +struct hfi_vnic_drvwrap { + enum hfi_vnic_drv_type type; + struct device_driver driver; +}; + +/** + * struct hfi_vnic_driver - HFI virtual NIC driver + * @drvwrap: driver wrapper + */ +struct hfi_vnic_driver { + struct hfi_vnic_drvwrap drvwrap; +}; + +/** + * struct hfi_vnic_ctrl_driver - HFI virtual NIC Control driver + * @drvwrap: driver wrapper + */ +struct hfi_vnic_ctrl_driver { + struct hfi_vnic_drvwrap drvwrap; +}; + +/* VNIC device interface functions */ +int hfi_vnic_driver_register(struct hfi_vnic_driver *drv); +void hfi_vnic_driver_unregister(struct hfi_vnic_driver *drv); + +struct hfi_vnic_device *hfi_vnic_device_register( + struct hfi_vnic_ctrl_device *cdev, + u8 port_num, u8 vport_num, void *priv, + struct hfi_vnic_ops *ops, + struct hfi_vnic_info hfi_info); +void hfi_vnic_device_unregister(struct hfi_vnic_device *vdev); + +struct hfi_vnic_device *hfi_vnic_get_dev(struct hfi_vnic_ctrl_device *cdev, + u8 port_num, u8 vport_num); + +/* VNIC control device interface functions */ +int hfi_vnic_ctrl_driver_register(struct hfi_vnic_ctrl_driver *drv); +void hfi_vnic_ctrl_driver_unregister(struct hfi_vnic_ctrl_driver *drv); + +struct hfi_vnic_ctrl_device *hfi_vnic_ctrl_device_register( + struct device *parent, + struct ib_device *ibdev, + u8 num_ports, void *priv, + struct hfi_vnic_ctrl_ops *ops); +void hfi_vnic_ctrl_device_unregister(struct hfi_vnic_ctrl_device *cdev); + +/* hfi_vdev_get - Get module and vdev reference counts */ +static inline int hfi_vdev_get(struct hfi_vnic_device *vdev) +{ + struct module *owner = vdev->dev.parent->driver->owner; + + if (owner && !try_module_get(owner)) + return -ENXIO; + + get_device(&vdev->dev); + return 0; +} + +/* hfi_vdev_put - Put module and vdev reference counts */ +static inline void hfi_vdev_put(struct hfi_vnic_device *vdev) +{ + struct module *owner = vdev->dev.parent->driver->owner; + + put_device(&vdev->dev); + module_put(owner); +} + +#endif /* _HFI_VNIC_H */