| Message ID | 20240713015253.121248-1-sdf@fomichev.me (mailing list archive) |
|---|---|
| Headers | show |
| Series | xsk: require XDP_UMEM_TX_METADATA_LEN to actuate tx_metadata_len | expand |
On 7/13/24 3:52 AM, Stanislav Fomichev wrote: > Julian reports that commit 341ac980eab9 ("xsk: Support tx_metadata_len") > can break existing use cases which don't zero-initialize xdp_umem_reg > padding. Fix it (while still breaking a minority of new users of tx > metadata), update the docs, update the selftest and sprinkle some > BUILD_BUG_ONs to hopefully catch similar issues in the future. > > Thank you Julian for the report and for helping to chase it down! > > Reported-by: Julian Schindel <mail@arctic-alpaca.de> > Cc: Magnus Karlsson <magnus.karlsson@gmail.com> > > Stanislav Fomichev (3): > xsk: require XDP_UMEM_TX_METADATA_LEN to actuate tx_metadata_len > selftests/bpf: Add XDP_UMEM_TX_METADATA_LEN to XSK TX metadata test > xsk: Try to make xdp_umem_reg extension a bit more future-proof > > Documentation/networking/xsk-tx-metadata.rst | 16 ++++++++----- > include/uapi/linux/if_xdp.h | 4 ++++ > net/xdp/xdp_umem.c | 9 +++++--- > net/xdp/xsk.c | 23 ++++++++++--------- > tools/include/uapi/linux/if_xdp.h | 4 ++++ > .../selftests/bpf/prog_tests/xdp_metadata.c | 3 ++- > 6 files changed, 38 insertions(+), 21 deletions(-) > Magnus or Maciej, ptal when you get a chance. Thanks, Daniel
> On 7/13/24 3:52 AM, Stanislav Fomichev wrote: > > Julian reports that commit 341ac980eab9 ("xsk: Support tx_metadata_len") > > can break existing use cases which don't zero-initialize xdp_umem_reg > > padding. Fix it (while still breaking a minority of new users of tx > > metadata), update the docs, update the selftest and sprinkle some > > BUILD_BUG_ONs to hopefully catch similar issues in the future. > > > > Thank you Julian for the report and for helping to chase it down! > > > > Reported-by: Julian Schindel <mail@arctic-alpaca.de> > > Cc: Magnus Karlsson <magnus.karlsson@gmail.com> > > > > Stanislav Fomichev (3): > > xsk: require XDP_UMEM_TX_METADATA_LEN to actuate tx_metadata_len > > selftests/bpf: Add XDP_UMEM_TX_METADATA_LEN to XSK TX metadata test > > xsk: Try to make xdp_umem_reg extension a bit more future-proof > > > > Documentation/networking/xsk-tx-metadata.rst | 16 ++++++++----- > > include/uapi/linux/if_xdp.h | 4 ++++ > > net/xdp/xdp_umem.c | 9 +++++--- > > net/xdp/xsk.c | 23 ++++++++++--------- > > tools/include/uapi/linux/if_xdp.h | 4 ++++ > > .../selftests/bpf/prog_tests/xdp_metadata.c | 3 ++- > > 6 files changed, 38 insertions(+), 21 deletions(-) > > > > Magnus or Maciej, ptal when you get a chance. I'll do so on Monday as I'll be back from vacation, Magnus will be out for yet another week. Hope it works for you? > > Thanks, > Daniel
On 7/19/24 5:29 PM, Fijalkowski, Maciej wrote: >> On 7/13/24 3:52 AM, Stanislav Fomichev wrote: >>> Julian reports that commit 341ac980eab9 ("xsk: Support tx_metadata_len") >>> can break existing use cases which don't zero-initialize xdp_umem_reg >>> padding. Fix it (while still breaking a minority of new users of tx >>> metadata), update the docs, update the selftest and sprinkle some >>> BUILD_BUG_ONs to hopefully catch similar issues in the future. >>> >>> Thank you Julian for the report and for helping to chase it down! >>> >>> Reported-by: Julian Schindel <mail@arctic-alpaca.de> >>> Cc: Magnus Karlsson <magnus.karlsson@gmail.com> >>> >>> Stanislav Fomichev (3): >>> xsk: require XDP_UMEM_TX_METADATA_LEN to actuate tx_metadata_len >>> selftests/bpf: Add XDP_UMEM_TX_METADATA_LEN to XSK TX metadata test >>> xsk: Try to make xdp_umem_reg extension a bit more future-proof >>> >>> Documentation/networking/xsk-tx-metadata.rst | 16 ++++++++----- >>> include/uapi/linux/if_xdp.h | 4 ++++ >>> net/xdp/xdp_umem.c | 9 +++++--- >>> net/xdp/xsk.c | 23 ++++++++++--------- >>> tools/include/uapi/linux/if_xdp.h | 4 ++++ >>> .../selftests/bpf/prog_tests/xdp_metadata.c | 3 ++- >>> 6 files changed, 38 insertions(+), 21 deletions(-) >>> >> >> Magnus or Maciej, ptal when you get a chance. > > I'll do so on Monday as I'll be back from vacation, Magnus will be out for > yet another week. Hope it works for you? Sounds good, just making sure this doesn't fall off the radar. :) Thanks Maciej!
On Fri, Jul 12, 2024 at 06:52:50PM -0700, Stanislav Fomichev wrote: > Julian reports that commit 341ac980eab9 ("xsk: Support tx_metadata_len") > can break existing use cases which don't zero-initialize xdp_umem_reg > padding. Fix it (while still breaking a minority of new users of tx > metadata), update the docs, update the selftest and sprinkle some > BUILD_BUG_ONs to hopefully catch similar issues in the future. > > Thank you Julian for the report and for helping to chase it down! > > Reported-by: Julian Schindel <mail@arctic-alpaca.de> > Cc: Magnus Karlsson <magnus.karlsson@gmail.com> For the content series, Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> However I was not sure about handling patch 3/3. Thanks! > > Stanislav Fomichev (3): > xsk: require XDP_UMEM_TX_METADATA_LEN to actuate tx_metadata_len > selftests/bpf: Add XDP_UMEM_TX_METADATA_LEN to XSK TX metadata test > xsk: Try to make xdp_umem_reg extension a bit more future-proof > > Documentation/networking/xsk-tx-metadata.rst | 16 ++++++++----- > include/uapi/linux/if_xdp.h | 4 ++++ > net/xdp/xdp_umem.c | 9 +++++--- > net/xdp/xsk.c | 23 ++++++++++--------- > tools/include/uapi/linux/if_xdp.h | 4 ++++ > .../selftests/bpf/prog_tests/xdp_metadata.c | 3 ++- > 6 files changed, 38 insertions(+), 21 deletions(-) > > -- > 2.45.2 > >
On 7/24/24 5:23 PM, Maciej Fijalkowski wrote: > On Fri, Jul 12, 2024 at 06:52:50PM -0700, Stanislav Fomichev wrote: >> Julian reports that commit 341ac980eab9 ("xsk: Support tx_metadata_len") >> can break existing use cases which don't zero-initialize xdp_umem_reg >> padding. Fix it (while still breaking a minority of new users of tx >> metadata), update the docs, update the selftest and sprinkle some >> BUILD_BUG_ONs to hopefully catch similar issues in the future. >> >> Thank you Julian for the report and for helping to chase it down! >> >> Reported-by: Julian Schindel <mail@arctic-alpaca.de> >> Cc: Magnus Karlsson <magnus.karlsson@gmail.com> > > For the content series, > > Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> > > However I was not sure about handling patch 3/3. Ok, then I'm taking in the first two for now as they seem to actually address the fix and the 3rd seems like an improvement which could also get routed via bpf-next. In either case, if one of you could follow-up on the latter, that would be great. Thanks, Daniel
On Thu, Jul 25, 2024 at 12:06:22PM +0200, Daniel Borkmann wrote: > On 7/24/24 5:23 PM, Maciej Fijalkowski wrote: > > On Fri, Jul 12, 2024 at 06:52:50PM -0700, Stanislav Fomichev wrote: > > > Julian reports that commit 341ac980eab9 ("xsk: Support tx_metadata_len") > > > can break existing use cases which don't zero-initialize xdp_umem_reg > > > padding. Fix it (while still breaking a minority of new users of tx > > > metadata), update the docs, update the selftest and sprinkle some > > > BUILD_BUG_ONs to hopefully catch similar issues in the future. > > > > > > Thank you Julian for the report and for helping to chase it down! > > > > > > Reported-by: Julian Schindel <mail@arctic-alpaca.de> > > > Cc: Magnus Karlsson <magnus.karlsson@gmail.com> > > > > For the content series, > > > > Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> > > > > However I was not sure about handling patch 3/3. > > Ok, then I'm taking in the first two for now as they seem to actually > address the fix and the 3rd seems like an improvement which could also > get routed via bpf-next. In either case, if one of you could follow-up > on the latter, that would be great. My first thought was about squashing 1 and 3 but I hope Stan doesn't mind sending 3 solely to bpf-next... > > Thanks, > Daniel
Julian reports that commit 341ac980eab9 ("xsk: Support tx_metadata_len") can break existing use cases which don't zero-initialize xdp_umem_reg padding. Fix it (while still breaking a minority of new users of tx metadata), update the docs, update the selftest and sprinkle some BUILD_BUG_ONs to hopefully catch similar issues in the future. Thank you Julian for the report and for helping to chase it down! Reported-by: Julian Schindel <mail@arctic-alpaca.de> Cc: Magnus Karlsson <magnus.karlsson@gmail.com> Stanislav Fomichev (3): xsk: require XDP_UMEM_TX_METADATA_LEN to actuate tx_metadata_len selftests/bpf: Add XDP_UMEM_TX_METADATA_LEN to XSK TX metadata test xsk: Try to make xdp_umem_reg extension a bit more future-proof Documentation/networking/xsk-tx-metadata.rst | 16 ++++++++----- include/uapi/linux/if_xdp.h | 4 ++++ net/xdp/xdp_umem.c | 9 +++++--- net/xdp/xsk.c | 23 ++++++++++--------- tools/include/uapi/linux/if_xdp.h | 4 ++++ .../selftests/bpf/prog_tests/xdp_metadata.c | 3 ++- 6 files changed, 38 insertions(+), 21 deletions(-)