netdev - Re: [for-next 4/6] net/mlx5: FPGA, Add basic support for Innova

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170526030701.du4345mbprhcwjyi@ast-mbp>
Date:   Thu, 25 May 2017 20:07:08 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Ilan Tayari <ilant@...lanox.com>
Cc:     Saeed Mahameed <saeedm@...lanox.com>,
        "David S. Miller" <davem@...emloft.net>,
        Doug Ledford <dledford@...hat.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
        "jsorensen@...com" <jsorensen@...com>
Subject: Re: [for-next 4/6] net/mlx5: FPGA, Add basic support for Innova

On Thu, May 25, 2017 at 05:20:04AM +0000, Ilan Tayari wrote:
> 
> If you do want this, then splitting some of the logic to a
> separate kernel object will not gain anything useful (logic would stay
> the same), and just pollute the exported symbol table and open up the door
> for issues of inter-module compatibility and so on.

in other words instead of properly designing inter-module api
you just want to add everything into one giant 'driver'
because it's easier to do so. Understandable, but it leads
to unmaintainable device infrastructure long term.

> Furthermore, the next patchset will introduce IPSec offload capabilities
> Which are declared in netdev->hw_features. Those cannot be modified
> after the netdevice is created, so all the extra logic has to be 
> integrated into the mlx5 ethernet driver. This is another reason why
> a separate driver is a bad idea.

that increases my concerns even more.
ipsec offload is a new feature for new hw, yet you're trying to
hide it in the mlx5 'driver'.

mlx5 already supports cx4/cx4-lx/cx5 and not even released cx6.
All of them have different feature sets.
The only common piece is driver/fw cmd interface and _some_ aspects
of rx/tx descriptors. Everything else is different.
cx4-lx doesn't have infiniband and rdma, yet there is a ton of code
in the driver that deals with it.
cx5 has fancy storage interfaces for nvme, etc I don't think they
are part of the mlx5 'driver' yet. Are you going to dump them
in there as well?
Take a look at the simplest feature-wise cx4-lx. It has
eswitch which is full l2 switch with mac table, drop policy,
counters and such. Yet kernel knows nothing about it.
Everything is hidden in mlx5 'driver' with its own special
interfaces to manage it.
Now you want to hide fpga with custom logic behind mlx5 'driver'
and manage it through mlx5 interfaces?
That's not acceptable.
mlx fpga got to have generic kernel representation that intel
and other fpga vendors can use.

mlx5 driver has to be modularized and since it's not too late
cx6 support has to be removed from it.
Thankfully only few cx6 pci ids were added to mlx5, but driver
knows nothing about cx6 features, so we can properly design it.

Since driver/fw interface is common between cx4+ this part
can be moved into mlx_core.ko or done as library
the way chelsio did it in libcxgb.