[PATCH net-next 01/10] net/smc: add missing dev_put

[PATCH net-next 01/10] net/smc: add missing dev_put

[Ursula Braun]

From: Hans Wippel <hwippel@linux.vnet.ibm.com>

In the infiniband part, SMC currently uses get_netdev which calls
dev_hold on the returned net device. However, the SMC code never calls
dev_put on that net device resulting in a wrong reference count.

This patch adds a dev_put after the usage of the net device to fix the
issue.

Signed-off-by: Hans Wippel <hwippel@linux.vnet.ibm.com>
Signed-off-by: Ursula Braun <ubraun@linux.vnet.ibm.com>
---
 net/smc/smc_ib.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/smc/smc_ib.c b/net/smc/smc_ib.c
index 547e0e113b17..0b5852299158 100644
--- a/net/smc/smc_ib.c
+++ b/net/smc/smc_ib.c
@@ -380,6 +380,7 @@ static int smc_ib_fill_gid_and_mac(struct smc_ib_device *smcibdev, u8 ibport)
 	ndev = smcibdev->ibdev->get_netdev(smcibdev->ibdev, ibport);
 	if (ndev) {
 		memcpy(&smcibdev->mac, ndev->dev_addr, ETH_ALEN);
+		dev_put(ndev);
 	} else if (!rc) {
 		memcpy(&smcibdev->mac[ibport - 1][0],
 		       &smcibdev->gid[ibport - 1].raw[8], 3);
-- 
2.13.5

RE: [PATCH net-next 01/10] net/smc: add missing dev_put

[Parav Pandit]

Hi Ursula, Dave, Hans,

> -----Original Message-----
> From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
> owner@vger.kernel.org] On Behalf Of Ursula Braun
> Sent: Wednesday, September 20, 2017 6:58 AM
> To: davem@davemloft.net
> Cc: netdev@vger.kernel.org; linux-rdma@vger.kernel.org; linux-
> s390@vger.kernel.org; jwi@linux.vnet.ibm.com; schwidefsky@de.ibm.com;
> heiko.carstens@de.ibm.com; raspl@linux.vnet.ibm.com;
> ubraun@linux.vnet.ibm.com
> Subject: [PATCH net-next 01/10] net/smc: add missing dev_put
> 
> From: Hans Wippel <hwippel@linux.vnet.ibm.com>
> 
> In the infiniband part, SMC currently uses get_netdev which calls dev_hold on
> the returned net device. However, the SMC code never calls dev_put on that net
> device resulting in a wrong reference count.
> 
> This patch adds a dev_put after the usage of the net device to fix the issue.
> 
> Signed-off-by: Hans Wippel <hwippel@linux.vnet.ibm.com>
> Signed-off-by: Ursula Braun <ubraun@linux.vnet.ibm.com>
> ---
>  net/smc/smc_ib.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/smc/smc_ib.c b/net/smc/smc_ib.c index
> 547e0e113b17..0b5852299158 100644
> --- a/net/smc/smc_ib.c
> +++ b/net/smc/smc_ib.c
> @@ -380,6 +380,7 @@ static int smc_ib_fill_gid_and_mac(struct smc_ib_device
> *smcibdev, u8 ibport)
>  	ndev = smcibdev->ibdev->get_netdev(smcibdev->ibdev, ibport);
>  	if (ndev) {
>  		memcpy(&smcibdev->mac, ndev->dev_addr, ETH_ALEN);
> +		dev_put(ndev);

I am sorry for providing late comments. smc_ib_fill_gid_and_mac() is not coded correctly.
Few fixes are needed.
1. ULP such as SMC should not open code/deference any function pointer like get_netdev() of the IB device.
2. Replace ib_query_gid(..., NULL)
With
ib_query_gid(..., gid_attr);

Use gid_attr.ndev to get the MAC address.
Do dev_put(gid_attr.ndev);

Code should look like below,

struct ib_gid_attr gid_attr;

rc = ib_query_gid(..., &gid_attr);
if (rc || !gid_addr.ndev)
	return -ENODEV;
else
	memcpy(smcibdev->mac, ndev->dev_addr, ETH_ALEN);
dev_put(gid_addr.ndev);

RE: [PATCH net-next 01/10] net/smc: add missing dev_put

[Parav Pandit]

> -----Original Message-----
> From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
> owner@vger.kernel.org] On Behalf Of Parav Pandit
> Sent: Monday, October 02, 2017 3:36 PM
> To: Ursula Braun <ubraun@linux.vnet.ibm.com>; davem@davemloft.net
> Cc: netdev@vger.kernel.org; linux-rdma@vger.kernel.org; linux-
> s390@vger.kernel.org; jwi@linux.vnet.ibm.com; schwidefsky@de.ibm.com;
> heiko.carstens@de.ibm.com; raspl@linux.vnet.ibm.com
> Subject: RE: [PATCH net-next 01/10] net/smc: add missing dev_put
> 
> Hi Ursula, Dave, Hans,
> 
> > -----Original Message-----
> > From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
> > owner@vger.kernel.org] On Behalf Of Ursula Braun
> > Sent: Wednesday, September 20, 2017 6:58 AM
> > To: davem@davemloft.net
> > Cc: netdev@vger.kernel.org; linux-rdma@vger.kernel.org; linux-
> > s390@vger.kernel.org; jwi@linux.vnet.ibm.com; schwidefsky@de.ibm.com;
> > heiko.carstens@de.ibm.com; raspl@linux.vnet.ibm.com;
> > ubraun@linux.vnet.ibm.com
> > Subject: [PATCH net-next 01/10] net/smc: add missing dev_put
> >
> > From: Hans Wippel <hwippel@linux.vnet.ibm.com>
> >
> > In the infiniband part, SMC currently uses get_netdev which calls
> > dev_hold on the returned net device. However, the SMC code never calls
> > dev_put on that net device resulting in a wrong reference count.
> >
> > This patch adds a dev_put after the usage of the net device to fix the issue.
> >
> > Signed-off-by: Hans Wippel <hwippel@linux.vnet.ibm.com>
> > Signed-off-by: Ursula Braun <ubraun@linux.vnet.ibm.com>
> > ---
> >  net/smc/smc_ib.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/net/smc/smc_ib.c b/net/smc/smc_ib.c index
> > 547e0e113b17..0b5852299158 100644
> > --- a/net/smc/smc_ib.c
> > +++ b/net/smc/smc_ib.c
> > @@ -380,6 +380,7 @@ static int smc_ib_fill_gid_and_mac(struct
> > smc_ib_device *smcibdev, u8 ibport)
> >  	ndev = smcibdev->ibdev->get_netdev(smcibdev->ibdev, ibport);
> >  	if (ndev) {
> >  		memcpy(&smcibdev->mac, ndev->dev_addr, ETH_ALEN);
> > +		dev_put(ndev);
> 
> I am sorry for providing late comments. smc_ib_fill_gid_and_mac() is not coded
> correctly.
> Few fixes are needed.
> 1. ULP such as SMC should not open code/deference any function pointer like
> get_netdev() of the IB device.
> 2. Replace ib_query_gid(..., NULL)
> With
> ib_query_gid(..., gid_attr);
> 
> Use gid_attr.ndev to get the MAC address.
> Do dev_put(gid_attr.ndev);
> 
> Code should look like below,
> 
> struct ib_gid_attr gid_attr;
> 
> rc = ib_query_gid(..., &gid_attr);
> if (rc || !gid_addr.ndev)
> 	return -ENODEV;
> else
> 	memcpy(smcibdev->mac, ndev->dev_addr, ETH_ALEN);
> dev_put(gid_addr.ndev);
> --

Also,
smc_link_determine_gid() doesn't do dev_put(gattr.ndev) in for loop.
Please fix it as well.

Re: [PATCH net-next 01/10] net/smc: add missing dev_put

[Ursula Braun]

On 10/02/2017 10:36 PM, Parav Pandit wrote:
> Hi Ursula, Dave, Hans,
> 
>> -----Original Message-----
>> From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
>> owner@vger.kernel.org] On Behalf Of Ursula Braun
>> Sent: Wednesday, September 20, 2017 6:58 AM
>> To: davem@davemloft.net
>> Cc: netdev@vger.kernel.org; linux-rdma@vger.kernel.org; linux-
>> s390@vger.kernel.org; jwi@linux.vnet.ibm.com; schwidefsky@de.ibm.com;
>> heiko.carstens@de.ibm.com; raspl@linux.vnet.ibm.com;
>> ubraun@linux.vnet.ibm.com
>> Subject: [PATCH net-next 01/10] net/smc: add missing dev_put
>>
>> From: Hans Wippel <hwippel@linux.vnet.ibm.com>
>>
>> In the infiniband part, SMC currently uses get_netdev which calls dev_hold on
>> the returned net device. However, the SMC code never calls dev_put on that net
>> device resulting in a wrong reference count.
>>
>> This patch adds a dev_put after the usage of the net device to fix the issue.
>>
>> Signed-off-by: Hans Wippel <hwippel@linux.vnet.ibm.com>
>> Signed-off-by: Ursula Braun <ubraun@linux.vnet.ibm.com>
>> ---
>>  net/smc/smc_ib.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/net/smc/smc_ib.c b/net/smc/smc_ib.c index
>> 547e0e113b17..0b5852299158 100644
>> --- a/net/smc/smc_ib.c
>> +++ b/net/smc/smc_ib.c
>> @@ -380,6 +380,7 @@ static int smc_ib_fill_gid_and_mac(struct smc_ib_device
>> *smcibdev, u8 ibport)
>>  	ndev = smcibdev->ibdev->get_netdev(smcibdev->ibdev, ibport);
>>  	if (ndev) {
>>  		memcpy(&smcibdev->mac, ndev->dev_addr, ETH_ALEN);
>> +		dev_put(ndev);
> 
> I am sorry for providing late comments. smc_ib_fill_gid_and_mac() is not coded correctly.
> Few fixes are needed.
> 1. ULP such as SMC should not open code/deference any function pointer like get_netdev() of the IB device.
> 2. Replace ib_query_gid(..., NULL)
> With
> ib_query_gid(..., gid_attr);
> 
> Use gid_attr.ndev to get the MAC address.
> Do dev_put(gid_attr.ndev);
> 
> Code should look like below,
> 
> struct ib_gid_attr gid_attr;
> 
> rc = ib_query_gid(..., &gid_attr);
> if (rc || !gid_addr.ndev)
> 	return -ENODEV;
> else
> 	memcpy(smcibdev->mac, ndev->dev_addr, ETH_ALEN);
> dev_put(gid_addr.ndev);
> 

Thanks, Parav!
Following your fix ideas I plan to change the function into this one:

static int smc_ib_fill_gid_and_mac(struct smc_ib_device *smcibdev, u8 ibport)
{
        struct ib_gid_attr gattr;
        int rc;

        rc = ib_query_gid(smcibdev->ibdev, ibport, 0,
                          &smcibdev->gid[ibport - 1], &gattr);
        /* the SMC protocol requires specification of the roce MAC address;
         * if net_device cannot be determined, it can be derived from gid 0
         */
        if (rc)
                return rc;

        if (gattr.ndev) {
                memcpy(&smcibdev->mac, gattr.ndev->dev_addr, ETH_ALEN);
                dev_put(gattr.ndev);
        } else {
                memcpy(&smcibdev->mac[ibport - 1][0],
                       &smcibdev->gid[ibport - 1].raw[8], 3);
                memcpy(&smcibdev->mac[ibport - 1][3],
                       &smcibdev->gid[ibport - 1].raw[13], 3);
                smcibdev->mac[ibport - 1][0] &= ~0x02;
        }
        return 0;
}

If you agree, I will submit the corresponding patch including a
	Suggested-by: Parav Pandit <parav@mellanox.com>

Regards, Ursula

RE: [PATCH net-next 01/10] net/smc: add missing dev_put

[Parav Pandit]

Hi Ursula,

> -----Original Message-----
> From: Ursula Braun [mailto:ubraun@linux.vnet.ibm.com]
> Sent: Thursday, October 05, 2017 2:54 AM
> To: Parav Pandit <parav@mellanox.com>
> Cc: davem@davemloft.net; netdev@vger.kernel.org; linux-
> rdma@vger.kernel.org; linux-s390@vger.kernel.org; jwi@linux.vnet.ibm.com;
> schwidefsky@de.ibm.com; heiko.carstens@de.ibm.com;
> raspl@linux.vnet.ibm.com
> Subject: Re: [PATCH net-next 01/10] net/smc: add missing dev_put
> 
> 
> 
> On 10/02/2017 10:36 PM, Parav Pandit wrote:
> > Hi Ursula, Dave, Hans,
> >
> >> -----Original Message-----
> >> From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
> >> owner@vger.kernel.org] On Behalf Of Ursula Braun
> >> Sent: Wednesday, September 20, 2017 6:58 AM
> >> To: davem@davemloft.net
> >> Cc: netdev@vger.kernel.org; linux-rdma@vger.kernel.org; linux-
> >> s390@vger.kernel.org; jwi@linux.vnet.ibm.com; schwidefsky@de.ibm.com;
> >> heiko.carstens@de.ibm.com; raspl@linux.vnet.ibm.com;
> >> ubraun@linux.vnet.ibm.com
> >> Subject: [PATCH net-next 01/10] net/smc: add missing dev_put
> >>
> >> From: Hans Wippel <hwippel@linux.vnet.ibm.com>
> >>
> >> In the infiniband part, SMC currently uses get_netdev which calls
> >> dev_hold on the returned net device. However, the SMC code never
> >> calls dev_put on that net device resulting in a wrong reference count.
> >>
> >> This patch adds a dev_put after the usage of the net device to fix the issue.
> >>
> >> Signed-off-by: Hans Wippel <hwippel@linux.vnet.ibm.com>
> >> Signed-off-by: Ursula Braun <ubraun@linux.vnet.ibm.com>
> >> ---
> >>  net/smc/smc_ib.c | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/net/smc/smc_ib.c b/net/smc/smc_ib.c index
> >> 547e0e113b17..0b5852299158 100644
> >> --- a/net/smc/smc_ib.c
> >> +++ b/net/smc/smc_ib.c
> >> @@ -380,6 +380,7 @@ static int smc_ib_fill_gid_and_mac(struct
> >> smc_ib_device *smcibdev, u8 ibport)
> >>  	ndev = smcibdev->ibdev->get_netdev(smcibdev->ibdev, ibport);
> >>  	if (ndev) {
> >>  		memcpy(&smcibdev->mac, ndev->dev_addr, ETH_ALEN);
> >> +		dev_put(ndev);
> >
> > I am sorry for providing late comments. smc_ib_fill_gid_and_mac() is not
> coded correctly.
> > Few fixes are needed.
> > 1. ULP such as SMC should not open code/deference any function pointer like
> get_netdev() of the IB device.
> > 2. Replace ib_query_gid(..., NULL)
> > With
> > ib_query_gid(..., gid_attr);
> >
> > Use gid_attr.ndev to get the MAC address.
> > Do dev_put(gid_attr.ndev);
> >
> > Code should look like below,
> >
> > struct ib_gid_attr gid_attr;
> >
> > rc = ib_query_gid(..., &gid_attr);
> > if (rc || !gid_addr.ndev)
> > 	return -ENODEV;
> > else
> > 	memcpy(smcibdev->mac, ndev->dev_addr, ETH_ALEN);
> > dev_put(gid_addr.ndev);
> >
> 
> Thanks, Parav!
> Following your fix ideas I plan to change the function into this one:
> 
> static int smc_ib_fill_gid_and_mac(struct smc_ib_device *smcibdev, u8 ibport) {
>         struct ib_gid_attr gattr;
>         int rc;
> 
>         rc = ib_query_gid(smcibdev->ibdev, ibport, 0,
>                           &smcibdev->gid[ibport - 1], &gattr);
>         /* the SMC protocol requires specification of the roce MAC address;
>          * if net_device cannot be determined, it can be derived from gid 0
>          */
>         if (rc)
>                 return rc;
> 
>         if (gattr.ndev) {
>                 memcpy(&smcibdev->mac, gattr.ndev->dev_addr, ETH_ALEN);
>                 dev_put(gattr.ndev);
>         } else {
>                 memcpy(&smcibdev->mac[ibport - 1][0],
>                        &smcibdev->gid[ibport - 1].raw[8], 3);
>                 memcpy(&smcibdev->mac[ibport - 1][3],
>                        &smcibdev->gid[ibport - 1].raw[13], 3);
>                 smcibdev->mac[ibport - 1][0] &= ~0x02;
This else part is not needed. You can fail the call as suggested above, inline below too.
if (rc || !gid_addr.ndev)
 	return -ENODEV;
There must be valid netdev for RoCE gid.
Rest code looks fine.

[PATCH net-next 02/10] net/smc: add receive timeout check

[Ursula Braun]

From: Hans Wippel <hwippel@linux.vnet.ibm.com>

The SMC receive function currently lacks a timeout check under the
condition that no data were received and no data are available. This
patch adds such a check.

Signed-off-by: Hans Wippel <hwippel@linux.vnet.ibm.com>
Signed-off-by: Ursula Braun <ubraun@linux.vnet.ibm.com>
---
 net/smc/smc_rx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/smc/smc_rx.c b/net/smc/smc_rx.c
index b17a333e9bb0..3e631ae4b6b6 100644
--- a/net/smc/smc_rx.c
+++ b/net/smc/smc_rx.c
@@ -148,6 +148,8 @@ int smc_rx_recvmsg(struct smc_sock *smc, struct msghdr *msg, size_t len,
 				read_done = sock_intr_errno(timeo);
 				break;
 			}
+			if (!timeo)
+				return -EAGAIN;
 		}
 
 		if (!atomic_read(&conn->bytes_to_rcv)) {
-- 
2.13.5

[PATCH net-next 04/10] net/smc: adjust net_device refcount

[Ursula Braun]

smc_pnet_fill_entry() uses dev_get_by_name() adding a refcount to ndev.
The following smc_pnet_enter() has to reduce the refcount if the entry
to be added exists already in the pnet table.

Signed-off-by: Ursula Braun <ubraun@linux.vnet.ibm.com>
---
 net/smc/smc_pnet.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/smc/smc_pnet.c b/net/smc/smc_pnet.c
index 78f7af28ae4f..31f8453c25c5 100644
--- a/net/smc/smc_pnet.c
+++ b/net/smc/smc_pnet.c
@@ -181,8 +181,10 @@ static int smc_pnet_enter(struct smc_pnetentry *new_pnetelem)
 			     sizeof(new_pnetelem->ndev->name)) ||
 		    smc_pnet_same_ibname(pnetelem,
 					 new_pnetelem->smcibdev->ibdev->name,
-					 new_pnetelem->ib_port))
+					 new_pnetelem->ib_port)) {
+			dev_put(pnetelem->ndev);
 			goto found;
+		}
 	}
 	list_add_tail(&new_pnetelem->list, &smc_pnettable.pnetlist);
 	rc = 0;
