Matias Elo(matiaselo) replied on github web page:
platform/linux-generic/pktio/socket_mmap.c
line 30
@@ -148,6 +155,11 @@ static uint8_t *pkt_mmap_vlan_insert(uint8_t *l2_hdr_ptr,
return l2_hdr_ptr;
}
+static inline unsigned next_frame(unsigned cur_frame, unsigned frame_count)
+{
+ return odp_unlikely(cur_frame + 1 >= frame_count) ? 0 : cur_frame + 1;
Comment:
ring->rd_num is not guaranteed to be a power of two. At least on my test system
(28 lcores) it is not.
> Matias Elo(matiaselo) wrote:
> Will fix.
>> Matias Elo(matiaselo) wrote:
>> This array is used to update `stats.out_octets` correctly if `enq_multi()`
>> fails to enqueue all packets. Without it a second for loop would be required
>> inside `if (ret > 0)` clause below.
>>> Matias Elo(matiaselo) wrote:
>>> The most specific error type an application can see is
>>> odp_packet_has_l2_error(), so it cannot trust packet contents anyway.
>>>> Matias Elo(matiaselo) wrote:
>>>> Will fix.
>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>> The error bit simply says the packet was truncated to max length. Perhaps
>>>>> the application is only interested in the first few bytes of a packet?
>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>> `ring->frame_num = next_frame(first_frame_num + nb_tx - 1,
>>>>>> frame_count);` is an alternative here.
>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>> Since `ring->rd_num` is a power of two, is the concern here that the
>>>>>>> `%` operator might result in a division if the compiler doesn't know
>>>>>>> that it's a power of two? In that case:
>>>>>>> ```
>>>>>>> return ++cur_frame & (frame_count - 1);
>>>>>>> ```
>>>>>>> solves that problem and avoids conditional branching.
>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>> If we're cleaning up, might as well delete that extraneous `+` in the
>>>>>>>> above line: `TPACKET_ALIGNMENT + + (pz - 1)) & (-pz);` I'm surprised
>>>>>>>> the compiler doesn't flag that since it's normally so picky about
>>>>>>>> issuing warnings.
>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>> Not sure I understand what `out_octets_tbl` is accumulating here. If
>>>>>>>>> I'm sending three 100 byte packets this will set `out_octets_tbl[0]`
>>>>>>>>> to 100, `out_octets_tbl[1]` to 200, and `out_octets_tbl[2]` to 300.
>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>> Since you're doing miscellaneous cleanups anyway, perhaps changing
>>>>>>>>>> `len` to `num` here should be considered? `len` is confusing since
>>>>>>>>>> it doesn't represent packet length.
>>>>>>>>>>> Matias Elo(matiaselo) wrote:
>>>>>>>>>>> The odp_pktin_maxlen() API states:
>>>>>>>>>>> > Maximum frame length in bytes that the packet IO interface can
>>>>>>>>>>> > receive.
>>>>>>>>>>>
>>>>>>>>>>> I interpret this so that the application shouln't see packets which
>>>>>>>>>>> exceed pktin max length.
>>>>>>>>>>>
>>>>>>>>>>>> muvarov wrote
>>>>>>>>>>>> same here.
>>>>>>>>>>>>> muvarov wrote
>>>>>>>>>>>>> maybe to just set error bit and deliver this packet to
>>>>>>>>>>>>> application?
https://github.com/Linaro/odp/pull/397#discussion_r162331896
updated_at 2018-01-18 12:48:57
