* Re: Re: [v4 PATCH 1/6] IPVS: Change of socket usage to enable name space exit.
From: Simon Horman @ 2011-05-02 12:26 UTC (permalink / raw)
To: Hans Schillstrom
Cc: ja, ebiederm, lvs-devel, netdev, netfilter-devel,
hans.schillstrom
In-Reply-To: <kqgtk9j.d83e4dced2edb2820c7e6329f634bb3f@obelix.schillstrom.com>
On Mon, May 02, 2011 at 01:40:04PM +0200, Hans Schillstrom wrote:
> >Sent: Mon, May 2, 2011, 13:03 PM
> >Subject: Re: [v4 PATCH 1/6] IPVS: Change of socket usage to enable name space exit.
> >
> >On Sun, May 01, 2011 at 06:50:13PM +0200, Hans Schillstrom wrote:
> >> To work this patch also needs the other patches in this series.
> >
> >Hi Hans,
> >
> >could you explain the implications statement above?
> >What kind of behaviour might I expect if only this patch is applied?
>
> Oops, left there by misstake.
> Can you remove that line before pushing it ?
Will do.
> >I'm planning to push this and 2/6 for 2.6.39, and as we have
> >discussed (offline) that combination works. But from a bisection
> >point of view, this patch needs to not "blow up" if used by itself.
>
> It should not be any bisect. problems from what I can see.
Thanks
^ permalink raw reply
* [PATCH 2/2] IPVS: labels at pos 0
From: Simon Horman @ 2011-05-02 12:47 UTC (permalink / raw)
To: lvs-devel, netdev, netfilter-devel, netfilter
Cc: Wensong Zhang, Julian Anastasov, Patrick McHardy,
Hans Schillstrom, Hans Schillstrom, Eric W. Biederman,
Simon Horman
In-Reply-To: <1304340447-21857-1-git-send-email-horms@verge.net.au>
From: Hans Schillstrom <hans@schillstrom.com>
Put goto labels at the beginig of row
acording to coding style example.
Signed-off-by: Hans Schillstrom <hans.schillstrom@ericsson.com>
Signed-off-by: Hans Schillstrom <hans@schillstrom.com>
Acked-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
net/netfilter/ipvs/ip_vs_core.c | 10 +++++-----
net/netfilter/ipvs/ip_vs_ctl.c | 8 ++++----
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
index a0791dc..d536b51 100644
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -1388,7 +1388,7 @@ ip_vs_in_icmp(struct sk_buff *skb, int *related, unsigned int hooknum)
verdict = NF_DROP;
}
- out:
+out:
__ip_vs_conn_put(cp);
return verdict;
@@ -1955,14 +1955,14 @@ static int __init ip_vs_init(void)
cleanup_sync:
ip_vs_sync_cleanup();
- cleanup_conn:
+cleanup_conn:
ip_vs_conn_cleanup();
- cleanup_app:
+cleanup_app:
ip_vs_app_cleanup();
- cleanup_protocol:
+cleanup_protocol:
ip_vs_protocol_cleanup();
ip_vs_control_cleanup();
- cleanup_estimator:
+cleanup_estimator:
ip_vs_estimator_cleanup();
unregister_pernet_subsys(&ipvs_core_ops); /* free ip_vs struct */
return ret;
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index ae47090..a31a70c 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -1327,9 +1327,9 @@ ip_vs_edit_service(struct ip_vs_service *svc, struct ip_vs_service_user_kern *u)
ip_vs_bind_pe(svc, pe);
}
- out_unlock:
+out_unlock:
write_unlock_bh(&__ip_vs_svc_lock);
- out:
+out:
ip_vs_scheduler_put(old_sched);
ip_vs_pe_put(old_pe);
return ret;
@@ -2387,7 +2387,7 @@ __ip_vs_get_service_entries(struct net *net,
count++;
}
}
- out:
+out:
return ret;
}
@@ -2625,7 +2625,7 @@ do_ip_vs_get_ctl(struct sock *sk, int cmd, void __user *user, int *len)
ret = -EINVAL;
}
- out:
+out:
mutex_unlock(&__ip_vs_mutex);
return ret;
}
--
1.7.4.1
^ permalink raw reply related
* [GIT PULL nf-2.6] IPVS
From: Simon Horman @ 2011-05-02 12:47 UTC (permalink / raw)
To: lvs-devel, netdev, netfilter-devel, netfilter
Cc: Wensong Zhang, Julian Anastasov, Patrick McHardy,
Hans Schillstrom, Hans Schillstrom, Eric W. Biederman
Hi Patrick,
please consider pulling
git://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs-2.6.git master
to get the following fix from Hans. They resolve some problems related
to his netns for IPVS work which was incorporated into 2.6.39-rc1.
The pull request is based on nf-2.6/master.
There are other less-pressing changes from Hans which
I plan to get you to pull into nf-next-2.6 once these
changes make it there (presumably via net-2.6 and then net-next-2.6).
Hans Schillstrom (2):
IPVS: Change of socket usage to enable name space exit.
IPVS: labels at pos 0
net/netfilter/ipvs/ip_vs_core.c | 12 ++++----
net/netfilter/ipvs/ip_vs_ctl.c | 8 +++---
net/netfilter/ipvs/ip_vs_sync.c | 58 +++++++++++++++++++++++++--------------
3 files changed, 47 insertions(+), 31 deletions(-)
^ permalink raw reply
* [PATCH 1/2] IPVS: Change of socket usage to enable name space exit.
From: Simon Horman @ 2011-05-02 12:47 UTC (permalink / raw)
To: lvs-devel, netdev, netfilter-devel, netfilter
Cc: Wensong Zhang, Julian Anastasov, Patrick McHardy,
Hans Schillstrom, Hans Schillstrom, Eric W. Biederman,
Simon Horman
In-Reply-To: <1304340447-21857-1-git-send-email-horms@verge.net.au>
From: Hans Schillstrom <hans@schillstrom.com>
If the sync daemons run in a name space while it crashes
or get killed, there is no way to stop them except for a reboot.
When all patches are there, ip_vs_core will handle register_pernet_(),
i.e. ip_vs_sync_init() and ip_vs_sync_cleanup() will be removed.
Kernel threads should not increment the use count of a socket.
By calling sk_change_net() after creating a socket this is avoided.
sock_release cant be used intead sk_release_kernel() should be used.
Thanks Eric W Biederman for your advices.
This patch is based on net-next-2.6 ver 2.6.39-rc2
Signed-off-by: Hans Schillstrom <hans.schillstrom@ericsson.com>
Signed-off-by: Hans Schillstrom <hans@schillstrom.com>
(minor edits to description)
Signed-off-by: Simon Horman <horms@verge.net.au>
---
net/netfilter/ipvs/ip_vs_core.c | 2 +-
net/netfilter/ipvs/ip_vs_sync.c | 58 +++++++++++++++++++++++++--------------
2 files changed, 38 insertions(+), 22 deletions(-)
diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
index 07accf6..a0791dc 100644
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -1896,7 +1896,7 @@ static int __net_init __ip_vs_init(struct net *net)
static void __net_exit __ip_vs_cleanup(struct net *net)
{
- IP_VS_DBG(10, "ipvs netns %d released\n", net_ipvs(net)->gen);
+ IP_VS_DBG(2, "ipvs netns %d released\n", net_ipvs(net)->gen);
}
static struct pernet_operations ipvs_core_ops = {
diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
index 3e7961e..0cce953 100644
--- a/net/netfilter/ipvs/ip_vs_sync.c
+++ b/net/netfilter/ipvs/ip_vs_sync.c
@@ -1303,13 +1303,18 @@ static struct socket *make_send_sock(struct net *net)
struct socket *sock;
int result;
- /* First create a socket */
- result = __sock_create(net, PF_INET, SOCK_DGRAM, IPPROTO_UDP, &sock, 1);
+ /* First create a socket move it to right name space later */
+ result = sock_create_kern(PF_INET, SOCK_DGRAM, IPPROTO_UDP, &sock);
if (result < 0) {
pr_err("Error during creation of socket; terminating\n");
return ERR_PTR(result);
}
-
+ /*
+ * Kernel sockets that are a part of a namespace, should not
+ * hold a reference to a namespace in order to allow to stop it.
+ * After sk_change_net should be released using sk_release_kernel.
+ */
+ sk_change_net(sock->sk, net);
result = set_mcast_if(sock->sk, ipvs->master_mcast_ifn);
if (result < 0) {
pr_err("Error setting outbound mcast interface\n");
@@ -1334,8 +1339,8 @@ static struct socket *make_send_sock(struct net *net)
return sock;
- error:
- sock_release(sock);
+error:
+ sk_release_kernel(sock->sk);
return ERR_PTR(result);
}
@@ -1350,12 +1355,17 @@ static struct socket *make_receive_sock(struct net *net)
int result;
/* First create a socket */
- result = __sock_create(net, PF_INET, SOCK_DGRAM, IPPROTO_UDP, &sock, 1);
+ result = sock_create_kern(PF_INET, SOCK_DGRAM, IPPROTO_UDP, &sock);
if (result < 0) {
pr_err("Error during creation of socket; terminating\n");
return ERR_PTR(result);
}
-
+ /*
+ * Kernel sockets that are a part of a namespace, should not
+ * hold a reference to a namespace in order to allow to stop it.
+ * After sk_change_net should be released using sk_release_kernel.
+ */
+ sk_change_net(sock->sk, net);
/* it is equivalent to the REUSEADDR option in user-space */
sock->sk->sk_reuse = 1;
@@ -1377,8 +1387,8 @@ static struct socket *make_receive_sock(struct net *net)
return sock;
- error:
- sock_release(sock);
+error:
+ sk_release_kernel(sock->sk);
return ERR_PTR(result);
}
@@ -1473,7 +1483,7 @@ static int sync_thread_master(void *data)
ip_vs_sync_buff_release(sb);
/* release the sending multicast socket */
- sock_release(tinfo->sock);
+ sk_release_kernel(tinfo->sock->sk);
kfree(tinfo);
return 0;
@@ -1513,7 +1523,7 @@ static int sync_thread_backup(void *data)
}
/* release the sending multicast socket */
- sock_release(tinfo->sock);
+ sk_release_kernel(tinfo->sock->sk);
kfree(tinfo->buf);
kfree(tinfo);
@@ -1601,7 +1611,7 @@ outtinfo:
outbuf:
kfree(buf);
outsocket:
- sock_release(sock);
+ sk_release_kernel(sock->sk);
out:
return result;
}
@@ -1610,6 +1620,7 @@ out:
int stop_sync_thread(struct net *net, int state)
{
struct netns_ipvs *ipvs = net_ipvs(net);
+ int retc = -EINVAL;
IP_VS_DBG(7, "%s(): pid %d\n", __func__, task_pid_nr(current));
@@ -1629,7 +1640,7 @@ int stop_sync_thread(struct net *net, int state)
spin_lock_bh(&ipvs->sync_lock);
ipvs->sync_state &= ~IP_VS_STATE_MASTER;
spin_unlock_bh(&ipvs->sync_lock);
- kthread_stop(ipvs->master_thread);
+ retc = kthread_stop(ipvs->master_thread);
ipvs->master_thread = NULL;
} else if (state == IP_VS_STATE_BACKUP) {
if (!ipvs->backup_thread)
@@ -1639,16 +1650,14 @@ int stop_sync_thread(struct net *net, int state)
task_pid_nr(ipvs->backup_thread));
ipvs->sync_state &= ~IP_VS_STATE_BACKUP;
- kthread_stop(ipvs->backup_thread);
+ retc = kthread_stop(ipvs->backup_thread);
ipvs->backup_thread = NULL;
- } else {
- return -EINVAL;
}
/* decrease the module use count */
ip_vs_use_count_dec();
- return 0;
+ return retc;
}
/*
@@ -1670,8 +1679,15 @@ static int __net_init __ip_vs_sync_init(struct net *net)
static void __ip_vs_sync_cleanup(struct net *net)
{
- stop_sync_thread(net, IP_VS_STATE_MASTER);
- stop_sync_thread(net, IP_VS_STATE_BACKUP);
+ int retc;
+
+ retc = stop_sync_thread(net, IP_VS_STATE_MASTER);
+ if (retc && retc != -ESRCH)
+ pr_err("Failed to stop Master Daemon\n");
+
+ retc = stop_sync_thread(net, IP_VS_STATE_BACKUP);
+ if (retc && retc != -ESRCH)
+ pr_err("Failed to stop Backup Daemon\n");
}
static struct pernet_operations ipvs_sync_ops = {
@@ -1682,10 +1698,10 @@ static struct pernet_operations ipvs_sync_ops = {
int __init ip_vs_sync_init(void)
{
- return register_pernet_subsys(&ipvs_sync_ops);
+ return register_pernet_device(&ipvs_sync_ops);
}
void ip_vs_sync_cleanup(void)
{
- unregister_pernet_subsys(&ipvs_sync_ops);
+ unregister_pernet_device(&ipvs_sync_ops);
}
--
1.7.4.1
^ permalink raw reply related
* Re: [GIT PULL nf-2.6] IPVS
From: Simon Horman @ 2011-05-02 13:03 UTC (permalink / raw)
To: lvs-devel, netdev, netfilter-devel, netfilter
Cc: Wensong Zhang, Julian Anastasov, Patrick McHardy,
Hans Schillstrom, Hans Schillstrom, Eric W. Biederman
In-Reply-To: <1304340447-21857-1-git-send-email-horms@verge.net.au>
On Mon, May 02, 2011 at 09:47:25PM +0900, Simon Horman wrote:
>
> Hi Patrick,
>
> please consider pulling
> git://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs-2.6.git master
> to get the following fix from Hans. They resolve some problems related
> to his netns for IPVS work which was incorporated into 2.6.39-rc1.
Sorry, please ignore this.
The second patch is one of the non-pressing ones.
I'll fix things up and repost.
> The pull request is based on nf-2.6/master.
>
> There are other less-pressing changes from Hans which
> I plan to get you to pull into nf-next-2.6 once these
> changes make it there (presumably via net-2.6 and then net-next-2.6).
>
> Hans Schillstrom (2):
> IPVS: Change of socket usage to enable name space exit.
> IPVS: labels at pos 0
>
> net/netfilter/ipvs/ip_vs_core.c | 12 ++++----
> net/netfilter/ipvs/ip_vs_ctl.c | 8 +++---
> net/netfilter/ipvs/ip_vs_sync.c | 58 +++++++++++++++++++++++++--------------
> 3 files changed, 47 insertions(+), 31 deletions(-)
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH] linux/stmmac.h: include <linux/platform_device.h> to remove compilation warning.
From: Giuseppe CAVALLARO @ 2011-05-02 12:46 UTC (permalink / raw)
To: Viresh Kumar
Cc: netdev, shiraz.hashim, armando.visconti, deepak.sikri,
viresh.linux
In-Reply-To: <2239eca1d15014be7b6d94f883f9492dc1fbc60a.1304317552.git.viresh.kumar@st.com>
On 5/2/2011 8:30 AM, Viresh Kumar wrote:
> stmmac.h uses struct platform_device and doesn't include
> <linux/platform_device.h>. And so we get following compilation warning while
> using this file:
> warning: ‘struct platform_device’ declared inside parameter list
>
> This patch includes <linux/platform_device.h> in stmmac.h to remove this warning
Hi Viresh
thanks for the patch that looks good for me.
We could also remove this inclusion (see commit
1f0f63885658889b3bcb8a08fbcb9532f8e536c9) from
drivers/net/stmmac/stmmac.h and keep it in linux/stmmac.h as you suggested.
What do you think?
Regards
Peppe
> Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
> ---
> include/linux/stmmac.h | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
> index 73d9b4e..d7dfe7d 100644
> --- a/include/linux/stmmac.h
> +++ b/include/linux/stmmac.h
> @@ -26,6 +26,8 @@
> #ifndef __STMMAC_PLATFORM_DATA
> #define __STMMAC_PLATFORM_DATA
>
> +#include <linux/platform_device.h>
> +
> #define STMAC_TYPE_0 0
> #define STMAC_TYPE_1 1
> #define STMAC_TYPE_2 2
^ permalink raw reply
* Re: [PATCH v4 5/5] iproute2: add can-j1939 support
From: Marc Kleine-Budde @ 2011-05-02 13:21 UTC (permalink / raw)
To: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20110427090302.GF757-ozGf4kBk5synFtIcQ8t7k3L8HoS0Hn3T@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 1000 bytes --]
On 04/27/2011 11:03 AM, Kurt Van Dijck wrote:
> Add j1939 support to iproute2
>
> Signed-off-by: Kurt Van Dijck <kurt.van.dijck-/BeEPy95v10@public.gmane.org>
> ---
> diff --git a/Makefile b/Makefile
> index d1ace1f..06bf281 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -27,9 +27,12 @@ ADDLIB+=dnet_ntop.o dnet_pton.o
> #options for ipx
> ADDLIB+=ipx_ntop.o ipx_pton.o
>
> +#options for j1939
> +ADDLIB+=j1939.o
> +
> CC = gcc
> HOSTCC = gcc
> -CCOPTS = -D_GNU_SOURCE -O2 -Wstrict-prototypes -Wall
^^^^
why do you remove -O2?
> +CCOPTS = -D_GNU_SOURCE -Wstrict-prototypes -Wall
> CFLAGS = $(CCOPTS) -I../include $(DEFINES)
> YACCFLAGS = -d -t -v
cheers, Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
[-- Attachment #2: Type: text/plain, Size: 188 bytes --]
_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core
^ permalink raw reply
* Re: [PATCH v4 5/5] iproute2: add can-j1939 support
From: Kurt Van Dijck @ 2011-05-02 13:36 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4DBEAFC6.7010308-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
On Mon, May 02, 2011 at 03:21:10PM +0200, Marc Kleine-Budde wrote:
> On 04/27/2011 11:03 AM, Kurt Van Dijck wrote:
> > Add j1939 support to iproute2
> >
> > Signed-off-by: Kurt Van Dijck <kurt.van.dijck-/BeEPy95v10@public.gmane.org>
> > -CCOPTS = -D_GNU_SOURCE -O2 -Wstrict-prototypes -Wall
> ^^^^
>
> why do you remove -O2?
I did -O0 for debugging, but must have forgotten to properly restore.
I need to fix this for a definite version.
Thanks,
Kurt
^ permalink raw reply
* [PATCH] can: fix SJA1000 dlc for RTR packets
From: Kurt Van Dijck @ 2011-05-02 14:50 UTC (permalink / raw)
To: socketcan-core, netdev
RTR frames do have a valid data length code on CAN.
The driver for SJA1000 did not handle that situation properly.
Signed-off-by: Kurt Van Dijck <kurt.van.dijck@eia.be>
---
drivers/net/can/sja1000/sja1000.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/can/sja1000/sja1000.c b/drivers/net/can/sja1000/sja1000.c
index a358ea9..f501bba 100644
--- a/drivers/net/can/sja1000/sja1000.c
+++ b/drivers/net/can/sja1000/sja1000.c
@@ -346,10 +346,10 @@ static void sja1000_rx(struct net_device *dev)
| (priv->read_reg(priv, REG_ID2) >> 5);
}
+ cf->can_dlc = get_can_dlc(fi & 0x0F);
if (fi & FI_RTR) {
id |= CAN_RTR_FLAG;
} else {
- cf->can_dlc = get_can_dlc(fi & 0x0F);
for (i = 0; i < cf->can_dlc; i++)
cf->data[i] = priv->read_reg(priv, dreg++);
}
^ permalink raw reply related
* Re: [PATCH] can: fix SJA1000 dlc for RTR packets
From: Marc Kleine-Budde @ 2011-05-02 15:15 UTC (permalink / raw)
To: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20110502145048.GF338-MxZ6Iy/zr/UdbCeoMzGj59i2O/JbrIOy@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 1520 bytes --]
On 05/02/2011 04:50 PM, Kurt Van Dijck wrote:
> RTR frames do have a valid data length code on CAN.
> The driver for SJA1000 did not handle that situation properly.
Looks good!
>
> Signed-off-by: Kurt Van Dijck <kurt.van.dijck-/BeEPy95v10@public.gmane.org>
Acked-by: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
> drivers/net/can/sja1000/sja1000.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/can/sja1000/sja1000.c b/drivers/net/can/sja1000/sja1000.c
> index a358ea9..f501bba 100644
> --- a/drivers/net/can/sja1000/sja1000.c
> +++ b/drivers/net/can/sja1000/sja1000.c
> @@ -346,10 +346,10 @@ static void sja1000_rx(struct net_device *dev)
> | (priv->read_reg(priv, REG_ID2) >> 5);
> }
>
> + cf->can_dlc = get_can_dlc(fi & 0x0F);
> if (fi & FI_RTR) {
> id |= CAN_RTR_FLAG;
> } else {
> - cf->can_dlc = get_can_dlc(fi & 0x0F);
> for (i = 0; i < cf->can_dlc; i++)
> cf->data[i] = priv->read_reg(priv, dreg++);
> }
> _______________________________________________
> Socketcan-core mailing list
> Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
> https://lists.berlios.de/mailman/listinfo/socketcan-core
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
[-- Attachment #2: Type: text/plain, Size: 188 bytes --]
_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core
^ permalink raw reply
* pull request: wireless-2.6 2011-05-02
From: John W. Linville @ 2011-05-02 18:20 UTC (permalink / raw)
To: davem; +Cc: linux-wireless, netdev, linux-kernel
Dave,
Here are a few more intended for 2.6.39. There is a one-liner for b43
which adds a missing MODULE_FIRMWARE declaration (so maybe someone's
initrd will get generated correctly), an iwlegacy fix from Wey-Yi that
corrects an LED flashing change that people are complaining about,
an iwl4965 WARNING fix from Stanislaw, and a matching pair of iwlagn
and iwl4965 regression fixes also from Stanislaw that correct problems
with using the wrong station ID for block ACK sessions.
Please let me know if there are problems!
John
P.S. I know the LED flashing fix is questionable, but it is an
annoyance and it should not effect any other hardware.
---
The following changes since commit 7cfd260910b881250cde76ba92ebe3cbf8493a8f:
ipv4: don't spam dmesg with "Using LC-trie" messages (2011-05-01 23:17:50 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-2.6.git master
Rafał Miłecki (1):
b43: trivial: update module info about ucode16_mimo firmware
Stanislaw Gruszka (3):
iwl4965: fix "TX Power requested while scanning"
iwlagn: fix "Received BA when not expected"
iwl4965: fix "Received BA when not expected"
Wey-Yi Guy (1):
iwlegacy: led stay solid on when no traffic
drivers/net/wireless/b43/main.c | 1 +
drivers/net/wireless/iwlegacy/iwl-4965-tx.c | 18 ++++++++++++------
drivers/net/wireless/iwlegacy/iwl-led.c | 20 +++++++++++++++++++-
drivers/net/wireless/iwlegacy/iwl4965-base.c | 8 ++++----
drivers/net/wireless/iwlwifi/iwl-agn-tx.c | 17 +++++++++++------
5 files changed, 47 insertions(+), 17 deletions(-)
diff --git a/drivers/net/wireless/b43/main.c b/drivers/net/wireless/b43/main.c
index 57eb5b6..7841b95 100644
--- a/drivers/net/wireless/b43/main.c
+++ b/drivers/net/wireless/b43/main.c
@@ -72,6 +72,7 @@ MODULE_FIRMWARE("b43/ucode11.fw");
MODULE_FIRMWARE("b43/ucode13.fw");
MODULE_FIRMWARE("b43/ucode14.fw");
MODULE_FIRMWARE("b43/ucode15.fw");
+MODULE_FIRMWARE("b43/ucode16_mimo.fw");
MODULE_FIRMWARE("b43/ucode5.fw");
MODULE_FIRMWARE("b43/ucode9.fw");
diff --git a/drivers/net/wireless/iwlegacy/iwl-4965-tx.c b/drivers/net/wireless/iwlegacy/iwl-4965-tx.c
index fbec88d..79ac081 100644
--- a/drivers/net/wireless/iwlegacy/iwl-4965-tx.c
+++ b/drivers/net/wireless/iwlegacy/iwl-4965-tx.c
@@ -316,12 +316,18 @@ int iwl4965_tx_skb(struct iwl_priv *priv, struct sk_buff *skb)
hdr_len = ieee80211_hdrlen(fc);
- /* Find index into station table for destination station */
- sta_id = iwl_legacy_sta_id_or_broadcast(priv, ctx, info->control.sta);
- if (sta_id == IWL_INVALID_STATION) {
- IWL_DEBUG_DROP(priv, "Dropping - INVALID STATION: %pM\n",
- hdr->addr1);
- goto drop_unlock;
+ /* For management frames use broadcast id to do not break aggregation */
+ if (!ieee80211_is_data(fc))
+ sta_id = ctx->bcast_sta_id;
+ else {
+ /* Find index into station table for destination station */
+ sta_id = iwl_legacy_sta_id_or_broadcast(priv, ctx, info->control.sta);
+
+ if (sta_id == IWL_INVALID_STATION) {
+ IWL_DEBUG_DROP(priv, "Dropping - INVALID STATION: %pM\n",
+ hdr->addr1);
+ goto drop_unlock;
+ }
}
IWL_DEBUG_TX(priv, "station Id %d\n", sta_id);
diff --git a/drivers/net/wireless/iwlegacy/iwl-led.c b/drivers/net/wireless/iwlegacy/iwl-led.c
index 15eb8b7..bda0d61 100644
--- a/drivers/net/wireless/iwlegacy/iwl-led.c
+++ b/drivers/net/wireless/iwlegacy/iwl-led.c
@@ -48,8 +48,21 @@ module_param(led_mode, int, S_IRUGO);
MODULE_PARM_DESC(led_mode, "0=system default, "
"1=On(RF On)/Off(RF Off), 2=blinking");
+/* Throughput OFF time(ms) ON time (ms)
+ * >300 25 25
+ * >200 to 300 40 40
+ * >100 to 200 55 55
+ * >70 to 100 65 65
+ * >50 to 70 75 75
+ * >20 to 50 85 85
+ * >10 to 20 95 95
+ * >5 to 10 110 110
+ * >1 to 5 130 130
+ * >0 to 1 167 167
+ * <=0 SOLID ON
+ */
static const struct ieee80211_tpt_blink iwl_blink[] = {
- { .throughput = 0 * 1024 - 1, .blink_time = 334 },
+ { .throughput = 0, .blink_time = 334 },
{ .throughput = 1 * 1024 - 1, .blink_time = 260 },
{ .throughput = 5 * 1024 - 1, .blink_time = 220 },
{ .throughput = 10 * 1024 - 1, .blink_time = 190 },
@@ -101,6 +114,11 @@ static int iwl_legacy_led_cmd(struct iwl_priv *priv,
if (priv->blink_on == on && priv->blink_off == off)
return 0;
+ if (off == 0) {
+ /* led is SOLID_ON */
+ on = IWL_LED_SOLID;
+ }
+
IWL_DEBUG_LED(priv, "Led blink time compensation=%u\n",
priv->cfg->base_params->led_compensation);
led_cmd.on = iwl_legacy_blink_compensation(priv, on,
diff --git a/drivers/net/wireless/iwlegacy/iwl4965-base.c b/drivers/net/wireless/iwlegacy/iwl4965-base.c
index d484c36..a62fe24 100644
--- a/drivers/net/wireless/iwlegacy/iwl4965-base.c
+++ b/drivers/net/wireless/iwlegacy/iwl4965-base.c
@@ -2984,15 +2984,15 @@ static void iwl4965_bg_txpower_work(struct work_struct *work)
struct iwl_priv *priv = container_of(work, struct iwl_priv,
txpower_work);
+ mutex_lock(&priv->mutex);
+
/* If a scan happened to start before we got here
* then just return; the statistics notification will
* kick off another scheduled work to compensate for
* any temperature delta we missed here. */
if (test_bit(STATUS_EXIT_PENDING, &priv->status) ||
test_bit(STATUS_SCANNING, &priv->status))
- return;
-
- mutex_lock(&priv->mutex);
+ goto out;
/* Regardless of if we are associated, we must reconfigure the
* TX power since frames can be sent on non-radar channels while
@@ -3002,7 +3002,7 @@ static void iwl4965_bg_txpower_work(struct work_struct *work)
/* Update last_temperature to keep is_calib_needed from running
* when it isn't needed... */
priv->last_temperature = priv->temperature;
-
+out:
mutex_unlock(&priv->mutex);
}
diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-tx.c b/drivers/net/wireless/iwlwifi/iwl-agn-tx.c
index 2dd7d54..0712b67 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn-tx.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn-tx.c
@@ -568,12 +568,17 @@ int iwlagn_tx_skb(struct iwl_priv *priv, struct sk_buff *skb)
hdr_len = ieee80211_hdrlen(fc);
- /* Find index into station table for destination station */
- sta_id = iwl_sta_id_or_broadcast(priv, ctx, info->control.sta);
- if (sta_id == IWL_INVALID_STATION) {
- IWL_DEBUG_DROP(priv, "Dropping - INVALID STATION: %pM\n",
- hdr->addr1);
- goto drop_unlock;
+ /* For management frames use broadcast id to do not break aggregation */
+ if (!ieee80211_is_data(fc))
+ sta_id = ctx->bcast_sta_id;
+ else {
+ /* Find index into station table for destination station */
+ sta_id = iwl_sta_id_or_broadcast(priv, ctx, info->control.sta);
+ if (sta_id == IWL_INVALID_STATION) {
+ IWL_DEBUG_DROP(priv, "Dropping - INVALID STATION: %pM\n",
+ hdr->addr1);
+ goto drop_unlock;
+ }
}
IWL_DEBUG_TX(priv, "station Id %d\n", sta_id);
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply related
* Re: [PATCH V3 6/8] macvtap/vhost TX zero copy support
From: Shirley Ma @ 2011-05-02 18:35 UTC (permalink / raw)
To: David Miller
Cc: Arnd Bergmann, mst, Eric Dumazet, Avi Kivity, netdev, kvm,
linux-kernel
In-Reply-To: <1303331235.19336.64.camel@localhost.localdomain>
Resubmit the patch with a bug being found by <liyonghelpme@foxmail.com>,
when he tested 2.6.38.2 kernel.
When the virtio_net doesn't enable GSO, vnet hdr_len is 0, we can't
use vnet hdr_len to allocate linear skb in zerocopy.
Signed-off-by: Shirley Ma <xma@us.ibm.com>
---
drivers/net/macvtap.c | 124 +++++++++++++++++++++++++++++++++++++++++++++----
1 files changed, 114 insertions(+), 10 deletions(-)
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 6696e56..e8dac4d 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -60,6 +60,7 @@ static struct proto macvtap_proto = {
*/
static dev_t macvtap_major;
#define MACVTAP_NUM_DEVS 65536
+#define GOODCOPY_LEN (L1_CACHE_BYTES < 128 ? 128 : L1_CACHE_BYTES)
static struct class *macvtap_class;
static struct cdev macvtap_cdev;
@@ -340,6 +341,7 @@ static int macvtap_open(struct inode *inode, struct file *file)
{
struct net *net = current->nsproxy->net_ns;
struct net_device *dev = dev_get_by_index(net, iminor(inode));
+ struct macvlan_dev *vlan = netdev_priv(dev);
struct macvtap_queue *q;
int err;
@@ -369,6 +371,16 @@ static int macvtap_open(struct inode *inode, struct file *file)
q->flags = IFF_VNET_HDR | IFF_NO_PI | IFF_TAP;
q->vnet_hdr_sz = sizeof(struct virtio_net_hdr);
+ /*
+ * so far only VM uses macvtap, enable zero copy between guest
+ * kernel and host kernel when lower device supports high memory
+ * DMA
+ */
+ if (vlan) {
+ if (vlan->lowerdev->features & NETIF_F_ZEROCOPY)
+ sock_set_flag(&q->sk, SOCK_ZEROCOPY);
+ }
+
err = macvtap_set_queue(dev, file, q);
if (err)
sock_put(&q->sk);
@@ -433,6 +445,80 @@ static inline struct sk_buff *macvtap_alloc_skb(struct sock *sk, size_t prepad,
return skb;
}
+/* set skb frags from iovec, this can move to core network code for reuse */
+static int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *from,
+ int offset, size_t count)
+{
+ int len = iov_length(from, count) - offset;
+ int copy = skb_headlen(skb);
+ int size, offset1 = 0;
+ int i = 0;
+ skb_frag_t *f;
+
+ /* Skip over from offset */
+ while (offset >= from->iov_len) {
+ offset -= from->iov_len;
+ ++from;
+ --count;
+ }
+
+ /* copy up to skb headlen */
+ while (copy > 0) {
+ size = min_t(unsigned int, copy, from->iov_len - offset);
+ if (copy_from_user(skb->data + offset1, from->iov_base + offset,
+ size))
+ return -EFAULT;
+ if (copy > size) {
+ ++from;
+ --count;
+ }
+ copy -= size;
+ offset1 += size;
+ offset = 0;
+ }
+
+ if (len == offset1)
+ return 0;
+
+ while (count--) {
+ struct page *page[MAX_SKB_FRAGS];
+ int num_pages;
+ unsigned long base;
+
+ len = from->iov_len - offset1;
+ if (!len) {
+ offset1 = 0;
+ ++from;
+ continue;
+ }
+ base = (unsigned long)from->iov_base + offset1;
+ size = ((base & ~PAGE_MASK) + len + ~PAGE_MASK) >> PAGE_SHIFT;
+ num_pages = get_user_pages_fast(base, size, 0, &page[i]);
+ if ((num_pages != size) ||
+ (num_pages > MAX_SKB_FRAGS - skb_shinfo(skb)->nr_frags))
+ /* put_page is in skb free */
+ return -EFAULT;
+ skb->data_len += len;
+ skb->len += len;
+ skb->truesize += len;
+ while (len) {
+ f = &skb_shinfo(skb)->frags[i];
+ f->page = page[i];
+ f->page_offset = base & ~PAGE_MASK;
+ f->size = min_t(int, len, PAGE_SIZE - f->page_offset);
+ skb_shinfo(skb)->nr_frags++;
+ /* increase sk_wmem_alloc */
+ atomic_add(f->size, &skb->sk->sk_wmem_alloc);
+ base += f->size;
+ len -= f->size;
+ i++;
+ }
+ offset1 = 0;
+ ++from;
+ }
+ return 0;
+}
+
/*
* macvtap_skb_from_vnet_hdr and macvtap_skb_to_vnet_hdr should
* be shared with the tun/tap driver.
@@ -515,17 +601,19 @@ static int macvtap_skb_to_vnet_hdr(const struct sk_buff *skb,
/* Get packet from user space buffer */
-static ssize_t macvtap_get_user(struct macvtap_queue *q,
- const struct iovec *iv, size_t count,
- int noblock)
+static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
+ const struct iovec *iv, unsigned long total_len,
+ size_t count, int noblock)
{
struct sk_buff *skb;
struct macvlan_dev *vlan;
- size_t len = count;
+ unsigned long len = total_len;
int err;
struct virtio_net_hdr vnet_hdr = { 0 };
int vnet_hdr_len = 0;
+ int headlen, copylen, zerocopy;
+ zerocopy = sock_flag(&q->sk, SOCK_ZEROCOPY) && (len > GOODCOPY_LEN);
if (q->flags & IFF_VNET_HDR) {
vnet_hdr_len = q->vnet_hdr_sz;
@@ -552,12 +640,28 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q,
if (unlikely(len < ETH_HLEN))
goto err;
- skb = macvtap_alloc_skb(&q->sk, NET_IP_ALIGN, len, vnet_hdr.hdr_len,
+ headlen = vnet_hdr.hdr_len;
+ if (zerocopy) {
+ /* create linear skb when vnet doesn't enable gso */
+ if (!vnet_hdr.hdr_len) {
+ headlen = GOODCOPY_LEN;
+ copylen = headlen;
+ } else
+ copylen = len;
+
+ skb = macvtap_alloc_skb(&q->sk, NET_IP_ALIGN, copylen, headlen,
noblock, &err);
if (!skb)
goto err;
- err = skb_copy_datagram_from_iovec(skb, 0, iv, vnet_hdr_len, len);
+ if (zerocopy)
+ err = zerocopy_sg_from_iovec(skb, iv, vnet_hdr_len, count);
+ else
+ err = skb_copy_datagram_from_iovec(skb, 0, iv, vnet_hdr_len,
+ len);
+ if (sock_flag(&q->sk, SOCK_ZEROCOPY))
+ memcpy(&skb_shinfo(skb)->ubuf, m->msg_control,
+ sizeof(struct skb_ubuf_info));
if (err)
goto err_kfree;
@@ -579,7 +683,7 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q,
kfree_skb(skb);
rcu_read_unlock_bh();
- return count;
+ return total_len;
err_kfree:
kfree_skb(skb);
@@ -601,8 +705,8 @@ static ssize_t macvtap_aio_write(struct kiocb *iocb, const struct iovec *iv,
ssize_t result = -ENOLINK;
struct macvtap_queue *q = file->private_data;
- result = macvtap_get_user(q, iv, iov_length(iv, count),
- file->f_flags & O_NONBLOCK);
+ result = macvtap_get_user(q, NULL, iv, iov_length(iv, count), count,
+ file->f_flags & O_NONBLOCK);
return result;
}
@@ -815,7 +919,7 @@ static int macvtap_sendmsg(struct kiocb *iocb, struct socket *sock,
struct msghdr *m, size_t total_len)
{
struct macvtap_queue *q = container_of(sock, struct macvtap_queue, sock);
- return macvtap_get_user(q, m->msg_iov, total_len,
+ return macvtap_get_user(q, m, m->msg_iov, total_len, m->msg_iovlen,
m->msg_flags & MSG_DONTWAIT);
}
^ permalink raw reply related
* Re: [PATCH V3 2/8] Add a new zerocopy device flag
From: Shirley Ma @ 2011-05-02 18:47 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: David Miller, Eric Dumazet, Avi Kivity, Arnd Bergmann, netdev,
kvm, linux-kernel
In-Reply-To: <20110502104257.GA21625@redhat.com>
On Mon, 2011-05-02 at 13:42 +0300, Michael S. Tsirkin wrote:
> This comment should specify what exactly is the promise the
> device makes by setting this flag. Specifically, the
> condition is that no skb fragments are used
> after the uinfo callback has been called.
>
> The way it's implemented, it probably means the device
> should not use any of skb_clone, expand head etc.
Agree. Or maybe force a copy when device uses skb_clone, expand
head ...?
Thanks
Shirley
^ permalink raw reply
* Re: r8169 : always copying the rx buffer to new skb
From: Chris Friesen @ 2011-05-02 19:04 UTC (permalink / raw)
To: John Lumby; +Cc: Francois Romieu, netdev, Ben Hutchings, nic_swsd
In-Reply-To: <4DB77D03.9070507@hotmail.com>
On 04/26/2011 08:18 PM, John Lumby wrote:
> So - question :
> is there any way, when returning from rtl8169_poll, to tell napi
> something like :
> " finish this interrupt context and let something else run on this CPU
> (always CPU0 on my machine) BUT reschedule another napi poll on this
> same device at some time after that "
Does the hardware support any options for interrupt mitigation? I've
used some devices where you can specify a minimum time between
interrupts such that even if NAPI re-enabled interrupts and there were
packets waiting the hardware would wait a certain time before raising a
new interrupt.
Chris
--
Chris Friesen
Software Developer
GENBAND
chris.friesen@genband.com
www.genband.com
^ permalink raw reply
* Re: pull request: wireless-2.6 2011-05-02
From: David Miller @ 2011-05-02 19:24 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20110502182013.GA6768@tuxdriver.com>
From: "John W. Linville" <linville@tuxdriver.com>
Date: Mon, 2 May 2011 14:20:13 -0400
> Dave,
>
> Here are a few more intended for 2.6.39. There is a one-liner for b43
> which adds a missing MODULE_FIRMWARE declaration (so maybe someone's
> initrd will get generated correctly), an iwlegacy fix from Wey-Yi that
> corrects an LED flashing change that people are complaining about,
> an iwl4965 WARNING fix from Stanislaw, and a matching pair of iwlagn
> and iwl4965 regression fixes also from Stanislaw that correct problems
> with using the wrong station ID for block ACK sessions.
>
> Please let me know if there are problems!
>
> John
>
> P.S. I know the LED flashing fix is questionable, but it is an
> annoyance and it should not effect any other hardware.
Looks fine, pulled, thanks John.
^ permalink raw reply
* [PATCH] ipheth.c: Enable IP header alignment
From: L. Alberto Giménez @ 2011-05-02 19:35 UTC (permalink / raw)
To: linux-kernel
Cc: dgiagio, dborca, davem, pmcenery, david.hill,
open list:USB SUBSYSTEM, open list:NETWORKING DRIVERS
In-Reply-To: <1304264799.2833.82.camel@localhost>
Since commit ea812ca1b06113597adcd8e70c0f84a413d97544 (x86: Align skb w/ start
of cacheline on newer core 2/Xeon Arch), NET_IP_ALIGN changed from 2 to 0, and
the constant is used to reserve more room for the socket buffer.
Some people have reported that tethering stopped working and David Hill
submited a patch that redefined NET_IP_ALIGN. Pointed by Ben Hutchings, the
patch has been reworked to use a private constant.
I have no more an iPhone device to test it, so it is only compile-tested.
Signed-off-by: L. Alberto Giménez <agimenez@sysvalve.es>
---
drivers/net/usb/ipheth.c | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/net/usb/ipheth.c b/drivers/net/usb/ipheth.c
index 7d42f9a..8f1ffc7 100644
--- a/drivers/net/usb/ipheth.c
+++ b/drivers/net/usb/ipheth.c
@@ -80,6 +80,8 @@
#define IPHETH_CARRIER_CHECK_TIMEOUT round_jiffies_relative(1 * HZ)
#define IPHETH_CARRIER_ON 0x04
+#define IPHETH_IP_ALIGN 2
+
static struct usb_device_id ipheth_table[] = {
{ USB_DEVICE_AND_INTERFACE_INFO(
USB_VENDOR_APPLE, USB_PRODUCT_IPHONE,
@@ -205,15 +207,15 @@ static void ipheth_rcvbulk_callback(struct urb *urb)
len = urb->actual_length;
buf = urb->transfer_buffer;
- skb = dev_alloc_skb(NET_IP_ALIGN + len);
+ skb = dev_alloc_skb(IPHETH_IP_ALIGN + len);
if (!skb) {
err("%s: dev_alloc_skb: -ENOMEM", __func__);
dev->net->stats.rx_dropped++;
return;
}
- skb_reserve(skb, NET_IP_ALIGN);
- memcpy(skb_put(skb, len), buf + NET_IP_ALIGN, len - NET_IP_ALIGN);
+ skb_reserve(skb, IPHETH_IP_ALIGN);
+ memcpy(skb_put(skb, len), buf + IPHETH_IP_ALIGN, len - IPHETH_IP_ALIGN);
skb->dev = dev->net;
skb->protocol = eth_type_trans(skb, dev->net);
--
1.7.5
^ permalink raw reply related
* Re: [PATCH] ipheth.c: Enable IP header alignment
From: David Miller @ 2011-05-02 19:46 UTC (permalink / raw)
To: agimenez
Cc: linux-kernel, dgiagio, dborca, pmcenery, david.hill, linux-usb,
netdev
In-Reply-To: <1304364912-15444-1-git-send-email-agimenez@sysvalve.es>
From: L. Alberto Giménez <agimenez@sysvalve.es>
Date: Mon, 2 May 2011 21:35:12 +0200
> Since commit ea812ca1b06113597adcd8e70c0f84a413d97544 (x86: Align skb w/ start
> of cacheline on newer core 2/Xeon Arch), NET_IP_ALIGN changed from 2 to 0, and
> the constant is used to reserve more room for the socket buffer.
>
> Some people have reported that tethering stopped working and David Hill
> submited a patch that redefined NET_IP_ALIGN. Pointed by Ben Hutchings, the
> patch has been reworked to use a private constant.
>
> I have no more an iPhone device to test it, so it is only compile-tested.
>
> Signed-off-by: L. Alberto Giménez <agimenez@sysvalve.es>
Why did this break things?
I'm not applying a fix when nobody can explain the reason why:
1) Things broke in the first place
2) Forcing reservation of 2 bytes fixes things
Where is the built in assumption about "2" and why does it exist? Why
can't we fix this code not to have such assumptions in the first
place?
^ permalink raw reply
* Re: [PATCH V3 2/8] Add a new zerocopy device flag
From: Michael S. Tsirkin @ 2011-05-02 19:53 UTC (permalink / raw)
To: Shirley Ma
Cc: David Miller, Eric Dumazet, Avi Kivity, Arnd Bergmann, netdev,
kvm, linux-kernel
In-Reply-To: <1304362028.20660.15.camel@localhost.localdomain>
On Mon, May 02, 2011 at 11:47:08AM -0700, Shirley Ma wrote:
> On Mon, 2011-05-02 at 13:42 +0300, Michael S. Tsirkin wrote:
> > This comment should specify what exactly is the promise the
> > device makes by setting this flag. Specifically, the
> > condition is that no skb fragments are used
> > after the uinfo callback has been called.
> >
> > The way it's implemented, it probably means the device
> > should not use any of skb_clone, expand head etc.
>
> Agree. Or maybe force a copy when device uses skb_clone, expand
> head ...?
>
> Thanks
> Shirley
Copy from userspace upfront without locking is probably cheaper?
--
MST
^ permalink raw reply
* Re: Frequent spurious tx_timeouts for libertas
From: Daniel Drake @ 2011-05-02 19:59 UTC (permalink / raw)
To: Ben Hutchings
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
libertas-dev-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-wireless
In-Reply-To: <1304303082.2833.159.camel@localhost>
On 2 May 2011 03:24, Ben Hutchings <bhutchings-s/n/eUQHGBpZroRs9YW3xA@public.gmane.org> wrote:
>> Also, while looking at this code, I spotted a bug in dev_watchdog():
>> /*
>> * old device drivers set dev->trans_start
>> */
>> trans_start = txq->trans_start ? : dev->trans_start;
>>
>> i.e. it is trying to figure out whether to read trans_start from txq
>> or dev. In both cases, trans_start is updated based on the value of
>> jiffies, which will occasionally be 0 (as it wraps around). Therefore
>> this line of code will occasionally make the wrong decision.
>
> No, I don't think so.
>
> If only dev->trans_start is being updated then the watchdog reads that.
> If both txq->trans_start and dev->trans_start are being updated then it
> doesn't matter much which the watchdog reads.
> If only txq->trans_start is being updated then dev->trans_start is
> always set to 0, so when txq->trans_start is 0 the watchdog still gets
> 0.
dev->trans_start is unconditionally initialized by dev_activate() in
sch_generic.c:
if (need_watchdog) {
dev->trans_start = jiffies;
dev_watchdog_up(dev);
}
so it is (usually) not 0.
Thanks for your input on the tx timeout issue, now that I understand
it better I think the right plan of action is to remove it from
libertas entirely, I'll CC you on the patch.
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] amd8111e: trivial typo spelling: Negotitate -> Negotiate
From: Joe Perches @ 2011-05-02 19:59 UTC (permalink / raw)
To: netdev
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/net/amd8111e.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/amd8111e.c b/drivers/net/amd8111e.c
index 88495c4..241b185 100644
--- a/drivers/net/amd8111e.c
+++ b/drivers/net/amd8111e.c
@@ -106,7 +106,7 @@ MODULE_DESCRIPTION ("AMD8111 based 10/100 Ethernet Controller. Driver Version "M
MODULE_LICENSE("GPL");
MODULE_DEVICE_TABLE(pci, amd8111e_pci_tbl);
module_param_array(speed_duplex, int, NULL, 0);
-MODULE_PARM_DESC(speed_duplex, "Set device speed and duplex modes, 0: Auto Negotitate, 1: 10Mbps Half Duplex, 2: 10Mbps Full Duplex, 3: 100Mbps Half Duplex, 4: 100Mbps Full Duplex");
+MODULE_PARM_DESC(speed_duplex, "Set device speed and duplex modes, 0: Auto Negotiate, 1: 10Mbps Half Duplex, 2: 10Mbps Full Duplex, 3: 100Mbps Half Duplex, 4: 100Mbps Full Duplex");
module_param_array(coalesce, bool, NULL, 0);
MODULE_PARM_DESC(coalesce, "Enable or Disable interrupt coalescing, 1: Enable, 0: Disable");
module_param_array(dynamic_ipg, bool, NULL, 0);
^ permalink raw reply related
* Re: Frequent spurious tx_timeouts for libertas
From: Daniel Drake @ 2011-05-02 20:01 UTC (permalink / raw)
To: Ben Hutchings
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
libertas-dev-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-wireless
In-Reply-To: <BANLkTi=FT4v=Jvv5bQpUX15R87kPudKq=Q@mail.gmail.com>
On 2 May 2011 20:59, Daniel Drake <dsd-2X9k7bc8m7Mdnm+yROfE0A@public.gmane.org> wrote:
> dev->trans_start is unconditionally initialized by dev_activate() in
> sch_generic.c:
Also, many drivers (including libertas) set it in their tx_timeout handlers:
dev->trans_start = jiffies; /* prevent tx timeout */
I don't understand why (the TX timeout has already occurred, it can't
be prevented at that stage).
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Frequent spurious tx_timeouts for libertas
From: Ben Hutchings @ 2011-05-02 20:47 UTC (permalink / raw)
To: Daniel Drake
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
libertas-dev-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-wireless
In-Reply-To: <BANLkTi=FT4v=Jvv5bQpUX15R87kPudKq=Q@mail.gmail.com>
On Mon, 2011-05-02 at 20:59 +0100, Daniel Drake wrote:
> On 2 May 2011 03:24, Ben Hutchings <bhutchings-s/n/eUQHGBpZroRs9YW3xA@public.gmane.org> wrote:
> >> Also, while looking at this code, I spotted a bug in dev_watchdog():
> >> /*
> >> * old device drivers set dev->trans_start
> >> */
> >> trans_start = txq->trans_start ? : dev->trans_start;
> >>
> >> i.e. it is trying to figure out whether to read trans_start from txq
> >> or dev. In both cases, trans_start is updated based on the value of
> >> jiffies, which will occasionally be 0 (as it wraps around). Therefore
> >> this line of code will occasionally make the wrong decision.
> >
> > No, I don't think so.
> >
> > If only dev->trans_start is being updated then the watchdog reads that.
> > If both txq->trans_start and dev->trans_start are being updated then it
> > doesn't matter much which the watchdog reads.
> > If only txq->trans_start is being updated then dev->trans_start is
> > always set to 0, so when txq->trans_start is 0 the watchdog still gets
> > 0.
>
> dev->trans_start is unconditionally initialized by dev_activate() in
> sch_generic.c:
>
> if (need_watchdog) {
> dev->trans_start = jiffies;
> dev_watchdog_up(dev);
> }
>
> so it is (usually) not 0.
[...]
You're right. Seems like we have an incomplete compatibility hack that
can hurt drivers that are doing the right thing.
For those few single-queue drivers that need to update the transmit
time, perhaps we could add a dev_trans_update() as a wrapper for
txq_trans_update(). Then delete net_device::trans_start and change
dev_trans_start() to avoid using it.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] ipheth.c: Enable IP header alignment
From: Ben Hutchings @ 2011-05-02 21:04 UTC (permalink / raw)
To: L. Alberto Giménez
Cc: linux-kernel, dgiagio, dborca, davem, pmcenery, david.hill,
open list:USB SUBSYSTEM, open list:NETWORKING DRIVERS
In-Reply-To: <1304364912-15444-1-git-send-email-agimenez@sysvalve.es>
On Mon, 2011-05-02 at 21:35 +0200, L. Alberto Giménez wrote:
> Since commit ea812ca1b06113597adcd8e70c0f84a413d97544 (x86: Align skb w/ start
> of cacheline on newer core 2/Xeon Arch), NET_IP_ALIGN changed from 2 to 0, and
> the constant is used to reserve more room for the socket buffer.
>
> Some people have reported that tethering stopped working and David Hill
> submited a patch that redefined NET_IP_ALIGN. Pointed by Ben Hutchings, the
> patch has been reworked to use a private constant.
>
> I have no more an iPhone device to test it, so it is only compile-tested.
>
> Signed-off-by: L. Alberto Giménez <agimenez@sysvalve.es>
> ---
> drivers/net/usb/ipheth.c | 8 +++++---
> 1 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/usb/ipheth.c b/drivers/net/usb/ipheth.c
> index 7d42f9a..8f1ffc7 100644
> --- a/drivers/net/usb/ipheth.c
> +++ b/drivers/net/usb/ipheth.c
> @@ -80,6 +80,8 @@
> #define IPHETH_CARRIER_CHECK_TIMEOUT round_jiffies_relative(1 * HZ)
> #define IPHETH_CARRIER_ON 0x04
>
> +#define IPHETH_IP_ALIGN 2
> +
> static struct usb_device_id ipheth_table[] = {
> { USB_DEVICE_AND_INTERFACE_INFO(
> USB_VENDOR_APPLE, USB_PRODUCT_IPHONE,
> @@ -205,15 +207,15 @@ static void ipheth_rcvbulk_callback(struct urb *urb)
> len = urb->actual_length;
> buf = urb->transfer_buffer;
>
> - skb = dev_alloc_skb(NET_IP_ALIGN + len);
> + skb = dev_alloc_skb(IPHETH_IP_ALIGN + len);
> if (!skb) {
> err("%s: dev_alloc_skb: -ENOMEM", __func__);
> dev->net->stats.rx_dropped++;
> return;
> }
>
> - skb_reserve(skb, NET_IP_ALIGN);
> - memcpy(skb_put(skb, len), buf + NET_IP_ALIGN, len - NET_IP_ALIGN);
> + skb_reserve(skb, IPHETH_IP_ALIGN);
> + memcpy(skb_put(skb, len), buf + IPHETH_IP_ALIGN, len - IPHETH_IP_ALIGN);
[...]
So this was using NET_IP_ALIGN as an offset into the URB. Which was
totally bogus, as its value has long been architecture-dependent. The
code is also claiming to put len bytes but only copying len - delta.
The correct code would be something like:
if (urb->actual_length <= IPHETH_IP_ALIGN) {
dev->net->stats.rx_length_errors++;
return;
}
len = urb->actual_length - IPHETH_IP_ALIGN;
buf = urb->transfer_buffer + IPHETH_IP_ALIGN;
dev_alloc_skb(len);
...
memcpy(skb_put(skb, len), buf, len);
Ben.
> skb->dev = dev->net;
> skb->protocol = eth_type_trans(skb, dev->net);
>
--
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH] ipheth.c: Enable IP header alignment
From: L. Alberto Giménez @ 2011-05-02 21:12 UTC (permalink / raw)
To: David Miller
Cc: linux-kernel, dgiagio, dborca, pmcenery, david.hill, linux-usb,
netdev
In-Reply-To: <20110502.124622.226785624.davem@davemloft.net>
On Mon, May 02, 2011 at 12:46:22PM -0700, David Miller wrote:
>
> Why did this break things?
Hi, I don't know. As upstream is unresponsive and is applying patches to his
private repo without submitting them to the list (which I can understand), I
decided to submit the particular fix so mainline users can get tethering working
again.
I received a forwarded email with the patch (I think that's because I submitted
the driver to mainline) asking for the mainline driver status and if it was
being maintained.
>
> I'm not applying a fix when nobody can explain the reason why:
>
> 1) Things broke in the first place
> 2) Forcing reservation of 2 bytes fixes things
Honestly, I can't answer either of those ones. I just submitted a patch that
*seemed* to fix the problem (I don't own an iPhone device since long time ago),
after explictly requesting upstream to submit by himself, and getting a
negative.
> Where is the built in assumption about "2" and why does it exist? Why
> can't we fix this code not to have such assumptions in the first
> place?
Ditto.
At this point, I think that David, Diego or Daniel should step in if they want
to keep on with this discussion. I won't have problems if you want to take this
off-list.
Best regards,
--
L. Alberto Giménez
JabberID agimenez@jabber.sysvalve.es
GnuPG key ID 0x3BAABDE1
^ permalink raw reply
* [PATCH net-next-2.6] ipv4: Make sure flowi4->{saddr,daddr} are always set.
From: David Miller @ 2011-05-02 21:38 UTC (permalink / raw)
To: netdev
Slow path output route resolution always makes sure that
->{saddr,daddr} are set, and also if we trigger into IPSEC resolution
we initialize them as well, because xfrm_lookup() expects them to be
fully resolved.
But if we hit the fast path and flowi4->flowi4_proto is zero, we won't
do this initialization.
Therefore, move the IPSEC path initialization to the route cache
lookup fast path to make sure these are always set.
Signed-off-by: David S. Miller <davem@davemloft.net>
---
net/ipv4/route.c | 11 +++++------
1 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 93f71be..64f360d 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2675,6 +2675,10 @@ struct rtable *__ip_route_output_key(struct net *net, struct flowi4 *flp4)
dst_use(&rth->dst, jiffies);
RT_CACHE_STAT_INC(out_hit);
rcu_read_unlock_bh();
+ if (!flp4->saddr)
+ flp4->saddr = rth->rt_src;
+ if (!flp4->daddr)
+ flp4->daddr = rth->rt_dst;
return rth;
}
RT_CACHE_STAT_INC(out_hlist_search);
@@ -2772,15 +2776,10 @@ struct rtable *ip_route_output_flow(struct net *net, struct flowi4 *flp4,
if (IS_ERR(rt))
return rt;
- if (flp4->flowi4_proto) {
- if (!flp4->saddr)
- flp4->saddr = rt->rt_src;
- if (!flp4->daddr)
- flp4->daddr = rt->rt_dst;
+ if (flp4->flowi4_proto)
rt = (struct rtable *) xfrm_lookup(net, &rt->dst,
flowi4_to_flowi(flp4),
sk, 0);
- }
return rt;
}
--
1.7.4.5
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox