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 <ip0> remote <ip1> tunnel_id <id0>
peer_tunnel_id <id1> udp_sport <p0> udp_dport <p1>
- ip l2tp add tunnel local <ip1> remote <ip0> tunnel_id <id1>
peer_tunnel_id <id0> udp_sport <p1> udp_dport <p0>
- ip l2tp add session name l2tp0 tunnel_id <id0> session_id <s0>
peer_session_id <s1> offset 8
- ip l2tp add session name l2tp0 tunnel_id <id1> session_id <s1>
peer_session_id <s0> 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.
