Bill Fischofer(Bill-Fischofer-Linaro) replied on github web page:
platform/linux-generic/odp_packet.c
line 169
@@ -1352,16 +1352,14 @@ void odp_packet_flow_hash_set(odp_packet_t pkt,
uint32_t flow_hash)
{
odp_packet_hdr_t *pkt_hdr = packet_hdr(pkt);
- pkt_hdr->flow_hash = flow_hash;
- pkt_hdr->p.input_flags.flow_hash = 1;
+ packet_set_flow_hash(pkt_hdr, flow_hash);
}
void odp_packet_ts_set(odp_packet_t pkt, odp_time_t timestamp)
{
odp_packet_hdr_t *pkt_hdr = packet_hdr(pkt);
- pkt_hdr->timestamp = timestamp;
- pkt_hdr->p.input_flags.timestamp = 1;
+ packet_set_ts(pkt_hdr, ×tamp);
Comment:
I'm OK with just a modified commit name. I agree we needn't be too granular
about these things.
> Petri Savolainen(psavol) wrote:
> I could rename the patch to:
> "linux-gen: packet: use inlined flow hash and ts set"
>
> ... if necessary. The comment says already that also ts set is modified the
> same way.
>> Petri Savolainen(psavol) wrote:
>> I did try several combinations of type casts of invalid values, also cast to
>> uintptr_t. The problem is that gcc and clang (clang version 3.8.0-2ubuntu4)
>> of my machine accepted all of those casts, but the clang version in Travis
>> (clang version 3.8.0-2ubuntu3~trusty5) does not accept anything.
>>
>> Maybe it's a bug in the particular version of clang. Anyway, addition of
>> this check even only on gcc side is improvement against what we currently
>> have (no check). Packet invalid is currently 0xfffffff in ABI spec, but
>> event/buffer invalid are 0.
>>
>> odp_packet.c:55:19: error: static_assert expression is not an integral
>> constant
>> expression
>> ODP_STATIC_ASSERT(((uintptr_t)ODP_PACKET_INVALID) == 0, "Packet invalid not
>> 0");
>> ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> ../../include/odp/api/abi-default/debug.h:26:53: note: expanded from macro
>> 'ODP_STATIC_ASSERT'
>> #define ODP_STATIC_ASSERT(cond, msg) _Static_assert(cond, msg)
>> ^~~~
>> odp_packet.c:55:20: note: cast that performs the conversions of a
>> reinterpret_cast is not allowed in a constant expression
>> ODP_STATIC_ASSERT(((uintptr_t)ODP_PACKET_INVALID) == 0, "Packet invalid not
>> 0");
>> ^
>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>> Since you want to breech the strong type you need to use the internal
>>> `odp_typeval()` macro here.
>>> ```
>>> ODP_STATIC_ASSERT(_odp_typeval(ODP_EVENT_INVALID) == 0, "Event invalid not
>>> 0");
>>> ```
>>> etc. I've verified that this works fine for clang, so no need for the
>>> conditional compilation.
>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>> Should this be in its own commit? Different function.
https://github.com/Linaro/odp/pull/437#discussion_r165045138
updated_at 2018-01-31 13:54:27