-- 
2.13.5

[PATCH net-next 05/10] net/smc: adapt send request completion notification

[Ursula Braun]

The solicited flag is meaningful for the receive completion queue.
Ask for next work completion of any type on the send queue.

Signed-off-by: Ursula Braun <ubraun@linux.vnet.ibm.com>
---
 net/smc/smc_wr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/smc/smc_wr.c b/net/smc/smc_wr.c
index ab56bda66783..525d91e0d57e 100644
--- a/net/smc/smc_wr.c
+++ b/net/smc/smc_wr.c
@@ -244,7 +244,7 @@ int smc_wr_tx_send(struct smc_link *link, struct smc_wr_tx_pend_priv *priv)
 	int rc;
 
 	ib_req_notify_cq(link->smcibdev->roce_cq_send,
-			 IB_CQ_SOLICITED_MASK | IB_CQ_REPORT_MISSED_EVENTS);
+			 IB_CQ_NEXT_COMP | IB_CQ_REPORT_MISSED_EVENTS);
 	pend = container_of(priv, struct smc_wr_tx_pend, priv);
 	rc = ib_post_send(link->roce_qp, &link->wr_tx_ibs[pend->idx],
 			  &failed_wr);
-- 
2.13.5

[PATCH net-next 06/10] net/smc: longer delay for client link group removal

