Matias Elo(matiaselo) 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:
Problem is that you cannot increase drop counters manually with socket pktio.
The counters are read directly from ethtool or sysfs. ODP_DBG() may be the best
option here.
> muvarov wrote
> 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_r162377995
updated_at 2018-01-18 15:38:07
