netdev - Re: [net-next v2 15/15] i40e: link_down_on

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: <20180123113826.6cbf0fd1@cakuba.netronome.com>
Date:   Tue, 23 Jan 2018 11:38:26 -0800
From:   Jakub Kicinski <kubakici@...pl>
To:     "Stachura, Mariusz" <mariusz.stachura@...el.com>
Cc:     "Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>,
        "'davem@...emloft.net'" <davem@...emloft.net>,
        "'netdev@...r.kernel.org'" <netdev@...r.kernel.org>,
        "'nhorman@...hat.com'" <nhorman@...hat.com>,
        "'sassmann@...hat.com'" <sassmann@...hat.com>,
        "'jogreene@...hat.com'" <jogreene@...hat.com>,
        "Williams, Mitch A" <mitch.a.williams@...el.com>
Subject: Re: [net-next v2 15/15] i40e: link_down_on_close private flag
 support

On Tue, 23 Jan 2018 10:55:33 +0000, Stachura, Mariusz wrote:
> Hello Kuba,

Hi Mariusz!

> First of all sorry for a long response.
> 
> Did you have a chance to try it using instruction I've sent?

Unfortunately, I can't take the machine with Fortville down ATM to swap
kernels.  Is this patch already applied to your out-of-tree packaged
driver?  Could I try that?

> As for review:
> 1) nit: reverse christmas tree
>     I'm fixing it.
> 2) Are abilities guaranteed to be filled in by this function?  Otherwise you will use uninitialized stack memory.
>     Good point, I'm changing dev_dbg to dev_err and returning immediately after printing it
> 3) nit: looks like a GENMASK_ULL() coded as a loop
>     You are right, but I just realized this is not what I wanted :). I wanted to include all enum values, however not all values between enums. In example, there is a gap between:
> 	I40E_PHY_TYPE_UNSUPPORTED		= 0xF,
> 	I40E_PHY_TYPE_100BASE_TX		= 0x11,
>     And I did not want to include 0x10. I am adding #define that inludes all ORed enums (in i40e_adminq_cmd.h, just after enum i40e_aq_phy_type definition). I will assign this define to the mask, instead of using GENMASK_ULL.

What about the error handling?  That was my main worry.  That the
communication with FW will fail on open and the link will remain
disabled.