[PATCH v3 3/4] ipq40xx: fix compatibility with linux-atm tools

[Martin Schiller]

On 2025-01-13 23:27, Sergey Ryazanov wrote:
> Hi Andre, Martin,
> 
> On 13.01.2025 14:17, Andre Heider wrote:
>> On 13/01/2025 9:17 am, Martin Schiller wrote:
>>> On 2025-01-12 15:09, Sergey Ryazanov wrote:
>>>> atm_qos struct should be the same both for user and kernel spaces. 
>>>> Via
>>>> the __SO_ENCODE() macro it is used to define the SO_ATMQOS socket 
>>>> IOC.
>>>> 
>>>> During the VRX518 support introduction, the atm_trafprm sturct 
>>>> nested
>>>> into the atm_qos stucture was update with newer fields that are
>>>> referenced by the ATM TC layer of the VRX518 TC driver. These new 
>>>> fields
>>>> are intended to communicate information for extra traffic classes
>>>> supported by the driver. But we are still using vanilla kernel 
>>>> headers
>>>> to build the toolchain. Due to the atm.h header incoherency 
>>>> br2684ctl
>>>> from linux-atm tools is incapable to configure the ATM bridge 
>>>> netdev:
>>>> 
>>>>   br2684ctl: Interface "dsl0" created sucessfully
>>>>   br2684ctl: Communicating over ATM 0.1.2, encapsulation: LLC
>>>>   br2684ctl: setsockopt SO_ATMQOS 22 <-- EINVAL errno
>>>>   br2684ctl: Fatal: failed to connect on socket; File descriptor in 
>>>> bad state
>>> 
>>> Hmmm... I have fixed this problem in the past by including the
>>> "user_headers". See commit e53ed6af8744 (“linux-atm: use target 
>>> specific
>>> kernel headers”) [1]. Felix suggested this to me in [2].
>>> Does this no longer work?
>> 
>> Good question, I didn't know about that patch.
> 
> Me too. As it was mentioned in cover letter, I've faced the ATM bridge
> issue on the latest stable release - 23.05.5. And the e53ed6af87
> ("linux-atm: use target specific kernel headers") commit was
> introduced after the branching and was never backported:
> 
> $ git log --oneline -n3 master package/network/utils/linux-atm/
> 3e024022c3 linux-atm: fix build with GCC 14
> e53ed6af87 linux-atm: use target specific kernel headers
> 9aa3d5b345 linux-atm: Include linux/sockios.h for SIOCGSTAMP
> 
> $ git log --oneline -n2 openwrt-23.05 package/network/utils/linux-atm/
> 9aa3d5b345 linux-atm: Include linux/sockios.h for SIOCGSTAMP
> 806354ab53 linux-atm: Fix compile warning
> 
> I developed the extra fields disabling patch for the stable release
> and then rebased. And then in V3 extended it to drop the Lantiq's ATM
> hacks entirely.
> 
> BTW, I checked the Martin's fix in the latest master just now. It
> works and picks the correct atm.h from the patched kernel.
> 
>> At least on ipq40xx there's nothing writing the custom classes to qos- 
>>  >txtp.traffic_class, it's all just code that reads and checks for 
>> those, so it's basically useless and dead code. This removal here 
>> allows us to drop the atm.h patch.
> 
> I checked the git history, it looks like since 2007 only Lantiq's
> legacy driver (now ltq-atm) and various compat patches (now
> MIPS-lantiq-add-atm-hack.patch) are referencing
> ATM_VBR_NRT/ATM_VBR_RT/ATM_UBR_PLUS/ATM_GFR.
> 
> We have two options now. (a) Keep the Lantiq's hack in the tree for
> both platform and keep the include path amend in linux-atm. (b)
> proceed with this patch and probably backport it to v23 and v24. And
> with a follow up patch remove the corresponding ATM hacks from Lantiq
> and eventually from linux-atm.
> 
>> If that's the case too for lantiq, we could apply the same logic there 
>> and make linux-atm shared again?
> 
> Yep. I like the plan 'b' for cleaning the tree from vendor's hacks,
> stop spreading them to other targets and simplifying maintenance.

I think it would be good if we remove this from the lantiq target as 
well. The fewer patches, the better.

> 
>> In any case, this set still works on my ipq40xx device and vdsl seems 
>> to work just fine!
> 
> Have you gave a change to this series, Andre?
> 
> --
> Sergey