On 02/01/18 20:08, James Chapman wrote:

On 02/01/18 18:05, Guillaume Nault wrote:

quoted

quoted

quoted

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?

quoted

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.

quoted

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.

I think the first one too. Also update iproute2 to remove or hide the 
offset and peer_offset parameters.

I just realised the peer_offset attribute changes are already applied in 
net-next. (I missed these when they were submitted just before 
Christmas.) Should these commits be reverted? We probably don't want 
v4.15 to get an additional l2tp peer_offset attribute if we are going to 
remove it and the rest of the code supporting configurable offset 
attributes in the next release.

81487bf Merge branch 'l2tp-next'
f15bc54 l2tp: add peer_offset parameter
820da53 l2tp: fix missing print session offset info