Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 1/1] net: neighbour: Simplify ifdefs around neigh_app_ns()
From: Tim Gardner @ 2013-08-28 18:24 UTC (permalink / raw)
  To: netdev, linux-kernel
  Cc: Tim Gardner, David S. Miller, Alexey Kuznetsov, James Morris,
	Hideaki YOSHIFUJI, Patrick McHardy, Eric W. Biederman, Gao feng,
	Joe Perches

Drop a couple of ifdef/endif pairs by moving the ifdef
surrounding neigh_app_ns() to the interior of neigh_app_ns().

Cc: "David S. Miller" <davem@davemloft.net>
Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Cc: James Morris <jmorris@namei.org>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: Patrick McHardy <kaber@trash.net>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Gao feng <gaofeng@cn.fujitsu.com>
Cc: Joe Perches <joe@perches.com>
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---

This is an admittedly trivial change. I stumbled on it while trying to figure
out why Ubuntu doesn't have CONFIG_ARPD enabled.

 net/core/neighbour.c |    4 ++--
 net/ipv4/arp.c       |    2 --
 net/ipv6/ndisc.c     |    2 --
 3 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 60533db..049dd9e 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -2759,13 +2759,13 @@ errout:
 		rtnl_set_sk_err(net, RTNLGRP_NEIGH, err);
 }
 
-#ifdef CONFIG_ARPD
 void neigh_app_ns(struct neighbour *n)
 {
+#ifdef CONFIG_ARPD
 	__neigh_notify(n, RTM_GETNEIGH, NLM_F_REQUEST);
+#endif /* CONFIG_ARPD */
 }
 EXPORT_SYMBOL(neigh_app_ns);
