>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. >
I will add to determine whether mp is empty. If mp is NULL, the code is processed according to the original logic, try to create mempool. If mp is not NULL, the new mempool should be enough, so that the dev will not be broken due to configuration changes. This patch is mainly used to prevent the physical nic from being broken due to the configuration changes. Can the above operations cover the rare cases you mentioned? >> 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
