All of lore.kernel.org
 help / color / mirror / Atom feed
* [patch net-next v2] nlmon: use standard rtnetlink link api for add/del devices
@ 2013-07-02  8:55 Jiri Pirko
  2013-07-02 19:47 ` Daniel Borkmann
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Jiri Pirko @ 2013-07-02  8:55 UTC (permalink / raw)
  To: netdev; +Cc: davem, dborkman

It is not nice when netdev is created right after module load and with
some implicit name. So rather change nlmon to use standard rtnl link API.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>

---
v1->v2: added forgotten nt.dev initialization

 drivers/net/nlmon.c | 49 ++++++++++++++++++++++++++++---------------------
 1 file changed, 28 insertions(+), 21 deletions(-)

diff --git a/drivers/net/nlmon.c b/drivers/net/nlmon.c
index a0baf56..b57ce5f 100644
--- a/drivers/net/nlmon.c
+++ b/drivers/net/nlmon.c
@@ -4,6 +4,7 @@
 #include <linux/netlink.h>
 #include <net/net_namespace.h>
 #include <linux/if_arp.h>
+#include <net/rtnetlink.h>
 
 struct pcpu_lstats {
 	u64 packets;
@@ -56,16 +57,24 @@ static void nlmon_dev_uninit(struct net_device *dev)
 	free_percpu(dev->lstats);
 }
 
-static struct netlink_tap nlmon_tap;
+struct nlmon {
+	struct netlink_tap nt;
+};
 
 static int nlmon_open(struct net_device *dev)
 {
-	return netlink_add_tap(&nlmon_tap);
+	struct nlmon *nlmon = netdev_priv(dev);
+
+	nlmon->nt.dev = dev;
+	nlmon->nt.module = THIS_MODULE;
+	return netlink_add_tap(&nlmon->nt);
 }
 
 static int nlmon_close(struct net_device *dev)
 {
-	return netlink_remove_tap(&nlmon_tap);
+	struct nlmon *nlmon = netdev_priv(dev);
+
+	return netlink_remove_tap(&nlmon->nt);
 }
 
 static struct rtnl_link_stats64 *
@@ -119,10 +128,6 @@ static const struct net_device_ops nlmon_ops = {
 	.ndo_change_mtu = nlmon_change_mtu,
 };
 
-static struct netlink_tap nlmon_tap __read_mostly = {
-	.module = THIS_MODULE,
-};
-
 static void nlmon_setup(struct net_device *dev)
 {
 	dev->type = ARPHRD_NETLINK;
@@ -142,27 +147,28 @@ static void nlmon_setup(struct net_device *dev)
 	dev->mtu = NLMSG_GOODSIZE;
 }
 
-static __init int nlmon_register(void)
+static int nlmon_validate(struct nlattr *tb[], struct nlattr *data[])
 {
-	int err;
-	struct net_device *nldev;
-
-	nldev = nlmon_tap.dev = alloc_netdev(0, "netlink", nlmon_setup);
-	if (unlikely(nldev == NULL))
-		return -ENOMEM;
+	if (tb[IFLA_ADDRESS])
+		return -EINVAL;
+	return 0;
+}
 
-	err = register_netdev(nldev);
-	if (unlikely(err))
-		free_netdev(nldev);
+static struct rtnl_link_ops nlmon_link_ops __read_mostly = {
+	.kind			= "nlmon",
+	.priv_size		= sizeof(struct nlmon),
+	.setup			= nlmon_setup,
+	.validate		= nlmon_validate,
+};
 
-	return err;
+static __init int nlmon_register(void)
+{
+	return rtnl_link_register(&nlmon_link_ops);
 }
 
 static __exit void nlmon_unregister(void)
 {
-	struct net_device *nldev = nlmon_tap.dev;
-
-	unregister_netdev(nldev);
+	rtnl_link_unregister(&nlmon_link_ops);
 }
 
 module_init(nlmon_register);
@@ -172,3 +178,4 @@ MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR("Daniel Borkmann <dborkman@redhat.com>");
 MODULE_AUTHOR("Mathieu Geli <geli@enseirb.fr>");
 MODULE_DESCRIPTION("Netlink monitoring device");
+MODULE_ALIAS_RTNL_LINK("nlmon");
-- 
1.8.1.4

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

* Re: [patch net-next v2] nlmon: use standard rtnetlink link api for add/del devices
  2013-07-02  8:55 [patch net-next v2] nlmon: use standard rtnetlink link api for add/del devices Jiri Pirko
@ 2013-07-02 19:47 ` Daniel Borkmann
  2013-07-02 19:53 ` David Miller
  2013-07-16 14:08 ` Vladimir Kondratiev
  2 siblings, 0 replies; 8+ messages in thread
From: Daniel Borkmann @ 2013-07-02 19:47 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev, davem

On 07/02/2013 10:55 AM, Jiri Pirko wrote:
> It is not nice when netdev is created right after module load and with
> some implicit name. So rather change nlmon to use standard rtnl link API.
>
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>

Acked-by: Daniel Borkmann <dborkman@redhat.com>

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

* Re: [patch net-next v2] nlmon: use standard rtnetlink link api for add/del devices
  2013-07-02  8:55 [patch net-next v2] nlmon: use standard rtnetlink link api for add/del devices Jiri Pirko
  2013-07-02 19:47 ` Daniel Borkmann
@ 2013-07-02 19:53 ` David Miller
  2013-07-16 14:08 ` Vladimir Kondratiev
  2 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2013-07-02 19:53 UTC (permalink / raw)
  To: jiri; +Cc: netdev, dborkman

From: Jiri Pirko <jiri@resnulli.us>
Date: Tue,  2 Jul 2013 10:55:31 +0200

> It is not nice when netdev is created right after module load and with
> some implicit name. So rather change nlmon to use standard rtnl link API.
> 
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>

Applied, thanks Jiri.

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

* Re: [patch net-next v2] nlmon: use standard rtnetlink link api for add/del devices
  2013-07-02  8:55 [patch net-next v2] nlmon: use standard rtnetlink link api for add/del devices Jiri Pirko
  2013-07-02 19:47 ` Daniel Borkmann
  2013-07-02 19:53 ` David Miller
@ 2013-07-16 14:08 ` Vladimir Kondratiev
  2013-07-16 14:49   ` Daniel Borkmann
  2 siblings, 1 reply; 8+ messages in thread
From: Vladimir Kondratiev @ 2013-07-16 14:08 UTC (permalink / raw)
  To: netdev

Jiri Pirko <jiri <at> resnulli.us> writes:

> 
> It is not nice when netdev is created right after module load and with
> some implicit name. So rather change nlmon to use standard rtnl link API.
> 

Could you please elaborate a bit - how to capture netlink skb's after your
patch? Before, it was netdev that may be used with tcpdump. Now, there is
no such netdev. How to create it?

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

* Re: [patch net-next v2] nlmon: use standard rtnetlink link api for add/del devices
  2013-07-16 14:08 ` Vladimir Kondratiev
@ 2013-07-16 14:49   ` Daniel Borkmann
  2013-07-16 15:59     ` Vladimir Kondratiev
  0 siblings, 1 reply; 8+ messages in thread
From: Daniel Borkmann @ 2013-07-16 14:49 UTC (permalink / raw)
  To: Vladimir Kondratiev; +Cc: netdev, Jiri Pirko

On 07/16/2013 04:08 PM, Vladimir Kondratiev wrote:
> Jiri Pirko <jiri <at> resnulli.us> writes:
>
>> It is not nice when netdev is created right after module load and with
>> some implicit name. So rather change nlmon to use standard rtnl link API.
>
> Could you please elaborate a bit - how to capture netlink skb's after your
> patch? Before, it was netdev that may be used with tcpdump. Now, there is
> no such netdev. How to create it?

modprobe nlmon
ip link add type nlmon
ip link set nlmon0 up

tcpdump -i nlmon0 ....

ip link set nlmon0 down
ip link del dev nlmon0
rmmod nlmon

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

* Re: [patch net-next v2] nlmon: use standard rtnetlink link api for add/del devices
  2013-07-16 14:49   ` Daniel Borkmann
@ 2013-07-16 15:59     ` Vladimir Kondratiev
  2013-07-16 16:10       ` Daniel Borkmann
  0 siblings, 1 reply; 8+ messages in thread
From: Vladimir Kondratiev @ 2013-07-16 15:59 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: netdev, Jiri Pirko

On Tuesday, July 16, 2013 04:49:53 PM Daniel Borkmann wrote:
> On 07/16/2013 04:08 PM, Vladimir Kondratiev wrote:
> > Jiri Pirko <jiri <at> resnulli.us> writes:
> >
> >> It is not nice when netdev is created right after module load and with
> >> some implicit name. So rather change nlmon to use standard rtnl link API.
> >
> > Could you please elaborate a bit - how to capture netlink skb's after your
> > patch? Before, it was netdev that may be used with tcpdump. Now, there is
> > no such netdev. How to create it?
> 
> modprobe nlmon
> ip link add type nlmon
> ip link set nlmon0 up
> 
> tcpdump -i nlmon0 ....
> 
> ip link set nlmon0 down
> ip link del dev nlmon0
> rmmod nlmon

Thanks a lot! I guess it is worth to have this mentioned somewhere.
It will save lots of questions. For example, in Kconfig:

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index b45b240..13acea2 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -247,8 +247,18 @@ config NLMON
          purpose of this is to analyze netlink messages with packet sockets.
          Thus applications like tcpdump will be able to see local netlink
          messages if they tap into the netlink device, record pcaps for further
-         diagnostics, etc. This is mostly intended for developers or support
-         to debug netlink issues. If unsure, say N.
+         diagnostics, etc. Typical flow is:
+
+           modprobe nlmon
+           ip link add type nlmon
+           ip link set nlmon0 up
+           tcpdump -i nlmon0 ....
+           ip link set nlmon0 down
+           ip link del dev nlmon0
+           rmmod nlmon
+
+         This is mostly intended for developers or support to debug netlink
+         issues. If unsure, say N.
 
 endif # NET_CORE

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

* Re: [patch net-next v2] nlmon: use standard rtnetlink link api for add/del devices
  2013-07-16 15:59     ` Vladimir Kondratiev
@ 2013-07-16 16:10       ` Daniel Borkmann
  2013-07-16 16:41         ` Jiri Pirko
  0 siblings, 1 reply; 8+ messages in thread
From: Daniel Borkmann @ 2013-07-16 16:10 UTC (permalink / raw)
  To: Vladimir Kondratiev; +Cc: netdev, Jiri Pirko

On 07/16/2013 05:59 PM, Vladimir Kondratiev wrote:
> On Tuesday, July 16, 2013 04:49:53 PM Daniel Borkmann wrote:
>> On 07/16/2013 04:08 PM, Vladimir Kondratiev wrote:
>>> Jiri Pirko <jiri <at> resnulli.us> writes:
>>>
>>>> It is not nice when netdev is created right after module load and with
>>>> some implicit name. So rather change nlmon to use standard rtnl link API.
>>>
>>> Could you please elaborate a bit - how to capture netlink skb's after your
>>> patch? Before, it was netdev that may be used with tcpdump. Now, there is
>>> no such netdev. How to create it?
>>
>> modprobe nlmon
>> ip link add type nlmon
>> ip link set nlmon0 up
>>
>> tcpdump -i nlmon0 ....
>>
>> ip link set nlmon0 down
>> ip link del dev nlmon0
>> rmmod nlmon
>
> Thanks a lot! I guess it is worth to have this mentioned somewhere.
> It will save lots of questions. For example, in Kconfig:

Ok, I don't have a strong opinion on the below patch, I'm fine either way.

But, given that for such devices it is *common* to use the standard rtnl
link API (Jiri is right that this is a better/cleaner approach), I'm not
sure if this needs to be documented then ...

> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> index b45b240..13acea2 100644
> --- a/drivers/net/Kconfig
> +++ b/drivers/net/Kconfig
> @@ -247,8 +247,18 @@ config NLMON
>            purpose of this is to analyze netlink messages with packet sockets.
>            Thus applications like tcpdump will be able to see local netlink
>            messages if they tap into the netlink device, record pcaps for further
> -         diagnostics, etc. This is mostly intended for developers or support
> -         to debug netlink issues. If unsure, say N.
> +         diagnostics, etc. Typical flow is:
> +
> +           modprobe nlmon
> +           ip link add type nlmon
> +           ip link set nlmon0 up
> +           tcpdump -i nlmon0 ....
> +           ip link set nlmon0 down
> +           ip link del dev nlmon0
> +           rmmod nlmon
> +
> +         This is mostly intended for developers or support to debug netlink
> +         issues. If unsure, say N.
>
>   endif # NET_CORE
>

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

* Re: [patch net-next v2] nlmon: use standard rtnetlink link api for add/del devices
  2013-07-16 16:10       ` Daniel Borkmann
@ 2013-07-16 16:41         ` Jiri Pirko
  0 siblings, 0 replies; 8+ messages in thread
From: Jiri Pirko @ 2013-07-16 16:41 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: Vladimir Kondratiev, netdev

Tue, Jul 16, 2013 at 06:10:03PM CEST, dborkman@redhat.com wrote:
>On 07/16/2013 05:59 PM, Vladimir Kondratiev wrote:
>>On Tuesday, July 16, 2013 04:49:53 PM Daniel Borkmann wrote:
>>>On 07/16/2013 04:08 PM, Vladimir Kondratiev wrote:
>>>>Jiri Pirko <jiri <at> resnulli.us> writes:
>>>>
>>>>>It is not nice when netdev is created right after module load and with
>>>>>some implicit name. So rather change nlmon to use standard rtnl link API.
>>>>
>>>>Could you please elaborate a bit - how to capture netlink skb's after your
>>>>patch? Before, it was netdev that may be used with tcpdump. Now, there is
>>>>no such netdev. How to create it?
>>>
>>>modprobe nlmon
>>>ip link add type nlmon
>>>ip link set nlmon0 up
>>>
>>>tcpdump -i nlmon0 ....
>>>
>>>ip link set nlmon0 down
>>>ip link del dev nlmon0
>>>rmmod nlmon
>>
>>Thanks a lot! I guess it is worth to have this mentioned somewhere.
>>It will save lots of questions. For example, in Kconfig:
>
>Ok, I don't have a strong opinion on the below patch, I'm fine either way.
>
>But, given that for such devices it is *common* to use the standard rtnl
>link API (Jiri is right that this is a better/cleaner approach), I'm not
>sure if this needs to be documented then ...

I believe that this does not need any further documentation. RTNL (ip
link add ..) is a standard api to add devices...


>
>>diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
>>index b45b240..13acea2 100644
>>--- a/drivers/net/Kconfig
>>+++ b/drivers/net/Kconfig
>>@@ -247,8 +247,18 @@ config NLMON
>>           purpose of this is to analyze netlink messages with packet sockets.
>>           Thus applications like tcpdump will be able to see local netlink
>>           messages if they tap into the netlink device, record pcaps for further
>>-         diagnostics, etc. This is mostly intended for developers or support
>>-         to debug netlink issues. If unsure, say N.
>>+         diagnostics, etc. Typical flow is:
>>+
>>+           modprobe nlmon
>>+           ip link add type nlmon
>>+           ip link set nlmon0 up
>>+           tcpdump -i nlmon0 ....
>>+           ip link set nlmon0 down
>>+           ip link del dev nlmon0
>>+           rmmod nlmon
>>+
>>+         This is mostly intended for developers or support to debug netlink
>>+         issues. If unsure, say N.
>>
>>  endif # NET_CORE
>>

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

end of thread, other threads:[~2013-07-16 16:41 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-02  8:55 [patch net-next v2] nlmon: use standard rtnetlink link api for add/del devices Jiri Pirko
2013-07-02 19:47 ` Daniel Borkmann
2013-07-02 19:53 ` David Miller
2013-07-16 14:08 ` Vladimir Kondratiev
2013-07-16 14:49   ` Daniel Borkmann
2013-07-16 15:59     ` Vladimir Kondratiev
2013-07-16 16:10       ` Daniel Borkmann
2013-07-16 16:41         ` Jiri Pirko

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.