Netdev List
 help / color / mirror / Atom feed
* [PATCH] net: pxa168_eth: move SET_NETDEV_DEV a bit earlier
From: Jisheng Zhang @ 2014-11-12 11:08 UTC (permalink / raw)
  To: davem, antoine.tenart, arnd, sebastian.hesselbarth
  Cc: netdev, linux-kernel, linux-arm-kernel, Jisheng Zhang

This is to ensure the net_device's dev.parent is set before we used it
in dma_zalloc_coherent() from init_hash_table().

Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
---
 drivers/net/ethernet/marvell/pxa168_eth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/pxa168_eth.c b/drivers/net/ethernet/marvell/pxa168_eth.c
index 21ddece..38f7cee 100644
--- a/drivers/net/ethernet/marvell/pxa168_eth.c
+++ b/drivers/net/ethernet/marvell/pxa168_eth.c
@@ -1540,8 +1540,8 @@ static int pxa168_eth_probe(struct platform_device *pdev)
 	if (err)
 		goto err_free_mdio;
 
-	pxa168_init_hw(pep);
 	SET_NETDEV_DEV(dev, &pdev->dev);
+	pxa168_init_hw(pep);
 	err = register_netdev(dev);
 	if (err)
 		goto err_mdiobus;
-- 
2.1.3

^ permalink raw reply related

* Re: [PATCH nf] netfilter: conntrack: fix race in __nf_conntrack_confirm against get_next_corpse
From: Jörg Marx @ 2014-11-12 10:57 UTC (permalink / raw)
  To: Jesper Dangaard Brouer, Pablo Neira Ayuso
  Cc: programme110, netfilter-devel, Florian Westphal, netdev,
	Patrick McHardy
In-Reply-To: <20141112083500.5404e5f4@redhat.com>

On 12.11.2014 08:35, Jesper Dangaard Brouer wrote:

Hi,

I wrote the patch in 2010, so find some arguments below:

>>> > > +	nf_ct_del_from_dying_or_unconfirmed_list(ct);
>>> > >  
>>> > >  	if (unlikely(nf_ct_is_dying(ct))) {
>>> > > +		nf_ct_add_to_dying_list(ct);
>>> > >  		nf_conntrack_double_unlock(hash, reply_hash);
>>> > >  		local_bh_enable();
>>> > >  		return NF_ACCEPT;
>> > 
>> > Not directly related to your patch, but I don't find a good reason why
>> > we're accepting this packet.
>> > 
>> > If the conntrack from the unconfirmed list is dying, then the object
>> > will be released by when the packet leaves the stack to its
>> > destination. With stateful filtering depending in place, the follow up
>> > packet in the reply direction will likely be considered invalid (if
>> > tcp tracking is on). Fortunately for us, the origin will likely
>> > retransmit the syn again, so the ct will be setup accordingly.
>> > 
>> > So, why should we allow this to go through?
> True, it also seems strange to me that we accept this packet.

The raise was triggered in a scenario when we tested high-load IPsec
tunnels and flushed the conntrack hashs from userspace.

For me there is little difference in choosing DROP or ACCEPT as verdict.
The packet/skb belongs to a formerly allowed connection, most likely
this connection is still allowed (but the conntrack hash entry is about
to be removed due to userspace is flushing the conntrack table).

To minimize the impact (lost packets -> retransmit) I decided to allow
the skb in flight, so were is no lost packet at this place.

When the connection is not allowed anymore (but was allowed up to now,
because the hash entry exists), the impact is one last packet 'slipping
through'.

Today I would still decide the way I did in 2010.

> 
>> > This return verdict was introduced in: fc35077 ("netfilter:
>> > nf_conntrack: fix a race in __nf_conntrack_confirm against
>> > nf_ct_get_next_corpse()") btw.
> And the commit does not argue why NF_ACCEPT was chosen...
> 
> -- Best regards, Jesper Dangaard Brouer MSc.CS, Sr. Network Kernel
> Developer at Red Hat Author of http://www.iptv-analyzer.org LinkedIn:
> http://www.linkedin.com/in/brouer

best regards
Jörg Marx

-- 

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

^ permalink raw reply

* Re: [PATCH] crypto: aesni-intel - avoid IPsec re-ordering
From: Ming Liu @ 2014-11-12 10:41 UTC (permalink / raw)
  To: Steffen Klassert; +Cc: herbert, davem, ying.xue, linux-crypto, netdev
In-Reply-To: <20141112084138.GL6390@secunet.com>

On 11/12/2014 04:41 PM, Steffen Klassert wrote:
> On Wed, Nov 12, 2014 at 01:49:31PM +0800, Ming Liu wrote:
>> So far, the encryption/decryption are asynchronously processed in
>> softirq and cryptd which would result in a implicit order of data,
>> therefore leads IPSec stack also out of order while encapsulating
>> or decapsulating packets.
>>
>> Consider the following scenario:
>>
>>               DECRYPTION INBOUND
>>                        |
>>                        |
>>               +-----Packet A
>>               |        |
>>               |        |
>>               |     Packet B
>>               |        |
>>      (cryptd) |        | (software interrupts)
>>               |      ......
>>               |        |
>>               |        |
>>               |     Packet B(decrypted)
>>               |        |
>>               |        |
>>               +---> Packet A(decrypted)
>>                        |
>>                        |
>>               DECRYPTION OUTBOUND
>>
>> Add cryptd_flush_queue function fixing it by being called from softirq
>> to flush all previous queued elements before processing a new one. To
>> prevent cryptd_flush_queue() being accessed from software interrupts,
>> local_bh_disable/enable needs to be relocated in several places.
>>
>> Signed-off-by: Ming Liu <ming.liu@windriver.com>
>> ---
>> I was told that I need resend this patch with git, so here it is:
>>
>> I've figured out a new patch for this issue reported by me previously,
>> the basic idea is adding a cryptd_flush_queue function fixing it by
>> being called from softirq to flush all previous queued elements
>> before processing a new one, and it works very well so far per my test.
> I doubt that this approach can work. The cryptd encrypt/decrypt functions
> assume to be called from a workqueue worker, they can't be called from
> atomic context.
Yes, you are correct, I am on board, I need rework the patch.

>
> Can't we just use cryptd unconditionally to fix this reordering problem?
>>   crypto/ablk_helper.c    |  10 ++++-
>>   crypto/cryptd.c         | 107 ++++++++++++++++++++++++++++++++++++++++--------
>>   include/crypto/cryptd.h |  13 ++++++
>>   3 files changed, 111 insertions(+), 19 deletions(-)
>>
>> diff --git a/crypto/ablk_helper.c b/crypto/ablk_helper.c
>> index ffe7278..600a70f 100644
>> --- a/crypto/ablk_helper.c
>> +++ b/crypto/ablk_helper.c
>> @@ -70,16 +70,19 @@ int ablk_encrypt(struct ablkcipher_request *req)
>>   {
>>   	struct crypto_ablkcipher *tfm = crypto_ablkcipher_reqtfm(req);
>>   	struct async_helper_ctx *ctx = crypto_ablkcipher_ctx(tfm);
>> +	struct crypto_tfm *req_tfm = crypto_ablkcipher_tfm(
>> +		crypto_ablkcipher_crt(&ctx->cryptd_tfm->base)->base);
>>   
>>   	if (!may_use_simd()) {
>>   		struct ablkcipher_request *cryptd_req =
>>   			ablkcipher_request_ctx(req);
>>   
>>   		*cryptd_req = *req;
>> -		ablkcipher_request_set_tfm(cryptd_req, &ctx->cryptd_tfm->base);
>> +		cryptd_req->base.tfm = req_tfm;
>>   
>>   		return crypto_ablkcipher_encrypt(cryptd_req);
>>   	} else {
>> +		cryptd_flush_queue(req_tfm, CRYPTD_ENCRYPT);
> Where is CRYPTD_ENCRYPT defined?
> This does not even compile when applied to the cryptodev tree.
Sorry, it really should be CRYPTD_BLKCIPHER_ENCRYPT, I made the original 
patch against 3.4 kernel, there must be something wrong when I adjusted 
it to mainline kernel, sorry for that.
>
>>   		return __ablk_encrypt(req);
>>   	}
>>   }
>> @@ -89,13 +92,15 @@ int ablk_decrypt(struct ablkcipher_request *req)
>>   {
>>   	struct crypto_ablkcipher *tfm = crypto_ablkcipher_reqtfm(req);
>>   	struct async_helper_ctx *ctx = crypto_ablkcipher_ctx(tfm);
>> +	struct crypto_tfm *req_tfm = crypto_ablkcipher_tfm(
>> +		crypto_ablkcipher_crt(&ctx->cryptd_tfm->base)->base);
>>   
>>   	if (!may_use_simd()) {
>>   		struct ablkcipher_request *cryptd_req =
>>   			ablkcipher_request_ctx(req);
>>   
>>   		*cryptd_req = *req;
>> -		ablkcipher_request_set_tfm(cryptd_req, &ctx->cryptd_tfm->base);
>> +		cryptd_req->base.tfm = req_tfm;
>>   
>>   		return crypto_ablkcipher_decrypt(cryptd_req);
>>   	} else {
>> @@ -105,6 +110,7 @@ int ablk_decrypt(struct ablkcipher_request *req)
>>   		desc.info = req->info;
>>   		desc.flags = 0;
>>   
>> +		cryptd_flush_queue(req_tfm, CRYPTD_DECRYPT);
> Same here.
Should be CRYPTD_BLKCIPHER_DECRYPT.

>
>>   		return crypto_blkcipher_crt(desc.tfm)->decrypt(
>>   			&desc, req->dst, req->src, req->nbytes);
>>   	}
>> diff --git a/crypto/cryptd.c b/crypto/cryptd.c
>> index e592c90..0b387a1 100644
>> --- a/crypto/cryptd.c
>> +++ b/crypto/cryptd.c
>> @@ -119,11 +119,13 @@ static int cryptd_enqueue_request(struct cryptd_queue *queue,
>>   	int cpu, err;
>>   	struct cryptd_cpu_queue *cpu_queue;
>>   
>> +	local_bh_disable();
>>   	cpu = get_cpu();
>>   	cpu_queue = this_cpu_ptr(queue->cpu_queue);
>>   	err = crypto_enqueue_request(&cpu_queue->queue, request);
>>   	queue_work_on(cpu, kcrypto_wq, &cpu_queue->work);
>>   	put_cpu();
>> +	local_bh_enable();
>>   
>>   	return err;
>>   }
>> @@ -147,11 +149,9 @@ static void cryptd_queue_worker(struct work_struct *work)
>>   	preempt_disable();
>>   	backlog = crypto_get_backlog(&cpu_queue->queue);
>>   	req = crypto_dequeue_request(&cpu_queue->queue);
>> -	preempt_enable();
>> -	local_bh_enable();
> Everything below the local_bh_enable() should not run in atomic context
> as the subsequent functions may set the CRYPTO_TFM_REQ_MAY_SLEEP flag.
If I turn off all the CRYPTO_TFM_REQ_MAY_SLEEP in cryptd.c, is that 
going to work?

>
>>   
>>   	if (!req)
>> -		return;
>> +		goto out;
>>   
>>   	if (backlog)
>>   		backlog->complete(backlog, -EINPROGRESS);
>> @@ -159,6 +159,9 @@ static void cryptd_queue_worker(struct work_struct *work)
>>   
>>   	if (cpu_queue->queue.qlen)
>>   		queue_work(kcrypto_wq, &cpu_queue->work);
>> +out:
>> +	preempt_enable();
>> +	local_bh_enable();
>>   }
> ...
>
>>   
>> +void cryptd_flush_queue(struct crypto_tfm *tfm, cryptd_type_t type)
>> +{
>> +	struct crypto_instance *inst = crypto_tfm_alg_instance(tfm);
>> +	struct cryptd_instance_ctx *ictx = crypto_instance_ctx(inst);
>> +	struct cryptd_queue *cryptd_queue = ictx->queue;
>> +	struct cryptd_cpu_queue *cpu_queue;
>> +	struct crypto_queue *queue;
>> +	struct crypto_async_request *req, *tmp, *backlog;
>> +	crypto_completion_t complete;
>> +	int cpu;
>> +	unsigned int len;
>> +
>> +	switch (type) {
>> +	case CRYPTD_BLKCIPHER_ENCRYPT:
>> +		complete = cryptd_blkcipher_encrypt;
>> +		break;
>> +	case CRYPTD_BLKCIPHER_DECRYPT:
>> +		complete = cryptd_blkcipher_decrypt;
>> +		break;
>> +	case CRYPTD_HASH_INIT:
>> +		complete = cryptd_hash_init;
>> +		break;
>> +	case CRYPTD_HASH_UPDATE:
>> +		complete = cryptd_hash_update;
>> +		break;
>> +	case CRYPTD_HASH_FINAL:
>> +		complete = cryptd_hash_final;
>> +		break;
>> +	case CRYPTD_HASH_FINUP:
>> +		complete = cryptd_hash_finup;
>> +		break;
>> +	case CRYPTD_HASH_DIGEST:
>> +		complete = cryptd_hash_digest;
>> +		break;
>> +	case CRYPTD_AEAD_ENCRYPT:
>> +		complete = cryptd_aead_encrypt;
>> +		break;
>> +	case CRYPTD_AEAD_DECRYPT:
>> +		complete = cryptd_aead_decrypt;
>> +		break;
>> +	default:
>> +		complete = NULL;
>> +	}
>> +
>> +	if (complete == NULL)
>> +		return;
>> +
>> +	local_bh_disable();
>> +	cpu = get_cpu();
>> +	cpu_queue = this_cpu_ptr(cryptd_queue->cpu_queue);
>> +	queue = &cpu_queue->queue;
>> +	len = queue->qlen;
>> +
>> +	if (!len)
>> +		goto out;
>> +
>> +	list_for_each_entry_safe(req, tmp, &queue->list, list) {
>> +		if (req->complete == complete) {
>> +			queue->qlen--;
>> +
>> +			if (queue->backlog != &queue->list) {
>> +				backlog = container_of(queue->backlog,
>> +					struct crypto_async_request, list);
>> +				queue->backlog = queue->backlog->next;
>> +			} else
>> +				backlog = NULL;
>> +
>> +			list_del(&req->list);
>> +
>> +			if (backlog)
>> +				backlog->complete(backlog, -EINPROGRESS);
>> +			req->complete(req, 0);
> Same here, the complete function can not run in atomic context.
> Also, this can not ensure that the request is finalized.
> Subsequent algorithms may run asynchronously too, so this
> does not fix the reordering problem completely.
I do not know that before. Then I need rework the patch.

//Ming Liu
>
>
>

^ permalink raw reply

* Re: [PATCH] crypto: aesni-intel - avoid IPsec re-ordering
From: Ming Liu @ 2014-11-12 10:41 UTC (permalink / raw)
  To: Herbert Xu; +Cc: Steffen Klassert, davem, ying.xue, linux-crypto, netdev
In-Reply-To: <20141112085148.GA26268@gondor.apana.org.au>

On 11/12/2014 04:51 PM, Herbert Xu wrote:
> On Wed, Nov 12, 2014 at 09:41:38AM +0100, Steffen Klassert wrote:
>> Can't we just use cryptd unconditionally to fix this reordering problem?
> I think the idea is that most of the time cryptd isn't required
> so we want to stick with direct processing to lower latency.
>
> I think the simplest fix would be to punt to cryptd as long as
> there are cryptd requests queued.
I've tried that method when I started to think about the fix, but it 
will cause 2 other issues per test while resolving the reordering one, 
as follows:
1 The work queue can not handle so many packets when the traffic is very 
high(over 200M/S), and it would drop most of them when the queue length 
is beyond CRYPTD_MAX_CPU_QLEN.
2 Soft lockups are observed, per my analysis, the cause is: in some 
extreme cases, all packets would be sent to the cryptd once there are 
requests already in it, this will let the net poll functions return 
pretty fast, and lead too many hw interrupts triggered in a certain period.

//Ming Liu
>
> Cheers,

^ permalink raw reply

* [PATCH net] net: ptp: fix time stamp matching logic for VLAN packets.
From: Richard Cochran @ 2014-11-12 10:33 UTC (permalink / raw)
  To: netdev; +Cc: David Miller, Stefan Sørensen

Commit ae5c6c6d "ptp: Classify ptp over ip over vlan packets" changed the
code in two drivers that matches time stamps with PTP frames, with the goal
of allowing VLAN tagged PTP packets to receive hardware time stamps.

However, that commit failed to account for the VLAN header when parsing
IPv4 packets. This patch fixes those two drivers to correctly match VLAN
tagged IPv4/UDP PTP messages with their time stamps.

This patch should also be applied to v3.17.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/net/ethernet/ti/cpts.c |    2 +-
 drivers/net/phy/dp83640.c      |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/ti/cpts.c b/drivers/net/ethernet/ti/cpts.c
index ab92f67..4a4388b 100644
--- a/drivers/net/ethernet/ti/cpts.c
+++ b/drivers/net/ethernet/ti/cpts.c
@@ -264,7 +264,7 @@ static int cpts_match(struct sk_buff *skb, unsigned int ptp_class,
 
 	switch (ptp_class & PTP_CLASS_PMASK) {
 	case PTP_CLASS_IPV4:
-		offset += ETH_HLEN + IPV4_HLEN(data) + UDP_HLEN;
+		offset += ETH_HLEN + IPV4_HLEN(data + offset) + UDP_HLEN;
 		break;
 	case PTP_CLASS_IPV6:
 		offset += ETH_HLEN + IP6_HLEN + UDP_HLEN;
diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index 2954052..e22e602 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -791,7 +791,7 @@ static int match(struct sk_buff *skb, unsigned int type, struct rxts *rxts)
 
 	switch (type & PTP_CLASS_PMASK) {
 	case PTP_CLASS_IPV4:
-		offset += ETH_HLEN + IPV4_HLEN(data) + UDP_HLEN;
+		offset += ETH_HLEN + IPV4_HLEN(data + offset) + UDP_HLEN;
 		break;
 	case PTP_CLASS_IPV6:
 		offset += ETH_HLEN + IP6_HLEN + UDP_HLEN;
@@ -934,7 +934,7 @@ static int is_sync(struct sk_buff *skb, int type)
 
 	switch (type & PTP_CLASS_PMASK) {
 	case PTP_CLASS_IPV4:
-		offset += ETH_HLEN + IPV4_HLEN(data) + UDP_HLEN;
+		offset += ETH_HLEN + IPV4_HLEN(data + offset) + UDP_HLEN;
 		break;
 	case PTP_CLASS_IPV6:
 		offset += ETH_HLEN + IP6_HLEN + UDP_HLEN;
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCH v1 net-next 1/2] bonding: Expand speed type bits of the AD Port Key
From: Jianhua Xie @ 2014-11-12  9:53 UTC (permalink / raw)
  To: Jay Vosburgh, David Miller; +Cc: netdev, vfalico, andy
In-Reply-To: <19882.1415735238@famine>

Thanks you two for the valuable comments.

If my understanding is right,  it is encouraged to use a counter
rather than a bitmask for the speed field, right?

if yes, how many bits are better to use for current speed and
future speed (like 100Gbps/400Gbps and etc.)?  I am not sure
that 5 bits are enough (2**5=32) or not. And I am clear to keep
"the duplex bit in the key " in my mind.

if not, what's your recommendation please?

Thanks & Best Regards,
Jianhua

在 2014年11月12日 03:47, Jay Vosburgh 写道:
> David Miller <davem@davemloft.net> wrote:
>
>> From: Xie Jianhua <Jianhua.Xie@freescale.com>
>> Date: Mon, 10 Nov 2014 15:16:40 +0800
>>
>>> From: Jianhua Xie <Jianhua.Xie@freescale.com>
>>>
>>> Port Key was determined as 16 bits according to the link speed,
>>> duplex and user key (which is yet not supported), in which key
>>> speed was 5 bits for 1Mbps/10Mbps/100Mbps/1Gbps/10Gbps as below:
>>> --------------------------------------------------------------
>>> Port key :|	User key	| Speed		|	Duplex|
>>> --------------------------------------------------------------
>>> 16			6		1		0
>>> This patch is expanding speed type from 5 bits to 9 bits for other
>>> speed 2.5Gbps/20Gbps/40Gbps/56Gbps and shrinking user key from 10
>>> bits to 6 bits.  New Port Key looks like below:
>>> --------------------------------------------------------------
>>> Port key :|	User key	| Speed		|	Duplex|
>>> --------------------------------------------------------------
>>> 16			10		1		0
>>>
>> Do we determine the layout of this value all ourselves?
> 	Yes, we do.  The precise format of the port key is not defined
> by the standard; IEEE 802.1AX 5.3.5, "Capability identification":
>
> "A given Key value is meaningful only in the context of the System that
> allocates it; there is no global significance to Key values."
>
> 	and
>
> "When a System assigns an operational Key value to a set of ports, it
> signifies that, in the absence of other constraints, the current
> operational state of the set of ports allows any subset of that set of
> ports (including the entire set) to be aggregated together from the
> perspective of the System making the assignment."
>
> 	So, basically, it's a magic cookie that indicates that all ports
> on a particular system with the same key value are suitable to be
> aggregated together.
>
>> If not, then is it exported to anything user-visible that we
>> might be breaking?
> 	The key values are not user-visible, and the "user" settable
> portion of the key has never been implemented.
>
>> If it is private, it makes no sense to use a bitmask for the speed.
>> We should instead change the field to be some numerically increasing
>> value.
>>
>> Otherwise we'll run out of bits again and keep having to adjust the
>> field layout more often than we really need to.
> 	Agreed.
>
> 	Also note that there are some internal dependencies within
> bonding on the format; in particular the duplex bit in the key is used
> to determine if a port is LACP-capable, and that functionality needs to
> be preserved.
>
> 	-J
>
> ---
> 	-Jay Vosburgh, jay.vosburgh@canonical.com

^ permalink raw reply

* RE: "asix: Don't reset PHY on if_up for ASIX 88772" breaks net on arndale platform
From: Stam, Michel [FINT] @ 2014-11-12  9:49 UTC (permalink / raw)
  To: Ben Hutchings, Charles Keepax
  Cc: Riku Voipio, davem, linux-usb, netdev, linux-kernel,
	linux-samsung-soc
In-Reply-To: <1415751802.3398.107.camel@decadent.org.uk>

Hello Ben,

Regarding the code snippet;

Good question, The original code didn't do this either, which is why I left it as it is. It could cause undesirable behaviour, agreed.

After a quick driver examination: I do see that asix_set_sw_mii and asix_set_hw_mii are called prior to the actual write (asix_mdio_write), it may be that this takes care of ensuring data is written to the chip, as asix_write_cmd waits for usbnet_write_cmd to send (and acknowledge) a USB CONTROL MSG. A lock to the phy is held during this time.

If I recall my USB knowledge, control messages are acknowledged, which would ensure data is written to the chip. Whether the ASIX requires further delay I would not know. I would have to dive deeper into the timings of the ASIX chip and the driver behaviour to see if that would be the case.

Kind regards,

Michel Stam
-----Original Message-----
From: Ben Hutchings [mailto:ben@decadent.org.uk] 
Sent: Wednesday, November 12, 2014 1:23 AM
To: Charles Keepax
Cc: Stam, Michel [FINT]; Riku Voipio; davem@davemloft.net; linux-usb@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org
Subject: Re: "asix: Don't reset PHY on if_up for ASIX 88772" breaks net on arndale platform

On Tue, 2014-11-04 at 20:09 +0000, Charles Keepax wrote:
> On Tue, Nov 04, 2014 at 11:23:06AM +0100, Stam, Michel [FINT] wrote:
> > Hello Riku,
> > 
> > >Fixing a bug (ethtool support) must not cause breakage elsewhere 
> > >(in
> > this case on arndale). This is now a regression of functionality 
> > from 3.17.
> > >
> > >I think it would better to revert the change now and with less 
> > >hurry
> > introduce a ethtool fix that doesn't break arndale.
> > 
> > I don't fully agree here;
> > I would like to point out that this commit is a revert itself. 
> > Fixing the armdale will then cause breakage in other 
> > implementations, such as ours. Blankly reverting breaks other peoples' implementations.
> > 
> > The PHY reset is the thing that breaks ethtool support, so any fix 
> > that appeases all would have to take existing PHY state into account.
[...]
> --- a/drivers/net/usb/asix_devices.c
> +++ b/drivers/net/usb/asix_devices.c
> @@ -299,6 +299,7 @@ static int ax88772_reset(struct usbnet *dev)  {
>         struct asix_data *data = (struct asix_data *)&dev->data;
>         int ret, embd_phy;
> +       int reg;
>         u16 rx_ctl;
> 
>         ret = asix_write_gpio(dev,
> @@ -359,8 +360,10 @@ static int ax88772_reset(struct usbnet *dev)
>         msleep(150);
> 
>         asix_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR, BMCR_RESET);
> -       asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
> -                       ADVERTISE_ALL | ADVERTISE_CSMA);
> +       reg = asix_mdio_read(dev->net, dev->mii.phy_id, MII_ADVERTISE);
> +       if (!reg)
> +               reg = ADVERTISE_ALL | ADVERTISE_CSMA;
> +       asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE, 
> + reg);
[...]

Why is there no sleep after setting the RESET bit?  Doesn't that make the following register writes unreliable?

Ben.

--
Ben Hutchings
Experience is directly proportional to the value of equipment destroyed.
                                                         - Carolyn Scheppner

^ permalink raw reply

* Re: [PATCH 8/9] net: phy: micrel: clean up led-mode setup
From: Johan Hovold @ 2014-11-12  9:29 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Sergei Shtylyov, Johan Hovold, David S. Miller, linux-kernel,
	netdev
In-Reply-To: <546282EA.7040508@gmail.com>

On Tue, Nov 11, 2014 at 01:43:06PM -0800, Florian Fainelli wrote:
> On 11/11/2014 01:41 PM, Sergei Shtylyov wrote:
> > Hello.
> > 
> > On 11/11/2014 10:00 PM, Johan Hovold wrote:
> > 
> >> Clean up led-mode setup by introducing proper defines for PHY Control
> >> registers 1 and 2 and only passing the register to the setup function.
> > 
> >    Not sure that's really better that it was before (modulo naming).
> 
> We do have proper error handling in kszphy_setup_led() which is already
> an improvement.

This also means handling the led mode in one place rather than spreading
it all over the driver, with multiple config functions providing one of
the same two combinations of register and shift.

It's usefulness will perhaps become more apparent if you look at the
follow up patches, which does further refactoring and store the led-mode
register in the type data.

Johan

^ permalink raw reply

* Re: [PATCH 2/5] stmmac: pci: use managed resources
From: Andy Shevchenko @ 2014-11-12  9:28 UTC (permalink / raw)
  To: Rayagond Kokatanur
  Cc: Giuseppe CAVALLARO, Sergei Shtylyov, netdev, Kweh Hock Leong,
	David S. Miller, Vince Bridgers
In-Reply-To: <CAJ3bTp79FLhOCvvQNG=+C17v5TOzy2ZvbyC24OQWTr2mmJ4OCw@mail.gmail.com>

On Wed, 2014-11-12 at 10:44 +0530, Rayagond Kokatanur wrote:
> Hi Andy,
> 
> Yes, I tested this pci driver on XILINX FPGA setup and its true that
> we can man any bar number by changing the configuration in bit-map.
> 
> But its always good to keep that for loop which try to map any bar
> number between 0 - 5, the new code breaks if bar number is other than
> zero.
> 
> We can make driver generic by keeping that loop.

Thanks for clarification, I had redone this in v2 which is applied.

> 
> wwr
> Rayagond
> 
> On Thu, Oct 30, 2014 at 1:35 PM, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Mon, 2014-10-27 at 16:28 +0100, Giuseppe CAVALLARO wrote:
> >> On 10/22/2014 10:36 AM, Andy Shevchenko wrote:
> >> > So, I was trying to find any specification on public regarding to boards
> >> > that have this IP, no luck so far. I guess that that code was created
> >> > due to XILINX FPGA usage which probably can provide any BAR user wants
> >> > to. Thus, I imply that in real applications the BAR most probably will
> >> > be 0. However, I left variable which can be overridden in future
> >> > (regarding to PCI ID).
> >> >
> >> > It would be nice to hear someone from ST about this. Giuseppe?
> >>
> >> Hello Andy
> >>
> >> this chip is on ST SoCs since long time but embedded. I have no PCI
> >> card. Added Rayagond on copy too
> >
> > Rayagond, what do you think about changing an approach that is used to
> > get resources from PCI?
> >
> > --
> > Andy Shevchenko <andriy.shevchenko@intel.com>
> > Intel Finland Oy
> >


-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy

^ permalink raw reply

* [PATCH net-next] cxgb4: Fix static checker warning
From: Hariprasad Shenai @ 2014-11-12  9:31 UTC (permalink / raw)
  To: netdev; +Cc: davem, leedom, anish, nirranjan, kumaras, Hariprasad Shenai

Fix static checker warning that got introduced in commit e2ac9628959cc152
("cxgb4: Cleanup macros so they follow the same style and look consistent, part
2") due to accidental checkin of bogus line.

Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>
---
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index d13d36a..660bf0f 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -5752,7 +5752,6 @@ static int adap_init0(struct adapter *adap)
 					    "No Configuration File present "
 					    "on adapter. Using hard-wired "
 					    "configuration parameters.\n");
-					goto bye;
 					ret = adap_init0_no_config(adap, reset);
 				}
 			}
-- 
1.7.1

^ permalink raw reply related

* Re: [PATCH 19/22] dt/bindings: add micrel,rmii_ref_clk_sel_25_mhz to eth-phy binding
From: Johan Hovold @ 2014-11-12  9:19 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Johan Hovold, Mark Rutland, Florian Fainelli, David S. Miller,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	devicetree@vger.kernel.org
In-Reply-To: <20141112070127.GF30369@pengutronix.de>

On Wed, Nov 12, 2014 at 08:01:27AM +0100, Sascha Hauer wrote:
> On Tue, Nov 11, 2014 at 07:18:25PM +0100, Johan Hovold wrote:
> > On Tue, Nov 11, 2014 at 05:57:42PM +0000, Mark Rutland wrote:
> > > On Tue, Nov 11, 2014 at 05:37:37PM +0000, Johan Hovold wrote:
> > > > Add "micrel,rmii_ref_clk_sel_25_mhz" to Micrel ethernet PHY binding
> > > > documentation.
> > > > 
> > > > Cc: devicetree@vger.kernel.org
> > > > Signed-off-by: Johan Hovold <johan@kernel.org>
> > > > ---
> > > >  Documentation/devicetree/bindings/net/micrel.txt | 5 +++++
> > > >  1 file changed, 5 insertions(+)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/net/micrel.txt b/Documentation/devicetree/bindings/net/micrel.txt
> > > > index a1bab5eaae02..9b08dd6551dd 100644
> > > > --- a/Documentation/devicetree/bindings/net/micrel.txt
> > > > +++ b/Documentation/devicetree/bindings/net/micrel.txt
> > > > @@ -19,6 +19,11 @@ Optional properties:
> > > >  
> > > >                See the respective PHY datasheet for the mode values.
> > > >  
> > > > + - micrel,rmii_ref_clk_sel_25_mhz: rmii_ref_clk_sel bit selects 25 MHz mode
> > > > +
> > > > +		Whether 25 MHz (rather than 50 Mhz) clock mode is selected
> > > > +		when the rmii_ref_clk_sel bit is set.
> > > 
> > > s/_/-/ in property names please.
> > 
> > Ouch, copied from variable name, sorry.
> > 
> > > That said, I don't follow the meaning. Does this cause the kernel to do
> > > something different, or is is simply that a 25MHz ref clock is wired up?
> > 
> > Yes, the driver currently sets this configuration bit based on a common
> > clock binding.
> > 
> > However, it turns out the meaning of the bit is reversed on some PHY
> > variants. On most PHYs 50 MHz mode is selected by setting this bit,
> > whereas on the PHYs that need this new property, setting it selects 25
> > MHz mode instead.
> 
> Maybe rename the property to something like rmii-ref-clk-25mhz-active-high
> then? Also you should probably make it more explicit that this is a
> hardware property and not for adjusting the clock.

You're right, but how about calling it

	micrel,rmii-reference-clock-select-inverted

Then no one will set it believing it will change the clock mode and
without reading the binding doc first.

The description could then read something like

	micrel,rmii-reference-clock-select-inverted: RMII Reference
		Clock Select bit is inverted

	The RMII Reference Clock Select bit is inverted so that setting
	it selects 25 MHz rather than 50 MHz clock mode. 

	Note that this is only needed for PHY variants that has this bit
	inverted and that a clock reference ("rmii-ref" below) is always
	needed to select the actual mode.

> It's not very nice from Micrel to create Phys with exactly the same
> device id but the meaning of this bit reversed.

Not very nice at all.

Thanks,
Johan

^ permalink raw reply

* Re: [PATCH] crypto: aesni-intel - avoid IPsec re-ordering
From: Steffen Klassert @ 2014-11-12  9:12 UTC (permalink / raw)
  To: Herbert Xu; +Cc: Ming Liu, davem, ying.xue, linux-crypto, netdev
In-Reply-To: <20141112085148.GA26268@gondor.apana.org.au>

On Wed, Nov 12, 2014 at 04:51:48PM +0800, Herbert Xu wrote:
> On Wed, Nov 12, 2014 at 09:41:38AM +0100, Steffen Klassert wrote:
> >
> > Can't we just use cryptd unconditionally to fix this reordering problem?
> 
> I think the idea is that most of the time cryptd isn't required
> so we want to stick with direct processing to lower latency.

Yes, I thought that. But is it really the case that doing it
asynchronous increases the latency? I tried this some time
ago and as far as I remember there was not too much difference
between the direct and the asynchronous case. Might depend
on the usecase of course.

> 
> I think the simplest fix would be to punt to cryptd as long as
> there are cryptd requests queued.

This would be the second option. We may need to adjust the
maximum cryptd queue lenght then, as the networking receive
softirq might enqueue a lot of packets before cryptd can
process them.

^ permalink raw reply

* Re: [PATCH] crypto: aesni-intel - avoid IPsec re-ordering
From: Herbert Xu @ 2014-11-12  8:51 UTC (permalink / raw)
  To: Steffen Klassert; +Cc: Ming Liu, davem, ying.xue, linux-crypto, netdev
In-Reply-To: <20141112084138.GL6390@secunet.com>

On Wed, Nov 12, 2014 at 09:41:38AM +0100, Steffen Klassert wrote:
>
> Can't we just use cryptd unconditionally to fix this reordering problem?

I think the idea is that most of the time cryptd isn't required
so we want to stick with direct processing to lower latency.

I think the simplest fix would be to punt to cryptd as long as
there are cryptd requests queued.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: How to make stack send broadcast ARP request when entry is STALE?
From: Ulf samuelsson @ 2014-11-12  8:46 UTC (permalink / raw)
  To: Brian Haley; +Cc: Netdev
In-Reply-To: <54623AF6.9020403@hp.com>



> 11 nov 2014 kl. 17:36 skrev Brian Haley <brian.haley@hp.com>:
> 
>> On 11/11/2014 05:08 AM, Ulf samuelsson wrote:
>> If I set ucast_solicit to '0', then I always send broadcast, which is not desirable.
> 
> That seems to contradict your statement:
> 
> "The HP router is configured by a customer, and they intentionally limit replies
> to broadcast, and that is how they want it."
> 

The equipment is beeing used in multiple locations, and this is but one.
Normally you want to follow IPv6 rules which says

* Start w broadcast
* Once reply arrives, start using unicast
* If unicast fails for 'ucast_probes', send up to 'mcast_probes' packets before FAIL.
* A reply to a broadcast packet will make the kernel revert to unicast


> So I'm not understanding your question.
> 
> The best way forward would be for you to send a patch out that gets your desired
> behaviour, and let others give feedback on it.
> 
> -Brian



Best Regards
Ulf Samuelsson



> 
>> In the PROBE state of the ARP state machine, "probes" count from 0 .. ucast_probes.
>> 
>> I can see the following arguments for letting "probes" count from 
>> 
>>   0.. (ucast_probes + app_probes + mcast_probes)
>> 
>> 
>> A: This is how the IPv6 is doing it. 
>>     It is not standardized in IPv4, but why should the IPv4 have a different behaviour?
>> 
>> B: If you do not send out broadcast if unicast fails, then a broadcast will be sent out 
>>     anyway, once the ARP entry is removed by the garbage collector.
>>     You get an annoyingly long delay before that happens.
>> 
>> C: If a large data centre does not want broadcasts to be sent out, 
>>     then they can set mcast_probes to 0, in which case no broadcasts will be sent
>>     out in PROBE state.
>> 
>> D:  When in other states, the counter runs until it a reply is had, or the counter wraps around.
>>      It is sending broadcast all the time.
>> 
>> 
>> Best Regards
>> Ulf Samuelsson
>> ulf@emagii.com
>> +46  (722) 427 437
>> 
>> 
>>>> 10 nov 2014 kl. 23:52 skrev Brian Haley <brian.haley@hp.com>:
>>>> 
>>>> On 11/07/2014 05:11 AM, Ulf samuelsson wrote:
>>>> The HP router is configured by a customer, and they intentionally limit replies
>>>> to broadcast, and that is how they want it.
>>> 
>>> So this is the crux of the problem - the customer has configured the router so
>>> that it doesn't play well with most modern network stacks that try and use
>>> unicast so they don't send unnecessary broadcast packets.  I don't know why I
>>> thought this was something wrong with the router software.
>>> 
>>> Did you try this?
>>> 
>>> $ sudo sysctl net.ipv4.neigh.eth0.ucast_solicit=0
>>> 
>>> It works for me.
>>> 
>>> And they really should re-think their decision on that configuration setting.
>>> 
>>> -Brian
>>> 
>>> 
>>>> In the previous version of the build system, the Interpeak stack was used
>>>> and this would in PROBE state send unicast ARP request, and if that failed
>>>> send broadcast ARP.
>>>> 
>>>> The native linux stack, when in PROBE state sends only unicast until it decides
>>>> that it should enter FAILED state.
>>>> 
>>>> The 'mcast_probes' variable seems to be totally ignored, except the first  time,
>>>> so I do not see why it is there.
>>>> 
>>>> Best Regards
>>>> Ulf Samuelsson
>>>> ulf@emagii.com
>>>> +46  (722) 427 437
>>>> 
>>>> 
>>>>>> 7 nov 2014 kl. 10:54 skrev Brian Haley <brian.haley@hp.com>:
>>>>>> 
>>>>>> On 11/05/2014 07:48 AM, Ulf samuelsson wrote:
>>>>>> Have a problem with an HP router at a certain location, which
>>>>>> is configured to only answer to broadcast ARP requests.
>>>>>> That cannot be changed.
>>>>> 
>>>>> Sorry to hear about the problem, but my only suggestions would be to try the latest firmware and/or put a call in to support.  I don't happen work in the division that makes routers...
>>>>> 
>>>>>> The first ARP request the kernel sends out, is a broadcast request,
>>>>>> which is fine, but after the reply, the kernel sends unicast requests,
>>>>>> which will not get any replies.
>>>>> 
>>>>> You might be able to hack this by inserting an ebtables rule - check the dnat target section of the man page - don't know the exact syntax but it would probably end in '-j dnat --to-destination ff:ff:ff:ff:ff:ff'
>>>>> 
>>>>> -Brian
>>>>> --
>>>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>>>>> the body of a message to majordomo@vger.kernel.org
>>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply

* Re: [PATCH] crypto: aesni-intel - avoid IPsec re-ordering
From: Steffen Klassert @ 2014-11-12  8:41 UTC (permalink / raw)
  To: Ming Liu; +Cc: herbert, davem, ying.xue, linux-crypto, netdev
In-Reply-To: <1415771371-30774-1-git-send-email-ming.liu@windriver.com>

On Wed, Nov 12, 2014 at 01:49:31PM +0800, Ming Liu wrote:
> So far, the encryption/decryption are asynchronously processed in
> softirq and cryptd which would result in a implicit order of data,
> therefore leads IPSec stack also out of order while encapsulating
> or decapsulating packets.
> 
> Consider the following scenario:
> 
>              DECRYPTION INBOUND
>                       |
>                       |
>              +-----Packet A
>              |        |
>              |        |
>              |     Packet B
>              |        |
>     (cryptd) |        | (software interrupts)
>              |      ......
>              |        |
>              |        |
>              |     Packet B(decrypted)
>              |        |
>              |        |
>              +---> Packet A(decrypted)
>                       |
>                       |
>              DECRYPTION OUTBOUND
> 
> Add cryptd_flush_queue function fixing it by being called from softirq
> to flush all previous queued elements before processing a new one. To
> prevent cryptd_flush_queue() being accessed from software interrupts,
> local_bh_disable/enable needs to be relocated in several places.
> 
> Signed-off-by: Ming Liu <ming.liu@windriver.com>
> ---
> I was told that I need resend this patch with git, so here it is:
> 
> I've figured out a new patch for this issue reported by me previously,
> the basic idea is adding a cryptd_flush_queue function fixing it by
> being called from softirq to flush all previous queued elements
> before processing a new one, and it works very well so far per my test.

I doubt that this approach can work. The cryptd encrypt/decrypt functions
assume to be called from a workqueue worker, they can't be called from
atomic context.

Can't we just use cryptd unconditionally to fix this reordering problem?

> 
>  crypto/ablk_helper.c    |  10 ++++-
>  crypto/cryptd.c         | 107 ++++++++++++++++++++++++++++++++++++++++--------
>  include/crypto/cryptd.h |  13 ++++++
>  3 files changed, 111 insertions(+), 19 deletions(-)
> 
> diff --git a/crypto/ablk_helper.c b/crypto/ablk_helper.c
> index ffe7278..600a70f 100644
> --- a/crypto/ablk_helper.c
> +++ b/crypto/ablk_helper.c
> @@ -70,16 +70,19 @@ int ablk_encrypt(struct ablkcipher_request *req)
>  {
>  	struct crypto_ablkcipher *tfm = crypto_ablkcipher_reqtfm(req);
>  	struct async_helper_ctx *ctx = crypto_ablkcipher_ctx(tfm);
> +	struct crypto_tfm *req_tfm = crypto_ablkcipher_tfm(
> +		crypto_ablkcipher_crt(&ctx->cryptd_tfm->base)->base);
>  
>  	if (!may_use_simd()) {
>  		struct ablkcipher_request *cryptd_req =
>  			ablkcipher_request_ctx(req);
>  
>  		*cryptd_req = *req;
> -		ablkcipher_request_set_tfm(cryptd_req, &ctx->cryptd_tfm->base);
> +		cryptd_req->base.tfm = req_tfm;
>  
>  		return crypto_ablkcipher_encrypt(cryptd_req);
>  	} else {
> +		cryptd_flush_queue(req_tfm, CRYPTD_ENCRYPT);

Where is CRYPTD_ENCRYPT defined?
This does not even compile when applied to the cryptodev tree.

>  		return __ablk_encrypt(req);
>  	}
>  }
> @@ -89,13 +92,15 @@ int ablk_decrypt(struct ablkcipher_request *req)
>  {
>  	struct crypto_ablkcipher *tfm = crypto_ablkcipher_reqtfm(req);
>  	struct async_helper_ctx *ctx = crypto_ablkcipher_ctx(tfm);
> +	struct crypto_tfm *req_tfm = crypto_ablkcipher_tfm(
> +		crypto_ablkcipher_crt(&ctx->cryptd_tfm->base)->base);
>  
>  	if (!may_use_simd()) {
>  		struct ablkcipher_request *cryptd_req =
>  			ablkcipher_request_ctx(req);
>  
>  		*cryptd_req = *req;
> -		ablkcipher_request_set_tfm(cryptd_req, &ctx->cryptd_tfm->base);
> +		cryptd_req->base.tfm = req_tfm;
>  
>  		return crypto_ablkcipher_decrypt(cryptd_req);
>  	} else {
> @@ -105,6 +110,7 @@ int ablk_decrypt(struct ablkcipher_request *req)
>  		desc.info = req->info;
>  		desc.flags = 0;
>  
> +		cryptd_flush_queue(req_tfm, CRYPTD_DECRYPT);

Same here.

>  		return crypto_blkcipher_crt(desc.tfm)->decrypt(
>  			&desc, req->dst, req->src, req->nbytes);
>  	}
> diff --git a/crypto/cryptd.c b/crypto/cryptd.c
> index e592c90..0b387a1 100644
> --- a/crypto/cryptd.c
> +++ b/crypto/cryptd.c
> @@ -119,11 +119,13 @@ static int cryptd_enqueue_request(struct cryptd_queue *queue,
>  	int cpu, err;
>  	struct cryptd_cpu_queue *cpu_queue;
>  
> +	local_bh_disable();
>  	cpu = get_cpu();
>  	cpu_queue = this_cpu_ptr(queue->cpu_queue);
>  	err = crypto_enqueue_request(&cpu_queue->queue, request);
>  	queue_work_on(cpu, kcrypto_wq, &cpu_queue->work);
>  	put_cpu();
> +	local_bh_enable();
>  
>  	return err;
>  }
> @@ -147,11 +149,9 @@ static void cryptd_queue_worker(struct work_struct *work)
>  	preempt_disable();
>  	backlog = crypto_get_backlog(&cpu_queue->queue);
>  	req = crypto_dequeue_request(&cpu_queue->queue);
> -	preempt_enable();
> -	local_bh_enable();

Everything below the local_bh_enable() should not run in atomic context
as the subsequent functions may set the CRYPTO_TFM_REQ_MAY_SLEEP flag.

>  
>  	if (!req)
> -		return;
> +		goto out;
>  
>  	if (backlog)
>  		backlog->complete(backlog, -EINPROGRESS);
> @@ -159,6 +159,9 @@ static void cryptd_queue_worker(struct work_struct *work)
>  
>  	if (cpu_queue->queue.qlen)
>  		queue_work(kcrypto_wq, &cpu_queue->work);
> +out:
> +	preempt_enable();
> +	local_bh_enable();
>  }

...

>  
> +void cryptd_flush_queue(struct crypto_tfm *tfm, cryptd_type_t type)
> +{
> +	struct crypto_instance *inst = crypto_tfm_alg_instance(tfm);
> +	struct cryptd_instance_ctx *ictx = crypto_instance_ctx(inst);
> +	struct cryptd_queue *cryptd_queue = ictx->queue;
> +	struct cryptd_cpu_queue *cpu_queue;
> +	struct crypto_queue *queue;
> +	struct crypto_async_request *req, *tmp, *backlog;
> +	crypto_completion_t complete;
> +	int cpu;
> +	unsigned int len;
> +
> +	switch (type) {
> +	case CRYPTD_BLKCIPHER_ENCRYPT:
> +		complete = cryptd_blkcipher_encrypt;
> +		break;
> +	case CRYPTD_BLKCIPHER_DECRYPT:
> +		complete = cryptd_blkcipher_decrypt;
> +		break;
> +	case CRYPTD_HASH_INIT:
> +		complete = cryptd_hash_init;
> +		break;
> +	case CRYPTD_HASH_UPDATE:
> +		complete = cryptd_hash_update;
> +		break;
> +	case CRYPTD_HASH_FINAL:
> +		complete = cryptd_hash_final;
> +		break;
> +	case CRYPTD_HASH_FINUP:
> +		complete = cryptd_hash_finup;
> +		break;
> +	case CRYPTD_HASH_DIGEST:
> +		complete = cryptd_hash_digest;
> +		break;
> +	case CRYPTD_AEAD_ENCRYPT:
> +		complete = cryptd_aead_encrypt;
> +		break;
> +	case CRYPTD_AEAD_DECRYPT:
> +		complete = cryptd_aead_decrypt;
> +		break;
> +	default:
> +		complete = NULL;
> +	}
> +
> +	if (complete == NULL)
> +		return;
> +
> +	local_bh_disable();
> +	cpu = get_cpu();
> +	cpu_queue = this_cpu_ptr(cryptd_queue->cpu_queue);
> +	queue = &cpu_queue->queue;
> +	len = queue->qlen;
> +
> +	if (!len)
> +		goto out;
> +
> +	list_for_each_entry_safe(req, tmp, &queue->list, list) {
> +		if (req->complete == complete) {
> +			queue->qlen--;
> +
> +			if (queue->backlog != &queue->list) {
> +				backlog = container_of(queue->backlog,
> +					struct crypto_async_request, list);
> +				queue->backlog = queue->backlog->next;
> +			} else
> +				backlog = NULL;
> +
> +			list_del(&req->list);
> +
> +			if (backlog)
> +				backlog->complete(backlog, -EINPROGRESS);
> +			req->complete(req, 0);

Same here, the complete function can not run in atomic context.

Also, this can not ensure that the request is finalized.
Subsequent algorithms may run asynchronously too, so this
does not fix the reordering problem completely.

^ permalink raw reply

* [patch -next] amd-xgbe: fix ->rss_hash_type
From: Dan Carpenter @ 2014-11-12  8:31 UTC (permalink / raw)
  To: Tom Lendacky; +Cc: netdev, kernel-janitors

There was a missing break statement so we set everything to
PKT_HASH_TYPE_L3 even when we intended to use PKT_HASH_TYPE_L4.

Fixes: 5b9dfe299e55 ('amd-xgbe: Provide support for receive side scaling')
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
This driver has a lot of Sparse endian warnings.
http://lwn.net/Articles/205624/

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
index 7daa2cd..55ba1dc 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
@@ -1601,6 +1601,7 @@ static int xgbe_dev_read(struct xgbe_channel *channel)
 		case RX_DESC3_L34T_IPV6_UDP:
 			packet->rss_hash_type = PKT_HASH_TYPE_L4;
 
+			break;
 		default:
 			packet->rss_hash_type = PKT_HASH_TYPE_L3;
 		}

^ permalink raw reply related

* Re: [patch net-next 2/2] sched: introduce vlan action
From: Jiri Pirko @ 2014-11-12  7:47 UTC (permalink / raw)
  To: Cong Wang
  Cc: netdev, David Miller, Jamal Hadi Salim, Pravin B Shelar,
	Tom Herbert, Eric Dumazet, willemb, Daniel Borkmann, mst,
	Florian Westphal, Paul.Durrant, Thomas Graf
In-Reply-To: <CAHA+R7P1PM5=TJYRPPrYc_U4AnLvsgQPZr0coTbeijAUGvB9zg@mail.gmail.com>

Wed, Nov 12, 2014 at 12:18:47AM CET, cwang@twopensource.com wrote:
>On Tue, Nov 11, 2014 at 2:13 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>> This tc action allows to work with vlan tagged skbs. Two supported
>> sub-actions are header pop and header push.
>>
>
>Can we add this to skbedit instead of adding a new action?
>
>I know vlan tag is not exactly the skb metadata, but still seems
>fits in skbedit for me.

I was thinking about that as well. It seems much clearer to add a new
action.

^ permalink raw reply

* Re: [PATCH nf] netfilter: conntrack: fix race in __nf_conntrack_confirm against get_next_corpse
From: Jesper Dangaard Brouer @ 2014-11-12  7:35 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: programme110, netfilter-devel, Florian Westphal, netdev, brouer,
	Patrick McHardy, Joerg Marx
In-Reply-To: <20141110165439.GA7788@salvia>

On Mon, 10 Nov 2014 17:54:39 +0100
Pablo Neira Ayuso <pablo@netfilter.org> wrote:

> On Thu, Nov 06, 2014 at 02:36:48PM +0100, Jesper Dangaard Brouer wrote:
> > From: bill bonaparte <programme110@gmail.com>
> > 
> > After removal of the central spinlock nf_conntrack_lock, in
> > commit 93bb0ceb75be2 ("netfilter: conntrack: remove central
> > spinlock nf_conntrack_lock"), it is possible to race against
> > get_next_corpse().
> > 
> > The race is against the get_next_corpse() cleanup on
> > the "unconfirmed" list (a per-cpu list with seperate locking),
> > which set the DYING bit.
> > 
> > Fix this race, in __nf_conntrack_confirm(), by removing the CT
> > from unconfirmed list before checking the DYING bit.  In case
> > race occured, re-add the CT to the dying list.
> 
> This seems correct to me, some side comments.
> 
> > Fixes: 93bb0ceb75be2 ("netfilter: conntrack: remove central spinlock nf_conntrack_lock")
> > Reported-by: bill bonaparte <programme110@gmail.com>
> > Signed-off-by: bill bonaparte <programme110@gmail.com>
> > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> > ---
> > 
> >  net/netfilter/nf_conntrack_core.c |    7 ++++---
> >  1 files changed, 4 insertions(+), 3 deletions(-)
> > 
> > diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
> > index 5016a69..1072650 100644
> > --- a/net/netfilter/nf_conntrack_core.c
> > +++ b/net/netfilter/nf_conntrack_core.c
> > @@ -611,12 +611,15 @@ __nf_conntrack_confirm(struct sk_buff *skb)
> >  	 */
> >  	NF_CT_ASSERT(!nf_ct_is_confirmed(ct));
> >  	pr_debug("Confirming conntrack %p\n", ct);
> > -	/* We have to check the DYING flag inside the lock to prevent
> > +
> > +	/* We have to check the DYING flag after unlink to prevent
> >  	   a race against nf_ct_get_next_corpse() possibly called from
> >  	   user context, else we insert an already 'dead' hash, blocking
> >  	   further use of that particular connection -JM */
> 
> While at this, I think it would be good to fix comment style to:
> 
>         /* We have ...
>          * ...
>          */
> 
> I can fix this here, no need to resend, just let me know.

Okay, I was just trying to keep the changes as minimal as possible, if
this should go into a stable-kernel.  Your choice.


> > +	nf_ct_del_from_dying_or_unconfirmed_list(ct);
> >  
> >  	if (unlikely(nf_ct_is_dying(ct))) {
> > +		nf_ct_add_to_dying_list(ct);
> >  		nf_conntrack_double_unlock(hash, reply_hash);
> >  		local_bh_enable();
> >  		return NF_ACCEPT;
> 
> Not directly related to your patch, but I don't find a good reason why
> we're accepting this packet.
> 
> If the conntrack from the unconfirmed list is dying, then the object
> will be released by when the packet leaves the stack to its
> destination. With stateful filtering depending in place, the follow up
> packet in the reply direction will likely be considered invalid (if
> tcp tracking is on). Fortunately for us, the origin will likely
> retransmit the syn again, so the ct will be setup accordingly.
> 
> So, why should we allow this to go through?

True, it also seems strange to me that we accept this packet.

> This return verdict was introduced in: fc35077 ("netfilter:
> nf_conntrack: fix a race in __nf_conntrack_confirm against
> nf_ct_get_next_corpse()") btw.

And the commit does not argue why NF_ACCEPT was chosen...

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Sr. Network Kernel Developer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* [PATCH net] cxgb4 : dcb open-lldp interop fixes
From: Anish Bhatt @ 2014-11-12  7:30 UTC (permalink / raw)
  To: netdev; +Cc: davem, hariprasad, leedom, Anish Bhatt

* In LLD_MANAGED mode, traffic classes were being returned in reverse order to
  lldp agent.
* Priotype of strict is no longer the default returned.
* Change behaviour of getdcbx() based on discussions on lldp-devel

These were missed as there was no working fetch interface for open-lldp when
running in LLD_MANAGED mode till now.

Fixes: 76bcb31efc06 ("cxgb4 : Add DCBx support codebase and dcbnl_ops")

Signed-off-by: Anish Bhatt <anish@chelsio.com>
---
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c | 28 +++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c
index b6fdb14..cca6049 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c
@@ -123,7 +123,11 @@ void cxgb4_dcb_state_fsm(struct net_device *dev,
 		case CXGB4_DCB_INPUT_FW_ENABLED: {
 			/* we're going to use Firmware DCB */
 			dcb->state = CXGB4_DCB_STATE_FW_INCOMPLETE;
-			dcb->supported = CXGB4_DCBX_FW_SUPPORT;
+			dcb->supported = DCB_CAP_DCBX_LLD_MANAGED;
+			if (dcb->dcb_version == FW_PORT_DCB_VER_IEEE)
+				dcb->supported |= DCB_CAP_DCBX_VER_IEEE;
+			else
+				dcb->supported |= DCB_CAP_DCBX_VER_CEE;
 			break;
 		}
 
@@ -437,14 +441,17 @@ static void cxgb4_getpgtccfg(struct net_device *dev, int tc,
 	*up_tc_map = (1 << tc);
 
 	/* prio_type is link strict */
-	*prio_type = 0x2;
+	if (*pgid != 0xF)
+		*prio_type = 0x2;
 }
 
 static void cxgb4_getpgtccfg_tx(struct net_device *dev, int tc,
 				u8 *prio_type, u8 *pgid, u8 *bw_per,
 				u8 *up_tc_map)
 {
-	return cxgb4_getpgtccfg(dev, tc, prio_type, pgid, bw_per, up_tc_map, 1);
+	/* tc 0 is written at MSB position */
+	return cxgb4_getpgtccfg(dev, (7 - tc), prio_type, pgid, bw_per,
+				up_tc_map, 1);
 }
 
 
@@ -452,7 +459,9 @@ static void cxgb4_getpgtccfg_rx(struct net_device *dev, int tc,
 				u8 *prio_type, u8 *pgid, u8 *bw_per,
 				u8 *up_tc_map)
 {
-	return cxgb4_getpgtccfg(dev, tc, prio_type, pgid, bw_per, up_tc_map, 0);
+	/* tc 0 is written at MSB position */
+	return cxgb4_getpgtccfg(dev, (7 - tc), prio_type, pgid, bw_per,
+				up_tc_map, 0);
 }
 
 static void cxgb4_setpgtccfg_tx(struct net_device *dev, int tc,
@@ -462,6 +471,7 @@ static void cxgb4_setpgtccfg_tx(struct net_device *dev, int tc,
 	struct fw_port_cmd pcmd;
 	struct port_info *pi = netdev2pinfo(dev);
 	struct adapter *adap = pi->adapter;
+	int fw_tc = 7 - tc;
 	u32 _pgid;
 	int err;
 
@@ -480,8 +490,8 @@ static void cxgb4_setpgtccfg_tx(struct net_device *dev, int tc,
 	}
 
 	_pgid = be32_to_cpu(pcmd.u.dcb.pgid.pgid);
-	_pgid &= ~(0xF << (tc * 4));
-	_pgid |= pgid << (tc * 4);
+	_pgid &= ~(0xF << (fw_tc * 4));
+	_pgid |= pgid << (fw_tc * 4);
 	pcmd.u.dcb.pgid.pgid = cpu_to_be32(_pgid);
 
 	INIT_PORT_DCB_WRITE_CMD(pcmd, pi->port_id);
@@ -594,7 +604,7 @@ static void cxgb4_getpfccfg(struct net_device *dev, int priority, u8 *pfccfg)
 	    priority >= CXGB4_MAX_PRIORITY)
 		*pfccfg = 0;
 	else
-		*pfccfg = (pi->dcb.pfcen >> priority) & 1;
+		*pfccfg = (pi->dcb.pfcen >> (7 - priority)) & 1;
 }
 
 /* Enable/disable Priority Pause Frames for the specified Traffic Class
@@ -619,9 +629,9 @@ static void cxgb4_setpfccfg(struct net_device *dev, int priority, u8 pfccfg)
 	pcmd.u.dcb.pfc.pfcen = pi->dcb.pfcen;
 
 	if (pfccfg)
-		pcmd.u.dcb.pfc.pfcen |= (1 << priority);
+		pcmd.u.dcb.pfc.pfcen |= (1 << (7 - priority));
 	else
-		pcmd.u.dcb.pfc.pfcen &= (~(1 << priority));
+		pcmd.u.dcb.pfc.pfcen &= (~(1 << (7 - priority)));
 
 	err = t4_wr_mbox(adap, adap->mbox, &pcmd, sizeof(pcmd), &pcmd);
 	if (err != FW_PORT_DCB_CFG_SUCCESS) {
-- 
2.1.3

^ permalink raw reply related

* Re: [PATCH 19/22] dt/bindings: add micrel,rmii_ref_clk_sel_25_mhz to eth-phy binding
From: Sascha Hauer @ 2014-11-12  7:01 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Mark Rutland, Florian Fainelli, David S. Miller,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	devicetree@vger.kernel.org
In-Reply-To: <20141111181825.GH29789@localhost>

On Tue, Nov 11, 2014 at 07:18:25PM +0100, Johan Hovold wrote:
> On Tue, Nov 11, 2014 at 05:57:42PM +0000, Mark Rutland wrote:
> > On Tue, Nov 11, 2014 at 05:37:37PM +0000, Johan Hovold wrote:
> > > Add "micrel,rmii_ref_clk_sel_25_mhz" to Micrel ethernet PHY binding
> > > documentation.
> > > 
> > > Cc: devicetree@vger.kernel.org
> > > Signed-off-by: Johan Hovold <johan@kernel.org>
> > > ---
> > >  Documentation/devicetree/bindings/net/micrel.txt | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/net/micrel.txt b/Documentation/devicetree/bindings/net/micrel.txt
> > > index a1bab5eaae02..9b08dd6551dd 100644
> > > --- a/Documentation/devicetree/bindings/net/micrel.txt
> > > +++ b/Documentation/devicetree/bindings/net/micrel.txt
> > > @@ -19,6 +19,11 @@ Optional properties:
> > >  
> > >                See the respective PHY datasheet for the mode values.
> > >  
> > > + - micrel,rmii_ref_clk_sel_25_mhz: rmii_ref_clk_sel bit selects 25 MHz mode
> > > +
> > > +		Whether 25 MHz (rather than 50 Mhz) clock mode is selected
> > > +		when the rmii_ref_clk_sel bit is set.
> > 
> > s/_/-/ in property names please.
> 
> Ouch, copied from variable name, sorry.
> 
> > That said, I don't follow the meaning. Does this cause the kernel to do
> > something different, or is is simply that a 25MHz ref clock is wired up?
> 
> Yes, the driver currently sets this configuration bit based on a common
> clock binding.
> 
> However, it turns out the meaning of the bit is reversed on some PHY
> variants. On most PHYs 50 MHz mode is selected by setting this bit,
> whereas on the PHYs that need this new property, setting it selects 25
> MHz mode instead.

Maybe rename the property to something like rmii-ref-clk-25mhz-active-high
then? Also you should probably make it more explicit that this is a
hardware property and not for adjusting the clock.

It's not very nice from Micrel to create Phys with exactly the same
device id but the meaning of this bit reversed.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply

* Re: [PATCH] brcmfmac: unlink URB when request timed out
From: Oliver Neukum @ 2014-11-12  7:00 UTC (permalink / raw)
  To: Mathy Vanhoef
  Cc: brudley-dY08KVG/lbpWk0Htik3J/w, Arend van Spriel, Franky Lin,
	meuleman-dY08KVG/lbpWk0Htik3J/w, John Linville,
	pieterpg-dY08KVG/lbpWk0Htik3J/w,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	brcm80211-dev-list-dY08KVG/lbpWk0Htik3J/w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <5462B1A3.9020401-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On Tue, 2014-11-11 at 20:02 -0500, Mathy Vanhoef wrote:
> On 11/10/2014 04:08 AM, Oliver Neukum wrote:

> > Which means that you are freeing memory that may still be used by DMA
> > at this time.
> > In addition you have no guarantee that the unlink is indeed finished
> > by the time the URB is reused.
> > If you wish to take this approach you better forget about this URB
> > and allocate a new one and free the buffer from the callback.
> 
> Hi Oliver,
> 
> Good catch. I think the DMA issue is also present in the current driver: it
> frees the buffer without unlinking/killing the URB at all. Can a malicious USB

Yes, it is present.

> device force a timeout to occur (i.e. delay the call to the completion
> handler)? If so this might be a use-after-free vulnerability.
> 
> It seems using usb_kill_urb instead of usb_unlink_urb in the patch prevents any
> possible use-after-free. Can someone double check?

usb_kill_urb() will do the job.

	Regards
		Oliver


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

^ permalink raw reply

* RE: [PATCH net-next 2/2] r8152: adjust rtl_start_rx
From: Hayes Wang @ 2014-11-12  6:29 UTC (permalink / raw)
  To: David Miller
  Cc: netdev@vger.kernel.org, nic_swsd, linux-kernel@vger.kernel.org,
	linux-usb@vger.kernel.org
In-Reply-To: <20141112.004331.1112374574697663525.davem@davemloft.net>

David Miller [mailto:davem@davemloft.net] 
> Sent: Wednesday, November 12, 2014 1:44 PM
[...]
> What do other USB network drivers do in similar situations?

According to the usbnet.c, it would make sure to submit the
number of min(10, RX_QLEN(dev)) rx buffers. If there are
not enough rx buffers, it schedule a tasklet for next try.

The brief flow is as following.
1. Call open().
   - schedule a tasklet.
2. Tasklet is called.
   if (dev->rxq.qlen < RX_QLEN(dev)) {
	   - submit rx buffers util the number of
	     min(10, RX_QLEN(dev)). If the error
	     occurs, break the loop.
	   - If the dev->rxq.qlen < RX_QLEN(dev),
	     schedule the tasklet.
   }

Best Regards,
Hayes

^ permalink raw reply

* [PATCH net-next] udp: Neaten function pointer calls and add braces
From: Joe Perches @ 2014-11-12  5:59 UTC (permalink / raw)
  To: netdev; +Cc: Peter Hurley

Standardize function pointer uses.

Convert calling style from:
	(*foo)(args...);
to:
	foo(args...);

Other miscellanea:

o Add braces around loops with single ifs on multiple lines
o Realign arguments around these functions
o Invert logic in if to return immediately.

Signed-off-by: Joe Perches <joe@perches.com>
---
 net/ipv4/udp.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 5d0fdca..6a0d940 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -144,7 +144,7 @@ static int udp_lib_lport_inuse(struct net *net, __u16 num,
 	struct hlist_nulls_node *node;
 	kuid_t uid = sock_i_uid(sk);
 
-	sk_nulls_for_each(sk2, node, &hslot->head)
+	sk_nulls_for_each(sk2, node, &hslot->head) {
 		if (net_eq(sock_net(sk2), net) &&
 		    sk2 != sk &&
 		    (bitmap || udp_sk(sk2)->udp_port_hash == num) &&
@@ -152,14 +152,13 @@ static int udp_lib_lport_inuse(struct net *net, __u16 num,
 		    (!sk2->sk_bound_dev_if || !sk->sk_bound_dev_if ||
 		     sk2->sk_bound_dev_if == sk->sk_bound_dev_if) &&
 		    (!sk2->sk_reuseport || !sk->sk_reuseport ||
-		      !uid_eq(uid, sock_i_uid(sk2))) &&
-		    (*saddr_comp)(sk, sk2)) {
-			if (bitmap)
-				__set_bit(udp_sk(sk2)->udp_port_hash >> log,
-					  bitmap);
-			else
+		     !uid_eq(uid, sock_i_uid(sk2))) &&
+		    saddr_comp(sk, sk2)) {
+			if (!bitmap)
 				return 1;
+			__set_bit(udp_sk(sk2)->udp_port_hash >> log, bitmap);
 		}
+	}
 	return 0;
 }
 
@@ -168,10 +167,10 @@ static int udp_lib_lport_inuse(struct net *net, __u16 num,
  * can insert/delete a socket with local_port == num
  */
 static int udp_lib_lport_inuse2(struct net *net, __u16 num,
-			       struct udp_hslot *hslot2,
-			       struct sock *sk,
-			       int (*saddr_comp)(const struct sock *sk1,
-						 const struct sock *sk2))
+				struct udp_hslot *hslot2,
+				struct sock *sk,
+				int (*saddr_comp)(const struct sock *sk1,
+						  const struct sock *sk2))
 {
 	struct sock *sk2;
 	struct hlist_nulls_node *node;
@@ -179,7 +178,7 @@ static int udp_lib_lport_inuse2(struct net *net, __u16 num,
 	int res = 0;
 
 	spin_lock(&hslot2->lock);
-	udp_portaddr_for_each_entry(sk2, node, &hslot2->head)
+	udp_portaddr_for_each_entry(sk2, node, &hslot2->head) {
 		if (net_eq(sock_net(sk2), net) &&
 		    sk2 != sk &&
 		    (udp_sk(sk2)->udp_port_hash == num) &&
@@ -187,11 +186,12 @@ static int udp_lib_lport_inuse2(struct net *net, __u16 num,
 		    (!sk2->sk_bound_dev_if || !sk->sk_bound_dev_if ||
 		     sk2->sk_bound_dev_if == sk->sk_bound_dev_if) &&
 		    (!sk2->sk_reuseport || !sk->sk_reuseport ||
-		      !uid_eq(uid, sock_i_uid(sk2))) &&
-		    (*saddr_comp)(sk, sk2)) {
+		     !uid_eq(uid, sock_i_uid(sk2))) &&
+		    saddr_comp(sk, sk2)) {
 			res = 1;
 			break;
 		}
+	}
 	spin_unlock(&hslot2->lock);
 	return res;
 }
@@ -206,8 +206,8 @@ static int udp_lib_lport_inuse2(struct net *net, __u16 num,
  *                   with NULL address
  */
 int udp_lib_get_port(struct sock *sk, unsigned short snum,
-		       int (*saddr_comp)(const struct sock *sk1,
-					 const struct sock *sk2),
+		     int (*saddr_comp)(const struct sock *sk1,
+				       const struct sock *sk2),
 		     unsigned int hash2_nulladdr)
 {
 	struct udp_hslot *hslot, *hslot2;
@@ -2032,7 +2032,7 @@ int udp_lib_setsockopt(struct sock *sk, int level, int optname,
 		} else {
 			up->corkflag = 0;
 			lock_sock(sk);
-			(*push_pending_frames)(sk);
+			push_pending_frames(sk);
 			release_sock(sk);
 		}
 		break;

^ permalink raw reply related

* [PATCH] crypto: aesni-intel - avoid IPsec re-ordering
From: Ming Liu @ 2014-11-12  5:49 UTC (permalink / raw)
  To: herbert, steffen.klassert; +Cc: davem, ying.xue, linux-crypto, netdev

So far, the encryption/decryption are asynchronously processed in
softirq and cryptd which would result in a implicit order of data,
therefore leads IPSec stack also out of order while encapsulating
or decapsulating packets.

Consider the following scenario:

             DECRYPTION INBOUND
                      |
                      |
             +-----Packet A
             |        |
             |        |
             |     Packet B
             |        |
    (cryptd) |        | (software interrupts)
             |      ......
             |        |
             |        |
             |     Packet B(decrypted)
             |        |
             |        |
             +---> Packet A(decrypted)
                      |
                      |
             DECRYPTION OUTBOUND

Add cryptd_flush_queue function fixing it by being called from softirq
to flush all previous queued elements before processing a new one. To
prevent cryptd_flush_queue() being accessed from software interrupts,
local_bh_disable/enable needs to be relocated in several places.

Signed-off-by: Ming Liu <ming.liu@windriver.com>
---
I was told that I need resend this patch with git, so here it is:

I've figured out a new patch for this issue reported by me previously,
the basic idea is adding a cryptd_flush_queue function fixing it by
being called from softirq to flush all previous queued elements
before processing a new one, and it works very well so far per my test.

 crypto/ablk_helper.c    |  10 ++++-
 crypto/cryptd.c         | 107 ++++++++++++++++++++++++++++++++++++++++--------
 include/crypto/cryptd.h |  13 ++++++
 3 files changed, 111 insertions(+), 19 deletions(-)

diff --git a/crypto/ablk_helper.c b/crypto/ablk_helper.c
index ffe7278..600a70f 100644
--- a/crypto/ablk_helper.c
+++ b/crypto/ablk_helper.c
@@ -70,16 +70,19 @@ int ablk_encrypt(struct ablkcipher_request *req)
 {
 	struct crypto_ablkcipher *tfm = crypto_ablkcipher_reqtfm(req);
 	struct async_helper_ctx *ctx = crypto_ablkcipher_ctx(tfm);
+	struct crypto_tfm *req_tfm = crypto_ablkcipher_tfm(
+		crypto_ablkcipher_crt(&ctx->cryptd_tfm->base)->base);
 
 	if (!may_use_simd()) {
 		struct ablkcipher_request *cryptd_req =
 			ablkcipher_request_ctx(req);
 
 		*cryptd_req = *req;
-		ablkcipher_request_set_tfm(cryptd_req, &ctx->cryptd_tfm->base);
+		cryptd_req->base.tfm = req_tfm;
 
 		return crypto_ablkcipher_encrypt(cryptd_req);
 	} else {
+		cryptd_flush_queue(req_tfm, CRYPTD_ENCRYPT);
 		return __ablk_encrypt(req);
 	}
 }
@@ -89,13 +92,15 @@ int ablk_decrypt(struct ablkcipher_request *req)
 {
 	struct crypto_ablkcipher *tfm = crypto_ablkcipher_reqtfm(req);
 	struct async_helper_ctx *ctx = crypto_ablkcipher_ctx(tfm);
+	struct crypto_tfm *req_tfm = crypto_ablkcipher_tfm(
+		crypto_ablkcipher_crt(&ctx->cryptd_tfm->base)->base);
 
 	if (!may_use_simd()) {
 		struct ablkcipher_request *cryptd_req =
 			ablkcipher_request_ctx(req);
 
 		*cryptd_req = *req;
-		ablkcipher_request_set_tfm(cryptd_req, &ctx->cryptd_tfm->base);
+		cryptd_req->base.tfm = req_tfm;
 
 		return crypto_ablkcipher_decrypt(cryptd_req);
 	} else {
@@ -105,6 +110,7 @@ int ablk_decrypt(struct ablkcipher_request *req)
 		desc.info = req->info;
 		desc.flags = 0;
 
+		cryptd_flush_queue(req_tfm, CRYPTD_DECRYPT);
 		return crypto_blkcipher_crt(desc.tfm)->decrypt(
 			&desc, req->dst, req->src, req->nbytes);
 	}
diff --git a/crypto/cryptd.c b/crypto/cryptd.c
index e592c90..0b387a1 100644
--- a/crypto/cryptd.c
+++ b/crypto/cryptd.c
@@ -119,11 +119,13 @@ static int cryptd_enqueue_request(struct cryptd_queue *queue,
 	int cpu, err;
 	struct cryptd_cpu_queue *cpu_queue;
 
+	local_bh_disable();
 	cpu = get_cpu();
 	cpu_queue = this_cpu_ptr(queue->cpu_queue);
 	err = crypto_enqueue_request(&cpu_queue->queue, request);
 	queue_work_on(cpu, kcrypto_wq, &cpu_queue->work);
 	put_cpu();
+	local_bh_enable();
 
 	return err;
 }
@@ -147,11 +149,9 @@ static void cryptd_queue_worker(struct work_struct *work)
 	preempt_disable();
 	backlog = crypto_get_backlog(&cpu_queue->queue);
 	req = crypto_dequeue_request(&cpu_queue->queue);
-	preempt_enable();
-	local_bh_enable();
 
 	if (!req)
-		return;
+		goto out;
 
 	if (backlog)
 		backlog->complete(backlog, -EINPROGRESS);
@@ -159,6 +159,9 @@ static void cryptd_queue_worker(struct work_struct *work)
 
 	if (cpu_queue->queue.qlen)
 		queue_work(kcrypto_wq, &cpu_queue->work);
+out:
+	preempt_enable();
+	local_bh_enable();
 }
 
 static inline struct cryptd_queue *cryptd_get_queue(struct crypto_tfm *tfm)
@@ -209,9 +212,7 @@ static void cryptd_blkcipher_crypt(struct ablkcipher_request *req,
 	req->base.complete = rctx->complete;
 
 out:
-	local_bh_disable();
 	rctx->complete(&req->base, err);
-	local_bh_enable();
 }
 
 static void cryptd_blkcipher_encrypt(struct crypto_async_request *req, int err)
@@ -446,9 +447,7 @@ static void cryptd_hash_init(struct crypto_async_request *req_async, int err)
 	req->base.complete = rctx->complete;
 
 out:
-	local_bh_disable();
 	rctx->complete(&req->base, err);
-	local_bh_enable();
 }
 
 static int cryptd_hash_init_enqueue(struct ahash_request *req)
@@ -471,9 +470,7 @@ static void cryptd_hash_update(struct crypto_async_request *req_async, int err)
 	req->base.complete = rctx->complete;
 
 out:
-	local_bh_disable();
 	rctx->complete(&req->base, err);
-	local_bh_enable();
 }
 
 static int cryptd_hash_update_enqueue(struct ahash_request *req)
@@ -494,9 +491,7 @@ static void cryptd_hash_final(struct crypto_async_request *req_async, int err)
 	req->base.complete = rctx->complete;
 
 out:
-	local_bh_disable();
 	rctx->complete(&req->base, err);
-	local_bh_enable();
 }
 
 static int cryptd_hash_final_enqueue(struct ahash_request *req)
@@ -517,9 +512,7 @@ static void cryptd_hash_finup(struct crypto_async_request *req_async, int err)
 	req->base.complete = rctx->complete;
 
 out:
-	local_bh_disable();
 	rctx->complete(&req->base, err);
-	local_bh_enable();
 }
 
 static int cryptd_hash_finup_enqueue(struct ahash_request *req)
@@ -546,9 +539,7 @@ static void cryptd_hash_digest(struct crypto_async_request *req_async, int err)
 	req->base.complete = rctx->complete;
 
 out:
-	local_bh_disable();
 	rctx->complete(&req->base, err);
-	local_bh_enable();
 }
 
 static int cryptd_hash_digest_enqueue(struct ahash_request *req)
@@ -641,9 +632,7 @@ static void cryptd_aead_crypt(struct aead_request *req,
 	err = crypt( req );
 	req->base.complete = rctx->complete;
 out:
-	local_bh_disable();
 	rctx->complete(&req->base, err);
-	local_bh_enable();
 }
 
 static void cryptd_aead_encrypt(struct crypto_async_request *areq, int err)
@@ -895,6 +884,90 @@ void cryptd_free_ahash(struct cryptd_ahash *tfm)
 }
 EXPORT_SYMBOL_GPL(cryptd_free_ahash);
 
+void cryptd_flush_queue(struct crypto_tfm *tfm, cryptd_type_t type)
+{
+	struct crypto_instance *inst = crypto_tfm_alg_instance(tfm);
+	struct cryptd_instance_ctx *ictx = crypto_instance_ctx(inst);
+	struct cryptd_queue *cryptd_queue = ictx->queue;
+	struct cryptd_cpu_queue *cpu_queue;
+	struct crypto_queue *queue;
+	struct crypto_async_request *req, *tmp, *backlog;
+	crypto_completion_t complete;
+	int cpu;
+	unsigned int len;
+
+	switch (type) {
+	case CRYPTD_BLKCIPHER_ENCRYPT:
+		complete = cryptd_blkcipher_encrypt;
+		break;
+	case CRYPTD_BLKCIPHER_DECRYPT:
+		complete = cryptd_blkcipher_decrypt;
+		break;
+	case CRYPTD_HASH_INIT:
+		complete = cryptd_hash_init;
+		break;
+	case CRYPTD_HASH_UPDATE:
+		complete = cryptd_hash_update;
+		break;
+	case CRYPTD_HASH_FINAL:
+		complete = cryptd_hash_final;
+		break;
+	case CRYPTD_HASH_FINUP:
+		complete = cryptd_hash_finup;
+		break;
+	case CRYPTD_HASH_DIGEST:
+		complete = cryptd_hash_digest;
+		break;
+	case CRYPTD_AEAD_ENCRYPT:
+		complete = cryptd_aead_encrypt;
+		break;
+	case CRYPTD_AEAD_DECRYPT:
+		complete = cryptd_aead_decrypt;
+		break;
+	default:
+		complete = NULL;
+	}
+
+	if (complete == NULL)
+		return;
+
+	local_bh_disable();
+	cpu = get_cpu();
+	cpu_queue = this_cpu_ptr(cryptd_queue->cpu_queue);
+	queue = &cpu_queue->queue;
+	len = queue->qlen;
+
+	if (!len)
+		goto out;
+
+	list_for_each_entry_safe(req, tmp, &queue->list, list) {
+		if (req->complete == complete) {
+			queue->qlen--;
+
+			if (queue->backlog != &queue->list) {
+				backlog = container_of(queue->backlog,
+					struct crypto_async_request, list);
+				queue->backlog = queue->backlog->next;
+			} else
+				backlog = NULL;
+
+			list_del(&req->list);
+
+			if (backlog)
+				backlog->complete(backlog, -EINPROGRESS);
+			req->complete(req, 0);
+		}
+
+		if (--len == 0)
+			goto out;
+	}
+
+out:
+	put_cpu();
+	local_bh_enable();
+}
+EXPORT_SYMBOL_GPL(cryptd_flush_queue);
+
 struct cryptd_aead *cryptd_alloc_aead(const char *alg_name,
 						  u32 type, u32 mask)
 {
diff --git a/include/crypto/cryptd.h b/include/crypto/cryptd.h
index ba98918..a63a296 100644
--- a/include/crypto/cryptd.h
+++ b/include/crypto/cryptd.h
@@ -16,6 +16,18 @@
 #include <linux/kernel.h>
 #include <crypto/hash.h>
 
+typedef enum {
+	CRYPTD_BLKCIPHER_ENCRYPT,
+	CRYPTD_BLKCIPHER_DECRYPT,
+	CRYPTD_HASH_INIT,
+	CRYPTD_HASH_UPDATE,
+	CRYPTD_HASH_FINAL,
+	CRYPTD_HASH_FINUP,
+	CRYPTD_HASH_DIGEST,
+	CRYPTD_AEAD_ENCRYPT,
+	CRYPTD_AEAD_DECRYPT,
+} cryptd_type_t;
+
 struct cryptd_ablkcipher {
 	struct crypto_ablkcipher base;
 };
@@ -48,6 +60,7 @@ struct cryptd_ahash *cryptd_alloc_ahash(const char *alg_name,
 struct crypto_shash *cryptd_ahash_child(struct cryptd_ahash *tfm);
 struct shash_desc *cryptd_shash_desc(struct ahash_request *req);
 void cryptd_free_ahash(struct cryptd_ahash *tfm);
+void cryptd_flush_queue(struct crypto_tfm *tfm, cryptd_type_t type);
 
 struct cryptd_aead {
 	struct crypto_aead base;
-- 
1.8.4.1

^ permalink raw reply related

* Re: [PATCH net-next 2/2] r8152: adjust rtl_start_rx
From: David Miller @ 2014-11-12  5:43 UTC (permalink / raw)
  To: hayeswang; +Cc: netdev, nic_swsd, linux-kernel, linux-usb
In-Reply-To: <0835B3720019904CB8F7AA43166CEEB2ECE442@RTITMBSV03.realtek.com.tw>

From: Hayes Wang <hayeswang@realtek.com>
Date: Wed, 12 Nov 2014 05:23:03 +0000

> David Miller [mailto:davem@davemloft.net] 
>> Sent: Wednesday, November 12, 2014 1:13 PM
> [...]
>> I really want to know why you are spending so much effort on this.
>> 
>> Is there a real situation that happened very often, which you
>> diagnosed in detail, and therefore you want to address?
> 
> No. I just consider the possible situation and want to
> make the driver better. If you think this is unnecessary,
> I would remove it.

What do other USB network drivers do in similar situations?

^ 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