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] [day] [month] [year] [list]

Message-ID: <20180124130134.5e70d400@cakuba.netronome.com>
Date:   Wed, 24 Jan 2018 13:01:34 -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 Wed, 24 Jan 2018 13:54:16 +0000, Stachura, Mariusz wrote:
> >On Tue, 23 Jan 2018 10:55:33 +0000, Stachura, Mariusz wrote:  
> >> 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.  
> 
> Hi again,
> 
> As I checked, this
> ( https://sourceforge.net/projects/e1000/files/i40e%20stable/2.4.3/i40e-2.4.3.tar.gz/download )
> version had this patch applied, feel free to give it a shot.

Cool thanks, I tried that and the link does go down as expected (y)

> Error handling was also addressed. i40e_open checks
> i40e_force_link_state and returns EBUSY if get/set capabilities was
> not successful.