netdev - Re: [PATCH] igb: Indicate failure on vf reset for empty mac address

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: <4F8303E0-EA1B-4D93-B898-10ACD072F404@suse.de>
Date:	Thu, 9 Oct 2014 00:38:46 +0200
From:	Alexander Graf <agraf@...e.de>
To:	Jeff Kirsher <jeffrey.t.kirsher@...el.com>
Cc:	"David S. Miller" <davem@...emloft.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	Mitch Williams <mitch.a.williams@...el.com>,
	Andy Gospodarek <andy@...yhouse.net>,
	Stefan Assmann <sassmann@...nic.de>,
	Aaron Brown <aaron.f.brown@...el.com>,
	Greg Rose <gregory.v.rose@...el.com>,
	John Ronciak <john.ronciak@...el.com>
Subject: Re: [PATCH] igb: Indicate failure on vf reset for empty mac address




> Am 09.10.2014 um 00:29 schrieb Jeff Kirsher <jeffrey.t.kirsher@...el.com>:
> 
>> On Wed, 2014-10-08 at 23:23 +0200, Alexander Graf wrote:
>> Commit 5ac6f91d changed the igb driver to expose a zero (empty) mac
>> address to the VF on reset rather than a random one.
>> 
>> However, that behavioral change also requires igbvf driver changes
>> which can be hard especially when we want to talk to proprietary
>> guest OSs.
>> 
>> Looking at the code previous to the commit in Linux that made igbvf
>> work with empty mac addresses (8d56b6d), we can see that on reset
>> failure the driver will try to generate a new mac address with both
>> the old and the new code.
>> 
>> Furthermore, ixgbe does send reset failure when it detects an empty
>> mac address (35055928c).
>> 
>> So I think it's safe to make igb behave the same. With this patch I
>> can successfully run a Windows 8.1 guest with an empty mac address
>> and an assigned igbvf device that has no mac address set by the host.
>> 
>> If anyone is aware of a guest driver that chokes on NACK returns of
>> VF RESET commands, please speak up.
>> 
>> Signed-off-by: Alexander Graf <agraf@...e.de>
>> ---
>> drivers/net/ethernet/intel/igb/igb_main.c | 8 ++++++--
>> 1 file changed, 6 insertions(+), 2 deletions(-)
> 
> Actually after further review of this patch and the reported bug in
> SuSE's bugzilla, we are NACK'ing this patch.
> 
> If the reset has not failed, why are we indicating that it has?

Very good question. It's what the ixgbe driver does and older Linux, newer Linux as well as Windows seem to interpret it as "I need to generate a mac address".

I have no idea whether that's the intended meaning of a reset nack, but if it isn't, ixgbe probably shouldn't nack it either then.

Is the mbox message protocol documented anywhere? I could only find references to register layouts so far, but nothing about the comminucation on top of them.

> We
> originally supplied the VF with a NULL MAC, so we should supply it
> again.  That way, the VF can choose to either regenerate a new random
> MAC or keep using the one that it had.

Where is this documented? And why doesn't ixgbe adhere to this?

> The current method was a fix that was requested by the community in the
> first place, also we cannot take into account "proprietary guest OS's".

I don't think I understand this comment. "The community" is a very broad term in Linux ;). Also, this patch merely moves igb to behave identically to ixgbe in how it tells an igbvf driver that the mac address is empty. Nothing changes wrt the original change to not generate random mac addresses. That stays identical.


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