Matias Elo(matiaselo) replied on github web page:
platform/linux-generic/pktio/socket_mmap.c
line 98
@@ -251,6 +263,50 @@ static inline unsigned pkt_mmap_v2_rx(pktio_entry_t
*pktio_entry,
return nb_rx;
}
+static unsigned handle_pending_frames(int sock, struct ring *ring, int frames)
+{
+ int i;
+ int retry = 0;
+ unsigned nb_tx = 0;
+ unsigned frame_num;
+ unsigned frame_count = ring->rd_num;
+ unsigned first_frame_num = ring->frame_num;
+
+ for (frame_num = first_frame_num, i = 0; i < frames; i++) {
+ struct tpacket2_hdr *hdr = ring->rd[frame_num].iov_base;
+
+ if (odp_likely(hdr->tp_status == TP_STATUS_AVAILABLE ||
+ hdr->tp_status == TP_STATUS_SENDING)) {
+ nb_tx++;
+ } else if (hdr->tp_status == TP_STATUS_SEND_REQUEST) {
+ if (retry++ < TX_RETRIES) {
+ struct timespec ts = { .tv_nsec = TX_RETRY_NSEC,
+ .tv_sec = 0 };
+
+ sendto(sock, NULL, 0, MSG_DONTWAIT, NULL, 0);
+ nanosleep(&ts, NULL);
+ i--;
+ continue;
+ } else {
+ hdr->tp_status = TP_STATUS_AVAILABLE;
+ }
+ } else { /* TP_STATUS_WRONG_FORMAT */
+ /* Don't try re-sending frames after failure */
+ for (; i < frames; i++) {
+ hdr = ring->rd[frame_num].iov_base;
+ hdr->tp_status = TP_STATUS_AVAILABLE;
+ frame_num = next_frame(frame_num, frame_count);
+ }
+ break;
+ }
+ frame_num = next_frame(frame_num, frame_count);
+ }
+
+ ring->frame_num = (first_frame_num + nb_tx) % frame_count;
Comment:
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_r162331990
updated_at 2018-01-18 12:49:24
