From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ferruh Yigit Subject: Re: [PATCH] drivers/net: update link status Date: Tue, 17 Apr 2018 12:26:42 +0100 Message-ID: References: <20180313180534.232296-1-ferruh.yigit@intel.com> <20180410154102.uv7og3ff4y5ylc3m@debian> <20180414105546.25vqtpcfkexwlkhp@debian> <20180417045427.6guh7x7yqpyfdnwj@debian> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: Qi Zhang , Xiao Wang , John McNamara , Marko Kovacevic , Beilei Xing , Wenzhuo Lu , Rasesh Mody , Harish Patil , Shahed Shaikh , Tetsuya Mukawa , Yuanhan Liu , Maxime Coquelin , Marcin Wojtas , Michal Krawczyk , Guy Tzalik , Evgeny Schemeilin , Konstantin Ananyev , Adrien Mazarguil , Nelio Laranjeiro , Yongseok Koh , dev@dpdk.org To: Tiwei Bie Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 4FB90A48F for ; Tue, 17 Apr 2018 13:26:49 +0200 (CEST) In-Reply-To: <20180417045427.6guh7x7yqpyfdnwj@debian> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 >>>>>> --- >>>>>> 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. Will send a new patch to reflect this. Thanks, ferruh > > Thanks > >> >> Other virtual PMDs doesn't report this feature, so removed from vhost as well >> for consistency. >> >>> >>> So vhost PMD reports meaningful link status and also generates >>> link status events. >> >> Yes PMD process user callbacks on link change [1], but I am not sure that is >> what meant from "link status event", what I understand is link interrupts >> supported in PMD level which seems not the case for vhost. >> >> [1] >> This is something else but why calling user callback in link update is in PMD >> discretion, shouldn't it be something done automatically in ethdev layer, somehow. >> >>> >>> Thanks >>> >>