[dpdk-dev] rte_table: fails on 32 bit platforms

[dpdk-dev] rte_table: fails on 32 bit platforms

[daniel chapiesky]

Hello,

I found a problem with the Packet Framework's rte_table library.

The rte_table library, when compiled against a 32-bit target, becomes
non-functional.

This is because certain code in the various table implementations assumes
64-bit (or 8 byte) pointers.

For example, the following code:

    if ((check_params_create_lru(p) != 0)  ||
        ((sizeof(struct rte_table_hash) % CACHE_LINE_SIZE) != 0) ||
        ((sizeof(struct rte_bucket_4_8) % CACHE_LINE_SIZE) != 0) ) {
        return NULL;
    }

when combined with the structure:

    struct rte_bucket_4_8 {
        /* Cache line 0 */
        uint64_t signature;
        uint64_t lru_list;
        struct rte_bucket_4_8 *next;

        uint64_t next_valid;

        uint64_t key[4];

        /* Cache line 1 */
        uint8_t data[0];
    };

will compile just fine, but the compiler optimizer will happily do constant
propagation,
note the math does not line up, and performs dead code removal, thus
removing everything
below this point without a word.


I was able to do a quick fix by adding a uint32_t pad variable, but that is
a hack.

In rte_table_acl.c we find the following assertion:

    RTE_BUILD_BUG_ON(((sizeof(struct rte_table_acl) % CACHE_LINE_SIZE)
        != 0));


This appears to be a better way to perform this kind of check, rather than
embedding it in an "if" statement.


While I have a git patch which fixes this problem, I would prefer that the
experts take a look at this problem and figure out if it can be more
elegantly fixed.

Sincerely,

Daniel Chapiesky
AllSource


Below is listed some of the files and offending bits of code:


--------------------------------
In rte_table_hash_key8.c we find:

    struct rte_bucket_4_8 {
        /* Cache line 0 */
        uint64_t signature;
        uint64_t lru_list;
        struct rte_bucket_4_8 *next;

        uint64_t next_valid;

        uint64_t key[4];

        /* Cache line 1 */
        uint8_t data[0];
    };

and the questionable checks:

    /* Check input parameters */
    if ((check_params_create_lru(p) != 0)  ||
        ((sizeof(struct rte_table_hash) % CACHE_LINE_SIZE) != 0) ||
        ((sizeof(struct rte_bucket_4_8) % CACHE_LINE_SIZE) != 0) ) {
        return NULL;
    }


This was fixed by me using 4 bytes for padding:

    struct rte_bucket_4_8 {
        /* Cache line 0 */
        uint64_t signature;
        uint64_t lru_list;
        struct rte_bucket_4_8 *next;

        uint32_t pad0; // <<< TOTAL HACK

        uint64_t next_valid;

        uint64_t key[4];

        /* Cache line 1 */
        uint8_t data[0];
    };

-------------------------------

The same code structure was found in these files as well:

rte_table_hash_key16.c

rte_table_hash_key32.c

rte_table_hash_lru.c


-------------------------------
Interestingly in rte_table_hash_ext.c we find:


    struct bucket {
        union {
            uintptr_t next;
            uint64_t lru_list;
        };
        uint16_t sig[KEYS_PER_BUCKET];
        uint32_t key_pos[KEYS_PER_BUCKET];
    };

and the questionable checks:

    /* Check input parameters */
    if ((check_params_create(p) != 0) ||
        (!rte_is_power_of_2(entry_size)) ||
        ((sizeof(struct rte_table_hash) % CACHE_LINE_SIZE) != 0) ||
        (sizeof(struct bucket) != (CACHE_LINE_SIZE / 2)))
        return NULL;


The union in struct bucket saves us from 32 bit pointers mucking up the
math.