Finally add support for filtering stats values, similar to
non-comparison mode filtering. For comparison mode 4 variants of stats
are important for filtering, as they allow to filter either A or B side,
but even more importantly they allow to filter based on value
difference, and for verdict stat value difference is MATCH/MISMATCH
classification. So with these changes it's finally possible to easily
check if there were any mismatches between failure/success outcomes on
two separate data sets. Like in an example below:
$ ./veristat -e file,prog,verdict,insns -C ~/baseline-results.csv ~/shortest-results.csv -f verdict_diff=mismatch
File Program Verdict (A) Verdict (B) Verdict (DIFF) Insns (A) Insns (B) Insns (DIFF)
------------------------------------- --------------------- ----------- ----------- -------------- --------- --------- -------------------
dynptr_success.bpf.linked1.o test_data_slice success failure MISMATCH 85 0 -85 (-100.00%)
dynptr_success.bpf.linked1.o test_read_write success failure MISMATCH 1992 0 -1992 (-100.00%)
dynptr_success.bpf.linked1.o test_ringbuf success failure MISMATCH 74 0 -74 (-100.00%)
kprobe_multi.bpf.linked1.o test_kprobe failure success MISMATCH 0 246 +246 (+100.00%)
kprobe_multi.bpf.linked1.o test_kprobe_manual failure success MISMATCH 0 246 +246 (+100.00%)
kprobe_multi.bpf.linked1.o test_kretprobe failure success MISMATCH 0 248 +248 (+100.00%)
kprobe_multi.bpf.linked1.o test_kretprobe_manual failure success MISMATCH 0 248 +248 (+100.00%)
kprobe_multi.bpf.linked1.o trigger failure success MISMATCH 0 2 +2 (+100.00%)
netcnt_prog.bpf.linked1.o bpf_nextcnt failure success MISMATCH 0 56 +56 (+100.00%)
pyperf600_nounroll.bpf.linked1.o on_event success failure MISMATCH 568128 1000001 +431873 (+76.02%)
ringbuf_bench.bpf.linked1.o bench_ringbuf success failure MISMATCH 8 0 -8 (-100.00%)
strobemeta.bpf.linked1.o on_event success failure MISMATCH 557149 1000001 +442852 (+79.49%)
strobemeta_nounroll1.bpf.linked1.o on_event success failure MISMATCH 57240 1000001 +942761 (+1647.03%)
strobemeta_nounroll2.bpf.linked1.o on_event success failure MISMATCH 501725 1000001 +498276 (+99.31%)
strobemeta_subprogs.bpf.linked1.o on_event success failure MISMATCH 65420 1000001 +934581 (+1428.59%)
test_map_in_map_invalid.bpf.linked1.o xdp_noop0 success failure MISMATCH 2 0 -2 (-100.00%)
test_mmap.bpf.linked1.o test_mmap success failure MISMATCH 46 0 -46 (-100.00%)
test_verif_scale3.bpf.linked1.o balancer_ingress success failure MISMATCH 845499 1000001 +154502 (+18.27%)
------------------------------------- --------------------- ----------- ----------- -------------- --------- --------- -------------------
Note that by filtering on verdict_diff=mismatch, it's now extremely easy and
fast to see any changes in verdict. Example above showcases both failure ->
success transitions (which are generally surprising) and success -> failure
transitions (which are expected if bugs are present).
Given veristat allows to query relative percent difference values, internal
logic for comparison mode is based on floating point numbers, so requires a bit
of epsilon precision logic, deviating from typical integer simple handling
rules.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/r/20221103055304.2904589-11-andrii@kernel.org
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Introduce the concept of "stat variant", by which it's possible to
specify whether to use the value from A (baseline) side, B (comparison
or control) side, the absolute difference value or relative (percentage)
difference value.
To support specifying this, veristat recognizes `_a`, `_b`, `_diff`,
`_pct` suffixes, which can be appended to stat name(s). In
non-comparison mode variants are ignored (there is only `_a` variant
effectively), if no variant suffix is provided, `_b` is assumed, as
control group is of primary interest in comparison mode.
These stat variants can be flexibly combined with asc/desc orders.
Here's an example of ordering results first by verdict match/mismatch (or n/a
if one of the sides is missing; n/a is always considered to be the lowest
value), and within each match/mismatch/n/a group further sort by number of
instructions in B side. In this case we don't have MISMATCH cases, but N/A are
split from MATCH, demonstrating this custom ordering.
$ ./veristat -e file,prog,verdict,insns -s verdict_diff,insns_b_ -C ~/base.csv ~/comp.csv
File Program Verdict (A) Verdict (B) Verdict (DIFF) Insns (A) Insns (B) Insns (DIFF)
------------------ ------------------------------ ----------- ----------- -------------- --------- --------- --------------
bpf_xdp.o tail_lb_ipv6 N/A success N/A N/A 151895 N/A
bpf_xdp.o tail_nodeport_nat_egress_ipv4 N/A success N/A N/A 15619 N/A
bpf_xdp.o tail_nodeport_ipv6_dsr N/A success N/A N/A 1206 N/A
bpf_xdp.o tail_nodeport_ipv4_dsr N/A success N/A N/A 1162 N/A
bpf_alignchecker.o tail_icmp6_send_echo_reply N/A failure N/A N/A 74 N/A
bpf_alignchecker.o __send_drop_notify success N/A N/A 53 N/A N/A
bpf_host.o __send_drop_notify success N/A N/A 53 N/A N/A
bpf_host.o cil_from_host success N/A N/A 762 N/A N/A
bpf_xdp.o tail_lb_ipv4 success success MATCH 71736 73430 +1694 (+2.36%)
bpf_xdp.o tail_handle_nat_fwd_ipv4 success success MATCH 21547 20920 -627 (-2.91%)
bpf_xdp.o tail_rev_nodeport_lb6 success success MATCH 17954 17905 -49 (-0.27%)
bpf_xdp.o tail_handle_nat_fwd_ipv6 success success MATCH 16974 17039 +65 (+0.38%)
bpf_xdp.o tail_nodeport_nat_ingress_ipv4 success success MATCH 7658 7713 +55 (+0.72%)
bpf_xdp.o tail_rev_nodeport_lb4 success success MATCH 7126 6934 -192 (-2.69%)
bpf_xdp.o tail_nodeport_nat_ingress_ipv6 success success MATCH 6405 6397 -8 (-0.12%)
bpf_xdp.o tail_nodeport_nat_ipv6_egress failure failure MATCH 752 752 +0 (+0.00%)
bpf_xdp.o cil_xdp_entry success success MATCH 423 423 +0 (+0.00%)
bpf_xdp.o __send_drop_notify success success MATCH 151 151 +0 (+0.00%)
bpf_alignchecker.o tail_icmp6_handle_ns failure failure MATCH 33 33 +0 (+0.00%)
------------------ ------------------------------ ----------- ----------- -------------- --------- --------- --------------
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/r/20221103055304.2904589-10-andrii@kernel.org
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
When comparing two datasets, if either side is missing corresponding
record with the same file and prog name, currently veristat emits
misleading zeros/failures, and even tried to calculate a difference,
even though there is no data to compare against.
This patch improves internal logic of handling such situations. Now
we'll emit "N/A" in places where data is missing and comparison is
non-sensical.
As an example, in an artificially truncated and mismatched Cilium
results, the output looks like below:
$ ./veristat -e file,prog,verdict,insns -C ~/base.csv ~/comp.csv
File Program Verdict (A) Verdict (B) Verdict (DIFF) Insns (A) Insns (B) Insns (DIFF)
------------------ ------------------------------ ----------- ----------- -------------- --------- --------- --------------
bpf_alignchecker.o __send_drop_notify success N/A N/A 53 N/A N/A
bpf_alignchecker.o tail_icmp6_handle_ns failure failure MATCH 33 33 +0 (+0.00%)
bpf_alignchecker.o tail_icmp6_send_echo_reply N/A failure N/A N/A 74 N/A
bpf_host.o __send_drop_notify success N/A N/A 53 N/A N/A
bpf_host.o cil_from_host success N/A N/A 762 N/A N/A
bpf_xdp.o __send_drop_notify success success MATCH 151 151 +0 (+0.00%)
bpf_xdp.o cil_xdp_entry success success MATCH 423 423 +0 (+0.00%)
bpf_xdp.o tail_handle_nat_fwd_ipv4 success success MATCH 21547 20920 -627 (-2.91%)
bpf_xdp.o tail_handle_nat_fwd_ipv6 success success MATCH 16974 17039 +65 (+0.38%)
bpf_xdp.o tail_lb_ipv4 success success MATCH 71736 73430 +1694 (+2.36%)
bpf_xdp.o tail_lb_ipv6 N/A success N/A N/A 151895 N/A
bpf_xdp.o tail_nodeport_ipv4_dsr N/A success N/A N/A 1162 N/A
bpf_xdp.o tail_nodeport_ipv6_dsr N/A success N/A N/A 1206 N/A
bpf_xdp.o tail_nodeport_nat_egress_ipv4 N/A success N/A N/A 15619 N/A
bpf_xdp.o tail_nodeport_nat_ingress_ipv4 success success MATCH 7658 7713 +55 (+0.72%)
bpf_xdp.o tail_nodeport_nat_ingress_ipv6 success success MATCH 6405 6397 -8 (-0.12%)
bpf_xdp.o tail_nodeport_nat_ipv6_egress failure failure MATCH 752 752 +0 (+0.00%)
bpf_xdp.o tail_rev_nodeport_lb4 success success MATCH 7126 6934 -192 (-2.69%)
bpf_xdp.o tail_rev_nodeport_lb6 success success MATCH 17954 17905 -49 (-0.27%)
------------------ ------------------------------ ----------- ----------- -------------- --------- --------- --------------
Internally veristat now separates joining two datasets and remembering the
join, and actually emitting a comparison view. This will come handy when we add
support for filtering and custom ordering in comparison mode.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/r/20221103055304.2904589-9-andrii@kernel.org
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Make veristat distinguish between table and CSV output formats and use
different default set of stats (columns) that are emitted. While for
human-readable table output it doesn't make sense to output all known
stats, it is very useful for CSV mode to record all possible data, so
that it can later be queried and filtered in replay or comparison mode.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/r/20221103055304.2904589-8-andrii@kernel.org
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Define simple expressions to filter not just by file and program name,
but also by resulting values of collected stats. Support usual
equality and inequality operators. Verdict, which is a boolean-like
field can be also filtered either as 0/1, failure/success (with f/s,
fail/succ, and failure/success aliases) symbols, or as false/true (f/t).
Aliases are case insensitive.
Currently this filtering is honored only in verification and replay
modes. Comparison mode support will be added in next patch.
Here's an example of verifying a bunch of BPF object files and emitting
only results for successfully validated programs that have more than 100
total instructions processed by BPF verifier, sorted by number of
instructions in ascending order:
$ sudo ./veristat *.bpf.o -s insns^ -f 'insns>100'
There can be many filters (both allow and deny flavors), all of them are
combined.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/r/20221103055304.2904589-7-andrii@kernel.org
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Allow to specify '^' at the end of stat name to designate that it should
be sorted in ascending order. Similarly, allow any of 'v', 'V', '.',
'!', or '_' suffix "symbols" to designate descending order. It's such
a zoo for descending order because there is no single intuitive symbol
that could be used (using 'v' looks pretty weird in practice), so few
symbols that are "downwards leaning or pointing" were chosen. Either
way, it shouldn't cause any troubles in practice.
This new feature allows to customize sortering order to match user's
needs.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/r/20221103055304.2904589-6-andrii@kernel.org
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Always fall back to unique file/prog comparison if user's custom order
specs are ambiguous. This ensures stable output no matter what.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/r/20221103055304.2904589-5-andrii@kernel.org
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Slightly change rules of specifying file/prog glob filters. In practice
it's quite often inconvenient to do `*/<prog-glob>` if that program glob
is unique enough and won't accidentally match any file names.
This patch changes the rules so that `-f <glob>` will apply specified
glob to both file and program names. User still has all the control by
doing '*/<prog-only-glob>' or '<file-only-glob/*'. We also now allow
'/<prog-glob>' and '<file-glob/' (all matching wildcard is assumed if
missing).
Also, internally unify file-only and file+prog checks
(should_process_file and should_process_prog are now
should_process_file_prog that can handle prog name as optional). This
makes maintaining and extending this code easier.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/r/20221103055304.2904589-4-andrii@kernel.org
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
In comparison mode the "Total " part is pretty useless, but takes
a considerable amount of horizontal space. Drop the "Total " parts.
Also make sure that table headers for numerical columns are aligned in
the same fashion as integer values in those columns. This looks better
and is now more obvious with shorter "Insns" and "States" column
headers.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/r/20221103055304.2904589-3-andrii@kernel.org
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Replay mode allow to parse previously stored CSV file with verification
results and present it in desired output (presumable human-readable
table, but CSV to CSV convertion is supported as well). While doing
that, it's possible to use veristat's sorting rules, specify subset of
columns, and filter by file and program name.
In subsequent patches veristat's filtering capabilities will just grow
making replay mode even more useful in practice for post-processing
results.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/r/20221103055304.2904589-2-andrii@kernel.org
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
To prepare the BPF verifier to handle special fields in both map values
and program allocated types coming from program BTF, we need to refactor
the kptr_off_tab handling code into something more generic and reusable
across both cases to avoid code duplication.
Later patches also require passing this data to helpers at runtime, so
that they can work on user defined types, initialize them, destruct
them, etc.
The main observation is that both map values and such allocated types
point to a type in program BTF, hence they can be handled similarly. We
can prepare a field metadata table for both cases and store them in
struct bpf_map or struct btf depending on the use case.
Hence, refactor the code into generic btf_record and btf_field member
structs. The btf_record represents the fields of a specific btf_type in
user BTF. The cnt indicates the number of special fields we successfully
recognized, and field_mask is a bitmask of fields that were found, to
enable quick determination of availability of a certain field.
Subsequently, refactor the rest of the code to work with these generic
types, remove assumptions about kptr and kptr_off_tab, rename variables
to more meaningful names, etc.
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Link: https://lore.kernel.org/r/20221103191013.1236066-7-memxor@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Tony Nguyen says:
====================
Intel Wired LAN Driver Updates 2022-11-02 (i40e, iavf)
This series contains updates to i40e and iavf drivers.
Joe Damato adds tracepoint information to i40e_napi_poll to expose helpful
debug information for users who'd like to get a better understanding of
how their NIC is performing as they adjust various parameters and tuning
knobs.
Note: this does not touch any XDP related code paths. This
tracepoint will only work when not using XDP. Care has been taken to avoid
changing control flow in i40e_napi_poll with this change.
Alicja adds error messaging for unsupported duplex settings for i40e.
Ye Xingchen replaces use of __FUNCTION__ with __func__ for iavf.
Bartosz changes tense of device removal message to be more clear on the
action for iavf.
* '40GbE' of git://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue:
iavf: Change information about device removal in dmesg
iavf: Replace __FUNCTION__ with __func__
i40e: Add appropriate error message logged for incorrect duplex setting
i40e: Add i40e_napi_poll tracepoint
i40e: Record number of RXes cleaned during NAPI
i40e: Record number TXes cleaned during NAPI
i40e: Store the irq number in i40e_q_vector
====================
Link: https://lore.kernel.org/r/20221102211011.2944983-1-anthony.l.nguyen@intel.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Tony Nguyen says:
====================
Intel Wired LAN Driver Updates 2022-11-02 (e1000e, e1000, igc)
This series contains updates to e1000e, e1000, and igc drivers.
For e1000e, Sasha adds a new board type to help distinguish platforms and
adds device id support for upcoming platforms. He also adds trace points
for CSME flows to aid in debugging.
Ani removes unnecessary kmap_atomic call for e1000 and e1000e.
Muhammad sets speed based transmit offsets for launchtime functionality to
reduce latency for igc.
* '1GbE' of git://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue:
igc: Correct the launchtime offset
e1000: Remove unnecessary use of kmap_atomic()
e1000e: Remove unnecessary use of kmap_atomic()
e1000e: Add e1000e trace module
e1000e: Add support for the next LOM generation
e1000e: Separate MTP board type from ADP
====================
Link: https://lore.kernel.org/r/20221102203957.2944396-1-anthony.l.nguyen@intel.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Second set of fixes for v6.1. Some fixes to char type usage in
drivers, memory leaks in the stack and also functionality fixes. The
rt2x00 char type fix is a larger (but still simple) commit, otherwise
the fixes are small in size.
-----BEGIN PGP SIGNATURE-----
iQFFBAABCgAvFiEEiBjanGPFTz4PRfLobhckVSbrbZsFAmNjt5ERHGt2YWxvQGtl
cm5lbC5vcmcACgkQbhckVSbrbZtkuQf8D3n5ugV/lWB0/YbpSWgWDzw2LyPYTcnh
1kqnCv/Zl7CDh/cwDpW1p0X7bMUv3bQpgVr/nWLMfgwygvWWR8GJENoaNOicCW0H
dj3cofnrv7T2VfJgmNwxRdNTh7LJaolWiDu2AwVxFEr0UeiBgmJQLgLiwBqOmWIo
RbpjUN7KvrUhZQsO1k9UtmOt5w1d/aOnopmP5EjoNnxEInA24g3V5yt51ykBoLMD
ZNJwKGGzFM9qc5bQgGyGPfKlfNEjUYZCXvEBjZq+cg1OtwbYJRjtovyC5L/pVTBM
vu8ORU7uLpJnCX15FvfE5cB32GtvJhShA9ExXQNsCSeXKZitHjlbnQ==
=O94P
-----END PGP SIGNATURE-----
Merge tag 'wireless-2022-11-03' of git://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless
Kalle Valo says:
====================
wireless fixes for v6.1
Second set of fixes for v6.1. Some fixes to char type usage in
drivers, memory leaks in the stack and also functionality fixes. The
rt2x00 char type fix is a larger (but still simple) commit, otherwise
the fixes are small in size.
* tag 'wireless-2022-11-03' of git://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless:
wifi: ath11k: avoid deadlock during regulatory update in ath11k_regd_update()
wifi: ath11k: Fix QCN9074 firmware boot on x86
wifi: mac80211: Set TWT Information Frame Disabled bit as 1
wifi: mac80211: Fix ack frame idr leak when mesh has no route
wifi: mac80211: fix general-protection-fault in ieee80211_subif_start_xmit()
wifi: brcmfmac: Fix potential buffer overflow in brcmf_fweh_event_worker()
wifi: airo: do not assign -1 to unsigned char
wifi: mac80211_hwsim: fix debugfs attribute ps with rc table support
wifi: cfg80211: Fix bitrates overflow issue
wifi: cfg80211: fix memory leak in query_regdb_file()
wifi: mac80211: fix memory free error when registering wiphy fail
wifi: cfg80211: silence a sparse RCU warning
wifi: rt2x00: use explicitly signed or unsigned types
====================
Link: https://lore.kernel.org/r/20221103125315.04E57C433C1@smtp.kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Since commit 3dcbdb134f ("net: gso: Fix skb_segment splat when
splitting gso_size mangled skb having linear-headed frag_list"), it is
allowed to change gso_size of a GRO packet. However, that commit assumes
that "checking the first list_skb member suffices; i.e if either of the
list_skb members have non head_frag head, then the first one has too".
It turns out this assumption does not hold. We've seen BUG_ON being hit
in skb_segment when skbs on the frag_list had differing head_frag with
the vmxnet3 driver. This happens because __netdev_alloc_skb and
__napi_alloc_skb can return a skb that is page backed or kmalloced
depending on the requested size. As the result, the last small skb in
the GRO packet can be kmalloced.
There are three different locations where this can be fixed:
(1) We could check head_frag in GRO and not allow GROing skbs with
different head_frag. However, that would lead to performance
regression on normal forward paths with unmodified gso_size, where
!head_frag in the last packet is not a problem.
(2) Set a flag in bpf_skb_net_grow and bpf_skb_net_shrink indicating
that NETIF_F_SG is undesirable. That would need to eat a bit in
sk_buff. Furthermore, that flag can be unset when all skbs on the
frag_list are page backed. To retain good performance,
bpf_skb_net_grow/shrink would have to walk the frag_list.
(3) Walk the frag_list in skb_segment when determining whether
NETIF_F_SG should be cleared. This of course slows things down.
This patch implements (3). To limit the performance impact in
skb_segment, the list is walked only for skbs with SKB_GSO_DODGY set
that have gso_size changed. Normal paths thus will not hit it.
We could check only the last skb but since we need to walk the whole
list anyway, let's stay on the safe side.
Fixes: 3dcbdb134f ("net: gso: Fix skb_segment splat when splitting gso_size mangled skb having linear-headed frag_list")
Signed-off-by: Jiri Benc <jbenc@redhat.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
Link: https://lore.kernel.org/r/e04426a6a91baf4d1081e1b478c82b5de25fdf21.1667407944.git.jbenc@redhat.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
With clang's kernel control flow integrity (kCFI, CONFIG_CFI_CLANG),
indirect call targets are validated against the expected function
pointer prototype to make sure the call target is valid to help mitigate
ROP attacks. If they are not identical, there is a failure at run time,
which manifests as either a kernel panic or thread getting killed. A
proposed warning in clang aims to catch these at compile time, which
reveals:
drivers/net/hamradio/baycom_epp.c:1119:25: error: incompatible function pointer types initializing 'netdev_tx_t (*)(struct sk_buff *, struct net_device *)' (aka 'enum netdev_tx (*)(struct sk_buff *, struct net_device *)') with an expression of type 'int (struct sk_buff *, struct net_device *)' [-Werror,-Wincompatible-function-pointer-types-strict]
.ndo_start_xmit = baycom_send_packet,
^~~~~~~~~~~~~~~~~~
1 error generated.
->ndo_start_xmit() in 'struct net_device_ops' expects a return type of
'netdev_tx_t', not 'int'. Adjust the return type of baycom_send_packet()
to match the prototype's to resolve the warning and CFI failure.
Link: https://github.com/ClangBuiltLinux/linux/issues/1750
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Reviewed-by: Kees Cook <keescook@chromium.org>
Link: https://lore.kernel.org/r/20221102160610.1186145-1-nathan@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
With clang's kernel control flow integrity (kCFI, CONFIG_CFI_CLANG),
indirect call targets are validated against the expected function
pointer prototype to make sure the call target is valid to help mitigate
ROP attacks. If they are not identical, there is a failure at run time,
which manifests as either a kernel panic or thread getting killed. A
proposed warning in clang aims to catch these at compile time, which
reveals:
drivers/net/ethernet/ti/netcp_core.c:1944:21: error: incompatible function pointer types initializing 'netdev_tx_t (*)(struct sk_buff *, struct net_device *)' (aka 'enum netdev_tx (*)(struct sk_buff *, struct net_device *)') with an expression of type 'int (struct sk_buff *, struct net_device *)' [-Werror,-Wincompatible-function-pointer-types-strict]
.ndo_start_xmit = netcp_ndo_start_xmit,
^~~~~~~~~~~~~~~~~~~~
1 error generated.
->ndo_start_xmit() in 'struct net_device_ops' expects a return type of
'netdev_tx_t', not 'int'. Adjust the return type of
netcp_ndo_start_xmit() to match the prototype's to resolve the warning
and CFI failure.
Link: https://github.com/ClangBuiltLinux/linux/issues/1750
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Reviewed-by: Kees Cook <keescook@chromium.org>
Link: https://lore.kernel.org/r/20221102160933.1601260-1-nathan@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
strtobool() is the same as kstrtobool().
However, the latter is more used within the kernel.
In order to remove strtobool() and slightly simplify kstrtox.h, switch to
the other function name.
While at it, include the corresponding header file (<linux/kstrtox.h>).
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Link: https://lore.kernel.org/r/d4432a67b6f769cac0a9ec910ac725298b64e102.1667336095.git.christophe.jaillet@wanadoo.fr
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Jiri Pirko says:
====================
net: fix netdev to devlink_port linkage and expose to user
Currently, the info about linkage from netdev to the related
devlink_port instance is done using ndo_get_devlink_port().
This is not sufficient, as it is up to the driver to implement it and
some of them don't do that. Also it leads to a lot of unnecessary
boilerplate code in all the drivers.
Instead of that, introduce a possibility for driver to expose this
relationship by new SET_NETDEV_DEVLINK_PORT macro which stores it into
dev->devlink_port. It is ensured by the driver init/fini flows that
the devlink_port pointer does not change during the netdev lifetime.
Devlink port is always registered before netdev register and
unregistered after netdev unregister.
Benefit from this linkage setup and remove explicit calls from driver
to devlink_port_type_eth_set() and clear(). Many of the driver
didn't use it correctly anyway. Let the devlink.c to track associated
netdev events and adjust type and type pointer accordingly. Also
use this events to to keep track on ifname change and remove RTNL lock
taking from devlink_nl_port_fill().
Finally, remove the ndo_get_devlink_port() ndo which is no longer used
and expose devlink_port handle as a new netdev netlink attribute to the
user. That way, during the ifname->devlink_port lookup, userspace app
does not have to dump whole devlink port list and instead it can just
do a simple RTM_GETLINK query.
====================
Link: https://lore.kernel.org/r/20221102160211.662752-1-jiri@resnulli.us
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Expose devlink port handle related to netdev over rtnetlink. Introduce a
new nested IFLA attribute to carry the info. Call into devlink code to
fill-up the nest with existing devlink attributes that are used over
devlink netlink.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Remove ndo_get_devlink_port which is no longer used alongside with the
implementations in drivers.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Use newly introduced devlink_port pointer instead of getting it calling
to ndo_get_devlink_port op.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
By the time port unregister is called. There should be no type set. Make
sure that the driver cleared it before and warn in case it didn't. This
enforces symmetricity with type set and port register.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
To avoid a need to take RTNL mutex in port_fill() function, benefit from
the introduce infrastructure that tracks netdevice notifier events.
Store the ifindex and ifname upon register and change name events.
Remove the rtnl_held bool propagated down to port_fill() function as it
is no longer needed.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
It is ensured by the netdevice notifier event processing, that only
netdev pointers from the same net namespaces are filled. Remove the
net namespace check from devlink_nl_port_fill() as it is no longer
needed.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Since devlink_port_type_eth_set() should no longer be called by any
driver with netdev pointer as it should rather use
SET_NETDEV_DEVLINK_PORT, remove the netdev arg. Add a warn to
type_clear()
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Benefit from the previously implemented tracking of netdev events in
devlink code and instead of calling devlink_port_type_eth_set() and
devlink_port_type_clear() to set devlink port type and link to related
netdev, use SET_NETDEV_DEVLINK_PORT() macro to assign devlink_port
pointer to netdevice which is about to be registered.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Currently, ethernet drivers are using devlink_port_type_eth_set() and
devlink_port_type_clear() to set devlink port type and link to related
netdev.
Instead of calling them directly, let the driver use
SET_NETDEV_DEVLINK_PORT macro to assign devlink_port pointer and let
devlink to track it. Note the devlink port pointer is static during
the time netdevice is registered.
In devlink code, use per-namespace netdev notifier to track
the netdevices with devlink_port assigned and change the internal
devlink_port type and related type pointer accordingly.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Follow-up patch is going to introduce a netdevice notifier event
processing which is called with RTNL mutex held. Processing of this will
eventually lead to call to port_notity() and port_fill() which currently
takes RTNL mutex internally. So as a temporary solution, propagate a
bool indicating if the mutex is already held. This will go away in one
of the follow-up patches.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
As __devlink_port_type_set() is going to be called directly from netdevice
notifier event handle in one of the follow-up patches, move the
port_type_netdev_checks() call there.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
As __devlink_port_type_set() is going to be called directly from netdevice
notifier event handle in one of the follow-up patches, move the
port_type_warn_schedule() call there.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Instead of storing type_dev as a void pointer, convert it to union and
use it to store either struct net_device or struct ib_device pointer.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Ido Schimmel says:
====================
bridge: Add MAC Authentication Bypass (MAB) support
Patch #1 adds MAB support in the bridge driver. See the commit message
for motivation, design choices and implementation details.
Patch #2 adds corresponding test cases.
Follow-up patchsets will add offload support in mlxsw and mv88e6xxx.
====================
Link: https://lore.kernel.org/r/20221101193922.2125323-1-idosch@nvidia.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Add four test cases to verify MAB functionality:
* Verify that a locked FDB entry can be generated by the bridge,
preventing a host from communicating via the bridge. Test that user
space can clear the "locked" flag by replacing the entry, thereby
authenticating the host and allowing it to communicate via the bridge.
* Test that an entry cannot roam to a locked port, but that it can roam
to an unlocked port.
* Test that MAB can only be enabled on a port that is both locked and
has learning enabled.
* Test that locked FDB entries are flushed from a port when MAB is
disabled.
Signed-off-by: Hans J. Schultz <netdev@kapio-technology.com>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Acked-by: Nikolay Aleksandrov <razor@blackwall.org>
Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Hosts that support 802.1X authentication are able to authenticate
themselves by exchanging EAPOL frames with an authenticator (Ethernet
bridge, in this case) and an authentication server. Access to the
network is only granted by the authenticator to successfully
authenticated hosts.
The above is implemented in the bridge using the "locked" bridge port
option. When enabled, link-local frames (e.g., EAPOL) can be locally
received by the bridge, but all other frames are dropped unless the host
is authenticated. That is, unless the user space control plane installed
an FDB entry according to which the source address of the frame is
located behind the locked ingress port. The entry can be dynamic, in
which case learning needs to be enabled so that the entry will be
refreshed by incoming traffic.
There are deployments in which not all the devices connected to the
authenticator (the bridge) support 802.1X. Such devices can include
printers and cameras. One option to support such deployments is to
unlock the bridge ports connecting these devices, but a slightly more
secure option is to use MAB. When MAB is enabled, the MAC address of the
connected device is used as the user name and password for the
authentication.
For MAB to work, the user space control plane needs to be notified about
MAC addresses that are trying to gain access so that they will be
compared against an allow list. This can be implemented via the regular
learning process with the sole difference that learned FDB entries are
installed with a new "locked" flag indicating that the entry cannot be
used to authenticate the device. The flag cannot be set by user space,
but user space can clear the flag by replacing the entry, thereby
authenticating the device.
Locked FDB entries implement the following semantics with regards to
roaming, aging and forwarding:
1. Roaming: Locked FDB entries can roam to unlocked (authorized) ports,
in which case the "locked" flag is cleared. FDB entries cannot roam
to locked ports regardless of MAB being enabled or not. Therefore,
locked FDB entries are only created if an FDB entry with the given {MAC,
VID} does not already exist. This behavior prevents unauthenticated
devices from disrupting traffic destined to already authenticated
devices.
2. Aging: Locked FDB entries age and refresh by incoming traffic like
regular entries.
3. Forwarding: Locked FDB entries forward traffic like regular entries.
If user space detects an unauthorized MAC behind a locked port and
wishes to prevent traffic with this MAC DA from reaching the host, it
can do so using tc or a different mechanism.
Enable the above behavior using a new bridge port option called "mab".
It can only be enabled on a bridge port that is both locked and has
learning enabled. Locked FDB entries are flushed from the port once MAB
is disabled. A new option is added because there are pure 802.1X
deployments that are not interested in notifications about locked FDB
entries.
Signed-off-by: Hans J. Schultz <netdev@kapio-technology.com>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Acked-by: Nikolay Aleksandrov <razor@blackwall.org>
Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
-----BEGIN PGP SIGNATURE-----
iHUEABYIAB0WIQTFp0I1jqZrAX+hPRXbK58LschIgwUCY2RS7QAKCRDbK58LschI
g6RVAQC1FdSXMrhn369NGCG1Vox1QYn2/5P32LSIV1BKqiQsywEAsxgYNrdCPTua
ie91Q5IJGT9pFl1UR50UrgL11DI5BgI=
=sdhO
-----END PGP SIGNATURE-----
Merge tag 'for-netdev' of https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf
Daniel Borkmann says:
====================
bpf 2022-11-04
We've added 8 non-merge commits during the last 3 day(s) which contain
a total of 10 files changed, 113 insertions(+), 16 deletions(-).
The main changes are:
1) Fix memory leak upon allocation failure in BPF verifier's stack state
tracking, from Kees Cook.
2) Fix address leakage when BPF progs release reference to an object,
from Youlin Li.
3) Fix BPF CI breakage from buggy in.h uapi header dependency,
from Andrii Nakryiko.
4) Fix bpftool pin sub-command's argument parsing, from Pu Lehui.
5) Fix BPF sockmap lockdep warning by cancelling psock work outside
of socket lock, from Cong Wang.
6) Follow-up for BPF sockmap to fix sk_forward_alloc accounting,
from Wang Yufen.
bpf-for-netdev
* tag 'for-netdev' of https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf:
selftests/bpf: Add verifier test for release_reference()
bpf: Fix wrong reg type conversion in release_reference()
bpf, sock_map: Move cancel_work_sync() out of sock lock
tools/headers: Pull in stddef.h to uapi to fix BPF selftests build in CI
net/ipv4: Fix linux/in.h header dependencies
bpftool: Fix NULL pointer dereference when pin {PROG, MAP, LINK} without FILE
bpf, sockmap: Fix the sk->sk_forward_alloc warning of sk_stream_kill_queues
bpf, verifier: Fix memory leak in array reallocation for stack state
====================
Link: https://lore.kernel.org/r/20221104000445.30761-1-daniel@iogearbox.net
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
McASP nodes defined in the top-level AM65x SoC dtsi files are incomplete
and will not be functional unless they are extended with pinmux
information.
As the pinmux is only known at the board integration level, these
nodes should only be enabled when provided with this information.
Disable the McASP nodes in the dtsi files and only enable the ones that
are actually pinned out on a given board.
Signed-off-by: Andrew Davis <afd@ti.com>
Signed-off-by: Nishanth Menon <nm@ti.com>
Tested-by: Jan Kiszka <jan.kiszka@siemens.com>
Reviewed-by: Bryan Brattlof <bb@ti.com>
Link: https://lore.kernel.org/r/20221028142417.10642-12-afd@ti.com
Mailbox nodes defined in the top-level AM65x SoC dtsi files are incomplete
and may not be functional unless they are extended with a chosen interrupt
and connection to a remote processor.
As the remote processors depend on memory nodes which are only known at
the board integration level, these nodes should only be enabled when
provided with the above information.
Disable the Mailbox nodes in the dtsi files and only enable the ones that
are actually used on a given board.
Signed-off-by: Andrew Davis <afd@ti.com>
Signed-off-by: Nishanth Menon <nm@ti.com>
Tested-by: Jan Kiszka <jan.kiszka@siemens.com>
Reviewed-by: Bryan Brattlof <bb@ti.com>
Link: https://lore.kernel.org/r/20221028142417.10642-11-afd@ti.com
PCIe nodes defined in the top-level AM65x SoC dtsi files are incomplete
and will not be functional unless they are extended with a SerDes PHY.
And usually only one of the two modes can be used at a time as they
share a SerDes link.
As the PHY and mode is only known at the board integration level, these
nodes should only be enabled when provided with this information.
Disable the PCIe nodes in the dtsi files and only enable the ones that
are actually pinned out on a given board.
Signed-off-by: Andrew Davis <afd@ti.com>
Signed-off-by: Nishanth Menon <nm@ti.com>
Tested-by: Jan Kiszka <jan.kiszka@siemens.com>
Reviewed-by: Bryan Brattlof <bb@ti.com>
Link: https://lore.kernel.org/r/20221028142417.10642-10-afd@ti.com
MCAN nodes defined in the top-level AM65x SoC dtsi files are incomplete
and will not be functional unless they are extended with pinmux
information.
As the pinmux is only known at the board integration level, these
nodes should only be enabled when provided with this information.
Disable the MCAN nodes in the dtsi files and only enable the ones that
are actually pinned out on a given board.
Signed-off-by: Andrew Davis <afd@ti.com>
Signed-off-by: Nishanth Menon <nm@ti.com>
Tested-by: Jan Kiszka <jan.kiszka@siemens.com>
Reviewed-by: Bryan Brattlof <bb@ti.com>
Link: https://lore.kernel.org/r/20221028142417.10642-9-afd@ti.com
MDIO nodes defined in the top-level AM65x SoC dtsi files are incomplete
and will not be functional unless they are extended with a pinmux.
As the attached PHY is only known about at the board integration level,
these nodes should only be enabled when provided with this information.
Disable the MDIO nodes in the dtsi files and only enable the ones that
are actually pinned out on a given board.
Signed-off-by: Andrew Davis <afd@ti.com>
Signed-off-by: Nishanth Menon <nm@ti.com>
Tested-by: Jan Kiszka <jan.kiszka@siemens.com>
Reviewed-by: Bryan Brattlof <bb@ti.com>
Link: https://lore.kernel.org/r/20221028142417.10642-8-afd@ti.com
Although usually integrated as a child of an Ethernet controller, MDIO
IP has an independent pinout. This pinout should be controlled by
the MDIO node (so if it was to be disabled for instance, the pinmux
state would reflect that).
Move the MDIO pins pinmux to the MIDO nodes.
Signed-off-by: Andrew Davis <afd@ti.com>
Signed-off-by: Nishanth Menon <nm@ti.com>
Tested-by: Jan Kiszka <jan.kiszka@siemens.com>
Reviewed-by: Bryan Brattlof <bb@ti.com>
Link: https://lore.kernel.org/r/20221028142417.10642-7-afd@ti.com
ECAP nodes defined in the top-level AM65x SoC dtsi files are incomplete
and will not be functional unless they are extended with pinmux
information. (These and the EPWM nodes could be used to trigger internal
actions but they are not used like that currently)
As the pinmux is only known at the board integration level, these
nodes should only be enabled when provided with this information.
Disable the ECAP nodes in the dtsi files and only enable the ones that
are actually pinned out on a given board.
Signed-off-by: Andrew Davis <afd@ti.com>
Signed-off-by: Nishanth Menon <nm@ti.com>
Tested-by: Jan Kiszka <jan.kiszka@siemens.com>
Reviewed-by: Bryan Brattlof <bb@ti.com>
Link: https://lore.kernel.org/r/20221028142417.10642-6-afd@ti.com
EPWM nodes defined in the top-level AM65x SoC dtsi files are incomplete
and will not be functional unless they are extended with pinmux
information.
As the pinmux is only known at the board integration level, these
nodes should only be enabled when provided with this information.
Disable the EPWM nodes in the dtsi files and only enable the ones that
are actually pinned out on a given board.
Signed-off-by: Andrew Davis <afd@ti.com>
Signed-off-by: Nishanth Menon <nm@ti.com>
Tested-by: Jan Kiszka <jan.kiszka@siemens.com>
Reviewed-by: Bryan Brattlof <bb@ti.com>
Link: https://lore.kernel.org/r/20221028142417.10642-5-afd@ti.com
SPI nodes defined in the top-level AM65x SoC dtsi files are incomplete
and will not be functional unless they are extended with pinmux
information.
As the pinmux is only known at the board integration level, these
nodes should only be enabled when provided with this information.
Disable the SPI nodes in the dtsi files and only enable the ones that
are actually pinned out on a given board.
Signed-off-by: Andrew Davis <afd@ti.com>
Signed-off-by: Nishanth Menon <nm@ti.com>
Tested-by: Jan Kiszka <jan.kiszka@siemens.com>
Reviewed-by: Bryan Brattlof <bb@ti.com>
Link: https://lore.kernel.org/r/20221028142417.10642-4-afd@ti.com
I2C nodes defined in the top-level AM65x SoC dtsi files are incomplete
and will not be functional unless they are extended with pinmux
information.
As the pinmux is only known at the board integration level, these
nodes should only be enabled when provided with this information.
Disable the I2C nodes in the dtsi files and only enable the ones that
are actually pinned out on a given board.
Signed-off-by: Andrew Davis <afd@ti.com>
Signed-off-by: Nishanth Menon <nm@ti.com>
Tested-by: Jan Kiszka <jan.kiszka@siemens.com>
Reviewed-by: Bryan Brattlof <bb@ti.com>
Link: https://lore.kernel.org/r/20221028142417.10642-3-afd@ti.com
UART nodes defined in the top-level AM65x SoC dtsi files are incomplete
and may not be functional unless they are extended with pinmux
information.
As the pinmux is only known at the board integration level, these
nodes should only be enabled when provided with this information.
Disable the UART nodes in the dtsi files and only enable the ones that
are actually pinned out on a given board.
Signed-off-by: Andrew Davis <afd@ti.com>
Signed-off-by: Nishanth Menon <nm@ti.com>
Tested-by: Jan Kiszka <jan.kiszka@siemens.com>
Reviewed-by: Bryan Brattlof <bb@ti.com>
Link: https://lore.kernel.org/r/20221028142417.10642-2-afd@ti.com
UAPI Changes:
Cross-subsystem Changes:
- dma-buf: locking improvements
- firmware: New API in the RaspberryPi firmware driver used by vc4
Core Changes:
- client: Null pointer dereference fix in drm_client_buffer_delete()
- mm/buddy: Add back random seed log
- ttm: Convert ttm_resource to use size_t for its size, fix for an
undefined behaviour
Driver Changes:
- bridge:
- adv7511: use dev_err_probe
- it6505: Fix return value check of pm_runtime_get_sync
- panel:
- sitronix: Fixes and clean-ups
- lcdif: Increase DMA burst size
- rockchip: runtime_pm improvements
- vc4: Fix for a regression preventing the use of 4k @ 60Hz, and
further HDMI rate constraints check.
- vmwgfx: Cursor improvements
-----BEGIN PGP SIGNATURE-----
iHUEABYKAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCY2N8/gAKCRDj7w1vZxhR
xT1TAQDiwdSyL/bzOk/WYggmj5wmFqb2PPFbFRt/DOTLl52ZdwEAmtE9I2WKvY8w
sxZYF9gfFnQC3id7YwNs+CDb1kAangc=
=6GBd
-----END PGP SIGNATURE-----
Merge tag 'drm-misc-next-2022-11-03' of git://anongit.freedesktop.org/drm/drm-misc into drm-next
drm-misc-next for 6.2:
UAPI Changes:
Cross-subsystem Changes:
- dma-buf: locking improvements
- firmware: New API in the RaspberryPi firmware driver used by vc4
Core Changes:
- client: Null pointer dereference fix in drm_client_buffer_delete()
- mm/buddy: Add back random seed log
- ttm: Convert ttm_resource to use size_t for its size, fix for an
undefined behaviour
Driver Changes:
- bridge:
- adv7511: use dev_err_probe
- it6505: Fix return value check of pm_runtime_get_sync
- panel:
- sitronix: Fixes and clean-ups
- lcdif: Increase DMA burst size
- rockchip: runtime_pm improvements
- vc4: Fix for a regression preventing the use of 4k @ 60Hz, and
further HDMI rate constraints check.
- vmwgfx: Cursor improvements
Signed-off-by: Dave Airlie <airlied@redhat.com>
From: Maxime Ripard <maxime@cerno.tech>
Link: https://patchwork.freedesktop.org/patch/msgid/20221103083437.ksrh3hcdvxaof62l@houat
