Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpftrace br_private.h file not found error #863

Closed
lyveng opened this issue Aug 19, 2019 · 5 comments
Closed

bpftrace br_private.h file not found error #863

lyveng opened this issue Aug 19, 2019 · 5 comments

Comments

@lyveng
Copy link

lyveng commented Aug 19, 2019

Environment
OS: Debian 9
Kernel: 4.19 from stretch-backports

We were writing a bpftrace program to debug why a packet that was received on the bridge interface from the VM's vnet interface was not forwarded outside via the physical interface(observed these using tcpdump). We had referred to Linux Bridge - how it works article to understand how the packet flows through the bridge.
The tcpdrop tool in bcc does not pick up these drops since these packets were dropped at a lower layer rather than tcp since we are using an L2 bridge.

We realised that the tcp syn packet came to these functions in order using kprobes: br_handle_frame -> br_handle_frame_finish -> br_forward. For some reason adding multiple probes in a single bt file did not work. We had run bpftrace with a single kprobe at a time. But after this the packet neither entered __br_forward(packet to be forwarded outside) nor deliver_clone(packet to be locally delivered).

void br_forward(const struct net_bridge_port *to,
                struct sk_buff *skb, bool local_rcv, bool local_orig)
{
        if (to && should_deliver(to, skb)) {
                if (local_rcv)
                        deliver_clone(to, skb, local_orig);
                else
                        __br_forward(to, skb, local_orig);
                return;
        }   

        if (!local_rcv)
                kfree_skb(skb);
}

The if condition in the beginning of the function br_forward had two predicates. We logged the first one and verified that it was not null. The function should_deliver was an inline function and from what I observed inline functions don't have kprobes(Pardon me I don't have much knowledge of kprobes. Just starting out with bpf).

static inline int should_deliver(const struct net_bridge_port *p,
                                 const struct sk_buff *skb)
{
        struct net_bridge_vlan_group *vg;

        vg = nbp_vlan_group_rcu(p);
        return ((p->flags & BR_HAIRPIN_MODE) || skb->dev != p->dev) &&
                br_allowed_egress(vg, skb) && p->state == BR_STATE_FORWARDING &&
                nbp_switchdev_allowed_egress(p, skb);
}

So to understand the return value we wanted to log the struct net_bridge_port which is the first argument to the function br_forward. This struct net_bridge_port was defined in br_filter.h. Since this file br_filter.h was not inside include directory, we included net/bridge/br_netfilter.h which in turn included br_private.h. Here is the bpftrace program that we wrote

#!/usr/bin/env bpftrace
#include <net/sock.h>
#include <linux/skbuff.h>
#include <linux/ip.h>
#include <net/netfilter/br_netfilter.h>

kprobe:br_forward
{
  $skb = ((sk_buff *) arg1);
  $port = ((net_bridge_port *) arg0);
  $ipheader = ((iphdr *) ($skb->head + $skb->network_header));
  if ($ipheader->daddr == 896409610) { 
    printf("inside br_forward");
    printf("port flags %d", $port->flags);
    printf("skb dev %s", str($skb->dev->name));
    printf("port dev %s", str($port->dev->name));
  }
}

This program fails with the following message

