DPDK patches and discussions
 help / color / mirror / Atom feed
From: Olivier Matz <olivier.matz@6wind.com>
To: dev@dpdk.org
Cc: Ferruh Yigit <ferruh.yigit@intel.com>,
	David Marchand <david.marchand@redhat.com>,
	Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [dpdk-dev] [PATCH] net/pcap: fix registration of timestamp dynamic field
Date: Fri, 13 Nov 2020 14:13:19 +0100
Message-ID: <20201113131319.GR1898@platinum> (raw)
In-Reply-To: <20201113103957.19068-1-olivier.matz@6wind.com>

On Fri, Nov 13, 2020 at 11:39:57AM +0100, Olivier Matz wrote:
> In pcap pmd, the timestamp mbuf dynamic field is mandatory. When the
> pcap pmd is created in a secondary process (this is the case for pdump),
> it cannot be registered because this is not allowed from a secondary
> process.
> 
> To ensure that the field is properly registered, do it from probe()
> instead of configure(). Indeed, probe() is invoked on the primary
> process when a device is created in a secondary.
> 
> Bugzilla ID: 571
> Fixes: d23d73d088c1 ("net/pcap: switch Rx timestamp to dynamic mbuf field")
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---

One additional comment about this patch.

While it solves the issue described in the bug report, there may still
be a gap when it is needed to register a dynamic field/flag from a
secondary process. This happens when registering and configuring devices
from a secondary process (is it supported?). It happens if the secondary
process initializes a library which is not initialized in primary, and
which requires a dynamic field.

From afar, it does not look too difficult to implement dynamic field
registration from secondary processes. The only thing missing is a way
to allocate the shared memory in the primary process at initialization.
Currently, there is no init callback that is invoked when eal init is
done.

This is the exact same problem than for this issue:
http://inbox.dpdk.org/dev/20200504074227.GA6327@platinum/#t


>  drivers/net/pcap/rte_eth_pcap.c | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
> index 4e6d49370e..4930d7d382 100644
> --- a/drivers/net/pcap/rte_eth_pcap.c
> +++ b/drivers/net/pcap/rte_eth_pcap.c
> @@ -661,15 +661,6 @@ eth_dev_stop(struct rte_eth_dev *dev)
>  static int
>  eth_dev_configure(struct rte_eth_dev *dev __rte_unused)
>  {
> -	int ret;
> -
> -	ret = rte_mbuf_dyn_rx_timestamp_register(&timestamp_dynfield_offset,
> -			&timestamp_rx_dynflag);
> -	if (ret != 0) {
> -		PMD_LOG(ERR, "Failed to register Rx timestamp field/flag");
> -		return -rte_errno;
> -	}
> -
>  	return 0;
>  }
>  
> @@ -1387,6 +1378,13 @@ pmd_pcap_probe(struct rte_vdev_device *dev)
>  	start_cycles = rte_get_timer_cycles();
>  	hz = rte_get_timer_hz();
>  
> +	ret = rte_mbuf_dyn_rx_timestamp_register(&timestamp_dynfield_offset,
> +			&timestamp_rx_dynflag);
> +	if (ret != 0) {
> +		PMD_LOG(ERR, "Failed to register Rx timestamp field/flag");
> +		return -1;
> +	}
> +
>  	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
>  		eth_dev = rte_eth_dev_attach_secondary(name);
>  		if (!eth_dev) {
> -- 
> 2.25.1
> 

  reply	other threads:[~2020-11-13 13:13 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-13 10:39 Olivier Matz
2020-11-13 13:13 ` Olivier Matz [this message]
2020-11-13 14:02   ` Ferruh Yigit
2020-11-13 17:23     ` Ferruh Yigit
2020-11-13 20:35     ` Olivier Matz

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=20201113131319.GR1898@platinum \
    --to=olivier.matz@6wind.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=thomas@monjalon.net \
    /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

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git