[Ursula Braun]

Client link group creation always follows the server linkgroup creation.
If peer creates a new server link group, client has to create a new
client link group. If peer reuses a server link group for a new
connection, client has to reuse its client link group as well. This
patch introduces a longer delay for client link group removal to make
sure this link group still exists, once the peer decides to reuse a
server link group. This avoids out-of-sync conditions for link groups.
If already scheduled, modify the delay.

Signed-off-by: Ursula Braun <ubraun@linux.vnet.ibm.com>
---
 net/smc/smc_core.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index 1a16d51e2330..20b66e79c5d6 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -25,8 +25,9 @@
 #include "smc_cdc.h"
 #include "smc_close.h"
 
-#define SMC_LGR_NUM_INCR	256
-#define SMC_LGR_FREE_DELAY	(600 * HZ)
+#define SMC_LGR_NUM_INCR		256
+#define SMC_LGR_FREE_DELAY_SERV		(600 * HZ)
+#define SMC_LGR_FREE_DELAY_CLNT		(SMC_LGR_FREE_DELAY_SERV + 10)
 
 static u32 smc_lgr_num;			/* unique link group number */
 
@@ -107,8 +108,15 @@ static void smc_lgr_unregister_conn(struct smc_connection *conn)
 		__smc_lgr_unregister_conn(conn);
 	}
 	write_unlock_bh(&lgr->conns_lock);
