From: Dust Li <dust.li@linux•alibaba.com>
To: "D. Wythe" <alibuda@linux•alibaba.com>,
kgraul@linux•ibm.com, wenjia@linux•ibm.com, jaka@linux•ibm.com,
wintera@linux•ibm.com, guwen@linux•alibaba.com
Cc: kuba@kernel•org, davem@davemloft•net, netdev@vger•kernel.org,
linux-s390@vger•kernel.org, linux-rdma@vger•kernel.org,
tonylu@linux•alibaba.com, pabeni@redhat•com, edumazet@google•com
Subject: Re: [PATCH net-next 3/3] net/smc: Isolate the smc_xxx_lgr_pending with ib_device
Date: Thu, 14 Nov 2024 09:51:25 +0800 [thread overview]
Message-ID: <20241114015125.GF89669@linux.alibaba.com> (raw)
In-Reply-To: <20241113071405.67421-4-alibuda@linux.alibaba.com>
On 2024-11-13 15:14:05, D. Wythe wrote:
>It is widely known that SMC introduced a global lock to protect the
>creation of the first connection. This lock not only brings performance
>issues but also poses a serious security risk. In a multi-tenant
>container environment, malicious tenants can construct attacks that keep
>the lock occupied for an extended period, thereby affecting the
>connections of other tenants.
>
>Considering that this lock is essentially meant to protect the QP, which
>belongs to a device, we can limit the scope of the lock to within the
>device rather than having it be global. This way, when a container
>exclusively occupies the device, it can avoid being affected by other
>malicious tenants.
>
>Also make on impact on SMC-D since the path of SMC-D is shorter.
>
>Signed-off-by: D. Wythe <alibuda@linux•alibaba.com>
>---
> net/smc/af_smc.c | 18 ++++++++++--------
> net/smc/smc_ib.c | 2 ++
> net/smc/smc_ib.h | 2 ++
> 3 files changed, 14 insertions(+), 8 deletions(-)
>
>diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
>index 19480d8affb0..d5b9ea7661db 100644
>--- a/net/smc/af_smc.c
>+++ b/net/smc/af_smc.c
>@@ -56,11 +56,8 @@
> #include "smc_loopback.h"
> #include "smc_inet.h"
>
>-static DEFINE_MUTEX(smc_server_lgr_pending); /* serialize link group
>- * creation on server
>- */
>-static DEFINE_MUTEX(smc_client_lgr_pending); /* serialize link group
>- * creation on client
>+static DEFINE_MUTEX(smcd_server_lgr_pending); /* serialize link group
>+ * creation on server for SMC-D
> */
Why not move the smcd_server_lgr_pending lock to the smcd_device level
as well ?
>
> static struct workqueue_struct *smc_tcp_ls_wq; /* wq for tcp listen work */
>@@ -1251,7 +1248,9 @@ static int smc_connect_rdma(struct smc_sock *smc,
> if (reason_code)
> return reason_code;
>
>- smc_lgr_pending_lock(ini, &smc_client_lgr_pending);
>+ smc_lgr_pending_lock(ini, (ini->smcr_version & SMC_V2) ?
>+ &ini->smcrv2.ib_dev_v2->smc_server_lgr_pending :
>+ &ini->ib_dev->smc_server_lgr_pending);
> reason_code = smc_conn_create(smc, ini);
> if (reason_code) {
> smc_lgr_pending_unlock(ini);
>@@ -1412,7 +1411,7 @@ static int smc_connect_ism(struct smc_sock *smc,
> ini->ism_peer_gid[ini->ism_selected].gid = ntohll(aclc->d0.gid);
>
> /* there is only one lgr role for SMC-D; use server lock */
>- smc_lgr_pending_lock(ini, &smc_server_lgr_pending);
>+ smc_lgr_pending_lock(ini, &smcd_server_lgr_pending);
> rc = smc_conn_create(smc, ini);
> if (rc) {
> smc_lgr_pending_unlock(ini);
>@@ -2044,6 +2043,9 @@ static int smc_listen_rdma_init(struct smc_sock *new_smc,
> {
> int rc;
>
>+ smc_lgr_pending_lock(ini, (ini->smcr_version & SMC_V2) ?
>+ &ini->smcrv2.ib_dev_v2->smc_server_lgr_pending :
>+ &ini->ib_dev->smc_server_lgr_pending);
> /* allocate connection / link group */
> rc = smc_conn_create(new_smc, ini);
> if (rc)
>@@ -2064,6 +2066,7 @@ static int smc_listen_ism_init(struct smc_sock *new_smc,
> {
> int rc;
>
>+ smc_lgr_pending_lock(ini, &smcd_server_lgr_pending);
> rc = smc_conn_create(new_smc, ini);
> if (rc)
> return rc;
>@@ -2478,7 +2481,6 @@ static void smc_listen_work(struct work_struct *work)
> if (rc)
> goto out_decl;
>
>- smc_lgr_pending_lock(ini, &smc_server_lgr_pending);
> smc_close_init(new_smc);
> smc_rx_init(new_smc);
> smc_tx_init(new_smc);
>diff --git a/net/smc/smc_ib.c b/net/smc/smc_ib.c
>index 9c563cdbea90..fb8b81b628b8 100644
>--- a/net/smc/smc_ib.c
>+++ b/net/smc/smc_ib.c
>@@ -952,6 +952,8 @@ static int smc_ib_add_dev(struct ib_device *ibdev)
> init_waitqueue_head(&smcibdev->lnks_deleted);
> mutex_init(&smcibdev->mutex);
> mutex_lock(&smc_ib_devices.mutex);
>+ mutex_init(&smcibdev->smc_server_lgr_pending);
>+ mutex_init(&smcibdev->smc_client_lgr_pending);
> list_add_tail(&smcibdev->list, &smc_ib_devices.list);
> mutex_unlock(&smc_ib_devices.mutex);
> ib_set_client_data(ibdev, &smc_ib_client, smcibdev);
>diff --git a/net/smc/smc_ib.h b/net/smc/smc_ib.h
>index ef8ac2b7546d..322547a5a23d 100644
>--- a/net/smc/smc_ib.h
>+++ b/net/smc/smc_ib.h
>@@ -57,6 +57,8 @@ struct smc_ib_device { /* ib-device infos for smc */
> atomic_t lnk_cnt_by_port[SMC_MAX_PORTS];
> /* number of links per port */
> int ndev_ifidx[SMC_MAX_PORTS]; /* ndev if indexes */
>+ struct mutex smc_server_lgr_pending; /* serialize link group creation on server */
>+ struct mutex smc_client_lgr_pending; /* serialize link group creation on client */
Align please.
Best regards,
Dust
next prev parent reply other threads:[~2024-11-14 1:51 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-11-13 7:14 [PATCH net-next 0/3] Reduce locks scope of-smc_xxx_lgr_pending D. Wythe
2024-11-13 7:14 ` [PATCH net-next 1/3] net/smc: refactoring lgr pending lock D. Wythe
2024-11-14 1:40 ` Dust Li
2024-11-13 7:14 ` [PATCH net-next 2/3] net/smc: reduce locks scope of smc_xxx_lgr_pending D. Wythe
2024-11-13 7:14 ` [PATCH net-next 3/3] net/smc: Isolate the smc_xxx_lgr_pending with ib_device D. Wythe
2024-11-14 1:51 ` Dust Li [this message]
2024-11-14 1:27 ` [PATCH net-next 0/3] Reduce locks scope of-smc_xxx_lgr_pending Dust Li
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20241114015125.GF89669@linux.alibaba.com \
--to=dust.li@linux$(echo .)alibaba.com \
--cc=alibuda@linux$(echo .)alibaba.com \
--cc=davem@davemloft$(echo .)net \
--cc=edumazet@google$(echo .)com \
--cc=guwen@linux$(echo .)alibaba.com \
--cc=jaka@linux$(echo .)ibm.com \
--cc=kgraul@linux$(echo .)ibm.com \
--cc=kuba@kernel$(echo .)org \
--cc=linux-rdma@vger$(echo .)kernel.org \
--cc=linux-s390@vger$(echo .)kernel.org \
--cc=netdev@vger$(echo .)kernel.org \
--cc=pabeni@redhat$(echo .)com \
--cc=tonylu@linux$(echo .)alibaba.com \
--cc=wenjia@linux$(echo .)ibm.com \
--cc=wintera@linux$(echo .)ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox