[#162] Fix 64bit atomic api issues for 32bit system-part2

[dganeshk]

*atomic apis are using only 64 bit assembly instructions([add,sub,..]q which are not supported in 32bit systems and compilation errors observed due to that. Fixing the assembly instruction for the api's add and subtract operator.

[tbarbette]

Is the technique used on atomics really equivalent? Can't it be half updated? Do you have a source for this technique? Maybe elements that use 64bit atomic should just not be compiled instead?

[dganeshk]

yes, here are some of the references that i checked, not straight forward changes.
https://cgit.freebsd.org/src/tree/sys/i386/include/atomic.h
mips : https://web.mit.edu/freebsd/head/sys/mips/include/atomic.h

Can't it be half updated?

do you mean to use the 32 bit instructions?

Maybe elements that use 64bit atomic should just not be compiled instead?

it is mostly counters which are spread across elements (checkipheader, checkarpheader,etc), we can #def the usage according to the platforms.
#if defined(x86_64)
atomic_uint64_t current;
#else
atomic_uint32_t current;
#endit

I can push the assembly implmentation changes for other functions as well towards the end of the week. Assembly instruction approach looked right so tried this approach.

[tbarbette]

Indeed the patcheset seems fine theoretically. Have you tested them? The CI does not have a 32bit build, so I'm a bit worried about that. I could add a 32bit build and also a series of tests for atomics, but I'm not sure when...

[dganeshk]

Indeed the patcheset seems fine theoretically. Have you tested them? The CI does not have a 32bit build, so I'm a bit worried about that. I could add a 32bit build and also a series of tests for atomics, but I'm not sure when...

yes i have tested it. I am running ubuntu18.04 32 bit image pulled into a virutal machine. have to a modify a little bit on the atomic test cases which is there already(elements/test/atomic.cc) but it is not really a 64 bit testing. modified a ittle bit to validate these changes. I will push the test cases improvement as well. changes works in the current test cases.

[dganeshk]

have to modify the formatting in the click chatter to match the uint64_t format, will modify the test cases to really validate the 64 bit operations. I wonder why this test case is with exit rather then just put out the result and let further elements to continue.

diff --git a/elements/test/atomic.cc b/elements/test/atomic.cc
index ca92181f6..3148ee1d8 100644
--- a/elements/test/atomic.cc
+++ b/elements/test/atomic.cc
@@ -86,13 +86,14 @@ static void test_64(ErrorHandler *)
 
     atomic_uint64_t a;
     a = 0;
-    click_chatter("[01] a=%i -> %s", a, a==0 ? "PASS" : "FAIL");
+    click_chatter("[01] a=%llu -> %s", uint64_t(a), uint64_t(a)==0 ? "PASS" : "FAIL");
     a++;
-    click_chatter("[02] a=%i -> %s", a, a==1 ? "PASS" : "FAIL");
+    click_chatter("[02] a=%llu -> %s", uint64_t(a), uint64_t(a)==1 ? "PASS" : "FAIL");
     a+=1;
-    click_chatter("[03] a=%i -> %s", a, a==2 ? "PASS" : "FAIL");
+    click_chatter("[03] a=%llu -> %s", uint64_t(a), uint64_t(a)==2 ? "PASS" : "FAIL");
     a-=1;
-    click_chatter("[04] a=%i -> %s", a, a==1 ? "PASS" : "FAIL");
+    click_chatter("[04] a=%llu -> %s", uint64_t(a), uint64_t(a)==1 ? "PASS" : "FAIL");
+ #if 0
     uint32_t u = a.fetch_and_add(10);
     click_chatter("[05] a=%i, u=%i -> %s", a, u,  a==11 && u==1 ? "PASS" : "FAIL");
 
@@ -110,6 +111,7 @@ static void test_64(ErrorHandler *)
     u = atomic_uint64_t::compare_swap(b,5,2);
     click_chatter("[09] b=%i, u=%i -> %s", b, u,  b==2 && u==5 ? "PASS" : "FAIL");
 #endif
+#endif
 }

[dganeshk]

@tbarbette have pushed another PR with that all apis are completed, All test cases in atomic.cc are passing with the changes. will push atomic.cc test cases changes in a separate PR which is common for both 64 bit and 32 bit platforms.

[tbarbette]

Could you do a unique PR so I can try all at once?