On Fri, Nov 13, 2020 at 02:02:38PM +0000, Ferruh Yigit wrote:
> On 11/13/2020 1:13 PM, Olivier Matz wrote:
> > 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.
> > >
>
> probe() invoked first in the primary, later in the secondary, both process
> calls the driver probe(). But for this case probe(), and dynfield register,
> being called first in primary seems solving the problem.
> Would you be OK to change last sentences as:
> "Indeed, probe() is first invoked on the primary process when a device is
> created in a secondary, this enables registering dynfield from secondary
> process."
Yes, it is more precise, thanks
> > > Bugzilla ID: 571
> > > Fixes: d23d73d088c1 ("net/pcap: switch Rx timestamp to dynamic mbuf
> > > field")
> > >
> > > Signed-off-by: Olivier Matz <olivier.m...@6wind.com>
>
> Reviewed-by: Ferruh Yigit <ferruh.yi...@intel.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.
> >
>
> I was checking this, it seems what prevents to register dnyfield from the
> secondary process is 'init_shared_mem()', so if primary process registers
> any dynfield first, secondary process can register dynfields too.
> Do you think should this limitation documented?
Currently, registration from secondary is explicitly denied, with a code
like this:
if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
rte_errno = EPERM;
return -1;
}
> > This is the exact same problem than for this issue:
> > http://inbox.dpdk.org/dev/20200504074227.GA6327@platinum/#t
> >
>
> <...>
>
