[RFC PATCH 0/5] net: lorawan: Refine the lorawan protocol module

[RFC PATCH 0/5] net: lorawan: Refine the lorawan protocol module

[Jian-Hong Pan]

Jian-Hong Pan (5):
  net: lorawan: Refine the coding style
  net: lorawan: Remove unused lrw_dev_hard_header function
  net; lorawan: Fix net device leakage
  net: lorawan: Fulfill the help text of Kconfig
  net: lorawan: Split skb definitions into another header

 include/linux/lora/lorawan_netdev.h | 25 +----------
 include/linux/lora/lorawan_skb.h    | 33 ++++++++++++++
 net/lorawan/Kconfig                 |  3 +-
 net/lorawan/socket.c                | 68 +++++++++++------------------
 4 files changed, 62 insertions(+), 67 deletions(-)
 create mode 100644 include/linux/lora/lorawan_skb.h

-- 
2.20.1

[RFC PATCH 1/5] net: lorawan: Refine the coding style

[Jian-Hong Pan]

Signed-off-by: Jian-Hong Pan <starnight@g.ncu.edu.tw>
---
 include/linux/lora/lorawan_netdev.h |  5 ++--
 net/lorawan/socket.c                | 43 ++++++++++++++---------------
 2 files changed, 22 insertions(+), 26 deletions(-)

diff --git a/include/linux/lora/lorawan_netdev.h b/include/linux/lora/lorawan_netdev.h
index 5bffb5164f95..a6c94cb96bf4 100644
--- a/include/linux/lora/lorawan_netdev.h
+++ b/include/linux/lora/lorawan_netdev.h
@@ -1,9 +1,8 @@
 /* SPDX-License-Identifier: GPL-2.0-or-later OR BSD-3-Clause */
-/*-
+/*
  * LoRaWAN stack related definitions
  *
- * Copyright (c) 2018 Jian-Hong, Pan <starnight@g.ncu.edu.tw>
- *
+ * Copyright (c) 2018 Jian-Hong Pan <starnight@g.ncu.edu.tw>
  */
 
 #ifndef __LORAWAN_NET_DEVICE_H__
diff --git a/net/lorawan/socket.c b/net/lorawan/socket.c
index 7ef106b877ca..0ec2d2bf1682 100644
--- a/net/lorawan/socket.c
+++ b/net/lorawan/socket.c
@@ -1,36 +1,33 @@
 // SPDX-License-Identifier: GPL-2.0-or-later OR BSD-3-Clause
-/*-
+/*
  * LoRaWAN stack related definitions
  *
- * Copyright (c) 2018 Jian-Hong, Pan <starnight@g.ncu.edu.tw>
- *
+ * Copyright (c) 2018 Jian-Hong Pan <starnight@g.ncu.edu.tw>
  */
 
 #define	LORAWAN_MODULE_NAME	"lorawan"
 
 #define	pr_fmt(fmt)		LORAWAN_MODULE_NAME ": " fmt
 
+#include <linux/if_arp.h>
 #include <linux/init.h>
-#include <linux/module.h>
 #include <linux/list.h>
+#include <linux/lora/lorawan_netdev.h>
+#include <linux/module.h>
 #include <linux/net.h>
-#include <linux/if_arp.h>
 #include <linux/termios.h>		/* For TIOCOUTQ/INQ */
 #include <net/sock.h>
-#include <linux/lora/lorawan_netdev.h>
 
 /**
  * dgram_sock - This structure holds the states of Datagram socket
  *
  * @sk:			network layer representation of the socket
- *			sk must be the first member of dgram_sock
  * @src_devaddr:	the LoRaWAN device address for this connection
  * @bound:		this socket is bound or not
  * @connected:		this socket is connected to the destination or not
- * @want_ack:		this socket needs to ack for the connection or not
  */
 struct dgram_sock {
-	struct sock sk;
+	struct sock sk;	/* sk must be the first member of dgram_sock */
 	u32 src_devaddr;
 
 	u8 bound:1;
@@ -136,7 +133,7 @@ dgram_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
 	size_t tlen;
 	int ret;
 
-	pr_debug("%s: going to send %zu bytes", __func__, size);
+	pr_debug("%s: going to send %zu bytes\n", __func__, size);
 	if (msg->msg_flags & MSG_OOB) {
 		pr_debug("msg->msg_flags = 0x%x\n", msg->msg_flags);
 		return -EOPNOTSUPP;
@@ -532,7 +529,7 @@ static const struct proto_ops lrw_dgram_ops = {
 };
 
 static int
-lorawan_creat(struct net *net, struct socket *sock, int protocol, int kern)
+lrw_create(struct net *net, struct socket *sock, int protocol, int kern)
 {
 	struct sock *sk;
 	int ret;
@@ -557,7 +554,7 @@ lorawan_creat(struct net *net, struct socket *sock, int protocol, int kern)
 		ret = sk->sk_prot->hash(sk);
 		if (ret) {
 			sk_common_release(sk);
-			goto lorawan_creat_end;
+			goto lrw_create_end;
 		}
 	}
 
@@ -567,14 +564,14 @@ lorawan_creat(struct net *net, struct socket *sock, int protocol, int kern)
 			sk_common_release(sk);
 	}
 
-lorawan_creat_end:
+lrw_create_end:
 	return ret;
 }
 
 static const struct net_proto_family lorawan_family_ops = {
 	.owner		= THIS_MODULE,
 	.family		= PF_LORAWAN,
-	.create		= lorawan_creat,
+	.create		= lrw_create,
 };
 
 static int
@@ -617,29 +614,29 @@ lrw_dgram_deliver(struct net_device *ndev, struct sk_buff *skb)
 }
 
 static int
-lorawan_rcv(struct sk_buff *skb, struct net_device *ndev,
+lrw_rcv(struct sk_buff *skb, struct net_device *ndev,
 	    struct packet_type *pt, struct net_device *orig_ndev)
 {
 	if (!netif_running(ndev))
-		goto lorawan_rcv_drop;
+		goto lrw_rcv_drop;
 
 	if (!net_eq(dev_net(ndev), &init_net))
-		goto lorawan_rcv_drop;
+		goto lrw_rcv_drop;
 
 	if (ndev->type != ARPHRD_LORAWAN)
-		goto lorawan_rcv_drop;
+		goto lrw_rcv_drop;
 
 	if (skb->pkt_type != PACKET_OTHERHOST)
 		return lrw_dgram_deliver(ndev, skb);
 
-lorawan_rcv_drop:
+lrw_rcv_drop:
 	kfree_skb(skb);
 	return NET_RX_DROP;
 }
 
 static struct packet_type lorawan_packet_type = {
 	.type		= htons(ETH_P_LORAWAN),
-	.func		= lorawan_rcv,
+	.func		= lrw_rcv,
 };
 
 static int __init
@@ -665,7 +662,7 @@ lrw_sock_init(void)
 	proto_unregister(&lrw_dgram_prot);
 
 lrw_sock_init_end:
-	return 0;
+	return ret;
 }
 
 static void __exit
@@ -680,7 +677,7 @@ lrw_sock_exit(void)
 module_init(lrw_sock_init);
 module_exit(lrw_sock_exit);
 
-MODULE_AUTHOR("Jian-Hong Pan, <starnight@g.ncu.edu.tw>");
-MODULE_DESCRIPTION("LoRaWAN socket kernel module");
+MODULE_AUTHOR("Jian-Hong Pan <starnight@g.ncu.edu.tw>");
+MODULE_DESCRIPTION("LoRaWAN socket protocol");
 MODULE_LICENSE("Dual BSD/GPL");
 MODULE_ALIAS_NETPROTO(PF_LORAWAN);
-- 
2.20.1

[RFC PATCH 2/5] net: lorawan: Remove unused lrw_dev_hard_header function

[Jian-Hong Pan]

The lorawan module is an abastraction layer over the LoRaWAN soft and
hard MAC.  It passes the original buffer to the real MAC layer.  So,
this patch removes the lrw_dev_hard_header function.

Signed-off-by: Jian-Hong Pan <starnight@g.ncu.edu.tw>
---
 net/lorawan/socket.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/net/lorawan/socket.c b/net/lorawan/socket.c
