Re: [PATCH v5 02/11] pci: add option for net failover

[Laine Stump <laine@redhat.com>]

On 10/23/19 5:15 PM, Alex Williamson wrote:
> On Wed, 23 Oct 2019 22:31:37 +0200
> Jens Freimann <jfreimann@redhat.com> wrote:
> 
>> On Wed, Oct 23, 2019 at 02:02:11PM -0600, Alex Williamson wrote:
>>> On Wed, 23 Oct 2019 21:30:35 +0200
>>> Jens Freimann <jfreimann@redhat.com> wrote:
>>>   
>>>> On Wed, Oct 23, 2019 at 12:06:48PM -0600, Alex Williamson wrote:
>>>>> On Wed, 23 Oct 2019 10:27:02 +0200
>>>>> Jens Freimann <jfreimann@redhat.com> wrote:
>> [...]
>>>>> Are there also multi-function considerations that
>>>>> should be prevented or documented?  For example, if a user tries to
>>>>> configure both the primary and failover NICs in the same slot, I assume
>>>>> bad things will happen.
>>>>
>>>>    I would have expected that this is already checked in pci code, but
>>>> it is not. I tried it and when I put both devices into the same slot
>>>> they are both unplugged from the guest during boot but nothing else
>>>> happens. I don't know what triggers that unplug of the devices.
>>>>
>>>> I'm not aware of any other problems regarding multi-function, which
>>>> doesn't mean there aren't any.
>>>
>>> Hmm, was the hidden device at function #0?  The guest won't find any
>>> functions if function #0 isn't present, but I don't know what would
>>> trigger the hotplug.  The angle I'm thinking is that we only have slot
>>> level granularity for hotplug, so any sort of automatic hotplug of a
>>> slot should probably think about bystander devices within the slot.
>>
>> Yes that would be a problem, but isn't it the same in the non-failover case
>> where a user configures it wrong? The slot where the device is plugged is not
>> chosen automatically it's configured by the user, no? I might be mixing something
>> up here.  I have no idea yet how to check if a slot is already populated, but
>> I'll think about it.
> 
> I don't think libvirt will automatically make use of multifunction
> endpoints, except maybe for some built-in devices, so yes it probably
> would be up to the user to explicitly create a multifunction device.

Correct. The only place libvirt will ever assign devices anywhere except 
function 0 is when we are adding pcie-root-ports - those are combined 
8-per-slot in order to conserve space on pcie.0 (this permits us to have 
up to 240 PCIe devices without needing to resort to upstream/downstream 
switches).


> But are there other scenarios that generate an automatic hot-unplug?
> If a user creates a multifunction slot and then triggers a hot-unplug
> themselves, it's easy to place the blame on the user if the result is
> unexpected, but is it so obviously a user configuration error if the
> hotplug occurs as an automatic response to a migration?  I'm not as
> sure about that.

I guess that's all a matter of opinion. If the user never enters in any 
PCI address info and it's all handled by someone else, then I wouldn't 
expect them to know exactly where the devices were (and only vaguely 
understand that their hostdev network interface is going to be unplugged 
during migration). In that case (as long as it's libvirt assigning the 
PCI addresses) the situation we're considering would never ever happen, 
so it's a non-issue.

If, on the other hand, the user wants to mess around assigning PCI 
addresses themselves, then they get to pick up all the pieces. It might 
be nice if they could be given a clue about why it broke though.

> 
> As indicated, I don't know whether this should just be documented or if
> we should spend time preventing it, but someone, somewhere will
> probably think it's a good idea to put their primary and failover NIC
> in the same slot and be confused that the underlying mechanisms cannot
> support it.  It doesn't appear that it would be too difficult to test
> QEMU_PCI_CAP_MULTIFUNCTION (not set) and PCI_FUNC (is 0) for the
> primary, but maybe I'm just being paranoid.  Thanks,

If, as you claim, it's not difficult, then I guess why not?