Re: [dpdk-dev] [RFC] ethdev: use special speed for virtual Ethernetdevices

[Stephen Hemminger <stephen@networkplumber.org>]

On Thu, 2 Apr 2020 15:50:00 +0200
Morten Brørup <mb@smartsharesystems.com> wrote:

> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ivan Dyukov
> > Sent: Thursday, April 2, 2020 2:54 PM
> > 
> > 01.04.2020 13:06, Morten Brørup пишет:  
> > >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> > >> Sent: Wednesday, April 1, 2020 11:54 AM
> > >>
> > >> 01/04/2020 11:33, Morten Brørup:  
> > >>> Thomas, Ferruh, Andrew (Ethernet API Maintainers),
> > >>>
> > >>> A command line option was recently added to set which speed a vNIC  
> > >> reports when the link is up. This makes sense for Spanning Tree and
> > >> other protocols which depend on link speed.
> > >>
> > >> Please could you reference the patch?  
> > > It is a patch for the virtio driver:
> > > https://protect2.fireeye.com/url?k=e37beb37-beabe3df-e37a6078-  
> > 000babff32e3-
> > 4aaaa0986ed7ec57&u=http://inbox.dpdk.org/dev/20191212085012.9170-1-
> > i.dyukov@samsung.com/T/#m052f90ea8c559406aeaefaea1fc24ed9bb573788
> > This patch is related to similar work in qemu & kernel virtio driver.
> > Please see
> > https://lists.oasis-open.org/archives/virtio-
> > comment/201911/msg00058.html.
> > These changes have been implemented and released in kernel and qemu.
> > speed is negotiated from qemu and then user may change the speed of
> > virtio device using ethtool utility. I have added similiar patchset for
> > pmd driver which do the same but for changing speed I used devargs
> > instead of ethtool.  
> 
> Very interesting link, indeed!
> 
> It gives the virtio driver the possibility to expose a specific speed in Mbit/s, which I assume - when used correctly - should reflect the speed of the underlying hardware. So it could be 20 Gbit/s for a link aggregate (in IEEE 802.3 Ethernet terminology; "bond" in Linux terminology) of two 10G ports.
> 
> It also provides a special value for unknown speed.
> 
> > >  
> > >>> However, I suspect that this workaround rarely reflects the  
> > physical  
> > >> truth, and suggest that the application should handle it instead.
> > >>
> > >> I don't understand why we need to define some speed for virtual
> > >> devices.
> > >>  
> > >>> In other words... Instead of faking it in the virtual Ethernet  
> > >> drivers, I suggest that rte_ethdev.h defines a special speed value  
> > for  
> > >> vNICs which really don't have a physical link speed:  
> > >>> #define ETH_SPEED_NUM_NONE         0 /**< Not defined */  
> > >> The only issue with this constant is the lack of RTE_ prefix :-)
> > >> Otherwise I think "0 - NONE - not defined" fits well with virtual
> > >> device case.
> > >>  
> > >>> +#define ETH_SPEED_NUM_UNKNOWN      1 /**< Unknown (virtual device)  
> > >> */
> > >>
> > >> 1 means 1 Mbps
> > >>  
> > >>> #define ETH_SPEED_NUM_10M         10 /**<  10 Mbps */
> > >>>
> > >>> Alternatively, we could expand the meaning of ETH_SPEED_NUM_NONE:
> > >>>
> > >>> -#define ETH_SPEED_NUM_NONE         0 /**< Not defined */
> > >>> +#define ETH_SPEED_NUM_NONE         0 /**< Not defined or unknown  
> > >> (virtual device) */
> > >>
> > >> Yes I agree with extending the comment for NONE.
> > >>  
> > >>> The special value could also be used in cases like this:
> > >>>  
> > >> https://protect2.fireeye.com/url?k=6154668d-3c846e65-6155edc2-  
> > 000babff32e3-
> > bf63d034253cac80&u=http://inbox.dpdk.org/dev/AM0PR0502MB401907ADE7CEA27
> > DC642DF35D2CB0@AM0P  
> > >> R0502MB4019.eurprd05.prod.outlook.com/T/#t
> > >>
> > >> Yes, if speed is unknown, it should be reported as 0.  
> 
> Could the DPDK vNIC PMDs be updated accordingly? At least the virtio driver...
> 
> > > So the next related question is: Should a vNIC be allowed to report a  
> > fake speed if it does not know that the underlying hardware actually
> > provides this speed?  
> 
> Your link to the Kernel/QEMU patch answers this question: Yes, because we assume it reflects the underlying speed.
> 
> 

The problem is that if devices start returning this new value,
then it is an API breakage for existing applications that look at speed.