* Re: Receive side performance issue with multi-10-GigE and NUMA
From: Andrew Gallatin @ 2009-08-08 1:03 UTC (permalink / raw)
To: Bill Fink; +Cc: Brice Goglin, Linux Network Developers, Yinghai Lu
In-Reply-To: <20090807175112.a1f57407.billfink@mindspring.com>
Bill Fink wrote:
> All sysfs local_cpus values are the same (00000000,000000ff),
> so yes they are also wrong.
How were you handling IRQ binding? If local_cpus is wrong,
the irqbalance will not be able to make good decisions about
where to bind the NICs' IRQs. Did you try manually binding
each NICs's interrupt to a separate CPU on the correct node?
Regards,
Drew
^ permalink raw reply
* Re: Receive side performance issue with multi-10-GigE and NUMA
From: Neil Horman @ 2009-08-08 1:56 UTC (permalink / raw)
To: Bill Fink; +Cc: Linux Network Developers, brice, gallatin
In-Reply-To: <20090807205442.32918186.billfink@mindspring.com>
On Fri, Aug 07, 2009 at 08:54:42PM -0400, Bill Fink wrote:
> On Fri, 7 Aug 2009, Neil Horman wrote:
>
> > You're timing is impeccable! I just posted a patch for an ftrace module to help
> > detect just these kind of conditions:
> > http://marc.info/?l=linux-netdev&m=124967650218846&w=2
> >
> > Hope that helps you out
> > Neil
>
> Thanks! It could be helpful. Do you have a pointer to documentation
> on how to use it? And does it require the latest GIT kernel or could
> it possibly be used with a 2.6.29.6 kernel?
>
> -Bill
>
It should apply to 2.6.29.6 no problem (might take a little massaging, but not
much).
No docs I'm afraid (sorry, I'm horrible about that)
Using it is easy though:
1) Patch, build and boot the kernel (make sure to have
CONFIG_SKB_SOURCES_TRACER, along with the other FTRACE requisite options)
2) mount -t debugfs nodev /sys/kernel/debug
3) cd /sys/kernel/debug/tracing
4) echo skb_sources > ./current_tracer
5) echo 1 > trace
6) cat ./trace
Step 5 clears the trace buffer. Step 6 provides you a list list this
PID ANID CNID RXQ CCPU LEN
Where:
PID - The process receiving an skb
ANID - The node which the skb being received was allocated on
CNID - The node which the process is running when it read this skb
RQQ - The NIC receive queue that received this skb
CCPU - The cpu the process was running on when it read the skb in question
LEN - The length of the skb being received
Each entry in the list denotes a unique skb (obviously), and with a clever awk
script you can identify which nodes each process in your system is receiving
frames from, so that you can use numactl or taskset to bias that process to run
on the same nodes cpus.
Note that step (6) wil show a larger list each time you cat that file (as trace
records aren't removed during a read. Step 5 is what actually clears the trace
buffer and resets the list length to zero.
Hope that helps. Please feel free to email me if you have any questions.
Regards
Neil
^ permalink raw reply
* [RESEND PATCH] gianfar: keep vlan related state when restart
From: Yong Zhang @ 2009-08-08 2:36 UTC (permalink / raw)
To: davem, afleming, dai.haruki; +Cc: netdev, linux-kernel
If vlan has been enabled. ifdown followed by ifup will lost hardware
related state.
Also remove duplicated operation in gfar_vlan_rx_register().
Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
---
drivers/net/gianfar.c | 12 ++++++++++--
1 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index f8ffcbf..e212f2c 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -936,6 +936,7 @@ int startup_gfar(struct net_device *dev)
struct gfar __iomem *regs = priv->regs;
int err = 0;
u32 rctrl = 0;
+ u32 tctrl = 0;
u32 attrs = 0;
gfar_write(®s->imask, IMASK_INIT_CLEAR);
@@ -1111,11 +1112,19 @@ int startup_gfar(struct net_device *dev)
rctrl |= RCTRL_PADDING(priv->padding);
}
+ /* keep vlan related bits if it's enabled */
+ if (priv->vlgrp) {
+ rctrl |= RCTRL_VLEX | RCTRL_PRSDEP_INIT;
+ tctrl |= TCTRL_VLINS;
+ }
+
/* Init rctrl based on our settings */
gfar_write(&priv->regs->rctrl, rctrl);
if (dev->features & NETIF_F_IP_CSUM)
- gfar_write(&priv->regs->tctrl, TCTRL_INIT_CSUM);
+ tctrl |= TCTRL_INIT_CSUM;
+
+ gfar_write(&priv->regs->tctrl, tctrl);
/* Set the extraction length and index */
attrs = ATTRELI_EL(priv->rx_stash_size) |
@@ -1450,7 +1459,6 @@ static void gfar_vlan_rx_register(struct net_device *dev,
/* Enable VLAN tag extraction */
tempval = gfar_read(&priv->regs->rctrl);
- tempval |= RCTRL_VLEX;
tempval |= (RCTRL_VLEX | RCTRL_PRSDEP_INIT);
gfar_write(&priv->regs->rctrl, tempval);
} else {
--
1.6.0.4
^ permalink raw reply related
* [PATCH] drivers/net/tokenring: Use status field rather than state field
From: Julia Lawall @ 2009-08-08 6:47 UTC (permalink / raw)
To: netdev, linux-kernel, kernel-janitors
From: Julia Lawall <julia@diku.dk>
In general in this file, it is the status field, not the state field, that
contains values like OPEN and CLOSED. Indeed, in the first error case, it
is the field status that is initialized. I have thus assumed that all of
the error handling code should be the same, and moved it to the end of the
function to emphasize its commonality.
Signed-off-by: Julia Lawall <julia@diku.dk>
---
drivers/net/tokenring/smctr.c | 36 +++++--------------
1 files changed, 10 insertions(+), 26 deletions(-)
diff --git a/drivers/net/tokenring/smctr.c b/drivers/net/tokenring/smctr.c
index 6515894..f789f51 100644
--- a/drivers/net/tokenring/smctr.c
+++ b/drivers/net/tokenring/smctr.c
@@ -3091,11 +3091,7 @@ static int smctr_lobe_media_test(struct net_device *dev)
/* Setup the lobe media test. */
smctr_lobe_media_test_cmd(dev);
if(smctr_wait_cmd(dev))
- {
- smctr_reset_adapter(dev);
- tp->status = CLOSED;
- return (LOBE_MEDIA_TEST_FAILED);
- }
+ goto err;
/* Tx lobe media test frames. */
for(i = 0; i < 1500; ++i)
@@ -3103,20 +3099,12 @@ static int smctr_lobe_media_test(struct net_device *dev)
if(smctr_send_lobe_media_test(dev))
{
if(perror)
- {
- smctr_reset_adapter(dev);
- tp->state = CLOSED;
- return (LOBE_MEDIA_TEST_FAILED);
- }
+ goto err;
else
{
perror = 1;
if(smctr_lobe_media_test_cmd(dev))
- {
- smctr_reset_adapter(dev);
- tp->state = CLOSED;
- return (LOBE_MEDIA_TEST_FAILED);
- }
+ goto err;
}
}
}
@@ -3124,28 +3112,24 @@ static int smctr_lobe_media_test(struct net_device *dev)
if(smctr_send_dat(dev))
{
if(smctr_send_dat(dev))
- {
- smctr_reset_adapter(dev);
- tp->state = CLOSED;
- return (LOBE_MEDIA_TEST_FAILED);
- }
+ goto err;
}
/* Check if any frames received during test. */
if((tp->rx_fcb_curr[MAC_QUEUE]->frame_status)
|| (tp->rx_fcb_curr[NON_MAC_QUEUE]->frame_status))
- {
- smctr_reset_adapter(dev);
- tp->state = CLOSED;
- return (LOBE_MEDIA_TEST_FAILED);
- }
+ goto err;
/* Set receive mask to "Promisc" mode. */
tp->receive_mask = saved_rcv_mask;
smctr_chg_rx_mask(dev);
- return (0);
+ return 0;
+err:
+ smctr_reset_adapter(dev);
+ tp->status = CLOSED;
+ return LOBE_MEDIA_TEST_FAILED;
}
static int smctr_lobe_media_test_cmd(struct net_device *dev)
^ permalink raw reply related
* [PATCH] drivres/net: Change constant name
From: Julia Lawall @ 2009-08-08 7:53 UTC (permalink / raw)
To: netdev, linux-kernel, kernel-janitors
From: Julia Lawall <julia@diku.dk>
In this series of tests the constants have the form XRXMAC_STATUS, except
in this one case. The values of XRXMAC_STAT_MSK_RXOCTET_CNT_EXP and
XRXMAC_STATUS_RXOCTET_CNT_EXP are furthermore the same.
Signed-off-by: Julia Lawall <julia@diku.dk>
---
drivers/net/niu.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/niu.c b/drivers/net/niu.c
index d2146d4..3ada7ea 100644
--- a/drivers/net/niu.c
+++ b/drivers/net/niu.c
@@ -4015,7 +4015,7 @@ static void niu_xmac_interrupt(struct niu *np)
mp->rx_hist_cnt6 += RXMAC_HIST_CNT6_COUNT;
if (val & XRXMAC_STATUS_RXHIST7_CNT_EXP)
mp->rx_hist_cnt7 += RXMAC_HIST_CNT7_COUNT;
- if (val & XRXMAC_STAT_MSK_RXOCTET_CNT_EXP)
+ if (val & XRXMAC_STATUS_RXOCTET_CNT_EXP)
mp->rx_octets += RXMAC_BT_CNT_COUNT;
if (val & XRXMAC_STATUS_CVIOLERR_CNT_EXP)
mp->rx_code_violations += RXMAC_CD_VIO_CNT_COUNT;
^ permalink raw reply related
* 6lowpan: IPv6 encapsulation question
From: Dmitry Eremin-Solenikov @ 2009-08-08 8:23 UTC (permalink / raw)
To: netdev; +Cc: pekkas
Hi,
We are going to work on the implementation of 6lowpan: the encapsulation
for IPv6 packets for IEEE 802.15.4 networks. RFC 4944 and several
following drafts from 6lowpan WG define several types of IPv6/UDP
headers compression, special fragmentation, ND, etc.
The question is: what is the most clean way to implement 6lowpan and to
hook it into the Linux kernel.
We've identified the following possible ways:
* Implement it just as a special net_device, accepting only IPv6 trafic,
and splitting/merging it on the top of our IEEE 802.15.4 net_devices
* Implement transmission/fragmentation by hooking into neighbouring output
routines
* Implement transmission/fragmentation by hooking into dst->output
routines
* Just hook into IPv6 stack by checking for ARPHRD_IEEE802154 devices
* ???
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [evb] RE: [PATCH][RFC] net/bridge: add basic VEPA support
From: Benny Amorsen @ 2009-08-08 8:50 UTC (permalink / raw)
To: Fischer, Anna
Cc: arnd@arndb.de, Yaron Haviv, bridge@lists.linux-foundation.org,
virtualization@lists.linux-foundation.org, adobriyan@gmail.com,
Paul Congdon (UC Davis), netdev@vger.kernel.org,
evb@yahoogroups.com, davem@davemloft.net
In-Reply-To: <0199E0D51A61344794750DC57738F58E6D6A6CD803__29862.6656564467$1249679159$gmane$org@GVW1118EXC.americas.hpqcorp.net>
"Fischer, Anna" <anna.fischer@hp.com> writes:
> If you do have a SRIOV NIC that supports VEPA, then I would think that
> you do not have QEMU or macvtap in the setup any more though. Simply
> because in that case the VM can directly access the VF on the physical
> device. That would be ideal.
I'm just trying to understand how this all works, so I'm probably asking
a stupid question:
Would a SRIOV NIC with VEPA support show up as multiple devices? I.e.
would I get e.g. eth0-eth7 for a NIC with support for 8 virtual
interfaces? Would they have different MAC addresses?
/Benny
^ permalink raw reply
* RE: [RESEND PATCH] gianfar: keep vlan related state when restart
From: Haruki Dai-R35557 @ 2009-08-08 9:07 UTC (permalink / raw)
To: Yong Zhang, davem, Fleming Andy-AFLEMING; +Cc: netdev, linux-kernel
In-Reply-To: <1249699012-26379-1-git-send-email-yong.zhang0@gmail.com>
> -----Original Message-----
> From: Yong Zhang [mailto:yong.zhang0@gmail.com]
> Sent: Friday, August 07, 2009 9:37 PM
> To: davem@davemloft.net; Fleming Andy-AFLEMING; Haruki Dai-R35557
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: [RESEND PATCH] gianfar: keep vlan related state when restart
>
> If vlan has been enabled. ifdown followed by ifup will lost hardware
> related state.
>
> Also remove duplicated operation in gfar_vlan_rx_register().
>
> Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
Thanks for the catch. This patch looks fine to me.
Acked-by: Dai Haruki <dai.haruki@freescale.com>
> ---
> drivers/net/gianfar.c | 12 ++++++++++--
> 1 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
> index f8ffcbf..e212f2c 100644
> --- a/drivers/net/gianfar.c
> +++ b/drivers/net/gianfar.c
> @@ -936,6 +936,7 @@ int startup_gfar(struct net_device *dev)
> struct gfar __iomem *regs = priv->regs;
> int err = 0;
> u32 rctrl = 0;
> + u32 tctrl = 0;
> u32 attrs = 0;
>
> gfar_write(®s->imask, IMASK_INIT_CLEAR);
> @@ -1111,11 +1112,19 @@ int startup_gfar(struct net_device *dev)
> rctrl |= RCTRL_PADDING(priv->padding);
> }
>
> + /* keep vlan related bits if it's enabled */
> + if (priv->vlgrp) {
> + rctrl |= RCTRL_VLEX | RCTRL_PRSDEP_INIT;
> + tctrl |= TCTRL_VLINS;
> + }
> +
> /* Init rctrl based on our settings */
> gfar_write(&priv->regs->rctrl, rctrl);
>
> if (dev->features & NETIF_F_IP_CSUM)
> - gfar_write(&priv->regs->tctrl, TCTRL_INIT_CSUM);
> + tctrl |= TCTRL_INIT_CSUM;
> +
> + gfar_write(&priv->regs->tctrl, tctrl);
>
> /* Set the extraction length and index */
> attrs = ATTRELI_EL(priv->rx_stash_size) |
> @@ -1450,7 +1459,6 @@ static void gfar_vlan_rx_register(struct
net_device *dev,
>
> /* Enable VLAN tag extraction */
> tempval = gfar_read(&priv->regs->rctrl);
> - tempval |= RCTRL_VLEX;
> tempval |= (RCTRL_VLEX | RCTRL_PRSDEP_INIT);
> gfar_write(&priv->regs->rctrl, tempval);
> } else {
> --
> 1.6.0.4
^ permalink raw reply
* Re: [evb] RE: [PATCH][RFC] net/bridge: add basic VEPA support
From: Arnd Bergmann @ 2009-08-08 9:22 UTC (permalink / raw)
To: Fischer, Anna
Cc: Yaron Haviv, evb@yahoogroups.com, shemminger@linux-foundation.org,
bridge@lists.linux-foundation.org, netdev@vger.kernel.org,
virtualization@lists.linux-foundation.org, davem@davemloft.net,
kaber@trash.net, adobriyan@gmail.com, Paul Congdon (UC Davis)
In-Reply-To: <0199E0D51A61344794750DC57738F58E6D6A6CD803@GVW1118EXC.americas.hpqcorp.net>
On Friday 07 August 2009, Fischer, Anna wrote:
> If you do have a SRIOV NIC that supports VEPA, then I would think
> that you do not have QEMU or macvtap in the setup any more though.
> Simply because in that case the VM can directly access the VF on
> the physical device. That would be ideal.
There may be reasons why even with an SR-IOV adapter you may want
to use the macvtap setup, with some extensions. E.g. guest migration
becomes a lot simpler if you don't have to deal with PCI passthrough
devices. If we manage to add both TX and RX zero-copy (into the
guest) to the macvlan driver, we can treat an SR-IOV adapter like
a VMDq adapter and get the best of both.
> I do think that the macvtap driver is a good addition as a simple
> and fast virtual network I/O interface, in case you do not need
> full bridge functionality. It does seem to assume though that the
> virtualization software uses QEMU/tap interfaces. How would this
> work with a Xen para-virtualized network interface? I guess there
> would need to be yet another driver?
I'm not sure how Xen guest networking works, but if neither the
traditional macvlan driver nor the macvtap driver are able to
connect it to the external NIC, then you can probably add a third
macvlan backend to handle that.
Arnd <><
^ permalink raw reply
* Re: [evb] RE: [PATCH][RFC] net/bridge: add basic VEPA support
From: Arnd Bergmann @ 2009-08-08 9:44 UTC (permalink / raw)
To: Benny Amorsen
Cc: Fischer, Anna, Yaron Haviv, evb, shemminger, davem, netdev,
bridge, adobriyan, Paul Congdon (UC Davis), virtualization
In-Reply-To: <m3d476hepp.fsf@ursa.amorsen.dk>
On Saturday 08 August 2009, Benny Amorsen wrote:
> Would a SRIOV NIC with VEPA support show up as multiple devices? I.e.
> would I get e.g. eth0-eth7 for a NIC with support for 8 virtual
> interfaces? Would they have different MAC addresses?
It could, but the idea of SR-IOV is that it shows up as 8 PCI
devices. One of them is owned by the host and is seen as eth0
there. The other seven PCI devices (virtual functions) are meant
to be assigned to the guest using PCI passthrough and will show
up as the guests eth0, each one with its own MAC address.
An other mode of operation is VMDq, where the host owns all
interfaces and you might see eth0-eth7 there. You can then attach
a qemu process with a raw packet socket or a single macvtap port
for each of those interfaces. This is not yet implemented in Linux,
so how it will be done is still open. It might all be integrated
into macvlan or some new subsystem alternatively.
AFAIK, every SR-IOV adapter can also be operated as a VMDq adapter,
but there are VMDq adapters that do not support SR-IOV.
Arnd <><
^ permalink raw reply
* Re: [PATCH][RFC] net/bridge: add basic VEPA support
From: Arnd Bergmann @ 2009-08-08 9:49 UTC (permalink / raw)
To: Paul Congdon (UC Davis)
Cc: 'Stephen Hemminger', 'Fischer, Anna', bridge,
linux-kernel, netdev, virtualization, evb, davem, kaber,
adobriyan
In-Reply-To: <004e01ca1790$fec36c30$fc4a4490$@edu>
On Friday 07 August 2009, Paul Congdon (UC Davis) wrote:
> As I understand the macvlan code, it currently doesn't allow two VMs on the
> same machine to communicate with one another.
There are patches to do that. I think if we add that, there should be
a way to choose the behavior between either bridging between the
guests or VEPA.
> I could imagine a hairpin mode on the adjacent bridge making this
> possible, but the macvlan code would need to be updated to filter
> reflected frames so a source did not receive his own packet.
Right, I missed this point so far. I'll follow up with a patch
to do that.
> I could imagine this being done as well, but to also
> support selective multicast usage, something similar to the bridge
> forwarding table would be needed. I think putting VEPA into a new driver
> would cause you to implement many things the bridge code already supports.
> Given that we expect the bridge standard to ultimately include VEPA, and the
> new functions are basic forwarding operations, it seems to make most sense
> to keep this consistent with the bridge module.
This is the interesting part of the discussion. The bridge and macvlan
drivers certainly have an overlap in functionality and you can argue
that you only need one. Then again, the bridge code is a little crufty
and we might not want to add much more to it for functionality that can
be implemented in a much simpler way elsewhere. My preferred way would
be to use bridge when you really need 802.1d MAC learning, netfilter-bridge
and STP, while we put the optimizations for stuff like VMDq, zero-copy
and multiqueue guest adapters only into the macvlan code.
Arnd <><
^ permalink raw reply
* Re: Receive side performance issue with multi-10-GigE and NUMA
From: Andrew Gallatin @ 2009-08-08 11:08 UTC (permalink / raw)
To: Bill Fink; +Cc: Brice Goglin, Linux Network Developers, Yinghai Lu
In-Reply-To: <20090807213557.d0faec23.billfink@mindspring.com>
Bill Fink wrote:
> On Fri, 07 Aug 2009, Andrew Gallatin wrote:
>
>> Bill Fink wrote:
>>
>>> All sysfs local_cpus values are the same (00000000,000000ff),
>>> so yes they are also wrong.
>> How were you handling IRQ binding? If local_cpus is wrong,
>> the irqbalance will not be able to make good decisions about
>> where to bind the NICs' IRQs. Did you try manually binding
>> each NICs's interrupt to a separate CPU on the correct node?
>
> Yes, all the NIC IRQs were bound to a CPU on the local NUMA node,
> and the nuttcp application had its CPU affinity set to the same
> CPU with its memory affinity bound to the same local NUMA node.
> And the irqbalance daemon wasn't running.
I must be misunderstanding something. I had thought that
alloc_pages() on NUMA would wind up doing alloc_pages_current(), which
would allocate based on default policy which (if not interleaved)
should allocate from the current NUMA node. And since restocking the
RX ring happens from a the driver's NAPI softirq context, then it
should always be restocking on the same node the memory is destined to
be consumed on.
Do I just not understand how alloc_pages() works on NUMA?
Thanks,
Drew
^ permalink raw reply
* Re: Receive side performance issue with multi-10-GigE and NUMA
From: Neil Horman @ 2009-08-08 11:26 UTC (permalink / raw)
To: Andrew Gallatin
Cc: Bill Fink, Brice Goglin, Linux Network Developers, Yinghai Lu
In-Reply-To: <4A7D5CA4.3030307@myri.com>
On Sat, Aug 08, 2009 at 07:08:20AM -0400, Andrew Gallatin wrote:
> Bill Fink wrote:
> > On Fri, 07 Aug 2009, Andrew Gallatin wrote:
> >
> >> Bill Fink wrote:
> >>
> >>> All sysfs local_cpus values are the same (00000000,000000ff),
> >>> so yes they are also wrong.
> >> How were you handling IRQ binding? If local_cpus is wrong,
> >> the irqbalance will not be able to make good decisions about
> >> where to bind the NICs' IRQs. Did you try manually binding
> >> each NICs's interrupt to a separate CPU on the correct node?
> >
> > Yes, all the NIC IRQs were bound to a CPU on the local NUMA node,
> > and the nuttcp application had its CPU affinity set to the same
> > CPU with its memory affinity bound to the same local NUMA node.
> > And the irqbalance daemon wasn't running.
>
> I must be misunderstanding something. I had thought that
> alloc_pages() on NUMA would wind up doing alloc_pages_current(), which
> would allocate based on default policy which (if not interleaved)
> should allocate from the current NUMA node. And since restocking the
> RX ring happens from a the driver's NAPI softirq context, then it
> should always be restocking on the same node the memory is destined to
> be consumed on.
>
> Do I just not understand how alloc_pages() works on NUMA?
>
Thats how alloc_works, but most drivers use netdev_alloc_skb to refill their rx
ring in their napi context. netdev_alloc_skb specifically allocates an skb from
memory in the node that the actually NIC is local to (rather than the cpu that
the interrupt is running on). That cuts out cross numa node chatter when the
device is dma-ing a frame from the hardware to the allocated skb. The offshoot
of that however (especially in 10G cards with lots of rx queues whos interrupts
are spread out through the system) is that the irq affinity for a given irq has
an increased risk of not being on the same node as the skb memory. The ftrace
module I referenced earlier will help illustrate this, as well as cases where
its causing applications to run on processors that create lots of cross-node
chatter.
Neil
> Thanks,
>
> Drew
>
> --
> 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: some bug in iproute2
From: jamal @ 2009-08-08 12:49 UTC (permalink / raw)
To: Sergey Popov; +Cc: Patrick McHardy, Stephen Hemminger, netdev
In-Reply-To: <1249734651.7101.38.camel@dogo.mojatatu.com>
Sergey,
Hope you dont mind if i cc some other people so we can
resolve this issue because i am looking at the git logs
and seeing tons of activities which affect what you are
trying to do.
On Sat, 2009-08-08 at 08:30 -0400, jamal wrote:
> Hi,
>
> On Fri, 2009-08-07 at 20:27 +0300, Sergey Popov wrote:
> > В Fri, 07 Aug 2009 10:28:05 -0400
> > Now i want to police the incoming traffic and drop all the packets
> > that exceeds the 1000 kbit and redirect packets
> > to ifb, but next line doesn't work as expected [by me]:
> >
>
> What distro are you running?
>
> > # tc f add dev $INETIF parent ffff: proto ip prio 1 u32 match u32 0 0 \
> > action police rate 1000kbit burst 1k drop \
> > action mirred egress redirect dev $IFBIF
> >
> > Illegal "action"
> > bad action parsing
> > parse_action: bad value (12:police)!
> > Illegal "action"
>
> Let me try simple version:
> dogo:~# tc q add dev lo ingress
> dogo:~# tc f add dev lo parent ffff: proto ip prio 1 u32 match u32 0 0
> action police rate 1000kbit burst 1k drop action mirred egress redirect
> dev eth0
> Illegal "action"
> bad action parsing
> parse_action: bad value (12:police)!
> Illegal "action"
> dogo:~#
>
> Ok, this seems to be a bug with policer...
> Although your syntax above seems wrong since you didnt specify flowid.
> You must specify flowid always or strange things will happen even if
> the syntax is accepted.
Your usage and syntax is perfectly legal and very useful.
The problem is in the old policer syntax the keyword "action" had some
speacial meaning. Some old scripts used it and broke if new syntax was
used.
The only clean option i see forward (which doesnt break any legacy stuff
or play acrobatics) is to introduce new action "npolice". Patrick,
thoughts?
cheers,
jamal
^ permalink raw reply
* Re: [PATCH] korina: Read buffer overflow
From: roel kluin @ 2009-08-08 13:14 UTC (permalink / raw)
To: Roel Kluin, netdev, Andrew Morton, David S. Miller, florian
In-Reply-To: <20090808004815.F10394CEAA@orbit.nwl.cc>
>> If the loop breaks with an i of 0, then we read lp->rd_ring[-1].
>> @@ -771,7 +771,7 @@ static void korina_alloc_ring(struct net_device *dev)
>> for (i = 0; i < KORINA_NUM_RDS; i++) {
>> skb = dev_alloc_skb(KORINA_RBSIZE + 2);
>> if (!skb)
>> - break;
>> + goto err_free;
>
> This implies that all KORINA_NUM_TDS receive descriptors need to be
> available for the driver to work.
> Also I guess there should be some error handling, as the driver probably
> wont be useful without a single receive descriptor. What do you think
> about the following:
A few comments:
> | diff --git a/drivers/net/korina.c b/drivers/net/korina.c
> | index a2701f5..d1c5276 100644
> | --- a/drivers/net/korina.c
> | +++ b/drivers/net/korina.c
> | @@ -772,6 +772,13 @@ static void korina_alloc_ring(struct net_device *dev)
> | lp->rd_ring[i].ca = CPHYSADDR(skb->data);
> | lp->rd_ring[i].link = CPHYSADDR(&lp->rd_ring[i+1]);
> | }
> | + if (!i) {
> | + printk(KERN_ERR DRV_NAME "%s: could not allocate a single"
> | + " receive descriptor\n", dev->name)
printk(KERN_ERR "%s: could not allocate a single receive
descriptor\n",
dev->name);
Don't interrupt strings, they are an exception in the 80 character
limit (since they are
easier to grep that way). Also I think the DRV_NAME should be omitted, Doesn't
`DRV_NAME "%s:...\n", dev->name)' result in "korinakorina:..."? It
occurs at more
places in the file. Also a semicolon was missing.
> | + return 1;
I think -ENOMEM is more appropriate.
> | + }
> | + printk(KERN_DEBUG DRV_NAME "%s: allocated %d receive descriptors\n",
> | + dev->name, i);
same `DRV_NAME "%s:...\n", dev->name)' issue.
> | @@ -824,7 +833,8 @@ static int korina_init(struct net_device *dev)
> | writel(ETH_INT_FC_EN, &lp->eth_regs->ethintfc);
> |
> | /* Allocate rings */
> | - korina_alloc_ring(dev);
> | + if (korina_alloc_ring(dev))
> | + return 1;
-ENOMEM
> |
> | writel(0, &lp->rx_dma_regs->dmas);
> | /* Start Rx DMA */
>
> Greetings, Phil
Otherwise looks fine to me.
Thanks,
Roel
^ permalink raw reply
* Re: [PATCH] drivers/net/wireless/ath/ath5k: Change constant name
From: Bob Copeland @ 2009-08-08 16:20 UTC (permalink / raw)
To: Julia Lawall
Cc: kernel-janitors-u79uwXL29TY76Z2rM5mHXA, Jiri Slaby,
netdev-u79uwXL29TY76Z2rM5mHXA, ath5k-devel-xDcbHBWguxEUs3QNXV6qNA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linville-2XuSBdqkA4R54TAoqtyWWQ
In-Reply-To: <Pine.LNX.4.64.0908081521210.27715-QfmoRoYWmW9knbxzx/v8hQ@public.gmane.org>
On Sat, Aug 08, 2009 at 03:22:26PM +0200, Julia Lawall wrote:
> From: Julia Lawall <julia-dAYI7NvHqcQ@public.gmane.org>
>
> Elsewhere, the tqi_type field is compared to constants having a name
> beginning with AR5K_TX_QUEUE, rather than AR5K_TX_QUEUE_ID. I have thus
> converted AR5K_TX_QUEUE_ID_CAB to AR5K_TX_QUEUE_CAB. This does, however,
> change the value, so perhaps something else was wanted.
>
> Signed-off-by: Julia Lawall <julia-dAYI7NvHqcQ@public.gmane.org>
Nice catch, thanks. Shouldn't cause any problem with 2.6.31 since we
don't use the CAB queue yet, but it was definitely wrong.
Acked-by: Bob Copeland <me-aXfl/3sk2vNUbtYUoyoikg@public.gmane.org>
> diff --git a/drivers/net/wireless/ath/ath5k/qcu.c b/drivers/net/wireless/ath/ath5k/qcu.c
> index 6d5aaf0..eeebb9a 100644
> --- a/drivers/net/wireless/ath/ath5k/qcu.c
> +++ b/drivers/net/wireless/ath/ath5k/qcu.c
> @@ -362,7 +362,7 @@ int ath5k_hw_reset_tx_queue(struct ath5k_hw *ah, unsigned int queue)
> }
>
> if (tq->tqi_ready_time &&
> - (tq->tqi_type != AR5K_TX_QUEUE_ID_CAB))
> + (tq->tqi_type != AR5K_TX_QUEUE_CAB))
> ath5k_hw_reg_write(ah, AR5K_REG_SM(tq->tqi_ready_time,
> AR5K_QCU_RDYTIMECFG_INTVAL) |
> AR5K_QCU_RDYTIMECFG_ENABLE,
>
--
Bob Copeland %% www.bobcopeland.com
^ permalink raw reply
* Re: [PATCH] korina: Read buffer overflow
From: Phil Sutter @ 2009-08-08 16:45 UTC (permalink / raw)
To: roel kluin; +Cc: netdev, Andrew Morton, David S. Miller, florian
In-Reply-To: <25e057c00908080614q6d7efa3x46789e51b0b544b3@mail.gmail.com>
Hi,
On Sat, Aug 08, 2009 at 03:14:57PM +0200, roel kluin wrote:
> A few comments:
>
> > | diff --git a/drivers/net/korina.c b/drivers/net/korina.c
> > | index a2701f5..d1c5276 100644
> > | --- a/drivers/net/korina.c
> > | +++ b/drivers/net/korina.c
>
> > | @@ -772,6 +772,13 @@ static void korina_alloc_ring(struct net_device *dev)
> > | lp->rd_ring[i].ca = CPHYSADDR(skb->data);
> > | lp->rd_ring[i].link = CPHYSADDR(&lp->rd_ring[i+1]);
> > | }
> > | + if (!i) {
> > | + printk(KERN_ERR DRV_NAME "%s: could not allocate a single"
> > | + " receive descriptor\n", dev->name)
>
> printk(KERN_ERR "%s: could not allocate a single receive
> descriptor\n",
> dev->name);
>
> Don't interrupt strings, they are an exception in the 80 character
> limit (since they are easier to grep that way).
Ah, good to know.
> Also I think the DRV_NAME should be omitted, Doesn't
> `DRV_NAME "%s:...\n", dev->name)' result in "korinakorina:..."? It
> occurs at more places in the file. Also a semicolon was missing.
Sounds like a valid point, I will check this. This patch was just a
quick hack to illustrate what I had in mind, without even
compile-testing it.
> Otherwise looks fine to me.
Great you like it, and many thanks for reviewing it. I'm still not
completely sure the NIC will still work with less than 64 receive
descriptors, but since the documentation wasn't really enlightening I'll
add some run-time test to my TODO. Maybe one of the other readers has
some advice here, I'll prepare a complete patch in the meantime.
Greetings, Phil
^ permalink raw reply
* Re: Receive side performance issue with multi-10-GigE and NUMA
From: Andrew Gallatin @ 2009-08-08 18:21 UTC (permalink / raw)
To: Neil Horman; +Cc: Bill Fink, Brice Goglin, Linux Network Developers, Yinghai Lu
In-Reply-To: <20090808112636.GB18518@localhost.localdomain>
Neil Horman wrote:
> On Sat, Aug 08, 2009 at 07:08:20AM -0400, Andrew Gallatin wrote:
>> Bill Fink wrote:
>>> On Fri, 07 Aug 2009, Andrew Gallatin wrote:
>>>
>>>> Bill Fink wrote:
>>>>
>>>>> All sysfs local_cpus values are the same (00000000,000000ff),
>>>>> so yes they are also wrong.
>>>> How were you handling IRQ binding? If local_cpus is wrong,
>>>> the irqbalance will not be able to make good decisions about
>>>> where to bind the NICs' IRQs. Did you try manually binding
>>>> each NICs's interrupt to a separate CPU on the correct node?
>>> Yes, all the NIC IRQs were bound to a CPU on the local NUMA node,
>>> and the nuttcp application had its CPU affinity set to the same
>>> CPU with its memory affinity bound to the same local NUMA node.
>>> And the irqbalance daemon wasn't running.
>> I must be misunderstanding something. I had thought that
>> alloc_pages() on NUMA would wind up doing alloc_pages_current(), which
>> would allocate based on default policy which (if not interleaved)
>> should allocate from the current NUMA node. And since restocking the
>> RX ring happens from a the driver's NAPI softirq context, then it
>> should always be restocking on the same node the memory is destined to
>> be consumed on.
>>
>> Do I just not understand how alloc_pages() works on NUMA?
>>
>
> Thats how alloc_works, but most drivers use netdev_alloc_skb to refill their rx
> ring in their napi context. netdev_alloc_skb specifically allocates an skb from
> memory in the node that the actually NIC is local to (rather than the cpu that
> the interrupt is running on). That cuts out cross numa node chatter when the
> device is dma-ing a frame from the hardware to the allocated skb. The offshoot
> of that however (especially in 10G cards with lots of rx queues whos interrupts
> are spread out through the system) is that the irq affinity for a given irq has
> an increased risk of not being on the same node as the skb memory. The ftrace
> module I referenced earlier will help illustrate this, as well as cases where
> its causing applications to run on processors that create lots of cross-node
> chatter.
One thing worth noting is that myri10ge is rather unusual in that
it fills its RX rings with pages, then attaches them to skbs after
the receive is done. Given how (I think) alloc_page() works, I
don't understand why correct CPU binding does not have the same
benefit as Bill's patch to assign the NUMA node manually.
I'm certainly willing to change to myri10ge to use alloc_pages_node()
based on NIC locality, if that provides a benefit, but I'd really
like to understand why CPU binding is not helping.
Drew
^ permalink raw reply
* Re: Receive side performance issue with multi-10-GigE and NUMA
From: Neil Horman @ 2009-08-08 18:32 UTC (permalink / raw)
To: Andrew Gallatin
Cc: Bill Fink, Brice Goglin, Linux Network Developers, Yinghai Lu
In-Reply-To: <4A7DC230.6060206@myri.com>
On Sat, Aug 08, 2009 at 02:21:36PM -0400, Andrew Gallatin wrote:
> Neil Horman wrote:
>> On Sat, Aug 08, 2009 at 07:08:20AM -0400, Andrew Gallatin wrote:
>>> Bill Fink wrote:
>>>> On Fri, 07 Aug 2009, Andrew Gallatin wrote:
>>>>
>>>>> Bill Fink wrote:
>>>>>
>>>>>> All sysfs local_cpus values are the same (00000000,000000ff),
>>>>>> so yes they are also wrong.
>>>>> How were you handling IRQ binding? If local_cpus is wrong,
>>>>> the irqbalance will not be able to make good decisions about
>>>>> where to bind the NICs' IRQs. Did you try manually binding
>>>>> each NICs's interrupt to a separate CPU on the correct node?
>>>> Yes, all the NIC IRQs were bound to a CPU on the local NUMA node,
>>>> and the nuttcp application had its CPU affinity set to the same
>>>> CPU with its memory affinity bound to the same local NUMA node.
>>>> And the irqbalance daemon wasn't running.
>>> I must be misunderstanding something. I had thought that
>>> alloc_pages() on NUMA would wind up doing alloc_pages_current(), which
>>> would allocate based on default policy which (if not interleaved)
>>> should allocate from the current NUMA node. And since restocking the
>>> RX ring happens from a the driver's NAPI softirq context, then it
>>> should always be restocking on the same node the memory is destined to
>>> be consumed on.
>>>
>>> Do I just not understand how alloc_pages() works on NUMA?
>>>
>>
>> Thats how alloc_works, but most drivers use netdev_alloc_skb to refill their rx
>> ring in their napi context. netdev_alloc_skb specifically allocates an skb from
>> memory in the node that the actually NIC is local to (rather than the cpu that
>> the interrupt is running on). That cuts out cross numa node chatter when the
>> device is dma-ing a frame from the hardware to the allocated skb. The offshoot
>> of that however (especially in 10G cards with lots of rx queues whos interrupts
>> are spread out through the system) is that the irq affinity for a given irq has
>> an increased risk of not being on the same node as the skb memory. The ftrace
>> module I referenced earlier will help illustrate this, as well as cases where
>> its causing applications to run on processors that create lots of cross-node
>> chatter.
>
> One thing worth noting is that myri10ge is rather unusual in that
> it fills its RX rings with pages, then attaches them to skbs after
> the receive is done. Given how (I think) alloc_page() works, I
> don't understand why correct CPU binding does not have the same
> benefit as Bill's patch to assign the NUMA node manually.
>
> I'm certainly willing to change to myri10ge to use alloc_pages_node()
> based on NIC locality, if that provides a benefit, but I'd really
> like to understand why CPU binding is not helping.
>
Thats hard to say. If binding the app to a cpu on the same node doesn't help,
that would suggest to me:
1) That the process binding isn't being honored
2) The cpu you're binding to isn't actually on the same node
3) The node which the skb's are allocated on is not the one you think it is
4) The cross numa chatter is improved, but another problem has taken its place
(like cpu contention between the process and the interrupt handler on the samme
cpu)
5) The problem is something else entirely.
Either way, I'd suggest applying and running the patch set that I referenced
previously. It will give you a good table representation of how skbs for this
process are being allocated and consumed, and let you confirm or eliminate items
1-4 above.
Neil
> Drew
>
^ permalink raw reply
* [PATCH 0/3] net: Add ftracer to help optimize process scheduling based on incomming frame allocations (v2)
From: Neil Horman @ 2009-08-08 23:13 UTC (permalink / raw)
To: netdev; +Cc: davem, rostedt
In-Reply-To: <20090807202130.GA26677@hmsreliant.think-freely.org>
n Fri, Aug 07, 2009 at 04:21:30PM -0400, Neil Horman wrote:
Hey all-
I put out an RFC about this awhile ago and didn't get any loud screams,
so I've gone ahead and implemented it
Currently, our network infrastructure allows net device drivers to
allocate skbs based on the the numa node the device itself is local to. This of
course cuts down on cross numa chatter when the device is DMA-ing network
traffic to the driver. Unfortuantely no such corresponding infrastrucuture
exists at the process level. The scheduler has no insight into the numa
locality of incomming data packets for a given process (and arguably it
shouldn't), and so there is every chance that a process will run on a different
numa node than the packets that its receiving lives on, creating cross numa node
traffic.
This patch aims to provide userspace with the opportunity to optimize
that scheduling. It consists of a tracepoint and an ftrace module which exports
a history of the packets each process receives, along with the numa node each
packet was received on, as well as the numa node the process was running on when
it copied the buffer to user space. With this information, exported via the
ftrace infrastructure to user space, a sysadim can identify high prirority
processes, and optimize their scheduling so that they are more likely to run on
the same node that they are primarily receiving data on, thereby cutting down
cross numa node traffic.
Tested by me, working well, applies against the head of the net-next tree
Version 2 change notes:
I noticed that I did something stupid in patch 3, and it added a duplicated
chunk which didn't apply, this new series simply removes that, everything else
is the same
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
--
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 1/3] net: Add ftracer to help optimize process scheduling based on incomming frame allocations (v2)
From: Neil Horman @ 2009-08-08 23:14 UTC (permalink / raw)
To: netdev; +Cc: davem, rostedt
In-Reply-To: <20090808231322.GA24414@localhost.localdomain>
skb allocation / cosumption tracer - Add consumption tracepoint
This patch adds a tracepoint to skb_copy_datagram_iovec, which is called each
time a userspace process copies a frame from a socket receive queue to a user
space buffer. It allows us to hook in and examine each sk_buff that the system
receives on a per-socket bases, and can be use to compile a list of which skb's
were received by which processes.
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
include/trace/events/skb.h | 20 ++++++++++++++++++++
net/core/datagram.c | 3 +++
2 files changed, 23 insertions(+)
diff --git a/include/trace/events/skb.h b/include/trace/events/skb.h
index 1e8fabb..bdc5c1c 100644
--- a/include/trace/events/skb.h
+++ b/include/trace/events/skb.h
@@ -2,6 +2,7 @@
#define _TRACE_SKB_H
#include <linux/skbuff.h>
+#include <linux/netdevice.h>
#include <linux/tracepoint.h>
#undef TRACE_SYSTEM
@@ -34,6 +35,25 @@ TRACE_EVENT(kfree_skb,
__entry->skbaddr, __entry->protocol, __entry->location)
);
+TRACE_EVENT(skb_copy_datagram_iovec,
+
+ TP_PROTO(const struct sk_buff *skb, int len),
+
+ TP_ARGS(skb, len),
+
+ TP_STRUCT__entry(
+ __field( const void *, skbaddr )
+ __field( int, len )
+ ),
+
+ TP_fast_assign(
+ __entry->skbaddr = skb;
+ __entry->len = len;
+ ),
+
+ TP_printk("skbaddr=%p len=%d", __entry->skbaddr, __entry->len)
+);
+
#endif /* _TRACE_SKB_H */
/* This part must be outside protection */
diff --git a/net/core/datagram.c b/net/core/datagram.c
index b0fe692..1c6cf3a 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -55,6 +55,7 @@
#include <net/checksum.h>
#include <net/sock.h>
#include <net/tcp_states.h>
+#include <trace/events/skb.h>
/*
* Is a socket 'connection oriented' ?
@@ -284,6 +285,8 @@ int skb_copy_datagram_iovec(const struct sk_buff *skb, int offset,
int i, copy = start - offset;
struct sk_buff *frag_iter;
+ trace_skb_copy_datagram_iovec(skb, len);
+
/* Copy header. */
if (copy > 0) {
if (copy > len)
--
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 related
* Re: [PATCH 2/3] net: Add ftracer to help optimize process scheduling based on incomming frame allocations (v2)
From: Neil Horman @ 2009-08-08 23:15 UTC (permalink / raw)
To: netdev; +Cc: davem, rostedt
In-Reply-To: <20090808231322.GA24414@localhost.localdomain>
skb allocation / consumption corelator - Add config option
This patch adds a Kconfig option to enable the addtition of the skb source
tracer.
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
Kconfig | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index 1551f47..1aeec05 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -234,6 +234,16 @@ config BOOT_TRACER
You must pass in ftrace=initcall to the kernel command line
to enable this on bootup.
+config SKB_SOURCES_TRACER
+ bool "Trace skb source information
+ select GENERIC_TRACER
+ help
+ This tracer helps developers/sysadmins correlate skb allocation and
+ consumption. The idea being that some processes will primarily consume data
+ that was allocated on certain numa nodes. By being able to visualize which
+ nodes the data was allocated on, a sysadmin or developer can optimize the
+ scheduling of those processes to cut back on cross node chatter.
+
config TRACE_BRANCH_PROFILING
bool
select GENERIC_TRACER
--
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 related
* Re: [PATCH 3/3] net: Add ftracer to help optimize process scheduling based on incomming frame allocations (v2)
From: Neil Horman @ 2009-08-08 23:15 UTC (permalink / raw)
To: netdev; +Cc: davem, rostedt
In-Reply-To: <20090808231322.GA24414@localhost.localdomain>
skb allocation / consumption correlator
Add ftracer module to kernel to print out a list that correlates a process id,
an skb it read, and the numa nodes on wich the process was running when it was
read along with the numa node the skbuff was allocated on.
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
index 844164d..ee5e5b1 100644
--- a/kernel/trace/Makefile
+++ b/kernel/trace/Makefile
@@ -49,6 +49,7 @@ obj-$(CONFIG_BLK_DEV_IO_TRACE) += blktrace.o
ifeq ($(CONFIG_BLOCK),y)
obj-$(CONFIG_EVENT_TRACING) += blktrace.o
endif
+obj-$(CONFIG_SKB_SOURCES_TRACER) += trace_skb_sources.o
obj-$(CONFIG_EVENT_TRACING) += trace_events.o
obj-$(CONFIG_EVENT_TRACING) += trace_export.o
obj-$(CONFIG_FTRACE_SYSCALLS) += trace_syscalls.o
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 3548ae5..8c1d458 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -11,6 +11,7 @@
#include <trace/boot.h>
#include <linux/kmemtrace.h>
#include <trace/power.h>
+#include <trace/events/skb.h>
#include <linux/trace_seq.h>
#include <linux/ftrace_event.h>
@@ -40,6 +41,7 @@ enum trace_type {
TRACE_KMEM_FREE,
TRACE_POWER,
TRACE_BLK,
+ TRACE_SKB_SOURCE,
__TRACE_LAST_TYPE,
};
@@ -171,6 +173,21 @@ struct trace_power {
struct power_trace state_data;
};
+struct skb_record {
+ pid_t pid; /* pid of the copying process */
+ int anid; /* node where skb was allocated */
+ int cnid; /* node to which skb was copied in userspace */
+ char ifname[IFNAMSIZ]; /* Name of the receiving interface */
+ int rx_queue; /* The rx queue the skb was received on */
+ int ccpu; /* Cpu the application got this frame from */
+ int len; /* length of the data copied */
+};
+
+struct trace_skb_event {
+ struct trace_entry ent;
+ struct skb_record event_data;
+};
+
enum kmemtrace_type_id {
KMEMTRACE_TYPE_KMALLOC = 0, /* kmalloc() or kfree(). */
KMEMTRACE_TYPE_CACHE, /* kmem_cache_*(). */
@@ -323,6 +340,8 @@ extern void __ftrace_bad_type(void);
TRACE_SYSCALL_ENTER); \
IF_ASSIGN(var, ent, struct syscall_trace_exit, \
TRACE_SYSCALL_EXIT); \
+ IF_ASSIGN(var, ent, struct trace_skb_event, \
+ TRACE_SKB_SOURCE); \
__ftrace_bad_type(); \
} while (0)
diff --git a/kernel/trace/trace_skb_sources.c b/kernel/trace/trace_skb_sources.c
new file mode 100644
index 0000000..4ba3671
--- /dev/null
+++ b/kernel/trace/trace_skb_sources.c
@@ -0,0 +1,154 @@
+/*
+ * ring buffer based tracer for analyzing per-socket skb sources
+ *
+ * Neil Horman <nhorman@tuxdriver.com>
+ * Copyright (C) 2009
+ *
+ *
+ */
+
+#include <linux/init.h>
+#include <linux/debugfs.h>
+#include <trace/events/skb.h>
+#include <linux/kallsyms.h>
+#include <linux/module.h>
+#include <linux/hardirq.h>
+#include <linux/netdevice.h>
+#include <net/sock.h>
+
+#include "trace.h"
+#include "trace_output.h"
+
+EXPORT_TRACEPOINT_SYMBOL_GPL(skb_copy_datagram_iovec);
+
+static struct trace_array *skb_trace;
+static int __read_mostly trace_skb_source_enabled;
+
+static void probe_skb_dequeue(const struct sk_buff *skb, int len)
+{
+ struct ring_buffer_event *event;
+ struct trace_skb_event *entry;
+ struct trace_array *tr = skb_trace;
+ struct net_device *dev;
+
+ if (!trace_skb_source_enabled)
+ return;
+
+ if (in_interrupt())
+ return;
+
+ event = trace_buffer_lock_reserve(tr, TRACE_SKB_SOURCE,
+ sizeof(*entry), 0, 0);
+ if (!event)
+ return;
+ entry = ring_buffer_event_data(event);
+
+ entry->event_data.pid = current->pid;
+ entry->event_data.anid = page_to_nid(virt_to_page(skb->data));
+ entry->event_data.cnid = cpu_to_node(smp_processor_id());
+ entry->event_data.len = len;
+ entry->event_data.rx_queue = skb->queue_mapping;
+ entry->event_data.ccpu = smp_processor_id();
+
+ dev = dev_get_by_index(sock_net(skb->sk), skb->iif);
+ if (dev) {
+ memcpy(entry->event_data.ifname, dev->name, IFNAMSIZ);
+ dev_put(dev);
+ } else {
+ strcpy(entry->event_data.ifname, "Unknown");
+ }
+
+ trace_buffer_unlock_commit(tr, event, 0, 0);
+}
+
+static int tracing_skb_source_register(void)
+{
+ int ret;
+
+ ret = register_trace_skb_copy_datagram_iovec(probe_skb_dequeue);
+ if (ret)
+ pr_info("skb source trace: Couldn't activate dequeue tracepoint");
+
+ return ret;
+}
+
+static void start_skb_source_trace(struct trace_array *tr)
+{
+ trace_skb_source_enabled = 1;
+}
+
+static void stop_skb_source_trace(struct trace_array *tr)
+{
+ trace_skb_source_enabled = 0;
+}
+
+static void skb_source_trace_reset(struct trace_array *tr)
+{
+ trace_skb_source_enabled = 0;
+ unregister_trace_skb_copy_datagram_iovec(probe_skb_dequeue);
+}
+
+
+static int skb_source_trace_init(struct trace_array *tr)
+{
+ int cpu;
+ skb_trace = tr;
+
+ trace_skb_source_enabled = 1;
+ tracing_skb_source_register();
+
+ for_each_cpu(cpu, cpu_possible_mask)
+ tracing_reset(tr, cpu);
+ return 0;
+}
+
+static enum print_line_t skb_source_print_line(struct trace_iterator *iter)
+{
+ int ret = 0;
+ struct trace_entry *entry = iter->ent;
+ struct trace_skb_event *event;
+ struct skb_record *record;
+ struct trace_seq *s = &iter->seq;
+
+ trace_assign_type(event, entry);
+ record = &event->event_data;
+ if (entry->type != TRACE_SKB_SOURCE)
+ return TRACE_TYPE_UNHANDLED;
+
+ ret = trace_seq_printf(s, " %d %d %d %s %d %d %d\n",
+ record->pid,
+ record->anid,
+ record->cnid,
+ record->ifname,
+ record->rx_queue,
+ record->ccpu,
+ record->len);
+
+ if (!ret)
+ return TRACE_TYPE_PARTIAL_LINE;
+
+ return TRACE_TYPE_HANDLED;
+}
+
+static void skb_source_print_header(struct seq_file *s)
+{
+ seq_puts(s, "# PID ANID CNID IFC RXQ CCPU LEN\n");
+ seq_puts(s, "# | | | | | | |\n");
+}
+
+static struct tracer skb_source_tracer __read_mostly =
+{
+ .name = "skb_sources",
+ .init = skb_source_trace_init,
+ .start = start_skb_source_trace,
+ .stop = stop_skb_source_trace,
+ .reset = skb_source_trace_reset,
+ .print_line = skb_source_print_line,
+ .print_header = skb_source_print_header,
+};
+
+static int init_skb_source_trace(void)
+{
+ return register_tracer(&skb_source_tracer);
+}
+device_initcall(init_skb_source_trace);
^ permalink raw reply related
* [PATCH 1/3] korina: fix printk formatting, add final info line
From: Phil Sutter @ 2009-08-09 0:06 UTC (permalink / raw)
To: netdev; +Cc: Andrew Morton, David S. Miller, florian, Roel Kluin
In-Reply-To: <1249776388-4626-1-git-send-email-n0-1@freewrt.org>
The macro DRV_NAME contains "korina", the field dev->name points to the
actual interface name. So messages were formerly prefixed with
'korinaeth2:' (on my system).
Signed-off-by: Phil Sutter <n0-1@freewrt.org>
---
drivers/net/korina.c | 31 ++++++++++++++++---------------
1 files changed, 16 insertions(+), 15 deletions(-)
diff --git a/drivers/net/korina.c b/drivers/net/korina.c
index a2701f5..c09e2ce 100644
--- a/drivers/net/korina.c
+++ b/drivers/net/korina.c
@@ -337,7 +337,7 @@ static irqreturn_t korina_rx_dma_interrupt(int irq, void *dev_id)
napi_schedule(&lp->napi);
if (dmas & DMA_STAT_ERR)
- printk(KERN_ERR DRV_NAME "%s: DMA error\n", dev->name);
+ printk(KERN_ERR "%s: DMA error\n", dev->name);
retval = IRQ_HANDLED;
} else
@@ -555,7 +555,7 @@ static void korina_tx(struct net_device *dev)
dev->stats.tx_dropped++;
/* Should never happen */
- printk(KERN_ERR DRV_NAME "%s: split tx ignored\n",
+ printk(KERN_ERR "%s: split tx ignored\n",
dev->name);
} else if (devcs & ETH_TX_TOK) {
dev->stats.tx_packets++;
@@ -641,7 +641,7 @@ korina_tx_dma_interrupt(int irq, void *dev_id)
dev->trans_start = jiffies;
}
if (dmas & DMA_STAT_ERR)
- printk(KERN_ERR DRV_NAME "%s: DMA error\n", dev->name);
+ printk(KERN_ERR "%s: DMA error\n", dev->name);
retval = IRQ_HANDLED;
} else
@@ -909,8 +909,7 @@ static int korina_restart(struct net_device *dev)
ret = korina_init(dev);
if (ret < 0) {
- printk(KERN_ERR DRV_NAME "%s: cannot restart device\n",
- dev->name);
+ printk(KERN_ERR "%s: cannot restart device\n", dev->name);
return ret;
}
korina_multicast_list(dev);
@@ -997,7 +996,7 @@ static int korina_open(struct net_device *dev)
/* Initialize */
ret = korina_init(dev);
if (ret < 0) {
- printk(KERN_ERR DRV_NAME "%s: cannot open device\n", dev->name);
+ printk(KERN_ERR "%s: cannot open device\n", dev->name);
goto out;
}
@@ -1007,14 +1006,14 @@ static int korina_open(struct net_device *dev)
ret = request_irq(lp->rx_irq, &korina_rx_dma_interrupt,
IRQF_DISABLED, "Korina ethernet Rx", dev);
if (ret < 0) {
- printk(KERN_ERR DRV_NAME "%s: unable to get Rx DMA IRQ %d\n",
+ printk(KERN_ERR "%s: unable to get Rx DMA IRQ %d\n",
dev->name, lp->rx_irq);
goto err_release;
}
ret = request_irq(lp->tx_irq, &korina_tx_dma_interrupt,
IRQF_DISABLED, "Korina ethernet Tx", dev);
if (ret < 0) {
- printk(KERN_ERR DRV_NAME "%s: unable to get Tx DMA IRQ %d\n",
+ printk(KERN_ERR "%s: unable to get Tx DMA IRQ %d\n",
dev->name, lp->tx_irq);
goto err_free_rx_irq;
}
@@ -1023,7 +1022,7 @@ static int korina_open(struct net_device *dev)
ret = request_irq(lp->ovr_irq, &korina_ovr_interrupt,
IRQF_DISABLED, "Ethernet Overflow", dev);
if (ret < 0) {
- printk(KERN_ERR DRV_NAME"%s: unable to get OVR IRQ %d\n",
+ printk(KERN_ERR "%s: unable to get OVR IRQ %d\n",
dev->name, lp->ovr_irq);
goto err_free_tx_irq;
}
@@ -1032,7 +1031,7 @@ static int korina_open(struct net_device *dev)
ret = request_irq(lp->und_irq, &korina_und_interrupt,
IRQF_DISABLED, "Ethernet Underflow", dev);
if (ret < 0) {
- printk(KERN_ERR DRV_NAME "%s: unable to get UND IRQ %d\n",
+ printk(KERN_ERR "%s: unable to get UND IRQ %d\n",
dev->name, lp->und_irq);
goto err_free_ovr_irq;
}
@@ -1111,7 +1110,7 @@ static int korina_probe(struct platform_device *pdev)
dev->base_addr = r->start;
lp->eth_regs = ioremap_nocache(r->start, r->end - r->start);
if (!lp->eth_regs) {
- printk(KERN_ERR DRV_NAME "cannot remap registers\n");
+ printk(KERN_ERR DRV_NAME ": cannot remap registers\n");
rc = -ENXIO;
goto probe_err_out;
}
@@ -1119,7 +1118,7 @@ static int korina_probe(struct platform_device *pdev)
r = platform_get_resource_byname(pdev, IORESOURCE_MEM, "korina_dma_rx");
lp->rx_dma_regs = ioremap_nocache(r->start, r->end - r->start);
if (!lp->rx_dma_regs) {
- printk(KERN_ERR DRV_NAME "cannot remap Rx DMA registers\n");
+ printk(KERN_ERR DRV_NAME ": cannot remap Rx DMA registers\n");
rc = -ENXIO;
goto probe_err_dma_rx;
}
@@ -1127,14 +1126,14 @@ static int korina_probe(struct platform_device *pdev)
r = platform_get_resource_byname(pdev, IORESOURCE_MEM, "korina_dma_tx");
lp->tx_dma_regs = ioremap_nocache(r->start, r->end - r->start);
if (!lp->tx_dma_regs) {
- printk(KERN_ERR DRV_NAME "cannot remap Tx DMA registers\n");
+ printk(KERN_ERR DRV_NAME ": cannot remap Tx DMA registers\n");
rc = -ENXIO;
goto probe_err_dma_tx;
}
lp->td_ring = kmalloc(TD_RING_SIZE + RD_RING_SIZE, GFP_KERNEL);
if (!lp->td_ring) {
- printk(KERN_ERR DRV_NAME "cannot allocate descriptors\n");
+ printk(KERN_ERR DRV_NAME ": cannot allocate descriptors\n");
rc = -ENXIO;
goto probe_err_td_ring;
}
@@ -1175,9 +1174,11 @@ static int korina_probe(struct platform_device *pdev)
rc = register_netdev(dev);
if (rc < 0) {
printk(KERN_ERR DRV_NAME
- ": cannot register net device %d\n", rc);
+ ": cannot register net device: %d\n", rc);
goto probe_err_register;
}
+ printk(KERN_INFO "%s: " DRV_NAME "-" DRV_VERSION " " DRV_RELDATE "\n",
+ dev->name);
out:
return rc;
--
1.6.0.6
^ permalink raw reply related
* [PATCH 2/3] korina: add error-handling to korina_alloc_ring
From: Phil Sutter @ 2009-08-09 0:06 UTC (permalink / raw)
To: netdev; +Cc: Andrew Morton, David S. Miller, florian, Roel Kluin
In-Reply-To: <1249776388-4626-2-git-send-email-n0-1@freewrt.org>
This also avoids a potential buffer overflow in case the very first
receive descriptor fails to allocate, as an index of -1 would be used
afterwards. Kudos to Roel Kluin for pointing this out and providing an
initial patch.
Signed-off-by: Phil Sutter <n0-1@freewrt.org>
---
drivers/net/korina.c | 12 +++++++++---
1 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/net/korina.c b/drivers/net/korina.c
index c09e2ce..d256022 100644
--- a/drivers/net/korina.c
+++ b/drivers/net/korina.c
@@ -741,7 +741,7 @@ static struct ethtool_ops netdev_ethtool_ops = {
.get_link = netdev_get_link,
};
-static void korina_alloc_ring(struct net_device *dev)
+static int korina_alloc_ring(struct net_device *dev)
{
struct korina_private *lp = netdev_priv(dev);
struct sk_buff *skb;
@@ -762,7 +762,7 @@ static void korina_alloc_ring(struct net_device *dev)
for (i = 0; i < KORINA_NUM_RDS; i++) {
skb = dev_alloc_skb(KORINA_RBSIZE + 2);
if (!skb)
- break;
+ return -ENOMEM;
skb_reserve(skb, 2);
skb->dev = dev;
lp->rx_skb[i] = skb;
@@ -782,6 +782,8 @@ static void korina_alloc_ring(struct net_device *dev)
lp->rx_chain_head = 0;
lp->rx_chain_tail = 0;
lp->rx_chain_status = desc_empty;
+
+ return 0;
}
static void korina_free_ring(struct net_device *dev)
@@ -824,7 +826,11 @@ static int korina_init(struct net_device *dev)
writel(ETH_INT_FC_EN, &lp->eth_regs->ethintfc);
/* Allocate rings */
- korina_alloc_ring(dev);
+ if (korina_alloc_ring(dev)) {
+ printk(KERN_ERR "%s: descriptor allocation failed\n", dev->name);
+ korina_free_ring(dev);
+ return -ENOMEM;
+ }
writel(0, &lp->rx_dma_regs->dmas);
/* Start Rx DMA */
--
1.6.0.6
^ 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