> On 02.11.2018 1:31, Ian Stokes wrote:
> > Report the link speed of the device in netdev_dpdk_get_status()
> > function.
> >
> > Link speed is already reported as part of the netdev_get_features()
> > function. However only link speeds defined in the OpenFlow specs are
> > supported so speeds such as 25 Gbps etc. are not shown. The link speed
> > for the device is available in Mbps in rte_eth_link.
> > This commit converts the link speed for a given dpdk device to an easy
> > to read string and reports it in get_status().
> >
> > Suggested-by: Flavio Leitner <[email protected]>
> > Signed-off-by: Ian Stokes <[email protected]>
> > ---
> > v1 -> v2
> > * Replace snprintf with a return call of for each string.
> > ---
> >  lib/netdev-dpdk.c | 35 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 35 insertions(+)
> >
> > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
> > 59f4ccbfe..5e6a72bc9 100644
> > --- a/lib/netdev-dpdk.c
> > +++ b/lib/netdev-dpdk.c
> > @@ -3021,11 +3021,37 @@ netdev_dpdk_vhost_user_get_status(const struct
> netdev *netdev,
> >      return 0;
> >  }
> >
> > +/*
> > + * Convert a given uint32_t link speed defined in DPDK to a string
> > + * equivalent.
> > + */
> > +static const char *
> > +netdev_dpdk_link_speed_to_str__(uint32_t link_speed) {
> > +    switch (link_speed) {
> > +    case ETH_SPEED_NUM_10M:    return "10Mbps";
> > +    case ETH_SPEED_NUM_100M:   return "100Mbps";
> > +    case ETH_SPEED_NUM_1G:     return "1Gbps";
> > +    case ETH_SPEED_NUM_2_5G:   return "2.5Gbps";
> > +    case ETH_SPEED_NUM_5G:     return "5Gbps";
> > +    case ETH_SPEED_NUM_10G:    return "10Gbps";
> > +    case ETH_SPEED_NUM_20G:    return "20Gbps";
> > +    case ETH_SPEED_NUM_25G:    return "25Gbps";
> > +    case ETH_SPEED_NUM_40G:    return "40Gbps";
> > +    case ETH_SPEED_NUM_50G:    return "50Gbps";
> > +    case ETH_SPEED_NUM_56G:    return "56Gbps";
> > +    case ETH_SPEED_NUM_100G:   return "100Gbps";
> > +    default:                   return "Not Defined";
> > +    }
> > +}
> > +
> >  static int
> >  netdev_dpdk_get_status(const struct netdev *netdev, struct smap
> > *args)  {
> >      struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
> >      struct rte_eth_dev_info dev_info;
> > +    struct rte_eth_link link;
> > +    const char *link_speed_str;
> >
> >      if (!rte_eth_dev_is_valid_port(dev->port_id)) {
> >          return ENODEV;
> > @@ -3033,6 +3059,7 @@ netdev_dpdk_get_status(const struct netdev
> > *netdev, struct smap *args)
> >
> >      ovs_mutex_lock(&dev->mutex);
> >      rte_eth_dev_info_get(dev->port_id, &dev_info);
> > +    link = dev->link;
> >      ovs_mutex_unlock(&dev->mutex);
> >
> >      smap_add_format(args, "port_no", DPDK_PORT_ID_FMT, dev->port_id);
> > @@ -3064,6 +3091,14 @@ netdev_dpdk_get_status(const struct netdev
> *netdev, struct smap *args)
> >                          dev_info.pci_dev->id.device_id);
> >      }
> >
> > +    /* Not all link speeds are defined in the OpenFlow specs e.g. 25
> Gbps.
> > +     * In that case the speed will not be reported as part of the usual
> > +     * call to get_features(). Get the link speed of the device and add
> it
> > +     * to the device status in an easy to read string format.
> > +     */
> > +    link_speed_str = netdev_dpdk_link_speed_to_str__(link.link_speed);
> > +    smap_add_format(args, "link_speed", "%s", link_speed_str);
> 
> How about this:
> 
>     smap_add(args, "link_speed",
>              netdev_dpdk_link_speed_to_str__(link.link_speed));
> 
> ?

Yes, I'm happy to use above.

On a side note, I was going to use this approach but wasn't sure as regards the 
preference of adding a parameter to a function call via a direct return from a 
another function.

I've come across examples in other reviews wherein this approach is not 
preferred, in particular when dealing with if statements. Maybe it's a coding 
style preference, there doesn’t seem to be any definition within the OVS style 
guide.

Ian
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to