* Re: [PATCH net-next v3 00/18] net/smc: implement virtual ISM extension and loopback-ism [not found] <1695302360-46691-1-git-send-email-guwen@linux.alibaba.com> @ 2023-09-21 23:31 ` Wenjia Zhang 2023-09-22 12:18 ` Wen Gu [not found] ` <1695302360-46691-13-git-send-email-guwen@linux.alibaba.com> ` (3 subsequent siblings) 4 siblings, 1 reply; 10+ messages in thread From: Wenjia Zhang @ 2023-09-21 23:31 UTC (permalink / raw) To: Wen Gu, kgraul, jaka, davem, edumazet, kuba, pabeni Cc: alibuda, tonylu, linux-s390, netdev, linux-kernel On 21.09.23 15:19, Wen Gu wrote: > Hi, all > > # Background > > SMC-D is now used in IBM z with ISM function to optimize network interconnect > for intra-CPC communications. Inspired by this, we try to make SMC-D available > on the non-s390 architecture through a software-simulated virtual ISM device, > such as loopback-ism device here, to accelerate inter-process or inter-containers > communication within the same OS. > > # Design > > This patch set includes 4 parts: > > - Patch #1-#3: decouple ISM device hard code from SMC-D stack. > - Patch #4-#8: implement virtual ISM extension defined in SMCv2.1. > - Patch #9-#13: implement loopback-ism device. > - Patch #14-#18: memory copy optimization for the case using loopback. > > The loopback-ism device is designed as a kernel device and not be limited to > a specific net namespace, ends of both inter-process connection (1/1' in diagram > below) or inter-container connection (2/2' in diagram below) will find that peer > shares the same loopback-ism device during the CLC handshake. Then loopback-ism > device will be chosen. > > Container 1 (ns1) Container 2 (ns2) > +-----------------------------------------+ +-------------------------+ > | +-------+ +-------+ +-------+ | | +-------+ | > | | App A | | App B | | App C | | | | App D |<-+ | > | +-------+ +---^---+ +-------+ | | +-------+ |(2') | > | |127.0.0.1 (1')| |192.168.0.11 192.168.0.12| | > | (1)| +--------+ | +--------+ |(2) | | +--------+ +--------+ | > | `-->| lo |-` | eth0 |<-` | | | lo | | eth0 | | > +---------+--|---^-+---+-----|--+---------+ +-+--------+---+-^------+-+ > | | | | > Kernel | | | | > +----+-------v---+-----------v----------------------------------+---+----+ > | | TCP | | > | | | | > | +--------------------------------------------------------------+ | > | | > | +--------------+ | > | | smc loopback | | > +---------------------------+--------------+-----------------------------+ > > > loopback-ism device allocs RMBs and sndbufs for each connection peer and 'moves' > data from sndbuf at one end to RMB at the other end. Since communication occurs > within the same kernel, the sndbuf can be mapped to peer RMB so that the data > copy in loopback-ism case can be avoided. > > Container 1 (ns1) Container 2 (ns2) > +-----------------------------------------+ +-------------------------+ > | +-------+ +-------+ +-------+ | | +-------+ | > | | App A | | App B | | App C | | | | App D | | > | +-------+ +--^----+ +-------+ | | +---^---+ | > | | | | | | | | > | (1) | (1') | (2) | | | (2') | | > | | | | | | | | > +-------|-----------|---------------|-----+ +------------|------------+ > | | | | > Kernel | | | | > +-------|-----------|---------------|-----------------------|------------+ > | +-----v-+ +-------+ +---v---+ +-------+ | > | | snd A |-+ | RMB B |<--+ | snd C |-+ +->| RMB D | | > | +-------+ | +-------+ | +-------+ | | +-------+ | > | +-------+ | +-------+ | +-------+ | | +-------+ | > | | RMB A | | | snd B | | | RMB C | | | | snd D | | > | +-------+ | +-------+ | +-------+ | | +-------+ | > | | +-------------v+ | | > | +-------------->| smc loopback |---------+ | > +---------------------------+--------------+-----------------------------+ > > # Benchmark Test > > * Test environments: > - VM with Intel Xeon Platinum 8 core 2.50GHz, 16 GiB mem. > - SMC sndbuf/RMB size 1MB. > > * Test object: > - TCP: run on TCP loopback. > - domain: run on UNIX domain. > - SMC lo: run on SMC loopback device. > > 1. ipc-benchmark (see [1]) > > - ./<foo> -c 1000000 -s 100 > > TCP domain SMC-lo > Message > rate (msg/s) 78855 107621(+36.41%) 153351(+94.47%) > > 2. sockperf > > - serv: <smc_run> taskset -c <cpu> sockperf sr --tcp > - clnt: <smc_run> taskset -c <cpu> sockperf { tp | pp } --tcp --msg-size={ 64000 for tp | 14 for pp } -i 127.0.0.1 -t 30 > > TCP SMC-lo > Bandwidth(MBps) 5169.250 8007.080(+54.89%) > Latency(us) 6.122 3.174(-48.15%) > > 3. nginx/wrk > > - serv: <smc_run> nginx > - clnt: <smc_run> wrk -t 8 -c 1000 -d 30 http://127.0.0.1:80 > > TCP SMC-lo > Requests/s 197432.19 261056.09(+32.22%) > > 4. redis-benchmark > > - serv: <smc_run> redis-server > - clnt: <smc_run> redis-benchmark -h 127.0.0.1 -q -t set,get -n 400000 -c 200 -d 1024 > > TCP SMC-lo > GET(Requests/s) 86244.07 122025.62(+41.48%) > SET(Requests/s) 86749.08 120048.02(+38.38%) > > [1] https://github.com/goldsborough/ipc-bench > > v2->v3: > - Fix build warning of patch#1 and patch#10. > > v1->v2: > - Fix build error on s390 arch. > > Wen Gu (18): > net/smc: decouple ism_dev from SMC-D device dump > net/smc: decouple ism_dev from SMC-D DMB registration > net/smc: extract v2 check helper from SMC-D device registration > net/smc: support SMCv2.x supplemental features negotiation > net/smc: reserve CHID range for SMC-D virtual device > net/smc: extend GID to 128bits for virtual ISM device > net/smc: disable SEID on non-s390 architecture > net/smc: enable virtual ISM device feature bit > net/smc: introduce SMC-D loopback device > net/smc: implement ID-related operations of loopback > net/smc: implement some unsupported operations of loopback > net/smc: implement DMB-related operations of loopback > net/smc: register loopback device as SMC-Dv2 device > net/smc: add operation for getting DMB attribute > net/smc: add operations for DMB attach and detach > net/smc: avoid data copy from sndbuf to peer RMB in SMC-D > net/smc: modify cursor update logic when sndbuf mapped to RMB > net/smc: add interface implementation of loopback device > > drivers/s390/net/ism_drv.c | 19 +- > include/net/smc.h | 32 ++- > include/uapi/linux/smc.h | 3 + > net/smc/Makefile | 2 +- > net/smc/af_smc.c | 73 +++++-- > net/smc/smc.h | 7 + > net/smc/smc_cdc.c | 56 ++++-- > net/smc/smc_cdc.h | 1 + > net/smc/smc_clc.c | 56 ++++-- > net/smc/smc_clc.h | 10 +- > net/smc/smc_core.c | 108 +++++++++- > net/smc/smc_core.h | 9 +- > net/smc/smc_diag.c | 6 +- > net/smc/smc_ism.c | 100 +++++++--- > net/smc/smc_ism.h | 24 ++- > net/smc/smc_loopback.c | 483 +++++++++++++++++++++++++++++++++++++++++++++ > net/smc/smc_loopback.h | 52 +++++ > net/smc/smc_pnet.c | 4 +- > 18 files changed, 946 insertions(+), 99 deletions(-) > create mode 100644 net/smc/smc_loopback.c > create mode 100644 net/smc/smc_loopback.h > Hi Wen, Thank you for the effort! You can find my comments in the respective patches. One general question from our team, could you please add a Kconfig option to turn off/on loopback-ism? BTW, I'm in vacation next week, my colleagues will follow on the answer and update. Thanks, Wenjia ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next v3 00/18] net/smc: implement virtual ISM extension and loopback-ism 2023-09-21 23:31 ` [PATCH net-next v3 00/18] net/smc: implement virtual ISM extension and loopback-ism Wenjia Zhang @ 2023-09-22 12:18 ` Wen Gu 0 siblings, 0 replies; 10+ messages in thread From: Wen Gu @ 2023-09-22 12:18 UTC (permalink / raw) To: Wenjia Zhang, kgraul, jaka, davem, edumazet, kuba, pabeni Cc: alibuda, tonylu, linux-s390, netdev, linux-kernel On 2023/9/22 07:31, Wenjia Zhang wrote: > > Hi Wen, > > Thank you for the effort! Thank you very much for review, Wenjia. > You can find my comments in the respective patches. One general question from our team, could you please add a Kconfig > option to turn off/on loopback-ism? Sure, I will add an option to turn off/on loopback-ism. > > BTW, I'm in vacation next week, my colleagues will follow on the answer and update. Thank you. Enjoy the vacation! Regards, Wen Gu > > Thanks, > Wenjia > ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <1695302360-46691-13-git-send-email-guwen@linux.alibaba.com>]
* Re: [PATCH net-next v3 12/18] net/smc: implement DMB-related operations of loopback [not found] ` <1695302360-46691-13-git-send-email-guwen@linux.alibaba.com> @ 2023-09-21 23:31 ` Wenjia Zhang 2023-09-22 7:42 ` Wen Gu 0 siblings, 1 reply; 10+ messages in thread From: Wenjia Zhang @ 2023-09-21 23:31 UTC (permalink / raw) To: Wen Gu, kgraul, jaka, davem, edumazet, kuba, pabeni Cc: alibuda, tonylu, linux-s390, netdev, linux-kernel On 21.09.23 15:19, Wen Gu wrote: > This patch implements DMB registration, unregistration and data move > operations of SMC-D loopback. > > Signed-off-by: Wen Gu <guwen@linux.alibaba.com> > --- > net/smc/smc_cdc.c | 6 +++ > net/smc/smc_cdc.h | 1 + > net/smc/smc_loopback.c | 128 +++++++++++++++++++++++++++++++++++++++++++++++-- > net/smc/smc_loopback.h | 13 +++++ > 4 files changed, 145 insertions(+), 3 deletions(-) > > diff --git a/net/smc/smc_cdc.c b/net/smc/smc_cdc.c > index 89105e9..2641800 100644 > --- a/net/smc/smc_cdc.c > +++ b/net/smc/smc_cdc.c > @@ -411,6 +411,12 @@ static void smc_cdc_msg_recv(struct smc_sock *smc, struct smc_cdc_msg *cdc) > static void smcd_cdc_rx_tsklet(struct tasklet_struct *t) > { > struct smc_connection *conn = from_tasklet(conn, t, rx_tsklet); > + > + smcd_cdc_rx_handler(conn); > +} > + > +void smcd_cdc_rx_handler(struct smc_connection *conn) > +{ > struct smcd_cdc_msg *data_cdc; > struct smcd_cdc_msg cdc; > struct smc_sock *smc; > diff --git a/net/smc/smc_cdc.h b/net/smc/smc_cdc.h > index 696cc11..11559d4 100644 > --- a/net/smc/smc_cdc.h > +++ b/net/smc/smc_cdc.h > @@ -301,5 +301,6 @@ int smcr_cdc_msg_send_validation(struct smc_connection *conn, > struct smc_wr_buf *wr_buf); > int smc_cdc_init(void) __init; > void smcd_cdc_rx_init(struct smc_connection *conn); > +void smcd_cdc_rx_handler(struct smc_connection *conn); > > #endif /* SMC_CDC_H */ > diff --git a/net/smc/smc_loopback.c b/net/smc/smc_loopback.c > index 9034ebd..cfbcabf 100644 > --- a/net/smc/smc_loopback.c > +++ b/net/smc/smc_loopback.c > @@ -16,6 +16,7 @@ > #include <linux/smc.h> > #include <net/smc.h> > > +#include "smc_cdc.h" > #include "smc_ism.h" > #include "smc_loopback.h" > > @@ -74,6 +75,93 @@ static int smc_lo_query_rgid(struct smcd_dev *smcd, struct smcd_gid *rgid, > return 0; > } > > +static int smc_lo_register_dmb(struct smcd_dev *smcd, struct smcd_dmb *dmb, > + void *client_priv) > +{ > + struct smc_lo_dmb_node *dmb_node, *tmp_node; > + struct smc_lo_dev *ldev = smcd->priv; > + int sba_idx, rc; > + > + /* check space for new dmb */ > + for_each_clear_bit(sba_idx, ldev->sba_idx_mask, SMC_LODEV_MAX_DMBS) { > + if (!test_and_set_bit(sba_idx, ldev->sba_idx_mask)) > + break; > + } > + if (sba_idx == SMC_LODEV_MAX_DMBS) > + return -ENOSPC; > + > + dmb_node = kzalloc(sizeof(*dmb_node), GFP_KERNEL); > + if (!dmb_node) { > + rc = -ENOMEM; > + goto err_bit; > + } > + > + dmb_node->sba_idx = sba_idx; > + dmb_node->cpu_addr = kzalloc(dmb->dmb_len, GFP_KERNEL | > + __GFP_NOWARN | __GFP_NORETRY | > + __GFP_NOMEMALLOC); kzalloc()/kmalloc() allocates physically contigueous memory. Are you sure it is suitable for allocating the dmb? > + if (!dmb_node->cpu_addr) { > + rc = -ENOMEM; > + goto err_node; > + } > + dmb_node->len = dmb->dmb_len; > + dmb_node->dma_addr = (dma_addr_t)dmb_node->cpu_addr; > + > +again: > + /* add new dmb into hash table */ > + get_random_bytes(&dmb_node->token, sizeof(dmb_node->token)); > + write_lock(&ldev->dmb_ht_lock); > + hash_for_each_possible(ldev->dmb_ht, tmp_node, list, dmb_node->token) { > + if (tmp_node->token == dmb_node->token) { > + write_unlock(&ldev->dmb_ht_lock); > + goto again; > + } > + } > + hash_add(ldev->dmb_ht, &dmb_node->list, dmb_node->token); > + write_unlock(&ldev->dmb_ht_lock); > + > + dmb->sba_idx = dmb_node->sba_idx; > + dmb->dmb_tok = dmb_node->token; > + dmb->cpu_addr = dmb_node->cpu_addr; > + dmb->dma_addr = dmb_node->dma_addr; > + dmb->dmb_len = dmb_node->len; > + > + return 0; > + > +err_node: > + kfree(dmb_node); > +err_bit: > + clear_bit(sba_idx, ldev->sba_idx_mask); > + return rc; > +} > + > +static int smc_lo_unregister_dmb(struct smcd_dev *smcd, struct smcd_dmb *dmb) > +{ > + struct smc_lo_dmb_node *dmb_node = NULL, *tmp_node; > + struct smc_lo_dev *ldev = smcd->priv; > + > + /* remove dmb from hash table */ > + write_lock(&ldev->dmb_ht_lock); > + hash_for_each_possible(ldev->dmb_ht, tmp_node, list, dmb->dmb_tok) { > + if (tmp_node->token == dmb->dmb_tok) { > + dmb_node = tmp_node; > + break; > + } > + } > + if (!dmb_node) { > + write_unlock(&ldev->dmb_ht_lock); > + return -EINVAL; > + } > + hash_del(&dmb_node->list); > + write_unlock(&ldev->dmb_ht_lock); > + > + clear_bit(dmb_node->sba_idx, ldev->sba_idx_mask); > + kfree(dmb_node->cpu_addr); > + kfree(dmb_node); > + > + return 0; > +} > + > static int smc_lo_add_vlan_id(struct smcd_dev *smcd, u64 vlan_id) > { > return -EOPNOTSUPP; > @@ -100,6 +188,38 @@ static int smc_lo_signal_event(struct smcd_dev *dev, struct smcd_gid *rgid, > return 0; > } > > +static int smc_lo_move_data(struct smcd_dev *smcd, u64 dmb_tok, unsigned int idx, > + bool sf, unsigned int offset, void *data, > + unsigned int size) > +{ > + struct smc_lo_dmb_node *rmb_node = NULL, *tmp_node; > + struct smc_lo_dev *ldev = smcd->priv; > + > + read_lock(&ldev->dmb_ht_lock); > + hash_for_each_possible(ldev->dmb_ht, tmp_node, list, dmb_tok) { > + if (tmp_node->token == dmb_tok) { > + rmb_node = tmp_node; > + break; > + } > + } > + if (!rmb_node) { > + read_unlock(&ldev->dmb_ht_lock); > + return -EINVAL; > + } > + read_unlock(&ldev->dmb_ht_lock); > + > + memcpy((char *)rmb_node->cpu_addr + offset, data, size); > + > + if (sf) { > + struct smc_connection *conn = > + smcd->conn[rmb_node->sba_idx]; > + > + if (conn && !conn->killed) > + smcd_cdc_rx_handler(conn); > + } > + return 0; > +} > + > static int smc_lo_supports_v2(void) > { > return SMC_LO_SUPPORTS_V2; > @@ -131,14 +251,14 @@ static struct device *smc_lo_get_dev(struct smcd_dev *smcd) > > static const struct smcd_ops lo_ops = { > .query_remote_gid = smc_lo_query_rgid, > - .register_dmb = NULL, > - .unregister_dmb = NULL, > + .register_dmb = smc_lo_register_dmb, > + .unregister_dmb = smc_lo_unregister_dmb, > .add_vlan_id = smc_lo_add_vlan_id, > .del_vlan_id = smc_lo_del_vlan_id, > .set_vlan_required = smc_lo_set_vlan_required, > .reset_vlan_required = smc_lo_reset_vlan_required, > .signal_event = smc_lo_signal_event, > - .move_data = NULL, > + .move_data = smc_lo_move_data, > .supports_v2 = smc_lo_supports_v2, > .get_system_eid = smc_lo_get_system_eid, > .get_local_gid = smc_lo_get_local_gid, > @@ -212,6 +332,8 @@ static void smc_lo_dev_release(struct device *dev) > static int smc_lo_dev_init(struct smc_lo_dev *ldev) > { > smc_lo_generate_id(ldev); > + rwlock_init(&ldev->dmb_ht_lock); > + hash_init(ldev->dmb_ht); > > return smcd_lo_register_dev(ldev); > } > diff --git a/net/smc/smc_loopback.h b/net/smc/smc_loopback.h > index 2156f22..943424f 100644 > --- a/net/smc/smc_loopback.h > +++ b/net/smc/smc_loopback.h > @@ -20,12 +20,25 @@ > > #define SMC_LO_CHID 0xFFFF > #define SMC_LODEV_MAX_DMBS 5000 > +#define SMC_LODEV_DMBS_HASH_BITS 12 > + > +struct smc_lo_dmb_node { > + struct hlist_node list; > + u64 token; > + u32 len; > + u32 sba_idx; > + void *cpu_addr; > + dma_addr_t dma_addr; > +}; > > struct smc_lo_dev { > struct smcd_dev *smcd; > struct device dev; > u16 chid; > struct smcd_gid local_gid; > + DECLARE_BITMAP(sba_idx_mask, SMC_LODEV_MAX_DMBS); > + rwlock_t dmb_ht_lock; > + DECLARE_HASHTABLE(dmb_ht, SMC_LODEV_DMBS_HASH_BITS); > }; > > int smc_loopback_init(void); ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next v3 12/18] net/smc: implement DMB-related operations of loopback 2023-09-21 23:31 ` [PATCH net-next v3 12/18] net/smc: implement DMB-related operations of loopback Wenjia Zhang @ 2023-09-22 7:42 ` Wen Gu 0 siblings, 0 replies; 10+ messages in thread From: Wen Gu @ 2023-09-22 7:42 UTC (permalink / raw) To: Wenjia Zhang, kgraul, jaka, davem, edumazet, kuba, pabeni Cc: alibuda, tonylu, linux-s390, netdev, linux-kernel On 2023/9/22 07:31, Wenjia Zhang wrote: > > <...> >> +static int smc_lo_register_dmb(struct smcd_dev *smcd, struct smcd_dmb *dmb, >> + void *client_priv) >> +{ >> + struct smc_lo_dmb_node *dmb_node, *tmp_node; >> + struct smc_lo_dev *ldev = smcd->priv; >> + int sba_idx, rc; >> + >> + /* check space for new dmb */ >> + for_each_clear_bit(sba_idx, ldev->sba_idx_mask, SMC_LODEV_MAX_DMBS) { >> + if (!test_and_set_bit(sba_idx, ldev->sba_idx_mask)) >> + break; >> + } >> + if (sba_idx == SMC_LODEV_MAX_DMBS) >> + return -ENOSPC; >> + >> + dmb_node = kzalloc(sizeof(*dmb_node), GFP_KERNEL); >> + if (!dmb_node) { >> + rc = -ENOMEM; >> + goto err_bit; >> + } >> + >> + dmb_node->sba_idx = sba_idx; >> + dmb_node->cpu_addr = kzalloc(dmb->dmb_len, GFP_KERNEL | >> + __GFP_NOWARN | __GFP_NORETRY | >> + __GFP_NOMEMALLOC); > kzalloc()/kmalloc() allocates physically contigueous memory. Are you sure it is suitable for allocating the dmb? > Yes, physically contigueous memory is little expensive here. I initially wanted to see the best performance. I tried using vzalloc here, and the performance dropped a bit (2%~8%) compared to kzalloc. I think it is acceptable. - ipc-benchmark kzalloc vzalloc Message rate (msg/s) 152076 145753(-4.16%) - sockperf kzalloc vzalloc Bandwidth(MBps) 8491.638 8002.380(-5.76%) Latency(us) 3.222 3.508(+8.88%) - nginx/wrk kzalloc vzalloc Requests/s 272519.36 256490.94(-5.88%) - redis-benchmark kzalloc vzalloc GET(Requests/s) 123304.56 120084.05(-2.61%) SET(Requests/s) 122062.87 118800.12(-2.67%) >> + if (!dmb_node->cpu_addr) { >> + rc = -ENOMEM; >> + goto err_node; >> + } >> + dmb_node->len = dmb->dmb_len; >> + dmb_node->dma_addr = (dma_addr_t)dmb_node->cpu_addr; >> + >> +again: >> + /* add new dmb into hash table */ >> + get_random_bytes(&dmb_node->token, sizeof(dmb_node->token)); >> + write_lock(&ldev->dmb_ht_lock); >> + hash_for_each_possible(ldev->dmb_ht, tmp_node, list, dmb_node->token) { >> + if (tmp_node->token == dmb_node->token) { >> + write_unlock(&ldev->dmb_ht_lock); >> + goto again; >> + } >> + } >> + hash_add(ldev->dmb_ht, &dmb_node->list, dmb_node->token); >> + write_unlock(&ldev->dmb_ht_lock); >> + >> + dmb->sba_idx = dmb_node->sba_idx; >> + dmb->dmb_tok = dmb_node->token; >> + dmb->cpu_addr = dmb_node->cpu_addr; >> + dmb->dma_addr = dmb_node->dma_addr; >> + dmb->dmb_len = dmb_node->len; >> + >> + return 0; >> + >> +err_node: >> + kfree(dmb_node); >> +err_bit: >> + clear_bit(sba_idx, ldev->sba_idx_mask); >> + return rc; >> +} >> + >> +static int smc_lo_unregister_dmb(struct smcd_dev *smcd, struct smcd_dmb *dmb) >> +{ >> + struct smc_lo_dmb_node *dmb_node = NULL, *tmp_node; >> + struct smc_lo_dev *ldev = smcd->priv; >> + >> + /* remove dmb from hash table */ >> + write_lock(&ldev->dmb_ht_lock); >> + hash_for_each_possible(ldev->dmb_ht, tmp_node, list, dmb->dmb_tok) { >> + if (tmp_node->token == dmb->dmb_tok) { >> + dmb_node = tmp_node; >> + break; >> + } >> + } >> + if (!dmb_node) { >> + write_unlock(&ldev->dmb_ht_lock); >> + return -EINVAL; >> + } >> + hash_del(&dmb_node->list); >> + write_unlock(&ldev->dmb_ht_lock); >> + >> + clear_bit(dmb_node->sba_idx, ldev->sba_idx_mask); >> + kfree(dmb_node->cpu_addr); >> + kfree(dmb_node); >> + >> + return 0; >> +} >> + ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <1695302360-46691-10-git-send-email-guwen@linux.alibaba.com>]
* Re: [PATCH net-next v3 09/18] net/smc: introduce SMC-D loopback device [not found] ` <1695302360-46691-10-git-send-email-guwen@linux.alibaba.com> @ 2023-09-21 23:32 ` Wenjia Zhang 2023-09-22 7:55 ` Wen Gu 0 siblings, 1 reply; 10+ messages in thread From: Wenjia Zhang @ 2023-09-21 23:32 UTC (permalink / raw) To: Wen Gu, kgraul, jaka, davem, edumazet, kuba, pabeni Cc: alibuda, tonylu, linux-s390, netdev, linux-kernel On 21.09.23 15:19, Wen Gu wrote: > This patch introduces a kind of loopback device for SMC-D. The device > is created when SMC module is loaded and destroyed when the SMC module > is unloaded. The loopback device is a kernel device used only by the > SMC module and is not restricted by net namespace, so it can be used > for local inter-process or inter-container communication. > > Signed-off-by: Wen Gu <guwen@linux.alibaba.com> > --- > net/smc/Makefile | 2 +- > net/smc/af_smc.c | 12 +++- > net/smc/smc_loopback.c | 157 +++++++++++++++++++++++++++++++++++++++++++++++++ > net/smc/smc_loopback.h | 31 ++++++++++ > 4 files changed, 200 insertions(+), 2 deletions(-) > create mode 100644 net/smc/smc_loopback.c > create mode 100644 net/smc/smc_loopback.h > > diff --git a/net/smc/Makefile b/net/smc/Makefile > index 875efcd..a8c3711 100644 > --- a/net/smc/Makefile > +++ b/net/smc/Makefile > @@ -4,5 +4,5 @@ obj-$(CONFIG_SMC) += smc.o > obj-$(CONFIG_SMC_DIAG) += smc_diag.o > smc-y := af_smc.o smc_pnet.o smc_ib.o smc_clc.o smc_core.o smc_wr.o smc_llc.o > smc-y += smc_cdc.o smc_tx.o smc_rx.o smc_close.o smc_ism.o smc_netlink.o smc_stats.o > -smc-y += smc_tracepoint.o > +smc-y += smc_tracepoint.o smc_loopback.o > smc-$(CONFIG_SYSCTL) += smc_sysctl.o > diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c > index 7eab600..bc4300e 100644 > --- a/net/smc/af_smc.c > +++ b/net/smc/af_smc.c > @@ -53,6 +53,7 @@ > #include "smc_stats.h" > #include "smc_tracepoint.h" > #include "smc_sysctl.h" > +#include "smc_loopback.h" > > static DEFINE_MUTEX(smc_server_lgr_pending); /* serialize link group > * creation on server > @@ -3552,15 +3553,23 @@ static int __init smc_init(void) > goto out_sock; > } > > + rc = smc_loopback_init(); > + if (rc) { > + pr_err("%s: smc_loopback_init fails with %d\n", __func__, rc); > + goto out_ib; > + } > + > rc = tcp_register_ulp(&smc_ulp_ops); > if (rc) { > pr_err("%s: tcp_ulp_register fails with %d\n", __func__, rc); > - goto out_ib; > + goto out_lo; > } > > static_branch_enable(&tcp_have_smc); > return 0; > > +out_lo: > + smc_loopback_exit(); > out_ib: > smc_ib_unregister_client(); > out_sock: > @@ -3598,6 +3607,7 @@ static void __exit smc_exit(void) > tcp_unregister_ulp(&smc_ulp_ops); > sock_unregister(PF_SMC); > smc_core_exit(); > + smc_loopback_exit(); > smc_ib_unregister_client(); > smc_ism_exit(); > destroy_workqueue(smc_close_wq); > diff --git a/net/smc/smc_loopback.c b/net/smc/smc_loopback.c > new file mode 100644 > index 0000000..7d88856 > --- /dev/null > +++ b/net/smc/smc_loopback.c > @@ -0,0 +1,157 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Shared Memory Communications Direct over loopback device. > + * > + * Provide a SMC-D loopback dummy device. > + * > + * Copyright (c) 2022, Alibaba Inc. > + * > + * Author: Wen Gu <guwen@linux.alibaba.com> > + * Tony Lu <tonylu@linux.alibaba.com> > + * > + */ > + > +#include <linux/device.h> > +#include <linux/types.h> > +#include <net/smc.h> > + > +#include "smc_ism.h" > +#include "smc_loopback.h" > + > +static const char smc_lo_dev_name[] = "smc_lo"; > +static struct smc_lo_dev *lo_dev; > + > +static const struct smcd_ops lo_ops = { > + .query_remote_gid = NULL, > + .register_dmb = NULL, > + .unregister_dmb = NULL, > + .add_vlan_id = NULL, > + .del_vlan_id = NULL, > + .set_vlan_required = NULL, > + .reset_vlan_required = NULL, > + .signal_event = NULL, > + .move_data = NULL, > + .supports_v2 = NULL, > + .get_system_eid = NULL, > + .get_local_gid = NULL, > + .get_chid = NULL, > + .get_dev = NULL, > +}; > + > +static struct smcd_dev *smcd_lo_alloc_dev(const struct smcd_ops *ops, > + int max_dmbs) > +{ > + struct smcd_dev *smcd; > + > + smcd = kzalloc(sizeof(*smcd), GFP_KERNEL); > + if (!smcd) > + return NULL; > + > + smcd->conn = kcalloc(max_dmbs, sizeof(struct smc_connection *), > + GFP_KERNEL); > + if (!smcd->conn) > + goto out_smcd; > + > + smcd->ops = ops; > + > + spin_lock_init(&smcd->lock); > + spin_lock_init(&smcd->lgr_lock); > + INIT_LIST_HEAD(&smcd->vlan); > + INIT_LIST_HEAD(&smcd->lgr_list); > + init_waitqueue_head(&smcd->lgrs_deleted); > + return smcd; > + > +out_smcd: > + kfree(smcd); > + return NULL; > +} > + > +static int smcd_lo_register_dev(struct smc_lo_dev *ldev) > +{ > + struct smcd_dev *smcd; > + > + smcd = smcd_lo_alloc_dev(&lo_ops, SMC_LODEV_MAX_DMBS); > + if (!smcd) > + return -ENOMEM; > + > + ldev->smcd = smcd; > + smcd->priv = ldev; > + > + /* TODO: > + * register smc_lo to smcd_dev list. > + */ > + return 0; > +} > + > +static void smcd_lo_unregister_dev(struct smc_lo_dev *ldev) > +{ > + /* TODO: > + * unregister smc_lo from smcd_dev list. > + */ > +} > + > +static void smc_lo_dev_release(struct device *dev) > +{ > + struct smc_lo_dev *ldev = > + container_of(dev, struct smc_lo_dev, dev); > + struct smcd_dev *smcd = ldev->smcd; > + > + kfree(smcd->conn); > + kfree(smcd); > + kfree(ldev); > +} > + > +static int smc_lo_dev_init(struct smc_lo_dev *ldev) > +{ > + return smcd_lo_register_dev(ldev); > +} > + > +static int smc_lo_dev_probe(void) > +{ > + struct smc_lo_dev *ldev; > + int ret; > + > + ldev = kzalloc(sizeof(*ldev), GFP_KERNEL); > + if (!ldev) > + return -ENOMEM; > + > + ldev->dev.parent = NULL; > + ldev->dev.release = smc_lo_dev_release; > + device_initialize(&ldev->dev); > + dev_set_name(&ldev->dev, smc_lo_dev_name); > + > + ret = smc_lo_dev_init(ldev); > + if (ret) > + goto free_dev; > + > + lo_dev = ldev; /* global loopback device */ > + return 0; > + > +free_dev: > + kfree(ldev); > + return ret; > +} > + > +static void smc_lo_dev_exit(struct smc_lo_dev *ldev) > +{ > + smcd_lo_unregister_dev(ldev); > +} > + > +static void smc_lo_dev_remove(void) > +{ > + if (!lo_dev) > + return; > + > + smc_lo_dev_exit(lo_dev); > + put_device(&lo_dev->dev); /* device_initialize in smc_lo_dev_probe */ > +} > + > +int smc_loopback_init(void) > +{ > + return smc_lo_dev_probe(); > +} > + > +void smc_loopback_exit(void) > +{ > + smc_lo_dev_remove(); > +} > diff --git a/net/smc/smc_loopback.h b/net/smc/smc_loopback.h > new file mode 100644 > index 0000000..0f7583c > --- /dev/null > +++ b/net/smc/smc_loopback.h > @@ -0,0 +1,31 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Shared Memory Communications Direct over loopback device. > + * > + * Provide a SMC-D loopback dummy device. > + * > + * Copyright (c) 2022, Alibaba Inc. > + * > + * Author: Wen Gu <guwen@linux.alibaba.com> > + * Tony Lu <tonylu@linux.alibaba.com> > + * > + */ > + > +#ifndef _SMC_LOOPBACK_H > +#define _SMC_LOOPBACK_H > + > +#include <linux/device.h> > +#include <linux/err.h> > +#include <net/smc.h> > + > +#define SMC_LODEV_MAX_DMBS 5000 > + > +struct smc_lo_dev { > + struct smcd_dev *smcd; > + struct device dev; > +}; > + I'm just wondering why don't use pointer for dev? > +int smc_loopback_init(void); > +void smc_loopback_exit(void); > + > +#endif /* _SMC_LOOPBACK_H */ ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next v3 09/18] net/smc: introduce SMC-D loopback device 2023-09-21 23:32 ` [PATCH net-next v3 09/18] net/smc: introduce SMC-D loopback device Wenjia Zhang @ 2023-09-22 7:55 ` Wen Gu 0 siblings, 0 replies; 10+ messages in thread From: Wen Gu @ 2023-09-22 7:55 UTC (permalink / raw) To: Wenjia Zhang, kgraul, jaka, davem, edumazet, kuba, pabeni Cc: alibuda, tonylu, linux-s390, netdev, linux-kernel On 2023/9/22 07:32, Wenjia Zhang wrote: > > <..> >> + >> +#ifndef _SMC_LOOPBACK_H >> +#define _SMC_LOOPBACK_H >> + >> +#include <linux/device.h> >> +#include <linux/err.h> >> +#include <net/smc.h> >> + >> +#define SMC_LODEV_MAX_DMBS 5000 >> + >> +struct smc_lo_dev { >> + struct smcd_dev *smcd; >> + struct device dev; >> +}; >> + > I'm just wondering why don't use pointer for dev? > The 'struct device' is a generic structure embeded, used to associate a Linux device with a specific device. So I think variable for dev here is OK. See also struct ism_dev. Thanks. >> +int smc_loopback_init(void); >> +void smc_loopback_exit(void); >> + >> +#endif /* _SMC_LOOPBACK_H */ ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <1695302360-46691-7-git-send-email-guwen@linux.alibaba.com>]
* Re: [PATCH net-next v3 06/18] net/smc: extend GID to 128bits for virtual ISM device [not found] ` <1695302360-46691-7-git-send-email-guwen@linux.alibaba.com> @ 2023-09-21 23:32 ` Wenjia Zhang 2023-09-22 12:12 ` Wen Gu 0 siblings, 1 reply; 10+ messages in thread From: Wenjia Zhang @ 2023-09-21 23:32 UTC (permalink / raw) To: Wen Gu, kgraul, jaka, davem, edumazet, kuba, pabeni Cc: alibuda, tonylu, linux-s390, netdev, linux-kernel On 21.09.23 15:19, Wen Gu wrote: > As the SMC-Dv2 protocol introduces virtual ISM devices, whose GIDs > are UUIDs generated by software, the maximum length of SMC-D GID has > been extended to 128 bits. > I think it is necessary to say that the GID is extended to 128 bits only for virtuel ISM device(s). > So this patch adapts the relevant code to make it compatible with > 128 bits GID. > > Signed-off-by: Wen Gu <guwen@linux.alibaba.com> > --- > drivers/s390/net/ism_drv.c | 17 +++++++++-------- > include/net/smc.h | 15 ++++++++++----- > include/uapi/linux/smc.h | 3 +++ > net/smc/af_smc.c | 45 +++++++++++++++++++++++++++++++++------------ > net/smc/smc_clc.c | 35 ++++++++++++++++++++++++----------- > net/smc/smc_clc.h | 4 ++-- > net/smc/smc_core.c | 38 +++++++++++++++++++++++++++++--------- > net/smc/smc_core.h | 7 ++++--- > net/smc/smc_diag.c | 6 ++++-- > net/smc/smc_ism.c | 18 ++++++++++++------ > net/smc/smc_ism.h | 3 ++- > net/smc/smc_pnet.c | 4 ++-- > 12 files changed, 134 insertions(+), 61 deletions(-) > > diff --git a/drivers/s390/net/ism_drv.c b/drivers/s390/net/ism_drv.c > index a34e913..7801a6a 100644 > --- a/drivers/s390/net/ism_drv.c > +++ b/drivers/s390/net/ism_drv.c > @@ -774,10 +774,10 @@ static void __exit ism_exit(void) > /*************************** SMC-D Implementation *****************************/ > > #if IS_ENABLED(CONFIG_SMC) > -static int smcd_query_rgid(struct smcd_dev *smcd, u64 rgid, u32 vid_valid, > - u32 vid) > +static int smcd_query_rgid(struct smcd_dev *smcd, struct smcd_gid *rgid, > + u32 vid_valid, u32 vid) > { > - return ism_query_rgid(smcd->priv, rgid, vid_valid, vid); > + return ism_query_rgid(smcd->priv, rgid->gid, vid_valid, vid); > } > > static int smcd_register_dmb(struct smcd_dev *smcd, struct smcd_dmb *dmb, > @@ -811,10 +811,10 @@ static int smcd_reset_vlan_required(struct smcd_dev *smcd) > return ism_cmd_simple(smcd->priv, ISM_RESET_VLAN); > } > > -static int smcd_signal_ieq(struct smcd_dev *smcd, u64 rgid, u32 trigger_irq, > - u32 event_code, u64 info) > +static int smcd_signal_ieq(struct smcd_dev *smcd, struct smcd_gid *rgid, > + u32 trigger_irq, u32 event_code, u64 info) > { > - return ism_signal_ieq(smcd->priv, rgid, trigger_irq, event_code, info); > + return ism_signal_ieq(smcd->priv, rgid->gid, trigger_irq, event_code, info); > } > > static int smcd_move(struct smcd_dev *smcd, u64 dmb_tok, unsigned int idx, > @@ -830,9 +830,10 @@ static int smcd_supports_v2(void) > SYSTEM_EID.type[0] != '0'; > } > > -static u64 smcd_get_local_gid(struct smcd_dev *smcd) > +static void smcd_get_local_gid(struct smcd_dev *smcd, > + struct smcd_gid *smcd_gid) > { > - return ism_get_local_gid(smcd->priv); > + smcd_gid->gid = ism_get_local_gid(smcd->priv); > } > > static u16 smcd_get_chid(struct smcd_dev *smcd) > diff --git a/include/net/smc.h b/include/net/smc.h > index f75116e..d8db5e1 100644 > --- a/include/net/smc.h > +++ b/include/net/smc.h > @@ -51,9 +51,14 @@ struct smcd_dmb { > > struct smcd_dev; > > +struct smcd_gid { > + u64 gid; > + u64 gid_ext; > +}; > + > struct smcd_ops { > - int (*query_remote_gid)(struct smcd_dev *dev, u64 rgid, u32 vid_valid, > - u32 vid); > + int (*query_remote_gid)(struct smcd_dev *dev, struct smcd_gid *rgid, > + u32 vid_valid, u32 vid); > int (*register_dmb)(struct smcd_dev *dev, struct smcd_dmb *dmb, > void *client); > int (*unregister_dmb)(struct smcd_dev *dev, struct smcd_dmb *dmb); > @@ -61,14 +66,14 @@ struct smcd_ops { > int (*del_vlan_id)(struct smcd_dev *dev, u64 vlan_id); > int (*set_vlan_required)(struct smcd_dev *dev); > int (*reset_vlan_required)(struct smcd_dev *dev); > - int (*signal_event)(struct smcd_dev *dev, u64 rgid, u32 trigger_irq, > - u32 event_code, u64 info); > + int (*signal_event)(struct smcd_dev *dev, struct smcd_gid *rgid, > + u32 trigger_irq, u32 event_code, u64 info); > int (*move_data)(struct smcd_dev *dev, u64 dmb_tok, unsigned int idx, > bool sf, unsigned int offset, void *data, > unsigned int size); > int (*supports_v2)(void); > u8* (*get_system_eid)(void); > - u64 (*get_local_gid)(struct smcd_dev *dev); > + void (*get_local_gid)(struct smcd_dev *dev, struct smcd_gid *gid); > u16 (*get_chid)(struct smcd_dev *dev); > struct device* (*get_dev)(struct smcd_dev *dev); > }; > diff --git a/include/uapi/linux/smc.h b/include/uapi/linux/smc.h > index 837fcd4..0d2f020 100644 > --- a/include/uapi/linux/smc.h > +++ b/include/uapi/linux/smc.h > @@ -99,6 +99,9 @@ enum { > SMC_NLA_LGR_V2_OS, /* u8 */ > SMC_NLA_LGR_V2_NEG_EID, /* string */ > SMC_NLA_LGR_V2_PEER_HOST, /* string */ > + SMC_NLA_LGR_V2_PAD, /* flag */ > + SMC_NLA_LGR_V2_GID_EXT, /* u64 */ > + SMC_NLA_LGR_V2_PEER_GID_EXT, /* u64 */ > __SMC_NLA_LGR_V2_MAX, > SMC_NLA_LGR_V2_MAX = __SMC_NLA_LGR_V2_MAX - 1 > }; > diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c > index bacdd97..9e31033 100644 > --- a/net/smc/af_smc.c > +++ b/net/smc/af_smc.c > @@ -1044,7 +1044,8 @@ static int smc_find_ism_v2_device_clnt(struct smc_sock *smc, > { > int rc = SMC_CLC_DECL_NOSMCDDEV; > struct smcd_dev *smcd; > - int i = 1; > + int i = 1, slot = 1; > + bool is_virtdev; > u16 chid; > > if (smcd_indicated(ini->smc_type_v1)) > @@ -1056,14 +1057,19 @@ static int smc_find_ism_v2_device_clnt(struct smc_sock *smc, > chid = smc_ism_get_chid(smcd); > if (!smc_find_ism_v2_is_unique_chid(chid, ini, i)) > continue; > + is_virtdev = __smc_ism_is_virtdev(chid); > if (!smc_pnet_is_pnetid_set(smcd->pnetid) || > smc_pnet_is_ndev_pnetid(sock_net(&smc->sk), smcd->pnetid)) { > + /* GID-CHID array is not enough for a virtual GID (128bits) */ > + if (is_virtdev && slot >= SMC_MAX_ISM_DEVS) > + continue; I'm wondering what the check is for. e.g. If slot is 8 after checking if (slot > SMC_MAX_ISM_DEVS) later, then does makes sense to continue the iteration? > ini->ism_dev[i] = smcd; > ini->ism_chid[i] = chid; > ini->is_smcd = true; > rc = 0; > i++; > - if (i > SMC_MAX_ISM_DEVS) > + slot = is_virtdev ? slot + 2 : slot + 1; > + if (slot > SMC_MAX_ISM_DEVS) > break; > } > } > @@ -1409,8 +1415,9 @@ static int smc_connect_ism(struct smc_sock *smc, > rc = smc_v2_determine_accepted_chid(aclc_v2, ini); > if (rc) > return rc; > + ini->ism_peer_gid[ini->ism_selected].gid_ext = aclc_v2->d1.gid_ext; > } > - ini->ism_peer_gid[ini->ism_selected] = aclc->d0.gid; > + ini->ism_peer_gid[ini->ism_selected].gid = aclc->d0.gid; > > /* there is only one lgr role for SMC-D; use server lock */ > mutex_lock(&smc_server_lgr_pending); > @@ -2101,7 +2108,8 @@ static bool smc_is_already_selected(struct smcd_dev *smcd, > > /* check for ISM devices matching proposed ISM devices */ > static void smc_check_ism_v2_match(struct smc_init_info *ini, > - u16 proposed_chid, u64 proposed_gid, > + u16 proposed_chid, > + struct smcd_gid *proposed_gid, > unsigned int *matches) > { > struct smcd_dev *smcd; > @@ -2113,7 +2121,10 @@ static void smc_check_ism_v2_match(struct smc_init_info *ini, > continue; > if (smc_ism_get_chid(smcd) == proposed_chid && > !smc_ism_cantalk(proposed_gid, ISM_RESERVED_VLANID, smcd)) { > - ini->ism_peer_gid[*matches] = proposed_gid; > + ini->ism_peer_gid[*matches].gid = proposed_gid->gid; > + if (__smc_ism_is_virtdev(proposed_chid)) > + ini->ism_peer_gid[*matches].gid_ext = > + proposed_gid->gid_ext; > ini->ism_dev[*matches] = smcd; > (*matches)++; > break; > @@ -2135,9 +2146,11 @@ static void smc_find_ism_v2_device_serv(struct smc_sock *new_smc, > struct smc_clc_v2_extension *smc_v2_ext; > struct smc_clc_msg_smcd *pclc_smcd; > unsigned int matches = 0; > + struct smcd_gid smcd_gid; > u8 smcd_version; > u8 *eid = NULL; > int i, rc; > + u16 chid; > > if (!(ini->smcd_version & SMC_V2) || !smcd_indicated(ini->smc_type_v2)) > goto not_found; > @@ -2147,18 +2160,26 @@ static void smc_find_ism_v2_device_serv(struct smc_sock *new_smc, > smcd_v2_ext = smc_get_clc_smcd_v2_ext(smc_v2_ext); > > mutex_lock(&smcd_dev_list.mutex); > - if (pclc_smcd->ism.chid) > + if (pclc_smcd->ism.chid) { > /* check for ISM device matching proposed native ISM device */ > + smcd_gid.gid = ntohll(pclc_smcd->ism.gid); > smc_check_ism_v2_match(ini, ntohs(pclc_smcd->ism.chid), > - ntohll(pclc_smcd->ism.gid), &matches); > + &smcd_gid, &matches); > + } > for (i = 1; i <= smc_v2_ext->hdr.ism_gid_cnt; i++) { > /* check for ISM devices matching proposed non-native ISM > * devices > */ > - smc_check_ism_v2_match(ini, > - ntohs(smcd_v2_ext->gidchid[i - 1].chid), > - ntohll(smcd_v2_ext->gidchid[i - 1].gid), > - &matches); > + smcd_gid.gid = ntohll(smcd_v2_ext->gidchid[i - 1].gid); > + chid = ntohs(smcd_v2_ext->gidchid[i - 1].chid); > + if (__smc_ism_is_virtdev(chid)) { > + /* check if extended entry exists and is valid */ > + if (i >= smc_v2_ext->hdr.ism_gid_cnt || > + chid != ntohs(smcd_v2_ext->gidchid[i].chid)) > + continue; > + smcd_gid.gid_ext = ntohll(smcd_v2_ext->gidchid[i++].gid); > + } > + smc_check_ism_v2_match(ini, chid, &smcd_gid, &matches); Here is also not that clean with smcd_gid. E.g. the 1st device is a virtual device, then smcd_gid.gid_ext = ntohll(smcd_v2_ext->gidchid[2].gid) while the 2nd device is not virtual one, then the smcd_gid.gid is rewritten, but the smcd_gid.gid_ext is still ntohll(smcd_v2_ext->gidchid[2].gid) > } > mutex_unlock(&smcd_dev_list.mutex); > IMO, I don't think that is a good idea to use the same varable smcd_gid for checking both native ISM device and non-native ISM device. Because the compiler can LOAD and Store the value you don't really want from the perspective of optimization. > @@ -2207,7 +2228,7 @@ static void smc_find_ism_v1_device_serv(struct smc_sock *new_smc, > if (!(ini->smcd_version & SMC_V1) || !smcd_indicated(ini->smc_type_v1)) > goto not_found; > ini->is_smcd = true; /* prepare ISM check */ > - ini->ism_peer_gid[0] = ntohll(pclc_smcd->ism.gid); > + ini->ism_peer_gid[0].gid = ntohll(pclc_smcd->ism.gid); > rc = smc_find_ism_device(new_smc, ini); > if (rc) > goto not_found; > diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c > index 125b0d2..c08e138 100644 > --- a/net/smc/smc_clc.c > +++ b/net/smc/smc_clc.c > @@ -882,11 +882,13 @@ int smc_clc_send_proposal(struct smc_sock *smc, struct smc_init_info *ini) > ETH_ALEN); > } > if (smcd_indicated(ini->smc_type_v1)) { > + struct smcd_gid smcd_gid; > + > /* add SMC-D specifics */ > if (ini->ism_dev[0]) { > smcd = ini->ism_dev[0]; > - pclc_smcd->ism.gid = > - htonll(smcd->ops->get_local_gid(smcd)); > + smcd->ops->get_local_gid(smcd, &smcd_gid); > + pclc_smcd->ism.gid = htonll(smcd_gid.gid); > pclc_smcd->ism.chid = > htons(smc_ism_get_chid(ini->ism_dev[0])); > } > @@ -919,10 +921,11 @@ int smc_clc_send_proposal(struct smc_sock *smc, struct smc_init_info *ini) > read_unlock(&smc_clc_eid_table.lock); > } > if (smcd_indicated(ini->smc_type_v2)) { > + struct smcd_gid smcd_gid; > u8 *eid = NULL; > + int slot = 0; > > v2_ext->hdr.flag.seid = smc_clc_eid_table.seid_enabled; > - v2_ext->hdr.ism_gid_cnt = ini->ism_offered_cnt; > v2_ext->hdr.smcd_v2_ext_offset = htons(sizeof(*v2_ext) - > offsetofend(struct smc_clnt_opts_area_hdr, > smcd_v2_ext_offset) + > @@ -934,14 +937,21 @@ int smc_clc_send_proposal(struct smc_sock *smc, struct smc_init_info *ini) > if (ini->ism_offered_cnt) { > for (i = 1; i <= ini->ism_offered_cnt; i++) { > smcd = ini->ism_dev[i]; > - gidchids[i - 1].gid = > - htonll(smcd->ops->get_local_gid(smcd)); > - gidchids[i - 1].chid = > + smcd->ops->get_local_gid(smcd, &smcd_gid); > + gidchids[slot].chid = > htons(smc_ism_get_chid(ini->ism_dev[i])); > + gidchids[slot].gid = htonll(smcd_gid.gid); > + /* virtual-ism takes two slots */ > + if (__smc_ism_is_virtdev(gidchids[slot].chid)) { > + gidchids[slot + 1].chid = gidchids[slot].chid; > + gidchids[slot + 1].gid = htonll(smcd_gid.gid_ext); > + slot++; > + } > + slot++; > } > - plen += ini->ism_offered_cnt * > - sizeof(struct smc_clc_smcd_gid_chid); > + plen += slot * sizeof(struct smc_clc_smcd_gid_chid); > } > + v2_ext->hdr.ism_gid_cnt = slot; > } > if (smcr_indicated(ini->smc_type_v2)) { > memcpy(v2_ext->roce, ini->smcrv2.ib_gid_v2, SMC_GID_SIZE); > @@ -977,7 +987,7 @@ int smc_clc_send_proposal(struct smc_sock *smc, struct smc_init_info *ini) > vec[i++].iov_len = sizeof(*smcd_v2_ext); > if (ini->ism_offered_cnt) { > vec[i].iov_base = gidchids; > - vec[i++].iov_len = ini->ism_offered_cnt * > + vec[i++].iov_len = v2_ext->hdr.ism_gid_cnt * > sizeof(struct smc_clc_smcd_gid_chid); > } > } > @@ -1019,12 +1029,14 @@ static int smc_clc_send_confirm_accept(struct smc_sock *smc, > if (first_contact) > clc->hdr.typev2 |= SMC_FIRST_CONTACT_MASK; > if (conn->lgr->is_smcd) { > + struct smcd_gid smcd_gid; > + > /* SMC-D specific settings */ > memcpy(clc->hdr.eyecatcher, SMCD_EYECATCHER, > sizeof(SMCD_EYECATCHER)); > + conn->lgr->smcd->ops->get_local_gid(conn->lgr->smcd, &smcd_gid); > clc->hdr.typev1 = SMC_TYPE_D; > - clc->d0.gid = > - conn->lgr->smcd->ops->get_local_gid(conn->lgr->smcd); > + clc->d0.gid = smcd_gid.gid; > clc->d0.token = conn->rmb_desc->token; > clc->d0.dmbe_size = conn->rmbe_size_comp; > clc->d0.dmbe_idx = 0; > @@ -1036,6 +1048,7 @@ static int smc_clc_send_confirm_accept(struct smc_sock *smc, > htons(smc_ism_get_chid(conn->lgr->smcd)); > if (eid && eid[0]) > memcpy(clc_v2->d1.eid, eid, SMC_MAX_EID_LEN); > + clc_v2->d1.gid_ext = smcd_gid.gid_ext; > len = SMCD_CLC_ACCEPT_CONFIRM_LEN_V2; > if (first_contact) { > fce_len = smc_clc_fill_fce(&fce, ini); > diff --git a/net/smc/smc_clc.h b/net/smc/smc_clc.h > index bcf37c8..611763a 100644 > --- a/net/smc/smc_clc.h > +++ b/net/smc/smc_clc.h > @@ -281,8 +281,8 @@ struct smc_clc_msg_accept_confirm_v2 { /* clc accept / confirm message */ > struct smcd_clc_msg_accept_confirm_common d0; > __be16 chid; > u8 eid[SMC_MAX_EID_LEN]; > - u8 reserved5[8]; > - } d1; > + __be64 gid_ext; > + } __packed d1; > }; > }; > > diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c > index d520ee6..c36500a 100644 > --- a/net/smc/smc_core.c > +++ b/net/smc/smc_core.c > @@ -284,6 +284,9 @@ static int smc_nl_fill_lgr_v2_common(struct smc_link_group *lgr, > { > char smc_host[SMC_MAX_HOSTNAME_LEN + 1]; > char smc_eid[SMC_MAX_EID_LEN + 1]; > + struct smcd_dev *smcd = lgr->smcd; > + struct smcd_gid smcd_gid; > + bool is_virtdev; > > if (nla_put_u8(skb, SMC_NLA_LGR_V2_VER, lgr->smc_version)) > goto errv2attr; > @@ -299,6 +302,16 @@ static int smc_nl_fill_lgr_v2_common(struct smc_link_group *lgr, > smc_eid[SMC_MAX_EID_LEN] = 0; > if (nla_put_string(skb, SMC_NLA_LGR_V2_NEG_EID, smc_eid)) > goto errv2attr; > + smcd->ops->get_local_gid(smcd, &smcd_gid); > + is_virtdev = smc_ism_is_virtdev(smcd); > + if (nla_put_u64_64bit(skb, SMC_NLA_LGR_V2_GID_EXT, > + is_virtdev ? smcd_gid.gid_ext : 0, > + SMC_NLA_LGR_V2_PAD)) > + goto errv2attr; > + if (nla_put_u64_64bit(skb, SMC_NLA_LGR_V2_PEER_GID_EXT, > + is_virtdev ? lgr->peer_gid.gid_ext : 0, > + SMC_NLA_LGR_V2_PAD)) > + goto errv2attr; > > nla_nest_end(skb, v2_attrs); > return 0; > @@ -506,6 +519,7 @@ static int smc_nl_fill_smcd_lgr(struct smc_link_group *lgr, > { > char smc_pnet[SMC_MAX_PNETID_LEN + 1]; > struct smcd_dev *smcd = lgr->smcd; > + struct smcd_gid smcd_gid; > struct nlattr *attrs; > void *nlh; > > @@ -521,11 +535,11 @@ static int smc_nl_fill_smcd_lgr(struct smc_link_group *lgr, > > if (nla_put_u32(skb, SMC_NLA_LGR_D_ID, *((u32 *)&lgr->id))) > goto errattr; > + smcd->ops->get_local_gid(smcd, &smcd_gid); > if (nla_put_u64_64bit(skb, SMC_NLA_LGR_D_GID, > - smcd->ops->get_local_gid(smcd), > - SMC_NLA_LGR_D_PAD)) > + smcd_gid.gid, SMC_NLA_LGR_D_PAD)) > goto errattr; > - if (nla_put_u64_64bit(skb, SMC_NLA_LGR_D_PEER_GID, lgr->peer_gid, > + if (nla_put_u64_64bit(skb, SMC_NLA_LGR_D_PEER_GID, lgr->peer_gid.gid, > SMC_NLA_LGR_D_PAD)) > goto errattr; > if (nla_put_u8(skb, SMC_NLA_LGR_D_VLAN_ID, lgr->vlan_id)) > @@ -1514,7 +1528,8 @@ void smc_lgr_terminate_sched(struct smc_link_group *lgr) > } > > /* Called when peer lgr shutdown (regularly or abnormally) is received */ > -void smc_smcd_terminate(struct smcd_dev *dev, u64 peer_gid, unsigned short vlan) > +void smc_smcd_terminate(struct smcd_dev *dev, struct smcd_gid *peer_gid, > + unsigned short vlan) > { > struct smc_link_group *lgr, *l; > LIST_HEAD(lgr_free_list); > @@ -1522,7 +1537,10 @@ void smc_smcd_terminate(struct smcd_dev *dev, u64 peer_gid, unsigned short vlan) > /* run common cleanup function and build free list */ > spin_lock_bh(&dev->lgr_lock); > list_for_each_entry_safe(lgr, l, &dev->lgr_list, list) { > - if ((!peer_gid || lgr->peer_gid == peer_gid) && > + if ((!peer_gid->gid || > + (lgr->peer_gid.gid == peer_gid->gid && > + !smc_ism_is_virtdev(dev) ? 1 : > + lgr->peer_gid.gid_ext == peer_gid->gid_ext)) && > (vlan == VLAN_VID_MASK || lgr->vlan_id == vlan)) { > if (peer_gid) /* peer triggered termination */ > lgr->peer_shutdown = 1; > @@ -1859,10 +1877,12 @@ static bool smcr_lgr_match(struct smc_link_group *lgr, u8 smcr_version, > return false; > } > > -static bool smcd_lgr_match(struct smc_link_group *lgr, > - struct smcd_dev *smcismdev, u64 peer_gid) > +static bool smcd_lgr_match(struct smc_link_group *lgr, struct smcd_dev *smcismdev, > + struct smcd_gid *peer_gid) > { > - return lgr->peer_gid == peer_gid && lgr->smcd == smcismdev; > + return lgr->peer_gid.gid == peer_gid->gid && lgr->smcd == smcismdev && > + smc_ism_is_virtdev(smcismdev) ? > + (lgr->peer_gid.gid_ext == peer_gid->gid_ext) : 1; > } > > /* create a new SMC connection (and a new link group if necessary) */ > @@ -1892,7 +1912,7 @@ int smc_conn_create(struct smc_sock *smc, struct smc_init_info *ini) > write_lock_bh(&lgr->conns_lock); > if ((ini->is_smcd ? > smcd_lgr_match(lgr, ini->ism_dev[ini->ism_selected], > - ini->ism_peer_gid[ini->ism_selected]) : > + &ini->ism_peer_gid[ini->ism_selected]) : > smcr_lgr_match(lgr, ini->smcr_version, > ini->peer_systemid, > ini->peer_gid, ini->peer_mac, role, > diff --git a/net/smc/smc_core.h b/net/smc/smc_core.h > index 9f65678..d57eb9b 100644 > --- a/net/smc/smc_core.h > +++ b/net/smc/smc_core.h > @@ -17,6 +17,7 @@ > #include <linux/pci.h> > #include <rdma/ib_verbs.h> > #include <net/genetlink.h> > +#include <net/smc.h> > > #include "smc.h" > #include "smc_ib.h" > @@ -355,7 +356,7 @@ struct smc_link_group { > /* max links can be added in lgr */ > }; > struct { /* SMC-D */ > - u64 peer_gid; > + struct smcd_gid peer_gid; > /* Peer GID (remote) */ > struct smcd_dev *smcd; > /* ISM device for VLAN reg. */ > @@ -417,7 +418,7 @@ struct smc_init_info { > u32 ib_clcqpn; > struct smc_init_info_smcrv2 smcrv2; > /* SMC-D */ > - u64 ism_peer_gid[SMC_MAX_ISM_DEVS + 1]; > + struct smcd_gid ism_peer_gid[SMC_MAX_ISM_DEVS + 1]; > struct smcd_dev *ism_dev[SMC_MAX_ISM_DEVS + 1]; > u16 ism_chid[SMC_MAX_ISM_DEVS + 1]; > u8 ism_offered_cnt; /* # of ISM devices offered */ > @@ -545,7 +546,7 @@ static inline void smc_set_pci_values(struct pci_dev *pci_dev, > void smc_lgr_put(struct smc_link_group *lgr); > void smcr_port_add(struct smc_ib_device *smcibdev, u8 ibport); > void smcr_port_err(struct smc_ib_device *smcibdev, u8 ibport); > -void smc_smcd_terminate(struct smcd_dev *dev, u64 peer_gid, > +void smc_smcd_terminate(struct smcd_dev *dev, struct smcd_gid *peer_gid, > unsigned short vlan); > void smc_smcd_terminate_all(struct smcd_dev *dev); > void smc_smcr_terminate_all(struct smc_ib_device *smcibdev); > diff --git a/net/smc/smc_diag.c b/net/smc/smc_diag.c > index 7ff2152..9c465cc 100644 > --- a/net/smc/smc_diag.c > +++ b/net/smc/smc_diag.c > @@ -168,12 +168,14 @@ static int __smc_diag_dump(struct sock *sk, struct sk_buff *skb, > struct smc_connection *conn = &smc->conn; > struct smcd_diag_dmbinfo dinfo; > struct smcd_dev *smcd = conn->lgr->smcd; > + struct smcd_gid smcd_gid; > > memset(&dinfo, 0, sizeof(dinfo)); > > dinfo.linkid = *((u32 *)conn->lgr->id); > - dinfo.peer_gid = conn->lgr->peer_gid; > - dinfo.my_gid = smcd->ops->get_local_gid(smcd); > + dinfo.peer_gid = conn->lgr->peer_gid.gid; > + smcd->ops->get_local_gid(smcd, &smcd_gid); > + dinfo.my_gid = smcd_gid.gid; > dinfo.token = conn->rmb_desc->token; > dinfo.peer_token = conn->peer_token; > > diff --git a/net/smc/smc_ism.c b/net/smc/smc_ism.c > index 8f1ba74..35d31e7 100644 > --- a/net/smc/smc_ism.c > +++ b/net/smc/smc_ism.c > @@ -44,7 +44,8 @@ static void smcd_handle_irq(struct ism_dev *ism, unsigned int dmbno, > #endif > > /* Test if an ISM communication is possible - same CPC */ > -int smc_ism_cantalk(u64 peer_gid, unsigned short vlan_id, struct smcd_dev *smcd) > +int smc_ism_cantalk(struct smcd_gid *peer_gid, unsigned short vlan_id, > + struct smcd_dev *smcd) > { > return smcd->ops->query_remote_gid(smcd, peer_gid, vlan_id ? 1 : 0, > vlan_id); > @@ -223,7 +224,7 @@ int smc_ism_register_dmb(struct smc_link_group *lgr, int dmb_len, > dmb.dmb_len = dmb_len; > dmb.sba_idx = dmb_desc->sba_idx; > dmb.vlan_id = lgr->vlan_id; > - dmb.rgid = lgr->peer_gid; > + dmb.rgid = lgr->peer_gid.gid; > rc = lgr->smcd->ops->register_dmb(lgr->smcd, &dmb, lgr->smcd->client); > if (!rc) { > dmb_desc->sba_idx = dmb.sba_idx; > @@ -354,17 +355,20 @@ struct smc_ism_event_work { > static void smcd_handle_sw_event(struct smc_ism_event_work *wrk) > { > union smcd_sw_event_info ev_info; > + struct smcd_gid peer_gid; > > ev_info.info = wrk->event.info; > switch (wrk->event.code) { > case ISM_EVENT_CODE_SHUTDOWN: /* Peer shut down DMBs */ > - smc_smcd_terminate(wrk->smcd, wrk->event.tok, ev_info.vlan_id); > + peer_gid.gid = wrk->event.tok; > + smc_smcd_terminate(wrk->smcd, &peer_gid, ev_info.vlan_id); > break; > case ISM_EVENT_CODE_TESTLINK: /* Activity timer */ > + peer_gid.gid = wrk->event.tok; > if (ev_info.code == ISM_EVENT_REQUEST) { > ev_info.code = ISM_EVENT_RESPONSE; > wrk->smcd->ops->signal_event(wrk->smcd, > - wrk->event.tok, > + &peer_gid, > ISM_EVENT_REQUEST_IR, > ISM_EVENT_CODE_TESTLINK, > ev_info.info); > @@ -378,10 +382,12 @@ static void smc_ism_event_work(struct work_struct *work) > { > struct smc_ism_event_work *wrk = > container_of(work, struct smc_ism_event_work, work); > + struct smcd_gid smcd_gid; > > switch (wrk->event.type) { > case ISM_EVENT_GID: /* GID event, token is peer GID */ > - smc_smcd_terminate(wrk->smcd, wrk->event.tok, VLAN_VID_MASK); > + smcd_gid.gid = wrk->event.tok; > + smc_smcd_terminate(wrk->smcd, &smcd_gid, VLAN_VID_MASK); > break; > case ISM_EVENT_DMB: > break; > @@ -530,7 +536,7 @@ int smc_ism_signal_shutdown(struct smc_link_group *lgr) > memcpy(ev_info.uid, lgr->id, SMC_LGR_ID_SIZE); > ev_info.vlan_id = lgr->vlan_id; > ev_info.code = ISM_EVENT_REQUEST; > - rc = lgr->smcd->ops->signal_event(lgr->smcd, lgr->peer_gid, > + rc = lgr->smcd->ops->signal_event(lgr->smcd, &lgr->peer_gid, > ISM_EVENT_REQUEST_IR, > ISM_EVENT_CODE_SHUTDOWN, > ev_info.info); > diff --git a/net/smc/smc_ism.h b/net/smc/smc_ism.h > index 2ecc8de..e6ea08c 100644 > --- a/net/smc/smc_ism.h > +++ b/net/smc/smc_ism.h > @@ -33,7 +33,8 @@ struct smc_ism_vlanid { /* VLAN id set on ISM device */ > > struct smcd_dev; > > -int smc_ism_cantalk(u64 peer_gid, unsigned short vlan_id, struct smcd_dev *dev); > +int smc_ism_cantalk(struct smcd_gid *peer_gid, unsigned short vlan_id, > + struct smcd_dev *dev); > void smc_ism_set_conn(struct smc_connection *conn); > void smc_ism_unset_conn(struct smc_connection *conn); > int smc_ism_get_vlan(struct smcd_dev *dev, unsigned short vlan_id); > diff --git a/net/smc/smc_pnet.c b/net/smc/smc_pnet.c > index 1177540..9f2c58c 100644 > --- a/net/smc/smc_pnet.c > +++ b/net/smc/smc_pnet.c > @@ -1103,8 +1103,8 @@ static void smc_pnet_find_ism_by_pnetid(struct net_device *ndev, > list_for_each_entry(ismdev, &smcd_dev_list.list, list) { > if (smc_pnet_match(ismdev->pnetid, ndev_pnetid) && > !ismdev->going_away && > - (!ini->ism_peer_gid[0] || > - !smc_ism_cantalk(ini->ism_peer_gid[0], ini->vlan_id, > + (!ini->ism_peer_gid[0].gid || > + !smc_ism_cantalk(&ini->ism_peer_gid[0], ini->vlan_id, > ismdev))) { > ini->ism_dev[0] = ismdev; > break; ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next v3 06/18] net/smc: extend GID to 128bits for virtual ISM device 2023-09-21 23:32 ` [PATCH net-next v3 06/18] net/smc: extend GID to 128bits for virtual ISM device Wenjia Zhang @ 2023-09-22 12:12 ` Wen Gu 0 siblings, 0 replies; 10+ messages in thread From: Wen Gu @ 2023-09-22 12:12 UTC (permalink / raw) To: Wenjia Zhang, kgraul, jaka, davem, edumazet, kuba, pabeni Cc: alibuda, tonylu, linux-s390, netdev, linux-kernel On 2023/9/22 07:32, Wenjia Zhang wrote: > > > On 21.09.23 15:19, Wen Gu wrote: >> As the SMC-Dv2 protocol introduces virtual ISM devices, whose GIDs >> are UUIDs generated by software, the maximum length of SMC-D GID has >> been extended to 128 bits. >> > I think it is necessary to say that the GID is extended to 128 bits only for virtuel ISM device(s). > Thanks, Wenjia. I will make it clear in the next version. >> So this patch adapts the relevant code to make it compatible with >> 128 bits GID. >> >> Signed-off-by: Wen Gu <guwen@linux.alibaba.com> >> --- <...> >> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c >> index bacdd97..9e31033 100644 >> --- a/net/smc/af_smc.c >> +++ b/net/smc/af_smc.c >> @@ -1044,7 +1044,8 @@ static int smc_find_ism_v2_device_clnt(struct smc_sock *smc, >> { >> int rc = SMC_CLC_DECL_NOSMCDDEV; >> struct smcd_dev *smcd; >> - int i = 1; >> + int i = 1, slot = 1; >> + bool is_virtdev; >> u16 chid; >> if (smcd_indicated(ini->smc_type_v1)) >> @@ -1056,14 +1057,19 @@ static int smc_find_ism_v2_device_clnt(struct smc_sock *smc, >> chid = smc_ism_get_chid(smcd); >> if (!smc_find_ism_v2_is_unique_chid(chid, ini, i)) >> continue; >> + is_virtdev = __smc_ism_is_virtdev(chid); >> if (!smc_pnet_is_pnetid_set(smcd->pnetid) || >> smc_pnet_is_ndev_pnetid(sock_net(&smc->sk), smcd->pnetid)) { >> + /* GID-CHID array is not enough for a virtual GID (128bits) */ >> + if (is_virtdev && slot >= SMC_MAX_ISM_DEVS) >> + continue; > > I'm wondering what the check is for. e.g. If slot is 8 after checking if (slot > SMC_MAX_ISM_DEVS) later, then does > makes sense to continue the iteration? If slot >= SMC_MAX_ISM_DEVS (actually, to be more precise, it should be 'if slot == SMC_MAX_ISM_DEVS', because it is impossible to have slot > SMC_MAX_ISM_DEVS here), e.g. slot == SMC_MAX_ISM_DEVS, which means it is the last slot of chid-gid array, it can no longer accommodate another virtual ISM device's GID, but it can accommodate an ISM device's GID. So here continue the iteration to carry another potential ISM device. I will change the judgement to 'if slot == SMC_MAX_ISM_DEVS', to be more precise. Thanks. >> ini->ism_dev[i] = smcd; >> ini->ism_chid[i] = chid; >> ini->is_smcd = true; >> rc = 0; >> i++; >> - if (i > SMC_MAX_ISM_DEVS) >> + slot = is_virtdev ? slot + 2 : slot + 1; >> + if (slot > SMC_MAX_ISM_DEVS) >> break; >> } >> } <...> >> @@ -2135,9 +2146,11 @@ static void smc_find_ism_v2_device_serv(struct smc_sock *new_smc, >> struct smc_clc_v2_extension *smc_v2_ext; >> struct smc_clc_msg_smcd *pclc_smcd; >> unsigned int matches = 0; >> + struct smcd_gid smcd_gid; >> u8 smcd_version; >> u8 *eid = NULL; >> int i, rc; >> + u16 chid; >> if (!(ini->smcd_version & SMC_V2) || !smcd_indicated(ini->smc_type_v2)) >> goto not_found; >> @@ -2147,18 +2160,26 @@ static void smc_find_ism_v2_device_serv(struct smc_sock *new_smc, >> smcd_v2_ext = smc_get_clc_smcd_v2_ext(smc_v2_ext); >> mutex_lock(&smcd_dev_list.mutex); >> - if (pclc_smcd->ism.chid) >> + if (pclc_smcd->ism.chid) { >> /* check for ISM device matching proposed native ISM device */ >> + smcd_gid.gid = ntohll(pclc_smcd->ism.gid); >> smc_check_ism_v2_match(ini, ntohs(pclc_smcd->ism.chid), >> - ntohll(pclc_smcd->ism.gid), &matches); >> + &smcd_gid, &matches); >> + } >> for (i = 1; i <= smc_v2_ext->hdr.ism_gid_cnt; i++) { >> /* check for ISM devices matching proposed non-native ISM >> * devices >> */ >> - smc_check_ism_v2_match(ini, >> - ntohs(smcd_v2_ext->gidchid[i - 1].chid), >> - ntohll(smcd_v2_ext->gidchid[i - 1].gid), >> - &matches); >> + smcd_gid.gid = ntohll(smcd_v2_ext->gidchid[i - 1].gid); >> + chid = ntohs(smcd_v2_ext->gidchid[i - 1].chid); >> + if (__smc_ism_is_virtdev(chid)) { >> + /* check if extended entry exists and is valid */ >> + if (i >= smc_v2_ext->hdr.ism_gid_cnt || >> + chid != ntohs(smcd_v2_ext->gidchid[i].chid)) >> + continue; >> + smcd_gid.gid_ext = ntohll(smcd_v2_ext->gidchid[i++].gid); >> + } >> + smc_check_ism_v2_match(ini, chid, &smcd_gid, &matches); > Here is also not that clean with smcd_gid. > E.g. the 1st device is a virtual device, then > smcd_gid.gid_ext = ntohll(smcd_v2_ext->gidchid[2].gid) > while the 2nd device is not virtual one, then the smcd_gid.gid is rewritten, but the smcd_gid.gid_ext is still > ntohll(smcd_v2_ext->gidchid[2].gid) smcd_gid.gid_ext in code is ensured to be meaningful when used. e.g. only when the device is virtual device, the smcd_gid will be used to fill CLC message or to compare with others. So even here 2nd device's smcd_gid is wrong, it won't be used. But I agree that we should keep the smcd_gid clean enough, I will check all the places to make it clean. Thanks! >> } >> mutex_unlock(&smcd_dev_list.mutex); > IMO, I don't think that is a good idea to use the same varable smcd_gid for checking both native ISM device and > non-native ISM device. Because the compiler can LOAD and Store the value you don't really want from the perspective of > optimization. > Are you worried that variables on the stack may be initialized to undefined values? Such as smcd_gid.gid_ext of an ISM device here is not assigned a value and could be any value? But in smc_check_ism_v2_match() we do 1. check CHID first, which means the following match will only occur between same type of devices (virtual ISM or ISM) 2. query remote gid, with variable smcd_gid. a. If devices are virtual ISM devices, both smcd_gid.gid and smcd_gid.gid_ext will be checked. e.g. smc_lo_query_rgid(). b. If devices are ISM devices, only smcd_gid.gid will be checked. e.g. smcd_query_rgid() in drivers/s390/net/ism_drv.c static int smcd_query_rgid(struct smcd_dev *smcd, struct smcd_gid *rgid, u32 vid_valid, u32 vid) { return ism_query_rgid(smcd->priv, rgid->gid, vid_valid, vid); ^^^^^^^^ only smcd_gid.gid will be passed to device for querying. } So I think it is safe to use the same variable smcd_gid for both virtual ISM and ISM. But as the previous reply said, I will make the ISM device's smcd_gid more clean (set smcd_gid.gid_ext to 0) Thanks. >> @@ -2207,7 +2228,7 @@ static void smc_find_ism_v1_device_serv(struct smc_sock *new_smc, >> if (!(ini->smcd_version & SMC_V1) || !smcd_indicated(ini->smc_type_v1)) >> goto not_found; >> ini->is_smcd = true; /* prepare ISM check */ >> - ini->ism_peer_gid[0] = ntohll(pclc_smcd->ism.gid); >> + ini->ism_peer_gid[0].gid = ntohll(pclc_smcd->ism.gid); >> rc = smc_find_ism_device(new_smc, ini); >> if (rc) >> goto not_found; >> diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c >> index 125b0d2..c08e138 100644 >> --- a/net/smc/smc_clc.c >> +++ b/net/smc/smc_clc.c >> @@ -882,11 +882,13 @@ int smc_clc_send_proposal(struct smc_sock *smc, struct smc_init_info *ini) >> ETH_ALEN); >> } >> if (smcd_indicated(ini->smc_type_v1)) { >> + struct smcd_gid smcd_gid; >> + >> /* add SMC-D specifics */ >> if (ini->ism_dev[0]) { >> smcd = ini->ism_dev[0]; >> - pclc_smcd->ism.gid = >> - htonll(smcd->ops->get_local_gid(smcd)); >> + smcd->ops->get_local_gid(smcd, &smcd_gid); >> + pclc_smcd->ism.gid = htonll(smcd_gid.gid); >> pclc_smcd->ism.chid = >> htons(smc_ism_get_chid(ini->ism_dev[0])); >> } >> @@ -919,10 +921,11 @@ int smc_clc_send_proposal(struct smc_sock *smc, struct smc_init_info *ini) >> read_unlock(&smc_clc_eid_table.lock); >> } >> if (smcd_indicated(ini->smc_type_v2)) { >> + struct smcd_gid smcd_gid; >> u8 *eid = NULL; >> + int slot = 0; >> v2_ext->hdr.flag.seid = smc_clc_eid_table.seid_enabled; >> - v2_ext->hdr.ism_gid_cnt = ini->ism_offered_cnt; >> v2_ext->hdr.smcd_v2_ext_offset = htons(sizeof(*v2_ext) - >> offsetofend(struct smc_clnt_opts_area_hdr, >> smcd_v2_ext_offset) + >> @@ -934,14 +937,21 @@ int smc_clc_send_proposal(struct smc_sock *smc, struct smc_init_info *ini) >> if (ini->ism_offered_cnt) { >> for (i = 1; i <= ini->ism_offered_cnt; i++) { >> smcd = ini->ism_dev[i]; >> - gidchids[i - 1].gid = >> - htonll(smcd->ops->get_local_gid(smcd)); >> - gidchids[i - 1].chid = >> + smcd->ops->get_local_gid(smcd, &smcd_gid); >> + gidchids[slot].chid = >> htons(smc_ism_get_chid(ini->ism_dev[i])); >> + gidchids[slot].gid = htonll(smcd_gid.gid); >> + /* virtual-ism takes two slots */ >> + if (__smc_ism_is_virtdev(gidchids[slot].chid)) { >> + gidchids[slot + 1].chid = gidchids[slot].chid; >> + gidchids[slot + 1].gid = htonll(smcd_gid.gid_ext); >> + slot++; >> + } >> + slot++; >> } >> - plen += ini->ism_offered_cnt * >> - sizeof(struct smc_clc_smcd_gid_chid); >> + plen += slot * sizeof(struct smc_clc_smcd_gid_chid); >> } >> + v2_ext->hdr.ism_gid_cnt = slot; >> } >> if (smcr_indicated(ini->smc_type_v2)) { >> memcpy(v2_ext->roce, ini->smcrv2.ib_gid_v2, SMC_GID_SIZE); >> @@ -977,7 +987,7 @@ int smc_clc_send_proposal(struct smc_sock *smc, struct smc_init_info *ini) >> vec[i++].iov_len = sizeof(*smcd_v2_ext); >> if (ini->ism_offered_cnt) { >> vec[i].iov_base = gidchids; >> - vec[i++].iov_len = ini->ism_offered_cnt * >> + vec[i++].iov_len = v2_ext->hdr.ism_gid_cnt * >> sizeof(struct smc_clc_smcd_gid_chid); >> } >> } >> @@ -1019,12 +1029,14 @@ static int smc_clc_send_confirm_accept(struct smc_sock *smc, >> if (first_contact) >> clc->hdr.typev2 |= SMC_FIRST_CONTACT_MASK; >> if (conn->lgr->is_smcd) { >> + struct smcd_gid smcd_gid; >> + >> /* SMC-D specific settings */ >> memcpy(clc->hdr.eyecatcher, SMCD_EYECATCHER, >> sizeof(SMCD_EYECATCHER)); >> + conn->lgr->smcd->ops->get_local_gid(conn->lgr->smcd, &smcd_gid); >> clc->hdr.typev1 = SMC_TYPE_D; >> - clc->d0.gid = >> - conn->lgr->smcd->ops->get_local_gid(conn->lgr->smcd); >> + clc->d0.gid = smcd_gid.gid; >> clc->d0.token = conn->rmb_desc->token; >> clc->d0.dmbe_size = conn->rmbe_size_comp; >> clc->d0.dmbe_idx = 0; >> @@ -1036,6 +1048,7 @@ static int smc_clc_send_confirm_accept(struct smc_sock *smc, >> htons(smc_ism_get_chid(conn->lgr->smcd)); >> if (eid && eid[0]) >> memcpy(clc_v2->d1.eid, eid, SMC_MAX_EID_LEN); >> + clc_v2->d1.gid_ext = smcd_gid.gid_ext; >> len = SMCD_CLC_ACCEPT_CONFIRM_LEN_V2; >> if (first_contact) { >> fce_len = smc_clc_fill_fce(&fce, ini); >> diff --git a/net/smc/smc_clc.h b/net/smc/smc_clc.h >> index bcf37c8..611763a 100644 >> --- a/net/smc/smc_clc.h >> +++ b/net/smc/smc_clc.h >> @@ -281,8 +281,8 @@ struct smc_clc_msg_accept_confirm_v2 { /* clc accept / confirm message */ >> struct smcd_clc_msg_accept_confirm_common d0; >> __be16 chid; >> u8 eid[SMC_MAX_EID_LEN]; >> - u8 reserved5[8]; >> - } d1; >> + __be64 gid_ext; >> + } __packed d1; >> }; >> }; >> diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c >> index d520ee6..c36500a 100644 >> --- a/net/smc/smc_core.c >> +++ b/net/smc/smc_core.c >> @@ -284,6 +284,9 @@ static int smc_nl_fill_lgr_v2_common(struct smc_link_group *lgr, >> { >> char smc_host[SMC_MAX_HOSTNAME_LEN + 1]; >> char smc_eid[SMC_MAX_EID_LEN + 1]; >> + struct smcd_dev *smcd = lgr->smcd; >> + struct smcd_gid smcd_gid; >> + bool is_virtdev; >> if (nla_put_u8(skb, SMC_NLA_LGR_V2_VER, lgr->smc_version)) >> goto errv2attr; >> @@ -299,6 +302,16 @@ static int smc_nl_fill_lgr_v2_common(struct smc_link_group *lgr, >> smc_eid[SMC_MAX_EID_LEN] = 0; >> if (nla_put_string(skb, SMC_NLA_LGR_V2_NEG_EID, smc_eid)) >> goto errv2attr; >> + smcd->ops->get_local_gid(smcd, &smcd_gid); >> + is_virtdev = smc_ism_is_virtdev(smcd); >> + if (nla_put_u64_64bit(skb, SMC_NLA_LGR_V2_GID_EXT, >> + is_virtdev ? smcd_gid.gid_ext : 0, >> + SMC_NLA_LGR_V2_PAD)) >> + goto errv2attr; >> + if (nla_put_u64_64bit(skb, SMC_NLA_LGR_V2_PEER_GID_EXT, >> + is_virtdev ? lgr->peer_gid.gid_ext : 0, >> + SMC_NLA_LGR_V2_PAD)) >> + goto errv2attr; >> nla_nest_end(skb, v2_attrs); >> return 0; >> @@ -506,6 +519,7 @@ static int smc_nl_fill_smcd_lgr(struct smc_link_group *lgr, >> { >> char smc_pnet[SMC_MAX_PNETID_LEN + 1]; >> struct smcd_dev *smcd = lgr->smcd; >> + struct smcd_gid smcd_gid; >> struct nlattr *attrs; >> void *nlh; >> @@ -521,11 +535,11 @@ static int smc_nl_fill_smcd_lgr(struct smc_link_group *lgr, >> if (nla_put_u32(skb, SMC_NLA_LGR_D_ID, *((u32 *)&lgr->id))) >> goto errattr; >> + smcd->ops->get_local_gid(smcd, &smcd_gid); >> if (nla_put_u64_64bit(skb, SMC_NLA_LGR_D_GID, >> - smcd->ops->get_local_gid(smcd), >> - SMC_NLA_LGR_D_PAD)) >> + smcd_gid.gid, SMC_NLA_LGR_D_PAD)) >> goto errattr; >> - if (nla_put_u64_64bit(skb, SMC_NLA_LGR_D_PEER_GID, lgr->peer_gid, >> + if (nla_put_u64_64bit(skb, SMC_NLA_LGR_D_PEER_GID, lgr->peer_gid.gid, >> SMC_NLA_LGR_D_PAD)) >> goto errattr; >> if (nla_put_u8(skb, SMC_NLA_LGR_D_VLAN_ID, lgr->vlan_id)) >> @@ -1514,7 +1528,8 @@ void smc_lgr_terminate_sched(struct smc_link_group *lgr) >> } >> /* Called when peer lgr shutdown (regularly or abnormally) is received */ >> -void smc_smcd_terminate(struct smcd_dev *dev, u64 peer_gid, unsigned short vlan) >> +void smc_smcd_terminate(struct smcd_dev *dev, struct smcd_gid *peer_gid, >> + unsigned short vlan) >> { >> struct smc_link_group *lgr, *l; >> LIST_HEAD(lgr_free_list); >> @@ -1522,7 +1537,10 @@ void smc_smcd_terminate(struct smcd_dev *dev, u64 peer_gid, unsigned short vlan) >> /* run common cleanup function and build free list */ >> spin_lock_bh(&dev->lgr_lock); >> list_for_each_entry_safe(lgr, l, &dev->lgr_list, list) { >> - if ((!peer_gid || lgr->peer_gid == peer_gid) && >> + if ((!peer_gid->gid || >> + (lgr->peer_gid.gid == peer_gid->gid && >> + !smc_ism_is_virtdev(dev) ? 1 : >> + lgr->peer_gid.gid_ext == peer_gid->gid_ext)) && >> (vlan == VLAN_VID_MASK || lgr->vlan_id == vlan)) { >> if (peer_gid) /* peer triggered termination */ >> lgr->peer_shutdown = 1; >> @@ -1859,10 +1877,12 @@ static bool smcr_lgr_match(struct smc_link_group *lgr, u8 smcr_version, >> return false; >> } >> -static bool smcd_lgr_match(struct smc_link_group *lgr, >> - struct smcd_dev *smcismdev, u64 peer_gid) >> +static bool smcd_lgr_match(struct smc_link_group *lgr, struct smcd_dev *smcismdev, >> + struct smcd_gid *peer_gid) >> { >> - return lgr->peer_gid == peer_gid && lgr->smcd == smcismdev; >> + return lgr->peer_gid.gid == peer_gid->gid && lgr->smcd == smcismdev && >> + smc_ism_is_virtdev(smcismdev) ? >> + (lgr->peer_gid.gid_ext == peer_gid->gid_ext) : 1; >> } >> /* create a new SMC connection (and a new link group if necessary) */ >> @@ -1892,7 +1912,7 @@ int smc_conn_create(struct smc_sock *smc, struct smc_init_info *ini) >> write_lock_bh(&lgr->conns_lock); >> if ((ini->is_smcd ? >> smcd_lgr_match(lgr, ini->ism_dev[ini->ism_selected], >> - ini->ism_peer_gid[ini->ism_selected]) : >> + &ini->ism_peer_gid[ini->ism_selected]) : >> smcr_lgr_match(lgr, ini->smcr_version, >> ini->peer_systemid, >> ini->peer_gid, ini->peer_mac, role, >> diff --git a/net/smc/smc_core.h b/net/smc/smc_core.h >> index 9f65678..d57eb9b 100644 >> --- a/net/smc/smc_core.h >> +++ b/net/smc/smc_core.h >> @@ -17,6 +17,7 @@ >> #include <linux/pci.h> >> #include <rdma/ib_verbs.h> >> #include <net/genetlink.h> >> +#include <net/smc.h> >> #include "smc.h" >> #include "smc_ib.h" >> @@ -355,7 +356,7 @@ struct smc_link_group { >> /* max links can be added in lgr */ >> }; >> struct { /* SMC-D */ >> - u64 peer_gid; >> + struct smcd_gid peer_gid; >> /* Peer GID (remote) */ >> struct smcd_dev *smcd; >> /* ISM device for VLAN reg. */ >> @@ -417,7 +418,7 @@ struct smc_init_info { >> u32 ib_clcqpn; >> struct smc_init_info_smcrv2 smcrv2; >> /* SMC-D */ >> - u64 ism_peer_gid[SMC_MAX_ISM_DEVS + 1]; >> + struct smcd_gid ism_peer_gid[SMC_MAX_ISM_DEVS + 1]; >> struct smcd_dev *ism_dev[SMC_MAX_ISM_DEVS + 1]; >> u16 ism_chid[SMC_MAX_ISM_DEVS + 1]; >> u8 ism_offered_cnt; /* # of ISM devices offered */ >> @@ -545,7 +546,7 @@ static inline void smc_set_pci_values(struct pci_dev *pci_dev, >> void smc_lgr_put(struct smc_link_group *lgr); >> void smcr_port_add(struct smc_ib_device *smcibdev, u8 ibport); >> void smcr_port_err(struct smc_ib_device *smcibdev, u8 ibport); >> -void smc_smcd_terminate(struct smcd_dev *dev, u64 peer_gid, >> +void smc_smcd_terminate(struct smcd_dev *dev, struct smcd_gid *peer_gid, >> unsigned short vlan); >> void smc_smcd_terminate_all(struct smcd_dev *dev); >> void smc_smcr_terminate_all(struct smc_ib_device *smcibdev); >> diff --git a/net/smc/smc_diag.c b/net/smc/smc_diag.c >> index 7ff2152..9c465cc 100644 >> --- a/net/smc/smc_diag.c >> +++ b/net/smc/smc_diag.c >> @@ -168,12 +168,14 @@ static int __smc_diag_dump(struct sock *sk, struct sk_buff *skb, >> struct smc_connection *conn = &smc->conn; >> struct smcd_diag_dmbinfo dinfo; >> struct smcd_dev *smcd = conn->lgr->smcd; >> + struct smcd_gid smcd_gid; >> memset(&dinfo, 0, sizeof(dinfo)); >> dinfo.linkid = *((u32 *)conn->lgr->id); >> - dinfo.peer_gid = conn->lgr->peer_gid; >> - dinfo.my_gid = smcd->ops->get_local_gid(smcd); >> + dinfo.peer_gid = conn->lgr->peer_gid.gid; >> + smcd->ops->get_local_gid(smcd, &smcd_gid); >> + dinfo.my_gid = smcd_gid.gid; >> dinfo.token = conn->rmb_desc->token; >> dinfo.peer_token = conn->peer_token; >> diff --git a/net/smc/smc_ism.c b/net/smc/smc_ism.c >> index 8f1ba74..35d31e7 100644 >> --- a/net/smc/smc_ism.c >> +++ b/net/smc/smc_ism.c >> @@ -44,7 +44,8 @@ static void smcd_handle_irq(struct ism_dev *ism, unsigned int dmbno, >> #endif >> /* Test if an ISM communication is possible - same CPC */ >> -int smc_ism_cantalk(u64 peer_gid, unsigned short vlan_id, struct smcd_dev *smcd) >> +int smc_ism_cantalk(struct smcd_gid *peer_gid, unsigned short vlan_id, >> + struct smcd_dev *smcd) >> { >> return smcd->ops->query_remote_gid(smcd, peer_gid, vlan_id ? 1 : 0, >> vlan_id); >> @@ -223,7 +224,7 @@ int smc_ism_register_dmb(struct smc_link_group *lgr, int dmb_len, >> dmb.dmb_len = dmb_len; >> dmb.sba_idx = dmb_desc->sba_idx; >> dmb.vlan_id = lgr->vlan_id; >> - dmb.rgid = lgr->peer_gid; >> + dmb.rgid = lgr->peer_gid.gid; >> rc = lgr->smcd->ops->register_dmb(lgr->smcd, &dmb, lgr->smcd->client); >> if (!rc) { >> dmb_desc->sba_idx = dmb.sba_idx; >> @@ -354,17 +355,20 @@ struct smc_ism_event_work { >> static void smcd_handle_sw_event(struct smc_ism_event_work *wrk) >> { >> union smcd_sw_event_info ev_info; >> + struct smcd_gid peer_gid; >> ev_info.info = wrk->event.info; >> switch (wrk->event.code) { >> case ISM_EVENT_CODE_SHUTDOWN: /* Peer shut down DMBs */ >> - smc_smcd_terminate(wrk->smcd, wrk->event.tok, ev_info.vlan_id); >> + peer_gid.gid = wrk->event.tok; >> + smc_smcd_terminate(wrk->smcd, &peer_gid, ev_info.vlan_id); >> break; >> case ISM_EVENT_CODE_TESTLINK: /* Activity timer */ >> + peer_gid.gid = wrk->event.tok; >> if (ev_info.code == ISM_EVENT_REQUEST) { >> ev_info.code = ISM_EVENT_RESPONSE; >> wrk->smcd->ops->signal_event(wrk->smcd, >> - wrk->event.tok, >> + &peer_gid, >> ISM_EVENT_REQUEST_IR, >> ISM_EVENT_CODE_TESTLINK, >> ev_info.info); >> @@ -378,10 +382,12 @@ static void smc_ism_event_work(struct work_struct *work) >> { >> struct smc_ism_event_work *wrk = >> container_of(work, struct smc_ism_event_work, work); >> + struct smcd_gid smcd_gid; >> switch (wrk->event.type) { >> case ISM_EVENT_GID: /* GID event, token is peer GID */ >> - smc_smcd_terminate(wrk->smcd, wrk->event.tok, VLAN_VID_MASK); >> + smcd_gid.gid = wrk->event.tok; >> + smc_smcd_terminate(wrk->smcd, &smcd_gid, VLAN_VID_MASK); >> break; >> case ISM_EVENT_DMB: >> break; >> @@ -530,7 +536,7 @@ int smc_ism_signal_shutdown(struct smc_link_group *lgr) >> memcpy(ev_info.uid, lgr->id, SMC_LGR_ID_SIZE); >> ev_info.vlan_id = lgr->vlan_id; >> ev_info.code = ISM_EVENT_REQUEST; >> - rc = lgr->smcd->ops->signal_event(lgr->smcd, lgr->peer_gid, >> + rc = lgr->smcd->ops->signal_event(lgr->smcd, &lgr->peer_gid, >> ISM_EVENT_REQUEST_IR, >> ISM_EVENT_CODE_SHUTDOWN, >> ev_info.info); >> diff --git a/net/smc/smc_ism.h b/net/smc/smc_ism.h >> index 2ecc8de..e6ea08c 100644 >> --- a/net/smc/smc_ism.h >> +++ b/net/smc/smc_ism.h >> @@ -33,7 +33,8 @@ struct smc_ism_vlanid { /* VLAN id set on ISM device */ >> struct smcd_dev; >> -int smc_ism_cantalk(u64 peer_gid, unsigned short vlan_id, struct smcd_dev *dev); >> +int smc_ism_cantalk(struct smcd_gid *peer_gid, unsigned short vlan_id, >> + struct smcd_dev *dev); >> void smc_ism_set_conn(struct smc_connection *conn); >> void smc_ism_unset_conn(struct smc_connection *conn); >> int smc_ism_get_vlan(struct smcd_dev *dev, unsigned short vlan_id); >> diff --git a/net/smc/smc_pnet.c b/net/smc/smc_pnet.c >> index 1177540..9f2c58c 100644 >> --- a/net/smc/smc_pnet.c >> +++ b/net/smc/smc_pnet.c >> @@ -1103,8 +1103,8 @@ static void smc_pnet_find_ism_by_pnetid(struct net_device *ndev, >> list_for_each_entry(ismdev, &smcd_dev_list.list, list) { >> if (smc_pnet_match(ismdev->pnetid, ndev_pnetid) && >> !ismdev->going_away && >> - (!ini->ism_peer_gid[0] || >> - !smc_ism_cantalk(ini->ism_peer_gid[0], ini->vlan_id, >> + (!ini->ism_peer_gid[0].gid || >> + !smc_ism_cantalk(&ini->ism_peer_gid[0], ini->vlan_id, >> ismdev))) { >> ini->ism_dev[0] = ismdev; >> break; ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <1695302360-46691-6-git-send-email-guwen@linux.alibaba.com>]
* Re: [PATCH net-next v3 05/18] net/smc: reserve CHID range for SMC-D virtual device [not found] ` <1695302360-46691-6-git-send-email-guwen@linux.alibaba.com> @ 2023-09-21 23:32 ` Wenjia Zhang 2023-09-22 8:42 ` Wen Gu 0 siblings, 1 reply; 10+ messages in thread From: Wenjia Zhang @ 2023-09-21 23:32 UTC (permalink / raw) To: Wen Gu, kgraul, jaka, davem, edumazet, kuba, pabeni Cc: alibuda, tonylu, linux-s390, netdev, linux-kernel On 21.09.23 15:19, Wen Gu wrote: > This patch reserve CHID range from 0xFF00 to 0xFFFF for SMC-D virtual > device and introduces helpers to identify them. > > Signed-off-by: Wen Gu <guwen@linux.alibaba.com> > --- > net/smc/smc_ism.h | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/net/smc/smc_ism.h b/net/smc/smc_ism.h > index 14d2e77..2ecc8de 100644 > --- a/net/smc/smc_ism.h > +++ b/net/smc/smc_ism.h > @@ -15,6 +15,9 @@ > > #include "smc.h" > > +#define SMC_VIRT_ISM_CHID_MAX 0xFFFF > +#define SMC_VIRT_ISM_CHID_MIN 0xFF00 > + > struct smcd_dev_list { /* List of SMCD devices */ > struct list_head list; > struct mutex mutex; /* Protects list of devices */ > @@ -57,4 +60,16 @@ static inline int smc_ism_write(struct smcd_dev *smcd, u64 dmb_tok, > return rc < 0 ? rc : 0; > } > > +static inline bool __smc_ism_is_virtdev(u16 chid) > +{ > + return (chid >= SMC_VIRT_ISM_CHID_MIN && chid <= SMC_VIRT_ISM_CHID_MAX); > +} > + > +static inline bool smc_ism_is_virtdev(struct smcd_dev *smcd) > +{ > + u16 chid = smcd->ops->get_chid(smcd); > + > + return __smc_ism_is_virtdev(chid); > +} > + I'm wondering if barrier is needed here. > #endif ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next v3 05/18] net/smc: reserve CHID range for SMC-D virtual device 2023-09-21 23:32 ` [PATCH net-next v3 05/18] net/smc: reserve CHID range for SMC-D virtual device Wenjia Zhang @ 2023-09-22 8:42 ` Wen Gu 0 siblings, 0 replies; 10+ messages in thread From: Wen Gu @ 2023-09-22 8:42 UTC (permalink / raw) To: Wenjia Zhang, kgraul, jaka, davem, edumazet, kuba, pabeni Cc: alibuda, tonylu, linux-s390, netdev, linux-kernel On 2023/9/22 07:32, Wenjia Zhang wrote: > > > On 21.09.23 15:19, Wen Gu wrote: >> This patch reserve CHID range from 0xFF00 to 0xFFFF for SMC-D virtual >> device and introduces helpers to identify them. >> >> Signed-off-by: Wen Gu <guwen@linux.alibaba.com> >> --- >> net/smc/smc_ism.h | 15 +++++++++++++++ >> 1 file changed, 15 insertions(+) >> >> diff --git a/net/smc/smc_ism.h b/net/smc/smc_ism.h >> index 14d2e77..2ecc8de 100644 >> --- a/net/smc/smc_ism.h >> +++ b/net/smc/smc_ism.h >> @@ -15,6 +15,9 @@ >> #include "smc.h" >> +#define SMC_VIRT_ISM_CHID_MAX 0xFFFF >> +#define SMC_VIRT_ISM_CHID_MIN 0xFF00 >> + >> struct smcd_dev_list { /* List of SMCD devices */ >> struct list_head list; >> struct mutex mutex; /* Protects list of devices */ >> @@ -57,4 +60,16 @@ static inline int smc_ism_write(struct smcd_dev *smcd, u64 dmb_tok, >> return rc < 0 ? rc : 0; >> } >> +static inline bool __smc_ism_is_virtdev(u16 chid) >> +{ >> + return (chid >= SMC_VIRT_ISM_CHID_MIN && chid <= SMC_VIRT_ISM_CHID_MAX); >> +} >> + >> +static inline bool smc_ism_is_virtdev(struct smcd_dev *smcd) >> +{ >> + u16 chid = smcd->ops->get_chid(smcd); >> + >> + return __smc_ism_is_virtdev(chid); >> +} >> + > I'm wondering if barrier is needed here. I think this helper doesn't involve memory race or multi-threaded/multi-processor cases that needs enforcing ordering and synchronization of memory operations. So IMHO barrier is no very necessary here. Thank you. >> #endif ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-09-22 12:18 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1695302360-46691-1-git-send-email-guwen@linux.alibaba.com>
2023-09-21 23:31 ` [PATCH net-next v3 00/18] net/smc: implement virtual ISM extension and loopback-ism Wenjia Zhang
2023-09-22 12:18 ` Wen Gu
[not found] ` <1695302360-46691-13-git-send-email-guwen@linux.alibaba.com>
2023-09-21 23:31 ` [PATCH net-next v3 12/18] net/smc: implement DMB-related operations of loopback Wenjia Zhang
2023-09-22 7:42 ` Wen Gu
[not found] ` <1695302360-46691-10-git-send-email-guwen@linux.alibaba.com>
2023-09-21 23:32 ` [PATCH net-next v3 09/18] net/smc: introduce SMC-D loopback device Wenjia Zhang
2023-09-22 7:55 ` Wen Gu
[not found] ` <1695302360-46691-7-git-send-email-guwen@linux.alibaba.com>
2023-09-21 23:32 ` [PATCH net-next v3 06/18] net/smc: extend GID to 128bits for virtual ISM device Wenjia Zhang
2023-09-22 12:12 ` Wen Gu
[not found] ` <1695302360-46691-6-git-send-email-guwen@linux.alibaba.com>
2023-09-21 23:32 ` [PATCH net-next v3 05/18] net/smc: reserve CHID range for SMC-D virtual device Wenjia Zhang
2023-09-22 8:42 ` Wen Gu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox