From eafdf87d80a5a7cf23c8255b7a421e89a1e35cec Mon Sep 17 00:00:00 2001 From: Simon Sundberg Date: Tue, 9 Feb 2021 13:00:28 +0100 Subject: [PATCH] pping: Fix struct alginment issues Move some members in network_tuple and rtt_event around to avoid holes. Also remove some uncecessary parentheses before & operator, and add local definitions of AF_INET and AF_INET6. Signed-off-by: Simon Sundberg --- pping/pping.c | 6 +++--- pping/pping.h | 4 ++-- pping/pping_helpers.h | 12 +++++------- pping/pping_kern_tc.c | 10 +++++----- pping/pping_kern_xdp.c | 7 +++---- 5 files changed, 18 insertions(+), 21 deletions(-) diff --git a/pping/pping.c b/pping/pping.c index bae883b..e6eb841 100644 --- a/pping/pping.c +++ b/pping/pping.c @@ -248,7 +248,7 @@ static int format_ip_address(int af, const struct in6_addr *addr, char *buf, size_t size) { if (af == AF_INET) - return inet_ntop(af, &(addr->s6_addr[12]), + return inet_ntop(af, &addr->s6_addr[12], buf, size) ? -errno : 0; else if (af == AF_INET6) return inet_ntop(af, addr, buf, size) ? -errno : 0; @@ -261,8 +261,8 @@ static void handle_rtt_event(void *ctx, int cpu, void *data, __u32 data_size) char saddr[INET6_ADDRSTRLEN]; char daddr[INET6_ADDRSTRLEN]; - format_ip_address(e->flow.ipv, &(e->flow.saddr), saddr, sizeof(saddr)); - format_ip_address(e->flow.ipv, &(e->flow.daddr), daddr, sizeof(daddr)); + format_ip_address(e->flow.ipv, &e->flow.saddr, saddr, sizeof(saddr)); + format_ip_address(e->flow.ipv, &e->flow.daddr, daddr, sizeof(daddr)); printf("%llu.%06llu ms %s:%d+%s:%d\n", e->rtt / NS_PER_MS, e->rtt % NS_PER_MS, saddr, ntohs(e->flow.sport), daddr, diff --git a/pping/pping.h b/pping/pping.h index 2dcb07b..79c3191 100644 --- a/pping/pping.h +++ b/pping/pping.h @@ -18,12 +18,12 @@ * is extended to work for other protocols than TCP */ struct network_tuple { - __u8 ipv; //AF_INET or AF_INET6 struct in6_addr saddr; struct in6_addr daddr; __u16 sport; __u16 dport; __u16 proto; //IPPROTO_TCP, IPPROTO_ICMP, QUIC etc + __u16 ipv; //AF_INET or AF_INET6 }; struct packet_id { @@ -37,8 +37,8 @@ struct packet_timestamp { }; struct rtt_event { - struct network_tuple flow; __u64 rtt; + struct network_tuple flow; }; #endif diff --git a/pping/pping_helpers.h b/pping/pping_helpers.h index 6e5b14f..c12a823 100644 --- a/pping/pping_helpers.h +++ b/pping/pping_helpers.h @@ -7,6 +7,8 @@ #include #include "pping.h" +#define AF_INET 2 +#define AF_INET6 10 #define MAX_TCP_OPTIONS 10 /* @@ -15,14 +17,10 @@ static __always_inline void map_ipv4_to_ipv6(__be32 ipv4, struct in6_addr *ipv6) { /* __u16 ipv4_prefix[6] = {0x0, 0x0, 0x0, 0x0, 0x0, 0xffff}; */ - /* memcpy(&(ipv6->in6_u.u6_addr8), ipv4_prefix, sizeof(ipv4_prefix)); */ - memset(&(ipv6->in6_u.u6_addr8[0]), 0x00, 10); - memset(&(ipv6->in6_u.u6_addr8[10]), 0xff, 2); -#if __UAPI_DEF_IN6_ADDR_ALT + /* memcpy(ipv6, ipv4_prefix, sizeof(ipv4_prefix)); // Won't load on TC */ + memset(&ipv6->in6_u.u6_addr8[0], 0x00, 10); + memset(&ipv6->in6_u.u6_addr8[10], 0xff, 2); ipv6->in6_u.u6_addr32[3] = ipv4; -#else - memcpy(&(ipv6->in6_u.u6_addr8[12]), &ipv4, sizeof(ipv4)); -#endif } /* diff --git a/pping/pping_kern_tc.c b/pping/pping_kern_tc.c index a4c2060..bc26697 100644 --- a/pping/pping_kern_tc.c +++ b/pping/pping_kern_tc.c @@ -56,13 +56,13 @@ int tc_bpf_prog_egress(struct __sk_buff *skb) struct packet_id p_id = { 0 }; struct packet_timestamp p_ts = { 0 }; - proto = bpf_ntohs(parse_ethhdr(&nh, data_end, ð)); + proto = parse_ethhdr(&nh, data_end, ð); // Parse IPv4/6 header - if (proto == ETH_P_IP) { + if (proto == bpf_htons(ETH_P_IP)) { p_id.flow.ipv = AF_INET; proto = parse_iphdr(&nh, data_end, &iph); - } else if (proto == ETH_P_IPV6) { + } else if (proto == bpf_htons(ETH_P_IPV6)) { p_id.flow.ipv = AF_INET6; proto = parse_ip6hdr(&nh, data_end, &ip6h); } else @@ -79,8 +79,8 @@ int tc_bpf_prog_egress(struct __sk_buff *skb) // We have a TCP timestamp, try adding it to the map p_id.identifier = tsval; if (p_id.flow.ipv == AF_INET) { - map_ipv4_to_ipv6(iph->saddr, &(p_id.flow.saddr)); - map_ipv4_to_ipv6(iph->daddr, &(p_id.flow.daddr)); + map_ipv4_to_ipv6(iph->saddr, &p_id.flow.saddr); + map_ipv4_to_ipv6(iph->daddr, &p_id.flow.daddr); } else { // IPv6 p_id.flow.saddr = ip6h->saddr; p_id.flow.daddr = ip6h->daddr; diff --git a/pping/pping_kern_xdp.c b/pping/pping_kern_xdp.c index c94ab36..5d4fe5f 100644 --- a/pping/pping_kern_xdp.c +++ b/pping/pping_kern_xdp.c @@ -76,8 +76,8 @@ int xdp_prog_ingress(struct xdp_md *ctx) p_id.flow.proto == proto; // Fill in reverse order of egress (dest <--> source) if (p_id.flow.ipv == AF_INET) { - map_ipv4_to_ipv6(iph->daddr, &(p_id.flow.saddr)); - map_ipv4_to_ipv6(iph->saddr, &(p_id.flow.daddr)); + map_ipv4_to_ipv6(iph->daddr, &p_id.flow.saddr); + map_ipv4_to_ipv6(iph->saddr, &p_id.flow.daddr); } else { // IPv6 p_id.flow.saddr = ip6h->daddr; p_id.flow.daddr = ip6h->saddr; @@ -98,8 +98,7 @@ int xdp_prog_ingress(struct xdp_md *ctx) p_ts->used = 1; // TODO - Optional delete of entry (if identifier is garantued unique) - memcpy(&(event.flow), &(p_id.flow), - sizeof(struct network_tuple)); + memcpy(&event.flow, &p_id.flow, sizeof(struct network_tuple)); event.rtt = bpf_ktime_get_ns() - p_ts->timestamp; bpf_perf_event_output(ctx, &rtt_events, BPF_F_CURRENT_CPU, &event, sizeof(event));