On Tue, Apr 09, 2024 at 09:50:57PM +0800, Mark Zhang wrote: > On 4/9/2024 9:26 PM, Leon Romanovsky wrote: > > External email: Use caution opening links or attachments > > > > > > From: Michael Guralnik <michaelgur@xxxxxxxxxx> > > > > The existing behavior of ib_umad, which maintains received MAD > > packets in an unbounded list, poses a risk of uncontrolled growth. > > As user-space applications extract packets from this list, the rate > > of extraction may not match the rate of incoming packets, leading > > to potential list overflow. > > > > To address this, we introduce a limit to the size of the list. After > > considering typical scenarios, such as OpenSM processing, which can > > handle approximately 100k packets per second, and the 1-second retry > > timeout for most packets, we set the list size limit to 200k. Packets > > received beyond this limit are dropped, assuming they are likely timed > > out by the time they are handled by user-space. > > > > Notably, packets queued on the receive list due to reasons like > > timed-out sends are preserved even when the list is full. > > > > Signed-off-by: Michael Guralnik <michaelgur@xxxxxxxxxx> > > Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxx> > > --- > > Changelog: > > v1: > > * Changed sysfs entry to hard coded value. > > * Rewrote the commit message. > > v0: https://lore.kernel.org/all/70029b5f256fbad6efbb98458deb9c46baa2c4b3.1712051390.git.leon@xxxxxxxxxx > > --- > > drivers/infiniband/core/user_mad.c | 16 +++++++++++++--- > > 1 file changed, 13 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/infiniband/core/user_mad.c b/drivers/infiniband/core/user_mad.c > > index f5feca7fa9b9..40756b573017 100644 > > --- a/drivers/infiniband/core/user_mad.c > > +++ b/drivers/infiniband/core/user_mad.c > > @@ -63,6 +63,8 @@ MODULE_AUTHOR("Roland Dreier"); > > MODULE_DESCRIPTION("InfiniBand userspace MAD packet access"); > > MODULE_LICENSE("Dual BSD/GPL"); > > > > +#define MAX_UMAD_RECV_LIST_SIZE 200000 > > + > > enum { > > IB_UMAD_MAX_PORTS = RDMA_MAX_PORTS, > > IB_UMAD_MAX_AGENTS = 32, > > @@ -113,6 +115,7 @@ struct ib_umad_file { > > struct mutex mutex; > > struct ib_umad_port *port; > > struct list_head recv_list; > > + atomic_t recv_list_size; > > struct list_head send_list; > > struct list_head port_list; > > spinlock_t send_lock; > > @@ -182,7 +185,8 @@ static struct ib_mad_agent *__get_agent(struct ib_umad_file *file, int id) > > > > static int queue_packet(struct ib_umad_file *file, > > struct ib_mad_agent *agent, > > - struct ib_umad_packet *packet) > > + struct ib_umad_packet *packet, > > + bool is_send_mad) > > { > > int ret = 1; > > > > @@ -192,7 +196,11 @@ static int queue_packet(struct ib_umad_file *file, > > packet->mad.hdr.id < IB_UMAD_MAX_AGENTS; > > packet->mad.hdr.id++) > > if (agent == __get_agent(file, packet->mad.hdr.id)) { > > + if (is_send_mad || atomic_read(&file->recv_list_size) > > > + MAX_UMAD_RECV_LIST_SIZE) > > Should it be: > > if (!is_send_mad && > atomic_read(&file->recv_list_size) > MAX_UMAD_RECV_LIST_SIZE)) > > Or maybe: > > if (is_recv_mad && > atomic_read(&file->recv_list_size) > MAX_UMAD_RECV_LIST_SIZE)) Of course you are right, I will fix it in the next version. Thanks > > > + break; > > list_add_tail(&packet->list, &file->recv_list); > > + atomic_inc(&file->recv_list_size); > > wake_up_interruptible(&file->recv_wait); > > ret = 0; > > break; > > @@ -224,7 +232,7 @@ static void send_handler(struct ib_mad_agent *agent, > > if (send_wc->status == IB_WC_RESP_TIMEOUT_ERR) { > > packet->length = IB_MGMT_MAD_HDR; > > packet->mad.hdr.status = ETIMEDOUT; > > - if (!queue_packet(file, agent, packet)) > > + if (!queue_packet(file, agent, packet, true)) > > return; > > } > > kfree(packet); > > @@ -284,7 +292,7 @@ static void recv_handler(struct ib_mad_agent *agent, > > rdma_destroy_ah_attr(&ah_attr); > > } > > > > - if (queue_packet(file, agent, packet)) > > + if (queue_packet(file, agent, packet, false)) > > goto err2; > > return; > > > > @@ -409,6 +417,7 @@ static ssize_t ib_umad_read(struct file *filp, char __user *buf, > > > > packet = list_entry(file->recv_list.next, struct ib_umad_packet, list); > > list_del(&packet->list); > > + atomic_dec(&file->recv_list_size); > > > > mutex_unlock(&file->mutex); > > > > @@ -421,6 +430,7 @@ static ssize_t ib_umad_read(struct file *filp, char __user *buf, > > /* Requeue packet */ > > mutex_lock(&file->mutex); > > list_add(&packet->list, &file->recv_list); > > + atomic_inc(&file->recv_list_size); > > mutex_unlock(&file->mutex); > > } else { > > if (packet->recv_wc) > > -- > > 2.44.0 > > > > >