On Thu, 2016-07-07 at 12:12 +0300, Or Gerlitz wrote: > On Tue, Jul 5, 2016 at 8:44 PM, Alexei Starovoitov > <alexei.starovoi...@gmail.com> wrote: > > On Mon, Jul 04, 2016 at 01:03:06AM +0300, Or Gerlitz wrote: > > > Or, since cx4 has ncsi as well, could you do a thorough review of this > > to make sure that it fits mellanox nics as well? > > Hi Alexei, all > > Yuval from our team who deals with host management did review on the > series, SB his feedback. > > 1. The initialization uses a single unicast MAC address which hints it > assumes that the management traffic is IPv4 only. The infrastructure > does not seem to be ready for IPv6 based management traffic.
You mean the transfer of the MAC address from the BMC to the NIC for filtering incoming traffic ? > 2. The code enables AEN messages (all the 3 messages defined by the > standard are enabled). AEN support mandates readiness on the BMC side > (running the reviewed code) and mandates support on the NIC. Not every > NIC can create AEN messages therefore I would recommend that the code > will only enable AENs which are declared as supported. To query > support for AEN messages, the BMC shall interpret the response for > "Get Capabilities" command and based on the returned response it can > detect which AEN message is supported by the NIC and may be enabled. Ok. > 3. When stating support for NC-SI it is important to indicate which > exact version of NC-SI is supported (or required). From my observation > this code is based on NC-SI 1.0.0 as it does not use any of the > features which were introduced in the later versions. Probably correct, 1.0.1 I think is the only one that was available when we started that work. I found 1.1.0 on the DMTF web site, are you aware of anything more recent ? Gavin, you can get it here: https://www.dmtf.org/sites/default/files/standards/documents/DSP0222_1.1.0.pdf > 4. HW arbitration is disabled in this implementation. It should be > defined how to add support to system which supports HW arbitration. > > 5. The code has an initialization sequence which is clear. Yet there > shall be a re-initialization sequence needed if a device goes through > asynchronous entry to initial state, which may happen if the NIC goes > through reset for any reason. Ok. Gavin: We can discuss these tomorrow if you need. Cheers, Ben.
