Date:	Fri, 21 Oct 2011 11:27:01 -0700
From:	Jay Vosburgh <fubar@...ibm.com>
To:	Jiri Pirko <jpirko@...hat.com>
cc:	netdev@...r.kernel.org, davem@...emloft.net,
	eric.dumazet@...il.com, bhutchings@...arflare.com,
	shemminger@...tta.com, andy@...yhouse.net, tgraf@...radead.org,
	ebiederm@...ssion.com, mirqus@...il.com, kaber@...sh.net,
	greearb@...delatech.com, jesse@...ira.com, fbl@...hat.com,
	benjamin.poirier@...il.com, jzupka@...hat.com
Subject: Re: [patch net-next V2] net: introduce ethernet teaming device

Jiri Pirko <jpirko@...hat.com> wrote:

>This patch introduces new network device called team. It supposes to be
>very fast, simple, userspace-driven alternative to existing bonding
>driver.
>
>Userspace library called libteam with couple of demo apps is available
>here:
>https://github.com/jpirko/libteam
>Note it's still in its dipers atm.
>
>team<->libteam use generic netlink for communication. That and rtnl
>suppose to be the only way to configure team device, no sysfs etc.
>
>Python binding basis for libteam was recently introduced (some need
>still need to be done on it though). Daemon providing arpmon/miimon
>active-backup functionality will be introduced shortly.
>All what's necessary is already implemented in kernel team driver.
>
>Signed-off-by: Jiri Pirko <jpirko@...hat.com>
>
>v1->v2:
>	- modes are made as modules. Makes team more modular and
>	  extendable.
>	- several commenters' nitpicks found on v1 were fixed
>	- several other bugs were fixed.
>	- note I ignored Eric's comment about roundrobin port selector
>	  as Eric's way may be easily implemented as another mode (mode
>	  "random") in future.
>---

[...]

>+static int team_port_add(struct team *team, struct net_device *port_dev)
>+{
>+	struct net_device *dev = team->dev;
>+	struct team_port *port;
>+	char *portname = port_dev->name;
>+	char tmp_addr[ETH_ALEN];
>+	int err;
>+
>+	if (port_dev->flags & IFF_LOOPBACK ||
>+	    port_dev->type != ARPHRD_ETHER) {
>+		netdev_err(dev, "Device %s is of an unsupported type\n",
>+			   portname);
>+		return -EINVAL;
>+	}
>+
>+	if (team_port_exists(port_dev)) {
>+		netdev_err(dev, "Device %s is already a port "
>+				"of a team device\n", portname);
>+		return -EBUSY;
>+	}
>+
>+	if (port_dev->flags & IFF_UP) {
>+		netdev_err(dev, "Device %s is up. Set it down before adding it as a team port\n",
>+			   portname);
>+		return -EBUSY;
>+	}
>+
>+	port = kzalloc(sizeof(struct team_port), GFP_KERNEL);
>+	if (!port)
>+		return -ENOMEM;
>+
>+	port->dev = port_dev;
>+	port->team = team;
>+
>+	port->orig.mtu = port_dev->mtu;
>+	err = dev_set_mtu(port_dev, dev->mtu);
>+	if (err) {
>+		netdev_dbg(dev, "Error %d calling dev_set_mtu\n", err);
>+		goto err_set_mtu;
>+	}
>+
>+	memcpy(port->orig.dev_addr, port_dev->dev_addr, ETH_ALEN);
>+	random_ether_addr(tmp_addr);
>+	err = __set_port_mac(port_dev, tmp_addr);
>+	if (err) {
>+		netdev_dbg(dev, "Device %s mac addr set failed\n",
>+			   portname);
>+		goto err_set_mac_rand;
>+	}
>+
>+	err = dev_open(port_dev);
>+	if (err) {
>+		netdev_dbg(dev, "Device %s opening failed\n",
>+			   portname);
>+		goto err_dev_open;
>+	}
>+
>+	err = team_port_set_orig_mac(port);
>+	if (err) {
>+		netdev_dbg(dev, "Device %s mac addr set failed - Device does not support addr change when it's opened\n",
>+			   portname);
>+		goto err_set_mac_opened;
>+	}

	This will exclude a number of devices that bonding currently
provides at least partial support for.

	Most of those are older 10 or 10/100 Ethernet drivers (anything
that uses eth_mac_addr for its ndo_set_mac_address, I think; there look
to be about 140 or so of those), but it also includes Infiniband (which
is excluded explicitly elsewhere).

	Another small set of Ethernet devices (those that currently need
bonding's fail_over_mac option) do permit setting the MAC while open,
but will misbehave if multiple ports are set to the same MAC.  The usual
suspects here are ehea and qeth, which are partition-aware devices for
IBM's pseries and zseries hardware, but there may be others I'm not
familiar with.

	If these will be permanent limitations of the team driver, then
this should (eventually) be in the documentation.

	Also, from looking at the code, it's not obvious if nesting of
teams is supported or not.  I'm not seeing anything in the code that
would prohibit adding a team device as a port to another team.  If
nesting of teams is undesirable, it should probably be explicitly tested
for and disallowed.

[...]

>+static int __init team_module_init(void)
>+{
>+	int err;
>+
>+	register_netdevice_notifier(&team_notifier_block);
>+
>+	err = rtnl_link_register(&team_link_ops);
>+	if (err)
>+		goto err_rtln_reg;
>+
>+	err = team_nl_init();
>+	if (err)
>+		goto err_nl_init;
>+
>+	return 0;
>+
>+err_nl_init:
>+	rtnl_link_unregister(&team_link_ops);
>+
>+err_rtln_reg:
>+	unregister_netdevice_notifier(&team_notifier_block);

	Minor nit: I suspect you meant "err_rtnl_reg" here, and in the
goto above.

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@...ibm.com

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists