[PATCH AUTOSEL 5.4 075/130] net/lapb: fix t1 timer handling for LAPB_STATE_0

[PATCH AUTOSEL 5.4 075/130] net/lapb: fix t1 timer handling for LAPB_STATE_0

[Sasha Levin]

From: Martin Schiller <ms@dev.tdt.de>

[ Upstream commit 62480b992ba3fb1d7260b11293aed9d6557831c7 ]

1. DTE interface changes immediately to LAPB_STATE_1 and start sending
   SABM(E).

2. DCE interface sends N2-times DM and changes to LAPB_STATE_1
   afterwards if there is no response in the meantime.

Signed-off-by: Martin Schiller <ms@dev.tdt.de>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 net/lapb/lapb_timer.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/net/lapb/lapb_timer.c b/net/lapb/lapb_timer.c
index 8f5b17001a076..baa247fe4ed05 100644
--- a/net/lapb/lapb_timer.c
+++ b/net/lapb/lapb_timer.c
@@ -85,11 +85,18 @@ static void lapb_t1timer_expiry(struct timer_list *t)
 	switch (lapb->state) {
 
 		/*
-		 *	If we are a DCE, keep going DM .. DM .. DM
+		 *	If we are a DCE, send DM up to N2 times, then switch to
+		 *	STATE_1 and send SABM(E).
 		 */
 		case LAPB_STATE_0:
-			if (lapb->mode & LAPB_DCE)
+			if (lapb->mode & LAPB_DCE &&
+			    lapb->n2count != lapb->n2) {
+				lapb->n2count++;
 				lapb_send_control(lapb, LAPB_DM, LAPB_POLLOFF, LAPB_RESPONSE);
+			} else {
+				lapb->state = LAPB_STATE_1;
+				lapb_establish_data_link(lapb);
+			}
 			break;
 
 		/*
-- 
2.27.0

Re: [PATCH AUTOSEL 5.4 075/130] net/lapb: fix t1 timer handling for LAPB_STATE_0

[Xie He]

> From: Martin Schiller <ms@dev.tdt.de>
>
> [ Upstream commit 62480b992ba3fb1d7260b11293aed9d6557831c7 ]
>
> 1. DTE interface changes immediately to LAPB_STATE_1 and start sending
>    SABM(E).
>
> 2. DCE interface sends N2-times DM and changes to LAPB_STATE_1
>    afterwards if there is no response in the meantime.

I don't think this patch is suitable for stable branches. This patch is
part of a patch series that changes the lapb module from "establishing the
L2 connection only when needed by L3", to "establishing the L2 connection
automatically whenever we are able to". This is a behavioral change. It
should be seen as a new feature. It is not a bug fix.

Re: [PATCH AUTOSEL 5.4 075/130] net/lapb: fix t1 timer handling for LAPB_STATE_0

[Xie He]

On Wed, Dec 23, 2020 at 9:01 AM Xie He <xie.he.0141@gmail.com> wrote:
>
> I don't think this patch is suitable for stable branches. This patch is
> part of a patch series that changes the lapb module from "establishing the
> L2 connection only when needed by L3", to "establishing the L2 connection
> automatically whenever we are able to". This is a behavioral change. It
> should be seen as a new feature. It is not a bug fix.

Applying this patch without other patches in the same series will also
introduce problems, because this patch relies on part of the changes
in the subsequent patch in the same series to be correct.

Hi Martin,

It's better that we avoid using words like "fix" in non-bug-fix
patches, and make every patch work on its own without subsequent
patches. Otherwise we'll make people confused.

Re: [PATCH AUTOSEL 5.4 075/130] net/lapb: fix t1 timer handling for LAPB_STATE_0

[Sasha Levin]

On Thu, Dec 24, 2020 at 01:49:47AM -0800, Xie He wrote:
>On Wed, Dec 23, 2020 at 9:01 AM Xie He <xie.he.0141@gmail.com> wrote:
>>
>> I don't think this patch is suitable for stable branches. This patch is
>> part of a patch series that changes the lapb module from "establishing the
>> L2 connection only when needed by L3", to "establishing the L2 connection
>> automatically whenever we are able to". This is a behavioral change. It
>> should be seen as a new feature. It is not a bug fix.
>
>Applying this patch without other patches in the same series will also
>introduce problems, because this patch relies on part of the changes
>in the subsequent patch in the same series to be correct.

I'll drop it, thanks!

-- 
Thanks,
Sasha

Re: [PATCH AUTOSEL 5.4 075/130] net/lapb: fix t1 timer handling for LAPB_STATE_0

[Martin Schiller]

On 2020-12-24 10:49, Xie He wrote:
> On Wed, Dec 23, 2020 at 9:01 AM Xie He <xie.he.0141@gmail.com> wrote:
>> 
>> I don't think this patch is suitable for stable branches. This patch 
>> is
>> part of a patch series that changes the lapb module from "establishing 
>> the
>> L2 connection only when needed by L3", to "establishing the L2 
>> connection
>> automatically whenever we are able to". This is a behavioral change. 
>> It
>> should be seen as a new feature. It is not a bug fix.
> 
> Applying this patch without other patches in the same series will also
> introduce problems, because this patch relies on part of the changes
> in the subsequent patch in the same series to be correct.
> 
> Hi Martin,
> 
> It's better that we avoid using words like "fix" in non-bug-fix
> patches, and make every patch work on its own without subsequent
> patches. Otherwise we'll make people confused.

Yes, you are right.