From mboxrd@z Thu Jan 1 00:00:00 1970 From: James Chapman Subject: Re: [PATCH net-next 2/2] l2tp: add peer_offset parameter Date: Tue, 2 Jan 2018 20:18:27 +0000 Message-ID: <95029884-ced2-253a-efb4-81cda15cbfe7@katalix.com> References: <20171228145340.GA1292@alphalink.fr> <20171228182347.GA8732@localhost.localdomain> <20171228194556.GA1256@alphalink.fr> <27e0a7c8-b95b-04eb-d808-bcae67e417b1@katalix.com> <20180102180557.GB1402@alphalink.fr> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: "David S. Miller" , netdev@vger.kernel.org, Hangbin Liu To: Lorenzo Bianconi , Guillaume Nault Return-path: Received: from mail.katalix.com ([82.103.140.233]:34059 "EHLO mail.katalix.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750726AbeABUUC (ORCPT ); Tue, 2 Jan 2018 15:20:02 -0500 In-Reply-To: Content-Language: en-GB Sender: netdev-owner@vger.kernel.org List-ID: On 02/01/18 19:28, Lorenzo Bianconi wrote: >>>> Lorenzo, is this being added to fix interoperability with another L2TPv3 >>>> implementation? If so, can you share more details? >>>> >>> Hi James, >>> >>> I introduced peer_offset parameter to fix a specific setup where >>> tunnel endpoints >>> running L2TPv3 would use different values for tx offset (since in >>> iproute2 there is no >>> restriction on it), not to fix a given an interoperability issue. >>> >> Yes, but was it just to test iproute2's peer_offset option? Or is there >> a plan to use it for real? >> >>> I introduced this feature since: >>> - offset has been added for long time to L2TPv3 implementation >>> (commit f7faffa3ff8ef6ae712ef16312b8a2aa7a1c95fe and >>> commit 309795f4bec2d69cd507a631f82065c2198a0825) and I wanted to >>> preserve UABI >>> - have the same degree of freedom for offset parameter we have in >>> L2TPv2 and fix the issue >>> described above >>> >> AFAIU, the current L2TPv2 implementation never sets the offset field >> and nobody ever realised. >> > Perhaps I am little bit polarized on UABI issue, but I was rethinking > about it and maybe removing offset parameter would lead to an > interoperability issue for device running L2TPv3 since offset > parameter is there and it is not a nope. > Please consider this setup: > - 2 endpoint running L2TPv3, the first running net-next and the second > running 4.14 > - both endpoint are configured using iproute2 in this way: > > - ip l2tp add tunnel local remote tunnel_id > peer_tunnel_id udp_sport udp_dport > - ip l2tp add tunnel local remote tunnel_id > peer_tunnel_id udp_sport udp_dport > - ip l2tp add session name l2tp0 tunnel_id session_id > peer_session_id offset 8 > - ip l2tp add session name l2tp0 tunnel_id session_id > peer_session_id offset 8 > > Can we assume offset is never used for L2TPv3? If offset is ever set non-zero, packets transmitted on the wire are not compliant with the L2TPv3 RFC. So if someone is using a non-zero offset in their config, it might be a good thing that it stops working with a kernel update. > Regards, > Lorenzo > >>> Now what we can do I guess is: >>> - as suggested by Guillaume drop completely the offset support without removing >>> netlink attribute in order to not break UABI >>> - fix offset support initializing properly padding bits >>> >> I'd go for the first one. I just wonder if that looks acceptable to >> David an James.