Petri Savolainen(psavol) replied on github web page:
include/odp/api/spec/threshold.h
@@ -0,0 +1,105 @@
+/* Copyright (c) 2017, Linaro Limited
+ * All rights reserved.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+
+/**
+ * @file
+ *
+ * ODP threshold descriptor
+ */
+
+#ifndef ODP_API_THRESHOLD_H_
+#define ODP_API_THRESHOLD_H_
+#include <odp/visibility_begin.h>
+#include <odp/api/std_types.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * Supported threshold types
+ *
+ * Supported threshold types in a bit field structure.
+ */
+typedef union odp_threshold_types_t {
+ /** bitfields for different threshold types */
+ struct {
+ /** Percentage of the total size of pool or queue */
+ uint8_t percent:1;
+
+ /** Total number of all transient packets */
+ uint8_t packet:1;
+
+ /** Total size of all transient packets in bytes */
+ uint8_t bytes:1;
+ };
+
+ /** All bits of the bit field structure */
+ uint8_t all_bits;
+} odp_threshold_types_t;
+
+/**
+ * ODP Threshold types
+ *
+ * Different types of threshold measurements
+ */
+typedef enum {
+ /** Percentage of the total size of pool or queue */
Comment:
Change tab to space between typedef and enum. Also typically the typedef'ed
enum/struct/union has also name, so that a debugger does not refer to it as
"enum <no name> does not ...".
typedef enum odp_threshold_type_t {
...
} odp_threshold_type_t;
> Petri Savolainen(psavol) wrote:
> Replace "@typedef odp_percent_t" doxygen tag with the short comment line
> ("Percent type"). The tag is not needed anymore as the typedef follows the
> comment.
>
> Also, remove the extra line feed.
>> Petri Savolainen(psavol) wrote:
>> bp_ prefixes can be removed from "bp_enable" and "bp_threshold".
>>
>> "BP is enabled when queue or pool value is equal to or greater than the max
>> backpressure threshold."
>> This should refer to "resource usage" (which has been defined on RED
>> documentation). "queue and pool value" are undefined terms.
>>> Petri Savolainen(psavol) wrote:
>>> Remove this extra line feed.
>>>> Petri Savolainen(psavol) wrote:
>>>> "... RED is enabled when the resource limit is equal to ..."
>>>>
>>>> This refers still to "resource limit", it should be "resource usage" which
>>>> is a term that has been define above.
>>>>> Petri Savolainen(psavol) wrote:
>>>>> red_ prefix can be dropped from the struct field names. When red param is
>>>>> used, the name of the variable is likely red or red_param, etc. So, after
>>>>> removing the prefix it looks like this
>>>>>
>>>>> red.enable = 1;
>>>>> red.threshold.packets.min = 1000;
>>>>>
>>>>> vs
>>>>>
>>>>> red.red_enable = 1;
>>>>> red.red_threshold....
>>>>> ...
>>>>>> Petri Savolainen(psavol) wrote:
>>>>>> "Reaches between" is not valid since drop probability is defined also
>>>>>> when <min and >max. "... when the packets in the queue/pool reaches
>>>>>> between the specified threshold." -> "... when the packets in the
>>>>>> queue/pool cross specified threshold values."
>>>>>>
>>>>>> The "resource limit" needs to be specified: is it free or used space? Or
>>>>>> is it different for pools and queues: free space in a pool, used space
>>>>>> in a queue? May be the text is easier to understand with a bullet list:
>>>>>> * drop probability is 100%, when resource usage > threshold.max
>>>>>> * drop probability is 0%, when resource usage < threshold.min
>>>>>> * drop probability is between 0 ... 100%, when resource usage is between
>>>>>> threshold.min and threshold.max
>>>>>>
>>>>>> ... and then define what "resource usage" means. Pools: space used,
>>>>>> queues packet/bytes in queue
>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>> This should match the type enum: byte. Also better description is
>>>>>>> needed: e.g. "Sum of all data bytes of all packets". Packet size does
>>>>>>> not tell if it's the size of one or many packets.
>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>> This should match the type enum: packet
>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>> When percent is not abbreviated, then this should not be either. So,
>>>>>>>>> _PERCENT and _PACKET, or PCT and PKT.
>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>> As we discussed during today's ARCH call. `max` and `min` can be
>>>>>>>>>> renamed `start` and `stop` to more accurately reflect the actions.
>>>>>>>>>> RED / backpressure begins when utilization hits the `start`
>>>>>>>>>> threshold and is deasserted when utilization drops back to the
>>>>>>>>>> `stop` threshold. So `start` >= `stop`. If the two are equal it
>>>>>>>>>> means there is no hysteresis.
>>>>>>>>>>
>>>>>>>>>> The question arises whether a third `max` threshold value should
>>>>>>>>>> exist at which drops / backpressure is held at 100%. The idea here
>>>>>>>>>> is to reserve a portion of the pool/queue resource for application
>>>>>>>>>> use independent of use by PktIOs. Since this may not be feasible in
>>>>>>>>>> all implementations this should probably be advertised with
>>>>>>>>>> additional capability info.
>>>>>>>>>>> Balasubramanian Manoharan(bala-manoharan) wrote:
>>>>>>>>>>> I can move the typedef here.
>>>>>>>>>>> Not sure if I understand the need to move to uint32_t, I have only
>>>>>>>>>>> defined 100% as 10,000 and we should be fine with uin16_t. Any
>>>>>>>>>>> other specific reason for using uint32_t?
>>>>>>>>>>>> Balasubramanian Manoharan(bala-manoharan) wrote:
>>>>>>>>>>>> The drop probability is 100% only when the pool is completely full
>>>>>>>>>>>> i.e there is no further buffer to allocate packet. The intention
>>>>>>>>>>>> of this description is that when resource usage is greater than
>>>>>>>>>>>> threshold.max then the drop probability is enabled and packets
>>>>>>>>>>>> will get dropped on an increasing drop probability and when it is
>>>>>>>>>>>> less than min threshold then drop probability is disabled.
>>>>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>>>>> I think it should be:
>>>>>>>>>>>>> * drop probability == 100%, when resource usage >
>>>>>>>>>>>>> threshold.max
>>>>>>>>>>>>> * drop probability == 0%, when resource usage < threshold.min
>>>>>>>>>>>>> * drop probability between 0 ... 100%, when resource usage
>>>>>>>>>>>>> between min and max
>>>>>>>>>>>>>
>>>>>>>>>>>>> Now the text describe min/max as hysteresis: > max enables, < min
>>>>>>>>>>>>> disables. Which is not the intention, I guess.
>>>>>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>>>>>> Another option for enums: ODP_THRESHOLD_PCT, ODP_THRESHOLD_PKT,
>>>>>>>>>>>>>> ODP_THRESHOLD_BYTE
>>>>>>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>>>>>>> the enum: odp_threshold_type_t
>>>>>>>>>>>>>>> the bitfield: odp_threshold_types_t
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> The enum values need to be UPPERCASE and contain a common
>>>>>>>>>>>>>>> prefix: ODP_THLD_PERCENT, ODP_THLD_PACKET, ODP_THLD_BYTE
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> If bitfield is only needed in one place (one capability struct)
>>>>>>>>>>>>>>> it could be defined there only (no typedef needed).
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>>>>>>>> Since API specifies already what odp_percent_t is, it's better
>>>>>>>>>>>>>>>> do also the typedef here. Also the documentation should say
>>>>>>>>>>>>>>>> that it's _unsigned_ integer. May be uint32_t is safer choice
>>>>>>>>>>>>>>>> than uint16_t, so that percent calculations do not easily wrap
>>>>>>>>>>>>>>>> around.
>>>>>>>>>>>>>>>>> Balasubramanian Manoharan(bala-manoharan) wrote:
>>>>>>>>>>>>>>>>> Done.
>>>>>>>>>>>>>>>>>> Balasubramanian Manoharan(bala-manoharan) wrote:
>>>>>>>>>>>>>>>>>> Currently, I have followed the syntax existing in api-next.
>>>>>>>>>>>>>>>>>> If ABI changes are merged first I will change my patch to
>>>>>>>>>>>>>>>>>> match the specifications. If my patch gets merged first,
>>>>>>>>>>>>>>>>>> change this as part of your ABI spec.
>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>>>>>> We shouldn't leave fields undefined in the returned
>>>>>>>>>>>>>>>>>>> `odp_cls_capability_t` struct. Either set `threshold_red`
>>>>>>>>>>>>>>>>>>> and `threshold_bp` explicitly or just clear the struct to
>>>>>>>>>>>>>>>>>>> zeros at the start of this routine.
>>>>>>>>>>>>>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>>>>>>>>>>>>>> No, `odp_bool_t` exact implementation is not part of the
>>>>>>>>>>>>>>>>>>>> spec, as it is part of platform ABI. Percent type is just
>>>>>>>>>>>>>>>>>>>> data, so from my point of view it should not be a part of
>>>>>>>>>>>>>>>>>>>> ABI, but rather be a part of API spec.
>>>>>>>>>>>>>>>>>>>>> Balasubramanian Manoharan(bala-manoharan) wrote:
>>>>>>>>>>>>>>>>>>>>> They typedef for odp_bool_t is still under
>>>>>>>>>>>>>>>>>>>>> platform/linux-generic directory hence I had this here. I
>>>>>>>>>>>>>>>>>>>>> am fine pushing this to spec/std_types.h but is
>>>>>>>>>>>>>>>>>>>>> odp_bool_t going to be moved later?
>>>>>>>>>>>>>>>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>>>>>>>>>>>>>>>> @bala-manoharan @Bill-Fischofer-Linaro In my opinion,
>>>>>>>>>>>>>>>>>>>>>> let's just push it into `odp/api/spec/std_types.h`.
>>>>>>>>>>>>>>>>>>>>>>> Balasubramanian Manoharan(bala-manoharan) wrote:
>>>>>>>>>>>>>>>>>>>>>>> Oops. Will take care of that in next version.
>>>>>>>>>>>>>>>>>>>>>>>> Balasubramanian Manoharan(bala-manoharan) wrote:
>>>>>>>>>>>>>>>>>>>>>>>> I felt using a bitfield is more useful in exposing the
>>>>>>>>>>>>>>>>>>>>>>>> platform capability whereas using an enum makes more
>>>>>>>>>>>>>>>>>>>>>>>> sense during configuration. I am open for suggestions
>>>>>>>>>>>>>>>>>>>>>>>> along these lines.
>>>>>>>>>>>>>>>>>>>>>>>>> Balasubramanian Manoharan(bala-manoharan) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>> Back pressure is to indicate the remote peer that
>>>>>>>>>>>>>>>>>>>>>>>>> there is a network congestion on this particular
>>>>>>>>>>>>>>>>>>>>>>>>> flow. Whether the remote peer takes respective action
>>>>>>>>>>>>>>>>>>>>>>>>> is beyond the scope of this RFC.
>>>>>>>>>>>>>>>>>>>>>>>>>> Balasubramanian Manoharan(bala-manoharan) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>> NXP is implementing RED at queue level. Hence I had
>>>>>>>>>>>>>>>>>>>>>>>>>> added this based on the feedback from Nikhil.
>>>>>>>>>>>>>>>>>>>>>>>>>>> Balasubramanian Manoharan(bala-manoharan) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>> The previous PR was handling this RED and BP at cos
>>>>>>>>>>>>>>>>>>>>>>>>>>> level. There were some concerns from NXP since CoS
>>>>>>>>>>>>>>>>>>>>>>>>>>> is a logical entity and application configuration
>>>>>>>>>>>>>>>>>>>>>>>>>>> should make sure that multiple CoS with different
>>>>>>>>>>>>>>>>>>>>>>>>>>> RED configuration should not direct to the same
>>>>>>>>>>>>>>>>>>>>>>>>>>> pool or queue.
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>> HW implements the RED and BP effectively either at
>>>>>>>>>>>>>>>>>>>>>>>>>>> the pool or the queue.
>>>>>>>>>>>>>>>>>>>>>>>>>>> Since a pool represents the real bottleneck for
>>>>>>>>>>>>>>>>>>>>>>>>>>> resource exhausion and packet drop.
>>>>>>>>>>>>>>>>>>>>>>>>>>> We can discuss further in todays Public call.
>>>>>>>>>>>>>>>>>>>>>>>>>>>> Balasubramanian Manoharan(bala-manoharan) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>> The implementation provides preference whether the
>>>>>>>>>>>>>>>>>>>>>>>>>>>> RED and BP are configured either in pool or queue
>>>>>>>>>>>>>>>>>>>>>>>>>>>> and provide support for which threshold method is
>>>>>>>>>>>>>>>>>>>>>>>>>>>> supported using capability.
>>>>>>>>>>>>>>>>>>>>>>>>>>>> I believe this is sufficient from the
>>>>>>>>>>>>>>>>>>>>>>>>>>>> implementation point of view.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Adding new files requires updates to the various
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Makefile.am and related autotools. That's why
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Travis is reporting failures since these new
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> files aren't able to be found / used.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Need to clarify under what conditions RED (and
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> BP) apply to pools. If an application calls
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> `odp_packet_alloc()` that should either succeed
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> or fail. "Drop probability" doesn't make any
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> obvious sense here. Tying these concepts to
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> `odp_pktio_t` configurations seems more in
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> keeping with what's needed, but I understand the
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> difficulties that that caused in the previous PR.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Perhaps tying this to a `odp_cos_t` might work?
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> An `odp_cos_t` has an associated queue and pool
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> and these can then be viewed as admission
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> controls to packets being added to a CoS. That
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> would mean that these features would only be
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> applicable if the classifier is being used, but
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> is that a significant drawback since we want to
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> encourage its use anyway?
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Back pressure in this context presumably means
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> that `odp_queue_enq()` calls stall until the
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> back pressure is relieved. Not sure if that's
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> what's really desired for queues in general.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> RED for a queue in doesn't seem to make sense
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> for queues in general. RED is used for
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> admission control, which is why it's tied to
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> PktIOs. If an application calls
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> `odp_queue_enq()` that should either succeed
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> or fail. A "drop probability" doesn't make
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> sense here. We need to clarify that somehow
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> and I'm not sure how that can be done without
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> tying the configuration back to an
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> `odp_pktio_t`.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Same comment as for pools. It would seem
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> these should be individual `odp_support_t`
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> indications.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Wouldn't these be better done as individual
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> `odp_support_t` configurations? That way
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> implementations can indicate not just
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> support but preferences.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> We've defined what we mean by RED as
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> assigning a drop probability but what we
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> mean by "back pressure" seems very fuzzy
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> here. This needs to be more specific if an
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> application is to know how to make use of
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> this.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro)
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> We use `_t` suffixes rather than `_e` for
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> `enums` so `odp_threshold_type_t` would be
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> more standard here, but we already have an
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> `odp_threshold_type_t` defined above. The
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> problem seems to be `_t` already implies
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> type, so "type type" is redundant.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Perhaps a better choice is an
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> `odp_threshold_t` has an
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> `odp_threshold_metric_t metric` field that
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> is the enum above. It's not clear how the
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> `odp_threshold_type_t` bits are intended
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> to be used. Capabilities?
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro)
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Need Doxygen for each struct and union
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro)
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Doxygen 1.8.13 requires every element to
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> be documented.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro)
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Again this new type should follow the
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> PR #250 model.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro)
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Since we've given conceptual approval
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> to PR #250, these changes should be
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> based on that and this definition
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> would be part of
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> abi-default/std_types.h.
https://github.com/Linaro/odp/pull/277#discussion_r151952497
updated_at 2017-11-20 13:07:56
