From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by dpdk.org (Postfix) with ESMTP id 68E4B8D93 for ; Wed, 18 Apr 2018 12:43:03 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga107.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 18 Apr 2018 03:43:02 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.48,464,1517904000"; d="scan'208";a="221376980" Received: from fyigit-mobl.ger.corp.intel.com (HELO [10.237.221.42]) ([10.237.221.42]) by fmsmga006.fm.intel.com with ESMTP; 18 Apr 2018 03:42:57 -0700 To: "Tan, Jianfeng" , Tiwei Bie 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 References: <20180313180534.232296-1-ferruh.yigit@intel.com> <20180410154102.uv7og3ff4y5ylc3m@debian> <20180414105546.25vqtpcfkexwlkhp@debian> <20180417045427.6guh7x7yqpyfdnwj@debian> <3517e7a5-dec6-fca1-b24f-8fdc2691216c@intel.com> From: Ferruh Yigit Openpgp: preference=signencrypt Autocrypt: addr=ferruh.yigit@intel.com; prefer-encrypt=mutual; keydata= xsFNBFXZCFABEADCujshBOAaqPZpwShdkzkyGpJ15lmxiSr3jVMqOtQS/sB3FYLT0/d3+bvy qbL9YnlbPyRvZfnP3pXiKwkRoR1RJwEo2BOf6hxdzTmLRtGtwWzI9MwrUPj6n/ldiD58VAGQ +iR1I/z9UBUN/ZMksElA2D7Jgg7vZ78iKwNnd+vLBD6I61kVrZ45Vjo3r+pPOByUBXOUlxp9 GWEKKIrJ4eogqkVNSixN16VYK7xR+5OUkBYUO+sE6etSxCr7BahMPKxH+XPlZZjKrxciaWQb +dElz3Ab4Opl+ZT/bK2huX+W+NJBEBVzjTkhjSTjcyRdxvS1gwWRuXqAml/sh+KQjPV1PPHF YK5LcqLkle+OKTCa82OvUb7cr+ALxATIZXQkgmn+zFT8UzSS3aiBBohg3BtbTIWy51jNlYdy ezUZ4UxKSsFuUTPt+JjHQBvF7WKbmNGS3fCid5Iag4tWOfZoqiCNzxApkVugltxoc6rG2TyX CmI2rP0mQ0GOsGXA3+3c1MCdQFzdIn/5tLBZyKy4F54UFo35eOX8/g7OaE+xrgY/4bZjpxC1 1pd66AAtKb3aNXpHvIfkVV6NYloo52H+FUE5ZDPNCGD0/btFGPWmWRmkPybzColTy7fmPaGz cBcEEqHK4T0aY4UJmE7Ylvg255Kz7s6wGZe6IR3N0cKNv++O7QARAQABzSVGZXJydWggWWln aXQgPGZlcnJ1aC55aWdpdEBpbnRlbC5jb20+wsF+BBMBAgAoAhsDBgsJCAcDAgYVCAIJCgsE FgIDAQIeAQIXgAUCWZR3VQUJB33WBQAKCRD5M+tD3xNhH6DWEACVhEb8q1epPwZrUDoxzu7E TS1b8tmabOmnjXZRs6+EXgUVHkp2xxkCfDmL3pa5bC0G/74aJnWjNsdvE05V1cb4YK4kRQ62 FwDQ+hlrFrwFB3PtDZk1tpkzCRHvJgnIil+0MuEh32Y57ig6hy8yO8ql7Lohyrnpfk/nNpm4 jQGEF5qEeHcEFe1AZQlPHN/STno8NZSz2nl0b2cw+cujN1krmvB52Ah/2KugQ6pprVyrGrzB c34ZQO9OsmSjJlETCZk6EZzuhfe16iqBFbOSadi9sPcJRwaUQBid+xdFWl7GQ8qC3zNPibSF HmU43yBZUqJDZlhIcl6/cFpOSjv2sDWdtjEXTDn5y/0FsuY0mFE78ItC4kCTIVk17VZoywcd fmbbnwOSWzDq7hiUYuQGkIudJw5k/A1CMsyLkoUEGN3sLfsw6KASgS4XrrmPO4UVr3mH5bP1 yC7i1OVNpzvOxtahmzm481ID8sk72GC2RktTOHb0cX+qdoiMMfYgo3wRRDYCBt6YoGYUxF1p msjocXyqToKhhnFbXLaZlVfnQ9i2i8jsj9SKig+ewC2p3lkPj6ncye9q95bzhmUeJO6sFhJg Hiz6syOMg8yCcq60j07airybAuHIDNFWk0gaWAmtHZxLObZx2PVn2nv9kLYGohFekw0AOsIW ta++5m48dnCoAc7BTQRX1ky+ARAApzQNvXvE2q1LAS+Z+ni2R13Bb1cDS1ZYq1jgpR13+OKN ipzd8MPngRJilXxBaPTErhgzR0vGcNTYhjGMSyFIHVOoBq1VbP1a0Fi/NqWzJOowo/fDfgVy K4vuitc/gCJs+2se4hdZA4EQJxVlNM51lgYDNpjPGIA43MX15OLAip73+ho6NPBMuc5qse3X pAClNhBKfENRCWN428pi3WVkT+ABRTE0taxjJNP7bb+9TQYNRqGwnGzX5/XISv44asWIQCaq vOkXSUJLd//cdVNTqtL1wreCVVR5pMXj7VIrlk07fmmJVALCmGbFr53BMb8O+8dgK2A5mitM n44d+8KdJWOwziRxcaMk/LclmZS3Iv1TERtiWt98Y9AjeAtcgYPkA3ld0BcUKONogP8pHVz1 Ed3s5rDQ91yr1S0wuAzW91fxGUO4wY+uPmxCtFVuBgd9VT9NAKTUL0qHM7CDgCnZPe0TW6Zj 8OqtdCCyAfvU9cW5xWM7Icxhde6AtPxhDSBwE8fL2ZmrDmaA4jmUKXp3i4JxRPSX84S08b+s DWXHPxy10UFU5A7EK/BEbZAKBwn9ROfm+WK+6X5xOGLoRE++OqNuUudxC1GDyLOPaqCbBCS9 +P6HsTHzxsjyJa27n4jcrcuY3P9TEcFJYSZSeSDh8mVGvugi0exnSJrrBZDyVCcAEQEAAcLB ZQQYAQIADwIbDAUCWZR1ZwUJA59cIQAKCRD5M+tD3xNhH5b+D/9XG44Ci6STdcA5RO/ur05J EE3Ux1DCHZ5V7vNAtX/8Wg4l4GZfweauXwuJ1w7Sp7fklwcNC6wsceI+EmNjGMqfIaukGetG +jBGqsQ7moOZodfXUoCK98gblKgt/BPYMVidzlGC8Q/+lZg1+o29sPnwImW+MXt/Z5az/Z17 Qc265g+p5cqJHzq6bpQdnF7Fu6btKU/kv6wJghENvgMXBuyThqsyFReJWFh2wfaKyuix3Zyj ccq7/blkhzIKmtFWgDcgaSc2UAuJU+x9nuYjihW6WobpKP/nlUDu3BIsbIq09UEke+uE/QK+ FJ8PTJkAsXOf1Bc2C0XbW4Y2hf103+YY6L8weUCBsWC5VH5VtVmeuh26ENURclwfeXhWQ9Og 77yzpTXWr5g1Z0oLpYpWPv745J4bE7pv+dzxOrFdM1xNkzY2pvXph/A8OjxZNQklDkHQ7PIB Lki5L2F4XkEOddUUQchJwzMqTPsggPDmGjgLZrqgO+s4ECZK5+nLD3HEpAbPa3JLDaScy+90 Nu1lAqPUHSnP3vYZVw85ZYm6UCxHE4VLMnnJsN09ZhsOSVR+GyP5Nyw9rT1V3lcsuH7M5Naa 2Xobn9m7l9bRCD/Ji8kG15eV1WTxx1HXVQGjdUYDI7UwegBNbwMLh17XDy+3sn/6SgcqtECA Q6pZKA2mTQxEKMLBZQQYAQIADwIbDAUCWZR3hQUJA59eRwAKCRD5M+tD3xNhH4a/D/4jLAZu UhvU1swWcNEVVCELZ0D3LOV14XcY2MXa3QOpeZ9Bgq7YYJ4S5YXK+SBQS0FkRZdjGNvlGZoG ZdpU+NsQmQFhqHGwX0IT9MeTFM8uvKgxNKGwMVcV9g0IOqwBhGHne+BFboRA9362fgGW5AYQ zT0mzzRKEoOh4r3AQvbM6kLISxo0k1ujdYiI5nj/5WoKDqxTwwfuN1uDUHsWo3tzenRmpMyU NyW3Dc+1ajvXLyo09sRRq7BnM99Rix1EGL8Qhwy+j0YAv+FuspWxUX9FxXYho5PvGLHLsHfK FYQ7x/RRbpMjkJWVfIe/xVnfvn4kz+MTA5yhvsuNi678fLwY9hBP0y4lO8Ob2IhEPdfnTuIs tFVxXuelJ9xAe5TyqP0f+fQjf1ixsBZkqOohsBXDfje0iaUpYa/OQ/BBeej0dUdg2JEu4jAC x41HpVCnP9ipLpD0fYz1d/dX0F/VY2ovW6Eba/y/ngOSAR6C+u881m7oH2l0G47MTwkaQCBA bLGXPj4TCdX3lftqt4bcBPBJ+rFAnJmRHtUuyyaewBnZ81ZU2YAptqFM1kTh+aSvMvGhfVsQ qZL2rk2OPN1hg+KXhErlbTZ6oPtLCFhSHQmuxQ4oc4U147wBTUuOdwNjtnNatUhRCp8POc+3 XphVR5G70mnca1E2vzC77z+XSlTyRA== Message-ID: Date: Wed, 18 Apr 2018 11:42:56 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <3517e7a5-dec6-fca1-b24f-8fdc2691216c@intel.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH] drivers/net: update link status X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 18 Apr 2018 10:43:04 -0000 On 4/18/2018 7:49 AM, Tan, Jianfeng wrote: > 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 >>>>>>>> --- >>>>>>>> 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? Hi Jianfeng, I think problem is the definition of the features are not clear, that is why I started a doc to document them (doc/guides/nics/features.rst) "Link status", I think we agree on this one. PMD should provide up-to-date, valid link data on rte_eth_dev_data->dev_link. So that this link information can be get by rte_eth_link_get(), rte_eth_link_get_nowait() ethdev APIs. rte_eth_link_get() uses dev_ops->link_update() to get the latest link information, for virtual PMDs, including vhost, this function does nothing, because there is no actual physical link. That is why I was not sure advertising link status feature for vhost, and other virtual PMDs doesn't report this feature. After Tiwei's comment that link status shows that qemu connected to vhost, added this feature back to vhost PMD. "Link status event", can be a- PMD calls application callback in link change b- PMD registers interrupt handler for link status change interrupts Based on how other PMDs report this feature, I believe it is (b), and I have documented that way. And vhost "Link status event" feature removed based on this. There are some set of config options and flags to control the LSC interrupt, that also effects rte_eth_link_get() and rte_eth_link_get_nowait() APIs, I believe that is the main concern here. As commented above, I don't understand why calling user register callback for link change event is something on PMD decision, it should be default behavior for PMD. What is the point of leaving this into PMD and think this as a feature of PMD? Overall, practical reason of this table is to inform developer/user about PMD features, which is indeed device + driver features, and help her to in development or on setting expectations. We can always discuss what helps more to developer/user and update the features table. Thanks, ferruh > > Thanks, > Jianfeng > >> >> Will send a new patch to reflect this. >> >> Thanks, >> ferruh >