[ovs-dev] netdev-dpdk: Support the link speed of XL710

Ian Stokes ian.stokes at intel.com
Thu Oct 25 14:25:31 UTC 2018


On 10/11/2018 11:39 AM, Federico Iezzi wrote:
> So, any news on the other link speeds like 25, 50, and 100Gbps?

Hi Federico,

I've sent out a patch series to help address this based on the previous 
discussion.

https://mail.openvswitch.org/pipermail/ovs-dev/2018-October/353292.html
https://mail.openvswitch.org/pipermail/ovs-dev/2018-October/353293.html

Feedback welcome.

Thanks
Ian

> 
> Thanks
> 
> On Mon, 3 Sep 2018 at 19:54, Flavio Leitner <fbl at sysclose.org 
> <mailto:fbl at sysclose.org>> wrote:
> 
>     On Fri, Aug 31, 2018 at 05:54:25AM +0200, Federico Iezzi wrote:
>      > Any comment here?
>      > This seems like a very easy commit :-)
>      >
>      >
>      >
>      > On Thu, 23 Aug 2018 at 13:34, Ian Stokes <ian.stokes at intel.com
>     <mailto:ian.stokes at intel.com>> wrote:
>      >
>      > > On 8/22/2018 6:14 PM, Federico Iezzi wrote:
>      > > > DPDK exposes API all the way from 10Mbps to 100Gbps.
>      > > > http://doc.dpdk.org/api/rte__ethdev_8h_source.html
>      > > >
>      > > > Can other cards be added? 25G is now getting really popular.
>      > > >
>      > > > Thanks
>      > >
>      > > It’s a good point, technically there’s nothing stopping users
>     from using
>      > > 25/50/56/100 Gbp HW.
>      > >
>      > > 25/50/56 Gb are not defined specifically as a port feature rate
>     in the
>      > > openflow specifications at this time so they would have to be
>     defined as
>      > > NETDEV_F_OTHER to correlate to the feature rate not being in the
>      > > ofp_port feature list in openflow.
>      > >
>      > > The following incremental on the patch below should suffice:
>      > >
>      > > @@ -2735,9 +2735,21 @@ netdev_dpdk_get_features(const struct netdev
>      > > *netdev,
>      > >           if (link.link_speed == ETH_SPEED_NUM_10G) {
>      > >               *current = NETDEV_F_10GB_FD;
>      > >           }
>      > > +        if (link.link_speed == ETH_SPEED_NUM_25G) {
>      > > +            *current = NETDEV_F_OTHER;
>      > > +        }
>      > >           if (link.link_speed == ETH_SPEED_NUM_40G) {
>      > >               *current = NETDEV_F_40GB_FD;
>      > >           }
>      > > +        if (link.link_speed == ETH_SPEED_NUM_50G) {
>      > > +            *current = NETDEV_F_OTHER;
>      > > +        }
>      > > +        if (link.link_speed == ETH_SPEED_NUM_56G) {
>      > > +            *current = NETDEV_F_OTHER;
>      > > +        }
>      > > +        if (link.link_speed == ETH_SPEED_NUM_100G) {
>      > > +            *current = NETDEV_F_100GB_FD;
>      > > +        }
>      > >
>      > > What are peoples thoughts? I can submit this as a separate patch if
>      > > preferred.
> 
>     That's all the supported speeds OF defines as you mentioned, but there
>     are free bits there if it's important to report a more accurate speed.
> 
>     Alternatively we could expose that information through get_status() as
>     a translated string most probably.
> 
>     What worries me is that 'current' variable is allocated in the stack and
>     dpdk doesn't zero it like bsd or linux does, so if speed falls out
>     of those
>     values, it will use whatever was in the stack before as reported
>     originally.
> 
>     Perhaps we could do:
> 
>           uint32_t supported = 0;
> 
>           if (link.link_duplex == ETH_LINK_FULL_DUPLEX) {
>                switch (link.link_speed) {
>                    /* OpenFlow defined values: see enum ofp_port_features */
>                    [...]
>                    case ETH_SPEED_NUM_10G:
>                       supported |= NETDEV_F_10GB_FD;
>                       break;
>                    case ETH_SPEED_NUM_40G:
>                       supported |= NETDEV_F_40GB_FD;
>                       break;
>                    case ETH_SPEED_NUM_100G:
>                       supported |= NETDEV_F_100GB_FD
>                       break;
>                    default:
>                      supported |= NETDEV_F_OTHER;
>                }
>           else if (link.link_duplex == ETH_LINK_HALF_DUPLEX) {
>                    [...]
>           }
> 
>           if (link.link_autoneg) {
>               supported |= NETDEV_F_AUTONEG;
>           }
> 
>           *current = supported;
>           *advertised = *supported = *peer = 0;
> 
>           return 0;
> 
> 
>     fbl
> 
>      > >
>      > > Thanks
>      > > Ian
>      > >
>      > > Ian
>      > >
>      > > >
>      > > > On Wed, 22 Aug 2018 at 16:28, Stokes, Ian
>     <ian.stokes at intel.com <mailto:ian.stokes at intel.com>
>      > > > <mailto:ian.stokes at intel.com <mailto:ian.stokes at intel.com>>>
>     wrote:
>      > > >
>      > > >      > In the scenario of XL710, the link speed which stored
>     in the
>      > > table of
>      > > >      > Interface is not 40G. Because the implementation of
>     query of link
>      > > >     speed
>      > > >      > only support to 10G, the parameter 'current' will be a
>     random
>      > > >     value in the
>      > > >      > scenario of higher link speed. In this case, incorrect
>     link speed
>      > > >     of XL710
>      > > >      > nic will be stored in the database.
>      > > >      >
>      > > >
>      > > >     Good catch, I've tested and it works as expected. I'll
>     add this to
>      > > >     the dpdk_merge pull request for this week and backport it
>     to the
>      > > >     previous release branches also.
>      > > >
>      > > >     Thanks
>      > > >     Ian
>      > > >
>      > > >      > Signed-off-by: Xu Binbin <xu.binbin1 at zte.com.cn
>     <mailto:xu.binbin1 at zte.com.cn>
>      > > >     <mailto:xu.binbin1 at zte.com.cn
>     <mailto:xu.binbin1 at zte.com.cn>>>
>      > > >      > ---
>      > > >      >  lib/netdev-dpdk.c | 3 +++
>      > > >      >  1 file changed, 3 insertions(+)
>      > > >      >
>      > > >      > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
>      > > >     ac02a09..e4b6ced
>      > > >      > 100644
>      > > >      > --- a/lib/netdev-dpdk.c
>      > > >      > +++ b/lib/netdev-dpdk.c
>      > > >      > @@ -2735,6 +2735,9 @@ netdev_dpdk_get_features(const
>     struct netdev
>      > > >      > *netdev,
>      > > >      >          if (link.link_speed == ETH_SPEED_NUM_10G) {
>      > > >      >              *current = NETDEV_F_10GB_FD;
>      > > >      >          }
>      > > >      > +        if (link.link_speed == ETH_SPEED_NUM_40G) {
>      > > >      > +            *current = NETDEV_F_40GB_FD;
>      > > >      > +        }
>      > > >      >      }
>      > > >      >
>      > > >      >      if (link.link_autoneg) {
>      > > >      > --
>      > > >      > 1.8.3.1
>      > > >      >
>      > > >      > _______________________________________________
>      > > >      > dev mailing list
>      > > >      > dev at openvswitch.org <mailto:dev at openvswitch.org>
>     <mailto:dev at openvswitch.org <mailto:dev at openvswitch.org>>
>      > > >      > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>      > > >     _______________________________________________
>      > > >     dev mailing list
>      > > > dev at openvswitch.org <mailto:dev at openvswitch.org>
>     <mailto:dev at openvswitch.org <mailto:dev at openvswitch.org>>
>      > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>      > > >
>      > >
>      > >
>      > _______________________________________________
>      > dev mailing list
>      > dev at openvswitch.org <mailto:dev at openvswitch.org>
>      > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 
>     -- 
>     Flavio
> 
> 



More information about the dev mailing list