Re: [lng-odp] [PATCH CATERPILLAR v1] [RFC] odp: pktio: added tcpdump like functionality
Mykyta Iziumtsev(MykytaI) replied on github web page: platform/linux-generic/include/odp_pcapng.h line 1 @@ -0,0 +1,56 @@ +/* Copyright (c) 2018, Linaro Limited Comment: Maybe consider moving code to separate .c file then if it has nothing to do with packet io ? > Ilias Apalodimas(apalos) wrote: > too much unrelated info to the packet io. I'd prefer this reside on a > different file >> Mykyta Iziumtsev(MykytaI) wrote: >> I believe sizeof(struct) directly in code is more informative. >>> Mykyta Iziumtsev(MykytaI) wrote: >>> I'm not sure event aggregation has any value here, but even if it does -- >>> we don't need buffer for 1024 events. Ilias Apalodimas(apalos) wrote: The + 16 is just a reasonable guess. The event inotify returns might or might not have an event->name of dynamic size. If the buffer is too small, the system call returns zero. Since it allows event slurping i am trying to read multiple events at once. i could change this and use FIONREAD ioctl to get the exact number of bytes i can read from the inotify fd > Ilias Apalodimas(apalos) wrote: > will fix >> Ilias Apalodimas(apalos) wrote: >> 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 and 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 he
Re: [lng-odp] [PATCH CATERPILLAR v1] [RFC] odp: pktio: added tcpdump like functionality
Ilias Apalodimas(apalos) replied on github web page: platform/linux-generic/include/odp_pcapng.h line 1 @@ -0,0 +1,56 @@ +/* Copyright (c) 2018, Linaro Limited Comment: too much unrelated info to the packet io. I'd prefer this reside on a different file > Mykyta Iziumtsev(MykytaI) wrote: > I believe sizeof(struct) directly in code is more informative. >> Mykyta Iziumtsev(MykytaI) wrote: >> I'm not sure event aggregation has any value here, but even if it does -- we >> don't need buffer for 1024 events. >>> Ilias Apalodimas(apalos) wrote: >>> The + 16 is just a reasonable guess. The event inotify returns might or >>> might not have an event->name of dynamic size. If the buffer is too small, >>> the system call returns zero. Since it allows event slurping i am trying to >>> read multiple events at once. >>> >>> i could change this and use FIONREAD ioctl to get the exact number of bytes >>> i can read from the inotify fd Ilias Apalodimas(apalos) wrote: will fix > Ilias Apalodimas(apalos) wrote: > 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 and 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:
Re: [lng-odp] [PATCH CATERPILLAR v1] [RFC] odp: pktio: added tcpdump like functionality
Mykyta Iziumtsev(MykytaI) replied on github web page:
platform/linux-generic/odp_packet_io.c
line 105
@@ -89,6 +96,274 @@ int odp_pktio_init_local(void)
return odp_pktio_ops_init_local(true);
}
+static int write_pcapng_hdr(pktio_entry_t *entry, int qidx)
+{
+ size_t len;
+ pcapng_section_hdr_block_t shb;
+ pcapng_interface_description_block_s idb;
+ int fd = entry->s.pcapng_info.pcapng_fd[qidx];
+
+ memset(&shb, 0, sizeof(shb));
+ memset(&idb, 0, sizeof(idb));
+
+ shb.block_type = PCAPNG_BLOCK_TYPE_SHB;
+ shb.block_total_length = sizeof(shb);
+ shb.block_total_length2 = sizeof(shb);
+ shb.magic = PCAPNG_ENDIAN_MAGIC;
+ shb.version_major = 0x1;
+ shb.version_minor = 0x0;
+ shb.section_len = -1;
+
+ len = write(fd, &shb, sizeof(shb));
+ fsync(fd);
+ /* fail to write shb/idb means the pcapng is unreadable */
+ if (!len) {
+ ODP_ERR("Failed to write pcapng section hdr\n");
+ close(fd);
+ return -1;
+ }
+
+ idb.block_type = PCAPNG_BLOCK_TYPE_IDB;
+ idb.block_total_length = sizeof(idb);
+ idb.block_total_length2 = sizeof(idb);
+ idb.linktype = 0x1; /* LINKTYPE_ETHERNET */
+ idb.snaplen = 0x0; /* unlimited */
+ len = write(fd, &idb, sizeof(idb));
+ if (!len) {
+ ODP_ERR("Failed to write pcapng interface description\n");
+ close(fd);
+ return -1;
+ }
+ fsync(fd);
+
+ entry->s.pcapng_info.state[qidx] = PCAPNG_WR_HDR;
+
+ return 0;
+}
+
+static int write_pcapng_pkts(pktio_entry_t *entry, int qidx,
+const odp_packet_t packets[], int num)
+{
+ int i = 0;
+ struct iovec packet_iov[3 * num];
+ pcapng_enhanced_packet_block_t epb;
+ int iovcnt = 0;
+ ssize_t len = 0;
+ int fd = entry->s.pcapng_info.pcapng_fd[qidx];
+
+ for (i = 0; i < num; i++) {
+ odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(packets[i]);
+ uint32_t pkt_len = _odp_packet_len(packets[i]);
+ char *buf = (char *)odp_packet_data(packets[i]);
+
+ epb.block_type = PCAPNG_BLOCK_TYPE_EPB;
+ epb.block_total_length = sizeof(epb) +
+ ROUNDUP_ALIGN(pkt_len, PCAP_DATA_ALIGN) +
+ PCAP_DATA_ALIGN;
+ epb.interface_idx = 0;
+ epb.timestamp_high = (uint32_t)(pkt_hdr->timestamp.u64 >> 32);
+ epb.timestamp_low = (uint32_t)(pkt_hdr->timestamp.u64);
+ epb.captured_len = pkt_len;
+ epb.packet_len = pkt_len;
+
+ /* epb */
+ packet_iov[iovcnt].iov_base = &epb;
+ packet_iov[iovcnt].iov_len = sizeof(epb);
+ iovcnt++;
+
+ /* data */
+ packet_iov[iovcnt].iov_base = buf;
+ packet_iov[iovcnt].iov_len =
+ ROUNDUP_ALIGN(pkt_len, PCAP_DATA_ALIGN);
+ iovcnt++;
+
+ /* trailing */
+ packet_iov[iovcnt].iov_base = &epb.block_total_length;
+ packet_iov[iovcnt].iov_len = sizeof(uint32_t);
+ iovcnt++;
+ }
+
+ /* we don't really care if we manage to write *all* data */
Comment:
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 and 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
Re: [lng-odp] [PATCH CATERPILLAR v1] [RFC] odp: pktio: added tcpdump like functionality
Mykyta Iziumtsev(MykytaI) replied on github web page: platform/linux-generic/include/odp_pcapng.h line 23 @@ -0,0 +1,56 @@ +/* Copyright (c) 2018, Linaro Limited + * All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +#define PCAPNG_BLOCK_TYPE_EPB 0x0006UL +#define PCAPNG_BLOCK_TYPE_SHB 0x0A0D0D0AUL +#define PCAPNG_BLOCK_TYPE_IDB 0x0001UL +#define PCAPNG_ENDIAN_MAGIC 0x1A2B3C4DUL +#define PCAP_DATA_ALIGN (4) + +/* inotify */ +#define INOTIFY_EVENT_SIZE (sizeof(struct inotify_event)) Comment: I believe sizeof(struct) directly in code is more informative. > Mykyta Iziumtsev(MykytaI) wrote: > I'm not sure event aggregation has any value here, but even if it does -- we > don't need buffer for 1024 events. >> Ilias Apalodimas(apalos) wrote: >> The + 16 is just a reasonable guess. The event inotify returns might or >> might not have an event->name of dynamic size. If the buffer is too small, >> the system call returns zero. Since it allows event slurping i am trying to >> read multiple events at once. >> >> i could change this and use FIONREAD ioctl to get the exact number of bytes >> i can read from the inotify fd >>> Ilias Apalodimas(apalos) wrote: >>> will fix Ilias Apalodimas(apalos) wrote: 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 and 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 helpfu
Re: [lng-odp] [PATCH CATERPILLAR v1] [RFC] odp: pktio: added tcpdump like functionality
Mykyta Iziumtsev(MykytaI) replied on github web page: platform/linux-generic/include/odp_pcapng.h line 24 @@ -0,0 +1,56 @@ +/* Copyright (c) 2018, Linaro Limited + * All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +#define PCAPNG_BLOCK_TYPE_EPB 0x0006UL +#define PCAPNG_BLOCK_TYPE_SHB 0x0A0D0D0AUL +#define PCAPNG_BLOCK_TYPE_IDB 0x0001UL +#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)) Comment: I'm not sure event aggregation has any value here, but even if it does -- we don't need buffer for 1024 events. > Ilias Apalodimas(apalos) wrote: > The + 16 is just a reasonable guess. The event inotify returns might or might > not have an event->name of dynamic size. If the buffer is too small, the > system call returns zero. Since it allows event slurping i am trying to read > multiple events at once. > > i could change this and use FIONREAD ioctl to get the exact number of bytes i > can read from the inotify fd >> Ilias Apalodimas(apalos) wrote: >> will fix >>> Ilias Apalodimas(apalos) wrote: >>> 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 and 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 mi
Re: [lng-odp] [PATCH CATERPILLAR v1] [RFC] odp: pktio: added tcpdump like functionality
Ilias Apalodimas(apalos) replied on github web page:
platform/linux-generic/include/odp_packet_io_internal.h
line 18
@@ -97,6 +97,20 @@ struct pktio_entry {
odp_queue_tqueue;
odp_pktout_queue_t pktout;
} out_queue[PKTIO_MAX_QUEUES];
+
+ /**< inotify instance for pcapng fifos */
+ struct {
+ int inotify_pcapng_fd;
+ int inotify_watch_fd;
+ pthread_t inotify_thread;
+ enum {
+ PCAPNG_WR_INVALID = 0,
+ PCAPNG_WR_STOP,
+ PCAPNG_WR_HDR,
+ PCAPNG_WR_PKT,
+ } state[PKTIO_MAX_QUEUES];
+ int pcapng_fd[PKTIO_MAX_QUEUES];
+ } pcapng_info;
};
Comment:
will fix
> Ilias Apalodimas(apalos) wrote:
> 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 and 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
>
Re: [lng-odp] [PATCH CATERPILLAR v1] [RFC] odp: pktio: added tcpdump like functionality
Ilias Apalodimas(apalos) replied on github web page: platform/linux-generic/include/odp_pcapng.h line 24 @@ -0,0 +1,56 @@ +/* Copyright (c) 2018, Linaro Limited + * All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +#define PCAPNG_BLOCK_TYPE_EPB 0x0006UL +#define PCAPNG_BLOCK_TYPE_SHB 0x0A0D0D0AUL +#define PCAPNG_BLOCK_TYPE_IDB 0x0001UL +#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)) Comment: The + 16 is just a reasonable guess. The event inotify returns might or might not have an event->name of dynamic size. If the buffer is too small, the system call returns zero. Since it allows event slurping i am trying to read multiple events at once. i could change this and use FIONREAD ioctl to get the exact number of bytes i can read from the inotify fd > Ilias Apalodimas(apalos) wrote: > will fix >> Ilias Apalodimas(apalos) wrote: >> 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 and 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`. >>
Re: [lng-odp] [PATCH CATERPILLAR v1] [RFC] odp: pktio: added tcpdump like functionality
Mykyta Iziumtsev(MykytaI) 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
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define PCAPNG_BLOCK_TYPE_EPB 0x0006UL
+#define PCAPNG_BLOCK_TYPE_SHB 0x0A0D0D0AUL
+#define PCAPNG_BLOCK_TYPE_IDB 0x0001UL
+#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:
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 and 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
Re: [lng-odp] [PATCH CATERPILLAR v1] [RFC] odp: pktio: added tcpdump like functionality
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
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define PCAPNG_BLOCK_TYPE_EPB 0x0006UL
+#define PCAPNG_BLOCK_TYPE_SHB 0x0A0D0D0AUL
+#define PCAPNG_BLOCK_TYPE_IDB 0x0001UL
+#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 and 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:
Thi
Re: [lng-odp] [PATCH CATERPILLAR v1] [RFC] odp: pktio: added tcpdump like functionality
Mykyta Iziumtsev(MykytaI) replied on github web page: platform/linux-generic/include/odp_pcapng.h line 24 @@ -0,0 +1,56 @@ +/* Copyright (c) 2018, Linaro Limited + * All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +#define PCAPNG_BLOCK_TYPE_EPB 0x0006UL +#define PCAPNG_BLOCK_TYPE_SHB 0x0A0D0D0AUL +#define PCAPNG_BLOCK_TYPE_IDB 0x0001UL +#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)) Comment: 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 and 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 no
Re: [lng-odp] [PATCH CATERPILLAR v1] [RFC] odp: pktio: added tcpdump like functionality
Mykyta Iziumtsev(MykytaI) replied on github web page: platform/linux-generic/include/odp_pcapng.h line 14 @@ -0,0 +1,56 @@ +/* Copyright (c) 2018, Linaro Limited + * All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +#include +#include +#include +#include +#include +#include +#include +#include Comment: I guess only and 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_r166566828 updated_at 2018-02-07 10:31:08
Re: [lng-odp] [PATCH CATERPILLAR v1] [RFC] odp: pktio: added tcpdump like functionality
Mykyta Iziumtsev(MykytaI) replied on github web page: platform/linux-generic/include/odp_pcapng.h line 1 @@ -0,0 +1,56 @@ +/* Copyright (c) 2018, Linaro Limited Comment: 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_r166565709 updated_at 2018-02-07 10:31:08
Re: [lng-odp] [PATCH CATERPILLAR v1] [RFC] odp: pktio: added tcpdump like functionality
Mykyta Iziumtsev(MykytaI) replied on github web page:
platform/linux-generic/include/odp_packet_io_internal.h
line 18
@@ -97,6 +97,20 @@ struct pktio_entry {
odp_queue_tqueue;
odp_pktout_queue_t pktout;
} out_queue[PKTIO_MAX_QUEUES];
+
+ /**< inotify instance for pcapng fifos */
+ struct {
+ int inotify_pcapng_fd;
+ int inotify_watch_fd;
+ pthread_t inotify_thread;
+ enum {
+ PCAPNG_WR_INVALID = 0,
+ PCAPNG_WR_STOP,
+ PCAPNG_WR_HDR,
+ PCAPNG_WR_PKT,
+ } state[PKTIO_MAX_QUEUES];
+ int pcapng_fd[PKTIO_MAX_QUEUES];
+ } pcapng_info;
};
Comment:
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_r166565206
updated_at 2018-02-07 10:31:08
Re: [lng-odp] [PATCH CATERPILLAR v1] [RFC] odp: pktio: added tcpdump like functionality
Bill Fischofer(Bill-Fischofer-Linaro) replied on github web page:
platform/linux-generic/odp_packet_io.c
line 237
@@ -89,6 +96,274 @@ int odp_pktio_init_local(void)
return odp_pktio_ops_init_local(true);
}
+static int write_pcapng_hdr(pktio_entry_t *entry, int qidx)
+{
+ size_t len;
+ pcapng_section_hdr_block_t shb;
+ pcapng_interface_description_block_s idb;
+ int fd = entry->s.pcapng_info.pcapng_fd[qidx];
+
+ memset(&shb, 0, sizeof(shb));
+ memset(&idb, 0, sizeof(idb));
+
+ shb.block_type = PCAPNG_BLOCK_TYPE_SHB;
+ shb.block_total_length = sizeof(shb);
+ shb.block_total_length2 = sizeof(shb);
+ shb.magic = PCAPNG_ENDIAN_MAGIC;
+ shb.version_major = 0x1;
+ shb.version_minor = 0x0;
+ shb.section_len = -1;
+
+ len = write(fd, &shb, sizeof(shb));
+ fsync(fd);
+ /* fail to write shb/idb means the pcapng is unreadable */
+ if (!len) {
+ ODP_ERR("Failed to write pcapng section hdr\n");
+ close(fd);
+ return -1;
+ }
+
+ idb.block_type = PCAPNG_BLOCK_TYPE_IDB;
+ idb.block_total_length = sizeof(idb);
+ idb.block_total_length2 = sizeof(idb);
+ idb.linktype = 0x1; /* LINKTYPE_ETHERNET */
+ idb.snaplen = 0x0; /* unlimited */
+ len = write(fd, &idb, sizeof(idb));
+ if (!len) {
+ ODP_ERR("Failed to write pcapng interface description\n");
+ close(fd);
+ return -1;
+ }
+ fsync(fd);
+
+ entry->s.pcapng_info.state[qidx] = PCAPNG_WR_HDR;
+
+ return 0;
+}
+
+static int write_pcapng_pkts(pktio_entry_t *entry, int qidx,
+const odp_packet_t packets[], int num)
+{
+ int i = 0;
+ struct iovec packet_iov[3 * num];
+ pcapng_enhanced_packet_block_t epb;
+ int iovcnt = 0;
+ ssize_t len = 0;
+ int fd = entry->s.pcapng_info.pcapng_fd[qidx];
+
+ for (i = 0; i < num; i++) {
+ odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(packets[i]);
+ uint32_t pkt_len = _odp_packet_len(packets[i]);
+ char *buf = (char *)odp_packet_data(packets[i]);
+
+ epb.block_type = PCAPNG_BLOCK_TYPE_EPB;
+ epb.block_total_length = sizeof(epb) +
+ ROUNDUP_ALIGN(pkt_len, PCAP_DATA_ALIGN) +
+ PCAP_DATA_ALIGN;
+ epb.interface_idx = 0;
+ epb.timestamp_high = (uint32_t)(pkt_hdr->timestamp.u64 >> 32);
+ epb.timestamp_low = (uint32_t)(pkt_hdr->timestamp.u64);
+ epb.captured_len = pkt_len;
+ epb.packet_len = pkt_len;
+
+ /* epb */
+ packet_iov[iovcnt].iov_base = &epb;
+ packet_iov[iovcnt].iov_len = sizeof(epb);
+ iovcnt++;
+
+ /* data */
+ packet_iov[iovcnt].iov_base = buf;
+ packet_iov[iovcnt].iov_len =
+ ROUNDUP_ALIGN(pkt_len, PCAP_DATA_ALIGN);
+ iovcnt++;
+
+ /* trailing */
+ packet_iov[iovcnt].iov_base = &epb.block_total_length;
+ packet_iov[iovcnt].iov_len = sizeof(uint32_t);
+ iovcnt++;
+ }
+
+ /* we don't really care if we manage to write *all* data */
+ len = writev(fd, packet_iov, iovcnt);
+ if (!len)
+ ODP_ERR("Failed to write pcapng data\n");
+ fsync(fd);
+
+ return len;
+}
+
+static void pcapng_drain_fifo(int fd)
+{
+ char c;
+ ssize_t len;
+
+ do {
+ len = read(fd, &c, sizeof(c));
+ } while (len > 0 && len != -1);
+ ODP_DBG("Drain pcap fifo %d\n", len);
+}
+
+static void *inotify_update(void *arg)
+{
+ pktio_entry_t *entry = (pktio_entry_t *)arg;
+ struct timeval time;
+ int ret;
+ ssize_t rdlen;
+ int i;
+ char buffer[INOTIFY_BUF_LEN];
+ unsigned int max_queue =
+ MAX(entry->s.num_in_queue, entry->s.num_out_queue);
+ fd_set rfds;
+
+ while (1) {
+ i = 0;
+ FD_ZERO(&rfds);
+ FD_SET(entry->s.pcapng_info.inotify_pcapng_fd, &rfds);
+ time.tv_sec = 2;
+ time.tv_usec = 0;
+ select(entry->s.pcapng_info.inotify_pcapng_fd + 1, &rfds, NULL,
+ NULL, &time);
+ if (FD_ISSET(entry->s.pcapng_info.inotify_pcapng_fd, &rfds)) {
+ rdlen = read(entry->s.pcapng_info.inotify_pcapng_fd,
+buffer, INOTIFY_BUF_LEN);
+ while (i < rdlen) {
+ char *e;
+ unsigned int qidx;
+ struct inotify_event *event =
+ (struct inotify_event *)(void *)
+&buffer[i];
+
+
Re: [lng-odp] [PATCH CATERPILLAR v1] [RFC] odp: pktio: added tcpdump like functionality
Ilias Apalodimas(apalos) replied on github web page:
platform/linux-generic/odp_packet_io.c
line 237
@@ -89,6 +96,274 @@ int odp_pktio_init_local(void)
return odp_pktio_ops_init_local(true);
}
+static int write_pcapng_hdr(pktio_entry_t *entry, int qidx)
+{
+ size_t len;
+ pcapng_section_hdr_block_t shb;
+ pcapng_interface_description_block_s idb;
+ int fd = entry->s.pcapng_info.pcapng_fd[qidx];
+
+ memset(&shb, 0, sizeof(shb));
+ memset(&idb, 0, sizeof(idb));
+
+ shb.block_type = PCAPNG_BLOCK_TYPE_SHB;
+ shb.block_total_length = sizeof(shb);
+ shb.block_total_length2 = sizeof(shb);
+ shb.magic = PCAPNG_ENDIAN_MAGIC;
+ shb.version_major = 0x1;
+ shb.version_minor = 0x0;
+ shb.section_len = -1;
+
+ len = write(fd, &shb, sizeof(shb));
+ fsync(fd);
+ /* fail to write shb/idb means the pcapng is unreadable */
+ if (!len) {
+ ODP_ERR("Failed to write pcapng section hdr\n");
+ close(fd);
+ return -1;
+ }
+
+ idb.block_type = PCAPNG_BLOCK_TYPE_IDB;
+ idb.block_total_length = sizeof(idb);
+ idb.block_total_length2 = sizeof(idb);
+ idb.linktype = 0x1; /* LINKTYPE_ETHERNET */
+ idb.snaplen = 0x0; /* unlimited */
+ len = write(fd, &idb, sizeof(idb));
+ if (!len) {
+ ODP_ERR("Failed to write pcapng interface description\n");
+ close(fd);
+ return -1;
+ }
+ fsync(fd);
+
+ entry->s.pcapng_info.state[qidx] = PCAPNG_WR_HDR;
+
+ return 0;
+}
+
+static int write_pcapng_pkts(pktio_entry_t *entry, int qidx,
+const odp_packet_t packets[], int num)
+{
+ int i = 0;
+ struct iovec packet_iov[3 * num];
+ pcapng_enhanced_packet_block_t epb;
+ int iovcnt = 0;
+ ssize_t len = 0;
+ int fd = entry->s.pcapng_info.pcapng_fd[qidx];
+
+ for (i = 0; i < num; i++) {
+ odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(packets[i]);
+ uint32_t pkt_len = _odp_packet_len(packets[i]);
+ char *buf = (char *)odp_packet_data(packets[i]);
+
+ epb.block_type = PCAPNG_BLOCK_TYPE_EPB;
+ epb.block_total_length = sizeof(epb) +
+ ROUNDUP_ALIGN(pkt_len, PCAP_DATA_ALIGN) +
+ PCAP_DATA_ALIGN;
+ epb.interface_idx = 0;
+ epb.timestamp_high = (uint32_t)(pkt_hdr->timestamp.u64 >> 32);
+ epb.timestamp_low = (uint32_t)(pkt_hdr->timestamp.u64);
+ epb.captured_len = pkt_len;
+ epb.packet_len = pkt_len;
+
+ /* epb */
+ packet_iov[iovcnt].iov_base = &epb;
+ packet_iov[iovcnt].iov_len = sizeof(epb);
+ iovcnt++;
+
+ /* data */
+ packet_iov[iovcnt].iov_base = buf;
+ packet_iov[iovcnt].iov_len =
+ ROUNDUP_ALIGN(pkt_len, PCAP_DATA_ALIGN);
+ iovcnt++;
+
+ /* trailing */
+ packet_iov[iovcnt].iov_base = &epb.block_total_length;
+ packet_iov[iovcnt].iov_len = sizeof(uint32_t);
+ iovcnt++;
+ }
+
+ /* we don't really care if we manage to write *all* data */
+ len = writev(fd, packet_iov, iovcnt);
+ if (!len)
+ ODP_ERR("Failed to write pcapng data\n");
+ fsync(fd);
+
+ return len;
+}
+
+static void pcapng_drain_fifo(int fd)
+{
+ char c;
+ ssize_t len;
+
+ do {
+ len = read(fd, &c, sizeof(c));
+ } while (len > 0 && len != -1);
+ ODP_DBG("Drain pcap fifo %d\n", len);
+}
+
+static void *inotify_update(void *arg)
+{
+ pktio_entry_t *entry = (pktio_entry_t *)arg;
+ struct timeval time;
+ int ret;
+ ssize_t rdlen;
+ int i;
+ char buffer[INOTIFY_BUF_LEN];
+ unsigned int max_queue =
+ MAX(entry->s.num_in_queue, entry->s.num_out_queue);
+ fd_set rfds;
+
+ while (1) {
+ i = 0;
+ FD_ZERO(&rfds);
+ FD_SET(entry->s.pcapng_info.inotify_pcapng_fd, &rfds);
+ time.tv_sec = 2;
+ time.tv_usec = 0;
+ select(entry->s.pcapng_info.inotify_pcapng_fd + 1, &rfds, NULL,
+ NULL, &time);
+ if (FD_ISSET(entry->s.pcapng_info.inotify_pcapng_fd, &rfds)) {
+ rdlen = read(entry->s.pcapng_info.inotify_pcapng_fd,
+buffer, INOTIFY_BUF_LEN);
+ while (i < rdlen) {
+ char *e;
+ unsigned int qidx;
+ struct inotify_event *event =
+ (struct inotify_event *)(void *)
+&buffer[i];
+
+ e
Re: [lng-odp] [PATCH CATERPILLAR v1] [RFC] odp: pktio: added tcpdump like functionality
Ilias Apalodimas(apalos) replied on github web page: platform/linux-generic/include/odp_pcapng.h line 20 @@ -0,0 +1,56 @@ +/* Copyright (c) 2018, Linaro Limited + * All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +#define PCAPNG_BLOCK_TYPE_EPB 0x0006UL +#define PCAPNG_BLOCK_TYPE_SHB 0x0A0D0D0AUL +#define PCAPNG_BLOCK_TYPE_IDB 0x0001UL +#define PCAPNG_ENDIAN_MAGIC 0x1A2B3C4DUL +#define PCAP_DATA_ALIGN (4) Comment: 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: > will update >> 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_r165633289 updated_at 2018-02-02 12:46:49
Re: [lng-odp] [PATCH CATERPILLAR v1] [RFC] odp: pktio: added tcpdump like functionality
Ilias Apalodimas(apalos) replied on github web page:
platform/linux-generic/odp_packet_io.c
line 137
@@ -89,6 +96,274 @@ int odp_pktio_init_local(void)
return odp_pktio_ops_init_local(true);
}
+static int write_pcapng_hdr(pktio_entry_t *entry, int qidx)
+{
+ size_t len;
+ pcapng_section_hdr_block_t shb;
+ pcapng_interface_description_block_s idb;
+ int fd = entry->s.pcapng_info.pcapng_fd[qidx];
+
+ memset(&shb, 0, sizeof(shb));
+ memset(&idb, 0, sizeof(idb));
+
+ shb.block_type = PCAPNG_BLOCK_TYPE_SHB;
+ shb.block_total_length = sizeof(shb);
+ shb.block_total_length2 = sizeof(shb);
+ shb.magic = PCAPNG_ENDIAN_MAGIC;
+ shb.version_major = 0x1;
+ shb.version_minor = 0x0;
+ shb.section_len = -1;
+
+ len = write(fd, &shb, sizeof(shb));
+ fsync(fd);
+ /* fail to write shb/idb means the pcapng is unreadable */
+ if (!len) {
+ ODP_ERR("Failed to write pcapng section hdr\n");
+ close(fd);
+ return -1;
+ }
+
+ idb.block_type = PCAPNG_BLOCK_TYPE_IDB;
+ idb.block_total_length = sizeof(idb);
+ idb.block_total_length2 = sizeof(idb);
+ idb.linktype = 0x1; /* LINKTYPE_ETHERNET */
+ idb.snaplen = 0x0; /* unlimited */
+ len = write(fd, &idb, sizeof(idb));
+ if (!len) {
+ ODP_ERR("Failed to write pcapng interface description\n");
+ close(fd);
+ return -1;
+ }
+ fsync(fd);
+
+ entry->s.pcapng_info.state[qidx] = PCAPNG_WR_HDR;
+
+ return 0;
+}
+
+static int write_pcapng_pkts(pktio_entry_t *entry, int qidx,
+const odp_packet_t packets[], int num)
+{
+ int i = 0;
+ struct iovec packet_iov[3 * num];
+ pcapng_enhanced_packet_block_t epb;
+ int iovcnt = 0;
+ ssize_t len = 0;
+ int fd = entry->s.pcapng_info.pcapng_fd[qidx];
+
+ for (i = 0; i < num; i++) {
+ odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(packets[i]);
+ uint32_t pkt_len = _odp_packet_len(packets[i]);
+ char *buf = (char *)odp_packet_data(packets[i]);
+
+ epb.block_type = PCAPNG_BLOCK_TYPE_EPB;
+ epb.block_total_length = sizeof(epb) +
+ ROUNDUP_ALIGN(pkt_len, PCAP_DATA_ALIGN) +
+ PCAP_DATA_ALIGN;
+ epb.interface_idx = 0;
+ epb.timestamp_high = (uint32_t)(pkt_hdr->timestamp.u64 >> 32);
+ epb.timestamp_low = (uint32_t)(pkt_hdr->timestamp.u64);
+ epb.captured_len = pkt_len;
+ epb.packet_len = pkt_len;
+
+ /* epb */
+ packet_iov[iovcnt].iov_base = &epb;
+ packet_iov[iovcnt].iov_len = sizeof(epb);
+ iovcnt++;
+
+ /* data */
+ packet_iov[iovcnt].iov_base = buf;
+ packet_iov[iovcnt].iov_len =
+ ROUNDUP_ALIGN(pkt_len, PCAP_DATA_ALIGN);
+ iovcnt++;
+
+ /* trailing */
+ packet_iov[iovcnt].iov_base = &epb.block_total_length;
+ packet_iov[iovcnt].iov_len = sizeof(uint32_t);
+ iovcnt++;
+ }
+
+ /* we don't really care if we manage to write *all* data */
+ len = writev(fd, packet_iov, iovcnt);
+ if (!len)
+ ODP_ERR("Failed to write pcapng data\n");
+ fsync(fd);
+
+ return len;
+}
+
+static void pcapng_drain_fifo(int fd)
+{
+ char c;
+ ssize_t len;
+
+ do {
+ len = read(fd, &c, sizeof(c));
+ } while (len > 0 && len != -1);
+ ODP_DBG("Drain pcap fifo %d\n", len);
+}
+
+static void *inotify_update(void *arg)
+{
+ pktio_entry_t *entry = (pktio_entry_t *)arg;
+ struct timeval time;
+ int ret;
+ ssize_t rdlen;
+ int i;
+ char buffer[INOTIFY_BUF_LEN];
+ unsigned int max_queue =
+ MAX(entry->s.num_in_queue, entry->s.num_out_queue);
+ fd_set rfds;
+
+ while (1) {
Comment:
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 Fisc
Re: [lng-odp] [PATCH CATERPILLAR v1] [RFC] odp: pktio: added tcpdump like functionality
Ilias Apalodimas(apalos) replied on github web page:
platform/linux-generic/odp_packet_io.c
line 121
@@ -89,6 +96,274 @@ int odp_pktio_init_local(void)
return odp_pktio_ops_init_local(true);
}
+static int write_pcapng_hdr(pktio_entry_t *entry, int qidx)
+{
+ size_t len;
+ pcapng_section_hdr_block_t shb;
+ pcapng_interface_description_block_s idb;
+ int fd = entry->s.pcapng_info.pcapng_fd[qidx];
+
+ memset(&shb, 0, sizeof(shb));
+ memset(&idb, 0, sizeof(idb));
+
+ shb.block_type = PCAPNG_BLOCK_TYPE_SHB;
+ shb.block_total_length = sizeof(shb);
+ shb.block_total_length2 = sizeof(shb);
+ shb.magic = PCAPNG_ENDIAN_MAGIC;
+ shb.version_major = 0x1;
+ shb.version_minor = 0x0;
+ shb.section_len = -1;
+
+ len = write(fd, &shb, sizeof(shb));
+ fsync(fd);
+ /* fail to write shb/idb means the pcapng is unreadable */
+ if (!len) {
+ ODP_ERR("Failed to write pcapng section hdr\n");
+ close(fd);
+ return -1;
+ }
+
+ idb.block_type = PCAPNG_BLOCK_TYPE_IDB;
+ idb.block_total_length = sizeof(idb);
+ idb.block_total_length2 = sizeof(idb);
+ idb.linktype = 0x1; /* LINKTYPE_ETHERNET */
+ idb.snaplen = 0x0; /* unlimited */
+ len = write(fd, &idb, sizeof(idb));
+ if (!len) {
+ ODP_ERR("Failed to write pcapng interface description\n");
+ close(fd);
+ return -1;
+ }
+ fsync(fd);
+
+ entry->s.pcapng_info.state[qidx] = PCAPNG_WR_HDR;
+
+ return 0;
+}
+
+static int write_pcapng_pkts(pktio_entry_t *entry, int qidx,
+const odp_packet_t packets[], int num)
+{
+ int i = 0;
+ struct iovec packet_iov[3 * num];
+ pcapng_enhanced_packet_block_t epb;
+ int iovcnt = 0;
+ ssize_t len = 0;
+ int fd = entry->s.pcapng_info.pcapng_fd[qidx];
+
+ for (i = 0; i < num; i++) {
+ odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(packets[i]);
+ uint32_t pkt_len = _odp_packet_len(packets[i]);
+ char *buf = (char *)odp_packet_data(packets[i]);
+
+ epb.block_type = PCAPNG_BLOCK_TYPE_EPB;
+ epb.block_total_length = sizeof(epb) +
+ ROUNDUP_ALIGN(pkt_len, PCAP_DATA_ALIGN) +
+ PCAP_DATA_ALIGN;
+ epb.interface_idx = 0;
+ epb.timestamp_high = (uint32_t)(pkt_hdr->timestamp.u64 >> 32);
+ epb.timestamp_low = (uint32_t)(pkt_hdr->timestamp.u64);
+ epb.captured_len = pkt_len;
+ epb.packet_len = pkt_len;
+
+ /* epb */
+ packet_iov[iovcnt].iov_base = &epb;
+ packet_iov[iovcnt].iov_len = sizeof(epb);
+ iovcnt++;
+
+ /* data */
+ packet_iov[iovcnt].iov_base = buf;
+ packet_iov[iovcnt].iov_len =
+ ROUNDUP_ALIGN(pkt_len, PCAP_DATA_ALIGN);
+ iovcnt++;
+
+ /* trailing */
+ packet_iov[iovcnt].iov_base = &epb.block_total_length;
+ packet_iov[iovcnt].iov_len = sizeof(uint32_t);
+ iovcnt++;
+ }
+
+ /* we don't really care if we manage to write *all* data */
+ len = writev(fd, packet_iov, iovcnt);
+ if (!len)
+ ODP_ERR("Failed to write pcapng data\n");
+ fsync(fd);
+
+ return len;
+}
+
+static void pcapng_drain_fifo(int fd)
+{
+ char c;
+ ssize_t len;
+
+ do {
+ len = read(fd, &c, sizeof(c));
+ } while (len > 0 && len != -1);
Comment:
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) wro
Re: [lng-odp] [PATCH CATERPILLAR v1] [RFC] odp: pktio: added tcpdump like functionality
Ilias Apalodimas(apalos) replied on github web page:
platform/linux-generic/odp_packet_io.c
line 76
@@ -89,6 +96,274 @@ int odp_pktio_init_local(void)
return odp_pktio_ops_init_local(true);
}
+static int write_pcapng_hdr(pktio_entry_t *entry, int qidx)
+{
+ size_t len;
+ pcapng_section_hdr_block_t shb;
+ pcapng_interface_description_block_s idb;
+ int fd = entry->s.pcapng_info.pcapng_fd[qidx];
+
+ memset(&shb, 0, sizeof(shb));
+ memset(&idb, 0, sizeof(idb));
+
+ shb.block_type = PCAPNG_BLOCK_TYPE_SHB;
+ shb.block_total_length = sizeof(shb);
+ shb.block_total_length2 = sizeof(shb);
+ shb.magic = PCAPNG_ENDIAN_MAGIC;
+ shb.version_major = 0x1;
+ shb.version_minor = 0x0;
+ shb.section_len = -1;
+
+ len = write(fd, &shb, sizeof(shb));
+ fsync(fd);
+ /* fail to write shb/idb means the pcapng is unreadable */
+ if (!len) {
+ ODP_ERR("Failed to write pcapng section hdr\n");
+ close(fd);
+ return -1;
+ }
+
+ idb.block_type = PCAPNG_BLOCK_TYPE_IDB;
+ idb.block_total_length = sizeof(idb);
+ idb.block_total_length2 = sizeof(idb);
+ idb.linktype = 0x1; /* LINKTYPE_ETHERNET */
+ idb.snaplen = 0x0; /* unlimited */
+ len = write(fd, &idb, sizeof(idb));
+ if (!len) {
+ ODP_ERR("Failed to write pcapng interface description\n");
+ close(fd);
+ return -1;
+ }
+ fsync(fd);
+
+ entry->s.pcapng_info.state[qidx] = PCAPNG_WR_HDR;
+
+ return 0;
+}
+
+static int write_pcapng_pkts(pktio_entry_t *entry, int qidx,
+const odp_packet_t packets[], int num)
+{
+ int i = 0;
+ struct iovec packet_iov[3 * num];
+ pcapng_enhanced_packet_block_t epb;
+ int iovcnt = 0;
+ ssize_t len = 0;
+ int fd = entry->s.pcapng_info.pcapng_fd[qidx];
+
+ for (i = 0; i < num; i++) {
+ odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(packets[i]);
+ uint32_t pkt_len = _odp_packet_len(packets[i]);
+ char *buf = (char *)odp_packet_data(packets[i]);
Comment:
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_r165571963
updated_at 2018-02-02 07:13:58
Re: [lng-odp] [PATCH CATERPILLAR v1] [RFC] odp: pktio: added tcpdump like functionality
Ilias Apalodimas(apalos) replied on github web page: platform/linux-generic/include/odp_pcapng.h line 20 @@ -0,0 +1,56 @@ +/* Copyright (c) 2018, Linaro Limited + * All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +#define PCAPNG_BLOCK_TYPE_EPB 0x0006UL +#define PCAPNG_BLOCK_TYPE_SHB 0x0A0D0D0AUL +#define PCAPNG_BLOCK_TYPE_IDB 0x0001UL +#define PCAPNG_ENDIAN_MAGIC 0x1A2B3C4DUL +#define PCAP_DATA_ALIGN (4) Comment: 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_r165571920 updated_at 2018-02-02 07:13:35
Re: [lng-odp] [PATCH CATERPILLAR v1] [RFC] odp: pktio: added tcpdump like functionality
Bill Fischofer(Bill-Fischofer-Linaro) replied on github web page:
platform/linux-generic/odp_packet_io.c
line 237
@@ -89,6 +96,274 @@ int odp_pktio_init_local(void)
return odp_pktio_ops_init_local(true);
}
+static int write_pcapng_hdr(pktio_entry_t *entry, int qidx)
+{
+ size_t len;
+ pcapng_section_hdr_block_t shb;
+ pcapng_interface_description_block_s idb;
+ int fd = entry->s.pcapng_info.pcapng_fd[qidx];
+
+ memset(&shb, 0, sizeof(shb));
+ memset(&idb, 0, sizeof(idb));
+
+ shb.block_type = PCAPNG_BLOCK_TYPE_SHB;
+ shb.block_total_length = sizeof(shb);
+ shb.block_total_length2 = sizeof(shb);
+ shb.magic = PCAPNG_ENDIAN_MAGIC;
+ shb.version_major = 0x1;
+ shb.version_minor = 0x0;
+ shb.section_len = -1;
+
+ len = write(fd, &shb, sizeof(shb));
+ fsync(fd);
+ /* fail to write shb/idb means the pcapng is unreadable */
+ if (!len) {
+ ODP_ERR("Failed to write pcapng section hdr\n");
+ close(fd);
+ return -1;
+ }
+
+ idb.block_type = PCAPNG_BLOCK_TYPE_IDB;
+ idb.block_total_length = sizeof(idb);
+ idb.block_total_length2 = sizeof(idb);
+ idb.linktype = 0x1; /* LINKTYPE_ETHERNET */
+ idb.snaplen = 0x0; /* unlimited */
+ len = write(fd, &idb, sizeof(idb));
+ if (!len) {
+ ODP_ERR("Failed to write pcapng interface description\n");
+ close(fd);
+ return -1;
+ }
+ fsync(fd);
+
+ entry->s.pcapng_info.state[qidx] = PCAPNG_WR_HDR;
+
+ return 0;
+}
+
+static int write_pcapng_pkts(pktio_entry_t *entry, int qidx,
+const odp_packet_t packets[], int num)
+{
+ int i = 0;
+ struct iovec packet_iov[3 * num];
+ pcapng_enhanced_packet_block_t epb;
+ int iovcnt = 0;
+ ssize_t len = 0;
+ int fd = entry->s.pcapng_info.pcapng_fd[qidx];
+
+ for (i = 0; i < num; i++) {
+ odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(packets[i]);
+ uint32_t pkt_len = _odp_packet_len(packets[i]);
+ char *buf = (char *)odp_packet_data(packets[i]);
+
+ epb.block_type = PCAPNG_BLOCK_TYPE_EPB;
+ epb.block_total_length = sizeof(epb) +
+ ROUNDUP_ALIGN(pkt_len, PCAP_DATA_ALIGN) +
+ PCAP_DATA_ALIGN;
+ epb.interface_idx = 0;
+ epb.timestamp_high = (uint32_t)(pkt_hdr->timestamp.u64 >> 32);
+ epb.timestamp_low = (uint32_t)(pkt_hdr->timestamp.u64);
+ epb.captured_len = pkt_len;
+ epb.packet_len = pkt_len;
+
+ /* epb */
+ packet_iov[iovcnt].iov_base = &epb;
+ packet_iov[iovcnt].iov_len = sizeof(epb);
+ iovcnt++;
+
+ /* data */
+ packet_iov[iovcnt].iov_base = buf;
+ packet_iov[iovcnt].iov_len =
+ ROUNDUP_ALIGN(pkt_len, PCAP_DATA_ALIGN);
+ iovcnt++;
+
+ /* trailing */
+ packet_iov[iovcnt].iov_base = &epb.block_total_length;
+ packet_iov[iovcnt].iov_len = sizeof(uint32_t);
+ iovcnt++;
+ }
+
+ /* we don't really care if we manage to write *all* data */
+ len = writev(fd, packet_iov, iovcnt);
+ if (!len)
+ ODP_ERR("Failed to write pcapng data\n");
+ fsync(fd);
+
+ return len;
+}
+
+static void pcapng_drain_fifo(int fd)
+{
+ char c;
+ ssize_t len;
+
+ do {
+ len = read(fd, &c, sizeof(c));
+ } while (len > 0 && len != -1);
+ ODP_DBG("Drain pcap fifo %d\n", len);
+}
+
+static void *inotify_update(void *arg)
+{
+ pktio_entry_t *entry = (pktio_entry_t *)arg;
+ struct timeval time;
+ int ret;
+ ssize_t rdlen;
+ int i;
+ char buffer[INOTIFY_BUF_LEN];
+ unsigned int max_queue =
+ MAX(entry->s.num_in_queue, entry->s.num_out_queue);
+ fd_set rfds;
+
+ while (1) {
+ i = 0;
+ FD_ZERO(&rfds);
+ FD_SET(entry->s.pcapng_info.inotify_pcapng_fd, &rfds);
+ time.tv_sec = 2;
+ time.tv_usec = 0;
+ select(entry->s.pcapng_info.inotify_pcapng_fd + 1, &rfds, NULL,
+ NULL, &time);
+ if (FD_ISSET(entry->s.pcapng_info.inotify_pcapng_fd, &rfds)) {
+ rdlen = read(entry->s.pcapng_info.inotify_pcapng_fd,
+buffer, INOTIFY_BUF_LEN);
+ while (i < rdlen) {
+ char *e;
+ unsigned int qidx;
+ struct inotify_event *event =
+ (struct inotify_event *)(void *)
+&buffer[i];
+
+
Re: [lng-odp] [PATCH CATERPILLAR v1] [RFC] odp: pktio: added tcpdump like functionality
Bill Fischofer(Bill-Fischofer-Linaro) replied on github web page:
platform/linux-generic/odp_packet_io.c
line 121
@@ -89,6 +96,274 @@ int odp_pktio_init_local(void)
return odp_pktio_ops_init_local(true);
}
+static int write_pcapng_hdr(pktio_entry_t *entry, int qidx)
+{
+ size_t len;
+ pcapng_section_hdr_block_t shb;
+ pcapng_interface_description_block_s idb;
+ int fd = entry->s.pcapng_info.pcapng_fd[qidx];
+
+ memset(&shb, 0, sizeof(shb));
+ memset(&idb, 0, sizeof(idb));
+
+ shb.block_type = PCAPNG_BLOCK_TYPE_SHB;
+ shb.block_total_length = sizeof(shb);
+ shb.block_total_length2 = sizeof(shb);
+ shb.magic = PCAPNG_ENDIAN_MAGIC;
+ shb.version_major = 0x1;
+ shb.version_minor = 0x0;
+ shb.section_len = -1;
+
+ len = write(fd, &shb, sizeof(shb));
+ fsync(fd);
+ /* fail to write shb/idb means the pcapng is unreadable */
+ if (!len) {
+ ODP_ERR("Failed to write pcapng section hdr\n");
+ close(fd);
+ return -1;
+ }
+
+ idb.block_type = PCAPNG_BLOCK_TYPE_IDB;
+ idb.block_total_length = sizeof(idb);
+ idb.block_total_length2 = sizeof(idb);
+ idb.linktype = 0x1; /* LINKTYPE_ETHERNET */
+ idb.snaplen = 0x0; /* unlimited */
+ len = write(fd, &idb, sizeof(idb));
+ if (!len) {
+ ODP_ERR("Failed to write pcapng interface description\n");
+ close(fd);
+ return -1;
+ }
+ fsync(fd);
+
+ entry->s.pcapng_info.state[qidx] = PCAPNG_WR_HDR;
+
+ return 0;
+}
+
+static int write_pcapng_pkts(pktio_entry_t *entry, int qidx,
+const odp_packet_t packets[], int num)
+{
+ int i = 0;
+ struct iovec packet_iov[3 * num];
+ pcapng_enhanced_packet_block_t epb;
+ int iovcnt = 0;
+ ssize_t len = 0;
+ int fd = entry->s.pcapng_info.pcapng_fd[qidx];
+
+ for (i = 0; i < num; i++) {
+ odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(packets[i]);
+ uint32_t pkt_len = _odp_packet_len(packets[i]);
+ char *buf = (char *)odp_packet_data(packets[i]);
+
+ epb.block_type = PCAPNG_BLOCK_TYPE_EPB;
+ epb.block_total_length = sizeof(epb) +
+ ROUNDUP_ALIGN(pkt_len, PCAP_DATA_ALIGN) +
+ PCAP_DATA_ALIGN;
+ epb.interface_idx = 0;
+ epb.timestamp_high = (uint32_t)(pkt_hdr->timestamp.u64 >> 32);
+ epb.timestamp_low = (uint32_t)(pkt_hdr->timestamp.u64);
+ epb.captured_len = pkt_len;
+ epb.packet_len = pkt_len;
+
+ /* epb */
+ packet_iov[iovcnt].iov_base = &epb;
+ packet_iov[iovcnt].iov_len = sizeof(epb);
+ iovcnt++;
+
+ /* data */
+ packet_iov[iovcnt].iov_base = buf;
+ packet_iov[iovcnt].iov_len =
+ ROUNDUP_ALIGN(pkt_len, PCAP_DATA_ALIGN);
+ iovcnt++;
+
+ /* trailing */
+ packet_iov[iovcnt].iov_base = &epb.block_total_length;
+ packet_iov[iovcnt].iov_len = sizeof(uint32_t);
+ iovcnt++;
+ }
+
+ /* we don't really care if we manage to write *all* data */
+ len = writev(fd, packet_iov, iovcnt);
+ if (!len)
+ ODP_ERR("Failed to write pcapng data\n");
+ fsync(fd);
+
+ return len;
+}
+
+static void pcapng_drain_fifo(int fd)
+{
+ char c;
+ ssize_t len;
+
+ do {
+ len = read(fd, &c, sizeof(c));
+ } while (len > 0 && len != -1);
Comment:
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_r165548254
updated_at 2018-02-02 03:09:47
Re: [lng-odp] [PATCH CATERPILLAR v1] [RFC] odp: pktio: added tcpdump like functionality
Bill Fischofer(Bill-Fischofer-Linaro) replied on github web page:
platform/linux-generic/odp_packet_io.c
line 137
@@ -89,6 +96,274 @@ int odp_pktio_init_local(void)
return odp_pktio_ops_init_local(true);
}
+static int write_pcapng_hdr(pktio_entry_t *entry, int qidx)
+{
+ size_t len;
+ pcapng_section_hdr_block_t shb;
+ pcapng_interface_description_block_s idb;
+ int fd = entry->s.pcapng_info.pcapng_fd[qidx];
+
+ memset(&shb, 0, sizeof(shb));
+ memset(&idb, 0, sizeof(idb));
+
+ shb.block_type = PCAPNG_BLOCK_TYPE_SHB;
+ shb.block_total_length = sizeof(shb);
+ shb.block_total_length2 = sizeof(shb);
+ shb.magic = PCAPNG_ENDIAN_MAGIC;
+ shb.version_major = 0x1;
+ shb.version_minor = 0x0;
+ shb.section_len = -1;
+
+ len = write(fd, &shb, sizeof(shb));
+ fsync(fd);
+ /* fail to write shb/idb means the pcapng is unreadable */
+ if (!len) {
+ ODP_ERR("Failed to write pcapng section hdr\n");
+ close(fd);
+ return -1;
+ }
+
+ idb.block_type = PCAPNG_BLOCK_TYPE_IDB;
+ idb.block_total_length = sizeof(idb);
+ idb.block_total_length2 = sizeof(idb);
+ idb.linktype = 0x1; /* LINKTYPE_ETHERNET */
+ idb.snaplen = 0x0; /* unlimited */
+ len = write(fd, &idb, sizeof(idb));
+ if (!len) {
+ ODP_ERR("Failed to write pcapng interface description\n");
+ close(fd);
+ return -1;
+ }
+ fsync(fd);
+
+ entry->s.pcapng_info.state[qidx] = PCAPNG_WR_HDR;
+
+ return 0;
+}
+
+static int write_pcapng_pkts(pktio_entry_t *entry, int qidx,
+const odp_packet_t packets[], int num)
+{
+ int i = 0;
+ struct iovec packet_iov[3 * num];
+ pcapng_enhanced_packet_block_t epb;
+ int iovcnt = 0;
+ ssize_t len = 0;
+ int fd = entry->s.pcapng_info.pcapng_fd[qidx];
+
+ for (i = 0; i < num; i++) {
+ odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(packets[i]);
+ uint32_t pkt_len = _odp_packet_len(packets[i]);
+ char *buf = (char *)odp_packet_data(packets[i]);
+
+ epb.block_type = PCAPNG_BLOCK_TYPE_EPB;
+ epb.block_total_length = sizeof(epb) +
+ ROUNDUP_ALIGN(pkt_len, PCAP_DATA_ALIGN) +
+ PCAP_DATA_ALIGN;
+ epb.interface_idx = 0;
+ epb.timestamp_high = (uint32_t)(pkt_hdr->timestamp.u64 >> 32);
+ epb.timestamp_low = (uint32_t)(pkt_hdr->timestamp.u64);
+ epb.captured_len = pkt_len;
+ epb.packet_len = pkt_len;
+
+ /* epb */
+ packet_iov[iovcnt].iov_base = &epb;
+ packet_iov[iovcnt].iov_len = sizeof(epb);
+ iovcnt++;
+
+ /* data */
+ packet_iov[iovcnt].iov_base = buf;
+ packet_iov[iovcnt].iov_len =
+ ROUNDUP_ALIGN(pkt_len, PCAP_DATA_ALIGN);
+ iovcnt++;
+
+ /* trailing */
+ packet_iov[iovcnt].iov_base = &epb.block_total_length;
+ packet_iov[iovcnt].iov_len = sizeof(uint32_t);
+ iovcnt++;
+ }
+
+ /* we don't really care if we manage to write *all* data */
+ len = writev(fd, packet_iov, iovcnt);
+ if (!len)
+ ODP_ERR("Failed to write pcapng data\n");
+ fsync(fd);
+
+ return len;
+}
+
+static void pcapng_drain_fifo(int fd)
+{
+ char c;
+ ssize_t len;
+
+ do {
+ len = read(fd, &c, sizeof(c));
+ } while (len > 0 && len != -1);
+ ODP_DBG("Drain pcap fifo %d\n", len);
+}
+
+static void *inotify_update(void *arg)
+{
+ pktio_entry_t *entry = (pktio_entry_t *)arg;
+ struct timeval time;
+ int ret;
+ ssize_t rdlen;
+ int i;
+ char buffer[INOTIFY_BUF_LEN];
+ unsigned int max_queue =
+ MAX(entry->s.num_in_queue, entry->s.num_out_queue);
+ fd_set rfds;
+
+ while (1) {
Comment:
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 suppl
Re: [lng-odp] [PATCH CATERPILLAR v1] [RFC] odp: pktio: added tcpdump like functionality
Bill Fischofer(Bill-Fischofer-Linaro) replied on github web page:
platform/linux-generic/odp_packet_io.c
line 76
@@ -89,6 +96,274 @@ int odp_pktio_init_local(void)
return odp_pktio_ops_init_local(true);
}
+static int write_pcapng_hdr(pktio_entry_t *entry, int qidx)
+{
+ size_t len;
+ pcapng_section_hdr_block_t shb;
+ pcapng_interface_description_block_s idb;
+ int fd = entry->s.pcapng_info.pcapng_fd[qidx];
+
+ memset(&shb, 0, sizeof(shb));
+ memset(&idb, 0, sizeof(idb));
+
+ shb.block_type = PCAPNG_BLOCK_TYPE_SHB;
+ shb.block_total_length = sizeof(shb);
+ shb.block_total_length2 = sizeof(shb);
+ shb.magic = PCAPNG_ENDIAN_MAGIC;
+ shb.version_major = 0x1;
+ shb.version_minor = 0x0;
+ shb.section_len = -1;
+
+ len = write(fd, &shb, sizeof(shb));
+ fsync(fd);
+ /* fail to write shb/idb means the pcapng is unreadable */
+ if (!len) {
+ ODP_ERR("Failed to write pcapng section hdr\n");
+ close(fd);
+ return -1;
+ }
+
+ idb.block_type = PCAPNG_BLOCK_TYPE_IDB;
+ idb.block_total_length = sizeof(idb);
+ idb.block_total_length2 = sizeof(idb);
+ idb.linktype = 0x1; /* LINKTYPE_ETHERNET */
+ idb.snaplen = 0x0; /* unlimited */
+ len = write(fd, &idb, sizeof(idb));
+ if (!len) {
+ ODP_ERR("Failed to write pcapng interface description\n");
+ close(fd);
+ return -1;
+ }
+ fsync(fd);
+
+ entry->s.pcapng_info.state[qidx] = PCAPNG_WR_HDR;
+
+ return 0;
+}
+
+static int write_pcapng_pkts(pktio_entry_t *entry, int qidx,
+const odp_packet_t packets[], int num)
+{
+ int i = 0;
+ struct iovec packet_iov[3 * num];
+ pcapng_enhanced_packet_block_t epb;
+ int iovcnt = 0;
+ ssize_t len = 0;
+ int fd = entry->s.pcapng_info.pcapng_fd[qidx];
+
+ for (i = 0; i < num; i++) {
+ odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(packets[i]);
+ uint32_t pkt_len = _odp_packet_len(packets[i]);
+ char *buf = (char *)odp_packet_data(packets[i]);
Comment:
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_r165547875
updated_at 2018-02-02 03:09:47
Re: [lng-odp] [PATCH CATERPILLAR v1] [RFC] odp: pktio: added tcpdump like functionality
Bill Fischofer(Bill-Fischofer-Linaro) replied on github web page: platform/linux-generic/include/odp_pcapng.h line 20 @@ -0,0 +1,56 @@ +/* Copyright (c) 2018, Linaro Limited + * All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +#define PCAPNG_BLOCK_TYPE_EPB 0x0006UL +#define PCAPNG_BLOCK_TYPE_SHB 0x0A0D0D0AUL +#define PCAPNG_BLOCK_TYPE_IDB 0x0001UL +#define PCAPNG_ENDIAN_MAGIC 0x1A2B3C4DUL +#define PCAP_DATA_ALIGN (4) Comment: 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_r165547037 updated_at 2018-02-02 03:09:47
Re: [lng-odp] [PATCH CATERPILLAR v1] [RFC] odp: pktio: added tcpdump like functionality
Ilias Apalodimas(apalos) replied on github web page:
platform/linux-generic/odp_packet_io.c
line 237
@@ -89,6 +96,274 @@ int odp_pktio_init_local(void)
return odp_pktio_ops_init_local(true);
}
+static int write_pcapng_hdr(pktio_entry_t *entry, int qidx)
+{
+ size_t len;
+ pcapng_section_hdr_block_t shb;
+ pcapng_interface_description_block_s idb;
+ int fd = entry->s.pcapng_info.pcapng_fd[qidx];
+
+ memset(&shb, 0, sizeof(shb));
+ memset(&idb, 0, sizeof(idb));
+
+ shb.block_type = PCAPNG_BLOCK_TYPE_SHB;
+ shb.block_total_length = sizeof(shb);
+ shb.block_total_length2 = sizeof(shb);
+ shb.magic = PCAPNG_ENDIAN_MAGIC;
+ shb.version_major = 0x1;
+ shb.version_minor = 0x0;
+ shb.section_len = -1;
+
+ len = write(fd, &shb, sizeof(shb));
+ fsync(fd);
+ /* fail to write shb/idb means the pcapng is unreadable */
+ if (!len) {
+ ODP_ERR("Failed to write pcapng section hdr\n");
+ close(fd);
+ return -1;
+ }
+
+ idb.block_type = PCAPNG_BLOCK_TYPE_IDB;
+ idb.block_total_length = sizeof(idb);
+ idb.block_total_length2 = sizeof(idb);
+ idb.linktype = 0x1; /* LINKTYPE_ETHERNET */
+ idb.snaplen = 0x0; /* unlimited */
+ len = write(fd, &idb, sizeof(idb));
+ if (!len) {
+ ODP_ERR("Failed to write pcapng interface description\n");
+ close(fd);
+ return -1;
+ }
+ fsync(fd);
+
+ entry->s.pcapng_info.state[qidx] = PCAPNG_WR_HDR;
+
+ return 0;
+}
+
+static int write_pcapng_pkts(pktio_entry_t *entry, int qidx,
+const odp_packet_t packets[], int num)
+{
+ int i = 0;
+ struct iovec packet_iov[3 * num];
+ pcapng_enhanced_packet_block_t epb;
+ int iovcnt = 0;
+ ssize_t len = 0;
+ int fd = entry->s.pcapng_info.pcapng_fd[qidx];
+
+ for (i = 0; i < num; i++) {
+ odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(packets[i]);
+ uint32_t pkt_len = _odp_packet_len(packets[i]);
+ char *buf = (char *)odp_packet_data(packets[i]);
+
+ epb.block_type = PCAPNG_BLOCK_TYPE_EPB;
+ epb.block_total_length = sizeof(epb) +
+ ROUNDUP_ALIGN(pkt_len, PCAP_DATA_ALIGN) +
+ PCAP_DATA_ALIGN;
+ epb.interface_idx = 0;
+ epb.timestamp_high = (uint32_t)(pkt_hdr->timestamp.u64 >> 32);
+ epb.timestamp_low = (uint32_t)(pkt_hdr->timestamp.u64);
+ epb.captured_len = pkt_len;
+ epb.packet_len = pkt_len;
+
+ /* epb */
+ packet_iov[iovcnt].iov_base = &epb;
+ packet_iov[iovcnt].iov_len = sizeof(epb);
+ iovcnt++;
+
+ /* data */
+ packet_iov[iovcnt].iov_base = buf;
+ packet_iov[iovcnt].iov_len =
+ ROUNDUP_ALIGN(pkt_len, PCAP_DATA_ALIGN);
+ iovcnt++;
+
+ /* trailing */
+ packet_iov[iovcnt].iov_base = &epb.block_total_length;
+ packet_iov[iovcnt].iov_len = sizeof(uint32_t);
+ iovcnt++;
+ }
+
+ /* we don't really care if we manage to write *all* data */
+ len = writev(fd, packet_iov, iovcnt);
+ if (!len)
+ ODP_ERR("Failed to write pcapng data\n");
+ fsync(fd);
+
+ return len;
+}
+
+static void pcapng_drain_fifo(int fd)
+{
+ char c;
+ ssize_t len;
+
+ do {
+ len = read(fd, &c, sizeof(c));
+ } while (len > 0 && len != -1);
+ ODP_DBG("Drain pcap fifo %d\n", len);
+}
+
+static void *inotify_update(void *arg)
+{
+ pktio_entry_t *entry = (pktio_entry_t *)arg;
+ struct timeval time;
+ int ret;
+ ssize_t rdlen;
+ int i;
+ char buffer[INOTIFY_BUF_LEN];
+ unsigned int max_queue =
+ MAX(entry->s.num_in_queue, entry->s.num_out_queue);
+ fd_set rfds;
+
+ while (1) {
+ i = 0;
+ FD_ZERO(&rfds);
+ FD_SET(entry->s.pcapng_info.inotify_pcapng_fd, &rfds);
+ time.tv_sec = 2;
+ time.tv_usec = 0;
+ select(entry->s.pcapng_info.inotify_pcapng_fd + 1, &rfds, NULL,
+ NULL, &time);
+ if (FD_ISSET(entry->s.pcapng_info.inotify_pcapng_fd, &rfds)) {
+ rdlen = read(entry->s.pcapng_info.inotify_pcapng_fd,
+buffer, INOTIFY_BUF_LEN);
+ while (i < rdlen) {
+ char *e;
+ unsigned int qidx;
+ struct inotify_event *event =
+ (struct inotify_event *)(void *)
+&buffer[i];
+
+ e
Re: [lng-odp] [PATCH CATERPILLAR v1] [RFC] odp: pktio: added tcpdump like functionality
muvarov replied on github web page:
platform/linux-generic/odp_packet_io.c
line 237
@@ -89,6 +96,274 @@ int odp_pktio_init_local(void)
return odp_pktio_ops_init_local(true);
}
+static int write_pcapng_hdr(pktio_entry_t *entry, int qidx)
+{
+ size_t len;
+ pcapng_section_hdr_block_t shb;
+ pcapng_interface_description_block_s idb;
+ int fd = entry->s.pcapng_info.pcapng_fd[qidx];
+
+ memset(&shb, 0, sizeof(shb));
+ memset(&idb, 0, sizeof(idb));
+
+ shb.block_type = PCAPNG_BLOCK_TYPE_SHB;
+ shb.block_total_length = sizeof(shb);
+ shb.block_total_length2 = sizeof(shb);
+ shb.magic = PCAPNG_ENDIAN_MAGIC;
+ shb.version_major = 0x1;
+ shb.version_minor = 0x0;
+ shb.section_len = -1;
+
+ len = write(fd, &shb, sizeof(shb));
+ fsync(fd);
+ /* fail to write shb/idb means the pcapng is unreadable */
+ if (!len) {
+ ODP_ERR("Failed to write pcapng section hdr\n");
+ close(fd);
+ return -1;
+ }
+
+ idb.block_type = PCAPNG_BLOCK_TYPE_IDB;
+ idb.block_total_length = sizeof(idb);
+ idb.block_total_length2 = sizeof(idb);
+ idb.linktype = 0x1; /* LINKTYPE_ETHERNET */
+ idb.snaplen = 0x0; /* unlimited */
+ len = write(fd, &idb, sizeof(idb));
+ if (!len) {
+ ODP_ERR("Failed to write pcapng interface description\n");
+ close(fd);
+ return -1;
+ }
+ fsync(fd);
+
+ entry->s.pcapng_info.state[qidx] = PCAPNG_WR_HDR;
+
+ return 0;
+}
+
+static int write_pcapng_pkts(pktio_entry_t *entry, int qidx,
+const odp_packet_t packets[], int num)
+{
+ int i = 0;
+ struct iovec packet_iov[3 * num];
+ pcapng_enhanced_packet_block_t epb;
+ int iovcnt = 0;
+ ssize_t len = 0;
+ int fd = entry->s.pcapng_info.pcapng_fd[qidx];
+
+ for (i = 0; i < num; i++) {
+ odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(packets[i]);
+ uint32_t pkt_len = _odp_packet_len(packets[i]);
+ char *buf = (char *)odp_packet_data(packets[i]);
+
+ epb.block_type = PCAPNG_BLOCK_TYPE_EPB;
+ epb.block_total_length = sizeof(epb) +
+ ROUNDUP_ALIGN(pkt_len, PCAP_DATA_ALIGN) +
+ PCAP_DATA_ALIGN;
+ epb.interface_idx = 0;
+ epb.timestamp_high = (uint32_t)(pkt_hdr->timestamp.u64 >> 32);
+ epb.timestamp_low = (uint32_t)(pkt_hdr->timestamp.u64);
+ epb.captured_len = pkt_len;
+ epb.packet_len = pkt_len;
+
+ /* epb */
+ packet_iov[iovcnt].iov_base = &epb;
+ packet_iov[iovcnt].iov_len = sizeof(epb);
+ iovcnt++;
+
+ /* data */
+ packet_iov[iovcnt].iov_base = buf;
+ packet_iov[iovcnt].iov_len =
+ ROUNDUP_ALIGN(pkt_len, PCAP_DATA_ALIGN);
+ iovcnt++;
+
+ /* trailing */
+ packet_iov[iovcnt].iov_base = &epb.block_total_length;
+ packet_iov[iovcnt].iov_len = sizeof(uint32_t);
+ iovcnt++;
+ }
+
+ /* we don't really care if we manage to write *all* data */
+ len = writev(fd, packet_iov, iovcnt);
+ if (!len)
+ ODP_ERR("Failed to write pcapng data\n");
+ fsync(fd);
+
+ return len;
+}
+
+static void pcapng_drain_fifo(int fd)
+{
+ char c;
+ ssize_t len;
+
+ do {
+ len = read(fd, &c, sizeof(c));
+ } while (len > 0 && len != -1);
+ ODP_DBG("Drain pcap fifo %d\n", len);
+}
+
+static void *inotify_update(void *arg)
+{
+ pktio_entry_t *entry = (pktio_entry_t *)arg;
+ struct timeval time;
+ int ret;
+ ssize_t rdlen;
+ int i;
+ char buffer[INOTIFY_BUF_LEN];
+ unsigned int max_queue =
+ MAX(entry->s.num_in_queue, entry->s.num_out_queue);
+ fd_set rfds;
+
+ while (1) {
+ i = 0;
+ FD_ZERO(&rfds);
+ FD_SET(entry->s.pcapng_info.inotify_pcapng_fd, &rfds);
+ time.tv_sec = 2;
+ time.tv_usec = 0;
+ select(entry->s.pcapng_info.inotify_pcapng_fd + 1, &rfds, NULL,
+ NULL, &time);
+ if (FD_ISSET(entry->s.pcapng_info.inotify_pcapng_fd, &rfds)) {
+ rdlen = read(entry->s.pcapng_info.inotify_pcapng_fd,
+buffer, INOTIFY_BUF_LEN);
+ while (i < rdlen) {
+ char *e;
+ unsigned int qidx;
+ struct inotify_event *event =
+ (struct inotify_event *)(void *)
+&buffer[i];
+
+ e = strrchr(event->
