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. 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. 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. 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. Or.
