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 <f...@sysclose.org <mailto:f...@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.sto...@intel.com
    <mailto:ian.sto...@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.sto...@intel.com <mailto:ian.sto...@intel.com>
     > > > <mailto:ian.sto...@intel.com <mailto:ian.sto...@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.binb...@zte.com.cn
    <mailto:xu.binb...@zte.com.cn>
     > > >     <mailto:xu.binb...@zte.com.cn
    <mailto:xu.binb...@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
     > > >      > d...@openvswitch.org <mailto:d...@openvswitch.org>
    <mailto:d...@openvswitch.org <mailto:d...@openvswitch.org>>
     > > >      > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
     > > >     _______________________________________________
     > > >     dev mailing list
     > > > d...@openvswitch.org <mailto:d...@openvswitch.org>
    <mailto:d...@openvswitch.org <mailto:d...@openvswitch.org>>
     > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
     > > >
     > >
     > >
     > _______________________________________________
     > dev mailing list
     > d...@openvswitch.org <mailto:d...@openvswitch.org>
     > https://mail.openvswitch.org/mailman/listinfo/ovs-dev

-- Flavio



_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to