[ovs-dev] [PATCH v9 2/2] netdev-afxdp: NUMA-aware memory allocation for XSK related memory

[Ilya Maximets]

On 18.01.2020 00:19, William Tu wrote:
> On Fri, Jan 17, 2020 at 2:58 PM Ilya Maximets <i.maximets at ovn.org> wrote:
>>
>> On 04.01.2020 02:13, Yi-Hung Wei wrote:
>>> Currently, the AF_XDP socket (XSK) related memory are allocated by main
>>> thread in the main thread's NUMA domain.  With the patch that detects
>>> netdev-linux's NUMA node id, the PMD thread of AF_XDP port will be run on
>>> the AF_XDP netdev's NUMA domain.  If the net device's NUMA domain
>>> is different from the main thread's NUMA domain, we will have two
>>> cross-NUMA memory accesses (netdev <-> memory, memory <-> CPU).
>>>
>>> This patch addresses the aforementioned issue by allocating
>>> the memory in the net device's NUMA domain.
>>>
>>> Signed-off-by: Yi-Hung Wei <yihung.wei at gmail.com>
>>
>>
>> Suggesting following incremental for both patches:
>>
>> ---
>> diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c
>> index 0e43c09ee..ae55944d4 100644
>> --- a/lib/netdev-afxdp.c
>> +++ b/lib/netdev-afxdp.c
>> @@ -674,7 +674,7 @@ netdev_afxdp_reconfigure(struct netdev *netdev)
>>      /* Allocate all the xsk related memory in the netdev's NUMA domain. */
>>      struct bitmask *old_bm = NULL;
>>      int old_policy, numa_id;
>> -    if (numa_available() != -1) {
>> +    if (numa_available() != -1 && ovs_numa_get_n_numas() > 1) {
>>          numa_id = netdev_get_numa_id(netdev);
>>          if (numa_id != NETDEV_NUMA_UNSPEC) {
>>              old_bm = numa_allocate_nodemask();
>> @@ -723,6 +723,9 @@ out:
>>      if (old_bm) {
>>          if (set_mempolicy(old_policy, old_bm->maskp, old_bm->size + 1)) {
>>              VLOG_WARN("Failed to restore NUMA memory policy.");
>> +            /* Can't restore correctly.  Try to use localalloc as the most
>> +             * likely default memory policy. */
>> +            numa_set_localalloc();
>>          }
>>          numa_bitmask_free(old_bm);
>>      }
>> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
>> index af2a34aa9..e1ef58bef 100644
>> --- a/lib/netdev-linux.c
>> +++ b/lib/netdev-linux.c
>> @@ -1459,6 +1459,11 @@ netdev_linux_get_numa_id__(struct netdev_linux *netdev)
>>      netdev->numa_id = 0;
>>      netdev->cache_valid |= VALID_NUMA_ID;
>>
>> +    if (ovs_numa_get_n_numas() < 2) {
>> +        /* No need to check on system with a single NUMA node. */
>> +        return 0;
>> +    }
>> +
>>      name = netdev_get_name(&netdev->up);
>>      if (strpbrk(name, "/\\")) {
>>          VLOG_ERR_RL(&rl, "\"%s\" is not a valid name for a port. "
>>
>> ---
>>
>>
>> It solves 3 issues:
>> 1. Avoiding warning while using physical device on system without NUMA topology.
>> 2. Attempt to restore memory policy to default and less likely to fail in case
>>    real restoring failed.
>> 3. Saving some time by avoiding checking NUMA node on system without NUMA.
>>
>>
>> If you're OK with that, I could squash this in while applying the patch.
>>
> 
> Hi Ilya,
> 
> Thanks for the diff!
> Yi-Hung is on vacation, and yes, the change looks good to me.

Thanks!  I folded this in and applied both patches to master.

Best regards, Ilya Maximets.