Re: [dpdk-dev] [PATCH] drivers/net: update link status

["Tan, Jianfeng" <jianfeng.tan@intel.com>]

Hi Ferruh,


On 4/17/2018 7:26 PM, Ferruh Yigit wrote:
> On 4/17/2018 5:54 AM, Tiwei Bie wrote:
>> On Mon, Apr 16, 2018 at 05:10:24PM +0100, Ferruh Yigit wrote:
>>> On 4/14/2018 11:55 AM, Tiwei Bie wrote:
>>>> On Fri, Apr 13, 2018 at 10:53:55PM +0100, Ferruh Yigit wrote:
>>>>> On 4/10/2018 4:41 PM, Tiwei Bie wrote:
>>>>>> On Tue, Mar 13, 2018 at 06:05:34PM +0000, Ferruh Yigit wrote:
>>>>>>> Update link status related feature document items and minor updates in
>>>>>>> some link status related functions.
>>>>>>>
>>>>>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>>>>>> ---
>>>>>>>   doc/guides/nics/features/fm10k.ini      | 2 ++
>>>>>>>   doc/guides/nics/features/fm10k_vf.ini   | 2 ++
>>>>>>>   doc/guides/nics/features/i40e_vf.ini    | 1 +
>>>>>>>   doc/guides/nics/features/igb_vf.ini     | 1 +
>>>>>>>   doc/guides/nics/features/qede.ini       | 1 -
>>>>>>>   doc/guides/nics/features/qede_vf.ini    | 1 -
>>>>>>>   doc/guides/nics/features/vhost.ini      | 2 --
>>>>>>>   doc/guides/nics/features/virtio_vec.ini | 1 +
>>>>>>>   drivers/net/e1000/em_ethdev.c           | 2 +-
>>>>>>>   drivers/net/ena/ena_ethdev.c            | 2 +-
>>>>>>>   drivers/net/fm10k/fm10k_ethdev.c        | 6 ++----
>>>>>>>   drivers/net/i40e/i40e_ethdev_vf.c       | 2 +-
>>>>>>>   drivers/net/ixgbe/ixgbe_ethdev.c        | 2 +-
>>>>>>>   drivers/net/mlx4/mlx4_ethdev.c          | 2 +-
>>>>>>>   drivers/net/mlx5/mlx5_ethdev.c          | 2 +-
>>>>>>>   15 files changed, 15 insertions(+), 14 deletions(-)
>>>>>> [...]
>>>>>>> diff --git a/doc/guides/nics/features/vhost.ini b/doc/guides/nics/features/vhost.ini
>>>>>>> index dffd1f493..31302745a 100644
>>>>>>> --- a/doc/guides/nics/features/vhost.ini
>>>>>>> +++ b/doc/guides/nics/features/vhost.ini
>>>>>>> @@ -4,8 +4,6 @@
>>>>>>>   ; Refer to default.ini for the full list of available PMD features.
>>>>>>>   ;
>>>>>>>   [Features]
>>>>>>> -Link status          = Y
>>>>>>> -Link status event    = Y
>>>>>> I think vhost PMD supports above features.
>>>>> I am not able to find where it is supported.
>>>>>
>>>>> Some virtual PMDs report fixed link, with empty link_update() dev_ops, and they
>>>>> are not reported as supporting Link status, as far as I can see vhost also one
>>>>> of them.
>>>>>
>>>>> And for Link status event, PMD needs to support LSC interrupts and should
>>>>> register interrupt handler for it, which I can't find for vhost.
>>>>>
>>>>> I will send next version without updating above one, please point me where these
>>>>> support added if I missed them.
>>>> In drivers/net/vhost/rte_eth_vhost.c you could find below functions:
>>>>
>>>> static int
>>>> new_device(int vid)
>>>> {
>>>> 	......
>>>>
>>>> 	eth_dev->data->dev_link.link_status = ETH_LINK_UP;
>>>>
>>>> 	......
>>>>
>>>> 	_rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL);
>>>>
>>>> 	......
>>>> }
>>>>
>>>> static void
>>>> destroy_device(int vid)
>>>> {
>>>> 	......
>>>>
>>>> 	eth_dev->data->dev_link.link_status = ETH_LINK_DOWN;
>>>>
>>>> 	......
>>>>
>>>> 	_rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL);
>>>>
>>>> 	......
>>>> }
>>>>
>>>> They are the callbacks for vhost library.
>>>>
>>>> When a frontend (e.g. QEMU) is connected to this vhost backend
>>>> and the frontend virtio device becomes ready, new_device() will
>>>> be called by the vhost library, and the link status will be
>>>> updated to UP.
>>>>
>>>> And when e.g. the connection is closed, destroy_device() will be
>>>> called by the vhost library, and the link status will be updated
>>>> to DOWN.
>>>
>>> Got it. This behavior is similar for virtual PMDs. Provide static link
>>> information and update link as UP during start and update it as DOWN during stop.
>> No, the link status isn't updated during vhost PMD start
>> and stop. When the vhost PMD has been started, the link
>> status still may be DOWN. The link status becomes UP only
>> when the QEMU (it's another virtual machine process which
>> has a virtio device) connects to this vhost PMD via a UNIX
>> socket and the virtio driver in the virtual machine has
>> setup the virtio device of the virtual machine.
>>
>> So if vhost PMD reports the link status as DOWN, it means
>> there is no QEMU (virtual machine) connects to it or the
>> virtio device in the virtual machine hasn't been setup.
>> (PS. The frontend can also be virtio-user PMD besides QEMU)
> I believe announcing link feature reporting on virtual pmds still in gray area,
> but because of qemu involvement in vhost case, I will keep link feature but will
> drop link event.

AFAIK, link status means we can get link status through APIs like 
rte_eth_link_get(); while link status event means applications can 
register link status events, and those events get called if link status 
is changed.

If I understand it correctly, for vhost, we can keep both link status 
and link status event for vhost.

Could you specify the reason why we remove link status event?

Thanks,
Jianfeng

>
> Will send a new patch to reflect this.
>
> Thanks,
> ferruh