netdev - Re: [RFC net-next] net/smc: update peer ID on device changes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <b56d4bbc-2a4e-634f-10d4-17bd0253c033@linux.ibm.com>
Date:   Thu, 27 Feb 2020 14:13:48 +0100
From:   Karsten Graul <kgraul@...ux.ibm.com>
To:     Hans Wippel <ndev@...pl.net>, ubraun@...ux.ibm.com,
        davem@...emloft.net
Cc:     netdev@...r.kernel.org
Subject: Re: [RFC net-next] net/smc: update peer ID on device changes

On 27/02/2020 12:39, Hans Wippel wrote:
> From: hwipl <ndev@...pl.net>
> 
> A SMC host's peer ID contains the MAC address of the first active RoCE
> device. However, if this device becomes inactive or is removed, the peer
> ID is not updated. This patch adds peer ID updates on device changes.

The peer ID is used to uniquely identify an SMC host and to check if there
are already established link groups to the peer which can be reused.
In failover scenarios RoCE devices can go down and get active again later,
but this must not change the current peer ID of the host.  
The part of the MAC address that is included in the peer ID is not used for
other purposes than the identification of an SMC host.

> 
> Signed-off-by: hwipl <ndev@...pl.net>
> ---
>  net/smc/smc_ib.c | 32 ++++++++++++++++++++++++--------
>  1 file changed, 24 insertions(+), 8 deletions(-)
> 
> diff --git a/net/smc/smc_ib.c b/net/smc/smc_ib.c
> index 3444de27fecd..5818636962c6 100644
> --- a/net/smc/smc_ib.c
> +++ b/net/smc/smc_ib.c
> @@ -159,11 +159,29 @@ static int smc_ib_fill_mac(struct smc_ib_device *smcibdev, u8 ibport)
>   * plus a random 2-byte number is used to create this identifier.
>   * This name is delivered to the peer during connection initialization.
>   */
> -static inline void smc_ib_define_local_systemid(struct smc_ib_device *smcibdev,
> -						u8 ibport)
> +static void smc_ib_update_local_systemid(void)
>  {
> -	memcpy(&local_systemid[2], &smcibdev->mac[ibport - 1],
> -	       sizeof(smcibdev->mac[ibport - 1]));
> +	struct smc_ib_device *smcibdev;
> +	u8 ibport;
> +
> +	/* get first ib device with an active port */
> +	spin_lock(&smc_ib_devices.lock);
> +	list_for_each_entry(smcibdev, &smc_ib_devices.list, list) {
> +		for (ibport = 1; ibport <= SMC_MAX_PORTS; ibport++) {
> +			if (smc_ib_port_active(smcibdev, ibport))
> +				goto out;
> +		}
> +	}
> +	smcibdev = NULL;
> +out:
> +	spin_unlock(&smc_ib_devices.lock);
> +
> +	/* set (new) mac address or reset to zero */
> +	if (smcibdev)
> +		ether_addr_copy(&local_systemid[2],
> +				(u8 *)&smcibdev->mac[ibport - 1]);
> +	else
> +		eth_zero_addr(&local_systemid[2]);
>  }
>  
>  bool smc_ib_is_valid_local_systemid(void)
> @@ -229,10 +247,6 @@ 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 (!smc_ib_is_valid_local_systemid() &&
> -	    smc_ib_port_active(smcibdev, ibport))
> -		/* create unique system identifier */
> -		smc_ib_define_local_systemid(smcibdev, ibport);
>  out:
>  	return rc;
>  }
> @@ -254,6 +268,7 @@ static void smc_ib_port_event_work(struct work_struct *work)
>  			clear_bit(port_idx, smcibdev->ports_going_away);
>  		}
>  	}
> +	smc_ib_update_local_systemid();
>  }
>  
>  /* can be called in IRQ context */
> @@ -599,6 +614,7 @@ static void smc_ib_remove_dev(struct ib_device *ibdev, void *client_data)
>  	smc_ib_cleanup_per_ibdev(smcibdev);
>  	ib_unregister_event_handler(&smcibdev->event_handler);
>  	kfree(smcibdev);
> +	smc_ib_update_local_systemid();
>  }
>  
>  static struct ib_client smc_ib_client = {
> 

-- 
Karsten

(I'm a dude)