Re: [PATCH bpf-next] samples/bpf: xdp_redirect_cpu: set MAX_CPUS according to NR_CPUS

[Yonghong Song]

On 5/12/20 3:51 AM, Lorenzo Bianconi wrote:

On 5/11/20 1:24 PM, Lorenzo Bianconi wrote:

xdp_redirect_cpu is currently failing in bpf_prog_load_xattr()
allocating cpu_map map if CONFIG_NR_CPUS is less than 64 since
cpu_map_alloc() requires max_entries to be less than NR_CPUS.
Set cpu_map max_entries according to NR_CPUS in xdp_redirect_cpu_kern.c
and get currently running cpus in xdp_redirect_cpu_user.c

Signed-off-by: Lorenzo Bianconi <lore...@kernel.org>
---
   samples/bpf/xdp_redirect_cpu_kern.c |  2 +-
   samples/bpf/xdp_redirect_cpu_user.c | 29 ++++++++++++++++-------------
   2 files changed, 17 insertions(+), 14 deletions(-)

[...]

   static void mark_cpus_unavailable(void)
   {
-       __u32 invalid_cpu = MAX_CPUS;
+       __u32 invalid_cpu = n_cpus;
        int ret, i;
-       for (i = 0; i < MAX_CPUS; i++) {
+       for (i = 0; i < n_cpus; i++) {
                ret = bpf_map_update_elem(cpus_available_map_fd, &i,
                                          &invalid_cpu, 0);
                if (ret) {
@@ -688,6 +689,8 @@ int main(int argc, char **argv)
        int prog_fd;
        __u32 qsize;
+       n_cpus = get_nprocs();

get_nprocs() gets the number of available cpus, not including offline cpus.
But gaps may exist in cpus, e.g., get_nprocs() returns 4, and cpus are
0-1,4-5. map updates will miss cpus 4-5. And in this situation,
map_update will fail on offline cpus.

This sample test does not need to deal with complication of
cpu offlining, I think. Maybe we can get
        n_cpus = get_nprocs();
        n_cpus_conf = get_nprocs_conf();
        if (n_cpus != n_cpus_conf) {
                /* message that some cpus are offline and not supported. */
                return error
        }

Hi Yonghong,

thanks for pointing this out. Why not just use:

n_cpus = get_nprocs_conf()

and let the user pick the right cpu id with 'c' option (since it is mandatory)?

Not aware of 'c' option. Yes, get_nprocs_conf() directly works too.

Regards,
Lorenzo

+
        /* Notice: choosing he queue size is very important with the
         * ixgbe driver, because it's driver page recycling trick is
         * dependend on pages being returned quickly.  The number of
@@ -757,7 +760,7 @@ int main(int argc, char **argv)
                case 'c':
                        /* Add multiple CPUs */
                        add_cpu = strtoul(optarg, NULL, 0);
-                       if (add_cpu >= MAX_CPUS) {
+                       if (add_cpu >= n_cpus) {
                                fprintf(stderr,
                                "--cpu nr too large for cpumap err(%d):%s\n",
                                        errno, strerror(errno));