service: fix shifts to operate on 64-bit integers
authorHarry van Haaren <harry.van.haaren@intel.com>
Mon, 31 Jul 2017 16:38:55 +0000 (17:38 +0100)
committerThomas Monjalon <thomas@monjalon.net>
Mon, 31 Jul 2017 20:36:09 +0000 (22:36 +0200)
This commit fixes shifts to an integer (1 << shift) which
is assumed to be a 32-bit integer. In this case, the shift is
variable and expected to be valid for 64-bit integers. Given that
the expectation to work with 64 bits exists, we must ensure that
the (1 << shift) one in that formula is actually a uin64_t.

The UINT64_C() macro portably adds the correct suffix to a constant,
informing the compiler that the value is to be assigned 64 bits.

The issue would only manifests when there were greater than 31
services registered.

Fixes: 21698354c832 ("service: introduce service cores concept")

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
lib/librte_eal/common/rte_service.c

index e82b9ad..7efb76d 100644 (file)
@@ -286,7 +286,7 @@ rte_service_unregister(struct rte_service_spec *spec)
        s->internal_flags &= ~(SERVICE_F_REGISTERED);
 
        for (i = 0; i < RTE_MAX_LCORE; i++)
-               lcore_states[i].service_mask &= ~(1 << service_id);
+               lcore_states[i].service_mask &= ~(UINT64_C(1) << service_id);
 
        memset(&rte_services[service_id], 0,
                        sizeof(struct rte_service_spec_impl));
@@ -327,7 +327,7 @@ rte_service_runner_func(void *arg)
                for (i = 0; i < rte_service_count; i++) {
                        struct rte_service_spec_impl *s = &rte_services[i];
                        if (s->runstate != RUNSTATE_RUNNING ||
-                                       !(service_mask & (1 << i)))
+                                       !(service_mask & (UINT64_C(1) << i)))
                                continue;
 
                        /* check do we need cmpset, if MT safe or <= 1 core
@@ -463,18 +463,19 @@ service_update(struct rte_service_spec *service, uint32_t lcore,
        if (!lcore_states[lcore].is_service_core)
                return -EINVAL;
 
+       uint64_t sid_mask = UINT64_C(1) << sid;
        if (set) {
                if (*set) {
-                       lcore_states[lcore].service_mask |=  (1 << sid);
+                       lcore_states[lcore].service_mask |= sid_mask;
                        rte_services[sid].num_mapped_cores++;
                } else {
-                       lcore_states[lcore].service_mask &= ~(1 << sid);
+                       lcore_states[lcore].service_mask &= ~(sid_mask);
                        rte_services[sid].num_mapped_cores--;
                }
        }
 
        if (enabled)
-               *enabled = (lcore_states[lcore].service_mask & (1 << sid));
+               *enabled = (lcore_states[lcore].service_mask & (sid_mask));
 
        rte_smp_wmb();
 
@@ -607,7 +608,8 @@ rte_service_lcore_stop(uint32_t lcore)
 
        uint32_t i;
        for (i = 0; i < RTE_SERVICE_NUM_MAX; i++) {
-               int32_t enabled = lcore_states[i].service_mask & (1 << i);
+               int32_t enabled =
+                       lcore_states[i].service_mask & (UINT64_C(1) << i);
                int32_t service_running = rte_services[i].runstate !=
                                                RUNSTATE_STOPPED;
                int32_t only_core = rte_services[i].num_mapped_cores == 1;