[ovs-dev] [PATCHv5] netdev-linux: Detect numa node id.

Ilya Maximets i.maximets at ovn.org
Tue Oct 29 16:15:26 UTC 2019


On 23.10.2019 23:08, William Tu wrote:
> The patch detects the numa node id from the name of the netdev,
> by reading the '/sys/class/net/<devname>/device/numa_node'.
> If not available, ex: virtual device, or any error happens,
> return numa id 0.  Currently only the afxdp netdev type uses it,
> other linux netdev types are disabled due to no use case.
> 
> Signed-off-by: William Tu <u9012063 at gmail.com>
> ---
> v5:
>    Feedbacks from Ilya
>    - reafactor the error handling
>    - add mutex lock
>    - Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/601947245
> 
> v4:
>    Feedbacks from Eelco
>    - Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/599308893
> 
> v3:
>    Feedbacks from Ilya and Eelco
>    - update doc, afxdp.rst
>    - fix coding style
>    - fix limit of numa node max, by using ovs_numa_numa_id_is_valid
>    - move the function to netdev-linux
>    - cache the result of numa_id
>    - add security check for netdev name
>    - use fscanf instead of read and convert to int
>    - revise some error message content
> 
> v2:
>    address feedback from Eelco
> 	fix memory leak of xaspintf
> 	log using INFO instead of WARN
> ---
>   Documentation/intro/install/afxdp.rst |  1 -
>   lib/netdev-afxdp.c                    | 11 ------
>   lib/netdev-linux-private.h            |  2 ++
>   lib/netdev-linux.c                    | 63 ++++++++++++++++++++++++++++++++++-
>   4 files changed, 64 insertions(+), 13 deletions(-)
> 
> diff --git a/Documentation/intro/install/afxdp.rst b/Documentation/intro/install/afxdp.rst
> index 820e9d993d8f..6c00c4ad1356 100644
> --- a/Documentation/intro/install/afxdp.rst
> +++ b/Documentation/intro/install/afxdp.rst
> @@ -309,7 +309,6 @@ Below is a script using namespaces and veth peer::
>   
>   Limitations/Known Issues
>   ------------------------
> -#. Device's numa ID is always 0, need a way to find numa id from a netdev.
>   #. No QoS support because AF_XDP netdev by-pass the Linux TC layer. A possible
>      work-around is to use OpenFlow meter action.
>   #. Most of the tests are done using i40e single port. Multiple ports and
> diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c
> index 8eb270c150e8..cfd93fab9f45 100644
> --- a/lib/netdev-afxdp.c
> +++ b/lib/netdev-afxdp.c
> @@ -543,17 +543,6 @@ out:
>       return err;
>   }
>   
> -int
> -netdev_afxdp_get_numa_id(const struct netdev *netdev)
> -{
> -    /* FIXME: Get netdev's PCIe device ID, then find
> -     * its NUMA node id.
> -     */
> -    VLOG_INFO("FIXME: Device %s always use numa id 0.",
> -              netdev_get_name(netdev));
> -    return 0;
> -}
> -
>   static void
>   xsk_remove_xdp_program(uint32_t ifindex, int xdpmode)
>   {
> diff --git a/lib/netdev-linux-private.h b/lib/netdev-linux-private.h
> index a350be151147..c8f2be47b10b 100644
> --- a/lib/netdev-linux-private.h
> +++ b/lib/netdev-linux-private.h
> @@ -96,6 +96,8 @@ struct netdev_linux {
>       /* LAG information. */
>       bool is_lag_master;         /* True if the netdev is a LAG master. */
>   
> +    int numa_id;                /* NUMA node id. */
> +
>   #ifdef HAVE_AF_XDP
>       /* AF_XDP information. */
>       struct xsk_socket_info **xsks;
> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> index f4819237370a..4dd05493b238 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -236,6 +236,7 @@ enum {
>       VALID_VPORT_STAT_ERROR  = 1 << 5,
>       VALID_DRVINFO           = 1 << 6,
>       VALID_FEATURES          = 1 << 7,
> +    VALID_NUMA_ID           = 1 << 8,
>   };
>   
>   struct linux_lag_slave {
> @@ -1391,6 +1392,66 @@ netdev_linux_tap_batch_send(struct netdev *netdev_,
>       return 0;
>   }
>   
> +static int
> +netdev_linux_get_numa_id__(const struct netdev *netdev_)

Since you've created a separate function, I think we need to add
a thread safety annotation here: OVS_REQUIRES(netdev->mutex)

For this purpose, you'll better pass the instance of 'netdev_linux'
in this function.  Original netdev only used to get the netdev name,
so it'll be not hard to use &netdev->up instead for that purpose.

Second thing is that we should preserve numa cache on netlink updates.
Something like this:

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 4dd05493b..0495a035f 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -821,9 +821,9 @@ netdev_linux_update__(struct netdev_linux *dev,
  {
      if (rtnetlink_type_is_rtnlgrp_link(change->nlmsg_type)) {
          if (change->nlmsg_type == RTM_NEWLINK) {
-            /* Keep drv-info, and ip addresses. */
+            /* Keep drv-info, ip addresses and NUMA id. */
              netdev_linux_changed(dev, change->ifi_flags,
-                                 VALID_DRVINFO | VALID_IN);
+                                 VALID_DRVINFO | VALID_IN | VALID_NUMA_ID);
  
              /* Update netdev from rtnl-change msg. */
              if (change->mtu) {
---

Without this change we're re-checking the numa id after each time device
flags/mtu/whatever updated.

Best regards, Ilya Maximets.


More information about the dev mailing list