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 <ms@xxxxxxxxxx> wrote:
>> I mean the change from only one hdlc<x> interface to both hdlc<x> and
>> hdlc<x>_x25.
>>
>> I can't estimate how many users are out there and how their setup
>> looks
>> like.
>
> I'm also thinking about solving this issue by adding new APIs to the
> HDLC subsystem (hdlc_stop_queue / hdlc_wake_queue) for hardware
> drivers to call instead of netif_stop_queue / netif_wake_queue. This
> way we can preserve backward compatibility.
>
> However I'm reluctant to change the code of all the hardware drivers
> because I'm afraid of introducing bugs, etc. When I look at the code
> of "wan/lmc/lmc_main.c", I feel I'm not able to make sure there are no
> bugs (related to stop_queue / wake_queue) after my change (and even
> before my change, actually). There are even serious style problems:
> the majority of its lines are indented by spaces.
>
> So I don't want to mess with all the hardware drivers. Hardware driver
> developers (if they wish to properly support hdlc_x25) should do the
> change themselves. This is not a problem for me, because I use my own
> out-of-tree hardware driver. However if I add APIs with no user code
> in the kernel, other developers may think these APIs are not
> necessary.
I don't think a change that affects the entire HDLC subsystem is
justified, since the actual problem only affects the hdlc_x25 area.
The approach with the additional hdlc<x>_x25 is clean and purposeful
and
I personally could live with it.
I just don't see myself in the position to decide such a change at the
moment.
@Jakub: What is your opinion on this.
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".
Can we cop out and add a knob somewhere to control spawning the extra
netdev? Let people who just want a newer kernel carry on without
distractions and those who want the extra layer can flip the switch?
Yes, that would be a good compromise.
I think a compile time selection option is enough here.
We could introduce a new config option CONFIG_HDLC_X25_LEGACY (or
something like that) and then implement the new or the old behavior in
the driver accordingly.
A switch that can be toggled at runtime (e.g. via sethdlc) would also be
conceivable, but I don't think this is necessary.
