Re: [PATCH net-next RFC v4] net: hdlc_x25: Queue outgoing LAPB frames

2021-03-03 Thread Xie He
On Wed, Mar 3, 2021 at 5:26 AM Martin Schiller wrote: > > On 2021-03-03 00:30, Jakub Kicinski wrote: > > > > Hard question to answer, existing users seem happy and Xie's driver > > isn't upstream, so the justification for potentially breaking backward > > compatibility isn't exactly "strong". > >

Re: [PATCH net-next RFC v4] net: hdlc_x25: Queue outgoing LAPB frames

2021-03-03 Thread Martin Schiller
On 2021-03-03 00:30, Jakub Kicinski wrote: On Tue, 02 Mar 2021 08:04:20 +0100 Martin Schiller wrote: On 2021-03-01 09:56, Xie He wrote: > On Sun, Feb 28, 2021 at 10:56 PM Martin Schiller wrote: >> I mean the change from only one hdlc interface to both hdlc and >> hdlc_x25. >> >> I can't

Re: [PATCH net-next RFC v4] net: hdlc_x25: Queue outgoing LAPB frames

2021-03-03 Thread Jakub Kicinski
On Tue, 02 Mar 2021 08:04:20 +0100 Martin Schiller wrote: > On 2021-03-01 09:56, Xie He wrote: > > On Sun, Feb 28, 2021 at 10:56 PM Martin Schiller wrote: > >> I mean the change from only one hdlc interface to both hdlc and > >> hdlc_x25. > >> > >> I can't estimate how many users are out there

Re: [PATCH net-next RFC v4] net: hdlc_x25: Queue outgoing LAPB frames

2021-03-02 Thread Martin Schiller
On 2021-03-01 09:56, Xie He wrote: On Sun, Feb 28, 2021 at 10:56 PM Martin Schiller wrote: >> Also, I have a hard time assessing if such a wrap is really >> enforceable. > > Sorry. I don't understand what you mean. What "wrap" are you referring > to? I mean the change from only one hdlc

Re: [PATCH net-next RFC v4] net: hdlc_x25: Queue outgoing LAPB frames

2021-03-01 Thread Xie He
On Sun, Feb 28, 2021 at 10:56 PM Martin Schiller wrote: > > >> Also, I have a hard time assessing if such a wrap is really > >> enforceable. > > > > Sorry. I don't understand what you mean. What "wrap" are you referring > > to? > > I mean the change from only one hdlc interface to both hdlc and >

Re: [PATCH net-next RFC v4] net: hdlc_x25: Queue outgoing LAPB frames

2021-02-28 Thread Martin Schiller
On 2021-02-27 00:03, Xie He wrote: On Fri, Feb 26, 2021 at 6:21 AM Martin Schiller wrote: I have now had a look at it. It works as expected. I just wonder if it would not be more appropriate to call the lapb_register() already in x25_hdlc_open(), so that the layer2 (lapb) can already "work"

Re: [PATCH net-next RFC v4] net: hdlc_x25: Queue outgoing LAPB frames

2021-02-26 Thread Xie He
On Fri, Feb 26, 2021 at 6:21 AM Martin Schiller wrote: > > I have now had a look at it. It works as expected. > I just wonder if it would not be more appropriate to call > the lapb_register() already in x25_hdlc_open(), so that the layer2 > (lapb) can already "work" before the hdlc_x25 interface

Re: [PATCH net-next RFC v4] net: hdlc_x25: Queue outgoing LAPB frames

2021-02-26 Thread Martin Schiller
On 2021-02-22 09:56, Xie He wrote: On Sun, Feb 21, 2021 at 11:14 PM Martin Schiller wrote: I'm not really happy with this change because it breaks compatibility. We then suddenly have 2 interfaces; the X.25 routings are to be set via the "new" hdlc_x25 interfaces instead of the hdlc

Re: [PATCH net-next RFC v4] net: hdlc_x25: Queue outgoing LAPB frames

2021-02-22 Thread Xie He
On Sun, Feb 21, 2021 at 11:14 PM Martin Schiller wrote: > > I'm not really happy with this change because it breaks compatibility. > We then suddenly have 2 interfaces; the X.25 routings are to be set via > the "new" hdlc_x25 interfaces instead of the hdlc interfaces. > > I currently just don't

Re: [PATCH net-next RFC v4] net: hdlc_x25: Queue outgoing LAPB frames

2021-02-21 Thread Martin Schiller
On 2021-02-19 21:28, Xie He wrote: On Fri, Feb 19, 2021 at 10:39 AM Jakub Kicinski wrote: Not entirely sure what the argument is about but adding constants would certainly help. Leon wants me to replace this: dev->needed_headroom = 3 - 1; with this: /* 2 is the result of 3 - 1 */

Re: [PATCH net-next RFC v4] net: hdlc_x25: Queue outgoing LAPB frames

2021-02-21 Thread Xie He
On Sat, Feb 20, 2021 at 10:39 PM Leon Romanovsky wrote: > > > Yes, this patch will break backward compatibility. Users with old > > scripts will find them no longer working. > > Did you search in debian/fedora code repositories to see if such scripts exist > as part of any distro package? I just

