* Re: [PATCH] ppp: eliminate shadowed variable name
From: David Miller @ 2010-06-02 12:17 UTC (permalink / raw)
To: shemminger; +Cc: paulus, netdev
In-Reply-To: <20100601090546.79c1fca0@nehalam>
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Tue, 1 Jun 2010 09:05:46 -0700
> Sparse complains about shadowed declaration of skb. So use other
> name.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next-2.6] net: CONFIG_NET_NS reduction
From: David Miller @ 2010-06-02 12:17 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, kaber
In-Reply-To: <1275411079.2738.231.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 01 Jun 2010 18:51:19 +0200
> Use read_pnet() and write_pnet() to reduce number of ifdef CONFIG_NET_NS
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied, thanks Eric.
^ permalink raw reply
* Re: [PATCH] cls_u32: use skb_copy_bits() to dereference data safely
From: jamal @ 2010-06-02 12:20 UTC (permalink / raw)
To: Changli Gao; +Cc: David S. Miller, netdev
In-Reply-To: <AANLkTilkweiJPmQOUv78lKL9ohZo1StzlJcBw0johYi0@mail.gmail.com>
Hi Changli,
On Wed, 2010-06-02 at 01:47 +0800, Changli Gao wrote:
>
> I added the following debug code into cls_u32.c
>
> for (i = n->sel.nkeys; i>0; i--, key++) {
> + int off;
> +
> + off = key->off+(off2&key->offmask) + (ptr - skb->data);
> + if (off + 4 > skb->len)
> + printk("skb->len: %d, off: %d\n",
> skb->len, off);
>
Ok, makes more sense. And thanks for taking time to construct a
meaningful example.
It is not a common use - but i agree it is a bug.
I am suprised we never caught this all this years and wondering why this
never crashed in your example?
Can we make the fix very simple please? i.e no copy bits, this is the
fast path.
> It isn't an optimization, but an error exit. :)
What i meant was if you can tell immediately what the maximum offset is
then you dont need to go through for loop making comparison with each
key. You could immediately bailout - which is an optimization ;->
cheers,
jamal
^ permalink raw reply
* Re: [PATCH] cls_u32: use skb_copy_bits() to dereference data safely
From: jamal @ 2010-06-02 12:25 UTC (permalink / raw)
To: Changli Gao; +Cc: David S. Miller, netdev
In-Reply-To: <1275481219.14363.6.camel@bigi>
On Wed, 2010-06-02 at 08:21 -0400, jamal wrote:
> Can we make the fix very simple please? i.e no copy bits, this is the
> fast path.
Example, something along lines of:
---
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 9627542..dde7a23 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -135,6 +135,9 @@ next_knode:
for (i = n->sel.nkeys; i>0; i--, key++) {
+ int toff = key->off+(off2&key->offmask)- 4;
+ if (unlikely(toff > skb->len))
+ /* bailout here - needs some thought */
if ((*(__be32*)(ptr+key->off+(off2&key->offmask))^key->v
n = n->next;
goto next_knode;
----
cheers,
jamal
^ permalink raw reply related
* Re: [PATCH] cls_u32: use skb_copy_bits() to dereference data safely
From: David Miller @ 2010-06-02 12:45 UTC (permalink / raw)
To: hadi; +Cc: xiaosuo, netdev
In-Reply-To: <1275481538.14363.10.camel@bigi>
From: jamal <hadi@cyberus.ca>
Date: Wed, 02 Jun 2010 08:25:38 -0400
> --- a/net/sched/cls_u32.c
> +++ b/net/sched/cls_u32.c
> @@ -135,6 +135,9 @@ next_knode:
>
> for (i = n->sel.nkeys; i>0; i--, key++) {
>
> + int toff = key->off+(off2&key->offmask)- 4;
> + if (unlikely(toff > skb->len))
> + /* bailout here - needs some thought */
> if ((*(__be32*)(ptr+key->off+(off2&key->offmask))^key->v
I don't think it's that simple.
You can't dereference from the skb->data linear area if your offset is
beyond "skb->len - skb->data_len" (aka. skb_headlen()) since that's
where the paged or fragmented portion starts.
We really need to use skb_copy_bits() if we want to allow
any offset into the SKB, and because of all the ways
packets can be transformed and constructed we absolutely
have to.
^ permalink raw reply
* Re: [PATCH] cls_u32: use skb_copy_bits() to dereference data safely
From: David Miller @ 2010-06-02 12:47 UTC (permalink / raw)
To: hadi; +Cc: xiaosuo, netdev
In-Reply-To: <1275481219.14363.6.camel@bigi>
From: jamal <hadi@cyberus.ca>
Date: Wed, 02 Jun 2010 08:20:19 -0400
> I am suprised we never caught this all this years and wondering why
> this never crashed in your example?
Well for one thing there is all sorts of "stuff" past the end of the
valid skb->data area. For example, there is some padding and then
there is skb_shared_info().
Furthermore, the kernel allocator can round up the size it uses for
SLAB objects which gives even more padding past the end of even
skb_shared_info().
Futrhermore, the chance of the page past the page skb->data is in
being invalid is very low. You'd have to have invalid memory in the
page after the skb->data.
All of this conspires to just letting blind reads work in a large
number of illegal cases.
^ permalink raw reply
* Re: [PATCH]: vxge: Fix checkstack warning in vxge_probe()
From: David Miller @ 2010-06-02 12:51 UTC (permalink / raw)
To: prarit; +Cc: netdev, mschmidt
In-Reply-To: <20100528175735.30903.21134.sendpatchset@prarit.bos.redhat.com>
From: Prarit Bhargava <prarit@redhat.com>
Date: Fri, 28 May 2010 14:01:49 -0400
> Linux 2.6.33 reports this checkstack warning:
>
> drivers/net/vxge/vxge-main.c: In function 'vxge_probe':
> drivers/net/vxge/vxge-main.c:4409: warning: the frame size of 1028 bytes is larger than 1024 bytes
>
> This warning does not occur in the latest linux-2.6 or linux-next, however,
> when I do a 'make -j32 CONFIG_FRAME_WARN=512' instead of 1024 I see
>
> drivers/net/vxge/vxge-main.c: In function ‘vxge_probe’:
> drivers/net/vxge/vxge-main.c:4423: warning: the frame size of 1024 bytes is larger than 512 bytes
>
> This patch moves the large vxge_config struct off the stack.
>
> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Applied to net-next-2.6, thanks.
^ permalink raw reply
* Re: [PATCH] packet_mmap: expose hw packet timestamps to network packet capture utilities
From: David Miller @ 2010-06-02 12:54 UTC (permalink / raw)
To: scott.a.mcmillan; +Cc: netdev, tcpdump-workers
In-Reply-To: <09ED21B37E0F694688A2317C4FED9ED3046E53F595@azsmsx504.amr.corp.intel.com>
From: "Mcmillan, Scott A" <scott.a.mcmillan@intel.com>
Date: Thu, 27 May 2010 09:58:19 -0700
> Subject: [RFC PATCH] packet_mmap: expose hw packet timestamps to network packet capture utilities
>
> This patch adds a setting, PACKET_TIMESTAMP, to specify the packet
> timestamp source that is exported to capture utilities like tcpdump
> by packet_mmap.
>
> PACKET_TIMESTAMP accepts the same integer bit field as
> SO_TIMESTAMPING. However, only the SOF_TIMESTAMPING_SYS_HARDWARE
> and SOF_TIMESTAMPING_RAW_HARDWARE values are currently recognized by
> PACKET_TIMESTAMP. SOF_TIMESTAMPING_SYS_HARDWARE takes precedence
> over SOF_TIMESTAMPING_RAW_HARDWARE if both bits are set.
>
> If PACKET_TIMESTAMP is not set, a software timestamp generated
> inside the networking stack is used (the behavior before this
> setting was added).
>
> Signed-off-by: Scott McMillan <scott.a.mcmillan@intel.com>
Ok, this looks fine, applied.
Thanks.
^ permalink raw reply
* Re: [PATCH] netfilter: Xtables: idletimer target implementation
From: Jan Engelhardt @ 2010-06-02 12:54 UTC (permalink / raw)
To: Luciano Coelho; +Cc: netdev, netfilter-devel, kaber, Timo Teras
In-Reply-To: <1275479897-16779-1-git-send-email-luciano.coelho@nokia.com>
On Wednesday 2010-06-02 13:58, Luciano Coelho wrote:
>+
>+#ifndef _XT_IDLETIMER_H
>+#define _XT_IDLETIMER_H
>+
>+#define MAX_LABEL_SIZE 32
>+
>+struct idletimer_tg_info {
>+ unsigned int timeout;
>+
>+ char label[MAX_LABEL_SIZE];
>+};
As per "Writing Netfilter Modules" e-book, using "int" is a no-no.
>
>+config NETFILTER_XT_TARGET_IDLETIMER
>+ tristate "IDLETIMER target support"
depends on NETFILTER_ADVANCED
>xt_IDLETIMER.c
>+struct idletimer_tg_attr {
>+ struct attribute attr;
>+ ssize_t (*show)(struct kobject *kobj,
>+ struct attribute *attr, char *buf);
>+};
Some indent seems to have gone wrong.
>+ attr->attr.name = kstrdup(info->label, GFP_KERNEL);
Need to check return value!
>+ attr->attr.mode = 0444;
attr->attr.mode = S_IRUGO;
>+static struct xt_target idletimer_tg __read_mostly = {
>+ .name = "IDLETIMER",
>+ .family = NFPROTO_IPV4,
NFPROTO_UNSPEC
>+ .target = idletimer_tg_target,
>+ .targetsize = sizeof(struct idletimer_tg_info),
>+ .checkentry = idletimer_tg_checkentry,
>+ .destroy = idletimer_tg_destroy,
>+ .me = THIS_MODULE,
>+};
>+
>+static int __init idletimer_tg_init(void)
>+{
>+ int ret;
>+
>+ idletimer_tg_kobj = kobject_create_and_add("idletimer",
>+ &THIS_MODULE->mkobj.kobj);
>+ if (!idletimer_tg_kobj)
>+ return -ENOMEM;
>+
>+ /* FIXME: do we want to keep it in the module or in the net class? */
I have only ever seen interfaces in /sys/class/net, so it might be
wise to keep it that way in light of scripts doing
echo /sys/class/net/* to get a list of interfaces.
Looks quite ok.
^ permalink raw reply
* Re: [PATCH] phylib: Add support for the LXT973 phy.
From: Richard Cochran @ 2010-06-02 12:55 UTC (permalink / raw)
To: Andy Fleming; +Cc: netdev
In-Reply-To: <AANLkTilKJVZiKLRUlL7AeaqSnlAs2u5sTW7o2vfvEKns@mail.gmail.com>
On Tue, Jun 01, 2010 at 05:39:22PM -0500, Andy Fleming wrote:
> That's a bit hacky. There is a dev_flags field, which could be used
> for this. Probably, we should add a more general way of saying what
> sort of port this is. But don't use the presence and absence of
> "priv", as it could one day get used for a different purpose, and this
> seems like it would leave us open to strange bugs.
Okay, I changed it.
At first, I was worried about using 'dev_flags' because I couldn't
tell exactly who may write to this field. Looking at tg.c and
broadcom.c, it appears that the MAC drivers may also write this
field. In contrast, the 'priv' field is surely private.
> Also, is this erratum true for all lxt973 models, or is it fixed in
> some revisions?
The documentation http://www.cortina-systems.com/products/download/266
says, "Status: This erratum has been previously fixed." However, I
could not find a reference to when this was fixed.
Richard
Date: Wed, 2 Jun 2010 13:47:02 +0200
Subject: [PATCH] phylib: Add support for the LXT973 phy.
This patch implements a work around for Erratum 5, "3.3 V Fiber Speed
Selection." If the hardware wiring does not respect this erratum, then
fiber optic mode will not work properly.
Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
---
drivers/net/phy/lxt.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 51 insertions(+), 1 deletions(-)
diff --git a/drivers/net/phy/lxt.c b/drivers/net/phy/lxt.c
index 8ee929b..4f97fdc 100644
--- a/drivers/net/phy/lxt.c
+++ b/drivers/net/phy/lxt.c
@@ -53,6 +53,9 @@
#define MII_LXT971_ISR 19 /* Interrupt Status Register */
+/* register definitions for the 973 */
+#define MII_LXT973_PCR 16 /* Port Configuration Register */
+#define PCR_FIBER_SELECT 1
MODULE_DESCRIPTION("Intel LXT PHY driver");
MODULE_AUTHOR("Andy Fleming");
@@ -119,6 +122,34 @@ static int lxt971_config_intr(struct phy_device *phydev)
return err;
}
+static int lxt973_probe(struct phy_device *phydev)
+{
+ int val = phy_read(phydev, MII_LXT973_PCR);
+
+ if (val & PCR_FIBER_SELECT) {
+ /*
+ * If fiber is selected, then the only correct setting
+ * is 100Mbps, full duplex, and auto negotiation off.
+ */
+ val = phy_read(phydev, MII_BMCR);
+ val |= (BMCR_SPEED100 | BMCR_FULLDPLX);
+ val &= ~BMCR_ANENABLE;
+ phy_write(phydev, MII_BMCR, val);
+ /* Remember that the port is in fiber mode. */
+ phydev->dev_flags = PCR_FIBER_SELECT;
+ } else {
+ phydev->dev_flags = 0;
+ }
+ return 0;
+}
+
+static int lxt973_config_aneg(struct phy_device *phydev)
+{
+ /* Do nothing if port is in fiber mode. */
+ return PCR_FIBER_SELECT == phydev->dev_flags ?
+ 0 : genphy_config_aneg(phydev);
+}
+
static struct phy_driver lxt970_driver = {
.phy_id = 0x78100000,
.name = "LXT970",
@@ -146,6 +177,18 @@ static struct phy_driver lxt971_driver = {
.driver = { .owner = THIS_MODULE,},
};
+static struct phy_driver lxt973_driver = {
+ .phy_id = 0x00137a10,
+ .name = "LXT973",
+ .phy_id_mask = 0xfffffff0,
+ .features = PHY_BASIC_FEATURES,
+ .flags = 0,
+ .probe = lxt973_probe,
+ .config_aneg = lxt973_config_aneg,
+ .read_status = genphy_read_status,
+ .driver = { .owner = THIS_MODULE,},
+};
+
static int __init lxt_init(void)
{
int ret;
@@ -157,9 +200,15 @@ static int __init lxt_init(void)
ret = phy_driver_register(&lxt971_driver);
if (ret)
goto err2;
+
+ ret = phy_driver_register(&lxt973_driver);
+ if (ret)
+ goto err3;
return 0;
- err2:
+ err3:
+ phy_driver_unregister(&lxt971_driver);
+ err2:
phy_driver_unregister(&lxt970_driver);
err1:
return ret;
@@ -169,6 +218,7 @@ static void __exit lxt_exit(void)
{
phy_driver_unregister(&lxt970_driver);
phy_driver_unregister(&lxt971_driver);
+ phy_driver_unregister(&lxt973_driver);
}
module_init(lxt_init);
--
1.6.3.3
^ permalink raw reply related
* Re: [PATCH] phylib: Add support for the LXT973 phy.
From: Richard Cochran @ 2010-06-02 13:07 UTC (permalink / raw)
To: Andy Fleming; +Cc: netdev
In-Reply-To: <20100602125527.GA20396@riccoc20.at.omicron.at>
On Wed, Jun 02, 2010 at 02:55:27PM +0200, Richard Cochran wrote:
> On Tue, Jun 01, 2010 at 05:39:22PM -0500, Andy Fleming wrote:
> > Also, is this erratum true for all lxt973 models, or is it fixed in
> > some revisions?
>
> The documentation http://www.cortina-systems.com/products/download/266
> says, "Status: This erratum has been previously fixed." However, I
> could not find a reference to when this was fixed.
In any case, the PHY only supports 100 Mbps when in fiber mode, so the
fix is always safe to use.
Richard
^ permalink raw reply
* Re: [PATCH 1/3] korina: fix deadlock on RX FIFO overrun
From: David Miller @ 2010-06-02 13:12 UTC (permalink / raw)
To: phil; +Cc: florian, netdev
In-Reply-To: <20100529232343.ABC1D4CD45@orbit.nwl.cc>
From: Phil Sutter <phil@nwl.cc>
Date: Sun, 30 May 2010 01:23:34 +0200
> By calling korina_restart(), the IRQ handler tries to disable the
> interrupt it's currently serving. This leads to a deadlock since
> disable_irq() waits for any running IRQ handlers to finish before
> returning. This patch addresses the issue by turning korina_restart()
> into a workqueue task, which is then scheduled when needed.
>
> Reproducing the deadlock is easily done using e.g. GNU netcat to send
> large amounts of UDP data to the host running this driver.
>
> Note that the same problem (and fix) applies to TX FIFO underruns, but
> apparently these are less easy to trigger.
>
> Signed-off-by: Phil Sutter <phil@nwl.cc>
Applied.
^ permalink raw reply
* Re: [PATCH 2/3] korina: use netdev_alloc_skb_ip_align() here, too
From: David Miller @ 2010-06-02 13:12 UTC (permalink / raw)
To: phil; +Cc: florian, netdev
In-Reply-To: <20100529232348.3179F4CD57@orbit.nwl.cc>
From: Phil Sutter <phil@nwl.cc>
Date: Sun, 30 May 2010 01:23:35 +0200
> This patch completes commit 89d71a66c40d629e3b1285def543ab1425558cd5
> which missed this spot, as it seems.
>
> Signed-off-by: Phil Sutter <phil@nwl.cc>
Applied.
^ permalink raw reply
* Re: [PATCH 3/3] korina: count RX DMA OVR as rx_fifo_error
From: David Miller @ 2010-06-02 13:13 UTC (permalink / raw)
To: phil; +Cc: florian, netdev
In-Reply-To: <20100529232345.CA3124CD45@orbit.nwl.cc>
From: Phil Sutter <phil@nwl.cc>
Date: Sun, 30 May 2010 01:23:36 +0200
> This way, RX DMA overruns (actually being caused by overrun of the
> 512byte input FIFO) show up in ifconfig output. The rx_fifo_errors
> counter is unused otherwise.
>
> Signed-off-by: Phil Sutter <phil@nwl.cc>
Applied.
^ permalink raw reply
* Re: [PATCH v2] net/fec: fix pm to survive to suspend/resume
From: David Miller @ 2010-06-02 13:13 UTC (permalink / raw)
To: eric; +Cc: netdev, s.hauer, sshtylyov, linux-arm-kernel, fabioestevam
In-Reply-To: <1275163490-13027-1-git-send-email-eric@eukrea.com>
From: Eric Bénard <eric@eukrea.com>
Date: Sat, 29 May 2010 22:04:50 +0200
> * in the actual driver, calling fec_stop and fec_enet_init doesn't
> allow to have a working network interface at resume (where a
> ifconfig down and up is required to recover the interface)
> * by using fec_enet_close and fec_enet_open, this patch solves this
> problem and handle the case where the link changed between suspend
> and resume
> * this patch also disable clock at suspend and reenable it at resume
>
> Signed-off-by: Eric Bénard <eric@eukrea.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] cls_u32: use skb_copy_bits() to dereference data safely
From: Changli Gao @ 2010-06-02 13:14 UTC (permalink / raw)
To: David Miller; +Cc: hadi, netdev
In-Reply-To: <20100602.054520.228955151.davem@davemloft.net>
On Wed, Jun 2, 2010 at 8:45 PM, David Miller <davem@davemloft.net> wrote:
> From: jamal <hadi@cyberus.ca>
> Date: Wed, 02 Jun 2010 08:25:38 -0400
>
>> --- a/net/sched/cls_u32.c
>> +++ b/net/sched/cls_u32.c
>> @@ -135,6 +135,9 @@ next_knode:
>>
>> for (i = n->sel.nkeys; i>0; i--, key++) {
>>
>> + int toff = key->off+(off2&key->offmask)- 4;
>> + if (unlikely(toff > skb->len))
>> + /* bailout here - needs some thought */
>> if ((*(__be32*)(ptr+key->off+(off2&key->offmask))^key->v
>
> I don't think it's that simple.
>
> You can't dereference from the skb->data linear area if your offset is
> beyond "skb->len - skb->data_len" (aka. skb_headlen()) since that's
> where the paged or fragmented portion starts.
>
> We really need to use skb_copy_bits() if we want to allow
> any offset into the SKB, and because of all the ways
> packets can be transformed and constructed we absolutely
> have to.
>
Maybe skb_header_pointer() is lighter.
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* sysfs class/net/ problem
From: Johannes Berg @ 2010-06-02 13:16 UTC (permalink / raw)
To: netdev; +Cc: Eric W. Biederman, Greg KH
Hi,
I didn't find anything related in the archives, if I missed it I'd
appreciate a pointer. I'm currently experiencing the following:
# ip link
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 16436 qdisc noqueue state UNKNOWN
link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UNKNOWN qlen 1000
link/ether 52:54:00:12:34:56 brd ff:ff:ff:ff:ff:ff
# ls -l /sys/class/net/
total 0
lrwxrwxrwx 1 root root 0 Jun 2 13:12 eth0 -> ../../devices/pci0000:00/0000:00:03.0/net/eth0
lrwxrwxrwx 1 root root 0 Jun 2 13:12 lo -> ../../devices/virtual/net/lo
# insmod cfg80211.ko ; insmod mac80211.ko ; insmod mac80211_hwsim.ko radios=3
# ip link
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 16436 qdisc noqueue state UNKNOWN
link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UNKNOWN qlen 1000
link/ether 52:54:00:12:34:56 brd ff:ff:ff:ff:ff:ff
5: wlan0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN qlen 1000
link/ether 02:00:00:00:00:00 brd ff:ff:ff:ff:ff:ff
6: wlan1: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN qlen 1000
link/ether 02:00:00:00:01:00 brd ff:ff:ff:ff:ff:ff
7: wlan2: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN qlen 1000
link/ether 02:00:00:00:02:00 brd ff:ff:ff:ff:ff:ff
8: hwsim0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN
link/ieee802.11/radiotap 12:00:00:00:00:00 brd ff:ff:ff:ff:ff:ff
# ls -l /sys/class/net/
total 0
lrwxrwxrwx 1 root root 0 Jun 2 13:12 eth0 -> ../../devices/pci0000:00/0000:00:03.0/net/eth0
lrwxrwxrwx 1 root root 0 Jun 2 13:14 hwsim0 -> ../../devices/virtual/net/hwsim0
lrwxrwxrwx 1 root root 0 Jun 2 13:12 lo -> ../../devices/virtual/net/lo
lrwxrwxrwx 1 root root 0 Jun 2 13:14 wlan0 -> ../../devices/virtual/mac80211_hwsim/hwsim0/wlan0
lrwxrwxrwx 1 root root 0 Jun 2 13:14 wlan1 -> ../../devices/virtual/mac80211_hwsim/hwsim1/wlan1
lrwxrwxrwx 1 root root 0 Jun 2 13:14 wlan2 -> ../../devices/virtual/mac80211_hwsim/hwsim2/wlan2
# rmmod mac80211_hwsim mac80211 cfg80211
# ip link
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 16436 qdisc noqueue state UNKNOWN
link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UNKNOWN qlen 1000
link/ether 52:54:00:12:34:56 brd ff:ff:ff:ff:ff:ff
# ls -l /sys/class/net/
total 0
lrwxrwxrwx 1 root root 0 Jun 2 13:12 eth0 -> ../../devices/pci0000:00/0000:00:03.0/net/eth0
lrwxrwxrwx 1 root root 0 Jun 2 13:12 lo -> ../../devices/virtual/net/lo
lrwxrwxrwx 1 root root 0 Jun 2 13:14 wlan0 -> ../../devices/virtual/mac80211_hwsim/hwsim0/wlan0
lrwxrwxrwx 1 root root 0 Jun 2 13:14 wlan1 -> ../../devices/virtual/mac80211_hwsim/hwsim1/wlan1
lrwxrwxrwx 1 root root 0 Jun 2 13:14 wlan2 -> ../../devices/virtual/mac80211_hwsim/hwsim2/wlan2
The same problem doesn't happen with veth, so I'm not sure what's
causing the problem.
Help please?
johannes
^ permalink raw reply
* Re: [PATCH] cls_u32: use skb_copy_bits() to dereference data safely
From: Changli Gao @ 2010-06-02 13:17 UTC (permalink / raw)
To: hadi; +Cc: David S. Miller, netdev
In-Reply-To: <1275481219.14363.6.camel@bigi>
On Wed, Jun 2, 2010 at 8:20 PM, jamal <hadi@cyberus.ca> wrote:
>
> What i meant was if you can tell immediately what the maximum offset is
> then you dont need to go through for loop making comparison with each
> key. You could immediately bailout - which is an optimization ;->
>
I got it. But it isn't easy for me.
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH] cls_u32: use skb_copy_bits() to dereference data safely
From: David Miller @ 2010-06-02 13:27 UTC (permalink / raw)
To: xiaosuo; +Cc: hadi, netdev
In-Reply-To: <AANLkTil0sLvcHRTcGTfYYlCYzDH07sl7xgk6EcjL99cl@mail.gmail.com>
From: Changli Gao <xiaosuo@gmail.com>
Date: Wed, 2 Jun 2010 21:14:55 +0800
> On Wed, Jun 2, 2010 at 8:45 PM, David Miller <davem@davemloft.net> wrote:
>> From: jamal <hadi@cyberus.ca>
>> Date: Wed, 02 Jun 2010 08:25:38 -0400
>>
>>> --- a/net/sched/cls_u32.c
>>> +++ b/net/sched/cls_u32.c
>>> @@ -135,6 +135,9 @@ next_knode:
>>>
>>> for (i = n->sel.nkeys; i>0; i--, key++) {
>>>
>>> + int toff = key->off+(off2&key->offmask)- 4;
>>> + if (unlikely(toff > skb->len))
>>> + /* bailout here - needs some thought */
>>> if ((*(__be32*)(ptr+key->off+(off2&key->offmask))^key->v
>>
>> I don't think it's that simple.
>>
>> You can't dereference from the skb->data linear area if your offset is
>> beyond "skb->len - skb->data_len" (aka. skb_headlen()) since that's
>> where the paged or fragmented portion starts.
>>
>> We really need to use skb_copy_bits() if we want to allow
>> any offset into the SKB, and because of all the ways
>> packets can be transformed and constructed we absolutely
>> have to.
>>
>
> Maybe skb_header_pointer() is lighter.
Yes, it should be. In fact, it's designed for this kind of situation
and that's why it's used extensively in netfilter.
^ permalink raw reply
* Re: [PATCH 2/2] pktgen: receive packets and process incoming rate
From: Daniel Turull @ 2010-06-02 13:28 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, robert, jens.laas
In-Reply-To: <1275483650.2725.173.camel@edumazet-laptop>
Eric Dumazet wrote:
> Le mercredi 02 juin 2010 à 13:49 +0200, Daniel Turull a écrit :
>> This patch adds receiver part to pktgen taking advantages of SMP systems
>> with multiple rx queues:
>> - Creation of new proc file /proc/net/pktgen/pgrx to control and display the receiver.
>> - It uses PER-CPU variable to store the results per each CPU.
>> - Results displayed per CPU and aggregated.
>> - The packet handler is add in the protocols handlers (dev_Add_pack())
>> - Available statistics: packets and bytes received, work time and rate
>> - Only process pktgen packets
>> - It is possible to select the incoming interface
>> - Documentation updated with the new commands to control the receiver part.
>>
>
> Interesting, but does it belongs to pktgen ?
Yes, it allows to receive a feedback of pktgen of the traffic received which comes from pktgen. It is an addition for pktgen. By default is disabled.
> other comments included :)
>
>> Signed-off-by: Daniel Turull <daniel.turull@gmail.com>
>>
>
>
>>
>> +/*Recevier parameters per cpu*/
>> +struct pktgen_rx {
>> + u64 rx_packets; /*packets arrived*/
>
> unsigned long rx_packets ?
Yes, I used the same type as the number of packets send of the transmission size
struct pktgen_dev
__u64 sofar;
>> + u64 rx_bytes; /*bytes arrived*/
>> +
>> + ktime_t start_time; /*first time stamp of a packet*/
>> + ktime_t last_time; /*last packet arrival */
>> +};
>
>
>> +int pktgen_rcv_basic(struct sk_buff *skb, struct net_device *dev,
>> + struct packet_type *pt, struct net_device *orig_dev)
>> +{
>> + /* Check magic*/
>> + struct iphdr *iph = ip_hdr(skb);
>> + struct pktgen_hdr *pgh;
>> + void *vaddr;
>
> Following code is ... well ... interesting... But ...
>
> 1) Is it IPV6 compatable ? pktgen can be ipv6 or ipv4
It is not IPV6 compatible yet.
> 2) Is it resistant to malicious packets ? (very small ones)
I only tried with pktgen traffic
> 3) No checksum ?
The PKTGEN_MAGIC is checked in order to validate that is a pktgen packet
> I think you should use standard mechanisms... (pskb_may_pull(), ...)
> Take a look at __netpoll_rx() for an example.
>> + if (skb_is_nonlinear(skb)) {
>> + vaddr = kmap_skb_frag(&skb_shinfo(skb)->frags[0]);
>> + pgh = (struct pktgen_hdr *)
>> + (vaddr+skb_shinfo(skb)->frags[0].page_offset);
>> + } else {
>> + pgh = (struct pktgen_hdr *)(((char *)(iph)) + 28);
>> + }
>> +
>> + if (unlikely(pgh->pgh_magic != PKTGEN_MAGIC_NET))
>> + goto end;
>> +
>> + if (unlikely(!__get_cpu_var(pktgen_rx_data).rx_packets))
>> + __get_cpu_var(pktgen_rx_data).start_time = ktime_now();
>> +
>> + __get_cpu_var(pktgen_rx_data).last_time = ktime_now();
>> +
>
> Its a bit suboptimal to use __get_cpu_var several time. Take a look at
> disassembly code :)
>
> Use a pointer instead
Ok, thanks for the advice. I will redo this part in order to make it compatabile with IPV6 and make it more optimal.
>
>> + /* Update counter of packets*/
>> + __get_cpu_var(pktgen_rx_data).rx_packets++;
>> + __get_cpu_var(pktgen_rx_data).rx_bytes += skb->len+14;
>
> This +14 seems suspect (what about vlan tags ?)
> Use ETH_HLEN instead at a very minimum?
This was added, in order to add the ethernet header in the bytes in order to have the same number in tx an rx, but yes I should use ETH_HLEN
>> +end:
>> + if (skb_is_nonlinear(skb))
>> + kunmap_skb_frag(vaddr);
>
> Should not recognised packets be allowed to flight in other parts of
> kernel stack ? This way, we could use ssh to remotely control this
> pktgen session ;)
This handler as a new packet type, but the packets are also delivered to the IP stack. It is possible to control the session with ssh.
>> + kfree_skb(skb);
>> + return 0;
>> +}
>> +
>
^ permalink raw reply
* Re: [PATCH] cls_u32: use skb_copy_bits() to dereference data safely
From: jamal @ 2010-06-02 13:36 UTC (permalink / raw)
To: Changli Gao; +Cc: David Miller, netdev
In-Reply-To: <AANLkTil0sLvcHRTcGTfYYlCYzDH07sl7xgk6EcjL99cl@mail.gmail.com>
On Wed, 2010-06-02 at 21:14 +0800, Changli Gao wrote:
> Maybe skb_header_pointer() is lighter.
A little worse than skb_copy_bits(). In any case, this change is going
to hurt.
Dave, can we assume the upper layers(qdiscs in this case) are
responsible for any linearizing?
Changli, if you have time - can you also audit tcf_pedit()
since it follows TheLinuxWay(tm).
cheers,
jamal
^ permalink raw reply
* Re: [PATCH] netfilter: Xtables: idletimer target implementation
From: Luciano Coelho @ 2010-06-02 13:37 UTC (permalink / raw)
To: ext Jan Engelhardt
Cc: netdev@vger.kernel.org, netfilter-devel@vger.kernel.org,
kaber@trash.net, Timo Teras
In-Reply-To: <alpine.LSU.2.01.1006021444480.14585@obet.zrqbmnf.qr>
Hi Jan,
Thanks for your prompt review! I'll send v2 with the fixes you
suggested.
On Wed, 2010-06-02 at 14:54 +0200, ext Jan Engelhardt wrote:
> On Wednesday 2010-06-02 13:58, Luciano Coelho wrote:
> >+
> >+#ifndef _XT_IDLETIMER_H
> >+#define _XT_IDLETIMER_H
> >+
> >+#define MAX_LABEL_SIZE 32
> >+
> >+struct idletimer_tg_info {
> >+ unsigned int timeout;
> >+
> >+ char label[MAX_LABEL_SIZE];
> >+};
>
> As per "Writing Netfilter Modules" e-book, using "int" is a no-no.
Sorry I missed that one. Fixed in v2.
> >+config NETFILTER_XT_TARGET_IDLETIMER
> >+ tristate "IDLETIMER target support"
>
> depends on NETFILTER_ADVANCED
Yes.
> >xt_IDLETIMER.c
> >+struct idletimer_tg_attr {
> >+ struct attribute attr;
> >+ ssize_t (*show)(struct kobject *kobj,
> >+ struct attribute *attr, char *buf);
> >+};
>
> Some indent seems to have gone wrong.
Fixed.
> >+ attr->attr.name = kstrdup(info->label, GFP_KERNEL);
>
> Need to check return value!
Oops! Fixed in v2. Also added sysfs_remove_file_from_group() if the
struct idletimer_tg allocation fails.
> >+ attr->attr.mode = 0444;
>
> attr->attr.mode = S_IRUGO;
Fixed.
>
> >+static struct xt_target idletimer_tg __read_mostly = {
> >+ .name = "IDLETIMER",
> >+ .family = NFPROTO_IPV4,
>
> NFPROTO_UNSPEC
Yeps, this is a remain from the previous (and ugly) read from ipt_ip.
Fixed.
>
> >+ .target = idletimer_tg_target,
> >+ .targetsize = sizeof(struct idletimer_tg_info),
> >+ .checkentry = idletimer_tg_checkentry,
> >+ .destroy = idletimer_tg_destroy,
> >+ .me = THIS_MODULE,
> >+};
> >+
> >+static int __init idletimer_tg_init(void)
> >+{
> >+ int ret;
> >+
> >+ idletimer_tg_kobj = kobject_create_and_add("idletimer",
> >+ &THIS_MODULE->mkobj.kobj);
> >+ if (!idletimer_tg_kobj)
> >+ return -ENOMEM;
> >+
> >+ /* FIXME: do we want to keep it in the module or in the net class? */
>
> I have only ever seen interfaces in /sys/class/net, so it might be
> wise to keep it that way in light of scripts doing
> echo /sys/class/net/* to get a list of interfaces.
Yes, this is the only reason why I haven't put it under the net class,
which would probably look cleaner. In other classes it seems to be
common to add misc attributes, but the net class (as of now) only
contains interface subclasses, as you said.
I'll change the FIXME to a clearer comment.
> Looks quite ok.
Thanks!
--
Cheers,
Luca.
^ permalink raw reply
* Re: [PATCH] cls_u32: use skb_copy_bits() to dereference data safely
From: David Miller @ 2010-06-02 13:43 UTC (permalink / raw)
To: hadi; +Cc: xiaosuo, netdev
In-Reply-To: <1275485797.3545.4.camel@bigi>
From: jamal <hadi@cyberus.ca>
Date: Wed, 02 Jun 2010 09:36:37 -0400
> On Wed, 2010-06-02 at 21:14 +0800, Changli Gao wrote:
>
>
>> Maybe skb_header_pointer() is lighter.
>
> A little worse than skb_copy_bits(). In any case, this change is going
> to hurt.
Umm, Jamal what are you talking about?
Using skb_header_pointer(), if the offset is in range, there is no
change from today other than a comparison.
If it is not in range, we use skb_copy_bits(). It's only for the
case where we have to fetch the value from the fragmented part
of the SKB.
^ permalink raw reply
* Re: [PATCH] cls_u32: use skb_copy_bits() to dereference data safely
From: Changli Gao @ 2010-06-02 13:43 UTC (permalink / raw)
To: hadi; +Cc: David Miller, netdev
In-Reply-To: <1275485797.3545.4.camel@bigi>
On Wed, Jun 2, 2010 at 9:36 PM, jamal <hadi@cyberus.ca> wrote:
> On Wed, 2010-06-02 at 21:14 +0800, Changli Gao wrote:
>
>
>> Maybe skb_header_pointer() is lighter.
>
> A little worse than skb_copy_bits(). In any case, this change is going
> to hurt.
Why? it is an inline function, and in most cases, there isn't any function call.
> Dave, can we assume the upper layers(qdiscs in this case) are
> responsible for any linearizing?
>
> Changli, if you have time - can you also audit tcf_pedit()
> since it follows TheLinuxWay(tm).
>
Yea, pedit has the same issue. Besides this issue, they should use
get_unaligned() instead.
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH] cls_u32: use skb_copy_bits() to dereference data safely
From: jamal @ 2010-06-02 13:47 UTC (permalink / raw)
To: David Miller; +Cc: xiaosuo, netdev
In-Reply-To: <20100602.064316.218079399.davem@davemloft.net>
On Wed, 2010-06-02 at 06:43 -0700, David Miller wrote:
> From: jamal <hadi@cyberus.ca>
> >
> > A little worse than skb_copy_bits(). In any case, this change is going
> > to hurt.
>
> Umm, Jamal what are you talking about?
;->
> Using skb_header_pointer(), if the offset is in range, there is no
> change from today other than a comparison.
Thats the part i glossed over - My eyes just saw "it calls
skb_copy_bits()" ;->
> If it is not in range, we use skb_copy_bits(). It's only for the
> case where we have to fetch the value from the fragmented part
> of the SKB.
cheers,
jamal
^ 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