[ovs-dev,v5,1/2] netdev-dpdk: extend netdev_dpdk_get_status to include if_type and if_descr

Message ID 1506955769-115602-1-git-send-email-michalx.weglicki@intel.com
State Changes Requested
Headers show
Series
  • [ovs-dev,v5,1/2] netdev-dpdk: extend netdev_dpdk_get_status to include if_type and if_descr
Related show

Commit Message

Michal Weglicki Oct. 2, 2017, 2:49 p.m.
This commit extends netdev_dpdk_get_status API to include additional
driver-related information: if_type and if_descr.

v2->v3: Code rebase.
v3->v4: Minor comments applied.

Co-authored-by: Michal Weglicki <michalx.weglicki@intel.com>
Signed-off-by: Michal Weglicki <michalx.weglicki@intel.com>
Signed-off-by: Przemyslaw Szczerbik <przemyslawx.szczerbik@intel.com>
---
 lib/netdev-dpdk.c | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Gregory Rose Oct. 3, 2017, 10:09 p.m. | #1
On 10/02/2017 07:49 AM, Michal Weglicki wrote:
> This commit extends netdev_dpdk_get_status API to include additional
> driver-related information: if_type and if_descr.
> 
> v2->v3: Code rebase.
> v3->v4: Minor comments applied.
> 
> Co-authored-by: Michal Weglicki <michalx.weglicki@intel.com>
> Signed-off-by: Michal Weglicki <michalx.weglicki@intel.com>
> Signed-off-by: Przemyslaw Szczerbik <przemyslawx.szczerbik@intel.com>
> ---
>   lib/netdev-dpdk.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index c60f46f..5cc70d1 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -36,6 +36,7 @@
>   #include <rte_meter.h>
>   #include <rte_pci.h>
>   #include <rte_vhost.h>
> +#include <rte_version.h>
>   
>   #include "dirs.h"
>   #include "dp-packet.h"
> @@ -2476,6 +2477,14 @@ netdev_dpdk_get_status(const struct netdev *netdev, struct smap *args)
>       smap_add_format(args, "max_vfs", "%u", dev_info.max_vfs);
>       smap_add_format(args, "max_vmdq_pools", "%u", dev_info.max_vmdq_pools);
>   
> +    /* Querying the DPDK library for iftype may be done in future, pending
> +     * support; cf. RFC 3635 Section 3.2.4. */
> +    enum { IF_TYPE_ETHERNETCSMACD = 6 };
> +
> +    smap_add_format(args, "if_type", "%"PRIu32, IF_TYPE_ETHERNETCSMACD);
> +    smap_add_format(args, "if_descr", "%s %s", rte_version(),
> +                                               dev_info.driver_name);
> +
>       if (dev_info.pci_dev) {
>           smap_add_format(args, "pci-vendor_id", "0x%u",
>                           dev_info.pci_dev->id.vendor_id);
> 

Tested-by: Greg Rose <gvrose8192@gmail.com>
Reviewed-by: Greg Rose <gvrose8192@gmail.com>
Ben Pfaff Oct. 30, 2017, 11:13 p.m. | #2
On Mon, Oct 02, 2017 at 03:49:28PM +0100, Michal Weglicki wrote:
> This commit extends netdev_dpdk_get_status API to include additional
> driver-related information: if_type and if_descr.
> 
> v2->v3: Code rebase.
> v3->v4: Minor comments applied.
> 
> Co-authored-by: Michal Weglicki <michalx.weglicki@intel.com>
> Signed-off-by: Michal Weglicki <michalx.weglicki@intel.com>
> Signed-off-by: Przemyslaw Szczerbik <przemyslawx.szczerbik@intel.com>

Thank you for this series.  It's always great to have more IPFIX support
and more status information for an interface.

I'm happy with this series, except for one thing that I noticed in this
particular patch: there is no documentation.  The various status keys
supported by dpdk should be documented in vswitch.xml near the existing
status keys, which currently start around line 2781:

      <column name="status">
        Key-value pairs that report port status.  Supported status values are
        <ref column="type"/>-dependent; some interfaces may not have a valid
        <ref column="status" key="driver_name"/>, for example.
      </column>

      <column name="status" key="driver_name">
        The name of the device driver controlling the network adapter.
      </column>

      <column name="status" key="driver_version">
        The version string of the device driver controlling the network
        adapter.
      </column>

and so on.  Since there are several for dpdk, maybe they should be
grouped together in a <group>...</group>.

I see that the existing dpdk status keys aren't documented either.  If
you can figure out what they are for, then it would be helpful to
document them too.

Thanks,

Ben.

Patch

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index c60f46f..5cc70d1 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -36,6 +36,7 @@ 
 #include <rte_meter.h>
 #include <rte_pci.h>
 #include <rte_vhost.h>
+#include <rte_version.h>
 
 #include "dirs.h"
 #include "dp-packet.h"
@@ -2476,6 +2477,14 @@  netdev_dpdk_get_status(const struct netdev *netdev, struct smap *args)
     smap_add_format(args, "max_vfs", "%u", dev_info.max_vfs);
     smap_add_format(args, "max_vmdq_pools", "%u", dev_info.max_vmdq_pools);
 
+    /* Querying the DPDK library for iftype may be done in future, pending
+     * support; cf. RFC 3635 Section 3.2.4. */
+    enum { IF_TYPE_ETHERNETCSMACD = 6 };
+
+    smap_add_format(args, "if_type", "%"PRIu32, IF_TYPE_ETHERNETCSMACD);
+    smap_add_format(args, "if_descr", "%s %s", rte_version(),
+                                               dev_info.driver_name);
+
     if (dev_info.pci_dev) {
         smap_add_format(args, "pci-vendor_id", "0x%u",
                         dev_info.pci_dev->id.vendor_id);