path: root/net/l2tp/l2tp_netlink.c
diff options
authorDavid S. Miller <davem@davemloft.net>2018-01-05 11:03:42 -0500
committerDavid S. Miller <davem@davemloft.net>2018-01-05 11:03:42 -0500
commit2e40b823f072e2b57a8d7c43a4bd95988ced5568 (patch)
tree044a578a4e26d713e90fd285b0508ff663e55236 /net/l2tp/l2tp_netlink.c
parentnet/mlx5e: hide an unused variable (diff)
parentl2tp: add comment in API header that L2TP_ATTR_OFFSET is not used (diff)
Merge branch 'l2tp-remove-configurable-offset-parameters'
James Chapman says: ==================== l2tp: remove configurable offset parameters This patch series removes all code to support a configurable offset in transmitted l2tp packets. Code to handle this is incomplete and buggy and has been this way for years. If anyone tried to configure an offset, it would be ignored for L2TPv2 tunnels, or for L2TPv3 tunnels, could result in L2TPv3 packets being transmitted which are not compliant with L2TPv3 RFC3931. This patch series removes the support for configurable offsets. No known userspace l2tp daemon configures an offset. However, iproute2's "ip l2tp" command has an offset parameter and if set, the value is passed to the kernel. This is the most likely use case where offsets might be configured, e.g. ip l2tp add tunnel local remote tunnel_id 1 \ peer_tunnel_id 2 encap ip ip l2tp add session name l2tp0 tunnel_id 1 session_id 1 \ peer_session_id 2 offset 8 The above would result in packets being transmitted to with 8 bytes padding between the L2TPv3 header and the payload. The peer would need to be configured with the same offset value. However, the packets are not compliant with the L2TPv3 RFC, hence I think it's unlikely that offset is being used. With this patch series applied, the offset would not be configured. The peer would need to be modified to remove its offset setting too. iproute2 should be modified to remove or ignore the ip l2tp offset parameter. This issue was discovered when reviewing a patch series from lorenzo.bianconi@redhat.com which adds another netlink attribute to configure the expected offset in received L2TPv3 packets. This change is reverted by this series because offsets do not exist in L2TPv3 packets. These commits are: commit f15bc54eeecd ("l2tp: add peer_offset parameter") commit 820da5357572 ("l2tp: fix missing print session offset info") In more detail: The L2TPv2 protocol supports a variable offset from the L2TPv2 header to the payload to give the sender implementation some flexibility for data alignment when adding L2TP headers on to payloads. The offset value is indicated by an optional field in the L2TP header. Our L2TP implementation already detects the presence of the optional offset in received packets and skips those bytes when parsing packets. All transmitted L2TPv2 packets are always transmitted with no offset. L2TPv3 has no optional offset field in the L2TPv3 packet header. Instead, L2TPv3 defines optional fields in a "Layer-2 Specific Sublayer". At the time when the original L2TP code was written, there was talk at IETF of offset being implemented in a new Layer-2 Specific Sublayer. A L2TP_ATTR_OFFSET netlink attribute was added so that this offset could be configured and the intention was to allow it to be also used to set the tx offset for L2TPv2. However, no L2TPv3 offset was ever specified and the L2TP_ATTR_OFFSET parameter was forgotten about. Setting L2TP_ATTR_OFFSET results in L2TPv3 packets being transmitted with the specified number of bytes padding between L2TPv3 header and payload. This is not compliant with L2TPv3 RFC3931. So this change removes the configurable offset altogether while retaining L2TP_ATTR_OFFSET in the API for backwards compatibility. If L2TP_ATTR_OFFSET is given, its value is now silently ignored. ==================== Reviewed-by: Guillaume Nault <g.nault@alphalink.fr> Tested-by: Guillaume Nault <g.nault@alphalink.fr> Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'net/l2tp/l2tp_netlink.c')
1 files changed, 0 insertions, 24 deletions
diff --git a/net/l2tp/l2tp_netlink.c b/net/l2tp/l2tp_netlink.c
index d7d4d7a7a54d..e1ca29f79821 100644
--- a/net/l2tp/l2tp_netlink.c
+++ b/net/l2tp/l2tp_netlink.c
@@ -547,25 +547,6 @@ static int l2tp_nl_cmd_session_create(struct sk_buff *skb, struct genl_info *inf
if (tunnel->version > 2) {
- if (info->attrs[L2TP_ATTR_PEER_OFFSET]) {
- struct nlattr *peer_offset;
- peer_offset = info->attrs[L2TP_ATTR_PEER_OFFSET];
- cfg.peer_offset = nla_get_u16(peer_offset);
- }
- if (info->attrs[L2TP_ATTR_OFFSET]) {
- cfg.offset = nla_get_u16(info->attrs[L2TP_ATTR_OFFSET]);
- /* in order to maintain compatibility with older
- * versions where offset was used for both tx and
- * rx side, update rx side with offset if peer_offset
- * is not provided by userspace
- */
- if (!info->attrs[L2TP_ATTR_PEER_OFFSET])
- cfg.peer_offset = cfg.offset;
- }
if (info->attrs[L2TP_ATTR_DATA_SEQ])
cfg.data_seq = nla_get_u8(info->attrs[L2TP_ATTR_DATA_SEQ]);
@@ -777,10 +758,6 @@ static int l2tp_nl_session_send(struct sk_buff *skb, u32 portid, u32 seq, int fl
if ((session->ifname[0] &&
nla_put_string(skb, L2TP_ATTR_IFNAME, session->ifname)) ||
- (session->offset &&
- nla_put_u16(skb, L2TP_ATTR_OFFSET, session->offset)) ||
- (session->peer_offset &&
- nla_put_u16(skb, L2TP_ATTR_PEER_OFFSET, session->peer_offset)) ||
(session->cookie_len &&
nla_put(skb, L2TP_ATTR_COOKIE, session->cookie_len,
&session->cookie[0])) ||
@@ -921,7 +898,6 @@ static const struct nla_policy l2tp_nl_policy[L2TP_ATTR_MAX + 1] = {
[L2TP_ATTR_PW_TYPE] = { .type = NLA_U16, },
[L2TP_ATTR_ENCAP_TYPE] = { .type = NLA_U16, },
[L2TP_ATTR_OFFSET] = { .type = NLA_U16, },
- [L2TP_ATTR_PEER_OFFSET] = { .type = NLA_U16, },
[L2TP_ATTR_DATA_SEQ] = { .type = NLA_U8, },
[L2TP_ATTR_L2SPEC_TYPE] = { .type = NLA_U8, },
[L2TP_ATTR_L2SPEC_LEN] = { .type = NLA_U8, },