Netdev List
 help / color / mirror / Atom feed
* Re: sendto() bug?
From: Chris Clark @ 2013-08-26 20:14 UTC (permalink / raw)
  To: Julian Anastasov; +Cc: davem, netdev
In-Reply-To: <alpine.LFD.2.00.1308241105220.1641@ja.ssi.bg>

On Sat, 24 Aug 2013, Julian Anastasov wrote:
> On Fri, 23 Aug 2013, Chris Clark wrote:
> > In the same vein as 2ad5b9e4, I'm soliciting feedback on something
> > similar for raw_sendmsg():
[snip]

> 	So, something like
> (inet->hdrincl ? FLOWI_FLAG_KNOWN_NH : 0) ?


Many thanks.  This patch indeed appears to resolve the problem:

diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index dd44e0a..61e60d6 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -571,7 +571,8 @@ static int raw_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
 	flowi4_init_output(&fl4, ipc.oif, sk->sk_mark, tos,
 			   RT_SCOPE_UNIVERSE,
 			   inet->hdrincl ? IPPROTO_RAW : sk->sk_protocol,
-			   inet_sk_flowi_flags(sk) | FLOWI_FLAG_CAN_SLEEP,
+			   inet_sk_flowi_flags(sk) | FLOWI_FLAG_CAN_SLEEP |
+			    (inet->hdrincl ? FLOWI_FLAG_KNOWN_NH : 0),
 			   daddr, saddr, 0, 0);

 	if (!inet->hdrincl) {


Chris

^ permalink raw reply related

* Re: sendto() bug?
From: David Miller @ 2013-08-26 20:15 UTC (permalink / raw)
  To: chris.clark; +Cc: ja, netdev
In-Reply-To: <alpine.DEB.2.02.1308261319370.23968@optio.utah.ind.alcatel.com>

From: Chris Clark <chris.clark@alcatel-lucent.com>
Date: Mon, 26 Aug 2013 14:14:21 -0600 (MDT)

> On Sat, 24 Aug 2013, Julian Anastasov wrote:
>> On Fri, 23 Aug 2013, Chris Clark wrote:
>> > In the same vein as 2ad5b9e4, I'm soliciting feedback on something
>> > similar for raw_sendmsg():
> [snip]
> 
>> 	So, something like
>> (inet->hdrincl ? FLOWI_FLAG_KNOWN_NH : 0) ?
> 
> 
> Many thanks.  This patch indeed appears to resolve the problem:

Someone please submit this properly with a clean, well formed, commit
message and signoffs.

Thanks.

^ permalink raw reply

* Re: [PATCH net-next] {ipv4,xfrm}: Introduce xfrm_tunnel_notifier for xfrm tunnel mode callback
From: David Miller @ 2013-08-26 20:21 UTC (permalink / raw)
  To: fan.du; +Cc: steffen.klassert, saurabh.mohan, herbert, netdev
In-Reply-To: <1377240424-11758-1-git-send-email-fan.du@windriver.com>

From: Fan Du <fan.du@windriver.com>
Date: Fri, 23 Aug 2013 14:47:04 +0800

> Some thoughts on IPv4 VTI implementation:
> 
> The connection between VTI receiving part and xfrm tunnel mode input process
> is hardly a "xfrm_tunnel", xfrm_tunnel is used in places where, e.g ipip/sit
> and xfrm4_tunnel, acts like a true "tunnel" device.
> 
> In addition, IMHO, VTI doesn't need vti_err to do something meaningful, as all
> VTI needs is just a notifier to be called whenever xfrm_input ingress a packet
> to update statistics.
> 
> So this patch introduce xfrm_tunnel_notifier and meanwhile wipe out vti_erri
> code.
> 
> Signed-off-by: Fan Du <fan.du@windriver.com>

I don't understand why VTI doesn't need to propagate a PMTU update via
ipv4_update_pmtu().  Why is it different from a real xfrm_tunnel?

Your changelog has to explain this better and in more detail.

Thanks.

^ permalink raw reply

* Re: [PATCH RFC 0/6] SYNPROXY target v2
From: David Miller @ 2013-08-26 20:24 UTC (permalink / raw)
  To: kaber; +Cc: pablo, netfilter-devel, netdev, mph, jesper.brouer, as
In-Reply-To: <1377244329-20146-1-git-send-email-kaber@trash.net>

From: Patrick McHardy <kaber@trash.net>
Date: Fri, 23 Aug 2013 09:52:03 +0200

> The following patches contain the current version of the SYNPROXY target.
> Changes this the last posting are:
> 
> - use sysctl_ip_default_ttl instead of hardcoding 64
> 
> - use MAX_TCP_HEADER instead of LL_MAX_HEADER
> 
> - add some comments requested by Jesper regarding ack_seq initialization
>   in the server's SYN packet
> 
> - use consume_skb() instead of kfree_skb() in the synproxy hook
> 
> - remove a fixme and add explicit check for "-p tcp" in the SYNPROXY rule
>   in the IPv6 version
> 
> - some whitespace fixes
> 
> - a larger number of fixes for properly handling retransmissions and
>   out of order packets, please see the changelog included in patch 6/6
>   for details.
>   
>   I've kept those in a seperate patch for now to ease review, we'll do
>   some more extensive testing on monday and I'll fold the changes in
>   their respective patches before the final submission.
> 
> Comments welcome.

No objections to the non-netfilter portions:

Acked-by: David S. Miller <davem@davemloft.net>

^ permalink raw reply

* Re: [PATCH v2 net-next] tcp: TSO packets automatic sizing
From: Eric Dumazet @ 2013-08-26 20:28 UTC (permalink / raw)
  To: Yuchung Cheng
  Cc: David Miller, netdev, Neal Cardwell, Van Jacobson, Tom Herbert
In-Reply-To: <CAK6E8=dkp2soUUrv_gP5soBCqe7mf7KNY-iO-FoS0J4Pi4zXpg@mail.gmail.com>

On Mon, 2013-08-26 at 12:09 -0700, Yuchung Cheng wrote:

> init write of 10MSS now looks good, but the delay still shows up if 9
> < write-size < 10 MSS...
> 
> I use packetdrill to do write(14599) bytes of MSS 1460
> 
> 0.000 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
> 0.000 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
> 0.000 bind(3, ..., ...) = 0
> 0.000 listen(3, 1) = 0
> 
> 0.100 < S 299245565:299245565(0) win 32792 <mss
> 1460,sackOK,nop,nop,nop,wscale 7>
> 0.100 > S. 0:0(0) ack 299245566 <mss 1460,nop,nop,sackOK,nop,wscale 6>
> 0.200 < . 1:1(0) ack 1 win 257
> 0.200 accept(3, ..., ...) = 4
> +.000 write(4, ..., 13141) = 13141
> +3 %{ print "done" }%
> 
> and tcpdump shows
> 
> 31.819958 IP cli > srv: S 299245565:299245565(0) win 32792 <mss 1460,sackOK,nop,
> nop,nop,wscale 7>
> 000034 IP srv > cli: S 203810874:203810874(0) ack 299245566 win 29200
> <mss 1460,nop,nop,sackOK,nop,wscale 6>
> 099966 IP cli > srv: . ack 1 win 257
> 000079 IP srv > cli: . 1:2921(2920) ack 1 win 457
> 000010 IP srv > cli: . 2921:5841(2920) ack 1 win 457
> 000005 IP srv > cli: . 5841:8761(2920) ack 1 win 457
> 000004 IP srv > cli: . 8761:11681(2920) ack 1 win 457
> 199659 IP srv > cli: . 11681:13141(1460) ack 1 win 457
> ...

It works correctly here, I wonder what's happening on your host.

lpq83:~# tcpdump -p -n -s 0 -i any port 8080 -ttt
tcpdump: verbose output suppressed, use -v or -vv for full protocol decode
listening on any, link-type LINUX_SLL (Linux cooked), capture size 65535 bytes
000000 IP 192.0.2.1.42148 > 192.168.0.1.8080: S 299245565:299245565(0) win 32792 <mss 1460,sackOK,nop,nop,nop,wscale 7>
000042 IP 192.168.0.1.8080 > 192.0.2.1.42148: S 223873324:223873324(0) ack 299245566 win 29200 <mss 1460,nop,nop,sackOK,nop,wscale 6>
099946 IP 192.0.2.1.42148 > 192.168.0.1.8080: . ack 1 win 257
000087 IP 192.168.0.1.8080 > 192.0.2.1.42148: . 1:2921(2920) ack 1 win 457
000006 IP 192.168.0.1.8080 > 192.0.2.1.42148: . 2921:5841(2920) ack 1 win 457
000004 IP 192.168.0.1.8080 > 192.0.2.1.42148: . 5841:8761(2920) ack 1 win 457
000004 IP 192.168.0.1.8080 > 192.0.2.1.42148: . 8761:11681(2920) ack 1 win 457
000005 IP 192.168.0.1.8080 > 192.0.2.1.42148: . 11681:13141(1460) ack 1 win 457
000002 IP 192.168.0.1.8080 > 192.0.2.1.42148: P 13141:13142(1) ack 1 win 457

^ permalink raw reply

* Re: [PATCH RESEND V3 net-next 2/3] net: huawei_cdc_ncm: Introduce the huawei_cdc_ncm driver
From: David Miller @ 2013-08-26 20:31 UTC (permalink / raw)
  To: mrkiko.rs; +Cc: gregkh, oliver, linux-kernel, linux-usb, netdev
In-Reply-To: <1377244590-8558-3-git-send-email-mrkiko.rs@gmail.com>

From: Enrico Mioso <mrkiko.rs@gmail.com>
Date: Fri, 23 Aug 2013 09:56:29 +0200

> +	if ((on && atomic_add_return(1, &drvstate->pmcount) == 1) || (!on && atomic_dec_and_test(&drvstate->pmcount))) {

These line significantly exceeds 80 columns.

> +		subdriver = usb_cdc_wdm_register(ctx->control,
> +						 &usbnet_dev->status->desc,
> +						 256, /* CDC-WMC r1.1 requires wMaxCommand to be "at least 256 decimal (0x100)" */
> +						 huawei_cdc_ncm_wdm_manage_power);

Likewise.

> +	if (intf == ctx->control && drvstate->subdriver && drvstate->subdriver->suspend)

Likewise.

> +	int ret = 0;
> +	struct usbnet *usbnet_dev = usb_get_intfdata(intf);
> +	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
> +	struct cdc_ncm_ctx *ctx = drvstate->ctx;
> +	bool callsub = (intf == ctx->control && drvstate->subdriver && drvstate->subdriver->resume); /* should we call subdriver's resume? ? */

Likewise, and order local function variable declarations by line
length, longest to shortest.

^ permalink raw reply

* [PATCH net-next v1 4/9] bonding: make bond_arp_send_all use upper device list
From: Veaceslav Falico @ 2013-08-26 20:32 UTC (permalink / raw)
  To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1377549162-7522-1-git-send-email-vfalico@redhat.com>

Currently, bond_arp_send_all() is aware only of vlans, which breaks
configurations like bond <- bridge (or any other 'upper' device) with IP
(which is quite a common scenario for virt setups).

To fix this we convert the bond_arp_send_all() to first verify if the rt
device is the bond itself, and if not - to go through its list of upper
devices to see if any of them match the route device for the target. If the
match is a vlan device - we also save its vlan_id and tag it in
bond_arp_send().

Also, clean the function a bit to be more readable.

v1: use netdev_for_each_upper_dev()

CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
 drivers/net/bonding/bond_main.c |   84 ++++++++++++++------------------------
 1 files changed, 31 insertions(+), 53 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 7407e65..40eb250 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2444,30 +2444,16 @@ static void bond_arp_send(struct net_device *slave_dev, int arp_op, __be32 dest_
 
 static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
 {
-	int i, vlan_id;
-	__be32 *targets = bond->params.arp_targets;
-	struct vlan_entry *vlan;
-	struct net_device *vlan_dev = NULL;
+	struct net_device *upper;
+	struct list_head *iter;
 	struct rtable *rt;
+	__be32 *targets = bond->params.arp_targets, addr;
+	int i, vlan_id;
 
-	for (i = 0; (i < BOND_MAX_ARP_TARGETS); i++) {
-		__be32 addr;
-		if (!targets[i])
-			break;
+	for (i = 0; i < BOND_MAX_ARP_TARGETS && targets[i]; i++) {
 		pr_debug("basa: target %pI4\n", &targets[i]);
-		if (!bond_vlan_used(bond)) {
-			pr_debug("basa: empty vlan: arp_send\n");
-			addr = bond_confirm_addr(bond->dev, targets[i], 0);
-			bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i],
-				      addr, 0);
-			continue;
-		}
 
-		/*
-		 * If VLANs are configured, we do a route lookup to
-		 * determine which VLAN interface would be used, so we
-		 * can tag the ARP with the proper VLAN tag.
-		 */
+		/* Find out through which dev should the packet go */
 		rt = ip_route_output(dev_net(bond->dev), targets[i], 0,
 				     RTO_ONLINK, 0);
 		if (IS_ERR(rt)) {
@@ -2478,47 +2464,39 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
 			continue;
 		}
 
-		/*
-		 * This target is not on a VLAN
-		 */
-		if (rt->dst.dev == bond->dev) {
-			ip_rt_put(rt);
-			pr_debug("basa: rtdev == bond->dev: arp_send\n");
-			addr = bond_confirm_addr(bond->dev, targets[i], 0);
-			bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i],
-				      addr, 0);
-			continue;
-		}
-
 		vlan_id = 0;
-		list_for_each_entry(vlan, &bond->vlan_list, vlan_list) {
-			rcu_read_lock();
-			vlan_dev = __vlan_find_dev_deep(bond->dev,
-							htons(ETH_P_8021Q),
-							vlan->vlan_id);
-			rcu_read_unlock();
-			if (vlan_dev == rt->dst.dev) {
-				vlan_id = vlan->vlan_id;
-				pr_debug("basa: vlan match on %s %d\n",
-				       vlan_dev->name, vlan_id);
-				break;
-			}
-		}
 
-		if (vlan_id && vlan_dev) {
-			ip_rt_put(rt);
-			addr = bond_confirm_addr(vlan_dev, targets[i], 0);
-			bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i],
-				      addr, vlan_id);
-			continue;
+		/* bond device itself */
+		if (rt->dst.dev == bond->dev)
+			goto found;
+
+		/* search for upper device, like vlan/bridge/team/etc */
+		rcu_read_lock();
+		netdev_for_each_upper_dev(bond->dev, upper, iter) {
+			if (upper == rt->dst.dev) {
+				/* if it's a vlan - get its VID */
+				if (is_vlan_dev(rt->dst.dev))
+					vlan_id = vlan_dev_vlan_id(rt->dst.dev);
+
+				rcu_read_unlock();
+				goto found;
+			}
 		}
+		rcu_read_unlock();
 
-		if (net_ratelimit()) {
+		/* Not our device - skip */
+		if (net_ratelimit())
 			pr_warning("%s: no path to arp_ip_target %pI4 via rt.dev %s\n",
 				   bond->dev->name, &targets[i],
 				   rt->dst.dev ? rt->dst.dev->name : "NULL");
-		}
 		ip_rt_put(rt);
+		continue;
+
+found:
+		addr = bond_confirm_addr(rt->dst.dev, targets[i], 0);
+		ip_rt_put(rt);
+		bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i],
+			      addr, vlan_id);
 	}
 }
 
-- 
1.7.1

^ permalink raw reply related

* [PATCH net-next v1 1/9] net: add netdev_upper_get_next_dev(dev, iter)
From: Veaceslav Falico @ 2013-08-26 20:32 UTC (permalink / raw)
  To: netdev
  Cc: Veaceslav Falico, David S. Miller, Eric Dumazet, Jiri Pirko,
	Alexander Duyck, Cong Wang
In-Reply-To: <1377549162-7522-1-git-send-email-vfalico@redhat.com>

This function returns the next dev in the dev->upper_dev_list after the
struct list_head **iter position, and updates *iter accordingly. Returns
NULL if there are no devices left.

v1: new patch

CC: "David S. Miller" <davem@davemloft.net>
CC: Eric Dumazet <edumazet@google.com>
CC: Jiri Pirko <jiri@resnulli.us>
CC: Alexander Duyck <alexander.h.duyck@intel.com>
CC: Cong Wang <amwang@redhat.com>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
 net/core/dev.c |   25 +++++++++++++++++++++++++
 1 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 1ed2b66..566e99a 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4477,6 +4477,31 @@ struct net_device *netdev_master_upper_dev_get(struct net_device *dev)
 }
 EXPORT_SYMBOL(netdev_master_upper_dev_get);
 
+/* netdev_upper_get_next_dev - Get the next dev from upper list
+ * @dev: device
+ * @iter: list_head ** of the current position
+ *
+ * Gets the next device from the dev's upper list, starting from iter
+ * position. The caller must hold RCU read lock.
+ */
+struct net_device *netdev_upper_get_next_dev(struct net_device *dev,
+					     struct list_head **iter)
+{
+	struct netdev_upper *upper;
+
+	WARN_ON_ONCE(!rcu_read_lock_held());
+
+	upper = list_entry_rcu((*iter)->next, struct netdev_upper, list);
+
+	if (&upper->list == &dev->upper_dev_list)
+		return NULL;
+
+	*iter = &upper->list;
+
+	return upper->dev;
+}
+EXPORT_SYMBOL(netdev_upper_get_next_dev);
+
 /**
  * netdev_master_upper_dev_get_rcu - Get master upper device
  * @dev: device
-- 
1.7.1

^ permalink raw reply related

* [PATCH net-next v1 5/9] bonding: convert bond_has_this_ip() to use upper devices
From: Veaceslav Falico @ 2013-08-26 20:32 UTC (permalink / raw)
  To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1377549162-7522-1-git-send-email-vfalico@redhat.com>

Currently, bond_has_this_ip() is aware only of vlan upper devices, and thus
will return false if the address is associated with the upper bridge or any
other device, and thus will break the arp logic.

Fix this by using the upper device list. For every upper device we verify
if the address associated with it is our address, and if yes - return true.

v1: use netdev_for_each_upper_dev()

CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
 drivers/net/bonding/bond_main.c |   25 +++++++++++++------------
 1 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 40eb250..dbac1ce 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2392,24 +2392,25 @@ re_arm:
 	}
 }
 
-static int bond_has_this_ip(struct bonding *bond, __be32 ip)
+static bool bond_has_this_ip(struct bonding *bond, __be32 ip)
 {
-	struct vlan_entry *vlan;
-	struct net_device *vlan_dev;
+	struct net_device *upper;
+	struct list_head *iter;
+	bool ret = false;
 
 	if (ip == bond_confirm_addr(bond->dev, 0, ip))
-		return 1;
+		return true;
 
-	list_for_each_entry(vlan, &bond->vlan_list, vlan_list) {
-		rcu_read_lock();
-		vlan_dev = __vlan_find_dev_deep(bond->dev, htons(ETH_P_8021Q),
-						vlan->vlan_id);
-		rcu_read_unlock();
-		if (vlan_dev && ip == bond_confirm_addr(vlan_dev, 0, ip))
-			return 1;
+	rcu_read_lock();
+	netdev_for_each_upper_dev(bond->dev, upper, iter) {
+		if (ip == bond_confirm_addr(upper, 0, ip)) {
+			ret = true;
+			break;
+		}
 	}
+	rcu_read_unlock();
 
-	return 0;
+	return ret;
 }
 
 /*
-- 
1.7.1

^ permalink raw reply related

* [PATCH net-next v1 0/9] bonding: remove vlan special handling
From: Veaceslav Falico @ 2013-08-26 20:32 UTC (permalink / raw)
  To: netdev
  Cc: Jay Vosburgh, Andy Gospodarek, David S. Miller, Eric Dumazet,
	Jiri Pirko, Alexander Duyck, Cong Wang, Veaceslav Falico

v1: Per Jiri's advice, remove the exported netdev_upper struct to keep it
    inside dev.c only, and instead implement a macro to iterate over the
    list and return only net_device *.

The aim of this patchset is to remove bondings' own vlan handling as much
as possible and replace it with the netdev upper device functionality.

This is achieved by adding a helper function to upper dev list handling -
netdev_upper_get_next_dev(dev, iter), which returns the next device after
the list_head **iter, and sets *iter to the next list_head *. This patchset
also adds netdev_for_each_upper_dev(dev, upper, iter), which iterates
through the whole dev->upper_dev_list, setting upper to the net_device.
The only special treatment of vlans remains in rlb code.

This patchset solves several issues with bonding, simplifies it overall,
RCUify further and exports upper list functions for any other users which
might also want to get rid of its own vlan_lists.

I'm testing it continuously currently, no issues found, will update on
anything.

CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
CC: "David S. Miller" <davem@davemloft.net>
CC: Eric Dumazet <edumazet@google.com>
CC: Jiri Pirko <jiri@resnulli.us>
CC: Alexander Duyck <alexander.h.duyck@intel.com>
CC: Cong Wang <amwang@redhat.com>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>

---
 drivers/net/bonding/bond_alb.c  |   75 ++++++------
 drivers/net/bonding/bond_alb.h  |    2 -
 drivers/net/bonding/bond_main.c |  244 ++++++++-------------------------------
 drivers/net/bonding/bonding.h   |   21 +++-
 include/linux/netdevice.h       |   10 ++
 net/core/dev.c                  |   25 ++++
 6 files changed, 133 insertions(+), 244 deletions(-)

^ permalink raw reply

* [PATCH net-next v1 2/9] net: add netdev_for_each_upper_dev()
From: Veaceslav Falico @ 2013-08-26 20:32 UTC (permalink / raw)
  To: netdev
  Cc: Veaceslav Falico, David S. Miller, Eric Dumazet, Jiri Pirko,
	Alexander Duyck, Cong Wang
In-Reply-To: <1377549162-7522-1-git-send-email-vfalico@redhat.com>

The new macro netdev_for_each_upper_dev(dev, upper, iter) iterates through
the dev->upper_dev_list starting from the first element, using the
netdev_upper_get_next_dev(dev, &iter).

Must be called under RCU read lock.

v1: new patch

CC: "David S. Miller" <davem@davemloft.net>
CC: Eric Dumazet <edumazet@google.com>
CC: Jiri Pirko <jiri@resnulli.us>
CC: Alexander Duyck <alexander.h.duyck@intel.com>
CC: Cong Wang <amwang@redhat.com>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
 include/linux/netdevice.h |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 077363d..85590b6 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2767,6 +2767,16 @@ extern int		bpf_jit_enable;
 extern bool netdev_has_upper_dev(struct net_device *dev,
 				 struct net_device *upper_dev);
 extern bool netdev_has_any_upper_dev(struct net_device *dev);
+extern struct net_device *netdev_upper_get_next_dev(struct net_device *dev,
+						    struct list_head **iter);
+
+/* iterate through upper list, must be called under RCU read lock */
+#define netdev_for_each_upper_dev(dev, upper, iter) \
+	for (iter = &(dev)->upper_dev_list, \
+	     upper = netdev_upper_get_next_dev(dev, &(iter)); \
+	     upper; \
+	     upper = netdev_upper_get_next_dev(dev, &(iter)))
+
 extern struct net_device *netdev_master_upper_dev_get(struct net_device *dev);
 extern struct net_device *netdev_master_upper_dev_get_rcu(struct net_device *dev);
 extern int netdev_upper_dev_link(struct net_device *dev,
-- 
1.7.1

^ permalink raw reply related

* [PATCH net-next v1 8/9] bonding: make alb_send_learning_packets() use upper dev list
From: Veaceslav Falico @ 2013-08-26 20:32 UTC (permalink / raw)
  To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1377549162-7522-1-git-send-email-vfalico@redhat.com>

Currently, if there are vlans on top of bond, alb_send_learning_packets()
will never send LPs from the bond itself (i.e. untagged), which might leave
untagged clients unupdated.

Also, the 'circular vlan' logic (i.e. update only MAX_LP_BURST vlans at a
time, and save the last vlan for the next update) is really suboptimal - in
case of lots of vlans it will take a lot of time to update every vlan. It
is also never called in any hot path and sends only a few small packets -
thus the optimization by itself is useless.

So remove the whole current_alb_vlan/MAX_LP_BURST logic from
alb_send_learning_packets(). Instead, we'll first send a packet untagged
and then traverse the upper dev list, sending a tagged packet for each vlan
found. Also, remove the MAX_LP_BURST define - we already don't need it.

v1: use netdev_for_each_upper_dev()

CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
 drivers/net/bonding/bond_alb.c |   33 +++++++++++++--------------------
 drivers/net/bonding/bond_alb.h |    1 -
 2 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 3ca3c85..676d6d6 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -1013,27 +1013,20 @@ static void alb_send_lp_vid(struct slave *slave, u8 mac_addr[],
 static void alb_send_learning_packets(struct slave *slave, u8 mac_addr[])
 {
 	struct bonding *bond = bond_get_bond_by_slave(slave);
-	u16 vlan_id;
-	int i;
-
-	for (i = 0; i < MAX_LP_BURST; i++) {
-		vlan_id = 0;
-
-		if (bond_vlan_used(bond)) {
-			struct vlan_entry *vlan;
+	struct net_device *upper;
+	struct list_head *iter;
 
-			vlan = bond_next_vlan(bond,
-					      bond->alb_info.current_alb_vlan);
-
-			bond->alb_info.current_alb_vlan = vlan;
-			if (!vlan)
-				continue;
-
-			vlan_id = vlan->vlan_id;
-		}
+	/* send untagged */
+	alb_send_lp_vid(slave, mac_addr, 0);
 
-		alb_send_lp_vid(slave, mac_addr, vlan_id);
+	/* loop through vlans and send one packet for each */
+	rcu_read_lock();
+	netdev_for_each_upper_dev(bond->dev, upper, iter) {
+		if (upper->priv_flags & IFF_802_1Q_VLAN)
+			alb_send_lp_vid(slave, mac_addr,
+					vlan_dev_vlan_id(upper));
 	}
+	rcu_read_unlock();
 }
 
 static int alb_set_slave_mac_addr(struct slave *slave, u8 addr[])
diff --git a/drivers/net/bonding/bond_alb.h b/drivers/net/bonding/bond_alb.h
index e7a5b8b..e139172 100644
--- a/drivers/net/bonding/bond_alb.h
+++ b/drivers/net/bonding/bond_alb.h
@@ -53,7 +53,6 @@ struct slave;
 
 
 #define TLB_NULL_INDEX		0xffffffff
-#define MAX_LP_BURST		3
 
 /* rlb defs */
 #define RLB_HASH_TABLE_SIZE	256
-- 
1.7.1

^ permalink raw reply related

* [PATCH net-next v1 9/9] bonding: remove vlan_list/current_alb_vlan
From: Veaceslav Falico @ 2013-08-26 20:32 UTC (permalink / raw)
  To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1377549162-7522-1-git-send-email-vfalico@redhat.com>

Currently there are no real users of vlan_list/current_alb_vlan, only the
helpers which maintain them, so remove them.

v1: no change

CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
 drivers/net/bonding/bond_alb.c  |    5 --
 drivers/net/bonding/bond_alb.h  |    1 -
 drivers/net/bonding/bond_main.c |  133 +--------------------------------------
 drivers/net/bonding/bonding.h   |    6 --
 4 files changed, 2 insertions(+), 143 deletions(-)

diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 676d6d6..17e2cef 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -1763,11 +1763,6 @@ int bond_alb_set_mac_address(struct net_device *bond_dev, void *addr)
 
 void bond_alb_clear_vlan(struct bonding *bond, unsigned short vlan_id)
 {
-	if (bond->alb_info.current_alb_vlan &&
-	    (bond->alb_info.current_alb_vlan->vlan_id == vlan_id)) {
-		bond->alb_info.current_alb_vlan = NULL;
-	}
-
 	if (bond->alb_info.rlb_enabled) {
 		rlb_clear_vlan(bond, vlan_id);
 	}
diff --git a/drivers/net/bonding/bond_alb.h b/drivers/net/bonding/bond_alb.h
index e139172..e02c9c5 100644
--- a/drivers/net/bonding/bond_alb.h
+++ b/drivers/net/bonding/bond_alb.h
@@ -169,7 +169,6 @@ struct alb_bond_info {
 						 * rx traffic should be
 						 * rebalanced
 						 */
-	struct vlan_entry	*current_alb_vlan;
 };
 
 int bond_alb_initialize(struct bonding *bond, int rlb_enabled);
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index aaf14a0..3757558 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -283,116 +283,6 @@ const char *bond_mode_name(int mode)
 /*---------------------------------- VLAN -----------------------------------*/
 
 /**
- * bond_add_vlan - add a new vlan id on bond
- * @bond: bond that got the notification
- * @vlan_id: the vlan id to add
- *
- * Returns -ENOMEM if allocation failed.
- */
-static int bond_add_vlan(struct bonding *bond, unsigned short vlan_id)
-{
-	struct vlan_entry *vlan;
-
-	pr_debug("bond: %s, vlan id %d\n",
-		 (bond ? bond->dev->name : "None"), vlan_id);
-
-	vlan = kzalloc(sizeof(struct vlan_entry), GFP_KERNEL);
-	if (!vlan)
-		return -ENOMEM;
-
-	INIT_LIST_HEAD(&vlan->vlan_list);
-	vlan->vlan_id = vlan_id;
-
-	write_lock_bh(&bond->lock);
-
-	list_add_tail(&vlan->vlan_list, &bond->vlan_list);
-
-	write_unlock_bh(&bond->lock);
-
-	pr_debug("added VLAN ID %d on bond %s\n", vlan_id, bond->dev->name);
-
-	return 0;
-}
-
-/**
- * bond_del_vlan - delete a vlan id from bond
- * @bond: bond that got the notification
- * @vlan_id: the vlan id to delete
- *
- * returns -ENODEV if @vlan_id was not found in @bond.
- */
-static int bond_del_vlan(struct bonding *bond, unsigned short vlan_id)
-{
-	struct vlan_entry *vlan;
-	int res = -ENODEV;
-
-	pr_debug("bond: %s, vlan id %d\n", bond->dev->name, vlan_id);
-
-	block_netpoll_tx();
-	write_lock_bh(&bond->lock);
-
-	list_for_each_entry(vlan, &bond->vlan_list, vlan_list) {
-		if (vlan->vlan_id == vlan_id) {
-			list_del(&vlan->vlan_list);
-
-			if (bond_is_lb(bond))
-				bond_alb_clear_vlan(bond, vlan_id);
-
-			pr_debug("removed VLAN ID %d from bond %s\n",
-				 vlan_id, bond->dev->name);
-
-			kfree(vlan);
-
-			res = 0;
-			goto out;
-		}
-	}
-
-	pr_debug("couldn't find VLAN ID %d in bond %s\n",
-		 vlan_id, bond->dev->name);
-
-out:
-	write_unlock_bh(&bond->lock);
-	unblock_netpoll_tx();
-	return res;
-}
-
-/**
- * bond_next_vlan - safely skip to the next item in the vlans list.
- * @bond: the bond we're working on
- * @curr: item we're advancing from
- *
- * Returns %NULL if list is empty, bond->next_vlan if @curr is %NULL,
- * or @curr->next otherwise (even if it is @curr itself again).
- *
- * Caller must hold bond->lock
- */
-struct vlan_entry *bond_next_vlan(struct bonding *bond, struct vlan_entry *curr)
-{
-	struct vlan_entry *next, *last;
-
-	if (list_empty(&bond->vlan_list))
-		return NULL;
-
-	if (!curr) {
-		next = list_entry(bond->vlan_list.next,
-				  struct vlan_entry, vlan_list);
-	} else {
-		last = list_entry(bond->vlan_list.prev,
-				  struct vlan_entry, vlan_list);
-		if (last == curr) {
-			next = list_entry(bond->vlan_list.next,
-					  struct vlan_entry, vlan_list);
-		} else {
-			next = list_entry(curr->vlan_list.next,
-					  struct vlan_entry, vlan_list);
-		}
-	}
-
-	return next;
-}
-
-/**
  * bond_dev_queue_xmit - Prepare skb for xmit.
  *
  * @bond: bond device that got this skb for tx.
@@ -451,13 +341,6 @@ static int bond_vlan_rx_add_vid(struct net_device *bond_dev,
 			goto unwind;
 	}
 
-	res = bond_add_vlan(bond, vid);
-	if (res) {
-		pr_err("%s: Error: Failed to add vlan id %d\n",
-		       bond_dev->name, vid);
-		goto unwind;
-	}
-
 	return 0;
 
 unwind:
@@ -478,17 +361,12 @@ static int bond_vlan_rx_kill_vid(struct net_device *bond_dev,
 {
 	struct bonding *bond = netdev_priv(bond_dev);
 	struct slave *slave;
-	int res;
 
 	bond_for_each_slave(bond, slave)
 		vlan_vid_del(slave->dev, proto, vid);
 
-	res = bond_del_vlan(bond, vid);
-	if (res) {
-		pr_err("%s: Error: Failed to remove vlan id %d\n",
-		       bond_dev->name, vid);
-		return res;
-	}
+	if (bond_is_lb(bond))
+		bond_alb_clear_vlan(bond, vid);
 
 	return 0;
 }
@@ -4121,7 +3999,6 @@ static void bond_setup(struct net_device *bond_dev)
 
 	/* Initialize pointers */
 	bond->dev = bond_dev;
-	INIT_LIST_HEAD(&bond->vlan_list);
 
 	/* Initialize the device entry points */
 	ether_setup(bond_dev);
@@ -4174,7 +4051,6 @@ static void bond_uninit(struct net_device *bond_dev)
 {
 	struct bonding *bond = netdev_priv(bond_dev);
 	struct slave *slave, *tmp_slave;
-	struct vlan_entry *vlan, *tmp;
 
 	bond_netpoll_cleanup(bond_dev);
 
@@ -4186,11 +4062,6 @@ static void bond_uninit(struct net_device *bond_dev)
 	list_del(&bond->bond_list);
 
 	bond_debug_unregister(bond);
-
-	list_for_each_entry_safe(vlan, tmp, &bond->vlan_list, vlan_list) {
-		list_del(&vlan->vlan_list);
-		kfree(vlan);
-	}
 }
 
 /*------------------------- Module initialization ---------------------------*/
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 9392e00..a26b134 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -185,11 +185,6 @@ struct bond_parm_tbl {
 
 #define BOND_MAX_MODENAME_LEN 20
 
-struct vlan_entry {
-	struct list_head vlan_list;
-	unsigned short vlan_id;
-};
-
 struct slave {
 	struct net_device *dev; /* first - useful for panic debug */
 	struct list_head list;
@@ -254,7 +249,6 @@ struct bonding {
 	struct   ad_bond_info ad_info;
 	struct   alb_bond_info alb_info;
 	struct   bond_params params;
-	struct   list_head vlan_list;
 	struct   workqueue_struct *wq;
 	struct   delayed_work mii_work;
 	struct   delayed_work arp_work;
-- 
1.7.1

^ permalink raw reply related

* [PATCH net-next v1 6/9] bonding: use vlan_uses_dev() in __bond_release_one()
From: Veaceslav Falico @ 2013-08-26 20:32 UTC (permalink / raw)
  To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1377549162-7522-1-git-send-email-vfalico@redhat.com>

We always hold the rtnl_lock() in __bond_release_one(), so use
vlan_uses_dev() instead of bond_vlan_used().

v1: no change

CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
 drivers/net/bonding/bond_main.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index dbac1ce..aaf14a0 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1954,7 +1954,7 @@ static int __bond_release_one(struct net_device *bond_dev,
 		bond_set_carrier(bond);
 		eth_hw_addr_random(bond_dev);
 
-		if (bond_vlan_used(bond)) {
+		if (vlan_uses_dev(bond_dev)) {
 			pr_warning("%s: Warning: clearing HW address of %s while it still has VLANs.\n",
 				   bond_dev->name, bond_dev->name);
 			pr_warning("%s: When re-adding slaves, make sure the bond's HW address matches its VLANs'.\n",
-- 
1.7.1

^ permalink raw reply related

* [PATCH net-next v1 7/9] bonding: split alb_send_learning_packets()
From: Veaceslav Falico @ 2013-08-26 20:32 UTC (permalink / raw)
  To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1377549162-7522-1-git-send-email-vfalico@redhat.com>

Create alb_send_lp_vid(), which will handle the skb/lp creation, vlan
tagging and sending, and use it in alb_send_learning_packets().

This way all the logic remains in alb_send_learning_packets(), which
becomes a lot more cleaner and easier to understand.

v1: no change

CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
 drivers/net/bonding/bond_alb.c |   59 +++++++++++++++++++++++----------------
 1 files changed, 35 insertions(+), 24 deletions(-)

diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 3a5db7b..3ca3c85 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -971,35 +971,53 @@ static void rlb_clear_vlan(struct bonding *bond, unsigned short vlan_id)
 
 /*********************** tlb/rlb shared functions *********************/
 
-static void alb_send_learning_packets(struct slave *slave, u8 mac_addr[])
+static void alb_send_lp_vid(struct slave *slave, u8 mac_addr[],
+			    u16 vid)
 {
-	struct bonding *bond = bond_get_bond_by_slave(slave);
 	struct learning_pkt pkt;
+	struct sk_buff *skb;
 	int size = sizeof(struct learning_pkt);
-	int i;
+	char *data;
 
 	memset(&pkt, 0, size);
 	memcpy(pkt.mac_dst, mac_addr, ETH_ALEN);
 	memcpy(pkt.mac_src, mac_addr, ETH_ALEN);
 	pkt.type = cpu_to_be16(ETH_P_LOOP);
 
-	for (i = 0; i < MAX_LP_BURST; i++) {
-		struct sk_buff *skb;
-		char *data;
+	skb = dev_alloc_skb(size);
+	if (!skb)
+		return;
 
-		skb = dev_alloc_skb(size);
+	data = skb_put(skb, size);
+	memcpy(data, &pkt, size);
+
+	skb_reset_mac_header(skb);
+	skb->network_header = skb->mac_header + ETH_HLEN;
+	skb->protocol = pkt.type;
+	skb->priority = TC_PRIO_CONTROL;
+	skb->dev = slave->dev;
+
+	if (vid) {
+		skb = vlan_put_tag(skb, htons(ETH_P_8021Q), vid);
 		if (!skb) {
+			pr_err("%s: Error: failed to insert VLAN tag\n",
+			       slave->bond->dev->name);
 			return;
 		}
+	}
 
-		data = skb_put(skb, size);
-		memcpy(data, &pkt, size);
+	dev_queue_xmit(skb);
+}
 
-		skb_reset_mac_header(skb);
-		skb->network_header = skb->mac_header + ETH_HLEN;
-		skb->protocol = pkt.type;
-		skb->priority = TC_PRIO_CONTROL;
-		skb->dev = slave->dev;
+
+static void alb_send_learning_packets(struct slave *slave, u8 mac_addr[])
+{
+	struct bonding *bond = bond_get_bond_by_slave(slave);
+	u16 vlan_id;
+	int i;
+
+	for (i = 0; i < MAX_LP_BURST; i++) {
+		vlan_id = 0;
 
 		if (bond_vlan_used(bond)) {
 			struct vlan_entry *vlan;
@@ -1008,20 +1026,13 @@ static void alb_send_learning_packets(struct slave *slave, u8 mac_addr[])
 					      bond->alb_info.current_alb_vlan);
 
 			bond->alb_info.current_alb_vlan = vlan;
-			if (!vlan) {
-				kfree_skb(skb);
+			if (!vlan)
 				continue;
-			}
 
-			skb = vlan_put_tag(skb, htons(ETH_P_8021Q), vlan->vlan_id);
-			if (!skb) {
-				pr_err("%s: Error: failed to insert VLAN tag\n",
-				       bond->dev->name);
-				continue;
-			}
+			vlan_id = vlan->vlan_id;
 		}
 
-		dev_queue_xmit(skb);
+		alb_send_lp_vid(slave, mac_addr, vlan_id);
 	}
 }
 
-- 
1.7.1

^ permalink raw reply related

* [PATCH net-next v1 3/9] bonding: use netdev_upper list in bond_vlan_used
From: Veaceslav Falico @ 2013-08-26 20:32 UTC (permalink / raw)
  To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1377549162-7522-1-git-send-email-vfalico@redhat.com>

Convert bond_vlan_used() to traverse the upper device list to see if we
have any vlans above us. It's protected by rcu, and in case we are holding
rtnl_lock we should call vlan_uses_dev() instead - it's faster.

v1: use netdev_for_each_upper_dev()

CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
 drivers/net/bonding/bonding.h |   15 ++++++++++++++-
 1 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 4bf52d5..9392e00 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -267,9 +267,22 @@ struct bonding {
 #endif /* CONFIG_DEBUG_FS */
 };
 
+/* if we hold rtnl_lock() - call vlan_uses_dev() */
 static inline bool bond_vlan_used(struct bonding *bond)
 {
-	return !list_empty(&bond->vlan_list);
+	struct net_device *upper;
+	struct list_head *iter;
+
+	rcu_read_lock();
+	netdev_for_each_upper_dev(bond->dev, upper, iter) {
+		if (upper->priv_flags & IFF_802_1Q_VLAN) {
+			rcu_read_unlock();
+			return true;
+		}
+	}
+	rcu_read_unlock();
+
+	return false;
 }
 
 #define bond_slave_get_rcu(dev) \
-- 
1.7.1

^ permalink raw reply related

* Re: [PATCH net-next 0/8] bonding: remove vlan special handling
From: Veaceslav Falico @ 2013-08-26 20:33 UTC (permalink / raw)
  To: netdev
  Cc: Jay Vosburgh, Andy Gospodarek, David S. Miller, Eric Dumazet,
	Jiri Pirko, Alexander Duyck, Cong Wang
In-Reply-To: <1377534533-6944-1-git-send-email-vfalico@redhat.com>

On Mon, Aug 26, 2013 at 06:28:45PM +0200, Veaceslav Falico wrote:
>The aim of this patchset is to remove bondings' own vlan handling as much
>as possible and replace it with the netdev upper device functionality.
>
>This is achieved by exporting the netdev_upper structure and thus permiting
>bonding to work directly with its upper devices. The only non-bonding
>change is the exporting of netdev_upper, and the only special treatment of
>vlans left is in the rlb mode.
>
>This patchset solves several issues with bonding, simplifies it overall,
>RCUify further and exports netdev_upper for any other users which might
>also want to get rid of its own vlan_lists.
>
>I'm testing it continuously currently, no issues found, will update on
>anything.

Self-NAK on the whole series, changed the upper device management approach,
as suggested by Jiri, and sent the new version.

^ permalink raw reply

* [guv v2 04/31] net: Replace __get_cpu_var uses
From: Christoph Lameter @ 2013-08-26 20:44 UTC (permalink / raw)
  To: Tejun Heo
  Cc: akpm, David S. Miller, netdev, linux-arch, Steven Rostedt,
	linux-kernel
In-Reply-To: <20130826204351.725357339@linux.com>

__get_cpu_var() is used for multiple purposes in the kernel source. One of them is
address calculation via the form &__get_cpu_var(x). This calculates the address for
the instance of the percpu variable of the current processor based on an offset.

Others usage cases are for storing and retrieving data from the current processors percpu area.
__get_cpu_var() can be used as an lvalue when writing data or on the right side of an assignment.

__get_cpu_var() is defined as :


#define __get_cpu_var(var) (*this_cpu_ptr(&(var)))



__get_cpu_var() always only does a address determination. However, store and retrieve operations
could use a segment prefix (or global register on other platforms) to avoid the address calculation.

this_cpu_write() and this_cpu_read() can directly take an offset into a percpu area and use
optimized assembly code to read and write per cpu variables.


This patch converts __get_cpu_var into either and explicit address calculation using this_cpu_ptr()
or into a use of this_cpu operations that use the offset. Thereby address calcualtions are avoided
and less registers are used when code is generated.

At the end of the patchset all uses of __get_cpu_var have been removed so the macro is removed too.

The patchset includes passes over all arches as well. Once these operations are used throughout then
specialized macros can be defined in non -x86 arches as well in order to optimize per cpu access by
f.e. using a global register that may be set to the per cpu base.




Transformations done to __get_cpu_var()


1. Determine the address of the percpu instance of the current processor.

	DEFINE_PER_CPU(int, y);
	int *x = &__get_cpu_var(y);

    Converts to

	int *x = this_cpu_ptr(&y);


2. Same as #1 but this time an array structure is involved.

	DEFINE_PER_CPU(int, y[20]);
	int *x = __get_cpu_var(y);

    Converts to

	int *x = this_cpu_ptr(y);


3. Retrieve the content of the current processors instance of a per cpu variable.

	DEFINE_PER_CPU(int, u);
	int x = __get_cpu_var(y)

   Converts to

	int x = __this_cpu_read(y);


4. Retrieve the content of a percpu struct

	DEFINE_PER_CPU(struct mystruct, y);
	struct mystruct x = __get_cpu_var(y);

   Converts to

	memcpy(this_cpu_ptr(&y), x, sizeof(x));


5. Assignment to a per cpu variable

	DEFINE_PER_CPU(int, y)
	__get_cpu_var(y) = x;

   Converts to

	this_cpu_write(y, x);


6. Increment/Decrement etc of a per cpu variable

	DEFINE_PER_CPU(int, y);
	__get_cpu_var(y)++

   Converts to

	this_cpu_inc(y)

Signed-off-by: Christoph Lameter <cl@linux.com>

Index: linux/net/core/dev.c
===================================================================
--- linux.orig/net/core/dev.c	2013-08-26 14:16:59.000000000 -0500
+++ linux/net/core/dev.c	2013-08-26 14:18:37.214005168 -0500
@@ -2129,7 +2129,7 @@ static inline void __netif_reschedule(st
 	unsigned long flags;
 
 	local_irq_save(flags);
-	sd = &__get_cpu_var(softnet_data);
+	sd = this_cpu_ptr(&softnet_data);
 	q->next_sched = NULL;
 	*sd->output_queue_tailp = q;
 	sd->output_queue_tailp = &q->next_sched;
@@ -2151,7 +2151,7 @@ void dev_kfree_skb_irq(struct sk_buff *s
 		unsigned long flags;
 
 		local_irq_save(flags);
-		sd = &__get_cpu_var(softnet_data);
+		sd = this_cpu_ptr(&softnet_data);
 		skb->next = sd->completion_queue;
 		sd->completion_queue = skb;
 		raise_softirq_irqoff(NET_TX_SOFTIRQ);
@@ -3111,7 +3111,7 @@ static void rps_trigger_softirq(void *da
 static int rps_ipi_queued(struct softnet_data *sd)
 {
 #ifdef CONFIG_RPS
-	struct softnet_data *mysd = &__get_cpu_var(softnet_data);
+	struct softnet_data *mysd = this_cpu_ptr(&softnet_data);
 
 	if (sd != mysd) {
 		sd->rps_ipi_next = mysd->rps_ipi_list;
@@ -3138,7 +3138,7 @@ static bool skb_flow_limit(struct sk_buf
 	if (qlen < (netdev_max_backlog >> 1))
 		return false;
 
-	sd = &__get_cpu_var(softnet_data);
+	sd = this_cpu_ptr(&softnet_data);
 
 	rcu_read_lock();
 	fl = rcu_dereference(sd->flow_limit);
@@ -3280,7 +3280,7 @@ EXPORT_SYMBOL(netif_rx_ni);
 
 static void net_tx_action(struct softirq_action *h)
 {
-	struct softnet_data *sd = &__get_cpu_var(softnet_data);
+	struct softnet_data *sd = this_cpu_ptr(&softnet_data);
 
 	if (sd->completion_queue) {
 		struct sk_buff *clist;
@@ -3700,7 +3700,7 @@ EXPORT_SYMBOL(netif_receive_skb);
 static void flush_backlog(void *arg)
 {
 	struct net_device *dev = arg;
-	struct softnet_data *sd = &__get_cpu_var(softnet_data);
+	struct softnet_data *sd = this_cpu_ptr(&softnet_data);
 	struct sk_buff *skb, *tmp;
 
 	rps_lock(sd);
@@ -4146,7 +4146,7 @@ void __napi_schedule(struct napi_struct
 	unsigned long flags;
 
 	local_irq_save(flags);
-	____napi_schedule(&__get_cpu_var(softnet_data), n);
+	____napi_schedule(this_cpu_ptr(&softnet_data), n);
 	local_irq_restore(flags);
 }
 EXPORT_SYMBOL(__napi_schedule);
@@ -4274,7 +4274,7 @@ EXPORT_SYMBOL(netif_napi_del);
 
 static void net_rx_action(struct softirq_action *h)
 {
-	struct softnet_data *sd = &__get_cpu_var(softnet_data);
+	struct softnet_data *sd = this_cpu_ptr(&softnet_data);
 	unsigned long time_limit = jiffies + 2;
 	int budget = netdev_budget;
 	void *have;
Index: linux/net/core/drop_monitor.c
===================================================================
--- linux.orig/net/core/drop_monitor.c	2013-08-26 14:16:59.000000000 -0500
+++ linux/net/core/drop_monitor.c	2013-08-26 14:18:37.218005126 -0500
@@ -142,7 +142,7 @@ static void trace_drop_common(struct sk_
 	unsigned long flags;
 
 	local_irq_save(flags);
-	data = &__get_cpu_var(dm_cpu_data);
+	data = this_cpu_ptr(&dm_cpu_data);
 	spin_lock(&data->lock);
 	dskb = data->skb;
 
Index: linux/net/core/skbuff.c
===================================================================
--- linux.orig/net/core/skbuff.c	2013-08-26 14:16:59.000000000 -0500
+++ linux/net/core/skbuff.c	2013-08-26 14:18:37.218005126 -0500
@@ -371,7 +371,7 @@ static void *__netdev_alloc_frag(unsigne
 	unsigned long flags;
 
 	local_irq_save(flags);
-	nc = &__get_cpu_var(netdev_alloc_cache);
+	nc = this_cpu_ptr(&netdev_alloc_cache);
 	if (unlikely(!nc->frag.page)) {
 refill:
 		for (order = NETDEV_FRAG_PAGE_MAX_ORDER; ;) {
Index: linux/net/ipv4/syncookies.c
===================================================================
--- linux.orig/net/ipv4/syncookies.c	2013-08-26 14:16:59.000000000 -0500
+++ linux/net/ipv4/syncookies.c	2013-08-26 14:18:37.210005210 -0500
@@ -44,7 +44,7 @@ static DEFINE_PER_CPU(__u32 [16 + 5 + SH
 static u32 cookie_hash(__be32 saddr, __be32 daddr, __be16 sport, __be16 dport,
 		       u32 count, int c)
 {
-	__u32 *tmp = __get_cpu_var(ipv4_cookie_scratch);
+	__u32 *tmp = this_cpu_ptr(ipv4_cookie_scratch);
 
 	memcpy(tmp + 4, syncookie_secret[c], sizeof(syncookie_secret[c]));
 	tmp[0] = (__force u32)saddr;
Index: linux/net/ipv4/tcp_output.c
===================================================================
--- linux.orig/net/ipv4/tcp_output.c	2013-08-26 14:16:59.000000000 -0500
+++ linux/net/ipv4/tcp_output.c	2013-08-26 14:18:37.218005126 -0500
@@ -810,7 +810,7 @@ void tcp_wfree(struct sk_buff *skb)
 
 		/* queue this socket to tasklet queue */
 		local_irq_save(flags);
-		tsq = &__get_cpu_var(tsq_tasklet);
+		tsq = this_cpu_ptr(&tsq_tasklet);
 		list_add(&tp->tsq_node, &tsq->head);
 		tasklet_schedule(&tsq->tasklet);
 		local_irq_restore(flags);
Index: linux/net/ipv6/syncookies.c
===================================================================
--- linux.orig/net/ipv6/syncookies.c	2013-08-26 14:16:59.000000000 -0500
+++ linux/net/ipv6/syncookies.c	2013-08-26 14:18:37.214005168 -0500
@@ -66,7 +66,7 @@ static DEFINE_PER_CPU(__u32 [16 + 5 + SH
 static u32 cookie_hash(const struct in6_addr *saddr, const struct in6_addr *daddr,
 		       __be16 sport, __be16 dport, u32 count, int c)
 {
-	__u32 *tmp = __get_cpu_var(ipv6_cookie_scratch);
+	__u32 *tmp = this_cpu_ptr(ipv6_cookie_scratch);
 
 	/*
 	 * we have 320 bits of information to hash, copy in the remaining
Index: linux/net/rds/ib_rdma.c
===================================================================
--- linux.orig/net/rds/ib_rdma.c	2013-08-26 14:16:59.000000000 -0500
+++ linux/net/rds/ib_rdma.c	2013-08-26 14:18:37.218005126 -0500
@@ -267,7 +267,7 @@ static inline struct rds_ib_mr *rds_ib_r
 	unsigned long *flag;
 
 	preempt_disable();
-	flag = &__get_cpu_var(clean_list_grace);
+	flag = this_cpu_ptr(&clean_list_grace);
 	set_bit(CLEAN_LIST_BUSY_BIT, flag);
 	ret = llist_del_first(&pool->clean_list);
 	if (ret)
Index: linux/include/net/netfilter/nf_conntrack.h
===================================================================
--- linux.orig/include/net/netfilter/nf_conntrack.h	2013-08-26 14:23:46.000000000 -0500
+++ linux/include/net/netfilter/nf_conntrack.h	2013-08-26 14:24:17.190395755 -0500
@@ -243,7 +243,7 @@ extern s16 (*nf_ct_nat_offset)(const str
 DECLARE_PER_CPU(struct nf_conn, nf_conntrack_untracked);
 static inline struct nf_conn *nf_ct_untracked_get(void)
 {
-	return &__raw_get_cpu_var(nf_conntrack_untracked);
+	return __this_cpu_ptr(&nf_conntrack_untracked);
 }
 extern void nf_ct_untracked_status_or(unsigned long bits);
 

^ permalink raw reply

* Re: [PATCH RESEND V3 net-next 2/3] net: huawei_cdc_ncm: Introduce the huawei_cdc_ncm driver
From: Joe Perches @ 2013-08-26 20:45 UTC (permalink / raw)
  To: David Miller; +Cc: mrkiko.rs, gregkh, oliver, linux-kernel, linux-usb, netdev
In-Reply-To: <20130826.163125.1134768821551948034.davem@davemloft.net>

On Mon, 2013-08-26 at 16:31 -0400, David Miller wrote:
> From: Enrico Mioso <mrkiko.rs@gmail.com>
[]
> > +     int ret = 0;
> > +     struct usbnet *usbnet_dev = usb_get_intfdata(intf);
> > +     struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
> > +     struct cdc_ncm_ctx *ctx = drvstate->ctx;
> > +     bool callsub = (intf == ctx->control && drvstate->subdriver && drvstate->subdriver->resume); /* should we call subdriver's resume? ? */
> 
> Likewise, and order local function variable declarations by line
> length, longest to shortest.

I think the premise of local variable
declaration by line length is flawed.

It can't work when local variables are
dependent on initialization order as
above.

int ret; could be moved down below callsub
if really desired though.

^ permalink raw reply

* Re: [PATCH net-next v1 5/9] bonding: convert bond_has_this_ip() to use upper devices
From: Jiri Pirko @ 2013-08-26 20:53 UTC (permalink / raw)
  To: Veaceslav Falico; +Cc: netdev, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1377549162-7522-6-git-send-email-vfalico@redhat.com>

Mon, Aug 26, 2013 at 10:32:38PM CEST, vfalico@redhat.com wrote:
>Currently, bond_has_this_ip() is aware only of vlan upper devices, and thus
>will return false if the address is associated with the upper bridge or any
>other device, and thus will break the arp logic.
>
>Fix this by using the upper device list. For every upper device we verify
>if the address associated with it is our address, and if yes - return true.
>
>v1: use netdev_for_each_upper_dev()
>
>CC: Jay Vosburgh <fubar@us.ibm.com>
>CC: Andy Gospodarek <andy@greyhouse.net>
>Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
>---
> drivers/net/bonding/bond_main.c |   25 +++++++++++++------------
> 1 files changed, 13 insertions(+), 12 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index 40eb250..dbac1ce 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -2392,24 +2392,25 @@ re_arm:
> 	}
> }
> 
>-static int bond_has_this_ip(struct bonding *bond, __be32 ip)
>+static bool bond_has_this_ip(struct bonding *bond, __be32 ip)
> {
>-	struct vlan_entry *vlan;
>-	struct net_device *vlan_dev;
>+	struct net_device *upper;
>+	struct list_head *iter;
>+	bool ret = false;
> 
> 	if (ip == bond_confirm_addr(bond->dev, 0, ip))
>-		return 1;
>+		return true;
> 
>-	list_for_each_entry(vlan, &bond->vlan_list, vlan_list) {
>-		rcu_read_lock();
>-		vlan_dev = __vlan_find_dev_deep(bond->dev, htons(ETH_P_8021Q),
>-						vlan->vlan_id);
>-		rcu_read_unlock();
>-		if (vlan_dev && ip == bond_confirm_addr(vlan_dev, 0, ip))
>-			return 1;
>+	rcu_read_lock();
>+	netdev_for_each_upper_dev(bond->dev, upper, iter) {
>+		if (ip == bond_confirm_addr(upper, 0, ip)) {
>+			ret = true;
>+			break;
>+		}

You need the same recursion __vlan_find_dev_deep() is doing. If you do
not do that, you will miss anything over the first upper level.


> 	}
>+	rcu_read_unlock();
> 
>-	return 0;
>+	return ret;
> }
> 
> /*
>-- 
>1.7.1
>
>--
>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 net-next v1 1/9] net: add netdev_upper_get_next_dev(dev, iter)
From: Jiri Pirko @ 2013-08-26 20:57 UTC (permalink / raw)
  To: Veaceslav Falico
  Cc: netdev, David S. Miller, Eric Dumazet, Alexander Duyck, Cong Wang
In-Reply-To: <1377549162-7522-2-git-send-email-vfalico@redhat.com>

Mon, Aug 26, 2013 at 10:32:34PM CEST, vfalico@redhat.com wrote:
>This function returns the next dev in the dev->upper_dev_list after the
>struct list_head **iter position, and updates *iter accordingly. Returns
>NULL if there are no devices left.
>
>v1: new patch
>
>CC: "David S. Miller" <davem@davemloft.net>
>CC: Eric Dumazet <edumazet@google.com>
>CC: Jiri Pirko <jiri@resnulli.us>
>CC: Alexander Duyck <alexander.h.duyck@intel.com>
>CC: Cong Wang <amwang@redhat.com>
>Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
>---
> net/core/dev.c |   25 +++++++++++++++++++++++++
> 1 files changed, 25 insertions(+), 0 deletions(-)
>
>diff --git a/net/core/dev.c b/net/core/dev.c
>index 1ed2b66..566e99a 100644
>--- a/net/core/dev.c
>+++ b/net/core/dev.c
>@@ -4477,6 +4477,31 @@ struct net_device *netdev_master_upper_dev_get(struct net_device *dev)
> }
> EXPORT_SYMBOL(netdev_master_upper_dev_get);
> 
>+/* netdev_upper_get_next_dev - Get the next dev from upper list
>+ * @dev: device
>+ * @iter: list_head ** of the current position
>+ *
>+ * Gets the next device from the dev's upper list, starting from iter
>+ * position. The caller must hold RCU read lock.
>+ */
>+struct net_device *netdev_upper_get_next_dev(struct net_device *dev,
>+					     struct list_head **iter)

		This should be probably rather named
		"netdev_upper_get_next_dev_rcu" That way it is clear
		right away. Also if you introduce non-rcu variant in
		future, you won't introduce confusion :)


>+{
>+	struct netdev_upper *upper;
>+
>+	WARN_ON_ONCE(!rcu_read_lock_held());
>+
>+	upper = list_entry_rcu((*iter)->next, struct netdev_upper, list);
>+
>+	if (&upper->list == &dev->upper_dev_list)
>+		return NULL;
>+
>+	*iter = &upper->list;
>+
>+	return upper->dev;
>+}
>+EXPORT_SYMBOL(netdev_upper_get_next_dev);
>+
> /**
>  * netdev_master_upper_dev_get_rcu - Get master upper device
>  * @dev: device
>-- 
>1.7.1
>

^ permalink raw reply

* Re: [PATCH RESEND V3 net-next 2/3] net: huawei_cdc_ncm: Introduce the huawei_cdc_ncm driver
From: David Miller @ 2013-08-26 20:58 UTC (permalink / raw)
  To: joe; +Cc: mrkiko.rs, gregkh, oliver, linux-kernel, linux-usb, netdev
In-Reply-To: <1377549913.1877.12.camel@joe-AO722>

From: Joe Perches <joe@perches.com>
Date: Mon, 26 Aug 2013 13:45:13 -0700

> I think the premise of local variable
> declaration by line length is flawed.

I disagree.

> It can't work when local variables are
> dependent on initialization order as
> above.

Move the initialization below the declarations.

^ permalink raw reply

* Re: [PATCH RESEND V3 net-next 2/3] net: huawei_cdc_ncm: Introduce the huawei_cdc_ncm driver
From: Joe Perches @ 2013-08-26 21:05 UTC (permalink / raw)
  To: David Miller; +Cc: mrkiko.rs, gregkh, oliver, linux-kernel, linux-usb, netdev
In-Reply-To: <20130826.165831.112649899191508957.davem@davemloft.net>

On Mon, 2013-08-26 at 16:58 -0400, David Miller wrote:
> From: Joe Perches <joe@perches.com>
> Date: Mon, 26 Aug 2013 13:45:13 -0700
> 
> > I think the premise of local variable
> > declaration by line length is flawed.
> 
> I disagree.
> 
> > It can't work when local variables are
> > dependent on initialization order as
> > above.
> 
> Move the initialization below the declarations.

So all initializations should be on separate
lines from declarations?

ick.

It's possible, but it doubles the associated
line count and I still think it's a very
dubious premise/practice/style.

As a general rule, it's I suppose OK, but as
a hardened rule, it's like most all hard rules.
It's made to be broken.

^ permalink raw reply

* Re: [PATCH RESEND V3 net-next 2/3] net: huawei_cdc_ncm: Introduce the huawei_cdc_ncm driver
From: David Miller @ 2013-08-26 21:09 UTC (permalink / raw)
  To: joe; +Cc: mrkiko.rs, gregkh, oliver, linux-kernel, linux-usb, netdev
In-Reply-To: <1377551155.1877.18.camel@joe-AO722>

From: Joe Perches <joe@perches.com>
Date: Mon, 26 Aug 2013 14:05:55 -0700

> On Mon, 2013-08-26 at 16:58 -0400, David Miller wrote:
>> From: Joe Perches <joe@perches.com>
>> Date: Mon, 26 Aug 2013 13:45:13 -0700
>> 
>> > I think the premise of local variable
>> > declaration by line length is flawed.
>> 
>> I disagree.
>> 
>> > It can't work when local variables are
>> > dependent on initialization order as
>> > above.
>> 
>> Move the initialization below the declarations.
> 
> So all initializations should be on separate
> lines from declarations?

No, you can often make it work out when the initialization
occurs on the same time.

Like everything else in life, you evaluate it on a case by
case basis.

Don't try to over-simplify things and act as if they are black
and white when they aren't.

^ permalink raw reply

* Re: travelling...
From: David Miller @ 2013-08-26 21:10 UTC (permalink / raw)
  To: ordex; +Cc: netdev, linux-wireless, netfilter-devel
In-Reply-To: <20130823201648.GC2896@ritirata.org>

From: Antonio Quartulli <ordex@autistici.org>
Date: Fri, 23 Aug 2013 22:16:48 +0200

> On Fri, Aug 23, 2013 at 08:29:43PM +0200, Hannes Frederic Sowa wrote:
>> On Fri, Aug 23, 2013 at 11:13:08AM -0700, David Miller wrote:
>> > 
>> > I'm travelling over the weekend so I won't be as responsive as
>> > usual.
>> > 
>> > I just pushed 'net' to Linus and he pulled it in.  I plan to work
>> > on my next set of -stable submissions this coming Monday.
>> > 
>> > If a merge of 'net' into 'net-next' would be useful for someone,
>> > now would be a really good time to tell me.
>> 
>> If it would not take you too much time, it would be nice. I depend on
>> a change in net to make another small patch. :)
> 
> If possible I'd need that too in order to send a couple of changes to net-next.

Ok, that's two requests so I did merge net into net-next just now.

^ 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