* [PATCH iproute2] tc: man: choke counts packets, not bytes
From: Florian Westphal @ 2012-04-08 22:34 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal
---
man/man8/tc-choke.8 | 11 ++++++-----
1 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/man/man8/tc-choke.8 b/man/man8/tc-choke.8
index 7b213be..620c7f6 100644
--- a/man/man8/tc-choke.8
+++ b/man/man8/tc-choke.8
@@ -4,11 +4,11 @@ choke \- choose and keep scheduler
.SH SYNOPSIS
.B tc qdisc ... choke
.B limit
-bytes
+packets
.B min
-bytes
+packets
.B max
-bytes
+packets
.B avpkt
bytes
.B burst
@@ -22,7 +22,7 @@ chance
CHOKe (CHOose and Keep for responsive flows, CHOose and Kill for unresponsive flows)
is a classless qdisc designed to both identify and penalize flows that monopolize the
-queue. CHOKe is a variation of RED, and the configuration is the same as RED.
+queue. CHOKe is a variation of RED, and the configuration is similar to RED.
.SH ALGORITHM
Once the queue hits a certain average length, a random packet is drawn from the
@@ -39,7 +39,8 @@ If the queue length exceeds
The marking probability computation is the same as used by the RED qdisc.
.SH PARAMETERS
-The parameters are the same as for RED, see
+The parameters are the same as for RED, except that RED uses bytes whereas choke
+counts packets. See
.BR tc-red (8)
for a description.
--
1.7.3.4
^ permalink raw reply related
* Re: [PATCH] net: orphan queued skbs if device tx can stall
From: Herbert Xu @ 2012-04-08 23:49 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: netdev, linux-kernel, David S. Miller, Jamal Hadi Salim,
Stephen Hemminger, Jason Wang, Neil Horman, Jiri Pirko,
Jeff Kirsher, Eric Dumazet, Michał Mirosław,
Ben Hutchings
In-Reply-To: <20120408171323.GA16012@redhat.com>
On Sun, Apr 08, 2012 at 08:13:25PM +0300, Michael S. Tsirkin wrote:
> commit 0110d6f22f392f976e84ab49da1b42f85b64a3c5
> tun: orphan an skb on tx
> Fixed a configuration where skbs get queued
> at the tun device forever, blocking senders.
>
> However this fix isn't waterproof:
> userspace can control whether the interface
> is stopped, and if it is, packets
> get queued in the qdisc, again potentially forever.
>
> Complete the fix by setting a private flag and orphaning
> at the qdisc level.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
1) Doesn't this break local UDP push-back?
2) Isn't the stall a bug in the backend and isn't this just
papering over that?
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [BUGFIX][PATCH 3/3] memcg/tcp: ignore tcp usage before accounting started
From: KAMEZAWA Hiroyuki @ 2012-04-09 0:58 UTC (permalink / raw)
To: Glauber Costa; +Cc: David Miller, netdev, akpm
In-Reply-To: <4F7B7A54.9000501@parallels.com>
(2012/04/04 7:31), Glauber Costa wrote:
> On 04/02/2012 07:41 AM, David Miller wrote:
>> From: Glauber Costa<glommer@parallels.com>
>> Date: Thu, 29 Mar 2012 11:21:07 +0200
>>
>>> On 03/29/2012 09:10 AM, KAMEZAWA Hiroyuki wrote:
>>>> tcp memcontrol starts accouting after res->limit is set. So, if a sockets
>>>> starts before setting res->limit, there are already used resource.
>>>> After setting res->limit, the resource (already used) will be uncharged and
>>>> make res_counter below 0 because they are not charged. This causes warning.
>>>>
>>>> This patch fixes that by adding res_counter_uncharge_nowarn().
>>>> (*) We cannot avoid this while we have 'account start' switch.
>>>>
>>>> Signed-off-by: KAMEZAWA Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com>
>>>
>>> Fine by me.
>>>
>>> Acked-by: Glauber Costa<glommer@parallels.com>
>>
>> I'm not applying patches that simply ignore accounting counter
>> underflows.
>>
>> You must either:
>>
>> 1) Integrate the socket's existing usage when the limit is set.
>>
>> 2) Avoid accounting completely for a socket that started before
>> the limit was set.
>>
>> No half-way solutions, please. Otherwise it is impossible to design
>> validations of the resource usage for a particular socket or group of
>> sockets, because they can always be potentially "wrong" and over the
>> limit. That's a design for a buggy system.
>>
>>
> Kame,
>
> I agree with Dave FWIW.
>
> We should be able to do this by dropping the reference count when the
> cgroup is finally destroyed, instead of from the remove callback. At
> that point, no more pending sockets should be attached to it.
>
> Prior to increasing the static key, they are all assigned to the global
> cgroup, so we shouldn't care about them.
>
Could you do the fix ?
Thanks,
-Kame
^ permalink raw reply
* Re: [BUGFIX][PATCH 3/3] memcg/tcp: ignore tcp usage before accounting started
From: Glauber Costa @ 2012-04-09 1:44 UTC (permalink / raw)
To: KAMEZAWA Hiroyuki; +Cc: David Miller, netdev, akpm
In-Reply-To: <4F823443.9070908@jp.fujitsu.com>
On 04/08/2012 09:58 PM, KAMEZAWA Hiroyuki wrote:
> (2012/04/04 7:31), Glauber Costa wrote:
>
>> On 04/02/2012 07:41 AM, David Miller wrote:
>>> From: Glauber Costa<glommer@parallels.com>
>>> Date: Thu, 29 Mar 2012 11:21:07 +0200
>>>
>>>> On 03/29/2012 09:10 AM, KAMEZAWA Hiroyuki wrote:
>>>>> tcp memcontrol starts accouting after res->limit is set. So, if a sockets
>>>>> starts before setting res->limit, there are already used resource.
>>>>> After setting res->limit, the resource (already used) will be uncharged and
>>>>> make res_counter below 0 because they are not charged. This causes warning.
>>>>>
>>>>> This patch fixes that by adding res_counter_uncharge_nowarn().
>>>>> (*) We cannot avoid this while we have 'account start' switch.
>>>>>
>>>>> Signed-off-by: KAMEZAWA Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com>
>>>>
>>>> Fine by me.
>>>>
>>>> Acked-by: Glauber Costa<glommer@parallels.com>
>>>
>>> I'm not applying patches that simply ignore accounting counter
>>> underflows.
>>>
>>> You must either:
>>>
>>> 1) Integrate the socket's existing usage when the limit is set.
>>>
>>> 2) Avoid accounting completely for a socket that started before
>>> the limit was set.
>>>
>>> No half-way solutions, please. Otherwise it is impossible to design
>>> validations of the resource usage for a particular socket or group of
>>> sockets, because they can always be potentially "wrong" and over the
>>> limit. That's a design for a buggy system.
>>>
>>>
>> Kame,
>>
>> I agree with Dave FWIW.
>>
>> We should be able to do this by dropping the reference count when the
>> cgroup is finally destroyed, instead of from the remove callback. At
>> that point, no more pending sockets should be attached to it.
>>
>> Prior to increasing the static key, they are all assigned to the global
>> cgroup, so we shouldn't care about them.
>>
>
> Could you do the fix ?
>
> Thanks,
> -Kame
>
>
I sent you a version already. Please just make sure it works for you
^ permalink raw reply
* Re: [PATCH v17 00/15] seccomp_filter: BPF-based syscall filtering
From: James Morris @ 2012-04-09 3:48 UTC (permalink / raw)
To: Andrew Morton
Cc: Will Drewry, linux-kernel, linux-security-module, linux-arch,
linux-doc, kernel-hardening, netdev, x86, arnd, davem, hpa, mingo,
Oleg Nesterov, peterz, rdunlap, mcgrathr, tglx, luto, eparis,
serge.hallyn, djm, scarybeasts, indan, pmoore, corbet,
eric.dumazet, markus, coreyb, keescook, Oleg Nesterov
In-Reply-To: <20120406142846.fd8dea70.akpm@linux-foundation.org>
On Fri, 6 Apr 2012, Andrew Morton wrote:
> > I've offered to take this via my tree -- if anyone else wants to merge it
> > via theirs instead, holler.
>
> OK by me, especially if it means that you'll carefully (re)review it! I
> had some minor quibbles with v17.
Sure.
>
> It would be good if Oleg could take a look at the signal changes, if he
> has not yet done so?
>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* [PATCH 1/2] qlcnic: Add default swtich case in 'qlcnic_can_start_firmware()'
From: santosh nayak @ 2012-04-09 5:59 UTC (permalink / raw)
To: anirban.chakraborty, rajesh.borundia
Cc: sony.chacko, linux-driver, netdev, kernel-janitors, Santosh Nayak
From: Santosh Nayak <santoshprasadnayak@gmail.com>
QLCRD32() may return bad state.
Its good to have default switch case:
1. to avoid unexpected system behaviour.
2. to avoid extra computation thats happening
after switch case.
Signed-off-by: Santosh Nayak <santoshprasadnayak@gmail.com>
---
drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
index 75c32e8..d30b9b8 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
@@ -2787,6 +2787,11 @@ qlcnic_can_start_firmware(struct qlcnic_adapter *adapter)
case QLCNIC_DEV_INITIALIZING:
case QLCNIC_DEV_QUISCENT:
break;
+
+ default:
+ dev_err(&adapter->pdev->dev, "Invalid state\n");
+ qlcnic_api_unlock(adapter);
+ return -EINVAL;
}
qlcnic_api_unlock(adapter);
--
1.7.4.4
^ permalink raw reply related
* [PATCH 2/2] qlcnic: Remove redundant NULL check on kfree().
From: santosh nayak @ 2012-04-09 6:11 UTC (permalink / raw)
To: anirban.chakraborty, rajesh.borundia
Cc: sony.chacko, linux-driver, netdev, kernel-janitors, Santosh Nayak
From: Santosh Nayak <santoshprasadnayak@gmail.com>
kfree() checks for NULL before freeing the memory.
Remove redundant NULL check.
This is just a clean up and also looks good from performance point of view.
Signed-off-by: Santosh Nayak <santoshprasadnayak@gmail.com>
---
drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 14 ++++----------
1 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
index d30b9b8..75b202d 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
@@ -160,9 +160,7 @@ qlcnic_alloc_sds_rings(struct qlcnic_recv_context *recv_ctx, int count)
static void
qlcnic_free_sds_rings(struct qlcnic_recv_context *recv_ctx)
{
- if (recv_ctx->sds_rings != NULL)
- kfree(recv_ctx->sds_rings);
-
+ kfree(recv_ctx->sds_rings);
recv_ctx->sds_rings = NULL;
}
@@ -1714,10 +1712,8 @@ static void __devexit qlcnic_remove(struct pci_dev *pdev)
qlcnic_detach(adapter);
- if (adapter->npars != NULL)
- kfree(adapter->npars);
- if (adapter->eswitch != NULL)
- kfree(adapter->eswitch);
+ kfree(adapter->npars);
+ kfree(adapter->eswitch);
qlcnic_clr_all_drv_state(adapter, 0);
@@ -1887,9 +1883,7 @@ void qlcnic_alloc_lb_filters_mem(struct qlcnic_adapter *adapter)
static void qlcnic_free_lb_filters_mem(struct qlcnic_adapter *adapter)
{
- if (adapter->fhash.fmax && adapter->fhash.fhead)
- kfree(adapter->fhash.fhead);
-
+ kfree(adapter->fhash.fhead);
adapter->fhash.fhead = NULL;
adapter->fhash.fmax = 0;
}
--
1.7.4.4
^ permalink raw reply related
* Re: [PATCH] net: orphan queued skbs if device tx can stall
From: Michael S. Tsirkin @ 2012-04-09 7:28 UTC (permalink / raw)
To: Herbert Xu
Cc: netdev, linux-kernel, David S. Miller, Jamal Hadi Salim,
Stephen Hemminger, Jason Wang, Neil Horman, Jiri Pirko,
Jeff Kirsher, Eric Dumazet, Michał Mirosław,
Ben Hutchings
In-Reply-To: <20120408234951.GA15993@gondor.apana.org.au>
On Mon, Apr 09, 2012 at 07:49:51AM +0800, Herbert Xu wrote:
> On Sun, Apr 08, 2012 at 08:13:25PM +0300, Michael S. Tsirkin wrote:
> > commit 0110d6f22f392f976e84ab49da1b42f85b64a3c5
> > tun: orphan an skb on tx
> > Fixed a configuration where skbs get queued
> > at the tun device forever, blocking senders.
> >
> > However this fix isn't waterproof:
> > userspace can control whether the interface
> > is stopped, and if it is, packets
> > get queued in the qdisc, again potentially forever.
> >
> > Complete the fix by setting a private flag and orphaning
> > at the qdisc level.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>
> 1) Doesn't this break local UDP push-back?
What is meant by UDP pushback here? Two tap
devices communicating by UDP packets locally?
This was always broken, see below.
> 2) Isn't the stall a bug in the backend and isn't this just
> papering over that?
>
> Cheers,
What do you mean by the backend? userspace? Yes, the stall is a result of
userspace not consuming packets:
if (skb_queue_len(&tun->socket.sk->sk_receive_queue) >= dev->tx_queue_len) {
if (!(tun->flags & TUN_ONE_QUEUE)) {
/* Normal queueing mode. */
/* Packet scheduler handles dropping of further packets. */
netif_stop_queue(dev);
/* We won't see all dropped packets individually, so overrun
* error is more appropriate. */
dev->stats.tx_fifo_errors++;
Thus we get this situation
tap1 sends packets, some of them to tap2, tap2 does not consume them,
as a result tap2 queue overflows, tap2 stops forever and
packets get queued in the qdisc, now tap1
send buffer gets full so it can not communicate to any destination.
So the problem is one VM can block all networking from another one.
As a solution this patch is always changing ownership if we're going
into a hostile device: it just does this early in qdisc instead of
at xmit time.
> --
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] net: orphan queued skbs if device tx can stall
From: Herbert Xu @ 2012-04-09 7:33 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: netdev, linux-kernel, David S. Miller, Jamal Hadi Salim,
Stephen Hemminger, Jason Wang, Neil Horman, Jiri Pirko,
Jeff Kirsher, Eric Dumazet, Michał Mirosław,
Ben Hutchings
In-Reply-To: <20120409072849.GA12014@redhat.com>
On Mon, Apr 09, 2012 at 10:28:49AM +0300, Michael S. Tsirkin wrote:
>
> > 1) Doesn't this break local UDP push-back?
>
> What is meant by UDP pushback here? Two tap
> devices communicating by UDP packets locally?
> This was always broken, see below.
I mean push-back from UDP transmission to the physical NIC.
Your patch breaks that as now the guest will have no push-back
whatsoever so anything that transmits UDP without protocol-level
congestion control will start dropping most of their packets.
Granted you can argue that these apps are broken, but they do
exist and we've always catered for them, both on baremetal and
under virtualisation.
> Thus we get this situation
> tap1 sends packets, some of them to tap2, tap2 does not consume them,
> as a result tap2 queue overflows, tap2 stops forever and
> packets get queued in the qdisc, now tap1
> send buffer gets full so it can not communicate to any destination.
>
> So the problem is one VM can block all networking from another one.
This should be addressed in the backend, as it can distinguish
between packets going out to physical and packets stuck going to
a local VM. In the latter case you can then duplicate and release
the sender's memory.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] net: orphan queued skbs if device tx can stall
From: Michael S. Tsirkin @ 2012-04-09 7:39 UTC (permalink / raw)
To: Herbert Xu
Cc: netdev, linux-kernel, David S. Miller, Jamal Hadi Salim,
Stephen Hemminger, Jason Wang, Neil Horman, Jiri Pirko,
Jeff Kirsher, Eric Dumazet, Michał Mirosław,
Ben Hutchings
In-Reply-To: <20120409073354.GA3218@gondor.apana.org.au>
On Mon, Apr 09, 2012 at 03:33:54PM +0800, Herbert Xu wrote:
> On Mon, Apr 09, 2012 at 10:28:49AM +0300, Michael S. Tsirkin wrote:
> >
> > > 1) Doesn't this break local UDP push-back?
> >
> > What is meant by UDP pushback here? Two tap
> > devices communicating by UDP packets locally?
> > This was always broken, see below.
>
> I mean push-back from UDP transmission to the physical NIC.
>
> Your patch breaks that
I think there's some misunderstanding. pushback is only disabled
for destinations that set IFF_TX_CAN_STALL. I expect that
no physical NICs set this flag - only tun and possibly
other userspace-controlled devices in the future.
> as now the guest will have no push-back
> whatsoever so anything that transmits UDP without protocol-level
> congestion control will start dropping most of their packets.
>
> Granted you can argue that these apps are broken, but they do
> exist and we've always catered for them, both on baremetal and
> under virtualisation.
> > Thus we get this situation
> > tap1 sends packets, some of them to tap2, tap2 does not consume them,
> > as a result tap2 queue overflows, tap2 stops forever and
> > packets get queued in the qdisc, now tap1
> > send buffer gets full so it can not communicate to any destination.
> >
> > So the problem is one VM can block all networking from another one.
>
> This should be addressed in the backend, as it can distinguish
> between packets going out to physical and packets stuck going to
> a local VM.
Sorry I still have no clue what you call a backend.
Could you clarify please? All packets from a tun device
go to userspace.
> In the latter case you can then duplicate and release
> the sender's memory.
>
> Cheers,
The problem this patch is trying to address is tun device
can get stopped forever, which causes packets to
accumulate in qdisc again forever. So tun does not
get a chance to release sender's memory for these packets.
> --
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] net: orphan queued skbs if device tx can stall
From: Herbert Xu @ 2012-04-09 8:29 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: netdev, linux-kernel, David S. Miller, Jamal Hadi Salim,
Stephen Hemminger, Jason Wang, Neil Horman, Jiri Pirko,
Jeff Kirsher, Eric Dumazet, Michał Mirosław,
Ben Hutchings
In-Reply-To: <20120409073953.GD12014@redhat.com>
On Mon, Apr 09, 2012 at 10:39:54AM +0300, Michael S. Tsirkin wrote:
> On Mon, Apr 09, 2012 at 03:33:54PM +0800, Herbert Xu wrote:
> > On Mon, Apr 09, 2012 at 10:28:49AM +0300, Michael S. Tsirkin wrote:
> > >
> > > > 1) Doesn't this break local UDP push-back?
> > >
> > > What is meant by UDP pushback here? Two tap
> > > devices communicating by UDP packets locally?
> > > This was always broken, see below.
> >
> > I mean push-back from UDP transmission to the physical NIC.
> >
> > Your patch breaks that
>
> I think there's some misunderstanding. pushback is only disabled
> for destinations that set IFF_TX_CAN_STALL. I expect that
> no physical NICs set this flag - only tun and possibly
> other userspace-controlled devices in the future.
I'm talking about an app running in the guest transmitting UDP
to the physical NIC via virtio/vhost. This will break with your
patch.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] net: orphan queued skbs if device tx can stall
From: Michael S. Tsirkin @ 2012-04-09 8:34 UTC (permalink / raw)
To: Herbert Xu
Cc: netdev, linux-kernel, David S. Miller, Jamal Hadi Salim,
Stephen Hemminger, Jason Wang, Neil Horman, Jiri Pirko,
Jeff Kirsher, Eric Dumazet, Michał Mirosław,
Ben Hutchings
In-Reply-To: <20120409082919.GA3471@gondor.apana.org.au>
On Mon, Apr 09, 2012 at 04:29:20PM +0800, Herbert Xu wrote:
> On Mon, Apr 09, 2012 at 10:39:54AM +0300, Michael S. Tsirkin wrote:
> > On Mon, Apr 09, 2012 at 03:33:54PM +0800, Herbert Xu wrote:
> > > On Mon, Apr 09, 2012 at 10:28:49AM +0300, Michael S. Tsirkin wrote:
> > > >
> > > > > 1) Doesn't this break local UDP push-back?
> > > >
> > > > What is meant by UDP pushback here? Two tap
> > > > devices communicating by UDP packets locally?
> > > > This was always broken, see below.
> > >
> > > I mean push-back from UDP transmission to the physical NIC.
> > >
> > > Your patch breaks that
> >
> > I think there's some misunderstanding. pushback is only disabled
> > for destinations that set IFF_TX_CAN_STALL. I expect that
> > no physical NICs set this flag - only tun and possibly
> > other userspace-controlled devices in the future.
>
> I'm talking about an app running in the guest transmitting UDP
> to the physical NIC via virtio/vhost. This will break with your
> patch.
>
> Cheers,
Puzzled. This patch orphans skbs only if the destination device sets
IFF_TX_CAN_STALL. Since the physical NIC doesn't stall forever it never
sets this flag.
So it seems that this patch should not affect the configuration
you describe at all.
Could you please clarify? What did I miss?
> --
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] net: orphan queued skbs if device tx can stall
From: Herbert Xu @ 2012-04-09 8:39 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: netdev, linux-kernel, David S. Miller, Jamal Hadi Salim,
Stephen Hemminger, Jason Wang, Neil Horman, Jiri Pirko,
Jeff Kirsher, Eric Dumazet, Michał Mirosław,
Ben Hutchings
In-Reply-To: <20120409083402.GA28249@redhat.com>
On Mon, Apr 09, 2012 at 11:34:02AM +0300, Michael S. Tsirkin wrote:
>
> Puzzled. This patch orphans skbs only if the destination device sets
> IFF_TX_CAN_STALL. Since the physical NIC doesn't stall forever it never
> sets this flag.
>
> So it seems that this patch should not affect the configuration
> you describe at all.
>
> Could you please clarify? What did I miss?
Oops, you're right. Somehow I was thinking you were patching
virtio-net instead of tun.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* [PATCH] net/key/af_key.c: add missing kfree_skb
From: Julia Lawall @ 2012-04-09 8:41 UTC (permalink / raw)
To: David S. Miller; +Cc: kernel-janitors, netdev, linux-kernel
From: Julia Lawall <Julia.Lawall@lip6.fr>
At the point of this error-handling code, alloc_skb has succeded, so free
the resulting skb by jumping to the err label.
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
net/key/af_key.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/key/af_key.c b/net/key/af_key.c
index 11dbb22..7e5d927 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -3480,7 +3480,7 @@ static int pfkey_send_migrate(const struct xfrm_selector *sel, u8 dir, u8 type,
/* Addresses to be used by KM for negotiation, if ext is available */
if (k != NULL && (set_sadb_kmaddress(skb, k) < 0))
- return -EINVAL;
+ goto err;
/* selector src */
set_sadb_address(skb, sasize_sel, SADB_EXT_ADDRESS_SRC, sel);
^ permalink raw reply related
* Re: [PATCH] net: orphan queued skbs if device tx can stall
From: Michael S. Tsirkin @ 2012-04-09 8:42 UTC (permalink / raw)
To: Herbert Xu
Cc: netdev, linux-kernel, David S. Miller, Jamal Hadi Salim,
Stephen Hemminger, Jason Wang, Neil Horman, Jiri Pirko,
Jeff Kirsher, Eric Dumazet, Michał Mirosław,
Ben Hutchings
In-Reply-To: <20120409083958.GA3951@gondor.apana.org.au>
On Mon, Apr 09, 2012 at 04:39:58PM +0800, Herbert Xu wrote:
> On Mon, Apr 09, 2012 at 11:34:02AM +0300, Michael S. Tsirkin wrote:
> >
> > Puzzled. This patch orphans skbs only if the destination device sets
> > IFF_TX_CAN_STALL. Since the physical NIC doesn't stall forever it never
> > sets this flag.
> >
> > So it seems that this patch should not affect the configuration
> > you describe at all.
> >
> > Could you please clarify? What did I miss?
>
> Oops, you're right. Somehow I was thinking you were patching
> virtio-net instead of tun.
>
> Cheers,
Cool. Want to ack the patch then just to make it clear you are happy?
> --
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [RFC] API to modify /proc/sys/net/ipv4/ip_local_reserved_ports
From: Cong Wang @ 2012-04-09 8:43 UTC (permalink / raw)
To: Helge Deller
Cc: Octavian Purdila, netdev, David Miller, Andrew Morton,
Eric W. Biederman, Frank Danapfel, Laszlo Ersek, shemminger
In-Reply-To: <4F7CADE8.3060205@gmx.de>
On Wed, 2012-04-04 at 22:24 +0200, Helge Deller wrote:
> I would like to follow up on my last patch series to be able to modify
> the contents of the /proc/sys/net/ipv4/ip_local_reserved_ports port list
> from userspace.
>
> My last patch (https://lkml.org/lkml/2012/3/10/187) was based on
> modifications to the proc interface, which - based on the feedback here
> on the list - seemed to not be the right way to go (although I personally
> still like the idea very much :-)).
>
> Anyway, with this RFC I would like to get feedback about a new proposed
> API and attached kernel patch.
>
> The idea is to introduce a new <optname> value for get/setsockopt()
> named SO_RESERVED_PORTS to get/set the ip_local_reserved_ports
> bitmap via standard get/setsockopt() syscalls.
> As far as I understand this seems to be similiar to how iptables works.
>
> An untested kernel patch for review and feedback is attached below.
>
> In userspace it then would be possible to write a new tool or to extend
> for example the "ip" tool to accept commands like:
> $> ip reserved_ports add 100-2000
> $> ip reserved_ports remove 50-60
> $> ip reserved_ports list (to show current reserved port list)
>
> This userspace tool could then read the port bitmap from kernel via
> a) socket(PF_INET, SOCK_RAW, IPPROTO_RAW)
> b) getsockopt(3, SOL_SOCKET, SO_RESERVED_PORTS, <bitmaplist>)
> and write back the results after modification via
> c) setsockopt(3, SOL_SOCKET, SO_RESERVED_PORTS, <bitmaplist>)
>
> Would that be an acceptable solution?
Hmm, it is indeed that bitmap fits for syscall rather than /proc file.
But it seems that using getsockopt()/setsockopt() makes it like it is a
per-socket setting, actually it is a system-wide setting. So I am
wondering if exporting a binary /proc file for this is a better
solution.
Thanks.
^ permalink raw reply
* [PATCH] net/wireless/wext-core.c: add missing kfree
From: Julia Lawall @ 2012-04-09 9:01 UTC (permalink / raw)
To: John W. Linville
Cc: kernel-janitors-u79uwXL29TY76Z2rM5mHXA, David S. Miller,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
From: Julia Lawall <Julia.Lawall-L2FTfq7BK8M@public.gmane.org>
Free extra as done in the error-handling code just above.
Signed-off-by: Julia Lawall <Julia.Lawall-L2FTfq7BK8M@public.gmane.org>
---
net/wireless/wext-core.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/wireless/wext-core.c b/net/wireless/wext-core.c
index 9f544c9..22adfeb 100644
--- a/net/wireless/wext-core.c
+++ b/net/wireless/wext-core.c
@@ -781,8 +781,10 @@ static int ioctl_standard_iw_point(struct iw_point *iwp, unsigned int cmd,
if (cmd == SIOCSIWENCODEEXT) {
struct iw_encode_ext *ee = (void *) extra;
- if (iwp->length < sizeof(*ee) + ee->key_len)
- return -EFAULT;
+ if (iwp->length < sizeof(*ee) + ee->key_len) {
+ err = -EFAULT;
+ goto out;
+ }
}
}
--
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 related
* Re: [PATCH] net: orphan queued skbs if device tx can stall
From: Eric Dumazet @ 2012-04-09 9:13 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Herbert Xu, netdev, linux-kernel, David S. Miller,
Jamal Hadi Salim, Stephen Hemminger, Jason Wang, Neil Horman,
Jiri Pirko, Jeff Kirsher, Michał Mirosław,
Ben Hutchings
In-Reply-To: <20120409084208.GA28362@redhat.com>
On Mon, 2012-04-09 at 11:42 +0300, Michael S. Tsirkin wrote:
> On Mon, Apr 09, 2012 at 04:39:58PM +0800, Herbert Xu wrote:
> > On Mon, Apr 09, 2012 at 11:34:02AM +0300, Michael S. Tsirkin wrote:
> > >
> > > Puzzled. This patch orphans skbs only if the destination device sets
> > > IFF_TX_CAN_STALL. Since the physical NIC doesn't stall forever it never
> > > sets this flag.
> > >
> > > So it seems that this patch should not affect the configuration
> > > you describe at all.
> > >
> > > Could you please clarify? What did I miss?
> >
> > Oops, you're right. Somehow I was thinking you were patching
> > virtio-net instead of tun.
> >
> > Cheers,
>
> Cool. Want to ack the patch then just to make it clear you are happy?
I am not happy with this patch.
Since its Easter, I dont have much time to comment right now, I'll do
that tomorrow.
^ permalink raw reply
* [PATCH] skbuff: struct ubuf_info callback type safety
From: Michael S. Tsirkin @ 2012-04-09 10:24 UTC (permalink / raw)
To: linux-kernel, kvm, virtualization, netdev
Cc: Shirley Ma, Stephen Hemminger, David S. Miller, Eric Dumazet,
Ian Campbell
The skb struct ubuf_info callback gets passed struct ubuf_info
itself, not the arg value as the field name and the function signature
seem to imply. Rename the arg field to ctx to match usage,
add documentation and change the callback argument type
to make usage clear and to have compiler check correctness.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/vhost/net.c | 2 +-
drivers/vhost/vhost.c | 5 ++---
drivers/vhost/vhost.h | 2 +-
include/linux/skbuff.h | 7 ++++---
4 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index f0da2c3..1f21d2a 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -238,7 +238,7 @@ static void handle_tx(struct vhost_net *net)
vq->heads[vq->upend_idx].len = len;
ubuf->callback = vhost_zerocopy_callback;
- ubuf->arg = vq->ubufs;
+ ubuf->ctx = vq->ubufs;
ubuf->desc = vq->upend_idx;
msg.msg_control = ubuf;
msg.msg_controllen = sizeof(ubuf);
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 947f00d..51e4c1e 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1598,10 +1598,9 @@ void vhost_ubuf_put_and_wait(struct vhost_ubuf_ref *ubufs)
kfree(ubufs);
}
-void vhost_zerocopy_callback(void *arg)
+void vhost_zerocopy_callback(struct ubuf_info *ubuf)
{
- struct ubuf_info *ubuf = arg;
- struct vhost_ubuf_ref *ubufs = ubuf->arg;
+ struct vhost_ubuf_ref *ubufs = ubuf->ctx;
struct vhost_virtqueue *vq = ubufs->vq;
/* set len = 1 to mark this desc buffers done DMA */
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 8dcf4cc..8de1fd5 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -188,7 +188,7 @@ bool vhost_enable_notify(struct vhost_dev *, struct vhost_virtqueue *);
int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
unsigned int log_num, u64 len);
-void vhost_zerocopy_callback(void *arg);
+void vhost_zerocopy_callback(struct ubuf_info *);
int vhost_zerocopy_signal_used(struct vhost_virtqueue *vq);
#define vq_err(vq, fmt, ...) do { \
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 3337027..4c3f138 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -238,11 +238,12 @@ enum {
/*
* The callback notifies userspace to release buffers when skb DMA is done in
* lower device, the skb last reference should be 0 when calling this.
- * The desc is used to track userspace buffer index.
+ * The ctx field is used to track device context.
+ * The desc field is used to track userspace buffer index.
*/
struct ubuf_info {
- void (*callback)(void *);
- void *arg;
+ void (*callback)(struct ubuf_info *);
+ void *ctx;
unsigned long desc;
};
--
1.7.9.111.gf3fb0
^ permalink raw reply related
* [PATCH] davinci_emac: Add cpu_freq support
From: Manjunathappa, Prakash @ 2012-04-09 10:49 UTC (permalink / raw)
To: netdev
Cc: davem, linux-kernel, davinci-linux-open-source,
Manjunathappa, Prakash
Reconfigure interrupt coalesce parameter for changed emac bus_freq
due to DVFS.
Signed-off-by: Manjunathappa, Prakash <prakash.pm@ti.com>
---
drivers/net/ethernet/ti/davinci_emac.c | 60 ++++++++++++++++++++++++++++++++
1 files changed, 60 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
index 174a334..11d3bd7 100644
--- a/drivers/net/ethernet/ti/davinci_emac.c
+++ b/drivers/net/ethernet/ti/davinci_emac.c
@@ -50,6 +50,7 @@
#include <linux/ctype.h>
#include <linux/spinlock.h>
#include <linux/dma-mapping.h>
+#include <linux/cpufreq.h>
#include <linux/clk.h>
#include <linux/platform_device.h>
#include <linux/semaphore.h>
@@ -344,6 +345,9 @@ struct emac_priv {
/*platform specific members*/
void (*int_enable) (void);
void (*int_disable) (void);
+#ifdef CONFIG_CPU_FREQ
+ struct notifier_block freq_transition;
+#endif
};
/* clock frequency for EMAC */
@@ -1761,6 +1765,46 @@ static const struct net_device_ops emac_netdev_ops = {
#endif
};
+#ifdef CONFIG_CPU_FREQ
+static int davinci_emac_cpufreq_transition(struct notifier_block *nb,
+ unsigned long val, void *data)
+{
+ int ret = 0;
+ struct emac_priv *priv;
+
+ priv = container_of(nb, struct emac_priv, freq_transition);
+ if (priv->coal_intvl != 0) {
+ if (val == CPUFREQ_POSTCHANGE) {
+ if (emac_bus_frequency != clk_get_rate(emac_clk)) {
+ struct ethtool_coalesce coal;
+
+ emac_bus_frequency = clk_get_rate(emac_clk);
+
+ priv->bus_freq_mhz = (u32)(emac_bus_frequency /
+ 1000000);
+ coal.rx_coalesce_usecs = (priv->coal_intvl
+ << 4);
+ ret = emac_set_coalesce(priv->ndev, &coal);
+ }
+ }
+ }
+ return ret;
+}
+
+static inline int davinci_emac_cpufreq_register(struct emac_priv *priv)
+{
+ priv->freq_transition.notifier_call = davinci_emac_cpufreq_transition;
+ return cpufreq_register_notifier(&priv->freq_transition,
+ CPUFREQ_TRANSITION_NOTIFIER);
+}
+
+static inline void davinci_emac_cpufreq_deregister(struct emac_priv *priv)
+{
+ cpufreq_unregister_notifier(&priv->freq_transition,
+ CPUFREQ_TRANSITION_NOTIFIER);
+}
+#endif
+
/**
* davinci_emac_probe: EMAC device probe
* @pdev: The DaVinci EMAC device that we are removing
@@ -1925,8 +1969,21 @@ static int __devinit davinci_emac_probe(struct platform_device *pdev)
"(regs: %p, irq: %d)\n",
(void *)priv->emac_base_phys, ndev->irq);
}
+
+#ifdef CONFIG_CPU_FREQ
+ rc = davinci_emac_cpufreq_register(priv);
+ if (rc) {
+ dev_err(&pdev->dev, "error in register_netdev\n");
+ rc = -ENODEV;
+ goto cpufreq_reg_err;
+ }
+#endif
return 0;
+#ifdef CONFIG_CPU_FREQ
+cpufreq_reg_err:
+ unregister_netdev(ndev);
+#endif
netdev_reg_err:
clk_disable(emac_clk);
no_irq_res:
@@ -1973,6 +2030,9 @@ static int __devexit davinci_emac_remove(struct platform_device *pdev)
release_mem_region(res->start, resource_size(res));
+#ifdef CONFIG_CPU_FREQ
+ davinci_emac_cpufreq_deregister(priv);
+#endif
unregister_netdev(ndev);
iounmap(priv->remap_addr);
free_netdev(ndev);
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] davinci_emac: Add cpu_freq support
From: Sergei Shtylyov @ 2012-04-09 11:29 UTC (permalink / raw)
To: Manjunathappa, Prakash
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1333968549-3282-1-git-send-email-prakash.pm-l0cyMroinI0@public.gmane.org>
Hello.
On 09-04-2012 14:49, Manjunathappa, Prakash wrote:
> Reconfigure interrupt coalesce parameter for changed emac bus_freq
> due to DVFS.
> Signed-off-by: Manjunathappa, Prakash<prakash.pm-l0cyMroinI0@public.gmane.org>
> ---
> drivers/net/ethernet/ti/davinci_emac.c | 60 ++++++++++++++++++++++++++++++++
> 1 files changed, 60 insertions(+), 0 deletions(-)
> diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
> index 174a334..11d3bd7 100644
> --- a/drivers/net/ethernet/ti/davinci_emac.c
> +++ b/drivers/net/ethernet/ti/davinci_emac.c
[...]
> @@ -1761,6 +1765,46 @@ static const struct net_device_ops emac_netdev_ops = {
> #endif
> };
>
> +#ifdef CONFIG_CPU_FREQ
> +static int davinci_emac_cpufreq_transition(struct notifier_block *nb,
> + unsigned long val, void *data)
> +{
> + int ret = 0;
> + struct emac_priv *priv;
> +
> + priv = container_of(nb, struct emac_priv, freq_transition);
> + if (priv->coal_intvl != 0) {
> + if (val == CPUFREQ_POSTCHANGE) {
> + if (emac_bus_frequency != clk_get_rate(emac_clk)) {
These 3 *if*s could be collapsed into one, and so indentation level
lowered significantly.
> + struct ethtool_coalesce coal;
> +
> + emac_bus_frequency = clk_get_rate(emac_clk);
> +
> + priv->bus_freq_mhz = (u32)(emac_bus_frequency /
> + 1000000);
> + coal.rx_coalesce_usecs = (priv->coal_intvl
> + << 4);
> + ret = emac_set_coalesce(priv->ndev,&coal);
> + }
> + }
> + }
> + return ret;
> +}
> +
> +static inline int davinci_emac_cpufreq_register(struct emac_priv *priv)
> +{
> + priv->freq_transition.notifier_call = davinci_emac_cpufreq_transition;
> + return cpufreq_register_notifier(&priv->freq_transition,
> + CPUFREQ_TRANSITION_NOTIFIER);
> +}
> +
> +static inline void davinci_emac_cpufreq_deregister(struct emac_priv *priv)
> +{
> + cpufreq_unregister_notifier(&priv->freq_transition,
> + CPUFREQ_TRANSITION_NOTIFIER);
> +}
> +#endif
> +
> /**
> * davinci_emac_probe: EMAC device probe
> * @pdev: The DaVinci EMAC device that we are removing
> @@ -1925,8 +1969,21 @@ static int __devinit davinci_emac_probe(struct platform_device *pdev)
> "(regs: %p, irq: %d)\n",
> (void *)priv->emac_base_phys, ndev->irq);
> }
> +
> +#ifdef CONFIG_CPU_FREQ
> + rc = davinci_emac_cpufreq_register(priv);
> + if (rc) {
> + dev_err(&pdev->dev, "error in register_netdev\n");
Really?
> + rc = -ENODEV;
> + goto cpufreq_reg_err;
> + }
> +#endif
> return 0;
>
> +#ifdef CONFIG_CPU_FREQ
> +cpufreq_reg_err:
> + unregister_netdev(ndev);
> +#endif
> netdev_reg_err:
> clk_disable(emac_clk);
> no_irq_res:
> @@ -1973,6 +2030,9 @@ static int __devexit davinci_emac_remove(struct platform_device *pdev)
>
> release_mem_region(res->start, resource_size(res));
>
> +#ifdef CONFIG_CPU_FREQ
> + davinci_emac_cpufreq_deregister(priv);
> +#endif
This is considered a bad practice to use #ifdef in the body of function.
Define the faunction you call here as empty inline in case CONFIG_CPU_FREQ is
not defined instead. The same about davinci_emac_cpufreq_register().
WBR, Sergei
^ permalink raw reply
* Re: [PATCH 6/6] tilegx network driver: initial support
From: Arnd Bergmann @ 2012-04-09 13:49 UTC (permalink / raw)
To: Chris Metcalf; +Cc: linux-kernel, netdev
In-Reply-To: <201204062101.q36L1uCi011406@farm-0027.internal.tilera.com>
On Friday 06 April 2012, Chris Metcalf wrote:
> This change adds support for the tilegx network driver based on the
> GXIO IORPC support in the tilegx software stack, using the on-chip
> mPIPE packet processing engine.
>
> Signed-off-by: Chris Metcalf <cmetcalf@tilera.com>
> ---
> drivers/net/ethernet/tile/Kconfig | 1 +
> drivers/net/ethernet/tile/Makefile | 4 +-
> drivers/net/ethernet/tile/tilegx.c | 2045 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 2048 insertions(+), 2 deletions(-)
> create mode 100644 drivers/net/ethernet/tile/tilegx.c
I think the directory name should be the company, not the architecture here, so make
it drivers/net/ethernet/tilera/tilegx.c instead.
> +
> +MODULE_AUTHOR("Tilera");
> +MODULE_LICENSE("GPL");
> +
MODULE_AUTHOR is normally a real person with an email address.
> +/* Statistics counters for a specific cpu and device. */
> +struct tile_net_stats_t {
> + u32 rx_packets;
> + u32 rx_bytes;
> + u32 tx_packets;
> + u32 tx_bytes;
> +};
I think you need to drop the _t postfix here, which presumably comes
from converting it from a typedef.
> +
> +/* The actual devices. */
> +static struct net_device *tile_net_devs[TILE_NET_DEVS];
> +
> +/* The device for a given channel. HACK: We use "32", not
> + * TILE_NET_CHANNELS, because it is fairly subtle that the 5 bit
> + * "idesc.channel" field never exceeds TILE_NET_CHANNELS.
> + */
> +static struct net_device *tile_net_devs_for_channel[32];
When you need to keep a list or array of device structures in a driver, you're
usually doing something very wrong. The convention is to just pass the pointer
around to where you need it.
> +
> +/* Convert a "buffer ptr" into a "buffer cpa". */
> +static inline void *buf_to_cpa(void *buf)
> +{
> + return (void *)__pa(buf);
> +}
> +
> +
> +/* Convert a "buffer cpa" into a "buffer ptr". */
> +static inline void *cpa_to_buf(void *cpa)
> +{
> + return (void *)__va(cpa);
> +}
This is almost certainly wrong: The type returned by __pa is a phys_addr_t,
which cannot be dereferenced like a pointer. On normal drivers, you would
use dma_map_single()/dma_unmap_single() to get a token that can get
passed into a dma engine. From what I can tell, this device is directly mapped,
while your PCI uses an IOMMU, so that would require two different
implementations of dma mapping operations.
> +/* Allocate and push a buffer. */
> +static bool tile_net_provide_buffer(bool small)
> +{
> + int stack = small ? small_buffer_stack : large_buffer_stack;
> +
> + /* Buffers must be aligned. */
> + const unsigned long align = 128;
> +
> + /* Note that "dev_alloc_skb()" adds NET_SKB_PAD more bytes,
> + * and also "reserves" that many bytes.
> + */
> + int len = sizeof(struct sk_buff **) + align + (small ? 128 : 1664);
> +
> + /* Allocate (or fail). */
> + struct sk_buff *skb = dev_alloc_skb(len);
> + if (skb == NULL)
> + return false;
> +
> + /* Make room for a back-pointer to 'skb'. */
> + skb_reserve(skb, sizeof(struct sk_buff **));
> +
> + /* Make sure we are aligned. */
> + skb_reserve(skb, -(long)skb->data & (align - 1));
> +
> + /* Save a back-pointer to 'skb'. */
> + *(struct sk_buff **)(skb->data - sizeof(struct sk_buff **)) = skb;
This looks very wrong: why would you put the pointer to the skb into the
skb itself?
> + /* Make sure "skb" and the back-pointer have been flushed. */
> + __insn_mf();
Try to use archicture independent names for flush operations like this
to make it more readable. I assume this should be smp_wmb()?
> +
> + /* Compute the "ip checksum". */
> + jsum = isum_hack + htons(s_len - eh_len) + htons(id);
> + jsum = __insn_v2sadu(jsum, 0);
> + jsum = __insn_v2sadu(jsum, 0);
> + jsum = (0xFFFF ^ jsum);
> + jh->check = jsum;
> +
> + /* Update the tcp "seq". */
> + uh->seq = htonl(seq);
> +
> + /* Update some flags. */
> + if (!final)
> + uh->fin = uh->psh = 0;
> +
> + /* Compute the tcp pseudo-header checksum. */
> + usum = tsum_hack + htons(s_len);
> + usum = __insn_v2sadu(usum, 0);
> + usum = __insn_v2sadu(usum, 0);
> + uh->check = usum;
Why to you open-code the ip checksum functions here? Normally the stack takes
care of this by calling the functions you already provide in
arch/tile/lib/checksum.c
Arnd
^ permalink raw reply
* [PATCH] net/garp: fix GID rbtree ordering
From: David Ward @ 2012-04-09 14:13 UTC (permalink / raw)
To: netdev; +Cc: Jorge Boncompte [DTI2], David Ward
The comparison operators were backwards in both garp_attr_lookup and
garp_attr_create, so the entire GID rbtree was in reverse order.
(There was no practical side effect to this though, except that PDUs
were sent with attributes listed in reverse order, which is still
valid by the protocol. This change is only for clarity.)
Signed-off-by: David Ward <david.ward@ll.mit.edu>
---
net/802/garp.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/802/garp.c b/net/802/garp.c
index a5c2248..8456f5d 100644
--- a/net/802/garp.c
+++ b/net/802/garp.c
@@ -157,9 +157,9 @@ static struct garp_attr *garp_attr_lookup(const struct garp_applicant *app,
while (parent) {
attr = rb_entry(parent, struct garp_attr, node);
d = garp_attr_cmp(attr, data, len, type);
- if (d < 0)
+ if (d > 0)
parent = parent->rb_left;
- else if (d > 0)
+ else if (d < 0)
parent = parent->rb_right;
else
return attr;
@@ -178,9 +178,9 @@ static struct garp_attr *garp_attr_create(struct garp_applicant *app,
parent = *p;
attr = rb_entry(parent, struct garp_attr, node);
d = garp_attr_cmp(attr, data, len, type);
- if (d < 0)
+ if (d > 0)
p = &parent->rb_left;
- else if (d > 0)
+ else if (d < 0)
p = &parent->rb_right;
else {
/* The attribute already exists; re-use it. */
--
1.7.4.1
^ permalink raw reply related
* [PATCH]Fix misplaces parenthesis in virtio_net.c
From: Torsten Kaiser @ 2012-04-09 15:14 UTC (permalink / raw)
To: David Miller, Rick Jones; +Cc: netdev, linux-kernel
Commit 2e57b79ccef1ff1422fdf45a9b28fe60f8f084f7 misplaced its
parenthesis and now tx_fifo_errors will only be incremented if an
ENOMEM error is not written to the syslog.
Correct the parenthesis and indentation to the original goal of
counting all non ENOMEM errors and ratelimiting only the messages.
Signed-of-by: Torsten Kaiser <just.for.lkml@googlemail.com>
--- a/drivers/net/virtio_net.c 2012-04-09 16:55:31.366847092 +0200
+++ b/drivers/net/virtio_net.c 2012-04-09 16:56:31.196847799 +0200
@@ -626,16 +626,15 @@
/* This can happen with OOM and indirect buffers. */
if (unlikely(capacity < 0)) {
if (likely(capacity == -ENOMEM)) {
- if (net_ratelimit()) {
+ if (net_ratelimit())
dev_warn(&dev->dev,
"TX queue failure: out of memory\n");
- } else {
+ } else {
dev->stats.tx_fifo_errors++;
if (net_ratelimit())
dev_warn(&dev->dev,
"Unexpected TX queue failure: %d\n",
capacity);
- }
}
dev->stats.tx_dropped++;
kfree_skb(skb);
^ permalink raw reply
* Re: [PATCH] davinci_emac: Add cpu_freq support
From: Ben Hutchings @ 2012-04-09 15:56 UTC (permalink / raw)
To: Manjunathappa, Prakash
Cc: netdev, davem, linux-kernel, davinci-linux-open-source
In-Reply-To: <1333968549-3282-1-git-send-email-prakash.pm@ti.com>
On Mon, 2012-04-09 at 16:19 +0530, Manjunathappa, Prakash wrote:
> Reconfigure interrupt coalesce parameter for changed emac bus_freq
> due to DVFS.
>
> Signed-off-by: Manjunathappa, Prakash <prakash.pm@ti.com>
> ---
> drivers/net/ethernet/ti/davinci_emac.c | 60 ++++++++++++++++++++++++++++++++
> 1 files changed, 60 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
> index 174a334..11d3bd7 100644
> --- a/drivers/net/ethernet/ti/davinci_emac.c
> +++ b/drivers/net/ethernet/ti/davinci_emac.c
> @@ -50,6 +50,7 @@
> #include <linux/ctype.h>
> #include <linux/spinlock.h>
> #include <linux/dma-mapping.h>
> +#include <linux/cpufreq.h>
> #include <linux/clk.h>
> #include <linux/platform_device.h>
> #include <linux/semaphore.h>
> @@ -344,6 +345,9 @@ struct emac_priv {
> /*platform specific members*/
> void (*int_enable) (void);
> void (*int_disable) (void);
> +#ifdef CONFIG_CPU_FREQ
> + struct notifier_block freq_transition;
> +#endif
> };
>
> /* clock frequency for EMAC */
> @@ -1761,6 +1765,46 @@ static const struct net_device_ops emac_netdev_ops = {
> #endif
> };
>
> +#ifdef CONFIG_CPU_FREQ
> +static int davinci_emac_cpufreq_transition(struct notifier_block *nb,
> + unsigned long val, void *data)
> +{
> + int ret = 0;
> + struct emac_priv *priv;
> +
> + priv = container_of(nb, struct emac_priv, freq_transition);
> + if (priv->coal_intvl != 0) {
> + if (val == CPUFREQ_POSTCHANGE) {
> + if (emac_bus_frequency != clk_get_rate(emac_clk)) {
> + struct ethtool_coalesce coal;
> +
> + emac_bus_frequency = clk_get_rate(emac_clk);
> +
> + priv->bus_freq_mhz = (u32)(emac_bus_frequency /
> + 1000000);
> + coal.rx_coalesce_usecs = (priv->coal_intvl
> + << 4);
> + ret = emac_set_coalesce(priv->ndev, &coal);
[...]
Ick. Why don't you break up emac_set_coalesce() so that you can push
this one value to the hardware without faking up a struct
ethtool_coalesce? Don't you need to push it on reset/resume anyway?
Ben.
--
Ben Hutchings, Staff 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
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