From: Wiles, Keith, Sunday, February 18, 2018 5:39 PM
quoted
On Feb 18, 2018, at 12:11 AM, Matan Azrad [off-list ref]
wrote:
quoted
Hi Pavan
Please see some comments below.
From: Pavan Nikhilesh, Saturday, February 17, 2018 12:50 PM
quoted
Add 32b and 64b API's to align the given integer to the previous power of
2.
quoted
quoted
Signed-off-by: Pavan Nikhilesh <redacted>
---
lib/librte_eal/common/include/rte_common.h | 36
++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/lib/librte_eal/common/include/rte_common.h
b/lib/librte_eal/common/include/rte_common.h
index c7803e41c..126914f07 100644
--- a/lib/librte_eal/common/include/rte_common.h
+++ b/lib/librte_eal/common/include/rte_common.h
@@ -259,6 +259,24 @@ rte_align32pow2(uint32_t x)
return x + 1;
}
+/**
+ * Aligns input parameter to the previous power of 2
+ *
+ * @param x
+ * The integer value to algin
+ *
+ * @return
+ * Input parameter aligned to the previous power of 2
I think the zero case(x=0) result should be documented.
quoted
+ */
+static inline uint32_t
+rte_align32lowpow2(uint32_t x)
What do you think about " rte_align32prevpow2"?
quoted
+{
+ x = rte_align32pow2(x);
In case of x is power of 2 number(already aligned), looks like the
result here is x and the final result is (x >> 1)?
quoted
Is it as you expect?
quoted
+ x--;
+
+ return x - (x >> 1);
Why can't the implementation just be:
return rte_align32pow2(x) >> 1;
If the above is correct, Are you sure we need this API?
quoted
+}
+
/**
* Aligns 64b input parameter to the next power of 2
*
@@ -282,6 +300,24 @@ rte_align64pow2(uint64_t v)
return v + 1;
}
+/**
+ * Aligns 64b input parameter to the previous power of 2
+ *
+ * @param v
+ * The 64b value to align
+ *
+ * @return
+ * Input parameter aligned to the previous power of 2
+ */
+static inline uint64_t
+rte_align64lowpow2(uint64_t v)
+{
+ v = rte_align64pow2(v);
+ v--;
+
+ return v - (v >> 1);
+}
+
Same comments for 64b API.
quoted
/*********** Macros for calculating min and max **********/
/**
--
2.16.1
If it is a new API, I think it should be added to the map file and to be tagged
as experimental. No?
quoted
Is this the type of API that needs to be marked experimental,
I think it is relevant to any exposed API(not only for internal libraries).
we should be able to prove these functions, correct?
Don't we need to prove any function in DPDK?
What is your point?
quoted
Matan
Regards,
Keith
On Feb 19, 2018, at 12:03 AM, Matan Azrad [off-list ref] wrote:
quoted
Is this the type of API that needs to be marked experimental,
I think it is relevant to any exposed API(not only for internal libraries).
quoted
we should be able to prove these functions, correct?
Don't we need to prove any function in DPDK?
What is your point?
My point is this is a inline function and can not be placed in the .map file as a external API. These simple type of APIs are easy to prove and making them experimental seems to just cause an extra step. If the functions are not required that is a different problem or if the API is really only ever used by a single function or module of files then it should be moved to the module/file and made locate to the module/file.
quoted
quoted
Matan
Regards,
Keith
Regards,
Keith
Hi Wiles
From: Wiles, Keith, Monday, February 19, 2018 3:56 PM
quoted
On Feb 19, 2018, at 12:03 AM, Matan Azrad [off-list ref] wrote:
quoted
Is this the type of API that needs to be marked experimental,
I think it is relevant to any exposed API(not only for internal libraries).
quoted
we should be able to prove these functions, correct?
Don't we need to prove any function in DPDK?
What is your point?
My point is this is a inline function and can not be placed in the .map file as a
external API.
Doesn't each API in .h file external? Why not?
If it shouldn't be external and should be in .h file, I think it should be marked as internal, no?
These simple type of APIs are easy to prove and making them
experimental seems to just cause an extra step.
As Thomas mentioned here:
https://dpdk.org/ml/archives/dev/2018-January/087719.html
Any new API should be experimental.
Thomas, Is it different for .h file inline APIs?
If the functions are not required that is a different problem or if the API is really only ever used by a
single function or module of files then it should be moved to the module/file
and made locate to the module/file.
Agree.
Looks like this function makes sense and may be used by other modules later.
On Feb 19, 2018, at 8:13 AM, Matan Azrad [off-list ref] wrote:
Hi Wiles
From: Wiles, Keith, Monday, February 19, 2018 3:56 PM
quoted
quoted
On Feb 19, 2018, at 12:03 AM, Matan Azrad [off-list ref] wrote:
quoted
Is this the type of API that needs to be marked experimental,
I think it is relevant to any exposed API(not only for internal libraries).
quoted
we should be able to prove these functions, correct?
Don't we need to prove any function in DPDK?
What is your point?
My point is this is a inline function and can not be placed in the .map file as a
external API.
Doesn't each API in .h file external? Why not?
If it shouldn't be external and should be in .h file, I think it should be marked as internal, no?
We do not do a great job of using private headers as most of our headers are public ones and in the case or private they would not be external.
quoted
These simple type of APIs are easy to prove and making them
experimental seems to just cause an extra step.
As Thomas mentioned here:
https://dpdk.org/ml/archives/dev/2018-January/087719.html
Any new API should be experimental.
Thomas, Is it different for .h file inline APIs?
quoted
If the functions are not required that is a different problem or if the API is really only ever used by a
single function or module of files then it should be moved to the module/file
and made locate to the module/file.
Agree.
Looks like this function makes sense and may be used by other modules later.
Regards,
Keith
19/02/2018 15:13, Matan Azrad:
Hi Wiles
From: Wiles, Keith, Monday, February 19, 2018 3:56 PM
quoted
quoted
On Feb 19, 2018, at 12:03 AM, Matan Azrad [off-list ref] wrote:
quoted
Is this the type of API that needs to be marked experimental,
I think it is relevant to any exposed API(not only for internal libraries).
quoted
we should be able to prove these functions, correct?
Don't we need to prove any function in DPDK?
What is your point?
My point is this is a inline function and can not be placed in the .map file as a
external API.
Doesn't each API in .h file external? Why not?
If it shouldn't be external and should be in .h file, I think it should be marked as internal, no?
quoted
These simple type of APIs are easy to prove and making them
experimental seems to just cause an extra step.
As Thomas mentioned here:
https://dpdk.org/ml/archives/dev/2018-January/087719.html
Any new API should be experimental.
Thomas, Is it different for .h file inline APIs?
No, it is not different for inline functions.
If we are discussing the policy for every function, it is not
a policy anymore :)
It was agreed to notify the users of the new functions,
so it gives time to confirm the function before making it "stable".
Even if the function looks obvious, I think we should follow the policy.
So, yes, please add the experimental tag.