Netdev List
 help / color / mirror / Atom feed
* Re: [RFC PATCH v2 net-next 2/7] tcp: Merge tx_flags/tskey/txstamp_ack in tcp_collapse_retrans
From: Martin KaFai Lau @ 2016-04-19 17:28 UTC (permalink / raw)
  To: Soheil Hassas Yeganeh
  Cc: netdev, Eric Dumazet, Neal Cardwell, Soheil Hassas Yeganeh,
	Willem de Bruijn, Yuchung Cheng, Kernel Team
In-Reply-To: <CACSApvY5ZdyUO1i2O0KPGJM5meWHFv3hdi9Ew89CH--jFV4yOw@mail.gmail.com>

On Tue, Apr 19, 2016 at 01:32:14AM -0400, Soheil Hassas Yeganeh wrote:
> > +               TCP_SKB_CB(skb)->txstamp_ack =
> > +                       !!(shinfo->tx_flags & SKBTX_ACK_TSTAMP);
>
> Maybe we can skip a conditional jump here (because of !!), by simply
> using the cached bit in next_skb:
> TCP_SKB_CB(skb)->txstamp_ack = TCP_SKB_CB(next_skb)->txstamp_ack;
Recall the tx_flags are merged/combined (and so should be the txstamp_ack).
Would there be a case that TCP_SKB_CB(skb)->txstamp_ack is 1 and
TCP_SKB_CB(next_skb)->txstamp_ack is 0?

I can change it like the following which may help in showing the intention:
if (TCP_SKB_CB(next_skb)->txstamp_ack)
	TCP_SKB_CB(skb)->txstamp_ack = 1;

A bit off topic, I feel like the SKBTX_ACK_TSTAMP and txstamp_ack are sort
of redundant but I have not look into the details yet, so not completely
sure.  It wwould be a separate cleanup patch if it is the case.

^ permalink raw reply

* Re: [RFC PATCH v2 net-next 2/7] tcp: Merge tx_flags/tskey/txstamp_ack in tcp_collapse_retrans
From: Eric Dumazet @ 2016-04-19 17:35 UTC (permalink / raw)
  To: Martin KaFai Lau
  Cc: Soheil Hassas Yeganeh, netdev, Neal Cardwell,
	Soheil Hassas Yeganeh, Willem de Bruijn, Yuchung Cheng,
	Kernel Team
In-Reply-To: <20160419172837.GA37697@kafai-mba.local>

On Tue, Apr 19, 2016 at 10:28 AM, Martin KaFai Lau <kafai@fb.com> wrote:

> A bit off topic, I feel like the SKBTX_ACK_TSTAMP and txstamp_ack are sort
> of redundant but I have not look into the details yet, so not completely
> sure.  It wwould be a separate cleanup patch if it is the case.

Please read 6b084928baac562ed61866f540a96120e9c9ddb7 changelog ;)

A cache line miss avoidance is critical

^ permalink raw reply

* [PATCH net 0/5] macsec: a few fixes
From: Sabrina Dubroca @ 2016-04-19 17:36 UTC (permalink / raw)
  To: netdev; +Cc: Hannes Frederic Sowa, Johannes Berg, Dan Carpenter,
	Sabrina Dubroca

Some small fixes for the macsec driver:
 - possible NULL pointer dereferences
 - netlink dumps fixes: RTNL locking, consistent dumps
 - a reference counting bug

Sabrina Dubroca (5):
  macsec: add missing NULL check after kmalloc
  macsec: take rtnl lock before for_each_netdev
  macsec: don't put a NULL rxsa
  macsec: fix rx_sa refcounting with decrypt callback
  macsec: add consistency check to netlink dumps

 drivers/net/macsec.c | 39 +++++++++++++++++++++++++--------------
 1 file changed, 25 insertions(+), 14 deletions(-)

-- 
2.8.0

^ permalink raw reply

* [PATCH net 1/5] macsec: add missing NULL check after kmalloc
From: Sabrina Dubroca @ 2016-04-19 17:36 UTC (permalink / raw)
  To: netdev; +Cc: Hannes Frederic Sowa, Johannes Berg, Dan Carpenter,
	Sabrina Dubroca
In-Reply-To: <cover.1460967681.git.sd@queasysnail.net>

Fixes: c09440f7dcb3 ("macsec: introduce IEEE 802.1AE driver")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 drivers/net/macsec.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index 84d3e5ca8817..f691030ee3df 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -1622,8 +1622,8 @@ static int macsec_add_rxsa(struct sk_buff *skb, struct genl_info *info)
 	}
 
 	rx_sa = kmalloc(sizeof(*rx_sa), GFP_KERNEL);
-	if (init_rx_sa(rx_sa, nla_data(tb_sa[MACSEC_SA_ATTR_KEY]), secy->key_len,
-		       secy->icv_len)) {
+	if (!rx_sa || init_rx_sa(rx_sa, nla_data(tb_sa[MACSEC_SA_ATTR_KEY]),
+				 secy->key_len, secy->icv_len)) {
 		rtnl_unlock();
 		return -ENOMEM;
 	}
-- 
2.8.0

^ permalink raw reply related

* [PATCH net 2/5] macsec: take rtnl lock before for_each_netdev
From: Sabrina Dubroca @ 2016-04-19 17:36 UTC (permalink / raw)
  To: netdev; +Cc: Hannes Frederic Sowa, Johannes Berg, Dan Carpenter,
	Sabrina Dubroca
In-Reply-To: <cover.1460967681.git.sd@queasysnail.net>

Fixes: c09440f7dcb3 ("macsec: introduce IEEE 802.1AE driver")
Reported-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 drivers/net/macsec.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index f691030ee3df..5f3ea8026074 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -2268,8 +2268,6 @@ static int dump_secy(struct macsec_secy *secy, struct net_device *dev,
 	if (!hdr)
 		return -EMSGSIZE;
 
-	rtnl_lock();
-
 	if (nla_put_u32(skb, MACSEC_ATTR_IFINDEX, dev->ifindex))
 		goto nla_put_failure;
 
@@ -2429,14 +2427,11 @@ static int dump_secy(struct macsec_secy *secy, struct net_device *dev,
 
 	nla_nest_end(skb, rxsc_list);
 
-	rtnl_unlock();
-
 	genlmsg_end(skb, hdr);
 
 	return 0;
 
 nla_put_failure:
-	rtnl_unlock();
 	genlmsg_cancel(skb, hdr);
 	return -EMSGSIZE;
 }
@@ -2450,6 +2445,7 @@ static int macsec_dump_txsc(struct sk_buff *skb, struct netlink_callback *cb)
 	dev_idx = cb->args[0];
 
 	d = 0;
+	rtnl_lock();
 	for_each_netdev(net, dev) {
 		struct macsec_secy *secy;
 
@@ -2467,6 +2463,7 @@ next:
 	}
 
 done:
+	rtnl_unlock();
 	cb->args[0] = d;
 	return skb->len;
 }
-- 
2.8.0

^ permalink raw reply related

* [PATCH net 3/5] macsec: don't put a NULL rxsa
From: Sabrina Dubroca @ 2016-04-19 17:36 UTC (permalink / raw)
  To: netdev; +Cc: Hannes Frederic Sowa, Johannes Berg, Dan Carpenter,
	Sabrina Dubroca
In-Reply-To: <cover.1460967681.git.sd@queasysnail.net>

The "deliver:" path of macsec_handle_frame can be called with
rx_sa == NULL.  Check rx_sa != NULL before calling macsec_rxsa_put().

Fixes: c09440f7dcb3 ("macsec: introduce IEEE 802.1AE driver")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 drivers/net/macsec.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index 5f3ea8026074..2a2136b7d324 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -1161,7 +1161,8 @@ deliver:
 			    macsec_extra_len(macsec_skb_cb(skb)->has_sci));
 	macsec_reset_skb(skb, secy->netdev);
 
-	macsec_rxsa_put(rx_sa);
+	if (rx_sa)
+		macsec_rxsa_put(rx_sa);
 	count_rx(dev, skb->len);
 
 	rcu_read_unlock();
-- 
2.8.0

^ permalink raw reply related

* [PATCH net 4/5] macsec: fix rx_sa refcounting with decrypt callback
From: Sabrina Dubroca @ 2016-04-19 17:36 UTC (permalink / raw)
  To: netdev; +Cc: Hannes Frederic Sowa, Johannes Berg, Dan Carpenter,
	Sabrina Dubroca
In-Reply-To: <cover.1460967681.git.sd@queasysnail.net>

The decrypt callback macsec_decrypt_done needs a reference on the rx_sa
and releases it before returning, but macsec_handle_frame already
put that reference after macsec_decrypt returned NULL.

Change macsec_decrypt to return error codes, and use -EINPROGRESS as
an indication that macsec_handle_frame must not release the reference.

