Skip to content

Commit 64f8171

Browse files
committed
Use correct byte ordering function for kernel struct member (skc_num)
Signed-off-by: Dom Del Nano <ddelnano@gmail.com>
1 parent 9ae53a8 commit 64f8171

File tree

2 files changed

+11
-6
lines changed

2 files changed

+11
-6
lines changed

src/stirling/source_connectors/tcp_stats/bcc_bpf/tcp_stats.c

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -79,12 +79,12 @@ static int tcp_sendstat(struct pt_regs* ctx, uint32_t tgid, uint32_t id, int siz
7979
event.upid.start_time_ticks = get_tgid_start_time();
8080

8181
if (family == AF_INET) {
82-
event.local_addr.in4.sin_port = ntohs(lport);
82+
event.local_addr.in4.sin_port = htons(lport);
8383
event.remote_addr.in4.sin_port = rport;
8484
BPF_PROBE_READ_KERNEL_VAR(event.local_addr.in4.sin_addr.s_addr, &sk_common->skc_rcv_saddr);
8585
BPF_PROBE_READ_KERNEL_VAR(event.remote_addr.in4.sin_addr.s_addr, &sk_common->skc_daddr);
8686
} else if (family == AF_INET6) {
87-
event.local_addr.in6.sin6_port = ntohs(lport);
87+
event.local_addr.in6.sin6_port = htons(lport);
8888
event.remote_addr.in6.sin6_port = rport;
8989
BPF_PROBE_READ_KERNEL_VAR(event.local_addr.in6.sin6_addr, &sk_common->skc_v6_rcv_saddr);
9090
BPF_PROBE_READ_KERNEL_VAR(event.remote_addr.in6.sin6_addr, &sk_common->skc_v6_daddr);
@@ -158,12 +158,12 @@ int probe_entry_tcp_cleanup_rbuf(struct pt_regs* ctx, struct sock* sk, int copie
158158
event.local_addr.sa.sa_family = family;
159159

160160
if (family == AF_INET) {
161-
event.local_addr.in4.sin_port = ntohs(lport);
161+
event.local_addr.in4.sin_port = htons(lport);
162162
event.remote_addr.in4.sin_port = rport;
163163
BPF_PROBE_READ_KERNEL_VAR(event.local_addr.in4.sin_addr.s_addr, &sk->__sk_common.skc_rcv_saddr);
164164
BPF_PROBE_READ_KERNEL_VAR(event.remote_addr.in4.sin_addr.s_addr, &sk->__sk_common.skc_daddr);
165165
} else if (family == AF_INET6) {
166-
event.local_addr.in6.sin6_port = ntohs(lport);
166+
event.local_addr.in6.sin6_port = htons(lport);
167167
event.remote_addr.in6.sin6_port = rport;
168168
BPF_PROBE_READ_KERNEL_VAR(event.local_addr.in6.sin6_addr, &sk->__sk_common.skc_v6_rcv_saddr);
169169
BPF_PROBE_READ_KERNEL_VAR(event.remote_addr.in6.sin6_addr, &sk->__sk_common.skc_v6_daddr);
@@ -196,6 +196,8 @@ int probe_entry_tcp_retransmit_skb(struct pt_regs* ctx, struct sock* skp, struct
196196

197197
int lport = -1;
198198
int rport = -1;
199+
// skc_num is stored in host byte order. The rest of our user space processing
200+
// assumes network byte order so convert it here.
199201
BPF_PROBE_READ_KERNEL_VAR(lport, &skp->__sk_common.skc_num);
200202
BPF_PROBE_READ_KERNEL_VAR(rport, &skp->__sk_common.skc_dport);
201203

@@ -204,13 +206,13 @@ int probe_entry_tcp_retransmit_skb(struct pt_regs* ctx, struct sock* skp, struct
204206
event.local_addr.sa.sa_family = family;
205207

206208
if (family == AF_INET) {
207-
event.local_addr.in4.sin_port = ntohs(lport);
209+
event.local_addr.in4.sin_port = htons(lport);
208210
event.remote_addr.in4.sin_port = rport;
209211
BPF_PROBE_READ_KERNEL_VAR(event.local_addr.in4.sin_addr.s_addr,
210212
&skp->__sk_common.skc_rcv_saddr);
211213
BPF_PROBE_READ_KERNEL_VAR(event.remote_addr.in4.sin_addr.s_addr, &skp->__sk_common.skc_daddr);
212214
} else if (family == AF_INET6) {
213-
event.local_addr.in6.sin6_port = ntohs(lport);
215+
event.local_addr.in6.sin6_port = htons(lport);
214216
event.remote_addr.in6.sin6_port = rport;
215217
BPF_PROBE_READ_KERNEL_VAR(event.local_addr.in6.sin6_addr, &skp->__sk_common.skc_v6_rcv_saddr);
216218
BPF_PROBE_READ_KERNEL_VAR(event.remote_addr.in6.sin6_addr, &skp->__sk_common.skc_v6_daddr);

src/stirling/source_connectors/tcp_stats/tcp_stats_bpf_test.cc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,9 @@ TEST_F(TcpTraceTest, Capture) {
123123
EXPECT_THAT(records, IsSupersetOf(expected));
124124
// TODO(RagalahariP): Explore options for testing retransmissions in a unit test case,
125125
// as retransmissions are blocking calls without known timeout value.
126+
127+
// TODO(ddelnano): Use a test case that verifies that local address of the socket is correct.
128+
// The current implementation is correct, but other source connectors have had bugs here.
126129
}
127130

128131
} // namespace stirling

0 commit comments

Comments
 (0)