index 0ec2d2bf1682..9c0722379e25 100644
--- a/net/lorawan/socket.c
+++ b/net/lorawan/socket.c
@@ -115,14 +115,6 @@ dgram_bind(struct sock *sk, struct sockaddr *uaddr, int len)
 	return ret;
 }
 
-static int
-lrw_dev_hard_header(struct sk_buff *skb, struct net_device *ndev,
-		    const u32 src_devaddr, size_t len)
-{
-	/* TODO: Prepare the LoRaWAN sending header here */
-	return 0;
-}
-
 static int
 dgram_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
 {
@@ -176,10 +168,6 @@ dgram_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
 	skb_reserve(skb, hlen);
 	skb_reset_network_header(skb);
 
-	ret = lrw_dev_hard_header(skb, ndev, 0, size);
-	if (ret < 0)
-		goto dgram_sendmsg_no_skb;
-
 	ret = memcpy_from_msg(skb_put(skb, size), msg, size);
 	if (ret > 0)
 		goto dgram_sendmsg_err_skb;
-- 
2.20.1

[RFC PATCH 3/5] net; lorawan: Fix net device leakage

[Jian-Hong Pan]

The net device may be missed to be put after error check.  This patch
fixes the issue to prevent the leakage.

Signed-off-by: Jian-Hong Pan <starnight@g.ncu.edu.tw>
---
 net/lorawan/socket.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/net/lorawan/socket.c b/net/lorawan/socket.c
index 9c0722379e25..7139fab63159 100644
--- a/net/lorawan/socket.c
+++ b/net/lorawan/socket.c
@@ -51,8 +51,10 @@ lrw_get_dev_by_addr(struct net *net, u32 devaddr)
 
 	rcu_read_lock();
 	ndev = dev_getbyhwaddr_rcu(net, ARPHRD_LORAWAN, (char *)&be_addr);
-	if (ndev)
+	if (ndev && ndev->type == ARPHRD_LORAWAN)
 		dev_hold(ndev);
+	else
+		ndev = NULL;
 	rcu_read_unlock();
 
 	return ndev;
@@ -99,11 +101,6 @@ dgram_bind(struct sock *sk, struct sockaddr *uaddr, int len)
 	}
 	netdev_dbg(ndev, "%s: get ndev\n", __func__);
 
-	if (ndev->type != ARPHRD_LORAWAN) {
-		ret = -ENODEV;
-		goto dgram_bind_end;
-	}
-
 	ro->src_devaddr = addr->addr_in.devaddr;
 	ro->bound = 1;
 	ret = 0;
@@ -152,7 +149,7 @@ dgram_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
 	if (size > ndev->mtu) {
 		netdev_dbg(ndev, "size = %zu, mtu = %u\n", size, ndev->mtu);
 		ret = -EMSGSIZE;
-		goto dgram_sendmsg_end;
+		goto dgram_sendmsg_no_skb;
 	}
 
 	netdev_dbg(ndev, "%s: create skb\n", __func__);
@@ -189,7 +186,6 @@ dgram_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
 	kfree_skb(skb);
 dgram_sendmsg_no_skb:
 	dev_put(ndev);
-
 dgram_sendmsg_end:
 	return ret;
 }
-- 
2.20.1

[RFC PATCH 4/5] net: lorawan: Fulfill the help text of Kconfig

[Jian-Hong Pan]

Mention the LoRaWAN network feature to distinguish it from other
Low-Power Wide-Area Network like Sigfox and NB-IoT.

Signed-off-by: Jian-Hong Pan <starnight@g.ncu.edu.tw>
---
 net/lorawan/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/lorawan/Kconfig b/net/lorawan/Kconfig
index bf6c9b77573b..ce3ed6e6d11c 100644
--- a/net/lorawan/Kconfig
+++ b/net/lorawan/Kconfig
@@ -4,7 +4,8 @@ config LORAWAN
 	  LoRaWAN defines low data rate, low power and long range wireless
 	  wide area networks. It was designed to organize networks of automation
 	  devices, such as sensors, switches and actuators. It can operate
-	  multiple kilometers wide.
+	  multiple kilometers wide.  The network is client/server technology
+	  centered around gateways.
 
 	  Say Y here to compile LoRaWAN support into the kernel or say M to
 	  compile it as a module.
-- 
2.20.1

[RFC PATCH 5/5] net: lorawan: Split skb definitions into another header

[Jian-Hong Pan]

Split LoRaWAN related skb definitions from lora/lorawan_netdev.h into
another header lora/lorawan_skb.h.

Signed-off-by: Jian-Hong Pan <starnight@g.ncu.edu.tw>
---
 include/linux/lora/lorawan_netdev.h | 20 -----------------
 include/linux/lora/lorawan_skb.h    | 33 +++++++++++++++++++++++++++++
 net/lorawan/socket.c                |  1 +
 3 files changed, 34 insertions(+), 20 deletions(-)
 create mode 100644 include/linux/lora/lorawan_skb.h

diff --git a/include/linux/lora/lorawan_netdev.h b/include/linux/lora/lorawan_netdev.h
index a6c94cb96bf4..e515ba1ab508 100644
--- a/include/linux/lora/lorawan_netdev.h
+++ b/include/linux/lora/lorawan_netdev.h
@@ -28,24 +28,4 @@ struct sockaddr_lorawan {
 	struct lrw_addr_in addr_in;
 };
 
-/**
- * lrw_mac_cb - This structure holds the control buffer (cb) of sk_buff
- *
- * @devaddr:	the LoRaWAN device address of this LoRaWAN hardware
- */
-struct lrw_mac_cb {
-	u32 devaddr;
-};
-
-/**
- * lrw_get_mac_cb - Get the LoRaWAN MAC control buffer of the sk_buff
- * @skb:	the exchanging sk_buff
- *
- * Return:	the pointer of LoRaWAN MAC control buffer
- */
-static inline struct lrw_mac_cb *lrw_get_mac_cb(struct sk_buff *skb)
-{
-	return (struct lrw_mac_cb *)skb->cb;
-}
-
 #endif
diff --git a/include/linux/lora/lorawan_skb.h b/include/linux/lora/lorawan_skb.h
new file mode 100644
index 000000000000..ea4f900b37be
--- /dev/null
+++ b/include/linux/lora/lorawan_skb.h
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later OR BSD-3-Clause */
+/*
+ * LoRaWAN socket buffer related definitions
+ *
+ * Copyright (c) 2018 Jian-Hong Pan <starnight@g.ncu.edu.tw>
+ */
+
+#ifndef __LORAWAN_SKB_H__
+#define __LORAWAN_SKB_H__
+
+#include <linux/skbuff.h>
+
+/**
+ * lrw_mac_cb - This structure holds the control buffer (cb) of sk_buff
+ *
+ * @devaddr:	the LoRaWAN device address of this LoRaWAN hardware
+ */
+struct lrw_mac_cb {
+	u32 devaddr;
+};
+
+/**
+ * lrw_get_mac_cb - Get the LoRaWAN MAC control buffer of the sk_buff
+ * @skb:	the exchanging sk_buff
+ *
+ * Return:	the pointer of LoRaWAN MAC control buffer
+ */
+static inline struct lrw_mac_cb *lrw_get_mac_cb(struct sk_buff *skb)
+{
+	return (struct lrw_mac_cb *)skb->cb;
+}
+
+#endif
diff --git a/net/lorawan/socket.c b/net/lorawan/socket.c
index 7139fab63159..50559ba0c538 100644
--- a/net/lorawan/socket.c
+++ b/net/lorawan/socket.c
@@ -13,6 +13,7 @@
 #include <linux/init.h>
 #include <linux/list.h>
 #include <linux/lora/lorawan_netdev.h>
+#include <linux/lora/lorawan_skb.h>
 #include <linux/module.h>
 #include <linux/net.h>
 #include <linux/termios.h>		/* For TIOCOUTQ/INQ */
-- 
2.20.1

[RFC PATCH 5/5] net: lorawan: Split skb definitions into another header

[Jian-Hong Pan]

Split LoRaWAN related skb definitions from lora/lorawan_netdev.h into
another header lora/lorawan_skb.h.

