Netdev List
 help / color / mirror / Atom feed
* IPV6 address lifetime update
From: Abhijeet Dharmapurikar @ 2010-12-15 19:33 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Alexey Kuznetsov, Pekka Savola (ipv6),
	James Morris, Hideaki YOSHIFUJI, Patrick McHardy

Our product uses Linux kernel 2.6.35.  For IPV6 support, the device is 
using stateless address configuration. We see the global scope address 
is assigned correctly when the network interface transitions to UP 
state.  However, we are seeing issue where subsequent router 
advertisement (RA) messages for assigned prefix results in *new* IPV6 
address on same interface and duplicate-address-detection for same.  Our 
goal is to have the subsequent router RA message simply update the 
address lifetime for existing IPV6 address(es) using the specific prefix 
per RFC2462 section 5.5.3(e).  Looking at /net/ipv6/addrconf.c function 
addrconf_prefix_rcv(), it does not seem like updating existing addresses 
only (without new address creation) is supported.  Is this correct?  Or 
what configuration options are required to achieve the desired behavior?
Below is an excerpt from ‘ip addr’ output, showing interface state after 
a few RA messages have been received.  Note we have configured the 
router to send RA frequently for testing purposes.  Ideally only one 
global address will be present, with lifetime updated on each RA arrival 
for same prefix.
Thank you in advance for guidance on this issue.

3: net0: <UP,LOWER_UP> mtu 1280 qdisc pfifo_fast qlen 1000
     link/[530]
     inet6 2002:c023:9c17:c23:f2f9:6c83:be61:a743/64 scope global dynamic
        valid_lft 7170sec preferred_lft 3570sec
     inet6 2002:c023:9c17:c23:1282:1b78:75dd:b9ed/64 scope global dynamic
        valid_lft 7150sec preferred_lft 3550sec
     inet6 2002:c023:9c17:c23:5326:714d:e25b:797e/64 scope global dynamic
        valid_lft 7134sec preferred_lft 3534sec
     inet6 2002:c023:9c17:c23:488c:95c8:34a2:587f/64 scope global dynamic
        valid_lft 7117sec preferred_lft 3517sec
     inet6 fe80::97e0:7661:c51d:bdb/64 scope link
        valid_lft forever preferred_lft forever

^ permalink raw reply

* Re: [PATCH net-next-2.6] Documentation/networking/l2tp.txt: update documentation.
From: David Miller @ 2010-12-15 19:14 UTC (permalink / raw)
  To: shemminger; +Cc: dshwatrz, netdev, jchapman
In-Reply-To: <20101215105207.29b1a837@nehalam>

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Wed, 15 Dec 2010 10:52:07 -0800

> I wish you would stop whining about it and just reimplement
> without using libnl as requested.

Seriously.

David, I'm not applying a patch where you complain about
a problem which you have self-inflicted upon yourself.

Stephen's request was beyond reasonable, and you're just
being unnecessarily difficult.

^ permalink raw reply

* Re: [PATCH net-next-2.6] net_sched: sch_sfq: add backlog info in sfq_dump_class_stats()
From: Eric Dumazet @ 2010-12-15 19:10 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Jarek Poplawski, Patrick McHardy
In-Reply-To: <1292437116.3427.386.camel@edumazet-laptop>

Le mercredi 15 décembre 2010 à 19:18 +0100, Eric Dumazet a écrit :
> We currently return for each active SFQ slot the number of packets in
> queue. We can also give number of bytes accounted for these packets.
> 
> tc -s class show dev ifb0
> 
> Before patch :
> 
> class sfq 11:3d9 parent 11: 
>  (dropped 0, overlimits 0 requeues 0) 
>  backlog 0b 3p requeues 0 
>  allot 1266 
> 
> After patch :
> 
> class sfq 11:3e4 parent 11: 
>  (dropped 0, overlimits 0 requeues 0) 
>  backlog 4380b 3p requeues 0 
>  allot 1212 
> 
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
>  net/sched/sch_sfq.c |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
> index 3cf478d..cb331de 100644
> --- a/net/sched/sch_sfq.c
> +++ b/net/sched/sch_sfq.c
> @@ -548,8 +548,13 @@ static int sfq_dump_class_stats(struct Qdisc *sch, unsigned long cl,
>  {
>  	struct sfq_sched_data *q = qdisc_priv(sch);
>  	sfq_index idx = q->ht[cl-1];
> -	struct gnet_stats_queue qs = { .qlen = q->qs[idx].qlen };
> +	struct sk_buff_head *list = &q->qs[idx];
> +	struct gnet_stats_queue qs = { .qlen = list->qlen };
>  	struct tc_sfq_xstats xstats = { .allot = q->allot[idx] };
> +	struct sk_buff *skb;
> +
> +	skb_queue_walk(list, skb)
> +		qs.backlog += qdisc_pkt_len(skb);
>  
>  	if (gnet_stats_copy_queue(d, &qs) < 0)
>  		return -1;
> 


By the way, I could not find out how to make "tc -s class show dev ifb0"
reports correct backlog information on parent class itself :

Here we can see 126p, and 0b in backlog :

class cbq 1:11 parent 1:1 leaf 11: rate 50000Kbit cell 8b mpu 64b (bounded) prio 2/2 weight 50000Kbit allot 2250b 
level 0 ewma 5 avpkt 1500b maxidle 0us 
 Sent 330418440 bytes 226314 pkt (dropped 280142, overlimits 689534 requeues 0) 
 rate 52200Kbit 4469pps backlog 0b 126p requeues 0 
  borrowed 0 overactions 182814 avgidle -3363 undertime 2812

qdisc is OK :

qdisc sfq 11: parent 1:11 limit 127p quantum 1514b flows 127/1024 perturb 60sec 
 Sent 1712372746 bytes 1172859 pkt (dropped 1451945, overlimits 0 requeues 0) 
 rate 52287Kbit 4477pps backlog 185420b 127p requeues 0 



^ permalink raw reply

* Re: [PATCH 3/3] net: Fix drivers advertising HW_CSUM feature to use csum_start
From: Michał Mirosław @ 2010-12-15 18:58 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: netdev, e1000-devel, Vasanthy Kolluri, Roopa Prabhu, David Wang,
	Andrew Gallatin, Brice Goglin, Shreyas Bhatewara, VMware, Inc.,
	David S. Miller, Eric Dumazet, Jiri Pirko, H Hartley Sweeten,
	Jie Yang, Joe Perches
In-Reply-To: <20101215102536.6430274b@nehalam>

On Wed, Dec 15, 2010 at 10:25:36AM -0800, Stephen Hemminger wrote:
> On Wed, 15 Dec 2010 19:16:55 +0100
> Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
> > On Tue, Dec 14, 2010 at 07:07:27PM -0800, Stephen Hemminger wrote:
> > > On Wed, 15 Dec 2010 02:24:08 +0100 (CET)
> > > Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
> > > > Some drivers are using skb_transport_offset(skb) instead of skb->csum_start
> > > > for NETIF_F_HW_CSUM offload.  This does not matter now, but if someone
> > > > implements checksumming of encapsulated packets then this will break silently.
[...]
> > > Which if any of these drivers did you test on real hardware?
> > I'm running it on skge, with additional patch that changes it to use
> > NETIF_F_HW_CSUM instead of NETIF_F_IP_CSUM. BTW, why it is not advertising
> > full HW checksum in vanilla kernel? Are there any known hardware bugs in there?
> The driver was written from vendor sk98lin driver and that driver
> only did IP checksum, therefore I did not trust the hardware.
> The chipset is old, and not used in new designs, no documentation
> is up to date (and I didn't have any).
> 
> Please don't enable non-IPv4 checksum offload on this hardware, there
> is too big a risk of it not working on all hardware.

Sure, this can stay in my private patchlist. I can also add a module option
(disabled by default) for the brave ones if you'd accept that.

Best Regards,
Michał Mirosław

^ permalink raw reply

* Re: [PATCH net-next-2.6] Documentation/networking/l2tp.txt: update documentation.
From: Stephen Hemminger @ 2010-12-15 18:52 UTC (permalink / raw)
  To: David Shwatrz; +Cc: davem, netdev, jchapman
In-Reply-To: <AANLkTimhKtb+p6-4bG7QXBmUZ5YX=m5u1ZkxEb94e1K1@mail.gmail.com>

On Wed, 15 Dec 2010 20:40:38 +0200
David Shwatrz <dshwatrz@gmail.com> wrote:

> Hi,
> 
> This patch updates Documentation/networking/l2tp.txt.
> Stephen Hemminger (maintainer of iproute2) rejected patches for
> iproute2 which enable
> "ip l2tp" commands. There is a utility called l2tpv3tun which
> has commands to doing the same (managing static L2TPv3 tunnel).
> see:
> http://www.spinics.net/lists/netdev/msg142037.html
> 
> 
> Regards,
> David Shwartz
> 
> Signed-off-by: David Shwartz <dshwatrz@gmail.com>

I wish you would stop whining about it and just reimplement
without using libnl as requested.

-- 

^ permalink raw reply

* [PATCH net-next-2.6] Documentation/networking/l2tp.txt: update documentation.
From: David Shwatrz @ 2010-12-15 18:40 UTC (permalink / raw)
  To: davem, netdev, jchapman

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

Hi,

This patch updates Documentation/networking/l2tp.txt.
Stephen Hemminger (maintainer of iproute2) rejected patches for
iproute2 which enable
"ip l2tp" commands. There is a utility called l2tpv3tun which
has commands to doing the same (managing static L2TPv3 tunnel).
see:
http://www.spinics.net/lists/netdev/msg142037.html


Regards,
David Shwartz

Signed-off-by: David Shwartz <dshwatrz@gmail.com>

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 1202 bytes --]

diff --git a/Documentation/networking/l2tp.txt b/Documentation/networking/l2tp.txt
index e7bf397..eb6510e 100644
--- a/Documentation/networking/l2tp.txt
+++ b/Documentation/networking/l2tp.txt
@@ -165,6 +165,10 @@ case -- the tunnel socket is created by the kernel and configured
 using parameters sent in the L2TP_CMD_TUNNEL_CREATE netlink
 request. The "ip" utility of iproute2 has commands for managing static
 L2TPv3 tunnels; do "ip l2tp help" for more information.
+The "l2tpv3tun" utility has commands for managing static
+L2TPv3 tunnels; do "l2tpv3tun help" for more information.   
+l2tpv3tun can be download from: hftp://ftp.openl2tp.org/releases
+
 
 Debugging
 =========
@@ -216,9 +220,9 @@ tunnel endpoints:-
 # modprobe l2tp_eth
 # modprobe l2tp_netlink
 
-# ip l2tp add tunnel tunnel_id 1 peer_tunnel_id 1 udp_sport 5000 \
+# l2tpv3tun add tunnel tunnel_id 1 peer_tunnel_id 1 udp_sport 5000 \
   udp_dport 5000 encap udp local 192.168.1.1 remote 192.168.1.2
-# ip l2tp add session tunnel_id 1 session_id 1 peer_session_id 1
+# l2tpv3tun add session tunnel_id 1 session_id 1 peer_session_id 1
 # ifconfig -a
 # ip addr add 10.5.1.2/32 peer 10.5.1.1/32 dev l2tpeth0
 # ifconfig l2tpeth0 up

^ permalink raw reply related

* [PATCH] msm: rmnet: msm rmnet smd virtual ethernet driver
From: Niranjana Vishwanathapura @ 2010-12-15 18:31 UTC (permalink / raw)
  To: netdev, linux-kernel
  Cc: linux-arm-msm, linux-arm-kernel, Niranjana Vishwanathapura,
	Brian Swetland

Virtual ethernet interface for MSM RMNET SMD transport.
This driver creates network devices which use underlaying
SMD ports as transport.  This driver enables sending and
receving IP packets to baseband processor in MSM chipsets.

Cc: Brian Swetland <swetland@google.com>
Signed-off-by: Niranjana Vishwanathapura <nvishwan@codeaurora.org>
---
 drivers/net/Kconfig         |   17 ++
 drivers/net/Makefile        |    1 +
 drivers/net/msm_rmnet_smd.c |  357 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 375 insertions(+), 0 deletions(-)
 create mode 100644 drivers/net/msm_rmnet_smd.c

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index f6668cd..94ddee3 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -3407,4 +3407,21 @@ config VMXNET3
          To compile this driver as a module, choose M here: the
          module will be called vmxnet3.
 
+config MSM_RMNET_SMD
+	bool "MSM RMNET Virtual Network Device over SMD transport"
+	depends on MSM_SMD
+	default n
+	help
+	  Virtual ethernet interface for MSM RMNET SMD transport.
+	  This driver creates network devices which use underlaying
+	  SMD ports as transport.  This driver enables sending and
+	  receving IP packets to baseband processor in MSM chipsets.
+
+config MSM_RMNET_DEBUG
+	bool "MSM RMNET debug interface"
+	depends on MSM_RMNET_SMD
+	default n
+	help
+	  Debug stats on wakeup counts for MSM RMNET devices.
+
 endif # NETDEVICES
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index 652fc6b..51c0443 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -301,3 +301,4 @@ obj-$(CONFIG_CAIF) += caif/
 
 obj-$(CONFIG_OCTEON_MGMT_ETHERNET) += octeon/
 obj-$(CONFIG_PCH_GBE) += pch_gbe/
+obj-$(CONFIG_MSM_RMNET_SMD) += msm_rmnet_smd.o
diff --git a/drivers/net/msm_rmnet_smd.c b/drivers/net/msm_rmnet_smd.c
new file mode 100644
index 0000000..465da4e
--- /dev/null
+++ b/drivers/net/msm_rmnet_smd.c
@@ -0,0 +1,357 @@
+/* drivers/net/msm_rmnet.c
+ *
+ * Virtual Ethernet Interface for MSM7K Networking
+ *
+ * Copyright (C) 2007 Google, Inc.
+ * Copyright (c) 2010, Code Aurora Forum. All rights reserved.
+ * Author: Brian Swetland <swetland@google.com>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/string.h>
+#include <linux/delay.h>
+#include <linux/errno.h>
+#include <linux/interrupt.h>
+#include <linux/init.h>
+#include <linux/netdevice.h>
+#include <linux/etherdevice.h>
+#include <linux/skbuff.h>
+
+#include <mach/msm_smd.h>
+
+#define POLL_DELAY 1000000 /* 1 second delay interval */
+
+struct rmnet_private {
+	smd_channel_t *ch;
+	struct net_device_stats stats;
+	const char *chname;
+#ifdef CONFIG_MSM_RMNET_DEBUG
+	ktime_t last_packet;
+	short active_countdown; /* Number of times left to check */
+	short restart_count; /* Number of polls seems so far */
+	unsigned long wakeups_xmit;
+	unsigned long wakeups_rcv;
+	unsigned long timeout_us;
+	unsigned long awake_time_ms;
+	struct delayed_work work;
+#endif
+};
+
+static int count_this_packet(void *_hdr, int len)
+{
+	struct ethhdr *hdr = _hdr;
+
+	if (len >= ETH_HLEN && hdr->h_proto == htons(ETH_P_ARP))
+		return 0;
+
+	return 1;
+}
+
+#ifdef CONFIG_MSM_RMNET_DEBUG
+static unsigned long timeout_us;
+static struct workqueue_struct *rmnet_wq;
+
+static void do_check_active(struct work_struct *work)
+{
+	struct rmnet_private *p =
+		container_of(work, struct rmnet_private, work.work);
+
+	p->restart_count++;
+	if (--p->active_countdown == 0) {
+		p->awake_time_ms += p->restart_count * POLL_DELAY / 1000;
+		p->restart_count = 0;
+	} else {
+		queue_delayed_work(rmnet_wq, &p->work,
+				   usecs_to_jiffies(POLL_DELAY));
+	}
+}
+
+/* Returns 1 if packet caused rmnet to wakeup, 0 otherwise. */
+static int rmnet_cause_wakeup(struct rmnet_private *p)
+{
+	int ret = 0;
+	ktime_t now;
+	if (p->timeout_us == 0) /* Check if disabled */
+		return 0;
+
+	/* Start timer on a wakeup packet */
+	if (p->active_countdown == 0) {
+		ret = 1;
+		now = ktime_get_real();
+		p->last_packet = now;
+		queue_delayed_work(rmnet_wq, &p->work,
+				usecs_to_jiffies(POLL_DELAY));
+	}
+
+	p->active_countdown = p->timeout_us / POLL_DELAY;
+
+	return ret;
+}
+
+static ssize_t wakeups_xmit_show(struct device *d,
+				 struct device_attribute *attr,
+				 char *buf)
+{
+	struct rmnet_private *p = netdev_priv(to_net_dev(d));
+	return sprintf(buf, "%lu\n", p->wakeups_xmit);
+}
+
+DEVICE_ATTR(wakeups_xmit, 0444, wakeups_xmit_show, NULL);
+
+static ssize_t wakeups_rcv_show(struct device *d, struct device_attribute *attr,
+				char *buf)
+{
+	struct rmnet_private *p = netdev_priv(to_net_dev(d));
+	return sprintf(buf, "%lu\n", p->wakeups_rcv);
+}
+
+DEVICE_ATTR(wakeups_rcv, 0444, wakeups_rcv_show, NULL);
+
+/* Set timeout in us. */
+static ssize_t timeout_store(struct device *d, struct device_attribute *attr,
+			     const char *buf, size_t n)
+{
+	if (strict_strtoul(buf, 10, &timeout_us))
+		return -EINVAL;
+	else
+		return n;
+}
+
+static ssize_t timeout_show(struct device *d, struct device_attribute *attr,
+			    char *buf)
+{
+	struct rmnet_private *p = netdev_priv(to_net_dev(d));
+	p = netdev_priv(to_net_dev(d));
+	return sprintf(buf, "%lu\n", timeout_us);
+}
+
+DEVICE_ATTR(timeout, 0664, timeout_show, timeout_store);
+
+/* Show total radio awake time in ms */
+static ssize_t awake_time_show(struct device *d, struct device_attribute *attr,
+			       char *buf)
+{
+	struct rmnet_private *p = netdev_priv(to_net_dev(d));
+	return sprintf(buf, "%lu\n", p->awake_time_ms);
+}
+DEVICE_ATTR(awake_time_ms, 0444, awake_time_show, NULL);
+
+#endif
+
+/* Called in soft-irq context */
+static void smd_net_data_handler(unsigned long arg)
+{
+	struct net_device *dev = (struct net_device *) arg;
+	struct rmnet_private *p = netdev_priv(dev);
+	struct sk_buff *skb;
+	void *ptr = 0;
+	int sz;
+
+	for (;;) {
+		sz = smd_cur_packet_size(p->ch);
+		if (sz == 0)
+			break;
+		if (smd_read_avail(p->ch) < sz)
+			break;
+
+		if (sz > 1514) {
+			pr_err("rmnet_recv() discarding %d len\n", sz);
+			ptr = 0;
+		} else {
+			skb = dev_alloc_skb(sz + NET_IP_ALIGN);
+			if (skb == NULL) {
+				pr_err("rmnet_recv() cannot allocate skb\n");
+			} else {
+				skb->dev = dev;
+				skb_reserve(skb, NET_IP_ALIGN);
+				ptr = skb_put(skb, sz);
+				if (smd_read(p->ch, ptr, sz) != sz) {
+					pr_err("rmnet_recv() "
+					       "smd lied about avail?!");
+					ptr = 0;
+					dev_kfree_skb_irq(skb);
+				} else {
+					skb->protocol = eth_type_trans(skb,
+								       dev);
+					if (count_this_packet(ptr, skb->len)) {
+#ifdef CONFIG_MSM_RMNET_DEBUG
+						p->wakeups_rcv +=
+							rmnet_cause_wakeup(p);
+#endif
+						p->stats.rx_packets++;
+						p->stats.rx_bytes += skb->len;
+					}
+					netif_rx(skb);
+				}
+				continue;
+			}
+		}
+		if (smd_read(p->ch, ptr, sz) != sz)
+			pr_err("rmnet_recv() smd lied about avail?!");
+	}
+}
+
+static DECLARE_TASKLET(smd_net_data_tasklet, smd_net_data_handler, 0);
+
+static void smd_net_notify(void *_dev, unsigned event)
+{
+	if (event != SMD_EVENT_DATA)
+		return;
+
+	smd_net_data_tasklet.data = (unsigned long) _dev;
+
+	tasklet_schedule(&smd_net_data_tasklet);
+}
+
+static int rmnet_open(struct net_device *dev)
+{
+	int r;
+	struct rmnet_private *p = netdev_priv(dev);
+
+	pr_info("rmnet_open()\n");
+	if (!p->ch) {
+		r = smd_open(p->chname, &p->ch, dev, smd_net_notify);
+
+		if (r < 0)
+			return -ENODEV;
+	}
+
+	netif_start_queue(dev);
+	return 0;
+}
+
+static int rmnet_stop(struct net_device *dev)
+{
+	pr_info("rmnet_stop()\n");
+	netif_stop_queue(dev);
+	return 0;
+}
+
+static int rmnet_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+	struct rmnet_private *p = netdev_priv(dev);
+	smd_channel_t *ch = p->ch;
+
+	if (smd_write_atomic(ch, skb->data, skb->len) != skb->len) {
+		pr_err("rmnet fifo full, dropping packet\n");
+	} else {
+		if (count_this_packet(skb->data, skb->len)) {
+			p->stats.tx_packets++;
+			p->stats.tx_bytes += skb->len;
+#ifdef CONFIG_MSM_RMNET_DEBUG
+			p->wakeups_xmit += rmnet_cause_wakeup(p);
+#endif
+		}
+	}
+
+	dev_kfree_skb_irq(skb);
+	return 0;
+}
+
+static struct net_device_stats *rmnet_get_stats(struct net_device *dev)
+{
+	struct rmnet_private *p = netdev_priv(dev);
+	return &p->stats;
+}
+
+static void rmnet_set_multicast_list(struct net_device *dev)
+{
+}
+
+static void rmnet_tx_timeout(struct net_device *dev)
+{
+	pr_info("rmnet_tx_timeout()\n");
+}
+
+static struct net_device_ops rmnet_ops = {
+	.ndo_open = rmnet_open,
+	.ndo_stop = rmnet_stop,
+	.ndo_start_xmit = rmnet_xmit,
+	.ndo_get_stats = rmnet_get_stats,
+	.ndo_set_multicast_list = rmnet_set_multicast_list,
+	.ndo_tx_timeout = rmnet_tx_timeout,
+};
+
+static void __init rmnet_setup(struct net_device *dev)
+{
+	dev->netdev_ops = &rmnet_ops;
+
+	dev->watchdog_timeo = 20;
+
+	ether_setup(dev);
+
+	random_ether_addr(dev->dev_addr);
+}
+
+
+static const char *ch_name[3] = {
+	"SMD_DATA5",
+	"SMD_DATA6",
+	"SMD_DATA7",
+};
+
+static int __init rmnet_init(void)
+{
+	int ret;
+	struct device *d;
+	struct net_device *dev;
+	struct rmnet_private *p;
+	unsigned n;
+
+#ifdef CONFIG_MSM_RMNET_DEBUG
+	timeout_us = 0;
+#endif
+
+#ifdef CONFIG_MSM_RMNET_DEBUG
+	rmnet_wq = create_workqueue("rmnet");
+#endif
+
+	for (n = 0; n < 3; n++) {
+		dev = alloc_netdev(sizeof(struct rmnet_private),
+				   "rmnet%d", rmnet_setup);
+
+		if (!dev)
+			return -ENOMEM;
+
+		d = &(dev->dev);
+		p = netdev_priv(dev);
+		p->chname = ch_name[n];
+#ifdef CONFIG_MSM_RMNET_DEBUG
+		p->timeout_us = timeout_us;
+		p->awake_time_ms = p->wakeups_xmit = p->wakeups_rcv = 0;
+		p->active_countdown = p->restart_count = 0;
+		INIT_DELAYED_WORK_DEFERRABLE(&p->work, do_check_active);
+#endif
+
+		ret = register_netdev(dev);
+		if (ret) {
+			free_netdev(dev);
+			return ret;
+		}
+
+#ifdef CONFIG_MSM_RMNET_DEBUG
+		if (device_create_file(d, &dev_attr_timeout))
+			continue;
+		if (device_create_file(d, &dev_attr_wakeups_xmit))
+			continue;
+		if (device_create_file(d, &dev_attr_wakeups_rcv))
+			continue;
+		if (device_create_file(d, &dev_attr_awake_time_ms))
+			continue;
+#endif
+	}
+	return 0;
+}
+
+module_init(rmnet_init);
-- 
1.5.6.3

Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

^ permalink raw reply related

* Re: [PATCH 3/3] net: Fix drivers advertising HW_CSUM feature to use csum_start
From: Stephen Hemminger @ 2010-12-15 18:25 UTC (permalink / raw)
  To: Michał Mirosław
  Cc: Roopa Prabhu, Dumazet, Jiri Pirko, e1000-devel, netdev, Vasanthy,
	Jie Yang, Eric, Kolluri, VMware, Inc., Brice Goglin, H,
	Andrew Gallatin, Joe Perches, David Wang, Shreyas Bhatewara,
	David S. Miller, Sweeten
In-Reply-To: <20101215181655.GA29962@rere.qmqm.pl>

On Wed, 15 Dec 2010 19:16:55 +0100
Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:

> On Tue, Dec 14, 2010 at 07:07:27PM -0800, Stephen Hemminger wrote:
> > On Wed, 15 Dec 2010 02:24:08 +0100 (CET)
> > Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
> > > Some drivers are using skb_transport_offset(skb) instead of skb->csum_start
> > > for NETIF_F_HW_CSUM offload.  This does not matter now, but if someone
> > > implements checksumming of encapsulated packets then this will break silently.
> > > 
> > > TSO output paths are left as they are, since they are for IP+TCP only
> > > (might be worth converting though).
> > > 
> > > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > Which if any of these drivers did you test on real hardware?
> 
> I'm running it on skge, with additional patch that changes it to use
> NETIF_F_HW_CSUM instead of NETIF_F_IP_CSUM. BTW, why it is not advertising
> full HW checksum in vanilla kernel? Are there any known hardware bugs in there?
> 
> Best Regards,
> Michał Mirosław

The driver was written from vendor sk98lin driver and that driver
only did IP checksum, therefore I did not trust the hardware.
The chipset is old, and not used in new designs, no documentation
is up to date (and I didn't have any).

Please don't enable non-IPv4 checksum offload on this hardware, there
is too big a risk of it not working on all hardware.

-- 

------------------------------------------------------------------------------
Lotusphere 2011
Register now for Lotusphere 2011 and learn how
to connect the dots, take your collaborative environment
to the next level, and enter the era of Social Business.
http://p.sf.net/sfu/lotusphere-d2d
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply

* Re: [2.6.37-rc5] Build error on parisc.
From: Eric Dumazet @ 2010-12-15 18:22 UTC (permalink / raw)
  To: David Miller; +Cc: penguin-kernel, netdev
In-Reply-To: <20101215.101925.71116399.davem@davemloft.net>

Le mercredi 15 décembre 2010 à 10:19 -0800, David Miller a écrit :
> From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Date: Wed, 15 Dec 2010 17:01:14 +0900
> 
> > [2.6.37-rc5] Build error on parisc.
> > 
> > Commit 5635c10d "net: make sure struct dst_entry refcount is aligned on 64 bytes"
> > uses manual padding. I triggered BUILD_BUG_ON() when using
> > http://kernel.org/pub/tools/crosstool/files/bin/i686/4.5.1/i686-gcc-4.5.1-nolibc_hppa64-linux.tar.bz2 .
> > 
> >   make -s CROSS_COMPILE=hppa64-linux- ARCH=parisc
> > 
> >   include/net/dst.h: In function 'dst_hold':
> >   include/net/dst.h:161:2: error: negative width in bit-field '<anonymous>'
> > 
> > I think below patch can fix the error but that commit says that we cannot use
> > __atribute((aligned)). Why?
> 
> We don't want to create holes of unused space in the structure, we want
> to know exactly how every byte of space is being used so nothing is
> wasted when all features are enabed.

Yes, sorry for missing your mail Tetsuo.




^ permalink raw reply

* Re: [2.6.37-rc5] Build error on parisc.
From: David Miller @ 2010-12-15 18:19 UTC (permalink / raw)
  To: penguin-kernel; +Cc: eric.dumazet, netdev
In-Reply-To: <201012150801.oBF81EVm047830@www262.sakura.ne.jp>

From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Wed, 15 Dec 2010 17:01:14 +0900

> [2.6.37-rc5] Build error on parisc.
> 
> Commit 5635c10d "net: make sure struct dst_entry refcount is aligned on 64 bytes"
> uses manual padding. I triggered BUILD_BUG_ON() when using
> http://kernel.org/pub/tools/crosstool/files/bin/i686/4.5.1/i686-gcc-4.5.1-nolibc_hppa64-linux.tar.bz2 .
> 
>   make -s CROSS_COMPILE=hppa64-linux- ARCH=parisc
> 
>   include/net/dst.h: In function 'dst_hold':
>   include/net/dst.h:161:2: error: negative width in bit-field '<anonymous>'
> 
> I think below patch can fix the error but that commit says that we cannot use
> __atribute((aligned)). Why?

We don't want to create holes of unused space in the structure, we want
to know exactly how every byte of space is being used so nothing is
wasted when all features are enabed.

^ permalink raw reply

* [PATCH net-next-2.6] net_sched: sch_sfq: add backlog info in sfq_dump_class_stats()
From: Eric Dumazet @ 2010-12-15 18:18 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Jarek Poplawski, Patrick McHardy
In-Reply-To: <1292434227.3427.377.camel@edumazet-laptop>

We currently return for each active SFQ slot the number of packets in
queue. We can also give number of bytes accounted for these packets.

tc -s class show dev ifb0

Before patch :

class sfq 11:3d9 parent 11: 
 (dropped 0, overlimits 0 requeues 0) 
 backlog 0b 3p requeues 0 
 allot 1266 

After patch :

class sfq 11:3e4 parent 11: 
 (dropped 0, overlimits 0 requeues 0) 
 backlog 4380b 3p requeues 0 
 allot 1212 


Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 net/sched/sch_sfq.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index 3cf478d..cb331de 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -548,8 +548,13 @@ static int sfq_dump_class_stats(struct Qdisc *sch, unsigned long cl,
 {
 	struct sfq_sched_data *q = qdisc_priv(sch);
 	sfq_index idx = q->ht[cl-1];
-	struct gnet_stats_queue qs = { .qlen = q->qs[idx].qlen };
+	struct sk_buff_head *list = &q->qs[idx];
+	struct gnet_stats_queue qs = { .qlen = list->qlen };
 	struct tc_sfq_xstats xstats = { .allot = q->allot[idx] };
+	struct sk_buff *skb;
+
+	skb_queue_walk(list, skb)
+		qs.backlog += qdisc_pkt_len(skb);
 
 	if (gnet_stats_copy_queue(d, &qs) < 0)
 		return -1;



^ permalink raw reply related

* Re: [PATCH 3/3] net: Fix drivers advertising HW_CSUM feature to use csum_start
From: Michał Mirosław @ 2010-12-15 18:16 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Roopa Prabhu, Eric Dumazet, Jiri Pirko, e1000-devel, netdev,
	Shreyas Bhatewara, Jie Yang, Vasanthy Kolluri, VMware, Inc.,
	Brice Goglin, Andrew Gallatin, Joe Perches, David Wang,
	David S. Miller, H Hartley Sweeten
In-Reply-To: <20101214190727.0ed1a925@nehalam>

On Tue, Dec 14, 2010 at 07:07:27PM -0800, Stephen Hemminger wrote:
> On Wed, 15 Dec 2010 02:24:08 +0100 (CET)
> Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
> > Some drivers are using skb_transport_offset(skb) instead of skb->csum_start
> > for NETIF_F_HW_CSUM offload.  This does not matter now, but if someone
> > implements checksumming of encapsulated packets then this will break silently.
> > 
> > TSO output paths are left as they are, since they are for IP+TCP only
> > (might be worth converting though).
> > 
> > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> Which if any of these drivers did you test on real hardware?

I'm running it on skge, with additional patch that changes it to use
NETIF_F_HW_CSUM instead of NETIF_F_IP_CSUM. BTW, why it is not advertising
full HW checksum in vanilla kernel? Are there any known hardware bugs in there?

Best Regards,
Michał Mirosław

---
lspci:

02:05.0 Ethernet controller: 3Com Corporation 3c940 10/100/1000Base-T [Marvell] (rev 12)
        Subsystem: ASUSTeK Computer Inc. A7V600/P4P800/K8V motherboard
        Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV+ VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx-
        Status: Cap+ 66MHz+ UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
        Latency: 64 (5750ns min, 7750ns max), Cache Line Size: 16 bytes
        Interrupt: pin A routed to IRQ 22
        Region 0: Memory at feafc000 (32-bit, non-prefetchable) [size=16K]
        Region 1: I/O ports at d800 [size=256]
        Capabilities: <access denied>
        Kernel driver in use: skge
        Kernel modules: skge

ethtool -k:

Offload parameters for eth3:
rx-checksumming: on
tx-checksumming: on
scatter-gather: on
tcp segmentation offload: off
udp fragmentation offload: off
generic segmentation offload: on
large receive offload: off


------------------------------------------------------------------------------
Lotusphere 2011
Register now for Lotusphere 2011 and learn how
to connect the dots, take your collaborative environment
to the next level, and enter the era of Social Business.
http://p.sf.net/sfu/lotusphere-d2d
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply

* RE: [PATCH] ixgb: Convert to new vlan model.
From: Tantilov, Emil S @ 2010-12-15 18:09 UTC (permalink / raw)
  To: Jesse Gross
  Cc: David Miller, netdev@vger.kernel.org, Kirsher, Jeffrey T,
	Duyck, Alexander H, Ben Hutchings
In-Reply-To: <AANLkTimnSedKQhhL8xXQVKnEm3FpbfRQ_Nd9hPmGbHLJ@mail.gmail.com>

Jesse Gross wrote:
> On Tue, Dec 14, 2010 at 11:15 AM, Ben Hutchings
> <bhutchings@solarflare.com> wrote:
>> On Tue, 2010-12-14 at 12:08 -0700, Tantilov, Emil S wrote:
>>> Ben Hutchings wrote:
>>>> On Tue, 2010-12-14 at 11:09 -0700, Tantilov, Emil S wrote:
>>>>> Ben Hutchings wrote:
>>>>>> On Mon, 2010-12-13 at 19:42 -0800, Jesse Gross wrote:
>>>>>>> This switches the ixgb driver to use the new vlan interfaces.
>>>>>>> In doing this, it completes the work begun in
>>>>>>> ae54496f9e8d40c89e5668205c181dccfa9ecda1 allowing the use of
>>>>>>> hardware vlan insertion without having a vlan group configured.
>>>>>>> [...] diff --git a/drivers/net/ixgb/ixgb_ethtool.c
>>>>>>> b/drivers/net/ixgb/ixgb_ethtool.c
>>>>>>> index 43994c1..0e4c527 100644
>>>>>>> --- a/drivers/net/ixgb/ixgb_ethtool.c
>>>>>>> +++ b/drivers/net/ixgb/ixgb_ethtool.c
>>>>>>> @@ -706,6 +706,45 @@ ixgb_get_strings(struct net_device *netdev,
>>>>>>> u32  stringset, u8 *data)        } }
>>>>>>> 
>>>>>>> +static int ixgb_set_flags(struct net_device *netdev, u32 data)
>>>>>>> +{ +        struct ixgb_adapter *adapter = netdev_priv(netdev);
>>>>>>> +   bool need_reset; +    int rc; + +        /* The hardware
>>>>>>> requires that RX vlan stripping and TX vlan insertion +       *
>>>>>>> be configured together.  Therefore, if one setting changes
>>>>>>> adjust the +      * other one to match. +         */ +      
>>>>>>> if (!!(data & ETH_FLAG_RXVLAN) != !!(data & ETH_FLAG_TXVLAN)) {
>>>>>>> +                if ((data & ETH_FLAG_RXVLAN) != +            
>>>>>>> (netdev->features & NETIF_F_HW_VLAN_RX)) +                    
>>>>>>> data ^= ETH_FLAG_TXVLAN; +                else if ((data &
>>>>>>> ETH_FLAG_TXVLAN) != +                    (netdev->features &
>>>>>>> NETIF_F_HW_VLAN_TX)) +                        data ^=
>>>>>>> ETH_FLAG_RXVLAN; +        }
>>>>>> [...]
>>>>>> 
>>>>>> I think this should reject attempts to change just one flag with
>>>>>> -EINVAL, rather than quietly 'fixing' the setting.
>>>>>> 
>>>>>> Ben.
>>>>> 
>>>>> I'm not sure this is a good idea. At least not without some sort
>>>>> of explanation. Since there is no way for the user to know that
>>>>> he needs to disable both.
>>>> 
>>>> Document the limitation in Documentation/networking/ixgb.txt.  You
>>>> could also send a patch for the ethtool manual page stating that
>>>> this restriction might exist. 
>>>> 
>>>> Ben.
>>> 
>>> Just to make sure it's clear - there is no hard requirement for both
>>> settings to be set at the same time. So setting:
>>> ethtool -K eth0 rxvlan off
>>> 
>>> Is a valid setting and will disable stripping on Rx, but because of
>>> the design, stripping on Tx will also be disabled.
>> 
>> Then it's *not* a valid setting for your hardware/driver.
> 
> Ben, I agree that limiting the settings to what is actually supported
> is conceptually cleaner but in practice it's not very intuitive.  If
> you try to turn something off and the response is that it's invalid,
> most people are going to assume that you just can't do it.  This is
> especially true since you actually can't turn these settings off in
> most drivers.
> 
> There's a precedent for this type of thing: turn off TX checksum
> offloading and watch scatter/gather and TSO be automatically disabled
> as well.  It makes sense - the user requested a change, we do what is
> necessary to make that happen without requiring them to understand why
> these features are interrelated.
> 
> Emil, I realized afterwards that, as you pointed out, TX vlan
> offloading can be disabled without requiring RX offloading to be
> disabled.  Feel free to make the modification yourself or I can
> resubmit, whichever is easier.

If it's OK with you I can make whatever changes are needed since I need
to test this first, so I don't want to go back and forth in case we find
other issues in testing.

Thanks,
Emil

^ permalink raw reply

* Re: pull request: wireless-2.6 2010-12-15
From: David Miller @ 2010-12-15 17:54 UTC (permalink / raw)
  To: linville-2XuSBdqkA4R54TAoqtyWWQ
  Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20101215155642.GE2377-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>

From: "John W. Linville" <linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
Date: Wed, 15 Dec 2010 10:56:42 -0500

> This is a batch of fixes intended for 2.6.37.
> 
> The biggest changes fix an issue where the iwlagn driver will misread
> the EEPROM information for some devices that will soon be in the wild.
> The initial impulse was to rewrite the EEPROM code, but I pushed back on
> that for 2.6.37.  The compromise is to add the new EEPROM reading code,
> but only to invoke it for the new devices.  Older devices will still use
> the existing (and tested) EEPROM code.  Note that all devices will use
> the new code in 2.6.38 and beyond.
> 
> This also includes a pull of bluetooth fixes from Gustavo Padovan.  That
> includes a null pointer fix and a fix for a regression that broke the
> DUN profile -- say goodbye to your emergency cell-phone Internet
> connection without that!
> 
> Other patches include some device ID additions for p54usb, a fix to
> avoid log spam resulting from suspend/resume with USB-based wireless
> devices that don't define their own suspend hook, a null ptr fix for the
> libertas driver, and another null pointer fix related to IBSS merges.
> 
> Please let me know if there are problems!

Looks good, pulled, thanks John.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH v3] net_sched: sch_sfq: fix allot handling
From: Eric Dumazet @ 2010-12-15 17:30 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: David Miller, netdev, Jarek Poplawski
In-Reply-To: <4D08F91C.4070000@trash.net>

Le mercredi 15 décembre 2010 à 18:21 +0100, Patrick McHardy a écrit :
> On 15.12.2010 18:09, Eric Dumazet wrote:
> > Le mercredi 15 décembre 2010 à 18:03 +0100, Patrick McHardy a écrit :
> > 
> >> Right, that's odd. It shouldn't be necessary anymore though since
> >> now we initialize allot in sfq_enqueue() for all new flows and
> >> increase allotment for all active flows once per round in sfq_dequeue().
> >> The above code causes a second increase for the flow following a flow
> >> which went inactive.
> > 
> > Well, we do this in three places. Each time we 'select' a flow as the
> > next packet provider, we increase its allot by quantum.
> > 
> > We could change this, adding quantum to the current slot when its allot
> > becomes negative (and we select the next slot for next round)
> 
> Right, I again missed that 'a' refers to the next flow in the second
> condition in sfq_dequeue().
> 
> I have to run now, will have another look at this tommorrow.

Here is v3 : Now we add a quantum only when current flow consumed all
its allot, not "in advance for next slot". this should use less cpu too.

Thanks

[PATCH v3] net_sched: sch_sfq: fix allot handling

When deploying SFQ/IFB here at work, I found the allot management was
pretty wrong in sfq, even changing allot from short to int...

We should init allot for each new flow, not using a previous value found
in slot.

Before patch, I saw bursts of several packets per flow, apparently
denying the default "quantum 1514" limit I had on my SFQ class.

class sfq 11:1 parent 11: 
 (dropped 0, overlimits 0 requeues 0) 
 backlog 0b 7p requeues 0 
 allot 11546 

class sfq 11:46 parent 11: 
 (dropped 0, overlimits 0 requeues 0) 
 backlog 0b 1p requeues 0 
 allot -23873 

class sfq 11:78 parent 11: 
 (dropped 0, overlimits 0 requeues 0) 
 backlog 0b 5p requeues 0 
 allot 11393 

After patch, better fairness among each flow, allot limit being
respected, allot is positive :

class sfq 11:e parent 11: 
 (dropped 0, overlimits 0 requeues 86) 
 backlog 0b 3p requeues 86 
 allot 596 

class sfq 11:94 parent 11: 
 (dropped 0, overlimits 0 requeues 0) 
 backlog 0b 3p requeues 0 
 allot 1468 

class sfq 11:a4 parent 11: 
 (dropped 0, overlimits 0 requeues 0) 
 backlog 0b 4p requeues 0 
 allot 650 

class sfq 11:bb parent 11: 
 (dropped 0, overlimits 0 requeues 0) 
 backlog 0b 3p requeues 0 
 allot 596 

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 net/sched/sch_sfq.c |   20 ++++++++------------
 1 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index 3cf478d..7150705 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -270,7 +270,6 @@ static unsigned int sfq_drop(struct Qdisc *sch)
 		/* It is difficult to believe, but ALL THE SLOTS HAVE LENGTH 1. */
 		d = q->next[q->tail];
 		q->next[q->tail] = q->next[d];
-		q->allot[q->next[d]] += q->quantum;
 		skb = q->qs[d].prev;
 		len = qdisc_pkt_len(skb);
 		__skb_unlink(skb, &q->qs[d]);
@@ -321,14 +320,13 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 	sfq_inc(q, x);
 	if (q->qs[x].qlen == 1) {		/* The flow is new */
 		if (q->tail == SFQ_DEPTH) {	/* It is the first flow */
-			q->tail = x;
 			q->next[x] = x;
-			q->allot[x] = q->quantum;
 		} else {
 			q->next[x] = q->next[q->tail];
 			q->next[q->tail] = x;
-			q->tail = x;
 		}
+		q->tail = x;
+		q->allot[x] = q->quantum;
 	}
 	if (++sch->q.qlen <= q->limit) {
 		sch->bstats.bytes += qdisc_pkt_len(skb);
@@ -359,13 +357,13 @@ sfq_dequeue(struct Qdisc *sch)
 {
 	struct sfq_sched_data *q = qdisc_priv(sch);
 	struct sk_buff *skb;
-	sfq_index a, old_a;
+	sfq_index a, next_a;
 
 	/* No active slots */
 	if (q->tail == SFQ_DEPTH)
 		return NULL;
 
-	a = old_a = q->next[q->tail];
+	a = q->next[q->tail];
 
 	/* Grab packet */
 	skb = __skb_dequeue(&q->qs[a]);
@@ -376,17 +374,15 @@ sfq_dequeue(struct Qdisc *sch)
 	/* Is the slot empty? */
 	if (q->qs[a].qlen == 0) {
 		q->ht[q->hash[a]] = SFQ_DEPTH;
-		a = q->next[a];
-		if (a == old_a) {
+		next_a = q->next[a];
+		if (a == next_a) {
 			q->tail = SFQ_DEPTH;
 			return skb;
 		}
-		q->next[q->tail] = a;
-		q->allot[a] += q->quantum;
+		q->next[q->tail] = next_a;
 	} else if ((q->allot[a] -= qdisc_pkt_len(skb)) <= 0) {
-		q->tail = a;
-		a = q->next[a];
 		q->allot[a] += q->quantum;
+		q->tail = a;
 	}
 	return skb;
 }



^ permalink raw reply related

* Re: [PATCH v2] net_sched: sch_sfq: fix allot handling
From: Patrick McHardy @ 2010-12-15 17:21 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev, Jarek Poplawski
In-Reply-To: <1292432980.3427.369.camel@edumazet-laptop>

On 15.12.2010 18:09, Eric Dumazet wrote:
> Le mercredi 15 décembre 2010 à 18:03 +0100, Patrick McHardy a écrit :
> 
>> Right, that's odd. It shouldn't be necessary anymore though since
>> now we initialize allot in sfq_enqueue() for all new flows and
>> increase allotment for all active flows once per round in sfq_dequeue().
>> The above code causes a second increase for the flow following a flow
>> which went inactive.
> 
> Well, we do this in three places. Each time we 'select' a flow as the
> next packet provider, we increase its allot by quantum.
> 
> We could change this, adding quantum to the current slot when its allot
> becomes negative (and we select the next slot for next round)

Right, I again missed that 'a' refers to the next flow in the second
condition in sfq_dequeue().

I have to run now, will have another look at this tommorrow.

^ permalink raw reply

* Re: [PATCH v2] net_sched: sch_sfq: fix allot handling
From: Eric Dumazet @ 2010-12-15 17:09 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: David Miller, netdev, Jarek Poplawski
In-Reply-To: <4D08F4F4.3050501@trash.net>

Le mercredi 15 décembre 2010 à 18:03 +0100, Patrick McHardy a écrit :

> Right, that's odd. It shouldn't be necessary anymore though since
> now we initialize allot in sfq_enqueue() for all new flows and
> increase allotment for all active flows once per round in sfq_dequeue().
> The above code causes a second increase for the flow following a flow
> which went inactive.

Well, we do this in three places. Each time we 'select' a flow as the
next packet provider, we increase its allot by quantum.

We could change this, adding quantum to the current slot when its allot
becomes negative (and we select the next slot for next round)

This basically was what my V1 was doing ;)




^ permalink raw reply

* Re: [PATCH v2] net_sched: sch_sfq: fix allot handling
From: Patrick McHardy @ 2010-12-15 17:03 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev, Jarek Poplawski
In-Reply-To: <1292432120.3427.366.camel@edumazet-laptop>

On 15.12.2010 17:55, Eric Dumazet wrote:
> Le mercredi 15 décembre 2010 à 17:43 +0100, Patrick McHardy a écrit :
> 
>> Now we could remove the allot increase in sfq_dequeue for
>> the case that the flow becomes inactive. It is incorrect
>> anyways.
> 
> Hmm, we increase the allot for the next slot, not for the slot now
> empty.
> 
> 
>         /* Is the slot empty? */
>         if (q->qs[a].qlen == 0) {
>                 q->ht[q->hash[a]] = SFQ_DEPTH;
>                 a = q->next[a]; // a = next slot index
>                 if (a == old_a) {
>                         q->tail = SFQ_DEPTH;
>                         return skb;
>                 }
>                 q->next[q->tail] = a;
>                 q->allot[a] += q->quantum;
> // HERE, q->allot[a] is for next slot, we give it its quantum for being
> activated

Right, that's odd. It shouldn't be necessary anymore though since
now we initialize allot in sfq_enqueue() for all new flows and
increase allotment for all active flows once per round in sfq_dequeue().
The above code causes a second increase for the flow following a flow
which went inactive.

> 
>         } else if ((q->allot[a] -= qdisc_pkt_len(skb)) <= 0) {
> 
> Maybe we should rename (a / old_a) by (a / next_a) to avoid confusion :)

That might have made things more clearer :)

> I was thinking in allowing more packets per SFQ (but keep the 126 active
> flows limit), what do you think ?

I keep forgetting why this limit exists, let me try to figure
it out once more :)

^ permalink raw reply

* Re: [PATCH v2] net_sched: sch_sfq: fix allot handling
From: Eric Dumazet @ 2010-12-15 16:55 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: David Miller, netdev, Jarek Poplawski
In-Reply-To: <4D08F025.5030603@trash.net>

Le mercredi 15 décembre 2010 à 17:43 +0100, Patrick McHardy a écrit :

> Now we could remove the allot increase in sfq_dequeue for
> the case that the flow becomes inactive. It is incorrect
> anyways.

Hmm, we increase the allot for the next slot, not for the slot now
empty.


        /* Is the slot empty? */
        if (q->qs[a].qlen == 0) {
                q->ht[q->hash[a]] = SFQ_DEPTH;
                a = q->next[a]; // a = next slot index
                if (a == old_a) {
                        q->tail = SFQ_DEPTH;
                        return skb;
                }
                q->next[q->tail] = a;
                q->allot[a] += q->quantum;
// HERE, q->allot[a] is for next slot, we give it its quantum for being
activated

        } else if ((q->allot[a] -= qdisc_pkt_len(skb)) <= 0) {

Maybe we should rename (a / old_a) by (a / next_a) to avoid confusion :)

I was thinking in allowing more packets per SFQ (but keep the 126 active
flows limit), what do you think ?




^ permalink raw reply

* Re: [PATCH v2] net_sched: sch_sfq: fix allot handling
From: Patrick McHardy @ 2010-12-15 16:43 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev, Jarek Poplawski
In-Reply-To: <1292431256.3427.358.camel@edumazet-laptop>

On 15.12.2010 17:40, Eric Dumazet wrote:
> Le mercredi 15 décembre 2010 à 17:27 +0100, Eric Dumazet a écrit :
> 
>> Hmm, you may be right, thanks a lot for reviewing !
>>
>> I noticed that with normal quantum (1514), my SFQ setup was sending two
>> full frames per flow after my patch, so was about to prepare a new
>> version ;)
>>
>> I'll post a v2 shortly.
> 
> Indeed, the missing init in sfq_enqueue() is enough to solve the
> problem : 
> 
> [PATCH v2] net_sched: sch_sfq: fix allot handling
> 
> When deploying SFQ/IFB here at work, I found the allot management was
> pretty wrong in sfq, even changing allot from short to int...
> 
> We should init allot for each new flow, not using a previous value found
> in slot.
> 
> Before patch, I saw bursts of several packets per flow, apparently
> denying the default "quantum 1514" limit I had on my SFQ class.
> 
> diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
> index 3cf478d..065a2a5 100644
> --- a/net/sched/sch_sfq.c
> +++ b/net/sched/sch_sfq.c
> @@ -321,14 +321,13 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch)
>  	sfq_inc(q, x);
>  	if (q->qs[x].qlen == 1) {		/* The flow is new */
>  		if (q->tail == SFQ_DEPTH) {	/* It is the first flow */
> -			q->tail = x;
>  			q->next[x] = x;
> -			q->allot[x] = q->quantum;
>  		} else {
>  			q->next[x] = q->next[q->tail];
>  			q->next[q->tail] = x;
> -			q->tail = x;
>  		}
> +		q->tail = x;
> +		q->allot[x] = q->quantum;
>  	}

Now we could remove the allot increase in sfq_dequeue for
the case that the flow becomes inactive. It is incorrect
anyways.

^ permalink raw reply

* [PATCH v2] net_sched: sch_sfq: fix allot handling
From: Eric Dumazet @ 2010-12-15 16:40 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: David Miller, netdev, Jarek Poplawski
In-Reply-To: <1292430424.3427.350.camel@edumazet-laptop>

Le mercredi 15 décembre 2010 à 17:27 +0100, Eric Dumazet a écrit :

> Hmm, you may be right, thanks a lot for reviewing !
> 
> I noticed that with normal quantum (1514), my SFQ setup was sending two
> full frames per flow after my patch, so was about to prepare a new
> version ;)
> 
> I'll post a v2 shortly.

Indeed, the missing init in sfq_enqueue() is enough to solve the
problem : 

[PATCH v2] net_sched: sch_sfq: fix allot handling

When deploying SFQ/IFB here at work, I found the allot management was
pretty wrong in sfq, even changing allot from short to int...

We should init allot for each new flow, not using a previous value found
in slot.

Before patch, I saw bursts of several packets per flow, apparently
denying the default "quantum 1514" limit I had on my SFQ class.

class sfq 11:1 parent 11: 
 (dropped 0, overlimits 0 requeues 0) 
 backlog 0b 7p requeues 0 
 allot 11546 

class sfq 11:46 parent 11: 
 (dropped 0, overlimits 0 requeues 0) 
 backlog 0b 1p requeues 0 
 allot -23873 

class sfq 11:78 parent 11: 
 (dropped 0, overlimits 0 requeues 0) 
 backlog 0b 5p requeues 0 
 allot 11393 

After patch, better fairness among each flow, allot limit being
respected.

class sfq 11:2e4 parent 11: 
 (dropped 0, overlimits 0 requeues 0) 
 backlog 0b 4p requeues 0 
 allot -1166 

class sfq 11:2f7 parent 11: 
 (dropped 0, overlimits 0 requeues 0) 
 backlog 0b 3p requeues 0 
 allot -1166 

class sfq 11:313 parent 11: 
 (dropped 0, overlimits 0 requeues 0) 
 backlog 0b 3p requeues 0 
 allot -1220 

class sfq 11:335 parent 11: 
 (dropped 0, overlimits 0 requeues 0) 
 backlog 0b 4p requeues 0 
 allot -1166 


Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 net/sched/sch_sfq.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index 3cf478d..065a2a5 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -321,14 +321,13 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 	sfq_inc(q, x);
 	if (q->qs[x].qlen == 1) {		/* The flow is new */
 		if (q->tail == SFQ_DEPTH) {	/* It is the first flow */
-			q->tail = x;
 			q->next[x] = x;
-			q->allot[x] = q->quantum;
 		} else {
 			q->next[x] = q->next[q->tail];
 			q->next[q->tail] = x;
-			q->tail = x;
 		}
+		q->tail = x;
+		q->allot[x] = q->quantum;
 	}
 	if (++sch->q.qlen <= q->limit) {
 		sch->bstats.bytes += qdisc_pkt_len(skb);



^ permalink raw reply related

* Re: After memory pressure: can't read from tape anymore
From: Vladislav Bolkhovitin @ 2010-12-15 16:27 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: James Bottomley, Lukas Kolbe, Kai Mäkisara, FUJITA Tomonori,
	linux-scsi, Kashyap Desai, netdev
In-Reply-To: <20101214142320.27d911d5@nehalam>

Stephen Hemminger, on 12/15/2010 01:23 AM wrote:
> On Tue, 14 Dec 2010 23:35:37 +0300
> Vladislav Bolkhovitin <vst@vlnb.net> wrote:
> 
>> What is interesting to me in this regard is how networking with 9K jumbo
>> frames manages to work acceptably reliable? Jumbo frames used
>> sufficiently often, including under high memory pressure.
>>
>> I'm not a deep networking guru, but network drivers need to allocate
>> physically continual memory for skbs, which means 16K per 9K packet,
>> which means order 2 allocations per skb.
> 
> Good network drivers support fragmentation and allocate a small portion
> for the header and allocate pages for the rest. This requires no higher
> order allocation. The networking stack takes fragmented data coming
> in and does the necessary copy/merging to access contiguous headers.
> 
> There are still some crap network drivers that require large contiguous
> allocation. These should not be used with jumbo frames in real
> environments.

I see. Thanks for clarifying it.

Vlad




^ permalink raw reply

* Re: [PATCH] net_sched: sch_sfq: fix allot handling
From: Eric Dumazet @ 2010-12-15 16:27 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: David Miller, netdev, Jarek Poplawski
In-Reply-To: <4D08E6C2.804@trash.net>

Le mercredi 15 décembre 2010 à 17:03 +0100, Patrick McHardy a écrit :
> On 15.12.2010 15:03, Eric Dumazet wrote:
> > When deploying SFQ/IFB here at work, I found the allot management was
> > pretty wrong in sfq, even changing allot from short to int...
> > 
> > We should init allot for each new flow turn, not using a previous value,
> > or else small packets can easily make allot overflow.
> > 
> > Before patch, I saw burst of several packets per flow, apparently
> > denying the "allot 1514" limit I had on my SFQ class.
> > 
> > class sfq 11:1 parent 11: 
> >  (dropped 0, overlimits 0 requeues 0) 
> >  backlog 0b 7p requeues 0 
> >  allot 11546 
> > 
> > class sfq 11:46 parent 11: 
> >  (dropped 0, overlimits 0 requeues 0) 
> >  backlog 0b 1p requeues 0 
> >  allot -23873 
> > 
> > class sfq 11:78 parent 11: 
> >  (dropped 0, overlimits 0 requeues 0) 
> >  backlog 0b 5p requeues 0 
> >  allot 11393 
> 
> These values definitely look wrong.
> 
> > diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
> > index 3cf478d..8c8a190 100644
> > --- a/net/sched/sch_sfq.c
> > +++ b/net/sched/sch_sfq.c
> > @@ -270,7 +270,7 @@ static unsigned int sfq_drop(struct Qdisc *sch)
> >  		/* It is difficult to believe, but ALL THE SLOTS HAVE LENGTH 1. */
> >  		d = q->next[q->tail];
> >  		q->next[q->tail] = q->next[d];
> > -		q->allot[q->next[d]] += q->quantum;
> > +		q->allot[q->next[d]] = q->quantum;
> >  		skb = q->qs[d].prev;
> >  		len = qdisc_pkt_len(skb);
> >  		__skb_unlink(skb, &q->qs[d]);
> 
> I'm not sure about this part, but lets ignore that for now since it
> shouldn't affect your testcase unless you're using CBQ.
> 




> > @@ -321,14 +321,13 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch)
> >  	sfq_inc(q, x);
> >  	if (q->qs[x].qlen == 1) {		/* The flow is new */
> >  		if (q->tail == SFQ_DEPTH) {	/* It is the first flow */
> > -			q->tail = x;
> >  			q->next[x] = x;
> > -			q->allot[x] = q->quantum;
> >  		} else {
> >  			q->next[x] = q->next[q->tail];
> >  			q->next[q->tail] = x;
> > -			q->tail = x;
> >  		}
> > +		q->tail = x;
> > +		q->allot[x] = q->quantum;
> >  	}
> 
> This looks correct, for new flows allot should be initialized from
> scratch.
> 
> >  	if (++sch->q.qlen <= q->limit) {
> >  		sch->bstats.bytes += qdisc_pkt_len(skb);
> > @@ -382,11 +381,11 @@ sfq_dequeue(struct Qdisc *sch)
> >  			return skb;
> >  		}
> >  		q->next[q->tail] = a;
> > -		q->allot[a] += q->quantum;
> > +		q->allot[a] = q->quantum;
> 
> The allot initialization doesn't seem necessary anymore at all
> now that you're reinitalizing allot for flows that became active
> unconditionally in sfq_enqueue().
> 



> >  	} else if ((q->allot[a] -= qdisc_pkt_len(skb)) <= 0) {
> >  		q->tail = a;
> >  		a = q->next[a];
> > -		q->allot[a] += q->quantum;
> > +		q->allot[a] = q->quantum;
> 
> This seems to break long-term fairness for active flows by not
> accounting for overshooting the allotment in the next round
> anymore.
> 
> I think either the change in sfq_enqueue() or the first change
> in sfq_dequeue() should be enough to fix the problem you're seeing.
> Basically what needs to be done is initialize allot once from
> scratch when the flow becomes active, then add one quantum per
> round while it stays active.

Hmm, you may be right, thanks a lot for reviewing !

I noticed that with normal quantum (1514), my SFQ setup was sending two
full frames per flow after my patch, so was about to prepare a new
version ;)

I'll post a v2 shortly.

Thanks



^ permalink raw reply

* Re: [PATCH 5/5 v2] net: add old_queue_mapping into skb->cb
From: Stephen Hemminger @ 2010-12-15 16:19 UTC (permalink / raw)
  To: Changli Gao
  Cc: Jamal Hadi Salim, David S. Miller, Eric Dumazet, Tom Herbert,
	Jiri Pirko, netdev, netem
In-Reply-To: <1292390636-3156-1-git-send-email-xiaosuo@gmail.com>

On Wed, 15 Dec 2010 13:23:56 +0800
Changli Gao <xiaosuo@gmail.com> wrote:

> For the skbs returned from ifb, we should use the queue_mapping
> saved before ifb.
> 
> We save old queue_mapping in old_queue_mapping just before calling 
> dev_queue_xmit, and restore the old_queue_mapping to queue_mapping
> just before reinjecting the skb.
> 
> dev_pick_tx() use the current queue_mapping for the skbs reinjected
> by ifb.
> 
> A new struct dev_skb_cb is added, and valid in qdisc and gso layer.
> The original qdisc_skb_cb and DEV_GSO_CB are placed after dev_skb_cb.
> 
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>

What about a more general mechanism that lets a layer push
some amount of data onto the skb and then pop it off.
Kind of link notes to self, maybe even encode them as netlink
(except netlink messages have excess padding).

This would allow nesting, and avoid cb[] clobbering.
The existing usage of cb[] is only because there wasn't a better
solution.

-- 

^ permalink raw reply

* Re: |PATCH net-next-2.6] ifb: use netif_receive_skb() instead of netif_rx()
From: Stephen Hemminger @ 2010-12-15 16:14 UTC (permalink / raw)
  To: hadi
  Cc: Eric Dumazet, Changli Gao, David S. Miller, Tom Herbert,
	Jiri Pirko, netdev, netem
In-Reply-To: <1292417363.2067.17.camel@mojatatu>

On Wed, 15 Dec 2010 07:49:23 -0500
jamal <hadi@cyberus.ca> wrote:

> On Wed, 2010-12-15 at 09:39 +0100, Eric Dumazet wrote:
> > In ri_tasklet(), we run from softirq, so can directly handle packet
> > through netif_receive_skb() instead of netif_rx().
> > There is no risk of recursion.
> 
> Eric, did you do at least a simple test on this one? 
> It used to be problematic (I cant remember why or
> what use case was problematic).

Only risk is stack overflow.

^ permalink raw reply


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