root@fk-cloud-g1a-ms-1312745:/home/livingstone.se# bpftrace l2drop_report.bt 
/bpftrace/include/stdarg.h:52:1: warning: null character ignored [-Wnull-character]
/lib/modules/4.19.0-0.bpo.5-amd64/source/arch/x86/include/asm/processor.h:555:17: warning: taking address of packed member 'sp0' of class or structure 'x86_hw_tss' may result in an unaligned pointer value [-Waddress-of-packed-member]
/lib/modules/4.19.0-0.bpo.5-amd64/source/arch/x86/include/asm/processor.h:572:30: warning: taking address of packed member 'sp1' of class or structure 'x86_hw_tss' may result in an unaligned pointer value [-Waddress-of-packed-member]
/lib/modules/4.19.0-0.bpo.5-amd64/source/arch/x86/include/asm/processor.h:572:30: warning: taking address of packed member 'sp1' of class or structure 'x86_hw_tss' may result in an unaligned pointer value [-Waddress-of-packed-member]
/lib/modules/4.19.0-0.bpo.5-amd64/source/arch/x86/include/asm/processor.h:572:30: warning: taking address of packed member 'sp1' of class or structure 'x86_hw_tss' may result in an unaligned pointer value [-Waddress-of-packed-member]
/lib/modules/4.19.0-0.bpo.5-amd64/source/arch/x86/include/asm/processor.h:572:30: warning: taking address of packed member 'sp1' of class or structure 'x86_hw_tss' may result in an unaligned pointer value [-Waddress-of-packed-member]
/lib/modules/4.19.0-0.bpo.5-amd64/source/arch/x86/include/asm/cpufeature.h:150:2: warning: "Compiler lacks ASM_GOTO support. Add -D __BPF_TRACING__ to your compiler arguments" [-W#warnings]
/lib/modules/4.19.0-0.bpo.5-amd64/source/include/linux/cgroup-defs.h:464:16: warning: field 'cgrp' with variable sized type 'struct cgroup' not at the end of a struct or class is a GNU extension [-Wgnu-variable-sized-type-not-at-end]
/lib/modules/4.19.0-0.bpo.5-amd64/source/include/net/netfilter/br_netfilter.h:5:10: fatal error: '../../../net/bridge/br_private.h' file not found
Unknown struct/union: 'net_bridge_port'

I checked the list of files in the packages linux-headers-4.19.0-0.bpo.5-amd64 and linux-headers-4.19.0-0.bpo.5-common. Both didn't have br_private.h.

It is mentioned in the reference guide that some structs might be missing in the linux headers and such structs have to be either fully or partially re-defined in the bpftrace program. I was trying to declare struct net_bridge_port in the bptrace code to log two fields, viz, flags and dev. But for there seems to be quite a lot of dependent structs that either need to be included or redefined.

Is there any simpler way to get this working or is only defining the relevant structs the only option.

@fbs
Copy link
Member

fbs commented Aug 19, 2019

I was trying to declare struct net_bridge_port in the bptrace code to log two fields, viz, flags and dev. But for there seems to be quite a lot of dependent structs that either need to be included or redefined.

This compiles: (I don't have any bridging so I can't test further)

#include <net/sock.h>
#include <linux/skbuff.h>
#include <linux/ip.h>

struct net_bridge_port {
	struct net_bridge		*br;
	struct net_device		*dev;
	struct list_head		list;
	unsigned long		flags;
}

kprobe:br_forward
{
  $skb = ((sk_buff *) arg1);
  $port = ((net_bridge_port *) arg0);
  $ipheader = ((iphdr *) ($skb->head + $skb->network_header));
	printf("inside br_forward");
	printf("port flags %d", $port->flags);
	printf("skb dev %s", $skb->dev->name);
	printf("port dev %s", $port->dev->name);
}

Is there any simpler way to get this working or is only defining the relevant structs the only option.

I'm afraid not. The current options are:

  • Get the struct from a source file somewhere
  • Define it yourself

BTF will help but once it's merged but it will require a modern kernel.

The function should_deliver was an inline function and from what I observed inline functions don't have kprobes

Correct, if they're inlined they're not functions anymore, see for example https://gcc.gnu.org/onlinedocs/gcc/Inline.html

For some reason adding multiple probes in a single bt file did not work. We had run bpftrace with a single kprobe at a time.

Do you have an example snippet? Multiple probes are supported. The output of bpftool prog could be useful here.

@lyveng
Copy link
Author

lyveng commented Aug 20, 2019

Thanks a lot for the response. It helped me debug an issue with NAT behaviour within 2 weeks of starting with bpf. Is there some existing bpf tool that helps us debug the cause of packet dropped by an l2 bridge(something similar to tcpdrop but at l2 layer).

Here is the complete struct net_bridge_port.

struct net_bridge_port
{
	struct net_bridge		*br;
	struct net_device		*dev;
	struct list_head		list;

	/* STP */
	u8				priority;
	u8				state;
	u16				port_no;
	unsigned char			topology_change_ack;
	unsigned char			config_pending;
	port_id				port_id;
	port_id				designated_port;
	bridge_id			designated_root;
	bridge_id			designated_bridge;
	u32				path_cost;
	u32				designated_cost;
	unsigned long			designated_age;

	struct timer_list		forward_delay_timer;
	struct timer_list		hold_timer;
	struct timer_list		message_age_timer;
	struct kobject			kobj;
	struct rcu_head			rcu;

	unsigned long 			flags;

#ifdef CONFIG_BRIDGE_IGMP_SNOOPING
	struct bridge_mcast_own_query	ip4_own_query;
#if IS_ENABLED(CONFIG_IPV6)
	struct bridge_mcast_own_query	ip6_own_query;
#endif /* IS_ENABLED(CONFIG_IPV6) */
	unsigned char			multicast_router;
	struct bridge_mcast_stats	__percpu *mcast_stats;
	struct timer_list		multicast_router_timer;
	struct hlist_head		mglist;
	struct hlist_node		rlist;
#endif

#ifdef CONFIG_SYSFS
	char				sysfs_name[IFNAMSIZ];
#endif

#ifdef CONFIG_NET_POLL_CONTROLLER
	struct netpoll			*np;
#endif
#ifdef CONFIG_BRIDGE_VLAN_FILTERING
	struct net_bridge_vlan_group	__rcu *vlgrp;
#endif
#ifdef CONFIG_NET_SWITCHDEV
	int				offload_fwd_mark;
#endif
};

I assumed that if we have to access the net_device struct which is the 2nd field in net_bridge_port, then we'll have to either include the header file which defines net_device struct or declare the net_device struct in the bt code. That was the reason that I was planning to include multiple other headers corresponding to structs like net_device, list_head, timer_list, etc.

Also does the order of fields in the struct that we declare in the bt code, i.e., net_bridge_port struct, affect the contents of the field when accessing it. I'm asking because in the original br_private.h code(pasted above) the flags field comes towards the end of the struct. Whereas in the code that you've suggested, it is declared as the 4th field. If the order is immaterial, then would it be enough if we just declare those fields that we would want to access?

For some reason adding multiple probes in a single bt file did not work. We had run bpftrace with a single kprobe at a time.

Do you have an example snippet? Multiple probes are supported. The output of bpftool prog could be useful here.
Here is a sample program where I faced this issue

#!/usr/bin/env bpftrace
#include <net/sock.h>
#include <linux/skbuff.h>
#include <linux/ip.h>

BEGIN
{
  printf("Tracing l2 drops. Hit Ctrl-C to end.\n");
}

kprobe:br_handle_frame
{
  $skb = ((sk_buff *) *arg0);
  $ipheader = ((iphdr *) ($skb->head + $skb->network_header));
  if ($ipheader->daddr == 896409610) { 
    printf("inside handle_frame: %u %u\n", $ipheader->saddr, $ipheader->daddr);
  }
}


kprobe:br_handle_frame_finish
{
  $skb = ((sk_buff *) arg2);
  $ipheader = ((iphdr *) ($skb->head + $skb->network_header));
  if ($ipheader->daddr == 896409610) { 
    printf("inside handle_frame_finish: %u %u\n", $ipheader->saddr, $ipheader->daddr);
  }
}

Here is the error

root@fk-cloud-g1a-ms-1312745:/home/livingstone.se# bpftrace test.bt 
error: <unknown>:0:0: in function kprobe:br_handle_frame_finish i64 (i8*): A call to built-in function 'abort' is not supported.

@fbs
Copy link
Member

fbs commented Aug 20, 2019

Also does the order of fields in the struct that we declare in the bt code, i.e., net_bridge_port struct, affect the contents of the field when accessing it.

Yes. The fields translate into byte offsets into memory. If you mix them up you don't know what happens.

Whereas in the code that you've suggested, it is declared as the 4th field.

I used https://elixir.bootlin.com/linux/v4.19/source/net/bridge/br_private.h#L231 as a reference. It seems like the field changed position in 4.10, are you sure your snippet is correct for your debian version?

@lyveng
Copy link
Author

lyveng commented Aug 21, 2019

I used https://elixir.bootlin.com/linux/v4.19/source/net/bridge/br_private.h#L231 as a reference. It seems like the field changed position in 4.10, are you sure your snippet is correct for your debian version?

It is my bad. Actually I was referring to the 4.9 kernel source which is the default on debian 9. I forgot to switch the kernel source I was referring to after upgrading to 4.19 kernel.

@lyveng lyveng closed this as completed Aug 21, 2019
@lyveng
Copy link
Author

lyveng commented Aug 23, 2019

For some reason adding multiple probes in a single bt file did not work. We had run bpftrace with a single kprobe at a time.

Do you have an example snippet? Multiple probes are supported. The output of bpftool prog could be useful here.
Here is a sample program where I faced this issue

#!/usr/bin/env bpftrace
#include <net/sock.h>
#include <linux/skbuff.h>
#include <linux/ip.h>

BEGIN
{
  printf("Tracing l2 drops. Hit Ctrl-C to end.\n");
}

kprobe:br_handle_frame
{
  $skb = ((sk_buff *) *arg0);
  $ipheader = ((iphdr *) ($skb->head + $skb->network_header));
  if ($ipheader->daddr == 896409610) { 
    printf("inside handle_frame: %u %u\n", $ipheader->saddr, $ipheader->daddr);
  }
}


kprobe:br_handle_frame_finish
{
  $skb = ((sk_buff *) arg2);
  $ipheader = ((iphdr *) ($skb->head + $skb->network_header));
  if ($ipheader->daddr == 896409610) { 
    printf("inside handle_frame_finish: %u %u\n", $ipheader->saddr, $ipheader->daddr);
  }
}

Here is the error

root@fk-cloud-g1a-ms-1312745:/home/livingstone.se# bpftrace test.bt 
error: <unknown>:0:0: in function kprobe:br_handle_frame_finish i64 (i8*): A call to built-in function 'abort' is not supported.

Just figured out the issue here. Looks like the variables don't follow function scoping in bpftrace. Even though intuitively it looks like a variable defined in a single kprobe action would be out of scope inside another kprobe action, reality differs. When we try to re-declare a variable using the same name in another kprobe action, it fails. So when I tried using a different variable name, it worked.

The bpftrace version that I had used 0.8 is old. Probably that's why the error looks misleading. Will try to build a newer version of bpftrace for debian.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants