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

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

[Jian-Hong Pan]

Hello,

This series refines the commit 48e5bb6cec79 ("net: Prepare LoRaWAN
socket module") for coding style.
Besides, sperates LoRaWAN related skb definitions from
lora/lorawan_netdev.h into another header lora/lorawan/skb.h.

Thanks,
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/skb.h    | 33 ++++++++++++++
 include/linux/lora/lorawan_netdev.h | 25 +----------
 net/lorawan/Kconfig                 |  3 +-
 net/lorawan/socket.c                | 70 +++++++++++------------------
 4 files changed, 63 insertions(+), 68 deletions(-)
 create mode 100644 include/linux/lora/lorawan/skb.h

-- 
2.20.1

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

[Jian-Hong Pan]

Fix the coding style.

Fixes: 48e5bb6cec79 ("net: Prepare LoRaWAN socket module")
Signed-off-by: Jian-Hong Pan <starnight@g.ncu.edu.tw>
---
v2:
- Modify the commit message
- Order the included header files

 include/linux/lora/lorawan_netdev.h |  5 ++--
 net/lorawan/socket.c                | 45 ++++++++++++++---------------
 2 files changed, 23 insertions(+), 27 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..31a77c3e5ee9 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,
-	    struct packet_type *pt, struct net_device *orig_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 v2 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.

Fixes: 48e5bb6cec79 ("net: Prepare LoRaWAN socket module")
Signed-off-by: Jian-Hong Pan <starnight@g.ncu.edu.tw>
---
v2:
- Modify the commit message

 net/lorawan/socket.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/net/lorawan/socket.c b/net/lorawan/socket.c
index 31a77c3e5ee9..38cee1ff02af 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 v2 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.

Fixes: 48e5bb6cec79 ("net: Prepare LoRaWAN socket module")
Signed-off-by: Jian-Hong Pan <starnight@g.ncu.edu.tw>
---
v2:
- Modify the commit message

 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 38cee1ff02af..c94dc0f3cf82 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 v2 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.

Fixes: 48e5bb6cec79 ("net: Prepare LoRaWAN socket module")
Signed-off-by: Jian-Hong Pan <starnight@g.ncu.edu.tw>
---
v2:
- Modify the commit message
- Fix the help text's space between two sentences

 net/lorawan/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/lorawan/Kconfig b/net/lorawan/Kconfig
index bf6c9b77573b..a55762b1aba0 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 v2 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>
---
v2:
- Modify the commit message
- Move lora/lorawan_netdev.h to lora/lorawan/skb.h

 include/linux/lora/lorawan/skb.h    | 33 +++++++++++++++++++++++++++++
 include/linux/lora/lorawan_netdev.h | 20 -----------------
 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/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/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/net/lorawan/socket.c b/net/lorawan/socket.c
index c94dc0f3cf82..e7cb45bd93d0 100644
--- a/net/lorawan/socket.c
+++ b/net/lorawan/socket.c
@@ -12,6 +12,7 @@
 #include <linux/if_arp.h>
 #include <linux/init.h>
 #include <linux/list.h>
+#include <linux/lora/lorawan/skb.h>
 #include <linux/lora/lorawan_netdev.h>
 #include <linux/module.h>
 #include <linux/net.h>
-- 
2.20.1

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

[Andreas Färber]

Hi Jian-Hong,

Am 31.01.19 um 17:21 schrieb Jian-Hong Pan:
> This series refines the commit 48e5bb6cec79 ("net: Prepare LoRaWAN
> socket module") for coding style.
> Besides, sperates LoRaWAN related skb definitions from
> lora/lorawan_netdev.h into another header lora/lorawan/skb.h.

Sorry for the long delay, I've cleaned up lora-next branch now, moving
unrelated patches to other branches and squashing my cfglora patches.
As your LoRaWAN code remained untouched, they all still applied.

> Jian-Hong Pan (5):
>   net: lorawan: Refine the coding style

This one also included a return value fix, too, but oh well. :)

>   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

Queued on lora-next branch now (for later squashing):
https://git.kernel.org/pub/scm/linux/kernel/git/afaerber/linux-lora.git/

Thanks,
Andreas

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