DPDK patches and discussions
 help / color / mirror / Atom feed
From: Marcelo Ricardo Leitner <mleitner@redhat.com>
To: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Cc: Shahaf Shuler <shahafs@mellanox.com>,
	Nelio Laranjeiro <nelio.laranjeiro@6wind.com>,
	dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v2 1/4] net/mlx: add debug checks to glue structure
Date: Mon, 5 Feb 2018 10:27:10 -0200	[thread overview]
Message-ID: <20180205122710.GC27676@localhost.localdomain> (raw)
In-Reply-To: <20180202164050.13017-2-adrien.mazarguil@6wind.com>

On Fri, Feb 02, 2018 at 05:46:12PM +0100, Adrien Mazarguil wrote:
> This code should catch mistakes early if a glue structure member is added
> without a corresponding implementation in the library.
> 
> Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> ---
>  drivers/net/mlx4/mlx4.c | 9 +++++++++
>  drivers/net/mlx5/mlx5.c | 9 +++++++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
> index 50a55ee52..201d39b6e 100644
> --- a/drivers/net/mlx4/mlx4.c
> +++ b/drivers/net/mlx4/mlx4.c
> @@ -799,6 +799,15 @@ rte_mlx4_pmd_init(void)
>  		return;
>  	assert(mlx4_glue);
>  #endif
> +#ifndef NDEBUG
> +	/* Glue structure must not contain any NULL pointers. */
> +	{
> +		unsigned int i;
> +
> +		for (i = 0; i != sizeof(*mlx4_glue) / sizeof(void *); ++i)
> +			assert(((const void *const *)mlx4_glue)[i]);
> +	}

This code will not catch the case on which mlx4_glue on PMD is smaller
than the one on the glue lib. Although this would be safe, as the PMD
won't place calls for functions that it is not aware of, I guess we
don't want to allow such situation anyhow.

One way to handle this is to add a size field and let the PMD check if
the size is the same. As this code is walking through it as an array
of pointers, an union around it to keep the alignment may be welcomed.

Also, this code would do read beyond buffer in the opposite case, when
mlx4_glue for the PMD is larger than the one on the glue lib.

> +#endif
>  	mlx4_glue->fork_init();
>  	rte_pci_register(&mlx4_driver);
>  }
> diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
> index 544599b01..050cfac0d 100644
> --- a/drivers/net/mlx5/mlx5.c
> +++ b/drivers/net/mlx5/mlx5.c
> @@ -1142,6 +1142,15 @@ rte_mlx5_pmd_init(void)
>  		return;
>  	assert(mlx5_glue);
>  #endif
> +#ifndef NDEBUG
> +	/* Glue structure must not contain any NULL pointers. */
> +	{
> +		unsigned int i;
> +
> +		for (i = 0; i != sizeof(*mlx5_glue) / sizeof(void *); ++i)
> +			assert(((const void *const *)mlx5_glue)[i]);
> +	}
> +#endif
>  	mlx5_glue->fork_init();
>  	rte_pci_register(&mlx5_driver);
>  }
> -- 
> 2.11.0
> 

  reply	other threads:[~2018-02-05 12:27 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-02 15:16 [dpdk-dev] [PATCH v1 0/4] net/mlx: enhance rdma-core glue configuration Adrien Mazarguil
2018-02-02 15:16 ` [dpdk-dev] [PATCH v1 1/4] net/mlx: add debug checks to glue structure Adrien Mazarguil
2018-02-02 15:16 ` [dpdk-dev] [PATCH v1 2/4] net/mlx: fix missing includes for rdma-core glue Adrien Mazarguil
2018-02-02 15:16 ` [dpdk-dev] [PATCH v1 3/4] net/mlx: version rdma-core glue libraries Adrien Mazarguil
2018-02-02 15:16 ` [dpdk-dev] [PATCH v1 4/4] net/mlx: make rdma-core glue path configurable Adrien Mazarguil
2018-02-02 16:46 ` [dpdk-dev] [PATCH v2 0/4] net/mlx: enhance rdma-core glue configuration Adrien Mazarguil
2018-02-02 16:46   ` [dpdk-dev] [PATCH v2 1/4] net/mlx: add debug checks to glue structure Adrien Mazarguil
2018-02-05 12:27     ` Marcelo Ricardo Leitner [this message]
2018-02-05 13:31       ` Adrien Mazarguil
2018-02-02 16:46   ` [dpdk-dev] [PATCH v2 2/4] net/mlx: fix missing includes for rdma-core glue Adrien Mazarguil
2018-02-02 16:46   ` [dpdk-dev] [PATCH v2 3/4] net/mlx: version rdma-core glue libraries Adrien Mazarguil
2018-02-04 14:29     ` Thomas Monjalon
2018-02-05 11:24       ` Adrien Mazarguil
2018-02-05 12:13         ` Marcelo Ricardo Leitner
2018-02-05 12:24           ` Van Haaren, Harry
2018-02-05 12:43             ` Marcelo Ricardo Leitner
2018-02-05 12:58             ` Marcelo Ricardo Leitner
2018-02-05 13:44               ` Adrien Mazarguil
2018-02-05 14:16                 ` Thomas Monjalon
2018-02-05 14:33                   ` Adrien Mazarguil
2018-02-05 14:37                   ` Marcelo Ricardo Leitner
2018-02-05 14:59                     ` Adrien Mazarguil
2018-02-05 15:29                       ` Marcelo Ricardo Leitner
2018-02-05 15:54                         ` Adrien Mazarguil
2018-02-05 17:06                           ` Marcelo Ricardo Leitner
2018-02-06 11:06                             ` Adrien Mazarguil
2018-02-02 16:46   ` [dpdk-dev] [PATCH v2 4/4] net/mlx: make rdma-core glue path configurable Adrien Mazarguil
2018-02-02 16:52   ` [dpdk-dev] [PATCH v2 0/4] net/mlx: enhance rdma-core glue configuration Nélio Laranjeiro
2018-02-06 11:31   ` Shahaf Shuler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180205122710.GC27676@localhost.localdomain \
    --to=mleitner@redhat.com \
    --cc=adrien.mazarguil@6wind.com \
    --cc=dev@dpdk.org \
    --cc=nelio.laranjeiro@6wind.com \
    --cc=shahafs@mellanox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).