netdev - Re: [PATCH net-next] igbvf: do not force carrier off in igbvf_msix

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: <5203C2C3.3030205@intel.com>
Date:	Thu, 08 Aug 2013 09:09:39 -0700
From:	Alexander Duyck <alexander.h.duyck@...el.com>
To:	Stefan Assmann <sassmann@...nic.de>
CC:	netdev@...r.kernel.org, e1000-devel@...ts.sourceforge.net,
	davem@...emloft.net, carolyn.wyborny@...el.com,
	jeffrey.t.kirsher@...el.com, gregory.v.rose@...el.com
Subject: Re: [PATCH net-next] igbvf: do not force carrier off in igbvf_msix_other()

On 08/08/2013 04:56 AM, Stefan Assmann wrote:
> Currently carrier is forced off in igbvf_msix_other(). This seems
> unnecessary and causes multiple calls to igbvf_watchdog_task(), resulting
> in multiple link up messages when calling dhclient for example.
> [  111.818106] igbvf 0000:00:04.0: Link is Up 1000 Mbps Full Duplex
> [  111.819347] IPv6: ADDRCONF(NETDEV_UP): eth5: link is not ready
> [  111.820509] IPv6: ADDRCONF(NETDEV_CHANGE): eth5: link becomes ready
> [  111.822983] igbvf 0000:00:04.0: Link is Up 1000 Mbps Full Duplex
> [  115.152421] igbvf 0000:00:04.0: Link is Up 1000 Mbps Full Duplex
> compared to
> [ 1040.422161] igbvf 0000:00:04.0: Link is Up 1000 Mbps Full Duplex
> [ 1040.423447] IPv6: ADDRCONF(NETDEV_UP): eth5: link is not ready
> [ 1040.424622] IPv6: ADDRCONF(NETDEV_CHANGE): eth5: link becomes ready
> when this patch is applied.
>
> Signed-off-by: Stefan Assmann <sassmann@...nic.de>
> ---
>  drivers/net/ethernet/intel/igbvf/netdev.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/igbvf/netdev.c b/drivers/net/ethernet/intel/igbvf/netdev.c
> index 93eb7ee..f041586 100644
> --- a/drivers/net/ethernet/intel/igbvf/netdev.c
> +++ b/drivers/net/ethernet/intel/igbvf/netdev.c
> @@ -876,8 +876,6 @@ static irqreturn_t igbvf_msix_other(int irq, void *data)
>  
>  	adapter->int_counter1++;
>  
> -	netif_carrier_off(netdev);
> -	hw->mac.get_link_status = 1;
>  	if (!test_bit(__IGBVF_DOWN, &adapter->state))
>  		mod_timer(&adapter->watchdog_timer, jiffies + 1);
>  

While this patch helps to squelch the messages, did you test to see what
happens if for example you bring the PF interface down while the VFs are
trying to function?  The reason for switching the carrier off is because
most interrupts on the mailbox indicate that something has been changed
in the underlying interface.  If for example the PF is about to disable
the interfaces it should be triggering this interrupt.  Otherwise you
are just setting up the VF to dump out a number of Tx hang and watchdog
messages.

Thanks,

Alex
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html