Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] netfilter: undefined reference to 'nf_conntrack_tstamp_*'
From: Patrick McHardy @ 2011-01-20 19:52 UTC (permalink / raw)
  To: John Fastabend; +Cc: netfilter-devel, netdev, pablo
In-Reply-To: <20110120191612.24205.73463.stgit@jf-dev1-dcblab>

[-- Attachment #1: Type: text/plain, Size: 538 bytes --]

Am 20.01.2011 20:16, schrieb John Fastabend:
> net/built-in.o: In function `nf_conntrack_init_net':
> net/netfilter/nf_conntrack_core.c:1521:
> 	undefined reference to `nf_conntrack_tstamp_init'
> net/netfilter/nf_conntrack_core.c:1531:
> 	undefined reference to `nf_conntrack_tstamp_fini'
> 
> Add 'selects' notation to Kconfig to include NF_CONNTRACK_TIMESTAMP
> this resolves all the config files I tested.

That's not the correct fix, NF_CONNTRACK_TIMESTAMP is supposed
to be option. Please try whether this patch fixes the problem.


[-- Attachment #2: x --]
[-- Type: text/plain, Size: 1418 bytes --]

commit 2f1e3176723d74ea2dd975e5be0ef6bb4fed2e2e
Author: Patrick McHardy <kaber@trash.net>
Date:   Thu Jan 20 20:46:52 2011 +0100

    netfilter: nf_conntrack: fix linker error with NF_CONNTRACK_TIMESTAMP=n
    
    net/built-in.o: In function `nf_conntrack_init_net':
    net/netfilter/nf_conntrack_core.c:1521:
    	undefined reference to `nf_conntrack_tstamp_init'
    net/netfilter/nf_conntrack_core.c:1531:
    	undefined reference to `nf_conntrack_tstamp_fini'
    
    Add dummy inline functions for the =n case to fix this.
    
    Reported-by: John Fastabend <john.r.fastabend@intel.com>
    Signed-off-by: Patrick McHardy <kaber@trash.net>

diff --git a/include/net/netfilter/nf_conntrack_timestamp.h b/include/net/netfilter/nf_conntrack_timestamp.h
index f17dcb6..fc9c82b 100644
--- a/include/net/netfilter/nf_conntrack_timestamp.h
+++ b/include/net/netfilter/nf_conntrack_timestamp.h
@@ -47,7 +47,19 @@ static inline void nf_ct_set_tstamp(struct net *net, bool enable)
 	net->ct.sysctl_tstamp = enable;
 }
 
+#ifdef CONFIG_NF_CONNTRACK_TIMESTAMP
 extern int nf_conntrack_tstamp_init(struct net *net);
 extern void nf_conntrack_tstamp_fini(struct net *net);
+#else
+static inline int nf_conntrack_tstamp_init(struct net *net)
+{
+	return 0;
+}
+
+static inline void nf_conntrack_tstamp_fini(struct net *net)
+{
+	return;
+}
+#endif /* CONFIG_NF_CONNTRACK_TIMESTAMP */
 
 #endif /* _NF_CONNTRACK_TSTAMP_H */

^ permalink raw reply related

* Re: [PATCH] Ensure that we unshare skbs prior to calling pskb_may_pull in bonding driver
From: Jay Vosburgh @ 2011-01-20 19:43 UTC (permalink / raw)
  To: Neil Horman; +Cc: netdev, Andy Gospodarek, David S. Miller
In-Reply-To: <1295550151-25913-1-git-send-email-nhorman@tuxdriver.com>

Neil Horman <nhorman@tuxdriver.com> wrote:

>Recently reported oops:
>
>kernel BUG at net/core/skbuff.c:813!
>invalid opcode: 0000 [#1] SMP
>last sysfs file: /sys/devices/virtual/net/bond0/broadcast
>CPU 8
>Modules linked in: sit tunnel4 cpufreq_ondemand acpi_cpufreq freq_table bonding
>ipv6 dm_mirror dm_region_hash dm_log cdc_ether usbnet mii serio_raw i2c_i801
>i2c_core iTCO_wdt iTCO_vendor_support shpchp ioatdma i7core_edac edac_core bnx2
>ixgbe dca mdio sg ext4 mbcache jbd2 sd_mod crc_t10dif mptsas mptscsih mptbase
>scsi_transport_sas dm_mod [last unloaded: microcode]
>
>Modules linked in: sit tunnel4 cpufreq_ondemand acpi_cpufreq freq_table bonding
>ipv6 dm_mirror dm_region_hash dm_log cdc_ether usbnet mii serio_raw i2c_i801
>i2c_core iTCO_wdt iTCO_vendor_support shpchp ioatdma i7core_edac edac_core bnx2
>ixgbe dca mdio sg ext4 mbcache jbd2 sd_mod crc_t10dif mptsas mptscsih mptbase
>scsi_transport_sas dm_mod [last unloaded: microcode]
>Pid: 0, comm: swapper Not tainted 2.6.32-71.el6.x86_64 #1 BladeCenter HS22
>-[7870AC1]-
>RIP: 0010:[<ffffffff81405b16>]  [<ffffffff81405b16>]
>pskb_expand_head+0x36/0x1e0
>RSP: 0018:ffff880028303b70  EFLAGS: 00010202
>RAX: 0000000000000002 RBX: ffff880c6458ec80 RCX: 0000000000000020
>RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff880c6458ec80
>RBP: ffff880028303bc0 R08: ffffffff818a6180 R09: ffff880c6458ed64
>R10: ffff880c622b36c0 R11: 0000000000000400 R12: 0000000000000000
>R13: 0000000000000180 R14: ffff880c622b3000 R15: 0000000000000000
>FS:  0000000000000000(0000) GS:ffff880028300000(0000) knlGS:0000000000000000
>CS:  0010 DS: 0018 ES: 0018 CR0: 000000008005003b
>CR2: 00000038653452a4 CR3: 0000000001001000 CR4: 00000000000006e0
>DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
>Process swapper (pid: 0, threadinfo ffff8806649c2000, task ffff880c64f16ab0)
>Stack:
> ffff880028303bc0 ffffffff8104fff9 000000000000001c 0000000100000000
><0> ffff880000047d80 ffff880c6458ec80 000000000000001c ffff880c6223da00
><0> ffff880c622b3000 0000000000000000 ffff880028303c10 ffffffff81407f7a
>Call Trace:
><IRQ>
> [<ffffffff8104fff9>] ? __wake_up_common+0x59/0x90
> [<ffffffff81407f7a>] __pskb_pull_tail+0x2aa/0x360
> [<ffffffffa0244530>] bond_arp_rcv+0x2c0/0x2e0 [bonding]
> [<ffffffff814a0857>] ? packet_rcv+0x377/0x440
> [<ffffffff8140f21b>] netif_receive_skb+0x2db/0x670
> [<ffffffff8140f788>] napi_skb_finish+0x58/0x70
> [<ffffffff8140fc89>] napi_gro_receive+0x39/0x50
> [<ffffffffa01286eb>] ixgbe_clean_rx_irq+0x35b/0x900 [ixgbe]
> [<ffffffffa01290f6>] ixgbe_clean_rxtx_many+0x136/0x240 [ixgbe]
> [<ffffffff8140fe53>] net_rx_action+0x103/0x210
> [<ffffffff81073bd7>] __do_softirq+0xb7/0x1e0
> [<ffffffff810d8740>] ? handle_IRQ_event+0x60/0x170
> [<ffffffff810142cc>] call_softirq+0x1c/0x30
> [<ffffffff81015f35>] do_softirq+0x65/0xa0
> [<ffffffff810739d5>] irq_exit+0x85/0x90
> [<ffffffff814cf915>] do_IRQ+0x75/0xf0
> [<ffffffff81013ad3>] ret_from_intr+0x0/0x11
> <EOI>
> [<ffffffff8101bc01>] ? mwait_idle+0x71/0xd0
> [<ffffffff814cd80a>] ? atomic_notifier_call_chain+0x1a/0x20
> [<ffffffff81011e96>] cpu_idle+0xb6/0x110
> [<ffffffff814c17c8>] start_secondary+0x1fc/0x23f
>
>Resulted from bonding driver registering packet handlers via dev_add_pack and
>then trying to call pskb_may_pull. If another packet handler (like for AF_PACKET
>sockets) gets called first, the delivered skb will have a user count > 1, which
>causes pskb_may_pull to BUG halt when it does its skb_shared check.  Fix this by
>calling skb_share_check prior to the may_pull call sites in the bonding driver
>to clone the skb when needed.  Tested by myself and the reported successfully.
>
>Signed-off-by: Neil Horman
>CC: Andy Gospodarek <andy@greyhouse.net>
>CC: Jay Vosburgh <fubar@us.ibm.com>
>CC: "David S. Miller" <davem@davemloft.net>

	Looks like a candidate for -stable as well.

	-J

Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>


>---
> drivers/net/bonding/bond_3ad.c  |    4 ++++
> drivers/net/bonding/bond_alb.c  |    4 ++++
> drivers/net/bonding/bond_main.c |    4 ++++
> 3 files changed, 12 insertions(+), 0 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>index 171782e..1024ae1 100644
>--- a/drivers/net/bonding/bond_3ad.c
>+++ b/drivers/net/bonding/bond_3ad.c
>@@ -2470,6 +2470,10 @@ int bond_3ad_lacpdu_recv(struct sk_buff *skb, struct net_device *dev, struct pac
> 	if (!(dev->flags & IFF_MASTER))
> 		goto out;
>
>+	skb = skb_share_check(skb, GFP_ATOMIC);
>+	if (!skb)
>+		goto out;
>+
> 	if (!pskb_may_pull(skb, sizeof(struct lacpdu)))
> 		goto out;
>
>diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
>index f4e638c..5c6fba8 100644
>--- a/drivers/net/bonding/bond_alb.c
>+++ b/drivers/net/bonding/bond_alb.c
>@@ -326,6 +326,10 @@ static int rlb_arp_recv(struct sk_buff *skb, struct net_device *bond_dev, struct
> 		goto out;
> 	}
>
>+	skb = skb_share_check(skb, GFP_ATOMIC);
>+	if (!skb)
>+		goto out;
>+
> 	if (!pskb_may_pull(skb, arp_hdr_len(bond_dev)))
> 		goto out;
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index b1025b8..163e0b0 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -2733,6 +2733,10 @@ static int bond_arp_rcv(struct sk_buff *skb, struct net_device *dev, struct pack
> 	if (!slave || !slave_do_arp_validate(bond, slave))
> 		goto out_unlock;
>
>+	skb = skb_share_check(skb, GFP_ATOMIC);
>+	if (!skb)
>+		goto out_unlock;
>+
> 	if (!pskb_may_pull(skb, arp_hdr_len(dev)))
> 		goto out_unlock;
>
>-- 
>1.7.3.4
>

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

^ permalink raw reply

* Re: [PATCH] e1000: add support for Marvell Alaska M88E1118R PHY
From: Dirk Brandewie @ 2011-01-20 18:49 UTC (permalink / raw)
  To: Jeff Kirsher; +Cc: Florian Fainelli, netdev@vger.kernel.org, David Miller
In-Reply-To: <AANLkTinDh-ZnrZy3f5H72+FuNwvbnY-BnCns17+cpR=Z@mail.gmail.com>

On Wed, 2011-01-19 at 22:51 -0800, Jeff Kirsher wrote:
> On Wed, Jan 19, 2011 at 01:09, Florian Fainelli <ffainelli@freebox.fr> wrote:
> > From: Florian Fainelli <ffainelli@freebox.fr>
> >
> > This patch adds support for Marvell Alask M88E188R PHY chips. Support for
> > other M88* PHYs is already there, so there is nothing more to add than its
> > PHY id.
> >
> > Signed-off-by: Florian Fainelli <ffainelli@freebox.fr>
> > CC: Dirk Brandewie <dirk.j.brandewie@intel.com>
> > CC: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > ---
> 
> The patch itself looks fine.  I am concerned about validation.
> 
> Dirk - is there a chance that the ce4100 will use this PHY?  If so,
> can you cover the validation?

Florian is working on a CE4100 based platform.  It looks like they used
a different PHY from the Flacon Falls reference platform. I can't
directly test this patch since I don't have their hardware.  I will be
testing .38-rc1 next week on falcon falls.

I think the best we can do without the hardware is to compare the data
sheet for the new PHY with the PHYs already supported and make sure they
are compatible.  If the datasheets match up for the features the driver
is using this seems pretty low risk IMHO.

--Dirk
> 
> For now I will add this to my net-next tree while I await word from Dirk.
> 



^ permalink raw reply

* Re: [PATCH] netfilter: add a missing include in nf_conntrack_reasm.c
From: Patrick McHardy @ 2011-01-20 19:18 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Netfilter Development Mailinglist, netdev, KOVACS Krisztian
In-Reply-To: <1295545997.2825.458.camel@edumazet-laptop>

Am 20.01.2011 18:53, schrieb Eric Dumazet:
> After commit ae90bdeaeac6b (netfilter: fix compilation when conntrack is
> disabled but tproxy is enabled) we have following warnings :
> 
> net/ipv6/netfilter/nf_conntrack_reasm.c:520:16: warning: symbol
> 'nf_ct_frag6_gather' was not declared. Should it be static?
> net/ipv6/netfilter/nf_conntrack_reasm.c:591:6: warning: symbol
> 'nf_ct_frag6_output' was not declared. Should it be static?
> net/ipv6/netfilter/nf_conntrack_reasm.c:612:5: warning: symbol
> 'nf_ct_frag6_init' was not declared. Should it be static?
> net/ipv6/netfilter/nf_conntrack_reasm.c:640:6: warning: symbol
> 'nf_ct_frag6_cleanup' was not declared. Should it be static?
> 
> Fix this including net/netfilter/ipv6/nf_defrag_ipv6.h
> 

I currently don't have access to all my trees, does this patch
apply to 2.6.38 or just the current -rc?

^ permalink raw reply

* [PATCH] netfilter: undefined reference to 'nf_conntrack_tstamp_*'
From: John Fastabend @ 2011-01-20 19:16 UTC (permalink / raw)
  To: kaber; +Cc: john.r.fastabend, netfilter-devel, netdev, pablo

net/built-in.o: In function `nf_conntrack_init_net':
net/netfilter/nf_conntrack_core.c:1521:
	undefined reference to `nf_conntrack_tstamp_init'
net/netfilter/nf_conntrack_core.c:1531:
	undefined reference to `nf_conntrack_tstamp_fini'

Add 'selects' notation to Kconfig to include NF_CONNTRACK_TIMESTAMP
this resolves all the config files I tested.

Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
CC: Pablo Neira Ayuso <pablo@netfilter.org>
---

 net/netfilter/Kconfig |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
index faf7412..2079911 100644
--- a/net/netfilter/Kconfig
+++ b/net/netfilter/Kconfig
@@ -27,6 +27,7 @@ config NETFILTER_NETLINK_LOG
 config NF_CONNTRACK
 	tristate "Netfilter connection tracking support"
 	default m if NETFILTER_ADVANCED=n
+	select NF_CONNTRACK_TIMESTAMP
 	help
 	  Connection tracking keeps a record of what packets have passed
 	  through your machine, in order to figure out how they are related
@@ -87,7 +88,7 @@ config NF_CONNTRACK_EVENTS
 
 config NF_CONNTRACK_TIMESTAMP
 	bool  'Connection tracking timestamping'
-	depends on NETFILTER_ADVANCED
+	default m if NETFILTER_ADVANCED
 	help
 	  This option enables support for connection tracking timestamping.
 	  This allows you to store the flow start-time and to obtain


^ permalink raw reply related

* [PATCH] Ensure that we unshare skbs prior to calling pskb_may_pull in bonding driver
From: Neil Horman @ 2011-01-20 19:02 UTC (permalink / raw)
  To: netdev; +Cc: Neil Horman, Andy Gospodarek, Jay Vosburgh, David S. Miller

Recently reported oops:

kernel BUG at net/core/skbuff.c:813!
invalid opcode: 0000 [#1] SMP
last sysfs file: /sys/devices/virtual/net/bond0/broadcast
CPU 8
Modules linked in: sit tunnel4 cpufreq_ondemand acpi_cpufreq freq_table bonding
ipv6 dm_mirror dm_region_hash dm_log cdc_ether usbnet mii serio_raw i2c_i801
i2c_core iTCO_wdt iTCO_vendor_support shpchp ioatdma i7core_edac edac_core bnx2
ixgbe dca mdio sg ext4 mbcache jbd2 sd_mod crc_t10dif mptsas mptscsih mptbase
scsi_transport_sas dm_mod [last unloaded: microcode]

Modules linked in: sit tunnel4 cpufreq_ondemand acpi_cpufreq freq_table bonding
ipv6 dm_mirror dm_region_hash dm_log cdc_ether usbnet mii serio_raw i2c_i801
i2c_core iTCO_wdt iTCO_vendor_support shpchp ioatdma i7core_edac edac_core bnx2
ixgbe dca mdio sg ext4 mbcache jbd2 sd_mod crc_t10dif mptsas mptscsih mptbase
scsi_transport_sas dm_mod [last unloaded: microcode]
Pid: 0, comm: swapper Not tainted 2.6.32-71.el6.x86_64 #1 BladeCenter HS22
-[7870AC1]-
RIP: 0010:[<ffffffff81405b16>]  [<ffffffff81405b16>]
pskb_expand_head+0x36/0x1e0
RSP: 0018:ffff880028303b70  EFLAGS: 00010202
RAX: 0000000000000002 RBX: ffff880c6458ec80 RCX: 0000000000000020
RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff880c6458ec80
RBP: ffff880028303bc0 R08: ffffffff818a6180 R09: ffff880c6458ed64
R10: ffff880c622b36c0 R11: 0000000000000400 R12: 0000000000000000
R13: 0000000000000180 R14: ffff880c622b3000 R15: 0000000000000000
FS:  0000000000000000(0000) GS:ffff880028300000(0000) knlGS:0000000000000000
CS:  0010 DS: 0018 ES: 0018 CR0: 000000008005003b
CR2: 00000038653452a4 CR3: 0000000001001000 CR4: 00000000000006e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process swapper (pid: 0, threadinfo ffff8806649c2000, task ffff880c64f16ab0)
Stack:
 ffff880028303bc0 ffffffff8104fff9 000000000000001c 0000000100000000
<0> ffff880000047d80 ffff880c6458ec80 000000000000001c ffff880c6223da00
<0> ffff880c622b3000 0000000000000000 ffff880028303c10 ffffffff81407f7a
Call Trace:
<IRQ>
 [<ffffffff8104fff9>] ? __wake_up_common+0x59/0x90
 [<ffffffff81407f7a>] __pskb_pull_tail+0x2aa/0x360
 [<ffffffffa0244530>] bond_arp_rcv+0x2c0/0x2e0 [bonding]
 [<ffffffff814a0857>] ? packet_rcv+0x377/0x440
 [<ffffffff8140f21b>] netif_receive_skb+0x2db/0x670
 [<ffffffff8140f788>] napi_skb_finish+0x58/0x70
 [<ffffffff8140fc89>] napi_gro_receive+0x39/0x50
 [<ffffffffa01286eb>] ixgbe_clean_rx_irq+0x35b/0x900 [ixgbe]
 [<ffffffffa01290f6>] ixgbe_clean_rxtx_many+0x136/0x240 [ixgbe]
 [<ffffffff8140fe53>] net_rx_action+0x103/0x210
 [<ffffffff81073bd7>] __do_softirq+0xb7/0x1e0
 [<ffffffff810d8740>] ? handle_IRQ_event+0x60/0x170
 [<ffffffff810142cc>] call_softirq+0x1c/0x30
 [<ffffffff81015f35>] do_softirq+0x65/0xa0
 [<ffffffff810739d5>] irq_exit+0x85/0x90
 [<ffffffff814cf915>] do_IRQ+0x75/0xf0
 [<ffffffff81013ad3>] ret_from_intr+0x0/0x11
 <EOI>
 [<ffffffff8101bc01>] ? mwait_idle+0x71/0xd0
 [<ffffffff814cd80a>] ? atomic_notifier_call_chain+0x1a/0x20
 [<ffffffff81011e96>] cpu_idle+0xb6/0x110
 [<ffffffff814c17c8>] start_secondary+0x1fc/0x23f

Resulted from bonding driver registering packet handlers via dev_add_pack and
then trying to call pskb_may_pull. If another packet handler (like for AF_PACKET
sockets) gets called first, the delivered skb will have a user count > 1, which
causes pskb_may_pull to BUG halt when it does its skb_shared check.  Fix this by
calling skb_share_check prior to the may_pull call sites in the bonding driver
to clone the skb when needed.  Tested by myself and the reported successfully.

Signed-off-by: Neil Horman
CC: Andy Gospodarek <andy@greyhouse.net>
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: "David S. Miller" <davem@davemloft.net>
---
 drivers/net/bonding/bond_3ad.c  |    4 ++++
 drivers/net/bonding/bond_alb.c  |    4 ++++
 drivers/net/bonding/bond_main.c |    4 ++++
 3 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 171782e..1024ae1 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -2470,6 +2470,10 @@ int bond_3ad_lacpdu_recv(struct sk_buff *skb, struct net_device *dev, struct pac
 	if (!(dev->flags & IFF_MASTER))
 		goto out;
 
+	skb = skb_share_check(skb, GFP_ATOMIC);
+	if (!skb)
+		goto out;
+
 	if (!pskb_may_pull(skb, sizeof(struct lacpdu)))
 		goto out;
 
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index f4e638c..5c6fba8 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -326,6 +326,10 @@ static int rlb_arp_recv(struct sk_buff *skb, struct net_device *bond_dev, struct
 		goto out;
 	}
 
+	skb = skb_share_check(skb, GFP_ATOMIC);
+	if (!skb)
+		goto out;
+
 	if (!pskb_may_pull(skb, arp_hdr_len(bond_dev)))
 		goto out;
 
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index b1025b8..163e0b0 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2733,6 +2733,10 @@ static int bond_arp_rcv(struct sk_buff *skb, struct net_device *dev, struct pack
 	if (!slave || !slave_do_arp_validate(bond, slave))
 		goto out_unlock;
 
+	skb = skb_share_check(skb, GFP_ATOMIC);
+	if (!skb)
+		goto out_unlock;
+
 	if (!pskb_may_pull(skb, arp_hdr_len(dev)))
 		goto out_unlock;
 
-- 
1.7.3.4


^ permalink raw reply related

* Re: [PATCH v3] net: add Faraday FTMAC100 10/100 Ethernet driver
From: Michał Mirosław @ 2011-01-20 19:01 UTC (permalink / raw)
  To: Po-Yu Chuang
  Cc: netdev, linux-kernel, bhutchings, eric.dumazet, joe, dilinger,
	Po-Yu Chuang
In-Reply-To: <1295537418-2057-1-git-send-email-ratbert.chuang@gmail.com>

2011/1/20 Po-Yu Chuang <ratbert.chuang@gmail.com>:
[...]
> +/******************************************************************************
> + * internal functions (receive descriptor)
> + *****************************************************************************/
> +static bool ftmac100_rxdes_first_segment(struct ftmac100_rxdes *rxdes)
> +{
> +       return le32_to_cpu(rxdes->rxdes0) & FTMAC100_RXDES0_FRS;
> +}
> +
> +static bool ftmac100_rxdes_last_segment(struct ftmac100_rxdes *rxdes)
> +{
> +       return le32_to_cpu(rxdes->rxdes0) & FTMAC100_RXDES0_LRS;
> +}
> +
[...]

You can change these and similar functions to use:

rxdes->rxdes0 & cpu_to_le32(FTMAC100_RXDES0_LRS)

This will be subject to constant folding by compiler and produce
better code for big-endian arches.

Best Regards,
Michał Mirosław

^ permalink raw reply

* Re: [PATCH v3] net: add Faraday FTMAC100 10/100 Ethernet driver
From: Joe Perches @ 2011-01-20 19:00 UTC (permalink / raw)
  To: Po-Yu Chuang
  Cc: netdev, linux-kernel, bhutchings, eric.dumazet, dilinger,
	Po-Yu Chuang
In-Reply-To: <1295537418-2057-1-git-send-email-ratbert.chuang@gmail.com>

On Thu, 2011-01-20 at 23:30 +0800, Po-Yu Chuang wrote:
>  drivers/net/ftmac100.c | 1243 ++++++++++++++++++++++++++++++++++++++++++++++++

[]

> +/******************************************************************************
> + * struct napi_struct functions
> + *****************************************************************************/
> +static int ftmac100_poll(struct napi_struct *napi, int budget)
> +{
> +	struct ftmac100 *priv = container_of(napi, struct ftmac100, napi);
> +	struct net_device *netdev = priv->netdev;
> +	unsigned int status;
> +	bool completed = true;
> +	int rx = 0;
> +
> +	status = ioread32(priv->base + FTMAC100_OFFSET_ISR);
> +
> +	if (status & (FTMAC100_INT_RPKT_FINISH | FTMAC100_INT_NORXBUF)) {
> +		/*
> +		 * FTMAC100_INT_RPKT_FINISH:
> +		 *	RX DMA has received packets into RX buffer successfully
> +		 *
> +		 * FTMAC100_INT_NORXBUF:
> +		 *	RX buffer unavailable
> +		 */
> +		bool retry;
> +
> +		do {
> +			retry = ftmac100_rx_packet(priv, &rx);
> +		} while (retry && rx < budget);
> +
> +		if (retry && rx == budget)
> +			completed = false;

Is it useful to retry the NORXBUF case?

> +	}
> +
> +	if (status & FTMAC100_INT_NORXBUF) {
> +		/* RX buffer unavailable */
> +		if (net_ratelimit())
> +			netdev_info(netdev, "INT_NORXBUF\n");
> +
> +		netdev->stats.rx_over_errors++;
> +	}

Perhaps this "if (status & FTMAC100_INT_NORXBUF)" block should be
moved into the test block above it before the retry?

> +
> +	if (status & (FTMAC100_INT_XPKT_OK | FTMAC100_INT_XPKT_LOST)) {
> +		/*
> +		 * FTMAC100_INT_XPKT_OK:
> +		 *	 packet transmitted to ethernet successfully
> +		 *
> +		 * FTMAC100_INT_XPKT_LOST:
> +		 *	packet transmitted to ethernet lost due to late
> +		 *	collision or excessive collision
> +		 */
> +		ftmac100_tx_complete(priv);
> +	}
> +
> +	if (status & FTMAC100_INT_RPKT_LOST) {
> +		/* received packet lost due to RX FIFO full */
> +		if (net_ratelimit())
> +			netdev_info(netdev, "INT_RPKT_LOST\n");
> +
> +		netdev->stats.rx_fifo_errors++;
> +	}
> +
> +	if (status & FTMAC100_INT_AHB_ERR) {
> +		/* AHB error */
> +		if (net_ratelimit())
> +			netdev_info(netdev, "INT_AHB_ERR\n");
> +
> +		/* do nothing */
> +	}
> +
> +	if (status & FTMAC100_INT_PHYSTS_CHG) {
> +		/* PHY link status change */
> +		if (net_ratelimit())
> +			netdev_info(netdev, "INT_PHYSTS_CHG\n");
> +
> +		mii_check_link(&priv->mii);
> +	}
> +
> +	if (completed) {
> +		/* stop polling */
> +		napi_complete(napi);
> +		ftmac100_enable_all_int(priv);
> +	}
> +
> +	return rx;
> +}

It's possible to miss multiple states because of the ratelimit.

If multiple ISR status bits are possible, it might be better to
combine all netdev_info uses into a single call.

Something like:

	if ((status & (FTMAC100_INT_NORXBUF | FTMAC100_INT_RPKT_LOST |
		       FTMAC100_INT_AHB_ERR | FTMAC100_INT_PHYSTS_CHG)) &&
	    net_ratelimit())
		netdev_info(netdev, "ISR status: %x%s%s%s%s\n",
			    status & FTMAC100_INT_NORXBUF ? ": INT_NORXBUF" : "",
			    status & FTMAC100_INT_RPKT_LOST ? ": INT_RPKT_LOST" : "",
			    status & FTMAC100_INT_AHB_ERR ? ": INT_AHB_ERR" : "",
			    status & FTMAC100_INT_PHYSTS_CHG ? " : INT_PHYSTS_CHG" : "");

^ permalink raw reply

* Re: [PATCH] CHOKe flow scheduler (0.10)
From: Eric Dumazet @ 2011-01-20 18:19 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Patrick McHardy, David Miller, netdev
In-Reply-To: <20110120093838.43f9f9b1@nehalam>

Le jeudi 20 janvier 2011 à 09:38 -0800, Stephen Hemminger a écrit :

...

Hi Stephen

I am contemplating choke_linearize_header() (yet another hash
function... oh well...) and say :

Instead of this boolean return, just use skb_get_rxhash(). It performs
what you want, and return 32bits of information, instead of one bit.

(if result (rxhash) is zero, it means you can drop packet because it is
not linearized)

So choke_match_flow() can be faster if rxhash differs (no cache miss on
skb->data on an old skb)
(Do the full check only if rxhash is equal on skb1 / skb2)

More over, the skb_get_rxhash() call is _really_ needed only if CHOKe
triggers :
if (p->qavg <= p->qth_min)
	we dont have to compute rxhash at all.

> +/* Make sure network and transport headers can be dereferenced */
> +static bool choke_linearize_header(struct sk_buff *skb)
> +{
> +	int nhoff, poff;
> +	struct ipv6hdr *ip6;
> +	struct iphdr *ip;
> +	u8 ip_proto;
> +	u32 ihl;
> +
> +	nhoff = skb_network_offset(skb);
> +
> +	switch (skb->protocol) {
> +	case __constant_htons(ETH_P_IP):
> +		if (!pskb_may_pull(skb, sizeof(*ip) + nhoff))
> +			return false;
> +
> +		ip = (struct iphdr *) (skb->data + nhoff);
> +		if (ip->frag_off & htons(IP_MF | IP_OFFSET))
> +			return false;
> +
> +		ip_proto = ip->protocol;
> +		ihl = ip->ihl;
> +		break;
> +	case __constant_htons(ETH_P_IPV6):
> +		if (!pskb_may_pull(skb, sizeof(*ip6) + nhoff))
> +			return false;
> +
> +		ip6 = (struct ipv6hdr *) (skb->data + nhoff);
> +		ip_proto = ip6->nexthdr;
> +		ihl = (40 >> 2);
> +		break;
> +	default:
> +		return true;
> +	}
> +
> +	poff = proto_ports_offset(ip_proto);
> +	if (poff >= 0) {
> +		nhoff += ihl * 4 + poff;
> +		if (!pskb_may_pull(skb, nhoff + 4))
> +			return false;
> +	}
> +
> +	return true;
> +}
> +


> +/*
> + * Compare flow of two packets
> + *  Returns true only if source and destination address and port match.
> + *          false for special cases
> + */
> +static bool choke_match_flow(const struct sk_buff *skb1,
> +			     const struct sk_buff *skb2)
> +{
> +	int off1, off2, poff;
> +
> +	if (skb1->protocol != skb2->protocol)
> +		return false;
> +
> +	off1 = skb_network_offset(skb1);
> +	off2 = skb_network_offset(skb2);
> +
> +	switch (skb1->protocol) {
> +	case __constant_htons(ETH_P_IP): {
> +		const struct iphdr *ip1, *ip2;
> +
> +		ip1 = (struct iphdr *) (skb1->data + off1);
> +		ip2 = (struct iphdr *) (skb2->data + off2);
> +
> +		if (ip1->protocol != ip2->protocol ||
> +		    ip1->saddr != ip2->saddr || ip1->daddr != ip2->daddr)
> +			return false;
> +
> +		if (ip1->frag_off & htons(IP_MF | IP_OFFSET) ||
> +		    ip2->frag_off & htons(IP_MF | IP_OFFSET))
> +			return ip1->id == ip2->id;
> +
> +		poff = proto_ports_offset(ip1->protocol);
> +		if (poff < 0)
> +			return true;
> +		else {
> +			const u32 *ports1, *ports2;
> +
> +			ports1 = (__force u32 *) (skb1->data + off1
> +						  + ip1->ihl * 4 + poff);
> +			ports2 = (__force u32 *) (skb2->data + off2
> +						  + ip2->ihl * 4 + poff);
> +
> +			return *ports1 == *ports2;
> +		}
> +		break;
> +	}
> +
> +	case __constant_htons(ETH_P_IPV6): {
> +		const struct ipv6hdr *ip1, *ip2;
> +
> +		ip1 = (struct ipv6hdr *) (skb1->data + off1);
> +		ip2 = (struct ipv6hdr *) (skb2->data + off2);
> +
> +		return (ip1->nexthdr == ip2->nexthdr &&
> +			ipv6_addr_cmp(&ip1->saddr, &ip2->saddr) == 0 &&
> +			ipv6_addr_cmp(&ip1->daddr, &ip2->daddr) == 0);

Hmm, we also need to check ports, you might just move the check from
case __constant_htons(ETH_P_IP) after the switch(skb1->protocol)


> +	}
> +
> +	default: /* Maybe compare MAC header here? */
> +		return false;
> +	}
> +	/* not reached */
> +}
> +

If you want, I can send a diff on your v0.10, just tell me !

Thanks !



^ permalink raw reply

* Re: [PATCH v3] net: add Faraday FTMAC100 10/100 Ethernet driver
From: Joe Perches @ 2011-01-20 17:56 UTC (permalink / raw)
  To: Po-Yu Chuang
  Cc: netdev, linux-kernel, bhutchings, eric.dumazet, dilinger,
	Po-Yu Chuang
In-Reply-To: <1295537418-2057-1-git-send-email-ratbert.chuang@gmail.com>

On Thu, 2011-01-20 at 23:30 +0800, Po-Yu Chuang wrote:
> From: Po-Yu Chuang <ratbert@faraday-tech.com>
[]
> +	/* map io memory */
> +	priv->res = request_mem_region(res->start, res->end - res->start,
> +				       dev_name(&pdev->dev));

Off by one?

	priv->res = request_mem_region(res->start, resource_size(res),
				       dev_name(&pdev->dev));

^ permalink raw reply

* [PATCH] netfilter: add a missing include in nf_conntrack_reasm.c
From: Eric Dumazet @ 2011-01-20 17:53 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: Netfilter Development Mailinglist, netdev, KOVACS Krisztian

After commit ae90bdeaeac6b (netfilter: fix compilation when conntrack is
disabled but tproxy is enabled) we have following warnings :

net/ipv6/netfilter/nf_conntrack_reasm.c:520:16: warning: symbol
'nf_ct_frag6_gather' was not declared. Should it be static?
net/ipv6/netfilter/nf_conntrack_reasm.c:591:6: warning: symbol
'nf_ct_frag6_output' was not declared. Should it be static?
net/ipv6/netfilter/nf_conntrack_reasm.c:612:5: warning: symbol
'nf_ct_frag6_init' was not declared. Should it be static?
net/ipv6/netfilter/nf_conntrack_reasm.c:640:6: warning: symbol
'nf_ct_frag6_cleanup' was not declared. Should it be static?

Fix this including net/netfilter/ipv6/nf_defrag_ipv6.h


Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
CC: KOVACS Krisztian <hidden@balabit.hu>
---
 net/ipv6/netfilter/nf_conntrack_reasm.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
index 66e003e..0857272 100644
--- a/net/ipv6/netfilter/nf_conntrack_reasm.c
+++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
@@ -45,6 +45,7 @@
 #include <linux/netfilter_ipv6.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <net/netfilter/ipv6/nf_defrag_ipv6.h>
 
 
 struct nf_ct_frag6_skb_cb



^ permalink raw reply related

* [PATCH] CHOKe flow scheduler (0.10)
From: Stephen Hemminger @ 2011-01-20 17:38 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Patrick McHardy, David Miller, netdev
In-Reply-To: <1295379257.9097.9.camel@edumazet-laptop>

CHOKe ("CHOose and Kill" or "CHOose and Keep") is an alternative
packet scheduler based on the Random Exponential Drop (RED) algorithm.

The core idea is:
  For every packet arrival:
  	Calculate Qave
	if (Qave < minth) 
	     Queue the new packet
	else 
	     Select randomly a packet from the queue 
	     if (both packets from same flow)
	     then Drop both the packets
	     else if (Qave > maxth)
	          Drop packet
	     else
	       	  Admit packet with proability p (same as RED)

See also:
  Rong Pan, Balaji Prabhakar, Konstantinos Psounis, "CHOKe: a stateless active
   queue management scheme for approximating fair bandwidth allocation", 
  Proceeding of INFOCOM'2000, March 2000.

Help from:
     Eric Dumazet <eric.dumazet@gmail.com>
     Patrick McHardy <kaber@trash.net>

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

---
0.10 The internal flow classifier now does full compare of IPv4/IPv6 headers
  to avoid possible DoS attack from hash collision. Internal flow match code
  needs review/testing.

 include/linux/pkt_sched.h |   29 +
 net/sched/Kconfig         |   11 
 net/sched/Makefile        |    1 
 net/sched/sch_choke.c     |  713 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 754 insertions(+)

--- a/net/sched/Kconfig	2011-01-18 09:27:32.224666104 -0800
+++ b/net/sched/Kconfig	2011-01-18 09:27:45.522274178 -0800
@@ -205,6 +205,17 @@ config NET_SCH_DRR
 
 	  If unsure, say N.
 
+config NET_SCH_CHOKE
+	tristate "CHOose and Keep responsive flow scheduler (CHOKE)"
+	help
+	  Say Y here if you want to use the CHOKe packet scheduler (CHOose
+	  and Keep for responsive flows, CHOose and Kill for unresponsive
+	  flows). This is a variation of RED which trys to penalize flows
+	  that monopolize the queue.
+
+	  To compile this code as a module, choose M here: the
+	  module will be called sch_choke.
+
 config NET_SCH_INGRESS
 	tristate "Ingress Qdisc"
 	depends on NET_CLS_ACT
--- a/net/sched/Makefile	2011-01-18 09:27:32.232666939 -0800
+++ b/net/sched/Makefile	2011-01-18 09:27:45.526274731 -0800
@@ -32,6 +32,7 @@ obj-$(CONFIG_NET_SCH_MULTIQ)	+= sch_mult
 obj-$(CONFIG_NET_SCH_ATM)	+= sch_atm.o
 obj-$(CONFIG_NET_SCH_NETEM)	+= sch_netem.o
 obj-$(CONFIG_NET_SCH_DRR)	+= sch_drr.o
+obj-$(CONFIG_NET_SCH_CHOKE)	+= sch_choke.o
 obj-$(CONFIG_NET_CLS_U32)	+= cls_u32.o
 obj-$(CONFIG_NET_CLS_ROUTE4)	+= cls_route.o
 obj-$(CONFIG_NET_CLS_FW)	+= cls_fw.o
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ b/net/sched/sch_choke.c	2011-01-20 08:33:06.021631871 -0800
@@ -0,0 +1,713 @@
+/*
+ * net/sched/sch_choke.c	CHOKE scheduler
+ *
+ * Copyright (c) 2011 Stephen Hemminger <shemminger@vyatta.com>
+ * Copyright (c) 2011 Eric Dumazet <eric.dumazet@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/skbuff.h>
+#include <linux/reciprocal_div.h>
+#include <net/pkt_sched.h>
+#include <net/inet_ecn.h>
+#include <net/red.h>
+#include <linux/ip.h>
+#include <net/ip.h>
+#include <linux/ipv6.h>
+#include <net/ipv6.h>
+
+/*
+   CHOKe stateless AQM for fair bandwidth allocation
+   =================================================
+
+   CHOKe (CHOose and Keep for responsive flows, CHOose and Kill for
+   unresponsive flows) is a variant of RED that penalizes misbehaving flows but
+   maintains no flow state. The difference from RED is an additional step
+   during the enqueuing process. If average queue size is over the
+   low threshold (qmin), a packet is chosen at random from the queue.
+   If both the new and chosen packet are from the same flow, both
+   are dropped. Unlike RED, CHOKe is not really a "classful" qdisc because it
+   needs to access packets in queue randomly. It has a minimal class
+   interface to allow overriding the builtin flow classifier with
+   filters.
+
+   Source:
+   R. Pan, B. Prabhakar, and K. Psounis, "CHOKe, A Stateless
+   Active Queue Management Scheme for Approximating Fair Bandwidth Allocation",
+   IEEE INFOCOM, 2000.
+
+   A. Tang, J. Wang, S. Low, "Understanding CHOKe: Throughput and Spatial
+   Characteristics", IEEE/ACM Transactions on Networking, 2004
+
+ */
+
+/* Upper bound on size of sk_buff table (packets) */
+#define CHOKE_MAX_QUEUE	(128*1024 - 1)
+
+struct choke_sched_data {
+/* Parameters */
+	u32		 limit;
+	unsigned char	 flags;
+
+	struct red_parms parms;
+
+/* Variables */
+	struct tcf_proto *filter_list;
+	struct {
+		u32	prob_drop;	/* Early probability drops */
+		u32	prob_mark;	/* Early probability marks */
+		u32	forced_drop;	/* Forced drops, qavg > max_thresh */
+		u32	forced_mark;	/* Forced marks, qavg > max_thresh */
+		u32	pdrop;          /* Drops due to queue limits */
+		u32	other;          /* Drops due to drop() calls */
+		u32	matched;	/* Drops to flow match */
+	} stats;
+
+	unsigned int	 head;
+	unsigned int	 tail;
+
+	unsigned int	 tab_mask; /* size - 1 */
+
+	struct sk_buff **tab;
+};
+
+/* deliver a random number between 0 and N - 1 */
+static u32 random_N(unsigned int N)
+{
+	return reciprocal_divide(random32(), N);
+}
+
+/* number of elements in queue including holes */
+static unsigned int choke_len(const struct choke_sched_data *q)
+{
+	return (q->tail - q->head) & q->tab_mask;
+}
+
+/* Is ECN parameter configured */
+static int use_ecn(const struct choke_sched_data *q)
+{
+	return q->flags & TC_RED_ECN;
+}
+
+/* Should packets over max just be dropped (versus marked) */
+static int use_harddrop(const struct choke_sched_data *q)
+{
+	return q->flags & TC_RED_HARDDROP;
+}
+
+/* Move head pointer forward to skip over holes */
+static void choke_zap_head_holes(struct choke_sched_data *q)
+{
+	do {
+		q->head = (q->head + 1) & q->tab_mask;
+		if (q->head == q->tail)
+			break;
+	} while (q->tab[q->head] == NULL);
+}
+
+/* Move tail pointer backwards to reuse holes */
+static void choke_zap_tail_holes(struct choke_sched_data *q)
+{
+	do {
+		q->tail = (q->tail - 1) & q->tab_mask;
+		if (q->head == q->tail)
+			break;
+	} while (q->tab[q->tail] == NULL);
+}
+
+/* Drop packet from queue array by creating a "hole" */
+static void choke_drop_by_idx(struct Qdisc *sch, unsigned int idx)
+{
+	struct choke_sched_data *q = qdisc_priv(sch);
+	struct sk_buff *skb = q->tab[idx];
+
+	q->tab[idx] = NULL;
+
+	if (idx == q->head)
+		choke_zap_head_holes(q);
+	if (idx == q->tail)
+		choke_zap_tail_holes(q);
+
+	sch->qstats.backlog -= qdisc_pkt_len(skb);
+	qdisc_drop(skb, sch);
+	qdisc_tree_decrease_qlen(sch, 1);
+	--sch->q.qlen;
+}
+
+/* Make sure network and transport headers can be dereferenced */
+static bool choke_linearize_header(struct sk_buff *skb)
+{
+	int nhoff, poff;
+	struct ipv6hdr *ip6;
+	struct iphdr *ip;
+	u8 ip_proto;
+	u32 ihl;
+
+	nhoff = skb_network_offset(skb);
+
+	switch (skb->protocol) {
+	case __constant_htons(ETH_P_IP):
+		if (!pskb_may_pull(skb, sizeof(*ip) + nhoff))
+			return false;
+
+		ip = (struct iphdr *) (skb->data + nhoff);
+		if (ip->frag_off & htons(IP_MF | IP_OFFSET))
+			return false;
+
+		ip_proto = ip->protocol;
+		ihl = ip->ihl;
+		break;
+	case __constant_htons(ETH_P_IPV6):
+		if (!pskb_may_pull(skb, sizeof(*ip6) + nhoff))
+			return false;
+
+		ip6 = (struct ipv6hdr *) (skb->data + nhoff);
+		ip_proto = ip6->nexthdr;
+		ihl = (40 >> 2);
+		break;
+	default:
+		return true;
+	}
+
+	poff = proto_ports_offset(ip_proto);
+	if (poff >= 0) {
+		nhoff += ihl * 4 + poff;
+		if (!pskb_may_pull(skb, nhoff + 4))
+			return false;
+	}
+
+	return true;
+}
+
+/*
+ * Compare flow of two packets
+ *  Returns true only if source and destination address and port match.
+ *          false for special cases
+ */
+static bool choke_match_flow(const struct sk_buff *skb1,
+			     const struct sk_buff *skb2)
+{
+	int off1, off2, poff;
+
+	if (skb1->protocol != skb2->protocol)
+		return false;
+
+	off1 = skb_network_offset(skb1);
+	off2 = skb_network_offset(skb2);
+
+	switch (skb1->protocol) {
+	case __constant_htons(ETH_P_IP): {
+		const struct iphdr *ip1, *ip2;
+
+		ip1 = (struct iphdr *) (skb1->data + off1);
+		ip2 = (struct iphdr *) (skb2->data + off2);
+
+		if (ip1->protocol != ip2->protocol ||
+		    ip1->saddr != ip2->saddr || ip1->daddr != ip2->daddr)
+			return false;
+
+		if (ip1->frag_off & htons(IP_MF | IP_OFFSET) ||
+		    ip2->frag_off & htons(IP_MF | IP_OFFSET))
+			return ip1->id == ip2->id;
+
+		poff = proto_ports_offset(ip1->protocol);
+		if (poff < 0)
+			return true;
+		else {
+			const u32 *ports1, *ports2;
+
+			ports1 = (__force u32 *) (skb1->data + off1
+						  + ip1->ihl * 4 + poff);
+			ports2 = (__force u32 *) (skb2->data + off2
+						  + ip2->ihl * 4 + poff);
+
+			return *ports1 == *ports2;
+		}
+		break;
+	}
+
+	case __constant_htons(ETH_P_IPV6): {
+		const struct ipv6hdr *ip1, *ip2;
+
+		ip1 = (struct ipv6hdr *) (skb1->data + off1);
+		ip2 = (struct ipv6hdr *) (skb2->data + off2);
+
+		return (ip1->nexthdr == ip2->nexthdr &&
+			ipv6_addr_cmp(&ip1->saddr, &ip2->saddr) == 0 &&
+			ipv6_addr_cmp(&ip1->daddr, &ip2->daddr) == 0);
+	}
+
+	default: /* Maybe compare MAC header here? */
+		return false;
+	}
+	/* not reached */
+}
+
+static inline void choke_set_classid(struct sk_buff *skb, u16 classid)
+{
+	*(unsigned int *)(qdisc_skb_cb(skb)->data) = classid;
+}
+
+static u16 choke_get_classid(const struct sk_buff *skb)
+{
+	return *(unsigned int *)(qdisc_skb_cb(skb)->data);
+}
+
+/*
+ * Classify flow using either:
+ *  1. pre-existing classification result in skb
+ *  2. fast internal classification
+ *  3. use TC filter based classification
+ */
+static bool choke_classify(struct sk_buff *skb,
+			   struct Qdisc *sch, int *qerr)
+
+{
+	struct choke_sched_data *q = qdisc_priv(sch);
+	struct tcf_result res;
+	int result;
+
+	result = tc_classify(skb, q->filter_list, &res);
+	if (result >= 0) {
+#ifdef CONFIG_NET_CLS_ACT
+		switch (result) {
+		case TC_ACT_STOLEN:
+		case TC_ACT_QUEUED:
+			*qerr = NET_XMIT_SUCCESS | __NET_XMIT_STOLEN;
+		case TC_ACT_SHOT:
+			return false;
+		}
+#endif
+		choke_set_classid(skb, TC_H_MIN(res.classid));
+		return true;
+	}
+
+	return false;
+}
+
+/*
+ * Select a packet at random from queue
+ * HACK: since queue can have holes from previous deletion; retry several
+ *   times to find a random skb but then just give up and return the head
+ * Will return NULL if queue is empty (q->head == q->tail)
+ */
+static struct sk_buff *choke_peek_random(const struct choke_sched_data *q,
+					 unsigned int *pidx)
+{
+	struct sk_buff *skb;
+	int retrys = 3;
+
+	do {
+		*pidx = (q->head + random_N(choke_len(q))) & q->tab_mask;
+		skb = q->tab[*pidx];
+		if (skb)
+			return skb;
+	} while (--retrys > 0);
+
+	return q->tab[*pidx = q->head];
+}
+
+/*
+ * Compare new packet with random packet in queue
+ * returns true if matched and sets *pidx
+ */
+static bool choke_match_random(const struct choke_sched_data *q,
+			       const struct sk_buff *nskb,
+			       unsigned int *pidx)
+{
+	const struct sk_buff *oskb;
+
+	if (q->head == q->tail)
+		return false;
+
+	oskb = choke_peek_random(q, pidx);
+	if (q->filter_list)
+		return choke_get_classid(nskb) == choke_get_classid(oskb);
+
+
+	return choke_match_flow(oskb, nskb);
+}
+
+static int choke_enqueue(struct sk_buff *skb, struct Qdisc *sch)
+{
+	struct choke_sched_data *q = qdisc_priv(sch);
+	struct red_parms *p = &q->parms;
+	int ret = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
+
+	if (q->filter_list) {
+		/* If using external classifiers, get result and record it. */
+		if (!choke_classify(skb, sch, &ret))
+			goto other_drop;	/* Packet was eaten by filter */
+	} else {
+		/* for internal classifier, make header accessible */
+		if (!choke_linearize_header(skb))
+			goto other_drop;
+	}
+
+	/* Compute average queue usage (see RED) */
+	p->qavg = red_calc_qavg(p, sch->q.qlen);
+	if (red_is_idling(p))
+		red_end_of_idle_period(p);
+
+	/* Is queue small? */
+	if (p->qavg <= p->qth_min)
+		p->qcount = -1;
+	else {
+		unsigned int idx;
+
+		/* Draw a packet at random from queue and compare flow */
+		if (choke_match_random(q, skb, &idx)) {
+			q->stats.matched++;
+			choke_drop_by_idx(sch, idx);
+			goto congestion_drop;
+		}
+
+		/* Queue is large, always mark/drop */
+		if (p->qavg > p->qth_max) {
+			p->qcount = -1;
+
+			sch->qstats.overlimits++;
+			if (use_harddrop(q) || !use_ecn(q) ||
+			    !INET_ECN_set_ce(skb)) {
+				q->stats.forced_drop++;
+				goto congestion_drop;
+			}
+
+			q->stats.forced_mark++;
+		} else if (++p->qcount) {
+			if (red_mark_probability(p, p->qavg)) {
+				p->qcount = 0;
+				p->qR = red_random(p);
+
+				sch->qstats.overlimits++;
+				if (!use_ecn(q) || !INET_ECN_set_ce(skb)) {
+					q->stats.prob_drop++;
+					goto congestion_drop;
+				}
+
+				q->stats.prob_mark++;
+			}
+		} else
+			p->qR = red_random(p);
+	}
+
+	/* Admit new packet */
+	if (sch->q.qlen < q->limit) {
+		q->tab[q->tail] = skb;
+		q->tail = (q->tail + 1) & q->tab_mask;
+		++sch->q.qlen;
+		sch->qstats.backlog += qdisc_pkt_len(skb);
+		return NET_XMIT_SUCCESS;
+	}
+
+	q->stats.pdrop++;
+	sch->qstats.drops++;
+	kfree_skb(skb);
+	return NET_XMIT_DROP;
+
+ congestion_drop:
+	qdisc_drop(skb, sch);
+	return NET_XMIT_CN;
+
+ other_drop:
+	if (ret & __NET_XMIT_BYPASS)
+		sch->qstats.drops++;
+	kfree_skb(skb);
+	return ret;
+}
+
+static struct sk_buff *choke_dequeue(struct Qdisc *sch)
+{
+	struct choke_sched_data *q = qdisc_priv(sch);
+	struct sk_buff *skb;
+
+	if (q->head == q->tail) {
+		if (!red_is_idling(&q->parms))
+			red_start_of_idle_period(&q->parms);
+		return NULL;
+	}
+
+	skb = q->tab[q->head];
+	q->tab[q->head] = NULL;
+	choke_zap_head_holes(q);
+	--sch->q.qlen;
+	sch->qstats.backlog -= qdisc_pkt_len(skb);
+	qdisc_bstats_update(sch, skb);
+
+	return skb;
+}
+
+static unsigned int choke_drop(struct Qdisc *sch)
+{
+	struct choke_sched_data *q = qdisc_priv(sch);
+	unsigned int len;
+
+	len = qdisc_queue_drop(sch);
+	if (len > 0)
+		q->stats.other++;
+	else {
+		if (!red_is_idling(&q->parms))
+			red_start_of_idle_period(&q->parms);
+	}
+
+	return len;
+}
+
+static void choke_reset(struct Qdisc *sch)
+{
+	struct choke_sched_data *q = qdisc_priv(sch);
+
+	red_restart(&q->parms);
+}
+
+static const struct nla_policy choke_policy[TCA_CHOKE_MAX + 1] = {
+	[TCA_CHOKE_PARMS]	= { .len = sizeof(struct tc_red_qopt) },
+	[TCA_CHOKE_STAB]	= { .len = RED_STAB_SIZE },
+};
+
+
+static void choke_free(void *addr)
+{
+	if (addr) {
+		if (is_vmalloc_addr(addr))
+			vfree(addr);
+		else
+			kfree(addr);
+	}
+}
+
+static int choke_change(struct Qdisc *sch, struct nlattr *opt)
+{
+	struct choke_sched_data *q = qdisc_priv(sch);
+	struct nlattr *tb[TCA_CHOKE_MAX + 1];
+	const struct tc_red_qopt *ctl;
+	int err;
+	struct sk_buff **old = NULL;
+	unsigned int mask;
+
+	if (opt == NULL)
+		return -EINVAL;
+
+	err = nla_parse_nested(tb, TCA_CHOKE_MAX, opt, choke_policy);
+	if (err < 0)
+		return err;
+
+	if (tb[TCA_CHOKE_PARMS] == NULL ||
+	    tb[TCA_CHOKE_STAB] == NULL)
+		return -EINVAL;
+
+	ctl = nla_data(tb[TCA_CHOKE_PARMS]);
+
+	if (ctl->limit > CHOKE_MAX_QUEUE)
+		return -EINVAL;
+
+	mask = roundup_pow_of_two(ctl->limit + 1) - 1;
+	if (mask != q->tab_mask) {
+		struct sk_buff **ntab;
+
+		ntab = kcalloc(mask + 1, sizeof(struct sk_buff *), GFP_KERNEL);
+		if (!ntab)
+			ntab = vzalloc((mask + 1) * sizeof(struct sk_buff *));
+		if (!ntab)
+			return -ENOMEM;
+
+		sch_tree_lock(sch);
+		old = q->tab;
+		if (old) {
+			unsigned int oqlen = sch->q.qlen, tail = 0;
+
+			while (q->head != q->tail) {
+				struct sk_buff *skb = q->tab[q->head];
+
+				q->head = (q->head + 1) & q->tab_mask;
+				if (!skb)
+					continue;
+				if (tail < mask) {
+					ntab[tail++] = skb;
+					continue;
+				}
+				sch->qstats.backlog -= qdisc_pkt_len(skb);
+				--sch->q.qlen;
+				qdisc_drop(skb, sch);
+			}
+			qdisc_tree_decrease_qlen(sch, oqlen - sch->q.qlen);
+			q->head = 0;
+			q->tail = tail;
+		}
+
+		q->tab_mask = mask;
+		q->tab = ntab;
+	} else
+		sch_tree_lock(sch);
+
+	q->flags = ctl->flags;
+	q->limit = ctl->limit;
+
+	red_set_parms(&q->parms, ctl->qth_min, ctl->qth_max, ctl->Wlog,
+		      ctl->Plog, ctl->Scell_log,
+		      nla_data(tb[TCA_CHOKE_STAB]));
+
+	if (q->head == q->tail)
+		red_end_of_idle_period(&q->parms);
+
+	sch_tree_unlock(sch);
+	choke_free(old);
+	return 0;
+}
+
+static int choke_init(struct Qdisc *sch, struct nlattr *opt)
+{
+	return choke_change(sch, opt);
+}
+
+static int choke_dump(struct Qdisc *sch, struct sk_buff *skb)
+{
+	struct choke_sched_data *q = qdisc_priv(sch);
+	struct nlattr *opts = NULL;
+	struct tc_red_qopt opt = {
+		.limit		= q->limit,
+		.flags		= q->flags,
+		.qth_min	= q->parms.qth_min >> q->parms.Wlog,
+		.qth_max	= q->parms.qth_max >> q->parms.Wlog,
+		.Wlog		= q->parms.Wlog,
+		.Plog		= q->parms.Plog,
+		.Scell_log	= q->parms.Scell_log,
+	};
+
+	opts = nla_nest_start(skb, TCA_OPTIONS);
+	if (opts == NULL)
+		goto nla_put_failure;
+
+	NLA_PUT(skb, TCA_CHOKE_PARMS, sizeof(opt), &opt);
+	return nla_nest_end(skb, opts);
+
+nla_put_failure:
+	nla_nest_cancel(skb, opts);
+	return -EMSGSIZE;
+}
+
+static int choke_dump_stats(struct Qdisc *sch, struct gnet_dump *d)
+{
+	struct choke_sched_data *q = qdisc_priv(sch);
+	struct tc_choke_xstats st = {
+		.early	= q->stats.prob_drop + q->stats.forced_drop,
+		.marked	= q->stats.prob_mark + q->stats.forced_mark,
+		.pdrop	= q->stats.pdrop,
+		.other	= q->stats.other,
+		.matched = q->stats.matched,
+	};
+
+	return gnet_stats_copy_app(d, &st, sizeof(st));
+}
+
+static void choke_destroy(struct Qdisc *sch)
+{
+	struct choke_sched_data *q = qdisc_priv(sch);
+
+	tcf_destroy_chain(&q->filter_list);
+	choke_free(q->tab);
+}
+
+static struct Qdisc *choke_leaf(struct Qdisc *sch, unsigned long arg)
+{
+	return NULL;
+}
+
+static unsigned long choke_get(struct Qdisc *sch, u32 classid)
+{
+	return 0;
+}
+
+static void choke_put(struct Qdisc *q, unsigned long cl)
+{
+}
+
+static unsigned long choke_bind(struct Qdisc *sch, unsigned long parent,
+				u32 classid)
+{
+	return 0;
+}
+
+static struct tcf_proto **choke_find_tcf(struct Qdisc *sch, unsigned long cl)
+{
+	struct choke_sched_data *q = qdisc_priv(sch);
+
+	if (cl)
+		return NULL;
+	return &q->filter_list;
+}
+
+static int choke_dump_class(struct Qdisc *sch, unsigned long cl,
+			  struct sk_buff *skb, struct tcmsg *tcm)
+{
+	tcm->tcm_handle |= TC_H_MIN(cl);
+	return 0;
+}
+
+static void choke_walk(struct Qdisc *sch, struct qdisc_walker *arg)
+{
+	if (!arg->stop) {
+		if (arg->fn(sch, 1, arg) < 0) {
+			arg->stop = 1;
+			return;
+		}
+		arg->count++;
+	}
+}
+
+static const struct Qdisc_class_ops choke_class_ops = {
+	.leaf		=	choke_leaf,
+	.get		=	choke_get,
+	.put		=	choke_put,
+	.tcf_chain	=	choke_find_tcf,
+	.bind_tcf	=	choke_bind,
+	.unbind_tcf	=	choke_put,
+	.dump		=	choke_dump_class,
+	.walk		=	choke_walk,
+};
+
+static struct sk_buff *choke_peek_head(struct Qdisc *sch)
+{
+	struct choke_sched_data *q = qdisc_priv(sch);
+
+	return (q->head != q->tail) ? q->tab[q->head] : NULL;
+}
+
+static struct Qdisc_ops choke_qdisc_ops __read_mostly = {
+	.id		=	"choke",
+	.priv_size	=	sizeof(struct choke_sched_data),
+
+	.enqueue	=	choke_enqueue,
+	.dequeue	=	choke_dequeue,
+	.peek		=	choke_peek_head,
+	.drop		=	choke_drop,
+	.init		=	choke_init,
+	.destroy	=	choke_destroy,
+	.reset		=	choke_reset,
+	.change		=	choke_change,
+	.dump		=	choke_dump,
+	.dump_stats	=	choke_dump_stats,
+	.owner		=	THIS_MODULE,
+};
+
+static int __init choke_module_init(void)
+{
+	return register_qdisc(&choke_qdisc_ops);
+}
+
+static void __exit choke_module_exit(void)
+{
+	unregister_qdisc(&choke_qdisc_ops);
+}
+
+module_init(choke_module_init)
+module_exit(choke_module_exit)
+
+MODULE_LICENSE("GPL");
--- a/include/linux/pkt_sched.h	2011-01-18 09:27:32.252669307 -0800
+++ b/include/linux/pkt_sched.h	2011-01-18 09:27:45.526274731 -0800
@@ -247,6 +247,35 @@ struct tc_gred_sopt {
 	__u16		pad1;
 };
 
+/* CHOKe section */
+
+enum {
+	TCA_CHOKE_UNSPEC,
+	TCA_CHOKE_PARMS,
+	TCA_CHOKE_STAB,
+	__TCA_CHOKE_MAX,
+};
+
+#define TCA_CHOKE_MAX (__TCA_CHOKE_MAX - 1)
+
+struct tc_choke_qopt {
+	__u32		limit;		/* Hard queue length (packets)	*/
+	__u32		qth_min;	/* Min average threshold (packets) */
+	__u32		qth_max;	/* Max average threshold (packets) */
+	unsigned char   Wlog;		/* log(W)		*/
+	unsigned char   Plog;		/* log(P_max/(qth_max-qth_min))	*/
+	unsigned char   Scell_log;	/* cell size for idle damping */
+	unsigned char	flags;		/* see RED flags */
+};
+
+struct tc_choke_xstats {
+	__u32		early;          /* Early drops */
+	__u32		pdrop;          /* Drops due to queue limits */
+	__u32		other;          /* Drops due to drop() calls */
+	__u32		marked;         /* Marked packets */
+	__u32		matched;	/* Drops due to flow match */
+};
+
 /* HTB section */
 #define TC_HTB_NUMPRIO		8
 #define TC_HTB_MAXDEPTH		8

^ permalink raw reply

* [PATCH] ipv6: raw: rcu annotations
From: Eric Dumazet @ 2011-01-20 17:37 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

Remove sparse warnings, using a function typedef to be able to use __rcu
annotation on mh_filter pointer.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 net/ipv6/raw.c |   14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c
index 86c3952..2bc6cd7 100644
--- a/net/ipv6/raw.c
+++ b/net/ipv6/raw.c
@@ -123,18 +123,18 @@ static __inline__ int icmpv6_filter(struct sock *sk, struct sk_buff *skb)
 }
 
 #if defined(CONFIG_IPV6_MIP6) || defined(CONFIG_IPV6_MIP6_MODULE)
-static int (*mh_filter)(struct sock *sock, struct sk_buff *skb);
+typedef int mh_filter_t(struct sock *sock, struct sk_buff *skb);
 
-int rawv6_mh_filter_register(int (*filter)(struct sock *sock,
-					   struct sk_buff *skb))
+static mh_filter_t __rcu *mh_filter __read_mostly;
+
+int rawv6_mh_filter_register(mh_filter_t filter)
 {
 	rcu_assign_pointer(mh_filter, filter);
 	return 0;
 }
 EXPORT_SYMBOL(rawv6_mh_filter_register);
 
-int rawv6_mh_filter_unregister(int (*filter)(struct sock *sock,
-					     struct sk_buff *skb))
+int rawv6_mh_filter_unregister(mh_filter_t filter)
 {
 	rcu_assign_pointer(mh_filter, NULL);
 	synchronize_rcu();
@@ -192,10 +192,10 @@ static int ipv6_raw_deliver(struct sk_buff *skb, int nexthdr)
 			 * policy is placed in rawv6_rcv() because it is
 			 * required for each socket.
 			 */
-			int (*filter)(struct sock *sock, struct sk_buff *skb);
+			mh_filter_t *filter;
 
 			filter = rcu_dereference(mh_filter);
-			filtered = filter ? filter(sk, skb) : 0;
+			filtered = filter ? (*filter)(sk, skb) : 0;
 			break;
 		}
 #endif



^ permalink raw reply related

* Re: Question on RX packet classification in ethtool userspace
From: Alexander H Duyck @ 2011-01-20 17:24 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: David Miller, santwona.behera@sun.com, jeff@garzik.org,
	netdev@vger.kernel.org
In-Reply-To: <1295479169.2906.53.camel@localhost>

On Wed, 2011-01-19 at 15:19 -0800, Ben Hutchings wrote:
> On Wed, 2011-01-19 at 14:52 -0800, Duyck, Alexander H wrote:
> > So I was looking into the option of replacing the ETHTOOL_GRXNTUPLE
> > call with something like ETHTOOL_GRXCLSRLALL and it seems like I
> > wasn't having much luck finding the userspace implementation in the
> > ethtool.
> 
> I wasn't aware until now that there was a public implementation!
> 
> > I eventually found a patch in patchwork at
> > http://patchwork.ozlabs.org/patch/23223/ which is supposed to add
> > support but the current ethtool git tree doesn't appear to contain
> > this code.  I was wondering if I am missing something and it is on a
> > branch somewhere, or is this something that was not applied to the
> > userspace for some specific reason?
> 
> Thanks for digging this up.  It's not in any branch that I know of.  I'm
> about to go on vacation, but I'll look at it when I get back.  I would
> appreciate it if someone would refresh the patch against current ethtool
> (preferably reusing some of the functions added for the RX n-tuple
> operations).
> 
> Ben.
> 

I'll look into updating the patch.

Thanks,

Alex


^ permalink raw reply

* [PATCH] net: ipv6: sit: fix rcu annotations
From: Eric Dumazet @ 2011-01-20 17:16 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

Fix minor __rcu annotations and remove sparse warnings

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 net/ipv6/sit.c |   23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index 8ce38f1..b1599a3 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -412,7 +412,7 @@ static void prl_list_destroy_rcu(struct rcu_head *head)
 
 	p = container_of(head, struct ip_tunnel_prl_entry, rcu_head);
 	do {
-		n = p->next;
+		n = rcu_dereference_protected(p->next, 1);
 		kfree(p);
 		p = n;
 	} while (p);
@@ -421,15 +421,17 @@ static void prl_list_destroy_rcu(struct rcu_head *head)
 static int
 ipip6_tunnel_del_prl(struct ip_tunnel *t, struct ip_tunnel_prl *a)
 {
-	struct ip_tunnel_prl_entry *x, **p;
+	struct ip_tunnel_prl_entry *x;
+	struct ip_tunnel_prl_entry __rcu **p;
 	int err = 0;
 
 	ASSERT_RTNL();
 
 	if (a && a->addr != htonl(INADDR_ANY)) {
-		for (p = &t->prl; *p; p = &(*p)->next) {
-			if ((*p)->addr == a->addr) {
-				x = *p;
+		for (p = &t->prl;
+		     (x = rtnl_dereference(*p)) != NULL;
+		     p = &x->next) {
+			if (x->addr == a->addr) {
 				*p = x->next;
 				call_rcu(&x->rcu_head, prl_entry_destroy_rcu);
 				t->prl_count--;
@@ -438,9 +440,9 @@ ipip6_tunnel_del_prl(struct ip_tunnel *t, struct ip_tunnel_prl *a)
 		}
 		err = -ENXIO;
 	} else {
-		if (t->prl) {
+		x = rtnl_dereference(t->prl);
+		if (x) {
 			t->prl_count = 0;
-			x = t->prl;
 			call_rcu(&x->rcu_head, prl_list_destroy_rcu);
 			t->prl = NULL;
 		}
@@ -1179,7 +1181,7 @@ static int __net_init ipip6_fb_tunnel_init(struct net_device *dev)
 	if (!dev->tstats)
 		return -ENOMEM;
 	dev_hold(dev);
-	sitn->tunnels_wc[0]	= tunnel;
+	rcu_assign_pointer(sitn->tunnels_wc[0], tunnel);
 	return 0;
 }
 
@@ -1196,11 +1198,12 @@ static void __net_exit sit_destroy_tunnels(struct sit_net *sitn, struct list_hea
 	for (prio = 1; prio < 4; prio++) {
 		int h;
 		for (h = 0; h < HASH_SIZE; h++) {
-			struct ip_tunnel *t = sitn->tunnels[prio][h];
+			struct ip_tunnel *t;
 
+			t = rtnl_dereference(sitn->tunnels[prio][h]);
 			while (t != NULL) {
 				unregister_netdevice_queue(t->dev, head);
-				t = t->next;
+				t = rtnl_dereference(t->next);
 			}
 		}
 	}



^ permalink raw reply related

* Re: [PATCH net-next-2.6] net_sched: RCU conversion of stab
From: Eric Dumazet @ 2011-01-20 16:39 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: David Miller, netdev, Jesper Dangaard Brouer, Jarek Poplawski,
	jamal
In-Reply-To: <4D384E2D.5050001@trash.net>

Le jeudi 20 janvier 2011 à 16:01 +0100, Patrick McHardy a écrit :
> Am 20.01.2011 14:48, schrieb Eric Dumazet:
> > This patch converts stab qdisc management to RCU, so that we can perform
> > the qdisc_calculate_pkt_len() call before getting qdisc lock.
> > 
> > This shortens the lock's held time in __dev_xmit_skb().
> > 
> > This permits more qdiscs to get TCQ_F_CAN_BYPASS status, avoiding lot of
> > cache misses and so reducing latencies.
> > 
> 
> Looks good to me.
> 

Thanks for reviewing Patrick !



^ permalink raw reply

* Re: Bonding on bond
From: Nicolas de Pesloüan @ 2011-01-20 16:12 UTC (permalink / raw)
  To: Jiri Bohac
  Cc: Jay Vosburgh, bonding-devel@lists.sourceforge.net,
	netdev@vger.kernel.org
In-Reply-To: <20110120153110.GA3931@midget.suse.cz>

Le 20/01/2011 16:31, Jiri Bohac a écrit :
> On Wed, Jan 19, 2011 at 09:33:19PM +0100, Nicolas de Pesloüan wrote:
>> Even if it is possible to test for slave and for master with a
>> single condition (IFF_BONDING), I suggest to split the tests and the
>> error messages, to give end user the best possible diagnostic.
>
> OK, why not. The below patch still uses IFF_BONDING to detect a
> master is being enslaved, because IFF_MASTER is also used by the
> eql driver. No idea if it works / someone ever uses it with
> bonding, but it might collide.

Thanks Jiri.

> bonding: prohibit enslaving of bonding masters
>
> Nested bonding is not supported and will result in strange problems, e.g.:
> - netif_receive_skb() will not properly change skb->dev to point to the
>    uppoer-most bonding master
> - arp monitor will not work (dev->last_rx is only updated by hardware drivers)
> - accidentally enslaving a bonding master to itself will cause an infinite
>    recursion in the TX path
>
> This patch prevents this by prohibiting a bonding master from being further enslaved.
>
> Signed-off-by: Jiri Bohac<jbohac@suse.cz>

Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>

> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index b1025b8..b117dd8 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1453,6 +1453,12 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
>   		return -EBUSY;
>   	}
>
> +	/* cannot enslave a master */
> +	if (slave_dev->priv_flags&  IFF_BONDING) {
> +		pr_debug("Error, cannot enslave a bonding master\n");
> +		return -EBUSY;
> +	}
> +
>   	/* vlan challenged mutual exclusion */
>   	/* no need to lock since we're protected by rtnl_lock */
>   	if (slave_dev->features&  NETIF_F_VLAN_CHALLENGED) {
>


^ permalink raw reply

* Re: [PATCH v3] net: add Faraday FTMAC100 10/100 Ethernet driver
From: Po-Yu Chuang @ 2011-01-20 15:54 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: netdev, linux-kernel, bhutchings, joe, dilinger, Po-Yu Chuang
In-Reply-To: <1295538105.2825.308.camel@edumazet-laptop>

Dear Eric,

On Thu, Jan 20, 2011 at 11:41 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le jeudi 20 janvier 2011 à 23:30 +0800, Po-Yu Chuang a écrit :
>
>> +static bool ftmac100_tx_complete_packet(struct ftmac100 *priv)
>> +{
>> +     struct net_device *netdev = priv->netdev;
>> +     struct ftmac100_txdes *txdes;
>> +     struct sk_buff *skb;
>> +     dma_addr_t map;
>> +
>> +     if (priv->tx_pending == 0)
>> +             return false;
>> +
>> +     txdes = ftmac100_current_clean_txdes(priv);
>> +
>> +     if (ftmac100_txdes_owned_by_dma(txdes))
>> +             return false;
>> +
>> +     skb = ftmac100_txdes_get_skb(txdes);
>> +     map = ftmac100_txdes_get_dma_addr(txdes);
>> +
>> +     if (unlikely(ftmac100_txdes_excessive_collision(txdes) ||
>> +                  ftmac100_txdes_late_collision(txdes))) {
>> +             /*
>> +              * packet transmitted to ethernet lost due to late collision
>> +              * or excessive collision
>> +              */
>> +             netdev->stats.tx_aborted_errors++;
>> +     } else {
>> +             netdev->stats.tx_packets++;
>> +             netdev->stats.tx_bytes += skb->len;
>> +     }
>> +
>> +     dma_unmap_single(priv->dev, map, skb_headlen(skb), DMA_TO_DEVICE);
>> +
>> +     dev_kfree_skb_irq(skb);
>> +
>> +     ftmac100_txdes_reset(txdes);
>> +
>> +     ftmac100_tx_clean_pointer_advance(priv);
>> +
>> +     priv->tx_pending--;
>> +     netif_wake_queue(netdev);
>> +
>> +     return true;
>> +}
>> +
>> +static void ftmac100_tx_complete(struct ftmac100 *priv)
>> +{
>> +     unsigned long flags;
>> +
>> +     spin_lock_irqsave(&priv->tx_lock, flags);
>> +     while (ftmac100_tx_complete_packet(priv))
>> +             ;
>> +     spin_unlock_irqrestore(&priv->tx_lock, flags);
>> +}
>> +
>
> I dont understand why you still block hard IRQS, after full NAPI
> conversion.
>
> Now you run from NAPI, and softirq handler, are you sure you still need
> to block hard IRQ and tx_lock ?
>
> It seems to me ftmac100_xmit() could only block softirqs (but they
> already are blocked by caller), so you could use spin_lock() from
> ftmac100_xmit()

I was not quite clear about when to use what kinds of locking.
After your explanation, now I understand.
I will submit v4 tomorrow.

really appreciate,
Po-Yu Chuang

^ permalink raw reply

* Re: [PATCH v3 00/16] make rpc_pipefs be mountable multiple time
From: J. Bruce Fields @ 2011-01-20 15:52 UTC (permalink / raw)
  To: Rob Landley
  Cc: Kirill A. Shutemov, Trond Myklebust, Neil Brown, Pavel Emelyanov,
	linux-nfs, David S. Miller, Al Viro, containers, netdev,
	linux-kernel
In-Reply-To: <4D383F60.4080907@parallels.com>

On Thu, Jan 20, 2011 at 07:57:52AM -0600, Rob Landley wrote:
> I've also mounted NFS shares in test environments I never mounted
> rpc_pipefs in.  (It's possible that the nfs.mount command was doing so
> for me, and apparently unmounting it again afterwards.)  My test
> environment is exporting with the userspace NFS daemon so it's not using
> the kernel cacheing infrastructure.

If you grep for rpc_mkpipe() you'll see it's only used in the idmapping
and rpcsec_gss code; which means you wouldn't need it unless using NFSv4
(which uses idmapping) or rpcsec_gss.

> > There is client dir (nfs/clntX) in rpc_pipefs for every sunrpc client.
> > Both client and server (see fs/nfsd/nfs4callback.c) can create sunrpc
> > client. So we rpc_pipefs on both side.
> 
> Ok, both client and server can create instances.  And use them to do what?
> 
> I understand that the kernel needs to handle RPC calls to service NFS
> requests, what I'm not figuring out is how userspace is involved after
> the initial mount except on the other side of filesystem-agnostic system
> calls.

If you want to see the code on the userland side of the interfaces, see
utils/gssd and utils/idmapd in the nfs-utils code.

--b.

^ permalink raw reply

* Re: [PATCH v3] net: add Faraday FTMAC100 10/100 Ethernet driver
From: Po-Yu Chuang @ 2011-01-20 15:43 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: netdev, linux-kernel, bhutchings, joe, dilinger, Po-Yu Chuang
In-Reply-To: <1295537746.2825.298.camel@edumazet-laptop>

Dear Eric,

On Thu, Jan 20, 2011 at 11:35 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le jeudi 20 janvier 2011 à 23:30 +0800, Po-Yu Chuang a écrit :
>
>> +     /* push packet to protocol stack */
>> +     netif_receive_skb(skb);
>> +
>> +     netdev->stats.rx_packets++;
>> +     netdev->stats.rx_bytes += skb->len;
>> +
>> +     (*processed)++;
>> +
>> +     return true;
>> +}
>> +
>
> Hmm, after call to netif_receive_skb(skb), you are not allowed to access
> skb anymore (maybe it was freed)
>
> netdev->stats.rx_packets++;
> netdev->stats.rx_bytes += skb->len;
> netif_receive_skb(skb);

Wow, you are totally right.

Thanks,
Po-Yu Chuang

^ permalink raw reply

* Re: MultiPath TCP in the Linux Kernel
From: Eric Dumazet @ 2011-01-20 15:42 UTC (permalink / raw)
  To: Hagen Paul Pfeifer; +Cc: christoph.paasch, netdev, bonding-devel, linux-sctp
In-Reply-To: <7e28a5ddc16848dcd98c05351113304f@localhost>

Le jeudi 20 janvier 2011 à 16:38 +0100, Hagen Paul Pfeifer a écrit :

> Hello Christoph,
> 
> if you want that your work becomes part of the official network stack you
> should align your effort on the official Linux way. This means you should
> split your work and publish patches on this maillinglist.

Hmm, they probably know that, and prefer to wait MTCP stuff is mature
before patch submission :)



^ permalink raw reply

* Re: [PATCH v3] net: add Faraday FTMAC100 10/100 Ethernet driver
From: Eric Dumazet @ 2011-01-20 15:41 UTC (permalink / raw)
  To: Po-Yu Chuang
  Cc: netdev, linux-kernel, bhutchings, joe, dilinger, Po-Yu Chuang
In-Reply-To: <1295537418-2057-1-git-send-email-ratbert.chuang@gmail.com>

Le jeudi 20 janvier 2011 à 23:30 +0800, Po-Yu Chuang a écrit :

> +static bool ftmac100_tx_complete_packet(struct ftmac100 *priv)
> +{
> +	struct net_device *netdev = priv->netdev;
> +	struct ftmac100_txdes *txdes;
> +	struct sk_buff *skb;
> +	dma_addr_t map;
> +
> +	if (priv->tx_pending == 0)
> +		return false;
> +
> +	txdes = ftmac100_current_clean_txdes(priv);
> +
> +	if (ftmac100_txdes_owned_by_dma(txdes))
> +		return false;
> +
> +	skb = ftmac100_txdes_get_skb(txdes);
> +	map = ftmac100_txdes_get_dma_addr(txdes);
> +
> +	if (unlikely(ftmac100_txdes_excessive_collision(txdes) ||
> +		     ftmac100_txdes_late_collision(txdes))) {
> +		/*
> +		 * packet transmitted to ethernet lost due to late collision
> +		 * or excessive collision
> +		 */
> +		netdev->stats.tx_aborted_errors++;
> +	} else {
> +		netdev->stats.tx_packets++;
> +		netdev->stats.tx_bytes += skb->len;
> +	}
> +
> +	dma_unmap_single(priv->dev, map, skb_headlen(skb), DMA_TO_DEVICE);
> +
> +	dev_kfree_skb_irq(skb);
> +
> +	ftmac100_txdes_reset(txdes);
> +
> +	ftmac100_tx_clean_pointer_advance(priv);
> +
> +	priv->tx_pending--;
> +	netif_wake_queue(netdev);
> +
> +	return true;
> +}
> +
> +static void ftmac100_tx_complete(struct ftmac100 *priv)
> +{
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&priv->tx_lock, flags);
> +	while (ftmac100_tx_complete_packet(priv))
> +		;
> +	spin_unlock_irqrestore(&priv->tx_lock, flags);
> +}
> +

I dont understand why you still block hard IRQS, after full NAPI
conversion.

Now you run from NAPI, and softirq handler, are you sure you still need
to block hard IRQ and tx_lock ?

It seems to me ftmac100_xmit() could only block softirqs (but they
already are blocked by caller), so you could use spin_lock() from
ftmac100_xmit()

^ permalink raw reply

* Re: MultiPath TCP in the Linux Kernel
From: Hagen Paul Pfeifer @ 2011-01-20 15:38 UTC (permalink / raw)
  To: christoph.paasch; +Cc: netdev, bonding-devel, linux-sctp
In-Reply-To: <201101201509.34536.christoph.paasch@uclouvain.be>


On Thu, 20 Jan 2011 15:09:34 +0100, Christoph Paasch wrote:

> Hi all,

> 

> The IETF is developing a new transport layer solution, MultiPath TCP,

> which 

> allows to efficiently exploit several Internet paths between a pair of

> hosts, 

> while presenting a single TCP connection to the application layer.

> 

> At the UCLouvain in Belgium we are developping the support for MultiPath

> TCP 

> in the Linux Kernel. The implementation is a major extension to the

Linux

> TCP-

> stack.



Hello Christoph,



if you want that your work becomes part of the official network stack you

should align your effort on the official Linux way. This means you should

split your work and publish patches on this maillinglist.



Cheers, Hagen

^ permalink raw reply

* Re: [PATCH v3] net: add Faraday FTMAC100 10/100 Ethernet driver
From: Eric Dumazet @ 2011-01-20 15:35 UTC (permalink / raw)
  To: Po-Yu Chuang
  Cc: netdev, linux-kernel, bhutchings, joe, dilinger, Po-Yu Chuang
In-Reply-To: <1295537418-2057-1-git-send-email-ratbert.chuang@gmail.com>

Le jeudi 20 janvier 2011 à 23:30 +0800, Po-Yu Chuang a écrit :

> +	/* push packet to protocol stack */
> +	netif_receive_skb(skb);
> +
> +	netdev->stats.rx_packets++;
> +	netdev->stats.rx_bytes += skb->len;
> +
> +	(*processed)++;
> +
> +	return true;
> +}
> +

Hmm, after call to netif_receive_skb(skb), you are not allowed to access
skb anymore (maybe it was freed)

netdev->stats.rx_packets++;
netdev->stats.rx_bytes += skb->len;
netif_receive_skb(skb);

^ permalink raw reply

* Re: Bonding on bond
From: Jiri Bohac @ 2011-01-20 15:31 UTC (permalink / raw)
  To: Nicolas de Pesloüan
  Cc: Jiri Bohac, Jay Vosburgh, bonding-devel@lists.sourceforge.net,
	netdev@vger.kernel.org
In-Reply-To: <4D374A8F.2020303@gmail.com>

On Wed, Jan 19, 2011 at 09:33:19PM +0100, Nicolas de Pesloüan wrote:
> Even if it is possible to test for slave and for master with a
> single condition (IFF_BONDING), I suggest to split the tests and the
> error messages, to give end user the best possible diagnostic.

OK, why not. The below patch still uses IFF_BONDING to detect a
master is being enslaved, because IFF_MASTER is also used by the
eql driver. No idea if it works / someone ever uses it with
bonding, but it might collide.

bonding: prohibit enslaving of bonding masters

Nested bonding is not supported and will result in strange problems, e.g.:
- netif_receive_skb() will not properly change skb->dev to point to the
  uppoer-most bonding master
- arp monitor will not work (dev->last_rx is only updated by hardware drivers)
- accidentally enslaving a bonding master to itself will cause an infinite
  recursion in the TX path

This patch prevents this by prohibiting a bonding master from being further enslaved.

Signed-off-by: Jiri Bohac <jbohac@suse.cz>

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index b1025b8..b117dd8 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1453,6 +1453,12 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
 		return -EBUSY;
 	}
 
+	/* cannot enslave a master */
+	if (slave_dev->priv_flags & IFF_BONDING) {
+		pr_debug("Error, cannot enslave a bonding master\n");
+		return -EBUSY;
+	}
+
 	/* vlan challenged mutual exclusion */
 	/* no need to lock since we're protected by rtnl_lock */
 	if (slave_dev->features & NETIF_F_VLAN_CHALLENGED) {

-- 
Jiri Bohac <jbohac@suse.cz>
SUSE Labs, SUSE CZ


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox