Attention is currently required from: daniel, laforge, osmith.
pespin has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-pcap/+/39265?usp=email )
Change subject: client: Support generation of pcapng file format
......................................................................
Patch Set 8:
(3 comments)
File doc/manuals/chapters/client.adoc:
https://gerrit.osmocom.org/c/osmo-pcap/+/39265/comment/01f3e45b_9d5c6dc3?us… :
PS8, Line 59: reate
> create
Done
File include/osmo-pcap/osmo_pcap_file.h:
https://gerrit.osmocom.org/c/osmo-pcap/+/39265/comment/0df37a3e_3053cca0?us… :
PS8, Line 87: /* uint32_t block_total_length */
> looks like a leftover, because block_total_length is already in line 85
Nope, it's fine, the field is duplicated at the end of block_body so that the file can be navigated backwards. See pcapng spec 😊
File src/osmo_client_network.c:
https://gerrit.osmocom.org/c/osmo-pcap/+/39265/comment/da2123ef_81082df2?us… :
PS8, Line 389: .
> (remove the dot?)
Done
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcap/+/39265?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: osmo-pcap
Gerrit-Branch: master
Gerrit-Change-Id: I3c80518a1e53a1f77e1aca8dfa83f683f9516ad6
Gerrit-Change-Number: 39265
Gerrit-PatchSet: 8
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 13 Jan 2025 16:14:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: osmith <osmith(a)sysmocom.de>
pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-pcap/+/39272?usp=email )
Change subject: server: Delay reopen of pcap only until necessary
......................................................................
server: Delay reopen of pcap only until necessary
If we are asked to reopen a pcap file (eg due to external SIGUSR1) while
in the middle of receiving a data packet (we already received the header
of the segment so data will arrive soon), code exists to delay reopening
so that we can include that last packet which arrived because the time
where we were asked to reopen.
However, the new pcap file was reopened only after next packet arrived.
Instead, we want to reopen it as soon as that last packet is received,
so that a new pcap file is created. This allows better tracking eg.
empty traffic during time in between last data packet before reopen and
the next data packet arriving.
While at it, rename the variable to make it more informative, and
convert it to a bool.
Change-Id: Id79500717a2e186aac979cded340a5af6ce3035b
---
M include/osmo-pcap/osmo_pcap_server.h
M src/osmo_server_network.c
2 files changed, 8 insertions(+), 7 deletions(-)
Approvals:
Jenkins Builder: Verified
osmith: Looks good to me, approved
laforge: Looks good to me, but someone else must approve
diff --git a/include/osmo-pcap/osmo_pcap_server.h b/include/osmo-pcap/osmo_pcap_server.h
index dad81d3..030a353 100644
--- a/include/osmo-pcap/osmo_pcap_server.h
+++ b/include/osmo-pcap/osmo_pcap_server.h
@@ -97,7 +97,7 @@
/* read buffering */
int state;
int pend;
- int reopen;
+ bool reopen_delayed;
struct osmo_pcap_data *data;
/* statistics */
diff --git a/src/osmo_server_network.c b/src/osmo_server_network.c
index 55c84cf..7d46b90 100644
--- a/src/osmo_server_network.c
+++ b/src/osmo_server_network.c
@@ -686,6 +686,12 @@
OSMO_ASSERT(0);
}
+ if (conn->reopen_delayed) {
+ LOGP(DSERVER, LOGL_INFO, "Reopening log for %s now.\n", conn->name);
+ restart_pcap(conn);
+ conn->reopen_delayed = false;
+ }
+
return rc;
}
@@ -693,11 +699,6 @@
static int dispatch_read(struct osmo_pcap_conn *conn)
{
if (conn->state == STATE_INITIAL) {
- if (conn->reopen) {
- LOGP(DSERVER, LOGL_INFO, "Reopening log for %s now.\n", conn->name);
- restart_pcap(conn);
- conn->reopen = 0;
- }
return read_cb_initial(conn);
} else if (conn->state == STATE_DATA) {
return read_cb_data(conn);
@@ -874,7 +875,7 @@
restart_pcap(conn);
} else {
LOGP(DSERVER, LOGL_INFO, "Delaying %s until current packet is complete.\n", conn->name);
- conn->reopen = 1;
+ conn->reopen_delayed = true;
}
}
}
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcap/+/39272?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: osmo-pcap
Gerrit-Branch: master
Gerrit-Change-Id: Id79500717a2e186aac979cded340a5af6ce3035b
Gerrit-Change-Number: 39272
Gerrit-PatchSet: 3
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-pcap/+/39273?usp=email )
Change subject: server: Store read buf max len as conn field
......................................................................
server: Store read buf max len as conn field
This will be useful later on when adding pcapng support, since checks
will become more complex due to different data types to be checked,
based on information from pcap vs pcapng gathered from magic field
inside the received link_hdr buffer.
Change-Id: I8f62aa0bdb04e73223c0c6803a58241f83a5ebe9
---
M include/osmo-pcap/osmo_pcap_server.h
M src/osmo_server_network.c
2 files changed, 4 insertions(+), 5 deletions(-)
Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, but someone else must approve
osmith: Looks good to me, approved
diff --git a/include/osmo-pcap/osmo_pcap_server.h b/include/osmo-pcap/osmo_pcap_server.h
index 030a353..ae578b4 100644
--- a/include/osmo-pcap/osmo_pcap_server.h
+++ b/include/osmo-pcap/osmo_pcap_server.h
@@ -99,6 +99,7 @@
int pend;
bool reopen_delayed;
struct osmo_pcap_data *data;
+ size_t data_max_len; /* size of allocated buffer in data->data. */
/* statistics */
struct rate_ctr_group *ctrg;
diff --git a/src/osmo_server_network.c b/src/osmo_server_network.c
index 7d46b90..044e0ec 100644
--- a/src/osmo_server_network.c
+++ b/src/osmo_server_network.c
@@ -504,7 +504,6 @@
{
struct rate_ctr_group_desc *desc;
struct osmo_pcap_conn *conn;
- size_t buf_size;
llist_for_each_entry(conn, &server->conn, entry) {
if (strcmp(conn->name, name) == 0)
@@ -518,10 +517,9 @@
return NULL;
}
- buf_size = sizeof(struct osmo_pcap_data);
- buf_size += OSMO_MAX(sizeof(struct pcap_file_header),
- sizeof(struct osmo_pcap_pkthdr) + server->max_snaplen);
- conn->data = talloc_zero_size(conn, buf_size);
+ conn->data_max_len = OSMO_MAX(sizeof(struct pcap_file_header),
+ sizeof(struct osmo_pcap_pkthdr) + server->max_snaplen);
+ conn->data = talloc_zero_size(conn, sizeof(struct osmo_pcap_data) + conn->data_max_len);
/* a bit nasty. we do not work with ids but names */
desc = talloc_zero(conn, struct rate_ctr_group_desc);
if (!desc) {
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcap/+/39273?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: osmo-pcap
Gerrit-Branch: master
Gerrit-Change-Id: I8f62aa0bdb04e73223c0c6803a58241f83a5ebe9
Gerrit-Change-Number: 39273
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-pcap/+/39283?usp=email )
Change subject: server: Rename func client_data() -> zmq_send_client_data()
......................................................................
server: Rename func client_data() -> zmq_send_client_data()
Change-Id: I1b22046464cf969fb5d74b6c2580aaec1feffefa
---
M src/osmo_server_network.c
1 file changed, 4 insertions(+), 4 deletions(-)
Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, approved
diff --git a/src/osmo_server_network.c b/src/osmo_server_network.c
index f05413e..64d772a 100644
--- a/src/osmo_server_network.c
+++ b/src/osmo_server_network.c
@@ -89,8 +89,8 @@
pcap_zmq_send(conn->server->zmq_publ, data, strlen(data), 0);
}
-static void client_data(struct osmo_pcap_conn *conn,
- struct osmo_pcap_data *data)
+static void zmq_send_client_data(struct osmo_pcap_conn *conn,
+ struct osmo_pcap_data *data)
{
char *event_name;
@@ -101,7 +101,7 @@
* This multi-part support is insane... so if we lose the first
* or the last part of the multipart message stuff is going out
* of sync. *great* As we can't do anything about it right now
- * just close the eyese and send it.
+ * just close the eyes and send it.
*/
event_name = talloc_asprintf(conn, "data.v1.%s", conn->name);
pcap_zmq_send(conn->server->zmq_publ, event_name, strlen(event_name), ZMQ_SNDMORE);
@@ -466,7 +466,7 @@
time_t now = time(NULL);
int rc;
- client_data(conn, data);
+ zmq_send_client_data(conn, data);
if (!conn->store) {
update_last_write(conn, now);
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcap/+/39283?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: osmo-pcap
Gerrit-Branch: master
Gerrit-Change-Id: I1b22046464cf969fb5d74b6c2580aaec1feffefa
Gerrit-Change-Number: 39283
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>