From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andy Gospodarek Subject: Re: [PATCH net-next] igbvf: fix setting addr_assign_type if PF is up Date: Wed, 16 Jan 2013 20:06:00 -0500 Message-ID: References: <1357725564-5581-1-git-send-email-sassmann@kpanic.de> <50EDAFC6.3070700@kpanic.de> <20130109133745.00004627@unknown> <20130114222542.GA29729@gospo.rdu.redhat.com> <20130115103141.00001e96@unknown> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0036018858599794392==" Cc: "e1000-devel@lists.sourceforge.net" , Stefan Assmann , "netdev@vger.kernel.org" To: "Williams, Mitch A" Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: e1000-devel-bounces@lists.sourceforge.net List-Id: netdev.vger.kernel.org --===============0036018858599794392== Content-Type: multipart/alternative; boundary=e89a8f92407823d9bb04d3719990 --e89a8f92407823d9bb04d3719990 Content-Type: text/plain; charset=ISO-8859-1 On Jan 16, 2013 7:42 PM, "Williams, Mitch A" wrote: > > > -----Original Message----- > > From: Rose, Gregory V > > Sent: Tuesday, January 15, 2013 10:32 AM > > To: Andy Gospodarek > > Cc: Williams, Mitch A; Stefan Assmann; netdev@vger.kernel.org; e1000- > > devel@lists.sourceforge.net > > Subject: Re: [E1000-devel] [PATCH net-next] igbvf: fix setting > > addr_assign_type if PF is up > > > > On Mon, 14 Jan 2013 17:25:42 -0500 > > Andy Gospodarek wrote: > > > > > On Wed, Jan 09, 2013 at 01:37:45PM -0800, Greg Rose wrote: > > > > On Wed, 9 Jan 2013 18:56:36 +0000 > > > > "Williams, Mitch A" wrote: > > > > > > > > > > >> When the PF is up and igbvf is loaded the MAC address is not > > > > > > >> generated using eth_hw_addr_random(). This results in > > > > > > >> addr_assign_type not to be set. > > > > > > >> Make sure it gets set. > > > > > > >> > > > > > > > > > > > > > > NAK - In this case, the address may or may not be random. The > > > > > > > user may have (and should have!) explicitly set this address > > > > > > > from the host to ensure that the VF device receives the same > > > > > > > address each time it > > > > > > boots. > > > > > > > > > > > > Maybe you can give me some advice on this then. Why is there > > > > > > different behaviour depending on the PF being up or down? The > > > > > > problem I'm facing is that if the user did not set a MAC address > > > > > > for the VF manually and the PF is up during igbvf_probe it will > > > > > > not be labelled as random although it is. > > > > > > What about checking IGB_VF_FLAG_PF_SET_MAC and only set > > > > > > NET_ADDR_RANDOM if the flag is cleared? > > > > > > > > > > > > > > > > The difference in behavior is because we cannot get any MAC > > > > > address at all if the PF is down. The interface won't operate at > > > > > all in this case, but if the PF comes up sometime later, we can > > > > > start working. The other alternative is to leave the MAC address > > > > > as all zeros and forcing the user to assign an address manually. > > > > > We chose to use a random address to at least give it a chance of > > > > > working once the PF woke up. > > > > > > > > Having been around at the inception of SR-IOV in Linux I recall that > > > > the primary reason we used a random ethernet address was so that the > > > > VF could at least work because there was no infrastructure to allow > > > > the host administrator to set the MAC address of the VF. > > > > This hobbled testing and validation because the user would have to > > > > go to each VM and use a command local to the VM to set the VF MAC > > > > address to some LAA via ifconfig or ip. When testing large numbers > > > > of VFs this was a definite pain. > > > > > > > > Now that has changed and I wonder if maybe we shouldn't back out the > > > > random ethernet address assignment and go ahead with all zeros, > > > > leaving the device non-functional until the user has intentionally > > > > set either an LAA through the VF itself, or an administratively > > > > assigned MAC through the ip tool via the PF. > > > > > > > > Use of the random MAC address is not recommended by Intel's own best > > > > known methods literature, it was used mostly so that we could get > > > > the technology working and it should probably be at least considered > > > > for deprecation or out right elimination. > > > > > > > > > > It would be great to remove the bits that created random MAC addresses > > > for VFs, but wouldn't that break Linus' rule to "not break userspace" > > > if it was removed? > > > > It may, I'm not sure but before we make any changes we'd want to do our > > due diligence. > > > > > > > > There are 2 options that immediately come to mind when looking to > > > resolve this: > > > > > > 1. Use some of the left-over bits in the mailbox messages to pass > > > along a flag with the E1000_VF_RESET messages to indicate whether the > > > MAC was randomly generated. This would be pretty easy, but there > > > could be compatibility issues for a while. > > > > We recently introduced the concept of mailbox message API versions in > > our PF and VF drivers to handle this sort of thing. We could probably > > leverage that method to introduce a new API version that supports the > > additional bits in the reset message. It would only be used if the VF > > could negotiate to the proper mailbox message API version with the PF. > > > > > > > > 2. Default to a MAC address of all zeros, and as a device with > > > all-zeros for a MAC is brought up, randomly create one with > > > eth_hw_addr_random. This may not immediately help cases where device > > > assignment are a problem, but it would ensure that any device with a > > > random MAC as assigned by the kernel, would have NET_ADDR_RANDOM set > > > in addr_assign_type. > > > > Thanks for the suggestions. We're considering some changes in this area > > but we (Intel) need to give this a lot of thought and right now we're > > just in a preliminary discussion mode about it. Stay tuned. > > > > - Greg > > OK, here's what I'm thinking. We don't need to change the communications > protocol for this, and it shouldn't break userspace. > > First, have the PF driver quit assigning random addresses. It will either > give the VF the address assigned by the administrator, or it will give > all zeros. > > Second, modify the VF driver init sequence slightly. If it gets all > zeros from the PF driver, then it should give itself a random address > and set NET_ADDR_RANDOM. > > If we do it this way, the VF will still come up with a random address if > one has not been assigned, and it will always know whether or not the > address that it is using is random. > > If there are no objections, I'll try to get some patches done in the next > few days and get them into our internal test queue. These would then > escape into the real world in a few weeks. > > -Mitch Oh, I like this even better than my proposed option 2. I say, go for it. --e89a8f92407823d9bb04d3719990-- --===============0036018858599794392== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline ------------------------------------------------------------------------------ Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS, MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft MVPs and experts. ON SALE this month only -- learn more at: http://p.sf.net/sfu/learnmore_122712 --===============0036018858599794392== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ E1000-devel mailing list E1000-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/e1000-devel To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired --===============0036018858599794392==--