From: Stefan Assmann <sassmann@kpanic•de>
To: "Williams, Mitch A" <mitch.a.williams@intel•com>
Cc: "Rose, Gregory V" <gregory.v.rose@intel•com>,
Andy Gospodarek <andy@greyhouse•net>,
"netdev@vger•kernel.org" <netdev@vger•kernel.org>,
"e1000-devel@lists•sourceforge.net"
<e1000-devel@lists•sourceforge.net>
Subject: Re: [E1000-devel] [PATCH net-next] igbvf: fix setting addr_assign_type if PF is up
Date: Thu, 17 Jan 2013 12:39:55 +0100 [thread overview]
Message-ID: <50F7E30B.1040007@kpanic.de> (raw)
In-Reply-To: <AAEA33E297BCAC4B9BB20A7C2DF0AB8D2A3468AE@FMSMSX107.amr.corp.intel.com>
On 17.01.2013 01:42, 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 <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.
Thanks Mitch! That sounds like a good idea. Let me know when you've got
something testable as I'd like to give it a try.
Stefan
next prev parent reply other threads:[~2013-01-17 11:40 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-01-09 9:59 [PATCH net-next] igbvf: fix setting addr_assign_type if PF is up Stefan Assmann
2013-01-09 17:09 ` Williams, Mitch A
2013-01-09 17:58 ` [E1000-devel] " Stefan Assmann
2013-01-09 18:56 ` Williams, Mitch A
2013-01-09 19:53 ` Stefan Assmann
2013-01-09 21:37 ` Greg Rose
2013-01-14 22:25 ` Andy Gospodarek
2013-01-15 18:31 ` Greg Rose
2013-01-17 0:42 ` Williams, Mitch A
2013-01-17 1:06 ` Andy Gospodarek
2013-01-17 1:10 ` [E1000-devel] " Andy Gospodarek
2013-01-17 11:39 ` Stefan Assmann [this message]
2013-01-17 17:08 ` Greg Rose
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=50F7E30B.1040007@kpanic.de \
--to=sassmann@kpanic$(echo .)de \
--cc=andy@greyhouse$(echo .)net \
--cc=e1000-devel@lists$(echo .)sourceforge.net \
--cc=gregory.v.rose@intel$(echo .)com \
--cc=mitch.a.williams@intel$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox