Netdev List
 help / color / mirror / Atom feed
* Re: [net 2/2] ixgbe: add warning when max_vfs is out of range.
From: David Miller @ 2013-11-11  5:46 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: jacob.e.keller, netdev, gospo, sassmann
In-Reply-To: <1384001552-18754-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Sat,  9 Nov 2013 04:52:32 -0800

> From: Jacob Keller <jacob.e.keller@intel.com>
> 
> The max_vfs parameter has a limit of 63 and silently fails (adding 0 vfs) when
> it is out of range. This patch adds a warning so that the user knows something
> went wrong. Also, this patch moves the warning in ixgbe_enable_sriov() to where
> max_vfs is checked, so that even an out of range value will show the deprecated
> warning. Previously, an out of range parameter didn't even warn the user to use
> the new sysfs interface instead.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

Applied.

^ permalink raw reply

* Re: [net 1/2] igb: Update link modes display in ethtool
From: David Miller @ 2013-11-11  5:46 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: carolyn.wyborny, netdev, gospo, sassmann
In-Reply-To: <1384001534-18709-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Sat,  9 Nov 2013 04:52:14 -0800

> From: Carolyn Wyborny <carolyn.wyborny@intel.com>
> 
> This patch fixes multiple problems in the link modes display in ethtool.
> Newer parts have more complicated methods to determine actual link
> capabilities.  Older parts cannot communicate with their SFP modules.
> Finally, all the available defines are not displayed by ethtool.  This
> updates the link modes to be as accurate as possible depending on what data
> is available to the driver at any given time.
> 
> Signed-off-by: Carolyn Wyborny <carolyn.wyborny@intel.com>
> Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next FIX] RDMA/cma: Fix build breakage when infiniband is built-in
From: David Miller @ 2013-11-11  5:43 UTC (permalink / raw)
  To: ogerlitz; +Cc: netdev, amirv, eyalpe
In-Reply-To: <20131110.210355.1669362951389274888.davem@davemloft.net>

From: David Miller <davem@davemloft.net>
Date: Sun, 10 Nov 2013 21:03:55 -0500 (EST)

> Please just move vlan_dev_get_egress_qos_mask into include/linux/if_vlan.h
> and make it an inline function.
> 
> These Kconfig restrictions are just beyond silly and unnecessary.
> 
> I bet the IPV6 one already there can be killed by moving some things
> into net/ipv6/*_core.c files as well.

I really want to merge the net-next tree as soon as possible so I
committed the following:

====================
[PATCH] vlan: Implement vlan_dev_get_egress_qos_mask as an inline.

This is to avoid very silly Kconfig dependencies for modules
using this routine.

Signed-off-by: David S. Miller <davem@davemloft.net>
---
 include/linux/if_vlan.h | 98 ++++++++++++++++++++++++++++++++++++++++++++++++-
 net/8021q/vlan.h        | 77 --------------------------------------
 net/8021q/vlan_dev.c    | 31 ++--------------
 3 files changed, 99 insertions(+), 107 deletions(-)

diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index f3088a0..f252deb 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -88,8 +88,102 @@ extern struct net_device *__vlan_find_dev_deep(struct net_device *real_dev,
 					       __be16 vlan_proto, u16 vlan_id);
 extern struct net_device *vlan_dev_real_dev(const struct net_device *dev);
 extern u16 vlan_dev_vlan_id(const struct net_device *dev);
-extern u16 vlan_dev_get_egress_qos_mask(struct net_device *dev,
-					u32 skprio);
+
+/**
+ *	struct vlan_priority_tci_mapping - vlan egress priority mappings
+ *	@priority: skb priority
+ *	@vlan_qos: vlan priority: (skb->priority << 13) & 0xE000
+ *	@next: pointer to next struct
+ */
+struct vlan_priority_tci_mapping {
+	u32					priority;
+	u16					vlan_qos;
+	struct vlan_priority_tci_mapping	*next;
+};
+
+/**
+ *	struct vlan_pcpu_stats - VLAN percpu rx/tx stats
+ *	@rx_packets: number of received packets
+ *	@rx_bytes: number of received bytes
+ *	@rx_multicast: number of received multicast packets
+ *	@tx_packets: number of transmitted packets
+ *	@tx_bytes: number of transmitted bytes
+ *	@syncp: synchronization point for 64bit counters
+ *	@rx_errors: number of rx errors
+ *	@tx_dropped: number of tx drops
+ */
+struct vlan_pcpu_stats {
+	u64			rx_packets;
+	u64			rx_bytes;
+	u64			rx_multicast;
+	u64			tx_packets;
+	u64			tx_bytes;
+	struct u64_stats_sync	syncp;
+	u32			rx_errors;
+	u32			tx_dropped;
+};
+
+struct proc_dir_entry;
+struct netpoll;
+
+/**
+ *	struct vlan_dev_priv - VLAN private device data
+ *	@nr_ingress_mappings: number of ingress priority mappings
+ *	@ingress_priority_map: ingress priority mappings
+ *	@nr_egress_mappings: number of egress priority mappings
+ *	@egress_priority_map: hash of egress priority mappings
+ *	@vlan_proto: VLAN encapsulation protocol
+ *	@vlan_id: VLAN identifier
+ *	@flags: device flags
+ *	@real_dev: underlying netdevice
+ *	@real_dev_addr: address of underlying netdevice
+ *	@dent: proc dir entry
+ *	@vlan_pcpu_stats: ptr to percpu rx stats
+ */
+struct vlan_dev_priv {
+	unsigned int				nr_ingress_mappings;
+	u32					ingress_priority_map[8];
+	unsigned int				nr_egress_mappings;
+	struct vlan_priority_tci_mapping	*egress_priority_map[16];
+
+	__be16					vlan_proto;
+	u16					vlan_id;
+	u16					flags;
+
+	struct net_device			*real_dev;
+	unsigned char				real_dev_addr[ETH_ALEN];
+
+	struct proc_dir_entry			*dent;
+	struct vlan_pcpu_stats __percpu		*vlan_pcpu_stats;
+#ifdef CONFIG_NET_POLL_CONTROLLER
+	struct netpoll				*netpoll;
+#endif
+};
+
+static inline struct vlan_dev_priv *vlan_dev_priv(const struct net_device *dev)
+{
+	return netdev_priv(dev);
+}
+
+static inline u16
+vlan_dev_get_egress_qos_mask(struct net_device *dev, u32 skprio)
+{
+	struct vlan_priority_tci_mapping *mp;
+
+	smp_rmb(); /* coupled with smp_wmb() in vlan_dev_set_egress_priority() */
+
+	mp = vlan_dev_priv(dev)->egress_priority_map[(skprio & 0xF)];
+	while (mp) {
+		if (mp->priority == skprio) {
+			return mp->vlan_qos; /* This should already be shifted
+					      * to mask correctly with the
+					      * VLAN's TCI */
+		}
+		mp = mp->next;
+	}
+	return 0;
+}
+
 extern bool vlan_do_receive(struct sk_buff **skb);
 extern struct sk_buff *vlan_untag(struct sk_buff *skb);
 
diff --git a/net/8021q/vlan.h b/net/8021q/vlan.h
index a2caf00..5704ed9 100644
--- a/net/8021q/vlan.h
+++ b/net/8021q/vlan.h
@@ -5,83 +5,6 @@
 #include <linux/u64_stats_sync.h>
 #include <linux/list.h>
 
-
-/**
- *	struct vlan_priority_tci_mapping - vlan egress priority mappings
- *	@priority: skb priority
- *	@vlan_qos: vlan priority: (skb->priority << 13) & 0xE000
- *	@next: pointer to next struct
- */
-struct vlan_priority_tci_mapping {
-	u32					priority;
-	u16					vlan_qos;
-	struct vlan_priority_tci_mapping	*next;
-};
-
-
-/**
- *	struct vlan_pcpu_stats - VLAN percpu rx/tx stats
- *	@rx_packets: number of received packets
- *	@rx_bytes: number of received bytes
- *	@rx_multicast: number of received multicast packets
- *	@tx_packets: number of transmitted packets
- *	@tx_bytes: number of transmitted bytes
- *	@syncp: synchronization point for 64bit counters
- *	@rx_errors: number of rx errors
- *	@tx_dropped: number of tx drops
- */
-struct vlan_pcpu_stats {
-	u64			rx_packets;
-	u64			rx_bytes;
-	u64			rx_multicast;
-	u64			tx_packets;
-	u64			tx_bytes;
-	struct u64_stats_sync	syncp;
-	u32			rx_errors;
-	u32			tx_dropped;
-};
-
-struct netpoll;
-
-/**
- *	struct vlan_dev_priv - VLAN private device data
- *	@nr_ingress_mappings: number of ingress priority mappings
- *	@ingress_priority_map: ingress priority mappings
- *	@nr_egress_mappings: number of egress priority mappings
- *	@egress_priority_map: hash of egress priority mappings
- *	@vlan_proto: VLAN encapsulation protocol
- *	@vlan_id: VLAN identifier
- *	@flags: device flags
- *	@real_dev: underlying netdevice
- *	@real_dev_addr: address of underlying netdevice
- *	@dent: proc dir entry
- *	@vlan_pcpu_stats: ptr to percpu rx stats
- */
-struct vlan_dev_priv {
-	unsigned int				nr_ingress_mappings;
-	u32					ingress_priority_map[8];
-	unsigned int				nr_egress_mappings;
-	struct vlan_priority_tci_mapping	*egress_priority_map[16];
-
-	__be16					vlan_proto;
-	u16					vlan_id;
-	u16					flags;
-
-	struct net_device			*real_dev;
-	unsigned char				real_dev_addr[ETH_ALEN];
-
-	struct proc_dir_entry			*dent;
-	struct vlan_pcpu_stats __percpu		*vlan_pcpu_stats;
-#ifdef CONFIG_NET_POLL_CONTROLLER
-	struct netpoll				*netpoll;
-#endif
-};
-
-static inline struct vlan_dev_priv *vlan_dev_priv(const struct net_device *dev)
-{
-	return netdev_priv(dev);
-}
-
 /* if this changes, algorithm will have to be reworked because this
  * depends on completely exhausting the VLAN identifier space.  Thus
  * it gives constant time look-up, but in many cases it wastes memory.
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 13904a4..8db1b98 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -68,31 +68,6 @@ static int vlan_dev_rebuild_header(struct sk_buff *skb)
 	return 0;
 }
 
-static inline u16
-__vlan_dev_get_egress_qos_mask(struct net_device *dev, u32 skprio)
-{
-	struct vlan_priority_tci_mapping *mp;
-
-	smp_rmb(); /* coupled with smp_wmb() in vlan_dev_set_egress_priority() */
-
-	mp = vlan_dev_priv(dev)->egress_priority_map[(skprio & 0xF)];
-	while (mp) {
-		if (mp->priority == skprio) {
-			return mp->vlan_qos; /* This should already be shifted
-					      * to mask correctly with the
-					      * VLAN's TCI */
-		}
-		mp = mp->next;
-	}
-	return 0;
-}
-
-u16 vlan_dev_get_egress_qos_mask(struct net_device *dev, u32 skprio)
-{
-	return __vlan_dev_get_egress_qos_mask(dev, skprio);
-}
-EXPORT_SYMBOL(vlan_dev_get_egress_qos_mask);
-
 /*
  *	Create the VLAN header for an arbitrary protocol layer
  *
@@ -117,7 +92,7 @@ static int vlan_dev_hard_header(struct sk_buff *skb, struct net_device *dev,
 		vhdr = (struct vlan_hdr *) skb_push(skb, VLAN_HLEN);
 
 		vlan_tci = vlan->vlan_id;
-		vlan_tci |= __vlan_dev_get_egress_qos_mask(dev, skb->priority);
+		vlan_tci |= vlan_dev_get_egress_qos_mask(dev, skb->priority);
 		vhdr->h_vlan_TCI = htons(vlan_tci);
 
 		/*
@@ -174,7 +149,7 @@ static netdev_tx_t vlan_dev_hard_start_xmit(struct sk_buff *skb,
 	    vlan->flags & VLAN_FLAG_REORDER_HDR) {
 		u16 vlan_tci;
 		vlan_tci = vlan->vlan_id;
-		vlan_tci |= __vlan_dev_get_egress_qos_mask(dev, skb->priority);
+		vlan_tci |= vlan_dev_get_egress_qos_mask(dev, skb->priority);
 		skb = __vlan_hwaccel_put_tag(skb, vlan->vlan_proto, vlan_tci);
 	}
 
@@ -259,7 +234,7 @@ int vlan_dev_set_egress_priority(const struct net_device *dev,
 	np->vlan_qos = vlan_qos;
 	/* Before inserting this element in hash table, make sure all its fields
 	 * are committed to memory.
-	 * coupled with smp_rmb() in __vlan_dev_get_egress_qos_mask()
+	 * coupled with smp_rmb() in vlan_dev_get_egress_qos_mask()
 	 */
 	smp_wmb();
 	vlan->egress_priority_map[skb_prio & 0xF] = np;
-- 
1.7.11.7

^ permalink raw reply related

* TCP performance regression
From: Sujith Manoharan @ 2013-11-11  5:30 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev

Hi,

The commit, "tcp: TSQ can use a dynamic limit" causes a large
performance drop in TCP transmission with the wireless driver ath9k.

With a 2-stream card (AR9462), the usual throughput is around 195 Mbps.
But, with this commit, it drops to ~125 Mbps, occasionally reaching 130.

If the commit is reverted, performance is normal again and I can get
190+ Mbps. Apparently, ath10k is also affected and a 250 Mbps drop
is seen (from an original 740 Mbps).

I am using Linville's wireless-testing tree.

>From the test machine:

root@linux-test ~# uname -a
Linux linux-test 3.12.0-wl-nodebug #104 SMP PREEMPT Mon Nov 11 10:27:56 IST 2013 x86_64 GNU/Linux

root@linux-test ~# tc -d -s qdisc show dev wlan0 
qdisc mq 0: root 
 Sent 342682272 bytes 226366 pkt (dropped 0, overlimits 0 requeues 0) 
 backlog 0b 0p requeues 0 

root@linux-test ~# zgrep -i net_sch /proc/config.gz
CONFIG_NET_SCHED=y
CONFIG_NET_SCH_CBQ=m
CONFIG_NET_SCH_HTB=m
CONFIG_NET_SCH_HFSC=m
CONFIG_NET_SCH_ATM=m
CONFIG_NET_SCH_PRIO=m
CONFIG_NET_SCH_MULTIQ=m
CONFIG_NET_SCH_RED=m
CONFIG_NET_SCH_SFB=m
CONFIG_NET_SCH_SFQ=m
CONFIG_NET_SCH_TEQL=m
CONFIG_NET_SCH_TBF=m
CONFIG_NET_SCH_GRED=m
CONFIG_NET_SCH_DSMARK=m
CONFIG_NET_SCH_NETEM=m
CONFIG_NET_SCH_DRR=m
CONFIG_NET_SCH_MQPRIO=m
CONFIG_NET_SCH_CHOKE=m
CONFIG_NET_SCH_QFQ=m
CONFIG_NET_SCH_CODEL=m
CONFIG_NET_SCH_FQ_CODEL=m
CONFIG_NET_SCH_FQ=m
CONFIG_NET_SCH_INGRESS=m
# CONFIG_NET_SCH_PLUG is not set
CONFIG_NET_SCH_FIFO=y


If more information is required, please let me know.

Sujith

^ permalink raw reply

* Re: [RFC PATCH net-next 2/2] udp: add sk opt to allow sending pkt with src 0.0.0.0
From: David Miller @ 2013-11-11  5:18 UTC (permalink / raw)
  To: nicolas.dichtel
  Cc: hannes, netdev, David.Laight, jiri, vyasevich, kuznet, jmorris,
	yoshfuji, kaber, thaller, stephen
In-Reply-To: <1383958474-6255-2-git-send-email-nicolas.dichtel@6wind.com>

From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Sat,  9 Nov 2013 01:54:34 +0100

> This feature allows to a send packets with address source set to 0.0.0.0 even if
> an ip address is available on another interface.
> 
> It's useful for DHCP client, to allow them to use UDP sockets and be compliant
> with the RFC2131, Section 4.1:
> 
> 4.1 Constructing and sending DHCP messages
> ...
>    DHCP messages broadcast by a client prior to that client obtaining
>    its IP address must have the source address field in the IP header
>    set to 0.
> 
> Based on a previous work from
> Guillaume Gaudonville <guillaume.gaudonville@6wind.com>.
> 
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

This requirement of the RFC is inconsistent with a host based
addressing model, that which Linux employs, it assumes an interface
based one.

The wording here is also very ambiguous.

This RFC fails to even remotely consider what the right behavior
should be in a host based addressing environment at all, and anyone
reading this RFC should just accept that.

Furthermore, the fact that you're implementing _addressing_ policy in
the UDP code makes this change even more unreasonable.

^ permalink raw reply

* Re: wrt. core/dev: set pkt_type after eth_type_trans() in dev_forward_skb()
From: Maciej Żenczykowski @ 2013-11-11  5:15 UTC (permalink / raw)
  To: Linux NetDev, Eric Dumazet, David Miller, Isaku Yamahata,
	David Decotigny, Alexei Starovoitov
In-Reply-To: <CAHo-OoysAJRagNagE4KF-5AXyFHt+X8tahmnRfgaNhww6V-ifw@mail.gmail.com>

Here's the previous email on this subject.

^ permalink raw reply

* Re: spurious ip forwarding inside namespaces
From: Maciej Żenczykowski @ 2013-11-11  5:12 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Isaku Yamahata, Nicolas Dichtel, David S. Miller, Linux NetDev
In-Reply-To: <CAMEtUuwKD76xEhysPbLum5wDYyJaX5p0JpdBQUcdHged40eHdg@mail.gmail.com>

I've already suggested a month or so back that that patch should be reverted.

On Tue, Nov 5, 2013 at 8:32 PM, Alexei Starovoitov <ast@plumgrid.com> wrote:
> Hi All,
>
> commit 06a23fe31c
> "core/dev: set pkt_type after eth_type_trans() in dev_forward_skb()"
> and refactoring 64261f230a
> "dev: move skb_scrub_packet() after eth_type_trans()"
>
> are forcing pkt_type to be PACKET_HOST when skb traverses veth.
>
> which means that ip forwarding will kick in inside netns
> even if skb->eth->h_dest != dev->dev_addr
>
> The way we hit the problem is the following:
> create 4 or more of netns with veth and attach them to a bridge.
> disable learning on a bridge (via setageing 0), so it will flood
> packets to all namespaces.
> Since ip forwarding and icmp redirect by default is on,
> single icmp request packet from one netns creates a storm of packets,
> since every netns receives it and incorrectly does ip forward on it
> and also incorrectly sends icmp redirect which are multiplied once
> again by flooding in the bridge.
> ping -c1 generates 1 outgoing packet and ~13K incoming packets with
> just 4 netns.
> Running any tcp traffic between such namespaces becomes impossible
> and ksoftirqd cpu usage goes to 100% on all cpus.
>
> Please suggest how to fix this cleanly.
>
> Thanks
> Alexei
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH V2 net-next] xfrm: check function pointer of xfrm_mgr before use it
From: David Miller @ 2013-11-11  5:09 UTC (permalink / raw)
  To: baker.kernel; +Cc: herbert, steffen.klassert, netdev
In-Reply-To: <1384126317-3687-1-git-send-email-baker.kernel@gmail.com>

From: baker.kernel@gmail.com
Date: Mon, 11 Nov 2013 07:31:57 +0800

> From: "baker.zhang" <baker.kernel@gmail.com>
> 
> Signed-off-by: baker.zhang <baker.kernel@gmail.com>

It is not valid to register a key manager that does not implement
these callbacks.

I'm not applying this patch, sorry.  Although I would apply a patch
that validates that all the callbacks are non-NULL at register time.

^ permalink raw reply

* [PATCH] usbnet: fix race condition caused spinlock bad magic issue
From: wangbiao @ 2013-11-11  3:08 UTC (permalink / raw)
  To: oneukum, netdev, linux-usb, linux-kernel
  Cc: akpm, mingo, a.p.zijlstra, rusty, william.douglas, biao.wang

From: wang, biao <biao.wang@intel.com>
Date: Mon, 11 Nov 2013 10:23:40 +0800
Subject: [PATCH] usbnet: fix race condition caused spinlock bad magic issue

there is race between usbnet_terminate_urbs and usbnet_bh, when
unlink_wakeup used in usbnet_bh, it may be already freed and used by
other function as unlink_wakeup was a local var on stack.

btw, dev->wait should be judged again before use it as there is race
too.

Signed-off-by: wang, biao <biao.wang@intel.com>
---
 drivers/net/usb/usbnet.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 90a429b..22fc27f 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -86,6 +86,7 @@ static const char driver_name [] = "usbnet";
 
 /* use ethtool to change the level for any given device */
 static int msg_level = -1;
+static wait_queue_head_t unlink_wakeup;
 module_param (msg_level, int, 0);
 MODULE_PARM_DESC (msg_level, "Override default message level");
 
@@ -761,7 +762,6 @@ EXPORT_SYMBOL_GPL(usbnet_unlink_rx_urbs);
 // precondition: never called in_interrupt
 static void usbnet_terminate_urbs(struct usbnet *dev)
 {
-	DECLARE_WAIT_QUEUE_HEAD_ONSTACK(unlink_wakeup);
 	DECLARE_WAITQUEUE(wait, current);
 	int temp;
 
@@ -1448,8 +1448,10 @@ static void usbnet_bh (unsigned long param)
 
 	// waiting for all pending urbs to complete?
 	if (dev->wait) {
+		wait_queue_head_t *wait_d = dev->wait;
 		if ((dev->txq.qlen + dev->rxq.qlen + dev->done.qlen) == 0) {
-			wake_up (dev->wait);
+			if (wait_d)
+				wake_up(wait_d);
 		}
 
 	// or are we maybe short a few urbs?
@@ -1602,6 +1604,7 @@ usbnet_probe (struct usb_interface *udev, const struct usb_device_id *prod)
 	init_timer (&dev->delay);
 	mutex_init (&dev->phy_mutex);
 	mutex_init(&dev->interrupt_mutex);
+	init_waitqueue_head(&unlink_wakeup);
 	dev->interrupt_count = 0;
 
 	dev->net = net;
-- 
1.7.0.4

^ permalink raw reply related

* [PATCH] ipv6: match those routes that have different metirc
From: Duan Jiong @ 2013-11-11  3:00 UTC (permalink / raw)
  To: David Miller; +Cc: netdev


Now the kernel only match those routes that have the same
metirc, so if those routes are non-reachable, then the packets
can't be sent out.

But according to the rfc 4191 section 3.2, if the best route
points to a non-reachable route, the next best route should be
consulted. So the kernel should not only match those routes that
have minimum metric, and should also match others.

Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
---
 net/ipv6/route.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 7faa9d5..96cd22a 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -629,7 +629,7 @@ static struct rt6_info *find_match(struct rt6_info *rt, int oif, int strict,
 	if (strict & RT6_LOOKUP_F_REACHABLE)
 		rt6_probe(rt);
 
-	if (m > *mpri) {
+	if (m > *mpri && (!match || rt->rt6i_metric == match->rt6i_metric)) {
 		*do_rr = match_do_rr;
 		*mpri = m;
 		match = rt;
@@ -639,19 +639,13 @@ out:
 }
 
 static struct rt6_info *find_rr_leaf(struct fib6_node *fn,
-				     struct rt6_info *rr_head,
-				     u32 metric, int oif, int strict,
-				     bool *do_rr)
+				     int oif, int strict, bool *do_rr)
 {
 	struct rt6_info *rt, *match;
 	int mpri = -1;
 
 	match = NULL;
-	for (rt = rr_head; rt && rt->rt6i_metric == metric;
-	     rt = rt->dst.rt6_next)
-		match = find_match(rt, oif, strict, &mpri, match, do_rr);
-	for (rt = fn->leaf; rt && rt != rr_head && rt->rt6i_metric == metric;
-	     rt = rt->dst.rt6_next)
+	for (rt = fn->leaf; rt; rt = rt->dst.rt6_next)
 		match = find_match(rt, oif, strict, &mpri, match, do_rr);
 
 	return match;
@@ -667,14 +661,13 @@ static struct rt6_info *rt6_select(struct fib6_node *fn, int oif, int strict)
 	if (!rt0)
 		fn->rr_ptr = rt0 = fn->leaf;
 
-	match = find_rr_leaf(fn, rt0, rt0->rt6i_metric, oif, strict,
-			     &do_rr);
+	match = find_rr_leaf(fn, oif, strict, &do_rr);
 
 	if (do_rr) {
 		struct rt6_info *next = rt0->dst.rt6_next;
 
 		/* no entries matched; do round-robin */
-		if (!next || next->rt6i_metric != rt0->rt6i_metric)
+		if (!next)
 			next = fn->leaf;
 
 		if (next != rt0)
-- 
1.8.3.1

^ permalink raw reply related

* Re: [PATCH net-next FIX] RDMA/cma: Fix build breakage when infiniband is built-in
From: David Miller @ 2013-11-11  2:03 UTC (permalink / raw)
  To: ogerlitz; +Cc: netdev, amirv, eyalpe
In-Reply-To: <1384090080-1862-1-git-send-email-ogerlitz@mellanox.com>

From: Or Gerlitz <ogerlitz@mellanox.com>
Date: Sun, 10 Nov 2013 15:28:00 +0200

> From: Eyal Perry <eyalpe@mellanox.com>
> 
> Commit eb072c4 ("RDMA/cma: Set IBoE SL (user-priority) by egress map when using
> vlans"), broke the build when CONFIG_VLAN_8021Q=m and CONFIG_INFINIBAND=y,
> because the rdma_cm module attempted to call vlan_dev_get_egress_qos_mask()
> which in that case is a modular code being called from built-in kernel code.
> 
> Fix this by adding a Kconfig dependency to prevent the rdma_cm code from being
> built in when 8021Q is built modular, in a similar manner we do with IPv6.
> 
> Signed-off-by: Eyal Perry <eyalpe@mellanox.com>
> Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
> ---
> 
> Dave, this fixes an issue with the mentioned commit which is in net-next

Please just move vlan_dev_get_egress_qos_mask into include/linux/if_vlan.h
and make it an inline function.

These Kconfig restrictions are just beyond silly and unnecessary.

I bet the IPV6 one already there can be killed by moving some things
into net/ipv6/*_core.c files as well.

^ permalink raw reply

* Re: [PATCH net-next 2/5] sctp: remove the else path
From: wangweidong @ 2013-11-11  1:57 UTC (permalink / raw)
  To: Sergei Shtylyov, vyasevich, nhorman
  Cc: dingtianhong, davem, linux-sctp, netdev
In-Reply-To: <527D3949.2000302@cogentembedded.com>

On 2013/11/9 3:19, Sergei Shtylyov wrote:
> Hello.
> 
> On 11/08/2013 05:55 AM, Wang Weidong wrote:
> 
>> Make the code more simplification.
> 
>> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
>> ---
>>   net/sctp/associola.c | 4 +---
>>   1 file changed, 1 insertion(+), 3 deletions(-)
> 
>> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
>> index 667f984..2d53d4c 100644
>> --- a/net/sctp/associola.c
>> +++ b/net/sctp/associola.c
>> @@ -954,15 +954,13 @@ int sctp_cmp_addr_exact(const union sctp_addr *ss1,
>>    */
>>   struct sctp_chunk *sctp_get_ecne_prepend(struct sctp_association *asoc)
>>   {
>> -    struct sctp_chunk *chunk;
>> +    struct sctp_chunk *chunk = NULL;
>>
>>       /* Send ECNE if needed.
>>        * Not being able to allocate a chunk here is not deadly.
>>        */
>>       if (asoc->need_ecne)
>>           chunk = sctp_make_ecne(asoc, asoc->last_ecne_tsn);
>> -    else
>> -        chunk = NULL;
> 
>    This is hardly better/simpler than it was.
> 
> WBR, Sergei
> 

Well, you are right. Joe had suggested a good idea to me. And I had fixed
it in the v2.
Thanks. 

> 
> 

^ permalink raw reply

* Re: [PATCH 2/2 net-next] openvswitch: Use skb_zerocopy() for upcall
From: Jesse Gross @ 2013-11-11  1:55 UTC (permalink / raw)
  To: Thomas Graf
  Cc: Ben Hutchings, David Miller, dev@openvswitch.org, netdev,
	Eric Dumazet
In-Reply-To: <20131110234033.GA18099@casper.infradead.org>

On Mon, Nov 11, 2013 at 7:40 AM, Thomas Graf <tgraf@suug.ch> wrote:
> On 11/09/13 at 10:02pm, Ben Hutchings wrote:
>> On Fri, 2013-11-08 at 10:15 +0100, Thomas Graf wrote:
>> > Use of skb_zerocopy() avoids the expensive call to memcpy() when
>> > copying the packet data into the Netlink skb. Completes checksum
>> > through skb_checksum_help() if needed.
>> >
>> > Netlink messaged must be properly padded and aligned to meet
>> > sanity checks of the user space counterpart.
>> >
>> > Cost of memcpy is significantly reduced from:
>> > +   7.48%       vhost-8471  [k] memcpy
>> > +   5.57%     ovs-vswitchd  [k] memcpy
>> > +   2.81%       vhost-8471  [k] csum_partial_copy_generic
>> >
>> > to:
>> > +   5.72%     ovs-vswitchd  [k] memcpy
>> > +   3.32%       vhost-5153  [k] memcpy
>> > +   0.68%       vhost-5153  [k] skb_zerocopy
>> >
>> > (megaflows disabled)
>> >
>> > Signed-off-by: Thomas Graf <tgraf@suug.ch>
>> > ---
>> >  net/openvswitch/datapath.c | 52 +++++++++++++++++++++++++++++++++++++++-------
>> >  1 file changed, 45 insertions(+), 7 deletions(-)
>> >
>> > diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
>> > index 1408adc..3f170e3 100644
>> > --- a/net/openvswitch/datapath.c
>> > +++ b/net/openvswitch/datapath.c
>> [...]
>> > @@ -441,13 +449,43 @@ static int queue_userspace_packet(struct net *net, int dp_ifindex,
>> >                       nla_len(upcall_info->userdata),
>> >                       nla_data(upcall_info->userdata));
>> >
>> > -   nla = __nla_reserve(user_skb, OVS_PACKET_ATTR_PACKET, skb->len);
>> > +   /* Only reserve room for attribute header, packet data is added
>> > +    * in skb_zerocopy() */
>> > +   if (!(nla = nla_reserve(user_skb, OVS_PACKET_ATTR_PACKET, 0)))
>> > +           goto out;
>> > +   nla->nla_len = nla_attr_size(skb->len);
>> >
>> > -   skb_copy_and_csum_dev(skb, nla_data(nla));
>> > +   skb_zerocopy(user_skb, skb, skb->len, hlen);
>> >
>> > -   genlmsg_end(user_skb, upcall);
>> > -   err = genlmsg_unicast(net, user_skb, upcall_info->portid);
>> > +   /* OVS user space expects the size of the message to be aligned to
>> > +    * NLA_ALIGNTO. Aligning nlmsg_len is not enough, the actual bytes
>> > +    * read must match nlmsg_len.
>> > +    */
>> > +   plen = NLA_ALIGN(user_skb->len) - user_skb->len;
>> > +   if (plen > 0) {
>> > +           int nr_frags = skb_shinfo(user_skb)->nr_frags;
>> > +
>> > +           if (nr_frags) {
>> > +                   skb_frag_t *frag;
>> > +
>> > +                   frag = &skb_shinfo(user_skb)->frags[nr_frags -1];
>> > +                   skb_frag_size_add(frag, plen);
>>
>> It looks like this is effectively padding with whatever happens to
>> follow the original packet content.  This could result in a small
>> information leak.  If the fragment has non-zero offset and already
>> extends to the end of a page, this could result in a segfault as the
>> next page may be unmapped.
>>
>> Perhaps you could add the padding as an extra fragment pointing to a
>> preallocated zero page.  If the skb already has the maximum number of
>> fragments, you would have to copy the last fragment in order to add
>> padding.
>
> You are right and thanks for the review Ben.
>
> Realizing how complex this becomes I'm leaning towards avoiding
> padding alltogether by fixing OVS user space to no longer enforce
> it, signal this capability via a flag to the kernel and only
> perform zerocopy for enabled OVS user space counterparts.

It seems like at a minimum it would be a good idea to start by
patching userspace now. That would at least begin to limit the scope
of the problem.

^ permalink raw reply

* Re: [PATCH 2/2 net-next] openvswitch: Use skb_zerocopy() for upcall
From: Thomas Graf @ 2013-11-10 23:40 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: dev-yBygre7rU0TnMu66kgdUjQ, netdev-u79uwXL29TY76Z2rM5mHXA,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q, eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <1384034549.3802.41.camel-/LGg1Z1CJKQ+9kgCwbf1HqK4ta4zdZpAajtMo4Cw6ucAvxtiuMwx3w@public.gmane.org>

On 11/09/13 at 10:02pm, Ben Hutchings wrote:
> On Fri, 2013-11-08 at 10:15 +0100, Thomas Graf wrote:
> > Use of skb_zerocopy() avoids the expensive call to memcpy() when
> > copying the packet data into the Netlink skb. Completes checksum
> > through skb_checksum_help() if needed.
> > 
> > Netlink messaged must be properly padded and aligned to meet
> > sanity checks of the user space counterpart.
> > 
> > Cost of memcpy is significantly reduced from:
> > +   7.48%       vhost-8471  [k] memcpy
> > +   5.57%     ovs-vswitchd  [k] memcpy
> > +   2.81%       vhost-8471  [k] csum_partial_copy_generic
> > 
> > to:
> > +   5.72%     ovs-vswitchd  [k] memcpy
> > +   3.32%       vhost-5153  [k] memcpy
> > +   0.68%       vhost-5153  [k] skb_zerocopy
> > 
> > (megaflows disabled)
> > 
> > Signed-off-by: Thomas Graf <tgraf-G/eBtMaohhA@public.gmane.org>
> > ---
> >  net/openvswitch/datapath.c | 52 +++++++++++++++++++++++++++++++++++++++-------
> >  1 file changed, 45 insertions(+), 7 deletions(-)
> > 
> > diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> > index 1408adc..3f170e3 100644
> > --- a/net/openvswitch/datapath.c
> > +++ b/net/openvswitch/datapath.c
> [...]
> > @@ -441,13 +449,43 @@ static int queue_userspace_packet(struct net *net, int dp_ifindex,
> >  			  nla_len(upcall_info->userdata),
> >  			  nla_data(upcall_info->userdata));
> >  
> > -	nla = __nla_reserve(user_skb, OVS_PACKET_ATTR_PACKET, skb->len);
> > +	/* Only reserve room for attribute header, packet data is added
> > +	 * in skb_zerocopy() */
> > +	if (!(nla = nla_reserve(user_skb, OVS_PACKET_ATTR_PACKET, 0)))
> > +		goto out;
> > +	nla->nla_len = nla_attr_size(skb->len);
> >  
> > -	skb_copy_and_csum_dev(skb, nla_data(nla));
> > +	skb_zerocopy(user_skb, skb, skb->len, hlen);
> >  
> > -	genlmsg_end(user_skb, upcall);
> > -	err = genlmsg_unicast(net, user_skb, upcall_info->portid);
> > +	/* OVS user space expects the size of the message to be aligned to
> > +	 * NLA_ALIGNTO. Aligning nlmsg_len is not enough, the actual bytes
> > +	 * read must match nlmsg_len.
> > +	 */
> > +	plen = NLA_ALIGN(user_skb->len) - user_skb->len;
> > +	if (plen > 0) {
> > +		int nr_frags = skb_shinfo(user_skb)->nr_frags;
> > +
> > +		if (nr_frags) {
> > +			skb_frag_t *frag;
> > +
> > +			frag = &skb_shinfo(user_skb)->frags[nr_frags -1];
> > +			skb_frag_size_add(frag, plen);
> 
> It looks like this is effectively padding with whatever happens to
> follow the original packet content.  This could result in a small
> information leak.  If the fragment has non-zero offset and already
> extends to the end of a page, this could result in a segfault as the
> next page may be unmapped.
> 
> Perhaps you could add the padding as an extra fragment pointing to a
> preallocated zero page.  If the skb already has the maximum number of
> fragments, you would have to copy the last fragment in order to add
> padding.

You are right and thanks for the review Ben.

Realizing how complex this becomes I'm leaning towards avoiding
padding alltogether by fixing OVS user space to no longer enforce
it, signal this capability via a flag to the kernel and only
perform zerocopy for enabled OVS user space counterparts.

^ permalink raw reply

* [PATCH V2 net-next] xfrm: check function pointer of xfrm_mgr before use it
From: baker.kernel @ 2013-11-10 23:31 UTC (permalink / raw)
  To: herbert, davem, steffen.klassert; +Cc: netdev, baker.zhang
In-Reply-To: <1384093556-8987-1-git-send-email-baker.kernel@gmail.com>

From: "baker.zhang" <baker.kernel@gmail.com>

Signed-off-by: baker.zhang <baker.kernel@gmail.com>
---
V1:
For current kernel source, there is no problem.

In our vpn product, we need a xfrm_km in kernel module
to monitor the xfrm state change.
thus, the 'acquire' and 'compile_policy' may be NULL.

So I think we should do the check before use it.

V2:
Align the continuation line according the networking coding style.

 net/xfrm/xfrm_state.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index b9c3f9e..d716031 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -1679,9 +1679,11 @@ int km_query(struct xfrm_state *x, struct xfrm_tmpl *t, struct xfrm_policy *pol)
 
 	rcu_read_lock();
 	list_for_each_entry_rcu(km, &xfrm_km_list, list) {
-		acqret = km->acquire(x, t, pol);
-		if (!acqret)
-			err = acqret;
+		if (km->acquire) {
+			acqret = km->acquire(x, t, pol);
+			if (!acqret)
+				err = acqret;
+		}
 	}
 	rcu_read_unlock();
 	return err;
@@ -1783,10 +1785,12 @@ int xfrm_user_policy(struct sock *sk, int optname, u8 __user *optval, int optlen
 	err = -EINVAL;
 	rcu_read_lock();
 	list_for_each_entry_rcu(km, &xfrm_km_list, list) {
-		pol = km->compile_policy(sk, optname, data,
-					 optlen, &err);
-		if (err >= 0)
-			break;
+		if (km->compile_policy) {
+			pol = km->compile_policy(sk, optname, data,
+						 optlen, &err);
+			if (err >= 0)
+				break;
+		}
 	}
 	rcu_read_unlock();
 
-- 
1.8.3.2

^ permalink raw reply related

* [PATCH v2] mac80211: add assoc beacon timeout logic
From: Felipe Contreras @ 2013-11-10 21:45 UTC (permalink / raw)
  To: linux-wireless-u79uwXL29TY76Z2rM5mHXA
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, John W. Linville, Johannes Berg,
	David S. Miller, Felipe Contreras

We don't want to be waiting forever for a beacon that will never come,
just continue the association.

Signed-off-by: Felipe Contreras <felipe.contreras-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---

The previous version depended on some cleanup patches, plus had some unclear
rerun logic.

 net/mac80211/ieee80211_i.h |  1 +
 net/mac80211/mlme.c        | 32 ++++++++++++++++++++++++++------
 2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 611abfc..e1f858d 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -358,6 +358,7 @@ struct ieee80211_mgd_assoc_data {
 	const u8 *supp_rates;
 
 	unsigned long timeout;
+	unsigned long beacon_timeout;
 	int tries;
 
 	u16 capability;
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 86e4ad5..68f76f7 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -38,6 +38,7 @@
 #define IEEE80211_ASSOC_TIMEOUT		(HZ / 5)
 #define IEEE80211_ASSOC_TIMEOUT_LONG	(HZ / 2)
 #define IEEE80211_ASSOC_TIMEOUT_SHORT	(HZ / 10)
+#define IEEE80211_ASSOC_BEACON_TIMEOUT	2 * HZ
 #define IEEE80211_ASSOC_MAX_TRIES	3
 
 static int max_nullfunc_tries = 2;
@@ -3475,6 +3476,24 @@ void ieee80211_mgd_conn_tx_status(struct ieee80211_sub_if_data *sdata,
 	ieee80211_queue_work(&local->hw, &sdata->work);
 }
 
+static int check_beacon(struct ieee80211_sub_if_data *sdata)
+{
+	struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
+	struct ieee80211_mgd_assoc_data *assoc_data = ifmgd->assoc_data;
+
+	if (!assoc_data->need_beacon || ifmgd->have_beacon)
+		return true;
+
+	if (time_after(jiffies, assoc_data->beacon_timeout)) {
+		sdata_info(sdata, "no beacon from %pM\n", assoc_data->bss->bssid);
+		return true;
+	}
+
+	assoc_data->timeout = TU_TO_EXP_TIME(assoc_data->bss->beacon_interval);
+	run_again(sdata, assoc_data->timeout);
+	return false;
+}
+
 void ieee80211_sta_work(struct ieee80211_sub_if_data *sdata)
 {
 	struct ieee80211_local *local = sdata->local;
@@ -3533,12 +3552,12 @@ void ieee80211_sta_work(struct ieee80211_sub_if_data *sdata)
 
 	if (ifmgd->assoc_data && ifmgd->assoc_data->timeout_started &&
 	    time_after(jiffies, ifmgd->assoc_data->timeout)) {
-		if ((ifmgd->assoc_data->need_beacon && !ifmgd->have_beacon) ||
-		    ieee80211_do_assoc(sdata)) {
-			struct cfg80211_bss *bss = ifmgd->assoc_data->bss;

^ permalink raw reply related

* [PATCH] mac80211: add assoc beacon timeout logic
From: Felipe Contreras @ 2013-11-10 20:26 UTC (permalink / raw)
  To: linux-wireless
  Cc: netdev, John W. Linville, Johannes Berg, David S. Miller,
	Felipe Contreras

We don't want to be waiting forever for a beacon that will never come,
just continue the association.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 net/mac80211/ieee80211_i.h |  1 +
 net/mac80211/mlme.c        | 18 ++++++++++++++----
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 611abfc..e1f858d 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -358,6 +358,7 @@ struct ieee80211_mgd_assoc_data {
 	const u8 *supp_rates;
 
 	unsigned long timeout;
+	unsigned long beacon_timeout;
 	int tries;
 
 	u16 capability;
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 529cef6..1c14484 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -38,6 +38,7 @@
 #define IEEE80211_ASSOC_TIMEOUT		(HZ / 5)
 #define IEEE80211_ASSOC_TIMEOUT_LONG	(HZ / 2)
 #define IEEE80211_ASSOC_TIMEOUT_SHORT	(HZ / 10)
+#define IEEE80211_ASSOC_BEACON_TIMEOUT	2 * HZ
 #define IEEE80211_ASSOC_MAX_TRIES	3
 
 static int max_nullfunc_tries = 2;
@@ -3537,10 +3538,18 @@ void ieee80211_sta_work(struct ieee80211_sub_if_data *sdata)
 
 	if (assoc_data && assoc_data->timeout_started) {
 		if (time_after(jiffies, assoc_data->timeout)) {
-			if ((assoc_data->need_beacon && !ifmgd->have_beacon) ||
-			    ieee80211_do_assoc(sdata)) {
-				struct cfg80211_bss *bss = assoc_data->bss;
-
+			struct cfg80211_bss *bss = assoc_data->bss;
+
+			if (assoc_data->need_beacon && !ifmgd->have_beacon) {
+				if (time_after(jiffies, assoc_data->beacon_timeout)) {
+					sdata_info(sdata, "no beacon from %pM\n", bss->bssid);
+					assoc_data->need_beacon = false;
+					assoc_data->timeout = jiffies;
+				} else {
+					assoc_data->timeout = TU_TO_EXP_TIME(bss->beacon_interval);
+				}
+				run_again(sdata, assoc_data->timeout);
+			} else if (ieee80211_do_assoc(sdata)) {
 				ieee80211_destroy_assoc_data(sdata, false);
 				cfg80211_assoc_timeout(sdata->dev, bss);
 			}
@@ -4341,6 +4350,7 @@ int ieee80211_mgd_assoc(struct ieee80211_sub_if_data *sdata,
 		sdata_info(sdata, "waiting for beacon from %pM\n",
 			   ifmgd->bssid);
 		assoc_data->timeout = TU_TO_EXP_TIME(req->bss->beacon_interval);
+		assoc_data->beacon_timeout = jiffies + IEEE80211_ASSOC_BEACON_TIMEOUT;
 		assoc_data->timeout_started = true;
 		assoc_data->need_beacon = true;
 	} else if (beacon_ies) {
-- 
1.8.4.2+fc1

^ permalink raw reply related

* [PATCHv2] net: wireless: iwlwifi: remove minor dead code
From: Michal Nazarewicz @ 2013-11-10 20:17 UTC (permalink / raw)
  To: Sergei Shtylyov, Johannes Berg, Emmanuel Grumbach,
	John W. Linville
  Cc: Intel Linux Wireless, linux-wireless, netdev, linux-kernel
In-Reply-To: <527FDB50.1050902@cogentembedded.com>


inta is checked to be zero in a IRQ_NONE branch so afterwards it
cannot be zero as it is never modified.
---
 drivers/net/wireless/iwlwifi/pcie/rx.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

On Sun, Nov 10 2013, Sergei Shtylyov wrote:
> The preferred way of formatting the multi-line comments in the
> networking code is this:
>
> /* bla
>   * bla
>   */

Works for me.  I used /* bla \n * bla */ because this is what the rest
of this function was using.

diff --git a/drivers/net/wireless/iwlwifi/pcie/rx.c b/drivers/net/wireless/iwlwifi/pcie/rx.c
index 3f237b4..7d0fdc0 100644
--- a/drivers/net/wireless/iwlwifi/pcie/rx.c
+++ b/drivers/net/wireless/iwlwifi/pcie/rx.c
@@ -1121,7 +1121,6 @@ static irqreturn_t iwl_pcie_isr(int irq, void *data)
 	struct iwl_trans *trans = data;
 	struct iwl_trans_pcie *trans_pcie = IWL_TRANS_GET_PCIE_TRANS(trans);
 	u32 inta, inta_mask;
-	irqreturn_t ret = IRQ_NONE;
 
 	lockdep_assert_held(&trans_pcie->irq_lock);
 
@@ -1150,7 +1149,14 @@ static irqreturn_t iwl_pcie_isr(int irq, void *data)
 	 * or due to sporadic interrupts thrown from our NIC. */
 	if (!inta) {
 		IWL_DEBUG_ISR(trans, "Ignore interrupt, inta == 0\n");
-		goto none;
+		/* re-enable interrupts here since we don't have anything to
+		 * service.  only Re-enable if disabled by irq and no
+		 * schedules tasklet.
+		 */
+		if (test_bit(STATUS_INT_ENABLED, &trans_pcie->status) &&
+		    !trans_pcie->inta)
+			iwl_enable_interrupts(trans);
+		return IRQ_NONE;
 	}
 
 	if ((inta == 0xFFFFFFFF) || ((inta & 0xFFFFFFF0) == 0xa5a5a5a0)) {
@@ -1168,19 +1174,7 @@ static irqreturn_t iwl_pcie_isr(int irq, void *data)
 
 	trans_pcie->inta |= inta;
 	/* the thread will service interrupts and re-enable them */
-	if (likely(inta))
-		return IRQ_WAKE_THREAD;
-
-	ret = IRQ_HANDLED;
-
-none:
-	/* re-enable interrupts here since we don't have anything to service. */
-	/* only Re-enable if disabled by irq  and no schedules tasklet. */
-	if (test_bit(STATUS_INT_ENABLED, &trans_pcie->status) &&
-	    !trans_pcie->inta)
-		iwl_enable_interrupts(trans);
-
-	return ret;
+	return IRQ_WAKE_THREAD;
 }
 
 /* interrupt handler using ict table, with this interrupt driver will
-- 
1.8.3.2

^ permalink raw reply related

* Re: [PATCH] net: wireless: iwlwifi: remove minor dead code
From: Sergei Shtylyov @ 2013-11-10 19:15 UTC (permalink / raw)
  To: Michal Nazarewicz, Johannes Berg, Emmanuel Grumbach,
	John W. Linville
  Cc: Intel Linux Wireless, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Michal Nazarewicz
In-Reply-To: <1384110397-24386-1-git-send-email-mpn-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>

Hello.

On 10-11-2013 23:06, Michal Nazarewicz wrote:

> From: Michal Nazarewicz <mina86-deATy8a+UHjQT0dZR+AlfA@public.gmane.org>

> inta is checked to be zero in a IRQ_NONE branch so afterwards it
> cannot be zero as it is never modified.

> Signed-off-by: Michal Nazarewicz <mina86-deATy8a+UHjQT0dZR+AlfA@public.gmane.org>
> ---
>   drivers/net/wireless/iwlwifi/pcie/rx.c | 23 ++++++++---------------
>   1 file changed, 8 insertions(+), 15 deletions(-)

> diff --git a/drivers/net/wireless/iwlwifi/pcie/rx.c b/drivers/net/wireless/iwlwifi/pcie/rx.c
> index 3f237b4..c32122a 100644
> --- a/drivers/net/wireless/iwlwifi/pcie/rx.c
> +++ b/drivers/net/wireless/iwlwifi/pcie/rx.c
[...]
> @@ -1150,7 +1149,13 @@ static irqreturn_t iwl_pcie_isr(int irq, void *data)
>   	 * or due to sporadic interrupts thrown from our NIC. */
>   	if (!inta) {
>   		IWL_DEBUG_ISR(trans, "Ignore interrupt, inta == 0\n");
> -		goto none;
> +		/* re-enable interrupts here since we don't have anything to
> +		 * service.  only Re-enable if disabled by irq and no
> +		 * schedules tasklet. */

    The preferred way of formatting the multi-line comments in the networking 
code is this:

/* bla
  * bla
  */

WBR, Sergei

--
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] net: wireless: iwlwifi: remove minor dead code
From: Michal Nazarewicz @ 2013-11-10 19:06 UTC (permalink / raw)
  To: Johannes Berg, Emmanuel Grumbach, John W. Linville
  Cc: Intel Linux Wireless, linux-wireless, netdev, linux-kernel,
	Michal Nazarewicz

From: Michal Nazarewicz <mina86@mina86.com>

inta is checked to be zero in a IRQ_NONE branch so afterwards it
cannot be zero as it is never modified.

Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
 drivers/net/wireless/iwlwifi/pcie/rx.c | 23 ++++++++---------------
 1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/pcie/rx.c b/drivers/net/wireless/iwlwifi/pcie/rx.c
index 3f237b4..c32122a 100644
--- a/drivers/net/wireless/iwlwifi/pcie/rx.c
+++ b/drivers/net/wireless/iwlwifi/pcie/rx.c
@@ -1121,7 +1121,6 @@ static irqreturn_t iwl_pcie_isr(int irq, void *data)
 	struct iwl_trans *trans = data;
 	struct iwl_trans_pcie *trans_pcie = IWL_TRANS_GET_PCIE_TRANS(trans);
 	u32 inta, inta_mask;
-	irqreturn_t ret = IRQ_NONE;
 
 	lockdep_assert_held(&trans_pcie->irq_lock);
 
@@ -1150,7 +1149,13 @@ static irqreturn_t iwl_pcie_isr(int irq, void *data)
 	 * or due to sporadic interrupts thrown from our NIC. */
 	if (!inta) {
 		IWL_DEBUG_ISR(trans, "Ignore interrupt, inta == 0\n");
-		goto none;
+		/* re-enable interrupts here since we don't have anything to
+		 * service.  only Re-enable if disabled by irq and no
+		 * schedules tasklet. */
+		if (test_bit(STATUS_INT_ENABLED, &trans_pcie->status) &&
+		    !trans_pcie->inta)
+			iwl_enable_interrupts(trans);
+		return IRQ_NONE;
 	}
 
 	if ((inta == 0xFFFFFFFF) || ((inta & 0xFFFFFFF0) == 0xa5a5a5a0)) {
@@ -1168,19 +1173,7 @@ static irqreturn_t iwl_pcie_isr(int irq, void *data)
 
 	trans_pcie->inta |= inta;
 	/* the thread will service interrupts and re-enable them */
-	if (likely(inta))
-		return IRQ_WAKE_THREAD;
-
-	ret = IRQ_HANDLED;
-
-none:
-	/* re-enable interrupts here since we don't have anything to service. */
-	/* only Re-enable if disabled by irq  and no schedules tasklet. */
-	if (test_bit(STATUS_INT_ENABLED, &trans_pcie->status) &&
-	    !trans_pcie->inta)
-		iwl_enable_interrupts(trans);
-
-	return ret;
+	return IRQ_WAKE_THREAD;
 }
 
 /* interrupt handler using ict table, with this interrupt driver will
-- 
1.8.3.2

^ permalink raw reply related

* Re: [PATCH net-next] xfrm: check function pointer of xfrm_mgr before use it
From: Sergei Shtylyov @ 2013-11-10 19:03 UTC (permalink / raw)
  To: baker.kernel, herbert, davem, steffen.klassert; +Cc: netdev
In-Reply-To: <1384093556-8987-1-git-send-email-baker.kernel@gmail.com>

Hello.

On 10-11-2013 18:25, baker.kernel@gmail.com wrote:

> From: "baker.zhang" <baker.kernel@gmail.com>

> Signed-off-by: baker.zhang <baker.kernel@gmail.com>
> ---
> For current kernel source, there is no problem.

> In our vpn product, we need a xfrm_km in kernel module
> to monitor the xfrm state change.
> thus, the 'acquire' and 'compile_policy' may be NULL.

> So I think we should do the check before use it.

>   net/xfrm/xfrm_state.c | 18 +++++++++++-------
>   1 file changed, 11 insertions(+), 7 deletions(-)

> diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
> index b9c3f9e..541f684 100644
> --- a/net/xfrm/xfrm_state.c
> +++ b/net/xfrm/xfrm_state.c
[...]
> @@ -1783,10 +1785,12 @@ int xfrm_user_policy(struct sock *sk, int optname, u8 __user *optval, int optlen
>   	err = -EINVAL;
>   	rcu_read_lock();
>   	list_for_each_entry_rcu(km, &xfrm_km_list, list) {
> -		pol = km->compile_policy(sk, optname, data,
> -					 optlen, &err);
> -		if (err >= 0)
> -			break;
> +		if (km->compile_policy) {
> +			pol = km->compile_policy(sk, optname, data,
> +					optlen, &err);

    According the networking coding style, the continuation line should start 
right under 'sk' on the previous line.

WBR, Sergei

^ permalink raw reply

* Re: [PATCH v3] can: add Renesas R-Car CAN driver
From: Marc Kleine-Budde @ 2013-11-10 17:57 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: netdev, wg, linux-can, linux-sh, vksavl
In-Reply-To: <527D6B88.3010409@cogentembedded.com>

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

On 11/08/2013 11:54 PM, Sergei Shtylyov wrote:
[...]

>>>> Please use a common prefix for all defines.
> 
>>>     OK, done now. Could you however explain why the file-local #define's
>>> should be prefixed? It's not quite obvious to me...
> 
>> It's about readability and maintainability. If you don't know the
>> driver, but all driver local defines and functions have a common prefix,
>> it's much easier to read if you are not the author of the driver IMHO.
> 
>    Well, actually I think the last changes somewhat impaired the
> readability. My idea was that you want to exclude name conflicts with
> #include'd headers this way...

IMHO this way it's clear to everyone, that certain defines are
driver/hardware specific.

>>> [...]
>>>>> +static netdev_tx_t rcar_can_start_xmit(struct sk_buff *skb,
>>>>> +                       struct net_device *ndev)
>>>>> +{
>>>>> +    struct rcar_can_priv *priv = netdev_priv(ndev);
>>>>> +    struct can_frame *cf = (struct can_frame *)skb->data;
>>>>> +    u32 data, mier1, mbxno, i;
>>>>> +    unsigned long flags;
>>>>> +    u8 mctl = 0;
>>>>> +
>>>>> +    if (can_dropped_invalid_skb(ndev, skb))
>>>>> +        return NETDEV_TX_OK;
>>>>> +
>>>>> +    spin_lock_irqsave(&priv->mier_lock, flags);
>>>>> +    mier1 = readl(&priv->regs->mier1);
>>>>> +    if (mier1) {
>>>>> +        i = __builtin_clz(mier1);
>>>>> +        mbxno = i ? N_MBX - i : FIRST_TX_MB;
>>>>> +    } else {
>>>>> +        mbxno = FIRST_TX_MB;
>>>>> +    }
> 
>>>> Can you explain how the hardware arbitration works, and you do you
>>>> guarantee the CAN frames are send by the hardware in the same order you
>>>> put them into the hardware.
> 
>>>     Tx mailbox with the smallest mailbox number has the highest
>>> priority.
>>> The other possible Tx mailbox selection rule (not used by the driver
>>> now) is ID priority transmit mode (as defined in the ISO 11898-1 specs).
>>> The algorithm used guarantees the mailboxes are filled sequentially.
> 
>    Well, not quite, unfortunately -- it wraps at the last mailbox...

Yes sure, that's the downside if there isn't a real FIFO in the
hardware. I suggest to have two pointers in your private struct:

    unsinged int tx_head;
    unsigned int tx_tail;

static inline unsigned int get_tx_head_mb(struct *priv)
{
       return priv->tx_head % RCAN_TX_QUEUE_SIZE;
}

static inline unsigned int get_tx_tail_mb(struct priv *priv)
{
       return priv->tx_tail % RCAN_TX_QUEUE_SIZE;
}

Put the next CAN frame into get_tx_head_mb(), increment priv->tx_head
and stop your queue if all buffers are used or wrap around.

       /* stop, if all buffers are used or wrap around */
       if ((priv->tx_head - priv->tx_tail == RCAN_TX_QUEUE_SIZE) &&
	    get_tx_head_mb(priv) == 0)
               netif_stop_queue(dev);

But I think, as you don't have any additional prio bits, it boils down to:

       /* stop, if wrap around */
       if (get_tx_head_mb(priv) == 0)
               netif_stop_queue(dev);

In your tx-complete, use something like this:

   reg = read_tx_complete();
   for (/* nix */; priv->tx_head - priv->tx_tail > 0; priv->tx_tail++) {
       mb = get_tx_tail_mb(priv);

       if (!reg & (1 << mb))
            break;

       read_can_frame();
   }

   if ((get_tx_head_mb(priv) != 0) || (get_tx_tail_mb(priv) == 0))
           netif_wake_queue(dev);

>> I see. You are using mier1 to track the used mailboxes....correct?
> 
>    Yes, the mailbox interrupts in MIER1 are enabled only for used
> mailboxes.
> 
>>> +    if (unlikely(mier1 == 0xffffffff))
>>> +        netif_stop_queue(ndev);
> 
>> Then you have a race condition in your tx-complete handler
>> rcar_can_tx_done(), as you call netif_wake_queue() unconditionally. If
> 
>    Yes, I'm seeing it now...
> 
>> mier1 == 0xffffffff you must wait until _all_ mailboxes are transmitted
> 
>    That 0xffffffff criterion seems wrong for me now, I changed the
> algorithm and moved the criterion but didn't update it. The correct one
> seems to be:
> 
>     if (unlikely(mier1 & 0x80000000))
>         netif_stop_queue(ndev);

Better handle it in software altogether as outlined above.

>> until you are allowed to reenable the mailboxes. Have a look at the
>> at91_can driver, it's using a similar scheme. The lowest mailbox is
>> transmitted first, but there are three additional bits that indicate the
>> priority.
> 
>    You mean 4 bits? Priorities are from 0 to 15...

Yes, 4 bits (0xf), but the algorithm isn't limited to 4 bits.

>>> I've used 'canfdtest' as suggested by Wolfgang Grandegger to verify, see
>>> the log below:
> 
>>> root@am335x-evm:~# ./canfdtest -v -g can0
>>> interface = can0, family = 29, type = 3, proto = 1
>>> ...............................................................................C
>>>
>>>
>>> Test messages sent and received: 483203
>>> Exiting...
>>> root@am335x-evm:~#
> 
>    As you can see, 'canfdtest' didn't detect any race, maybe you could
> recommend a test which would help to detect it?

It's better to think to detect a race condition, then to test for it. I
suggest to use the algorithm outlined above, as implemented in the
at91_can.c driver.

>>> [...]
>>>>> +static int rcar_can_rx_poll(struct napi_struct *napi, int quota)
>>>>> +{
>>>>> +    struct rcar_can_priv *priv = container_of(napi,
>>>>> +                          struct rcar_can_priv, napi);
>>>>> +    int num_pkts = 0;
>>>>> +
>>>>> +    /* Find mailbox */
>>>>> +    while (num_pkts < quota) {
>>>>> +        u8 mctl, mbx;
>>>>> +
>>>>> +        mbx = readb(&priv->regs->mssr);
> 
>>>> How does the RX work? Is it a hardware FIFO?
> 
>>>     In short, the MSSR register provides the smallest Rx mailbox number
>>> that is looked up in the Rx search mode. We read MSSR until no search
>>> results can be obtained, so it is some sort of FIFO.
> 
>> This looks racy....
> 
>    Could you please elaborate?

Consider the hardware fills mailboxes 0...8, then your rx_poll starts to
read mailbox 0, clear mailbox 0, read mb 1, clear mb 2, then another CAN
frame arrives. Which mailbox will be filled next? Which mailbox will be
read next by rx_poll?

>>>     And there is separate FIFO operation mode: some mailboxes can be
>>> configured as FIFO and serviced by special registers but this operation
>>> mode is not supported by the driver.
> 
>> if you hardware supports a real FIFO then I strongly suggest to make use
>> of it.
> 
>    Well, Tx/Rx FIFOs are only 4 frames deep (although I haven't seen
> more than 2 Rx mailboxes ready in a row, there are 32 Tx mailboxes);
> also FIFO mode is less documented than mailbox mode (hence some nasty
> surprises are possible). We still can use mailboxes when in FIFO mode,
> it's just 8 of them are reserved for Tx/Rx FIFOs.

Who will the current rx_poll behave in the above outlined situation?

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

^ permalink raw reply

* [PATCH net-next] xfrm: check function pointer of xfrm_mgr before use it
From: baker.kernel @ 2013-11-10 14:25 UTC (permalink / raw)
  To: herbert, davem, steffen.klassert; +Cc: netdev, baker.zhang

From: "baker.zhang" <baker.kernel@gmail.com>

Signed-off-by: baker.zhang <baker.kernel@gmail.com>
---
For current kernel source, there is no problem.

In our vpn product, we need a xfrm_km in kernel module
to monitor the xfrm state change.
thus, the 'acquire' and 'compile_policy' may be NULL.

So I think we should do the check before use it.

 net/xfrm/xfrm_state.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index b9c3f9e..541f684 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -1679,9 +1679,11 @@ int km_query(struct xfrm_state *x, struct xfrm_tmpl *t, struct xfrm_policy *pol)
 
 	rcu_read_lock();
 	list_for_each_entry_rcu(km, &xfrm_km_list, list) {
-		acqret = km->acquire(x, t, pol);
-		if (!acqret)
-			err = acqret;
+		if (km->acquire) {
+			acqret = km->acquire(x, t, pol);
+			if (!acqret)
+				err = acqret;
+		}
 	}
 	rcu_read_unlock();
 	return err;
@@ -1783,10 +1785,12 @@ int xfrm_user_policy(struct sock *sk, int optname, u8 __user *optval, int optlen
 	err = -EINVAL;
 	rcu_read_lock();
 	list_for_each_entry_rcu(km, &xfrm_km_list, list) {
-		pol = km->compile_policy(sk, optname, data,
-					 optlen, &err);
-		if (err >= 0)
-			break;
+		if (km->compile_policy) {
+			pol = km->compile_policy(sk, optname, data,
+					optlen, &err);
+			if (err >= 0)
+				break;
+		}
 	}
 	rcu_read_unlock();
 
-- 
1.8.3.2

^ permalink raw reply related

* Re: [PATCH v4 net-next] net: introduce dev_set_forwarding()
From: Herbert Xu @ 2013-11-10 14:05 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, bhutchings, christoph.paasch, netdev, hkchu,
	mwdalton
In-Reply-To: <1383884730.9412.175.camel@edumazet-glaptop2.roam.corp.google.com>

On Thu, Nov 07, 2013 at 08:25:30PM -0800, Eric Dumazet wrote:
> On Fri, 2013-11-08 at 11:59 +0800, Herbert Xu wrote:
> 
> > However, I still have one reason for preferring my patch, it'll
> > be easier to prodce TSO packets with it.  Let me see if I can
> > fix up the arbitrary frag boundary issue without making it too
> > ugly.
> 
> Sure !

I ended up giving up on the recursion idea and borrowed your
iterative approach.  I haven't yet had the chance to test it
yet but here is the WIP.

The main assumptions are that virtio_net frag_list is always non-
linear and GRO frag_list may only contain a linear head part that
is exactly MSS bytes long.

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 3735fad..fab44ff 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2776,6 +2776,7 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
 	struct sk_buff *segs = NULL;
 	struct sk_buff *tail = NULL;
 	struct sk_buff *fskb = skb_shinfo(skb)->frag_list;
+	skb_frag_t *skb_frag = skb_shinfo(skb)->frags;
 	unsigned int mss = skb_shinfo(skb)->gso_size;
 	unsigned int doffset = skb->data - skb_mac_header(skb);
 	unsigned int offset = doffset;
@@ -2815,16 +2816,23 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
 		if (hsize > len || !sg)
 			hsize = len;
 
-		if (!hsize && i >= nfrags) {
-			BUG_ON(fskb->len != len);
+		if (!hsize && i >= nfrags && skb_headlen(fskb)) {
+			BUG_ON(skb_headlen(fskb) != len);
 
 			pos += len;
+			i = 0;
+			nfrags = skb_shinfo(fskb)->nr_frags;
+			skb_frag = skb_shinfo(fskb)->frags;
+
 			nskb = skb_clone(fskb, GFP_ATOMIC);
 			fskb = fskb->next;
 
 			if (unlikely(!nskb))
 				goto err;
 
+			if (unlikely(pskb_trim(nskb, len)))
+				goto err;
+
 			hsize = skb_end_offset(nskb);
 			if (skb_cow_head(nskb, doffset + headroom)) {
 				kfree_skb(nskb);
@@ -2861,7 +2869,8 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
 						 nskb->data - tnl_hlen,
 						 doffset + tnl_hlen);
 
-		if (fskb != skb_shinfo(skb)->frag_list)
+		if (fskb != skb_shinfo(skb)->frag_list &&
+		    nskb->len == len + doffset)
 			goto perform_csum_check;
 
 		if (!sg) {
@@ -2879,8 +2888,20 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
 
 		skb_shinfo(nskb)->tx_flags = skb_shinfo(skb)->tx_flags & SKBTX_SHARED_FRAG;
 
-		while (pos < offset + len && i < nfrags) {
-			*frag = skb_shinfo(skb)->frags[i];
+		while (pos < offset + len) {
+			if (i >= nfrags) {
+				BUG_ON(skb_headlen(fskb));
+
+				i = 0;
+				nfrags = skb_shinfo(fskb)->nr_frags;
+				skb_frag = skb_shinfo(fskb)->frags;
+
+				BUG_ON(!nfrags);
+
+				fskb = fskb->next;
+			}
+
+			*frag = *skb_frag;
 			__skb_frag_ref(frag);
 			size = skb_frag_size(frag);
 
@@ -2891,37 +2912,26 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
 
 			skb_shinfo(nskb)->nr_frags++;
 
-			if (pos + size <= offset + len) {
-				i++;
-				pos += size;
-			} else {
-				skb_frag_size_sub(frag, pos + size - (offset + len));
-				goto skip_fraglist;
+			if (pos + size >= offset + len) {
+				skb_frag_size_sub(frag,
+						  pos + size - (offset + len));
+				break;
 			}
 
+			skb_frag++;
+			i++;
+			pos += size;
 			frag++;
-		}
-
-		if (pos < offset + len) {
-			struct sk_buff *fskb2 = fskb;
 
-			BUG_ON(pos + fskb->len != offset + len);
-
-			pos += fskb->len;
-			fskb = fskb->next;
-
-			if (fskb2->next) {
-				fskb2 = skb_clone(fskb2, GFP_ATOMIC);
-				if (!fskb2)
-					goto err;
-			} else
-				skb_get(fskb2);
-
-			SKB_FRAG_ASSERT(nskb);
-			skb_shinfo(nskb)->frag_list = fskb2;
+			if (unlikely(skb_shinfo(nskb)->nr_frags >=
+				     MAX_SKB_FRAGS)) {
+				net_warn_ratelimited(
+					"skb_segment: too many frags: %u %u\n",
+					pos, mss);
+				goto err;
+			}
 		}
 
-skip_fraglist:
 		nskb->data_len = len - hsize;
 		nskb->len += nskb->data_len;
 		nskb->truesize += nskb->data_len;

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply related

* [BUG,REGRESSION?] 3.11.6+,3.12: GbE iface rate drops to few KB/s
From: Arnaud Ebalard @ 2013-11-10 13:53 UTC (permalink / raw)
  To: Eric Dumazet, David S. Miller, Greg Kroah-Hartman
  Cc: netdev, stable, linux-arm-kernel

Hi,

I decided to upgrade the kernel on one of my ReadyNAS 102 from 3.11.1 to 
3.11.7. The device is based on Marvell Armada 370 SoC and uses mvneta
driver. Mine runs Debian armel unstable but I can confirm the issue also
happens on a debian harmhf unstable.

Doing some scp transfers of files located on the NAS (1000baseT-FD on
both side), I noticed the transfers rate is ridiculously slow (280KB/s).
I did the same test with a 3.12 kernel and got the same results,
i.e. AFAICT the bug also exist upstream.

So, I decided to go to hell and start digging a bit: I run a 'git bisect'
session on stable tree from 3.11.1 (known good) to 3.11.7 (known
bad). The results are given below.

I decided to reboot on my old 3.11.1 kernel and do 20 files transfers
of a 1GB file located on the NAS to my laptop via scp. I took the 20+
minutes and let them all finish: each transfer took between 1min5s and
1min7s (around 16MB/s, the limitation in that case being the crypto part).

I rebooted again and did the exact same thing on the 3.11.7 and after
the completion of the first file transfer in 1m6s (16MB/s), the second
one gave me that:

arno@small:~scp RN102:/tmp/random /dev/null
random                               0% 1664KB 278.9KB/s 1:05:37 ETA^C

And it continued that way for the remaining transfers (i did ^c after
some seconds to restart the transfer when the rate was low):

$ for k in $(seq 1 20) ; do scp RN102:random /dev/null ; done 
random                             100% 1024MB  15.6MB/s   01:06 ETA^C 
random                               0% 9856KB 282.2KB/s 1:01:20 ETA^C
random                              16%  168MB 563.9KB/s   25:54 ETA^C
random                               0% 2816KB 273.5KB/s 1:03:43 ETA^C
random                             100% 1024MB  15.5MB/s   01:06    
random                               1%   17MB 282.3KB/s 1:00:54 ETA^C
random                               0%  544KB 259.2KB/s 1:07:23 ETA^C
random                               0% 4224KB 277.3KB/s 1:02:45 ETA^C
random                               0%  832KB 262.1KB/s 1:06:37 ETA^C
random                               0% 3360KB 273.4KB/s 1:03:43 ETA^C
random                               0% 3072KB 271.8KB/s 1:04:07 ETA^C
random                               0%  832KB 262.1KB/s 1:06:37 ETA^C
random                               0% 1408KB 267.0KB/s 1:05:21 ETA^C
random                               0% 1120KB 264.7KB/s 1:05:57 ETA
...

To be sure, I did 2 additional reboots, one on each kernel and the
result is consistent. Perfect on 3.11.1 and slow rate most of the time
on 3.11.7 (Both kernel are compiled from a fresh make clean, using the
same config file).

Then, knowing that, I started a git bisect session on stable tree to end
up with the following suspects. I failed to go any further to a single
commit, due to crashes, but I could recompile a kernel w/ debug info and
report what I get if neeeded.

commit dc0791aee672 tcp: do not forget FIN in tcp_shifted_skb()     [bad]
commit 18ddf5127c9f tcp: must unclone packets before mangling them
commit 80bd5d8968d8 tcp: TSQ can use a dynamic limit
commit dbeb18b22197 tcp: TSO packets automatic sizing
commit 50704410d014 Linux 3.11.6                                    [good]

Eric, David, if it has already been reported and fixed, just tell
me. Otherwise, if you have any ideas, I'll be happy to test this
evening.

Cheers,

a+


Just in case it may be useful, here is what ethtool reports on RN102:

# ethtool -i eth0
driver: mvneta
version: 1.0
firmware-version: 
bus-info: eth0
supports-statistics: no
supports-test: no
supports-eeprom-access: no
supports-register-dump: no
supports-priv-flags: no

# ethtool -k eth0
Features for eth0:
rx-checksumming: off [fixed]
tx-checksumming: on
        tx-checksum-ipv4: on
        tx-checksum-ip-generic: off [fixed]
        tx-checksum-ipv6: off [fixed]
        tx-checksum-fcoe-crc: off [fixed]
        tx-checksum-sctp: off [fixed]
scatter-gather: on
        tx-scatter-gather: on
        tx-scatter-gather-fraglist: off [fixed]
tcp-segmentation-offload: off
        tx-tcp-segmentation: off [fixed]
        tx-tcp-ecn-segmentation: off [fixed]
        tx-tcp6-segmentation: off [fixed]
udp-fragmentation-offload: off [fixed]
generic-segmentation-offload: on
generic-receive-offload: on
large-receive-offload: off [fixed]
rx-vlan-offload: off [fixed]
tx-vlan-offload: off [fixed]
ntuple-filters: off [fixed]
receive-hashing: off [fixed]
highdma: off [fixed]
rx-vlan-filter: off [fixed]
vlan-challenged: off [fixed]
tx-lockless: off [fixed]
netns-local: off [fixed]
tx-gso-robust: off [fixed]
tx-fcoe-segmentation: off [fixed]
tx-gre-segmentation: off [fixed]
tx-udp_tnl-segmentation: off [fixed]
tx-mpls-segmentation: off [fixed]
fcoe-mtu: off [fixed]
tx-nocache-copy: on
loopback: off [fixed]
rx-fcs: off [fixed]
rx-all: off [fixed]
tx-vlan-stag-hw-insert: off [fixed]
rx-vlan-stag-hw-parse: off [fixed]
rx-vlan-stag-filter: off [fixed]

^ 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