muvarov replied on github web page:
platform/linux-generic/pktio/socket.c
line 14
@@ -643,6 +643,10 @@ static int sock_mmsg_recv(pktio_entry_t *pktio_entry, int
index ODP_UNUSED,
uint16_t pkt_len = msgvec[i].msg_len;
int ret;
+ if (odp_unlikely(msgvec[i].msg_hdr.msg_flags & MSG_TRUNC)) {
+ odp_packet_free(pkt);
Comment:
inconsistent with counters. You do not increase drop counter for pktio and
silently drop packet. I would add at least ODP_DBG() so people on debug can see
what is happening.
> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
> OK. From the original code's use of `%` I assumed that was the case. Perhaps
> that's why this is a fix. :)
>> Matias Elo(matiaselo) wrote:
>> Will fix.
>>> Matias Elo(matiaselo) wrote:
>>> 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_r162343000
updated_at 2018-01-18 13:37:16
