netdev - Re: [REPOST patch net-next V6] net: introduce ethernet teaming device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1320882789.5825.10.camel@edumazet-laptop>
Date:	Thu, 10 Nov 2011 00:53:09 +0100
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Jiri Pirko <jpirko@...hat.com>
Cc:	netdev@...r.kernel.org, davem@...emloft.net,
	bhutchings@...arflare.com, shemminger@...tta.com, fubar@...ibm.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, ivecera@...hat.com
Subject: Re: [REPOST patch net-next V6] net: introduce ethernet teaming
 device

Le jeudi 10 novembre 2011 à 00:12 +0100, Eric Dumazet a écrit :
> Le mercredi 09 novembre 2011 à 23:13 +0100, Jiri Pirko a écrit :
> > This patch introduces new network device called team. It supposes to be
> > very fast, simple, userspace-driven alternative to existing bonding
> > driver.
> 
> ...
> 
> > +/*
> > + * note: already called with rcu_read_lock
> > + */
> > +static netdev_tx_t team_xmit(struct sk_buff *skb, struct net_device *dev)
> > +{
> > +	struct team *team = netdev_priv(dev);
> > +	bool tx_success = false;
> > +	unsigned int len = skb->len;
> > +
> > +	/*
> > +	 * Ensure transmit function is called only in case there is at least
> > +	 * one port present.
> > +	 */
> > +	if (likely(!list_empty(&team->port_list) && team->mode_ops.transmit))
> > +		tx_success = team->mode_ops.transmit(team, skb);

Not clear why its so complex here.

When you manipulate team->port_list and make it empty, why dont you set
team->mode_ops.transmit to a helper function, freeing the skb and
returning false.

Also, instead of setting .transmit to NULL, you also could set it to
same helper function.

This way you could just do in fast path :

	tx_succcess = team->mode_ops.transmit(team, skb);

Avoiding two tests

> > +	if (tx_success) {
> > +		struct team_pcpu_stats *pcpu_stats;
> > +
> > +		pcpu_stats = this_cpu_ptr(team->pcpu_stats);
> > +		u64_stats_update_begin(&pcpu_stats->syncp);
> > +		pcpu_stats->tx_packets++;
> > +		pcpu_stats->tx_bytes += len;
> > +		u64_stats_update_end(&pcpu_stats->syncp);
> > +	} else {
> > +		this_cpu_inc(team->pcpu_stats->tx_dropped);
> > +	}
> > +
> > +	return NETDEV_TX_OK;
> > +}
> > +
> 



--
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