All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC net-next] net/smc: update peer ID on device changes
@ 2020-02-27 11:39 Hans Wippel
  2020-02-27 13:13 ` Karsten Graul
  0 siblings, 1 reply; 6+ messages in thread
From: Hans Wippel @ 2020-02-27 11:39 UTC (permalink / raw)
  To: kgraul, ubraun, davem; +Cc: netdev, hwipl

From: hwipl <ndev@hwipl.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.

Signed-off-by: hwipl <ndev@hwipl.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 = {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [RFC net-next] net/smc: update peer ID on device changes
  2020-02-27 11:39 [RFC net-next] net/smc: update peer ID on device changes Hans Wippel
@ 2020-02-27 13:13 ` Karsten Graul
  2020-02-27 14:09   ` Hans Wippel
  0 siblings, 1 reply; 6+ messages in thread
From: Karsten Graul @ 2020-02-27 13:13 UTC (permalink / raw)
  To: Hans Wippel, ubraun, davem; +Cc: netdev

On 27/02/2020 12:39, Hans Wippel wrote:
> From: hwipl <ndev@hwipl.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@hwipl.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)


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [RFC net-next] net/smc: update peer ID on device changes
  2020-02-27 13:13 ` Karsten Graul
@ 2020-02-27 14:09   ` Hans Wippel
  2020-02-27 14:44     ` Karsten Graul
  0 siblings, 1 reply; 6+ messages in thread
From: Hans Wippel @ 2020-02-27 14:09 UTC (permalink / raw)
  To: Karsten Graul; +Cc: ubraun, davem, netdev

On Thu, 27 Feb 2020 14:13:48 +0100
Karsten Graul <kgraul@linux.ibm.com> wrote:

> On 27/02/2020 12:39, Hans Wippel wrote:
> > From: hwipl <ndev@hwipl.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.

Is it OK to keep the peer ID if, for example, the device is removed and
used in a different VM?

Hans


> > 
> > Signed-off-by: hwipl <ndev@hwipl.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)
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [RFC net-next] net/smc: update peer ID on device changes
  2020-02-27 14:09   ` Hans Wippel
@ 2020-02-27 14:44     ` Karsten Graul
  2020-02-27 15:03       ` Hans Wippel
  0 siblings, 1 reply; 6+ messages in thread
From: Karsten Graul @ 2020-02-27 14:44 UTC (permalink / raw)
  To: Hans Wippel; +Cc: ubraun, davem, netdev

On 27/02/2020 15:09, Hans Wippel wrote:
> On Thu, 27 Feb 2020 14:13:48 +0100
> Karsten Graul <kgraul@linux.ibm.com> wrote:
> 
>> On 27/02/2020 12:39, Hans Wippel wrote:
>>> From: hwipl <ndev@hwipl.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.
> 
> Is it OK to keep the peer ID if, for example, the device is removed and
> used in a different VM?
> 
> Hans
> 

Yes, exactly this case is described in the RFC (instance id):

https://tools.ietf.org/html/rfc7609#page-93

-- 
Karsten

(I'm a dude)


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [RFC net-next] net/smc: update peer ID on device changes
  2020-02-27 14:44     ` Karsten Graul
@ 2020-02-27 15:03       ` Hans Wippel
  2020-02-27 16:08         ` Karsten Graul
  0 siblings, 1 reply; 6+ messages in thread
From: Hans Wippel @ 2020-02-27 15:03 UTC (permalink / raw)
  To: Karsten Graul; +Cc: ubraun, davem, netdev

On Thu, 27 Feb 2020 15:44:52 +0100
Karsten Graul <kgraul@linux.ibm.com> wrote:

> On 27/02/2020 15:09, Hans Wippel wrote:
> > On Thu, 27 Feb 2020 14:13:48 +0100
> > Karsten Graul <kgraul@linux.ibm.com> wrote:
> > 
> >> On 27/02/2020 12:39, Hans Wippel wrote:
> >>> From: hwipl <ndev@hwipl.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.
> > 
> > Is it OK to keep the peer ID if, for example, the device is removed and
> > used in a different VM?
> > 
> > Hans
> > 
> 
> Yes, exactly this case is described in the RFC (instance id):
> 
> https://tools.ietf.org/html/rfc7609#page-93

OK, thanks for clarifying. I guess, you can ignore the RFC/patch then ;)
  Hans

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [RFC net-next] net/smc: update peer ID on device changes
  2020-02-27 15:03       ` Hans Wippel
@ 2020-02-27 16:08         ` Karsten Graul
  0 siblings, 0 replies; 6+ messages in thread
From: Karsten Graul @ 2020-02-27 16:08 UTC (permalink / raw)
  To: Hans Wippel; +Cc: ubraun, davem, netdev

On 27/02/2020 16:03, Hans Wippel wrote:
> On Thu, 27 Feb 2020 15:44:52 +0100
> Karsten Graul <kgraul@linux.ibm.com> wrote:
> 
>> On 27/02/2020 15:09, Hans Wippel wrote:
>>> On Thu, 27 Feb 2020 14:13:48 +0100
>>> Karsten Graul <kgraul@linux.ibm.com> wrote:
>>>
>>>> On 27/02/2020 12:39, Hans Wippel wrote:
>>>>> From: hwipl <ndev@hwipl.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.
>>>
>>> Is it OK to keep the peer ID if, for example, the device is removed and
>>> used in a different VM?
>>>
>>> Hans
>>>
>>
>> Yes, exactly this case is described in the RFC (instance id):
>>
>> https://tools.ietf.org/html/rfc7609#page-93
> 
> OK, thanks for clarifying. I guess, you can ignore the RFC/patch then ;)
>   Hans
> 

Thanks for taking care Hans!

-- 
Karsten

(I'm a dude)


^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-02-27 16:09 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-27 11:39 [RFC net-next] net/smc: update peer ID on device changes Hans Wippel
2020-02-27 13:13 ` Karsten Graul
2020-02-27 14:09   ` Hans Wippel
2020-02-27 14:44     ` Karsten Graul
2020-02-27 15:03       ` Hans Wippel
2020-02-27 16:08         ` Karsten Graul

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.