Re: [PATCH net-next RFC v4] net: hdlc_x25: Queue outgoing LAPB frames

2021-02-20 Thread Leon Romanovsky
On Fri, Feb 19, 2021 at 12:28:12PM -0800, Xie He wrote: > On Fri, Feb 19, 2021 at 10:39 AM Jakub Kicinski wrote: > > > > Not entirely sure what the argument is about but adding constants would > > certainly help. > > Leon wants me to replace this: > > dev->needed_headroom = 3 - 1; > > with this:

Re: [PATCH net-next RFC v4] net: hdlc_x25: Queue outgoing LAPB frames

2021-02-19 Thread Xie He
On Fri, Feb 19, 2021 at 10:39 AM Jakub Kicinski wrote: > > Not entirely sure what the argument is about but adding constants would > certainly help. Leon wants me to replace this: dev->needed_headroom = 3 - 1; with this: /* 2 is the result of 3 - 1 */ dev->needed_headroom = 2; But I don't

Re: [PATCH net-next RFC v4] net: hdlc_x25: Queue outgoing LAPB frames

2021-02-19 Thread Jakub Kicinski
On Thu, 18 Feb 2021 12:23:28 -0800 Xie He wrote: > On Thu, Feb 18, 2021 at 12:06 PM Xie He wrote: > > > > On Thu, Feb 18, 2021 at 11:55 AM Leon Romanovsky wrote: > > > > > > This is how we write code, we use defines instead of constant numbers, > > > comments to describe tricky parts and

Re: [PATCH net-next RFC v4] net: hdlc_x25: Queue outgoing LAPB frames

2021-02-18 Thread Xie He
On Thu, Feb 18, 2021 at 12:06 PM Xie He wrote: > > On Thu, Feb 18, 2021 at 11:55 AM Leon Romanovsky wrote: > > > > This is how we write code, we use defines instead of constant numbers, > > comments to describe tricky parts and assign already preprocessed result. > > > > There is nothing I can

Re: [PATCH net-next RFC v4] net: hdlc_x25: Queue outgoing LAPB frames

2021-02-18 Thread Xie He
On Thu, Feb 18, 2021 at 11:55 AM Leon Romanovsky wrote: > > This is how we write code, we use defines instead of constant numbers, > comments to describe tricky parts and assign already preprocessed result. > > There is nothing I can do If you don't like or don't want to use Linux kernel > style.

Re: [PATCH net-next RFC v4] net: hdlc_x25: Queue outgoing LAPB frames

2021-02-18 Thread Leon Romanovsky
On Thu, Feb 18, 2021 at 09:36:54AM -0800, Xie He wrote: > On Thu, Feb 18, 2021 at 2:37 AM Leon Romanovsky wrote: > > > > It is not me who didn't explain, it is you who didn't want to write clear > > comment that describes the headroom size without need of "3 - 1". > > Why do I need to write

Re: [PATCH net-next RFC v4] net: hdlc_x25: Queue outgoing LAPB frames

2021-02-18 Thread Xie He
On Thu, Feb 18, 2021 at 2:37 AM Leon Romanovsky wrote: > > It is not me who didn't explain, it is you who didn't want to write clear > comment that describes the headroom size without need of "3 - 1". Why do I need to write unnecessary comments when "3 - 1" and the current comment already

Re: [PATCH net-next RFC v4] net: hdlc_x25: Queue outgoing LAPB frames

2021-02-18 Thread Leon Romanovsky
On Thu, Feb 18, 2021 at 01:07:13AM -0800, Xie He wrote: > On Thu, Feb 18, 2021 at 12:57 AM Leon Romanovsky wrote: > > > > It is nice that you are resending your patch without the resolution. > > However it will be awesome if you don't ignore review comments and fix this > > "3 - 1" > > by

Re: [PATCH net-next RFC v4] net: hdlc_x25: Queue outgoing LAPB frames

2021-02-18 Thread Xie He
On Thu, Feb 18, 2021 at 12:57 AM Leon Romanovsky wrote: > > It is nice that you are resending your patch without the resolution. > However it will be awesome if you don't ignore review comments and fix this > "3 - 1" > by writing solid comment above. I thought you already agreed with me? It

Re: [PATCH net-next RFC v4] net: hdlc_x25: Queue outgoing LAPB frames

2021-02-18 Thread Leon Romanovsky
On Tue, Feb 16, 2021 at 12:18:13PM -0800, Xie He wrote: > When sending packets, we will first hand over the (L3) packets to the > LAPB module. The LAPB module will then hand over the corresponding LAPB > (L2) frames back to us for us to transmit. > > The LAPB module can also emit LAPB (L2) frames

[PATCH net-next RFC v4] net: hdlc_x25: Queue outgoing LAPB frames

2021-02-16 Thread Xie He
When sending packets, we will first hand over the (L3) packets to the LAPB module. The LAPB module will then hand over the corresponding LAPB (L2) frames back to us for us to transmit. The LAPB module can also emit LAPB (L2) frames at any time, even without an (L3) packet currently being sent on