>>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?
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