Fixes: c09440f7dcb3 ("macsec: introduce IEEE 802.1AE driver")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 drivers/net/macsec.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index 2a2136b7d324..1fd2b147fda1 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -880,12 +880,12 @@ static struct sk_buff *macsec_decrypt(struct sk_buff *skb,
 	macsec_skb_cb(skb)->valid = false;
 	skb = skb_share_check(skb, GFP_ATOMIC);
 	if (!skb)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 
 	req = aead_request_alloc(rx_sa->key.tfm, GFP_ATOMIC);
 	if (!req) {
 		kfree_skb(skb);
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 	}
 
 	hdr = (struct macsec_eth_header *)skb->data;
@@ -905,7 +905,7 @@ static struct sk_buff *macsec_decrypt(struct sk_buff *skb,
 		skb = skb_unshare(skb, GFP_ATOMIC);
 		if (!skb) {
 			aead_request_free(req);
-			return NULL;
+			return ERR_PTR(-ENOMEM);
 		}
 	} else {
 		/* integrity only: all headers + data authenticated */
@@ -921,14 +921,14 @@ static struct sk_buff *macsec_decrypt(struct sk_buff *skb,
 	dev_hold(dev);
 	ret = crypto_aead_decrypt(req);
 	if (ret == -EINPROGRESS) {
-		return NULL;
+		return ERR_PTR(ret);
 	} else if (ret != 0) {
 		/* decryption/authentication failed
 		 * 10.6 if validateFrames is disabled, deliver anyway
 		 */
 		if (ret != -EBADMSG) {
 			kfree_skb(skb);
-			skb = NULL;
+			skb = ERR_PTR(ret);
 		}
 	} else {
 		macsec_skb_cb(skb)->valid = true;
@@ -1146,8 +1146,10 @@ static rx_handler_result_t macsec_handle_frame(struct sk_buff **pskb)
 	    secy->validate_frames != MACSEC_VALIDATE_DISABLED)
 		skb = macsec_decrypt(skb, dev, rx_sa, sci, secy);
 
-	if (!skb) {
-		macsec_rxsa_put(rx_sa);
+	if (IS_ERR(skb)) {
+		/* the decrypt callback needs the reference */
+		if (PTR_ERR(skb) != -EINPROGRESS)
+			macsec_rxsa_put(rx_sa);
 		rcu_read_unlock();
 		*pskb = NULL;
 		return RX_HANDLER_CONSUMED;
-- 
2.8.0

^ permalink raw reply related

* [PATCH net 5/5] macsec: add consistency check to netlink dumps
From: Sabrina Dubroca @ 2016-04-19 17:36 UTC (permalink / raw)
  To: netdev; +Cc: Hannes Frederic Sowa, Johannes Berg, Dan Carpenter,
	Sabrina Dubroca
In-Reply-To: <cover.1460967681.git.sd@queasysnail.net>

Use genl_dump_check_consistent in dump_secy.

Fixes: c09440f7dcb3 ("macsec: introduce IEEE 802.1AE driver")
Suggested-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 drivers/net/macsec.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index 1fd2b147fda1..41fbe556ba6d 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -2271,6 +2271,8 @@ static int dump_secy(struct macsec_secy *secy, struct net_device *dev,
 	if (!hdr)
 		return -EMSGSIZE;
 
+	genl_dump_check_consistent(cb, hdr, &macsec_fam);
+
 	if (nla_put_u32(skb, MACSEC_ATTR_IFINDEX, dev->ifindex))
 		goto nla_put_failure;
 
@@ -2439,6 +2441,8 @@ nla_put_failure:
 	return -EMSGSIZE;
 }
 
+static int macsec_generation = 1; /* protected by RTNL */
+
 static int macsec_dump_txsc(struct sk_buff *skb, struct netlink_callback *cb)
 {
 	struct net *net = sock_net(skb->sk);
@@ -2449,6 +2453,9 @@ static int macsec_dump_txsc(struct sk_buff *skb, struct netlink_callback *cb)
 
 	d = 0;
 	rtnl_lock();
+
+	cb->seq = macsec_generation;
+
 	for_each_netdev(net, dev) {
 		struct macsec_secy *secy;
 
@@ -2920,6 +2927,8 @@ static void macsec_dellink(struct net_device *dev, struct list_head *head)
 	struct net_device *real_dev = macsec->real_dev;
 	struct macsec_rxh_data *rxd = macsec_data_rtnl(real_dev);
 
+	macsec_generation++;
+
 	unregister_netdevice_queue(dev, head);
 	list_del_rcu(&macsec->secys);
 	if (list_empty(&rxd->secys))
@@ -3066,6 +3075,8 @@ static int macsec_newlink(struct net *net, struct net_device *dev,
 	if (err < 0)
 		goto del_dev;
 
+	macsec_generation++;
+
 	dev_hold(real_dev);
 
 	return 0;
-- 
2.8.0

^ permalink raw reply related

* Re: [PATCH net-next V2 05/11] net/mlx5e: Support RX multi-packet WQE (Striding RQ)
From: Mel Gorman @ 2016-04-19 17:39 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: Eric Dumazet, Saeed Mahameed, Saeed Mahameed, David S. Miller,
	Linux Netdev List, Or Gerlitz, Tal Alon, Tariq Toukan,
	Eran Ben Elisha, Achiad Shochat, linux-mm
In-Reply-To: <20160419182532.423d3c05@redhat.com>

On Tue, Apr 19, 2016 at 06:25:32PM +0200, Jesper Dangaard Brouer wrote:
> On Mon, 18 Apr 2016 07:17:13 -0700
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
> 
> > On Mon, 2016-04-18 at 16:05 +0300, Saeed Mahameed wrote:
> > > On Mon, Apr 18, 2016 at 3:48 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:  
> > > > On Sun, 2016-04-17 at 17:29 -0700, Eric Dumazet wrote:
> > > >  
> > > >>
> > > >> If really you need to allocate physically contiguous memory, have you
> > > >> considered converting the order-5 pages into 32 order-0 ones ?  
> > > >
> > > > Search for split_page() call sites for examples.
> > > >
> > > >  
> > > 
> > > Thanks Eric, we are already evaluating split_page as we speak.
> > > 
> > > We did look but could not find any specific alloc_pages API that

alloc_pages_exact()

> > > allocates many physically contiguous pages with order0 ! so we assume
> > > it is ok to use split_page.  
> > 
> > Note: I have no idea of split_page() performance :
> 
> Maybe Mel knows?

Irrelevant in comparison to the cost of allocating an order-5 pages if
one is not already available.

> And maybe Mel have an opinion about if this is a good
> or bad approach, e.g. will this approach stress the page allocator in a
> bad way?
> 

It'll contend on the zone lock minimally but again, irrelevant in
comparison to having to reclaim/compact an order-5 page if one is not
already free.

It'll appear to work well in benchmarks and then fall apart when the
system is running for long enough.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [RFC PATCH net-next 2/8] sfc: batch up RX delivery on EF10
From: Edward Cree @ 2016-04-19 17:42 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: netdev, David Miller, Jesper Dangaard Brouer, linux-net-drivers
In-Reply-To: <1461086440.10638.208.camel@edumazet-glaptop3.roam.corp.google.com>

On 19/04/16 18:20, Eric Dumazet wrote:
> It seems all the discussions about fast kernel networking these days is
> adding yet another queues, code duplication and complexity, batches, and
> add latencies, on top of a single NIC RX queue.
>
> Apparently the multiqueue nature of a NIC is obsolete and people want to
> process 10+Mpps on a single queue.
The real goal here is to speed up packet processing generally.
Doing everything on a single queue (and thus a single CPU) is just a
convenient way of measuring that, while making sure performance is limited
by the RX side rather than the TX not being able to generate enough packets
to keep us busy.  Similarly, measuring single-byte UDP packet rate with one
CPU running flat-out is easier than measuring CPU usage while receiving
line-rate TCP in 1400-byte chunks.

> We could probably get ~100% improvement in UDP if we really cared, just
> by changing net/ipv[46]/udp.c, not changing other layers.
Well, I don't know how to achieve that, but it sounds like you do, so why
not go ahead and show us ;)
If you submitted a patch series to make UDP twice as fast, I think people
would "really care" about an improvement of that magnitude.

But RX batching should speed up all traffic, not just UDP.  Or at least,
that's the theory.

-Ed

^ permalink raw reply

* Re: [RFC PATCH v2 net-next 1/7] tcp: Carry txstamp_ack in tcp_fragment_tstamp
From: Martin KaFai Lau @ 2016-04-19 17:39 UTC (permalink / raw)
  To: Soheil Hassas Yeganeh
  Cc: netdev, Eric Dumazet, Neal Cardwell, Soheil Hassas Yeganeh,
	Willem de Bruijn, Yuchung Cheng, Kernel Team
In-Reply-To: <CACSApvaRU64xz0N8CVdZrQ__i9MS2aAh5X1kKwhjdv3hz-OU9w@mail.gmail.com>

On Tue, Apr 19, 2016 at 01:21:04AM -0400, Soheil Hassas Yeganeh wrote:
> Could you please submit the timestamping patches separately as non RFCs? Thanks!
Agree.  I will re-spin.

^ permalink raw reply

* Re: [RFC PATCH v2 net-next 2/7] tcp: Merge tx_flags/tskey/txstamp_ack in tcp_collapse_retrans
From: Soheil Hassas Yeganeh @ 2016-04-19 17:42 UTC (permalink / raw)
  To: Martin KaFai Lau
  Cc: netdev, Eric Dumazet, Neal Cardwell, Soheil Hassas Yeganeh,
	Willem de Bruijn, Yuchung Cheng, Kernel Team
In-Reply-To: <20160419172837.GA37697@kafai-mba.local>

On Tue, Apr 19, 2016 at 1:28 PM, Martin KaFai Lau <kafai@fb.com> wrote:
> On Tue, Apr 19, 2016 at 01:32:14AM -0400, Soheil Hassas Yeganeh wrote:
>> > +               TCP_SKB_CB(skb)->txstamp_ack =
>> > +                       !!(shinfo->tx_flags & SKBTX_ACK_TSTAMP);
>>
>> Maybe we can skip a conditional jump here (because of !!), by simply
>> using the cached bit in next_skb:
>> TCP_SKB_CB(skb)->txstamp_ack = TCP_SKB_CB(next_skb)->txstamp_ack;
> Recall the tx_flags are merged/combined (and so should be the txstamp_ack).

Oh sure, sorry, I missed an "or":

TCP_SKB_CB(skb)->txstamp_ack |= TCP_SKB_CB(next_skb)->txstamp_ack;

> Would there be a case that TCP_SKB_CB(skb)->txstamp_ack is 1 and
> TCP_SKB_CB(next_skb)->txstamp_ack is 0?
>
> I can change it like the following which may help in showing the intention:
> if (TCP_SKB_CB(next_skb)->txstamp_ack)
>         TCP_SKB_CB(skb)->txstamp_ack = 1;
>
> A bit off topic, I feel like the SKBTX_ACK_TSTAMP and txstamp_ack are sort
> of redundant but I have not look into the details yet, so not completely
> sure.  It wwould be a separate cleanup patch if it is the case.

As Eric mentioned, this is needed to avoid a cache-line miss in
accessing the shared info.

^ permalink raw reply

* Re: [PATCH v2 1/1] drivers: net: cpsw: Prevent NUll pointer dereference with two PHYs
From: Florian Fainelli @ 2016-04-19 17:44 UTC (permalink / raw)
  To: David Rivshin (Allworx), Grygorii Strashko, David S. Miller
  Cc: Andrew Goodbody, netdev, linux-kernel, linux-omap, mugunthanvnm,
	tony, andrew
In-Reply-To: <20160419131443.7a3b3ef7.drivshin.allworx@gmail.com>

On 19/04/16 10:14, David Rivshin (Allworx) wrote:
>> Ah Ok. There are no user of cpsw_platform_data outside of net/ethernet/ti/,
>> so yes, looks like your patch 1 does exactly what's needed.
> 
> Given that the v1 of Andrew's patch is already in Dave's net tree, and 
> would obviously have many conflicts with mine, how should I proceed? 
> Since you already requested Dave revert that patch, should I just wait 
> for that to happen and then resubmit my series? 
> 
> Dave, Does that sound good to you? 

At some point, it would be good for the cpsw driver to be moved to the
DSA subsystem, a lot of these tiny little details, like how to attach
the PHY to the internal, external, fixed MDIO bus have been solved
already using standard DT bindings. This leaves you with the fun part:
how to program your switch such that it works in response to the
networking stack sollicitations (bridge, VLAN, ethtool etc.).
-- 
Florian

^ permalink raw reply

* Re: [RFC PATCH v2 net-next 1/7] tcp: Carry txstamp_ack in tcp_fragment_tstamp
From: Soheil Hassas Yeganeh @ 2016-04-19 17:44 UTC (permalink / raw)
  To: Martin KaFai Lau
  Cc: netdev, Eric Dumazet, Neal Cardwell, Soheil Hassas Yeganeh,
	Willem de Bruijn, Yuchung Cheng, Kernel Team
In-Reply-To: <20160419173911.GB39131@kafai-mba.local>

On Tue, Apr 19, 2016 at 1:39 PM, Martin KaFai Lau <kafai@fb.com> wrote:
> On Tue, Apr 19, 2016 at 01:21:04AM -0400, Soheil Hassas Yeganeh wrote:
>> Could you please submit the timestamping patches separately as non RFCs? Thanks!
> Agree.  I will re-spin.

Great, thank you very much!

^ permalink raw reply

* Re: [PATCH net 1/5] macsec: add missing NULL check after kmalloc
From: Lance Richardson @ 2016-04-19 17:45 UTC (permalink / raw)
  To: Sabrina Dubroca
  Cc: netdev, Hannes Frederic Sowa, Johannes Berg, Dan Carpenter
In-Reply-To: <c986bb2421c817eeb00416bd2055f69040924af5.1460967681.git.sd@queasysnail.net>

----- Original Message -----
> From: "Sabrina Dubroca" <sd@queasysnail.net>
> To: netdev@vger.kernel.org
> Cc: "Hannes Frederic Sowa" <hannes@stressinduktion.org>, "Johannes Berg" <johannes@sipsolutions.net>, "Dan Carpenter"
> <dan.carpenter@oracle.com>, "Sabrina Dubroca" <sd@queasysnail.net>
> Sent: Tuesday, April 19, 2016 1:36:38 PM
> Subject: [PATCH net 1/5] macsec: add missing NULL check after kmalloc
> 
> Fixes: c09440f7dcb3 ("macsec: introduce IEEE 802.1AE driver")
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> ---
>  drivers/net/macsec.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
> index 84d3e5ca8817..f691030ee3df 100644
> --- a/drivers/net/macsec.c
> +++ b/drivers/net/macsec.c
> @@ -1622,8 +1622,8 @@ static int macsec_add_rxsa(struct sk_buff *skb, struct
> genl_info *info)
>  	}
>  
>  	rx_sa = kmalloc(sizeof(*rx_sa), GFP_KERNEL);
> -	if (init_rx_sa(rx_sa, nla_data(tb_sa[MACSEC_SA_ATTR_KEY]), secy->key_len,
> -		       secy->icv_len)) {
> +	if (!rx_sa || init_rx_sa(rx_sa, nla_data(tb_sa[MACSEC_SA_ATTR_KEY]),
> +				 secy->key_len, secy->icv_len)) {

Doesn't this leak rx_sa if kmalloc() succeeds but init_rx_sa fails?

>  		rtnl_unlock();
>  		return -ENOMEM;
>  	}
> --
> 2.8.0
> 
> 

^ permalink raw reply

* RE: [Intel-wired-lan] i40e error handling LLDP messages from Arista switches
From: Parikh, Neerav @ 2016-04-19 17:53 UTC (permalink / raw)
  To: Jeremy Ashton
  Cc: netdev@vger.kernel.org, intel-wired-lan@lists.osuosl.org,
	Parikh, Neerav
In-Reply-To: <739353123C992A4C8800D77E20942590905248DF@ORSMSX103.amr.corp.intel.com>

Resending as the original email was sent in HTLM format and hence my reply went in that format
as well; which got rejected by netdev mailer.

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On Behalf Of Parikh, Neerav
> Sent: Tuesday, April 19, 2016 10:45 AM
> To: Jeremy Ashton <jeremy.ashton@shopify.com>
> Cc: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org
> Subject: Re: [Intel-wired-lan] i40e error handling LLDP messages from Arista switches
>
> > From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On Behalf Of Jeremy Ashton
> > Sent: Tuesday, April 19, 2016 10:26 AM
> > Cc: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org
> > Subject: [Intel-wired-lan] i40e error handling LLDP messages from Arista switches
> >
> > I have been trying to get pf_ring with zc running on ubuntu 14.04.  The thread can be found here:
> >
> > https://github.com/ntop/PF_RING/issues/81
> >
> > Unfortunately it seems there is a bug in i40e which is carried into the code base for i40e_zc.
> >
> > $ modinfo i40e
> > filename:       /lib/modules/3.19.0-58-generic/kernel/drivers/net/ethernet/intel/i40e/i40e.ko
> > version:        1.2.2-k
> > license:        GPL
> > description:    Intel(R) Ethernet Connection XL710 Network Driver
> > author:         Intel Corporation, <e1000-devel@lists.sourceforge.net>
> > srcversion:     E3DEEE00F49BBFBB8FF33A7
> > alias:          pci:v00008086d00001586sv*sd*bc*sc*i*
> > alias:          pci:v00008086d00001585sv*sd*bc*sc*i*
> > alias:          pci:v00008086d00001584sv*sd*bc*sc*i*
> > alias:          pci:v00008086d00001583sv*sd*bc*sc*i*
> > alias:          pci:v00008086d00001581sv*sd*bc*sc*i*
> > alias:          pci:v00008086d00001580sv*sd*bc*sc*i*
> > alias:          pci:v00008086d0000157Fsv*sd*bc*sc*i*
> > alias:          pci:v00008086d00001574sv*sd*bc*sc*i*
> > alias:          pci:v00008086d00001572sv*sd*bc*sc*i*
> > depends:        ptp,vxlan
> > intree:         Y
> > vermagic:       3.19.0-58-generic SMP mod_unload modversions
> > signer:         Magrathea: Glacier signing key
> > sig_key:        DE:B3:43:0A:26:E6:7D:3D:3B:54:B9:DD:13:25:B3:3A:46:B2:F2:DD
> > sig_hashalgo:   sha512
> > part:           debug:Debug level (0=none,...,16=all) (int)
> >
> >
> > I wonder if someone might offer some insight as to the next steps to debug/resolve this.
> >
> > Cheers.
> >
> > Can you send us the dmesg output after running i40e debugfs commands 
> > viz. “dump port”, “llpd get local” , “lldp get remote”, for the port that is showing
> > this behavior? Also, do you happen to know what is the Arista side configuration?
> > This will help us debug better.
> >
> > Thanks,
> > Neerav

^ permalink raw reply

* Re: [PATCHv2] wlcore: spi: add wl18xx support
From: Mark Brown @ 2016-04-19 17:54 UTC (permalink / raw)
  To: Reizer, Eyal
  Cc: kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org,
	אייל רייזר,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <oe8hqfknrcljrjawnrxlohvr.1461087275186-2ueSQiBKiTY7tOexoI0I+QC/G2K4zDHf@public.gmane.org>

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

On Tue, Apr 19, 2016 at 05:38:02PM +0000, Reizer, Eyal wrote:
> Hi Mark,
> 
> Hope you can see the attached picture that illustrates what need to sent for sucesfull SPI init.

I think what the picture shows is that you just need to send at least
one byte at the end of the transfer *after* deasserting /CS which is
totally not what I'd have understood from any of the previous
discussion.  That is already supported, just send an extra byte using
.cs_change to do what is says.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply

* Re: [RFC PATCH net-next 7/8] net: ipv4: listified version of ip_rcv
From: Eric Dumazet @ 2016-04-19 17:54 UTC (permalink / raw)
  To: Edward Cree
  Cc: Tom Herbert, Linux Kernel Network Developers, David Miller,
	Jesper Dangaard Brouer, linux-net-drivers
In-Reply-To: <57166719.4070209@solarflare.com>

On Tue, 2016-04-19 at 18:12 +0100, Edward Cree wrote:

> I think if we pushed bundled RX all the way up to the TCP layer, it might
> potentially also be faster than GRO, because it avoids the work of
> coalescing superframes; plus going through the GRO callbacks for each
> packet could end up blowing icache in the same way the regular stack does.
> If bundling did prove faster, we could then remove GRO, and overall
> complexity would be _reduced_.

Oh well. You really are coming 8 years too late.

I receive ~40 Gbit on a _single_ TCP flow with GRO, I have no idea what
you expect to get without GRO, but my educated guess is :

It will be horrible, and will add memory overhead. (remember GRO shares
a single sk_buff/head for all the segs)

And as a bonus, GRO works also on forwarding workloads, when this packet
is delivered to another NIC or a virtual device.

Are you claiming TSO is useless ?

It is not because a model works well in some other stack (userspace I
guess), that we have to copy it in linux kernel.

Feel free to experiment things, but if your non GRO solution is slower
than GRO, don't even mention it here.

Remember to test things with a realistic iptables setup.

^ permalink raw reply

* Re: [patch -next] geneve: testing the wrong variable in geneve6_build_skb()
From: Alexander Duyck @ 2016-04-19 17:57 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: David S. Miller, Alexander Duyck, Jesse Gross, John W. Linville,
	Pravin B Shelar, Jiri Benc, Daniel Borkmann, Tom Herbert, Netdev,
	kernel-janitors
In-Reply-To: <20160419143056.GD4876@mwanda>

On Tue, Apr 19, 2016 at 7:30 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> We intended to test "err" and not "skb".
>
> Fixes: aed069df099c ('ip_tunnel_core: iptunnel_handle_offloads returns int and doesn't free skb')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
> index efbc7ce..512dbe0 100644
> --- a/drivers/net/geneve.c
> +++ b/drivers/net/geneve.c
> @@ -733,7 +733,7 @@ static int geneve6_build_skb(struct dst_entry *dst, struct sk_buff *skb,
>                 goto free_dst;
>
>         err = udp_tunnel_handle_offloads(skb, udp_sum);
> -       if (IS_ERR(skb))
> +       if (err)
>                 goto free_dst;
>
>         gnvh = (struct genevehdr *)__skb_push(skb, sizeof(*gnvh) + opt_len);


Yeah, this looks like I missed a spot.  Thanks for catching this.

Acked-by: Alexander Duyck <aduyck@mirantis.com>

^ permalink raw reply

* Re: [RFC PATCH net-next 2/8] sfc: batch up RX delivery on EF10
From: Eric Dumazet @ 2016-04-19 18:02 UTC (permalink / raw)
  To: Edward Cree
  Cc: netdev, David Miller, Jesper Dangaard Brouer, linux-net-drivers
In-Reply-To: <57166E17.6030002@solarflare.com>

On Tue, 2016-04-19 at 18:42 +0100, Edward Cree wrote:

> Well, I don't know how to achieve that, but it sounds like you do, so why
> not go ahead and show us ;)
> If you submitted a patch series to make UDP twice as fast, I think people
> would "really care" about an improvement of that magnitude.

Yes I definitely can do that, but Google uses SO_REUSEPORT so we do not
care at all consuming more cycles, if we still can absorb the load.

Just try it, and you'll reach 10 Mpps just fine.

^ permalink raw reply

* [net-next PATCH 1/2] netdev_features: Fold NETIF_F_ALL_TSO into NETIF_F_GSO_SOFTWARE
From: Alexander Duyck @ 2016-04-19 18:02 UTC (permalink / raw)
  To: netdev, davem, alexander.duyck

This patch folds NETIF_F_ALL_TSO into the bitmask for NETIF_F_GSO_SOFTWARE.
The idea is to avoid duplication of defines since the only difference
between the two was the GSO_UDP bit.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 include/linux/netdev_features.h |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
index 15eb0b12fff9..bc8736266749 100644
--- a/include/linux/netdev_features.h
+++ b/include/linux/netdev_features.h
@@ -152,11 +152,6 @@ enum {
 #define NETIF_F_GSO_MASK	(__NETIF_F_BIT(NETIF_F_GSO_LAST + 1) - \
 		__NETIF_F_BIT(NETIF_F_GSO_SHIFT))
 
-/* List of features with software fallbacks. */
-#define NETIF_F_GSO_SOFTWARE	(NETIF_F_TSO | NETIF_F_TSO_ECN | \
-				 NETIF_F_TSO_MANGLEID | \
-				 NETIF_F_TSO6 | NETIF_F_UFO)
-
 /* List of IP checksum features. Note that NETIF_F_ HW_CSUM should not be
  * set in features when NETIF_F_IP_CSUM or NETIF_F_IPV6_CSUM are set--
  * this would be contradictory
@@ -170,6 +165,9 @@ enum {
 #define NETIF_F_ALL_FCOE	(NETIF_F_FCOE_CRC | NETIF_F_FCOE_MTU | \
 				 NETIF_F_FSO)
 
+/* List of features with software fallbacks. */
+#define NETIF_F_GSO_SOFTWARE	(NETIF_F_ALL_TSO | NETIF_F_UFO)
+
 /*
  * If one device supports one of these features, then enable them
  * for all in netdev_increment_features.

^ permalink raw reply related

* [net-next PATCH 2/2] veth: Update features to include all tunnel GSO types
From: Alexander Duyck @ 2016-04-19 18:02 UTC (permalink / raw)
  To: netdev, davem, alexander.duyck
In-Reply-To: <20160419180219.11003.78651.stgit@ahduyck-xeon-server>

This patch adds support for the checksum enabled versions of UDP and GRE
tunnels.  With this change we should be able to send and receive GSO frames
of these types over the veth pair without needing to segment the packets.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 drivers/net/veth.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 4f30a6ae50d0..f37a6e61d4ad 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -312,10 +312,9 @@ static const struct net_device_ops veth_netdev_ops = {
 	.ndo_set_rx_headroom	= veth_set_rx_headroom,
 };
 
-#define VETH_FEATURES (NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_ALL_TSO |    \
-		       NETIF_F_HW_CSUM | NETIF_F_RXCSUM | NETIF_F_HIGHDMA | \
-		       NETIF_F_GSO_GRE | NETIF_F_GSO_UDP_TUNNEL |	    \
-		       NETIF_F_GSO_IPIP | NETIF_F_GSO_SIT | NETIF_F_UFO	|   \
+#define VETH_FEATURES (NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_HW_CSUM | \
+		       NETIF_F_RXCSUM | NETIF_F_HIGHDMA | \
+		       NETIF_F_GSO_SOFTWARE | NETIF_F_GSO_ENCAP_ALL | \
 		       NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_CTAG_RX | \
 		       NETIF_F_HW_VLAN_STAG_TX | NETIF_F_HW_VLAN_STAG_RX )
 

^ permalink raw reply related

* Re: [RFC PATCH net-next 7/8] net: ipv4: listified version of ip_rcv
From: Eric Dumazet @ 2016-04-19 18:06 UTC (permalink / raw)
  To: Edward Cree
  Cc: netdev, David Miller, Jesper Dangaard Brouer, linux-net-drivers
In-Reply-To: <571661E5.7010304@solarflare.com>

On Tue, 2016-04-19 at 17:50 +0100, Edward Cree wrote:
> On 19/04/16 15:50, Eric Dumazet wrote:
> > The main problem in UDP stack today is having to lock the socket because
> > of the dumb forward allocation problem.
> I'm not quite sure what you're referring to here, care to educate me?


This was added for memory accounting, commit
95766fff6b9a78d11fc2d3812dd035381690b55d

TCP stack does not reclaim its forward allocations as aggressively.
(It has a concept of 'memory pressure')

^ permalink raw reply

* [PATCH] drivers: net: cpsw: fix wrong regs access in cpsw_ndo_open
From: Grygorii Strashko @ 2016-04-19 18:09 UTC (permalink / raw)
  To: David S. Miller, netdev, Mugunthan V N
  Cc: Sekhar Nori, linux-kernel, linux-omap, Grygorii Strashko

The cpsw_ndo_open() could try to access CPSW registers before
calling pm_runtime_get_sync(). This will trigger L3 error:

 WARNING: CPU: 0 PID: 21 at drivers/bus/omap_l3_noc.c:147 l3_interrupt_handler+0x220/0x34c()
 44000000.ocp:L3 Custom Error: MASTER M2 (64-bit) TARGET L4_FAST (Idle): Data Access in Supervisor mode during Functional access

and CPSW will stop functioning.

Hence, fix it by moving pm_runtime_get_sync() before the first access
to CPSW registers in cpsw_ndo_open().

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/net/ethernet/ti/cpsw.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 54bcc38..0fa75a8 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -1244,12 +1244,12 @@ static int cpsw_ndo_open(struct net_device *ndev)
 	int i, ret;
 	u32 reg;
 
+	pm_runtime_get_sync(&priv->pdev->dev);
+
 	if (!cpsw_common_res_usage_state(priv))
 		cpsw_intr_disable(priv);
 	netif_carrier_off(ndev);
 
-	pm_runtime_get_sync(&priv->pdev->dev);
-
 	reg = priv->version;
 
 	dev_info(priv->dev, "initializing cpsw version %d.%d (%d)\n",
-- 
2.8.1

^ permalink raw reply related

* Re: [Intel-wired-lan] i40e error handling LLDP messages from Arista switches
From: Jeremy Ashton @ 2016-04-19 18:10 UTC (permalink / raw)
  To: Parikh, Neerav; +Cc: netdev@vger.kernel.org, intel-wired-lan@lists.osuosl.org
In-Reply-To: <739353123C992A4C8800D77E2094259090524953@ORSMSX103.amr.corp.intel.com>

So, what commands exactly are you looking to have run?  Would packet
capture of the lldp traffic help?

On Tue, Apr 19, 2016 at 1:53 PM, Parikh, Neerav <neerav.parikh@intel.com> wrote:
> Resending as the original email was sent in HTLM format and hence my reply went in that format
> as well; which got rejected by netdev mailer.
>
>> -----Original Message-----
>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On Behalf Of Parikh, Neerav
>> Sent: Tuesday, April 19, 2016 10:45 AM
>> To: Jeremy Ashton <jeremy.ashton@shopify.com>
>> Cc: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org
>> Subject: Re: [Intel-wired-lan] i40e error handling LLDP messages from Arista switches
>>
>> > From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On Behalf Of Jeremy Ashton
>> > Sent: Tuesday, April 19, 2016 10:26 AM
>> > Cc: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org
>> > Subject: [Intel-wired-lan] i40e error handling LLDP messages from Arista switches
>> >
>> > I have been trying to get pf_ring with zc running on ubuntu 14.04.  The thread can be found here:
>> >
>> > https://github.com/ntop/PF_RING/issues/81
>> >
>> > Unfortunately it seems there is a bug in i40e which is carried into the code base for i40e_zc.
>> >
>> > $ modinfo i40e
>> > filename:       /lib/modules/3.19.0-58-generic/kernel/drivers/net/ethernet/intel/i40e/i40e.ko
>> > version:        1.2.2-k
>> > license:        GPL
>> > description:    Intel(R) Ethernet Connection XL710 Network Driver
>> > author:         Intel Corporation, <e1000-devel@lists.sourceforge.net>
>> > srcversion:     E3DEEE00F49BBFBB8FF33A7
>> > alias:          pci:v00008086d00001586sv*sd*bc*sc*i*
>> > alias:          pci:v00008086d00001585sv*sd*bc*sc*i*
>> > alias:          pci:v00008086d00001584sv*sd*bc*sc*i*
>> > alias:          pci:v00008086d00001583sv*sd*bc*sc*i*
>> > alias:          pci:v00008086d00001581sv*sd*bc*sc*i*
>> > alias:          pci:v00008086d00001580sv*sd*bc*sc*i*
>> > alias:          pci:v00008086d0000157Fsv*sd*bc*sc*i*
>> > alias:          pci:v00008086d00001574sv*sd*bc*sc*i*
>> > alias:          pci:v00008086d00001572sv*sd*bc*sc*i*
>> > depends:        ptp,vxlan
>> > intree:         Y
>> > vermagic:       3.19.0-58-generic SMP mod_unload modversions
>> > signer:         Magrathea: Glacier signing key
>> > sig_key:        DE:B3:43:0A:26:E6:7D:3D:3B:54:B9:DD:13:25:B3:3A:46:B2:F2:DD
>> > sig_hashalgo:   sha512
>> > part:           debug:Debug level (0=none,...,16=all) (int)
>> >
>> >
>> > I wonder if someone might offer some insight as to the next steps to debug/resolve this.
>> >
>> > Cheers.
>> >
>> > Can you send us the dmesg output after running i40e debugfs commands
>> > viz. “dump port”, “llpd get local” , “lldp get remote”, for the port that is showing
>> > this behavior? Also, do you happen to know what is the Arista side configuration?
>> > This will help us debug better.
>> >
>> > Thanks,
>> > Neerav

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox