Re: [PATCH net-next] igbvf: fix setting addr_assign_type if PF is up

[Andy Gospodarek <andy@greyhouse.net>]

[-- Attachment #1.1: Type: text/plain, Size: 6045 bytes --]

On Jan 16, 2013 7:42 PM, "Williams, Mitch A" <mitch.a.williams@intel.com>
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 <andy@greyhouse.net> 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" <mitch.a.williams@intel.com> 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.

[-- Attachment #2: Type: text/plain, Size: 383 bytes --]

------------------------------------------------------------------------------
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

[-- Attachment #3: Type: text/plain, Size: 257 bytes --]

_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired