Netdev List
 help / color / mirror / Atom feed
* [next-next PATCH 4/7] fib_trie: Add collapse() and should_collapse() to resize
From: Alexander Duyck @ 2015-01-22 23:51 UTC (permalink / raw)
  To: netdev; +Cc: davem
In-Reply-To: <20150122234652.5779.44251.stgit@ahduyck-vm-fedora20>

This patch really does two things.

First it pulls the logic for determining if we should collapse one node out
of the tree and the actual code doing the collapse into a separate pair of
functions.  This helps to make the changes to these areas more readable.

Second it encodes the upper 32b of the empty_children value onto the
full_children value in the case of bits == KEYLENGTH.  By doing this we are
able to handle the case of a 32b node where empty_children would appear to
be 0 when it was actually 1ul << 32.

Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
 net/ipv4/fib_trie.c |  100 +++++++++++++++++++++++++++++++++------------------
 1 file changed, 65 insertions(+), 35 deletions(-)

diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index 80892f5..f874e18 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -83,7 +83,8 @@
 
 #define MAX_STAT_DEPTH 32
 
-#define KEYLENGTH (8*sizeof(t_key))
+#define KEYLENGTH	(8*sizeof(t_key))
+#define KEY_MAX		((t_key)~0)
 
 typedef unsigned int t_key;
 
@@ -102,8 +103,8 @@ struct tnode {
 	union {
 		/* The fields in this struct are valid if bits > 0 (TNODE) */
 		struct {
-			unsigned int full_children;  /* KEYLENGTH bits needed */
-			unsigned int empty_children; /* KEYLENGTH bits needed */
+			t_key empty_children; /* KEYLENGTH bits needed */
+			t_key full_children;  /* KEYLENGTH bits needed */
 			struct tnode __rcu *child[0];
 		};
 		/* This list pointer if valid if bits == 0 (LEAF) */
@@ -302,6 +303,16 @@ static struct tnode *tnode_alloc(size_t size)
 		return vzalloc(size);
 }
 
+static inline void empty_child_inc(struct tnode *n)
+{
+	++n->empty_children ? : ++n->full_children;
+}
+
+static inline void empty_child_dec(struct tnode *n)
+{
+	n->empty_children-- ? : n->full_children--;
+}
+
 static struct tnode *leaf_new(t_key key)
 {
 	struct tnode *l = kmem_cache_alloc(trie_leaf_kmem, GFP_KERNEL);
@@ -335,7 +346,7 @@ static struct leaf_info *leaf_info_new(int plen)
 
 static struct tnode *tnode_new(t_key key, int pos, int bits)
 {
-	size_t sz = offsetof(struct tnode, child[1 << bits]);
+	size_t sz = offsetof(struct tnode, child[1ul << bits]);
 	struct tnode *tn = tnode_alloc(sz);
 	unsigned int shift = pos + bits;
 
@@ -348,8 +359,10 @@ static struct tnode *tnode_new(t_key key, int pos, int bits)
 		tn->pos = pos;
 		tn->bits = bits;
 		tn->key = (shift < KEYLENGTH) ? (key >> shift) << shift : 0;
-		tn->full_children = 0;
-		tn->empty_children = 1<<bits;
+		if (bits == KEYLENGTH)
+			tn->full_children = 1;
+		else
+			tn->empty_children = 1ul << bits;
 	}
 
 	pr_debug("AT %p s=%zu %zu\n", tn, sizeof(struct tnode),
@@ -375,11 +388,11 @@ static void put_child(struct tnode *tn, unsigned long i, struct tnode *n)
 
 	BUG_ON(i >= tnode_child_length(tn));
 
-	/* update emptyChildren */
+	/* update emptyChildren, overflow into fullChildren */
 	if (n == NULL && chi != NULL)
-		tn->empty_children++;
-	else if (n != NULL && chi == NULL)
-		tn->empty_children--;
+		empty_child_inc(tn);
+	if (n != NULL && chi == NULL)
+		empty_child_dec(tn);
 
 	/* update fullChildren */
 	wasfull = tnode_full(tn, chi);
@@ -630,6 +643,24 @@ static int halve(struct trie *t, struct tnode *oldtnode)
 	return 0;
 }
 
+static void collapse(struct trie *t, struct tnode *oldtnode)
+{
+	struct tnode *n, *tp;
+	unsigned long i;
+
+	/* scan the tnode looking for that one child that might still exist */
+	for (n = NULL, i = tnode_child_length(oldtnode); !n && i;)
+		n = tnode_get_child(oldtnode, --i);
+
+	/* compress one level */
+	tp = node_parent(oldtnode);
+	put_child_root(tp, t, oldtnode->key, n);
+	node_set_parent(n, tp);
+
+	/* drop dead node */
+	node_free(oldtnode);
+}
+
 static unsigned char update_suffix(struct tnode *tn)
 {
 	unsigned char slen = tn->pos;
@@ -729,10 +760,12 @@ static bool should_inflate(const struct tnode *tp, const struct tnode *tn)
 
 	/* Keep root node larger */
 	threshold *= tp ? inflate_threshold : inflate_threshold_root;
-	used += tn->full_children;
 	used -= tn->empty_children;
+	used += tn->full_children;
 
-	return tn->pos && ((50 * used) >= threshold);
+	/* if bits == KEYLENGTH then pos = 0, and will fail below */
+
+	return (used > 1) && tn->pos && ((50 * used) >= threshold);
 }
 
 static bool should_halve(const struct tnode *tp, const struct tnode *tn)
@@ -744,13 +777,29 @@ static bool should_halve(const struct tnode *tp, const struct tnode *tn)
 	threshold *= tp ? halve_threshold : halve_threshold_root;
 	used -= tn->empty_children;
 
-	return (tn->bits > 1) && ((100 * used) < threshold);
+	/* if bits == KEYLENGTH then used = 100% on wrap, and will fail below */
+
+	return (used > 1) && (tn->bits > 1) && ((100 * used) < threshold);
+}
+
+static bool should_collapse(const struct tnode *tn)
+{
+	unsigned long used = tnode_child_length(tn);
+
+	used -= tn->empty_children;
+
+	/* account for bits == KEYLENGTH case */
+	if ((tn->bits == KEYLENGTH) && tn->full_children)
+		used -= KEY_MAX;
+
+	/* One child or none, time to drop us from the trie */
+	return used < 2;
 }
 
 #define MAX_WORK 10
 static void resize(struct trie *t, struct tnode *tn)
 {
-	struct tnode *tp = node_parent(tn), *n = NULL;
+	struct tnode *tp = node_parent(tn);
 	struct tnode __rcu **cptr;
 	int max_work = MAX_WORK;
 
@@ -764,14 +813,6 @@ static void resize(struct trie *t, struct tnode *tn)
 	cptr = tp ? &tp->child[get_index(tn->key, tp)] : &t->trie;
 	BUG_ON(tn != rtnl_dereference(*cptr));
 
-	/* No children */
-	if (tn->empty_children > (tnode_child_length(tn) - 1))
-		goto no_children;
-
-	/* One child */
-	if (tn->empty_children == (tnode_child_length(tn) - 1))
-		goto one_child;
-
 	/* Double as long as the resulting node has a number of
 	 * nonempty nodes that are above the threshold.
 	 */
@@ -807,19 +848,8 @@ static void resize(struct trie *t, struct tnode *tn)
 	}
 
 	/* Only one child remains */
-	if (tn->empty_children == (tnode_child_length(tn) - 1)) {
-		unsigned long i;
-one_child:
-		for (i = tnode_child_length(tn); !n && i;)
-			n = tnode_get_child(tn, --i);
-no_children:
-		/* compress one level */
-		put_child_root(tp, t, tn->key, n);
-		node_set_parent(n, tp);
-
-		/* drop dead node */
-		tnode_free_init(tn);
-		tnode_free(tn);
+	if (should_collapse(tn)) {
+		collapse(t, tn);
 		return;
 	}
 

^ permalink raw reply related

* [next-next PATCH 5/7] fib_trie: Use empty_children instead of counting empty nodes in stats collection
From: Alexander Duyck @ 2015-01-22 23:51 UTC (permalink / raw)
  To: netdev; +Cc: davem
In-Reply-To: <20150122234652.5779.44251.stgit@ahduyck-vm-fedora20>

It doesn't make much sense to count the pointers ourselves when
empty_children already has a count for the number of NULL pointers stored
in the tnode.  As such save ourselves the cycles and just use
empty_children.

Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
 net/ipv4/fib_trie.c |    8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index f874e18..90654bb 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -1954,16 +1954,10 @@ static void trie_collect_stats(struct trie *t, struct trie_stat *s)
 			hlist_for_each_entry_rcu(li, &n->list, hlist)
 				++s->prefixes;
 		} else {
-			unsigned long i;
-
 			s->tnodes++;
 			if (n->bits < MAX_STAT_DEPTH)
 				s->nodesizes[n->bits]++;
-
-			for (i = tnode_child_length(n); i--;) {
-				if (!rcu_access_pointer(n->child[i]))
-					s->nullpointers++;
-			}
+			s->nullpointers += n->empty_children;
 		}
 	}
 	rcu_read_unlock();

^ permalink raw reply related

* [next-next PATCH 6/7] fib_trie: Move fib_find_alias to file where it is used
From: Alexander Duyck @ 2015-01-22 23:51 UTC (permalink / raw)
  To: netdev; +Cc: davem
In-Reply-To: <20150122234652.5779.44251.stgit@ahduyck-vm-fedora20>

The function fib_find_alias is only accessed by functions in fib_trie.c as
such it makes sense to relocate it and cast it as static so that the
compiler can take advantage of optimizations it can do to it as a local
function.

Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
 net/ipv4/fib_lookup.h    |    1 -
 net/ipv4/fib_semantics.c |   18 ------------------
 net/ipv4/fib_trie.c      |   20 ++++++++++++++++++++
 3 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/net/ipv4/fib_lookup.h b/net/ipv4/fib_lookup.h
index 1e4f660..825981b 100644
--- a/net/ipv4/fib_lookup.h
+++ b/net/ipv4/fib_lookup.h
@@ -32,7 +32,6 @@ int fib_dump_info(struct sk_buff *skb, u32 pid, u32 seq, int event, u32 tb_id,
 		  unsigned int);
 void rtmsg_fib(int event, __be32 key, struct fib_alias *fa, int dst_len,
 	       u32 tb_id, const struct nl_info *info, unsigned int nlm_flags);
-struct fib_alias *fib_find_alias(struct list_head *fah, u8 tos, u32 prio);
 
 static inline void fib_result_assign(struct fib_result *res,
 				     struct fib_info *fi)
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index 265cb72..1e2090e 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -411,24 +411,6 @@ errout:
 		rtnl_set_sk_err(info->nl_net, RTNLGRP_IPV4_ROUTE, err);
 }
 
-/* Return the first fib alias matching TOS with
- * priority less than or equal to PRIO.
- */
-struct fib_alias *fib_find_alias(struct list_head *fah, u8 tos, u32 prio)
-{
-	if (fah) {
-		struct fib_alias *fa;
-		list_for_each_entry(fa, fah, fa_list) {
-			if (fa->fa_tos > tos)
-				continue;
-			if (fa->fa_info->fib_priority >= prio ||
-			    fa->fa_tos < tos)
-				return fa;
-		}
-	}
-	return NULL;
-}
-
 static int fib_detect_death(struct fib_info *fi, int order,
 			    struct fib_info **last_resort, int *last_idx,
 			    int dflt)
diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index 90654bb..7f34226 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -998,6 +998,26 @@ static struct tnode *fib_find_node(struct trie *t, u32 key)
 	return n;
 }
 
+/* Return the first fib alias matching TOS with
+ * priority less than or equal to PRIO.
+ */
+static struct fib_alias *fib_find_alias(struct list_head *fah, u8 tos, u32 prio)
+{
+	struct fib_alias *fa;
+
+	if (!fah)
+		return NULL;
+
+	list_for_each_entry(fa, fah, fa_list) {
+		if (fa->fa_tos > tos)
+			continue;
+		if (fa->fa_info->fib_priority >= prio || fa->fa_tos < tos)
+			return fa;
+	}
+
+	return NULL;
+}
+
 static void trie_rebalance(struct trie *t, struct tnode *tn)
 {
 	struct tnode *tp;

^ permalink raw reply related

* [next-next PATCH 7/7] fib_trie: Various clean-ups for handling slen
From: Alexander Duyck @ 2015-01-22 23:51 UTC (permalink / raw)
  To: netdev; +Cc: davem
In-Reply-To: <20150122234652.5779.44251.stgit@ahduyck-vm-fedora20>

While doing further work on the fib_trie I noted a few items.

First I was using calls that were far more complicated than they needed to
be for determining when to push/pull the suffix length.  I have updated the
code to reflect the simplier logic.

The second issue is that I realised we weren't necessarily handling the
case of a leaf_info struct surviving a flush.  I have updated the logic so
that now we will call pull_suffix in the event of having a leaf info value
left in the leaf after flushing it.

Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
 net/ipv4/fib_trie.c |   49 +++++++++++++++++++++++++++++--------------------
 1 file changed, 29 insertions(+), 20 deletions(-)

diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index 7f34226..3daf022 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -917,27 +917,20 @@ static void leaf_push_suffix(struct tnode *l)
 
 static void remove_leaf_info(struct tnode *l, struct leaf_info *old)
 {
-	struct hlist_node *prev;
-
-	/* record the location of the pointer to this object */
-	prev = rtnl_dereference(hlist_pprev_rcu(&old->hlist));
+	/* record the location of the previous list_info entry */
+	struct hlist_node **pprev = old->hlist.pprev;
+	struct leaf_info *li = hlist_entry(pprev, typeof(*li), hlist.next);
 
 	/* remove the leaf info from the list */
 	hlist_del_rcu(&old->hlist);
 
-	/* if we emptied the list this leaf will be freed and we can sort
-	 * out parent suffix lengths as a part of trie_rebalance
-	 */
-	if (hlist_empty(&l->list))
+	/* only access li if it is pointing at the last valid hlist_node */
+	if (hlist_empty(&l->list) || (*pprev))
 		return;
 
-	/* if we removed the tail then we need to update slen */
-	if (!rcu_access_pointer(hlist_next_rcu(prev))) {
-		struct leaf_info *li = hlist_entry(prev, typeof(*li), hlist);
-
-		l->slen = KEYLENGTH - li->plen;
-		leaf_pull_suffix(l);
-	}
+	/* update the trie with the latest suffix length */
+	l->slen = KEYLENGTH - li->plen;
+	leaf_pull_suffix(l);
 }
 
 static void insert_leaf_info(struct tnode *l, struct leaf_info *new)
@@ -961,7 +954,7 @@ static void insert_leaf_info(struct tnode *l, struct leaf_info *new)
 	}
 
 	/* if we added to the tail node then we need to update slen */
-	if (!rcu_access_pointer(hlist_next_rcu(&new->hlist))) {
+	if (l->slen < (KEYLENGTH - new->plen)) {
 		l->slen = KEYLENGTH - new->plen;
 		leaf_push_suffix(l);
 	}
@@ -1613,6 +1606,7 @@ static int trie_flush_leaf(struct tnode *l)
 	struct hlist_head *lih = &l->list;
 	struct hlist_node *tmp;
 	struct leaf_info *li = NULL;
+	unsigned char plen = KEYLENGTH;
 
 	hlist_for_each_entry_safe(li, tmp, lih, hlist) {
 		found += trie_flush_list(&li->falh);
@@ -1620,8 +1614,14 @@ static int trie_flush_leaf(struct tnode *l)
 		if (list_empty(&li->falh)) {
 			hlist_del_rcu(&li->hlist);
 			free_leaf_info(li);
+			continue;
 		}
+
+		plen = li->plen;
 	}
+
+	l->slen = KEYLENGTH - plen;
+
 	return found;
 }
 
@@ -1700,13 +1700,22 @@ int fib_table_flush(struct fib_table *tb)
 	for (l = trie_firstleaf(t); l; l = trie_nextleaf(l)) {
 		found += trie_flush_leaf(l);
 
-		if (ll && hlist_empty(&ll->list))
-			trie_leaf_remove(t, ll);
+		if (ll) {
+			if (hlist_empty(&ll->list))
+				trie_leaf_remove(t, ll);
+			else
+				leaf_pull_suffix(ll);
+		}
+
 		ll = l;
 	}
 
-	if (ll && hlist_empty(&ll->list))
-		trie_leaf_remove(t, ll);
+	if (ll) {
+		if (hlist_empty(&ll->list))
+			trie_leaf_remove(t, ll);
+		else
+			leaf_pull_suffix(ll);
+	}
 
 	pr_debug("trie_flush found=%d\n", found);
 	return found;

^ permalink raw reply related

* Re: subtle change in behavior with tun driver
From: Ani Sinha @ 2015-01-23  0:28 UTC (permalink / raw)
  To: Michael S. Tsirkin, fruggeri; +Cc: netdev@vger.kernel.org
In-Reply-To: <20150122095349.GC26178@redhat.com>

On Thu, Jan 22, 2015 at 1:53 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Wed, Jan 21, 2015 at 02:36:17PM -0800, Ani Sinha wrote:
>> Hi guys :
>>
>> Commit 5d097109257c03 ("tun: only queue packets on device") seems to
>> have introduced a subtle change in behavior in the tun driver in the
>> default (non IFF_ONE_QUEUE) case. Previously when the queues got full
>> and eventually sk_wmem_alloc of the socket exceeded sk_sndbuf value,
>> the user would be given a feedback by returning EAGAIN from sendto()
>> etc. That way, the user could retry sending the packet again.
>
> This behaviour is common, but by no means guaranteed.
> For example, if socket buffer size is large enough,
> packets are small enough, or there are multiple sockets
> transmitting through tun, packets would previously
> accumulate in qdisc, followed by packet drops
> without EAGAIN.

Ah I see. pfifo_fast_enqueue() also starts dropping packets when it's
length exceeds a threshold. So I supposed we do not have a strong
argument for bringing back the old semantics because it wasn't
guranteed in every scenario in the first place.

>
>> Unfortunately, with this new  default single queue mode, the driver
>> silently drops the packet when the device queue is full without giving
>> userland any feedback. This makes it appear to userland as though the
>> packet was transmitted successfully. It seems there is a semantic
>> change in the driver with this commit.
>>
>> If the receiving process gets stuck for a short interval and is unable
>> to drain packets and then restarts again, one might see strange packet
>> drops in the kernel without getting any error back on the sender's
>> side. It kind of feels wrong.
>>
>> Any thoughts?
>>
>> Ani
>
> Unfortunately - since it's pretty common for unpriveledged userspace to
> drive the tun device - blocking the queue indefinitely as was done
> previously leads to deadlocks for some apps, this was deemed worse than
> some performance degradation.

agreed. However applications which needs protection from deadlock can
exclusively set IFF_ONE_QUEUE mode and live with the fact that they
might see dropped packets.

>
> As a simple work-around, if you want packets to accumulate in the qdisc,
> it's easy to implement by using a non work conserving qdisc.
> Set the limits to match the speed at which your application
> is able to consume the packets.
>
> I've been thinking about using some kind of watchdog to
> make it safe to put the old non IFF_ONE_QUEUE semantics back,
> unfortunately due to application being able to consume packets at the
> same time it's not trivial to do in a non-racy way.

I do not have an answer to this issue either. I agree that this is a
hard issue to solve.

^ permalink raw reply

* Re: [PATCH 1/2] if_link: Add VF multicast promiscuous mode control
From: Hiroshi Shimamoto @ 2015-01-23  0:32 UTC (permalink / raw)
  To: Skidmore, Donald C, Bjørn Mork
  Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
	Choi, Sy Jong, linux-kernel@vger.kernel.org, David Laight,
	Hayato Momma
In-Reply-To: <F6FB0E698C9B3143BDF729DF222866469129C09C@ORSMSX110.amr.corp.intel.com>

> Subject: RE: [E1000-devel] [PATCH 1/2] if_link: Add VF multicast promiscuous mode control
> >
> > "Skidmore, Donald C" <donald.c.skidmore@intel.com> writes:
> >
> > > My hang up is more related to: without the nob to enable it (off by
> > > default) we are letting one VF dictate policy for all the other VFs
> > > and the PF.  If one VF needs to be in promiscuous multicast so is
> > > everyone else.  Their stacks now needs to deal with all the extra
> > > multicast packets.  As you point out this might not be a direct
> > > concern for isolation in that the VM could have 'chosen' to join any
> > > Multicast group and seen this traffic.  My concern over isolation is
> > > one VF has chosen that all the other VM now have to see this multicast
> > > traffic.
> >
> > Apologies if this question is stupid, but I just have to ask about stuff I don't
> > understand...
> >
> > Looking at the proposed implementation, the promiscous multicast flag
> > seems to be a per-VF flag:
> >
> > +int ixgbe_ndo_set_vf_mc_promisc(struct net_device *netdev, int vf, bool
> > +setting) {
> > +	struct ixgbe_adapter *adapter = netdev_priv(netdev);
> > +	struct ixgbe_hw *hw = &adapter->hw;
> > +	u32 vmolr;
> > +
> > +	if (vf >= adapter->num_vfs)
> > +		return -EINVAL;
> > +
> > +	adapter->vfinfo[vf].mc_promisc_enabled = setting;
> > +
> > +	vmolr = IXGBE_READ_REG(hw, IXGBE_VMOLR(vf));
> > +	if (setting) {
> > +		e_info(drv, "VF %u: enabling multicast promiscuous\n", vf);
> > +		vmolr |= IXGBE_VMOLR_MPE;
> > +	} else {
> > +		e_info(drv, "VF %u: disabling multicast promiscuous\n", vf);
> > +		vmolr &= ~IXGBE_VMOLR_MPE;
> > +	}
> > +
> > +	IXGBE_WRITE_REG(hw, IXGBE_VMOLR(vf), vmolr);
> > +
> > +	return 0;
> > +}
> > +
> >
> > I haven't read the data sheet, but I took a quick look at the excellent high
> > level driver docs:
> > http://www.intel.com/content/dam/doc/design-guide/82599-sr-iov-driver-
> > companion-guide.pdf
> >
> > It mentions "Multicast Promiscuous Enable" in its "Thoughts for
> > Customization" section:
> >
> >  7.1 Multicast Promiscuous Enable
> >
> >  The controller has provisions to allow each VF to be put into Multicast
> > Promiscuous mode.  The Intel reference driver does not configure this
> > option .
> >
> >  The capability can be enabled/disabled by manipulating the MPE field  (bit
> > 28) of the PF VF L2 Control Register (PFVML2FLT – 0x0F000)
> >
> > and showing a section from the data sheet describing the "PF VM L2 Control
> > Register - PFVML2FLT[n]  (0x0F000 + 4 * n, n=0...63; RW)"
> >
> > To me it looks like enabling Promiscuos Multicast for a VF won't affect any
> > other VF at all.  Is this really not the case?
> >
> >
> >
> > Bjørn
> 
> Clearly not a dumb question at all and I'm glad you mentioned that. :)  I was going off the assumption, been awhile since
> I read the patch, that the patch was using FCTRL.MPE or MANC.MCST_PASS_L2 which would turn multicast promiscuous on for
> everyone.  Since the patch is using PFVML2FLT.MPE this lessens my concern over effect on the entire system.

I believe the patches for this VF multicast promiscuous mode is per VF.

> 
> That said I still would prefer having a way to override this behavior on the PF, although I admit my argument is weaker.
> I'm still concerned about a VF changing the behavior of the PF without any way to prevent it.  This might be one part
> philosophical (PF sets policy not the VF) but this still could have a noticeable effect on the overall system.  If any
> other VFs (or the PF) are receiving MC packets these will have to be replicated which will be a performance hit.  When
> we use the MC hash this is limited vs. when anyone is in MC promiscuous every MC packet used by another pool would be
> replicated.  I could imagine in some environments (i.e. public clouds) where you don't trust what is running in your VM
> you might what to block this from happening.

I understand your request and I'm thinking to submit the patches
  1) Add new mbox API between ixgbe/ixgbevf to turn MC promiscuous on,
     and enables it when ixgbevf needs over 30 MC addresses.
  2) Add a policy knob to prevent enabling it from the PF.

Does it seem okay?

BTW, I'm bit worried about to use ndo interface for 2) because adding a
new hook makes core code complicated.
Is it really reasonable to do it with ndo?
I haven't find any other suitable method to do it, right now. And using
ndo VF hook looks standard way to control VF functionality.
Then, I think it's the best way to implement this policy in ndo hook.

> 
> In some ways it is almost the mirror image of the issue you brought up:
> 
> Adding a new hook for this seems over-complicated to me.  And it still
> doesn't solve the real problems that
>  a) the user has to know about this limit, and
>  b) manually configure the feature
> 
> My reverse argument might be that if this happens automatically.  It might take the VM provider a long time to realize
> performance has taken a hit because some VM asked to join 31 multicast groups and entered MC promiscuous.  Then only to
> find that they have no way to block such behavior.
> 
> Maybe I wouldn't as concerned if the patch author could provide some performance results to show this won't have as a
> negative effect as I'm afraid it might?

Hm, what kind of test case would you like to have?
The user A who has 30 MC addresses vs the user B who has 31 MC addresses, which
means that MC promiscuous mode, and coming MC packets the user doesn't expect?

thanks,
Hiroshi
------------------------------------------------------------------------------
New Year. New Location. New Benefits. New Data Center in Ashburn, VA.
GigeNET is offering a free month of service with a new server in Ashburn.
Choose from 2 high performing configs, both with 100TB of bandwidth.
Higher redundancy.Lower latency.Increased capacity.Completely compliant.
http://p.sf.net/sfu/gigenet
_______________________________________________
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

* [PATCH net 0/2] bpf: fix two bugs
From: Alexei Starovoitov @ 2015-01-23  1:11 UTC (permalink / raw)
  To: David S. Miller; +Cc: Michael Holzheu, Martin Schwidefsky, netdev, linux-kernel

Michael Holzheu caught two issues (in bpf syscall and in the test).
Fix them. Details in corresponding patches.

Alexei Starovoitov (2):
  bpf: rcu lock must not be held when calling copy_to_user()
  samples: bpf: relax test_maps check

 kernel/bpf/syscall.c    |   25 +++++++++++++++++--------
 samples/bpf/test_maps.c |    4 ++--
 2 files changed, 19 insertions(+), 10 deletions(-)

-- 
1.7.9.5

^ permalink raw reply

* [PATCH net 1/2] bpf: rcu lock must not be held when calling copy_to_user()
From: Alexei Starovoitov @ 2015-01-23  1:11 UTC (permalink / raw)
  To: David S. Miller; +Cc: Michael Holzheu, Martin Schwidefsky, netdev, linux-kernel
In-Reply-To: <1421975469-13035-1-git-send-email-ast@plumgrid.com>

BUG: sleeping function called from invalid context at mm/memory.c:3732
in_atomic(): 0, irqs_disabled(): 0, pid: 671, name: test_maps
1 lock held by test_maps/671:
 #0:  (rcu_read_lock){......}, at: [<0000000000264190>] map_lookup_elem+0xe8/0x260
Call Trace:
([<0000000000115b7e>] show_trace+0x12e/0x150)
 [<0000000000115c40>] show_stack+0xa0/0x100
 [<00000000009b163c>] dump_stack+0x74/0xc8
 [<000000000017424a>] ___might_sleep+0x23a/0x248
 [<00000000002b58e8>] might_fault+0x70/0xe8
 [<0000000000264230>] map_lookup_elem+0x188/0x260
 [<0000000000264716>] SyS_bpf+0x20e/0x840

Fix it by allocating temporary buffer to store map element value.

Fixes: db20fd2b0108 ("bpf: add lookup/update/delete/iterate methods to BPF maps")
Reported-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
 kernel/bpf/syscall.c |   25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 088ac0b..536edc2 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -150,7 +150,7 @@ static int map_lookup_elem(union bpf_attr *attr)
 	int ufd = attr->map_fd;
 	struct fd f = fdget(ufd);
 	struct bpf_map *map;
-	void *key, *value;
+	void *key, *value, *ptr;
 	int err;
 
 	if (CHECK_ATTR(BPF_MAP_LOOKUP_ELEM))
@@ -169,20 +169,29 @@ static int map_lookup_elem(union bpf_attr *attr)
 	if (copy_from_user(key, ukey, map->key_size) != 0)
 		goto free_key;
 
-	err = -ENOENT;
-	rcu_read_lock();
-	value = map->ops->map_lookup_elem(map, key);
+	err = -ENOMEM;
+	value = kmalloc(map->value_size, GFP_USER);
 	if (!value)
-		goto err_unlock;
+		goto free_key;
+
+	rcu_read_lock();
+	ptr = map->ops->map_lookup_elem(map, key);
+	if (ptr)
+		memcpy(value, ptr, map->value_size);
+	rcu_read_unlock();
+
+	err = -ENOENT;
+	if (!ptr)
+		goto free_value;
 
 	err = -EFAULT;
 	if (copy_to_user(uvalue, value, map->value_size) != 0)
-		goto err_unlock;
+		goto free_value;
 
 	err = 0;
 
-err_unlock:
-	rcu_read_unlock();
+free_value:
+	kfree(value);
 free_key:
 	kfree(key);
 err_put:
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH net 2/2] samples: bpf: relax test_maps check
From: Alexei Starovoitov @ 2015-01-23  1:11 UTC (permalink / raw)
  To: David S. Miller; +Cc: Michael Holzheu, Martin Schwidefsky, netdev, linux-kernel
In-Reply-To: <1421975469-13035-1-git-send-email-ast@plumgrid.com>

hash map is unordered, so get_next_key() iterator shouldn't
rely on particular order of elements. So relax this test.

Fixes: ffb65f27a155 ("bpf: add a testsuite for eBPF maps")
Reported-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
 samples/bpf/test_maps.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/samples/bpf/test_maps.c b/samples/bpf/test_maps.c
index e286b42..6299ee9 100644
--- a/samples/bpf/test_maps.c
+++ b/samples/bpf/test_maps.c
@@ -69,9 +69,9 @@ static void test_hashmap_sanity(int i, void *data)
 
 	/* iterate over two elements */
 	assert(bpf_get_next_key(map_fd, &key, &next_key) == 0 &&
-	       next_key == 2);
+	       (next_key == 1 || next_key == 2));
 	assert(bpf_get_next_key(map_fd, &next_key, &next_key) == 0 &&
-	       next_key == 1);
+	       (next_key == 1 || next_key == 2));
 	assert(bpf_get_next_key(map_fd, &next_key, &next_key) == -1 &&
 	       errno == ENOENT);
 
-- 
1.7.9.5

^ permalink raw reply related

* RE: [E1000-devel] [PATCH 1/2] if_link: Add VF multicast promiscuous mode control
From: Skidmore, Donald C @ 2015-01-23  1:21 UTC (permalink / raw)
  To: Hiroshi Shimamoto, Bjørn Mork
  Cc: David Laight, e1000-devel@lists.sourceforge.net,
	netdev@vger.kernel.org, Choi, Sy Jong,
	linux-kernel@vger.kernel.org, Hayato Momma
In-Reply-To: <7F861DC0615E0C47A872E6F3C5FCDDBD05E0B6B1@BPXM14GP.gisp.nec.co.jp>



> -----Original Message-----
> From: Hiroshi Shimamoto [mailto:h-shimamoto@ct.jp.nec.com]
> Sent: Thursday, January 22, 2015 4:32 PM
> To: Skidmore, Donald C; Bjørn Mork
> Cc: David Laight; e1000-devel@lists.sourceforge.net;
> netdev@vger.kernel.org; Choi, Sy Jong; linux-kernel@vger.kernel.org;
> Hayato Momma
> Subject: RE: [E1000-devel] [PATCH 1/2] if_link: Add VF multicast promiscuous
> mode control
> 
> > Subject: RE: [E1000-devel] [PATCH 1/2] if_link: Add VF multicast
> > promiscuous mode control
> > >
> > > "Skidmore, Donald C" <donald.c.skidmore@intel.com> writes:
> > >
> > > > My hang up is more related to: without the nob to enable it (off
> > > > by
> > > > default) we are letting one VF dictate policy for all the other
> > > > VFs and the PF.  If one VF needs to be in promiscuous multicast so
> > > > is everyone else.  Their stacks now needs to deal with all the
> > > > extra multicast packets.  As you point out this might not be a
> > > > direct concern for isolation in that the VM could have 'chosen' to
> > > > join any Multicast group and seen this traffic.  My concern over
> > > > isolation is one VF has chosen that all the other VM now have to
> > > > see this multicast traffic.
> > >
> > > Apologies if this question is stupid, but I just have to ask about
> > > stuff I don't understand...
> > >
> > > Looking at the proposed implementation, the promiscous multicast
> > > flag seems to be a per-VF flag:
> > >
> > > +int ixgbe_ndo_set_vf_mc_promisc(struct net_device *netdev, int vf,
> > > +bool
> > > +setting) {
> > > +	struct ixgbe_adapter *adapter = netdev_priv(netdev);
> > > +	struct ixgbe_hw *hw = &adapter->hw;
> > > +	u32 vmolr;
> > > +
> > > +	if (vf >= adapter->num_vfs)
> > > +		return -EINVAL;
> > > +
> > > +	adapter->vfinfo[vf].mc_promisc_enabled = setting;
> > > +
> > > +	vmolr = IXGBE_READ_REG(hw, IXGBE_VMOLR(vf));
> > > +	if (setting) {
> > > +		e_info(drv, "VF %u: enabling multicast promiscuous\n", vf);
> > > +		vmolr |= IXGBE_VMOLR_MPE;
> > > +	} else {
> > > +		e_info(drv, "VF %u: disabling multicast promiscuous\n", vf);
> > > +		vmolr &= ~IXGBE_VMOLR_MPE;
> > > +	}
> > > +
> > > +	IXGBE_WRITE_REG(hw, IXGBE_VMOLR(vf), vmolr);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > >
> > > I haven't read the data sheet, but I took a quick look at the
> > > excellent high level driver docs:
> > > http://www.intel.com/content/dam/doc/design-guide/82599-sr-iov-
> drive
> > > r-
> > > companion-guide.pdf
> > >
> > > It mentions "Multicast Promiscuous Enable" in its "Thoughts for
> > > Customization" section:
> > >
> > >  7.1 Multicast Promiscuous Enable
> > >
> > >  The controller has provisions to allow each VF to be put into
> > > Multicast Promiscuous mode.  The Intel reference driver does not
> > > configure this option .
> > >
> > >  The capability can be enabled/disabled by manipulating the MPE
> > > field  (bit
> > > 28) of the PF VF L2 Control Register (PFVML2FLT – 0x0F000)
> > >
> > > and showing a section from the data sheet describing the "PF VM L2
> > > Control Register - PFVML2FLT[n]  (0x0F000 + 4 * n, n=0...63; RW)"
> > >
> > > To me it looks like enabling Promiscuos Multicast for a VF won't
> > > affect any other VF at all.  Is this really not the case?
> > >
> > >
> > >
> > > Bjørn
> >
> > Clearly not a dumb question at all and I'm glad you mentioned that. :)
> > I was going off the assumption, been awhile since I read the patch,
> > that the patch was using FCTRL.MPE or MANC.MCST_PASS_L2 which would
> turn multicast promiscuous on for everyone.  Since the patch is using
> PFVML2FLT.MPE this lessens my concern over effect on the entire system.
> 
> I believe the patches for this VF multicast promiscuous mode is per VF.
> 
> >
> > That said I still would prefer having a way to override this behavior on the
> PF, although I admit my argument is weaker.
> > I'm still concerned about a VF changing the behavior of the PF without
> > any way to prevent it.  This might be one part philosophical (PF sets
> > policy not the VF) but this still could have a noticeable effect on
> > the overall system.  If any other VFs (or the PF) are receiving MC
> > packets these will have to be replicated which will be a performance
> > hit.  When we use the MC hash this is limited vs. when anyone is in MC
> promiscuous every MC packet used by another pool would be replicated.  I
> could imagine in some environments (i.e. public clouds) where you don't
> trust what is running in your VM you might what to block this from
> happening.
> 
> I understand your request and I'm thinking to submit the patches
>   1) Add new mbox API between ixgbe/ixgbevf to turn MC promiscuous on,
>      and enables it when ixgbevf needs over 30 MC addresses.
>   2) Add a policy knob to prevent enabling it from the PF.
> 
> Does it seem okay?

This sounds totally fine to me.  That way users that need this functionality can get it while anyone concerned about side effects don't enable it.

> 
> BTW, I'm bit worried about to use ndo interface for 2) because adding a new
> hook makes core code complicated.
> Is it really reasonable to do it with ndo?
> I haven't find any other suitable method to do it, right now. And using ndo VF
> hook looks standard way to control VF functionality.

To be honest I had been thinking it would be enabled by PF.  Basically a hook saying the PF was willing to let any VF's go into MC promiscuous and thus so not bothering to brake it out by VF.  Since the advice effects I was worried about would be felt on all the VF's and PF even if it was enabled on only some VF's.   I imagine there would be some benefit to being able to control how many VF's were able to enter this mode but I would be ok with either.

> Then, I think it's the best way to implement this policy in ndo hook.

That seems reasonable to me.  If not I'm not sure what else would be any better, maybe ethtool --set-priv-flags but that wouldn't make much sense if you set it by VF.  Likewise I'm not sure how excited people would be about including policy like this in ethtool.

> 
> >
> > In some ways it is almost the mirror image of the issue you brought up:
> >
> > Adding a new hook for this seems over-complicated to me.  And it still
> > doesn't solve the real problems that
> >  a) the user has to know about this limit, and
> >  b) manually configure the feature
> >
> > My reverse argument might be that if this happens automatically.  It
> > might take the VM provider a long time to realize performance has
> > taken a hit because some VM asked to join 31 multicast groups and
> entered MC promiscuous.  Then only to find that they have no way to block
> such behavior.
> >
> > Maybe I wouldn't as concerned if the patch author could provide some
> > performance results to show this won't have as a negative effect as I'm
> afraid it might?
> 
> Hm, what kind of test case would you like to have?
> The user A who has 30 MC addresses vs the user B who has 31 MC addresses,
> which means that MC promiscuous mode, and coming MC packets the user
> doesn't expect?
> 
> thanks,
> Hiroshi

^ permalink raw reply

* Re: [PATCH iproute2] update obsolete fields in slabstat_ids[]
From: Bryton Lee @ 2015-01-23  1:50 UTC (permalink / raw)
  To: netdev, stephen; +Cc: Bryton Lee
In-Reply-To: <CAC2pzGe3Yd6sjeQG4UYcf9jyK4txjVR-CADzHEtkevMhWDTCOA@mail.gmail.com>

Any one has any comment?  Have you didn't saw this issue before when run ss -s ?

On Fri, Jan 16, 2015 at 10:27 AM, Bryton Lee <brytonlee01@gmail.com> wrote:
> Hi Stephen:
>
> This is the first time I submit a patch to iproute2, I forgot CC to a
> iproute2's maintainer. It's my fault. Could you please review my
> patch? :)
>
> this patch can help some old system get TCP info correctly.  such as
> Red Hat 5u7,  although  is not perfect.  ss searching  tcp_bind_bucket
> and request_sock_TCP in /proc/slabinfo will fail on current upstream
> kernel,
> because on current upstream kernel slab tcp_bind_bucket and
> request_sock_TCP have merged into kmalloc-xx when they are creating.
> so  in /proc/slabinfo  we cannot see them any more.
>
> I have submitted other patch to mm subsystem to prevent such merging,
> but it still not accepted. (Yep, finally, I know my patch has problem,
> it doesn't make any sense, but may be we should still think about how
> to prevent slab merging in slab layer. )
>
>
> On Mon, Dec 29, 2014 at 8:16 PM, Bryton Lee <brytonlee01@gmail.com> wrote:
>> From: Bryton Lee <brytonlee01@gmail.com>
>> Date: Mon, 29 Dec 2014 19:56:23 +0800
>> Subject: [PATCH] tcp_open_request and tcp_tw_bucket are obsolete in current
>>  stable kernel version, replace by request_sock_TCP and tw_sock_TCP
>>
>> Signed-off-by: Bryton Lee <brytonlee01@gmail.com>
>> ---
>>  misc/ss.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/misc/ss.c b/misc/ss.c
>> index f0c7b34..24d58c7 100644
>> --- a/misc/ss.c
>> +++ b/misc/ss.c
>> @@ -506,8 +506,8 @@ static const char *slabstat_ids[] =
>>  {
>>      "sock",
>>      "tcp_bind_bucket",
>> -    "tcp_tw_bucket",
>> -    "tcp_open_request",
>> +    "tw_sock_TCP",
>> +    "request_sock_TCP",
>>      "skbuff_head_cache",
>>  };
>>
>> --
>> 2.0.5
>>
>> --
>> Best Regards
>>
>> Bryton.Lee
>>
>
>
>
> --
> Best Regards
>
> Bryton.Lee



-- 
Best Regards

Bryton.Lee

^ permalink raw reply

* [net-next 00/15][pull request] Intel Wired LAN Driver Updates 2015-01-22
From: Jeff Kirsher @ 2015-01-23  2:36 UTC (permalink / raw)
  To: davem; +Cc: Jeff Kirsher, netdev, nhorman, sassmann, jogreene

This series contains updates to e1000, e1000e, igb, fm10k and virtio_net.

Asaf Vertz provides a fix for e1000 to future-proof the time comparisons
by using time_after_eq() instead of plain math.

Mathias Koehrer provides a fix for e1000e to add a check to e1000_xmit_frame()
to ensure a work queue will not be scheduled that has not been initialized.

Jacob adds the use of software timestamping via the virtio_net driver.

Alex Duyck cleans up page reuse code in igb and fm10k.  Cleans up the
page reuse code from getting into a state where all the workarounds
needed are in place as well as cleaning up oversights, such as using
__free_pages instead of put_page to drop a locally allocated page.

Richard Cochran provides 4 patches for igb dealing with time sync.
First provides a helper function since the code that handles the time
sync interrupt is repeated in three different places.  Then serializes
the access to the time sync interrupt since the registers may be
manipulated from different contexts.  Enables the use of i210 device
interrupt to generate an internal PPS event for adjusting the kernel
system time.  The i210 device offers a number of special PTP hardware
clock features on the Software Defined Pins (SDPs), so added support for
two of the possible functions (time stamping external events and
periodic output signals).

Or Gerlitz fixes fm10k from double setting of NETIF_F_SG since the
networking core does it for the driver during registration time.

Joe Stringer adds support for up to 104 bytes of inner+outer headers in
fm10k and adds an initial check to fail encapsulation offload if these
are too large.

Matthew increases the timeout for the data path reset based on feedback
from the hardware team, since 100us is too short of a time to wait for
the data path reset to complete.

Alexander Graf provides a fix for igb to indicate failure on VF reset
for an empty MAC address, to mirror the behavior of ixgbe.

Florian Westphal updates e1000 and e1000e to support txtd update delay
via xmit_more, this way we won't update the Tx tail descriptor if the
queue has not been stopped and we know at least one more skb will be
sent right away.

The following are changes since commit 0c49087462e8587c12ecfeaf1dd46fdc0ddc4532:
  Merge tag 'mac80211-next-for-davem-2015-01-19' of git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next master

Alexander Duyck (2):
  igb: Clean-up page reuse code
  fm10k: Clean-up page reuse code

Alexander Graf (1):
  igb: Indicate failure on vf reset for empty mac address

Asaf Vertz (1):
  e1000: fix time comparison

Florian Westphal (2):
  net: e1000: support txtd update delay via xmit_more
  net: e1000e: support txtd update delay via xmit_more

Jacob Keller (1):
  virtio_net: add software timestamp support

Joe Stringer (1):
  fm10k: Check tunnel header length in encap offload

Mathias Koehrer (1):
  e1000e: Fix 82572EI that has no hardware timestamp support

Matthew Vick (1):
  fm10k: Increase the timeout for the data path reset

Or Gerlitz (1):
  net/fm10k: Avoid double setting of NETIF_F_SG for the HW encapsulation
    feature mask

Richard Cochran (4):
  igb: refactor time sync interrupt handling
  igb: serialize access to the time sync interrupt registers
  igb: enable internal PPS for the i210
  igb: enable auxiliary PHC functions for the i210

 drivers/net/ethernet/intel/e1000/e1000_ethtool.c |   3 +-
 drivers/net/ethernet/intel/e1000/e1000_main.c    |  15 +-
 drivers/net/ethernet/intel/e1000e/netdev.c       |  30 +--
 drivers/net/ethernet/intel/fm10k/fm10k_main.c    |  40 ++--
 drivers/net/ethernet/intel/fm10k/fm10k_netdev.c  |  13 +-
 drivers/net/ethernet/intel/fm10k/fm10k_type.h    |   2 +-
 drivers/net/ethernet/intel/igb/igb.h             |   9 +
 drivers/net/ethernet/intel/igb/igb_main.c        | 153 +++++++++-----
 drivers/net/ethernet/intel/igb/igb_ptp.c         | 256 ++++++++++++++++++++++-
 drivers/net/virtio_net.c                         |   4 +
 10 files changed, 424 insertions(+), 101 deletions(-)

-- 
1.9.3

^ permalink raw reply

* [net-next 02/15] e1000e: Fix 82572EI that has no hardware timestamp support
From: Jeff Kirsher @ 2015-01-23  2:36 UTC (permalink / raw)
  To: davem; +Cc: Mathias Koehrer, netdev, nhorman, sassmann, jogreene,
	Jeff Kirsher
In-Reply-To: <1421980631-1955-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Mathias Koehrer <mathias.koehrer@etas.com>

With the Intel 82527EI (driver: e1000e) there is an issue when running
the ptpd2 program, that leads to a kernel oops.  The reason is here that
in e1000_xmit_frame() a work queue will be scheduled that has not been
initialized in this case.  The work queue "tx_hwstamp_work" will only be
initialized if adapter->flags & FLAG_HAS_HW_TIMESTAMP set.  This check
is missing in e1000_xmit_frame().

The following patch adds the missing check.

Signed-off-by: Mathias Koehrer <mathias.koehrer@etas.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/e1000e/netdev.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 38cb586..aa39a81 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -5636,8 +5636,9 @@ static netdev_tx_t e1000_xmit_frame(struct sk_buff *skb,
 	count = e1000_tx_map(tx_ring, skb, first, adapter->tx_fifo_limit,
 			     nr_frags);
 	if (count) {
-		if (unlikely((skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) &&
-			     !adapter->tx_hwtstamp_skb)) {
+		if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) &&
+		    (adapter->flags & FLAG_HAS_HW_TIMESTAMP) &&
+		    !adapter->tx_hwtstamp_skb) {
 			skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
 			tx_flags |= E1000_TX_FLAGS_HWTSTAMP;
 			adapter->tx_hwtstamp_skb = skb_get(skb);
-- 
1.9.3

^ permalink raw reply related

* [net-next 01/15] e1000: fix time comparison
From: Jeff Kirsher @ 2015-01-23  2:36 UTC (permalink / raw)
  To: davem; +Cc: Asaf Vertz, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <1421980631-1955-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Asaf Vertz <asaf.vertz@tandemg.com>

To be future-proof and for better readability the time comparisons are
modified to use time_after_eq() instead of plain, error-prone math.

Signed-off-by: Asaf Vertz <asaf.vertz@tandemg.com>
Acked-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/e1000/e1000_ethtool.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
index b691eb4..4270ad2 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
@@ -24,6 +24,7 @@
 /* ethtool support for e1000 */
 
 #include "e1000.h"
+#include <linux/jiffies.h>
 #include <linux/uaccess.h>
 
 enum {NETDEV_STATS, E1000_STATS};
@@ -1460,7 +1461,7 @@ static int e1000_run_loopback_test(struct e1000_adapter *adapter)
 			ret_val = 13; /* ret_val is the same as mis-compare */
 			break;
 		}
-		if (jiffies >= (time + 2)) {
+		if (time_after_eq(jiffies, time + 2)) {
 			ret_val = 14; /* error code for time out error */
 			break;
 		}
-- 
1.9.3

^ permalink raw reply related

* [net-next 04/15] igb: Clean-up page reuse code
From: Jeff Kirsher @ 2015-01-23  2:37 UTC (permalink / raw)
  To: davem; +Cc: Alexander Duyck, netdev, nhorman, sassmann, jogreene,
	Jeff Kirsher
In-Reply-To: <1421980631-1955-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Alexander Duyck <alexander.h.duyck@redhat.com>

This patch cleans up the page reuse code getting it into a state where all
the workarounds needed are in place as well as cleaning up a few minor
oversights such as using __free_pages instead of put_page to drop a locally
allocated page.

It also cleans up how we clear the descriptor status bits.  Previously they
were zeroed as a part of clearing the hdr_addr.  However the hdr_addr is a
64 bit field and 64 bit writes can be a bit more expensive on on 32 bit
systems.  Since we are no longer using the header split feature the upper
32 bits of the address no longer need to be cleared.  As a result we can
just clear the status bits and leave the length and VLAN fields as-is which
should provide more information in debugging.

Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_main.c | 35 ++++++++++++++-----------------
 1 file changed, 16 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 6c25ec3..d1480f3 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -6527,15 +6527,17 @@ static void igb_reuse_rx_page(struct igb_ring *rx_ring,
 					 DMA_FROM_DEVICE);
 }
 
+static inline bool igb_page_is_reserved(struct page *page)
+{
+	return (page_to_nid(page) != numa_mem_id()) || page->pfmemalloc;
+}
+
 static bool igb_can_reuse_rx_page(struct igb_rx_buffer *rx_buffer,
 				  struct page *page,
 				  unsigned int truesize)
 {
 	/* avoid re-using remote pages */
-	if (unlikely(page_to_nid(page) != numa_node_id()))
-		return false;
-
-	if (unlikely(page->pfmemalloc))
+	if (unlikely(igb_page_is_reserved(page)))
 		return false;
 
 #if (PAGE_SIZE < 8192)
@@ -6545,22 +6547,19 @@ static bool igb_can_reuse_rx_page(struct igb_rx_buffer *rx_buffer,
 
 	/* flip page offset to other buffer */
 	rx_buffer->page_offset ^= IGB_RX_BUFSZ;
-
-	/* Even if we own the page, we are not allowed to use atomic_set()
-	 * This would break get_page_unless_zero() users.
-	 */
-	atomic_inc(&page->_count);
 #else
 	/* move offset up to the next cache line */
 	rx_buffer->page_offset += truesize;
 
 	if (rx_buffer->page_offset > (PAGE_SIZE - IGB_RX_BUFSZ))
 		return false;
-
-	/* bump ref count on page before it is given to the stack */
-	get_page(page);
 #endif
 
+	/* Even if we own the page, we are not allowed to use atomic_set()
+	 * This would break get_page_unless_zero() users.
+	 */
+	atomic_inc(&page->_count);
+
 	return true;
 }
 
@@ -6603,13 +6602,12 @@ static bool igb_add_rx_frag(struct igb_ring *rx_ring,
 
 		memcpy(__skb_put(skb, size), va, ALIGN(size, sizeof(long)));
 
-		/* we can reuse buffer as-is, just make sure it is local */
-		if (likely((page_to_nid(page) == numa_node_id()) &&
-			   !page->pfmemalloc))
+		/* page is not reserved, we can reuse buffer as-is */
+		if (likely(!igb_page_is_reserved(page)))
 			return true;
 
 		/* this page cannot be reused so discard it */
-		put_page(page);
+		__free_page(page);
 		return false;
 	}
 
@@ -6627,7 +6625,6 @@ static struct sk_buff *igb_fetch_rx_buffer(struct igb_ring *rx_ring,
 	struct page *page;
 
 	rx_buffer = &rx_ring->rx_buffer_info[rx_ring->next_to_clean];
-
 	page = rx_buffer->page;
 	prefetchw(page);
 
@@ -7042,8 +7039,8 @@ void igb_alloc_rx_buffers(struct igb_ring *rx_ring, u16 cleaned_count)
 			i -= rx_ring->count;
 		}
 
-		/* clear the hdr_addr for the next_to_use descriptor */
-		rx_desc->read.hdr_addr = 0;
+		/* clear the status bits for the next_to_use descriptor */
+		rx_desc->wb.upper.status_error = 0;
 
 		cleaned_count--;
 	} while (cleaned_count);
-- 
1.9.3

^ permalink raw reply related

* [net-next 03/15] virtio_net: add software timestamp support
From: Jeff Kirsher @ 2015-01-23  2:36 UTC (permalink / raw)
  To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <1421980631-1955-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Jacob Keller <jacob.e.keller@intel.com>

This patch enables the use of software timestamping via the virtio_net
driver.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/virtio_net.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 11e2e81..9bd71d5 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -925,6 +925,9 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
 	/* Free up any pending old buffers before queueing new ones. */
 	free_old_xmit_skbs(sq);
 
+	/* timestamp packet in software */
+	skb_tx_timestamp(skb);
+
 	/* Try to transmit */
 	err = xmit_skb(sq, skb);
 
@@ -1376,6 +1379,7 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
 	.get_ringparam = virtnet_get_ringparam,
 	.set_channels = virtnet_set_channels,
 	.get_channels = virtnet_get_channels,
+	.get_ts_info = ethtool_op_get_ts_info,
 };
 
 #define MIN_MTU 68
-- 
1.9.3

^ permalink raw reply related

* [net-next 05/15] fm10k: Clean-up page reuse code
From: Jeff Kirsher @ 2015-01-23  2:37 UTC (permalink / raw)
  To: davem; +Cc: Alexander Duyck, netdev, nhorman, sassmann, jogreene,
	Jeff Kirsher
In-Reply-To: <1421980631-1955-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Alexander Duyck <alexander.h.duyck@redhat.com>

This patch cleans up the page reuse code getting it into a state where all
the workarounds needed are in place as well as cleaning up a few minor
oversights such as using __free_pages instead of put_page to drop a locally
allocated page.

It also cleans up how we clear the descriptor status bits.  Previously they
were zeroed as a part of clearing the hdr_addr.  However the hdr_addr is a
64 bit field and 64 bit writes can be a bit more expensive on on 32 bit
systems.  Since we are no longer using the header split feature the upper
32 bits of the address no longer need to be cleared.  As a result we can
just clear the status bits and leave the length and VLAN fields as-is which
should provide more information in debugging.

Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
Tested-by: Krishneil Singh <Krishneil.k.singh@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_main.c | 34 +++++++++++++--------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_main.c b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
index caa43f7..c7a19a5 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_main.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
@@ -97,7 +97,6 @@ static bool fm10k_alloc_mapped_page(struct fm10k_ring *rx_ring,
 	 */
 	if (dma_mapping_error(rx_ring->dev, dma)) {
 		__free_page(page);
-		bi->page = NULL;
 
 		rx_ring->rx_stats.alloc_failed++;
 		return false;
@@ -147,8 +146,8 @@ void fm10k_alloc_rx_buffers(struct fm10k_ring *rx_ring, u16 cleaned_count)
 			i -= rx_ring->count;
 		}
 
-		/* clear the hdr_addr for the next_to_use descriptor */
-		rx_desc->q.hdr_addr = 0;
+		/* clear the status bits for the next_to_use descriptor */
+		rx_desc->d.staterr = 0;
 
 		cleaned_count--;
 	} while (cleaned_count);
@@ -194,7 +193,7 @@ static void fm10k_reuse_rx_page(struct fm10k_ring *rx_ring,
 	rx_ring->next_to_alloc = (nta < rx_ring->count) ? nta : 0;
 
 	/* transfer page from old buffer to new buffer */
-	memcpy(new_buff, old_buff, sizeof(struct fm10k_rx_buffer));
+	*new_buff = *old_buff;
 
 	/* sync the buffer for use by the device */
 	dma_sync_single_range_for_device(rx_ring->dev, old_buff->dma,
@@ -203,12 +202,17 @@ static void fm10k_reuse_rx_page(struct fm10k_ring *rx_ring,
 					 DMA_FROM_DEVICE);
 }
 
+static inline bool fm10k_page_is_reserved(struct page *page)
+{
+	return (page_to_nid(page) != numa_mem_id()) || page->pfmemalloc;
+}
+
 static bool fm10k_can_reuse_rx_page(struct fm10k_rx_buffer *rx_buffer,
 				    struct page *page,
 				    unsigned int truesize)
 {
 	/* avoid re-using remote pages */
-	if (unlikely(page_to_nid(page) != numa_mem_id()))
+	if (unlikely(fm10k_page_is_reserved(page)))
 		return false;
 
 #if (PAGE_SIZE < 8192)
@@ -218,22 +222,19 @@ static bool fm10k_can_reuse_rx_page(struct fm10k_rx_buffer *rx_buffer,
 
 	/* flip page offset to other buffer */
 	rx_buffer->page_offset ^= FM10K_RX_BUFSZ;
-
-	/* Even if we own the page, we are not allowed to use atomic_set()
-	 * This would break get_page_unless_zero() users.
-	 */
-	atomic_inc(&page->_count);
 #else
 	/* move offset up to the next cache line */
 	rx_buffer->page_offset += truesize;
 
 	if (rx_buffer->page_offset > (PAGE_SIZE - FM10K_RX_BUFSZ))
 		return false;
-
-	/* bump ref count on page before it is given to the stack */
-	get_page(page);
 #endif
 
+	/* Even if we own the page, we are not allowed to use atomic_set()
+	 * This would break get_page_unless_zero() users.
+	 */
+	atomic_inc(&page->_count);
+
 	return true;
 }
 
@@ -270,12 +271,12 @@ static bool fm10k_add_rx_frag(struct fm10k_ring *rx_ring,
 
 		memcpy(__skb_put(skb, size), va, ALIGN(size, sizeof(long)));
 
-		/* we can reuse buffer as-is, just make sure it is local */
-		if (likely(page_to_nid(page) == numa_mem_id()))
+		/* page is not reserved, we can reuse buffer as-is */
+		if (likely(!fm10k_page_is_reserved(page)))
 			return true;
 
 		/* this page cannot be reused so discard it */
-		put_page(page);
+		__free_page(page);
 		return false;
 	}
 
@@ -293,7 +294,6 @@ static struct sk_buff *fm10k_fetch_rx_buffer(struct fm10k_ring *rx_ring,
 	struct page *page;
 
 	rx_buffer = &rx_ring->rx_buffer[rx_ring->next_to_clean];
-
 	page = rx_buffer->page;
 	prefetchw(page);
 
-- 
1.9.3

^ permalink raw reply related

* [net-next 07/15] igb: serialize access to the time sync interrupt registers
From: Jeff Kirsher @ 2015-01-23  2:37 UTC (permalink / raw)
  To: davem; +Cc: Richard Cochran, netdev, nhorman, sassmann, jogreene,
	Jeff Kirsher
In-Reply-To: <1421980631-1955-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Richard Cochran <richardcochran@gmail.com>

The time sync related interrupt registers may be manipulated from
different contexts. This patch protects the registers from being
asynchronously changed by the reset function.

Also, the patch removes a misleading comment. The reset function
is disabling a bunch of functions, not enabling them.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_ptp.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index 5e7a4e3..8389bb4 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -900,6 +900,7 @@ void igb_ptp_stop(struct igb_adapter *adapter)
 void igb_ptp_reset(struct igb_adapter *adapter)
 {
 	struct e1000_hw *hw = &adapter->hw;
+	unsigned long flags;
 
 	if (!(adapter->flags & IGB_FLAG_PTP))
 		return;
@@ -907,6 +908,8 @@ void igb_ptp_reset(struct igb_adapter *adapter)
 	/* reset the tstamp_config */
 	igb_ptp_set_timestamp_mode(adapter, &adapter->tstamp_config);
 
+	spin_lock_irqsave(&adapter->tmreg_lock, flags);
+
 	switch (adapter->hw.mac.type) {
 	case e1000_82576:
 		/* Dial the nominal frequency. */
@@ -917,23 +920,24 @@ void igb_ptp_reset(struct igb_adapter *adapter)
 	case e1000_i350:
 	case e1000_i210:
 	case e1000_i211:
-		/* Enable the timer functions and interrupts. */
 		wr32(E1000_TSAUXC, 0x0);
 		wr32(E1000_TSIM, TSYNC_INTERRUPTS);
 		wr32(E1000_IMS, E1000_IMS_TS);
 		break;
 	default:
 		/* No work to do. */
-		return;
+		goto out;
 	}
 
 	/* Re-initialize the timer. */
 	if ((hw->mac.type == e1000_i210) || (hw->mac.type == e1000_i211)) {
 		struct timespec ts = ktime_to_timespec(ktime_get_real());
 
-		igb_ptp_settime_i210(&adapter->ptp_caps, &ts);
+		igb_ptp_write_i210(adapter, &ts);
 	} else {
 		timecounter_init(&adapter->tc, &adapter->cc,
 				 ktime_to_ns(ktime_get_real()));
 	}
+out:
+	spin_unlock_irqrestore(&adapter->tmreg_lock, flags);
 }
-- 
1.9.3

^ permalink raw reply related

* [net-next 06/15] igb: refactor time sync interrupt handling
From: Jeff Kirsher @ 2015-01-23  2:37 UTC (permalink / raw)
  To: davem; +Cc: Richard Cochran, netdev, nhorman, sassmann, jogreene,
	Jeff Kirsher
In-Reply-To: <1421980631-1955-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Richard Cochran <richardcochran@gmail.com>

The code that handles the time sync interrupt is repeated in three
different places. This patch refactors the identical code blocks into
a single helper function.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_main.c | 49 ++++++++++++-------------------
 1 file changed, 19 insertions(+), 30 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index d1480f3..135ac5c 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -5384,6 +5384,19 @@ void igb_update_stats(struct igb_adapter *adapter,
 	}
 }
 
+static void igb_tsync_interrupt(struct igb_adapter *adapter)
+{
+	struct e1000_hw *hw = &adapter->hw;
+	u32 tsicr = rd32(E1000_TSICR);
+
+	if (tsicr & E1000_TSICR_TXTS) {
+		/* acknowledge the interrupt */
+		wr32(E1000_TSICR, E1000_TSICR_TXTS);
+		/* retrieve hardware timestamp */
+		schedule_work(&adapter->ptp_tx_work);
+	}
+}
+
 static irqreturn_t igb_msix_other(int irq, void *data)
 {
 	struct igb_adapter *adapter = data;
@@ -5415,16 +5428,8 @@ static irqreturn_t igb_msix_other(int irq, void *data)
 			mod_timer(&adapter->watchdog_timer, jiffies + 1);
 	}
 
-	if (icr & E1000_ICR_TS) {
-		u32 tsicr = rd32(E1000_TSICR);
-
-		if (tsicr & E1000_TSICR_TXTS) {
-			/* acknowledge the interrupt */
-			wr32(E1000_TSICR, E1000_TSICR_TXTS);
-			/* retrieve hardware timestamp */
-			schedule_work(&adapter->ptp_tx_work);
-		}
-	}
+	if (icr & E1000_ICR_TS)
+		igb_tsync_interrupt(adapter);
 
 	wr32(E1000_EIMS, adapter->eims_other);
 
@@ -6203,16 +6208,8 @@ static irqreturn_t igb_intr_msi(int irq, void *data)
 			mod_timer(&adapter->watchdog_timer, jiffies + 1);
 	}
 
-	if (icr & E1000_ICR_TS) {
-		u32 tsicr = rd32(E1000_TSICR);
-
-		if (tsicr & E1000_TSICR_TXTS) {
-			/* acknowledge the interrupt */
-			wr32(E1000_TSICR, E1000_TSICR_TXTS);
-			/* retrieve hardware timestamp */
-			schedule_work(&adapter->ptp_tx_work);
-		}
-	}
+	if (icr & E1000_ICR_TS)
+		igb_tsync_interrupt(adapter);
 
 	napi_schedule(&q_vector->napi);
 
@@ -6257,16 +6254,8 @@ static irqreturn_t igb_intr(int irq, void *data)
 			mod_timer(&adapter->watchdog_timer, jiffies + 1);
 	}
 
-	if (icr & E1000_ICR_TS) {
-		u32 tsicr = rd32(E1000_TSICR);
-
-		if (tsicr & E1000_TSICR_TXTS) {
-			/* acknowledge the interrupt */
-			wr32(E1000_TSICR, E1000_TSICR_TXTS);
-			/* retrieve hardware timestamp */
-			schedule_work(&adapter->ptp_tx_work);
-		}
-	}
+	if (icr & E1000_ICR_TS)
+		igb_tsync_interrupt(adapter);
 
 	napi_schedule(&q_vector->napi);
 
-- 
1.9.3

^ permalink raw reply related

* [net-next 09/15] igb: enable auxiliary PHC functions for the i210
From: Jeff Kirsher @ 2015-01-23  2:37 UTC (permalink / raw)
  To: davem; +Cc: Richard Cochran, netdev, nhorman, sassmann, jogreene,
	Jeff Kirsher
In-Reply-To: <1421980631-1955-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Richard Cochran <richardcochran@gmail.com>

The i210 device offers a number of special PTP Hardware Clock features on
the Software Defined Pins (SDPs). This patch adds support for two of the
possible functions, namely time stamping external events, and periodic
output signals.

The assignment of PHC functions to the four SDP can be freely chosen by
the user.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/igb.h      |   9 ++
 drivers/net/ethernet/intel/igb/igb_main.c |  51 ++++++-
 drivers/net/ethernet/intel/igb/igb_ptp.c  | 222 +++++++++++++++++++++++++++++-
 3 files changed, 276 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index ee22da3..c2bd4f9 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -343,6 +343,9 @@ struct hwmon_buff {
 	};
 #endif
 
+#define IGB_N_EXTTS	2
+#define IGB_N_PEROUT	2
+#define IGB_N_SDP	4
 #define IGB_RETA_SIZE	128
 
 /* board specific private data structure */
@@ -439,6 +442,12 @@ struct igb_adapter {
 	u32 tx_hwtstamp_timeouts;
 	u32 rx_hwtstamp_cleared;
 
+	struct ptp_pin_desc sdp_config[IGB_N_SDP];
+	struct {
+		struct timespec start;
+		struct timespec period;
+	} perout[IGB_N_PEROUT];
+
 	char fw_version[32];
 #ifdef CONFIG_IGB_HWMON
 	struct hwmon_buff *igb_hwmon_buff;
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index e844162..6e65449 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -5388,7 +5388,8 @@ static void igb_tsync_interrupt(struct igb_adapter *adapter)
 {
 	struct e1000_hw *hw = &adapter->hw;
 	struct ptp_clock_event event;
-	u32 ack = 0, tsicr = rd32(E1000_TSICR);
+	struct timespec ts;
+	u32 ack = 0, tsauxc, sec, nsec, tsicr = rd32(E1000_TSICR);
 
 	if (tsicr & TSINTR_SYS_WRAP) {
 		event.type = PTP_CLOCK_PPS;
@@ -5405,6 +5406,54 @@ static void igb_tsync_interrupt(struct igb_adapter *adapter)
 		ack |= E1000_TSICR_TXTS;
 	}
 
+	if (tsicr & TSINTR_TT0) {
+		spin_lock(&adapter->tmreg_lock);
+		ts = timespec_add(adapter->perout[0].start,
+				  adapter->perout[0].period);
+		wr32(E1000_TRGTTIML0, ts.tv_nsec);
+		wr32(E1000_TRGTTIMH0, ts.tv_sec);
+		tsauxc = rd32(E1000_TSAUXC);
+		tsauxc |= TSAUXC_EN_TT0;
+		wr32(E1000_TSAUXC, tsauxc);
+		adapter->perout[0].start = ts;
+		spin_unlock(&adapter->tmreg_lock);
+		ack |= TSINTR_TT0;
+	}
+
+	if (tsicr & TSINTR_TT1) {
+		spin_lock(&adapter->tmreg_lock);
+		ts = timespec_add(adapter->perout[1].start,
+				  adapter->perout[1].period);
+		wr32(E1000_TRGTTIML1, ts.tv_nsec);
+		wr32(E1000_TRGTTIMH1, ts.tv_sec);
+		tsauxc = rd32(E1000_TSAUXC);
+		tsauxc |= TSAUXC_EN_TT1;
+		wr32(E1000_TSAUXC, tsauxc);
+		adapter->perout[1].start = ts;
+		spin_unlock(&adapter->tmreg_lock);
+		ack |= TSINTR_TT1;
+	}
+
+	if (tsicr & TSINTR_AUTT0) {
+		nsec = rd32(E1000_AUXSTMPL0);
+		sec  = rd32(E1000_AUXSTMPH0);
+		event.type = PTP_CLOCK_EXTTS;
+		event.index = 0;
+		event.timestamp = sec * 1000000000ULL + nsec;
+		ptp_clock_event(adapter->ptp_clock, &event);
+		ack |= TSINTR_AUTT0;
+	}
+
+	if (tsicr & TSINTR_AUTT1) {
+		nsec = rd32(E1000_AUXSTMPL1);
+		sec  = rd32(E1000_AUXSTMPH1);
+		event.type = PTP_CLOCK_EXTTS;
+		event.index = 1;
+		event.timestamp = sec * 1000000000ULL + nsec;
+		ptp_clock_event(adapter->ptp_clock, &event);
+		ack |= TSINTR_AUTT1;
+	}
+
 	/* acknowledge the interrupts */
 	wr32(E1000_TSICR, ack);
 }
diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index 98c58d9..d20fc8e 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -355,16 +355,204 @@ static int igb_ptp_settime_i210(struct ptp_clock_info *ptp,
 	return 0;
 }
 
+static void igb_pin_direction(int pin, int input, u32 *ctrl, u32 *ctrl_ext)
+{
+	u32 *ptr = pin < 2 ? ctrl : ctrl_ext;
+	u32 mask[IGB_N_SDP] = {
+		E1000_CTRL_SDP0_DIR,
+		E1000_CTRL_SDP1_DIR,
+		E1000_CTRL_EXT_SDP2_DIR,
+		E1000_CTRL_EXT_SDP3_DIR,
+	};
+
+	if (input)
+		*ptr &= ~mask[pin];
+	else
+		*ptr |= mask[pin];
+}
+
+static void igb_pin_extts(struct igb_adapter *igb, int chan, int pin)
+{
+	struct e1000_hw *hw = &igb->hw;
+	u32 aux0_sel_sdp[IGB_N_SDP] = {
+		AUX0_SEL_SDP0, AUX0_SEL_SDP1, AUX0_SEL_SDP2, AUX0_SEL_SDP3,
+	};
+	u32 aux1_sel_sdp[IGB_N_SDP] = {
+		AUX1_SEL_SDP0, AUX1_SEL_SDP1, AUX1_SEL_SDP2, AUX1_SEL_SDP3,
+	};
+	u32 ts_sdp_en[IGB_N_SDP] = {
+		TS_SDP0_EN, TS_SDP1_EN, TS_SDP2_EN, TS_SDP3_EN,
+	};
+	u32 ctrl, ctrl_ext, tssdp = 0;
+
+	ctrl = rd32(E1000_CTRL);
+	ctrl_ext = rd32(E1000_CTRL_EXT);
+	tssdp = rd32(E1000_TSSDP);
+
+	igb_pin_direction(pin, 1, &ctrl, &ctrl_ext);
+
+	/* Make sure this pin is not enabled as an output. */
+	tssdp &= ~ts_sdp_en[pin];
+
+	if (chan == 1) {
+		tssdp &= ~AUX1_SEL_SDP3;
+		tssdp |= aux1_sel_sdp[pin] | AUX1_TS_SDP_EN;
+	} else {
+		tssdp &= ~AUX0_SEL_SDP3;
+		tssdp |= aux0_sel_sdp[pin] | AUX0_TS_SDP_EN;
+	}
+
+	wr32(E1000_TSSDP, tssdp);
+	wr32(E1000_CTRL, ctrl);
+	wr32(E1000_CTRL_EXT, ctrl_ext);
+}
+
+static void igb_pin_perout(struct igb_adapter *igb, int chan, int pin)
+{
+	struct e1000_hw *hw = &igb->hw;
+	u32 aux0_sel_sdp[IGB_N_SDP] = {
+		AUX0_SEL_SDP0, AUX0_SEL_SDP1, AUX0_SEL_SDP2, AUX0_SEL_SDP3,
+	};
+	u32 aux1_sel_sdp[IGB_N_SDP] = {
+		AUX1_SEL_SDP0, AUX1_SEL_SDP1, AUX1_SEL_SDP2, AUX1_SEL_SDP3,
+	};
+	u32 ts_sdp_en[IGB_N_SDP] = {
+		TS_SDP0_EN, TS_SDP1_EN, TS_SDP2_EN, TS_SDP3_EN,
+	};
+	u32 ts_sdp_sel_tt0[IGB_N_SDP] = {
+		TS_SDP0_SEL_TT0, TS_SDP1_SEL_TT0,
+		TS_SDP2_SEL_TT0, TS_SDP3_SEL_TT0,
+	};
+	u32 ts_sdp_sel_tt1[IGB_N_SDP] = {
+		TS_SDP0_SEL_TT1, TS_SDP1_SEL_TT1,
+		TS_SDP2_SEL_TT1, TS_SDP3_SEL_TT1,
+	};
+	u32 ts_sdp_sel_clr[IGB_N_SDP] = {
+		TS_SDP0_SEL_FC1, TS_SDP1_SEL_FC1,
+		TS_SDP2_SEL_FC1, TS_SDP3_SEL_FC1,
+	};
+	u32 ctrl, ctrl_ext, tssdp = 0;
+
+	ctrl = rd32(E1000_CTRL);
+	ctrl_ext = rd32(E1000_CTRL_EXT);
+	tssdp = rd32(E1000_TSSDP);
+
+	igb_pin_direction(pin, 0, &ctrl, &ctrl_ext);
+
+	/* Make sure this pin is not enabled as an input. */
+	if ((tssdp & AUX0_SEL_SDP3) == aux0_sel_sdp[pin])
+		tssdp &= ~AUX0_TS_SDP_EN;
+
+	if ((tssdp & AUX1_SEL_SDP3) == aux1_sel_sdp[pin])
+		tssdp &= ~AUX1_TS_SDP_EN;
+
+	tssdp &= ~ts_sdp_sel_clr[pin];
+	if (chan == 1)
+		tssdp |= ts_sdp_sel_tt1[pin];
+	else
+		tssdp |= ts_sdp_sel_tt0[pin];
+
+	tssdp |= ts_sdp_en[pin];
+
+	wr32(E1000_TSSDP, tssdp);
+	wr32(E1000_CTRL, ctrl);
+	wr32(E1000_CTRL_EXT, ctrl_ext);
+}
+
 static int igb_ptp_feature_enable_i210(struct ptp_clock_info *ptp,
 				       struct ptp_clock_request *rq, int on)
 {
 	struct igb_adapter *igb =
 		container_of(ptp, struct igb_adapter, ptp_caps);
 	struct e1000_hw *hw = &igb->hw;
+	u32 tsauxc, tsim, tsauxc_mask, tsim_mask, trgttiml, trgttimh;
 	unsigned long flags;
-	u32 tsim;
+	struct timespec ts;
+	int pin;
+	s64 ns;
 
 	switch (rq->type) {
+	case PTP_CLK_REQ_EXTTS:
+		if (on) {
+			pin = ptp_find_pin(igb->ptp_clock, PTP_PF_EXTTS,
+					   rq->extts.index);
+			if (pin < 0)
+				return -EBUSY;
+		}
+		if (rq->extts.index == 1) {
+			tsauxc_mask = TSAUXC_EN_TS1;
+			tsim_mask = TSINTR_AUTT1;
+		} else {
+			tsauxc_mask = TSAUXC_EN_TS0;
+			tsim_mask = TSINTR_AUTT0;
+		}
+		spin_lock_irqsave(&igb->tmreg_lock, flags);
+		tsauxc = rd32(E1000_TSAUXC);
+		tsim = rd32(E1000_TSIM);
+		if (on) {
+			igb_pin_extts(igb, rq->extts.index, pin);
+			tsauxc |= tsauxc_mask;
+			tsim |= tsim_mask;
+		} else {
+			tsauxc &= ~tsauxc_mask;
+			tsim &= ~tsim_mask;
+		}
+		wr32(E1000_TSAUXC, tsauxc);
+		wr32(E1000_TSIM, tsim);
+		spin_unlock_irqrestore(&igb->tmreg_lock, flags);
+		return 0;
+
+	case PTP_CLK_REQ_PEROUT:
+		if (on) {
+			pin = ptp_find_pin(igb->ptp_clock, PTP_PF_PEROUT,
+					   rq->perout.index);
+			if (pin < 0)
+				return -EBUSY;
+		}
+		ts.tv_sec = rq->perout.period.sec;
+		ts.tv_nsec = rq->perout.period.nsec;
+		ns = timespec_to_ns(&ts);
+		ns = ns >> 1;
+		if (on && ns < 500000LL) {
+			/* 2k interrupts per second is an awful lot. */
+			return -EINVAL;
+		}
+		ts = ns_to_timespec(ns);
+		if (rq->perout.index == 1) {
+			tsauxc_mask = TSAUXC_EN_TT1;
+			tsim_mask = TSINTR_TT1;
+			trgttiml = E1000_TRGTTIML1;
+			trgttimh = E1000_TRGTTIMH1;
+		} else {
+			tsauxc_mask = TSAUXC_EN_TT0;
+			tsim_mask = TSINTR_TT0;
+			trgttiml = E1000_TRGTTIML0;
+			trgttimh = E1000_TRGTTIMH0;
+		}
+		spin_lock_irqsave(&igb->tmreg_lock, flags);
+		tsauxc = rd32(E1000_TSAUXC);
+		tsim = rd32(E1000_TSIM);
+		if (on) {
+			int i = rq->perout.index;
+
+			igb_pin_perout(igb, i, pin);
+			igb->perout[i].start.tv_sec = rq->perout.start.sec;
+			igb->perout[i].start.tv_nsec = rq->perout.start.nsec;
+			igb->perout[i].period.tv_sec = ts.tv_sec;
+			igb->perout[i].period.tv_nsec = ts.tv_nsec;
+			wr32(trgttiml, rq->perout.start.sec);
+			wr32(trgttimh, rq->perout.start.nsec);
+			tsauxc |= tsauxc_mask;
+			tsim |= tsim_mask;
+		} else {
+			tsauxc &= ~tsauxc_mask;
+			tsim &= ~tsim_mask;
+		}
+		wr32(E1000_TSAUXC, tsauxc);
+		wr32(E1000_TSIM, tsim);
+		spin_unlock_irqrestore(&igb->tmreg_lock, flags);
+		return 0;
+
 	case PTP_CLK_REQ_PPS:
 		spin_lock_irqsave(&igb->tmreg_lock, flags);
 		tsim = rd32(E1000_TSIM);
@@ -375,9 +563,6 @@ static int igb_ptp_feature_enable_i210(struct ptp_clock_info *ptp,
 		wr32(E1000_TSIM, tsim);
 		spin_unlock_irqrestore(&igb->tmreg_lock, flags);
 		return 0;
-
-	default:
-		break;
 	}
 
 	return -EOPNOTSUPP;
@@ -389,6 +574,20 @@ static int igb_ptp_feature_enable(struct ptp_clock_info *ptp,
 	return -EOPNOTSUPP;
 }
 
+static int igb_ptp_verify_pin(struct ptp_clock_info *ptp, unsigned int pin,
+			      enum ptp_pin_function func, unsigned int chan)
+{
+	switch (func) {
+	case PTP_PF_NONE:
+	case PTP_PF_EXTTS:
+	case PTP_PF_PEROUT:
+		break;
+	case PTP_PF_PHYSYNC:
+		return -1;
+	}
+	return 0;
+}
+
 /**
  * igb_ptp_tx_work
  * @work: pointer to work struct
@@ -779,6 +978,7 @@ void igb_ptp_init(struct igb_adapter *adapter)
 {
 	struct e1000_hw *hw = &adapter->hw;
 	struct net_device *netdev = adapter->netdev;
+	int i;
 
 	switch (hw->mac.type) {
 	case e1000_82576:
@@ -821,16 +1021,27 @@ void igb_ptp_init(struct igb_adapter *adapter)
 		break;
 	case e1000_i210:
 	case e1000_i211:
+		for (i = 0; i < IGB_N_SDP; i++) {
+			struct ptp_pin_desc *ppd = &adapter->sdp_config[i];
+
+			snprintf(ppd->name, sizeof(ppd->name), "SDP%d", i);
+			ppd->index = i;
+			ppd->func = PTP_PF_NONE;
+		}
 		snprintf(adapter->ptp_caps.name, 16, "%pm", netdev->dev_addr);
 		adapter->ptp_caps.owner = THIS_MODULE;
 		adapter->ptp_caps.max_adj = 62499999;
-		adapter->ptp_caps.n_ext_ts = 0;
+		adapter->ptp_caps.n_ext_ts = IGB_N_EXTTS;
+		adapter->ptp_caps.n_per_out = IGB_N_PEROUT;
+		adapter->ptp_caps.n_pins = IGB_N_SDP;
 		adapter->ptp_caps.pps = 1;
+		adapter->ptp_caps.pin_config = adapter->sdp_config;
 		adapter->ptp_caps.adjfreq = igb_ptp_adjfreq_82580;
 		adapter->ptp_caps.adjtime = igb_ptp_adjtime_i210;
 		adapter->ptp_caps.gettime = igb_ptp_gettime_i210;
 		adapter->ptp_caps.settime = igb_ptp_settime_i210;
 		adapter->ptp_caps.enable = igb_ptp_feature_enable_i210;
+		adapter->ptp_caps.verify = igb_ptp_verify_pin;
 		/* Enable the timer functions by clearing bit 31. */
 		wr32(E1000_TSAUXC, 0x0);
 		break;
@@ -949,6 +1160,7 @@ void igb_ptp_reset(struct igb_adapter *adapter)
 	case e1000_i210:
 	case e1000_i211:
 		wr32(E1000_TSAUXC, 0x0);
+		wr32(E1000_TSSDP, 0x0);
 		wr32(E1000_TSIM, TSYNC_INTERRUPTS);
 		wr32(E1000_IMS, E1000_IMS_TS);
 		break;
-- 
1.9.3

^ permalink raw reply related

* [net-next 08/15] igb: enable internal PPS for the i210
From: Jeff Kirsher @ 2015-01-23  2:37 UTC (permalink / raw)
  To: davem; +Cc: Richard Cochran, netdev, nhorman, sassmann, jogreene,
	Jeff Kirsher
In-Reply-To: <1421980631-1955-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Richard Cochran <richardcochran@gmail.com>

The i210 device can produce an interrupt on the full second. This
patch allows using this interrupt to generate an internal PPS event
for adjusting the kernel system time.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_main.c | 18 ++++++++++++++---
 drivers/net/ethernet/intel/igb/igb_ptp.c  | 32 +++++++++++++++++++++++++++++--
 2 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 135ac5c..e844162 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -5387,14 +5387,26 @@ void igb_update_stats(struct igb_adapter *adapter,
 static void igb_tsync_interrupt(struct igb_adapter *adapter)
 {
 	struct e1000_hw *hw = &adapter->hw;
-	u32 tsicr = rd32(E1000_TSICR);
+	struct ptp_clock_event event;
+	u32 ack = 0, tsicr = rd32(E1000_TSICR);
+
+	if (tsicr & TSINTR_SYS_WRAP) {
+		event.type = PTP_CLOCK_PPS;
+		if (adapter->ptp_caps.pps)
+			ptp_clock_event(adapter->ptp_clock, &event);
+		else
+			dev_err(&adapter->pdev->dev, "unexpected SYS WRAP");
+		ack |= TSINTR_SYS_WRAP;
+	}
 
 	if (tsicr & E1000_TSICR_TXTS) {
-		/* acknowledge the interrupt */
-		wr32(E1000_TSICR, E1000_TSICR_TXTS);
 		/* retrieve hardware timestamp */
 		schedule_work(&adapter->ptp_tx_work);
+		ack |= E1000_TSICR_TXTS;
 	}
+
+	/* acknowledge the interrupts */
+	wr32(E1000_TSICR, ack);
 }
 
 static irqreturn_t igb_msix_other(int irq, void *data)
diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index 8389bb4..98c58d9 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -355,6 +355,34 @@ static int igb_ptp_settime_i210(struct ptp_clock_info *ptp,
 	return 0;
 }
 
+static int igb_ptp_feature_enable_i210(struct ptp_clock_info *ptp,
+				       struct ptp_clock_request *rq, int on)
+{
+	struct igb_adapter *igb =
+		container_of(ptp, struct igb_adapter, ptp_caps);
+	struct e1000_hw *hw = &igb->hw;
+	unsigned long flags;
+	u32 tsim;
+
+	switch (rq->type) {
+	case PTP_CLK_REQ_PPS:
+		spin_lock_irqsave(&igb->tmreg_lock, flags);
+		tsim = rd32(E1000_TSIM);
+		if (on)
+			tsim |= TSINTR_SYS_WRAP;
+		else
+			tsim &= ~TSINTR_SYS_WRAP;
+		wr32(E1000_TSIM, tsim);
+		spin_unlock_irqrestore(&igb->tmreg_lock, flags);
+		return 0;
+
+	default:
+		break;
+	}
+
+	return -EOPNOTSUPP;
+}
+
 static int igb_ptp_feature_enable(struct ptp_clock_info *ptp,
 				  struct ptp_clock_request *rq, int on)
 {
@@ -797,12 +825,12 @@ void igb_ptp_init(struct igb_adapter *adapter)
 		adapter->ptp_caps.owner = THIS_MODULE;
 		adapter->ptp_caps.max_adj = 62499999;
 		adapter->ptp_caps.n_ext_ts = 0;
-		adapter->ptp_caps.pps = 0;
+		adapter->ptp_caps.pps = 1;
 		adapter->ptp_caps.adjfreq = igb_ptp_adjfreq_82580;
 		adapter->ptp_caps.adjtime = igb_ptp_adjtime_i210;
 		adapter->ptp_caps.gettime = igb_ptp_gettime_i210;
 		adapter->ptp_caps.settime = igb_ptp_settime_i210;
-		adapter->ptp_caps.enable = igb_ptp_feature_enable;
+		adapter->ptp_caps.enable = igb_ptp_feature_enable_i210;
 		/* Enable the timer functions by clearing bit 31. */
 		wr32(E1000_TSAUXC, 0x0);
 		break;
-- 
1.9.3

^ permalink raw reply related

* [net-next 10/15] net/fm10k: Avoid double setting of NETIF_F_SG for the HW encapsulation feature mask
From: Jeff Kirsher @ 2015-01-23  2:37 UTC (permalink / raw)
  To: davem; +Cc: Or Gerlitz, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <1421980631-1955-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Or Gerlitz <ogerlitz@mellanox.com>

The networking core does it for the driver during registration time.

Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
Acked-by: Matthew Vick <matthew.vick@intel.com>
Tested-by: Krishneil Singh <Krishneil.k.singh@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
index 945b35d..cfde8ba 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
@@ -1414,13 +1414,12 @@ struct net_device *fm10k_alloc_netdev(void)
 	dev->vlan_features |= dev->features;
 
 	/* configure tunnel offloads */
-	dev->hw_enc_features = NETIF_F_IP_CSUM |
-			       NETIF_F_TSO |
-			       NETIF_F_TSO6 |
-			       NETIF_F_TSO_ECN |
-			       NETIF_F_GSO_UDP_TUNNEL |
-			       NETIF_F_IPV6_CSUM |
-			       NETIF_F_SG;
+	dev->hw_enc_features |= NETIF_F_IP_CSUM |
+				NETIF_F_TSO |
+				NETIF_F_TSO6 |
+				NETIF_F_TSO_ECN |
+				NETIF_F_GSO_UDP_TUNNEL |
+				NETIF_F_IPV6_CSUM;
 
 	/* we want to leave these both on as we cannot disable VLAN tag
 	 * insertion or stripping on the hardware since it is contained
-- 
1.9.3

^ permalink raw reply related

* [net-next 12/15] fm10k: Increase the timeout for the data path reset
From: Jeff Kirsher @ 2015-01-23  2:37 UTC (permalink / raw)
  To: davem; +Cc: Matthew Vick, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <1421980631-1955-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Matthew Vick <matthew.vick@intel.com>

Based on feedback from the hardware team, 100us is too short of a time
to wait for the data path reset to complete and the recommendation is to
increase this timeout to 150us.

Signed-off-by: Matthew Vick <matthew.vick@intel.com>
Tested-by: Krishneil Singh <Krishneil.k.singh@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_type.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_type.h b/drivers/net/ethernet/intel/fm10k/fm10k_type.h
index 280296f..7c6d9d5 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_type.h
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_type.h
@@ -354,7 +354,7 @@ struct fm10k_hw;
 
 /* Define timeouts for resets and disables */
 #define FM10K_QUEUE_DISABLE_TIMEOUT		100
-#define FM10K_RESET_TIMEOUT			100
+#define FM10K_RESET_TIMEOUT			150
 
 /* VF registers */
 #define FM10K_VFCTRL		0x00000
-- 
1.9.3

^ permalink raw reply related

* [net-next 11/15] fm10k: Check tunnel header length in encap offload
From: Jeff Kirsher @ 2015-01-23  2:37 UTC (permalink / raw)
  To: davem; +Cc: Joe Stringer, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <1421980631-1955-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Joe Stringer <joestringer@nicira.com>

fm10k supports up to 184 bytes of inner+outer headers. Add an initial
check to fail encap offload if these are too large.

Signed-off-by: Joe Stringer <joestringer@nicira.com>
Tested-by: Krishneil Singh <Krishneil.k.singh@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_main.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_main.c b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
index c7a19a5..84ab9ee 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_main.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
@@ -727,6 +727,12 @@ static __be16 fm10k_tx_encap_offload(struct sk_buff *skb)
 	struct ethhdr *eth_hdr;
 	u8 l4_hdr = 0;
 
+/* fm10k supports 184 octets of outer+inner headers. Minus 20 for inner L4. */
+#define FM10K_MAX_ENCAP_TRANSPORT_OFFSET	164
+	if (skb_inner_transport_header(skb) - skb_mac_header(skb) >
+	    FM10K_MAX_ENCAP_TRANSPORT_OFFSET)
+		return 0;
+
 	switch (vlan_get_protocol(skb)) {
 	case htons(ETH_P_IP):
 		l4_hdr = ip_hdr(skb)->protocol;
-- 
1.9.3

^ permalink raw reply related

* [net-next 13/15] igb: Indicate failure on vf reset for empty mac address
From: Jeff Kirsher @ 2015-01-23  2:37 UTC (permalink / raw)
  To: davem; +Cc: Alexander Graf, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <1421980631-1955-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Alexander Graf <agraf@suse.de>

Commit 5ac6f91d changed the igb driver to expose a zero (empty) mac
address to the VF on reset rather than a random one.

However, that behavioral change also requires igbvf driver changes
which can be hard especially when we want to talk to proprietary
guest OSs.

Looking at the code previous to the commit in Linux that made igbvf
work with empty mac addresses (8d56b6d), we can see that on reset
failure the driver will try to generate a new mac address with both
the old and the new code.

Furthermore, ixgbe does send reset failure when it detects an empty
mac address (35055928c).

So I think it's safe to make igb behave the same. With this patch I
can successfully run a Windows 8.1 guest with an empty mac address
and an assigned igbvf device that has no mac address set by the host.

If anyone is aware of a guest driver that chokes on NACK returns of
VF RESET commands, please speak up.

Signed-off-by: Alexander Graf <agraf@suse.de>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_main.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 6e65449..f366b3b 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -6077,8 +6077,12 @@ static void igb_vf_reset_msg(struct igb_adapter *adapter, u32 vf)
 	adapter->vf_data[vf].flags |= IGB_VF_FLAG_CTS;
 
 	/* reply to reset with ack and vf mac address */
-	msgbuf[0] = E1000_VF_RESET | E1000_VT_MSGTYPE_ACK;
-	memcpy(addr, vf_mac, ETH_ALEN);
+	if (!is_zero_ether_addr(vf_mac)) {
+		msgbuf[0] = E1000_VF_RESET | E1000_VT_MSGTYPE_ACK;
+		memcpy(addr, vf_mac, ETH_ALEN);
+	} else {
+		msgbuf[0] = E1000_VF_RESET | E1000_VT_MSGTYPE_NACK;
+	}
 	igb_write_mbx(hw, msgbuf, 3, vf);
 }
 
-- 
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