Signed-off-by: Jian-Hong Pan <starnight@g.ncu.edu.tw>
---
 include/linux/lora/lorawan_netdev.h | 20 -----------------
 include/linux/lora/lorawan_skb.h    | 33 +++++++++++++++++++++++++++++
 net/lorawan/socket.c                |  1 +
 3 files changed, 34 insertions(+), 20 deletions(-)
 create mode 100644 include/linux/lora/lorawan_skb.h

diff --git a/include/linux/lora/lorawan_netdev.h b/include/linux/lora/lorawan_netdev.h
index a6c94cb96bf4..e515ba1ab508 100644
--- a/include/linux/lora/lorawan_netdev.h
+++ b/include/linux/lora/lorawan_netdev.h
@@ -28,24 +28,4 @@ struct sockaddr_lorawan {
 	struct lrw_addr_in addr_in;
 };
 
-/**
- * lrw_mac_cb - This structure holds the control buffer (cb) of sk_buff
- *
- * @devaddr:	the LoRaWAN device address of this LoRaWAN hardware
- */
-struct lrw_mac_cb {
-	u32 devaddr;
-};
-
-/**
- * lrw_get_mac_cb - Get the LoRaWAN MAC control buffer of the sk_buff
- * @skb:	the exchanging sk_buff
- *
- * Return:	the pointer of LoRaWAN MAC control buffer
- */
-static inline struct lrw_mac_cb *lrw_get_mac_cb(struct sk_buff *skb)
-{
-	return (struct lrw_mac_cb *)skb->cb;
-}
-
 #endif
diff --git a/include/linux/lora/lorawan_skb.h b/include/linux/lora/lorawan_skb.h
new file mode 100644
index 000000000000..ea4f900b37be
--- /dev/null
+++ b/include/linux/lora/lorawan_skb.h
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later OR BSD-3-Clause */
+/*
+ * LoRaWAN socket buffer related definitions
+ *
+ * Copyright (c) 2018 Jian-Hong Pan <starnight@g.ncu.edu.tw>
+ */
+
+#ifndef __LORAWAN_SKB_H__
+#define __LORAWAN_SKB_H__
+
+#include <linux/skbuff.h>
+
+/**
+ * lrw_mac_cb - This structure holds the control buffer (cb) of sk_buff
+ *
+ * @devaddr:	the LoRaWAN device address of this LoRaWAN hardware
+ */
+struct lrw_mac_cb {
+	u32 devaddr;
+};
+
+/**
+ * lrw_get_mac_cb - Get the LoRaWAN MAC control buffer of the sk_buff
+ * @skb:	the exchanging sk_buff
+ *
+ * Return:	the pointer of LoRaWAN MAC control buffer
+ */
+static inline struct lrw_mac_cb *lrw_get_mac_cb(struct sk_buff *skb)
+{
+	return (struct lrw_mac_cb *)skb->cb;
+}
+
+#endif
diff --git a/net/lorawan/socket.c b/net/lorawan/socket.c
index 7139fab63159..50559ba0c538 100644
--- a/net/lorawan/socket.c
+++ b/net/lorawan/socket.c
@@ -13,6 +13,7 @@
 #include <linux/init.h>
 #include <linux/list.h>
 #include <linux/lora/lorawan_netdev.h>
+#include <linux/lora/lorawan_skb.h>
 #include <linux/module.h>
 #include <linux/net.h>
 #include <linux/termios.h>		/* For TIOCOUTQ/INQ */
-- 
2.20.1

Re: [RFC PATCH 1/5] net: lorawan: Refine the coding style

[Jiri Pirko]

Wed, Jan 16, 2019 at 03:24:54PM CET, starnight@g.ncu.edu.tw wrote:
>Signed-off-by: Jian-Hong Pan <starnight@g.ncu.edu.tw>
>---

Patches like this are in general frowned upon. Do one change in one
patch. Put some patch description.

Re: [RFC PATCH 3/5] net; lorawan: Fix net device leakage

[Jiri Pirko]

Wed, Jan 16, 2019 at 03:24:58PM CET, starnight@g.ncu.edu.tw wrote:
>The net device may be missed to be put after error check.  This patch
>fixes the issue to prevent the leakage.
>
>Signed-off-by: Jian-Hong Pan <starnight@g.ncu.edu.tw>
>---
> net/lorawan/socket.c | 12 ++++--------
> 1 file changed, 4 insertions(+), 8 deletions(-)
>
>diff --git a/net/lorawan/socket.c b/net/lorawan/socket.c
>index 9c0722379e25..7139fab63159 100644
>--- a/net/lorawan/socket.c
>+++ b/net/lorawan/socket.c
>@@ -51,8 +51,10 @@ lrw_get_dev_by_addr(struct net *net, u32 devaddr)
> 
> 	rcu_read_lock();
> 	ndev = dev_getbyhwaddr_rcu(net, ARPHRD_LORAWAN, (char *)&be_addr);
>-	if (ndev)
>+	if (ndev && ndev->type == ARPHRD_LORAWAN)
> 		dev_hold(ndev);
>+	else
>+		ndev = NULL;
> 	rcu_read_unlock();
> 
> 	return ndev;
>@@ -99,11 +101,6 @@ dgram_bind(struct sock *sk, struct sockaddr *uaddr, int len)
> 	}
> 	netdev_dbg(ndev, "%s: get ndev\n", __func__);
> 
>-	if (ndev->type != ARPHRD_LORAWAN) {
>-		ret = -ENODEV;
>-		goto dgram_bind_end;
>-	}
>-
> 	ro->src_devaddr = addr->addr_in.devaddr;
> 	ro->bound = 1;
> 	ret = 0;
>@@ -152,7 +149,7 @@ dgram_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
> 	if (size > ndev->mtu) {
> 		netdev_dbg(ndev, "size = %zu, mtu = %u\n", size, ndev->mtu);
> 		ret = -EMSGSIZE;
>-		goto dgram_sendmsg_end;
>+		goto dgram_sendmsg_no_skb;
> 	}
> 
> 	netdev_dbg(ndev, "%s: create skb\n", __func__);
>@@ -189,7 +186,6 @@ dgram_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
> 	kfree_skb(skb);
> dgram_sendmsg_no_skb:
> 	dev_put(ndev);
>-

Please avoid hunks like this one.


> dgram_sendmsg_end:
> 	return ret;
> }
>-- 
>2.20.1
>
>
>_______________________________________________
>linux-lpwan mailing list
>linux-lpwan@lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/linux-lpwan

Re: [RFC PATCH 4/5] net: lorawan: Fulfill the help text of Kconfig

[Jiri Pirko]

Wed, Jan 16, 2019 at 03:25:00PM CET, starnight@g.ncu.edu.tw wrote:
>Mention the LoRaWAN network feature to distinguish it from other
>Low-Power Wide-Area Network like Sigfox and NB-IoT.
>
>Signed-off-by: Jian-Hong Pan <starnight@g.ncu.edu.tw>
>---
> net/lorawan/Kconfig | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/net/lorawan/Kconfig b/net/lorawan/Kconfig
>index bf6c9b77573b..ce3ed6e6d11c 100644
>--- a/net/lorawan/Kconfig
>+++ b/net/lorawan/Kconfig
>@@ -4,7 +4,8 @@ config LORAWAN
> 	  LoRaWAN defines low data rate, low power and long range wireless
> 	  wide area networks. It was designed to organize networks of automation
> 	  devices, such as sensors, switches and actuators. It can operate
>-	  multiple kilometers wide.
>+	  multiple kilometers wide.  The network is client/server technology

Please avoid double spaces.


>+	  centered around gateways.
> 
> 	  Say Y here to compile LoRaWAN support into the kernel or say M to
> 	  compile it as a module.
>-- 
>2.20.1
>
>
>_______________________________________________
>linux-lpwan mailing list
>linux-lpwan@lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/linux-lpwan

Re: [RFC PATCH 5/5] net: lorawan: Split skb definitions into another header

[Jiri Pirko]

Wed, Jan 16, 2019 at 03:25:02PM CET, starnight@g.ncu.edu.tw wrote:
>Split LoRaWAN related skb definitions from lora/lorawan_netdev.h into
>another header lora/lorawan_skb.h.

