lkml.org 
[lkml]   [2020]   [Sep]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v3 00/14] Adding GAUDI NIC code to habanalabs driver
    On Wed, Sep 16, 2020 at 12:04 AM Jakub Kicinski <kuba@kernel.org> wrote:
    >
    > On Tue, 15 Sep 2020 23:46:58 +0300 Oded Gabbay wrote:
    > > On Tue, Sep 15, 2020 at 11:35 PM Jakub Kicinski <kuba@kernel.org> wrote:
    > > > On Tue, 15 Sep 2020 20:10:08 +0300 Oded Gabbay wrote:
    > > > > Hello,
    > > > >
    > > > > This is the second version of the patch-set to upstream the GAUDI NIC code
    > > > > into the habanalabs driver.
    > > > >
    > > > > The only modification from v2 is in the ethtool patch (patch 12). Details
    > > > > are in that patch's commit message.
    > > >
    > > > You keep reposting this, yet this SDK shim^W^W driver is still living in
    > > > drivers/misc. If you want to make it look like a NIC, the code belongs
    > > > where NIC drivers are.
    > > >
    > > > Then again, is it a NIC? Why do you have those custom IOCTLs? That's far
    > > > from normal.
    > >
    > > I'm sorry but from your question it seems as if you didn't read my
    > > cover letter at all, as I took great lengths in explaining exactly
    > > what our device is and why we use custom IOCTLs.
    > > TL;DR
    > > We have an accelerator for deep learning (GAUDI) which uses RDMA as
    > > infrastructure for communication between multiple accelerators. Same
    > > as Nvidia uses NVlink, we use RDMA that we have inside our ASIC.
    > > The RDMA implementation we did does NOT support some basic RDMA
    > > IBverbs (such as MR and PD) and therefore, we can't use the rdma-core
    > > library or to connect to the rdma infrastructure in the kernel. We
    > > wanted to do it but when we analyzed it, we saw we wouldn't be able to
    > > support basic stuff and therefore we had to revert to our IOCTLs.
    > > To sum it up, because our NIC is used for intra-communication, we
    > > don't expose nor intend users to use it as a NIC per-se. However, to
    > > be able to get statistics and manage them in a standard way, and
    > > support control plane over Ethernet, we do register each port to the
    > > net subsystem (i.e. create netdev per port).
    > >
    > > I hope this short summary explains this better.
    >
    > I read your cover letter. Networking drivers don't get to define random
    > IOCTLs as they please. You have to take that part out of the "NIC"
    > driver.

    The IOCTLs are not for the Ethernet part. They are strictly for the
    RDMA operations. RDMA drivers also have IOCTLs as interfaces in the
    drivers/infiniband area, so I don't think I'm doing something
    different here.
    And my driver is not networking. It is an accelerator which has some
    network ports.
    btw, this is only a single new IOCTL call. The rest of the IOCTLs are
    already upstreamed and are for the rest of the ASIC's compute
    functionality. What I'm trying to say is that it's very common to
    define IOCTLs for accelerators.

    >
    > > As per your request that this code lives in the net subsystem, I think
    > > that will make it only more complicated and hard to upstream and
    > > maintain.
    > > I see there are other examples (e.g. sgi-xp) that contain networking
    > > driver code in misc so I don't understand this objection.
    >
    > The maintenance structure and CI systems for the kernel depend on the
    > directory layout. If you don't understand that I don't know how to help
    > you.
    I completely understand but you didn't answer my question. How come
    there are drivers which create netdev objects, and specifically sgi-xp
    in misc (but I also saw it in usb drivers) that live outside
    drivers/net ? Why doesn't your request apply to them as well ?
    When we wrote the code, we saw those examples and therefore assumed it was fine.

    >
    > > > Please make sure to CC linux-rdma. You clearly stated that the device
    > > > does RDMA-like transfers.
    > >
    > > We don't use the RDMA infrastructure in the kernel and we can't
    > > connect to it due to the lack of H/W support we have so I don't see
    > > why we need to CC linux-rdma.
    >
    > You have it backward. You don't get to pick and choose which parts of
    > the infrastructure you use, and therefore who reviews your drivers.
    > The device uses RDMA under the hood so Linux RDMA experts must very
    > much be okay with it getting merged. That's how we ensure Linux
    > interfaces are consistent and good quality.

    I understand your point of view but If my H/W doesn't support the
    basic requirements of the RDMA infrastructure and interfaces, then
    really there is nothing I can do about it. I can't use them.
    I wish I was able to use that infrastructure but I can't. That's why
    we wrote the IOCTLs in our accelerator driver.

    Thanks,
    Oded

    \
     
     \ /
      Last update: 2020-09-15 23:24    [W:3.259 / U:0.736 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site