Netdev Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2 1/2] wan/hdlc_x25: make lapb params configurable
@ 2020-01-14 14:02 Martin Schiller
  2020-01-14 14:02 ` [PATCH v2 2/2] wan/hdlc_x25: fix skb handling Martin Schiller
  2020-01-15 21:43 ` [PATCH v2 1/2] wan/hdlc_x25: make lapb params configurable David Miller
  0 siblings, 2 replies; 4+ messages in thread
From: Martin Schiller @ 2020-01-14 14:02 UTC (permalink / raw)
  To: kubakici, khc, davem; +Cc: linux-x25, netdev, linux-kernel, Martin Schiller

This enables you to configure mode (DTE/DCE), Modulo, Window, T1, T2, N2 via
sethdlc (which needs to be patched as well).

Signed-off-by: Martin Schiller <ms@dev.tdt.de>
---
 drivers/net/wan/hdlc_x25.c      | 78 ++++++++++++++++++++++++++++++++-
 include/uapi/linux/hdlc/ioctl.h |  9 ++++
 include/uapi/linux/if.h         |  1 +
 3 files changed, 86 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wan/hdlc_x25.c b/drivers/net/wan/hdlc_x25.c
index 5643675ff724..0479a2bf42f7 100644
--- a/drivers/net/wan/hdlc_x25.c
+++ b/drivers/net/wan/hdlc_x25.c
@@ -21,8 +21,17 @@
 #include <linux/skbuff.h>
 #include <net/x25device.h>
 
+struct x25_state {
+	x25_hdlc_proto settings;
+};
+
 static int x25_ioctl(struct net_device *dev, struct ifreq *ifr);
 
+static struct x25_state* state(hdlc_device *hdlc)
+{
+	return (struct x25_state *)hdlc->state;
+}
+
 /* These functions are callbacks called by LAPB layer */
 
 static void x25_connect_disconnect(struct net_device *dev, int reason, int code)
@@ -132,6 +141,8 @@ static netdev_tx_t x25_xmit(struct sk_buff *skb, struct net_device *dev)
 static int x25_open(struct net_device *dev)
 {
 	int result;
+	hdlc_device *hdlc = dev_to_hdlc(dev);
+	struct lapb_parms_struct params;
 	static const struct lapb_register_struct cb = {
 		.connect_confirmation = x25_connected,
 		.connect_indication = x25_connected,
@@ -144,6 +155,26 @@ static int x25_open(struct net_device *dev)
 	result = lapb_register(dev, &cb);
 	if (result != LAPB_OK)
 		return result;
+
+	result = lapb_getparms(dev, &params);
+	if (result != LAPB_OK)
+		return result;
+
+	if (state(hdlc)->settings.dce)
+		params.mode = params.mode | LAPB_DCE;
+
+	if (state(hdlc)->settings.modulo == 128)
+		params.mode = params.mode | LAPB_EXTENDED;
+
+	params.window = state(hdlc)->settings.window;
+	params.t1 = state(hdlc)->settings.t1;
+	params.t2 = state(hdlc)->settings.t2;
+	params.n2 = state(hdlc)->settings.n2;
+
+	result = lapb_setparms(dev, &params);
+	if (result != LAPB_OK)
+		return result;
+
 	return 0;
 }
 
@@ -186,6 +217,9 @@ static struct hdlc_proto proto = {
 
 static int x25_ioctl(struct net_device *dev, struct ifreq *ifr)
 {
+	x25_hdlc_proto __user *x25_s = ifr->ifr_settings.ifs_ifsu.x25;
+	const size_t size = sizeof(x25_hdlc_proto);
+	x25_hdlc_proto new_settings;
 	hdlc_device *hdlc = dev_to_hdlc(dev);
 	int result;
 
@@ -194,7 +228,13 @@ static int x25_ioctl(struct net_device *dev, struct ifreq *ifr)
 		if (dev_to_hdlc(dev)->proto != &proto)
 			return -EINVAL;
 		ifr->ifr_settings.type = IF_PROTO_X25;
-		return 0; /* return protocol only, no settable parameters */
+		if (ifr->ifr_settings.size < size) {
+			ifr->ifr_settings.size = size; /* data size wanted */
+			return -ENOBUFS;
+		}
+		if (copy_to_user(x25_s, &state(hdlc)->settings, size))
+			return -EFAULT;
+		return 0;
 
 	case IF_PROTO_X25:
 		if (!capable(CAP_NET_ADMIN))
@@ -203,12 +243,46 @@ static int x25_ioctl(struct net_device *dev, struct ifreq *ifr)
 		if (dev->flags & IFF_UP)
 			return -EBUSY;
 
+		/* backward compatibility */
+		if (ifr->ifr_settings.size = 0) {
+			new_settings.dce = 0;
+			new_settings.modulo = 8;
+			new_settings.window = 7;
+			new_settings.t1 = 3;
+			new_settings.t2 = 1;
+			new_settings.n2 = 10;
+		}
+		else {
+			if (copy_from_user(&new_settings, x25_s, size))
+				return -EFAULT;
+
+			if ((new_settings.dce != 0 &&
+			new_settings.dce != 1) ||
+			(new_settings.modulo != 8 &&
+			new_settings.modulo != 128) ||
+			new_settings.window < 1 ||
+			(new_settings.modulo == 8 &&
+			new_settings.window > 7) ||
+			(new_settings.modulo == 128 &&
+			new_settings.window > 127) ||
+			new_settings.t1 < 1 ||
+			new_settings.t1 > 255 ||
+			new_settings.t2 < 1 ||
+			new_settings.t2 > 255 ||
+			new_settings.n2 < 1 ||
+			new_settings.n2 > 255)
+				return -EINVAL;
+		}
+
 		result=hdlc->attach(dev, ENCODING_NRZ,PARITY_CRC16_PR1_CCITT);
 		if (result)
 			return result;
 
-		if ((result = attach_hdlc_protocol(dev, &proto, 0)))
+		if ((result = attach_hdlc_protocol(dev, &proto,
+						   sizeof(struct x25_state))))
 			return result;
+
+		memcpy(&state(hdlc)->settings, &new_settings, size);
 		dev->type = ARPHRD_X25;
 		call_netdevice_notifiers(NETDEV_POST_TYPE_CHANGE, dev);
 		netif_dormant_off(dev);
diff --git a/include/uapi/linux/hdlc/ioctl.h b/include/uapi/linux/hdlc/ioctl.h
index 0fe4238e8246..b06341acab5e 100644
--- a/include/uapi/linux/hdlc/ioctl.h
+++ b/include/uapi/linux/hdlc/ioctl.h
@@ -79,6 +79,15 @@ typedef struct {
     unsigned int timeout;
 } cisco_proto;
 
+typedef struct {
+	unsigned short dce; /* 1 for DCE (network side) operation */
+	unsigned int modulo; /* modulo (8 = basic / 128 = extended) */
+	unsigned int window; /* frame window size */
+	unsigned int t1; /* timeout t1 */
+	unsigned int t2; /* timeout t2 */
+	unsigned int n2; /* frame retry counter */
+} x25_hdlc_proto;
+
 /* PPP doesn't need any info now - supply length = 0 to ioctl */
 
 #endif /* __ASSEMBLY__ */
diff --git a/include/uapi/linux/if.h b/include/uapi/linux/if.h
index 4bf33344aab1..be714cd8c826 100644
--- a/include/uapi/linux/if.h
+++ b/include/uapi/linux/if.h
@@ -213,6 +213,7 @@ struct if_settings {
 		fr_proto		__user *fr;
 		fr_proto_pvc		__user *fr_pvc;
 		fr_proto_pvc_info	__user *fr_pvc_info;
+		x25_hdlc_proto		__user *x25;
 
 		/* interface settings */
 		sync_serial_settings	__user *sync;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2 2/2] wan/hdlc_x25: fix skb handling
  2020-01-14 14:02 [PATCH v2 1/2] wan/hdlc_x25: make lapb params configurable Martin Schiller
@ 2020-01-14 14:02 ` Martin Schiller
  2020-01-15 21:43 ` [PATCH v2 1/2] wan/hdlc_x25: make lapb params configurable David Miller
  1 sibling, 0 replies; 4+ messages in thread
From: Martin Schiller @ 2020-01-14 14:02 UTC (permalink / raw)
  To: kubakici, khc, davem; +Cc: linux-x25, netdev, linux-kernel, Martin Schiller

 o call skb_reset_network_header() before hdlc->xmit()
 o change skb proto to HDLC (0x0019) before hdlc->xmit()
 o call dev_queue_xmit_nit() before hdlc->xmit()

This changes make it possible to trace (tcpdump) outgoing layer2
(ETH_P_HDLC) packets

Additionally call skb_reset_network_header() after each skb_push() /
skb_pull().

Signed-off-by: Martin Schiller <ms@dev.tdt.de>
---
 drivers/net/wan/hdlc_x25.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wan/hdlc_x25.c b/drivers/net/wan/hdlc_x25.c
index 0479a2bf42f7..9eaad151c27f 100644
--- a/drivers/net/wan/hdlc_x25.c
+++ b/drivers/net/wan/hdlc_x25.c
@@ -71,11 +71,12 @@ static int x25_data_indication(struct net_device *dev, struct sk_buff *skb)
 {
 	unsigned char *ptr;
 
-	skb_push(skb, 1);
-
 	if (skb_cow(skb, 1))
 		return NET_RX_DROP;
 
+	skb_push(skb, 1);
+	skb_reset_network_header(skb);
+
 	ptr  = skb->data;
 	*ptr = X25_IFACE_DATA;
 
@@ -88,6 +89,13 @@ static int x25_data_indication(struct net_device *dev, struct sk_buff *skb)
 static void x25_data_transmit(struct net_device *dev, struct sk_buff *skb)
 {
 	hdlc_device *hdlc = dev_to_hdlc(dev);
+
+	skb_reset_network_header(skb);
+	skb->protocol = hdlc_type_trans(skb, dev);
+
+	if (dev_nit_active(dev))
+		dev_queue_xmit_nit(skb, dev);
+
 	hdlc->xmit(skb, dev); /* Ignore return value :-( */
 }
 
@@ -102,6 +110,7 @@ static netdev_tx_t x25_xmit(struct sk_buff *skb, struct net_device *dev)
 	switch (skb->data[0]) {
 	case X25_IFACE_DATA:	/* Data to be transmitted */
 		skb_pull(skb, 1);
+		skb_reset_network_header(skb);
 		if ((result = lapb_data_request(dev, skb)) != LAPB_OK)
 			dev_kfree_skb(skb);
 		return NETDEV_TX_OK;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 1/2] wan/hdlc_x25: make lapb params configurable
  2020-01-14 14:02 [PATCH v2 1/2] wan/hdlc_x25: make lapb params configurable Martin Schiller
  2020-01-14 14:02 ` [PATCH v2 2/2] wan/hdlc_x25: fix skb handling Martin Schiller
@ 2020-01-15 21:43 ` David Miller
  2020-01-16  6:03   ` Martin Schiller
  1 sibling, 1 reply; 4+ messages in thread
From: David Miller @ 2020-01-15 21:43 UTC (permalink / raw)
  To: ms; +Cc: kubakici, khc, linux-x25, netdev, linux-kernel

From: Martin Schiller <ms@dev.tdt.de>
Date: Tue, 14 Jan 2020 15:02:22 +0100

> This enables you to configure mode (DTE/DCE), Modulo, Window, T1, T2, N2 via
> sethdlc (which needs to be patched as well).
> 
> Signed-off-by: Martin Schiller <ms@dev.tdt.de>

I don't know how wise it is to add new ioctls to this old driver.

Also, none of these ioctls even have COMPAT handling so they will
never work from a 32-bit binary running on a 64-bit kernel for
example.

Also:

> +static struct x25_state* state(hdlc_device *hdlc)

It is always "type *func" never "type* func"

>  static int x25_open(struct net_device *dev)
>  {
>  	int result;
> +	hdlc_device *hdlc = dev_to_hdlc(dev);
> +	struct lapb_parms_struct params;
>  	static const struct lapb_register_struct cb = {

Please make this reverse christmas tree ordered.

> @@ -186,6 +217,9 @@ static struct hdlc_proto proto = {
>  
>  static int x25_ioctl(struct net_device *dev, struct ifreq *ifr)
>  {
> +	x25_hdlc_proto __user *x25_s = ifr->ifr_settings.ifs_ifsu.x25;
> +	const size_t size = sizeof(x25_hdlc_proto);
> +	x25_hdlc_proto new_settings;
>  	hdlc_device *hdlc = dev_to_hdlc(dev);
>  	int result;

Likewise.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 1/2] wan/hdlc_x25: make lapb params configurable
  2020-01-15 21:43 ` [PATCH v2 1/2] wan/hdlc_x25: make lapb params configurable David Miller
@ 2020-01-16  6:03   ` Martin Schiller
  0 siblings, 0 replies; 4+ messages in thread
From: Martin Schiller @ 2020-01-16  6:03 UTC (permalink / raw)
  To: David Miller; +Cc: kubakici, khc, linux-x25, netdev, linux-kernel

On 2020-01-15 22:43, David Miller wrote:
> From: Martin Schiller <ms@dev.tdt.de>
> Date: Tue, 14 Jan 2020 15:02:22 +0100
> 
>> This enables you to configure mode (DTE/DCE), Modulo, Window, T1, T2, 
>> N2 via
>> sethdlc (which needs to be patched as well).
>> 
>> Signed-off-by: Martin Schiller <ms@dev.tdt.de>
> 
> I don't know how wise it is to add new ioctls to this old driver.

As an user of this framework I can tell you that you need to be able to
tune this parameters if it's used in an professional environment.

> 
> Also, none of these ioctls even have COMPAT handling so they will
> never work from a 32-bit binary running on a 64-bit kernel for
> example.

How often does this constellation occur in reality? I really have no
idea. Our software is either 32-bit or 64-bit, not a mix.

> 
> Also:
> 
>> +static struct x25_state* state(hdlc_device *hdlc)
> 
> It is always "type *func" never "type* func"

Ok. I will change that. I've copied that from hdlc_fr.c to keep the
same coding style. But you are right:
"Don't look back, always look forward." :)
> 
>>  static int x25_open(struct net_device *dev)
>>  {
>>  	int result;
>> +	hdlc_device *hdlc = dev_to_hdlc(dev);
>> +	struct lapb_parms_struct params;
>>  	static const struct lapb_register_struct cb = {
> 
> Please make this reverse christmas tree ordered.

OK, will do.

> 
>> @@ -186,6 +217,9 @@ static struct hdlc_proto proto = {
>> 
>>  static int x25_ioctl(struct net_device *dev, struct ifreq *ifr)
>>  {
>> +	x25_hdlc_proto __user *x25_s = ifr->ifr_settings.ifs_ifsu.x25;
>> +	const size_t size = sizeof(x25_hdlc_proto);
>> +	x25_hdlc_proto new_settings;
>>  	hdlc_device *hdlc = dev_to_hdlc(dev);
>>  	int result;
> 
> Likewise.

ditto.


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-14 14:02 [PATCH v2 1/2] wan/hdlc_x25: make lapb params configurable Martin Schiller
2020-01-14 14:02 ` [PATCH v2 2/2] wan/hdlc_x25: fix skb handling Martin Schiller
2020-01-15 21:43 ` [PATCH v2 1/2] wan/hdlc_x25: make lapb params configurable David Miller
2020-01-16  6:03   ` Martin Schiller

Netdev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netdev/0 netdev/git/0.git
	git clone --mirror https://lore.kernel.org/netdev/1 netdev/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netdev netdev/ https://lore.kernel.org/netdev \
		netdev@vger.kernel.org
	public-inbox-index netdev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netdev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git