2019-01-16 22:13:34

by Jian-Hong Pan

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

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



2019-01-16 22:14:07

by Jian-Hong Pan

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

Signed-off-by: Jian-Hong Pan <[email protected]>
---
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 <[email protected]>
- *
+ * Copyright (c) 2018 Jian-Hong Pan <[email protected]>
*/

#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 <[email protected]>
- *
+ * Copyright (c) 2018 Jian-Hong Pan <[email protected]>
*/

#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, <[email protected]>");
-MODULE_DESCRIPTION("LoRaWAN socket kernel module");
+MODULE_AUTHOR("Jian-Hong Pan <[email protected]>");
+MODULE_DESCRIPTION("LoRaWAN socket protocol");
MODULE_LICENSE("Dual BSD/GPL");
MODULE_ALIAS_NETPROTO(PF_LORAWAN);
--
2.20.1


2019-01-16 22:14:35

by Jian-Hong Pan

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

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 <[email protected]>
---
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


2019-01-16 22:14:52

by Jian-Hong Pan

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

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 <[email protected]>
---
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


2019-01-16 22:16:03

by Jian-Hong Pan

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

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 <[email protected]>
---
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


2019-01-16 22:20:35

by Jiri Pirko

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

Wed, Jan 16, 2019 at 03:24:54PM CET, [email protected] wrote:
>Signed-off-by: Jian-Hong Pan <[email protected]>
>---

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

2019-01-16 22:22:24

by Jiri Pirko

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

Wed, Jan 16, 2019 at 03:25:00PM CET, [email protected] 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 <[email protected]>
>---
> 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
>[email protected]
>http://lists.infradead.org/mailman/listinfo/linux-lpwan

2019-01-16 22:24:10

by Jiri Pirko

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

Wed, Jan 16, 2019 at 03:25:02PM CET, [email protected] 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?

2019-01-16 22:25:52

by Andreas Färber

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

Am 16.01.19 um 15:36 schrieb Jiri Pirko:
> Wed, Jan 16, 2019 at 03:25:02PM CET, [email protected] 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)

2019-01-16 22:54:30

by Jian-Hong Pan

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

Andreas Färber <[email protected]> 於 2019年1月16日 週三 下午11:07寫道:
>
> Am 16.01.19 um 15:33 schrieb Jiri Pirko:
> > Wed, Jan 16, 2019 at 03:24:54PM CET, [email protected] wrote:
> >> Signed-off-by: Jian-Hong Pan <[email protected]>
> >> ---
> >
> > 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. ;)

2019-01-17 04:38:31

by Jian-Hong Pan

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

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

Signed-off-by: Jian-Hong Pan <[email protected]>
---
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 <[email protected]>
+ */
+
+#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


2019-01-17 05:16:11

by Andreas Färber

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

Am 16.01.19 um 15:33 schrieb Jiri Pirko:
> Wed, Jan 16, 2019 at 03:24:54PM CET, [email protected] wrote:
>> Signed-off-by: Jian-Hong Pan <[email protected]>
>> ---
>
> 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)

2019-01-17 05:37:50

by Jian-Hong Pan

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

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

Signed-off-by: Jian-Hong Pan <[email protected]>
---
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 <[email protected]>
+ */
+
+#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


2019-01-17 05:41:22

by Jiri Pirko

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

Wed, Jan 16, 2019 at 03:24:58PM CET, [email protected] 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 <[email protected]>
>---
> 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
>[email protected]
>http://lists.infradead.org/mailman/listinfo/linux-lpwan

2019-01-17 07:29:24

by Jian-Hong Pan

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

Andreas Färber <[email protected]> 於 2019年1月16日 週三 下午10:50寫道:
>
> Am 16.01.19 um 15:36 schrieb Jiri Pirko:
> > Wed, Jan 16, 2019 at 03:25:02PM CET, [email protected] 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