Re: [dpdk-dev] [PATCH v4 0/2] ethdev: add port speed capability bitmap

["Nélio Laranjeiro" <nelio.laranjeiro@6wind.com>]

On Mon, Sep 07, 2015 at 10:52:53PM +0200, Marc Sune wrote:
> 2015-08-29 2:16 GMT+02:00 Marc Sune <marcdevel@gmail.com>:
> 
> > The current rte_eth_dev_info abstraction does not provide any mechanism to
> > get the supported speed(s) of an ethdev.
> >
> > For some drivers (e.g. ixgbe), an educated guess can be done based on the
> > driver's name (driver_name in rte_eth_dev_info), see:
> >
> > http://dpdk.org/ml/archives/dev/2013-August/000412.html
> >
> > However, i) doing string comparisons is annoying, and can silently
> > break existing applications if PMDs change their names ii) it does not
> > provide all the supported capabilities of the ethdev iii) for some drivers
> > it
> > is impossible determine correctly the (max) speed by the application
> > (e.g. in i40, distinguish between XL710 and X710).
> >
> > This small patch adds speed_capa bitmap in rte_eth_dev_info, which is
> > filled
> > by the PMDs according to the physical device capabilities.
> >
> > v2: rebase, converted speed_capa into 32 bits bitmap, fixed alignment
> > (checkpatch).
> >
> > v3: rebase to v2.1. unified ETH_LINK_SPEED and ETH_SPEED_CAP into
> > ETH_SPEED.
> >     Converted field speed in struct rte_eth_conf to speeds, to allow a
> > bitmap
> >     for defining the announced speeds, as suggested by M. Brorup. Fixed
> >     spelling issues.
> >
> > v4: fixed errata in the documentation of field speeds of rte_eth_conf, and
> >     commit 1/2 message. rebased to v2.1.0. v3 was incorrectly based on
> >     ~2.1.0-rc1.
> >
> 
> Thomas,
> 
> Since mostly you were commenting for v1 and v2; any opinion on this one?
> 
> Regards
> marc

Hi Marc,

I have read your patches, and there are a few mistakes, for instance mlx4
(ConnectX-3 devices) does not support 100Gbps.

In addition, it seems your new bitmap does not support all kind of
speeds, take a look at the header of Ethtool, in the Linux kernel
(include/uapi/linux/ethtool.h) which already consumes 30bits without even
managing speeds above 56Gbps.  

It would be nice to keep the field to represent the real speed of the
link, in case it is not represented by the bitmap, it could be also
useful for aggregated links (bonding for instance).  The current API
already works this way, it just needs to be extended from 16 to 32 bit
to manage speed above 64Gbps.
 
>[...]

Nélio
-- 
Nélio Laranjeiro
6WIND