-#endif /* CONFIG_ARPD */
 
 #ifdef CONFIG_SYSCTL
 static int zero;
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 4429b01..7808093 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -368,9 +368,7 @@ static void arp_solicit(struct neighbour *neigh, struct sk_buff *skb)
 	} else {
 		probes -= neigh->parms->app_probes;
 		if (probes < 0) {
-#ifdef CONFIG_ARPD
 			neigh_app_ns(neigh);
-#endif
 			return;
 		}
 	}
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index 04d31c2..d5693ad 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -663,9 +663,7 @@ static void ndisc_solicit(struct neighbour *neigh, struct sk_buff *skb)
 		}
 		ndisc_send_ns(dev, neigh, target, target, saddr);
 	} else if ((probes -= neigh->parms->app_probes) < 0) {
-#ifdef CONFIG_ARPD
 		neigh_app_ns(neigh);
-#endif
 	} else {
 		addrconf_addr_solict_mult(target, &mcaddr);
 		ndisc_send_ns(dev, NULL, target, &mcaddr, saddr);
-- 
1.7.9.5

^ permalink raw reply related

* Re: [PATCH net-next v3 07/13] bonding: make bond_arp_send_all use upper device list
From: Veaceslav Falico @ 2013-08-28 18:11 UTC (permalink / raw)
  To: Vlad Yasevich; +Cc: netdev, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <20130828172335.GM1992@redhat.com>

On Wed, Aug 28, 2013 at 07:23:35PM +0200, Veaceslav Falico wrote:
>On Wed, Aug 28, 2013 at 12:45:38PM -0400, Vlad Yasevich wrote:
>>On 08/28/2013 12:03 PM, Veaceslav Falico wrote:
>...snip...
>>>+		/* search for upper device, like vlan/bridge/team/etc */
>>>+		rcu_read_lock();
>>>+		netdev_for_each_upper_dev_rcu(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;
>>>+			}
>>
>>How does this work in the following config:
>>	eth0,eth1 <--- bond <--- vlans 1,2,3 <---- bridge (ip address)
>
>Great catch, it won't work. I'm not sure if the old code does work (cause
>bond->dev already has vlans on top of it, so it has the ->vlan_info, and
>thus __vlan_find_dev_deep() won't go recursive, but rather check the
>bond->dev for the ip address, though I might be wrong), but I'll try to
>take a look at the new code to make it work.
>
>First what comes to mind (untested, not compiled):

(sorry for the spam - I've sent the mail from android and because it still
can't send anything but html email it bounced from netdev list)

actually, we should just go through the upper list
twice - first looking at all upper vlan's master devices
via __vlan_find_dev_deep(), and if found - get the tag,
and then loop through all devices.

i'll test it and send the next version, if nothing else
comes up.

thanks for the catch and the review.

^ permalink raw reply

* Re: [PATCH net-next v10 11/11] ipv6: Add generic UDP Tunnel segmentation
From: Pravin Shelar @ 2013-08-28 17:55 UTC (permalink / raw)
  To: Cong Wang; +Cc: netdev, David S. Miller, Jesse Gross, Stephen Hemminger
In-Reply-To: <1377667379-2315-12-git-send-email-amwang@redhat.com>

On Tue, Aug 27, 2013 at 10:22 PM, Cong Wang <amwang@redhat.com> wrote:
> From: Cong Wang <amwang@redhat.com>
>
> Similar to commit 731362674580cb0c696cd1b1a03d8461a10cf90a
> (tunneling: Add generic Tunnel segmentation)
>
Can we factor out ipv4-udp-tunnel code so that there is no duplicating code?

^ permalink raw reply

* Re: [PATCH net-next] qdisc: allow setting default queuing discipline
From: Stephen Hemminger @ 2013-08-28 17:28 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <1377710453.8277.11.camel@edumazet-glaptop>

On Wed, 28 Aug 2013 10:20:53 -0700
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> On Wed, 2013-08-28 at 09:18 -0700, Stephen Hemminger wrote:
> 
> > Although your idea adds completeness, not sure if it is worth it.
> > The thing is if user is using HTB, they already are using tc
> > commands to set it up, therefore the additional script steps to choose some
> > other qdisc for the leaf nodes doesn't seem like a burden.
> > Therefore the sysctl doesn't help much and might even create more
> > confusion because there is now two places to update.
> 
> Well, I need such a facility because I cannot change HTB setup.
> 
> I can boot a new kernel, yet not really change user scripts, especially
> when tc setup is actually not a script, but a complex binary doing
> constant changes in a complex and dynamic HTB hierarchy.
> 
> 
> 

Hate to say it but maybe module parameter to htb would be an alternative
if you have to deal with that.

^ permalink raw reply

* Re: [PATCH net-next v3 07/13] bonding: make bond_arp_send_all use upper device list
From: Veaceslav Falico @ 2013-08-28 17:23 UTC (permalink / raw)
  To: Vlad Yasevich; +Cc: netdev, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <521E2932.6060307@redhat.com>

On Wed, Aug 28, 2013 at 12:45:38PM -0400, Vlad Yasevich wrote:
>On 08/28/2013 12:03 PM, Veaceslav Falico wrote:
...snip...
>>+		/* search for upper device, like vlan/bridge/team/etc */
>>+		rcu_read_lock();
>>+		netdev_for_each_upper_dev_rcu(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;
>>+			}
>
>How does this work in the following config:
>	eth0,eth1 <--- bond <--- vlans 1,2,3 <---- bridge (ip address)

Great catch, it won't work. I'm not sure if the old code does work (cause
bond->dev already has vlans on top of it, so it has the ->vlan_info, and
thus __vlan_find_dev_deep() won't go recursive, but rather check the
bond->dev for the ip address, though I might be wrong), but I'll try to
take a look at the new code to make it work.

First what comes to mind (untested, not compiled):

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 47e1052..36d6899 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2323,7 +2323,7 @@ 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)
  {
-	struct net_device *upper;
+	struct net_device *upper, *real_dev;
  	struct list_head *iter;
  	struct rtable *rt;
  	__be32 *targets = bond->params.arp_targets, addr;
@@ -2350,6 +2350,16 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
  		/* search for upper device, like vlan/bridge/team/etc */
  		rcu_read_lock();
  		netdev_for_each_upper_dev_rcu(bond->dev, upper, iter) {
+			if (is_vlan_dev(upper)) {
+				real_dev = __vlan_find_dev_deep(netdev_master_upper_dev_get_rcu(upper),
+								htons(ETH_P_8021Q),
+								vlan_dev_vlan_id(upper));
+				if (real_dev == rt->dst.dev){
+					vlan_id = vlan_dev_vlan_id(upper);
+					upper = real_dev;
+				}
+			}
+
  			if (upper == rt->dst.dev) {
  				/* if it's a vlan - get its VID */
  				if (is_vlan_dev(rt->dst.dev))

>
>-vlad

^ permalink raw reply related

* Re: [PATCH net-next] qdisc: allow setting default queuing discipline
From: Eric Dumazet @ 2013-08-28 17:20 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David Miller, netdev
In-Reply-To: <20130828091836.5c8285f4@nehalam.linuxnetplumber.net>

On Wed, 2013-08-28 at 09:18 -0700, Stephen Hemminger wrote:

> Although your idea adds completeness, not sure if it is worth it.
> The thing is if user is using HTB, they already are using tc
> commands to set it up, therefore the additional script steps to choose some
> other qdisc for the leaf nodes doesn't seem like a burden.
> Therefore the sysctl doesn't help much and might even create more
> confusion because there is now two places to update.

Well, I need such a facility because I cannot change HTB setup.

I can boot a new kernel, yet not really change user scripts, especially
when tc setup is actually not a script, but a complex binary doing
constant changes in a complex and dynamic HTB hierarchy.

^ permalink raw reply

* Re: [PATCH net-next v3 07/13] bonding: make bond_arp_send_all use upper device list
From: Vlad Yasevich @ 2013-08-28 16:45 UTC (permalink / raw)
  To: Veaceslav Falico; +Cc: netdev, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1377705842-8276-8-git-send-email-vfalico@redhat.com>

On 08/28/2013 12:03 PM, Veaceslav Falico wrote:
> 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()
> v2: use netdev_for_each_upper_dev_rcu()
> v3: 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 |   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..5f38188 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_rcu(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;
> +			}

How does this work in the following config:
	eth0,eth1 <--- bond <--- vlans 1,2,3 <---- bridge (ip address)

-vlad

>   		}
> +		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);
>   	}
>   }
>
>

^ permalink raw reply

* Re: [net-next  06/10] ixgbe: cleanup some log messages
From: Joe Perches @ 2013-08-28 16:36 UTC (permalink / raw)
  To: Skidmore, Donald C, Bruce Allan
  Cc: Kirsher, Jeffrey T, davem@davemloft.net, netdev@vger.kernel.org,
	gospo@redhat.com, sassmann@redhat.com
In-Reply-To: <F6FB0E698C9B3143BDF729DF222866466FD4530B@ORSMSX110.amr.corp.intel.com>

On Wed, 2013-08-28 at 16:19 +0000, Skidmore, Donald C wrote:
> > -----Original Message-----
> > From: Joe Perches [mailto:joe@perches.com]
[]
> > At some point it might be nice to remove the remaining bool direct
> > comparisons to true/false
> > 
> > $ git grep -E -n "(==|!=)\s*(true|false)" drivers/net/ethernet/intel
> > drivers/net/ethernet/intel/e1000e/netdev.c:4871:
> > (hw->mac.autoneg == true) &&
> > drivers/net/ethernet/intel/e1000e/phy.c:2651: *  Assumes semaphore is
> > already acquired.  When page_set==true, assumes
> > drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c:763:	if (hw-
> > >phy.reset_disable == false) {
[etc...]
> Thanks for pointing this out Joe. 

Just a bit of code style ocd... ;)

>  I'll [] talk with the e1000e owner about doing it with their driver.

I believe the e1000e driver isn't using a bool
comparison as autoneg is a u8, but it could still use
	hw->mac.autoneg
for consistency with the other tests in the same driver.

Maybe:
---
 drivers/net/ethernet/intel/e1000e/netdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 2fead50..be9c033 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -4869,7 +4869,7 @@ static void e1000_watchdog_task(struct work_struct *work)
 			 */
 			if ((hw->phy.type == e1000_phy_igp_3 ||
 			     hw->phy.type == e1000_phy_bm) &&
-			    (hw->mac.autoneg == true) &&
+			    hw->mac.autoneg &&
 			    (adapter->link_speed == SPEED_10 ||
 			     adapter->link_speed == SPEED_100) &&
 			    (adapter->link_duplex == HALF_DUPLEX)) {

^ permalink raw reply related

* Re: [PATCH net-next v1 5/9] bonding: convert bond_has_this_ip() to use upper devices
From: Veaceslav Falico @ 2013-08-28 16:32 UTC (permalink / raw)
  To: Vlad Yasevich; +Cc: Jiri Pirko, netdev, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <521E0FA0.5070506@redhat.com>

On Wed, Aug 28, 2013 at 10:56:32AM -0400, Vlad Yasevich wrote:
...snip...
>I've been playing with approach 2 for some work I am doing for
>macvtap-over-bond, but I like you idea for 3 better.
>
>It would make things simpler in the long run.

I've just sent the third version of the 3rd approach, which implements
lower/upper device lists for every device that are guaranteed, under RCU,
tu have all the upper/lower devices linked to the device. It also permits
to distinguish the 'neighbour' devices (i.e. linked on the first level,
i.e. direct-linked) from the 'distant' ones, which are not directly linked
but rather inherited.

It also implements the first function (and the only one that I've needed
to get rid of vlan lists in bonding) - netdev_for_each_upper_dev_rcu(),
which can be used as a loop:

struct net_device *upper;
struct list_head *iter;

rcu_read_lock();
netdev_for_each_upper_dev_rcu(dev, upper, iter) {
	... do whatever you want with the upper dev ...
}
rcu_read_unlock();


In case you'll need, you can easily implement the same way
netdev_for_each_lower_dev_rcu().

Hope that helps.

>
>-vlad

^ permalink raw reply

* Re: [PATCH net-next v3 13/13] bonding: pr_debug instead of pr_warn in bond_arp_send_all
From: Veaceslav Falico @ 2013-08-28 16:22 UTC (permalink / raw)
  To: Joe Perches; +Cc: Jason Baron, netdev, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1377706548.1928.16.camel@joe-AO722>

On Wed, Aug 28, 2013 at 09:15:48AM -0700, Joe Perches wrote:
...snip...
>On Wed, 2013-08-28 at 18:04 +0200, Veaceslav Falico wrote:
...snip...
>> v2: new patch
>> v3: no change
>
>Hi Veaceslav:

Hi Joe,

>This changelog bit maybe is better placed below the ---

Yep, got it, will modify the next version (in case there will be one -
hopefully it's not a reason to resend the whole patchset - though if anyone
insists I can easily do it).

Thanks for the input!

^ permalink raw reply

* RE: [net-next  06/10] ixgbe: cleanup some log messages
From: Skidmore, Donald C @ 2013-08-28 16:19 UTC (permalink / raw)
  To: Joe Perches, Kirsher, Jeffrey T
  Cc: davem@davemloft.net, netdev@vger.kernel.org, gospo@redhat.com,
	sassmann@redhat.com
In-Reply-To: <1377701368.1928.10.camel@joe-AO722>

> -----Original Message-----
> From: Joe Perches [mailto:joe@perches.com]
> Sent: Wednesday, August 28, 2013 7:49 AM
> To: Kirsher, Jeffrey T
> Cc: davem@davemloft.net; Skidmore, Donald C; netdev@vger.kernel.org;
> gospo@redhat.com; sassmann@redhat.com
> Subject: Re: [net-next 06/10] ixgbe: cleanup some log messages
> 
> On Wed, 2013-08-28 at 03:33 -0700, Jeff Kirsher wrote:
> > From: Don Skidmore <donald.c.skidmore@intel.com>
> 
> > Some minor log messages cleanup, changing the level one message is
> > logged, adding a bit of detail to another and put all the text on one line.
> 
> Not a comment about this patch but:
> 
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
> > b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
> []
> > @@ -1282,7 +1282,7 @@ s32 ixgbe_identify_qsfp_module_generic(struct
> ixgbe_hw *hw)
> >  				status = 0;
> >  			} else {
> >  				if (hw->allow_unsupported_sfp == true) {
> 
> At some point it might be nice to remove the remaining bool direct
> comparisons to true/false
> 
> $ git grep -E -n "(==|!=)\s*(true|false)" drivers/net/ethernet/intel
> drivers/net/ethernet/intel/e1000e/netdev.c:4871:
> (hw->mac.autoneg == true) &&
> drivers/net/ethernet/intel/e1000e/phy.c:2651: *  Assumes semaphore is
> already acquired.  When page_set==true, assumes
> drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c:763:	if (hw-
> >phy.reset_disable == false) {
> drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c:819:	if (hw-
> >mac.orig_link_settings_stored == false) {
> drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c:954:
> (hw->phy.smart_speed_active == false))
> drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c:1099:	if (hw-
> >phy.reset_disable == false && hw->phy.ops.reset != NULL)
> drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c:1167:	if (hw-
> >mac.orig_link_settings_stored == false) {
> drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c:2371:	if (hw-
> >phy.qsfp_shared_i2c_bus == true) {
> drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c:2397:	if (hw-
> >phy.qsfp_shared_i2c_bus == true) {
> drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c:2424:	if (hw-
> >phy.qsfp_shared_i2c_bus == true) {
> drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c:2450:	if (hw-
> >phy.qsfp_shared_i2c_bus == true) {
> drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c:1273:
> 	if (hw->allow_unsupported_sfp == true) {
> 

Thanks for pointing this out Joe. 

 I'll work on a patch to do that for ixgbe and talk with the e1000e owner about doing it with their driver.

^ permalink raw reply

* Re: [PATCH net-next] qdisc: allow setting default queuing discipline
From: Stephen Hemminger @ 2013-08-28 16:18 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <1377665870.8828.163.camel@edumazet-glaptop>

On Tue, 27 Aug 2013 21:57:50 -0700
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> On Tue, 2013-08-27 at 21:00 -0700, Eric Dumazet wrote:
> > On Tue, 2013-08-27 at 16:19 -0700, Stephen Hemminger wrote:
> > > By default, the pfifo_fast queue discipline has been used by default
> > > for all devices. But we have better choices now.
> > > 
> > > This patch allow setting the default queueing discipline with sysctl.
> > > This allows easy use of better queueing disciplines on all devices
> > > without having to use tc qdisc scripts. It is intended to allow
> > > an easy path for distributions to make fq_codel or sfq the default
> > > qdisc.
> > > 
> > > This patch also makes pfifo_fast more of a first class qdisc, since
> > > it is now possible to manually override the default and explicitly
> > > use pfifo_fast. The behavior for systems who do not use the sysctl
> > > is unchanged, they still get pfifo_fast
> > > 
> > > Also removes leftover random # in sysctl net core.
> > > 
> > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > > 
> > > ---
> > 
> > I find this quite incredible, because I was going to write such patch
> > tonight ;)
> > 
> > Now I can just relax, thanks so much Stephen !
> > 
> > Acked-by: Eric Dumazet <edumazet@google.com>
> > 
> 
> I was wondering if we also could redefine the pfifo default, used for
> example in HTB classes.
> 
> ie calls to qdisc_create_dflt( ... &pfifo_qdisc_ops )
> 
> /proc/sys/net/core/default_qdisc would 'define' the default pfifo_fast,
> 
> /proc/sys/net/core/default_pfifo_qdisc would 'define' the default pfifo,
> 
> Or something like that...

Although your idea adds completeness, not sure if it is worth it.
The thing is if user is using HTB, they already are using tc
commands to set it up, therefore the additional script steps to choose some
other qdisc for the leaf nodes doesn't seem like a burden.
Therefore the sysctl doesn't help much and might even create more
confusion because there is now two places to update.

^ permalink raw reply

* Re: [PATCH net-next v3 13/13] bonding: pr_debug instead of pr_warn in bond_arp_send_all
From: Joe Perches @ 2013-08-28 16:15 UTC (permalink / raw)
  To: Veaceslav Falico, Jason Baron; +Cc: netdev, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1377705842-8276-14-git-send-email-vfalico@redhat.com>

(adding Jason Baron to comment on dynamic_debug)

On Wed, 2013-08-28 at 18:04 +0200, Veaceslav Falico wrote:
> They're simply annoying and will spam dmesg constantly if we hit them, so
> convert to pr_debug so that we still can access them in case of debugging.
> 
> v2: new patch
> v3: no change

Hi Veaceslav:

This changelog bit maybe is better placed below the ---

Hey Jason:

This conversion loses the net_ratelimit().

This maybe is an example of where dynamic_debug
could be better with some global rate control.

> 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 |   14 ++++++--------
>  1 files changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index a52cff1..47e1052 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -2336,10 +2336,8 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
>  		rt = ip_route_output(dev_net(bond->dev), targets[i], 0,
>  				     RTO_ONLINK, 0);
>  		if (IS_ERR(rt)) {
> -			if (net_ratelimit()) {
> -				pr_warning("%s: no route to arp_ip_target %pI4\n",
> -					   bond->dev->name, &targets[i]);
> -			}
> +			pr_debug("%s: no route to arp_ip_target %pI4\n",
> +				 bond->dev->name, &targets[i]);
>  			continue;
>  		}
>  
> @@ -2364,10 +2362,10 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
>  		rcu_read_unlock();
>  
>  		/* 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");
> +		pr_debug("%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;
>  

^ permalink raw reply

* Re: [PATCH net-next v10 09/11] vxlan: add ipv6 proxy support
From: Stephen Hemminger @ 2013-08-28 16:13 UTC (permalink / raw)
  To: Cong Wang; +Cc: netdev, David S. Miller, David Stevens
In-Reply-To: <1377667379-2315-10-git-send-email-amwang@redhat.com>

On Wed, 28 Aug 2013 13:22:57 +0800
Cong Wang <amwang@redhat.com> wrote:

> +#if IS_ENABLED(CONFIG_IPV6)
> +static int neigh_reduce(struct net_device *dev, struct sk_buff *skb)
> +{
> +	struct vxlan_dev *vxlan = netdev_priv(dev);
> +	struct neighbour *n;
> +	union vxlan_addr ipa;
> +	const struct ipv6hdr *iphdr;
> +	const struct in6_addr *saddr, *daddr;
> +	struct nd_msg *msg;
> +	struct inet6_dev *in6_dev = NULL;
> +
> +	WARN_ON(!rcu_read_lock_held() && !rcu_read_lock_bh_held());

You may have needed warn during debugging, but you should be able
to audit all the codepaths (this is a static function) to be sure
that this code is correct (ie rcu lock always held).

^ permalink raw reply

* Re: [PATCH net-next v10 03/11] ipv6: export a stub for IPv6 symbols used by vxlan
From: Stephen Hemminger @ 2013-08-28 16:12 UTC (permalink / raw)
  To: Cong Wang; +Cc: netdev, David S. Miller, Ben Hutchings
In-Reply-To: <1377667379-2315-4-git-send-email-amwang@redhat.com>

On Wed, 28 Aug 2013 13:22:51 +0800
Cong Wang <amwang@redhat.com> wrote:

> +struct ipv6_stub {
> +	int (*ipv6_sock_mc_join)(struct sock *sk, int ifindex,
> +				 const struct in6_addr *addr);
> +	int (*ipv6_sock_mc_drop)(struct sock *sk, int ifindex,
> +				 const struct in6_addr *addr);
> +	int (*ipv6_dst_lookup)(struct sock *sk, struct dst_entry **dst,
> +				struct flowi6 *fl6);
> +	void (*udpv6_encap_enable)(void);
> +};
> +extern const struct ipv6_stub *ipv6_stub __read_mostly;
> +

Since IPv6 really can't ever be safely unloaded. why not:
 * admit that, and get rid of ipv6_module exit.
 * then this stub can be a static const table.

You see virtual function tables that are writeable are potential malware
targets, although eliminating all of them is impossible, I hate to
see adding new ones.

^ permalink raw reply

* Re: [PATCH net-next 0/3] xen-netback: switch to NAPI + kthread 1:1 model
From: David Miller @ 2013-08-28 16:08 UTC (permalink / raw)
  To: wei.liu2; +Cc: netdev, xen-devel, ian.campbell, msw, konrad.wilk, annie.li
In-Reply-To: <20130828144149.GJ15729@zion.uk.xensource.com>

From: Wei Liu <wei.liu2@citrix.com>
Date: Wed, 28 Aug 2013 15:41:49 +0100

> Just that you applied some later posted patches and I thought that
> you didn't notice those acks.

I apply patches in the order that are convenient for me and match my
cognitive quota at the given point in time.

This means I'll review a lot of easy to review patches before those
that require more thought and consideration.

Therefore never read anything into the order in which I apply patches.

^ permalink raw reply

* Re: [PATCH net-next v10 07/11] vxlan: add ipv6 route short circuit support
From: Stephen Hemminger @ 2013-08-28 16:08 UTC (permalink / raw)
  To: Cong Wang; +Cc: netdev, David S. Miller, David Stevens
In-Reply-To: <1377667379-2315-8-git-send-email-amwang@redhat.com>

On Wed, 28 Aug 2013 13:22:55 +0800
Cong Wang <amwang@redhat.com> wrote:

> +		n = neigh_lookup(ipv6_stub->nd_tbl, &pip6->daddr, dev);
> +		if (!n && vxlan->flags & VXLAN_F_L3MISS) {
> +			union vxlan_addr ipa;
> +			ipa.sin6.sin6_addr = pip6->daddr;
> +			ipa.sa.sa_family = AF_INET6;

Please always add paren's when doing bitwise mask in comparison, gcc will even warn
about this.

You could use C99 initialization for this which has added benefit
of zeroing unused fields.
			union vxlan_addr ipa = {
				.sa.sa_family = AF_INET6,
				.sin6.sin6_sin6)addr = pip6->daddr,
			};

^ permalink raw reply

* [PATCH net-next v3 11/13] bonding: make alb_send_learning_packets() use upper dev list
From: Veaceslav Falico @ 2013-08-28 16:04 UTC (permalink / raw)
  To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1377705842-8276-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()
v2: use netdev_for_each_upper_dev_rcu()
v3: 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 |   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..b6a68b4 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_rcu(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 v3 07/13] bonding: make bond_arp_send_all use upper device list
From: Veaceslav Falico @ 2013-08-28 16:03 UTC (permalink / raw)
  To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1377705842-8276-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()
v2: use netdev_for_each_upper_dev_rcu()
v3: 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 |   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..5f38188 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_rcu(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 v3 0/13] bonding: remove vlan special handling
From: Veaceslav Falico @ 2013-08-28 16:03 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 *.
v2: Jiri noted that we need to see every upper device, but not only the
    first level. Modify the netdev_upper logic to include a list of lower
    devices and for both upper/lower lists every device would see both its
    first-level devices and every other devices that is lower/upper of it.
    Also, convert some annoying spamming warnings to pr_debug in
    bond_arp_send_all.
v3: move renaming part completely to patch 1 (did I forget to git add
    before commiting?) and address Jiri's input about comments/style of
    patch 2.

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.

The upper device functionality is extended to include also lower devices
and to have, for each device, a full view of every lower and upper device,
but not only the first-level ones. This might permit in the future to
avoid, for any grouping/teaming/upper/lower devices, to maintain its own
lists of slaves/vlans/etc.

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 or slaves.

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 |  254 ++++++-----------------------
 drivers/net/bonding/bonding.h   |   21 ++-
 include/linux/netdevice.h       |   11 ++
 net/core/dev.c                  |  345 ++++++++++++++++++++++++++++++++-------
 6 files changed, 396 insertions(+), 312 deletions(-)

^ permalink raw reply

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

On Wed, Aug 28, 2013 at 02:02:19PM +0200, Veaceslav Falico wrote:
>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 *.
>v2: Jiri noted that we need to see every upper device, but not only the
>    first level. Modify the netdev_upper logic to include a list of lower
>    devices and for both upper/lower lists every device would see both its
>    first-level devices and every other devices that is lower/upper of it.
>    Also, convert some annoying spamming warnings to pr_debug in
>    bond_arp_send_all.

Self-NAK on this patchset, will send the updated v3 in a few moments.

^ permalink raw reply

* [PATCH net-next v3 13/13] bonding: pr_debug instead of pr_warn in bond_arp_send_all
From: Veaceslav Falico @ 2013-08-28 16:04 UTC (permalink / raw)
  To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1377705842-8276-1-git-send-email-vfalico@redhat.com>

They're simply annoying and will spam dmesg constantly if we hit them, so
convert to pr_debug so that we still can access them in case of debugging.

v2: new patch
v3: 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 |   14 ++++++--------
 1 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index a52cff1..47e1052 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2336,10 +2336,8 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
 		rt = ip_route_output(dev_net(bond->dev), targets[i], 0,
 				     RTO_ONLINK, 0);
 		if (IS_ERR(rt)) {
-			if (net_ratelimit()) {
-				pr_warning("%s: no route to arp_ip_target %pI4\n",
-					   bond->dev->name, &targets[i]);
-			}
+			pr_debug("%s: no route to arp_ip_target %pI4\n",
+				 bond->dev->name, &targets[i]);
 			continue;
 		}
 
@@ -2364,10 +2362,10 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
 		rcu_read_unlock();
 
 		/* 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");
+		pr_debug("%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;
 
-- 
1.7.1

^ permalink raw reply related

* [PATCH net-next v3 12/13] bonding: remove vlan_list/current_alb_vlan
From: Veaceslav Falico @ 2013-08-28 16:04 UTC (permalink / raw)
  To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1377705842-8276-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
v2: no change
v3: 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 b6a68b4..0182352 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 d61f80a..a52cff1 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 230197d..4abc925 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 v3 10/13] bonding: split alb_send_learning_packets()
From: Veaceslav Falico @ 2013-08-28 16:03 UTC (permalink / raw)
  To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1377705842-8276-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
v2: no change
v3: 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 v3 09/13] bonding: use vlan_uses_dev() in __bond_release_one()
From: Veaceslav Falico @ 2013-08-28 16:03 UTC (permalink / raw)
  To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1377705842-8276-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
v2: no change
v3: 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 0fb3122..d61f80a 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


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