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

Flavio Leitner fbl at sysclose.org
Mon Sep 3 17:54:35 UTC 2018


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> 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>> 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>>
> > >      > ---
> > >      >  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>
> > >      > 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
> > >
> >
> >
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

-- 
Flavio




More information about the dev mailing list