You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2022/07/24 05:38:17 UTC
[GitHub] [skywalking-rover] mrproliu opened a new pull request, #40: Enhance the network profiling
mrproliu opened a new pull request, #40:
URL: https://github.com/apache/skywalking-rover/pull/40
* Optimized eBPF event handler
* Optimize the connection closing event handle process
* Fix traffic direction identification errors
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-rover] wu-sheng commented on a diff in pull request #40: Enhance the network profiling
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #40:
URL: https://github.com/apache/skywalking-rover/pull/40#discussion_r928201010
##########
bpf/profiling/network/netmonitor.c:
##########
@@ -285,14 +318,20 @@ static __always_inline void process_write_data(void *ctx, __u64 id, struct sock_
if (args->buf != NULL) {
buf_reader = read_socket_data(args->buf, bytes_count);
} else if (args->iovec != NULL) {
- struct iovec *iovec = _(args->iovec);
- char *buff = _(iovec->iov_base);
- buf_reader = read_socket_data(buff, bytes_count);
+ struct iovec iov;
+ int err = bpf_probe_read(&iov, sizeof(iov), args->iovec);
+ if (err >= 0) {
+ __u64 size = iov.iov_len;
+ if (size > bytes_count) {
+ size = bytes_count;
+ }
+// bpf_printk("iov read buffer size: %lld, connect event send status: %d\n", size, conn->connect_event_send);
Review Comment:
Please remove commented codes if it is pointless. Or log it out if needed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-rover] wu-sheng merged pull request #40: Enhance the network profiling
Posted by GitBox <gi...@apache.org>.
wu-sheng merged PR #40:
URL: https://github.com/apache/skywalking-rover/pull/40
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-rover] wu-sheng commented on a diff in pull request #40: Enhance the network profiling
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #40:
URL: https://github.com/apache/skywalking-rover/pull/40#discussion_r928200946
##########
bpf/profiling/network/netmonitor.c:
##########
@@ -127,63 +153,36 @@ static __always_inline void submit_new_connection(struct pt_regs* ctx, __u32 fun
// fill the connection
event->role = con.role;
event->socket_family = con.socket_family;
- __u16 port;
- if (socket != NULL) {
- event->need_complete_addr = 0;
- // only get from accept function(server side)
- struct sock* s;
- BPF_CORE_READ_INTO(&s, socket, sk);
-
- if (con.socket_family == AF_INET) {
- BPF_CORE_READ_INTO(&port, s, __sk_common.skc_num);
- event->local_port = port;
- BPF_CORE_READ_INTO(&event->local_addr_v4, s, __sk_common.skc_rcv_saddr);
- BPF_CORE_READ_INTO(&port, s, __sk_common.skc_dport);
- event->remote_port = bpf_ntohs(port);
- BPF_CORE_READ_INTO(&event->remote_addr_v4, s, __sk_common.skc_daddr);
- } else if (con.socket_family == AF_INET6) {
- BPF_CORE_READ_INTO(&port, s, __sk_common.skc_num);
- event->local_port = port;
- BPF_CORE_READ_INTO(&event->local_addr_v6, s, __sk_common.skc_v6_rcv_saddr.in6_u.u6_addr8);
- BPF_CORE_READ_INTO(&port, s, __sk_common.skc_dport);
- event->remote_port = bpf_ntohs(port);
- BPF_CORE_READ_INTO(&event->remote_addr_v6, s, __sk_common.skc_v6_daddr.in6_u.u6_addr8);
- } else {
- event->local_port = 0;
- event->remote_port = 0;
- }
- } else if (addr != NULL) {
- event->need_complete_addr = 1;
- if (con.socket_family == AF_INET) {
- struct sockaddr_in *daddr = (struct sockaddr_in *)addr;
- bpf_probe_read(&event->remote_addr_v4, sizeof(event->remote_addr_v4), &daddr->sin_addr.s_addr);
- bpf_probe_read(&port, sizeof(port), &daddr->sin_port);
- event->remote_port = bpf_ntohs(port);
- } else if (con.socket_family == AF_INET6) {
- struct sockaddr_in6 *daddr = (struct sockaddr_in6 *)addr;
- bpf_probe_read(&event->remote_addr_v6, sizeof(event->remote_addr_v6), &daddr->sin6_addr.s6_addr);
- bpf_probe_read(&port, sizeof(port), &daddr->sin6_port);
- event->remote_port = bpf_ntohs(port);
- } else {
- event->remote_port = 0;
+ event->need_complete_addr = need_complete_addr;
+ event->local_addr_v4 = con.local_addr_v4;
+ __builtin_memcpy(&event->local_addr_v6, &con.local_addr_v4, 16*sizeof(__u8));
+ event->local_port = con.local_port;
+ event->remote_addr_v4 = con.remote_addr_v4;
+ __builtin_memcpy(&event->remote_addr_v6, &con.remote_addr_v6, 16*sizeof(__u8));
+ event->remote_port = con.remote_port;
+
+ __u32 ret = bpf_perf_event_output(ctx, &socket_connection_event_queue, BPF_F_CURRENT_CPU, event, sizeof(*event));
+ // if not send event success, then update to the event not sent
+ if (ret < 0) {
+ struct active_connection_t *con = bpf_map_lookup_elem(&active_connection_map, &conid);
+ if (con != NULL) {
+ con->connect_event_send = false;
+ bpf_map_update_elem(&active_connection_map, &conid, con, 0);
}
- } else {
- event->need_complete_addr = 1;
- // clean the cache in LRU(remote port is enough)
- event->remote_port = 0;
}
-
- bpf_perf_event_output(ctx, &socket_connection_event_queue, BPF_F_CURRENT_CPU, event, sizeof(*event));
}
static __inline void notify_close_connection(struct pt_regs* ctx, __u64 conid, struct active_connection_t* con, __u64 start_time, __u64 end_time) {
- // only trace ipv4, v6, or unknown
- if (family_should_trace(con->socket_family) == false) {
- return;
- }
- // pid is contains
- if (tgid_should_trace(con->pid) == false) {
- return;
+ // if the connect event not send, then check the pid or socket family
+ if (con->connect_event_send == false) {
+ // only trace ipv4, v6, or unknown
+ if (family_should_trace(con->socket_family) == false) {
+ return;
+ }
+ // pid is contains
Review Comment:
This comment seems not clear, could you polish it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-rover] wu-sheng commented on a diff in pull request #40: Enhance the network profiling
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #40:
URL: https://github.com/apache/skywalking-rover/pull/40#discussion_r928200963
##########
bpf/profiling/network/netmonitor.c:
##########
@@ -273,9 +301,14 @@ static __always_inline void process_write_data(void *ctx, __u64 id, struct sock_
return;
}
- // pid is contains
- if (tgid_should_trace(tgid) == false) {
- return;
+ // if connection not sent
Review Comment:
```suggestion
// if connect event is not sent
```
I think you mean this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-rover] mrproliu commented on a diff in pull request #40: Enhance the network profiling
Posted by GitBox <gi...@apache.org>.
mrproliu commented on code in PR #40:
URL: https://github.com/apache/skywalking-rover/pull/40#discussion_r928219298
##########
go.mod:
##########
@@ -8,6 +8,7 @@ require (
github.com/hashicorp/go-multierror v1.1.1
github.com/hashicorp/golang-lru v0.5.4
github.com/ianlancetaylor/demangle v0.0.0-20200824232613-28f6c0f3b639
+ github.com/orcaman/concurrent-map v1.0.0
Review Comment:
Yes, this works to reduce lock contention during map operations in receiving the event from BPF. I will update the license before release.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-rover] wu-sheng commented on a diff in pull request #40: Enhance the network profiling
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #40:
URL: https://github.com/apache/skywalking-rover/pull/40#discussion_r928201054
##########
bpf/profiling/network/netmonitor.c:
##########
@@ -301,10 +340,12 @@ static __always_inline void process_write_data(void *ctx, __u64 id, struct sock_
// if send response data to remote address or receive request data from remote address
// then, recognized current connection is server
- } else if ((msg_type == kResponse && data_direction == SOCK_DATA_DIRECTION_INGRESS) ||
- (msg_type == kRequest && data_direction == SOCK_DATA_DIRECTION_EGRESS)) {
+ } else if ((msg_type == kResponse && data_direction == SOCK_DATA_DIRECTION_EGRESS) ||
+ (msg_type == kRequest && data_direction == SOCK_DATA_DIRECTION_INGRESS)) {
conn->role = CONNECTION_ROLE_TYPE_SERVER;
}
+// bpf_printk("connection: %lld, data_len: %d, original len: %d", conid, buf_reader->data_len, bytes_count);
+// bpf_printk("message type: %d, data_direction: %d, new role: %d", msg_type, data_direction, conn->role);
Review Comment:
Please remove commented codes if it is pointless. Or log it out if needed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-rover] wu-sheng commented on a diff in pull request #40: Enhance the network profiling
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #40:
URL: https://github.com/apache/skywalking-rover/pull/40#discussion_r928201076
##########
go.mod:
##########
@@ -8,6 +8,7 @@ require (
github.com/hashicorp/go-multierror v1.1.1
github.com/hashicorp/golang-lru v0.5.4
github.com/ianlancetaylor/demangle v0.0.0-20200824232613-28f6c0f3b639
+ github.com/orcaman/concurrent-map v1.0.0
Review Comment:
Is this a new dependency?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org