Message ID | 20220414223704.341028-12-alobakin@pm.me (mailing list archive) |
---|---|
State | Changes Requested |
Delegated to: | BPF |
Headers | show |
Series | bpf: random unpopular userspace fixes (32 bit et al.) | expand |
Context | Check | Description |
---|---|---|
bpf/vmtest-bpf-next-PR | success | PR summary |
netdev/tree_selection | success | Clearly marked for bpf-next, async |
netdev/fixes_present | success | Fixes tag not required for -next series |
netdev/subject_prefix | success | Link |
netdev/cover_letter | success | Series has a cover letter |
netdev/patch_count | success | Link |
netdev/header_inline | success | No static functions without inline keyword in header files |
netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
netdev/cc_maintainers | success | CCed 20 of 20 maintainers |
netdev/build_clang | success | Errors and warnings before: 0 this patch: 0 |
netdev/module_param | success | Was 0 now: 0 |
netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
netdev/verify_fixes | success | Fixes tag looks correct |
netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 8 lines checked |
netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
netdev/source_inline | success | Was 0 now: 0 |
bpf/vmtest-bpf-next-VM_Test-1 | success | Logs for Kernel LATEST on ubuntu-latest + selftests |
bpf/vmtest-bpf-next-VM_Test-2 | success | Logs for Kernel LATEST on z15 + selftests |
On Thu, Apr 14, 2022 at 10:47:20PM +0000, Alexander Lobakin wrote: > 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") > Signed-off-by: Alexander Lobakin <alobakin@pm.me> Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> Magnus would tell you that you should fix this on libxdp side instead :) > --- > 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 399b999fcec2..1dc7ad5dbef4 100644 > --- a/samples/bpf/xdpsock_user.c > +++ b/samples/bpf/xdpsock_user.c > @@ -1496,7 +1496,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.35.2 > >
On Thu, Apr 14, 2022 at 3:47 PM Alexander Lobakin <alobakin@pm.me> wrote: > > 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") > Signed-off-by: Alexander Lobakin <alobakin@pm.me> Acked-by: Song Liu <songliubraving@fb.com> > --- > 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 399b999fcec2..1dc7ad5dbef4 100644 > --- a/samples/bpf/xdpsock_user.c > +++ b/samples/bpf/xdpsock_user.c > @@ -1496,7 +1496,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.35.2 > >
diff --git a/samples/bpf/xdpsock_user.c b/samples/bpf/xdpsock_user.c index 399b999fcec2..1dc7ad5dbef4 100644 --- a/samples/bpf/xdpsock_user.c +++ b/samples/bpf/xdpsock_user.c @@ -1496,7 +1496,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) <
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") Signed-off-by: Alexander Lobakin <alobakin@pm.me> --- samples/bpf/xdpsock_user.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.35.2