| Message ID | 20240328143051.1069575-9-arnd@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | a5535e5336943b33689f558199366102387b7bbf |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | address remaining -Wtautological-constant-out-of-range-compare | expand |
On Thu, 28 Mar 2024 at 15:30:46 +0100, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > When building with 64KB pages, clang points out that xsk->chunk_size > can never be PAGE_SIZE: > > drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c:19:22: error: result of comparison of constant 65536 with expression of type 'u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare] > if (xsk->chunk_size > PAGE_SIZE || > ~~~~~~~~~~~~~~~ ^ ~~~~~~~~~ > > In older versions of this code, using PAGE_SIZE was the only > possibility, so this would have never worked on 64KB page kernels, > but the patch apparently did not address this case completely. > > As Maxim Mikityanskiy suggested, 64KB chunks are really not all that > useful, so just shut up the warning by adding a cast. > > Fixes: 282c0c798f8e ("net/mlx5e: Allow XSK frames smaller than a page") > Link: https://lore.kernel.org/netdev/20211013150232.2942146-1-arnd@kernel.org/ > Link: https://lore.kernel.org/lkml/a7b27541-0ebb-4f2d-bd06-270a4d404613@app.fastmail.com/ > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c > index 06592b9f0424..9240cfe25d10 100644 > --- a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c > @@ -28,8 +28,10 @@ bool mlx5e_validate_xsk_param(struct mlx5e_params *params, > struct mlx5e_xsk_param *xsk, > struct mlx5_core_dev *mdev) > { > - /* AF_XDP doesn't support frames larger than PAGE_SIZE. */ > - if (xsk->chunk_size > PAGE_SIZE || xsk->chunk_size < MLX5E_MIN_XSK_CHUNK_SIZE) { > + /* AF_XDP doesn't support frames larger than PAGE_SIZE, > + * and xsk->chunk_size is limited to 65535 bytes. > + */ > + if ((size_t)xsk->chunk_size > PAGE_SIZE || xsk->chunk_size < MLX5E_MIN_XSK_CHUNK_SIZE) { Acked-by: Maxim Mikityanskiy <maxtram95@gmail.com> > mlx5_core_err(mdev, "XSK chunk size %u out of bounds [%u, %lu]\n", xsk->chunk_size, > MLX5E_MIN_XSK_CHUNK_SIZE, PAGE_SIZE); > return false; > -- > 2.39.2 >
On Thu, Mar 28, 2024 at 7:32 AM Arnd Bergmann <arnd@kernel.org> wrote: > > From: Arnd Bergmann <arnd@arndb.de> > > When building with 64KB pages, clang points out that xsk->chunk_size > can never be PAGE_SIZE: This is under W=1 right? Otherwise this is a mighty annoying warning. > > drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c:19:22: error: result of comparison of constant 65536 with expression of type 'u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare] > if (xsk->chunk_size > PAGE_SIZE || > ~~~~~~~~~~~~~~~ ^ ~~~~~~~~~ > > In older versions of this code, using PAGE_SIZE was the only > possibility, so this would have never worked on 64KB page kernels, > but the patch apparently did not address this case completely. > > As Maxim Mikityanskiy suggested, 64KB chunks are really not all that > useful, so just shut up the warning by adding a cast. > > Fixes: 282c0c798f8e ("net/mlx5e: Allow XSK frames smaller than a page") > Link: https://lore.kernel.org/netdev/20211013150232.2942146-1-arnd@kernel.org/ > Link: https://lore.kernel.org/lkml/a7b27541-0ebb-4f2d-bd06-270a4d404613@app.fastmail.com/ > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Reviewed-by: Justin Stitt <justinstitt@google.com> > --- > drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c > index 06592b9f0424..9240cfe25d10 100644 > --- a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c > @@ -28,8 +28,10 @@ bool mlx5e_validate_xsk_param(struct mlx5e_params *params, > struct mlx5e_xsk_param *xsk, > struct mlx5_core_dev *mdev) > { > - /* AF_XDP doesn't support frames larger than PAGE_SIZE. */ > - if (xsk->chunk_size > PAGE_SIZE || xsk->chunk_size < MLX5E_MIN_XSK_CHUNK_SIZE) { > + /* AF_XDP doesn't support frames larger than PAGE_SIZE, > + * and xsk->chunk_size is limited to 65535 bytes. > + */ > + if ((size_t)xsk->chunk_size > PAGE_SIZE || xsk->chunk_size < MLX5E_MIN_XSK_CHUNK_SIZE) { > mlx5_core_err(mdev, "XSK chunk size %u out of bounds [%u, %lu]\n", xsk->chunk_size, > MLX5E_MIN_XSK_CHUNK_SIZE, PAGE_SIZE); > return false; > -- > 2.39.2 >
On Thu, Mar 28, 2024, at 23:09, Justin Stitt wrote: > On Thu, Mar 28, 2024 at 7:32 AM Arnd Bergmann <arnd@kernel.org> wrote: >> >> From: Arnd Bergmann <arnd@arndb.de> >> >> When building with 64KB pages, clang points out that xsk->chunk_size >> can never be PAGE_SIZE: > > This is under W=1 right? Otherwise this is a mighty annoying warning. At the moment yes. I'm fairly sure that I've covered all the common cases with thousands of randconfig builds, so we should be able to make it the default when this series is fully merged. Arnd
On 28/03/2024 16:30, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > When building with 64KB pages, clang points out that xsk->chunk_size > can never be PAGE_SIZE: > > drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c:19:22: error: result of comparison of constant 65536 with expression of type 'u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare] > if (xsk->chunk_size > PAGE_SIZE || > ~~~~~~~~~~~~~~~ ^ ~~~~~~~~~ > > In older versions of this code, using PAGE_SIZE was the only > possibility, so this would have never worked on 64KB page kernels, > but the patch apparently did not address this case completely. > > As Maxim Mikityanskiy suggested, 64KB chunks are really not all that > useful, so just shut up the warning by adding a cast. > > Fixes: 282c0c798f8e ("net/mlx5e: Allow XSK frames smaller than a page") > Link: https://lore.kernel.org/netdev/20211013150232.2942146-1-arnd@kernel.org/ > Link: https://lore.kernel.org/lkml/a7b27541-0ebb-4f2d-bd06-270a4d404613@app.fastmail.com/ > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Thanks for your patch. Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c index 06592b9f0424..9240cfe25d10 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c @@ -28,8 +28,10 @@ bool mlx5e_validate_xsk_param(struct mlx5e_params *params, struct mlx5e_xsk_param *xsk, struct mlx5_core_dev *mdev) { - /* AF_XDP doesn't support frames larger than PAGE_SIZE. */ - if (xsk->chunk_size > PAGE_SIZE || xsk->chunk_size < MLX5E_MIN_XSK_CHUNK_SIZE) { + /* AF_XDP doesn't support frames larger than PAGE_SIZE, + * and xsk->chunk_size is limited to 65535 bytes. + */ + if ((size_t)xsk->chunk_size > PAGE_SIZE || xsk->chunk_size < MLX5E_MIN_XSK_CHUNK_SIZE) { mlx5_core_err(mdev, "XSK chunk size %u out of bounds [%u, %lu]\n", xsk->chunk_size, MLX5E_MIN_XSK_CHUNK_SIZE, PAGE_SIZE); return false;