Re: [dpdk-dev] Question on mlx5 PMD txq memory registration

["Nélio Laranjeiro" <nelio.laranjeiro@6wind.com>]

Hello Sagi,

On Mon, Jul 17, 2017 at 04:29:34PM +0300, Sagi Grimberg wrote:
> Hi,
> 
> Looking at the code, it looks like mlx5 keeps a MR cache per TX queue
> (each MR registers a rte_mempool).
> 
> Once a TX queue is created, mlx5 scans existing mempools and
> pre-registers a MR for each mempool it meets (using rte_mempool_walk).
> For each MR registration that exceeds the TX queue cache, it removes the
> first entry from the cache and deregisters that MR (in txq_mp2mr_reg).
> 
> Now on TX burst, if the driver sees a mbuf from an unknown mempool, it
> registers the mempool on the fly and again *deregister* the first MR in
> the cache.
> 
> My question is, what guarantees that no inflight send operations posted
> on the TX queue when we deregister and remove a MR from the cache?

There is none, if you send a burst of 9 packets each one coming from a
different mempool the first one will be dropped.

> AFAICT, it is the driver responsibility to guarantee to never deregister
> a memory region that has inflight send operations posted, otherwise
> the send operation *will* complete with a local protection error. Is
> that taken care of?

Up to now we have assumed that the user knows its application needs and
he can increase this cache size to its needs through the configuration
item.
This way this limit and guarantee becomes true.

> Another question, why is the MR cache maintained per TX queue and not
> per device? If the application starts N TX queues then a single mempool
> will be registered N times instead of just once. Having lots of MR
> instances will pollute the device ICMC pretty badly. Am I missing
> something?

Having this cache per device needs a lock on the device structure while
threads are sending packets.  Having such locks cost cycles, that is why
the cache is per queue.  Another point is, having several mempool per
device is something common, whereas having several mempool per queues is
not, it seems logical to have this cache per queue for those two
reasons.


I am currently re-working this part of the code to improve it using
reference counters instead. The cache will remain for performance
purpose.  This will fix the issues you are pointing.

Are you facing some kind of issue?  Maybe you can share it, it can help
to improve things.

Thanks,

-- 
Nélio Laranjeiro
6WIND