From: Martin Schiller <ms@dev.tdt.de>
To: Xie He <xie.he.0141@gmail.com>
Cc: Andrew Hendry <andrew.hendry@gmail.com>,
"David S. Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>,
Linux X25 <linux-x25@vger.kernel.org>,
Linux Kernel Network Developers <netdev@vger.kernel.org>,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net-next v2 5/6] net/lapb: support netdev events
Date: Wed, 18 Nov 2020 09:49:12 +0100 [thread overview]
Message-ID: <c60fe64ff67e244bbe9971cfa08713db@dev.tdt.de> (raw)
In-Reply-To: <CAJht_EO=G94_xoCupr_7Tt_-kjYxZVfs2n4CTa14mXtu7oYMjg@mail.gmail.com>
On 2020-11-17 19:28, Xie He wrote:
> On Tue, Nov 17, 2020 at 5:26 AM Martin Schiller <ms@dev.tdt.de> wrote:
>>
>> On 2020-11-17 12:32, Xie He wrote:
>> >
>> > I think for a DCE, it doesn't need to initiate the L2
>> > connection on device-up. It just needs to wait for a connection to
>> > come. But L3 seems to be still instructing it to initiate the L2
>> > connection. This seems to be a problem.
>>
>> The "ITU-T Recommendation X.25 (10/96) aka "Blue Book" [1] says under
>> point 2.4.4.1:
>> "Either the DTE or the DCE may initiate data link set-up."
>>
>> Experience shows that there are also DTEs that do not establish a
>> connection themselves.
>>
>> That is also the reason why I've added this patch:
>> https://patchwork.kernel.org/project/netdevbpf/patch/20201116135522.21791-7-ms@dev.tdt.de/
>
> Yes, I understand that either the DTE or the DCE *may* initiate the L2
> connection. This is also the way the current code (before this patch
> set) works. But I see both the DTE and the DCE will try to initiate
> the L2 connection after device-up, because according to your 1st
> patch, L3 will always instruct L2 to do this on device-up. However,
> looking at your 6th patch (in the link you gave), you seem to want the
> DCE to wait for a while before initiating the connection by itself. So
> I'm unclear which way you want to go. Making DCE initiate the L2
> connection on device-up, or making DCE wait for a while before
> initiating the L2 connection? I think the second way is more
> reasonable.
Ah, ok. Now I see what you mean.
Yes, we should check the lapb->mode in lapb_connect_request().
>> > It feels unclean to me that the L2 connection will sometimes be
>> > initiated by L3 and sometimes by L2 itself. Can we make L2 connections
>> > always be initiated by L2 itself? If L3 needs to do something after L2
>> > links up, L2 will notify it anyway.
>>
>> My original goal was to change as little as possible of the original
>> code. And in the original code the NETDEV_UP/NETDEV_DOWN events
>> were/are
>> handled in L3. But it is of course conceivable to shift this to L2.
>
> I suggested moving L2 connection handling to L2 because I think having
> both L2 and L3 to handle this makes the logic of the code too complex.
> For example, after a device-up event, L3 will instruct L2 to initiate
> the L2 connection. But L2 code has its own way of initiating
> connections. For a DCE, L2 wants to wait a while before initiating the
> connection. So currently L2 and L3 want to do things differently and
> they are doing things at the same time.
>
>> But you have to keep in mind that the X.25 L3 stack can also be used
>> with tap interfaces (e.g. for XOT), where you do not have a L2 at all.
>
> Can we treat XOT the same as LAPB? I think XOT should be considered a
> L2 in this case. So maybe XOT can establish the TCP connection by
> itself without being instructed by L3. I'm not sure if this is
> feasible in practice but it'd be good if it is.
>
> This also simplifies the L3 code.
I also have a patch here that implements an "on demand" link feature,
which we used for ISDN dialing connections.
As ISDN is de facto dead, this is not relevant anymore. But if we want
such kind of feature, I think we need to stay with the method to control
L2 link state from L3.
next prev parent reply other threads:[~2020-11-18 8:49 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-16 13:55 [PATCH net-next v2 0/6] netdev event handling + neighbour config Martin Schiller
2020-11-16 13:55 ` [PATCH net-next v2 1/6] net/x25: handle additional netdev events Martin Schiller
2020-11-16 13:55 ` [PATCH net-next v2 2/6] net/x25: make neighbour params configurable Martin Schiller
2020-11-16 17:05 ` David Laight
2020-11-16 13:55 ` [PATCH net-next v2 3/6] net/x25: replace x25_kill_by_device with x25_kill_by_neigh Martin Schiller
2020-11-17 19:50 ` Xie He
2020-11-18 8:28 ` Martin Schiller
2020-11-16 13:55 ` [PATCH net-next v2 4/6] net/x25: support NETDEV_CHANGE notifier Martin Schiller
2020-11-17 11:41 ` Xie He
2020-11-17 12:30 ` Martin Schiller
2020-11-16 13:55 ` [PATCH net-next v2 5/6] net/lapb: support netdev events Martin Schiller
2020-11-16 20:16 ` Xie He
2020-11-17 9:52 ` Martin Schiller
2020-11-17 11:32 ` Xie He
2020-11-17 13:26 ` Martin Schiller
2020-11-17 18:28 ` Xie He
2020-11-18 8:49 ` Martin Schiller [this message]
2020-11-18 13:03 ` Xie He
2020-11-18 13:46 ` Xie He
2020-11-18 13:57 ` Martin Schiller
2020-11-16 13:55 ` [PATCH net-next v2 6/6] net/lapb: fix t1 timer handling Martin Schiller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c60fe64ff67e244bbe9971cfa08713db@dev.tdt.de \
--to=ms@dev.tdt.de \
--cc=andrew.hendry@gmail.com \
--cc=davem@davemloft.net \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-x25@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=xie.he.0141@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).