Re: [PATCH] w1: Fix refcount leak in netlink connector
I've been doing an overhaul of the w1 netlink system and have a patch outstanding to address this issue. It requires patches already accepted in gregkh char-misc-next. w1: fix netlink refcnt leak on error path I avoided the issue by checking the length before taking any references to avoid adding another goto target. Actually the changes in char-misc-next removed one goto target so with my patch series that routine only has one jump target, much easier to track what's going on than having three. Accepted patches (still need to apply the refcnt patch) git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git char-misc-next Here's my current set of patches on top of char-misc-next. 7cf8baf26cdc2c27bcff562e9b2328ac891dc3a5 https://github.com/dfries/linux.git w1_rework For my setup I have 14 ds18b20 one wire temperature sensors inside and outside of my house connected to the ds2490 USB dongle that I have a program talking over netlink to read every 30 seconds. I'm curious what's your setup? I would appreciate it if I could get some testers on the w1_rework branch and verify it doesn't break anything with other programs. The biggest user visible changes from the w1_rework branch are. w1: new netlink commands, add/remove/list slaves w1: process w1 netlink commands in w1_process thread w1: reply only to the requester portid ds2490 hardware search, much faster search in general, 23 seconds to .3 seconds w1: fix netlink refcnt leak on error path w1: optional bundling of netlink kernel replies With all these changes the program is no longer blocked when issuing commands because the w1_process thread now executes them. That's a pretty big deal if your program has something else to do while waiting for a reply. Then there's the optional bundling, so I can now send one netlink packet which will request a conversion or read a previous conversion for all temperature sensors and the kernel will reply with one netlink packet with all the status or results, instead of something like 56 individual packets. On Sun, Mar 09, 2014 at 12:08:48AM +0100, Richard Weinberger wrote: > If userspace sends a w1 message of length 0 we leak > the refcount. > > Signed-off-by: Richard Weinberger > --- > drivers/w1/w1_netlink.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/w1/w1_netlink.c b/drivers/w1/w1_netlink.c > index 40788c9..7131777 100644 > --- a/drivers/w1/w1_netlink.c > +++ b/drivers/w1/w1_netlink.c > @@ -355,7 +355,7 @@ static void w1_cn_callback(struct cn_msg *msg, struct > netlink_skb_parms *nsp) > > err = 0; > if (!mlen) > - goto out_cont; > + goto out_dec; > > mutex_lock(>mutex); > > @@ -384,10 +384,11 @@ static void w1_cn_callback(struct cn_msg *msg, struct > netlink_skb_parms *nsp) > mlen -= cmd->len + sizeof(struct w1_netlink_cmd); > } > out_up: > + mutex_unlock(>mutex); > +out_dec: > atomic_dec(>refcnt); > if (sl) > atomic_dec(>refcnt); > - mutex_unlock(>mutex); > out_cont: > if (!cmd || err) > w1_netlink_send_error(msg, m, cmd, err); > -- > 1.8.4.2 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- David Fries PGP pub CB1EE8F0 http://fries.net/~david/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] w1: Fix refcount leak in netlink connector
I've been doing an overhaul of the w1 netlink system and have a patch outstanding to address this issue. It requires patches already accepted in gregkh char-misc-next. w1: fix netlink refcnt leak on error path I avoided the issue by checking the length before taking any references to avoid adding another goto target. Actually the changes in char-misc-next removed one goto target so with my patch series that routine only has one jump target, much easier to track what's going on than having three. Accepted patches (still need to apply the refcnt patch) git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git char-misc-next Here's my current set of patches on top of char-misc-next. 7cf8baf26cdc2c27bcff562e9b2328ac891dc3a5 https://github.com/dfries/linux.git w1_rework For my setup I have 14 ds18b20 one wire temperature sensors inside and outside of my house connected to the ds2490 USB dongle that I have a program talking over netlink to read every 30 seconds. I'm curious what's your setup? I would appreciate it if I could get some testers on the w1_rework branch and verify it doesn't break anything with other programs. The biggest user visible changes from the w1_rework branch are. w1: new netlink commands, add/remove/list slaves w1: process w1 netlink commands in w1_process thread w1: reply only to the requester portid ds2490 hardware search, much faster search in general, 23 seconds to .3 seconds w1: fix netlink refcnt leak on error path w1: optional bundling of netlink kernel replies With all these changes the program is no longer blocked when issuing commands because the w1_process thread now executes them. That's a pretty big deal if your program has something else to do while waiting for a reply. Then there's the optional bundling, so I can now send one netlink packet which will request a conversion or read a previous conversion for all temperature sensors and the kernel will reply with one netlink packet with all the status or results, instead of something like 56 individual packets. On Sun, Mar 09, 2014 at 12:08:48AM +0100, Richard Weinberger wrote: If userspace sends a w1 message of length 0 we leak the refcount. Signed-off-by: Richard Weinberger rich...@nod.at --- drivers/w1/w1_netlink.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/w1/w1_netlink.c b/drivers/w1/w1_netlink.c index 40788c9..7131777 100644 --- a/drivers/w1/w1_netlink.c +++ b/drivers/w1/w1_netlink.c @@ -355,7 +355,7 @@ static void w1_cn_callback(struct cn_msg *msg, struct netlink_skb_parms *nsp) err = 0; if (!mlen) - goto out_cont; + goto out_dec; mutex_lock(dev-mutex); @@ -384,10 +384,11 @@ static void w1_cn_callback(struct cn_msg *msg, struct netlink_skb_parms *nsp) mlen -= cmd-len + sizeof(struct w1_netlink_cmd); } out_up: + mutex_unlock(dev-mutex); +out_dec: atomic_dec(dev-refcnt); if (sl) atomic_dec(sl-refcnt); - mutex_unlock(dev-mutex); out_cont: if (!cmd || err) w1_netlink_send_error(msg, m, cmd, err); -- 1.8.4.2 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- David Fries da...@fries.netPGP pub CB1EE8F0 http://fries.net/~david/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] w1: Fix refcount leak in netlink connector
If userspace sends a w1 message of length 0 we leak the refcount. Signed-off-by: Richard Weinberger --- drivers/w1/w1_netlink.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/w1/w1_netlink.c b/drivers/w1/w1_netlink.c index 40788c9..7131777 100644 --- a/drivers/w1/w1_netlink.c +++ b/drivers/w1/w1_netlink.c @@ -355,7 +355,7 @@ static void w1_cn_callback(struct cn_msg *msg, struct netlink_skb_parms *nsp) err = 0; if (!mlen) - goto out_cont; + goto out_dec; mutex_lock(>mutex); @@ -384,10 +384,11 @@ static void w1_cn_callback(struct cn_msg *msg, struct netlink_skb_parms *nsp) mlen -= cmd->len + sizeof(struct w1_netlink_cmd); } out_up: + mutex_unlock(>mutex); +out_dec: atomic_dec(>refcnt); if (sl) atomic_dec(>refcnt); - mutex_unlock(>mutex); out_cont: if (!cmd || err) w1_netlink_send_error(msg, m, cmd, err); -- 1.8.4.2 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] w1: Fix refcount leak in netlink connector
If userspace sends a w1 message of length 0 we leak the refcount. Signed-off-by: Richard Weinberger rich...@nod.at --- drivers/w1/w1_netlink.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/w1/w1_netlink.c b/drivers/w1/w1_netlink.c index 40788c9..7131777 100644 --- a/drivers/w1/w1_netlink.c +++ b/drivers/w1/w1_netlink.c @@ -355,7 +355,7 @@ static void w1_cn_callback(struct cn_msg *msg, struct netlink_skb_parms *nsp) err = 0; if (!mlen) - goto out_cont; + goto out_dec; mutex_lock(dev-mutex); @@ -384,10 +384,11 @@ static void w1_cn_callback(struct cn_msg *msg, struct netlink_skb_parms *nsp) mlen -= cmd-len + sizeof(struct w1_netlink_cmd); } out_up: + mutex_unlock(dev-mutex); +out_dec: atomic_dec(dev-refcnt); if (sl) atomic_dec(sl-refcnt); - mutex_unlock(dev-mutex); out_cont: if (!cmd || err) w1_netlink_send_error(msg, m, cmd, err); -- 1.8.4.2 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[v2.6.34-stable 121/213] w1: fix oops when w1_search is called from netlink connector
From: Marcin Jurkowski
---
This is a commit scheduled for the next v2.6.34 longterm release.
http://git.kernel.org/?p=linux/kernel/git/paulg/longterm-queue-2.6.34.git
If you see a problem with using this for longterm, please comment.
---
commit 9d1817cab2f030f6af360e961cc69bb1da8ad765 upstream.
On Sat, Mar 02, 2013 at 10:45:10AM +0100, Sven Geggus wrote:
> This is the bad commit I found doing git bisect:
> 04f482faf50535229a5a5c8d629cf963899f857c is the first bad commit
> commit 04f482faf50535229a5a5c8d629cf963899f857c
> Author: Patrick McHardy
> Date: Mon Mar 28 08:39:36 2011 +
Good job. I was too lazy to bisect for bad commit;)
Reading the code I found problematic kthread_should_stop call from netlink
connector which causes the oops. After applying a patch, I've been testing
owfs+w1 setup for nearly two days and it seems to work very reliable (no
hangs, no memleaks etc).
More detailed description and possible fix is given below:
Function w1_search can be called from either kthread or netlink callback.
While the former works fine, the latter causes oops due to kthread_should_stop
invocation.
This patch adds a check if w1_search is serving netlink command, skipping
kthread_should_stop invocation if so.
Signed-off-by: Marcin Jurkowski
Acked-by: Evgeniy Polyakov
Cc: Josh Boyer
Tested-by: Sven Geggus
Signed-off-by: Greg Kroah-Hartman
Signed-off-by: Paul Gortmaker
---
drivers/w1/w1.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index ad5897dc4495..cf05f4c82baa 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -918,7 +918,8 @@ void w1_search(struct w1_master *dev, u8 search_type,
w1_slave_found_callback cb
tmp64 = (triplet_ret >> 2);
rn |= (tmp64 << i);
- if (kthread_should_stop()) {
+ /* ensure we're called from kthread and not by netlink
callback */
+ if (!dev->priv && kthread_should_stop()) {
dev_dbg(>dev, "Abort w1_search\n");
return;
}
--
1.8.5.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[v2.6.34-stable 121/213] w1: fix oops when w1_search is called from netlink connector
From: Marcin Jurkowski marci...@gmail.com
---
This is a commit scheduled for the next v2.6.34 longterm release.
http://git.kernel.org/?p=linux/kernel/git/paulg/longterm-queue-2.6.34.git
If you see a problem with using this for longterm, please comment.
---
commit 9d1817cab2f030f6af360e961cc69bb1da8ad765 upstream.
On Sat, Mar 02, 2013 at 10:45:10AM +0100, Sven Geggus wrote:
This is the bad commit I found doing git bisect:
04f482faf50535229a5a5c8d629cf963899f857c is the first bad commit
commit 04f482faf50535229a5a5c8d629cf963899f857c
Author: Patrick McHardy ka...@trash.net
Date: Mon Mar 28 08:39:36 2011 +
Good job. I was too lazy to bisect for bad commit;)
Reading the code I found problematic kthread_should_stop call from netlink
connector which causes the oops. After applying a patch, I've been testing
owfs+w1 setup for nearly two days and it seems to work very reliable (no
hangs, no memleaks etc).
More detailed description and possible fix is given below:
Function w1_search can be called from either kthread or netlink callback.
While the former works fine, the latter causes oops due to kthread_should_stop
invocation.
This patch adds a check if w1_search is serving netlink command, skipping
kthread_should_stop invocation if so.
Signed-off-by: Marcin Jurkowski marci...@gmail.com
Acked-by: Evgeniy Polyakov z...@ioremap.net
Cc: Josh Boyer jwbo...@gmail.com
Tested-by: Sven Geggus li...@fuchsschwanzdomain.de
Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org
Signed-off-by: Paul Gortmaker paul.gortma...@windriver.com
---
drivers/w1/w1.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index ad5897dc4495..cf05f4c82baa 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -918,7 +918,8 @@ void w1_search(struct w1_master *dev, u8 search_type,
w1_slave_found_callback cb
tmp64 = (triplet_ret 2);
rn |= (tmp64 i);
- if (kthread_should_stop()) {
+ /* ensure we're called from kthread and not by netlink
callback */
+ if (!dev-priv kthread_should_stop()) {
dev_dbg(dev-dev, Abort w1_search\n);
return;
}
--
1.8.5.2
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[ 39/39] Tools: hv: verify origin of netlink connector message
3.0-stable review patch. If anyone has any objections, please let me know.
--
From: Olaf Hering
commit bcc2c9c3fff859e0eb019fe6fec26f9b8eba795c upstream.
The SuSE security team suggested to use recvfrom instead of recv to be
certain that the connector message is originated from kernel.
CVE-2012-2669
Signed-off-by: Olaf Hering
Signed-off-by: Marcus Meissner
Signed-off-by: Sebastian Krahmer
Signed-off-by: K. Y. Srinivasan
Signed-off-by: Greg Kroah-Hartman
Signed-off-by: Jiri Slaby
---
drivers/staging/hv/tools/hv_kvp_daemon.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
--- a/drivers/staging/hv/tools/hv_kvp_daemon.c
+++ b/drivers/staging/hv/tools/hv_kvp_daemon.c
@@ -378,14 +378,18 @@ int main(void)
pfd.fd = fd;
while (1) {
+ struct sockaddr *addr_p = (struct sockaddr *)
+ socklen_t addr_l = sizeof(addr);
pfd.events = POLLIN;
pfd.revents = 0;
poll(, 1, -1);
- len = recv(fd, kvp_recv_buffer, sizeof(kvp_recv_buffer), 0);
+ len = recvfrom(fd, kvp_recv_buffer, sizeof(kvp_recv_buffer), 0,
+ addr_p, _l);
- if (len < 0) {
- syslog(LOG_ERR, "recv failed; error:%d", len);
+ if (len < 0 || addr.nl_pid) {
+ syslog(LOG_ERR, "recvfrom failed; pid:%u error:%d %s",
+ addr.nl_pid, errno, strerror(errno));
close(fd);
return -1;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[ 39/39] Tools: hv: verify origin of netlink connector message
3.0-stable review patch. If anyone has any objections, please let me know.
--
From: Olaf Hering o...@aepfle.de
commit bcc2c9c3fff859e0eb019fe6fec26f9b8eba795c upstream.
The SuSE security team suggested to use recvfrom instead of recv to be
certain that the connector message is originated from kernel.
CVE-2012-2669
Signed-off-by: Olaf Hering o...@aepfle.de
Signed-off-by: Marcus Meissner meiss...@suse.de
Signed-off-by: Sebastian Krahmer krah...@suse.de
Signed-off-by: K. Y. Srinivasan k...@microsoft.com
Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org
Signed-off-by: Jiri Slaby jsl...@suse.cz
---
drivers/staging/hv/tools/hv_kvp_daemon.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
--- a/drivers/staging/hv/tools/hv_kvp_daemon.c
+++ b/drivers/staging/hv/tools/hv_kvp_daemon.c
@@ -378,14 +378,18 @@ int main(void)
pfd.fd = fd;
while (1) {
+ struct sockaddr *addr_p = (struct sockaddr *) addr;
+ socklen_t addr_l = sizeof(addr);
pfd.events = POLLIN;
pfd.revents = 0;
poll(pfd, 1, -1);
- len = recv(fd, kvp_recv_buffer, sizeof(kvp_recv_buffer), 0);
+ len = recvfrom(fd, kvp_recv_buffer, sizeof(kvp_recv_buffer), 0,
+ addr_p, addr_l);
- if (len 0) {
- syslog(LOG_ERR, recv failed; error:%d, len);
+ if (len 0 || addr.nl_pid) {
+ syslog(LOG_ERR, recvfrom failed; pid:%u error:%d %s,
+ addr.nl_pid, errno, strerror(errno));
close(fd);
return -1;
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 014/150] w1: fix oops when w1_search is called from netlink connector
3.5.7.9 -stable review patch. If anyone has any objections, please let me know.
--
From: Marcin Jurkowski
commit 9d1817cab2f030f6af360e961cc69bb1da8ad765 upstream.
On Sat, Mar 02, 2013 at 10:45:10AM +0100, Sven Geggus wrote:
> This is the bad commit I found doing git bisect:
> 04f482faf50535229a5a5c8d629cf963899f857c is the first bad commit
> commit 04f482faf50535229a5a5c8d629cf963899f857c
> Author: Patrick McHardy
> Date: Mon Mar 28 08:39:36 2011 +
Good job. I was too lazy to bisect for bad commit;)
Reading the code I found problematic kthread_should_stop call from netlink
connector which causes the oops. After applying a patch, I've been testing
owfs+w1 setup for nearly two days and it seems to work very reliable (no
hangs, no memleaks etc).
More detailed description and possible fix is given below:
Function w1_search can be called from either kthread or netlink callback.
While the former works fine, the latter causes oops due to kthread_should_stop
invocation.
This patch adds a check if w1_search is serving netlink command, skipping
kthread_should_stop invocation if so.
Signed-off-by: Marcin Jurkowski
Acked-by: Evgeniy Polyakov
Cc: Josh Boyer
Tested-by: Sven Geggus
Signed-off-by: Greg Kroah-Hartman
[ luis: adjust context ]
Signed-off-by: Luis Henriques
---
drivers/w1/w1.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index 2f2e894..2b48bd2 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -928,7 +928,8 @@ void w1_search(struct w1_master *dev, u8 search_type,
w1_slave_found_callback cb
tmp64 = (triplet_ret >> 2);
rn |= (tmp64 << i);
- if (kthread_should_stop()) {
+ /* ensure we're called from kthread and not by netlink
callback */
+ if (!dev->priv && kthread_should_stop()) {
dev_dbg(>dev, "Abort w1_search\n");
return;
}
--
1.8.1.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 014/150] w1: fix oops when w1_search is called from netlink connector
3.5.7.9 -stable review patch. If anyone has any objections, please let me know.
--
From: Marcin Jurkowski marci...@gmail.com
commit 9d1817cab2f030f6af360e961cc69bb1da8ad765 upstream.
On Sat, Mar 02, 2013 at 10:45:10AM +0100, Sven Geggus wrote:
This is the bad commit I found doing git bisect:
04f482faf50535229a5a5c8d629cf963899f857c is the first bad commit
commit 04f482faf50535229a5a5c8d629cf963899f857c
Author: Patrick McHardy ka...@trash.net
Date: Mon Mar 28 08:39:36 2011 +
Good job. I was too lazy to bisect for bad commit;)
Reading the code I found problematic kthread_should_stop call from netlink
connector which causes the oops. After applying a patch, I've been testing
owfs+w1 setup for nearly two days and it seems to work very reliable (no
hangs, no memleaks etc).
More detailed description and possible fix is given below:
Function w1_search can be called from either kthread or netlink callback.
While the former works fine, the latter causes oops due to kthread_should_stop
invocation.
This patch adds a check if w1_search is serving netlink command, skipping
kthread_should_stop invocation if so.
Signed-off-by: Marcin Jurkowski marci...@gmail.com
Acked-by: Evgeniy Polyakov z...@ioremap.net
Cc: Josh Boyer jwbo...@gmail.com
Tested-by: Sven Geggus li...@fuchsschwanzdomain.de
Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org
[ luis: adjust context ]
Signed-off-by: Luis Henriques luis.henriq...@canonical.com
---
drivers/w1/w1.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index 2f2e894..2b48bd2 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -928,7 +928,8 @@ void w1_search(struct w1_master *dev, u8 search_type,
w1_slave_found_callback cb
tmp64 = (triplet_ret 2);
rn |= (tmp64 i);
- if (kthread_should_stop()) {
+ /* ensure we're called from kthread and not by netlink
callback */
+ if (!dev-priv kthread_should_stop()) {
dev_dbg(dev-dev, Abort w1_search\n);
return;
}
--
1.8.1.2
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[ 21/48] w1: fix oops when w1_search is called from netlink connector
3.4-stable review patch. If anyone has any objections, please let me know.
--
From: Marcin Jurkowski
commit 9d1817cab2f030f6af360e961cc69bb1da8ad765 upstream.
On Sat, Mar 02, 2013 at 10:45:10AM +0100, Sven Geggus wrote:
> This is the bad commit I found doing git bisect:
> 04f482faf50535229a5a5c8d629cf963899f857c is the first bad commit
> commit 04f482faf50535229a5a5c8d629cf963899f857c
> Author: Patrick McHardy
> Date: Mon Mar 28 08:39:36 2011 +
Good job. I was too lazy to bisect for bad commit;)
Reading the code I found problematic kthread_should_stop call from netlink
connector which causes the oops. After applying a patch, I've been testing
owfs+w1 setup for nearly two days and it seems to work very reliable (no
hangs, no memleaks etc).
More detailed description and possible fix is given below:
Function w1_search can be called from either kthread or netlink callback.
While the former works fine, the latter causes oops due to kthread_should_stop
invocation.
This patch adds a check if w1_search is serving netlink command, skipping
kthread_should_stop invocation if so.
Signed-off-by: Marcin Jurkowski
Acked-by: Evgeniy Polyakov
Cc: Josh Boyer
Tested-by: Sven Geggus
Signed-off-by: Greg Kroah-Hartman
---
drivers/w1/w1.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -928,7 +928,8 @@ void w1_search(struct w1_master *dev, u8
tmp64 = (triplet_ret >> 2);
rn |= (tmp64 << i);
- if (kthread_should_stop()) {
+ /* ensure we're called from kthread and not by netlink
callback */
+ if (!dev->priv && kthread_should_stop()) {
dev_dbg(>dev, "Abort w1_search\n");
return;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[ 19/40] w1: fix oops when w1_search is called from netlink connector
3.0-stable review patch. If anyone has any objections, please let me know.
--
From: Marcin Jurkowski
commit 9d1817cab2f030f6af360e961cc69bb1da8ad765 upstream.
On Sat, Mar 02, 2013 at 10:45:10AM +0100, Sven Geggus wrote:
> This is the bad commit I found doing git bisect:
> 04f482faf50535229a5a5c8d629cf963899f857c is the first bad commit
> commit 04f482faf50535229a5a5c8d629cf963899f857c
> Author: Patrick McHardy
> Date: Mon Mar 28 08:39:36 2011 +
Good job. I was too lazy to bisect for bad commit;)
Reading the code I found problematic kthread_should_stop call from netlink
connector which causes the oops. After applying a patch, I've been testing
owfs+w1 setup for nearly two days and it seems to work very reliable (no
hangs, no memleaks etc).
More detailed description and possible fix is given below:
Function w1_search can be called from either kthread or netlink callback.
While the former works fine, the latter causes oops due to kthread_should_stop
invocation.
This patch adds a check if w1_search is serving netlink command, skipping
kthread_should_stop invocation if so.
Signed-off-by: Marcin Jurkowski
Acked-by: Evgeniy Polyakov
Cc: Josh Boyer
Tested-by: Sven Geggus
Signed-off-by: Greg Kroah-Hartman
---
drivers/w1/w1.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -918,7 +918,8 @@ void w1_search(struct w1_master *dev, u8
tmp64 = (triplet_ret >> 2);
rn |= (tmp64 << i);
- if (kthread_should_stop()) {
+ /* ensure we're called from kthread and not by netlink
callback */
+ if (!dev->priv && kthread_should_stop()) {
dev_dbg(>dev, "Abort w1_search\n");
return;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[ 35/75] w1: fix oops when w1_search is called from netlink connector
3.8-stable review patch. If anyone has any objections, please let me know.
--
From: Marcin Jurkowski
commit 9d1817cab2f030f6af360e961cc69bb1da8ad765 upstream.
On Sat, Mar 02, 2013 at 10:45:10AM +0100, Sven Geggus wrote:
> This is the bad commit I found doing git bisect:
> 04f482faf50535229a5a5c8d629cf963899f857c is the first bad commit
> commit 04f482faf50535229a5a5c8d629cf963899f857c
> Author: Patrick McHardy
> Date: Mon Mar 28 08:39:36 2011 +
Good job. I was too lazy to bisect for bad commit;)
Reading the code I found problematic kthread_should_stop call from netlink
connector which causes the oops. After applying a patch, I've been testing
owfs+w1 setup for nearly two days and it seems to work very reliable (no
hangs, no memleaks etc).
More detailed description and possible fix is given below:
Function w1_search can be called from either kthread or netlink callback.
While the former works fine, the latter causes oops due to kthread_should_stop
invocation.
This patch adds a check if w1_search is serving netlink command, skipping
kthread_should_stop invocation if so.
Signed-off-by: Marcin Jurkowski
Acked-by: Evgeniy Polyakov
Cc: Josh Boyer
Tested-by: Sven Geggus
Signed-off-by: Greg Kroah-Hartman
---
drivers/w1/w1.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -924,7 +924,8 @@ void w1_search(struct w1_master *dev, u8
tmp64 = (triplet_ret >> 2);
rn |= (tmp64 << i);
- if (kthread_should_stop()) {
+ /* ensure we're called from kthread and not by netlink
callback */
+ if (!dev->priv && kthread_should_stop()) {
mutex_unlock(>bus_mutex);
dev_dbg(>dev, "Abort w1_search\n");
return;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[ 35/75] w1: fix oops when w1_search is called from netlink connector
3.8-stable review patch. If anyone has any objections, please let me know.
--
From: Marcin Jurkowski marci...@gmail.com
commit 9d1817cab2f030f6af360e961cc69bb1da8ad765 upstream.
On Sat, Mar 02, 2013 at 10:45:10AM +0100, Sven Geggus wrote:
This is the bad commit I found doing git bisect:
04f482faf50535229a5a5c8d629cf963899f857c is the first bad commit
commit 04f482faf50535229a5a5c8d629cf963899f857c
Author: Patrick McHardy ka...@trash.net
Date: Mon Mar 28 08:39:36 2011 +
Good job. I was too lazy to bisect for bad commit;)
Reading the code I found problematic kthread_should_stop call from netlink
connector which causes the oops. After applying a patch, I've been testing
owfs+w1 setup for nearly two days and it seems to work very reliable (no
hangs, no memleaks etc).
More detailed description and possible fix is given below:
Function w1_search can be called from either kthread or netlink callback.
While the former works fine, the latter causes oops due to kthread_should_stop
invocation.
This patch adds a check if w1_search is serving netlink command, skipping
kthread_should_stop invocation if so.
Signed-off-by: Marcin Jurkowski marci...@gmail.com
Acked-by: Evgeniy Polyakov z...@ioremap.net
Cc: Josh Boyer jwbo...@gmail.com
Tested-by: Sven Geggus li...@fuchsschwanzdomain.de
Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org
---
drivers/w1/w1.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -924,7 +924,8 @@ void w1_search(struct w1_master *dev, u8
tmp64 = (triplet_ret 2);
rn |= (tmp64 i);
- if (kthread_should_stop()) {
+ /* ensure we're called from kthread and not by netlink
callback */
+ if (!dev-priv kthread_should_stop()) {
mutex_unlock(dev-bus_mutex);
dev_dbg(dev-dev, Abort w1_search\n);
return;
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[ 19/40] w1: fix oops when w1_search is called from netlink connector
3.0-stable review patch. If anyone has any objections, please let me know.
--
From: Marcin Jurkowski marci...@gmail.com
commit 9d1817cab2f030f6af360e961cc69bb1da8ad765 upstream.
On Sat, Mar 02, 2013 at 10:45:10AM +0100, Sven Geggus wrote:
This is the bad commit I found doing git bisect:
04f482faf50535229a5a5c8d629cf963899f857c is the first bad commit
commit 04f482faf50535229a5a5c8d629cf963899f857c
Author: Patrick McHardy ka...@trash.net
Date: Mon Mar 28 08:39:36 2011 +
Good job. I was too lazy to bisect for bad commit;)
Reading the code I found problematic kthread_should_stop call from netlink
connector which causes the oops. After applying a patch, I've been testing
owfs+w1 setup for nearly two days and it seems to work very reliable (no
hangs, no memleaks etc).
More detailed description and possible fix is given below:
Function w1_search can be called from either kthread or netlink callback.
While the former works fine, the latter causes oops due to kthread_should_stop
invocation.
This patch adds a check if w1_search is serving netlink command, skipping
kthread_should_stop invocation if so.
Signed-off-by: Marcin Jurkowski marci...@gmail.com
Acked-by: Evgeniy Polyakov z...@ioremap.net
Cc: Josh Boyer jwbo...@gmail.com
Tested-by: Sven Geggus li...@fuchsschwanzdomain.de
Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org
---
drivers/w1/w1.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -918,7 +918,8 @@ void w1_search(struct w1_master *dev, u8
tmp64 = (triplet_ret 2);
rn |= (tmp64 i);
- if (kthread_should_stop()) {
+ /* ensure we're called from kthread and not by netlink
callback */
+ if (!dev-priv kthread_should_stop()) {
dev_dbg(dev-dev, Abort w1_search\n);
return;
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[ 21/48] w1: fix oops when w1_search is called from netlink connector
3.4-stable review patch. If anyone has any objections, please let me know.
--
From: Marcin Jurkowski marci...@gmail.com
commit 9d1817cab2f030f6af360e961cc69bb1da8ad765 upstream.
On Sat, Mar 02, 2013 at 10:45:10AM +0100, Sven Geggus wrote:
This is the bad commit I found doing git bisect:
04f482faf50535229a5a5c8d629cf963899f857c is the first bad commit
commit 04f482faf50535229a5a5c8d629cf963899f857c
Author: Patrick McHardy ka...@trash.net
Date: Mon Mar 28 08:39:36 2011 +
Good job. I was too lazy to bisect for bad commit;)
Reading the code I found problematic kthread_should_stop call from netlink
connector which causes the oops. After applying a patch, I've been testing
owfs+w1 setup for nearly two days and it seems to work very reliable (no
hangs, no memleaks etc).
More detailed description and possible fix is given below:
Function w1_search can be called from either kthread or netlink callback.
While the former works fine, the latter causes oops due to kthread_should_stop
invocation.
This patch adds a check if w1_search is serving netlink command, skipping
kthread_should_stop invocation if so.
Signed-off-by: Marcin Jurkowski marci...@gmail.com
Acked-by: Evgeniy Polyakov z...@ioremap.net
Cc: Josh Boyer jwbo...@gmail.com
Tested-by: Sven Geggus li...@fuchsschwanzdomain.de
Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org
---
drivers/w1/w1.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -928,7 +928,8 @@ void w1_search(struct w1_master *dev, u8
tmp64 = (triplet_ret 2);
rn |= (tmp64 i);
- if (kthread_should_stop()) {
+ /* ensure we're called from kthread and not by netlink
callback */
+ if (!dev-priv kthread_should_stop()) {
dev_dbg(dev-dev, Abort w1_search\n);
return;
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[ 63/82] w1: fix oops when w1_search is called from netlink connector
3.2-stable review patch. If anyone has any objections, please let me know.
--
From: Marcin Jurkowski
commit 9d1817cab2f030f6af360e961cc69bb1da8ad765 upstream.
On Sat, Mar 02, 2013 at 10:45:10AM +0100, Sven Geggus wrote:
> This is the bad commit I found doing git bisect:
> 04f482faf50535229a5a5c8d629cf963899f857c is the first bad commit
> commit 04f482faf50535229a5a5c8d629cf963899f857c
> Author: Patrick McHardy
> Date: Mon Mar 28 08:39:36 2011 +
Good job. I was too lazy to bisect for bad commit;)
Reading the code I found problematic kthread_should_stop call from netlink
connector which causes the oops. After applying a patch, I've been testing
owfs+w1 setup for nearly two days and it seems to work very reliable (no
hangs, no memleaks etc).
More detailed description and possible fix is given below:
Function w1_search can be called from either kthread or netlink callback.
While the former works fine, the latter causes oops due to kthread_should_stop
invocation.
This patch adds a check if w1_search is serving netlink command, skipping
kthread_should_stop invocation if so.
Signed-off-by: Marcin Jurkowski
Acked-by: Evgeniy Polyakov
Cc: Josh Boyer
Tested-by: Sven Geggus
Signed-off-by: Greg Kroah-Hartman
[bwh: Backported to 3.2: adjust context]
Signed-off-by: Ben Hutchings
---
drivers/w1/w1.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -918,7 +918,8 @@ void w1_search(struct w1_master *dev, u8
tmp64 = (triplet_ret >> 2);
rn |= (tmp64 << i);
- if (kthread_should_stop()) {
+ /* ensure we're called from kthread and not by netlink
callback */
+ if (!dev->priv && kthread_should_stop()) {
dev_dbg(>dev, "Abort w1_search\n");
return;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[ 63/82] w1: fix oops when w1_search is called from netlink connector
3.2-stable review patch. If anyone has any objections, please let me know.
--
From: Marcin Jurkowski marci...@gmail.com
commit 9d1817cab2f030f6af360e961cc69bb1da8ad765 upstream.
On Sat, Mar 02, 2013 at 10:45:10AM +0100, Sven Geggus wrote:
This is the bad commit I found doing git bisect:
04f482faf50535229a5a5c8d629cf963899f857c is the first bad commit
commit 04f482faf50535229a5a5c8d629cf963899f857c
Author: Patrick McHardy ka...@trash.net
Date: Mon Mar 28 08:39:36 2011 +
Good job. I was too lazy to bisect for bad commit;)
Reading the code I found problematic kthread_should_stop call from netlink
connector which causes the oops. After applying a patch, I've been testing
owfs+w1 setup for nearly two days and it seems to work very reliable (no
hangs, no memleaks etc).
More detailed description and possible fix is given below:
Function w1_search can be called from either kthread or netlink callback.
While the former works fine, the latter causes oops due to kthread_should_stop
invocation.
This patch adds a check if w1_search is serving netlink command, skipping
kthread_should_stop invocation if so.
Signed-off-by: Marcin Jurkowski marci...@gmail.com
Acked-by: Evgeniy Polyakov z...@ioremap.net
Cc: Josh Boyer jwbo...@gmail.com
Tested-by: Sven Geggus li...@fuchsschwanzdomain.de
Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org
[bwh: Backported to 3.2: adjust context]
Signed-off-by: Ben Hutchings b...@decadent.org.uk
---
drivers/w1/w1.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -918,7 +918,8 @@ void w1_search(struct w1_master *dev, u8
tmp64 = (triplet_ret 2);
rn |= (tmp64 i);
- if (kthread_should_stop()) {
+ /* ensure we're called from kthread and not by netlink
callback */
+ if (!dev-priv kthread_should_stop()) {
dev_dbg(dev-dev, Abort w1_search\n);
return;
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] w1: fix oops when w1_search is called from netlink connector
On Sun, Mar 3, 2013 at 5:41 PM, GregKH wrote: > On Mon, Mar 04, 2013 at 12:54:52AM +0400, Evgeniy Polyakov wrote: >> Hi >> >> Marcin, thanks a lot for the fix, I have to sorry I'm not on this bug yet :( >> Sven confirmed patch fixes it, Greg please pull it into your tree. > > Ok, will do once 3.9-rc1 is out. 3.9-rc2 is out now. I don't see this in any of your trees. Just a reminder. josh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] w1: fix oops when w1_search is called from netlink connector
On Sun, Mar 3, 2013 at 5:41 PM, GregKH g...@kroah.com wrote: On Mon, Mar 04, 2013 at 12:54:52AM +0400, Evgeniy Polyakov wrote: Hi Marcin, thanks a lot for the fix, I have to sorry I'm not on this bug yet :( Sven confirmed patch fixes it, Greg please pull it into your tree. Ok, will do once 3.9-rc1 is out. 3.9-rc2 is out now. I don't see this in any of your trees. Just a reminder. josh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] w1: fix oops when w1_search is called from netlink connector
On Mon, Mar 04, 2013 at 12:54:52AM +0400, Evgeniy Polyakov wrote: > Hi > > Marcin, thanks a lot for the fix, I have to sorry I'm not on this bug yet :( > Sven confirmed patch fixes it, Greg please pull it into your tree. Ok, will do once 3.9-rc1 is out. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] w1: fix oops when w1_search is called from netlink connector
Hi
Marcin, thanks a lot for the fix, I have to sorry I'm not on this bug yet :(
Sven confirmed patch fixes it, Greg please pull it into your tree.
I believe this is stable material.
Thanks everyone.
Acked-by: Evgeniy Polyakov
On Sat, Mar 02, 2013 at 02:50:15PM +0100, Marcin Jurkowski (marci...@gmail.com)
wrote:
> On Sat, Mar 02, 2013 at 10:45:10AM +0100, Sven Geggus wrote:
> > This is the bad commit I found doing git bisect:
> > 04f482faf50535229a5a5c8d629cf963899f857c is the first bad commit
> > commit 04f482faf50535229a5a5c8d629cf963899f857c
> > Author: Patrick McHardy
> > Date: Mon Mar 28 08:39:36 2011 +
>
> Good job. I was too lazy to bisect for bad commit;)
>
> Reading the code I found problematic kthread_should_stop call from netlink
> connector which causes the oops. After applying a patch, I've been testing
> owfs+w1 setup for nearly two days and it seems to work very reliable (no
> hangs, no memleaks etc).
> More detailed description and possible fix is given below:
>
> Function w1_search can be called from either kthread or netlink callback.
> While the former works fine, the latter causes oops due to kthread_should_stop
> invocation.
>
> This patch adds a check if w1_search is serving netlink command, skipping
> kthread_should_stop invocation if so.
>
> Signed-off-by: Marcin Jurkowski
> ---
> drivers/w1/w1.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
> index 7994d933..7e2220d 100644
> --- a/drivers/w1/w1.c
> +++ b/drivers/w1/w1.c
> @@ -924,7 +924,8 @@ void w1_search(struct w1_master *dev, u8 search_type,
> w1_slave_found_callback cb
> tmp64 = (triplet_ret >> 2);
> rn |= (tmp64 << i);
>
> - if (kthread_should_stop()) {
> + /* ensure we're called from kthread and not by netlink
> callback */
> + if (!dev->priv && kthread_should_stop()) {
> mutex_unlock(>bus_mutex);
> dev_dbg(>dev, "Abort w1_search\n");
> return;
> --
> 1.7.12.4
--
Evgeniy Polyakov
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] w1: fix oops when w1_search is called from netlink connector
Marcin Jurkowski schrieb am Samstag, den 02. März um 14:50 Uhr: > This patch adds a check if w1_search is serving netlink command, skipping > kthread_should_stop invocation if so. Works fine on my Raspberry Pi! Any chance to get this fix into mainline? Regards Sven -- # Turn on/off security. Off is currently the default (found in MongoDB default configfile) /me is giggls@ircnet, http://sven.gegg.us/ on the Web -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] w1: fix oops when w1_search is called from netlink connector
Marcin Jurkowski schrieb am Samstag, den 02. März um 14:50 Uhr: This patch adds a check if w1_search is serving netlink command, skipping kthread_should_stop invocation if so. Works fine on my Raspberry Pi! Any chance to get this fix into mainline? Regards Sven -- # Turn on/off security. Off is currently the default (found in MongoDB default configfile) /me is giggls@ircnet, http://sven.gegg.us/ on the Web -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] w1: fix oops when w1_search is called from netlink connector
Hi
Marcin, thanks a lot for the fix, I have to sorry I'm not on this bug yet :(
Sven confirmed patch fixes it, Greg please pull it into your tree.
I believe this is stable material.
Thanks everyone.
Acked-by: Evgeniy Polyakov z...@ioremap.net
On Sat, Mar 02, 2013 at 02:50:15PM +0100, Marcin Jurkowski (marci...@gmail.com)
wrote:
On Sat, Mar 02, 2013 at 10:45:10AM +0100, Sven Geggus wrote:
This is the bad commit I found doing git bisect:
04f482faf50535229a5a5c8d629cf963899f857c is the first bad commit
commit 04f482faf50535229a5a5c8d629cf963899f857c
Author: Patrick McHardy ka...@trash.net
Date: Mon Mar 28 08:39:36 2011 +
Good job. I was too lazy to bisect for bad commit;)
Reading the code I found problematic kthread_should_stop call from netlink
connector which causes the oops. After applying a patch, I've been testing
owfs+w1 setup for nearly two days and it seems to work very reliable (no
hangs, no memleaks etc).
More detailed description and possible fix is given below:
Function w1_search can be called from either kthread or netlink callback.
While the former works fine, the latter causes oops due to kthread_should_stop
invocation.
This patch adds a check if w1_search is serving netlink command, skipping
kthread_should_stop invocation if so.
Signed-off-by: Marcin Jurkowski marci...@gmail.com
---
drivers/w1/w1.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index 7994d933..7e2220d 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -924,7 +924,8 @@ void w1_search(struct w1_master *dev, u8 search_type,
w1_slave_found_callback cb
tmp64 = (triplet_ret 2);
rn |= (tmp64 i);
- if (kthread_should_stop()) {
+ /* ensure we're called from kthread and not by netlink
callback */
+ if (!dev-priv kthread_should_stop()) {
mutex_unlock(dev-bus_mutex);
dev_dbg(dev-dev, Abort w1_search\n);
return;
--
1.7.12.4
--
Evgeniy Polyakov
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] w1: fix oops when w1_search is called from netlink connector
On Mon, Mar 04, 2013 at 12:54:52AM +0400, Evgeniy Polyakov wrote: Hi Marcin, thanks a lot for the fix, I have to sorry I'm not on this bug yet :( Sven confirmed patch fixes it, Greg please pull it into your tree. Ok, will do once 3.9-rc1 is out. thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] w1: fix oops when w1_search is called from netlink connector
On Sat, Mar 02, 2013 at 10:45:10AM +0100, Sven Geggus wrote:
> This is the bad commit I found doing git bisect:
> 04f482faf50535229a5a5c8d629cf963899f857c is the first bad commit
> commit 04f482faf50535229a5a5c8d629cf963899f857c
> Author: Patrick McHardy
> Date: Mon Mar 28 08:39:36 2011 +
Good job. I was too lazy to bisect for bad commit;)
Reading the code I found problematic kthread_should_stop call from netlink
connector which causes the oops. After applying a patch, I've been testing
owfs+w1 setup for nearly two days and it seems to work very reliable (no
hangs, no memleaks etc).
More detailed description and possible fix is given below:
Function w1_search can be called from either kthread or netlink callback.
While the former works fine, the latter causes oops due to kthread_should_stop
invocation.
This patch adds a check if w1_search is serving netlink command, skipping
kthread_should_stop invocation if so.
Signed-off-by: Marcin Jurkowski
---
drivers/w1/w1.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index 7994d933..7e2220d 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -924,7 +924,8 @@ void w1_search(struct w1_master *dev, u8 search_type,
w1_slave_found_callback cb
tmp64 = (triplet_ret >> 2);
rn |= (tmp64 << i);
- if (kthread_should_stop()) {
+ /* ensure we're called from kthread and not by netlink
callback */
+ if (!dev->priv && kthread_should_stop()) {
mutex_unlock(>bus_mutex);
dev_dbg(>dev, "Abort w1_search\n");
return;
--
1.7.12.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] w1: fix oops when w1_search is called from netlink connector
On Sat, Mar 02, 2013 at 10:45:10AM +0100, Sven Geggus wrote:
This is the bad commit I found doing git bisect:
04f482faf50535229a5a5c8d629cf963899f857c is the first bad commit
commit 04f482faf50535229a5a5c8d629cf963899f857c
Author: Patrick McHardy ka...@trash.net
Date: Mon Mar 28 08:39:36 2011 +
Good job. I was too lazy to bisect for bad commit;)
Reading the code I found problematic kthread_should_stop call from netlink
connector which causes the oops. After applying a patch, I've been testing
owfs+w1 setup for nearly two days and it seems to work very reliable (no
hangs, no memleaks etc).
More detailed description and possible fix is given below:
Function w1_search can be called from either kthread or netlink callback.
While the former works fine, the latter causes oops due to kthread_should_stop
invocation.
This patch adds a check if w1_search is serving netlink command, skipping
kthread_should_stop invocation if so.
Signed-off-by: Marcin Jurkowski marci...@gmail.com
---
drivers/w1/w1.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index 7994d933..7e2220d 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -924,7 +924,8 @@ void w1_search(struct w1_master *dev, u8 search_type,
w1_slave_found_callback cb
tmp64 = (triplet_ret 2);
rn |= (tmp64 i);
- if (kthread_should_stop()) {
+ /* ensure we're called from kthread and not by netlink
callback */
+ if (!dev-priv kthread_should_stop()) {
mutex_unlock(dev-bus_mutex);
dev_dbg(dev-dev, Abort w1_search\n);
return;
--
1.7.12.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[ 001/187] Tools: hv: verify origin of netlink connector message
From: Greg KH
3.4-stable review patch. If anyone has any objections, please let me know.
--
From: Olaf Hering
commit bcc2c9c3fff859e0eb019fe6fec26f9b8eba795c upstream.
The SuSE security team suggested to use recvfrom instead of recv to be
certain that the connector message is originated from kernel.
CVE-2012-2669
Signed-off-by: Olaf Hering
Signed-off-by: Marcus Meissner
Signed-off-by: Sebastian Krahmer
Signed-off-by: K. Y. Srinivasan
Signed-off-by: Greg Kroah-Hartman
---
tools/hv/hv_kvp_daemon.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -701,14 +701,18 @@ int main(void)
pfd.fd = fd;
while (1) {
+ struct sockaddr *addr_p = (struct sockaddr *)
+ socklen_t addr_l = sizeof(addr);
pfd.events = POLLIN;
pfd.revents = 0;
poll(, 1, -1);
- len = recv(fd, kvp_recv_buffer, sizeof(kvp_recv_buffer), 0);
+ len = recvfrom(fd, kvp_recv_buffer, sizeof(kvp_recv_buffer), 0,
+ addr_p, _l);
- if (len < 0) {
- syslog(LOG_ERR, "recv failed; error:%d", len);
+ if (len < 0 || addr.nl_pid) {
+ syslog(LOG_ERR, "recvfrom failed; pid:%u error:%d %s",
+ addr.nl_pid, errno, strerror(errno));
close(fd);
return -1;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[ 001/187] Tools: hv: verify origin of netlink connector message
From: Greg KH gre...@linuxfoundation.org
3.4-stable review patch. If anyone has any objections, please let me know.
--
From: Olaf Hering o...@aepfle.de
commit bcc2c9c3fff859e0eb019fe6fec26f9b8eba795c upstream.
The SuSE security team suggested to use recvfrom instead of recv to be
certain that the connector message is originated from kernel.
CVE-2012-2669
Signed-off-by: Olaf Hering o...@aepfle.de
Signed-off-by: Marcus Meissner meiss...@suse.de
Signed-off-by: Sebastian Krahmer krah...@suse.de
Signed-off-by: K. Y. Srinivasan k...@microsoft.com
Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org
---
tools/hv/hv_kvp_daemon.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -701,14 +701,18 @@ int main(void)
pfd.fd = fd;
while (1) {
+ struct sockaddr *addr_p = (struct sockaddr *) addr;
+ socklen_t addr_l = sizeof(addr);
pfd.events = POLLIN;
pfd.revents = 0;
poll(pfd, 1, -1);
- len = recv(fd, kvp_recv_buffer, sizeof(kvp_recv_buffer), 0);
+ len = recvfrom(fd, kvp_recv_buffer, sizeof(kvp_recv_buffer), 0,
+ addr_p, addr_l);
- if (len 0) {
- syslog(LOG_ERR, recv failed; error:%d, len);
+ if (len 0 || addr.nl_pid) {
+ syslog(LOG_ERR, recvfrom failed; pid:%u error:%d %s,
+ addr.nl_pid, errno, strerror(errno));
close(fd);
return -1;
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Netlink connector
On Tue, Jul 26, 2005 at 04:42:14AM -0400, Harald Welte ([EMAIL PROTECTED]) wrote: > On Mon, Jul 25, 2005 at 02:02:10AM -0400, James Morris wrote: > > On Sun, 24 Jul 2005, David S. Miller wrote: > > >From: Evgeniy Polyakov <[EMAIL PROTECTED]> > > >Date: Sat, 23 Jul 2005 13:14:55 +0400 > > >>Andrew has no objection against connector and it lives in -mm > > >A patch sitting in -mm has zero significance. > > > > The significance I think is that Andrew is trying to gently encourage some > > further progress in the area. > > Patrick McHardy is currently working on some ideas on how to extend > netlink. > > The fundamental problem that the connector is trying to solve: > > 1) provide more 'groups' (to transport more different kinds of events) > 2) provide an abstract API for other kernel code, so it doesn't have to >know anything about skb's or networking. > > IMHO issue number '1' should (and can) be adressed within netlink. Wait > for Patrick's work on this to show up on netdev. We can then think > whether the connctor API (or something similar) can be put on top of it. Fair enough. Let's do it this way. > -- > - Harald Welte <[EMAIL PROTECTED]> http://netfilter.org/ > > "Fragmentation is like classful addressing -- an interesting early >architectural error that shows how much experimentation was going >on while IP was being designed."-- Paul Vixie -- Evgeniy Polyakov - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Netlink connector
On Mon, Jul 25, 2005 at 11:33:51PM +0400, Evgeniy Polyakov wrote: > Netlink is transport protocol - no need to add complexity into it, > it must be as simple as possible and thus extensible. yes. but when you run into a serious addressing shortage (like the internet does with ipv4), you develop something that provides more addresses (such as ipv6). That's why support for more groups than 32 (per family) is something that should be put in the netlink protocol. I totally agree that we need a higher-level api on top of that, in order to hide the details of the networking stack for those not interested in it. -- - Harald Welte <[EMAIL PROTECTED]> http://netfilter.org/ "Fragmentation is like classful addressing -- an interesting early architectural error that shows how much experimentation was going on while IP was being designed."-- Paul Vixie pgpyoFvK5oqLK.pgp Description: PGP signature
Re: Netlink connector
On Mon, Jul 25, 2005 at 02:02:10AM -0400, James Morris wrote: > On Sun, 24 Jul 2005, David S. Miller wrote: > >From: Evgeniy Polyakov <[EMAIL PROTECTED]> > >Date: Sat, 23 Jul 2005 13:14:55 +0400 > >>Andrew has no objection against connector and it lives in -mm > >A patch sitting in -mm has zero significance. > > The significance I think is that Andrew is trying to gently encourage some > further progress in the area. Patrick McHardy is currently working on some ideas on how to extend netlink. The fundamental problem that the connector is trying to solve: 1) provide more 'groups' (to transport more different kinds of events) 2) provide an abstract API for other kernel code, so it doesn't have to know anything about skb's or networking. IMHO issue number '1' should (and can) be adressed within netlink. Wait for Patrick's work on this to show up on netdev. We can then think whether the connctor API (or something similar) can be put on top of it. -- - Harald Welte <[EMAIL PROTECTED]> http://netfilter.org/ "Fragmentation is like classful addressing -- an interesting early architectural error that shows how much experimentation was going on while IP was being designed."-- Paul Vixie pgpR29EQBIMUR.pgp Description: PGP signature
Re: Netlink connector
On Tue, Jul 26, 2005 at 08:14:47AM +0200, Thomas Graf ([EMAIL PROTECTED]) wrote: > * Evgeniy Polyakov <[EMAIL PROTECTED]> 2005-07-26 08:45 > > On Tue, Jul 26, 2005 at 01:46:04AM +0200, Patrick McHardy ([EMAIL > > PROTECTED]) wrote: > > > Usually netlink is easily extendable by using nested TLVs. By hiding > > > this you basically remove this extensibility. > > > > Current netlink is not extensible for _many_ different users. > > Patrick's key point was that by hiding some of the functionality > you remove a lot of the flexbility. > > > It has only 32 sockets. > > You mean MAX_LINKS? That is the current number of reserved > netlink protocols. The ethertaps are obsolete and can be > reused so we're currently using 16 out of 256 possible > protocols. If that is not enough there are ways to work > around this. However, I also see a need for a generic protocol > providing a simplified interface for small applications. > Nevertheless we should take the time and work things out on > the netlink level first, netlink has issues and we should not > work around them in a upper layer. > > > > But my main objection is that it sends everything to userspace even > > > if noone is listening. This can't be used for things that generate > > > lots of events, and also will get problematic is the number of users > > > increases. > > > > It is a problem for existing netlink - either check in bind time, > > what could be done for connector, or in socket creation time. > > No, I think you are misunderstanding something. As I said, we can > easly add a function netlink_nr_subscribers(sk, groups) so the > check can be done before starting to build the message. This is > no problem, it simply didn't make sense so far because netlink > event messages were mostly used for rare events. Yep. > > Actually it is not even a problem, since checking is being done, > > but after allocation and message filling, such check can be moved into > > cn_netlink_send() in connector, but different netlink users, > > who prefers to use different sockets, must perform it by itself in each > > place, where skb is allocated... > > Sure, which is the right thing, it makes perfect sense to check > before starting the process of building and event and sending it. > > > Connector is a solution for current situation, > > it can be deployed with few casualties. > > The problem is that netlink is likely to change in order > to cope with some recent needs, e.g. ctnetlink but also other > current issues which need to be addressed. Therefore I suggest > to build connector on top of the updated netlink so you we have > one thing less to worry about when thinking about compatibility. > > > Creating a new netlink2 socket for device, which wants to replace ioctl > > controlling or broadcast it's state is a wrong way. > > Slowly, we might need netlink2 _in case_ we cannot work things > out without breaking compatibility. This has nothing to do with > the connector, there are netlink users which have new needs such > as more groups, at least some of them need the flexibility of > netlink itself so we have to work things out for them. > > > Different sockets/flows does not allow easy flow control. > > I'm not sure what you mean. Concider socket overrun - message will be dropped, using special flags in connector [it's size field was selected to be 4 bytes, and thus has big reserve] this subsystem can requeue message later after timeout or something similar... > > We have one pipe - ethernet, and many protocols inside this pipe > > with different headers - it is the same here - netlink is such a pipe, > > and with connector it allows to have different protocols in it. > > At least parts of your connector is just a redudant implementation > of what netlink is already capable of doing. Sure, some of them > have issues but there is no reason to just build a new protocol on > top of another one if the protocol beneath has issues which can be > resolved. > > > > You still have to take care of mixed 64/32 bit environments, u64 fields > > > for example are differently alligned. > > > > Connector has a size in it's header - ioctl does not. > > You have exactly the same issues as netlink as soon as you transfer > structs, believe it or not. > > > It does not "fix" the "problem" of skb management knowledge, which I > > described. > > Yes ok, this is a different issue and as Patrick stated already > those have been mostly worked out by providing a new set of > macros. Except for a few leftovers, which will be addressed, there > is no need to call skb functions anymore. The reason the plain > skb interface was used is simply that the authors of most of the > netlink using code are in fact very familiar with the skb interface, > that's it. I saw your changes - theay are very usefull, but _only_ for sending part. Kernel receiver still needs dequeuing, freeing and NLKMSG macros. In first netlink days it also needed skb_recv_msg() or something similar... >
Re: Netlink connector
* Evgeniy Polyakov <[EMAIL PROTECTED]> 2005-07-26 08:45 > On Tue, Jul 26, 2005 at 01:46:04AM +0200, Patrick McHardy ([EMAIL PROTECTED]) > wrote: > > Usually netlink is easily extendable by using nested TLVs. By hiding > > this you basically remove this extensibility. > > Current netlink is not extensible for _many_ different users. Patrick's key point was that by hiding some of the functionality you remove a lot of the flexbility. > It has only 32 sockets. You mean MAX_LINKS? That is the current number of reserved netlink protocols. The ethertaps are obsolete and can be reused so we're currently using 16 out of 256 possible protocols. If that is not enough there are ways to work around this. However, I also see a need for a generic protocol providing a simplified interface for small applications. Nevertheless we should take the time and work things out on the netlink level first, netlink has issues and we should not work around them in a upper layer. > > But my main objection is that it sends everything to userspace even > > if noone is listening. This can't be used for things that generate > > lots of events, and also will get problematic is the number of users > > increases. > > It is a problem for existing netlink - either check in bind time, > what could be done for connector, or in socket creation time. No, I think you are misunderstanding something. As I said, we can easly add a function netlink_nr_subscribers(sk, groups) so the check can be done before starting to build the message. This is no problem, it simply didn't make sense so far because netlink event messages were mostly used for rare events. > Actually it is not even a problem, since checking is being done, > but after allocation and message filling, such check can be moved into > cn_netlink_send() in connector, but different netlink users, > who prefers to use different sockets, must perform it by itself in each > place, where skb is allocated... Sure, which is the right thing, it makes perfect sense to check before starting the process of building and event and sending it. > Connector is a solution for current situation, > it can be deployed with few casualties. The problem is that netlink is likely to change in order to cope with some recent needs, e.g. ctnetlink but also other current issues which need to be addressed. Therefore I suggest to build connector on top of the updated netlink so you we have one thing less to worry about when thinking about compatibility. > Creating a new netlink2 socket for device, which wants to replace ioctl > controlling or broadcast it's state is a wrong way. Slowly, we might need netlink2 _in case_ we cannot work things out without breaking compatibility. This has nothing to do with the connector, there are netlink users which have new needs such as more groups, at least some of them need the flexibility of netlink itself so we have to work things out for them. > Different sockets/flows does not allow easy flow control. I'm not sure what you mean. > We have one pipe - ethernet, and many protocols inside this pipe > with different headers - it is the same here - netlink is such a pipe, > and with connector it allows to have different protocols in it. At least parts of your connector is just a redudant implementation of what netlink is already capable of doing. Sure, some of them have issues but there is no reason to just build a new protocol on top of another one if the protocol beneath has issues which can be resolved. > > You still have to take care of mixed 64/32 bit environments, u64 fields > > for example are differently alligned. > > Connector has a size in it's header - ioctl does not. You have exactly the same issues as netlink as soon as you transfer structs, believe it or not. > It does not "fix" the "problem" of skb management knowledge, which I > described. Yes ok, this is a different issue and as Patrick stated already those have been mostly worked out by providing a new set of macros. Except for a few leftovers, which will be addressed, there is no need to call skb functions anymore. The reason the plain skb interface was used is simply that the authors of most of the netlink using code are in fact very familiar with the skb interface, that's it. > > You can still built this stuff on top, but the workarounds for netlink > > limitations need to be fixed in netlink. > > I could not call it workaround, I think it is a management layer, > which allows : Listen, nobody wants to take away your baby. ;-> There are some objections of things which would rather be fixed in the netlink layer first and the remaining part that is missing goes into the connector. I see a lot of replicated netlink code in the connector which is no necessary. I perfectly agree with you that we require some form of simplified addressing and easier message handling for simple applications but just building another layer on top of netlink without respecting the capabilities of netlink itself is
Re: Netlink connector
* Evgeniy Polyakov [EMAIL PROTECTED] 2005-07-26 08:45 On Tue, Jul 26, 2005 at 01:46:04AM +0200, Patrick McHardy ([EMAIL PROTECTED]) wrote: Usually netlink is easily extendable by using nested TLVs. By hiding this you basically remove this extensibility. Current netlink is not extensible for _many_ different users. Patrick's key point was that by hiding some of the functionality you remove a lot of the flexbility. It has only 32 sockets. You mean MAX_LINKS? That is the current number of reserved netlink protocols. The ethertaps are obsolete and can be reused so we're currently using 16 out of 256 possible protocols. If that is not enough there are ways to work around this. However, I also see a need for a generic protocol providing a simplified interface for small applications. Nevertheless we should take the time and work things out on the netlink level first, netlink has issues and we should not work around them in a upper layer. But my main objection is that it sends everything to userspace even if noone is listening. This can't be used for things that generate lots of events, and also will get problematic is the number of users increases. It is a problem for existing netlink - either check in bind time, what could be done for connector, or in socket creation time. No, I think you are misunderstanding something. As I said, we can easly add a function netlink_nr_subscribers(sk, groups) so the check can be done before starting to build the message. This is no problem, it simply didn't make sense so far because netlink event messages were mostly used for rare events. Actually it is not even a problem, since checking is being done, but after allocation and message filling, such check can be moved into cn_netlink_send() in connector, but different netlink users, who prefers to use different sockets, must perform it by itself in each place, where skb is allocated... Sure, which is the right thing, it makes perfect sense to check before starting the process of building and event and sending it. Connector is a solution for current situation, it can be deployed with few casualties. The problem is that netlink is likely to change in order to cope with some recent needs, e.g. ctnetlink but also other current issues which need to be addressed. Therefore I suggest to build connector on top of the updated netlink so you we have one thing less to worry about when thinking about compatibility. Creating a new netlink2 socket for device, which wants to replace ioctl controlling or broadcast it's state is a wrong way. Slowly, we might need netlink2 _in case_ we cannot work things out without breaking compatibility. This has nothing to do with the connector, there are netlink users which have new needs such as more groups, at least some of them need the flexibility of netlink itself so we have to work things out for them. Different sockets/flows does not allow easy flow control. I'm not sure what you mean. We have one pipe - ethernet, and many protocols inside this pipe with different headers - it is the same here - netlink is such a pipe, and with connector it allows to have different protocols in it. At least parts of your connector is just a redudant implementation of what netlink is already capable of doing. Sure, some of them have issues but there is no reason to just build a new protocol on top of another one if the protocol beneath has issues which can be resolved. You still have to take care of mixed 64/32 bit environments, u64 fields for example are differently alligned. Connector has a size in it's header - ioctl does not. You have exactly the same issues as netlink as soon as you transfer structs, believe it or not. It does not fix the problem of skb management knowledge, which I described. Yes ok, this is a different issue and as Patrick stated already those have been mostly worked out by providing a new set of macros. Except for a few leftovers, which will be addressed, there is no need to call skb functions anymore. The reason the plain skb interface was used is simply that the authors of most of the netlink using code are in fact very familiar with the skb interface, that's it. You can still built this stuff on top, but the workarounds for netlink limitations need to be fixed in netlink. I could not call it workaround, I think it is a management layer, which allows : Listen, nobody wants to take away your baby. ;- There are some objections of things which would rather be fixed in the netlink layer first and the remaining part that is missing goes into the connector. I see a lot of replicated netlink code in the connector which is no necessary. I perfectly agree with you that we require some form of simplified addressing and easier message handling for simple applications but just building another layer on top of netlink without respecting the capabilities of netlink itself is not the way to go as I see it. For example, we'll
Re: Netlink connector
On Tue, Jul 26, 2005 at 08:14:47AM +0200, Thomas Graf ([EMAIL PROTECTED]) wrote: * Evgeniy Polyakov [EMAIL PROTECTED] 2005-07-26 08:45 On Tue, Jul 26, 2005 at 01:46:04AM +0200, Patrick McHardy ([EMAIL PROTECTED]) wrote: Usually netlink is easily extendable by using nested TLVs. By hiding this you basically remove this extensibility. Current netlink is not extensible for _many_ different users. Patrick's key point was that by hiding some of the functionality you remove a lot of the flexbility. It has only 32 sockets. You mean MAX_LINKS? That is the current number of reserved netlink protocols. The ethertaps are obsolete and can be reused so we're currently using 16 out of 256 possible protocols. If that is not enough there are ways to work around this. However, I also see a need for a generic protocol providing a simplified interface for small applications. Nevertheless we should take the time and work things out on the netlink level first, netlink has issues and we should not work around them in a upper layer. But my main objection is that it sends everything to userspace even if noone is listening. This can't be used for things that generate lots of events, and also will get problematic is the number of users increases. It is a problem for existing netlink - either check in bind time, what could be done for connector, or in socket creation time. No, I think you are misunderstanding something. As I said, we can easly add a function netlink_nr_subscribers(sk, groups) so the check can be done before starting to build the message. This is no problem, it simply didn't make sense so far because netlink event messages were mostly used for rare events. Yep. Actually it is not even a problem, since checking is being done, but after allocation and message filling, such check can be moved into cn_netlink_send() in connector, but different netlink users, who prefers to use different sockets, must perform it by itself in each place, where skb is allocated... Sure, which is the right thing, it makes perfect sense to check before starting the process of building and event and sending it. Connector is a solution for current situation, it can be deployed with few casualties. The problem is that netlink is likely to change in order to cope with some recent needs, e.g. ctnetlink but also other current issues which need to be addressed. Therefore I suggest to build connector on top of the updated netlink so you we have one thing less to worry about when thinking about compatibility. Creating a new netlink2 socket for device, which wants to replace ioctl controlling or broadcast it's state is a wrong way. Slowly, we might need netlink2 _in case_ we cannot work things out without breaking compatibility. This has nothing to do with the connector, there are netlink users which have new needs such as more groups, at least some of them need the flexibility of netlink itself so we have to work things out for them. Different sockets/flows does not allow easy flow control. I'm not sure what you mean. Concider socket overrun - message will be dropped, using special flags in connector [it's size field was selected to be 4 bytes, and thus has big reserve] this subsystem can requeue message later after timeout or something similar... We have one pipe - ethernet, and many protocols inside this pipe with different headers - it is the same here - netlink is such a pipe, and with connector it allows to have different protocols in it. At least parts of your connector is just a redudant implementation of what netlink is already capable of doing. Sure, some of them have issues but there is no reason to just build a new protocol on top of another one if the protocol beneath has issues which can be resolved. You still have to take care of mixed 64/32 bit environments, u64 fields for example are differently alligned. Connector has a size in it's header - ioctl does not. You have exactly the same issues as netlink as soon as you transfer structs, believe it or not. It does not fix the problem of skb management knowledge, which I described. Yes ok, this is a different issue and as Patrick stated already those have been mostly worked out by providing a new set of macros. Except for a few leftovers, which will be addressed, there is no need to call skb functions anymore. The reason the plain skb interface was used is simply that the authors of most of the netlink using code are in fact very familiar with the skb interface, that's it. I saw your changes - theay are very usefull, but _only_ for sending part. Kernel receiver still needs dequeuing, freeing and NLKMSG macros. In first netlink days it also needed skb_recv_msg() or something similar... You can still built this stuff on top, but the workarounds for netlink limitations need to be fixed in netlink. I could not call it
Re: Netlink connector
On Mon, Jul 25, 2005 at 02:02:10AM -0400, James Morris wrote: On Sun, 24 Jul 2005, David S. Miller wrote: From: Evgeniy Polyakov [EMAIL PROTECTED] Date: Sat, 23 Jul 2005 13:14:55 +0400 Andrew has no objection against connector and it lives in -mm A patch sitting in -mm has zero significance. The significance I think is that Andrew is trying to gently encourage some further progress in the area. Patrick McHardy is currently working on some ideas on how to extend netlink. The fundamental problem that the connector is trying to solve: 1) provide more 'groups' (to transport more different kinds of events) 2) provide an abstract API for other kernel code, so it doesn't have to know anything about skb's or networking. IMHO issue number '1' should (and can) be adressed within netlink. Wait for Patrick's work on this to show up on netdev. We can then think whether the connctor API (or something similar) can be put on top of it. -- - Harald Welte [EMAIL PROTECTED] http://netfilter.org/ Fragmentation is like classful addressing -- an interesting early architectural error that shows how much experimentation was going on while IP was being designed.-- Paul Vixie pgpR29EQBIMUR.pgp Description: PGP signature
Re: Netlink connector
On Tue, Jul 26, 2005 at 04:42:14AM -0400, Harald Welte ([EMAIL PROTECTED]) wrote: On Mon, Jul 25, 2005 at 02:02:10AM -0400, James Morris wrote: On Sun, 24 Jul 2005, David S. Miller wrote: From: Evgeniy Polyakov [EMAIL PROTECTED] Date: Sat, 23 Jul 2005 13:14:55 +0400 Andrew has no objection against connector and it lives in -mm A patch sitting in -mm has zero significance. The significance I think is that Andrew is trying to gently encourage some further progress in the area. Patrick McHardy is currently working on some ideas on how to extend netlink. The fundamental problem that the connector is trying to solve: 1) provide more 'groups' (to transport more different kinds of events) 2) provide an abstract API for other kernel code, so it doesn't have to know anything about skb's or networking. IMHO issue number '1' should (and can) be adressed within netlink. Wait for Patrick's work on this to show up on netdev. We can then think whether the connctor API (or something similar) can be put on top of it. Fair enough. Let's do it this way. -- - Harald Welte [EMAIL PROTECTED] http://netfilter.org/ Fragmentation is like classful addressing -- an interesting early architectural error that shows how much experimentation was going on while IP was being designed.-- Paul Vixie -- Evgeniy Polyakov - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Netlink connector
On Mon, Jul 25, 2005 at 11:33:51PM +0400, Evgeniy Polyakov wrote: Netlink is transport protocol - no need to add complexity into it, it must be as simple as possible and thus extensible. yes. but when you run into a serious addressing shortage (like the internet does with ipv4), you develop something that provides more addresses (such as ipv6). That's why support for more groups than 32 (per family) is something that should be put in the netlink protocol. I totally agree that we need a higher-level api on top of that, in order to hide the details of the networking stack for those not interested in it. -- - Harald Welte [EMAIL PROTECTED] http://netfilter.org/ Fragmentation is like classful addressing -- an interesting early architectural error that shows how much experimentation was going on while IP was being designed.-- Paul Vixie pgpyoFvK5oqLK.pgp Description: PGP signature
Re: Netlink connector
On Mon, Jul 25, 2005 at 09:56:56PM -0700, Stephen Hemminger ([EMAIL PROTECTED]) wrote: > Evgeniy Polyakov wrote: > > >On Tue, Jul 26, 2005 at 01:46:04AM +0200, Patrick McHardy > >([EMAIL PROTECTED]) wrote: > > > > > >>Evgeniy Polyakov wrote: > >> > >> > >>>On Mon, Jul 25, 2005 at 04:32:32PM +0200, Patrick McHardy > >>>([EMAIL PROTECTED]) wrote: > >>> > >>> > >>> > If I understand correctly it tries to workaround some netlink > limitations (limited number of netlink families and multicast groups) > by sending everything to userspace and demultiplexing it there. > Same in the other direction, an additional layer on top of netlink > does basically the same thing netlink already does. This looks like > a step in the wrong direction to me, netlink should instead be fixed > to support what is needed. > > > >>>Not only it. > >>>The main _first_ idea was to simplify userspace mesasge handling as much > >>>as possible. > >>>In first releases I called it ioctl-ng - any module that want ot > >>>communicate with userspace in the way ioctl does, > >>> > >>> > >>Usually netlink is easily extendable by using nested TLVs. By hiding > >>this you basically remove this extensibility. > >> > >> > > > >Current netlink is not extensible for _many_ different users. > >It has only 32 sockets. > > > > > > > >>>requires skb allocation/freeing/handling. > >>>Does RTC driver writer need to know what is the difference between > >>>shared and cloned skb? Should kernel user of such message bus > >>>have to know about skb at all? > >>> > >>> > >>Netlink users don't have to care about shared or cloned skbs. I don't > >>think its a big issue to use alloc_skb and then the usual netlink > >>macros. Thomas added a number of macros that simplfiy use a lot. > >> > >> > > > >Kernel user also must know about difference between unicast/broadcast, > >how to dequeue the skb, how to free it and in what context. > >ioctl users do not need to know how file_operations is bound to file. > > > > > > > >>But my main objection is that it sends everything to userspace even > >>if noone is listening. This can't be used for things that generate > >>lots of events, and also will get problematic is the number of users > >>increases. > >> > >> > > > >It is a problem for existing netlink - either check in bind time, > >what could be done for connector, or in socket creation time. > > > >Actually it is not even a problem, since checking is being done, > >but after allocation and message filling, such check can be moved into > >cn_netlink_send() in connector, but different netlink users, > >who prefers to use different sockets, must perform it by itself in each > >place, where skb is allocated... > > > >Connector is a solution for current situation, > >it can be deployed with few casualties. > >Creating a new netlink2 socket for device, which wants to replace ioctl > >controlling or broadcast it's state is a wrong way. > >Different sockets/flows does not allow easy flow control. > > > >We have one pipe - ethernet, and many protocols inside this pipe > >with different headers - it is the same here - netlink is such a pipe, > >and with connector it allows to have different protocols in it. > > > > > > > >>>With char device I only need to register my callback - with kernel > >>>connector it is the same, but allows to use the whole power of netlink, > >>>especially without nice ioctl features like different pointer size > >>>in userspace and kernelspace. > >>> > >>> > >>You still have to take care of mixed 64/32 bit environments, u64 fields > >>for example are differently alligned. > >> > >> > > > >Connector has a size in it's header - ioctl does not. > > > > > > > >>>And number of free netlink sockets is _very_ small, especially > >>>if allocate new one for simple notifications, which can be easily done > >>>using connector. > >>> > >>> > >>Then fix it so we can use more families and groups. I started some work > >>on this, but I'm not sure if I have time to complete it. > >> > >> > > > >It does not "fix" the "problem" of skb management knowledge, which I > >described. > >Netlink is a transport protocol, some general logic must be created on > >top of it, like it is done in TCP/IP. > > > > > > > >>>And netlink can be extended to support it - netlink is a transport > >>>protocol, it should not care about higher layer message handling, > >>>connector instead will deliver message to the end user in a very > >>>convenient form. > >>> > >>> > >>You can still built this stuff on top, but the workarounds for netlink > >>limitations need to be fixed in netlink. > >> > >> > > > >I could not call it workaround, I think it is a management layer, > >which allows : > >1. easy usage. Just register a callback and that is all. Callback will > >be invoced each time new message arrives. No need to > >dequeue/free/anything. > >2. easy usage. Call one function for message
Re: Netlink connector
Evgeniy Polyakov wrote: On Tue, Jul 26, 2005 at 01:46:04AM +0200, Patrick McHardy ([EMAIL PROTECTED]) wrote: Evgeniy Polyakov wrote: On Mon, Jul 25, 2005 at 04:32:32PM +0200, Patrick McHardy ([EMAIL PROTECTED]) wrote: If I understand correctly it tries to workaround some netlink limitations (limited number of netlink families and multicast groups) by sending everything to userspace and demultiplexing it there. Same in the other direction, an additional layer on top of netlink does basically the same thing netlink already does. This looks like a step in the wrong direction to me, netlink should instead be fixed to support what is needed. Not only it. The main _first_ idea was to simplify userspace mesasge handling as much as possible. In first releases I called it ioctl-ng - any module that want ot communicate with userspace in the way ioctl does, Usually netlink is easily extendable by using nested TLVs. By hiding this you basically remove this extensibility. Current netlink is not extensible for _many_ different users. It has only 32 sockets. requires skb allocation/freeing/handling. Does RTC driver writer need to know what is the difference between shared and cloned skb? Should kernel user of such message bus have to know about skb at all? Netlink users don't have to care about shared or cloned skbs. I don't think its a big issue to use alloc_skb and then the usual netlink macros. Thomas added a number of macros that simplfiy use a lot. Kernel user also must know about difference between unicast/broadcast, how to dequeue the skb, how to free it and in what context. ioctl users do not need to know how file_operations is bound to file. But my main objection is that it sends everything to userspace even if noone is listening. This can't be used for things that generate lots of events, and also will get problematic is the number of users increases. It is a problem for existing netlink - either check in bind time, what could be done for connector, or in socket creation time. Actually it is not even a problem, since checking is being done, but after allocation and message filling, such check can be moved into cn_netlink_send() in connector, but different netlink users, who prefers to use different sockets, must perform it by itself in each place, where skb is allocated... Connector is a solution for current situation, it can be deployed with few casualties. Creating a new netlink2 socket for device, which wants to replace ioctl controlling or broadcast it's state is a wrong way. Different sockets/flows does not allow easy flow control. We have one pipe - ethernet, and many protocols inside this pipe with different headers - it is the same here - netlink is such a pipe, and with connector it allows to have different protocols in it. With char device I only need to register my callback - with kernel connector it is the same, but allows to use the whole power of netlink, especially without nice ioctl features like different pointer size in userspace and kernelspace. You still have to take care of mixed 64/32 bit environments, u64 fields for example are differently alligned. Connector has a size in it's header - ioctl does not. And number of free netlink sockets is _very_ small, especially if allocate new one for simple notifications, which can be easily done using connector. Then fix it so we can use more families and groups. I started some work on this, but I'm not sure if I have time to complete it. It does not "fix" the "problem" of skb management knowledge, which I described. Netlink is a transport protocol, some general logic must be created on top of it, like it is done in TCP/IP. And netlink can be extended to support it - netlink is a transport protocol, it should not care about higher layer message handling, connector instead will deliver message to the end user in a very convenient form. You can still built this stuff on top, but the workarounds for netlink limitations need to be fixed in netlink. I could not call it workaround, I think it is a management layer, which allows : 1. easy usage. Just register a callback and that is all. Callback will be invoced each time new message arrives. No need to dequeue/free/anything. 2. easy usage. Call one function for message delivering, which can care of nonexistent users, perform flow control, congestion control, guarantee delivery and any other. 3. Easily deployable - current implementation is so simple, and it does work with existing netlink. 4. It is logical level on top of transport protocol, it is UDP/IP over ethernet :) If it is a transport, then it should be in the kernel. Otherwise, it becomes painful for applications with multiple input sources. Think of epoll/poll/select and threads, doing the demultiplexing in user space would be a pain for applications and libraries. The other way to go is
Re: Netlink connector
On Tue, Jul 26, 2005 at 01:46:04AM +0200, Patrick McHardy ([EMAIL PROTECTED]) wrote: > Evgeniy Polyakov wrote: > >On Mon, Jul 25, 2005 at 04:32:32PM +0200, Patrick McHardy > >([EMAIL PROTECTED]) wrote: > > > >>If I understand correctly it tries to workaround some netlink > >>limitations (limited number of netlink families and multicast groups) > >>by sending everything to userspace and demultiplexing it there. > >>Same in the other direction, an additional layer on top of netlink > >>does basically the same thing netlink already does. This looks like > >>a step in the wrong direction to me, netlink should instead be fixed > >>to support what is needed. > > > >Not only it. > >The main _first_ idea was to simplify userspace mesasge handling as much > >as possible. > >In first releases I called it ioctl-ng - any module that want ot > >communicate with userspace in the way ioctl does, > > Usually netlink is easily extendable by using nested TLVs. By hiding > this you basically remove this extensibility. Current netlink is not extensible for _many_ different users. It has only 32 sockets. > >requires skb allocation/freeing/handling. > >Does RTC driver writer need to know what is the difference between > >shared and cloned skb? Should kernel user of such message bus > >have to know about skb at all? > > Netlink users don't have to care about shared or cloned skbs. I don't > think its a big issue to use alloc_skb and then the usual netlink > macros. Thomas added a number of macros that simplfiy use a lot. Kernel user also must know about difference between unicast/broadcast, how to dequeue the skb, how to free it and in what context. ioctl users do not need to know how file_operations is bound to file. > But my main objection is that it sends everything to userspace even > if noone is listening. This can't be used for things that generate > lots of events, and also will get problematic is the number of users > increases. It is a problem for existing netlink - either check in bind time, what could be done for connector, or in socket creation time. Actually it is not even a problem, since checking is being done, but after allocation and message filling, such check can be moved into cn_netlink_send() in connector, but different netlink users, who prefers to use different sockets, must perform it by itself in each place, where skb is allocated... Connector is a solution for current situation, it can be deployed with few casualties. Creating a new netlink2 socket for device, which wants to replace ioctl controlling or broadcast it's state is a wrong way. Different sockets/flows does not allow easy flow control. We have one pipe - ethernet, and many protocols inside this pipe with different headers - it is the same here - netlink is such a pipe, and with connector it allows to have different protocols in it. > >With char device I only need to register my callback - with kernel > >connector it is the same, but allows to use the whole power of netlink, > >especially without nice ioctl features like different pointer size > >in userspace and kernelspace. > > You still have to take care of mixed 64/32 bit environments, u64 fields > for example are differently alligned. Connector has a size in it's header - ioctl does not. > >And number of free netlink sockets is _very_ small, especially > >if allocate new one for simple notifications, which can be easily done > >using connector. > > Then fix it so we can use more families and groups. I started some work > on this, but I'm not sure if I have time to complete it. It does not "fix" the "problem" of skb management knowledge, which I described. Netlink is a transport protocol, some general logic must be created on top of it, like it is done in TCP/IP. > >And netlink can be extended to support it - netlink is a transport > >protocol, it should not care about higher layer message handling, > >connector instead will deliver message to the end user in a very > >convenient form. > > You can still built this stuff on top, but the workarounds for netlink > limitations need to be fixed in netlink. I could not call it workaround, I think it is a management layer, which allows : 1. easy usage. Just register a callback and that is all. Callback will be invoced each time new message arrives. No need to dequeue/free/anything. 2. easy usage. Call one function for message delivering, which can care of nonexistent users, perform flow control, congestion control, guarantee delivery and any other. 3. Easily deployable - current implementation is so simple, and it does work with existing netlink. 4. It is logical level on top of transport protocol, it is UDP/IP over ethernet :) > >P.S. I've removed [EMAIL PROTECTED] - please do not add subscribers-only > >private mail lists. > > Wasn't me :) Yep :) -- Evgeniy Polyakov - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info
Re: Netlink connector
* Patrick McHardy <[EMAIL PROTECTED]> 2005-07-26 02:16 > Thomas Graf wrote: > >* Patrick McHardy <[EMAIL PROTECTED]> 2005-07-26 01:46 > > > >>You still have to take care of mixed 64/32 bit environments, u64 fields > >>for example are differently alligned. > > > >My solution to this (in the same patchset) is that we never > >derference u64s but instead copy them. > > I don't understand. The problem is mainly u64 embedded in structures, > the structs have different sizes if the u64 is not 8 byte aligned > and the structure size padded to a multiple of 8. Like in gnet_stats, yes. I thought you meant usages like *(u64 *) which we shouldn't do either. > I started working on it after the OLS party, so no postable code yet :) > The idea for more groups is basically to remove the fixed groups > bitmask from struct sockaddr_nl and use setsockopt to add/remove > multicast subscriptions. If we add the limitation that a packet > can only be multicasted to a single group we can support an arbitary > number of groups, otherwise we would still be limited by size of > skb->cb. I was thinking of subscription messages over netlink itself for the advantage that we could use it within the distributed netlink protocol that has to come up sometime soon. Well, both ways are ok I guess, the ease of distributive usage is my only argument. > This limitation shouldn't be a problem, AFAIK nothing is > multicasting to multiple groups at once right now and the increased > number of groups will allow a better granularity anyway. I'm not aware of any and I agree. We don't need n<->n subscriptions, 1<->n is perfectly fine as I see it. > The main > problem is keeping it backwards-compatible for current netlink users. > If this isn't possible we may need to call it netlink2. I think Jamal has a moral patent on the name netlink2 so be careful ;-> It should be possible to remain compatible, I don't see any unresolveable issues right now. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Netlink connector
Thomas Graf wrote: * Patrick McHardy <[EMAIL PROTECTED]> 2005-07-26 01:46 You still have to take care of mixed 64/32 bit environments, u64 fields for example are differently alligned. My solution to this (in the same patchset) is that we never derference u64s but instead copy them. I don't understand. The problem is mainly u64 embedded in structures, the structs have different sizes if the u64 is not 8 byte aligned and the structure size padded to a multiple of 8. Then fix it so we can use more families and groups. I started some work on this, but I'm not sure if I have time to complete it. Great, this is one of the remaining issues I haven't solved yet. If you want me to take over just hand over your unfinished work and I'll integrate it into my patchset. I started working on it after the OLS party, so no postable code yet :) The idea for more groups is basically to remove the fixed groups bitmask from struct sockaddr_nl and use setsockopt to add/remove multicast subscriptions. If we add the limitation that a packet can only be multicasted to a single group we can support an arbitary number of groups, otherwise we would still be limited by size of skb->cb. This limitation shouldn't be a problem, AFAIK nothing is multicasting to multiple groups at once right now and the increased number of groups will allow a better granularity anyway. The main problem is keeping it backwards-compatible for current netlink users. If this isn't possible we may need to call it netlink2. Regards Patrick - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Netlink connector
* Patrick McHardy <[EMAIL PROTECTED]> 2005-07-26 01:46 > Netlink users don't have to care about shared or cloned skbs. I don't > think its a big issue to use alloc_skb and then the usual netlink > macros. Thomas added a number of macros that simplfiy use a lot. Once I've finished the generic netlink attribute macros the usage will be even simpler. I wrote down all the things I want to do today in a park and I intend to write the code once I'm back from my vacation. > But my main objection is that it sends everything to userspace even > if noone is listening. This can't be used for things that generate > lots of events, and also will get problematic is the number of users > increases. My patches will include a new function netlink_nr_subscribers() taking the socket and a mask of groups. I posted something simliar during an earlier connector discussion already. > You still have to take care of mixed 64/32 bit environments, u64 fields > for example are differently alligned. My solution to this (in the same patchset) is that we never derference u64s but instead copy them. > Then fix it so we can use more families and groups. I started some work > on this, but I'm not sure if I have time to complete it. Great, this is one of the remaining issues I haven't solved yet. If you want me to take over just hand over your unfinished work and I'll integrate it into my patchset. I'm sorry to not being able to provide any code yet, it's one of the first things I'll do once I'm back. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Netlink connector
Evgeniy Polyakov wrote: On Mon, Jul 25, 2005 at 04:32:32PM +0200, Patrick McHardy ([EMAIL PROTECTED]) wrote: If I understand correctly it tries to workaround some netlink limitations (limited number of netlink families and multicast groups) by sending everything to userspace and demultiplexing it there. Same in the other direction, an additional layer on top of netlink does basically the same thing netlink already does. This looks like a step in the wrong direction to me, netlink should instead be fixed to support what is needed. Not only it. The main _first_ idea was to simplify userspace mesasge handling as much as possible. In first releases I called it ioctl-ng - any module that want ot communicate with userspace in the way ioctl does, Usually netlink is easily extendable by using nested TLVs. By hiding this you basically remove this extensibility. requires skb allocation/freeing/handling. Does RTC driver writer need to know what is the difference between shared and cloned skb? Should kernel user of such message bus have to know about skb at all? Netlink users don't have to care about shared or cloned skbs. I don't think its a big issue to use alloc_skb and then the usual netlink macros. Thomas added a number of macros that simplfiy use a lot. But my main objection is that it sends everything to userspace even if noone is listening. This can't be used for things that generate lots of events, and also will get problematic is the number of users increases. With char device I only need to register my callback - with kernel connector it is the same, but allows to use the whole power of netlink, especially without nice ioctl features like different pointer size in userspace and kernelspace. You still have to take care of mixed 64/32 bit environments, u64 fields for example are differently alligned. And number of free netlink sockets is _very_ small, especially if allocate new one for simple notifications, which can be easily done using connector. Then fix it so we can use more families and groups. I started some work on this, but I'm not sure if I have time to complete it. And netlink can be extended to support it - netlink is a transport protocol, it should not care about higher layer message handling, connector instead will deliver message to the end user in a very convenient form. You can still built this stuff on top, but the workarounds for netlink limitations need to be fixed in netlink. P.S. I've removed [EMAIL PROTECTED] - please do not add subscribers-only private mail lists. Wasn't me :) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Netlink connector
On Mon, Jul 25, 2005 at 04:43:43PM +0200, Eric Leblond ([EMAIL PROTECTED]) wrote: > Le lundi 25 juillet 2005 à 16:32 +0200, Patrick McHardy a écrit : > > Evgeniy Polyakov wrote: > > > On Mon, Jul 25, 2005 at 02:02:10AM -0400, James Morris ([EMAIL > > > PROTECTED]) wrote: > > If I understand correctly it tries to workaround some netlink > > limitations (limited number of netlink families and multicast groups) > > by sending everything to userspace and demultiplexing it there. > > Same in the other direction, an additional layer on top of netlink > > does basically the same thing netlink already does. This looks like > > a step in the wrong direction to me, netlink should instead be fixed > > to support what is needed. > > I totally agree with you, it could be great to fix netlink to support > multiple queue. > I like to be able to use projects like snort-inline or nufw together. > This will make Netfilter really stronger. > Furthermore, there's a repetition of filtering capabilities with such a > solution. Netfilter has to filter to send to netlink and this is the > same with the queue dispatcher. I think this introduce too much > complexity. Netlink is transport protocol - no need to add complexity into it, it must be as simple as possible and thus extensible. Multiple queues and filtering should be created on different layer, like it is done for TCP/IP and other protocols. I'm not advertising, but connector is exactly the place where it can be implemented. > my 0.02$ > > BR, > -- > Éric Leblond, [EMAIL PROTECTED] > Téléphone : 01 44 89 46 40, Fax : 01 44 89 45 01 > INL, http://www.inl.fr > -- Evgeniy Polyakov - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Netlink connector
On Mon, Jul 25, 2005 at 04:32:32PM +0200, Patrick McHardy ([EMAIL PROTECTED]) wrote: > Evgeniy Polyakov wrote: > >On Mon, Jul 25, 2005 at 02:02:10AM -0400, James Morris > >([EMAIL PROTECTED]) wrote: > > > >>On Sun, 24 Jul 2005, David S. Miller wrote: > >> > >>>From: Evgeniy Polyakov <[EMAIL PROTECTED]> > >>>Date: Sat, 23 Jul 2005 13:14:55 +0400 > >>> > >>> > Andrew has no objection against connector and it lives in -mm > >>> > >>>A patch sitting in -mm has zero significance. > > > >That is why I'm asking netdev@ people again... > > If I understand correctly it tries to workaround some netlink > limitations (limited number of netlink families and multicast groups) > by sending everything to userspace and demultiplexing it there. > Same in the other direction, an additional layer on top of netlink > does basically the same thing netlink already does. This looks like > a step in the wrong direction to me, netlink should instead be fixed > to support what is needed. Not only it. The main _first_ idea was to simplify userspace mesasge handling as much as possible. In first releases I called it ioctl-ng - any module that want ot communicate with userspace in the way ioctl does, requires skb allocation/freeing/handling. Does RTC driver writer need to know what is the difference between shared and cloned skb? Should kernel user of such message bus have to know about skb at all? With char device I only need to register my callback - with kernel connector it is the same, but allows to use the whole power of netlink, especially without nice ioctl features like different pointer size in userspace and kernelspace. And number of free netlink sockets is _very_ small, especially if allocate new one for simple notifications, which can be easily done using connector. No need to allocate skb, no need to know who are those monsters in header and so on. And netlink can be extended to support it - netlink is a transport protocol, it should not care about higher layer message handling, connector instead will deliver message to the end user in a very convenient form. P.S. I've removed [EMAIL PROTECTED] - please do not add subscribers-only private mail lists. -- Evgeniy Polyakov - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Netlink connector
Le lundi 25 juillet 2005 à 16:32 +0200, Patrick McHardy a écrit : > Evgeniy Polyakov wrote: > > On Mon, Jul 25, 2005 at 02:02:10AM -0400, James Morris ([EMAIL PROTECTED]) > > wrote: > If I understand correctly it tries to workaround some netlink > limitations (limited number of netlink families and multicast groups) > by sending everything to userspace and demultiplexing it there. > Same in the other direction, an additional layer on top of netlink > does basically the same thing netlink already does. This looks like > a step in the wrong direction to me, netlink should instead be fixed > to support what is needed. I totally agree with you, it could be great to fix netlink to support multiple queue. I like to be able to use projects like snort-inline or nufw together. This will make Netfilter really stronger. Furthermore, there's a repetition of filtering capabilities with such a solution. Netfilter has to filter to send to netlink and this is the same with the queue dispatcher. I think this introduce too much complexity. my 0.02$ BR, -- Éric Leblond, [EMAIL PROTECTED] Téléphone : 01 44 89 46 40, Fax : 01 44 89 45 01 INL, http://www.inl.fr - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Netlink connector
Evgeniy Polyakov wrote: On Mon, Jul 25, 2005 at 02:02:10AM -0400, James Morris ([EMAIL PROTECTED]) wrote: On Sun, 24 Jul 2005, David S. Miller wrote: From: Evgeniy Polyakov <[EMAIL PROTECTED]> Date: Sat, 23 Jul 2005 13:14:55 +0400 Andrew has no objection against connector and it lives in -mm A patch sitting in -mm has zero significance. That is why I'm asking netdev@ people again... If I understand correctly it tries to workaround some netlink limitations (limited number of netlink families and multicast groups) by sending everything to userspace and demultiplexing it there. Same in the other direction, an additional layer on top of netlink does basically the same thing netlink already does. This looks like a step in the wrong direction to me, netlink should instead be fixed to support what is needed. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Netlink connector
On Mon, Jul 25, 2005 at 02:02:10AM -0400, James Morris ([EMAIL PROTECTED]) wrote: > On Sun, 24 Jul 2005, David S. Miller wrote: > >From: Evgeniy Polyakov <[EMAIL PROTECTED]> > >Date: Sat, 23 Jul 2005 13:14:55 +0400 > > > >>Andrew has no objection against connector and it lives in -mm > > > >A patch sitting in -mm has zero significance. That is why I'm asking netdev@ people again... > The significance I think is that Andrew is trying to gently encourage some > further progress in the area. > > I recall some netconf discussion about TIPC over Netlink, or more likely a > variation thereof, which may be a better way forward. > > It's cool stuff http://tipc.sourceforge.net/ I read it quite long ago - I'm sure you do not want to use that monster for event bus. It was designed and implemented for heavy intermachine communications, and it is quite hard to setup for userspace <-> kernelspace message bus. > > - James > -- > James Morris > <[EMAIL PROTECTED]> -- Evgeniy Polyakov - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Netlink connector
On Sun, 24 Jul 2005, David S. Miller wrote: From: Evgeniy Polyakov <[EMAIL PROTECTED]> Date: Sat, 23 Jul 2005 13:14:55 +0400 Andrew has no objection against connector and it lives in -mm A patch sitting in -mm has zero significance. The significance I think is that Andrew is trying to gently encourage some further progress in the area. I recall some netconf discussion about TIPC over Netlink, or more likely a variation thereof, which may be a better way forward. It's cool stuff http://tipc.sourceforge.net/ - James -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Netlink connector
On Sun, 24 Jul 2005, David S. Miller wrote: From: Evgeniy Polyakov [EMAIL PROTECTED] Date: Sat, 23 Jul 2005 13:14:55 +0400 Andrew has no objection against connector and it lives in -mm A patch sitting in -mm has zero significance. The significance I think is that Andrew is trying to gently encourage some further progress in the area. I recall some netconf discussion about TIPC over Netlink, or more likely a variation thereof, which may be a better way forward. It's cool stuff http://tipc.sourceforge.net/ - James -- James Morris [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Netlink connector
On Mon, Jul 25, 2005 at 02:02:10AM -0400, James Morris ([EMAIL PROTECTED]) wrote: On Sun, 24 Jul 2005, David S. Miller wrote: From: Evgeniy Polyakov [EMAIL PROTECTED] Date: Sat, 23 Jul 2005 13:14:55 +0400 Andrew has no objection against connector and it lives in -mm A patch sitting in -mm has zero significance. That is why I'm asking netdev@ people again... The significance I think is that Andrew is trying to gently encourage some further progress in the area. I recall some netconf discussion about TIPC over Netlink, or more likely a variation thereof, which may be a better way forward. It's cool stuff http://tipc.sourceforge.net/ I read it quite long ago - I'm sure you do not want to use that monster for event bus. It was designed and implemented for heavy intermachine communications, and it is quite hard to setup for userspace - kernelspace message bus. - James -- James Morris [EMAIL PROTECTED] -- Evgeniy Polyakov - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Netlink connector
Evgeniy Polyakov wrote: On Mon, Jul 25, 2005 at 02:02:10AM -0400, James Morris ([EMAIL PROTECTED]) wrote: On Sun, 24 Jul 2005, David S. Miller wrote: From: Evgeniy Polyakov [EMAIL PROTECTED] Date: Sat, 23 Jul 2005 13:14:55 +0400 Andrew has no objection against connector and it lives in -mm A patch sitting in -mm has zero significance. That is why I'm asking netdev@ people again... If I understand correctly it tries to workaround some netlink limitations (limited number of netlink families and multicast groups) by sending everything to userspace and demultiplexing it there. Same in the other direction, an additional layer on top of netlink does basically the same thing netlink already does. This looks like a step in the wrong direction to me, netlink should instead be fixed to support what is needed. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Netlink connector
Le lundi 25 juillet 2005 à 16:32 +0200, Patrick McHardy a écrit : Evgeniy Polyakov wrote: On Mon, Jul 25, 2005 at 02:02:10AM -0400, James Morris ([EMAIL PROTECTED]) wrote: If I understand correctly it tries to workaround some netlink limitations (limited number of netlink families and multicast groups) by sending everything to userspace and demultiplexing it there. Same in the other direction, an additional layer on top of netlink does basically the same thing netlink already does. This looks like a step in the wrong direction to me, netlink should instead be fixed to support what is needed. I totally agree with you, it could be great to fix netlink to support multiple queue. I like to be able to use projects like snort-inline or nufw together. This will make Netfilter really stronger. Furthermore, there's a repetition of filtering capabilities with such a solution. Netfilter has to filter to send to netlink and this is the same with the queue dispatcher. I think this introduce too much complexity. my 0.02$ BR, -- Éric Leblond, [EMAIL PROTECTED] Téléphone : 01 44 89 46 40, Fax : 01 44 89 45 01 INL, http://www.inl.fr - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Netlink connector
On Mon, Jul 25, 2005 at 04:32:32PM +0200, Patrick McHardy ([EMAIL PROTECTED]) wrote: Evgeniy Polyakov wrote: On Mon, Jul 25, 2005 at 02:02:10AM -0400, James Morris ([EMAIL PROTECTED]) wrote: On Sun, 24 Jul 2005, David S. Miller wrote: From: Evgeniy Polyakov [EMAIL PROTECTED] Date: Sat, 23 Jul 2005 13:14:55 +0400 Andrew has no objection against connector and it lives in -mm A patch sitting in -mm has zero significance. That is why I'm asking netdev@ people again... If I understand correctly it tries to workaround some netlink limitations (limited number of netlink families and multicast groups) by sending everything to userspace and demultiplexing it there. Same in the other direction, an additional layer on top of netlink does basically the same thing netlink already does. This looks like a step in the wrong direction to me, netlink should instead be fixed to support what is needed. Not only it. The main _first_ idea was to simplify userspace mesasge handling as much as possible. In first releases I called it ioctl-ng - any module that want ot communicate with userspace in the way ioctl does, requires skb allocation/freeing/handling. Does RTC driver writer need to know what is the difference between shared and cloned skb? Should kernel user of such message bus have to know about skb at all? With char device I only need to register my callback - with kernel connector it is the same, but allows to use the whole power of netlink, especially without nice ioctl features like different pointer size in userspace and kernelspace. And number of free netlink sockets is _very_ small, especially if allocate new one for simple notifications, which can be easily done using connector. No need to allocate skb, no need to know who are those monsters in header and so on. And netlink can be extended to support it - netlink is a transport protocol, it should not care about higher layer message handling, connector instead will deliver message to the end user in a very convenient form. P.S. I've removed [EMAIL PROTECTED] - please do not add subscribers-only private mail lists. -- Evgeniy Polyakov - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Netlink connector
On Mon, Jul 25, 2005 at 04:43:43PM +0200, Eric Leblond ([EMAIL PROTECTED]) wrote: Le lundi 25 juillet 2005 à 16:32 +0200, Patrick McHardy a écrit : Evgeniy Polyakov wrote: On Mon, Jul 25, 2005 at 02:02:10AM -0400, James Morris ([EMAIL PROTECTED]) wrote: If I understand correctly it tries to workaround some netlink limitations (limited number of netlink families and multicast groups) by sending everything to userspace and demultiplexing it there. Same in the other direction, an additional layer on top of netlink does basically the same thing netlink already does. This looks like a step in the wrong direction to me, netlink should instead be fixed to support what is needed. I totally agree with you, it could be great to fix netlink to support multiple queue. I like to be able to use projects like snort-inline or nufw together. This will make Netfilter really stronger. Furthermore, there's a repetition of filtering capabilities with such a solution. Netfilter has to filter to send to netlink and this is the same with the queue dispatcher. I think this introduce too much complexity. Netlink is transport protocol - no need to add complexity into it, it must be as simple as possible and thus extensible. Multiple queues and filtering should be created on different layer, like it is done for TCP/IP and other protocols. I'm not advertising, but connector is exactly the place where it can be implemented. my 0.02$ BR, -- Éric Leblond, [EMAIL PROTECTED] Téléphone : 01 44 89 46 40, Fax : 01 44 89 45 01 INL, http://www.inl.fr -- Evgeniy Polyakov - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Netlink connector
Evgeniy Polyakov wrote: On Mon, Jul 25, 2005 at 04:32:32PM +0200, Patrick McHardy ([EMAIL PROTECTED]) wrote: If I understand correctly it tries to workaround some netlink limitations (limited number of netlink families and multicast groups) by sending everything to userspace and demultiplexing it there. Same in the other direction, an additional layer on top of netlink does basically the same thing netlink already does. This looks like a step in the wrong direction to me, netlink should instead be fixed to support what is needed. Not only it. The main _first_ idea was to simplify userspace mesasge handling as much as possible. In first releases I called it ioctl-ng - any module that want ot communicate with userspace in the way ioctl does, Usually netlink is easily extendable by using nested TLVs. By hiding this you basically remove this extensibility. requires skb allocation/freeing/handling. Does RTC driver writer need to know what is the difference between shared and cloned skb? Should kernel user of such message bus have to know about skb at all? Netlink users don't have to care about shared or cloned skbs. I don't think its a big issue to use alloc_skb and then the usual netlink macros. Thomas added a number of macros that simplfiy use a lot. But my main objection is that it sends everything to userspace even if noone is listening. This can't be used for things that generate lots of events, and also will get problematic is the number of users increases. With char device I only need to register my callback - with kernel connector it is the same, but allows to use the whole power of netlink, especially without nice ioctl features like different pointer size in userspace and kernelspace. You still have to take care of mixed 64/32 bit environments, u64 fields for example are differently alligned. And number of free netlink sockets is _very_ small, especially if allocate new one for simple notifications, which can be easily done using connector. Then fix it so we can use more families and groups. I started some work on this, but I'm not sure if I have time to complete it. And netlink can be extended to support it - netlink is a transport protocol, it should not care about higher layer message handling, connector instead will deliver message to the end user in a very convenient form. You can still built this stuff on top, but the workarounds for netlink limitations need to be fixed in netlink. P.S. I've removed [EMAIL PROTECTED] - please do not add subscribers-only private mail lists. Wasn't me :) - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Netlink connector
* Patrick McHardy [EMAIL PROTECTED] 2005-07-26 01:46 Netlink users don't have to care about shared or cloned skbs. I don't think its a big issue to use alloc_skb and then the usual netlink macros. Thomas added a number of macros that simplfiy use a lot. Once I've finished the generic netlink attribute macros the usage will be even simpler. I wrote down all the things I want to do today in a park and I intend to write the code once I'm back from my vacation. But my main objection is that it sends everything to userspace even if noone is listening. This can't be used for things that generate lots of events, and also will get problematic is the number of users increases. My patches will include a new function netlink_nr_subscribers() taking the socket and a mask of groups. I posted something simliar during an earlier connector discussion already. You still have to take care of mixed 64/32 bit environments, u64 fields for example are differently alligned. My solution to this (in the same patchset) is that we never derference u64s but instead copy them. Then fix it so we can use more families and groups. I started some work on this, but I'm not sure if I have time to complete it. Great, this is one of the remaining issues I haven't solved yet. If you want me to take over just hand over your unfinished work and I'll integrate it into my patchset. I'm sorry to not being able to provide any code yet, it's one of the first things I'll do once I'm back. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Netlink connector
Thomas Graf wrote: * Patrick McHardy [EMAIL PROTECTED] 2005-07-26 01:46 You still have to take care of mixed 64/32 bit environments, u64 fields for example are differently alligned. My solution to this (in the same patchset) is that we never derference u64s but instead copy them. I don't understand. The problem is mainly u64 embedded in structures, the structs have different sizes if the u64 is not 8 byte aligned and the structure size padded to a multiple of 8. Then fix it so we can use more families and groups. I started some work on this, but I'm not sure if I have time to complete it. Great, this is one of the remaining issues I haven't solved yet. If you want me to take over just hand over your unfinished work and I'll integrate it into my patchset. I started working on it after the OLS party, so no postable code yet :) The idea for more groups is basically to remove the fixed groups bitmask from struct sockaddr_nl and use setsockopt to add/remove multicast subscriptions. If we add the limitation that a packet can only be multicasted to a single group we can support an arbitary number of groups, otherwise we would still be limited by size of skb-cb. This limitation shouldn't be a problem, AFAIK nothing is multicasting to multiple groups at once right now and the increased number of groups will allow a better granularity anyway. The main problem is keeping it backwards-compatible for current netlink users. If this isn't possible we may need to call it netlink2. Regards Patrick - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Netlink connector
* Patrick McHardy [EMAIL PROTECTED] 2005-07-26 02:16 Thomas Graf wrote: * Patrick McHardy [EMAIL PROTECTED] 2005-07-26 01:46 You still have to take care of mixed 64/32 bit environments, u64 fields for example are differently alligned. My solution to this (in the same patchset) is that we never derference u64s but instead copy them. I don't understand. The problem is mainly u64 embedded in structures, the structs have different sizes if the u64 is not 8 byte aligned and the structure size padded to a multiple of 8. Like in gnet_stats, yes. I thought you meant usages like *(u64 *) which we shouldn't do either. I started working on it after the OLS party, so no postable code yet :) The idea for more groups is basically to remove the fixed groups bitmask from struct sockaddr_nl and use setsockopt to add/remove multicast subscriptions. If we add the limitation that a packet can only be multicasted to a single group we can support an arbitary number of groups, otherwise we would still be limited by size of skb-cb. I was thinking of subscription messages over netlink itself for the advantage that we could use it within the distributed netlink protocol that has to come up sometime soon. Well, both ways are ok I guess, the ease of distributive usage is my only argument. This limitation shouldn't be a problem, AFAIK nothing is multicasting to multiple groups at once right now and the increased number of groups will allow a better granularity anyway. I'm not aware of any and I agree. We don't need n-n subscriptions, 1-n is perfectly fine as I see it. The main problem is keeping it backwards-compatible for current netlink users. If this isn't possible we may need to call it netlink2. I think Jamal has a moral patent on the name netlink2 so be careful ;- It should be possible to remain compatible, I don't see any unresolveable issues right now. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Netlink connector
On Tue, Jul 26, 2005 at 01:46:04AM +0200, Patrick McHardy ([EMAIL PROTECTED]) wrote: Evgeniy Polyakov wrote: On Mon, Jul 25, 2005 at 04:32:32PM +0200, Patrick McHardy ([EMAIL PROTECTED]) wrote: If I understand correctly it tries to workaround some netlink limitations (limited number of netlink families and multicast groups) by sending everything to userspace and demultiplexing it there. Same in the other direction, an additional layer on top of netlink does basically the same thing netlink already does. This looks like a step in the wrong direction to me, netlink should instead be fixed to support what is needed. Not only it. The main _first_ idea was to simplify userspace mesasge handling as much as possible. In first releases I called it ioctl-ng - any module that want ot communicate with userspace in the way ioctl does, Usually netlink is easily extendable by using nested TLVs. By hiding this you basically remove this extensibility. Current netlink is not extensible for _many_ different users. It has only 32 sockets. requires skb allocation/freeing/handling. Does RTC driver writer need to know what is the difference between shared and cloned skb? Should kernel user of such message bus have to know about skb at all? Netlink users don't have to care about shared or cloned skbs. I don't think its a big issue to use alloc_skb and then the usual netlink macros. Thomas added a number of macros that simplfiy use a lot. Kernel user also must know about difference between unicast/broadcast, how to dequeue the skb, how to free it and in what context. ioctl users do not need to know how file_operations is bound to file. But my main objection is that it sends everything to userspace even if noone is listening. This can't be used for things that generate lots of events, and also will get problematic is the number of users increases. It is a problem for existing netlink - either check in bind time, what could be done for connector, or in socket creation time. Actually it is not even a problem, since checking is being done, but after allocation and message filling, such check can be moved into cn_netlink_send() in connector, but different netlink users, who prefers to use different sockets, must perform it by itself in each place, where skb is allocated... Connector is a solution for current situation, it can be deployed with few casualties. Creating a new netlink2 socket for device, which wants to replace ioctl controlling or broadcast it's state is a wrong way. Different sockets/flows does not allow easy flow control. We have one pipe - ethernet, and many protocols inside this pipe with different headers - it is the same here - netlink is such a pipe, and with connector it allows to have different protocols in it. With char device I only need to register my callback - with kernel connector it is the same, but allows to use the whole power of netlink, especially without nice ioctl features like different pointer size in userspace and kernelspace. You still have to take care of mixed 64/32 bit environments, u64 fields for example are differently alligned. Connector has a size in it's header - ioctl does not. And number of free netlink sockets is _very_ small, especially if allocate new one for simple notifications, which can be easily done using connector. Then fix it so we can use more families and groups. I started some work on this, but I'm not sure if I have time to complete it. It does not fix the problem of skb management knowledge, which I described. Netlink is a transport protocol, some general logic must be created on top of it, like it is done in TCP/IP. And netlink can be extended to support it - netlink is a transport protocol, it should not care about higher layer message handling, connector instead will deliver message to the end user in a very convenient form. You can still built this stuff on top, but the workarounds for netlink limitations need to be fixed in netlink. I could not call it workaround, I think it is a management layer, which allows : 1. easy usage. Just register a callback and that is all. Callback will be invoced each time new message arrives. No need to dequeue/free/anything. 2. easy usage. Call one function for message delivering, which can care of nonexistent users, perform flow control, congestion control, guarantee delivery and any other. 3. Easily deployable - current implementation is so simple, and it does work with existing netlink. 4. It is logical level on top of transport protocol, it is UDP/IP over ethernet :) P.S. I've removed [EMAIL PROTECTED] - please do not add subscribers-only private mail lists. Wasn't me :) Yep :) -- Evgeniy Polyakov - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Netlink connector
Evgeniy Polyakov wrote: On Tue, Jul 26, 2005 at 01:46:04AM +0200, Patrick McHardy ([EMAIL PROTECTED]) wrote: Evgeniy Polyakov wrote: On Mon, Jul 25, 2005 at 04:32:32PM +0200, Patrick McHardy ([EMAIL PROTECTED]) wrote: If I understand correctly it tries to workaround some netlink limitations (limited number of netlink families and multicast groups) by sending everything to userspace and demultiplexing it there. Same in the other direction, an additional layer on top of netlink does basically the same thing netlink already does. This looks like a step in the wrong direction to me, netlink should instead be fixed to support what is needed. Not only it. The main _first_ idea was to simplify userspace mesasge handling as much as possible. In first releases I called it ioctl-ng - any module that want ot communicate with userspace in the way ioctl does, Usually netlink is easily extendable by using nested TLVs. By hiding this you basically remove this extensibility. Current netlink is not extensible for _many_ different users. It has only 32 sockets. requires skb allocation/freeing/handling. Does RTC driver writer need to know what is the difference between shared and cloned skb? Should kernel user of such message bus have to know about skb at all? Netlink users don't have to care about shared or cloned skbs. I don't think its a big issue to use alloc_skb and then the usual netlink macros. Thomas added a number of macros that simplfiy use a lot. Kernel user also must know about difference between unicast/broadcast, how to dequeue the skb, how to free it and in what context. ioctl users do not need to know how file_operations is bound to file. But my main objection is that it sends everything to userspace even if noone is listening. This can't be used for things that generate lots of events, and also will get problematic is the number of users increases. It is a problem for existing netlink - either check in bind time, what could be done for connector, or in socket creation time. Actually it is not even a problem, since checking is being done, but after allocation and message filling, such check can be moved into cn_netlink_send() in connector, but different netlink users, who prefers to use different sockets, must perform it by itself in each place, where skb is allocated... Connector is a solution for current situation, it can be deployed with few casualties. Creating a new netlink2 socket for device, which wants to replace ioctl controlling or broadcast it's state is a wrong way. Different sockets/flows does not allow easy flow control. We have one pipe - ethernet, and many protocols inside this pipe with different headers - it is the same here - netlink is such a pipe, and with connector it allows to have different protocols in it. With char device I only need to register my callback - with kernel connector it is the same, but allows to use the whole power of netlink, especially without nice ioctl features like different pointer size in userspace and kernelspace. You still have to take care of mixed 64/32 bit environments, u64 fields for example are differently alligned. Connector has a size in it's header - ioctl does not. And number of free netlink sockets is _very_ small, especially if allocate new one for simple notifications, which can be easily done using connector. Then fix it so we can use more families and groups. I started some work on this, but I'm not sure if I have time to complete it. It does not fix the problem of skb management knowledge, which I described. Netlink is a transport protocol, some general logic must be created on top of it, like it is done in TCP/IP. And netlink can be extended to support it - netlink is a transport protocol, it should not care about higher layer message handling, connector instead will deliver message to the end user in a very convenient form. You can still built this stuff on top, but the workarounds for netlink limitations need to be fixed in netlink. I could not call it workaround, I think it is a management layer, which allows : 1. easy usage. Just register a callback and that is all. Callback will be invoced each time new message arrives. No need to dequeue/free/anything. 2. easy usage. Call one function for message delivering, which can care of nonexistent users, perform flow control, congestion control, guarantee delivery and any other. 3. Easily deployable - current implementation is so simple, and it does work with existing netlink. 4. It is logical level on top of transport protocol, it is UDP/IP over ethernet :) If it is a transport, then it should be in the kernel. Otherwise, it becomes painful for applications with multiple input sources. Think of epoll/poll/select and threads, doing the demultiplexing in user space would be a pain for applications and libraries. The other way to go is to
Re: Netlink connector
On Mon, Jul 25, 2005 at 09:56:56PM -0700, Stephen Hemminger ([EMAIL PROTECTED]) wrote: Evgeniy Polyakov wrote: On Tue, Jul 26, 2005 at 01:46:04AM +0200, Patrick McHardy ([EMAIL PROTECTED]) wrote: Evgeniy Polyakov wrote: On Mon, Jul 25, 2005 at 04:32:32PM +0200, Patrick McHardy ([EMAIL PROTECTED]) wrote: If I understand correctly it tries to workaround some netlink limitations (limited number of netlink families and multicast groups) by sending everything to userspace and demultiplexing it there. Same in the other direction, an additional layer on top of netlink does basically the same thing netlink already does. This looks like a step in the wrong direction to me, netlink should instead be fixed to support what is needed. Not only it. The main _first_ idea was to simplify userspace mesasge handling as much as possible. In first releases I called it ioctl-ng - any module that want ot communicate with userspace in the way ioctl does, Usually netlink is easily extendable by using nested TLVs. By hiding this you basically remove this extensibility. Current netlink is not extensible for _many_ different users. It has only 32 sockets. requires skb allocation/freeing/handling. Does RTC driver writer need to know what is the difference between shared and cloned skb? Should kernel user of such message bus have to know about skb at all? Netlink users don't have to care about shared or cloned skbs. I don't think its a big issue to use alloc_skb and then the usual netlink macros. Thomas added a number of macros that simplfiy use a lot. Kernel user also must know about difference between unicast/broadcast, how to dequeue the skb, how to free it and in what context. ioctl users do not need to know how file_operations is bound to file. But my main objection is that it sends everything to userspace even if noone is listening. This can't be used for things that generate lots of events, and also will get problematic is the number of users increases. It is a problem for existing netlink - either check in bind time, what could be done for connector, or in socket creation time. Actually it is not even a problem, since checking is being done, but after allocation and message filling, such check can be moved into cn_netlink_send() in connector, but different netlink users, who prefers to use different sockets, must perform it by itself in each place, where skb is allocated... Connector is a solution for current situation, it can be deployed with few casualties. Creating a new netlink2 socket for device, which wants to replace ioctl controlling or broadcast it's state is a wrong way. Different sockets/flows does not allow easy flow control. We have one pipe - ethernet, and many protocols inside this pipe with different headers - it is the same here - netlink is such a pipe, and with connector it allows to have different protocols in it. With char device I only need to register my callback - with kernel connector it is the same, but allows to use the whole power of netlink, especially without nice ioctl features like different pointer size in userspace and kernelspace. You still have to take care of mixed 64/32 bit environments, u64 fields for example are differently alligned. Connector has a size in it's header - ioctl does not. And number of free netlink sockets is _very_ small, especially if allocate new one for simple notifications, which can be easily done using connector. Then fix it so we can use more families and groups. I started some work on this, but I'm not sure if I have time to complete it. It does not fix the problem of skb management knowledge, which I described. Netlink is a transport protocol, some general logic must be created on top of it, like it is done in TCP/IP. And netlink can be extended to support it - netlink is a transport protocol, it should not care about higher layer message handling, connector instead will deliver message to the end user in a very convenient form. You can still built this stuff on top, but the workarounds for netlink limitations need to be fixed in netlink. I could not call it workaround, I think it is a management layer, which allows : 1. easy usage. Just register a callback and that is all. Callback will be invoced each time new message arrives. No need to dequeue/free/anything. 2. easy usage. Call one function for message delivering, which can care of nonexistent users, perform flow control, congestion control, guarantee delivery and any other. 3. Easily deployable - current implementation is so simple, and it does work with existing netlink. 4. It is logical level on top of transport protocol, it is UDP/IP over ethernet :) If it is a transport, then it should be in the kernel. Otherwise, it becomes
Re: Netlink Connector / CBUS
On Tue, 5 Apr 2005, Evgeniy Polyakov wrote: > On Tue, 2005-04-05 at 01:05 -0400, James Morris wrote: > > Evgeniy, > > > > Please send networking patches to [EMAIL PROTECTED] > > It was sent there two times. I googled around quite a lot to track the origin of the patches down but didn't do the obvious and look in the netdev archives, sorry. - James -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Netlink Connector / CBUS
On Tue, 2005-04-05 at 07:00 -0400, jamal wrote:
> and, oh yeah - wheres the documentation Evgeniy? ;->
In the tree :)
Documentation/connector/connector.txt - some notes, API.
Documentation/connector/cn_test.c - kernel example.
Uses cn_netlink_send(), notification feature.
I will send today a pathc that adds in-source documentation
bits with some code cleanups.
> cheers,
> jamal
>
> On Tue, 2005-04-05 at 06:44, jamal wrote:
> > To be fair to Evgeniy I am not against the Konnector idea. I think that
> > it is a useful feature to have an easy to use messaging between
> > kernel-kernel and kernel-userspace. The fact that he leveraged netlink
> > instead of inventing things is a bonus. Having said that i have not
> > seriously scrutinized the code - and i think the idea of this new thing
> > hes tossing around called CBUS maybe pushing it.
> >
> > cheers,
> > jamal
> >
> > On Tue, 2005-04-05 at 03:34, Evgeniy Polyakov wrote:
> > > On Tue, 2005-04-05 at 01:10 -0400, Herbert Xu wrote:
> > > >On Tue, Apr 05, 2005 at 11:03:16AM +0400, Evgeniy Polyakov wrote:
> > > >>
> > > >> I received comments and feature requests from Herbert Xu and Jamal Hadi
> > > >> Salim,
> > > >> almost all were successfully resolved.
> > > >
> > > >Please do not construe my involvement in these threads as endorsement
> > > >for this system.
> > >
> > > Sure.
> > > I remember you are against it :).
> > >
> > > >In fact to this day I still don't understand what problems this thing is
> > > >meant to solve.
> > >
> > > Hmm, what else can I add to my words?
> > > May be checking the size of the code needed to broadcast kobject changes
> > > in kobject_uevent.c for example...
> > > Netlink socket allocation + skb handling against call to
> > > cn_netlink_send().
> > >
> > > >--
> > > >Visit Openswan at http://www.openswan.org/
> > > >Email: Herbert Xu ~{PmV>HI~} <[EMAIL PROTECTED]>
> > > >Home Page: http://gondor.apana.org.au/~herbert/
> > > >PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
> > >
> >
> >
> >
--
Evgeniy Polyakov
Crash is better than data corruption -- Arthur Grabowski
signature.asc
Description: This is a digitally signed message part
Re: Netlink Connector / CBUS
and, oh yeah - wheres the documentation Evgeniy? ;->
cheers,
jamal
On Tue, 2005-04-05 at 06:44, jamal wrote:
> To be fair to Evgeniy I am not against the Konnector idea. I think that
> it is a useful feature to have an easy to use messaging between
> kernel-kernel and kernel-userspace. The fact that he leveraged netlink
> instead of inventing things is a bonus. Having said that i have not
> seriously scrutinized the code - and i think the idea of this new thing
> hes tossing around called CBUS maybe pushing it.
>
> cheers,
> jamal
>
> On Tue, 2005-04-05 at 03:34, Evgeniy Polyakov wrote:
> > On Tue, 2005-04-05 at 01:10 -0400, Herbert Xu wrote:
> > >On Tue, Apr 05, 2005 at 11:03:16AM +0400, Evgeniy Polyakov wrote:
> > >>
> > >> I received comments and feature requests from Herbert Xu and Jamal Hadi
> > >> Salim,
> > >> almost all were successfully resolved.
> > >
> > >Please do not construe my involvement in these threads as endorsement
> > >for this system.
> >
> > Sure.
> > I remember you are against it :).
> >
> > >In fact to this day I still don't understand what problems this thing is
> > >meant to solve.
> >
> > Hmm, what else can I add to my words?
> > May be checking the size of the code needed to broadcast kobject changes
> > in kobject_uevent.c for example...
> > Netlink socket allocation + skb handling against call to cn_netlink_send().
> >
> > >--
> > >Visit Openswan at http://www.openswan.org/
> > >Email: Herbert Xu ~{PmV>HI~} <[EMAIL PROTECTED]>
> > >Home Page: http://gondor.apana.org.au/~herbert/
> > >PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
> >
>
>
>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Netlink Connector / CBUS
To be fair to Evgeniy I am not against the Konnector idea. I think that
it is a useful feature to have an easy to use messaging between
kernel-kernel and kernel-userspace. The fact that he leveraged netlink
instead of inventing things is a bonus. Having said that i have not
seriously scrutinized the code - and i think the idea of this new thing
hes tossing around called CBUS maybe pushing it.
cheers,
jamal
On Tue, 2005-04-05 at 03:34, Evgeniy Polyakov wrote:
> On Tue, 2005-04-05 at 01:10 -0400, Herbert Xu wrote:
> >On Tue, Apr 05, 2005 at 11:03:16AM +0400, Evgeniy Polyakov wrote:
> >>
> >> I received comments and feature requests from Herbert Xu and Jamal Hadi
> >> Salim,
> >> almost all were successfully resolved.
> >
> >Please do not construe my involvement in these threads as endorsement
> >for this system.
>
> Sure.
> I remember you are against it :).
>
> >In fact to this day I still don't understand what problems this thing is
> >meant to solve.
>
> Hmm, what else can I add to my words?
> May be checking the size of the code needed to broadcast kobject changes
> in kobject_uevent.c for example...
> Netlink socket allocation + skb handling against call to cn_netlink_send().
>
> >--
> >Visit Openswan at http://www.openswan.org/
> >Email: Herbert Xu ~{PmV>HI~} <[EMAIL PROTECTED]>
> >Home Page: http://gondor.apana.org.au/~herbert/
> >PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Netlink Connector / CBUS
On Tue, 2005-04-05 at 11:34 +0400, Evgeniy Polyakov wrote: > On Tue, 2005-04-05 at 01:10 -0400, Herbert Xu wrote: > > >In fact to this day I still don't understand what problems this thing is > >meant to solve. > > Hmm, what else can I add to my words? > May be checking the size of the code needed to broadcast kobject changes > in kobject_uevent.c for example... > Netlink socket allocation + skb handling against call to cn_netlink_send(). And It's the same for the fork connector. It allows to send a message to a user space application when a fork occurs by adding only one line (two with the #include) in the kernel/fork.c file. Thus, the netlink connector is a very simple and fast mechanism when you need to send a small amount of information from kernel space to user space. Regards, Guillaume - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Netlink Connector / CBUS
On Tue, 2005-04-05 at 01:10 -0400, Herbert Xu wrote:
>On Tue, Apr 05, 2005 at 11:03:16AM +0400, Evgeniy Polyakov wrote:
>>
>> I received comments and feature requests from Herbert Xu and Jamal Hadi
>> Salim,
>> almost all were successfully resolved.
>
>Please do not construe my involvement in these threads as endorsement
>for this system.
Sure.
I remember you are against it :).
>In fact to this day I still don't understand what problems this thing is
>meant to solve.
Hmm, what else can I add to my words?
May be checking the size of the code needed to broadcast kobject changes
in kobject_uevent.c for example...
Netlink socket allocation + skb handling against call to cn_netlink_send().
>--
>Visit Openswan at http://www.openswan.org/
>Email: Herbert Xu ~{PmV>HI~} <[EMAIL PROTECTED]>
>Home Page: http://gondor.apana.org.au/~herbert/
>PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
Evgeniy Polyakov
Crash is better than data corruption -- Arthur Grabowski
signature.asc
Description: This is a digitally signed message part
Re: Netlink Connector / CBUS
On Tue, Apr 05, 2005 at 11:03:16AM +0400, Evgeniy Polyakov wrote:
>
> I received comments and feature requests from Herbert Xu and Jamal Hadi
> Salim,
> almost all were successfully resolved.
Please do not construe my involvement in these threads as endorsement
for this system.
In fact to this day I still don't understand what problems this thing is
meant to solve.
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[EMAIL PROTECTED]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Netlink Connector / CBUS
On Tue, 2005-04-05 at 01:10 -0400, James Morris wrote: > On Tue, 5 Apr 2005, James Morris wrote: > > > A few questions: > > Also, please allow cn_add_callback() allow it to be passed a NULL > callback function, so the caller doesn't pass in a dummy function and your > code doesn't waste time dealing with something which isn't real. Why can anyone want to add callback that will not supposed to be usefull? Callback is called when someone sends netlink message with appropriate idx/val inside, if there is no registered callback with such ID, nothing will be called and skb will be freed. > > - James -- Evgeniy Polyakov Crash is better than data corruption -- Arthur Grabowski signature.asc Description: This is a digitally signed message part
Re: Netlink Connector / CBUS
On Tue, 2005-04-05 at 01:05 -0400, James Morris wrote:
> Evgeniy,
>
> Please send networking patches to [EMAIL PROTECTED]
It was sent there two times.
> Your connector code (under drivers/connector) is now in the -mm tree and
> as far as I can tell, has not received any review from the network
> developers.
I received comments and feature requests from Herbert Xu and Jamal Hadi
Salim,
almost all were successfully resolved.
> Looking at it briefly, it seems quite unfinished.
Hmmm...
I think it is fully functional and ready for inclusion.
> I'm not entirely sure what it's purpose is.
1. Provide very flexible userspace control over netlink.
2. Provide very flexible notification mechanism.
> A clear explanation of its purpose would be helpful (to me, at least), as
> well as documentation of the API and majore data structures (which akpm
> has also asked for, IIRC).
Documentation exists in Documentation/connector/connector.txt.
Patch with brief source documentation was already created,
so I will post it with other minor updates soon.
> I can see one example of where it's being used with kobject_uevent, and it
> seems to have arrived via Greg-KH's I2C tree...
It also is used in SuperIO and acrypto subsystems.
> If you're trying to add a generic, psuedo-reliable Netlink communication
> system, perhaps this should be built into Netlink itself as an extension
> of the existing Netlink API.
So, you recommend to create for each driver, that wants to be controlled
over netlink, new netlink socket, register it's unit and learn
how SKB is allocated, processed and so on?
This is wrong.
Much easier to just register a callback.
> I don't think this should be done as a separate "driver" off somewhere
> else with a new API.
It is much easier to use connector instead of direct netlink sockets.
One should only register callback and identifier. When driver receives
special netlink message with appropriate identifier, appropriate
callback will be called.
From the userspace point of view it's quite straightforward:
socket();
bind();
send();
recv();
But if kernelspace want to use full power of such connections, driver
writer must create special sockets, must know about struct sk_buff
handling...
Connector allows any kernelspace agents to use netlink based
networking for inter-process communication in a significantly easier
way:
int cn_add_callback(struct cb_id *id, char *name, void (*callback) (void
*));
void cn_netlink_send(struct cn_msg *msg, u32 __groups);
>
> A few questions:
>
> - Why does it by default use NETLINK_NFLOG a kernel socket, and also allow
> this to be overriden by a module parameter?
Because while this driver lived outside kernel tree there were no empty
registered socket.
It can be changed if driver will go upstream.
> - Why does the cn.o module (poor namespace choice) add a callback itself
> on initialization?
Because that callback is used for notification requests.
> - Where is the userspace code which uses this? I checked out dbus from
> cvs and couldn't see anything obvious.
I posted it with SuperIO, kobject_uevent, acrypto and fork changes.
It is quite straightforward:
s = socket(PF_NETLINK, SOCK_DGRAM, NETLINK_NFLOG);
if (s == -1) {
perror("socket");
return -1;
}
l_local.nl_family = AF_NETLINK;
l_local.nl_groups = CN_ACRYPTO_IDX;
l_local.nl_pid = getpid();
if (bind(s, (struct sockaddr *)_local, sizeof(struct
sockaddr_nl)) == -1) {
perror("bind");
close(s);
return -1;
}
case NLMSG_DONE:
data = (struct cn_msg *)NLMSG_DATA(reply);
m = (struct crypto_conn_data *)(data + 1);
stat = (struct crypto_device_stat *)(m+1);
time();
fprintf(out,
"%.24s : [%x.%x] [seq=%u, ack=%u], name=
%s, cmd=%#02x, "
"sesions: completed=%llu, started=%llu,
finished=%llu, cache_failed=%llu.\n",
ctime(), data->id.idx, data->id.val,
data->seq, data->ack, m->name, m->cmd,
stat->scompleted, stat->sstarted, stat-
>sfinished, stat->cache_failed);
fflush(out);
break;
>
> Thanks,
Thank you for your comments.
>
> - James
--
Evgeniy Polyakov
Crash is better than data corruption -- Arthur Grabowski
signature.asc
Description: This is a digitally signed message part
Re: Netlink Connector / CBUS
On Tue, 5 Apr 2005, Evgeniy Polyakov wrote: On Tue, 2005-04-05 at 01:05 -0400, James Morris wrote: Evgeniy, Please send networking patches to [EMAIL PROTECTED] It was sent there two times. I googled around quite a lot to track the origin of the patches down but didn't do the obvious and look in the netdev archives, sorry. - James -- James Morris [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Netlink Connector / CBUS
On Tue, 2005-04-05 at 01:05 -0400, James Morris wrote:
Evgeniy,
Please send networking patches to [EMAIL PROTECTED]
It was sent there two times.
Your connector code (under drivers/connector) is now in the -mm tree and
as far as I can tell, has not received any review from the network
developers.
I received comments and feature requests from Herbert Xu and Jamal Hadi
Salim,
almost all were successfully resolved.
Looking at it briefly, it seems quite unfinished.
Hmmm...
I think it is fully functional and ready for inclusion.
I'm not entirely sure what it's purpose is.
1. Provide very flexible userspace control over netlink.
2. Provide very flexible notification mechanism.
A clear explanation of its purpose would be helpful (to me, at least), as
well as documentation of the API and majore data structures (which akpm
has also asked for, IIRC).
Documentation exists in Documentation/connector/connector.txt.
Patch with brief source documentation was already created,
so I will post it with other minor updates soon.
I can see one example of where it's being used with kobject_uevent, and it
seems to have arrived via Greg-KH's I2C tree...
It also is used in SuperIO and acrypto subsystems.
If you're trying to add a generic, psuedo-reliable Netlink communication
system, perhaps this should be built into Netlink itself as an extension
of the existing Netlink API.
So, you recommend to create for each driver, that wants to be controlled
over netlink, new netlink socket, register it's unit and learn
how SKB is allocated, processed and so on?
This is wrong.
Much easier to just register a callback.
I don't think this should be done as a separate driver off somewhere
else with a new API.
It is much easier to use connector instead of direct netlink sockets.
One should only register callback and identifier. When driver receives
special netlink message with appropriate identifier, appropriate
callback will be called.
From the userspace point of view it's quite straightforward:
socket();
bind();
send();
recv();
But if kernelspace want to use full power of such connections, driver
writer must create special sockets, must know about struct sk_buff
handling...
Connector allows any kernelspace agents to use netlink based
networking for inter-process communication in a significantly easier
way:
int cn_add_callback(struct cb_id *id, char *name, void (*callback) (void
*));
void cn_netlink_send(struct cn_msg *msg, u32 __groups);
A few questions:
- Why does it by default use NETLINK_NFLOG a kernel socket, and also allow
this to be overriden by a module parameter?
Because while this driver lived outside kernel tree there were no empty
registered socket.
It can be changed if driver will go upstream.
- Why does the cn.o module (poor namespace choice) add a callback itself
on initialization?
Because that callback is used for notification requests.
- Where is the userspace code which uses this? I checked out dbus from
cvs and couldn't see anything obvious.
I posted it with SuperIO, kobject_uevent, acrypto and fork changes.
It is quite straightforward:
s = socket(PF_NETLINK, SOCK_DGRAM, NETLINK_NFLOG);
if (s == -1) {
perror(socket);
return -1;
}
l_local.nl_family = AF_NETLINK;
l_local.nl_groups = CN_ACRYPTO_IDX;
l_local.nl_pid = getpid();
if (bind(s, (struct sockaddr *)l_local, sizeof(struct
sockaddr_nl)) == -1) {
perror(bind);
close(s);
return -1;
}
case NLMSG_DONE:
data = (struct cn_msg *)NLMSG_DATA(reply);
m = (struct crypto_conn_data *)(data + 1);
stat = (struct crypto_device_stat *)(m+1);
time(tm);
fprintf(out,
%.24s : [%x.%x] [seq=%u, ack=%u], name=
%s, cmd=%#02x,
sesions: completed=%llu, started=%llu,
finished=%llu, cache_failed=%llu.\n,
ctime(tm), data-id.idx, data-id.val,
data-seq, data-ack, m-name, m-cmd,
stat-scompleted, stat-sstarted, stat-
sfinished, stat-cache_failed);
fflush(out);
break;
Thanks,
Thank you for your comments.
- James
--
Evgeniy Polyakov
Crash is better than data corruption -- Arthur Grabowski
signature.asc
Description: This is a digitally signed message part
Re: Netlink Connector / CBUS
On Tue, 2005-04-05 at 01:10 -0400, James Morris wrote: On Tue, 5 Apr 2005, James Morris wrote: A few questions: Also, please allow cn_add_callback() allow it to be passed a NULL callback function, so the caller doesn't pass in a dummy function and your code doesn't waste time dealing with something which isn't real. Why can anyone want to add callback that will not supposed to be usefull? Callback is called when someone sends netlink message with appropriate idx/val inside, if there is no registered callback with such ID, nothing will be called and skb will be freed. - James -- Evgeniy Polyakov Crash is better than data corruption -- Arthur Grabowski signature.asc Description: This is a digitally signed message part
Re: Netlink Connector / CBUS
On Tue, Apr 05, 2005 at 11:03:16AM +0400, Evgeniy Polyakov wrote:
I received comments and feature requests from Herbert Xu and Jamal Hadi
Salim,
almost all were successfully resolved.
Please do not construe my involvement in these threads as endorsement
for this system.
In fact to this day I still don't understand what problems this thing is
meant to solve.
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmVHI~} [EMAIL PROTECTED]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Netlink Connector / CBUS
On Tue, 2005-04-05 at 01:10 -0400, Herbert Xu wrote:
On Tue, Apr 05, 2005 at 11:03:16AM +0400, Evgeniy Polyakov wrote:
I received comments and feature requests from Herbert Xu and Jamal Hadi
Salim,
almost all were successfully resolved.
Please do not construe my involvement in these threads as endorsement
for this system.
Sure.
I remember you are against it :).
In fact to this day I still don't understand what problems this thing is
meant to solve.
Hmm, what else can I add to my words?
May be checking the size of the code needed to broadcast kobject changes
in kobject_uevent.c for example...
Netlink socket allocation + skb handling against call to cn_netlink_send().
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmVHI~} [EMAIL PROTECTED]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
Evgeniy Polyakov
Crash is better than data corruption -- Arthur Grabowski
signature.asc
Description: This is a digitally signed message part
Re: Netlink Connector / CBUS
On Tue, 2005-04-05 at 11:34 +0400, Evgeniy Polyakov wrote: On Tue, 2005-04-05 at 01:10 -0400, Herbert Xu wrote: In fact to this day I still don't understand what problems this thing is meant to solve. Hmm, what else can I add to my words? May be checking the size of the code needed to broadcast kobject changes in kobject_uevent.c for example... Netlink socket allocation + skb handling against call to cn_netlink_send(). And It's the same for the fork connector. It allows to send a message to a user space application when a fork occurs by adding only one line (two with the #include) in the kernel/fork.c file. Thus, the netlink connector is a very simple and fast mechanism when you need to send a small amount of information from kernel space to user space. Regards, Guillaume - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Netlink Connector / CBUS
To be fair to Evgeniy I am not against the Konnector idea. I think that
it is a useful feature to have an easy to use messaging between
kernel-kernel and kernel-userspace. The fact that he leveraged netlink
instead of inventing things is a bonus. Having said that i have not
seriously scrutinized the code - and i think the idea of this new thing
hes tossing around called CBUS maybe pushing it.
cheers,
jamal
On Tue, 2005-04-05 at 03:34, Evgeniy Polyakov wrote:
On Tue, 2005-04-05 at 01:10 -0400, Herbert Xu wrote:
On Tue, Apr 05, 2005 at 11:03:16AM +0400, Evgeniy Polyakov wrote:
I received comments and feature requests from Herbert Xu and Jamal Hadi
Salim,
almost all were successfully resolved.
Please do not construe my involvement in these threads as endorsement
for this system.
Sure.
I remember you are against it :).
In fact to this day I still don't understand what problems this thing is
meant to solve.
Hmm, what else can I add to my words?
May be checking the size of the code needed to broadcast kobject changes
in kobject_uevent.c for example...
Netlink socket allocation + skb handling against call to cn_netlink_send().
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmVHI~} [EMAIL PROTECTED]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Netlink Connector / CBUS
and, oh yeah - wheres the documentation Evgeniy? ;-
cheers,
jamal
On Tue, 2005-04-05 at 06:44, jamal wrote:
To be fair to Evgeniy I am not against the Konnector idea. I think that
it is a useful feature to have an easy to use messaging between
kernel-kernel and kernel-userspace. The fact that he leveraged netlink
instead of inventing things is a bonus. Having said that i have not
seriously scrutinized the code - and i think the idea of this new thing
hes tossing around called CBUS maybe pushing it.
cheers,
jamal
On Tue, 2005-04-05 at 03:34, Evgeniy Polyakov wrote:
On Tue, 2005-04-05 at 01:10 -0400, Herbert Xu wrote:
On Tue, Apr 05, 2005 at 11:03:16AM +0400, Evgeniy Polyakov wrote:
I received comments and feature requests from Herbert Xu and Jamal Hadi
Salim,
almost all were successfully resolved.
Please do not construe my involvement in these threads as endorsement
for this system.
Sure.
I remember you are against it :).
In fact to this day I still don't understand what problems this thing is
meant to solve.
Hmm, what else can I add to my words?
May be checking the size of the code needed to broadcast kobject changes
in kobject_uevent.c for example...
Netlink socket allocation + skb handling against call to cn_netlink_send().
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmVHI~} [EMAIL PROTECTED]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Netlink Connector / CBUS
On Tue, 2005-04-05 at 07:00 -0400, jamal wrote:
and, oh yeah - wheres the documentation Evgeniy? ;-
In the tree :)
Documentation/connector/connector.txt - some notes, API.
Documentation/connector/cn_test.c - kernel example.
Uses cn_netlink_send(), notification feature.
I will send today a pathc that adds in-source documentation
bits with some code cleanups.
cheers,
jamal
On Tue, 2005-04-05 at 06:44, jamal wrote:
To be fair to Evgeniy I am not against the Konnector idea. I think that
it is a useful feature to have an easy to use messaging between
kernel-kernel and kernel-userspace. The fact that he leveraged netlink
instead of inventing things is a bonus. Having said that i have not
seriously scrutinized the code - and i think the idea of this new thing
hes tossing around called CBUS maybe pushing it.
cheers,
jamal
On Tue, 2005-04-05 at 03:34, Evgeniy Polyakov wrote:
On Tue, 2005-04-05 at 01:10 -0400, Herbert Xu wrote:
On Tue, Apr 05, 2005 at 11:03:16AM +0400, Evgeniy Polyakov wrote:
I received comments and feature requests from Herbert Xu and Jamal Hadi
Salim,
almost all were successfully resolved.
Please do not construe my involvement in these threads as endorsement
for this system.
Sure.
I remember you are against it :).
In fact to this day I still don't understand what problems this thing is
meant to solve.
Hmm, what else can I add to my words?
May be checking the size of the code needed to broadcast kobject changes
in kobject_uevent.c for example...
Netlink socket allocation + skb handling against call to
cn_netlink_send().
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmVHI~} [EMAIL PROTECTED]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
Evgeniy Polyakov
Crash is better than data corruption -- Arthur Grabowski
signature.asc
Description: This is a digitally signed message part
Re: Netlink Connector / CBUS
On Tue, 5 Apr 2005, James Morris wrote: > A few questions: Also, please allow cn_add_callback() allow it to be passed a NULL callback function, so the caller doesn't pass in a dummy function and your code doesn't waste time dealing with something which isn't real. - James -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Netlink Connector / CBUS
Evgeniy, Please send networking patches to [EMAIL PROTECTED] Your connector code (under drivers/connector) is now in the -mm tree and as far as I can tell, has not received any review from the network developers. Looking at it briefly, it seems quite unfinished. I'm not entirely sure what it's purpose is. A clear explanation of its purpose would be helpful (to me, at least), as well as documentation of the API and majore data structures (which akpm has also asked for, IIRC). I can see one example of where it's being used with kobject_uevent, and it seems to have arrived via Greg-KH's I2C tree... If you're trying to add a generic, psuedo-reliable Netlink communication system, perhaps this should be built into Netlink itself as an extension of the existing Netlink API. I don't think this should be done as a separate "driver" off somewhere else with a new API. A few questions: - Why does it by default use NETLINK_NFLOG a kernel socket, and also allow this to be overriden by a module parameter? - Why does the cn.o module (poor namespace choice) add a callback itself on initialization? - Where is the userspace code which uses this? I checked out dbus from cvs and couldn't see anything obvious. Thanks, - James -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Netlink Connector / CBUS
Evgeniy, Please send networking patches to [EMAIL PROTECTED] Your connector code (under drivers/connector) is now in the -mm tree and as far as I can tell, has not received any review from the network developers. Looking at it briefly, it seems quite unfinished. I'm not entirely sure what it's purpose is. A clear explanation of its purpose would be helpful (to me, at least), as well as documentation of the API and majore data structures (which akpm has also asked for, IIRC). I can see one example of where it's being used with kobject_uevent, and it seems to have arrived via Greg-KH's I2C tree... If you're trying to add a generic, psuedo-reliable Netlink communication system, perhaps this should be built into Netlink itself as an extension of the existing Netlink API. I don't think this should be done as a separate driver off somewhere else with a new API. A few questions: - Why does it by default use NETLINK_NFLOG a kernel socket, and also allow this to be overriden by a module parameter? - Why does the cn.o module (poor namespace choice) add a callback itself on initialization? - Where is the userspace code which uses this? I checked out dbus from cvs and couldn't see anything obvious. Thanks, - James -- James Morris [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Netlink Connector / CBUS
On Tue, 5 Apr 2005, James Morris wrote: A few questions: Also, please allow cn_add_callback() allow it to be passed a NULL callback function, so the caller doesn't pass in a dummy function and your code doesn't waste time dealing with something which isn't real. - James -- James Morris [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
