Ilias Apalodimas(apalos) replied on github web page:
platform/linux-generic/include/odp_pcapng.h
line 45
@@ -0,0 +1,56 @@
+/* Copyright (c) 2018, Linaro Limited
+ * All rights reserved.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+
+#include <protocols/eth.h>
+#include <protocols/ip.h>
+#include <odp/api/plat/packet_inlines.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <sys/fcntl.h>
+#include <unistd.h>
+#include <pthread.h>
+
+#define PCAPNG_BLOCK_TYPE_EPB 0x00000006UL
+#define PCAPNG_BLOCK_TYPE_SHB 0x0A0D0D0AUL
+#define PCAPNG_BLOCK_TYPE_IDB 0x00000001UL
+#define PCAPNG_ENDIAN_MAGIC 0x1A2B3C4DUL
+#define PCAP_DATA_ALIGN (4)
+
+/* inotify */
+#define INOTIFY_EVENT_SIZE (sizeof(struct inotify_event))
+#define INOTIFY_BUF_LEN (1024 * (INOTIFY_EVENT_SIZE + 16))
+#define PCAPNG_WATCH_DIR "/var/run/odp/"
+
+/* pcapng: enhanced packet block file encoding */
+typedef struct ODP_PACKED pcapng_section_hdr_block_s {
+ uint32_t block_type;
+ uint32_t block_total_length;
+ uint32_t magic;
+ uint16_t version_major;
+ uint16_t version_minor;
+ int64_t section_len;
+ uint32_t block_total_length2;
+} pcapng_section_hdr_block_t;
+
+typedef struct pcapng_interface_description_block {
+ uint32_t block_type;
+ uint32_t block_total_length;
+ uint16_t linktype;
+ uint16_t reserved;
+ uint32_t snaplen;
+ uint32_t block_total_length2;
+} pcapng_interface_description_block_s;
Comment:
will fix
> Mykyta Iziumtsev(MykytaI) wrote:
> Any justification for +16 and such a big buffer on a stack ? Do we really
> need 1024 events at once ?
>> Mykyta Iziumtsev(MykytaI) wrote:
>> Shall be _t
>>> Mykyta Iziumtsev(MykytaI) wrote:
>>> In fact we do care. We can't tolerate partial packet writes, otherwise
>>> pcapng file will be unreadable. The proper way to address that is to see
>>> how much space is left in the pipe (buf_size - TIOCOUTQ) and append packets
>>> to iovec as long as they fit in the calculated 'budget'.
>>>
>>> Another thing to consider -- is increasing pipe buffer size. The linux
>>> default size is 64K, so 'dd' command you used doesn't make much sense.
>>> Please consider setting it with fcntl to max size (1M according to
>>> /proc/sys/fs/pipe-max-size).
>>>> Mykyta Iziumtsev(MykytaI) wrote:
>>>> I guess only <stdint.h> and <sys/inotify.h> are needed here.
>>>>> Mykyta Iziumtsev(MykytaI) wrote:
>>>>> Is there any reason to put all of this into separate header file ? From
>>>>> what I see only one .c file is using it, so why not move it there ?
>>>>>> Mykyta Iziumtsev(MykytaI) wrote:
>>>>>> IMHO, only `int pcapng_fds[PKTIO_MAX_QUEUES]` belong here, the rest is
>>>>>> superfluous or belongs to global context. Why not initialize
>>>>>> pcapng_fds[0 .. ARRAY_SIZE - 1] = -1, open the pipes from inotify thread
>>>>>> and assign fds when the header is already written out ?
>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>> @apalos No, start and stop are good times for this as well. An
>>>>>>> application can stop a device, reconfigure it, and then restart it, so
>>>>>>> having that be the trigger for capture file cleanup and close makes
>>>>>>> sense.
>>>>>>>> Ilias Apalodimas(apalos) wrote:
>>>>>>>> wireshark re-defines them in every c file they want to use them.
>>>>>>>> Keeping in mind this is the pcapng file format specification, these
>>>>>>>> are high unlikely to change.
>>>>>>>>
>>>>>>>> The only relevant library i found is
>>>>>>>> ntar(https://www.winpcap.org/ntar/changelog.htm) but it seems
>>>>>>>> abandoned for many many years.
>>>>>>>>> Ilias Apalodimas(apalos) wrote:
>>>>>>>>> @Bill-Fischofer-Linaro pcapng_destroy() is called from _pktio_stop()
>>>>>>>>> which is called from odp_pktio_term_global().
>>>>>>>>> pcapng_prepare() on the other hand is called from odp_pktio_start().
>>>>>>>>> Want me to move it in odp_pktio_init_global()?
>>>>>>>>>> Ilias Apalodimas(apalos) wrote:
>>>>>>>>>> This essentially "reads" the inotify triggers for opening/closing
>>>>>>>>>> the fifos and sets the appropriate variables for starting/stopping
>>>>>>>>>> the pcapng dump on fifos. Will try to make it easier to read on the
>>>>>>>>>> final PR
>>>>>>>>>>> Ilias Apalodimas(apalos) wrote:
>>>>>>>>>>> will fix
>>>>>>>>>>>> Ilias Apalodimas(apalos) wrote:
>>>>>>>>>>>> will fix
>>>>>>>>>>>>> Ilias Apalodimas(apalos) wrote:
>>>>>>>>>>>>> I'll check and update for the final PR
>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>> General cleanup should be part of `odp_term_global()`
>>>>>>>>>>>>>> processing. Presumably `odp_pktio_close()` should also do
>>>>>>>>>>>>>> cleanup for specific pktios.
>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>> A few comments about what this routine is doing would be very
>>>>>>>>>>>>>>> helpful here. This is not easy code to follow. Modularizing it
>>>>>>>>>>>>>>> into some `static inline` helpers might also make the logic
>>>>>>>>>>>>>>> simpler.
>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>>> This is redundant. If `len > 0` then by definition `len != -1`.
>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>>>> This fails for multi-segment packets. `odp_packet_data(pkt)`
>>>>>>>>>>>>>>>>> is shorthand for `odp_packet_offset(pkt, 0, NULL, NULL)` so
>>>>>>>>>>>>>>>>> you don't know how long the first segment really is. First
>>>>>>>>>>>>>>>>> cut would be to only dump the first segment, so correct code
>>>>>>>>>>>>>>>>> is:
>>>>>>>>>>>>>>>>> ```
>>>>>>>>>>>>>>>>> uint32_t seg_len;
>>>>>>>>>>>>>>>>> char *buf = (char *)odp_packet_offset(packets[i], 0,
>>>>>>>>>>>>>>>>> &seg_len, NULL);
>>>>>>>>>>>>>>>>> ```
>>>>>>>>>>>>>>>>> And use `seg_len` rather than `pkt_len` in this routine.
>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>>>>> Does pcapng not supply a `.h` file for these "magic numbers"
>>>>>>>>>>>>>>>>>> and associated structs?
>>>>>>>>>>>>>>>>>>> Ilias Apalodimas(apalos) wrote:
>>>>>>>>>>>>>>>>>>> Will fix. This will also remove the requirement of manually
>>>>>>>>>>>>>>>>>>> deleting the fifos
>>>>>>>>>>>>>>>>>>>> muvarov wrote
>>>>>>>>>>>>>>>>>>>> odp_global_data_s->main_pid needs to be here.
https://github.com/Linaro/odp/pull/435#discussion_r166581095
updated_at 2018-02-07 10:49:30
