* [PATCH net-next 0/2] net/smc: improve peer ID in CLC decline
@ 2020-02-24 23:58 Hans Wippel
2020-02-24 23:59 ` [PATCH net-next 1/2] net/smc: rework peer ID handling Hans Wippel
2020-02-24 23:59 ` [PATCH net-next 2/2] net/smc: improve peer ID in CLC decline for SMC-R Hans Wippel
0 siblings, 2 replies; 5+ messages in thread
From: Hans Wippel @ 2020-02-24 23:58 UTC (permalink / raw)
To: kgraul, ubraun, davem; +Cc: netdev, Hans Wippel
The following two patches improve the peer ID in CLC decline messages if
RoCE devices are present in the host but no suitable device is found for
a connection. The first patch reworks the peer ID initialization. The
second patch contains the actual changes of the CLC decline messages.
Changes since the RFC version:
* split the patch into two parts
* removed zero assignment to global variable (thanks Leon)
Thanks to Leon Romanovsky and Karsten Graul for the feedback!
Hans Wippel (2):
net/smc: rework peer ID handling
net/smc: improve peer ID in CLC decline for SMC-R
net/smc/smc_clc.c | 9 ++++++---
net/smc/smc_ib.c | 19 ++++++++++++-------
net/smc/smc_ib.h | 1 +
3 files changed, 19 insertions(+), 10 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH net-next 1/2] net/smc: rework peer ID handling
2020-02-24 23:58 [PATCH net-next 0/2] net/smc: improve peer ID in CLC decline Hans Wippel
@ 2020-02-24 23:59 ` Hans Wippel
2020-02-25 15:17 ` Karsten Graul
2020-02-24 23:59 ` [PATCH net-next 2/2] net/smc: improve peer ID in CLC decline for SMC-R Hans Wippel
1 sibling, 1 reply; 5+ messages in thread
From: Hans Wippel @ 2020-02-24 23:59 UTC (permalink / raw)
To: kgraul, ubraun, davem; +Cc: netdev, Hans Wippel
This patch initializes the peer ID to a random instance ID and a zero
MAC address. If a RoCE device is in the host, the MAC address part of
the peer ID is overwritten with the respective address. Also, a function
for checking if the peer ID is valid is added. A peer ID is considered
valid if the MAC address part contains a non-zero MAC address.
Signed-off-by: Hans Wippel <ndev@hwipl.net>
---
net/smc/smc_ib.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/net/smc/smc_ib.c b/net/smc/smc_ib.c
index 6756bd5a3fe4..3444de27fecd 100644
--- a/net/smc/smc_ib.c
+++ b/net/smc/smc_ib.c
@@ -37,11 +37,7 @@ struct smc_ib_devices smc_ib_devices = { /* smc-registered ib devices */
.list = LIST_HEAD_INIT(smc_ib_devices.list),
};
-#define SMC_LOCAL_SYSTEMID_RESET "%%%%%%%"
-
-u8 local_systemid[SMC_SYSTEMID_LEN] = SMC_LOCAL_SYSTEMID_RESET; /* unique system
- * identifier
- */
+u8 local_systemid[SMC_SYSTEMID_LEN]; /* unique system identifier */
static int smc_ib_modify_qp_init(struct smc_link *lnk)
{
@@ -168,6 +164,15 @@ static inline void smc_ib_define_local_systemid(struct smc_ib_device *smcibdev,
{
memcpy(&local_systemid[2], &smcibdev->mac[ibport - 1],
sizeof(smcibdev->mac[ibport - 1]));
+}
+
+bool smc_ib_is_valid_local_systemid(void)
+{
+ return !is_zero_ether_addr(&local_systemid[2]);
+}
+
+static void smc_ib_init_local_systemid(void)
+{
get_random_bytes(&local_systemid[0], 2);
}
@@ -224,8 +229,7 @@ static int smc_ib_remember_port_attr(struct smc_ib_device *smcibdev, u8 ibport)
rc = smc_ib_fill_mac(smcibdev, ibport);
if (rc)
goto out;
- if (!strncmp(local_systemid, SMC_LOCAL_SYSTEMID_RESET,
- sizeof(local_systemid)) &&
+ if (!smc_ib_is_valid_local_systemid() &&
smc_ib_port_active(smcibdev, ibport))
/* create unique system identifier */
smc_ib_define_local_systemid(smcibdev, ibport);
@@ -605,6 +609,7 @@ static struct ib_client smc_ib_client = {
int __init smc_ib_register_client(void)
{
+ smc_ib_init_local_systemid();
return ib_register_client(&smc_ib_client);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH net-next 2/2] net/smc: improve peer ID in CLC decline for SMC-R
2020-02-24 23:58 [PATCH net-next 0/2] net/smc: improve peer ID in CLC decline Hans Wippel
2020-02-24 23:59 ` [PATCH net-next 1/2] net/smc: rework peer ID handling Hans Wippel
@ 2020-02-24 23:59 ` Hans Wippel
2020-02-25 15:17 ` Karsten Graul
1 sibling, 1 reply; 5+ messages in thread
From: Hans Wippel @ 2020-02-24 23:59 UTC (permalink / raw)
To: kgraul, ubraun, davem; +Cc: netdev, Hans Wippel
According to RFC 7609, all CLC messages contain a peer ID that consists
of a unique instance ID and the MAC address of one of the host's RoCE
devices. But if a SMC-R connection cannot be established, e.g., because
no matching pnet table entry is found, the current implementation uses a
zero value in the CLC decline message although the host's peer ID is set
to a proper value.
If no RoCE and no ISM device is usable for a connection, there is no LGR
and the LGR check in smc_clc_send_decline() prevents that the peer ID is
copied into the CLC decline message for both SMC-D and SMC-R. So, this
patch modifies the check to also accept the case of no LGR. Also, only a
valid peer ID is copied into the decline message.
Signed-off-by: Hans Wippel <ndev@hwipl.net>
---
net/smc/smc_clc.c | 9 ++++++---
net/smc/smc_ib.h | 1 +
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
index 3e16b887cfcf..e2d3b5b95632 100644
--- a/net/smc/smc_clc.c
+++ b/net/smc/smc_clc.c
@@ -372,9 +372,12 @@ int smc_clc_send_decline(struct smc_sock *smc, u32 peer_diag_info)
dclc.hdr.length = htons(sizeof(struct smc_clc_msg_decline));
dclc.hdr.version = SMC_CLC_V1;
dclc.hdr.flag = (peer_diag_info == SMC_CLC_DECL_SYNCERR) ? 1 : 0;
- if (smc->conn.lgr && !smc->conn.lgr->is_smcd)
- memcpy(dclc.id_for_peer, local_systemid,
- sizeof(local_systemid));
+ if (!smc->conn.lgr || !smc->conn.lgr->is_smcd) {
+ if (smc_ib_is_valid_local_systemid()) {
+ memcpy(dclc.id_for_peer, local_systemid,
+ sizeof(local_systemid));
+ }
+ }
dclc.peer_diagnosis = htonl(peer_diag_info);
memcpy(dclc.trl.eyecatcher, SMC_EYECATCHER, sizeof(SMC_EYECATCHER));
diff --git a/net/smc/smc_ib.h b/net/smc/smc_ib.h
index 255db87547d3..5c2b115d36da 100644
--- a/net/smc/smc_ib.h
+++ b/net/smc/smc_ib.h
@@ -84,4 +84,5 @@ void smc_ib_sync_sg_for_device(struct smc_ib_device *smcibdev,
enum dma_data_direction data_direction);
int smc_ib_determine_gid(struct smc_ib_device *smcibdev, u8 ibport,
unsigned short vlan_id, u8 gid[], u8 *sgid_index);
+bool smc_ib_is_valid_local_systemid(void);
#endif
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net-next 1/2] net/smc: rework peer ID handling
2020-02-24 23:59 ` [PATCH net-next 1/2] net/smc: rework peer ID handling Hans Wippel
@ 2020-02-25 15:17 ` Karsten Graul
0 siblings, 0 replies; 5+ messages in thread
From: Karsten Graul @ 2020-02-25 15:17 UTC (permalink / raw)
To: Hans Wippel, ubraun, davem; +Cc: netdev
On 25/02/2020 00:59, Hans Wippel wrote:
> This patch initializes the peer ID to a random instance ID and a zero
> MAC address. If a RoCE device is in the host, the MAC address part of
> the peer ID is overwritten with the respective address. Also, a function
> for checking if the peer ID is valid is added. A peer ID is considered
> valid if the MAC address part contains a non-zero MAC address.
>
> Signed-off-by: Hans Wippel <ndev@hwipl.net>
> ---
> net/smc/smc_ib.c | 19 ++++++++++++-------
> 1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/net/smc/smc_ib.c b/net/smc/smc_ib.c
> index 6756bd5a3fe4..3444de27fecd 100644
> --- a/net/smc/smc_ib.c
> +++ b/net/smc/smc_ib.c
> @@ -37,11 +37,7 @@ struct smc_ib_devices smc_ib_devices = { /* smc-registered ib devices */
> .list = LIST_HEAD_INIT(smc_ib_devices.list),
> };
>
> -#define SMC_LOCAL_SYSTEMID_RESET "%%%%%%%"
> -
> -u8 local_systemid[SMC_SYSTEMID_LEN] = SMC_LOCAL_SYSTEMID_RESET; /* unique system
> - * identifier
> - */
> +u8 local_systemid[SMC_SYSTEMID_LEN]; /* unique system identifier */
>
> static int smc_ib_modify_qp_init(struct smc_link *lnk)
> {
> @@ -168,6 +164,15 @@ static inline void smc_ib_define_local_systemid(struct smc_ib_device *smcibdev,
> {
> memcpy(&local_systemid[2], &smcibdev->mac[ibport - 1],
> sizeof(smcibdev->mac[ibport - 1]));
> +}
> +
> +bool smc_ib_is_valid_local_systemid(void)
You introduce a non-static but its used only from this module.
> +{
> + return !is_zero_ether_addr(&local_systemid[2]);
> +}
> +
> +static void smc_ib_init_local_systemid(void)
> +{
> get_random_bytes(&local_systemid[0], 2);
> }
>
> @@ -224,8 +229,7 @@ static int smc_ib_remember_port_attr(struct smc_ib_device *smcibdev, u8 ibport)
> rc = smc_ib_fill_mac(smcibdev, ibport);
> if (rc)
> goto out;
> - if (!strncmp(local_systemid, SMC_LOCAL_SYSTEMID_RESET,
> - sizeof(local_systemid)) &&
> + if (!smc_ib_is_valid_local_systemid() &&
> smc_ib_port_active(smcibdev, ibport))
> /* create unique system identifier */
> smc_ib_define_local_systemid(smcibdev, ibport);
> @@ -605,6 +609,7 @@ static struct ib_client smc_ib_client = {
>
> int __init smc_ib_register_client(void)
> {
> + smc_ib_init_local_systemid();
> return ib_register_client(&smc_ib_client);
> }
>
>
--
Karsten
(I'm a dude)
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next 2/2] net/smc: improve peer ID in CLC decline for SMC-R
2020-02-24 23:59 ` [PATCH net-next 2/2] net/smc: improve peer ID in CLC decline for SMC-R Hans Wippel
@ 2020-02-25 15:17 ` Karsten Graul
0 siblings, 0 replies; 5+ messages in thread
From: Karsten Graul @ 2020-02-25 15:17 UTC (permalink / raw)
To: Hans Wippel, ubraun, davem; +Cc: netdev
On 25/02/2020 00:59, Hans Wippel wrote:
> According to RFC 7609, all CLC messages contain a peer ID that consists
> of a unique instance ID and the MAC address of one of the host's RoCE
> devices. But if a SMC-R connection cannot be established, e.g., because
> no matching pnet table entry is found, the current implementation uses a
> zero value in the CLC decline message although the host's peer ID is set
> to a proper value.
>
> If no RoCE and no ISM device is usable for a connection, there is no LGR
> and the LGR check in smc_clc_send_decline() prevents that the peer ID is
> copied into the CLC decline message for both SMC-D and SMC-R. So, this
> patch modifies the check to also accept the case of no LGR. Also, only a
> valid peer ID is copied into the decline message.
>
> Signed-off-by: Hans Wippel <ndev@hwipl.net>
> ---
> net/smc/smc_clc.c | 9 ++++++---
> net/smc/smc_ib.h | 1 +
> 2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
> index 3e16b887cfcf..e2d3b5b95632 100644
> --- a/net/smc/smc_clc.c
> +++ b/net/smc/smc_clc.c
> @@ -372,9 +372,12 @@ int smc_clc_send_decline(struct smc_sock *smc, u32 peer_diag_info)
> dclc.hdr.length = htons(sizeof(struct smc_clc_msg_decline));
> dclc.hdr.version = SMC_CLC_V1;
> dclc.hdr.flag = (peer_diag_info == SMC_CLC_DECL_SYNCERR) ? 1 : 0;
> - if (smc->conn.lgr && !smc->conn.lgr->is_smcd)
> - memcpy(dclc.id_for_peer, local_systemid,
> - sizeof(local_systemid));
> + if (!smc->conn.lgr || !smc->conn.lgr->is_smcd) {
> + if (smc_ib_is_valid_local_systemid()) {
> + memcpy(dclc.id_for_peer, local_systemid,
> + sizeof(local_systemid));
> + }
^
Adds curly braces for no reason.
> + }
> dclc.peer_diagnosis = htonl(peer_diag_info);
> memcpy(dclc.trl.eyecatcher, SMC_EYECATCHER, sizeof(SMC_EYECATCHER));
>
> diff --git a/net/smc/smc_ib.h b/net/smc/smc_ib.h
> index 255db87547d3..5c2b115d36da 100644
> --- a/net/smc/smc_ib.h
> +++ b/net/smc/smc_ib.h
> @@ -84,4 +84,5 @@ void smc_ib_sync_sg_for_device(struct smc_ib_device *smcibdev,
> enum dma_data_direction data_direction);
> int smc_ib_determine_gid(struct smc_ib_device *smcibdev, u8 ibport,
> unsigned short vlan_id, u8 gid[], u8 *sgid_index);
> +bool smc_ib_is_valid_local_systemid(void);
> #endif
>
--
Karsten
(I'm a dude)
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-02-25 15:17 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-24 23:58 [PATCH net-next 0/2] net/smc: improve peer ID in CLC decline Hans Wippel
2020-02-24 23:59 ` [PATCH net-next 1/2] net/smc: rework peer ID handling Hans Wippel
2020-02-25 15:17 ` Karsten Graul
2020-02-24 23:59 ` [PATCH net-next 2/2] net/smc: improve peer ID in CLC decline for SMC-R Hans Wippel
2020-02-25 15:17 ` Karsten Graul
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).