-	if (reduced && !lgr->conns_num)
-		schedule_delayed_work(&lgr->free_work, SMC_LGR_FREE_DELAY);
+	if (!reduced || lgr->conns_num)
+		return;
+	/* client link group creation always follows the server link group
+	 * creation. For client use a somewhat higher removal delay time,
+	 * otherwise there is a risk of out-of-sync link groups.
+	 */
+	mod_delayed_work(system_wq, &lgr->free_work,
+			 lgr->role == SMC_CLNT ? SMC_LGR_FREE_DELAY_CLNT :
+						 SMC_LGR_FREE_DELAY_SERV);
 }
 
 static void smc_lgr_free_work(struct work_struct *work)
-- 
2.13.5

[PATCH net-next 07/10] net/smc: terminate link group if out-of-sync is received

[Ursula Braun]

An out-of-sync condition can just be detected by the client.
If the server receives a CLC DECLINE message indicating an out-of-sync
condition for the link groups, the server must clean up the out-of-sync
link group.
There is no need for an extra third parameter in smc_clc_send_decline().

Signed-off-by: Ursula Braun <ubraun@linux.vnet.ibm.com>
---
 net/smc/af_smc.c  |  6 ++----
 net/smc/smc_clc.c | 10 +++++-----
 net/smc/smc_clc.h |  3 +--
 3 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index 2e8d2dabac0c..745f145d4c4d 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -513,7 +513,7 @@ static int smc_connect_rdma(struct smc_sock *smc)
 	/* RDMA setup failed, switch back to TCP */
 	smc->use_fallback = true;
 	if (reason_code && (reason_code != SMC_CLC_DECL_REPLY)) {
-		rc = smc_clc_send_decline(smc, reason_code, 0);
+		rc = smc_clc_send_decline(smc, reason_code);
 		if (rc < sizeof(struct smc_clc_msg_decline))
 			goto out_err;
 	}
@@ -808,8 +808,6 @@ static void smc_listen_work(struct work_struct *work)
 		rc = local_contact;
 		if (rc == -ENOMEM)
 			reason_code = SMC_CLC_DECL_MEM;/* insufficient memory*/
-		else if (rc == -ENOLINK)
-			reason_code = SMC_CLC_DECL_SYNCERR; /* synchr. error */
 		goto decline_rdma;
 	}
 	link = &new_smc->conn.lgr->lnk[SMC_SINGLE_LINK];
@@ -903,7 +901,7 @@ static void smc_listen_work(struct work_struct *work)
 	smc_conn_free(&new_smc->conn);
 	new_smc->use_fallback = true;
 	if (reason_code && (reason_code != SMC_CLC_DECL_REPLY)) {
-		rc = smc_clc_send_decline(new_smc, reason_code, 0);
+		rc = smc_clc_send_decline(new_smc, reason_code);
 		if (rc < sizeof(struct smc_clc_msg_decline))
 			goto out_err;
 	}
diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
index 3934913ab835..b7dd2743fb5c 100644
--- a/net/smc/smc_clc.c
+++ b/net/smc/smc_clc.c
@@ -95,9 +95,10 @@ int smc_clc_wait_msg(struct smc_sock *smc, void *buf, int buflen,
 	}
 	if (clcm->type == SMC_CLC_DECLINE) {
 		reason_code = SMC_CLC_DECL_REPLY;
-		if (ntohl(((struct smc_clc_msg_decline *)buf)->peer_diagnosis)
-			== SMC_CLC_DECL_SYNCERR)
+		if (((struct smc_clc_msg_decline *)buf)->hdr.flag) {
 			smc->conn.lgr->sync_err = true;
+			smc_lgr_terminate(smc->conn.lgr);
+		}
 	}
 
 out:
@@ -105,8 +106,7 @@ int smc_clc_wait_msg(struct smc_sock *smc, void *buf, int buflen,
 }
 
 /* send CLC DECLINE message across internal TCP socket */
