[PATCH iproute2 0/3] l2tp: remove unused fields in struct l2tp_parm

[PATCH iproute2 0/3] l2tp: remove unused fields in struct l2tp_parm

[Guillaume Nault]

Several fields of struct l2tp_parm are handled by create_session() but
can't actually be set by user.
Most of these fields can also be set by get_response(), but are ignored
afterwards.

Since these fields can't have any visible effect, let's just remove
them.

Guillaume Nault (3):
  l2tp: drop data_seq
  l2tp: drop mtu
  l2tp: drop lns_mode

 ip/ipl2tp.c | 13 -------------
 1 file changed, 13 deletions(-)

-- 
2.18.0

[PATCH iproute2 1/3] l2tp: drop data_seq

[Guillaume Nault]

This option can't be set by user and is never printed. Furthermore,
L2TP_ATTR_DATA_SEQ has always been a noop in Linux.

Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
---
 ip/ipl2tp.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/ip/ipl2tp.c b/ip/ipl2tp.c
index 05e96387..aca9912c 100644
--- a/ip/ipl2tp.c
+++ b/ip/ipl2tp.c
@@ -60,7 +60,6 @@ struct l2tp_parm {
 	unsigned int recv_seq:1;
 	unsigned int send_seq:1;
 	unsigned int lns_mode:1;
-	unsigned int data_seq:2;
 	unsigned int tunnel:1;
 	unsigned int session:1;
 	int reorder_timeout;
@@ -167,8 +166,6 @@ static int create_session(struct l2tp_parm *p)
 		addattr8(&req.n, 1024, L2TP_ATTR_SEND_SEQ, 1);
 	if (p->lns_mode)
 		addattr(&req.n, 1024, L2TP_ATTR_LNS_MODE);
-	if (p->data_seq)
-		addattr8(&req.n, 1024, L2TP_ATTR_DATA_SEQ, p->data_seq);
 	if (p->reorder_timeout)
 		addattr64(&req.n, 1024, L2TP_ATTR_RECV_TIMEOUT,
 					  p->reorder_timeout);
@@ -359,8 +356,6 @@ static int get_response(struct nlmsghdr *n, void *arg)
 		p->pw_type = rta_getattr_u16(attrs[L2TP_ATTR_PW_TYPE]);
 	if (attrs[L2TP_ATTR_ENCAP_TYPE])
 		p->encap = rta_getattr_u16(attrs[L2TP_ATTR_ENCAP_TYPE]);
-	if (attrs[L2TP_ATTR_DATA_SEQ])
-		p->data_seq = rta_getattr_u16(attrs[L2TP_ATTR_DATA_SEQ]);
 	if (attrs[L2TP_ATTR_CONN_ID])
 		p->tunnel_id = rta_getattr_u32(attrs[L2TP_ATTR_CONN_ID]);
 	if (attrs[L2TP_ATTR_PEER_CONN_ID])
-- 
2.18.0

[PATCH iproute2 2/3] l2tp: drop mtu

[Guillaume Nault]

This option can't be set by user and is never printed.

Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
---
 ip/ipl2tp.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/ip/ipl2tp.c b/ip/ipl2tp.c
index aca9912c..41fefb85 100644
--- a/ip/ipl2tp.c
+++ b/ip/ipl2tp.c
@@ -53,7 +53,6 @@ struct l2tp_parm {
 	inet_prefix peer_ip;
 
 	uint16_t pw_type;
-	uint16_t mtu;
 	unsigned int udp6_csum_tx:1;
 	unsigned int udp6_csum_rx:1;
 	unsigned int udp_csum:1;
@@ -158,8 +157,6 @@ static int create_session(struct l2tp_parm *p)
 	addattr8(&req.n, 1024, L2TP_ATTR_L2SPEC_TYPE, p->l2spec_type);
 	addattr8(&req.n, 1024, L2TP_ATTR_L2SPEC_LEN, p->l2spec_len);
 
-	if (p->mtu)
-		addattr16(&req.n, 1024, L2TP_ATTR_MTU, p->mtu);
 	if (p->recv_seq)
 		addattr8(&req.n, 1024, L2TP_ATTR_RECV_SEQ, 1);
 	if (p->send_seq)
@@ -413,8 +410,6 @@ static int get_response(struct nlmsghdr *n, void *arg)
 		p->local_udp_port = rta_getattr_u16(attrs[L2TP_ATTR_UDP_SPORT]);
 	if (attrs[L2TP_ATTR_UDP_DPORT])
 		p->peer_udp_port = rta_getattr_u16(attrs[L2TP_ATTR_UDP_DPORT]);
-	if (attrs[L2TP_ATTR_MTU])
-		p->mtu = rta_getattr_u16(attrs[L2TP_ATTR_MTU]);
 	if (attrs[L2TP_ATTR_IFNAME])
 		p->ifname = rta_getattr_str(attrs[L2TP_ATTR_IFNAME]);
 
-- 
2.18.0

[PATCH iproute2 3/3] l2tp: drop lns_mode

[Guillaume Nault]

This option is never set.

Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
---
 ip/ipl2tp.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/ip/ipl2tp.c b/ip/ipl2tp.c
index 41fefb85..5e7f0390 100644
--- a/ip/ipl2tp.c
+++ b/ip/ipl2tp.c
@@ -58,7 +58,6 @@ struct l2tp_parm {
 	unsigned int udp_csum:1;
 	unsigned int recv_seq:1;
 	unsigned int send_seq:1;
-	unsigned int lns_mode:1;
 	unsigned int tunnel:1;
 	unsigned int session:1;
 	int reorder_timeout;
@@ -161,8 +160,6 @@ static int create_session(struct l2tp_parm *p)
 		addattr8(&req.n, 1024, L2TP_ATTR_RECV_SEQ, 1);
 	if (p->send_seq)
 		addattr8(&req.n, 1024, L2TP_ATTR_SEND_SEQ, 1);
-	if (p->lns_mode)
-		addattr(&req.n, 1024, L2TP_ATTR_LNS_MODE);
 	if (p->reorder_timeout)
 		addattr64(&req.n, 1024, L2TP_ATTR_RECV_TIMEOUT,
 					  p->reorder_timeout);
-- 
2.18.0

Re: [PATCH iproute2 0/3] l2tp: remove unused fields in struct l2tp_parm

[Stephen Hemminger]

On Fri, 27 Jul 2018 12:26:28 +0200
Guillaume Nault <g.nault@alphalink.fr> wrote:

> Several fields of struct l2tp_parm are handled by create_session() but
> can't actually be set by user.
> Most of these fields can also be set by get_response(), but are ignored
> afterwards.
> 
> Since these fields can't have any visible effect, let's just remove
> them.
> 
> Guillaume Nault (3):
>   l2tp: drop data_seq
>   l2tp: drop mtu
>   l2tp: drop lns_mode
> 
>  ip/ipl2tp.c | 13 -------------
>  1 file changed, 13 deletions(-)
> 

These make sense for iproute2 next

Re: [PATCH iproute2 0/3] l2tp: remove unused fields in struct l2tp_parm

[Guillaume Nault]

On Fri, Jul 27, 2018 at 07:57:12AM -0700, Stephen Hemminger wrote:
> On Fri, 27 Jul 2018 12:26:28 +0200
> Guillaume Nault <g.nault@alphalink.fr> wrote:
> 
> > Several fields of struct l2tp_parm are handled by create_session() but
> > can't actually be set by user.
> > Most of these fields can also be set by get_response(), but are ignored
> > afterwards.
> > 
> > Since these fields can't have any visible effect, let's just remove
> > them.
> > 
> > Guillaume Nault (3):
> >   l2tp: drop data_seq
> >   l2tp: drop mtu
> >   l2tp: drop lns_mode
> > 
> >  ip/ipl2tp.c | 13 -------------
> >  1 file changed, 13 deletions(-)
> > 
> 
> These make sense for iproute2 next
These patches haven't been rejected in patchwork. Does that mean that
David A. will pick them up? Or should I repost to iproute2-next anyway?