On 12/12/2022 02:29, yangchang wrote:

On 07/12/2022 11:53, yangchang wrote:
For eth dev, dpdk X_rx_queue_start fuction will pre alloc mbuf
for rxq, if n_mbufs < n_rxq * rxq_size, dev will start error
becuse of it cannot allocate memory. If n_mbufs are not enough,
the mempool is not necessary to create.


On the good side, catching the issue here makes it more recoverable by
the user by changing the num rxq or rxq size desc.

The issue is that it is based on the requested sizes, and there is
fallback for them to move to lower sizes, so the requested size might
not actually be what is used for setting up the rxqs. Meaning there
could be a case where the mempool would have been big enough but we
don't create it.

So I agree it's an improvement for most cases, but it's a backwards step
for the (rare?) case where requested rxqs is too big and is adjusted
downwards during dpdk init making the mempool big enough.

Thanks for reviewing this patch. I don't understand it's a backwards step
for the (rare?) case.  Could you describe the case in detail?


Sure. As an example,

Present code:
- User requests 32 rxqs and rxq size 4096.
- Request mempool and after some retry can only get 64K mbuf size mempool (too small for 32 x 4096, but continue) - DPDK NIC is configured, requested 32 rxq is rejected as too big and reduced to 16 rxq
- Configure works as mempool is big enough for new rxq size

New code:
- User requests 32 rxqs and rxq size 4096.
- Request mempool and after some retry can only get 64K mbuf size mempool (too small for 32 x 4096, but *not* continue)
- NIC not configured

I realise this should be a rare case, but i guess the question is - is it wise to check here considering the config might change, or just to try and configure the NIC and see if it works?

I do note in a test I did it was easier to recover by changing the requested rxqs with the changes in this patch.

thanks.
yangchang

Some comments on the implementation below.

Signed-off-by: yangchang <[email protected]>
---
   lib/netdev-dpdk.c | 13 +++++++++++++
   1 file changed, 13 insertions(+)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 72e7a32..7e0a819 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -757,6 +757,19 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu)
       n_mbufs = dpdk_calculate_mbufs(dev, mtu);

       do {
+        /* For eth dev, dpdk X_rx_queue_start fuction will pre alloc mbuf

function

+         * for rxq, if n_mbufs < n_rxq * rxq_size, dev will start error
+         * becuse of it cannot allocate memory.

because

+         */
+        if ((!per_port_memory) && (dev->type == DPDK_DEV_ETH) &&
+            (n_mbufs < dev->requested_n_rxq * dev->requested_rxq_size)) {

Some of the () are unnecessary and can be removed.

+            VLOG_ERR("Port %s: a mempool of %u mbuffs is not enough "
+                     "for %d Rx queues and %d queue size",

s/mbuffs/mbufs/

I think "Rx queues of %d queue size" would sound better

+                     netdev_name, n_mbufs, dev->requested_n_rxq,
+                     dev->requested_rxq_size);
+             break;

Breaking here leads to an incorrect log message:
netdev_dpdk|ERR|Failed to create mempool "�" with a request of 262144 mbufs

thanks,
Kevin.

+        }
+
           /* Full DPDK memory pool name must be unique and cannot be
            * longer than RTE_MEMPOOL_NAMESIZE. Note that for the shared
            * mempool case this can result in one device using a mempool
--
1.8.3.1


[email protected]
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to