On 02.11.2018 12:23, Stokes, Ian wrote: >> 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.
IMHO, it's OK for the constant string. > > 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