What is the motivation for this change?

Re: [RFC PATCH 5/5] net: lorawan: Split skb definitions into another header

[Andreas Färber]

Am 16.01.19 um 15:36 schrieb Jiri Pirko:
> Wed, Jan 16, 2019 at 03:25:02PM CET, starnight@g.ncu.edu.tw wrote:
>> Split LoRaWAN related skb definitions from lora/lorawan_netdev.h into
>> another header lora/lorawan_skb.h.
> 
> What is the motivation for this change?

I suggested it because skbs could be used with either LoRa or LoRaWAN
netdevs. I have a lora/skb.h.

In general the lorawan_foo.h looks ugly to me, so I thought I suggested
to avoid that by using [lora/]lorawan/skb.h?

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

Re: [RFC PATCH 1/5] net: lorawan: Refine the coding style

[Andreas Färber]

Am 16.01.19 um 15:33 schrieb Jiri Pirko:
> Wed, Jan 16, 2019 at 03:24:54PM CET, starnight@g.ncu.edu.tw wrote:
>> Signed-off-by: Jian-Hong Pan <starnight@g.ncu.edu.tw>
>> ---
> 
> Patches like this are in general frowned upon. Do one change in one
> patch. Put some patch description.

This patch simply shouldn't have gone to netdev and LKML, as it is a
fixup against a non-stable tree (missing "lora-next"). Once squashed, it
doesn't matter whether it once had a verbose explanation or not. And I
prefer to not mix style and functional changes.

I simply was too impatient to wait for more respins before being able to
base work on it. ;)

Cheers,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

Re: [RFC PATCH 1/5] net: lorawan: Refine the coding style

[Jian-Hong Pan]

Andreas Färber <afaerber@suse.de> 於 2019年1月16日 週三 下午11:07寫道：
>
> Am 16.01.19 um 15:33 schrieb Jiri Pirko:
> > Wed, Jan 16, 2019 at 03:24:54PM CET, starnight@g.ncu.edu.tw wrote:
> >> Signed-off-by: Jian-Hong Pan <starnight@g.ncu.edu.tw>
> >> ---
> >
> > Patches like this are in general frowned upon. Do one change in one
> > patch. Put some patch description.
>
> This patch simply shouldn't have gone to netdev and LKML, as it is a
> fixup against a non-stable tree (missing "lora-next"). Once squashed, it
> doesn't matter whether it once had a verbose explanation or not. And I
> prefer to not mix style and functional changes.

Uh!  I see.

Regards,
Jian-Hong Pan

> I simply was too impatient to wait for more respins before being able to
> base work on it. ;)

Re: [RFC PATCH 5/5] net: lorawan: Split skb definitions into another header

[Jian-Hong Pan]

Andreas Färber <afaerber@suse.de> 於 2019年1月16日 週三 下午10:50寫道：
>
> Am 16.01.19 um 15:36 schrieb Jiri Pirko:
> > Wed, Jan 16, 2019 at 03:25:02PM CET, starnight@g.ncu.edu.tw wrote:
> >> Split LoRaWAN related skb definitions from lora/lorawan_netdev.h into
> >> another header lora/lorawan_skb.h.
> >
> > What is the motivation for this change?
>
> I suggested it because skbs could be used with either LoRa or LoRaWAN
> netdevs. I have a lora/skb.h.
>
> In general the lorawan_foo.h looks ugly to me, so I thought I suggested
> to avoid that by using [lora/]lorawan/skb.h?

Seems we have some choices:
1. Split the lorawan skb stuff from lora/lorawan_netdev.h and merge
into include/linux/lora/skb.h
2. Split the lorawan skb stuff from lora/lorawan_netdev.h to
include/linux/lora/lorawan_skb.h
3. Split the lorawan skb stuff from lora/lorawan_netdev.h to
include/linux/lora/lorawan/skb.h
4. Split the lorawan skb stuff from lora/lorawan_netdev.h to
include/linux/lorawan/skb.h

#1, #2 and #3 are good to me.
So, the intersection is choice #3.

Regards,
Jian-Hong Pan