-int smc_clc_send_decline(struct smc_sock *smc, u32 peer_diag_info,
-			 u8 out_of_sync)
+int smc_clc_send_decline(struct smc_sock *smc, u32 peer_diag_info)
 {
 	struct smc_clc_msg_decline dclc;
 	struct msghdr msg;
@@ -118,7 +118,7 @@ int smc_clc_send_decline(struct smc_sock *smc, u32 peer_diag_info,
 	dclc.hdr.type = SMC_CLC_DECLINE;
 	dclc.hdr.length = htons(sizeof(struct smc_clc_msg_decline));
 	dclc.hdr.version = SMC_CLC_V1;
-	dclc.hdr.flag = out_of_sync ? 1 : 0;
+	dclc.hdr.flag = (peer_diag_info == SMC_CLC_DECL_SYNCERR) ? 1 : 0;
 	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_clc.h b/net/smc/smc_clc.h
index 13db8ce177c9..1c55414041d4 100644
--- a/net/smc/smc_clc.h
+++ b/net/smc/smc_clc.h
@@ -106,8 +106,7 @@ struct smc_ib_device;
 
 int smc_clc_wait_msg(struct smc_sock *smc, void *buf, int buflen,
 		     u8 expected_type);
-int smc_clc_send_decline(struct smc_sock *smc, u32 peer_diag_info,
-			 u8 out_of_sync);
+int smc_clc_send_decline(struct smc_sock *smc, u32 peer_diag_info);
 int smc_clc_send_proposal(struct smc_sock *smc, struct smc_ib_device *smcibdev,
 			  u8 ibport);
 int smc_clc_send_confirm(struct smc_sock *smc);
-- 
2.13.5

[PATCH net-next 08/10] net/smc: introduce a delay

[Ursula Braun]

The number of outstanding work requests is limited. If all work
requests are in use, tx processing is postponed to another scheduling
of the tx worker. Switch to a delayed worker to have a gap for tx
completion queue events before the next retry.

Signed-off-by: Ursula Braun <ubraun@linux.vnet.ibm.com>
---
 net/smc/smc.h       |  2 +-
 net/smc/smc_close.c | 12 +++++++-----
 net/smc/smc_tx.c    | 12 ++++++++----
 3 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/net/smc/smc.h b/net/smc/smc.h
index 6e44313e4467..0ccd6fa387ad 100644
--- a/net/smc/smc.h
+++ b/net/smc/smc.h
@@ -149,7 +149,7 @@ struct smc_connection {
 	atomic_t		sndbuf_space;	/* remaining space in sndbuf */
 	u16			tx_cdc_seq;	/* sequence # for CDC send */
 	spinlock_t		send_lock;	/* protect wr_sends */
-	struct work_struct	tx_work;	/* retry of smc_cdc_msg_send */
+	struct delayed_work	tx_work;	/* retry of smc_cdc_msg_send */
 
 	struct smc_host_cdc_msg	local_rx_ctrl;	/* filled during event_handl.
 						 * .prod cf. TCP rcv_nxt
diff --git a/net/smc/smc_close.c b/net/smc/smc_close.c
index 3c2e166b5d22..5201bc103bd8 100644
--- a/net/smc/smc_close.c
+++ b/net/smc/smc_close.c
@@ -208,7 +208,7 @@ int smc_close_active(struct smc_sock *smc)
 	case SMC_ACTIVE:
 		smc_close_stream_wait(smc, timeout);
 		release_sock(sk);
-		cancel_work_sync(&conn->tx_work);
+		cancel_delayed_work_sync(&conn->tx_work);
 		lock_sock(sk);
 		if (sk->sk_state == SMC_ACTIVE) {
 			/* send close request */
@@ -234,7 +234,7 @@ int smc_close_active(struct smc_sock *smc)
 		if (!smc_cdc_rxed_any_close(conn))
 			smc_close_stream_wait(smc, timeout);
 		release_sock(sk);
-		cancel_work_sync(&conn->tx_work);
+		cancel_delayed_work_sync(&conn->tx_work);
 		lock_sock(sk);
 		if (sk->sk_err != ECONNABORTED) {
 			/* confirm close from peer */
@@ -263,7 +263,9 @@ int smc_close_active(struct smc_sock *smc)
 		/* peer sending PeerConnectionClosed will cause transition */
 		break;
 	case SMC_PROCESSABORT:
-		cancel_work_sync(&conn->tx_work);
+		release_sock(sk);
+		cancel_delayed_work_sync(&conn->tx_work);
+		lock_sock(sk);
 		smc_close_abort(conn);
 		sk->sk_state = SMC_CLOSED;
 		smc_close_wait_tx_pends(smc);
@@ -425,7 +427,7 @@ int smc_close_shutdown_write(struct smc_sock *smc)
 	case SMC_ACTIVE:
 		smc_close_stream_wait(smc, timeout);
 		release_sock(sk);
-		cancel_work_sync(&conn->tx_work);
+		cancel_delayed_work_sync(&conn->tx_work);
 		lock_sock(sk);
 		/* send close wr request */
 		rc = smc_close_wr(conn);
@@ -439,7 +441,7 @@ int smc_close_shutdown_write(struct smc_sock *smc)
 		if (!smc_cdc_rxed_any_close(conn))
 			smc_close_stream_wait(smc, timeout);
 		release_sock(sk);
-		cancel_work_sync(&conn->tx_work);
+		cancel_delayed_work_sync(&conn->tx_work);
 		lock_sock(sk);
 		/* confirm close from peer */
 		rc = smc_close_wr(conn);
diff --git a/net/smc/smc_tx.c b/net/smc/smc_tx.c
index 3c656beb8820..3866573288dd 100644
--- a/net/smc/smc_tx.c
+++ b/net/smc/smc_tx.c
@@ -24,6 +24,8 @@
 #include "smc_cdc.h"
 #include "smc_tx.h"
 
+#define SMC_TX_WORK_DELAY	HZ
+
 /***************************** sndbuf producer *******************************/
 
 /* callback implementation for sk.sk_write_space()
@@ -406,7 +408,8 @@ int smc_tx_sndbuf_nonempty(struct smc_connection *conn)
 				goto out_unlock;
 			}
 			rc = 0;
-			schedule_work(&conn->tx_work);
+			schedule_delayed_work(&conn->tx_work,
+					      SMC_TX_WORK_DELAY);
 		}
 		goto out_unlock;
 	}
@@ -430,7 +433,7 @@ int smc_tx_sndbuf_nonempty(struct smc_connection *conn)
  */
 static void smc_tx_work(struct work_struct *work)
 {
-	struct smc_connection *conn = container_of(work,
+	struct smc_connection *conn = container_of(to_delayed_work(work),
 						   struct smc_connection,
 						   tx_work);
 	struct smc_sock *smc = container_of(conn, struct smc_sock, conn);
@@ -468,7 +471,8 @@ void smc_tx_consumer_update(struct smc_connection *conn)
 		if (!rc)
 			rc = smc_cdc_msg_send(conn, wr_buf, pend);
 		if (rc < 0) {
-			schedule_work(&conn->tx_work);
+			schedule_delayed_work(&conn->tx_work,
+					      SMC_TX_WORK_DELAY);
 			return;
 		}
 		smc_curs_write(&conn->rx_curs_confirmed,
@@ -487,6 +491,6 @@ void smc_tx_consumer_update(struct smc_connection *conn)
 void smc_tx_init(struct smc_sock *smc)
 {
 	smc->sk.sk_write_space = smc_tx_write_space;
-	INIT_WORK(&smc->conn.tx_work, smc_tx_work);
+	INIT_DELAYED_WORK(&smc->conn.tx_work, smc_tx_work);
 	spin_lock_init(&smc->conn.send_lock);
 }
-- 
2.13.5

Re: [PATCH net-next 08/10] net/smc: introduce a delay

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 388 bytes --]

On Wed, Sep 20, 2017 at 01:58:11PM +0200, Ursula Braun wrote:
> The number of outstanding work requests is limited. If all work
> requests are in use, tx processing is postponed to another scheduling
> of the tx worker. Switch to a delayed worker to have a gap for tx
> completion queue events before the next retry.
>

How will delay prevent and protect the resource exhausting?

Thanks

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH net-next 08/10] net/smc: introduce a delay

[Ursula Braun]

[-- Attachment #1.1.1: Type: text/plain, Size: 1004 bytes --]



On 09/20/2017 04:03 PM, Leon Romanovsky wrote:
> On Wed, Sep 20, 2017 at 01:58:11PM +0200, Ursula Braun wrote:
>> The number of outstanding work requests is limited. If all work
>> requests are in use, tx processing is postponed to another scheduling
>> of the tx worker. Switch to a delayed worker to have a gap for tx
>> completion queue events before the next retry.
>>
> 
> How will delay prevent and protect the resource exhausting?
> 
> Thanks
> 

SMC runs with a fixed number of in-flight work requests per QP (constant
SMC_WR_BUF_CNT) to prevent resource exhausting. If all work requests are
currently in use, sending of another work request has to wait till some
outstanding work request is confirmed via send completion queue. If sending
is done in a context which is not allowed to wait, the tx_worker is
scheduled instead.
With this patch a small delay is added to avoid too many unsuccessful send
retries due to a still ongoing "all work requests in use" condition.

[-- Attachment #1.1.2: 0xC5ED6645.asc --]
[-- Type: application/pgp-keys, Size: 9949 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH net-next 09/10] net/smc: parameter cleanup in smc_cdc_get_free_slot()

[Ursula Braun]

Use the smc_connection as first parameter with smc_cdc_get_free_slot().
This is just a small code cleanup, no functional change.

Signed-off-by: Ursula Braun <ubraun@linux.vnet.ibm.com>
---
 net/smc/smc_cdc.c | 7 ++++---
 net/smc/smc_cdc.h | 3 ++-
 net/smc/smc_tx.c  | 6 ++----
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/net/smc/smc_cdc.c b/net/smc/smc_cdc.c
index a7294edbc221..5ef97e5a5f78 100644
--- a/net/smc/smc_cdc.c
+++ b/net/smc/smc_cdc.c
@@ -62,10 +62,12 @@ static void smc_cdc_tx_handler(struct smc_wr_tx_pend_priv *pnd_snd,
 	bh_unlock_sock(&smc->sk);
 }
 
-int smc_cdc_get_free_slot(struct smc_link *link,
+int smc_cdc_get_free_slot(struct smc_connection *conn,
 			  struct smc_wr_buf **wr_buf,
 			  struct smc_cdc_tx_pend **pend)
 {
+	struct smc_link *link = &conn->lgr->lnk[SMC_SINGLE_LINK];
+
 	return smc_wr_tx_get_free_slot(link, smc_cdc_tx_handler, wr_buf,
 				       (struct smc_wr_tx_pend_priv **)pend);
 }
@@ -118,8 +120,7 @@ int smc_cdc_get_slot_and_msg_send(struct smc_connection *conn)
 	struct smc_wr_buf *wr_buf;
 	int rc;
 
-	rc = smc_cdc_get_free_slot(&conn->lgr->lnk[SMC_SINGLE_LINK], &wr_buf,
-				   &pend);
+	rc = smc_cdc_get_free_slot(conn, &wr_buf, &pend);
 	if (rc)
 		return rc;
 
diff --git a/net/smc/smc_cdc.h b/net/smc/smc_cdc.h
index 8e1d76f26007..56f883d1159c 100644
--- a/net/smc/smc_cdc.h
+++ b/net/smc/smc_cdc.h
@@ -206,7 +206,8 @@ static inline void smc_cdc_msg_to_host(struct smc_host_cdc_msg *local,
 
 struct smc_cdc_tx_pend;
 
-int smc_cdc_get_free_slot(struct smc_link *link, struct smc_wr_buf **wr_buf,
+int smc_cdc_get_free_slot(struct smc_connection *conn,
+			  struct smc_wr_buf **wr_buf,
 			  struct smc_cdc_tx_pend **pend);
 void smc_cdc_tx_dismiss_slots(struct smc_connection *conn);
 int smc_cdc_msg_send(struct smc_connection *conn, struct smc_wr_buf *wr_buf,
diff --git a/net/smc/smc_tx.c b/net/smc/smc_tx.c
index 3866573288dd..ec49bc3c3283 100644
--- a/net/smc/smc_tx.c
+++ b/net/smc/smc_tx.c
@@ -396,8 +396,7 @@ int smc_tx_sndbuf_nonempty(struct smc_connection *conn)
 	int rc;
 
 	spin_lock_bh(&conn->send_lock);
-	rc = smc_cdc_get_free_slot(&conn->lgr->lnk[SMC_SINGLE_LINK], &wr_buf,
-				   &pend);
+	rc = smc_cdc_get_free_slot(conn, &wr_buf, &pend);
 	if (rc < 0) {
 		if (rc == -EBUSY) {
 			struct smc_sock *smc =
@@ -466,8 +465,7 @@ void smc_tx_consumer_update(struct smc_connection *conn)
 	    ((to_confirm > conn->rmbe_update_limit) &&
 	     ((to_confirm > (conn->rmbe_size / 2)) ||
 	      conn->local_rx_ctrl.prod_flags.write_blocked))) {
-		rc = smc_cdc_get_free_slot(&conn->lgr->lnk[SMC_SINGLE_LINK],
-					   &wr_buf, &pend);
+		rc = smc_cdc_get_free_slot(conn, &wr_buf, &pend);
 		if (!rc)
 			rc = smc_cdc_msg_send(conn, wr_buf, pend);
 		if (rc < 0) {
-- 
2.13.5

[PATCH net-next 10/10] net/smc: no close wait in case of process shut down

[Ursula Braun]

Usually socket closing is delayed if there is still data available in
the send buffer to be transmitted. If a process is killed, the delay
should be avoided.

Signed-off-by: Ursula Braun <ubraun@linux.vnet.ibm.com>
---
 net/smc/smc_close.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/net/smc/smc_close.c b/net/smc/smc_close.c
index 5201bc103bd8..f0d16fb825f7 100644
--- a/net/smc/smc_close.c
+++ b/net/smc/smc_close.c
@@ -174,15 +174,15 @@ int smc_close_active(struct smc_sock *smc)
 {
 	struct smc_cdc_conn_state_flags *txflags =
 		&smc->conn.local_tx_ctrl.conn_state_flags;
-	long timeout = SMC_MAX_STREAM_WAIT_TIMEOUT;
 	struct smc_connection *conn = &smc->conn;
 	struct sock *sk = &smc->sk;
 	int old_state;
+	long timeout;
 	int rc = 0;
 
-	if (sock_flag(sk, SOCK_LINGER) &&
-	    !(current->flags & PF_EXITING))
-		timeout = sk->sk_lingertime;
+	timeout = current->flags & PF_EXITING ?
+		  0 : sock_flag(sk, SOCK_LINGER) ?
+		      sk->sk_lingertime : SMC_MAX_STREAM_WAIT_TIMEOUT;
 
 again:
 	old_state = sk->sk_state;
@@ -413,13 +413,14 @@ void smc_close_sock_put_work(struct work_struct *work)
 int smc_close_shutdown_write(struct smc_sock *smc)
 {
 	struct smc_connection *conn = &smc->conn;
-	long timeout = SMC_MAX_STREAM_WAIT_TIMEOUT;
 	struct sock *sk = &smc->sk;
 	int old_state;
+	long timeout;
 	int rc = 0;
 
-	if (sock_flag(sk, SOCK_LINGER))
-		timeout = sk->sk_lingertime;
+	timeout = current->flags & PF_EXITING ?
+		  0 : sock_flag(sk, SOCK_LINGER) ?
+		      sk->sk_lingertime : SMC_MAX_STREAM_WAIT_TIMEOUT;
 
 again:
 	old_state = sk->sk_state;
-- 
2.13.5

Re: [PATCH net-next 00/10] net/smc: updates 2017-09-20

[David Miller]

From: Ursula Braun <ubraun@linux.vnet.ibm.com>
Date: Wed, 20 Sep 2017 13:58:03 +0200

> here is a collection of small smc-patches built for net-next
> improving the smc code in different areas.

There are bug fixes in here, which should be targetted at 'net'.