* Re: [PATCH 4/5] net: phy: marvell: Add LED accessors for Marvell 88E1510
From: Kai-Heng Feng @ 2022-04-21 3:11 UTC (permalink / raw)
To: Andrew Lunn
Cc: hkallweit1, linux, peppe.cavallaro, alexandre.torgue, joabreu,
davem, kuba, pabeni, netdev, linux-kernel
In-Reply-To: <YmAgq1pm37Glw2v+@lunn.ch>
On Wed, Apr 20, 2022 at 11:03 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Wed, Apr 20, 2022 at 08:40:51PM +0800, Kai-Heng Feng wrote:
> > Implement get_led_config() and set_led_config() callbacks so phy core
> > can use firmware LED as platform requested.
> >
> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > ---
> > drivers/net/phy/marvell.c | 26 ++++++++++++++++++++++++++
> > 1 file changed, 26 insertions(+)
> >
> > diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
> > index 2702faf7b0f60..c5f13e09b0692 100644
> > --- a/drivers/net/phy/marvell.c
> > +++ b/drivers/net/phy/marvell.c
> > @@ -750,6 +750,30 @@ static int m88e1510_config_aneg(struct phy_device *phydev)
> > return err;
> > }
> >
> > +static int marvell_get_led_config(struct phy_device *phydev)
> > +{
> > + int led;
> > +
> > + led = phy_read_paged(phydev, MII_MARVELL_LED_PAGE, MII_PHY_LED_CTRL);
> > + if (led < 0) {
> > + phydev_warn(phydev, "Fail to get marvell phy LED.\n");
> > + led = 0;
> > + }
>
> I've said this multiple times, there are three LED registers, The
> Function Control register, the Priority Control register and the Timer
> control register. It is the combination of all three that defines how
> the LEDs work. You need to save all of them.
OK, will do.
>
> And you need to make your API generic enough that the PHY driver can
> save anywhere from 1 bit to 42 bytes of configuration.
OK, I guess I'll let the implementation stays within driver callback,
so each driver can decide how many bits need to be saved.
>
> I don't know ACPI, but i'm pretty sure this is not the ACPI way of
> doing this. I think you should be defining an ACPI method, which
> phylib can call after initialising the hardware to allow the firmware
> to configure the LED.
This is not feasible.
If BIOS can define a method and restore the LED by itself, it can put
the method inside its S3 method and I don't have to work on this at
the first place.
Kai-Heng
>
> Andrew
^ permalink raw reply
* Re: [PATCH bpf-next v2 2/6] ftrace: Fix deadloop caused by direct call in ftrace selftest
From: Xu Kuohai @ 2022-04-21 3:01 UTC (permalink / raw)
To: Steven Rostedt
Cc: bpf, linux-arm-kernel, linux-kernel, netdev, linux-kselftest,
Catalin Marinas, Will Deacon, Ingo Molnar, Daniel Borkmann,
Alexei Starovoitov, Zi Shen Lim, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, David S . Miller, Hideaki YOSHIFUJI, David Ahern,
Thomas Gleixner, Borislav Petkov, Dave Hansen, x86, hpa,
Shuah Khan, Mark Rutland, Ard Biesheuvel, Pasha Tatashin,
Peter Collingbourne, Daniel Kiss, Sudeep Holla, Steven Price,
Marc Zyngier, Mark Brown, Kumar Kartikeya Dwivedi,
Delyan Kratunov
In-Reply-To: <20220420192405.4e43a966@gandalf.local.home>
On 4/21/2022 7:24 AM, Steven Rostedt wrote:
> On Thu, 14 Apr 2022 12:22:16 -0400
> Xu Kuohai <xukuohai@huawei.com> wrote:
>
>> After direct call is enabled for arm64, ftrace selftest enters a
>> dead loop:
>>
>> <trace_selftest_dynamic_test_func>:
>> 00 bti c
>> 01 mov x9, x30 <trace_direct_tramp>:
>> 02 bl <trace_direct_tramp> ----------> ret
>> |
>> lr/x30 is 03, return to 03
>> |
>> 03 mov w0, #0x0 <-----------------------------|
>> | |
>> | dead loop! |
>> | |
>> 04 ret ---- lr/x30 is still 03, go back to 03 ----|
>>
>> The reason is that when the direct caller trace_direct_tramp() returns
>> to the patched function trace_selftest_dynamic_test_func(), lr is still
>> the address after the instrumented instruction in the patched function,
>> so when the patched function exits, it returns to itself!
>>
>> To fix this issue, we need to restore lr before trace_direct_tramp()
>> exits, so make trace_direct_tramp() a weak symbol and rewrite it for
>> arm64.
>>
>> To detect this issue directly, call DYN_FTRACE_TEST_NAME() before
>> register_ftrace_graph().
>>
>> Reported-by: Li Huafei <lihuafei1@huawei.com>
>> Signed-off-by: Xu Kuohai <xukuohai@huawei.com>
>> ---
>> arch/arm64/kernel/entry-ftrace.S | 10 ++++++++++
>> kernel/trace/trace_selftest.c | 4 +++-
>> 2 files changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/kernel/entry-ftrace.S b/arch/arm64/kernel/entry-ftrace.S
>> index dfe62c55e3a2..e58eb06ec9b2 100644
>> --- a/arch/arm64/kernel/entry-ftrace.S
>> +++ b/arch/arm64/kernel/entry-ftrace.S
>> @@ -357,3 +357,13 @@ SYM_CODE_START(return_to_handler)
>> ret
>> SYM_CODE_END(return_to_handler)
>> #endif /* CONFIG_FUNCTION_GRAPH_TRACER */
>> +
>> +#ifdef CONFIG_FTRACE_SELFTEST
>> +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
>> +SYM_FUNC_START(trace_direct_tramp)
>> + mov x10, x30
>> + mov x30, x9
>> + ret x10
>> +SYM_FUNC_END(trace_direct_tramp)
>> +#endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
>> +#endif /* CONFIG_FTRACE_SELFTEST */
>> diff --git a/kernel/trace/trace_selftest.c b/kernel/trace/trace_selftest.c
>> index abcadbe933bb..38b0d5c9a1e0 100644
>> --- a/kernel/trace/trace_selftest.c
>> +++ b/kernel/trace/trace_selftest.c
>> @@ -785,7 +785,7 @@ static struct fgraph_ops fgraph_ops __initdata = {
>> };
>>
>> #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
>> -noinline __noclone static void trace_direct_tramp(void) { }
>> +void __weak trace_direct_tramp(void) { }
>> #endif
>>
>> /*
>
>
>> @@ -868,6 +868,8 @@ trace_selftest_startup_function_graph(struct tracer *trace,
>> if (ret)
>> goto out;
>>
>> + DYN_FTRACE_TEST_NAME();
>
> This doesn't look like it belongs in this patch.
>
> -- Steve
This was added to run trace_direct_tramp() separately before registering
function graph, so the dead loop can be caught accurately. However, the
dead loop can also be caught when running function graph test, so this
is somewhat unnecessary and will be removed in v3.
>
>> +
>> ret = register_ftrace_graph(&fgraph_ops);
>> if (ret) {
>> warn_failed_init_tracer(trace, ret);
>
> .
^ permalink raw reply
* Re: [PATCH 1/5] net: mdio: Mask PHY only when its ACPI node is present
From: Kai-Heng Feng @ 2022-04-21 2:58 UTC (permalink / raw)
To: Andrew Lunn
Cc: hkallweit1, linux, peppe.cavallaro, alexandre.torgue, joabreu,
davem, kuba, pabeni, netdev, linux-kernel
In-Reply-To: <YmAc+dzroa4D1ny2@lunn.ch>
On Wed, Apr 20, 2022 at 10:47 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Wed, Apr 20, 2022 at 08:40:48PM +0800, Kai-Heng Feng wrote:
> > Not all PHY has an ACPI node, for those nodes auto probing is still
> > needed.
>
> Why do you need this?
>
> Documentation/firmware-guide/acpi/dsd/phy.rst
>
> There is nothing here about there being PHYs which are not listed in
> ACPI. If you have decided to go the ACPI route, you need to list the
> PHYs.
This is for backward-compatibility. MAC can have ACPI node but PHY may
not have one.
On ACPI based platform, stmmac is using mdiobus_register() and its PHY
is using autoprobing, so masking all PHYs from autoprobing will break
those stmmac users.
Kai-Heng
>
> Andrew
^ permalink raw reply
* [PATCH] tsnep: Remove useless null check before call of_node_put()
From: Haowen Bai @ 2022-04-21 2:48 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Paolo Abeni
Cc: Haowen Bai, netdev, linux-kernel
No need to add null check before call of_node_put(), since the
implementation of of_node_put() has done it.
Signed-off-by: Haowen Bai <baihaowen@meizu.com>
---
drivers/net/ethernet/engleder/tsnep_main.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/engleder/tsnep_main.c b/drivers/net/ethernet/engleder/tsnep_main.c
index 904f3304727e..49c93aa38862 100644
--- a/drivers/net/ethernet/engleder/tsnep_main.c
+++ b/drivers/net/ethernet/engleder/tsnep_main.c
@@ -1091,8 +1091,7 @@ static int tsnep_mdio_init(struct tsnep_adapter *adapter)
retval = of_mdiobus_register(adapter->mdiobus, np);
out:
- if (np)
- of_node_put(np);
+ of_node_put(np);
return retval;
}
--
2.7.4
^ permalink raw reply related
* Re: [net-next v1] bpf: add bpf_ktime_get_real_ns helper
From: Tonghao Zhang @ 2022-04-21 2:37 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Toke Høiland-Jørgensen, Networking, bpf,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Jiri Olsa, Dave Marchevsky, Kuniyuki Iwashima,
Joanne Koong, Geliang Tang, David S. Miller, Jakub Kicinski,
Eric Dumazet
In-Reply-To: <CAEf4Bzafe3Am5uep7erd7r+-pgdGRc9hsJASYfFH47ty8x9mTA@mail.gmail.com>
On Thu, Apr 21, 2022 at 12:17 AM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Apr 20, 2022 at 5:53 AM Toke Høiland-Jørgensen <toke@kernel.org> wrote:
> >
> > xiangxia.m.yue@gmail.com writes:
> >
> > > From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> > >
> > > This patch introduce a new bpf_ktime_get_real_ns helper, which may
> > > help us to measure the skb latency in the ingress/forwarding path:
> > >
> > > HW/SW[1] -> ip_rcv/tcp_rcv_established -> tcp_recvmsg_locked/tcp_update_recv_tstamps
> > >
> > > * Insert BPF kprobe into ip_rcv/tcp_rcv_established invoking this helper.
> > > Then we can inspect how long time elapsed since HW/SW.
> > > * If inserting BPF kprobe tcp_update_recv_tstamps invoked by tcp_recvmsg,
> > > we can measure how much latency skb in tcp receive queue. The reason for
> > > this can be application fetch the TCP messages too late.
> >
> > Why not just use one of the existing ktime helpers and also add a BPF
> > probe to set the initial timestamp instead of relying on skb->tstamp?
> >
>
> You don't even need a BPF probe for this. See [0] for how retsnoop is
> converting bpf_ktime_get_ns() into real time.
>
> [0] https://github.com/anakryiko/retsnoop/blob/master/src/retsnoop.c#L649-L668
I try to calculate this offset too. But one case:
If administrator manually or NTP changes the clock, we should
calculate the offset.
How do we know the changes, one solution is that inserting kprobe in
tk_set_wall_to_mono() kernel function,
and using perf_event to notify userspace.
> > -Toke
--
Best regards, Tonghao
^ permalink raw reply
* Re: [PATCH net-next] net/af_packet: add VLAN support for AF_PACKET SOCK_RAW GSO
From: Hangbin Liu @ 2022-04-21 2:31 UTC (permalink / raw)
To: Willem de Bruijn
Cc: netdev, Michael S . Tsirkin, Jason Wang, David S . Miller,
Jakub Kicinski, Paolo Abeni, Maxim Mikityanskiy, virtualization,
Balazs Nemeth, Mike Pattrick, Eric Dumazet
In-Reply-To: <CA+FuTScyF4BKEcNSCYOv8SBA_EmB806YtKA17jb3F+fymVF-Pg@mail.gmail.com>
On Wed, Apr 20, 2022 at 09:45:15AM -0400, Willem de Bruijn wrote:
> On Wed, Apr 20, 2022 at 4:28 AM Hangbin Liu <liuhangbin@gmail.com> wrote:
> >
> > Currently, the kernel drops GSO VLAN tagged packet if it's created with
> > socket(AF_PACKET, SOCK_RAW, 0) plus virtio_net_hdr.
> >
> > The reason is AF_PACKET doesn't adjust the skb network header if there is
> > a VLAN tag. Then after virtio_net_hdr_set_proto() called, the skb->protocol
> > will be set to ETH_P_IP/IPv6. And in later inet/ipv6_gso_segment() the skb
> > is dropped as network header position is invalid.
> >
> > Let's handle VLAN packets by adjusting network header position in
> > packet_parse_headers(), and move the function in packet_snd() before
> > calling virtio_net_hdr_set_proto().
>
> The network header is set in
>
> skb_reset_network_header(skb);
>
> err = -EINVAL;
> if (sock->type == SOCK_DGRAM) {
> offset = dev_hard_header(skb, dev, ntohs(proto), addr,
> NULL, len);
> if (unlikely(offset < 0))
> goto out_free;
> } else if (reserve) {
> skb_reserve(skb, -reserve);
> if (len < reserve + sizeof(struct ipv6hdr) &&
> dev->min_header_len != dev->hard_header_len)
> skb_reset_network_header(skb);
> }
>
> If all that is needed is to move the network header beyond an optional
> VLAN tag in the case of SOCK_RAW, then this can be done in the else
> for Ethernet packets.
Before we set network position, we need to check the skb->protocol to make
sure it's a VLAN packet.
If we set skb->protocol and adjust network header here, like
packet_parse_headers() does. How should we do with later
skb->protocol = proto;
skb->dev = dev;
settings?
If you are afraid that skb_probe_transport_header(skb) would affect the
virtio_net_hdr operation. How about split the skb_probe_transport_header()
from packet_parse_headers()? Something like
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1924,13 +1924,19 @@ static int packet_rcv_spkt(struct sk_buff *skb, struct net_device *dev,
static void packet_parse_headers(struct sk_buff *skb, struct socket *sock)
{
+ int depth;
+
if ((!skb->protocol || skb->protocol == htons(ETH_P_ALL)) &&
sock->type == SOCK_RAW) {
skb_reset_mac_header(skb);
skb->protocol = dev_parse_header_protocol(skb);
}
- skb_probe_transport_header(skb);
+ /* Move network header to the right position for VLAN tagged packets */
+ if (likely(skb->dev->type == ARPHRD_ETHER) &&
+ eth_type_vlan(skb->protocol) &&
+ __vlan_get_protocol(skb, skb->protocol, &depth) != 0)
+ skb_set_network_header(skb, depth);
}
/*
@@ -3047,6 +3055,8 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
skb->mark = sockc.mark;
skb->tstamp = sockc.transmit_time;
+ packet_parse_headers(skb, sock);
+
if (has_vnet_hdr) {
err = virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le());
if (err)
@@ -3055,7 +3065,7 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
virtio_net_hdr_set_proto(skb, &vnet_hdr);
}
- packet_parse_headers(skb, sock);
+ skb_probe_transport_header(skb);
if (unlikely(extra_len == 4))
skb->no_fcs = 1;
>
> It is not safe to increase reserve, as that would eat into the
> reserved hlen LL_RESERVED_SPACE(dev), which does not account for
> optional VLAN headers.
>
> Instead of setting here first, then patching up again later in
> packet_parse_headers.
>
> This change affects all packets with VLAN headers, not just those with
> GSO. I imagine that moving the network header is safe for all, but
> don't know that code well enough to verify that it does not have
> unintended side effects. Does dev_queue_xmit expect the network header
> to point to the start of the VLAN headers or after, for instance?
I think adjust the network position should be safe, as tap device also did that.
Thanks
Hangbin
^ permalink raw reply
* Re: [net-next v1] bpf: add bpf_ktime_get_real_ns helper
From: Tonghao Zhang @ 2022-04-21 2:27 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: Linux Kernel Network Developers, bpf, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
Yonghong Song, John Fastabend, KP Singh, Jiri Olsa,
Dave Marchevsky, Kuniyuki Iwashima, Joanne Koong, Geliang Tang,
David S. Miller, Jakub Kicinski, Eric Dumazet
In-Reply-To: <878rrzj4r6.fsf@toke.dk>
On Wed, Apr 20, 2022 at 8:53 PM Toke Høiland-Jørgensen <toke@kernel.org> wrote:
>
> xiangxia.m.yue@gmail.com writes:
>
> > From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> >
> > This patch introduce a new bpf_ktime_get_real_ns helper, which may
> > help us to measure the skb latency in the ingress/forwarding path:
> >
> > HW/SW[1] -> ip_rcv/tcp_rcv_established -> tcp_recvmsg_locked/tcp_update_recv_tstamps
> >
> > * Insert BPF kprobe into ip_rcv/tcp_rcv_established invoking this helper.
> > Then we can inspect how long time elapsed since HW/SW.
> > * If inserting BPF kprobe tcp_update_recv_tstamps invoked by tcp_recvmsg,
> > we can measure how much latency skb in tcp receive queue. The reason for
> > this can be application fetch the TCP messages too late.
>
> Why not just use one of the existing ktime helpers and also add a BPF
> probe to set the initial timestamp instead of relying on skb->tstamp?
Yes, That also looks good to me.
> -Toke
--
Best regards, Tonghao
^ permalink raw reply
* Re: [PATCH v2 net] tcp: md5: incorrect tcp_header_len for incoming connections
From: Eric Dumazet @ 2022-04-21 2:00 UTC (permalink / raw)
To: Francesco Ruggeri
Cc: Paolo Abeni, Jakub Kicinski, David Ahern, Hideaki YOSHIFUJI,
David Miller, LKML, netdev
In-Reply-To: <20220421005026.686A45EC01F2@us226.sjc.aristanetworks.com>
On Wed, Apr 20, 2022 at 5:50 PM Francesco Ruggeri <fruggeri@arista.com> wrote:
>
> In tcp_create_openreq_child we adjust tcp_header_len for md5 using the
> remote address in newsk. But that address is still 0 in newsk at this
> point, and it is only set later by the callers (tcp_v[46]_syn_recv_sock).
> Use the address from the request socket instead.
>
>
>
> Fixes: cfb6eeb4c860 ("[TCP]: MD5 Signature Option (RFC2385) support.")
> Signed-off-by: Francesco Ruggeri <fruggeri@arista.com>
Thanks for fixing this Francesco.
Reviewed-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* Re: [PATCH net-next] Revert "rtnetlink: return EINVAL when request cannot succeed"
From: Eric Dumazet @ 2022-04-21 1:15 UTC (permalink / raw)
To: Florent Fourcot, netdev; +Cc: Stephen Hemminger, Brian Baboch
In-Reply-To: <20220419125151.15589-1-florent.fourcot@wifirst.fr>
On 4/19/22 05:51, Florent Fourcot wrote:
> This reverts commit b6177d3240a4
>
> ip-link command is testing kernel capability by sending a RTM_NEWLINK
> request, without any argument. It accepts everything in reply, except
> EOPNOTSUPP and EINVAL (functions iplink_have_newlink / accept_msg)
>
> So we must keep compatiblity here, invalid empty message should not
> return EINVAL
>
> Signed-off-by: Florent Fourcot <florent.fourcot@wifirst.fr>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Thanks for fixing this issue.
^ permalink raw reply
* [PATCH v2 net] tcp: md5: incorrect tcp_header_len for incoming connections
From: Francesco Ruggeri @ 2022-04-21 0:50 UTC (permalink / raw)
To: pabeni, kuba, dsahern, yoshfuji, davem, edumazet, linux-kernel,
netdev, fruggeri
In tcp_create_openreq_child we adjust tcp_header_len for md5 using the
remote address in newsk. But that address is still 0 in newsk at this
point, and it is only set later by the callers (tcp_v[46]_syn_recv_sock).
Use the address from the request socket instead.
v2: Added "Fixes:" line.
Fixes: cfb6eeb4c860 ("[TCP]: MD5 Signature Option (RFC2385) support.")
Signed-off-by: Francesco Ruggeri <fruggeri@arista.com>
---
net/ipv4/tcp_minisocks.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index 6366df7aaf2a..6854bb1fb32b 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -531,7 +531,7 @@ struct sock *tcp_create_openreq_child(const struct sock *sk,
newtp->tsoffset = treq->ts_off;
#ifdef CONFIG_TCP_MD5SIG
newtp->md5sig_info = NULL; /*XXX*/
- if (newtp->af_specific->md5_lookup(sk, newsk))
+ if (treq->af_specific->req_md5_lookup(sk, req_to_sk(req)))
newtp->tcp_header_len += TCPOLEN_MD5SIG_ALIGNED;
#endif
if (skb->len >= TCP_MSS_DEFAULT + newtp->tcp_header_len)
--
2.28.0
^ permalink raw reply related
* [PATCH v2 bpf 06/11] samples/bpf: use host bpftool to generate vmlinux.h, not target
From: Alexander Lobakin @ 2022-04-21 0:39 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: Alexander Lobakin, Maciej Fijalkowski, Song Liu,
Kumar Kartikeya Dwivedi, bpf, netdev, linux-kernel
In-Reply-To: <20220421003152.339542-1-alobakin@pm.me>
Use the host build of bpftool (bootstrap) instead of the target one
to generate vmlinux.h/skeletons for the BPF samples. Otherwise, when
host != target, samples compilation fails with:
/bin/sh: line 1: samples/bpf/bpftool/bpftool: failed to exec: Exec
format error
Fixes: 384b6b3bbf0d ("samples: bpf: Add vmlinux.h generation support")
Acked-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Acked-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Alexander Lobakin <alobakin@pm.me>
---
samples/bpf/Makefile | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 70323ac1114f..2bb9088a8d91 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -291,12 +291,13 @@ $(LIBBPF): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OU
BPFTOOLDIR := $(TOOLS_PATH)/bpf/bpftool
BPFTOOL_OUTPUT := $(abspath $(BPF_SAMPLES_PATH))/bpftool
-BPFTOOL := $(BPFTOOL_OUTPUT)/bpftool
+BPFTOOL := $(BPFTOOL_OUTPUT)/bootstrap/bpftool
$(BPFTOOL): $(LIBBPF) $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) | $(BPFTOOL_OUTPUT)
$(MAKE) -C $(BPFTOOLDIR) srctree=$(BPF_SAMPLES_PATH)/../../ \
OUTPUT=$(BPFTOOL_OUTPUT)/ \
LIBBPF_OUTPUT=$(LIBBPF_OUTPUT)/ \
- LIBBPF_DESTDIR=$(LIBBPF_DESTDIR)/
+ LIBBPF_DESTDIR=$(LIBBPF_DESTDIR)/ \
+ bootstrap
$(LIBBPF_OUTPUT) $(BPFTOOL_OUTPUT):
$(call msg,MKDIR,$@)
--
2.36.0
^ permalink raw reply related
* [PATCH v2 bpf 05/11] samples/bpf: add 'asm/mach-generic' include path for every MIPS
From: Alexander Lobakin @ 2022-04-21 0:39 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: Alexander Lobakin, Maciej Fijalkowski, Song Liu,
Kumar Kartikeya Dwivedi, bpf, netdev, linux-kernel
In-Reply-To: <20220421003152.339542-1-alobakin@pm.me>
Fix the following:
In file included from samples/bpf/tracex2_kern.c:7:
In file included from ./include/linux/skbuff.h:13:
In file included from ./include/linux/kernel.h:22:
In file included from ./include/linux/bitops.h:33:
In file included from ./arch/mips/include/asm/bitops.h:20:
In file included from ./arch/mips/include/asm/barrier.h:11:
./arch/mips/include/asm/addrspace.h:13:10: fatal error: 'spaces.h' file not found
#include <spaces.h>
^~~~~~~~~~
'arch/mips/include/asm/mach-generic' should always be included as
many other MIPS include files rely on this.
Move it from under CONFIG_MACH_LOONGSON64 to let it be included
for every MIPS.
Fixes: 058107abafc7 ("samples/bpf: Add include dir for MIPS Loongson64 to fix build errors")
Acked-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Alexander Lobakin <alobakin@pm.me>
---
samples/bpf/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 38638845db9d..70323ac1114f 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -194,8 +194,8 @@ ifeq ($(ARCH), mips)
TPROGS_CFLAGS += -D__SANE_USERSPACE_TYPES__
ifdef CONFIG_MACH_LOONGSON64
BPF_EXTRA_CFLAGS += -I$(srctree)/arch/mips/include/asm/mach-loongson64
-BPF_EXTRA_CFLAGS += -I$(srctree)/arch/mips/include/asm/mach-generic
endif
+BPF_EXTRA_CFLAGS += -I$(srctree)/arch/mips/include/asm/mach-generic
endif
TPROGS_CFLAGS += -Wall -O2
--
2.36.0
^ permalink raw reply related
* [PATCH v2 bpf 10/11] samples/bpf: fix -Wsequence-point
From: Alexander Lobakin @ 2022-04-21 0:39 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: Alexander Lobakin, Maciej Fijalkowski, Song Liu,
Kumar Kartikeya Dwivedi, bpf, netdev, linux-kernel
In-Reply-To: <20220421003152.339542-1-alobakin@pm.me>
In some libc implementations, CPU_SET() may utilize its first
argument several times. When combined with a post-increment,
it leads to:
samples/bpf/test_lru_dist.c:233:36: warning: operation on 'next_to_try' may be undefined [-Wsequence-point]
233 | CPU_SET(next_to_try++, &cpuset);
| ^
Macros must always define local copies of arguments to avoid
reusing, but since several libc versions already and still have
that, split the sentence into two standalone operations to fix
this.
Fixes: 5db58faf989f ("bpf: Add tests for the LRU bpf_htab")
Acked-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Alexander Lobakin <alobakin@pm.me>
---
samples/bpf/test_lru_dist.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/samples/bpf/test_lru_dist.c b/samples/bpf/test_lru_dist.c
index 75e877853596..d09ccd5370e8 100644
--- a/samples/bpf/test_lru_dist.c
+++ b/samples/bpf/test_lru_dist.c
@@ -230,7 +230,8 @@ static int sched_next_online(int pid, int next_to_try)
while (next_to_try < nr_cpus) {
CPU_ZERO(&cpuset);
- CPU_SET(next_to_try++, &cpuset);
+ CPU_SET(next_to_try, &cpuset);
+ next_to_try++;
if (!sched_setaffinity(pid, sizeof(cpuset), &cpuset))
break;
}
--
2.36.0
^ permalink raw reply related
* [PATCH v2 bpf 07/11] samples/bpf: fix uin64_t format literals
From: Alexander Lobakin @ 2022-04-21 0:39 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: Alexander Lobakin, Maciej Fijalkowski, Song Liu,
Kumar Kartikeya Dwivedi, bpf, netdev, linux-kernel
In-Reply-To: <20220421003152.339542-1-alobakin@pm.me>
There's a couple places where uin64_t is being passed as an %lu
format argument. That type is defined as unsigned long on 64-bit
systems and as unsigned long long on 32-bit, so neither %lu nor
%llu are not universal.
One of the options is %PRIu64, but since it's always 8-byte long,
just cast it to the _proper_ __u64 and print as %llu.
Fixes: 51570a5ab2b7 ("A Sample of using socket cookie and uid for traffic monitoring")
Fixes: 00f660eaf378 ("Sample program using SO_COOKIE")
Signed-off-by: Alexander Lobakin <alobakin@pm.me>
---
samples/bpf/cookie_uid_helper_example.c | 12 ++++++------
samples/bpf/lwt_len_hist_user.c | 7 ++++---
2 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/samples/bpf/cookie_uid_helper_example.c b/samples/bpf/cookie_uid_helper_example.c
index f0df3dda4b1f..269fac58fd5c 100644
--- a/samples/bpf/cookie_uid_helper_example.c
+++ b/samples/bpf/cookie_uid_helper_example.c
@@ -207,9 +207,9 @@ static void print_table(void)
error(1, errno, "fail to get entry value of Key: %u\n",
curN);
} else {
- printf("cookie: %u, uid: 0x%x, Packet Count: %lu,"
- " Bytes Count: %lu\n", curN, curEntry.uid,
- curEntry.packets, curEntry.bytes);
+ printf("cookie: %u, uid: 0x%x, Packet Count: %llu, Bytes Count: %llu\n",
+ curN, curEntry.uid, (__u64)curEntry.packets,
+ (__u64)curEntry.bytes);
}
}
}
@@ -265,9 +265,9 @@ static void udp_client(void)
if (res < 0)
error(1, errno, "lookup sk stat failed, cookie: %lu\n",
cookie);
- printf("cookie: %lu, uid: 0x%x, Packet Count: %lu,"
- " Bytes Count: %lu\n\n", cookie, dataEntry.uid,
- dataEntry.packets, dataEntry.bytes);
+ printf("cookie: %llu, uid: 0x%x, Packet Count: %llu, Bytes Count: %llu\n\n",
+ (__u64)cookie, dataEntry.uid, (__u64)dataEntry.packets,
+ (__u64)dataEntry.bytes);
}
close(s_send);
close(s_rcv);
diff --git a/samples/bpf/lwt_len_hist_user.c b/samples/bpf/lwt_len_hist_user.c
index 430a4b7e353e..c682faa75a2b 100644
--- a/samples/bpf/lwt_len_hist_user.c
+++ b/samples/bpf/lwt_len_hist_user.c
@@ -44,7 +44,8 @@ int main(int argc, char **argv)
while (bpf_map_get_next_key(map_fd, &key, &next_key) == 0) {
if (next_key >= MAX_INDEX) {
- fprintf(stderr, "Key %lu out of bounds\n", next_key);
+ fprintf(stderr, "Key %llu out of bounds\n",
+ (__u64)next_key);
continue;
}
@@ -66,8 +67,8 @@ int main(int argc, char **argv)
for (i = 1; i <= max_key + 1; i++) {
stars(starstr, data[i - 1], max_value, MAX_STARS);
- printf("%8ld -> %-8ld : %-8ld |%-*s|\n",
- (1l << i) >> 1, (1l << i) - 1, data[i - 1],
+ printf("%8ld -> %-8ld : %-8lld |%-*s|\n",
+ (1l << i) >> 1, (1l << i) - 1, (__u64)data[i - 1],
MAX_STARS, starstr);
}
--
2.36.0
^ permalink raw reply related
* Re: [PATCH v2 bpf 00/11] bpf: random unpopular userspace fixes (32 bit et al)
From: Alexei Starovoitov @ 2022-04-21 0:40 UTC (permalink / raw)
To: Alexander Lobakin
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Maciej Fijalkowski, Song Liu, Kumar Kartikeya Dwivedi, bpf,
Network Development, LKML
In-Reply-To: <20220421003152.339542-1-alobakin@pm.me>
On Wed, Apr 20, 2022 at 5:38 PM Alexander Lobakin <alobakin@pm.me> wrote:
Again?
-----BEGIN PGP MESSAGE-----
Version: ProtonMail
wcFMA165ASBBe6s8AQ/8C9y4TqXgASA5xBT7UIf2GyTQRjKWcy/6kT1dkjkF
FldAOhehhgLYjLJzNAIkecOQfz/XNapW3GdrQDq11pq9Bzs1SJJekGXlHVIW
Sorry I'm tossing the series out of patchwork.
^ permalink raw reply
* [PATCH v2 bpf 11/11] samples/bpf: xdpsock: fix -Wmaybe-uninitialized
From: Alexander Lobakin @ 2022-04-21 0:39 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: Alexander Lobakin, Maciej Fijalkowski, Song Liu,
Kumar Kartikeya Dwivedi, bpf, netdev, linux-kernel
In-Reply-To: <20220421003152.339542-1-alobakin@pm.me>
Fix two sort-of-false-positives in the xdpsock userspace part:
samples/bpf/xdpsock_user.c: In function 'main':
samples/bpf/xdpsock_user.c:1531:47: warning: 'tv_usec' may be used uninitialized in this function [-Wmaybe-uninitialized]
1531 | pktgen_hdr->tv_usec = htonl(tv_usec);
| ^~~~~~~~~~~~~~
samples/bpf/xdpsock_user.c:1500:26: note: 'tv_usec' was declared here
1500 | u32 idx, tv_sec, tv_usec;
| ^~~~~~~
samples/bpf/xdpsock_user.c:1530:46: warning: 'tv_sec' may be used uninitialized in this function [-Wmaybe-uninitialized]
1530 | pktgen_hdr->tv_sec = htonl(tv_sec);
| ^~~~~~~~~~~~~
samples/bpf/xdpsock_user.c:1500:18: note: 'tv_sec' was declared here
1500 | u32 idx, tv_sec, tv_usec;
| ^~~~~~
Both variables are always initialized when @opt_tstamp == true and
they're being used also only when @opt_tstamp == true. However, that
variable comes from the BSS and is being toggled from another
function. They can't be executed simultaneously to actually trigger
undefined behaviour, but purely technically it is a correct warning.
Just initialize them with zeroes.
Fixes: eb68db45b747 ("samples/bpf: xdpsock: Add timestamp for Tx-only operation")
Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Acked-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Alexander Lobakin <alobakin@pm.me>
---
samples/bpf/xdpsock_user.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/samples/bpf/xdpsock_user.c b/samples/bpf/xdpsock_user.c
index 9747d47a0a8f..a6d8291c8b38 100644
--- a/samples/bpf/xdpsock_user.c
+++ b/samples/bpf/xdpsock_user.c
@@ -1497,7 +1497,7 @@ static void rx_drop_all(void)
static int tx_only(struct xsk_socket_info *xsk, u32 *frame_nb,
int batch_size, unsigned long tx_ns)
{
- u32 idx, tv_sec, tv_usec;
+ u32 idx, tv_sec = 0, tv_usec = 0;
unsigned int i;
while (xsk_ring_prod__reserve(&xsk->tx, batch_size, &idx) <
--
2.36.0
^ permalink raw reply related
* [PATCH v2 bpf 09/11] samples/bpf: fix include order for non-Glibc environments
From: Alexander Lobakin @ 2022-04-21 0:39 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: Alexander Lobakin, Maciej Fijalkowski, Song Liu,
Kumar Kartikeya Dwivedi, bpf, netdev, linux-kernel
In-Reply-To: <20220421003152.339542-1-alobakin@pm.me>
Some standard C library implementations, e.g. Musl, ship the UAPI
definitions themselves to not be dependent on the UAPI headers and
their versions. Their kernel UAPI counterparts are usually guarded
with some definitions which the formers set in order to avoid
duplicate definitions.
In such cases, include order matters. Change it in two samples: in
the first, kernel UAPI ioctl definitions should go before the libc
ones, and the opposite story with the second, where the kernel
includes should go later to avoid struct redefinitions.
Fixes: b4b8faa1ded7 ("samples/bpf: sample application and documentation for AF_XDP sockets")
Fixes: e55190f26f92 ("samples/bpf: Fix build for task_fd_query_user.c")
Acked-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Alexander Lobakin <alobakin@pm.me>
---
samples/bpf/task_fd_query_user.c | 2 +-
samples/bpf/xdpsock_user.c | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/samples/bpf/task_fd_query_user.c b/samples/bpf/task_fd_query_user.c
index c9a0ca8351fd..c0ecca01d890 100644
--- a/samples/bpf/task_fd_query_user.c
+++ b/samples/bpf/task_fd_query_user.c
@@ -9,11 +9,11 @@
#include <stdint.h>
#include <fcntl.h>
#include <linux/bpf.h>
+#include <linux/perf_event.h>
#include <sys/ioctl.h>
#include <sys/resource.h>
#include <sys/types.h>
#include <sys/stat.h>
-#include <linux/perf_event.h>
#include <bpf/bpf.h>
#include <bpf/libbpf.h>
diff --git a/samples/bpf/xdpsock_user.c b/samples/bpf/xdpsock_user.c
index 6f3fe30ad283..9747d47a0a8f 100644
--- a/samples/bpf/xdpsock_user.c
+++ b/samples/bpf/xdpsock_user.c
@@ -7,14 +7,15 @@
#include <linux/bpf.h>
#include <linux/if_link.h>
#include <linux/if_xdp.h>
-#include <linux/if_ether.h>
#include <linux/ip.h>
#include <linux/limits.h>
+#include <linux/net.h>
#include <linux/udp.h>
#include <arpa/inet.h>
#include <locale.h>
#include <net/ethernet.h>
#include <netinet/ether.h>
+#include <linux/if_ether.h>
#include <net/if.h>
#include <poll.h>
#include <pthread.h>
--
2.36.0
^ permalink raw reply related
* [PATCH v2 bpf 08/11] samples/bpf: fix false-positive right-shift underflow warnings
From: Alexander Lobakin @ 2022-04-21 0:39 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: Alexander Lobakin, Maciej Fijalkowski, Song Liu,
Kumar Kartikeya Dwivedi, bpf, netdev, linux-kernel
In-Reply-To: <20220421003152.339542-1-alobakin@pm.me>
On 32 bit systems, shifting an unsigned long by 32 positions
yields the following warning:
samples/bpf/tracex2_kern.c:60:23: warning: shift count >= width of type [-Wshift-count-overflow]
unsigned int hi = v >> 32;
^ ~~
sizeof(long) is always 8 for the BPF architecture, so this is not
correct, but the BPF samples Makefile still uses the Clang native +
LLC combo which enforces that.
Until the samples are switched to `-target bpf`, do it the usual
way: shift by 16 two times (see upper_32_bits() macro in the
kernel).
Fixes: d822a1926849 ("samples/bpf: Add counting example for kfree_skb() function calls and the write() syscall")
Fixes: 0fb1170ee68a ("bpf: BPF based latency tracing")
Fixes: f74599f7c530 ("bpf: Add tests and samples for LWT-BPF")
Acked-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Alexander Lobakin <alobakin@pm.me>
---
samples/bpf/lathist_kern.c | 2 +-
samples/bpf/lwt_len_hist_kern.c | 2 +-
samples/bpf/tracex2_kern.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/samples/bpf/lathist_kern.c b/samples/bpf/lathist_kern.c
index 4adfcbbe6ef4..9744ed547abe 100644
--- a/samples/bpf/lathist_kern.c
+++ b/samples/bpf/lathist_kern.c
@@ -53,7 +53,7 @@ static unsigned int log2(unsigned int v)
static unsigned int log2l(unsigned long v)
{
- unsigned int hi = v >> 32;
+ unsigned int hi = (v >> 16) >> 16;
if (hi)
return log2(hi) + 32;
diff --git a/samples/bpf/lwt_len_hist_kern.c b/samples/bpf/lwt_len_hist_kern.c
index 1fa14c54963a..bf32fa04c91f 100644
--- a/samples/bpf/lwt_len_hist_kern.c
+++ b/samples/bpf/lwt_len_hist_kern.c
@@ -49,7 +49,7 @@ static unsigned int log2(unsigned int v)
static unsigned int log2l(unsigned long v)
{
- unsigned int hi = v >> 32;
+ unsigned int hi = (v >> 16) >> 16;
if (hi)
return log2(hi) + 32;
else
diff --git a/samples/bpf/tracex2_kern.c b/samples/bpf/tracex2_kern.c
index 5bc696bac27d..6bf22056ff95 100644
--- a/samples/bpf/tracex2_kern.c
+++ b/samples/bpf/tracex2_kern.c
@@ -57,7 +57,7 @@ static unsigned int log2(unsigned int v)
static unsigned int log2l(unsigned long v)
{
- unsigned int hi = v >> 32;
+ unsigned int hi = (v >> 16) >> 16;
if (hi)
return log2(hi) + 32;
else
--
2.36.0
^ permalink raw reply related
* [PATCH v2 bpf 04/11] bpftool: fix fcntl.h include
From: Alexander Lobakin @ 2022-04-21 0:39 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: Alexander Lobakin, Maciej Fijalkowski, Song Liu,
Kumar Kartikeya Dwivedi, bpf, netdev, linux-kernel
In-Reply-To: <20220421003152.339542-1-alobakin@pm.me>
Fix the following (on some libc implementations):
CC tracelog.o
In file included from tracelog.c:12:
include/sys/fcntl.h:1:2: warning: #warning redirecting incorrect #include <sys/fcntl.h> to <fcntl.h> [-Wcpp]
1 | #warning redirecting incorrect #include <sys/fcntl.h> to <fcntl.h>
| ^~~~~~~
<sys/fcntl.h> is anyway just a wrapper over <fcntl.h> (backcomp
stuff).
Fixes: 30da46b5dc3a ("tools: bpftool: add a command to dump the trace pipe")
Acked-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Alexander Lobakin <alobakin@pm.me>
---
tools/bpf/bpftool/tracelog.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/bpf/bpftool/tracelog.c b/tools/bpf/bpftool/tracelog.c
index e80a5c79b38f..bf1f02212797 100644
--- a/tools/bpf/bpftool/tracelog.c
+++ b/tools/bpf/bpftool/tracelog.c
@@ -9,7 +9,7 @@
#include <string.h>
#include <unistd.h>
#include <linux/magic.h>
-#include <sys/fcntl.h>
+#include <fcntl.h>
#include <sys/vfs.h>
#include "main.h"
--
2.36.0
^ permalink raw reply related
* [PATCH v2 bpf 03/11] bpftool: use a local bpf_perf_event_value to fix accessing its fields
From: Alexander Lobakin @ 2022-04-21 0:39 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: Alexander Lobakin, Maciej Fijalkowski, Song Liu,
Kumar Kartikeya Dwivedi, bpf, netdev, linux-kernel
In-Reply-To: <20220421003152.339542-1-alobakin@pm.me>
Fix the following error when building bpftool:
CLANG profiler.bpf.o
CLANG pid_iter.bpf.o
skeleton/profiler.bpf.c:18:21: error: invalid application of 'sizeof' to an incomplete type 'struct bpf_perf_event_value'
__uint(value_size, sizeof(struct bpf_perf_event_value));
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/bpf/bpftool/bootstrap/libbpf/include/bpf/bpf_helpers.h:13:39: note: expanded from macro '__uint'
tools/bpf/bpftool/bootstrap/libbpf/include/bpf/bpf_helper_defs.h:7:8: note: forward declaration of 'struct bpf_perf_event_value'
struct bpf_perf_event_value;
^
struct bpf_perf_event_value is being used in the kernel only when
CONFIG_BPF_EVENTS is enabled, so it misses a BTF entry then.
Define struct bpf_perf_event_value___local with the
`preserve_access_index` attribute inside the pid_iter BPF prog to
allow compiling on any configs. It is a full mirror of a UAPI
structure, so is compatible both with and w/o CO-RE.
bpf_perf_event_read_value() requires a pointer of the original type,
so a cast is needed.
Fixes: 47c09d6a9f67 ("bpftool: Introduce "prog profile" command")
Suggested-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Alexander Lobakin <alobakin@pm.me>
---
tools/bpf/bpftool/skeleton/profiler.bpf.c | 27 ++++++++++++++---------
1 file changed, 17 insertions(+), 10 deletions(-)
diff --git a/tools/bpf/bpftool/skeleton/profiler.bpf.c b/tools/bpf/bpftool/skeleton/profiler.bpf.c
index ce5b65e07ab1..2f80edc682f1 100644
--- a/tools/bpf/bpftool/skeleton/profiler.bpf.c
+++ b/tools/bpf/bpftool/skeleton/profiler.bpf.c
@@ -4,6 +4,12 @@
#include <bpf/bpf_helpers.h>
#include <bpf/bpf_tracing.h>
+struct bpf_perf_event_value___local {
+ __u64 counter;
+ __u64 enabled;
+ __u64 running;
+} __attribute__((preserve_access_index));
+
/* map of perf event fds, num_cpu * num_metric entries */
struct {
__uint(type, BPF_MAP_TYPE_PERF_EVENT_ARRAY);
@@ -15,14 +21,14 @@ struct {
struct {
__uint(type, BPF_MAP_TYPE_PERCPU_ARRAY);
__uint(key_size, sizeof(u32));
- __uint(value_size, sizeof(struct bpf_perf_event_value));
+ __uint(value_size, sizeof(struct bpf_perf_event_value___local));
} fentry_readings SEC(".maps");
/* accumulated readings */
struct {
__uint(type, BPF_MAP_TYPE_PERCPU_ARRAY);
__uint(key_size, sizeof(u32));
- __uint(value_size, sizeof(struct bpf_perf_event_value));
+ __uint(value_size, sizeof(struct bpf_perf_event_value___local));
} accum_readings SEC(".maps");
/* sample counts, one per cpu */
@@ -39,7 +45,7 @@ const volatile __u32 num_metric = 1;
SEC("fentry/XXX")
int BPF_PROG(fentry_XXX)
{
- struct bpf_perf_event_value *ptrs[MAX_NUM_MATRICS];
+ struct bpf_perf_event_value___local *ptrs[MAX_NUM_MATRICS];
u32 key = bpf_get_smp_processor_id();
u32 i;
@@ -53,10 +59,10 @@ int BPF_PROG(fentry_XXX)
}
for (i = 0; i < num_metric && i < MAX_NUM_MATRICS; i++) {
- struct bpf_perf_event_value reading;
+ struct bpf_perf_event_value___local reading;
int err;
- err = bpf_perf_event_read_value(&events, key, &reading,
+ err = bpf_perf_event_read_value(&events, key, (void *)&reading,
sizeof(reading));
if (err)
return 0;
@@ -68,14 +74,14 @@ int BPF_PROG(fentry_XXX)
}
static inline void
-fexit_update_maps(u32 id, struct bpf_perf_event_value *after)
+fexit_update_maps(u32 id, struct bpf_perf_event_value___local *after)
{
- struct bpf_perf_event_value *before, diff;
+ struct bpf_perf_event_value___local *before, diff;
before = bpf_map_lookup_elem(&fentry_readings, &id);
/* only account samples with a valid fentry_reading */
if (before && before->counter) {
- struct bpf_perf_event_value *accum;
+ struct bpf_perf_event_value___local *accum;
diff.counter = after->counter - before->counter;
diff.enabled = after->enabled - before->enabled;
@@ -93,7 +99,7 @@ fexit_update_maps(u32 id, struct bpf_perf_event_value *after)
SEC("fexit/XXX")
int BPF_PROG(fexit_XXX)
{
- struct bpf_perf_event_value readings[MAX_NUM_MATRICS];
+ struct bpf_perf_event_value___local readings[MAX_NUM_MATRICS];
u32 cpu = bpf_get_smp_processor_id();
u32 i, zero = 0;
int err;
@@ -102,7 +108,8 @@ int BPF_PROG(fexit_XXX)
/* read all events before updating the maps, to reduce error */
for (i = 0; i < num_metric && i < MAX_NUM_MATRICS; i++) {
err = bpf_perf_event_read_value(&events, cpu + i * num_cpu,
- readings + i, sizeof(*readings));
+ (void *)(readings + i),
+ sizeof(*readings));
if (err)
return 0;
}
--
2.36.0
^ permalink raw reply related
* [PATCH v2 bpf 02/11] bpftool: define a local bpf_perf_link to fix accessing its fields
From: Alexander Lobakin @ 2022-04-21 0:38 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: Alexander Lobakin, Maciej Fijalkowski, Song Liu,
Kumar Kartikeya Dwivedi, bpf, netdev, linux-kernel
In-Reply-To: <20220421003152.339542-1-alobakin@pm.me>
When building bpftool with !CONFIG_PERF_EVENTS:
skeleton/pid_iter.bpf.c:47:14: error: incomplete definition of type 'struct bpf_perf_link'
perf_link = container_of(link, struct bpf_perf_link, link);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/bpf/bpftool/bootstrap/libbpf/include/bpf/bpf_helpers.h:74:22: note: expanded from macro 'container_of'
((type *)(__mptr - offsetof(type, member))); \
^~~~~~~~~~~~~~~~~~~~~~
tools/bpf/bpftool/bootstrap/libbpf/include/bpf/bpf_helpers.h:68:60: note: expanded from macro 'offsetof'
#define offsetof(TYPE, MEMBER) ((unsigned long)&((TYPE *)0)->MEMBER)
~~~~~~~~~~~^
skeleton/pid_iter.bpf.c:44:9: note: forward declaration of 'struct bpf_perf_link'
struct bpf_perf_link *perf_link;
^
&bpf_perf_link is being defined and used only under the ifdef.
Define struct bpf_perf_link___local with the `preserve_access_index`
attribute inside the pid_iter BPF prog to allow compiling on any
configs. CO-RE will substitute it with the real struct bpf_perf_link
accesses later on.
container_of() is not CO-REd, but it is a noop for
bpf_perf_link <-> bpf_link and the local copy is a full mirror of
the original structure.
Fixes: cbdaf71f7e65 ("bpftool: Add bpf_cookie to link output")
Suggested-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Alexander Lobakin <alobakin@pm.me>
---
tools/bpf/bpftool/skeleton/pid_iter.bpf.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/tools/bpf/bpftool/skeleton/pid_iter.bpf.c b/tools/bpf/bpftool/skeleton/pid_iter.bpf.c
index e2af8e5fb29e..3a4c4f7d83d8 100644
--- a/tools/bpf/bpftool/skeleton/pid_iter.bpf.c
+++ b/tools/bpf/bpftool/skeleton/pid_iter.bpf.c
@@ -15,6 +15,11 @@ enum bpf_obj_type {
BPF_OBJ_BTF,
};
+struct bpf_perf_link___local {
+ struct bpf_link link;
+ struct file *perf_file;
+} __attribute__((preserve_access_index));
+
struct perf_event___local {
u64 bpf_cookie;
} __attribute__((preserve_access_index));
@@ -45,10 +50,10 @@ static __always_inline __u32 get_obj_id(void *ent, enum bpf_obj_type type)
/* could be used only with BPF_LINK_TYPE_PERF_EVENT links */
static __u64 get_bpf_cookie(struct bpf_link *link)
{
+ struct bpf_perf_link___local *perf_link;
struct perf_event___local *event;
- struct bpf_perf_link *perf_link;
- perf_link = container_of(link, struct bpf_perf_link, link);
+ perf_link = container_of(link, struct bpf_perf_link___local, link);
event = BPF_CORE_READ(perf_link, perf_file, private_data);
return BPF_CORE_READ(event, bpf_cookie);
}
--
2.36.0
^ permalink raw reply related
* [PATCH v2 bpf 01/11] bpftool: use a local copy of perf_event to fix accessing ::bpf_cookie
From: Alexander Lobakin @ 2022-04-21 0:38 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: Alexander Lobakin, Maciej Fijalkowski, Song Liu,
Kumar Kartikeya Dwivedi, bpf, netdev, linux-kernel
In-Reply-To: <20220421003152.339542-1-alobakin@pm.me>
When CONFIG_PERF_EVENTS is not set, struct perf_event remains empty.
However, the structure is being used by bpftool indirectly via BTF.
This leads to:
skeleton/pid_iter.bpf.c:49:30: error: no member named 'bpf_cookie' in 'struct perf_event'
return BPF_CORE_READ(event, bpf_cookie);
~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
...
skeleton/pid_iter.bpf.c:49:9: error: returning 'void' from a function with incompatible result type '__u64' (aka 'unsigned long long')
return BPF_CORE_READ(event, bpf_cookie);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Tools and samples can't use any CONFIG_ definitions, so the fields
used there should always be present.
Define struct perf_event___local with the `preserve_access_index`
attribute inside the pid_iter BPF prog to allow compiling on any
configs. CO-RE will substitute it with the real struct perf_event
accesses later on.
Fixes: cbdaf71f7e65 ("bpftool: Add bpf_cookie to link output")
Suggested-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Alexander Lobakin <alobakin@pm.me>
---
tools/bpf/bpftool/skeleton/pid_iter.bpf.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/tools/bpf/bpftool/skeleton/pid_iter.bpf.c b/tools/bpf/bpftool/skeleton/pid_iter.bpf.c
index eb05ea53afb1..e2af8e5fb29e 100644
--- a/tools/bpf/bpftool/skeleton/pid_iter.bpf.c
+++ b/tools/bpf/bpftool/skeleton/pid_iter.bpf.c
@@ -15,6 +15,10 @@ enum bpf_obj_type {
BPF_OBJ_BTF,
};
+struct perf_event___local {
+ u64 bpf_cookie;
+} __attribute__((preserve_access_index));
+
extern const void bpf_link_fops __ksym;
extern const void bpf_map_fops __ksym;
extern const void bpf_prog_fops __ksym;
@@ -41,8 +45,8 @@ static __always_inline __u32 get_obj_id(void *ent, enum bpf_obj_type type)
/* could be used only with BPF_LINK_TYPE_PERF_EVENT links */
static __u64 get_bpf_cookie(struct bpf_link *link)
{
+ struct perf_event___local *event;
struct bpf_perf_link *perf_link;
- struct perf_event *event;
perf_link = container_of(link, struct bpf_perf_link, link);
event = BPF_CORE_READ(perf_link, perf_file, private_data);
--
2.36.0
^ permalink raw reply related
* [PATCH v2 bpf 00/11] bpf: random unpopular userspace fixes (32 bit et al)
From: Alexander Lobakin @ 2022-04-21 0:38 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: Alexander Lobakin, Maciej Fijalkowski, Song Liu,
Kumar Kartikeya Dwivedi, bpf, netdev, linux-kernel
This mostly issues the cross build (1) errors for 32 bit (2)
MIPS (3) with minimal configuration (4) on Musl (5). The majority
of them aren't yesterday's, so it is a "who does need it outside
of x86_64 or ARM64?" moment again.
Trivial stuff in general, not counting the first three (they are
50/50).
From v1[0]:
- use *___local struct definitions for BPF programs instead of
BTF_TYPE_EMIT() and ifdef-play (Andrii);
- cast uin64_t to unsigned long long to *really* fix the format
literal warnings (Song, David, Andrii);
- collect Acked-bys for the rest (Maciej, Kumar, Song);
- adjust the subjects to match their usual look (Andrii);
- expand the commit messages a bit for 0008
(-Wshift-count-overflow) and 0010 (-Wsequence-point) a bit to
mention they actually mitigate the third-party issues (Andrii);
- rebase and send to bpf instead of bpf-next (hope the first three
are okay for it).
[0] https://lore.kernel.org/bpf/20220414223704.341028-1-alobakin@pm.me
Alexander Lobakin (11):
bpftool: use a local copy of perf_event to fix accessing ::bpf_cookie
bpftool: define a local bpf_perf_link to fix accessing its fields
bpftool: use a local bpf_perf_event_value to fix accessing its fields
bpftool: fix fcntl.h include
samples/bpf: add 'asm/mach-generic' include path for every MIPS
samples/bpf: use host bpftool to generate vmlinux.h, not target
samples/bpf: fix uin64_t format literals
samples/bpf: fix false-positive right-shift underflow warnings
samples/bpf: fix include order for non-Glibc environments
samples/bpf: fix -Wsequence-point
samples/bpf: xdpsock: fix -Wmaybe-uninitialized
samples/bpf/Makefile | 7 +++---
samples/bpf/cookie_uid_helper_example.c | 12 +++++-----
samples/bpf/lathist_kern.c | 2 +-
samples/bpf/lwt_len_hist_kern.c | 2 +-
samples/bpf/lwt_len_hist_user.c | 7 +++---
samples/bpf/task_fd_query_user.c | 2 +-
samples/bpf/test_lru_dist.c | 3 ++-
samples/bpf/tracex2_kern.c | 2 +-
samples/bpf/xdpsock_user.c | 5 +++--
tools/bpf/bpftool/skeleton/pid_iter.bpf.c | 15 ++++++++++---
tools/bpf/bpftool/skeleton/profiler.bpf.c | 27 ++++++++++++++---------
tools/bpf/bpftool/tracelog.c | 2 +-
12 files changed, 53 insertions(+), 33 deletions(-)
--
2.36.0
^ permalink raw reply
* Re: [PATCH] tcp: md5: incorrect tcp_header_len for incoming connections
From: Eric Dumazet @ 2022-04-21 0:37 UTC (permalink / raw)
To: Francesco Ruggeri
Cc: Paolo Abeni, Jakub Kicinski, David Ahern, Hideaki YOSHIFUJI,
David Miller, LKML, netdev
In-Reply-To: <CA+HUmGiGE9sp3fyfobU6kx73-4zKx=i3KeBHqcCyiru3Z+3jrQ@mail.gmail.com>
On Wed, Apr 20, 2022 at 5:32 PM Francesco Ruggeri <fruggeri@arista.com> wrote:
>
> On Wed, Apr 20, 2022 at 5:20 PM Eric Dumazet <edumazet@google.com> wrote:
> > On Wed, Apr 20, 2022 at 4:57 PM Francesco Ruggeri <fruggeri@arista.com> wrote:
> > This seems like a day-0 bug, right ?
> >
> > Do you agree on adding
> >
> > Fixes: cfb6eeb4c860 ("[TCP]: MD5 Signature Option (RFC2385) support.")
> >
> > Thanks.
> >
> I also think it is a day-0 bug. Should I resubmit with "Fixes:" ?
I think so. It will make things a bit easier for network maintainers,
because I do not think patchwork catches Fixes: tags added during code review.
Also please add the net prefix, as in [PATCH v2 net] title (look at
the warning at https://patchwork.kernel.org/project/netdevbpf/patch/20220420235659.830155EC021C@us226.sjc.aristanetworks.com/
)
Thanks.
^ permalink raw reply
* Re: [PATCH] tcp: md5: incorrect tcp_header_len for incoming connections
From: Francesco Ruggeri @ 2022-04-21 0:32 UTC (permalink / raw)
To: Eric Dumazet
Cc: Paolo Abeni, Jakub Kicinski, David Ahern, Hideaki YOSHIFUJI,
David Miller, LKML, netdev
In-Reply-To: <CANn89iJjwV2gAKMc4iydUt_MqtnB-4_EKdVrqQO9q4Dt17Lf9w@mail.gmail.com>
On Wed, Apr 20, 2022 at 5:20 PM Eric Dumazet <edumazet@google.com> wrote:
> On Wed, Apr 20, 2022 at 4:57 PM Francesco Ruggeri <fruggeri@arista.com> wrote:
> This seems like a day-0 bug, right ?
>
> Do you agree on adding
>
> Fixes: cfb6eeb4c860 ("[TCP]: MD5 Signature Option (RFC2385) support.")
>
> Thanks.
>
I also think it is a day-0 bug. Should I resubmit with "Fixes:" ?
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox