Netdev List
 help / color / mirror / Atom feed
* [patch net-next 11/15] vlan: remove usage of dev->master in __vlan_find_dev_deep()
From: Jiri Pirko @ 2012-12-29 10:38 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, bhutchings, faisal.latif, shemminger, fbl,
	roland, sean.hefty, hal.rosenstock, fubar, andy, divy,
	jitendra.kalsaria, sony.chacko, linux-driver, kaber, ursula.braun,
	blaschka, schwidefsky, heiko.carstens, ebiederm, joe, amwang,
	nhorman, john.r.fastabend, pablo
In-Reply-To: <1356777522-19652-1-git-send-email-jiri@resnulli.us>

Also, since all users call __vlan_find_dev_deep() with rcu_read_lock,
make no possibility to call this with rtnl mutex held only.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 net/8021q/vlan_core.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index 65e06ab..380440b 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -60,21 +60,25 @@ bool vlan_do_receive(struct sk_buff **skbp)
 	return true;
 }
 
-/* Must be invoked with rcu_read_lock or with RTNL. */
-struct net_device *__vlan_find_dev_deep(struct net_device *real_dev,
+/* Must be invoked with rcu_read_lock. */
+struct net_device *__vlan_find_dev_deep(struct net_device *dev,
 					u16 vlan_id)
 {
-	struct vlan_info *vlan_info = rcu_dereference_rtnl(real_dev->vlan_info);
+	struct vlan_info *vlan_info = rcu_dereference(dev->vlan_info);
 
 	if (vlan_info) {
 		return vlan_group_get_device(&vlan_info->grp, vlan_id);
 	} else {
 		/*
-		 * Bonding slaves do not have grp assigned to themselves.
-		 * Grp is assigned to bonding master instead.
+		 * Lower devices of master uppers (bonding, team) do not have
+		 * grp assigned to themselves. Grp is assigned to upper device
+		 * instead.
 		 */
-		if (netif_is_bond_slave(real_dev))
-			return __vlan_find_dev_deep(real_dev->master, vlan_id);
+		struct net_device *upper_dev;
+
+		upper_dev = netdev_master_upper_dev_get_rcu(dev);
+		if (upper_dev)
+			return __vlan_find_dev_deep(upper_dev, vlan_id);
 	}
 
 	return NULL;
-- 
1.8.0

^ permalink raw reply related

* [patch net-next 12/15] nes: remove usage of dev->master
From: Jiri Pirko @ 2012-12-29 10:38 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, bhutchings, faisal.latif, shemminger, fbl,
	roland, sean.hefty, hal.rosenstock, fubar, andy, divy,
	jitendra.kalsaria, sony.chacko, linux-driver, kaber, ursula.braun,
	blaschka, schwidefsky, heiko.carstens, ebiederm, joe, amwang,
	nhorman, john.r.fastabend, pablo
In-Reply-To: <1356777522-19652-1-git-send-email-jiri@resnulli.us>

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 drivers/infiniband/hw/nes/nes.c    | 8 +++++---
 drivers/infiniband/hw/nes/nes_cm.c | 2 +-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/hw/nes/nes.c b/drivers/infiniband/hw/nes/nes.c
index 748db2d..d129c36 100644
--- a/drivers/infiniband/hw/nes/nes.c
+++ b/drivers/infiniband/hw/nes/nes.c
@@ -135,6 +135,7 @@ static int nes_inetaddr_event(struct notifier_block *notifier,
 	struct net_device *event_netdev = ifa->ifa_dev->dev;
 	struct nes_device *nesdev;
 	struct net_device *netdev;
+	struct net_device *upper_dev;
 	struct nes_vnic *nesvnic;
 	unsigned int is_bonded;
 
@@ -145,8 +146,9 @@ static int nes_inetaddr_event(struct notifier_block *notifier,
 				nesdev, nesdev->netdev[0]->name);
 		netdev = nesdev->netdev[0];
 		nesvnic = netdev_priv(netdev);
+		upper_dev = netdev_master_upper_dev_get(netdev);
 		is_bonded = netif_is_bond_slave(netdev) &&
-			    (netdev->master == event_netdev);
+			    (upper_dev == event_netdev);
 		if ((netdev == event_netdev) || is_bonded) {
 			if (nesvnic->rdma_enabled == 0) {
 				nes_debug(NES_DBG_NETDEV, "Returning without processing event for %s since"
@@ -179,9 +181,9 @@ static int nes_inetaddr_event(struct notifier_block *notifier,
 					/* fall through */
 				case NETDEV_CHANGEADDR:
 					/* Add the address to the IP table */
-					if (netdev->master)
+					if (upper_dev)
 						nesvnic->local_ipaddr =
-							((struct in_device *)netdev->master->ip_ptr)->ifa_list->ifa_address;
+							((struct in_device *)upper_dev->ip_ptr)->ifa_list->ifa_address;
 					else
 						nesvnic->local_ipaddr = ifa->ifa_address;
 
diff --git a/drivers/infiniband/hw/nes/nes_cm.c b/drivers/infiniband/hw/nes/nes_cm.c
index 22ea67e..ca6fbae 100644
--- a/drivers/infiniband/hw/nes/nes_cm.c
+++ b/drivers/infiniband/hw/nes/nes_cm.c
@@ -1340,7 +1340,7 @@ static int nes_addr_resolve_neigh(struct nes_vnic *nesvnic, u32 dst_ip, int arpi
 	}
 
 	if (netif_is_bond_slave(nesvnic->netdev))
-		netdev = nesvnic->netdev->master;
+		netdev = netdev_unique_upper_dev_get(nesvnic->netdev);
 	else
 		netdev = nesvnic->netdev;
 
-- 
1.8.0

^ permalink raw reply related

* [patch net-next 13/15] bonding: remove usage of dev->master
From: Jiri Pirko @ 2012-12-29 10:38 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, bhutchings, faisal.latif, shemminger, fbl,
	roland, sean.hefty, hal.rosenstock, fubar, andy, divy,
	jitendra.kalsaria, sony.chacko, linux-driver, kaber, ursula.braun,
	blaschka, schwidefsky, heiko.carstens, ebiederm, joe, amwang,
	nhorman, john.r.fastabend, pablo
In-Reply-To: <1356777522-19652-1-git-send-email-jiri@resnulli.us>

Benefit from new upper dev list and free bonding from dev->master usage.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 drivers/net/bonding/bond_3ad.c  | 30 ++++++-------
 drivers/net/bonding/bond_alb.c  |  6 +--
 drivers/net/bonding/bond_main.c | 94 ++++++++++++++++++++++++-----------------
 drivers/net/bonding/bonding.h   | 14 +++---
 net/core/rtnetlink.c            |  1 +
 5 files changed, 81 insertions(+), 64 deletions(-)

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index a030e63..84fabd6 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -1127,7 +1127,7 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
 				// INFO_RECEIVED_LOOPBACK_FRAMES
 				pr_err("%s: An illegal loopback occurred on adapter (%s).\n"
 				       "Check the configuration to verify that all adapters are connected to 802.3ad compliant switch ports\n",
-				       port->slave->dev->master->name, port->slave->dev->name);
+				       port->slave->bond->dev->name, port->slave->dev->name);
 				return;
 			}
 			__update_selected(lacpdu, port);
@@ -1306,7 +1306,7 @@ static void ad_port_selection_logic(struct port *port)
 		}
 		if (!curr_port) { // meaning: the port was related to an aggregator but was not on the aggregator port list
 			pr_warning("%s: Warning: Port %d (on %s) was related to aggregator %d but was not on its port list\n",
-				   port->slave->dev->master->name,
+				   port->slave->bond->dev->name,
 				   port->actor_port_number,
 				   port->slave->dev->name,
 				   port->aggregator->aggregator_identifier);
@@ -1386,7 +1386,7 @@ static void ad_port_selection_logic(struct port *port)
 				 port->aggregator->aggregator_identifier);
 		} else {
 			pr_err("%s: Port %d (on %s) did not find a suitable aggregator\n",
-			       port->slave->dev->master->name,
+			       port->slave->bond->dev->name,
 			       port->actor_port_number, port->slave->dev->name);
 		}
 	}
@@ -1463,7 +1463,7 @@ static struct aggregator *ad_agg_selection_test(struct aggregator *best,
 
 	default:
 		pr_warning("%s: Impossible agg select mode %d\n",
-			   curr->slave->dev->master->name,
+			   curr->slave->bond->dev->name,
 			   __get_agg_selection_mode(curr->lag_ports));
 		break;
 	}
@@ -1571,7 +1571,7 @@ static void ad_agg_selection_logic(struct aggregator *agg)
 		// check if any partner replys
 		if (best->is_individual) {
 			pr_warning("%s: Warning: No 802.3ad response from the link partner for any adapters in the bond\n",
-				   best->slave ? best->slave->dev->master->name : "NULL");
+				   best->slave ? best->slave->bond->dev->name : "NULL");
 		}
 
 		best->is_active = 1;
@@ -1898,7 +1898,7 @@ int bond_3ad_bind_slave(struct slave *slave)
 
 	if (bond == NULL) {
 		pr_err("%s: The slave %s is not attached to its bond\n",
-		       slave->dev->master->name, slave->dev->name);
+		       slave->bond->dev->name, slave->dev->name);
 		return -1;
 	}
 
@@ -1973,7 +1973,7 @@ void bond_3ad_unbind_slave(struct slave *slave)
 	// if slave is null, the whole port is not initialized
 	if (!port->slave) {
 		pr_warning("Warning: %s: Trying to unbind an uninitialized port on %s\n",
-			   slave->dev->master->name, slave->dev->name);
+			   slave->bond->dev->name, slave->dev->name);
 		return;
 	}
 
@@ -2009,7 +2009,7 @@ void bond_3ad_unbind_slave(struct slave *slave)
 
 				if ((new_aggregator->lag_ports == port) && new_aggregator->is_active) {
 					pr_info("%s: Removing an active aggregator\n",
-						aggregator->slave->dev->master->name);
+						aggregator->slave->bond->dev->name);
 					// select new active aggregator
 					 select_new_active_agg = 1;
 				}
@@ -2040,7 +2040,7 @@ void bond_3ad_unbind_slave(struct slave *slave)
 					ad_agg_selection_logic(__get_first_agg(port));
 			} else {
 				pr_warning("%s: Warning: unbinding aggregator, and could not find a new aggregator for its ports\n",
-					   slave->dev->master->name);
+					   slave->bond->dev->name);
 			}
 		} else { // in case that the only port related to this aggregator is the one we want to remove
 			select_new_active_agg = aggregator->is_active;
@@ -2048,7 +2048,7 @@ void bond_3ad_unbind_slave(struct slave *slave)
 			ad_clear_agg(aggregator);
 			if (select_new_active_agg) {
 				pr_info("%s: Removing an active aggregator\n",
-					slave->dev->master->name);
+					slave->bond->dev->name);
 				// select new active aggregator
 				ad_agg_selection_logic(__get_first_agg(port));
 			}
@@ -2076,7 +2076,7 @@ void bond_3ad_unbind_slave(struct slave *slave)
 					ad_clear_agg(temp_aggregator);
 					if (select_new_active_agg) {
 						pr_info("%s: Removing an active aggregator\n",
-							slave->dev->master->name);
+							slave->bond->dev->name);
 						// select new active aggregator
 						ad_agg_selection_logic(__get_first_agg(port));
 					}
@@ -2184,7 +2184,7 @@ static int bond_3ad_rx_indication(struct lacpdu *lacpdu, struct slave *slave, u1
 
 		if (!port->slave) {
 			pr_warning("%s: Warning: port of slave %s is uninitialized\n",
-				   slave->dev->name, slave->dev->master->name);
+				   slave->dev->name, slave->bond->dev->name);
 			return ret;
 		}
 
@@ -2240,7 +2240,7 @@ void bond_3ad_adapter_speed_changed(struct slave *slave)
 	// if slave is null, the whole port is not initialized
 	if (!port->slave) {
 		pr_warning("Warning: %s: speed changed for uninitialized port on %s\n",
-			   slave->dev->master->name, slave->dev->name);
+			   slave->bond->dev->name, slave->dev->name);
 		return;
 	}
 
@@ -2268,7 +2268,7 @@ void bond_3ad_adapter_duplex_changed(struct slave *slave)
 	// if slave is null, the whole port is not initialized
 	if (!port->slave) {
 		pr_warning("%s: Warning: duplex changed for uninitialized port on %s\n",
-			   slave->dev->master->name, slave->dev->name);
+			   slave->bond->dev->name, slave->dev->name);
 		return;
 	}
 
@@ -2297,7 +2297,7 @@ void bond_3ad_handle_link_change(struct slave *slave, char link)
 	// if slave is null, the whole port is not initialized
 	if (!port->slave) {
 		pr_warning("Warning: %s: link status changed for uninitialized port on %s\n",
-			   slave->dev->master->name, slave->dev->name);
+			   slave->bond->dev->name, slave->dev->name);
 		return;
 	}
 
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 7c9d136..f5e0527 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -507,7 +507,7 @@ static void rlb_update_client(struct rlb_client_info *client_info)
 				 client_info->mac_dst);
 		if (!skb) {
 			pr_err("%s: Error: failed to create an ARP packet\n",
-			       client_info->slave->dev->master->name);
+			       client_info->slave->bond->dev->name);
 			continue;
 		}
 
@@ -517,7 +517,7 @@ static void rlb_update_client(struct rlb_client_info *client_info)
 			skb = vlan_put_tag(skb, client_info->vlan_id);
 			if (!skb) {
 				pr_err("%s: Error: failed to insert VLAN tag\n",
-				       client_info->slave->dev->master->name);
+				       client_info->slave->bond->dev->name);
 				continue;
 			}
 		}
@@ -1043,7 +1043,7 @@ static int alb_set_slave_mac_addr(struct slave *slave, u8 addr[])
 	if (dev_set_mac_address(dev, &s_addr)) {
 		pr_err("%s: Error: dev_set_mac_address of dev %s failed!\n"
 		       "ALB mode requires that the base driver support setting the hw address also when the network device's interface is open\n",
-		       dev->master->name, dev->name);
+		       slave->bond->dev->name, dev->name);
 		return -EOPNOTSUPP;
 	}
 	return 0;
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index b7d45f3..c495298 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -746,11 +746,9 @@ static void __bond_resend_igmp_join_requests(struct net_device *dev)
 {
 	struct in_device *in_dev;
 
-	rcu_read_lock();
 	in_dev = __in_dev_get_rcu(dev);
 	if (in_dev)
 		ip_mc_rejoin_groups(in_dev);
-	rcu_read_unlock();
 }
 
 /*
@@ -760,9 +758,10 @@ static void __bond_resend_igmp_join_requests(struct net_device *dev)
  */
 static void bond_resend_igmp_join_requests(struct bonding *bond)
 {
-	struct net_device *bond_dev, *vlan_dev, *master_dev;
+	struct net_device *bond_dev, *vlan_dev, *upper_dev;
 	struct vlan_entry *vlan;
 
+	rcu_read_lock();
 	read_lock(&bond->lock);
 
 	bond_dev = bond->dev;
@@ -774,18 +773,14 @@ static void bond_resend_igmp_join_requests(struct bonding *bond)
 	 * if bond is enslaved to a bridge,
 	 * then rejoin all groups on its master
 	 */
-	master_dev = bond_dev->master;
-	if (master_dev)
-		if ((master_dev->priv_flags & IFF_EBRIDGE)
-			&& (bond_dev->priv_flags & IFF_BRIDGE_PORT))
-			__bond_resend_igmp_join_requests(master_dev);
+	upper_dev = netdev_master_upper_dev_get_rcu(bond_dev);
+	if (upper_dev && upper_dev->priv_flags & IFF_EBRIDGE)
+		__bond_resend_igmp_join_requests(upper_dev);
 
 	/* rejoin all groups on vlan devices */
 	list_for_each_entry(vlan, &bond->vlan_list, vlan_list) {
-		rcu_read_lock();
 		vlan_dev = __vlan_find_dev_deep(bond_dev,
 						vlan->vlan_id);
-		rcu_read_unlock();
 		if (vlan_dev)
 			__bond_resend_igmp_join_requests(vlan_dev);
 	}
@@ -794,13 +789,16 @@ static void bond_resend_igmp_join_requests(struct bonding *bond)
 		queue_delayed_work(bond->wq, &bond->mcast_work, HZ/5);
 
 	read_unlock(&bond->lock);
+	rcu_read_unlock();
 }
 
 static void bond_resend_igmp_join_requests_delayed(struct work_struct *work)
 {
 	struct bonding *bond = container_of(work, struct bonding,
 					    mcast_work.work);
+	rcu_read_lock();
 	bond_resend_igmp_join_requests(bond);
+	rcu_read_unlock();
 }
 
 /*
@@ -1493,6 +1491,27 @@ static rx_handler_result_t bond_handle_frame(struct sk_buff **pskb)
 	return ret;
 }
 
+static int bond_master_upper_dev_link(struct net_device *bond_dev,
+				      struct net_device *slave_dev)
+{
+	int err;
+
+	err = netdev_master_upper_dev_link(slave_dev, bond_dev);
+	if (err)
+		return err;
+	slave_dev->flags |= IFF_SLAVE;
+	rtmsg_ifinfo(RTM_NEWLINK, slave_dev, IFF_SLAVE);
+	return 0;
+}
+
+static void bond_upper_dev_unlink(struct net_device *bond_dev,
+				  struct net_device *slave_dev)
+{
+	netdev_upper_dev_unlink(slave_dev, bond_dev);
+	slave_dev->flags &= ~IFF_SLAVE;
+	rtmsg_ifinfo(RTM_NEWLINK, slave_dev, IFF_SLAVE);
+}
+
 /* enslave device <slave> to bond device <master> */
 int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
 {
@@ -1655,9 +1674,9 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
 		}
 	}
 
-	res = netdev_set_bond_master(slave_dev, bond_dev);
+	res = bond_master_upper_dev_link(bond_dev, slave_dev);
 	if (res) {
-		pr_debug("Error %d calling netdev_set_bond_master\n", res);
+		pr_debug("Error %d calling bond_master_upper_dev_link\n", res);
 		goto err_restore_mac;
 	}
 
@@ -1891,7 +1910,7 @@ err_close:
 	dev_close(slave_dev);
 
 err_unset_master:
-	netdev_set_bond_master(slave_dev, NULL);
+	bond_upper_dev_unlink(bond_dev, slave_dev);
 
 err_restore_mac:
 	if (!bond->params.fail_over_mac) {
@@ -1936,7 +1955,7 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev)
 
 	/* slave is not a slave or master is not master of this slave */
 	if (!(slave_dev->flags & IFF_SLAVE) ||
-	    (slave_dev->master != bond_dev)) {
+	    !netdev_has_upper_dev(slave_dev, bond_dev)) {
 		pr_err("%s: Error: cannot release %s.\n",
 		       bond_dev->name, slave_dev->name);
 		return -EINVAL;
@@ -2080,7 +2099,7 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev)
 		netif_addr_unlock_bh(bond_dev);
 	}
 
-	netdev_set_bond_master(slave_dev, NULL);
+	bond_upper_dev_unlink(bond_dev, slave_dev);
 
 	slave_disable_netpoll(slave);
 
@@ -2195,7 +2214,7 @@ static int bond_release_all(struct net_device *bond_dev)
 			netif_addr_unlock_bh(bond_dev);
 		}
 
-		netdev_set_bond_master(slave_dev, NULL);
+		bond_upper_dev_unlink(bond_dev, slave_dev);
 
 		slave_disable_netpoll(slave);
 
@@ -2259,8 +2278,9 @@ static int bond_ioctl_change_active(struct net_device *bond_dev, struct net_devi
 	if (!USES_PRIMARY(bond->params.mode))
 		return -EINVAL;
 
-	/* Verify that master_dev is indeed the master of slave_dev */
-	if (!(slave_dev->flags & IFF_SLAVE) || (slave_dev->master != bond_dev))
+	/* Verify that bond_dev is indeed the master of slave_dev */
+	if (!(slave_dev->flags & IFF_SLAVE) ||
+	    !netdev_has_upper_dev(slave_dev, bond_dev))
 		return -EINVAL;
 
 	read_lock(&bond->lock);
@@ -3258,36 +3278,32 @@ static int bond_master_netdev_event(unsigned long event,
 static int bond_slave_netdev_event(unsigned long event,
 				   struct net_device *slave_dev)
 {
-	struct net_device *bond_dev = slave_dev->master;
-	struct bonding *bond = netdev_priv(bond_dev);
-	struct slave *slave = NULL;
+	struct slave *slave = bond_slave_get_rtnl(slave_dev);
+	struct bonding *bond = slave->bond;
+	struct net_device *bond_dev = slave->bond->dev;
+	u32 old_speed;
+	u8 old_duplex;
 
 	switch (event) {
 	case NETDEV_UNREGISTER:
-		if (bond_dev) {
-			if (bond->setup_by_slave)
-				bond_release_and_destroy(bond_dev, slave_dev);
-			else
-				bond_release(bond_dev, slave_dev);
-		}
+		if (bond->setup_by_slave)
+			bond_release_and_destroy(bond_dev, slave_dev);
+		else
+			bond_release(bond_dev, slave_dev);
 		break;
 	case NETDEV_UP:
 	case NETDEV_CHANGE:
-		slave = bond_get_slave_by_dev(bond, slave_dev);
-		if (slave) {
-			u32 old_speed = slave->speed;
-			u8  old_duplex = slave->duplex;
+		old_speed = slave->speed;
+		old_duplex = slave->duplex;
 
-			bond_update_speed_duplex(slave);
+		bond_update_speed_duplex(slave);
 
-			if (bond->params.mode == BOND_MODE_8023AD) {
-				if (old_speed != slave->speed)
-					bond_3ad_adapter_speed_changed(slave);
-				if (old_duplex != slave->duplex)
-					bond_3ad_adapter_duplex_changed(slave);
-			}
+		if (bond->params.mode == BOND_MODE_8023AD) {
+			if (old_speed != slave->speed)
+				bond_3ad_adapter_speed_changed(slave);
+			if (old_duplex != slave->duplex)
+				bond_3ad_adapter_duplex_changed(slave);
 		}
-
 		break;
 	case NETDEV_DOWN:
 		/*
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 21b68e5..0d282d2 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -258,6 +258,9 @@ static inline bool bond_vlan_used(struct bonding *bond)
 #define bond_slave_get_rcu(dev) \
 	((struct slave *) rcu_dereference(dev->rx_handler_data))
 
+#define bond_slave_get_rtnl(dev) \
+	((struct slave *) rtnl_dereference(dev->rx_handler_data))
+
 /**
  * Returns NULL if the net_device does not belong to any of the bond's slaves
  *
@@ -280,11 +283,9 @@ static inline struct slave *bond_get_slave_by_dev(struct bonding *bond,
 
 static inline struct bonding *bond_get_bond_by_slave(struct slave *slave)
 {
-	if (!slave || !slave->dev->master) {
+	if (!slave || !slave->bond)
 		return NULL;
-	}
-
-	return netdev_priv(slave->dev->master);
+	return slave->bond;
 }
 
 static inline bool bond_is_lb(const struct bonding *bond)
@@ -360,10 +361,9 @@ static inline void bond_netpoll_send_skb(const struct slave *slave,
 
 static inline void bond_set_slave_inactive_flags(struct slave *slave)
 {
-	struct bonding *bond = netdev_priv(slave->dev->master);
-	if (!bond_is_lb(bond))
+	if (!bond_is_lb(slave->bond))
 		bond_set_backup_slave(slave);
-	if (!bond->params.all_slaves_active)
+	if (!slave->bond->params.all_slaves_active)
 		slave->inactive = 1;
 }
 
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index ae612f4..095bbc7 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2003,6 +2003,7 @@ errout:
 	if (err < 0)
 		rtnl_set_sk_err(net, RTNLGRP_LINK, err);
 }
+EXPORT_SYMBOL(rtmsg_ifinfo);
 
 static int nlmsg_populate_fdb_fill(struct sk_buff *skb,
 				   struct net_device *dev,
-- 
1.8.0

^ permalink raw reply related

* [patch net-next 14/15] net: remove no longer used netdev_set_bond_master() and netdev_set_master()
From: Jiri Pirko @ 2012-12-29 10:38 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, bhutchings, faisal.latif, shemminger, fbl,
	roland, sean.hefty, hal.rosenstock, fubar, andy, divy,
	jitendra.kalsaria, sony.chacko, linux-driver, kaber, ursula.braun,
	blaschka, schwidefsky, heiko.carstens, ebiederm, joe, amwang,
	nhorman, john.r.fastabend, pablo
In-Reply-To: <1356777522-19652-1-git-send-email-jiri@resnulli.us>

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 include/linux/netdevice.h |  6 +----
 net/core/dev.c            | 63 -----------------------------------------------
 2 files changed, 1 insertion(+), 68 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 2e5a217..94b5843 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -856,8 +856,7 @@ struct netdev_fcoe_hbainfo {
  *	flow_id is a flow ID to be passed to rps_may_expire_flow() later.
  *	Return the filter ID on success, or a negative error code.
  *
- *	Slave management functions (for bridge, bonding, etc). User should
- *	call netdev_set_master() to set dev->master properly.
+ *	Slave management functions (for bridge, bonding, etc).
  * int (*ndo_add_slave)(struct net_device *dev, struct net_device *slave_dev);
  *	Called to make another netdev an underling.
  *
@@ -2647,9 +2646,6 @@ extern int netdev_master_upper_dev_link(struct net_device *dev,
 					struct net_device *upper_dev);
 extern void netdev_upper_dev_unlink(struct net_device *dev,
 				    struct net_device *upper_dev);
-extern int		netdev_set_master(struct net_device *dev, struct net_device *master);
-extern int netdev_set_bond_master(struct net_device *dev,
-				  struct net_device *master);
 extern int skb_checksum_help(struct sk_buff *skb);
 extern struct sk_buff *skb_gso_segment(struct sk_buff *skb,
 	netdev_features_t features);
diff --git a/net/core/dev.c b/net/core/dev.c
index cf63a85..cdf7a19 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4842,69 +4842,6 @@ void netdev_upper_dev_unlink(struct net_device *dev,
 }
 EXPORT_SYMBOL(netdev_upper_dev_unlink);
 
-/**
- *	netdev_set_master	-	set up master pointer
- *	@slave: slave device
- *	@master: new master device
- *
- *	Changes the master device of the slave. Pass %NULL to break the
- *	bonding. The caller must hold the RTNL semaphore. On a failure
- *	a negative errno code is returned. On success the reference counts
- *	are adjusted and the function returns zero.
- */
-int netdev_set_master(struct net_device *slave, struct net_device *master)
-{
-	struct net_device *old = slave->master;
-	int err;
-
-	ASSERT_RTNL();
-
-	if (master) {
-		if (old)
-			return -EBUSY;
-		err = netdev_master_upper_dev_link(slave, master);
-		if (err)
-			return err;
-	}
-
-	slave->master = master;
-
-	if (old)
-		netdev_upper_dev_unlink(slave, master);
-
-	return 0;
-}
-EXPORT_SYMBOL(netdev_set_master);
-
-/**
- *	netdev_set_bond_master	-	set up bonding master/slave pair
- *	@slave: slave device
- *	@master: new master device
- *
- *	Changes the master device of the slave. Pass %NULL to break the
- *	bonding. The caller must hold the RTNL semaphore. On a failure
- *	a negative errno code is returned. On success %RTM_NEWLINK is sent
- *	to the routing socket and the function returns zero.
- */
-int netdev_set_bond_master(struct net_device *slave, struct net_device *master)
-{
-	int err;
-
-	ASSERT_RTNL();
-
-	err = netdev_set_master(slave, master);
-	if (err)
-		return err;
-	if (master)
-		slave->flags |= IFF_SLAVE;
-	else
-		slave->flags &= ~IFF_SLAVE;
-
-	rtmsg_ifinfo(RTM_NEWLINK, slave, IFF_SLAVE);
-	return 0;
-}
-EXPORT_SYMBOL(netdev_set_bond_master);
-
 static void dev_change_rx_flags(struct net_device *dev, int flags)
 {
 	const struct net_device_ops *ops = dev->netdev_ops;
-- 
1.8.0

^ permalink raw reply related

* [patch net-next 15/15] net: kill dev->master
From: Jiri Pirko @ 2012-12-29 10:38 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, bhutchings, faisal.latif, shemminger, fbl,
	roland, sean.hefty, hal.rosenstock, fubar, andy, divy,
	jitendra.kalsaria, sony.chacko, linux-driver, kaber, ursula.braun,
	blaschka, schwidefsky, heiko.carstens, ebiederm, joe, amwang,
	nhorman, john.r.fastabend, pablo
In-Reply-To: <1356777522-19652-1-git-send-email-jiri@resnulli.us>

Nobody uses this now. Remove it.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 include/linux/netdevice.h | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 94b5843..07ad1c3 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1167,10 +1167,6 @@ struct net_device {
 						 * avoid dirtying this cache line.
 						 */
 
-	struct net_device	*master; /* Pointer to master device of a group,
-					  * which this device is member of.
-					  */
-
 	struct list_head	upper_dev_list; /* List of upper devices */
 
 	/* Interface address info used in eth_type_trans() */
-- 
1.8.0

^ permalink raw reply related

* Re: ppoll() stuck on POLLIN while TCP peer is sending
From: Eric Wong @ 2012-12-29 11:34 UTC (permalink / raw)
  To: netdev, linux-kernel
  Cc: Andreas Voellmy, viro, linux-fsdevel, Junchang(Jason) Wang
In-Reply-To: <20121228070652.GA6292@dcvr.yhbt.net>

Eric Wong <normalperson@yhbt.net> wrote:
> Eric Wong <normalperson@yhbt.net> wrote:
> > I'm finding ppoll() unexpectedly stuck when waiting for POLLIN on a
> > local TCP socket.  The isolated code below can reproduces the issue
> > after many minutes (<1 hour).  It might be easier to reproduce on
> > a busy system while disk I/O is happening.
> 
> Ugh, I can't seem to reproduce this anymore...  Will try something
> else tomorrow.

The good news is I'm not imagining this...

The bad news is the issue is real and took a long time to reproduce
again.  This issue happens even without preempt, and without
tcp_low_latency on 3.7.1

While running `toosleepy', I also needed to run heavy (not loopback)
network and disk activity (several USB, SATA, and eSATA drives
simultaneously) for many hours before hitting this.

Hopefully this report is helpful in solving the issue.  Looking in at
the various pieces in net and select/poll paths, there's several
references to race conditions in the comments so this is hopefully
familiar territory to someone here...

^ permalink raw reply

* tc filter ouput, no filter hits?
From: julius @ 2012-12-29 13:46 UTC (permalink / raw)
  To: netdev@vger.kernel.org

hi,

ive seen some examples where the output from:
tc -s filter show dev ifb0 parent 1: 
shows a filter hit count, like the output shown below:

filter protocol ip pref 1 u32
filter protocol ip pref 1 u32 fh 800: ht divisor 1
filter protocol ip pref 1 u32 fh 800::800 order 2048 key ht 800 bkt 0 flowid 
1:1  (rule hit 2 success 0)



however in my openwrt 12.09 and on my ubuntu machine i dont see the part in the paranthese, why is that?

ubuntu:
iproute (20120521-3ubuntu1)
3.2.0-23-generic #36-Ubuntu


greets

^ permalink raw reply

* Re: Slow speed of tcp connections in a network namespace
From: Eric Dumazet @ 2012-12-29 13:53 UTC (permalink / raw)
  To: Andrew Vagin; +Cc: netdev, vvs, Michał Mirosław
In-Reply-To: <20121229092417.GA4038@paralelels.com>

On Sat, 2012-12-29 at 13:24 +0400, Andrew Vagin wrote:
> We found a few nodes, where network works slow in containers.
> 
> For testing speed of TCP connections we use wget, which downloads iso
> images from the internet.
> 
> wget in the new netns reports only 1.5 MB/s, but wget in the root netns
> reports 33MB/s.
> 
> A few facts:
>  * Experiments shows that window size for CT traffic does not increases
>    up to ~900, however for host traffic window size increases up to ~14000
>  * packets are shuffled in the netns sometimes.
>  * tso/gro/gso changes on interfaces does not help
>  * issue was _NOT_ reproduced if kernel booted with maxcpus=1 or bnx2.disable_msi=1
> 
> I reduced steps to reproduce:
> * Create a new network namespace "test" and a veth pair.
>   # ip netns add test
>   # ip link add name veth0 type veth peer name veth1
> 
> * Move veth1 into the netns test
>   # ip link set veth1 netns test
> 
> * Set ip address on veth1 and proper routing rules are added for this ip
>   in the root netns.
>   # ip link set up dev veth0;  ip link set up dev veth0
>   # ip netns exec test ip a add REMOTE dev veth1
>   # ip netns exec test ip r a default via veth1
>   # ip r a REMOTE/32 via dev veth0
> 
> Tcpdump for both cases are attached to this message.
> tcpdump.host - wget in the root netns
> tcpdump.netns.host - tcpdump for the host device, wget in the new netns
> tcpdump.netns.veth - tcpdump for the veth1 device, wget in the new netns
> 
> 3.8-rc1 is used for experiments.
> 
> Do you have any ideas where is a problem?

veth has absolutely no offload features

It needs some care...

At the very miminum, let TCP coalesce do its job by allowing SG

CC Michał Mirosław <mirq-linux@rere.qmqm.pl> for insights.

Please try following patch :

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 95814d9..9fefeb3 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -259,6 +259,10 @@ static const struct net_device_ops veth_netdev_ops = {
 	.ndo_set_mac_address = eth_mac_addr,
 };
 
+#define VETH_FEATURES (NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_TSO |	\
+		       NETIF_F_HW_CSUM | NETIF_F_HIGHDMA |		\
+		       NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX)
+
 static void veth_setup(struct net_device *dev)
 {
 	ether_setup(dev);
@@ -269,9 +273,10 @@ static void veth_setup(struct net_device *dev)
 	dev->netdev_ops = &veth_netdev_ops;
 	dev->ethtool_ops = &veth_ethtool_ops;
 	dev->features |= NETIF_F_LLTX;
+	dev->features |= VETH_FEATURES;
 	dev->destructor = veth_dev_free;
 
-	dev->hw_features = NETIF_F_HW_CSUM | NETIF_F_SG | NETIF_F_RXCSUM;
+	dev->hw_features = VETH_FEATURES;
 }
 
 /*

^ permalink raw reply related

* Re: tc filter ouput, no filter hits?
From: Eric Dumazet @ 2012-12-29 13:55 UTC (permalink / raw)
  To: julius; +Cc: netdev@vger.kernel.org
In-Reply-To: <1356788777.97050.YahooMailNeo@web165006.mail.bf1.yahoo.com>

On Sat, 2012-12-29 at 05:46 -0800, julius wrote:
> hi,
> 
> ive seen some examples where the output from:
> tc -s filter show dev ifb0 parent 1: 
> shows a filter hit count, like the output shown below:
> 
> filter protocol ip pref 1 u32
> filter protocol ip pref 1 u32 fh 800: ht divisor 1
> filter protocol ip pref 1 u32 fh 800::800 order 2048 key ht 800 bkt 0 flowid 
> 1:1  (rule hit 2 success 0)
> 
> 
> 
> however in my openwrt 12.09 and on my ubuntu machine i dont see the part in the paranthese, why is that?
> 
> ubuntu:
> iproute (20120521-3ubuntu1)
> 3.2.0-23-generic #36-Ubuntu
> 

Because its optional, kernel compilation needs :

CONFIG_CLS_U32_PERF=y

^ permalink raw reply

* Re: Slow speed of tcp connections in a network namespace
From: Andrew Vagin @ 2012-12-29 14:50 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, vvs, Michał Mirosław
In-Reply-To: <1356789203.21409.3923.camel@edumazet-glaptop>

On Sat, Dec 29, 2012 at 05:53:23AM -0800, Eric Dumazet wrote:
> > 3.8-rc1 is used for experiments.
> > 
> > Do you have any ideas where is a problem?
> 
> veth has absolutely no offload features
> 
> It needs some care...
> 
> At the very miminum, let TCP coalesce do its job by allowing SG
> 
> CC Michał Mirosław <mirq-linux@rere.qmqm.pl> for insights.
> 
> Please try following patch :

Hello Eric,

Thanks for your feedback.

With this patch the results is a bit better (~4MB/s), but it's much less
than in the root netns.

> 
> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> index 95814d9..9fefeb3 100644
> --- a/drivers/net/veth.c
> +++ b/drivers/net/veth.c
> @@ -259,6 +259,10 @@ static const struct net_device_ops veth_netdev_ops = {
>  	.ndo_set_mac_address = eth_mac_addr,
>  };
>  
> +#define VETH_FEATURES (NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_TSO |	\
> +		       NETIF_F_HW_CSUM | NETIF_F_HIGHDMA |		\
> +		       NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX)
> +
>  static void veth_setup(struct net_device *dev)
>  {
>  	ether_setup(dev);
> @@ -269,9 +273,10 @@ static void veth_setup(struct net_device *dev)
>  	dev->netdev_ops = &veth_netdev_ops;
>  	dev->ethtool_ops = &veth_ethtool_ops;
>  	dev->features |= NETIF_F_LLTX;
> +	dev->features |= VETH_FEATURES;
>  	dev->destructor = veth_dev_free;
>  
> -	dev->hw_features = NETIF_F_HW_CSUM | NETIF_F_SG | NETIF_F_RXCSUM;
> +	dev->hw_features = VETH_FEATURES;
>  }
>  
>  /*
> 
> 

^ permalink raw reply

* Re: [Patch RFC] ndisc: Fix skb allocation size for link layer options.
From: YOSHIFUJI Hideaki @ 2012-12-29 15:29 UTC (permalink / raw)
  To: Stephan Gatzka; +Cc: netdev, YOSHIFUJI Hideaki
In-Reply-To: <1356702410-32293-1-git-send-email-stephan.gatzka@gmail.com>

Stephan Gatzka wrote:
> Signed-off-by: Stephan Gatzka <stephan.gatzka@gmail.com>
> ---
>  net/ipv6/ndisc.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
> index 6574175..b12b94c 100644
> --- a/net/ipv6/ndisc.c
> +++ b/net/ipv6/ndisc.c
> @@ -392,7 +392,7 @@ static struct sk_buff *ndisc_build_skb(struct net_device *dev,
>  
>  	len = sizeof(struct icmp6hdr) + (target ? sizeof(*target) : 0);
>  	if (llinfo)
> -		len += ndisc_opt_addr_space(dev);
> +		len += sizeof(struct nd_opt_hdr) + ndisc_opt_addr_space(dev);
>  
>  	skb = sock_alloc_send_skb(sk,
>  				  (MAX_HEADER + sizeof(struct ipv6hdr) +
> @@ -1424,7 +1424,8 @@ void ndisc_send_redirect(struct sk_buff *skb, const struct in6_addr *target)
>  			memcpy(ha_buf, neigh->ha, dev->addr_len);
>  			read_unlock_bh(&neigh->lock);
>  			ha = ha_buf;
> -			len += ndisc_opt_addr_space(dev);
> +			len += sizeof(struct nd_opt_hdr) +
> +				ndisc_opt_addr_space(dev);
>  		} else
>  			read_unlock_bh(&neigh->lock);
>  
> 

Disagree.  NDISC_OPT_SPACE() takes care size of nd option header.

--yoshfuji

^ permalink raw reply

* Re: [Patch RFC] ndisc: Fix skb allocation size for link layer options.
From: YOSHIFUJI Hideaki @ 2012-12-29 15:32 UTC (permalink / raw)
  To: Stephan Gatzka; +Cc: YOSHIFUJI Hideaki, netdev
In-Reply-To: <50DF0C4A.5020401@linux-ipv6.org>

YOSHIFUJI Hideaki wrote:
> Stephan Gatzka wrote:
>> Signed-off-by: Stephan Gatzka <stephan.gatzka@gmail.com>
>> ---
>>  net/ipv6/ndisc.c |    5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
>> index 6574175..b12b94c 100644
>> --- a/net/ipv6/ndisc.c
>> +++ b/net/ipv6/ndisc.c
>> @@ -392,7 +392,7 @@ static struct sk_buff *ndisc_build_skb(struct net_device *dev,
>>  
>>  	len = sizeof(struct icmp6hdr) + (target ? sizeof(*target) : 0);
>>  	if (llinfo)
>> -		len += ndisc_opt_addr_space(dev);
>> +		len += sizeof(struct nd_opt_hdr) + ndisc_opt_addr_space(dev);
>>  
>>  	skb = sock_alloc_send_skb(sk,
>>  				  (MAX_HEADER + sizeof(struct ipv6hdr) +
>> @@ -1424,7 +1424,8 @@ void ndisc_send_redirect(struct sk_buff *skb, const struct in6_addr *target)
>>  			memcpy(ha_buf, neigh->ha, dev->addr_len);
>>  			read_unlock_bh(&neigh->lock);
>>  			ha = ha_buf;
>> -			len += ndisc_opt_addr_space(dev);
>> +			len += sizeof(struct nd_opt_hdr) +
>> +				ndisc_opt_addr_space(dev);
>>  		} else
>>  			read_unlock_bh(&neigh->lock);
>>  
>>
> 
> Disagree.  NDISC_OPT_SPACE() takes care size of nd option header.

Please note:

static inline int ndisc_opt_addr_space(struct net_device *dev)
{
        return NDISC_OPT_SPACE(dev->addr_len + ndisc_addr_option_pad(dev->type));
}

--yoshfuji

^ permalink raw reply

* Re: Slow speed of tcp connections in a network namespace
From: Michał Mirosław @ 2012-12-29 16:01 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Andrew Vagin, netdev, vvs, Michał Mirosław
In-Reply-To: <1356789203.21409.3923.camel@edumazet-glaptop>

2012/12/29 Eric Dumazet <eric.dumazet@gmail.com>:
> On Sat, 2012-12-29 at 13:24 +0400, Andrew Vagin wrote:
>> We found a few nodes, where network works slow in containers.
>>
>> For testing speed of TCP connections we use wget, which downloads iso
>> images from the internet.
>>
>> wget in the new netns reports only 1.5 MB/s, but wget in the root netns
>> reports 33MB/s.
>>
>> A few facts:
>>  * Experiments shows that window size for CT traffic does not increases
>>    up to ~900, however for host traffic window size increases up to ~14000
>>  * packets are shuffled in the netns sometimes.
>>  * tso/gro/gso changes on interfaces does not help
>>  * issue was _NOT_ reproduced if kernel booted with maxcpus=1 or bnx2.disable_msi=1
>>
>> I reduced steps to reproduce:
>> * Create a new network namespace "test" and a veth pair.
>>   # ip netns add test
>>   # ip link add name veth0 type veth peer name veth1
>>
>> * Move veth1 into the netns test
>>   # ip link set veth1 netns test
>>
>> * Set ip address on veth1 and proper routing rules are added for this ip
>>   in the root netns.
>>   # ip link set up dev veth0;  ip link set up dev veth0
>>   # ip netns exec test ip a add REMOTE dev veth1
>>   # ip netns exec test ip r a default via veth1
>>   # ip r a REMOTE/32 via dev veth0
>>
>> Tcpdump for both cases are attached to this message.
>> tcpdump.host - wget in the root netns
>> tcpdump.netns.host - tcpdump for the host device, wget in the new netns
>> tcpdump.netns.veth - tcpdump for the veth1 device, wget in the new netns
>>
>> 3.8-rc1 is used for experiments.
>>
>> Do you have any ideas where is a problem?
>
> veth has absolutely no offload features
>
> It needs some care...
>
> At the very miminum, let TCP coalesce do its job by allowing SG
>
> CC Michał Mirosław <mirq-linux@rere.qmqm.pl> for insights.

veth is just like a tunnel device. In terms of offloads, it can do anything
we have software fallbacks for (in case packets get forwarded to real hardware).

> Please try following patch :
>
> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> index 95814d9..9fefeb3 100644
> --- a/drivers/net/veth.c
> +++ b/drivers/net/veth.c
> @@ -259,6 +259,10 @@ static const struct net_device_ops veth_netdev_ops = {
>         .ndo_set_mac_address = eth_mac_addr,
>  };
>
> +#define VETH_FEATURES (NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_TSO |   \
> +                      NETIF_F_HW_CSUM | NETIF_F_HIGHDMA |              \
> +                      NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX)
> +
>  static void veth_setup(struct net_device *dev)
>  {
>         ether_setup(dev);
> @@ -269,9 +273,10 @@ static void veth_setup(struct net_device *dev)
>         dev->netdev_ops = &veth_netdev_ops;
>         dev->ethtool_ops = &veth_ethtool_ops;
>         dev->features |= NETIF_F_LLTX;
> +       dev->features |= VETH_FEATURES;
>         dev->destructor = veth_dev_free;
>
> -       dev->hw_features = NETIF_F_HW_CSUM | NETIF_F_SG | NETIF_F_RXCSUM;
> +       dev->hw_features = VETH_FEATURES;
>  }

You missed NETIF_F_RXCSUM in VETH_FEATURES. We might support
NETIF_F_ALL_TSO, not just the IPv4 version.

Best Regards,
Michał Mirosław

^ permalink raw reply

* Re: [Patch RFC] ndisc: Fix skb allocation size for link layer options.
From: Stephan Gatzka @ 2012-12-29 16:37 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki; +Cc: netdev
In-Reply-To: <50DF0C4A.5020401@linux-ipv6.org>


> 
> Disagree.  NDISC_OPT_SPACE() takes care size of nd option header.
Agree. I misunderstood this, my fault.

Maybe it's better to write:

#define NDISC_OPT_SPACE(len) (((len)+sizeof(nd_opt_hdr)+7)&~7)

Regards,

Stephan

^ permalink raw reply

* Re: Slow speed of tcp connections in a network namespace
From: Eric Dumazet @ 2012-12-29 17:40 UTC (permalink / raw)
  To: Andrew Vagin; +Cc: netdev, vvs, Michał Mirosław
In-Reply-To: <20121229145030.GA7959@paralelels.com>

On Sat, 2012-12-29 at 18:50 +0400, Andrew Vagin wrote:
> On Sat, Dec 29, 2012 at 05:53:23AM -0800, Eric Dumazet wrote:
> > > 3.8-rc1 is used for experiments.
> > > 
> > > Do you have any ideas where is a problem?
> > 
> > veth has absolutely no offload features
> > 
> > It needs some care...
> > 
> > At the very miminum, let TCP coalesce do its job by allowing SG
> > 
> > CC Michał Mirosław <mirq-linux@rere.qmqm.pl> for insights.
> > 
> > Please try following patch :
> 
> Hello Eric,
> 
> Thanks for your feedback.
> 
> With this patch the results is a bit better (~4MB/s), but it's much less
> than in the root netns.

Please post your new tcpdump then ;)

also post "netstat -s" from root and test ns after your wgets

^ permalink raw reply

* Re: Slow speed of tcp connections in a network namespace
From: Andrew Vagin @ 2012-12-29 18:29 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, vvs, Michał Mirosław
In-Reply-To: <1356802828.21409.4623.camel@edumazet-glaptop>

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

On Sat, Dec 29, 2012 at 09:40:28AM -0800, Eric Dumazet wrote:
> On Sat, 2012-12-29 at 18:50 +0400, Andrew Vagin wrote:
> > On Sat, Dec 29, 2012 at 05:53:23AM -0800, Eric Dumazet wrote:
> > > > 3.8-rc1 is used for experiments.
> > > > 
> > > > Do you have any ideas where is a problem?
> > > 
> > > veth has absolutely no offload features
> > > 
> > > It needs some care...
> > > 
> > > At the very miminum, let TCP coalesce do its job by allowing SG
> > > 
> > > CC Michał Mirosław <mirq-linux@rere.qmqm.pl> for insights.
> > > 
> > > Please try following patch :
> > 
> > Hello Eric,
> > 
> > Thanks for your feedback.
> > 
> > With this patch the results is a bit better (~4MB/s), but it's much less
> > than in the root netns.
> 
> Please post your new tcpdump then ;)

I have rebooted the host and a speed in a netns is again about 1.7MB/s. I
don't know why it was 4MB/s in the previous time.

new tcpdump and netstat are attached

> 
> also post "netstat -s" from root and test ns after your wgets
> 
> 
> 

[-- Attachment #2: tcpdump.host.gz --]
[-- Type: application/x-gzip, Size: 165716 bytes --]

[-- Attachment #3: tcpdump.netns.host.gz --]
[-- Type: application/x-gzip, Size: 180703 bytes --]

[-- Attachment #4: tcpdump.netns.veth.gz --]
[-- Type: application/x-gzip, Size: 181615 bytes --]

[-- Attachment #5: netstat.host --]
[-- Type: text/plain, Size: 1821 bytes --]

Ip:
    277536 total packets received
    20 forwarded
    0 incoming packets discarded
    202326 incoming packets delivered
    108228 requests sent out
    30 dropped because of missing route
Icmp:
    10 ICMP messages received
    0 input ICMP message failed.
    ICMP input histogram:
        destination unreachable: 5
        echo requests: 5
    6 ICMP messages sent
    0 ICMP messages failed
    ICMP output histogram:
        destination unreachable: 1
        echo replies: 5
IcmpMsg:
        InType3: 5
        InType8: 5
        OutType0: 5
        OutType3: 1
Tcp:
    1491 active connections openings
    12 passive connection openings
    14 failed connection attempts
    72 connection resets received
    2 connections established
    201920 segments received
    107815 segments send out
    0 segments retransmited
    0 bad segments received.
    1338 resets sent
Udp:
    387 packets received
    0 packets to unknown port received.
    0 packet receive errors
    389 packets sent
UdpLite:
TcpExt:
    3 invalid SYN cookies received
    4 TCP sockets finished time wait in fast timer
    63 delayed acks sent
    9 delayed acks further delayed because of locked socket
    236 packets directly queued to recvmsg prequeue.
    38600456 packets directly received from backlog
    298101 packets directly received from prequeue
    163501 packets header predicted
    27103 packets header predicted and directly queued to user
    2578 acknowledgments not containing data received
    1018 predicted acknowledgments
    15 connections reset due to unexpected data
    72 connections reset due to early user close
    TCPRcvCoalesce: 123
    TCPOFOQueue: 1187
IpExt:
    InBcastPkts: 9
    OutBcastPkts: 1
    InOctets: 296395504
    OutOctets: 6965311
    InBcastOctets: 2789
    OutBcastOctets: 165

[-- Attachment #6: netstat.netns --]
[-- Type: text/plain, Size: 1463 bytes --]

Ip:
    25483 total packets received
    0 forwarded
    0 incoming packets discarded
    25483 incoming packets delivered
    14572 requests sent out
Icmp:
    4 ICMP messages received
    0 input ICMP message failed.
    ICMP input histogram:
        echo requests: 2
        echo replies: 2
    4 ICMP messages sent
    0 ICMP messages failed
    ICMP output histogram:
        echo request: 2
        echo replies: 2
IcmpMsg:
        InType0: 2
        InType8: 2
        OutType0: 2
        OutType8: 2
Tcp:
    1 active connections openings
    0 passive connection openings
    0 failed connection attempts
    0 connection resets received
    0 connections established
    25473 segments received
    14562 segments send out
    0 segments retransmited
    0 bad segments received.
    77 resets sent
Udp:
    6 packets received
    0 packets to unknown port received.
    0 packet receive errors
    6 packets sent
UdpLite:
TcpExt:
    38 delayed acks sent
    Quick ack mode was activated 2 times
    52 packets directly queued to recvmsg prequeue.
    4916752 packets directly received from backlog
    11584 packets directly received from prequeue
    12538 packets header predicted
    2649 packets header predicted and directly queued to user
    1 acknowledgments not containing data received
    2 DSACKs sent for old packets
    1 connections reset due to unexpected data
    TCPOFOQueue: 1580
IpExt:
    InOctets: 38201843
    OutOctets: 829966

^ permalink raw reply

* Re: Slow speed of tcp connections in a network namespace
From: Eric Dumazet @ 2012-12-29 18:58 UTC (permalink / raw)
  To: Andrew Vagin; +Cc: netdev, vvs, Michał Mirosław
In-Reply-To: <1356802828.21409.4623.camel@edumazet-glaptop>

Le samedi 29 décembre 2012 à 09:40 -0800, Eric Dumazet a écrit :

> 
> Please post your new tcpdump then ;)
> 
> also post "netstat -s" from root and test ns after your wgets

Also try following bnx2 patch.

It should help GRO / TCP coalesce

bnx2 should be the last driver not using skb head_frag


diff --git a/drivers/net/ethernet/broadcom/bnx2.c b/drivers/net/ethernet/broadcom/bnx2.c
index a1adfaf..08a2d40 100644
--- a/drivers/net/ethernet/broadcom/bnx2.c
+++ b/drivers/net/ethernet/broadcom/bnx2.c
@@ -2726,6 +2726,14 @@ bnx2_free_rx_page(struct bnx2 *bp, struct bnx2_rx_ring_info *rxr, u16 index)
 	rx_pg->page = NULL;
 }
 
+static void bnx2_frag_free(const struct bnx2 *bp, void *data)
+{
+	if (bp->rx_frag_size)
+		put_page(virt_to_head_page(data));
+	else
+		kfree(data);
+}
+
 static inline int
 bnx2_alloc_rx_data(struct bnx2 *bp, struct bnx2_rx_ring_info *rxr, u16 index, gfp_t gfp)
 {
@@ -2735,7 +2743,10 @@ bnx2_alloc_rx_data(struct bnx2 *bp, struct bnx2_rx_ring_info *rxr, u16 index, gf
 	struct bnx2_rx_bd *rxbd =
 		&rxr->rx_desc_ring[BNX2_RX_RING(index)][BNX2_RX_IDX(index)];
 
-	data = kmalloc(bp->rx_buf_size, gfp);
+	if (bp->rx_frag_size)
+		data = netdev_alloc_frag(bp->rx_frag_size);
+	else
+		data = kmalloc(bp->rx_buf_size, gfp);
 	if (!data)
 		return -ENOMEM;
 
@@ -2744,7 +2755,7 @@ bnx2_alloc_rx_data(struct bnx2 *bp, struct bnx2_rx_ring_info *rxr, u16 index, gf
 				 bp->rx_buf_use_size,
 				 PCI_DMA_FROMDEVICE);
 	if (dma_mapping_error(&bp->pdev->dev, mapping)) {
-		kfree(data);
+		bnx2_frag_free(bp, data);
 		return -EIO;
 	}
 
@@ -3014,9 +3025,9 @@ error:
 
 	dma_unmap_single(&bp->pdev->dev, dma_addr, bp->rx_buf_use_size,
 			 PCI_DMA_FROMDEVICE);
-	skb = build_skb(data, 0);
+	skb = build_skb(data, bp->rx_frag_size);
 	if (!skb) {
-		kfree(data);
+		bnx2_frag_free(bp, data);
 		goto error;
 	}
 	skb_reserve(skb, ((u8 *)get_l2_fhdr(data) - data) + BNX2_RX_OFFSET);
@@ -5358,6 +5369,10 @@ bnx2_set_rx_ring_size(struct bnx2 *bp, u32 size)
 	/* hw alignment + build_skb() overhead*/
 	bp->rx_buf_size = SKB_DATA_ALIGN(bp->rx_buf_use_size + BNX2_RX_ALIGN) +
 		NET_SKB_PAD + SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
+	if (bp->rx_buf_size <= PAGE_SIZE)
+		bp->rx_frag_size = bp->rx_buf_size;
+	else
+		bp->rx_frag_size = 0;
 	bp->rx_jumbo_thresh = rx_size - BNX2_RX_OFFSET;
 	bp->rx_ring_size = size;
 	bp->rx_max_ring = bnx2_find_max_ring(size, BNX2_MAX_RX_RINGS);
@@ -5436,7 +5451,7 @@ bnx2_free_rx_skbs(struct bnx2 *bp)
 
 			rx_buf->data = NULL;
 
-			kfree(data);
+			bnx2_frag_free(bp, data);
 		}
 		for (j = 0; j < bp->rx_max_pg_ring_idx; j++)
 			bnx2_free_rx_page(bp, rxr, j);
diff --git a/drivers/net/ethernet/broadcom/bnx2.h b/drivers/net/ethernet/broadcom/bnx2.h
index 172efbe..11f5dee 100644
--- a/drivers/net/ethernet/broadcom/bnx2.h
+++ b/drivers/net/ethernet/broadcom/bnx2.h
@@ -6804,6 +6804,7 @@ struct bnx2 {
 
 	u32			rx_buf_use_size;	/* useable size */
 	u32			rx_buf_size;		/* with alignment */
+	u32			rx_frag_size; /* 0 if kmalloced(), or rx_buf_size */
 	u32			rx_copy_thresh;
 	u32			rx_jumbo_thresh;
 	u32			rx_max_ring_idx;

^ permalink raw reply related

* Re: Slow speed of tcp connections in a network namespace
From: Eric Dumazet @ 2012-12-29 19:41 UTC (permalink / raw)
  To: Andrew Vagin; +Cc: netdev, vvs, Michał Mirosław
In-Reply-To: <1356807516.4102.4.camel@edumazet-laptop>

On Sat, 2012-12-29 at 19:58 +0100, Eric Dumazet wrote:
> Le samedi 29 décembre 2012 à 09:40 -0800, Eric Dumazet a écrit :
> 
> > 
> > Please post your new tcpdump then ;)
> > 
> > also post "netstat -s" from root and test ns after your wgets
> 
> Also try following bnx2 patch.
> 
> It should help GRO / TCP coalesce
> 
> bnx2 should be the last driver not using skb head_frag

And of course, you should make sure all your bnx2 interrupts are handled
by the same cpu.

Or else, packets might be reordered because the way dev_forward_skb()
works.

(CPU X gets a bunch of packets from eth0, forward them via netif_rx() in
the local CPU X queue, NAPI is ended on eth0)

CPU Y gets a bunch of packets from eth0, forward them via netif_rx() in
the local CPU Y queue.

CPU X and Y process their local queue in // -> packets are delivered Out
of order to TCP stack

Alternative is to setup RPS on your veth1 device, to force packets being
delivered/handled by a given cpu

^ permalink raw reply

* Re: Is keepalive behaving as expected in 3.7.0+/net-next?
From: Jamie Gloudon @ 2012-12-29 19:47 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: rick.jones2, netdev
In-Reply-To: <1356645265.30414.1542.camel@edumazet-glaptop>

On Thu, Dec 27, 2012 at 01:54:25PM -0800, Eric Dumazet wrote:
> On Fri, 2012-12-21 at 14:05 -0800, Rick Jones wrote:
> > I was looking to do a bit more documentation clean-up and thought I 
> > would work on the descriptions of the "keepalive" sysctls, but first I 
> > wanted to see if they behaved as the existing descriptions suggested:
> > 
> > > tcp_keepalive_time - INTEGER
> > >         How often TCP sends out keepalive messages when keepalive is enabled.
> > >         Default: 2hours.
> > >
> > > tcp_keepalive_probes - INTEGER
> > >         How many keepalive probes TCP sends out, until it decides that the
> > >         connection is broken. Default value: 9.
> > >
> > > tcp_keepalive_intvl - INTEGER
> > >         How frequently the probes are send out. Multiplied by
> > >         tcp_keepalive_probes it is time to kill not responding connection,
> > >         after probes started. Default value: 75sec i.e. connection
> > >         will be aborted after ~11 minutes of retries.
> > 
> > I interpreted all that that as:  When a connection is idle, TCP will 
> > send a keepalive probe every tcp_keepalive_time seconds.  If a response 
> > to a keepalive probe is not received, TCP will resend (retransmit) it 
> > every tcp_keepalive_intvl seconds.
> > 
> > However, what I see is that on a connection where the remote is indeed 
> > still there, only the first keepalive probe is sent after 
> > tcp_keepalive_time, and thereafter it is sent every tcp_keepalive_intvl 
> > seconds.
> > 
> > Now, some of this may relate to my being impatient - rather than wait 
> > two hours for the first probe, I set tcp_keepalive_time to 3 seconds, 
> > and tcp_keepalive_intvl to 7 seconds.  I then kicked-off a ./configure 
> > --intervals-enable netperf TCP_RR test with a burst of one and a wait 
> > time of 90 seconds and got the following (trimmed) trace:
> > 
> > 13:43:46.879133 IP netnextraj.43054 > netnextraj2.srvr: Flags [S], seq 
> > 807869796, win 14600, options [mss 1460,sackOK,TS val 133470 ecr 
> > 0,nop,wscale 7], length 0
> > 13:43:46.880091 IP netnextraj2.srvr > netnextraj.43054: Flags [S.], seq 
> > 1522345902, ack 807869797, win 14480, options [mss 1460,sackOK,TS val 
> > 136186 ecr 133470,nop,wscale 4], length 0
> > 13:43:46.880114 IP netnextraj.43054 > netnextraj2.srvr: Flags [.], ack 
> > 1, win 115, options [nop,nop,TS val 133470 ecr 136186], length 0
> > 13:43:46.880306 IP netnextraj.43054 > netnextraj2.srvr: Flags [P.], seq 
> > 1:11, ack 1, win 115, options [nop,nop,TS val 133470 ecr 136186], length 10
> > 13:43:46.880948 IP netnextraj2.srvr > netnextraj.43054: Flags [.], ack 
> > 11, win 905, options [nop,nop,TS val 136187 ecr 133470], length 0
> > 13:43:46.880964 IP netnextraj2.srvr > netnextraj.43054: Flags [P.], seq 
> > 1:11, ack 11, win 905, options [nop,nop,TS val 136187 ecr 133470], length 10
> > 13:43:46.881161 IP netnextraj.43054 > netnextraj2.srvr: Flags [.], ack 
> > 11, win 115, options [nop,nop,TS val 133470 ecr 136187], length 0
> > 
> > The first probe above comes after 3 seconds - tcp_keepalive_time - at 
> > 13:43:49
> > 
> > 13:43:49.886752 IP netnextraj.43054 > netnextraj2.srvr: Flags [.], ack 
> > 11, win 115, options [nop,nop,TS val 134222 ecr 136187], length 0
> > 
> > And it does seem to elicit a response:
> > 
> > 13:43:49.887530 IP netnextraj2.srvr > netnextraj.43054: Flags [.], ack 
> > 11, win 905, options [nop,nop,TS val 136938 ecr 133470], length 0
> > 
> 
> 
> > Now it starts sending probes every 7 seconds (tcp_keepalive_intvl):
> > 
> > 13:43:56.903576 IP netnextraj.43054 > netnextraj2.srvr: Flags [.], ack 
> > 11, win 115, options [nop,nop,TS val 135976 ecr 136938], length 0
> > 13:43:56.904480 IP netnextraj2.srvr > netnextraj.43054: Flags [.], ack 
> > 11, win 905, options [nop,nop,TS val 138693 ecr 133470], length 0
> > 13:44:03.910744 IP netnextraj.43054 > netnextraj2.srvr: Flags [.], ack 
> > 11, win 115, options [nop,nop,TS val 137728 ecr 138693], length 0
> > 13:44:03.911623 IP netnextraj2.srvr > netnextraj.43054: Flags [.], ack 
> > 11, win 905, options [nop,nop,TS val 140444 ecr 133470], length 0
> > 
> > I;ve deleted the next 9 or so probes...  It continues, and doesn't 
> > terminate the connection, so I assume it was happy with the responses to 
> > the probes.
> > 
> > 13:45:13.990746 IP netnextraj.43054 > netnextraj2.srvr: Flags [.], ack 
> > 11, win 115, options [nop,nop,TS val 155248 ecr 156213], length 0
> > 13:45:13.991578 IP netnextraj2.srvr > netnextraj.43054: Flags [.], ack 
> > 11, win 905, options [nop,nop,TS val 157965 ecr 133470], length 0
> > 
> > Now the next netperf transaction happens:
> > 
> > 13:45:16.879222 IP netnextraj.43054 > netnextraj2.srvr: Flags [P.], seq 
> > 11:21, ack 11, win 115, options [nop,nop,TS val 155970 ecr 157965], 
> > length 10
> > 13:45:16.880033 IP netnextraj2.srvr > netnextraj.43054: Flags [P.], seq 
> > 11:21, ack 21, win 905, options [nop,nop,TS val 158687 ecr 155970], 
> > length 10
> > 13:45:16.880220 IP netnextraj.43054 > netnextraj2.srvr: Flags [.], ack 
> > 21, win 115, options [nop,nop,TS val 155970 ecr 158687], length 0
> > 
> > But the next keepalive probe is tcp_keepalive_intvl seconds after the 
> > last one, rather than that many, or tcp_keepalive_time seconds after the 
> > connection was last "active."
> > 
> > 13:45:20.998739 IP netnextraj.43054 > netnextraj2.srvr: Flags [.], ack 
> > 21, win 115, options [nop,nop,TS val 157000 ecr 158687], length 0
> > 13:45:20.999754 IP netnextraj2.srvr > netnextraj.43054: Flags [.], ack 
> > 21, win 905, options [nop,nop,TS val 159717 ecr 155970], length 0
> > 13:45:28.006747 IP netnextraj.43054 > netnextraj2.srvr: Flags [.], ack 
> > 21, win 115, options [nop,nop,TS val 158752 ecr 159717], length 0
> > 13:45:28.007624 IP netnextraj2.srvr > netnextraj.43054: Flags [.], ack 
> > 21, win 905, options [nop,nop,TS val 161469 ecr 155970], length 0
> > 
> > Is this the expected behaviour?  If I reverse the values - make 
> > tcp_keepalive_time 7 and tcp_keepalive_intvl 3, it seems that all the 
> > probes are after 7 seconds.
> > 
> > rick jones
> 
> Not sure if it makes sense to have 
> tcp_keepalive_intvl > tcp_keepalive_time
> 
> time should be an order of magnitude bigger than intvl.
> 
> keepalive timer is not reset each time we receive a valid frame, it
> would be very expensive.
> 
> Its a long period timer.
> 
> First interval is tcp_keepalive_time, and subsequent interval are
> tcp_keepalive_intvl
> 
> Each time timer is fired (once every 7200 seconds), we re-arm it with
> the observed elapsed time (keepalive_time_elapsed)
> 
> Fixing this would require to add a timestamp in inet socket, to remember
> time of next/last probe, and firing the timer using
> min(keepalive_time_when(tp), keepalive_intvl_when(tp))
> 
> Probably not worth it.
> 
>

Make a lot of sense. However, I got the impression from Rick that having tcp_keepalive_intvl > tcp_keepalive_time behaved correctly in older versions of the kernel. 

Regards,
Jamie Gloudon

^ permalink raw reply

* Re: Slow speed of tcp connections in a network namespace
From: Andrew Vagin @ 2012-12-29 20:08 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, vvs, Michał Mirosław
In-Reply-To: <1356810062.21409.4991.camel@edumazet-glaptop>

On Sat, Dec 29, 2012 at 11:41:02AM -0800, Eric Dumazet wrote:
> On Sat, 2012-12-29 at 19:58 +0100, Eric Dumazet wrote:
> > Le samedi 29 décembre 2012 à 09:40 -0800, Eric Dumazet a écrit :
> > 
> > > 
> > > Please post your new tcpdump then ;)
> > > 
> > > also post "netstat -s" from root and test ns after your wgets
> > 
> > Also try following bnx2 patch.
> > 
> > It should help GRO / TCP coalesce
> > 
> > bnx2 should be the last driver not using skb head_frag

I don't have access to the host. I'm going to test your patch tomorrow.
Thanks.

> 
> And of course, you should make sure all your bnx2 interrupts are handled
> by the same cpu.
All bnx interrupts are handled on all cpus. They are handled on the same
cpu, if a kernel is booted with msi_disable=1.

Is it right, that a received window will be less, if packets are not sorted?
Looks like a bug.

I want to say, that probably it works correctly, if packets are sorted.
But I think if packets are not sorted, it should work with the same
speed, cpu load and memory consumption may be a bit more.

> 
> Or else, packets might be reordered because the way dev_forward_skb()
> works.
> 
> (CPU X gets a bunch of packets from eth0, forward them via netif_rx() in
> the local CPU X queue, NAPI is ended on eth0)
> 
> CPU Y gets a bunch of packets from eth0, forward them via netif_rx() in
> the local CPU Y queue.
> 
> CPU X and Y process their local queue in // -> packets are delivered Out
> of order to TCP stack
> 
> Alternative is to setup RPS on your veth1 device, to force packets being
> delivered/handled by a given cpu
> 
> 
> 
> 
> 

^ permalink raw reply

* Re: Slow speed of tcp connections in a network namespace
From: Eric Dumazet @ 2012-12-29 20:20 UTC (permalink / raw)
  To: Andrew Vagin; +Cc: netdev, vvs, Michał Mirosław
In-Reply-To: <20121229200848.GA3389@paralelels.com>

On Sun, 2012-12-30 at 00:08 +0400, Andrew Vagin wrote:
> On Sat, Dec 29, 2012 at 11:41:02AM -0800, Eric Dumazet wrote:
> > On Sat, 2012-12-29 at 19:58 +0100, Eric Dumazet wrote:
> > > Le samedi 29 décembre 2012 à 09:40 -0800, Eric Dumazet a écrit :
> > > 
> > > > 
> > > > Please post your new tcpdump then ;)
> > > > 
> > > > also post "netstat -s" from root and test ns after your wgets
> > > 
> > > Also try following bnx2 patch.
> > > 
> > > It should help GRO / TCP coalesce
> > > 
> > > bnx2 should be the last driver not using skb head_frag
> 
> I don't have access to the host. I'm going to test your patch tomorrow.
> Thanks.
> 
> > 
> > And of course, you should make sure all your bnx2 interrupts are handled
> > by the same cpu.
> All bnx interrupts are handled on all cpus. They are handled on the same
> cpu, if a kernel is booted with msi_disable=1.
> 
> Is it right, that a received window will be less, if packets are not sorted?
> Looks like a bug.
> 
> I want to say, that probably it works correctly, if packets are sorted.
> But I think if packets are not sorted, it should work with the same
> speed, cpu load and memory consumption may be a bit more.

Without veth, it doesnt really matter that IRQ are spread on multiple
cpus, because packets are handled in NAPI, and only one cpu runs the
eth0 NAPI handler at one time.

But as soon as packets are queued (by netif_rx()) for 'later'
processing, you can have dramatic performance decrease.

Thats why you really should make sure IRQ on your eth0 device
are handled by a single cpu.

It will help to get better performance in most cases.

echo 1 >/proc/irq/*/eth0/../smp_affinity

If it doesnt work, you might try instead :

echo 1 >/proc/irq/default_smp_affinity
<you might need to reload bnx2 module, or ifdown/ifup eth0 >

^ permalink raw reply

* Re: Slow speed of tcp connections in a network namespace
From: Andrew Vagin @ 2012-12-29 21:07 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, vvs, Michał Mirosław
In-Reply-To: <1356812407.21409.5116.camel@edumazet-glaptop>

On Sat, Dec 29, 2012 at 12:20:07PM -0800, Eric Dumazet wrote:
> On Sun, 2012-12-30 at 00:08 +0400, Andrew Vagin wrote:
> > On Sat, Dec 29, 2012 at 11:41:02AM -0800, Eric Dumazet wrote:
> > > On Sat, 2012-12-29 at 19:58 +0100, Eric Dumazet wrote:
> > > > Le samedi 29 décembre 2012 à 09:40 -0800, Eric Dumazet a écrit :
> > > > 
> > > > > 
> > > > > Please post your new tcpdump then ;)
> > > > > 
> > > > > also post "netstat -s" from root and test ns after your wgets
> > > > 
> > > > Also try following bnx2 patch.
> > > > 
> > > > It should help GRO / TCP coalesce
> > > > 
> > > > bnx2 should be the last driver not using skb head_frag
> > 
> > I don't have access to the host. I'm going to test your patch tomorrow.
> > Thanks.
> > 
> > > 
> > > And of course, you should make sure all your bnx2 interrupts are handled
> > > by the same cpu.
> > All bnx interrupts are handled on all cpus. They are handled on the same
> > cpu, if a kernel is booted with msi_disable=1.
> > 
> > Is it right, that a received window will be less, if packets are not sorted?
> > Looks like a bug.
> > 
> > I want to say, that probably it works correctly, if packets are sorted.
> > But I think if packets are not sorted, it should work with the same
> > speed, cpu load and memory consumption may be a bit more.
> 
> Without veth, it doesnt really matter that IRQ are spread on multiple
> cpus, because packets are handled in NAPI, and only one cpu runs the
> eth0 NAPI handler at one time.
> 
> But as soon as packets are queued (by netif_rx()) for 'later'
> processing, you can have dramatic performance decrease.
> 
> Thats why you really should make sure IRQ on your eth0 device
> are handled by a single cpu.
> 
> It will help to get better performance in most cases.

I understand this fact, but so big difference looks strange for me.

Default configuration (with the bug):
# cat /proc/interrupts  | grep eth0
  68:      10187      10188      10187      10023      10190      10185
10187      10019   PCI-MSI-edge      eth0

> 
> echo 1 >/proc/irq/*/eth0/../smp_affinity

This doesn't help.

I tryed echo 0 > /proc/irq/68/smp_affinity_list. This doesn't help too.

> 
> If it doesnt work, you might try instead :
> 
> echo 1 >/proc/irq/default_smp_affinity
> <you might need to reload bnx2 module, or ifdown/ifup eth0 >

This helps, and the bug are not reproduced in this case.

# cat /proc/interrupts  | grep eth0
  68:      60777          0          0          0          0          0
0          0   PCI-MSI-edge      eth0

Thanks.

^ permalink raw reply

* Re: Slow speed of tcp connections in a network namespace
From: Eric Dumazet @ 2012-12-29 21:12 UTC (permalink / raw)
  To: Andrew Vagin; +Cc: netdev, vvs, Michał Mirosław
In-Reply-To: <20121229200848.GA3389@paralelels.com>

On Sun, 2012-12-30 at 00:08 +0400, Andrew Vagin wrote:

> Is it right, that a received window will be less, if packets are not sorted?
> Looks like a bug.

Not really a bug.

TCP is very sensitive to packet reorders. I wont elaborate here as
its a bit off topic.

Try to reorders credits/debits on your bank account, I am pretty sure
you'll lose some money or even get serious troubles.

Of course, enabling GRO on eth0 would definitely help a bit...

(once/iff veth driver features are fixed to allow GSO packets being
forwarded without being segmented again)

^ permalink raw reply

* Re: Slow speed of tcp connections in a network namespace
From: Andrew Vagin @ 2012-12-29 21:15 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, vvs, Michał Mirosław
In-Reply-To: <1356807516.4102.4.camel@edumazet-laptop>

On Sat, Dec 29, 2012 at 07:58:36PM +0100, Eric Dumazet wrote:
> Le samedi 29 décembre 2012 à 09:40 -0800, Eric Dumazet a écrit :
> 
> > 
> > Please post your new tcpdump then ;)
> > 
> > also post "netstat -s" from root and test ns after your wgets
> 
> Also try following bnx2 patch.
> 
> It should help GRO / TCP coalesce
> 
> bnx2 should be the last driver not using skb head_frag
> 

This patch breaks nothing. I don't know what kind of profit I should get
with it:).

FYI:
I forgot to say, that I disable gro before collecting tcpdump, because
in this case tcpdump from veth and from eth0 can be compared easier.

> 
> diff --git a/drivers/net/ethernet/broadcom/bnx2.c b/drivers/net/ethernet/broadcom/bnx2.c
> index a1adfaf..08a2d40 100644
> --- a/drivers/net/ethernet/broadcom/bnx2.c
> +++ b/drivers/net/ethernet/broadcom/bnx2.c
> @@ -2726,6 +2726,14 @@ bnx2_free_rx_page(struct bnx2 *bp, struct bnx2_rx_ring_info *rxr, u16 index)
>  	rx_pg->page = NULL;
>  }
>  
> +static void bnx2_frag_free(const struct bnx2 *bp, void *data)
> +{
> +	if (bp->rx_frag_size)
> +		put_page(virt_to_head_page(data));
> +	else
> +		kfree(data);
> +}
> +
>  static inline int
>  bnx2_alloc_rx_data(struct bnx2 *bp, struct bnx2_rx_ring_info *rxr, u16 index, gfp_t gfp)
>  {
> @@ -2735,7 +2743,10 @@ bnx2_alloc_rx_data(struct bnx2 *bp, struct bnx2_rx_ring_info *rxr, u16 index, gf
>  	struct bnx2_rx_bd *rxbd =
>  		&rxr->rx_desc_ring[BNX2_RX_RING(index)][BNX2_RX_IDX(index)];
>  
> -	data = kmalloc(bp->rx_buf_size, gfp);
> +	if (bp->rx_frag_size)
> +		data = netdev_alloc_frag(bp->rx_frag_size);
> +	else
> +		data = kmalloc(bp->rx_buf_size, gfp);
>  	if (!data)
>  		return -ENOMEM;
>  
> @@ -2744,7 +2755,7 @@ bnx2_alloc_rx_data(struct bnx2 *bp, struct bnx2_rx_ring_info *rxr, u16 index, gf
>  				 bp->rx_buf_use_size,
>  				 PCI_DMA_FROMDEVICE);
>  	if (dma_mapping_error(&bp->pdev->dev, mapping)) {
> -		kfree(data);
> +		bnx2_frag_free(bp, data);
>  		return -EIO;
>  	}
>  
> @@ -3014,9 +3025,9 @@ error:
>  
>  	dma_unmap_single(&bp->pdev->dev, dma_addr, bp->rx_buf_use_size,
>  			 PCI_DMA_FROMDEVICE);
> -	skb = build_skb(data, 0);
> +	skb = build_skb(data, bp->rx_frag_size);
>  	if (!skb) {
> -		kfree(data);
> +		bnx2_frag_free(bp, data);
>  		goto error;
>  	}
>  	skb_reserve(skb, ((u8 *)get_l2_fhdr(data) - data) + BNX2_RX_OFFSET);
> @@ -5358,6 +5369,10 @@ bnx2_set_rx_ring_size(struct bnx2 *bp, u32 size)
>  	/* hw alignment + build_skb() overhead*/
>  	bp->rx_buf_size = SKB_DATA_ALIGN(bp->rx_buf_use_size + BNX2_RX_ALIGN) +
>  		NET_SKB_PAD + SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
> +	if (bp->rx_buf_size <= PAGE_SIZE)
> +		bp->rx_frag_size = bp->rx_buf_size;
> +	else
> +		bp->rx_frag_size = 0;
>  	bp->rx_jumbo_thresh = rx_size - BNX2_RX_OFFSET;
>  	bp->rx_ring_size = size;
>  	bp->rx_max_ring = bnx2_find_max_ring(size, BNX2_MAX_RX_RINGS);
> @@ -5436,7 +5451,7 @@ bnx2_free_rx_skbs(struct bnx2 *bp)
>  
>  			rx_buf->data = NULL;
>  
> -			kfree(data);
> +			bnx2_frag_free(bp, data);
>  		}
>  		for (j = 0; j < bp->rx_max_pg_ring_idx; j++)
>  			bnx2_free_rx_page(bp, rxr, j);
> diff --git a/drivers/net/ethernet/broadcom/bnx2.h b/drivers/net/ethernet/broadcom/bnx2.h
> index 172efbe..11f5dee 100644
> --- a/drivers/net/ethernet/broadcom/bnx2.h
> +++ b/drivers/net/ethernet/broadcom/bnx2.h
> @@ -6804,6 +6804,7 @@ struct bnx2 {
>  
>  	u32			rx_buf_use_size;	/* useable size */
>  	u32			rx_buf_size;		/* with alignment */
> +	u32			rx_frag_size; /* 0 if kmalloced(), or rx_buf_size */
>  	u32			rx_copy_thresh;
>  	u32			rx_jumbo_thresh;
>  	u32			rx_max_ring_idx;
> 
> 

^ permalink raw reply

* Re: Slow speed of tcp connections in a network namespace
From: Andrew Vagin @ 2012-12-29 21:19 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, vvs, Michał Mirosław
In-Reply-To: <1356815546.21409.5277.camel@edumazet-glaptop>

On Sat, Dec 29, 2012 at 01:12:26PM -0800, Eric Dumazet wrote:
> On Sun, 2012-12-30 at 00:08 +0400, Andrew Vagin wrote:
> 
> > Is it right, that a received window will be less, if packets are not sorted?
> > Looks like a bug.
> 
> Not really a bug.
> 
> TCP is very sensitive to packet reorders. I wont elaborate here as
> its a bit off topic.
> 
> Try to reorders credits/debits on your bank account, I am pretty sure
> you'll lose some money or even get serious troubles.
> 
> Of course, enabling GRO on eth0 would definitely help a bit...
> 
> (once/iff veth driver features are fixed to allow GSO packets being
> forwarded without being segmented again)
> 

Eric, thank you for the help.
I need time for thinking. I will ask you, if new questions will appear.

> 
> 

^ 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