All of lore.kernel.org
 help / color / mirror / Atom feed
* Instructions to enable spin locks?
@ 2021-08-04  0:43 Fingerhut, John Andy
  2021-08-04 15:33 ` Pedro Tammela
  0 siblings, 1 reply; 3+ messages in thread
From: Fingerhut, John Andy @ 2021-08-04  0:43 UTC (permalink / raw)
  To: xdp-newbies

Greetings:

I believe the way that this project is using EBPF hash maps in an EBPF program that processes received packets on the XDP receive hook, we need to acquire spin locks while updating map entries, or else we are subject to multiple packets being processed in parallel on multiple CPU cores, and causing incorrect updates to the map values.

https://github.com/intel/host-int

In particular, we should use spin locks at least for this map in the program intmd_xdp_ksink.c: https://github.com/intel/host-int/blob/main/src/xdp/intmd_xdp_ksink.c#L27-L31

I have made at least a few small code changes that seem like they might be correct, in this commit (on my personal fork of that Gitub repo above): https://github.com/jafingerhut/host-int/commit/55577a6efd9d2f5c067f7761aeae01d5669b57fc

While those modifications still allow me to compile the program intmd_xdp_ksink.c, the program fails to pass the kernel verifier when I attempt to load it.  The error messages I see from the verifier can be found at the end of this file: https://github.com/jafingerhut/host-int/blob/partial-spin-lock-implementation/README-verifier-error.txt

The last file I linked also shows steps to reproduce the error, at least on an Ubuntu 20.04 Linux system.  I have not tried reproducing it on other Linux distributions.  I am not aware of any part of that code that depends upon the Linux distribution, other than the Bash install scripts, which install particular Ubuntu packages for build dependencies.

As mentioned in that README, my best guess is that the main piece I am missing is a BTF description of the map, mentioned as a requirement for using spin locks in the bpf-helpers man page in the section describing the bpf_spin_lock function: https://man7.org/linux/man-pages/man7/bpf-helpers.7.html

I would greatly appreciate any steps or example code that would help me get to a working implementation of spin locks for this map.  Bonus points for me if the steps work on a freshly installed Ubuntu 20.04 Linux system, but hopefully an example working on a different Linux distribution would help me see what I'm missing, and lead me to a working EBPF program.

I have attempted to compile all of the EBPF programs in the samples/bpf directory of an Ubuntu-specific Linux kernel source tree, but so far have failed to determine the exact sequence of package installations and/or commands required to successfully compile them all.  I know there is at least one sample program there using spin locks.  I have heard that DWARF and/or pahole might be useful here, and I see pahole mentioned in the kernel samples/bpf/Makefile, but have not yet been able to see the pahole command ever executed by the Makefile (likely because I am missing some necessary setup steps before trying to use that Makefile).

Thanks,
Andy Fingerhut

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Instructions to enable spin locks?
  2021-08-04  0:43 Instructions to enable spin locks? Fingerhut, John Andy
@ 2021-08-04 15:33 ` Pedro Tammela
  2021-08-04 16:36   ` Fingerhut, John Andy
  0 siblings, 1 reply; 3+ messages in thread
From: Pedro Tammela @ 2021-08-04 15:33 UTC (permalink / raw)
  To: Fingerhut, John Andy; +Cc: xdp-newbies

>
> Greetings:
>
> I believe the way that this project is using EBPF hash maps in an EBPF program that processes received packets on the XDP receive hook, we need to acquire spin locks while updating map entries, or else we are subject to multiple packets being processed in parallel on multiple CPU cores, and causing incorrect updates to the map values.
>
> https://github.com/intel/host-int
>
> In particular, we should use spin locks at least for this map in the program intmd_xdp_ksink.c: https://github.com/intel/host-int/blob/main/src/xdp/intmd_xdp_ksink.c#L27-L31
If I may, I would suggest using percpu hash maps and aggregating the
stats from the same flow_key in your userspace daemon.
That way you can avoid spinlocks completely as it models one key to n
values, where n is the number of CPUs.
You can even leverage batching if your map has a considerable amount
of keys[1], which in my experience can handle large maps without
noticeable overhead.

Pedro

[1] https://elixir.bootlin.com/linux/latest/source/tools/testing/selftests/bpf/map_tests/htab_map_batch_ops.c

^ permalink raw reply	[flat|nested] 3+ messages in thread

* RE: Instructions to enable spin locks?
  2021-08-04 15:33 ` Pedro Tammela
@ 2021-08-04 16:36   ` Fingerhut, John Andy
  0 siblings, 0 replies; 3+ messages in thread
From: Fingerhut, John Andy @ 2021-08-04 16:36 UTC (permalink / raw)
  To: Pedro Tammela; +Cc: xdp-newbies

> https://github.com/intel/host-int
>
> In particular, we should use spin locks at least for this map in the 
> program intmd_xdp_ksink.c: 
> https://github.com/intel/host-int/blob/main/src/xdp/intmd_xdp_ksink.c#
> L27-L31

> If I may, I would suggest using percpu hash maps and aggregating the
> stats from the same flow_key in your userspace daemon.  That way you
> can avoid spinlocks completely as it models one key to n values, where
> n is the number of CPUs.  You can even leverage batching if your map
> has a considerable amount of keys[1], which in my experience can
> handle large maps without noticeable overhead.
> 
> Pedro
> 
> [1] https://elixir.bootlin.com/linux/latest/source/tools/testing/selftests/bpf/map_tests/htab_map_batch_ops.c

Thanks for the suggestion, Pedro.

For the EBPF programs I have in mind, they are doing things like
inserting a per-application-flow sequence number into new headers in
each packet in a source host, and then maintaining state in the
receiver hosts for packets that have those sequence numbers added, to
detect whether there are packet drops in the network, i.e. some
sequence numbers are never received.

I know that per-cpu maps exist in EBPF, and they are perfect
when all you want to do is to maintain things like packet or byte
counters, or counters for some other events, because the per-cpu
entries can be combined by adding their counts together.

However, for our use case, I do not see any effective way to make use
of per-cpu maps, and still perform our desired packet processing
functions.  Hence the desire for using spin locks.

Thanks,
Andy Fingerhut

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-08-04 16:36 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-04  0:43 Instructions to enable spin locks? Fingerhut, John Andy
2021-08-04 15:33 ` Pedro Tammela
2021-08-04 16:36   ` Fingerhut, John Andy

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.