Netdev List
 help / color / mirror / Atom feed
* Re: [patch net-next RFC 03/12] net: introduce generic switch devices support
From: Scott Feldman @ 2014-08-22 23:12 UTC (permalink / raw)
  To: John Fastabend
  Cc: Sergey Ryazanov, jasowang-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
	John Fastabend, Neil Jerram, Eric Dumazet, Andy Gospodarek, dev,
	Felix Fietkau, Florian Fainelli, ronye-VPRAkNaXOzVWk0Htik3J/w,
	Jeff Kirsher, ogerlitz, Ben Hutchings, Lennert Buytenhek,
	Jiri Pirko, Roopa Prabhu, Jamal Hadi Salim, Aviad Raveh,
	Nicolas Dichtel, vyasevic, Neil Horman, netdev, Stephen Hemminger,
	dborkman <dbork
In-Reply-To: <53F7969C.1060509-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>


On Aug 22, 2014, at 12:14 PM, John Fastabend <john.fastabend-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> On 08/22/2014 05:56 AM, Jiri Pirko wrote:
>> Fri, Aug 22, 2014 at 02:42:04PM CEST, jhs-jkUAjuhPggJWk0Htik3J/w@public.gmane.org wrote:
>>> On 08/21/14 13:05, Florian Fainelli wrote:
>>>> 2014-08-21 9:18 GMT-07:00 Jiri Pirko <jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org>:
>>>>> The goal of this is to provide a possibility to suport various switch
>>>>> chips. Drivers should implement relevant ndos to do so. Now there is a
>>>>> couple of ndos defines:
>>>>> - for getting physical switch id is in place.
>>>>> - for work with flows.
>>>>> 
>>>>> Note that user can use random port netdevice to access the switch.
>>>> 
>>>> I read through this patch set, and I still think that DSA is the
>>>> generic switch infrastructure we already have because it does provide
>>>> the following:
>>>> 
>>>> - taking a generic platform data structure (C struct or Device Tree),
>>>> validate, parse it and map it to internal kernel structures
>>>> - instantiate per-port network devices based on the configuration data provided
>>>> - delegate netdev_ops to the switch driver and/or the CPU NIC when relevant
>>>> - provide support for hooking RX and TX traffic coming from the CPU NIC
>>>> 
>>>> I would rather we build on the existing DSA infrastructure and add the
>>>> flow-related netdev_ops rather than having the two remain in
>>>> disconnect while flow-oriented switches driver get progressively
>>>> added. I guess I should take a closer look at the rocker driver to see
>>>> how hard would that be for you.
>>>> 
>>>> What do you think?
>>> 
>>> 
>>> I thought we had concluded that DSA was a good path forward?  Or maybe at
>>> this stage we need to have several alternative approaches
>>> and we eventually converge?
>> 
>> That is true. I'm still unsure how to fit this on to DSA or how to change DSA
>> the way this fits. This is my quest now. Will report back in a week or so.
>> 
> 
> I would like to use the flow ops in some of our NICs that have
> a limited flow table in hardware. It might be easier to use the
> NICs as the first implementers of the API even though they are
> usually not as capable or large as flow tables in some of the
> larger switch asics.
> 
> In my opinion it can replace the ioctl flow director APIs although
> I don't like how it is tied to OVS in the some of the later RFC patches
> but we can work on that.

I think parallel efforts to get flow ops working on a real NIC with limited capabilities as well our fake enterprise-class rocker switch with full capabilities will really help solidify the swdev API.  Maybe even someone in the background that has access to a real enterprise switch can play along ;).  I hope you’re convinced from my other reply that the intent of swdev is to be independent of OVS.  If the implementation in the RFC doesn’t match the intent, we should fix it.  The elements of the swdev API we need convergence on are:

	- ndo_swdev_* ops to identify switch port and add/del flow match/action
	- struct sw_flow as generic flow match/action description
	- port netdevs to represent physical device ports

And maybe:

	- new device class to represent the switch itself
	- capabilities to express HW capabilities (limits, constraints, etc)


-scott

^ permalink raw reply

* [PATCH] net: prevent of emerging cross-namespace symlinks
From: Alexander Y. Fomichev @ 2014-08-22 23:15 UTC (permalink / raw)
  To: netdev; +Cc: Alexander Y. Fomichev, vfalico, David Miller
In-Reply-To: <20140822.113039.520798752693024139.davem@davemloft.net>

Code manipulating sysfs symlinks on adjacent net_devices(s)
currently doesn't take into account that devices potentially
belong to different namespaces.

This patch trying to fix an issue as follows:
- check for net_ns before creating / deleting symlink.
  for now only netdev_adjacent_rename_links and
  __netdev_adjacent_dev_remove are affected, afaics
  __netdev_adjacent_dev_insert implies both net_devs 
  belong to the same namespace.
- Drop all existing symlinks to / from all adj_devs before
  switching namespace and recreate them just after.

Signed-off-by: Alexander Y. Fomichev <git.user@gmail.com>
---
 net/core/dev.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 61 insertions(+), 1 deletion(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index b65a5051361f..683cedfdad8e 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4889,7 +4889,8 @@ static void __netdev_adjacent_dev_remove(struct net_device *dev,
 	if (adj->master)
 		sysfs_remove_link(&(dev->dev.kobj), "master");
 
-	if (netdev_adjacent_is_neigh_list(dev, dev_list))
+	if (netdev_adjacent_is_neigh_list(dev, dev_list) &&
+		net_eq(dev_net(dev),dev_net(adj_dev)))
 		netdev_adjacent_sysfs_del(dev, adj_dev->name, dev_list);
 
 	list_del_rcu(&adj->list);
@@ -5159,11 +5160,65 @@ void netdev_upper_dev_unlink(struct net_device *dev,
 }
 EXPORT_SYMBOL(netdev_upper_dev_unlink);
 
+void netdev_adjacent_add_links(struct net_device *dev)
+{
+	struct netdev_adjacent *iter;
+
+	struct net *net = dev_net(dev);
+
+	list_for_each_entry(iter, &dev->adj_list.upper, list) {
+		if (!net_eq(net,dev_net(iter->dev)))
+			continue;
+		netdev_adjacent_sysfs_add(iter->dev, dev,
+					  &iter->dev->adj_list.lower);
+		netdev_adjacent_sysfs_add(dev, iter->dev,
+					  &dev->adj_list.upper);
+	}
+
+	list_for_each_entry(iter, &dev->adj_list.lower, list) {
+		if (!net_eq(net,dev_net(iter->dev)))
+			continue;
+		netdev_adjacent_sysfs_add(iter->dev, dev,
+					  &iter->dev->adj_list.upper);
+		netdev_adjacent_sysfs_add(dev, iter->dev,
+					  &dev->adj_list.lower);
+	}
+}
+
+void netdev_adjacent_del_links(struct net_device *dev)
+{
+	struct netdev_adjacent *iter;
+
+	struct net *net = dev_net(dev);
+
+	list_for_each_entry(iter, &dev->adj_list.upper, list) {
+		if (!net_eq(net,dev_net(iter->dev)))
+			continue;
+		netdev_adjacent_sysfs_del(iter->dev, dev->name,
+					  &iter->dev->adj_list.lower);
+		netdev_adjacent_sysfs_del(dev, iter->dev->name,
+					  &dev->adj_list.upper);
+	}
+
+	list_for_each_entry(iter, &dev->adj_list.lower, list) {
+		if (!net_eq(net,dev_net(iter->dev)))
+			continue;
+		netdev_adjacent_sysfs_del(iter->dev, dev->name,
+					  &iter->dev->adj_list.upper);
+		netdev_adjacent_sysfs_del(dev, iter->dev->name,
+					  &dev->adj_list.lower);
+	}
+}
+
 void netdev_adjacent_rename_links(struct net_device *dev, char *oldname)
 {
 	struct netdev_adjacent *iter;
 
+	struct net *net = dev_net(dev);
+
 	list_for_each_entry(iter, &dev->adj_list.upper, list) {
+		if (!net_eq(net,dev_net(iter->dev)))
+			continue;
 		netdev_adjacent_sysfs_del(iter->dev, oldname,
 					  &iter->dev->adj_list.lower);
 		netdev_adjacent_sysfs_add(iter->dev, dev,
@@ -5171,6 +5226,8 @@ void netdev_adjacent_rename_links(struct net_device *dev, char *oldname)
 	}
 
 	list_for_each_entry(iter, &dev->adj_list.lower, list) {
+		if (!net_eq(net,dev_net(iter->dev)))
+			continue;
 		netdev_adjacent_sysfs_del(iter->dev, oldname,
 					  &iter->dev->adj_list.upper);
 		netdev_adjacent_sysfs_add(iter->dev, dev,
@@ -6771,8 +6828,10 @@ int dev_change_net_namespace(struct net_device *dev, struct net *net, const char
 	dev_uc_flush(dev);
 	dev_mc_flush(dev);
 
+
 	/* Send a netdev-removed uevent to the old namespace */
 	kobject_uevent(&dev->dev.kobj, KOBJ_REMOVE);
+	netdev_adjacent_del_links(dev);
 
 	/* Actually switch the network namespace */
 	dev_net_set(dev, net);
@@ -6787,6 +6846,7 @@ int dev_change_net_namespace(struct net_device *dev, struct net *net, const char
 
 	/* Send a netdev-add uevent to the new namespace */
 	kobject_uevent(&dev->dev.kobj, KOBJ_ADD);
+	netdev_adjacent_add_links(dev);
 
 	/* Fixup kobjects */
 	err = device_rename(&dev->dev, dev->name);
-- 
2.0.4

^ permalink raw reply related

* Re: [PATCH v2 3/3] tg3: Fix tx_pending checks for tg3_tso_bug
From: Prashant Sreedharan @ 2014-08-22 23:17 UTC (permalink / raw)
  To: Benjamin Poirier; +Cc: Michael Chan, netdev, linux-kernel
In-Reply-To: <1408658240-6811-3-git-send-email-bpoirier@suse.de>

Benjamin, thanks for the patch. Broadcom QA will be testing the changes.
Couple of comments below.
>  	segs = skb_gso_segment(skb, tp->dev->features &
>  				    ~(NETIF_F_TSO | NETIF_F_TSO6));
> -	if (IS_ERR(segs) || !segs)
> +	if (IS_ERR_OR_NULL(segs))
>  		goto tg3_tso_bug_end;
>  
>  	do {
> +		unsigned int desc_cnt = skb_shinfo(segs)->nr_frags + 1;
> +
>  		nskb = segs;
>  		segs = segs->next;
>  		nskb->next = NULL;
> -		tg3_start_xmit(nskb, tp->dev);
> +
> +		if (tg3_tx_avail(tnapi) <= segs_remaining - 1 + desc_cnt &&
> +		    skb_linearize(nskb)) {
> +			nskb->next = segs;
> +			segs = nskb;
> +			do {
> +				nskb = segs->next;
> +
> +				dev_kfree_skb_any(segs);
> +				segs = nskb;
> +			} while (segs);

If skb_linearize() fails need to increment the tp->tx_dropped count

> +			goto tg3_tso_bug_end;
> +		}
> +		segs_remaining--;
> +		if (segs_remaining)
> +			__tg3_start_xmit(nskb, tp->dev, segs_remaining);

To clarify passing segs_remaining will make sure the queue is never
stopped correct ?

> +		else
> +			tg3_start_xmit(nskb, tp->dev);
>  	} while (segs);
>  

^ permalink raw reply

* Re: [PATCH v2] carl9170: Remove redundant protection check
From: Eric Dumazet @ 2014-08-22 23:23 UTC (permalink / raw)
  To: Christian Lamparter
  Cc: Andreea-Cristina Bernat, linville, linux-wireless, netdev,
	linux-kernel, paulmck
In-Reply-To: <1931285.KsvFbLZH2Q@debian64>

On Fri, 2014-08-22 at 23:53 +0200, Christian Lamparter wrote:

> The sta_info->agg[tid] check is not needed (for reference, see [0]).
> (There is already a check in mac80211 which prevents the leak of
> sta_info->agg[tid] [1]).
> 
> Regards
> Christian
> 
> [0] <https://lkml.org/lkml/2014/8/20/725>
> [1] <http://lxr.free-electrons.com/source/net/mac80211/agg-tx.c#L583>
> 

Hmpfff... this code is quite confusing. RCU is used both in tricky way
(carl9170_ampdu_gc() is an example) and a talisman (the part you remove)

Why is rcu_assign_pointer(sta_info->agg[tid], tid_info);
done inside the spinlock protected region, I don't know.

If this code relies on external protection, a comment would help its
comprehension for sure.

For example, you could add a 
BUG_ON(rcu_access_pointer(sta_info->agg[tid]));
so that we are sure requirements are not changed in the callers one day.

^ permalink raw reply

* Re: [PATCH net-next v3 2/3] ethtool: Add generic options for tunables
From: Ben Hutchings @ 2014-08-22 23:26 UTC (permalink / raw)
  To: Govindarajulu Varadarajan; +Cc: davem, netdev, stephen, ssujith, benve
In-Reply-To: <1408008560-1067-3-git-send-email-_govind@gmx.com>

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

On Thu, 2014-08-14 at 14:59 +0530, Govindarajulu Varadarajan wrote:
> This patch adds new ethtool cmd, ETHTOOL_GTUNABLE & ETHTOOL_STUNABLE for getting
> tunable values from driver.
> 
> Add ethtool_tunable_ops array to ethtool_ops. The array index will be
> the tunable tcmd as defined in enum tunable_cmd. ethtool_tunable_ops has two
> function pointers set & get. This is set by the driver for getting/setting
> particular tunable.
> 
> Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
> ---
>  include/linux/ethtool.h      |  6 ++++++
>  include/uapi/linux/ethtool.h | 17 +++++++++++++++
>  net/core/ethtool.c           | 51 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 74 insertions(+)
> 
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index e658229..33bff94 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -77,6 +77,11 @@ static inline u32 ethtool_rxfh_indir_default(u32 index, u32 n_rx_rings)
>  	return index % n_rx_rings;
>  }
>  
> +struct ethtool_tunable_ops {
> +	int (*set)(struct net_device *, struct ethtool_tunable *);

The second pointer type should be const-qualified.

> +	int (*get)(struct net_device *, struct ethtool_tunable *);
> +};
> +
>  /**
>   * struct ethtool_ops - optional netdev operations
>   * @get_settings: Get various device settings including Ethernet link
> @@ -257,6 +262,7 @@ struct ethtool_ops {
>  				     struct ethtool_eeprom *, u8 *);
>  	int	(*get_eee)(struct net_device *, struct ethtool_eee *);
>  	int	(*set_eee)(struct net_device *, struct ethtool_eee *);
> +	struct ethtool_tunable_ops tunable_ops[ETHTOOL_TUNABLE_MAX];

This is OK but if we add a lot of tunables then it bloats up each driver
with a (probably quite sparse) array of function pointers.

>  
> 
>  };
> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> index e3c7a71..99e43ca 100644
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
> @@ -209,6 +209,21 @@ struct ethtool_value {
>  	__u32	data;
>  };
>  
> +enum tunable_cmd {
> +	ETHTOOL_RX_COPYBREAK = 0,
> +	ETHTOOL_TUNABLE_MAX,
> +};
> +
> +struct ethtool_tunable {
> +	u32 cmd;
> +	u32 tcmd;
> +	u32 len;
> +	union {
> +		u32 rx_copybreak;
> +		u32 data[48];
> +	} data;
[...]

This is not at all generic.  If tunables don't all have the same type,
then the type - not just the length - should be explicit.

I also think that if the length of a value can vary then we should not
declare a data member at all.  So we would have something like:

struct ethtool_tunable {
	__u32	cmd;
	__u32	id;
	__u32	type_id;
	__u32	len;
	__u8	data[0];
};

Then the value buffer would be passed to the driver functions separately
(as for other variable-length command structures).

By the way, you must use double-underscore prefixes on fixed-width
integer type names in UAPI headers.

Ben.

-- 
Ben Hutchings
Lowery's Law:
             If it jams, force it. If it breaks, it needed replacing anyway.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

^ permalink raw reply

* Re: [patch net-next RFC 03/12] net: introduce generic switch devices support
From: Florian Fainelli @ 2014-08-23  1:02 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Sergey Ryazanov, jasowang-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
	John Fastabend, Neil Jerram, Eric Dumazet, Andy Gospodarek, dev,
	Felix Fietkau, ronye-VPRAkNaXOzVWk0Htik3J/w, Jeff Kirsher,
	ogerlitz, Ben Hutchings, Lennert Buytenhek, Roopa Prabhu,
	Jamal Hadi Salim, Aviad Raveh, Nicolas Dichtel, vyasevic,
	Neil Horman, netdev, Stephen Hemminger, dborkman,
	Eric W. Biederman, David 
In-Reply-To: <20140822125655.GB1916-6KJVSR23iU488b5SBfVpbw@public.gmane.org>

2014-08-22 5:56 GMT-07:00 Jiri Pirko <jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org>:
> Fri, Aug 22, 2014 at 02:42:04PM CEST, jhs-jkUAjuhPggJWk0Htik3J/w@public.gmane.org wrote:
>>On 08/21/14 13:05, Florian Fainelli wrote:
>>>2014-08-21 9:18 GMT-07:00 Jiri Pirko <jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org>:
>>>>The goal of this is to provide a possibility to suport various switch
>>>>chips. Drivers should implement relevant ndos to do so. Now there is a
>>>>couple of ndos defines:
>>>>- for getting physical switch id is in place.
>>>>- for work with flows.
>>>>
>>>>Note that user can use random port netdevice to access the switch.
>>>
>>>I read through this patch set, and I still think that DSA is the
>>>generic switch infrastructure we already have because it does provide
>>>the following:
>>>
>>>- taking a generic platform data structure (C struct or Device Tree),
>>>validate, parse it and map it to internal kernel structures
>>>- instantiate per-port network devices based on the configuration data provided
>>>- delegate netdev_ops to the switch driver and/or the CPU NIC when relevant
>>>- provide support for hooking RX and TX traffic coming from the CPU NIC
>>>
>>>I would rather we build on the existing DSA infrastructure and add the
>>>flow-related netdev_ops rather than having the two remain in
>>>disconnect while flow-oriented switches driver get progressively
>>>added. I guess I should take a closer look at the rocker driver to see
>>>how hard would that be for you.
>>>
>>>What do you think?
>>
>>
>>I thought we had concluded that DSA was a good path forward?  Or maybe at
>>this stage we need to have several alternative approaches
>>and we eventually converge?
>
> That is true. I'm still unsure how to fit this on to DSA or how to change DSA
> the way this fits. This is my quest now. Will report back in a week or so.

I don't want to hold off this patch series, so let's proceed with your
submission, since I believe John Fastabend would also directly benefit
from this.

In the meantime, I will keep working on DSA, and prototype changes
with the rocker driver.

Once we are confident we have bridged the gap, we can unify things.
How does that sound?
-- 
Florian

^ permalink raw reply

* [PATCH net-next] net: use ktime_get_ns() and ktime_get_real_ns() helpers
From: Eric Dumazet @ 2014-08-23  1:32 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

From: Eric Dumazet <edumazet@google.com>

ktime_get_ns() replaces ktime_to_ns(ktime_get())

ktime_get_real_ns() replaces ktime_to_ns(ktime_get_real())

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/net/codel.h                     |    2 +-
 include/net/pkt_sched.h                 |    2 +-
 net/core/secure_seq.c                   |    6 +++---
 net/netfilter/nf_conntrack_core.c       |    2 +-
 net/netfilter/nf_conntrack_netlink.c    |    2 +-
 net/netfilter/nf_conntrack_standalone.c |    2 +-
 net/sched/act_police.c                  |    4 ++--
 net/sched/sch_fq.c                      |    4 ++--
 net/sched/sch_htb.c                     |    6 +++---
 net/sched/sch_tbf.c                     |    6 +++---
 10 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/include/net/codel.h b/include/net/codel.h
index fe0eab32ce76..aeee28081245 100644
--- a/include/net/codel.h
+++ b/include/net/codel.h
@@ -66,7 +66,7 @@ typedef s32 codel_tdiff_t;
 
 static inline codel_time_t codel_get_time(void)
 {
-	u64 ns = ktime_to_ns(ktime_get());
+	u64 ns = ktime_get_ns();
 
 	return ns >> CODEL_SHIFT;
 }
diff --git a/include/net/pkt_sched.h b/include/net/pkt_sched.h
index ec030cd76616..8bbe626e9ece 100644
--- a/include/net/pkt_sched.h
+++ b/include/net/pkt_sched.h
@@ -50,7 +50,7 @@ typedef long	psched_tdiff_t;
 
 static inline psched_time_t psched_get_time(void)
 {
-	return PSCHED_NS2TICKS(ktime_to_ns(ktime_get()));
+	return PSCHED_NS2TICKS(ktime_get_ns());
 }
 
 static inline psched_tdiff_t
diff --git a/net/core/secure_seq.c b/net/core/secure_seq.c
index ba71212f0251..51dd3193a33e 100644
--- a/net/core/secure_seq.c
+++ b/net/core/secure_seq.c
@@ -35,7 +35,7 @@ static u32 seq_scale(u32 seq)
 	 *	overlaps less than one time per MSL (2 minutes).
 	 *	Choosing a clock of 64 ns period is OK. (period of 274 s)
 	 */
-	return seq + (ktime_to_ns(ktime_get_real()) >> 6);
+	return seq + (ktime_get_real_ns() >> 6);
 }
 #endif
 
@@ -135,7 +135,7 @@ u64 secure_dccp_sequence_number(__be32 saddr, __be32 daddr,
 	md5_transform(hash, net_secret);
 
 	seq = hash[0] | (((u64)hash[1]) << 32);
-	seq += ktime_to_ns(ktime_get_real());
+	seq += ktime_get_real_ns();
 	seq &= (1ull << 48) - 1;
 
 	return seq;
@@ -163,7 +163,7 @@ u64 secure_dccpv6_sequence_number(__be32 *saddr, __be32 *daddr,
 	md5_transform(hash, secret);
 
 	seq = hash[0] | (((u64)hash[1]) << 32);
-	seq += ktime_to_ns(ktime_get_real());
+	seq += ktime_get_real_ns();
 	seq &= (1ull << 48) - 1;
 
 	return seq;
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index de88c4ab5146..0b634e7f1652 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -358,7 +358,7 @@ bool nf_ct_delete(struct nf_conn *ct, u32 portid, int report)
 
 	tstamp = nf_conn_tstamp_find(ct);
 	if (tstamp && tstamp->stop == 0)
-		tstamp->stop = ktime_to_ns(ktime_get_real());
+		tstamp->stop = ktime_get_real_ns();
 
 	if (nf_ct_is_dying(ct))
 		goto delete;
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 355a5c4ef763..1bd9ed9e62f6 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -1737,7 +1737,7 @@ ctnetlink_create_conntrack(struct net *net, u16 zone,
 	}
 	tstamp = nf_conn_tstamp_find(ct);
 	if (tstamp)
-		tstamp->start = ktime_to_ns(ktime_get_real());
+		tstamp->start = ktime_get_real_ns();
 
 	err = nf_conntrack_hash_check_insert(ct);
 	if (err < 0)
diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
index f641751dba9d..cf65a1e040dd 100644
--- a/net/netfilter/nf_conntrack_standalone.c
+++ b/net/netfilter/nf_conntrack_standalone.c
@@ -101,7 +101,7 @@ static void *ct_seq_start(struct seq_file *seq, loff_t *pos)
 {
 	struct ct_iter_state *st = seq->private;
 
-	st->time_now = ktime_to_ns(ktime_get_real());
+	st->time_now = ktime_get_real_ns();
 	rcu_read_lock();
 	return ct_get_idx(seq, *pos);
 }
diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index 0566e4606a4a..f32bcb094915 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -231,7 +231,7 @@ override:
 	if (ret != ACT_P_CREATED)
 		return ret;
 
-	police->tcfp_t_c = ktime_to_ns(ktime_get());
+	police->tcfp_t_c = ktime_get_ns();
 	police->tcf_index = parm->index ? parm->index :
 		tcf_hash_new_index(hinfo);
 	h = tcf_hash(police->tcf_index, POL_TAB_MASK);
@@ -279,7 +279,7 @@ static int tcf_act_police(struct sk_buff *skb, const struct tc_action *a,
 			return police->tcfp_result;
 		}
 
-		now = ktime_to_ns(ktime_get());
+		now = ktime_get_ns();
 		toks = min_t(s64, now - police->tcfp_t_c,
 			     police->tcfp_burst);
 		if (police->peak_present) {
diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c
index ba32c2b005d0..e12f997e1b4c 100644
--- a/net/sched/sch_fq.c
+++ b/net/sched/sch_fq.c
@@ -416,7 +416,7 @@ static void fq_check_throttled(struct fq_sched_data *q, u64 now)
 static struct sk_buff *fq_dequeue(struct Qdisc *sch)
 {
 	struct fq_sched_data *q = qdisc_priv(sch);
-	u64 now = ktime_to_ns(ktime_get());
+	u64 now = ktime_get_ns();
 	struct fq_flow_head *head;
 	struct sk_buff *skb;
 	struct fq_flow *f;
@@ -787,7 +787,7 @@ nla_put_failure:
 static int fq_dump_stats(struct Qdisc *sch, struct gnet_dump *d)
 {
 	struct fq_sched_data *q = qdisc_priv(sch);
-	u64 now = ktime_to_ns(ktime_get());
+	u64 now = ktime_get_ns();
 	struct tc_fq_qd_stats st = {
 		.gc_flows		= q->stat_gc_flows,
 		.highprio_packets	= q->stat_internal_packets,
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 9f949abcacef..aea942ce6008 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -895,7 +895,7 @@ ok:
 
 	if (!sch->q.qlen)
 		goto fin;
-	q->now = ktime_to_ns(ktime_get());
+	q->now = ktime_get_ns();
 	start_at = jiffies;
 
 	next_event = q->now + 5LLU * NSEC_PER_SEC;
@@ -1225,7 +1225,7 @@ static void htb_parent_to_leaf(struct htb_sched *q, struct htb_class *cl,
 	parent->un.leaf.q = new_q ? new_q : &noop_qdisc;
 	parent->tokens = parent->buffer;
 	parent->ctokens = parent->cbuffer;
-	parent->t_c = ktime_to_ns(ktime_get());
+	parent->t_c = ktime_get_ns();
 	parent->cmode = HTB_CAN_SEND;
 }
 
@@ -1455,7 +1455,7 @@ static int htb_change_class(struct Qdisc *sch, u32 classid,
 		cl->tokens = PSCHED_TICKS2NS(hopt->buffer);
 		cl->ctokens = PSCHED_TICKS2NS(hopt->cbuffer);
 		cl->mbuffer = 60ULL * NSEC_PER_SEC;	/* 1min */
-		cl->t_c = ktime_to_ns(ktime_get());
+		cl->t_c = ktime_get_ns();
 		cl->cmode = HTB_CAN_SEND;
 
 		/* attach to the hash list and parent's family */
diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
index 18ff63433709..0c39b754083b 100644
--- a/net/sched/sch_tbf.c
+++ b/net/sched/sch_tbf.c
@@ -239,7 +239,7 @@ static struct sk_buff *tbf_dequeue(struct Qdisc *sch)
 		s64 ptoks = 0;
 		unsigned int len = qdisc_pkt_len(skb);
 
-		now = ktime_to_ns(ktime_get());
+		now = ktime_get_ns();
 		toks = min_t(s64, now - q->t_c, q->buffer);
 
 		if (tbf_peak_present(q)) {
@@ -292,7 +292,7 @@ static void tbf_reset(struct Qdisc *sch)
 
 	qdisc_reset(q->qdisc);
 	sch->q.qlen = 0;
-	q->t_c = ktime_to_ns(ktime_get());
+	q->t_c = ktime_get_ns();
 	q->tokens = q->buffer;
 	q->ptokens = q->mtu;
 	qdisc_watchdog_cancel(&q->watchdog);
@@ -431,7 +431,7 @@ static int tbf_init(struct Qdisc *sch, struct nlattr *opt)
 	if (opt == NULL)
 		return -EINVAL;
 
-	q->t_c = ktime_to_ns(ktime_get());
+	q->t_c = ktime_get_ns();
 	qdisc_watchdog_init(&q->watchdog, sch);
 	q->qdisc = &noop_qdisc;
 

^ permalink raw reply related

* Re: [PATCH v2] carl9170: Remove redundant protection check
From: Christian Lamparter @ 2014-08-23  1:36 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Andreea-Cristina Bernat, linville, linux-wireless, netdev,
	linux-kernel, paulmck, j
In-Reply-To: <1408749799.5604.38.camel@edumazet-glaptop2.roam.corp.google.com>

On Friday, August 22, 2014 04:23:19 PM Eric Dumazet wrote:
> On Fri, 2014-08-22 at 23:53 +0200, Christian Lamparter wrote:
> 
> > The sta_info->agg[tid] check is not needed (for reference, see [0]).
> > (There is already a check in mac80211 which prevents the leak of
> > sta_info->agg[tid] [1]).
> > 
> > Regards
> > Christian
> > 
> > [0] <https://lkml.org/lkml/2014/8/20/725>
> > [1] <http://lxr.free-electrons.com/source/net/mac80211/agg-tx.c#L583>
> > 
> 
> Hmpfff... this code is quite confusing. 
That's true. Furthermore, parts of the logic are also embedded in
the mac80211-stack and above. So, it's very hard to see the whole
big picture, just by looking at the driver code.

> RCU is used both in tricky way (carl9170_ampdu_gc() is an example)
> and a talisman (the part you remove)
I know that game ;-). But fair enough: if you have concerns about
the complexity of the code in question: I'm willing to help you
and explain the quirks in detail if necessary. I think this is a
valuable addition, since "external consultants" are hard to come
by.

> Why is rcu_assign_pointer(sta_info->agg[tid], tid_info);
> done inside the spinlock protected region, I don't know.
The pointer in sta_info->agg[tid] is used exclusively by 
the tx.c code... It is queried only if an outgoing frame
has the IEEE80211_TX_CTL_AMPDU flag set. 

But for this flag to be set, the aggregation session has
to be operational. This requires two calls to ampdu_action [0].
(first with: IEEE80211_AMPDU_TX_START and later with:
IEEE80211_AMPDU_TX_OPERATIONAL).

=> If you want to make a patch to move this rcu_assign_pointer(...)
after the spin_unlock_bh() - Then: Yes, GO FOR IT!
 
> If this code relies on external protection, a comment would help its
> comprehension for sure.
> 
> For example, you could add a 
> BUG_ON(rcu_access_pointer(sta_info->agg[tid]));
> so that we are sure requirements are not changed
> in the callers one day.
Maybe, but then: Is a "specific driver" the right place for this?
Other drivers may also depend on ampdu_action not changing.
As for the logic: The AMPDU handshake itself is part of the 802.11
spec. If you are interested you can get 802.11-2012 [1] and look 
into Section 9.21 "Block Acknowledgment". It contains a message
sequence chart and details about the setup and tear down procedures
for aggregation session [which is at the heart of the ampdu_action
callback issue].

Note: mac80211 has a "software simulator" mac80211_hwsim [2].
It can be (and is) used to test most of the mac80211 functionality.
So what do you think?

Regards
Christian 

[0] <https://www.kernel.org/doc/htmldocs/80211/aggregation.html>
[1] <http://standards.ieee.org/findstds/standard/802.11-2012.html>
[2] <http://wireless.kernel.org/en/users/Drivers/mac80211_hwsim>

^ permalink raw reply

* [PATCH net-next 0/7] net: phy: bcm7xxx: APD and EEE support
From: Florian Fainelli @ 2014-08-23  1:55 UTC (permalink / raw)
  To: netdev; +Cc: davem, Florian Fainelli

Hi David,

This patch series enables Auto-power down and EEE for the BCM7xxx integrated
Gigabit PHYs.

I also put a fix for the fixed PHY that would allow clause 45 over clause 22
reads/writes but would return bogus data by using e.g: ethtool --show-eee

Thanks!

Florian Fainelli (7):
  net: phy: broadcom: extract all registers to brcmphy.h
  net: phy: broadcom: move shadow 0x1C register accessors to brcmphy.h
  net: phy: bcm7xxx: enable auto power down
  net: phy: fixed: return an error for Clause 45 over 22 reads
  net: phy: export phy_{read,write}_mmd_indirect
  net: phy: allow phy_init_eee() to work with internal PHYs
  net: phy: bcm7xxx: enable EEE at the PHY level

 drivers/net/phy/bcm7xxx.c  |  58 ++++++++++++++++++++-
 drivers/net/phy/broadcom.c | 122 -------------------------------------------
 drivers/net/phy/fixed.c    |  11 ++++
 drivers/net/phy/phy.c      |  12 +++--
 include/linux/brcmphy.h    | 126 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/phy.h        |  27 ++++++++++
 6 files changed, 229 insertions(+), 127 deletions(-)

-- 
1.9.1

^ permalink raw reply

* [PATCH net-next 1/7] net: phy: broadcom: extract all registers to brcmphy.h
From: Florian Fainelli @ 2014-08-23  1:55 UTC (permalink / raw)
  To: netdev; +Cc: davem, Florian Fainelli
In-Reply-To: <1408758945-18908-1-git-send-email-f.fainelli@gmail.com>

Commit 439d39a9ac8fbbba9c04581361188f33f21ced50 ("net: phy: broadcom:
extract register definitions") added a bunch of registers to brcmphy.h
but left some to broadcom.c, move all of them to the header file since
the BCM54xx and BCM7xxx PHY drivers do share all of these registers.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/phy/broadcom.c | 104 ---------------------------------------------
 include/linux/brcmphy.h    | 103 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 103 insertions(+), 104 deletions(-)

diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
index 34088d60da74..6001d76d8676 100644
--- a/drivers/net/phy/broadcom.c
+++ b/drivers/net/phy/broadcom.c
@@ -25,110 +25,6 @@
 #define BRCM_PHY_REV(phydev) \
 	((phydev)->drv->phy_id & ~((phydev)->drv->phy_id_mask))
 
-/*
- * Broadcom LED source encodings.  These are used in BCM5461, BCM5481,
- * BCM5482, and possibly some others.
- */
-#define BCM_LED_SRC_LINKSPD1	0x0
-#define BCM_LED_SRC_LINKSPD2	0x1
-#define BCM_LED_SRC_XMITLED	0x2
-#define BCM_LED_SRC_ACTIVITYLED	0x3
-#define BCM_LED_SRC_FDXLED	0x4
-#define BCM_LED_SRC_SLAVE	0x5
-#define BCM_LED_SRC_INTR	0x6
-#define BCM_LED_SRC_QUALITY	0x7
-#define BCM_LED_SRC_RCVLED	0x8
-#define BCM_LED_SRC_MULTICOLOR1	0xa
-#define BCM_LED_SRC_OPENSHORT	0xb
-#define BCM_LED_SRC_OFF		0xe	/* Tied high */
-#define BCM_LED_SRC_ON		0xf	/* Tied low */
-
-
-/*
- * BCM5482: Shadow registers
- * Shadow values go into bits [14:10] of register 0x1c to select a shadow
- * register to access.
- */
-/* 00101: Spare Control Register 3 */
-#define BCM54XX_SHD_SCR3		0x05
-#define  BCM54XX_SHD_SCR3_DEF_CLK125	0x0001
-#define  BCM54XX_SHD_SCR3_DLLAPD_DIS	0x0002
-#define  BCM54XX_SHD_SCR3_TRDDAPD	0x0004
-
-/* 01010: Auto Power-Down */
-#define BCM54XX_SHD_APD			0x0a
-#define  BCM54XX_SHD_APD_EN		0x0020
-
-#define BCM5482_SHD_LEDS1	0x0d	/* 01101: LED Selector 1 */
-					/* LED3 / ~LINKSPD[2] selector */
-#define BCM5482_SHD_LEDS1_LED3(src)	((src & 0xf) << 4)
-					/* LED1 / ~LINKSPD[1] selector */
-#define BCM5482_SHD_LEDS1_LED1(src)	((src & 0xf) << 0)
-#define BCM54XX_SHD_RGMII_MODE	0x0b	/* 01011: RGMII Mode Selector */
-#define BCM5482_SHD_SSD		0x14	/* 10100: Secondary SerDes control */
-#define BCM5482_SHD_SSD_LEDM	0x0008	/* SSD LED Mode enable */
-#define BCM5482_SHD_SSD_EN	0x0001	/* SSD enable */
-#define BCM5482_SHD_MODE	0x1f	/* 11111: Mode Control Register */
-#define BCM5482_SHD_MODE_1000BX	0x0001	/* Enable 1000BASE-X registers */
-
-
-/*
- * EXPANSION SHADOW ACCESS REGISTERS.  (PHY REG 0x15, 0x16, and 0x17)
- */
-#define MII_BCM54XX_EXP_AADJ1CH0		0x001f
-#define  MII_BCM54XX_EXP_AADJ1CH0_SWP_ABCD_OEN	0x0200
-#define  MII_BCM54XX_EXP_AADJ1CH0_SWSEL_THPF	0x0100
-#define MII_BCM54XX_EXP_AADJ1CH3		0x601f
-#define  MII_BCM54XX_EXP_AADJ1CH3_ADCCKADJ	0x0002
-#define MII_BCM54XX_EXP_EXP08			0x0F08
-#define  MII_BCM54XX_EXP_EXP08_RJCT_2MHZ	0x0001
-#define  MII_BCM54XX_EXP_EXP08_EARLY_DAC_WAKE	0x0200
-#define MII_BCM54XX_EXP_EXP75			0x0f75
-#define  MII_BCM54XX_EXP_EXP75_VDACCTRL		0x003c
-#define  MII_BCM54XX_EXP_EXP75_CM_OSC		0x0001
-#define MII_BCM54XX_EXP_EXP96			0x0f96
-#define  MII_BCM54XX_EXP_EXP96_MYST		0x0010
-#define MII_BCM54XX_EXP_EXP97			0x0f97
-#define  MII_BCM54XX_EXP_EXP97_MYST		0x0c0c
-
-/*
- * BCM5482: Secondary SerDes registers
- */
-#define BCM5482_SSD_1000BX_CTL		0x00	/* 1000BASE-X Control */
-#define BCM5482_SSD_1000BX_CTL_PWRDOWN	0x0800	/* Power-down SSD */
-#define BCM5482_SSD_SGMII_SLAVE		0x15	/* SGMII Slave Register */
-#define BCM5482_SSD_SGMII_SLAVE_EN	0x0002	/* Slave mode enable */
-#define BCM5482_SSD_SGMII_SLAVE_AD	0x0001	/* Slave auto-detection */
-
-
-/*****************************************************************************/
-/* Fast Ethernet Transceiver definitions. */
-/*****************************************************************************/
-
-#define MII_BRCM_FET_INTREG		0x1a	/* Interrupt register */
-#define MII_BRCM_FET_IR_MASK		0x0100	/* Mask all interrupts */
-#define MII_BRCM_FET_IR_LINK_EN		0x0200	/* Link status change enable */
-#define MII_BRCM_FET_IR_SPEED_EN	0x0400	/* Link speed change enable */
-#define MII_BRCM_FET_IR_DUPLEX_EN	0x0800	/* Duplex mode change enable */
-#define MII_BRCM_FET_IR_ENABLE		0x4000	/* Interrupt enable */
-
-#define MII_BRCM_FET_BRCMTEST		0x1f	/* Brcm test register */
-#define MII_BRCM_FET_BT_SRE		0x0080	/* Shadow register enable */
-
-
-/*** Shadow register definitions ***/
-
-#define MII_BRCM_FET_SHDW_MISCCTRL	0x10	/* Shadow misc ctrl */
-#define MII_BRCM_FET_SHDW_MC_FAME	0x4000	/* Force Auto MDIX enable */
-
-#define MII_BRCM_FET_SHDW_AUXMODE4	0x1a	/* Auxiliary mode 4 */
-#define MII_BRCM_FET_SHDW_AM4_LED_MASK	0x0003
-#define MII_BRCM_FET_SHDW_AM4_LED_MODE1 0x0001
-
-#define MII_BRCM_FET_SHDW_AUXSTAT2	0x1b	/* Auxiliary status 2 */
-#define MII_BRCM_FET_SHDW_AS2_APDE	0x0020	/* Auto power down enable */
-
-
 MODULE_DESCRIPTION("Broadcom PHY driver");
 MODULE_AUTHOR("Maciej W. Rozycki");
 MODULE_LICENSE("GPL");
diff --git a/include/linux/brcmphy.h b/include/linux/brcmphy.h
index 6f76277baf39..bf902d1c1b19 100644
--- a/include/linux/brcmphy.h
+++ b/include/linux/brcmphy.h
@@ -93,4 +93,107 @@
 
 #define MII_BCM54XX_AUXCTL_SHDWSEL_AUXCTL	0x0000
 
+/*
+ * Broadcom LED source encodings.  These are used in BCM5461, BCM5481,
+ * BCM5482, and possibly some others.
+ */
+#define BCM_LED_SRC_LINKSPD1	0x0
+#define BCM_LED_SRC_LINKSPD2	0x1
+#define BCM_LED_SRC_XMITLED	0x2
+#define BCM_LED_SRC_ACTIVITYLED	0x3
+#define BCM_LED_SRC_FDXLED	0x4
+#define BCM_LED_SRC_SLAVE	0x5
+#define BCM_LED_SRC_INTR	0x6
+#define BCM_LED_SRC_QUALITY	0x7
+#define BCM_LED_SRC_RCVLED	0x8
+#define BCM_LED_SRC_MULTICOLOR1	0xa
+#define BCM_LED_SRC_OPENSHORT	0xb
+#define BCM_LED_SRC_OFF		0xe	/* Tied high */
+#define BCM_LED_SRC_ON		0xf	/* Tied low */
+
+
+/*
+ * BCM5482: Shadow registers
+ * Shadow values go into bits [14:10] of register 0x1c to select a shadow
+ * register to access.
+ */
+/* 00101: Spare Control Register 3 */
+#define BCM54XX_SHD_SCR3		0x05
+#define  BCM54XX_SHD_SCR3_DEF_CLK125	0x0001
+#define  BCM54XX_SHD_SCR3_DLLAPD_DIS	0x0002
+#define  BCM54XX_SHD_SCR3_TRDDAPD	0x0004
+
+/* 01010: Auto Power-Down */
+#define BCM54XX_SHD_APD			0x0a
+#define  BCM54XX_SHD_APD_EN		0x0020
+
+#define BCM5482_SHD_LEDS1	0x0d	/* 01101: LED Selector 1 */
+					/* LED3 / ~LINKSPD[2] selector */
+#define BCM5482_SHD_LEDS1_LED3(src)	((src & 0xf) << 4)
+					/* LED1 / ~LINKSPD[1] selector */
+#define BCM5482_SHD_LEDS1_LED1(src)	((src & 0xf) << 0)
+#define BCM54XX_SHD_RGMII_MODE	0x0b	/* 01011: RGMII Mode Selector */
+#define BCM5482_SHD_SSD		0x14	/* 10100: Secondary SerDes control */
+#define BCM5482_SHD_SSD_LEDM	0x0008	/* SSD LED Mode enable */
+#define BCM5482_SHD_SSD_EN	0x0001	/* SSD enable */
+#define BCM5482_SHD_MODE	0x1f	/* 11111: Mode Control Register */
+#define BCM5482_SHD_MODE_1000BX	0x0001	/* Enable 1000BASE-X registers */
+
+
+/*
+ * EXPANSION SHADOW ACCESS REGISTERS.  (PHY REG 0x15, 0x16, and 0x17)
+ */
+#define MII_BCM54XX_EXP_AADJ1CH0		0x001f
+#define  MII_BCM54XX_EXP_AADJ1CH0_SWP_ABCD_OEN	0x0200
+#define  MII_BCM54XX_EXP_AADJ1CH0_SWSEL_THPF	0x0100
+#define MII_BCM54XX_EXP_AADJ1CH3		0x601f
+#define  MII_BCM54XX_EXP_AADJ1CH3_ADCCKADJ	0x0002
+#define MII_BCM54XX_EXP_EXP08			0x0F08
+#define  MII_BCM54XX_EXP_EXP08_RJCT_2MHZ	0x0001
+#define  MII_BCM54XX_EXP_EXP08_EARLY_DAC_WAKE	0x0200
+#define MII_BCM54XX_EXP_EXP75			0x0f75
+#define  MII_BCM54XX_EXP_EXP75_VDACCTRL		0x003c
+#define  MII_BCM54XX_EXP_EXP75_CM_OSC		0x0001
+#define MII_BCM54XX_EXP_EXP96			0x0f96
+#define  MII_BCM54XX_EXP_EXP96_MYST		0x0010
+#define MII_BCM54XX_EXP_EXP97			0x0f97
+#define  MII_BCM54XX_EXP_EXP97_MYST		0x0c0c
+
+/*
+ * BCM5482: Secondary SerDes registers
+ */
+#define BCM5482_SSD_1000BX_CTL		0x00	/* 1000BASE-X Control */
+#define BCM5482_SSD_1000BX_CTL_PWRDOWN	0x0800	/* Power-down SSD */
+#define BCM5482_SSD_SGMII_SLAVE		0x15	/* SGMII Slave Register */
+#define BCM5482_SSD_SGMII_SLAVE_EN	0x0002	/* Slave mode enable */
+#define BCM5482_SSD_SGMII_SLAVE_AD	0x0001	/* Slave auto-detection */
+
+
+/*****************************************************************************/
+/* Fast Ethernet Transceiver definitions. */
+/*****************************************************************************/
+
+#define MII_BRCM_FET_INTREG		0x1a	/* Interrupt register */
+#define MII_BRCM_FET_IR_MASK		0x0100	/* Mask all interrupts */
+#define MII_BRCM_FET_IR_LINK_EN		0x0200	/* Link status change enable */
+#define MII_BRCM_FET_IR_SPEED_EN	0x0400	/* Link speed change enable */
+#define MII_BRCM_FET_IR_DUPLEX_EN	0x0800	/* Duplex mode change enable */
+#define MII_BRCM_FET_IR_ENABLE		0x4000	/* Interrupt enable */
+
+#define MII_BRCM_FET_BRCMTEST		0x1f	/* Brcm test register */
+#define MII_BRCM_FET_BT_SRE		0x0080	/* Shadow register enable */
+
+
+/*** Shadow register definitions ***/
+
+#define MII_BRCM_FET_SHDW_MISCCTRL	0x10	/* Shadow misc ctrl */
+#define MII_BRCM_FET_SHDW_MC_FAME	0x4000	/* Force Auto MDIX enable */
+
+#define MII_BRCM_FET_SHDW_AUXMODE4	0x1a	/* Auxiliary mode 4 */
+#define MII_BRCM_FET_SHDW_AM4_LED_MASK	0x0003
+#define MII_BRCM_FET_SHDW_AM4_LED_MODE1 0x0001
+
+#define MII_BRCM_FET_SHDW_AUXSTAT2	0x1b	/* Auxiliary status 2 */
+#define MII_BRCM_FET_SHDW_AS2_APDE	0x0020	/* Auto power down enable */
+
 #endif /* _LINUX_BRCMPHY_H */
-- 
1.9.1

^ permalink raw reply related

* [PATCH net-next 2/7] net: phy: broadcom: move shadow 0x1C register accessors to brcmphy.h
From: Florian Fainelli @ 2014-08-23  1:55 UTC (permalink / raw)
  To: netdev; +Cc: davem, Florian Fainelli
In-Reply-To: <1408758945-18908-1-git-send-email-f.fainelli@gmail.com>

The shadow register 0x1C is used both by the BCM54xxx PHYs and the
BCM7xxx internal PHYs, move the accessors to a common location so both
drivers can use them.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/phy/broadcom.c | 18 ------------------
 include/linux/brcmphy.h    | 20 ++++++++++++++++++++
 2 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
index 6001d76d8676..854f2c9a7b2b 100644
--- a/drivers/net/phy/broadcom.c
+++ b/drivers/net/phy/broadcom.c
@@ -29,24 +29,6 @@ MODULE_DESCRIPTION("Broadcom PHY driver");
 MODULE_AUTHOR("Maciej W. Rozycki");
 MODULE_LICENSE("GPL");
 
-/*
- * Indirect register access functions for the 1000BASE-T/100BASE-TX/10BASE-T
- * 0x1c shadow registers.
- */
-static int bcm54xx_shadow_read(struct phy_device *phydev, u16 shadow)
-{
-	phy_write(phydev, MII_BCM54XX_SHD, MII_BCM54XX_SHD_VAL(shadow));
-	return MII_BCM54XX_SHD_DATA(phy_read(phydev, MII_BCM54XX_SHD));
-}
-
-static int bcm54xx_shadow_write(struct phy_device *phydev, u16 shadow, u16 val)
-{
-	return phy_write(phydev, MII_BCM54XX_SHD,
-			 MII_BCM54XX_SHD_WRITE |
-			 MII_BCM54XX_SHD_VAL(shadow) |
-			 MII_BCM54XX_SHD_DATA(val));
-}
-
 /* Indirect register access functions for the Expansion Registers */
 static int bcm54xx_exp_read(struct phy_device *phydev, u16 regnum)
 {
diff --git a/include/linux/brcmphy.h b/include/linux/brcmphy.h
index bf902d1c1b19..c221e6df2bc4 100644
--- a/include/linux/brcmphy.h
+++ b/include/linux/brcmphy.h
@@ -196,4 +196,24 @@
 #define MII_BRCM_FET_SHDW_AUXSTAT2	0x1b	/* Auxiliary status 2 */
 #define MII_BRCM_FET_SHDW_AS2_APDE	0x0020	/* Auto power down enable */
 
+/*
+ * Indirect register access functions for the 1000BASE-T/100BASE-TX/10BASE-T
+ * 0x1c shadow registers.
+ */
+static inline int bcm54xx_shadow_read(struct phy_device *phydev, u16 shadow)
+{
+	phy_write(phydev, MII_BCM54XX_SHD, MII_BCM54XX_SHD_VAL(shadow));
+	return MII_BCM54XX_SHD_DATA(phy_read(phydev, MII_BCM54XX_SHD));
+}
+
+static inline int bcm54xx_shadow_write(struct phy_device *phydev, u16 shadow,
+				       u16 val)
+{
+	return phy_write(phydev, MII_BCM54XX_SHD,
+			 MII_BCM54XX_SHD_WRITE |
+			 MII_BCM54XX_SHD_VAL(shadow) |
+			 MII_BCM54XX_SHD_DATA(val));
+}
+
+
 #endif /* _LINUX_BRCMPHY_H */
-- 
1.9.1

^ permalink raw reply related

* [PATCH net-next 3/7] net: phy: bcm7xxx: enable auto power down
From: Florian Fainelli @ 2014-08-23  1:55 UTC (permalink / raw)
  To: netdev; +Cc: davem, Florian Fainelli
In-Reply-To: <1408758945-18908-1-git-send-email-f.fainelli@gmail.com>

The 28nm process BCM7xxx internal Gigabit PHYs all support automatic
power down, turn on that feature as part of the configuration
initialization callback.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/phy/bcm7xxx.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/bcm7xxx.c b/drivers/net/phy/bcm7xxx.c
index 526b94cea569..9dcf241b62ef 100644
--- a/drivers/net/phy/bcm7xxx.c
+++ b/drivers/net/phy/bcm7xxx.c
@@ -146,6 +146,27 @@ static int bcm7xxx_28nm_afe_config_init(struct phy_device *phydev)
 	return 0;
 }
 
+static int bcm7xxx_apd_enable(struct phy_device *phydev)
+{
+	int val;
+
+	/* Enable powering down of the DLL during auto-power down */
+	val = bcm54xx_shadow_read(phydev, BCM54XX_SHD_SCR3);
+	if (val < 0)
+		return val;
+
+	val |= BCM54XX_SHD_SCR3_DLLAPD_DIS;
+	bcm54xx_shadow_write(phydev, BCM54XX_SHD_SCR3, val);
+
+	/* Enable auto-power down */
+	val = bcm54xx_shadow_read(phydev, BCM54XX_SHD_APD);
+	if (val < 0)
+		return val;
+
+	val |= BCM54XX_SHD_APD_EN;
+	return bcm54xx_shadow_write(phydev, BCM54XX_SHD_APD, val);
+}
+
 static int bcm7xxx_28nm_config_init(struct phy_device *phydev)
 {
 	int ret;
@@ -154,7 +175,11 @@ static int bcm7xxx_28nm_config_init(struct phy_device *phydev)
 	if (ret)
 		return ret;
 
-	return bcm7xxx_28nm_afe_config_init(phydev);
+	ret = bcm7xxx_28nm_afe_config_init(phydev);
+	if (ret)
+		return ret;
+
+	return bcm7xxx_apd_enable(phydev);
 }
 
 static int phy_set_clr_bits(struct phy_device *dev, int location,
-- 
1.9.1

^ permalink raw reply related

* [PATCH net-next 4/7] net: phy: fixed: return an error for Clause 45 over 22 reads
From: Florian Fainelli @ 2014-08-23  1:55 UTC (permalink / raw)
  To: netdev; +Cc: davem, Florian Fainelli
In-Reply-To: <1408758945-18908-1-git-send-email-f.fainelli@gmail.com>

The fixed PHY driver does not properly emulate Clause 45 over Clause 22
MDIO reads, and as such, will return bogus values when we access such
registers.

Return an error when accessing these registers in order to prevent
advertising bogus capabilities such as EEE support and such.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/phy/fixed.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/net/phy/fixed.c b/drivers/net/phy/fixed.c
index d60d875cb445..5b19fbbda6d4 100644
--- a/drivers/net/phy/fixed.c
+++ b/drivers/net/phy/fixed.c
@@ -124,6 +124,17 @@ static int fixed_mdio_read(struct mii_bus *bus, int phy_addr, int reg_num)
 	if (reg_num >= MII_REGS_NUM)
 		return -1;
 
+	/* We do not support emulating Clause 45 over Clause 22 register reads
+	 * return an error instead of bogus data.
+	 */
+	switch (reg_num) {
+	case MII_MMD_CTRL:
+	case MII_MMD_DATA:
+		return -1;
+	default:
+		break;
+	}
+
 	list_for_each_entry(fp, &fmb->phys, node) {
 		if (fp->addr == phy_addr) {
 			/* Issue callback if user registered it. */
-- 
1.9.1

^ permalink raw reply related

* [PATCH net-next 5/7] net: phy: export phy_{read,write}_mmd_indirect
From: Florian Fainelli @ 2014-08-23  1:55 UTC (permalink / raw)
  To: netdev; +Cc: davem, Florian Fainelli
In-Reply-To: <1408758945-18908-1-git-send-email-f.fainelli@gmail.com>

Some PHY drivers might need to access Clause 45 registers in Clause 22
compatibility mode to e.g: properly advertise EEE support when disabled
by default.

Export these two helper functions: phy_read_mmd_indirect() and
phy_write_mmd_indirect() for drivers to use them.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/phy/phy.c |  6 ++++--
 include/linux/phy.h   | 27 +++++++++++++++++++++++++++
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index c94e2a27446a..e7a5893f32ff 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -955,7 +955,7 @@ static inline void mmd_phy_indirect(struct mii_bus *bus, int prtad, int devad,
  * 3) Write reg 13 // MMD Data Command for MMD DEVAD
  * 3) Read  reg 14 // Read MMD data
  */
-static int phy_read_mmd_indirect(struct phy_device *phydev, int prtad,
+int phy_read_mmd_indirect(struct phy_device *phydev, int prtad,
 				 int devad, int addr)
 {
 	struct phy_driver *phydrv = phydev->drv;
@@ -971,6 +971,7 @@ static int phy_read_mmd_indirect(struct phy_device *phydev, int prtad,
 	}
 	return value;
 }
+EXPORT_SYMBOL(phy_read_mmd_indirect);
 
 /**
  * phy_write_mmd_indirect - writes data to the MMD registers
@@ -988,7 +989,7 @@ static int phy_read_mmd_indirect(struct phy_device *phydev, int prtad,
  * 3) Write reg 13 // MMD Data Command for MMD DEVAD
  * 3) Write reg 14 // Write MMD data
  */
-static void phy_write_mmd_indirect(struct phy_device *phydev, int prtad,
+void phy_write_mmd_indirect(struct phy_device *phydev, int prtad,
 				   int devad, int addr, u32 data)
 {
 	struct phy_driver *phydrv = phydev->drv;
@@ -1002,6 +1003,7 @@ static void phy_write_mmd_indirect(struct phy_device *phydev, int prtad,
 		phydrv->write_mmd_indirect(phydev, prtad, devad, addr, data);
 	}
 }
+EXPORT_SYMBOL(phy_write_mmd_indirect);
 
 /**
  * phy_init_eee - init and check the EEE feature
diff --git a/include/linux/phy.h b/include/linux/phy.h
index ed39956b5613..d090cfcaa167 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -598,6 +598,19 @@ static inline int phy_read_mmd(struct phy_device *phydev, int devad, u32 regnum)
 }
 
 /**
+ * phy_read_mmd_indirect - reads data from the MMD registers
+ * @phydev: The PHY device bus
+ * @prtad: MMD Address
+ * @devad: MMD DEVAD
+ * @addr: PHY address on the MII bus
+ *
+ * Description: it reads data from the MMD registers (clause 22 to access to
+ * clause 45) of the specified phy address.
+ */
+int phy_read_mmd_indirect(struct phy_device *phydev, int prtad,
+			  int devad, int addr);
+
+/**
  * phy_read - Convenience function for reading a given PHY register
  * @phydev: the phy_device struct
  * @regnum: register number to read
@@ -668,6 +681,20 @@ static inline int phy_write_mmd(struct phy_device *phydev, int devad,
 	return mdiobus_write(phydev->bus, phydev->addr, regnum, val);
 }
 
+/**
+ * phy_write_mmd_indirect - writes data to the MMD registers
+ * @phydev: The PHY device
+ * @prtad: MMD Address
+ * @devad: MMD DEVAD
+ * @addr: PHY address on the MII bus
+ * @data: data to write in the MMD register
+ *
+ * Description: Write data from the MMD registers of the specified
+ * phy address.
+ */
+void phy_write_mmd_indirect(struct phy_device *phydev, int prtad,
+			    int devad, int addr, u32 data);
+
 struct phy_device *phy_device_create(struct mii_bus *bus, int addr, int phy_id,
 				     bool is_c45,
 				     struct phy_c45_device_ids *c45_ids);
-- 
1.9.1

^ permalink raw reply related

* [PATCH net-next 6/7] net: phy: allow phy_init_eee() to work with internal PHYs
From: Florian Fainelli @ 2014-08-23  1:55 UTC (permalink / raw)
  To: netdev; +Cc: davem, Florian Fainelli
In-Reply-To: <1408758945-18908-1-git-send-email-f.fainelli@gmail.com>

Internal PHYs do not have any specific phy_interface_t defined because
they are within an Ethernet MAC or a larger IC, they will fail the early
check in phy_init_eee(). Allow these PHYs to proceed with EEE
initialization and report error/success by checking the standard C45
EEE-related registers.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/phy/phy.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index e7a5893f32ff..932190e04d08 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -1019,12 +1019,14 @@ int phy_init_eee(struct phy_device *phydev, bool clk_stop_enable)
 {
 	/* According to 802.3az,the EEE is supported only in full duplex-mode.
 	 * Also EEE feature is active when core is operating with MII, GMII
-	 * or RGMII.
+	 * or RGMII. Internal PHYs are also allowed to proceed and should
+	 * return an error if they do not support EEE.
 	 */
 	if ((phydev->duplex == DUPLEX_FULL) &&
 	    ((phydev->interface == PHY_INTERFACE_MODE_MII) ||
 	    (phydev->interface == PHY_INTERFACE_MODE_GMII) ||
-	    (phydev->interface == PHY_INTERFACE_MODE_RGMII))) {
+	    (phydev->interface == PHY_INTERFACE_MODE_RGMII) ||
+	     phy_is_internal(phydev))) {
 		int eee_lp, eee_cap, eee_adv;
 		u32 lp, cap, adv;
 		int status;
-- 
1.9.1

^ permalink raw reply related

* [PATCH net-next 7/7] net: phy: bcm7xxx: enable EEE at the PHY level
From: Florian Fainelli @ 2014-08-23  1:55 UTC (permalink / raw)
  To: netdev; +Cc: davem, Florian Fainelli
In-Reply-To: <1408758945-18908-1-git-send-email-f.fainelli@gmail.com>

The 28nm Gigabit PHY on BCM7xxx chips comes out of reset with absolutely
no EEE capabilities, such that we would actually return that we do not
support EEE when accessing 3.20 (MDIO_PCS_EEE_ABLE) registers.

Poke through the vendor-specific C45 register to enable EEE globally at
the PHY level, and advertise supported EEE modes.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/phy/bcm7xxx.c | 31 +++++++++++++++++++++++++++++++
 include/linux/brcmphy.h   |  3 +++
 2 files changed, 34 insertions(+)

diff --git a/drivers/net/phy/bcm7xxx.c b/drivers/net/phy/bcm7xxx.c
index 9dcf241b62ef..1e8c611831bf 100644
--- a/drivers/net/phy/bcm7xxx.c
+++ b/drivers/net/phy/bcm7xxx.c
@@ -14,6 +14,7 @@
 #include <linux/delay.h>
 #include <linux/bitops.h>
 #include <linux/brcmphy.h>
+#include <linux/mdio.h>
 
 /* Broadcom BCM7xxx internal PHY registers */
 #define MII_BCM7XXX_CHANNEL_WIDTH	0x2000
@@ -167,6 +168,32 @@ static int bcm7xxx_apd_enable(struct phy_device *phydev)
 	return bcm54xx_shadow_write(phydev, BCM54XX_SHD_APD, val);
 }
 
+static int bcm7xxx_eee_enable(struct phy_device *phydev)
+{
+	int val;
+
+	val = phy_read_mmd_indirect(phydev, BRCM_CL45VEN_EEE_CONTROL,
+				    MDIO_MMD_AN, phydev->addr);
+	if (val < 0)
+		return val;
+
+	/* Enable general EEE feature at the PHY level */
+	val |= LPI_FEATURE_EN | LPI_FEATURE_EN_DIG1000X;
+
+	phy_write_mmd_indirect(phydev, BRCM_CL45VEN_EEE_CONTROL,
+			       MDIO_MMD_AN, phydev->addr, val);
+
+	/* Advertise supported modes */
+	val = phy_read_mmd_indirect(phydev, MDIO_AN_EEE_ADV,
+				    MDIO_MMD_AN, phydev->addr);
+
+	val |= (MDIO_AN_EEE_ADV_100TX | MDIO_AN_EEE_ADV_1000T);
+	phy_write_mmd_indirect(phydev, MDIO_AN_EEE_ADV,
+			       MDIO_MMD_AN, phydev->addr, val);
+
+	return 0;
+}
+
 static int bcm7xxx_28nm_config_init(struct phy_device *phydev)
 {
 	int ret;
@@ -179,6 +206,10 @@ static int bcm7xxx_28nm_config_init(struct phy_device *phydev)
 	if (ret)
 		return ret;
 
+	ret = bcm7xxx_eee_enable(phydev);
+	if (ret)
+		return ret;
+
 	return bcm7xxx_apd_enable(phydev);
 }
 
diff --git a/include/linux/brcmphy.h b/include/linux/brcmphy.h
index c221e6df2bc4..fcf7675d5d92 100644
--- a/include/linux/brcmphy.h
+++ b/include/linux/brcmphy.h
@@ -215,5 +215,8 @@ static inline int bcm54xx_shadow_write(struct phy_device *phydev, u16 shadow,
 			 MII_BCM54XX_SHD_DATA(val));
 }
 
+#define BRCM_CL45VEN_EEE_CONTROL	0x803d
+#define LPI_FEATURE_EN			0x8000
+#define LPI_FEATURE_EN_DIG1000X		0x4000
 
 #endif /* _LINUX_BRCMPHY_H */
-- 
1.9.1

^ permalink raw reply related

* [PATCH 1/8] e1000e: Fix TSO with non-accelerated vlans
From: vyasevich @ 2014-08-23  2:17 UTC (permalink / raw)
  To: netdev
  Cc: Vladislav Yasevich, Linux NICS, e1000-devel, Bruce Allan,
	Jesse Brandeburg, John Ronciak
In-Reply-To: <1408760230-7457-1-git-send-email-vysevich@gmail.com>

From: Vladislav Yasevich <vyasevic@redhat.com>

This device claims  TSO support for vlans.  It also allows a
user to control vlan acceleration offloading.  As such, it is
possible to turn off vlan acceleration and configure a vlan
which will continue to support TSO.

In such situation the packet passed down the the device will contain
a vlan header and skb->protocol will be set to ETH_P_8021Q.
The device assumes that skb->protocol contains network protocol
value and uses that value to set up TSO information.  This results
in corrupted frames sent on the wire.  Corruptions include
incorrect IP total length and invalid IP checksum.

This patch extract the protocol value correctly and corrects TSO
for non-accelerated traffic.

CC: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
CC: Jesse Brandeburg <jesse.brandeburg@intel.com>
CC: Bruce Allan <bruce.w.allan@intel.com>
CC: Carolyn Wyborny <carolyn.wyborny@intel.com>
CC: Don Skidmore <donald.c.skidmore@intel.com>
CC: Greg Rose <gregory.v.rose@intel.com>
CC: Alex Duyck <alexander.h.duyck@intel.com>
CC: John Ronciak <john.ronciak@intel.com>
CC: Mitch Williams <mitch.a.williams@intel.com>
CC: Linux NICS <linux.nics@intel.com>
CC: e1000-devel@lists.sourceforge.net
Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
 drivers/net/ethernet/intel/e1000e/netdev.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 65c3aef..247335d 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -5164,7 +5164,8 @@ link_up:
 #define E1000_TX_FLAGS_VLAN_MASK	0xffff0000
 #define E1000_TX_FLAGS_VLAN_SHIFT	16
 
-static int e1000_tso(struct e1000_ring *tx_ring, struct sk_buff *skb)
+static int e1000_tso(struct e1000_ring *tx_ring, struct sk_buff *skb,
+		     __be16 protocol)
 {
 	struct e1000_context_desc *context_desc;
 	struct e1000_buffer *buffer_info;
@@ -5183,7 +5184,7 @@ static int e1000_tso(struct e1000_ring *tx_ring, struct sk_buff *skb)
 
 	hdr_len = skb_transport_offset(skb) + tcp_hdrlen(skb);
 	mss = skb_shinfo(skb)->gso_size;
-	if (skb->protocol == htons(ETH_P_IP)) {
+	if (protocol == htons(ETH_P_IP)) {
 		struct iphdr *iph = ip_hdr(skb);
 		iph->tot_len = 0;
 		iph->check = 0;
@@ -5231,7 +5232,8 @@ static int e1000_tso(struct e1000_ring *tx_ring, struct sk_buff *skb)
 	return 1;
 }
 
-static bool e1000_tx_csum(struct e1000_ring *tx_ring, struct sk_buff *skb)
+static bool e1000_tx_csum(struct e1000_ring *tx_ring, struct sk_buff *skb,
+			  __be16 protocol)
 {
 	struct e1000_adapter *adapter = tx_ring->adapter;
 	struct e1000_context_desc *context_desc;
@@ -5239,16 +5241,10 @@ static bool e1000_tx_csum(struct e1000_ring *tx_ring, struct sk_buff *skb)
 	unsigned int i;
 	u8 css;
 	u32 cmd_len = E1000_TXD_CMD_DEXT;
-	__be16 protocol;
 
 	if (skb->ip_summed != CHECKSUM_PARTIAL)
 		return false;
 
-	if (skb->protocol == cpu_to_be16(ETH_P_8021Q))
-		protocol = vlan_eth_hdr(skb)->h_vlan_encapsulated_proto;
-	else
-		protocol = skb->protocol;
-
 	switch (protocol) {
 	case cpu_to_be16(ETH_P_IP):
 		if (ip_hdr(skb)->protocol == IPPROTO_TCP)
@@ -5546,6 +5542,7 @@ static netdev_tx_t e1000_xmit_frame(struct sk_buff *skb,
 	int count = 0;
 	int tso;
 	unsigned int f;
+	__be16 protocol = vlan_get_protocol(skb);
 
 	if (test_bit(__E1000_DOWN, &adapter->state)) {
 		dev_kfree_skb_any(skb);
@@ -5620,7 +5617,7 @@ static netdev_tx_t e1000_xmit_frame(struct sk_buff *skb,
 
 	first = tx_ring->next_to_use;
 
-	tso = e1000_tso(tx_ring, skb);
+	tso = e1000_tso(tx_ring, skb, protocol);
 	if (tso < 0) {
 		dev_kfree_skb_any(skb);
 		return NETDEV_TX_OK;
@@ -5628,14 +5625,14 @@ static netdev_tx_t e1000_xmit_frame(struct sk_buff *skb,
 
 	if (tso)
 		tx_flags |= E1000_TX_FLAGS_TSO;
-	else if (e1000_tx_csum(tx_ring, skb))
+	else if (e1000_tx_csum(tx_ring, skb, protocol))
 		tx_flags |= E1000_TX_FLAGS_CSUM;
 
 	/* Old method was to assume IPv4 packet by default if TSO was enabled.
 	 * 82571 hardware supports TSO capabilities for IPv6 as well...
 	 * no longer assume, we must.
 	 */
-	if (skb->protocol == htons(ETH_P_IP))
+	if (protocol == htons(ETH_P_IP))
 		tx_flags |= E1000_TX_FLAGS_IPV4;
 
 	if (unlikely(skb->no_fcs))
-- 
1.9.3


------------------------------------------------------------------------------
Slashdot TV.  
Video for Nerds.  Stuff that matters.
http://tv.slashdot.org/
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply related

* [PATCH 0/8] Fix TSO and checksum issues with non-accelerated vlan traffic.
From: vyasevich @ 2014-08-23  2:17 UTC (permalink / raw)
  To: netdev
  Cc: Vladislav Yasevich, Jeff Kirsher, Jesse Brandeburg, Bruce Allan,
	Carolyn Wyborny, Don Skidmore, Greg Rose, Alex Duyck,
	John Ronciak, Mitch Williams, Linux NICS, e1000-devel,
	Rasesh Mody, Thadeu Lima de Souza Cascardo, Thomas Petazzoni,
	Shahed Shaikh, Jitendra Kalsaria, Ron Mercer, linux-driver,
	Chris Metcalf

From: Vladislav Yasevich <vyasevic@redhat.com>

I've recently ran across something rather interesting when testing vlans
from inside VMs.  In some scenarios I was getting awfull thruput.
Some debugging uncovered a very scary packet corruption.  I was
seeing packets that had original TSO length as IP total length
and their ip checksum was 0.  This was with e1000e driver.

A bit more debugging uncovered an assumption made by that driver
that skb->protocol will contain l3 protocol information.  This
was not the case in my setup since I manually turned off vlan
tx acceleration for the device.  This caused the driver to not
initialize the tso information correctly and resulted in
corrupt TSO frames on the wire.

I decided to do some auditing of the usage of skb->protocols
in the drivers.  Out of all the drivers, the included 8 appear
to be effected.  They all allow user to control vlan acceleration
settings, all support TSO on vlan devices, and all use
skb->protocol to decide how to encode TSO information.  Some
also have similar problems when initializing hw checksum information.
On such device, it is simple enough to reproduce the issue.
Simply turn off TX VLAN acceleration on the device, create a vlan,
and run you favorite network performance tool.

There is 1 driver I ran across that I belive will trigger a BUG
in the system when used with vlans.  That driver is tile/tilepro.c
I have not changed it in this patch set and would hope that
the maintainer has time to look at it.

Thanks
-vlad

Vladislav Yasevich (8):
  e1000e: Fix TSO with non-accelerated vlans
  e1000: Fix TSO for non-accelerated vlan traffic
  bna: Support TSO and partial checksum with non-accelerated vlans.
  ehea: Fix TSO and hw checksums with non-accelerated vlan packets.
  i40e: Fix TSO and hw checksums for non-accelerated vlan packets.
  i40evf: Fix TSO and hw checksums for non-accelerated vlan packets.
  mvneta: Fix TSO and checksum for non-acceleration vlan traffic
  qlge: Fix TSO for non-accelerated vlan traffic

 drivers/net/ethernet/brocade/bna/bnad.c       |  7 ++++---
 drivers/net/ethernet/ibm/ehea/ehea_main.c     |  2 +-
 drivers/net/ethernet/intel/e1000/e1000_main.c | 19 +++++++++++--------
 drivers/net/ethernet/intel/e1000e/netdev.c    | 21 +++++++++------------
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   |  2 +-
 drivers/net/ethernet/intel/i40evf/i40e_txrx.c |  2 +-
 drivers/net/ethernet/marvell/mvneta.c         |  7 ++++---
 drivers/net/ethernet/qlogic/qlge/qlge_main.c  |  5 +++--
 8 files changed, 34 insertions(+), 31 deletions(-)

-- 
1.9.3

^ permalink raw reply

* [PATCH 3/8] bna: Support TSO and partial checksum with non-accelerated vlans.
From: vyasevich @ 2014-08-23  2:17 UTC (permalink / raw)
  To: netdev; +Cc: Vladislav Yasevich, Rasesh Mody
In-Reply-To: <1408760230-7457-1-git-send-email-vysevich@gmail.com>

From: Vladislav Yasevich <vyasevic@redhat.com>

This device claims TSO and checksum support for vlans.  It also
allows a user to control vlan acceleration offloading.  As such,
it is possible to turn off vlan acceleration and configure a vlan
which will continue to support TSO.

In such situation the packet passed down the the device will contain
a vlan header and skb->protocol will be set to ETH_P_8021Q.
The device assumes that skb->protocol contains network protocol
value and uses that value to set up TSO information.  This results
in corrupted frames sent on the wire.

This patch extract the protocol value correctly and corrects TSO
and checksums for non-accelerated traffic.

CC: Rasesh Mody <rmody@brocade.com>
Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
 drivers/net/ethernet/brocade/bna/bnad.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/brocade/bna/bnad.c b/drivers/net/ethernet/brocade/bna/bnad.c
index ff8cae5..ffc92a4 100644
--- a/drivers/net/ethernet/brocade/bna/bnad.c
+++ b/drivers/net/ethernet/brocade/bna/bnad.c
@@ -2506,7 +2506,7 @@ bnad_tso_prepare(struct bnad *bnad, struct sk_buff *skb)
 	 * For TSO, the TCP checksum field is seeded with pseudo-header sum
 	 * excluding the length field.
 	 */
-	if (skb->protocol == htons(ETH_P_IP)) {
+	if (vlan_get_protocol(skb) == htons(ETH_P_IP)) {
 		struct iphdr *iph = ip_hdr(skb);
 
 		/* Do we really need these? */
@@ -2870,12 +2870,13 @@ bnad_txq_wi_prepare(struct bnad *bnad, struct bna_tcb *tcb,
 		}
 
 		if (skb->ip_summed == CHECKSUM_PARTIAL) {
+			__be16 net_proto = vlan_get_protocol(skb);
 			u8 proto = 0;
 
-			if (skb->protocol == htons(ETH_P_IP))
+			if (net_proto == htons(ETH_P_IP))
 				proto = ip_hdr(skb)->protocol;
 #ifdef NETIF_F_IPV6_CSUM
-			else if (skb->protocol == htons(ETH_P_IPV6)) {
+			else if (net_proto == htons(ETH_P_IPV6)) {
 				/* nexthdr may not be TCP immediately. */
 				proto = ipv6_hdr(skb)->nexthdr;
 			}
-- 
1.9.3

^ permalink raw reply related

* [PATCH 4/8] ehea: Fix TSO and hw checksums with non-accelerated vlan packets.
From: vyasevich @ 2014-08-23  2:17 UTC (permalink / raw)
  To: netdev; +Cc: Vladislav Yasevich, Thadeu Lima de Souza Cascardo
In-Reply-To: <1408760230-7457-1-git-send-email-vysevich@gmail.com>

From: Vladislav Yasevich <vyasevic@redhat.com>

The driver claims that it can do TSO and IP checksums on vlan
devices and also allows user to control vlan acceleration offloading.
This makes it possible to push traffic to this driver that has TSO or
partial checksums set, but also have a non-accelearted vlan
header.  In this case, the driver will fail to correctly
identify such traffic and will not correctly perform
segmentation and checksum calculation.

Fix this by using vlan_get_protocol() helper instead of
assuming skb->protocol always has this information.

CC: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
 drivers/net/ethernet/ibm/ehea/ehea_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/ehea/ehea_main.c b/drivers/net/ethernet/ibm/ehea/ehea_main.c
index a0b418e..566b17d 100644
--- a/drivers/net/ethernet/ibm/ehea/ehea_main.c
+++ b/drivers/net/ethernet/ibm/ehea/ehea_main.c
@@ -1994,7 +1994,7 @@ static void xmit_common(struct sk_buff *skb, struct ehea_swqe *swqe)
 {
 	swqe->tx_control |= EHEA_SWQE_IMM_DATA_PRESENT | EHEA_SWQE_CRC;
 
-	if (skb->protocol != htons(ETH_P_IP))
+	if (vlan_get_protocol(skb) != htons(ETH_P_IP))
 		return;
 
 	if (skb->ip_summed == CHECKSUM_PARTIAL)
-- 
1.9.3

^ permalink raw reply related

* [PATCH 2/8] e1000: Fix TSO for non-accelerated vlan traffic
From: vyasevich @ 2014-08-23  2:17 UTC (permalink / raw)
  To: netdev
  Cc: Vladislav Yasevich, Jeff Kirsher, Jesse Brandeburg, Bruce Allan,
	Carolyn Wyborny, Don Skidmore, Greg Rose, Alex Duyck,
	John Ronciak, Mitch Williams, Linux NICS, e1000-devel
In-Reply-To: <1408760230-7457-1-git-send-email-vysevich@gmail.com>

From: Vladislav Yasevich <vyasevic@redhat.com>

This device claims TSO and checksum support for vlans.  It also
allows a user to control vlan acceleration offloading.  As such,
it is possible to turn off vlan acceleration and configure a vlan
which will continue to support TSO.

In such situation the packet passed down the the device will contain
a vlan header and skb->protocol will be set to ETH_P_8021Q.
The device assumes that skb->protocol contains network protocol
value and uses that value to set up TSO and checksum information.
This will results in corrupted frames sent on the wire.

This patch extract the protocol value correctly and corrects TSO
for non-accelerated traffic.

CC: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
CC: Jesse Brandeburg <jesse.brandeburg@intel.com>
CC: Bruce Allan <bruce.w.allan@intel.com>
CC: Carolyn Wyborny <carolyn.wyborny@intel.com>
CC: Don Skidmore <donald.c.skidmore@intel.com>
CC: Greg Rose <gregory.v.rose@intel.com>
CC: Alex Duyck <alexander.h.duyck@intel.com>
CC: John Ronciak <john.ronciak@intel.com>
CC: Mitch Williams <mitch.a.williams@intel.com>
CC: Linux NICS <linux.nics@intel.com>
CC: e1000-devel@lists.sourceforge.net
Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
 drivers/net/ethernet/intel/e1000/e1000_main.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
index cbc330b..ad3d5d1 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_main.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
@@ -2674,7 +2674,8 @@ set_itr_now:
 #define E1000_TX_FLAGS_VLAN_SHIFT	16
 
 static int e1000_tso(struct e1000_adapter *adapter,
-		     struct e1000_tx_ring *tx_ring, struct sk_buff *skb)
+		     struct e1000_tx_ring *tx_ring, struct sk_buff *skb,
+		     __be16 protocol)
 {
 	struct e1000_context_desc *context_desc;
 	struct e1000_buffer *buffer_info;
@@ -2692,7 +2693,7 @@ static int e1000_tso(struct e1000_adapter *adapter,
 
 		hdr_len = skb_transport_offset(skb) + tcp_hdrlen(skb);
 		mss = skb_shinfo(skb)->gso_size;
-		if (skb->protocol == htons(ETH_P_IP)) {
+		if (protocol == htons(ETH_P_IP)) {
 			struct iphdr *iph = ip_hdr(skb);
 			iph->tot_len = 0;
 			iph->check = 0;
@@ -2702,7 +2703,7 @@ static int e1000_tso(struct e1000_adapter *adapter,
 								 0);
 			cmd_length = E1000_TXD_CMD_IP;
 			ipcse = skb_transport_offset(skb) - 1;
-		} else if (skb->protocol == htons(ETH_P_IPV6)) {
+		} else if (skb_is_gso_v6(skb)) {
 			ipv6_hdr(skb)->payload_len = 0;
 			tcp_hdr(skb)->check =
 				~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
@@ -2745,7 +2746,8 @@ static int e1000_tso(struct e1000_adapter *adapter,
 }
 
 static bool e1000_tx_csum(struct e1000_adapter *adapter,
-			  struct e1000_tx_ring *tx_ring, struct sk_buff *skb)
+			  struct e1000_tx_ring *tx_ring, struct sk_buff *skb,
+			  __be16 protocol)
 {
 	struct e1000_context_desc *context_desc;
 	struct e1000_buffer *buffer_info;
@@ -2756,7 +2758,7 @@ static bool e1000_tx_csum(struct e1000_adapter *adapter,
 	if (skb->ip_summed != CHECKSUM_PARTIAL)
 		return false;
 
-	switch (skb->protocol) {
+	switch (protocol) {
 	case cpu_to_be16(ETH_P_IP):
 		if (ip_hdr(skb)->protocol == IPPROTO_TCP)
 			cmd_len |= E1000_TXD_CMD_TCP;
@@ -3097,6 +3099,7 @@ static netdev_tx_t e1000_xmit_frame(struct sk_buff *skb,
 	int count = 0;
 	int tso;
 	unsigned int f;
+	__be16 protocol = vlan_get_protocol(skb);
 
 	/* This goes back to the question of how to logically map a Tx queue
 	 * to a flow.  Right now, performance is impacted slightly negatively
@@ -3210,7 +3213,7 @@ static netdev_tx_t e1000_xmit_frame(struct sk_buff *skb,
 
 	first = tx_ring->next_to_use;
 
-	tso = e1000_tso(adapter, tx_ring, skb);
+	tso = e1000_tso(adapter, tx_ring, skb, protocol);
 	if (tso < 0) {
 		dev_kfree_skb_any(skb);
 		return NETDEV_TX_OK;
@@ -3220,10 +3223,10 @@ static netdev_tx_t e1000_xmit_frame(struct sk_buff *skb,
 		if (likely(hw->mac_type != e1000_82544))
 			tx_ring->last_tx_tso = true;
 		tx_flags |= E1000_TX_FLAGS_TSO;
-	} else if (likely(e1000_tx_csum(adapter, tx_ring, skb)))
+	} else if (likely(e1000_tx_csum(adapter, tx_ring, skb, protocol)))
 		tx_flags |= E1000_TX_FLAGS_CSUM;
 
-	if (likely(skb->protocol == htons(ETH_P_IP)))
+	if (protocol == htons(ETH_P_IP))
 		tx_flags |= E1000_TX_FLAGS_IPV4;
 
 	if (unlikely(skb->no_fcs))
-- 
1.9.3

^ permalink raw reply related

* [PATCH 5/8] i40e: Fix TSO and hw checksums for non-accelerated vlan packets.
From: vyasevich @ 2014-08-23  2:17 UTC (permalink / raw)
  To: netdev
  Cc: Vladislav Yasevich, Jeff Kirsher, Jesse Brandeburg, Bruce Allan,
	Carolyn Wyborny, Don Skidmore, Greg Rose, Alex Duyck,
	John Ronciak, Mitch Williams, Linux NICS, e1000-devel
In-Reply-To: <1408760230-7457-1-git-send-email-vysevich@gmail.com>

From: Vladislav Yasevich <vyasevic@redhat.com>

This device claims TSO and checksum support for vlans.  It also
allows a user to control vlan acceleration offloading.  As such,
it is possible to turn off vlan acceleration and configure a vlan
which will continue to support TSO and hw checksums.

In such situation the packet passed down the the device will contain
a vlan header and skb->protocol will be set to ETH_P_8021Q.
The device assumes that skb->protocol contains network protocol
value and uses that value to set up TSO and checksum information.
This results in corrupted frames sent on the wire.

This patch extract the protocol value correctly and corrects TSO
and checksums for non-accelerated traffic.

Fix this by using vlan_get_protocol() helper.

CC: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
CC: Jesse Brandeburg <jesse.brandeburg@intel.com>
CC: Bruce Allan <bruce.w.allan@intel.com>
CC: Carolyn Wyborny <carolyn.wyborny@intel.com>
CC: Don Skidmore <donald.c.skidmore@intel.com>
CC: Greg Rose <gregory.v.rose@intel.com>
CC: Alex Duyck <alexander.h.duyck@intel.com>
CC: John Ronciak <john.ronciak@intel.com>
CC: Mitch Williams <mitch.a.williams@intel.com>
CC: Linux NICS <linux.nics@intel.com>
CC: e1000-devel@lists.sourceforge.net
Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index a51aa37..3247057 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -2295,7 +2295,7 @@ static netdev_tx_t i40e_xmit_frame_ring(struct sk_buff *skb,
 		goto out_drop;
 
 	/* obtain protocol of skb */
-	protocol = skb->protocol;
+	protocol = get_vlan_protocol(skb);
 
 	/* record the location of the first descriptor for this packet */
 	first = &tx_ring->tx_bi[tx_ring->next_to_use];
-- 
1.9.3

^ permalink raw reply related

* [PATCH 6/8] i40evf: Fix TSO and hw checksums for non-accelerated vlan packets.
From: vyasevich @ 2014-08-23  2:17 UTC (permalink / raw)
  To: netdev
  Cc: Vladislav Yasevich, Jeff Kirsher, Jesse Brandeburg, Bruce Allan,
	Carolyn Wyborny, Don Skidmore, Greg Rose, Alex Duyck,
	John Ronciak, Mitch Williams, Linux NICS, e1000-devel
In-Reply-To: <1408760230-7457-1-git-send-email-vysevich@gmail.com>

From: Vladislav Yasevich <vyasevic@redhat.com>

This device claims TSO and checksum support for vlans.  It also
allows a user to control vlan acceleration offloading.  As such,
it is possible to turn off vlan acceleration and configure a vlan
which will continue to support TSO and hw checksums.

In such situation the packet passed down the the device will contain
a vlan header and skb->protocol will be set to ETH_P_8021Q.
The device assumes that skb->protocol contains network protocol
value and uses that value to set up TSO and checksum information.
This results in corrupted frames sent on the wire.

This patch extract the protocol value correctly and corrects TSO
and checksums for non-accelerated traffic.

Fix this by using vlan_get_protocol() helper.

CC: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
CC: Jesse Brandeburg <jesse.brandeburg@intel.com>
CC: Bruce Allan <bruce.w.allan@intel.com>
CC: Carolyn Wyborny <carolyn.wyborny@intel.com>
CC: Don Skidmore <donald.c.skidmore@intel.com>
CC: Greg Rose <gregory.v.rose@intel.com>
CC: Alex Duyck <alexander.h.duyck@intel.com>
CC: John Ronciak <john.ronciak@intel.com>
CC: Mitch Williams <mitch.a.williams@intel.com>
CC: Linux NICS <linux.nics@intel.com>
CC: e1000-devel@lists.sourceforge.net
Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
 drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
index 79bf96c..95a3ec2 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
@@ -1597,7 +1597,7 @@ static netdev_tx_t i40e_xmit_frame_ring(struct sk_buff *skb,
 		goto out_drop;
 
 	/* obtain protocol of skb */
-	protocol = skb->protocol;
+	protocol = vlan_get_protocol(skb);
 
 	/* record the location of the first descriptor for this packet */
 	first = &tx_ring->tx_bi[tx_ring->next_to_use];
-- 
1.9.3

^ permalink raw reply related

* [PATCH 7/8] mvneta: Fix TSO and checksum for non-acceleration vlan traffic
From: vyasevich @ 2014-08-23  2:17 UTC (permalink / raw)
  To: netdev; +Cc: Vladislav Yasevich, Thomas Petazzoni
In-Reply-To: <1408760230-7457-1-git-send-email-vysevich@gmail.com>

From: Vladislav Yasevich <vyasevic@redhat.com>

This driver doesn't appear to support vlan acceleration at
all.  However, it does claim to support TSO and IP checksums
for vlan devices.  Thus any configured vlan device would
end up passing down partial checksums or TSO frames.

The driver also uses the value from skb->protocol to
determine TSO and checksum offload information, but assumes
that skb->protocol holds the l3 protocol information.
As a result, vlan traffic with partial checksums or TSO
will fail those checks and TSO will not happen.

Fix this by using vlan_get_protocol() helper.

CC: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
 drivers/net/ethernet/marvell/mvneta.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index c9f1d1b..133f8c6 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -1371,15 +1371,16 @@ static u32 mvneta_skb_tx_csum(struct mvneta_port *pp, struct sk_buff *skb)
 {
 	if (skb->ip_summed == CHECKSUM_PARTIAL) {
 		int ip_hdr_len = 0;
+		__be16 l3_proto = vlan_get_protocol(skb);
 		u8 l4_proto;
 
-		if (skb->protocol == htons(ETH_P_IP)) {
+		if (l3_proto == htons(ETH_P_IP)) {
 			struct iphdr *ip4h = ip_hdr(skb);
 
 			/* Calculate IPv4 checksum and L4 checksum */
 			ip_hdr_len = ip4h->ihl;
 			l4_proto = ip4h->protocol;
-		} else if (skb->protocol == htons(ETH_P_IPV6)) {
+		} else if (l3_proto == htons(ETH_P_IPV6)) {
 			struct ipv6hdr *ip6h = ipv6_hdr(skb);
 
 			/* Read l4_protocol from one of IPv6 extra headers */
@@ -1390,7 +1391,7 @@ static u32 mvneta_skb_tx_csum(struct mvneta_port *pp, struct sk_buff *skb)
 			return MVNETA_TX_L4_CSUM_NOT;
 
 		return mvneta_txq_desc_csum(skb_network_offset(skb),
-				skb->protocol, ip_hdr_len, l4_proto);
+					    l3_proto, ip_hdr_len, l4_proto);
 	}
 
 	return MVNETA_TX_L4_CSUM_NOT;
-- 
1.9.3

^ permalink raw reply related

* [PATCH 8/8] qlge: Fix TSO for non-accelerated vlan traffic
From: vyasevich @ 2014-08-23  2:17 UTC (permalink / raw)
  To: netdev
  Cc: Vladislav Yasevich, Shahed Shaikh, Jitendra Kalsaria, Ron Mercer,
	linux-driver
In-Reply-To: <1408760230-7457-1-git-send-email-vysevich@gmail.com>

From: Vladislav Yasevich <vyasevic@redhat.com>

This device claims TSO support for vlans.  It also allows a user to
control vlan acceleration offloading.  As such, it is possible to turn
off vlan acceleration and configure a vlan which will continue to send
TSO traffic.

In such situation the packet passed down the the device will contain
a vlan header and skb->protocol will be set to ETH_P_8021Q.
The device assumes that skb->protocol contains network protocol
value and uses that value to set up TSO information.
This results in corrupted frames sent on the wire.

This patch extracts the protocol value correctly by using a
vlan_get_protocol() helper and corrects corrupt TSO frames.

CC: Shahed Shaikh <shahed.shaikh@qlogic.com>
CC: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
CC: Ron Mercer <ron.mercer@qlogic.com>
CC: linux-driver@qlogic.com
Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
 drivers/net/ethernet/qlogic/qlge/qlge_main.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlge/qlge_main.c b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
index 188626e..3e96f26 100644
--- a/drivers/net/ethernet/qlogic/qlge/qlge_main.c
+++ b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
@@ -2556,6 +2556,7 @@ static int ql_tso(struct sk_buff *skb, struct ob_mac_tso_iocb_req *mac_iocb_ptr)
 
 	if (skb_is_gso(skb)) {
 		int err;
+		__be16 l3_proto = vlan_get_protocol(skb);
 
 		err = skb_cow_head(skb, 0);
 		if (err < 0)
@@ -2572,7 +2573,7 @@ static int ql_tso(struct sk_buff *skb, struct ob_mac_tso_iocb_req *mac_iocb_ptr)
 				<< OB_MAC_TRANSPORT_HDR_SHIFT);
 		mac_iocb_ptr->mss = cpu_to_le16(skb_shinfo(skb)->gso_size);
 		mac_iocb_ptr->flags2 |= OB_MAC_TSO_IOCB_LSO;
-		if (likely(skb->protocol == htons(ETH_P_IP))) {
+		if (likely(l3_proto == htons(ETH_P_IP))) {
 			struct iphdr *iph = ip_hdr(skb);
 			iph->check = 0;
 			mac_iocb_ptr->flags1 |= OB_MAC_TSO_IOCB_IP4;
@@ -2580,7 +2581,7 @@ static int ql_tso(struct sk_buff *skb, struct ob_mac_tso_iocb_req *mac_iocb_ptr)
 								 iph->daddr, 0,
 								 IPPROTO_TCP,
 								 0);
-		} else if (skb->protocol == htons(ETH_P_IPV6)) {
+		} else if (l3_proto == htons(ETH_P_IPV6)) {
 			mac_iocb_ptr->flags1 |= OB_MAC_TSO_IOCB_IP6;
 			tcp_hdr(skb)->check =
 			    ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
-- 
1.9.3

^ 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