pespin submitted this change.

View Change

Approvals: laforge: Looks good to me, but someone else must approve fixeria: Looks good to me, approved Jenkins Builder: Verified
pcap-client: Decouple dev capture handling from client

A new struct osmo_pcap_handle is introduced, and struct osmo_pcap_client
manages a list of it.
This is a first step towards supporting capturing from multiple
interfaces. VTY still enforces only 1 configured interface since further
work is required.

Related: SYS#5822
Change-Id: I02f33f11cbcb7ed5ff6475df9442741618178379
---
M include/osmo-pcap/osmo_pcap_client.h
M src/osmo_client_core.c
M src/osmo_client_main.c
M src/osmo_client_network.c
M src/osmo_client_vty.c
5 files changed, 192 insertions(+), 104 deletions(-)

diff --git a/include/osmo-pcap/osmo_pcap_client.h b/include/osmo-pcap/osmo_pcap_client.h
index ce2fb8f..6725201 100644
--- a/include/osmo-pcap/osmo_pcap_client.h
+++ b/include/osmo-pcap/osmo_pcap_client.h
@@ -82,22 +82,26 @@
struct osmo_pcap_client *client;
};

-struct osmo_pcap_client {
- char *device;
+struct osmo_pcap_handle {
+ struct llist_head entry; /* item in (struct osmo_pcap_client)->handles */
+ struct osmo_pcap_client *client; /* back pointer */
+ char *devname;
pcap_t *handle;
- char errbuf[PCAP_ERRBUF_SIZE];
-
+ struct osmo_fd fd;
u_int last_ps_recv;
u_int last_ps_drop;
u_int last_ps_ifdrop;
struct osmo_timer_list pcap_stat_timer;
-
struct bpf_program bpf;
- char *filter_string;
+};
+
+struct osmo_pcap_client {
+ struct llist_head handles; /* list of struct osmo_pcap_handle */
+
+ char *filter_string;
int filter_itself;
int gprs_filtering;
int snaplen;
- struct osmo_fd fd;

struct osmo_pcap_client_conn *conn;
struct llist_head conns;
@@ -111,12 +115,14 @@
struct osmo_pcap_client *osmo_pcap_client_alloc(void *tall_ctx);
int vty_client_init(void);

-int osmo_client_capture(struct osmo_pcap_client *client, const char *device);
+int osmo_client_start_capture(struct osmo_pcap_client *client);
int osmo_client_filter(struct osmo_pcap_client *client, const char *filter);

struct osmo_pcap_client_conn *osmo_client_find_or_create_conn(struct osmo_pcap_client *, const char *name);
struct osmo_pcap_client_conn *osmo_client_find_conn(struct osmo_pcap_client *, const char *name);

+struct osmo_pcap_handle *osmo_client_find_handle(struct osmo_pcap_client *client, const char *devname);
+
struct osmo_pcap_client_conn *osmo_client_conn_alloc(struct osmo_pcap_client *client, const char *name);
void osmo_client_conn_free(struct osmo_pcap_client_conn *conn);
void osmo_client_conn_send_data(struct osmo_pcap_client_conn *conn,
@@ -125,3 +131,8 @@
void osmo_client_conn_connect(struct osmo_pcap_client_conn *conn);
void osmo_client_conn_disconnect(struct osmo_pcap_client_conn *conn);
void osmo_client_conn_reconnect(struct osmo_pcap_client_conn *conn);
+
+
+struct osmo_pcap_handle *osmo_pcap_handle_alloc(struct osmo_pcap_client *client, const char *devname);
+void osmo_pcap_handle_free(struct osmo_pcap_handle *ph);
+int osmo_pcap_handle_start_capture(struct osmo_pcap_handle *ph);
diff --git a/src/osmo_client_core.c b/src/osmo_client_core.c
index 41f4dfc..009362c 100644
--- a/src/osmo_client_core.c
+++ b/src/osmo_client_core.c
@@ -102,6 +102,7 @@

static int can_forward_packet(
struct osmo_pcap_client *client,
+ struct osmo_pcap_handle *ph,
struct pcap_pkthdr *hdr,
const u_char *data)
{
@@ -116,7 +117,7 @@
if (!client->gprs_filtering)
return 1;

- ll_type = pcap_datalink(client->handle);
+ ll_type = pcap_datalink(ph->handle);
switch (ll_type) {
case DLT_EN10MB:
offset = 14;
@@ -154,18 +155,19 @@

static int pcap_read_cb(struct osmo_fd *fd, unsigned int what)
{
- struct osmo_pcap_client *client = fd->data;
+ struct osmo_pcap_handle *ph = fd->data;
+ struct osmo_pcap_client *client = ph->client;
struct osmo_pcap_client_conn *conn;
struct pcap_pkthdr hdr;
const u_char *data;

- data = pcap_next(client->handle, &hdr);
+ data = pcap_next(ph->handle, &hdr);
if (!data) {
rate_ctr_inc(rate_ctr_group_get_ctr(client->ctrg, CLIENT_CTR_PERR));
return -1;
}

- if (!can_forward_packet(client, &hdr, data))
+ if (!can_forward_packet(client, ph, &hdr, data))
return 0;

llist_for_each_entry(conn, &client->conns, entry)
@@ -207,138 +209,90 @@
*old_val = new_val;
}

-static void pcap_check_stats_cb(void *_client)
+static void pcap_check_stats_cb(void *_ph)
{
struct pcap_stat stat;
- struct osmo_pcap_client *client = _client;
+ struct osmo_pcap_handle *ph = _ph;
+ struct osmo_pcap_client *client = ph->client;
int rc;

/* reschedule */
- osmo_timer_schedule(&client->pcap_stat_timer, 10, 0);
+ osmo_timer_schedule(&ph->pcap_stat_timer, 10, 0);

memset(&stat, 0, sizeof(stat));
- rc = pcap_stats(client->handle, &stat);
+ rc = pcap_stats(ph->handle, &stat);
if (rc != 0) {
LOGP(DCLIENT, LOGL_ERROR, "Failed to query pcap stats: %s\n",
- pcap_geterr(client->handle));
+ pcap_geterr(ph->handle));
rate_ctr_inc(rate_ctr_group_get_ctr(client->ctrg, CLIENT_CTR_PERR));
return;
}

- add_psbl_wrapped_ctr(client, &client->last_ps_recv, stat.ps_recv, CLIENT_CTR_P_RECV);
- add_psbl_wrapped_ctr(client, &client->last_ps_drop, stat.ps_drop, CLIENT_CTR_P_DROP);
- add_psbl_wrapped_ctr(client, &client->last_ps_ifdrop, stat.ps_ifdrop, CLIENT_CTR_P_IFDROP);
+ add_psbl_wrapped_ctr(client, &ph->last_ps_recv, stat.ps_recv, CLIENT_CTR_P_RECV);
+ add_psbl_wrapped_ctr(client, &ph->last_ps_drop, stat.ps_drop, CLIENT_CTR_P_DROP);
+ add_psbl_wrapped_ctr(client, &ph->last_ps_ifdrop, stat.ps_ifdrop, CLIENT_CTR_P_IFDROP);
}

-static int osmo_install_filter(struct osmo_pcap_client *client)
+static int osmo_pcap_handle_install_filter(struct osmo_pcap_handle *ph)
{
int rc;
- pcap_freecode(&client->bpf);
+ pcap_freecode(&ph->bpf);

- if (!client->handle) {
+ if (!ph->handle) {
LOGP(DCLIENT, LOGL_NOTICE,
"Filter will only be applied later.\n");
- return 1;
+ return 0;
}

- rc = pcap_compile(client->handle, &client->bpf,
- client->filter_string, 1, PCAP_NETMASK_UNKNOWN);
+ rc = pcap_compile(ph->handle, &ph->bpf,
+ ph->client->filter_string, 1, PCAP_NETMASK_UNKNOWN);
if (rc != 0) {
LOGP(DCLIENT, LOGL_ERROR,
"Failed to compile the filter: %s\n",
- pcap_geterr(client->handle));
+ pcap_geterr(ph->handle));
return rc;
}

- rc = pcap_setfilter(client->handle, &client->bpf);
+ rc = pcap_setfilter(ph->handle, &ph->bpf);
if (rc != 0) {
LOGP(DCLIENT, LOGL_ERROR,
"Failed to set the filter on the interface: %s\n",
- pcap_geterr(client->handle));
- pcap_freecode(&client->bpf);
+ pcap_geterr(ph->handle));
+ pcap_freecode(&ph->bpf);
return rc;
}

return rc;
}

-static void free_all(struct osmo_pcap_client *client)
+int osmo_client_start_capture(struct osmo_pcap_client *client)
{
- if (!client->handle)
- return;
-
- pcap_freecode(&client->bpf);
-
- if (client->fd.fd >= 0) {
- osmo_fd_unregister(&client->fd);
- client->fd.fd = -1;
- }
-
- pcap_close(client->handle);
- osmo_timer_del(&client->pcap_stat_timer);
- client->handle = NULL;
-}
-
-int osmo_client_capture(struct osmo_pcap_client *client, const char *device)
-{
+ struct osmo_pcap_handle *ph;
struct osmo_pcap_client_conn *conn;
- int fd;
+ int rc;

- talloc_free(client->device);
- free_all(client);
-
- client->device = talloc_strdup(client, device);
- if (!client->device) {
- LOGP(DCLIENT, LOGL_ERROR, "Failed to copy string.\n");
- return 1;
+ llist_for_each_entry(ph, &client->handles, entry) {
+ rc = osmo_pcap_handle_start_capture(ph);
+ if (rc < 0)
+ return rc;
}

- LOGP(DCLIENT, LOGL_INFO, "Opening device %s for capture with snaplen %zu\n",
- client->device, (size_t) client->snaplen);
- client->handle = pcap_open_live(client->device, client->snaplen, 0,
- 1000, client->errbuf);
- if (!client->handle) {
- LOGP(DCLIENT, LOGL_ERROR,
- "Failed to open the device: %s\n", client->errbuf);
- return 2;
- }
-
- fd = pcap_fileno(client->handle);
- if (fd == -1) {
- LOGP(DCLIENT, LOGL_ERROR,
- "No file descriptor provided.\n");
- free_all(client);
- return 3;
- }
-
- osmo_fd_setup(&client->fd, fd, OSMO_FD_READ, pcap_read_cb, client, 0);
- if (osmo_fd_register(&client->fd) != 0) {
- LOGP(DCLIENT, LOGL_ERROR,
- "Failed to register the fd.\n");
- client->fd.fd = -1;
- free_all(client);
- return 4;
- }
-
- client->pcap_stat_timer.data = client;
- client->pcap_stat_timer.cb = pcap_check_stats_cb;
- pcap_check_stats_cb(client);
-
llist_for_each_entry(conn, &client->conns, entry)
osmo_client_conn_send_link(conn);
-
- if (client->filter_string) {
- osmo_install_filter(client);
- }
-
return 0;
}

int osmo_client_filter(struct osmo_pcap_client *client, const char *filter)
{
+ struct osmo_pcap_handle *ph;
+ int rc = 0;
talloc_free(client->filter_string);
client->filter_string = talloc_strdup(client, filter);
- return osmo_install_filter(client);
+
+ llist_for_each_entry(ph, &client->handles, entry)
+ rc |= osmo_pcap_handle_install_filter(ph);
+
+ return rc;
}

struct osmo_pcap_client *osmo_pcap_client_alloc(void *tall_ctx)
@@ -348,8 +302,8 @@
if (!client)
return NULL;

- client->fd.fd = -1;
client->snaplen = DEFAULT_SNAPLEN;
+ INIT_LLIST_HEAD(&client->handles);
INIT_LLIST_HEAD(&client->conns);

return client;
@@ -407,3 +361,89 @@
conn = osmo_client_conn_alloc(client, name);
return conn;
}
+
+struct osmo_pcap_handle *osmo_client_find_handle(struct osmo_pcap_client *client, const char *devname)
+{
+ struct osmo_pcap_handle *ph;
+
+ llist_for_each_entry(ph, &client->handles, entry)
+ if (strcmp(ph->devname, devname) == 0)
+ return ph;
+ return NULL;
+}
+
+struct osmo_pcap_handle *osmo_pcap_handle_alloc(struct osmo_pcap_client *client, const char *devname)
+{
+ struct osmo_pcap_handle *ph;
+
+ ph = talloc_zero(client, struct osmo_pcap_handle);
+ OSMO_ASSERT(ph);
+
+ ph->devname = talloc_strdup(ph, devname);
+ OSMO_ASSERT(ph->devname);
+
+ ph->client = client;
+ ph->fd.fd = -1;
+
+ llist_add_tail(&ph->entry, &client->handles);
+ return ph;
+}
+
+void osmo_pcap_handle_free(struct osmo_pcap_handle *ph)
+{
+ if (!ph)
+ return;
+ llist_del(&ph->entry);
+
+ osmo_timer_del(&ph->pcap_stat_timer);
+
+ pcap_freecode(&ph->bpf);
+
+ if (ph->fd.fd >= 0) {
+ osmo_fd_unregister(&ph->fd);
+ ph->fd.fd = -1;
+ }
+
+ if (ph->handle) {
+ pcap_close(ph->handle);
+ ph->handle = NULL;
+ }
+
+ talloc_free(ph);
+}
+
+int osmo_pcap_handle_start_capture(struct osmo_pcap_handle *ph)
+{
+ struct osmo_pcap_client *client = ph->client;
+ int fd;
+ char errbuf[PCAP_ERRBUF_SIZE];
+
+ LOGP(DCLIENT, LOGL_INFO, "Opening device %s for capture with snaplen %zu\n",
+ ph->devname, (size_t) client->snaplen);
+ ph->handle = pcap_open_live(ph->devname, client->snaplen, 0, 1000, errbuf);
+ if (!ph->handle) {
+ LOGP(DCLIENT, LOGL_ERROR,
+ "Failed to open the device: %s\n", errbuf);
+ return -2;
+ }
+
+ fd = pcap_fileno(ph->handle);
+ if (fd == -1) {
+ LOGP(DCLIENT, LOGL_ERROR, "No file descriptor provided.\n");
+ return -3;
+ }
+
+ osmo_fd_setup(&ph->fd, fd, OSMO_FD_READ, pcap_read_cb, ph, 0);
+ if (osmo_fd_register(&ph->fd) != 0) {
+ LOGP(DCLIENT, LOGL_ERROR,
+ "Failed to register the fd.\n");
+ return -4;
+ }
+
+ osmo_timer_setup(&ph->pcap_stat_timer, pcap_check_stats_cb, ph);
+ pcap_check_stats_cb(ph);
+
+ if (client->filter_string)
+ osmo_pcap_handle_install_filter(ph);
+ return 0;
+}
diff --git a/src/osmo_client_main.c b/src/osmo_client_main.c
index c7af965..3fa0ae4 100644
--- a/src/osmo_client_main.c
+++ b/src/osmo_client_main.c
@@ -283,6 +283,11 @@
exit(1);
}

+ rc = osmo_client_start_capture(pcap_client);
+ if (rc < 0) {
+ LOGP(DCLIENT, LOGL_ERROR, "Failed to start capturing on interfaces\n");
+ exit(1);
+ }

/* attempt to connect to the remote */
if (pcap_client->conn->srv_ip && pcap_client->conn->srv_port > 0)
diff --git a/src/osmo_client_network.c b/src/osmo_client_network.c
index 52246d4..e27a91f 100644
--- a/src/osmo_client_network.c
+++ b/src/osmo_client_network.c
@@ -169,6 +169,7 @@
struct osmo_pcap_pkthdr *hdr;
struct msgb *msg;
int offset, ip_len;
+ struct osmo_pcap_handle *ph;

if (in_hdr->len > in_hdr->caplen) {
LOGP(DCLIENT, LOGL_ERROR,
@@ -204,7 +205,13 @@
rate_ctr_inc(rate_ctr_group_get_ctr(conn->client->ctrg, CLIENT_CTR_PKTS));
break;
case PROTOCOL_IPIP:
- offset = get_iphdr_offset(pcap_datalink(conn->client->handle));
+ /* TODO: support capturing from multiple interfaces here: */
+ ph = llist_first_entry_or_null(&conn->client->handles, struct osmo_pcap_handle, entry);
+ if (!ph) {
+ msgb_free(msg);
+ return;
+ }
+ offset = get_iphdr_offset(pcap_datalink(ph->handle));
if (offset < 0) {
msgb_free(msg);
return;
@@ -226,6 +233,7 @@

void osmo_client_conn_send_link(struct osmo_pcap_client_conn *conn)
{
+ struct osmo_pcap_handle *ph;
struct pcap_file_header *hdr;
struct osmo_pcap_data *om_hdr;
struct msgb *msg;
@@ -234,7 +242,9 @@
if (conn->protocol == PROTOCOL_IPIP)
return;

- if (!conn->client->handle) {
+ /* TODO: support capturing from multiple interfaces here: */
+ ph = llist_first_entry_or_null(&conn->client->handles, struct osmo_pcap_handle, entry);
+ if (!ph || !ph->handle) {
LOGP(DCLIENT, LOGL_ERROR,
"No pcap_handle not sending link info to conn=%s\n", conn->name);
return;
@@ -258,7 +268,7 @@
hdr->thiszone = 0;
hdr->sigfigs = 0;
hdr->snaplen = conn->client->snaplen;
- hdr->linktype = pcap_datalink(conn->client->handle);
+ hdr->linktype = pcap_datalink(ph->handle);

write_data(conn, msg);
}
diff --git a/src/osmo_client_vty.c b/src/osmo_client_vty.c
index 84f7e05..c976a1a 100644
--- a/src/osmo_client_vty.c
+++ b/src/osmo_client_vty.c
@@ -126,11 +126,14 @@

static int config_write_client(struct vty *vty)
{
+ struct osmo_pcap_handle *ph;
+
vty_out(vty, "client%s", VTY_NEWLINE);

- if (pcap_client->device)
+ llist_for_each_entry(ph, &pcap_client->handles, entry) {
vty_out(vty, " pcap device %s%s",
- pcap_client->device, VTY_NEWLINE);
+ ph->devname, VTY_NEWLINE);
+ }
if (pcap_client->snaplen != DEFAULT_SNAPLEN)
vty_out(vty, " pcap snaplen %d%s",
pcap_client->snaplen, VTY_NEWLINE);
@@ -147,12 +150,34 @@
return CMD_SUCCESS;
}

+DEFUN(cfg_client_no_device,
+ cfg_client_no_device_cmd,
+ "no pcap device NAME",
+ NO_STR PCAP_STRING "the device to filter\n" "device name\n")
+{
+ struct osmo_pcap_handle *ph = osmo_client_find_handle(pcap_client, argv[0]);
+ if (!ph) {
+ vty_out(vty, "%% Device %s not found!%s", argv[0], VTY_NEWLINE);
+ return CMD_WARNING;
+ }
+ osmo_pcap_handle_free(ph);
+ return CMD_SUCCESS;
+}
+
DEFUN(cfg_client_device,
cfg_client_device_cmd,
"pcap device NAME",
PCAP_STRING "the device to filter\n" "device name\n")
{
- osmo_client_capture(pcap_client, argv[0]);
+ struct osmo_pcap_handle *ph = osmo_client_find_handle(pcap_client, argv[0]);
+ if (!ph) {
+ /* Only allow max one for now:*/
+ if (llist_count(&pcap_client->handles) > 0) {
+ vty_out(vty, "Only one 'pcap device' allowed! Remove the old one with 'no pcap device' first!%s", VTY_NEWLINE);
+ return CMD_WARNING;
+ }
+ osmo_pcap_handle_alloc(pcap_client, argv[0]);
+ }
return CMD_SUCCESS;
}

@@ -161,10 +186,6 @@
"pcap snaplen <1-262144>", /* MAXIMUM_SNAPLEN */
PCAP_STRING "snapshot length\n" "Bytes\n")
{
- if (pcap_client->handle) {
- vty_out(vty, "'pcap snaplen' must be set before 'pcap device' to take effect!%s", VTY_NEWLINE);
- return CMD_WARNING;
- }
pcap_client->snaplen = atoi(argv[0]);
return CMD_SUCCESS;
}
@@ -544,6 +565,7 @@

install_node(&server_node, config_write_server);

+ install_element(CLIENT_NODE, &cfg_client_no_device_cmd);
install_element(CLIENT_NODE, &cfg_client_device_cmd);
install_element(CLIENT_NODE, &cfg_client_snaplen_cmd);
install_element(CLIENT_NODE, &cfg_client_filter_cmd);

To view, visit change 39170. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-MessageType: merged
Gerrit-Project: osmo-pcap
Gerrit-Branch: master
Gerrit-Change-Id: I02f33f11cbcb7ed5ff6475df9442741618178379
Gerrit-Change-Number: 39170
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin@sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy@sysmocom.de>
Gerrit-Reviewer: laforge <laforge@osmocom.org>
Gerrit-Reviewer: pespin <pespin@sysmocom.de>