[libvirt] [PATCH] netdev: assure that SRIOV PF is online before modifying VF params
mprivozn at redhat.com
Fri May 15 15:30:10 UTC 2015
On 15.05.2015 17:13, Laine Stump wrote:
> On 05/15/2015 05:35 AM, Michal Privoznik wrote:
>> On 14.05.2015 21:38, Laine Stump wrote:
>>> The kernel won't complain if you set the mac address and vlan tag for
>>> an SRIOV VF via its PF, and it will even let you assign the PF to a
>>> guest using PCI device assignment or macvtap passthrough. But if the
>>> PF isn't online, the device won't be usable in the guest. This patch
>>> makes sure that it is turned on.
>>> Since multiple guests/VFs could use the same PF, there is no point in
>>> ever setting the PF *off*line.
>>> This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=893738
>>> Originally filed against RHEL6, but present in every version of
>>> libvirt until today.
>>> src/util/virnetdev.c | 11 +++++++++++
>>> 1 file changed, 11 insertions(+)
>>> diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
>>> index e14b401..7022dfa 100644
>>> --- a/src/util/virnetdev.c
>>> +++ b/src/util/virnetdev.c
>>> @@ -2258,6 +2258,17 @@ virNetDevReplaceVfConfig(const char *pflinkdev, int vf,
>>> char macstr[VIR_MAC_STRING_BUFLEN];
>>> char *fileData = NULL;
>>> int ifindex = -1;
>>> + bool pfIsOnline;
>>> + /* Assure that PF is online prior to twiddling with the VF. It
>>> + * *should* be, but if the PF isn't online the changes made to the
>>> + * VF via the PF won't take effect, yet there will be no error
>>> + * reported.
>>> + */
>>> + if (virNetDevGetOnline(pflinkdev, &pfIsOnline) < 0)
>>> + return ret;
>>> + if (!pfIsOnline && virNetDevSetOnline(pflinkdev, true) < 0)
>>> + return ret;
>>> if (virNetDevGetVfConfig(pflinkdev, vf, &oldmac, &oldvlanid) < 0)
>>> goto cleanup;
>> ACK. Should we set the device back to its previous state if something
>> goes wrong later in the function?
> I don't think so. The PF needs to be on in order for any VF to work
> properly, and we can't assume that nobody else has started trying to use
> it since we set it online. (this is similar to what we do with the
> ip_forward setting in the network driver).
> I guess an alternative would be to *never* set the PF online in libvirt,
> but instead to log an error an fail; still better than silently failing
> to pass traffic. Any opinions on which is the better approach?
I think the better is if libvirt sets the PF online. So my ACK to the
patch still holds.
More information about the libvir-list