mbox series

[net-next,v3,0/6] net/x25: netdev event handling

Message ID 20201118135919.1447-1-ms@dev.tdt.de (mailing list archive)
Headers show
Series net/x25: netdev event handling | expand

Message

Martin Schiller Nov. 18, 2020, 1:59 p.m. UTC
---
Changes to v2:
o restructure complete patch-set
o keep netdev event handling in layer3 (X.25)
o add patch to fix lapb_connect_request() for DCE
o add patch to handle carrier loss correctly in lapb
o drop patch for x25_neighbour param handling
  this may need fixes/cleanup and will be resubmitted later.

Changes to v1:
o fix 'subject_prefix' and 'checkpatch' warnings

---

Martin Schiller (6):
  net/x25: handle additional netdev events
  net/lapb: fix lapb_connect_request() for DCE
  net/lapb: handle carrier loss correctly
  net/lapb: fix t1 timer handling for DCE
  net/x25: fix restart request/confirm handling
  net/x25: remove x25_kill_by_device()

 include/net/x25.h     |  2 +
 net/lapb/lapb_iface.c | 22 +++++++++--
 net/lapb/lapb_timer.c | 11 +++++-
 net/x25/af_x25.c      | 66 +++++++++++++++++++++++--------
 net/x25/x25_link.c    | 90 ++++++++++++++++++++++++++++++++++++-------
 net/x25/x25_route.c   |  3 --
 6 files changed, 155 insertions(+), 39 deletions(-)

Comments

Xie He Nov. 18, 2020, 2:47 p.m. UTC | #1
On Wed, Nov 18, 2020 at 5:59 AM Martin Schiller <ms@dev.tdt.de> wrote:
>
> ---
> Changes to v2:
> o restructure complete patch-set
> o keep netdev event handling in layer3 (X.25)

But... Won't it be better to handle L2 connections in L2 code?

For example, if we are running X.25 over XOT, we can decide in the XOT
layer whether and when we reconnect in case the TCP connection is
dropped. We can decide how long we wait for responses before we
consider the TCP connection to be dropped.

If we still want "on-demand" connections in certain L2's, we can also
implement it in that L2 without the need to change L3.

Every L2 has its own characteristics. It might be better to let
different L2's handle their connections in their own way. This gives
L2 the flexibility to handle their connections according to their
actual link characteristics.

Letting L3 handle L2 connections also makes L2 code too related to /
coupled with L3 code, which makes the logic complex.

> o add patch to fix lapb_connect_request() for DCE
> o add patch to handle carrier loss correctly in lapb
> o drop patch for x25_neighbour param handling
>   this may need fixes/cleanup and will be resubmitted later.
>
> Changes to v1:
> o fix 'subject_prefix' and 'checkpatch' warnings
>
> ---
>
> Martin Schiller (6):
>   net/x25: handle additional netdev events
>   net/lapb: fix lapb_connect_request() for DCE
>   net/lapb: handle carrier loss correctly
>   net/lapb: fix t1 timer handling for DCE
>   net/x25: fix restart request/confirm handling
>   net/x25: remove x25_kill_by_device()
Martin Schiller Nov. 19, 2020, 7:02 a.m. UTC | #2
On 2020-11-18 15:47, Xie He wrote:
> On Wed, Nov 18, 2020 at 5:59 AM Martin Schiller <ms@dev.tdt.de> wrote:
>> 
>> ---
>> Changes to v2:
>> o restructure complete patch-set
>> o keep netdev event handling in layer3 (X.25)
> 
> But... Won't it be better to handle L2 connections in L2 code?
> 
> For example, if we are running X.25 over XOT, we can decide in the XOT
> layer whether and when we reconnect in case the TCP connection is
> dropped. We can decide how long we wait for responses before we
> consider the TCP connection to be dropped.
> 
> If we still want "on-demand" connections in certain L2's, we can also
> implement it in that L2 without the need to change L3.
> 
> Every L2 has its own characteristics. It might be better to let
> different L2's handle their connections in their own way. This gives
> L2 the flexibility to handle their connections according to their
> actual link characteristics.
> 
> Letting L3 handle L2 connections also makes L2 code too related to /
> coupled with L3 code, which makes the logic complex.

OK, I will give it a try. But we need to keep the possibility to
initiate and terminate the L2 connection from L3.

In the on demand scenario i mentioned, the L2 should be connected when
the first L3 logical channel goes up and needs to be disconnected, when
the last L3 logical channel on an interface is cleared.

> 
>> o add patch to fix lapb_connect_request() for DCE
>> o add patch to handle carrier loss correctly in lapb
>> o drop patch for x25_neighbour param handling
>>   this may need fixes/cleanup and will be resubmitted later.
>> 
>> Changes to v1:
>> o fix 'subject_prefix' and 'checkpatch' warnings
>> 
>> ---
>> 
>> Martin Schiller (6):
>>   net/x25: handle additional netdev events
>>   net/lapb: fix lapb_connect_request() for DCE
>>   net/lapb: handle carrier loss correctly
>>   net/lapb: fix t1 timer handling for DCE
>>   net/x25: fix restart request/confirm handling
>>   net/x25: remove x25_kill_by_device()
Xie He Nov. 19, 2020, 8:09 a.m. UTC | #3
On Wed, Nov 18, 2020 at 11:02 PM Martin Schiller <ms@dev.tdt.de> wrote:
>
> On 2020-11-18 15:47, Xie He wrote:
> >
> > But... Won't it be better to handle L2 connections in L2 code?
> >
> > For example, if we are running X.25 over XOT, we can decide in the XOT
> > layer whether and when we reconnect in case the TCP connection is
> > dropped. We can decide how long we wait for responses before we
> > consider the TCP connection to be dropped.
> >
> > If we still want "on-demand" connections in certain L2's, we can also
> > implement it in that L2 without the need to change L3.
> >
> > Every L2 has its own characteristics. It might be better to let
> > different L2's handle their connections in their own way. This gives
> > L2 the flexibility to handle their connections according to their
> > actual link characteristics.
> >
> > Letting L3 handle L2 connections also makes L2 code too related to /
> > coupled with L3 code, which makes the logic complex.
>
> OK, I will give it a try. But we need to keep the possibility to
> initiate and terminate the L2 connection from L3.

OK. Thanks so much!

> In the on demand scenario i mentioned, the L2 should be connected when
> the first L3 logical channel goes up and needs to be disconnected, when
> the last L3 logical channel on an interface is cleared.

I see. Maybe we can do it this way:

When L3 wants to initiate the first L3 connection, it can check
whether the L2 connection is established, and if it is not, it can
instruct L2 to connect. This is the same as what the current code
(before this series) does.

When the last L3 connection is terminated, we can let L3 use the
one-byte header to "suggest" (rather than "instruct") L2 to terminate
the L2 connection. L2 can choose to either terminate the connection or
continue to keep it, based on whether it is in on-demand mode.