Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next 2/2] net: 3com: fix warning for incorrect type in argument
From: David Miller @ 2014-01-14  7:31 UTC (permalink / raw)
  To: dingtianhong; +Cc: netdev, joe, julia.lawall
In-Reply-To: <52D0FF89.2020206@huawei.com>

From: Ding Tianhong <dingtianhong@huawei.com>
Date: Sat, 11 Jan 2014 16:23:37 +0800

> The commit c466a9b2b329f7d9982c14eedc83a923d3bc711c
> (net: 3com: slight optimization of addr compare)
> cause a warning: "passing argument 1 of 'ether_addr_equal'
> from incompatible pointer type", so fix it.
> 
> I think julia will convert ether_addr_equal to ether_addr_equal_64bits later.
> 
> Cc: "David S. Miller" <davem@davemloft.net>
> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next 1/2] net: qlcnic: fix warning for incorrect type in argument
From: David Miller @ 2014-01-14  7:31 UTC (permalink / raw)
  To: dingtianhong; +Cc: himanshu.madhani, rajesh.borundia, netdev, joe, julia.lawall
In-Reply-To: <52D0FF87.4090806@huawei.com>

From: Ding Tianhong <dingtianhong@huawei.com>
Date: Sat, 11 Jan 2014 16:23:35 +0800

> The commit 6878f79a8b71e8c7b0587a1185584f54fd31f185
> (net: qlcnic: slight optimization of addr compare)
> cause a warning "sparse: incorrect type in argument 2
> (different type sizes)", so fix it.
> 
> I think julia will convert ether_addr_equal to ether_addr_equal_64bits later.
> 
> Cc: Himanshu Madhani <himanshu.madhani@qlogic.com>
> Cc: Rajesh Borundia <rajesh.borundia@qlogic.com>
> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>

Applied.

^ permalink raw reply

* Re: [PATCH 5/5] net: mvneta: replace Tx timer with a real interrupt
From: Willy Tarreau @ 2014-01-14  7:30 UTC (permalink / raw)
  To: Arnaud Ebalard
  Cc: davem, netdev, Thomas Petazzoni, Gregory CLEMENT, Eric Dumazet
In-Reply-To: <87y52jeack.fsf@natisbad.org>

On Tue, Jan 14, 2014 at 12:22:03AM +0100, Arnaud Ebalard wrote:
> Hi Willy,
> 
> Willy Tarreau <w@1wt.eu> writes:
> 
> > @@ -1935,14 +1907,22 @@ static int mvneta_poll(struct napi_struct *napi, int budget)
> >  
> >  	/* Read cause register */
> >  	cause_rx_tx = mvreg_read(pp, MVNETA_INTR_NEW_CAUSE) &
> > -		MVNETA_RX_INTR_MASK(rxq_number);
> > +		(MVNETA_RX_INTR_MASK(rxq_number) | MVNETA_TX_INTR_MASK(txq_number));
> > +
> > +	/* Release Tx descriptors */
> > +	if (cause_rx_tx & MVNETA_TX_INTR_MASK_ALL) {
> > +		int tx_todo = 0;
> > +
> > +		mvneta_tx_done_gbe(pp, (cause_rx_tx & MVNETA_TX_INTR_MASK_ALL), &tx_todo);
> > +		cause_rx_tx &= ~MVNETA_TX_INTR_MASK_ALL;
> > +	}
> 
> Unless I missed something, tx_todo above is just here to make the
> compiler happy w/ current prototype of mvneta_tx_done_gbe() but is
> otherwise unused: you could simply remove the third parameter of the
> function (it is only used here) and remove tx_todo.

A number of such changes could be done but should be merged separately,
along with the cleanup and improvement series.

> Additionally, as you do not use the return value of the function, you
> could probably make it void and spare some additional cycles by removing
> the computation of the return value. While at it, mvneta_txq_done()
> could also be made void.
> 
> The patch below gives the idea, it's compile-tested only and applies on
> your whole set (fixes + perf).

You should propose your patches for net-next on top of my series, really,
it's not too late.

Please see my comments below.

> Index: linux/drivers/net/ethernet/marvell/mvneta.c
> ===================================================================
> --- linux.orig/drivers/net/ethernet/marvell/mvneta.c	2014-01-14 00:07:18.728729578 +0100
> +++ linux/drivers/net/ethernet/marvell/mvneta.c	2014-01-14 00:11:57.740949448 +0100
> @@ -1314,25 +1314,23 @@
>  }
>  
>  /* Handle end of transmission */
> -static int mvneta_txq_done(struct mvneta_port *pp,
> +static void mvneta_txq_done(struct mvneta_port *pp,
>  			   struct mvneta_tx_queue *txq)
>  {
>  	struct netdev_queue *nq = netdev_get_tx_queue(pp->dev, txq->id);
>  	int tx_done;
>  
>  	tx_done = mvneta_txq_sent_desc_proc(pp, txq);
> -	if (tx_done == 0)
> -		return tx_done;
> -	mvneta_txq_bufs_free(pp, txq, tx_done);
> +	if (tx_done) {
> +		mvneta_txq_bufs_free(pp, txq, tx_done);

Better just use "if (tx_done == 0) return" above and avoid adding an
extra indent level by inverting the if, that makes the code more readable.

> -	txq->count -= tx_done;
> +		txq->count -= tx_done;
>  
> -	if (netif_tx_queue_stopped(nq)) {
> -		if (txq->size - txq->count >= MAX_SKB_FRAGS + 1)
> -			netif_tx_wake_queue(nq);
> +		if (netif_tx_queue_stopped(nq)) {
> +			if (txq->size - txq->count >= MAX_SKB_FRAGS + 1)
> +				netif_tx_wake_queue(nq);
> +		}
>  	}
> -
> -	return tx_done;
>  }
>  
>  static void *mvneta_frag_alloc(const struct mvneta_port *pp)
> @@ -1704,30 +1702,23 @@
>  /* Handle tx done - called in softirq context. The <cause_tx_done> argument
>   * must be a valid cause according to MVNETA_TXQ_INTR_MASK_ALL.
>   */
> -static u32 mvneta_tx_done_gbe(struct mvneta_port *pp, u32 cause_tx_done,
> -			      int *tx_todo)
> +static void mvneta_tx_done_gbe(struct mvneta_port *pp, u32 cause_tx_done)
>  {
>  	struct mvneta_tx_queue *txq;
> -	u32 tx_done = 0;
>  	struct netdev_queue *nq;
>  
> -	*tx_todo = 0;
>  	while (cause_tx_done) {
>  		txq = mvneta_tx_done_policy(pp, cause_tx_done);
>  
>  		nq = netdev_get_tx_queue(pp->dev, txq->id);
>  		__netif_tx_lock(nq, smp_processor_id());
>  
> -		if (txq->count) {
> -			tx_done += mvneta_txq_done(pp, txq);
> -			*tx_todo += txq->count;
> -		}
> +		if (txq->count)
> +			mvneta_txq_done(pp, txq);
>  
>  		__netif_tx_unlock(nq);
>  		cause_tx_done &= ~((1 << txq->id));
>  	}
> -
> -	return tx_done;
>  }

Seems fine.

>  /* Compute crc8 of the specified address, using a unique algorithm ,
> @@ -1961,9 +1952,7 @@
>  
>  	/* Release Tx descriptors */
>  	if (cause_rx_tx & MVNETA_TX_INTR_MASK_ALL) {
> -		int tx_todo = 0;
> -
> -		mvneta_tx_done_gbe(pp, (cause_rx_tx & MVNETA_TX_INTR_MASK_ALL), &tx_todo);
> +		mvneta_tx_done_gbe(pp, (cause_rx_tx & MVNETA_TX_INTR_MASK_ALL));
>  		cause_rx_tx &= ~MVNETA_TX_INTR_MASK_ALL;
>  	}

Seems fine as well.

Thanks!
Willy

^ permalink raw reply

* Re: [PATCH] sh_eth: fix garbled TX error message
From: David Miller @ 2014-01-14  7:29 UTC (permalink / raw)
  To: sergei.shtylyov; +Cc: netdev, linux-sh
In-Reply-To: <201401110241.49471.sergei.shtylyov@cogentembedded.com>

From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Sat, 11 Jan 2014 02:41:49 +0300

> sh_eth_error() in case of a TX error tries to print a message using 2 dev_err()
> calls with the first string not finished by '\n', so that the resulting message
> would inevitably come out garbled, with something like "3net eth0: " inserted
> in the middle.  Avoid that by merging 2 calls into one.
> 
> While at it, insert an empty line after the nearby declaration.
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

Applied, thanks.

I don't think this is really -stable material, sorry.

^ permalink raw reply

* Re: [PATCH 0/5] Assorted mvneta fixes
From: Willy Tarreau @ 2014-01-14  7:24 UTC (permalink / raw)
  To: Arnaud Ebalard
  Cc: davem, netdev, Thomas Petazzoni, Gregory CLEMENT, Eric Dumazet
In-Reply-To: <87d2jvfr1m.fsf@natisbad.org>

Hi Arnaud,

On Mon, Jan 13, 2014 at 11:36:05PM +0100, Arnaud Ebalard wrote:
> Hi,
> 
> Willy Tarreau <w@1wt.eu> writes:
> 
> >> Funny enough, I spent some time this week-end trying to find the root
> >> cause of some kernel freezes and panics appearing randomly after some GB
> >> read on a ReadyNAS 102 configured as a NFS server. 
> >> 
> >> I tested your fixes and performance series together on top of current
> >> 3.13.0-rc7 and I am now unable to reproduce the freeze and panics after
> >> having read more than the 300GB of traffic from the NAS: following
> >> bandwith with a bwm-ng shows the rate is also far more stable than w/
> >> previous driver logic (55MB/sec). So, FWIW:
> >> 
> >> Tested-by: Arnaud Ebalard <arno@natisbad.org>
> >
> > Thanks for this.
> >
> > BTW, the "performance" series is not supposed to fix anything, 
> 
> I was lazy and wanted to give the whole set a try in a single pass.
> 
> 
> > and still it seems difficult to me to find what patch might have fixed
> > your problem. Maybe the timer used in place of an IRQ has an even
> > worse effect than what we could imagine ?
> 
> I guess so.
> 
> 
> >> Willy, I can extend the test to RN2120 if you think it is useful to also
> >> do additional tests on a dual-core armada XP.
> >
> > It's up to you. These patches have run extensively on my Mirabox (Armada370),
> > OpenBlocks AX3 (ArmadaXP dual core) and the XP-GP board (ArmadaXP quad core),
> > and fixed the stability issues and performance issues I was facing there. But
> > you may be interested in testing them with your workloads (none of my boxes
> > is used as an NFS server, NAS or whatever, they mainly see HTTP and very small
> > packets used in stress tests).
> 
> Well, I spent the evening on my RN104 (Aramda370 w/ 2 GbE ifaces) and my
> RN2120 (Dual core ArmadaXP w/ 2GbE ifaces) using one as a router and
> serving NFS traffic from the other (and then changing roles). I passed
> hundreds of GB of TCP/NFS traffic and did not see any issue.
> 
> Additionally, FWIW, testing both using netperf show they easily support
> routing traffic w/ line rate perf.
> 
> Regarding the patches, the problem they solve impacts all Armada boards
> (370 and XP) which are used for network tasks. I think it would be nice
> to have those backported to stable. I can commit to do the tests of the
> backports both on XP and 370 hardware down to 3.12 or 3.11 kernel if it
> can help. 

I think so. I've been successfully using them from 3.10 and upwards.

Cheers,
Willy

^ permalink raw reply

* Re: pull request (net-next): ipsec-next 2014-01-14
From: David Miller @ 2014-01-14  7:14 UTC (permalink / raw)
  To: steffen.klassert; +Cc: herbert, netdev
In-Reply-To: <1389682159-3260-1-git-send-email-steffen.klassert@secunet.com>

From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Tue, 14 Jan 2014 07:49:04 +0100

> This pull request has a merge conflict between commits be7928d20bab
> ("net: xfrm: xfrm_policy: fix inline not at beginning of declaration") and
> da7c224b1baa ("net: xfrm: xfrm_policy: silence compiler warning") from
> the net-next tree and commit 2f3ea9a95c58 ("xfrm: checkpatch erros with
> inline keyword position") from the ipsec-next tree.
> 
> The version from net-next can be used, like it is done in linux-next.
> 
> 1) Checkpatch cleanups, from Weilong Chen.
> 
> 2) Fix lockdep complaints when pktgen is used with IPsec,
>    from Fan Du.
> 
> 3) Update pktgen to allow any combination of IPsec transport/tunnel mode
>    and AH/ESP/IPcomp type, from Fan Du.
> 
> 4) Make pktgen_dst_metrics static, Fengguang Wu.
> 
> 5) Compile fix for pktgen when CONFIG_XFRM is not set,
>    from Fan Du.
> 
> Please pull or let me know if there are problems.

Pulled, thanks for the heads up about the merge conflicts.

^ permalink raw reply

* Re: [PATCH V2 0/4] misc: xgene: Add support for APM X-Gene SoC Queue Manager/Traffic Manager
From: Arnd Bergmann @ 2014-01-14  6:58 UTC (permalink / raw)
  To: Ravi Patel
  Cc: Greg KH, Loc Ho, davem, netdev, linux-kernel,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	Jon Masters, patches@apm.com, Keyur Chudgar
In-Reply-To: <CAN1v_Pt9rOGn8zmf+u7MR6-GJPDPswBbuhXu6i=MJVa2mwqNtw@mail.gmail.com>

On Monday 13 January 2014, Ravi Patel wrote:
> > For inbound messages, the QMTM serves a similar purpose as an MSI
> > controller, ensuring that inbound DMA data has arrived in RAM
> > before an interrupt is delivered to the CPU and thereby avoiding
> > the need for an expensive MMIO read to serialize the DMA.
> 
> For inbound messages, slave device generates message on a completion
> of a inbound DMA operation or any relevant operation targeted to the
> CPU. The QMTM's role is to just trigger an interrupt to CPU when there
> is a new message arrived from a slave device. QMTM doesn't know what
> the message was for. It is upto the upper layer drivers to decide how
> to process this message.

That doesn't seem to contradict what I wrote above. The DMA ordering
would be an implicit side-effect of the message generated by the
slave device if the QMTM is on the same bus as the external memory
controller and the message has the "strict ordering" bit set on the
bus transaction.

	Arnd

^ permalink raw reply

* Re: [PATCH net-next 1/3] bonding: update the primary slave when slave's name changed
From: Ding Tianhong @ 2014-01-14  6:51 UTC (permalink / raw)
  To: Veaceslav Falico; +Cc: Jay Vosburgh, David S. Miller, Netdev
In-Reply-To: <20140114063847.GB7798@redhat.com>

On 2014/1/14 14:38, Veaceslav Falico wrote:
> On Tue, Jan 14, 2014 at 10:36:56AM +0800, Ding Tianhong wrote:
>> If the slave's name changed, and the bond params primary is exist,
>> the bond should deal with the situation in two ways:
>>
>> 1) If the slave is the primary slave yet, clean the primary slave
>>   and reselect active slave.
>> 2) If the slave's new name is as same as bond primary, set the slave
>>   as primary slave and reselect active slave.
>>
>> Thanks for Veaceslav's suggestion.
>>
>> Suggested-by: Veaceslav Falico <vfalico@redhat.com>
> 
> As in my previous email - please, don't use my name until I say so.
> 
> I'll add my signed-off-by to any patch that I've worked enough on.
> 
>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>> ---
>> drivers/net/bonding/bond_main.c | 30 ++++++++++++++++++++++++++++--
>> 1 file changed, 28 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>> index e06c445..63d6533 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -2860,9 +2860,35 @@ static int bond_slave_netdev_event(unsigned long event,
>>          */
>>         break;
>>     case NETDEV_CHANGENAME:
>> -        /*
>> -         * TODO: handle changing the primary's name
>> +        /* Handle changing the slave's name:
>> +         * 1) If the slave is primary save yet,
>> +         * clean the primary slave and reselect
>> +         * active slave.
> 
> I usually don't mind bad english (as I myself am speaking quite horrible
> one), but I can't really understand what you've meant here. And given that
> it's a comment in code - please, proof-read it first.
> 
>> +         * 2) If the slave's new name is bond
>> +         * primary, set the slave as primary
>> +         * slave and reselect active slave.
>>          */
>> +        if (USES_PRIMARY(bond->params.mode) &&
>> +            bond->params.primary[0]) {
> 
> Too many indentions. Verify if we're not using primary or primary string
> name is null and break, otherwise go further.
> 
>> +            if (bond->primary_slave &&
>> +                slave == bond->primary_slave) {
> 
> Useless verification, slave can't be NULL.
> 
>> +                pr_info("%s: Setting primary slave to None.\n",
>> +                    bond->dev->name);
>> +                bond->primary_slave = NULL;
>> +                write_lock_bh(&bond->curr_slave_lock);
>> +                bond_select_active_slave(bond);
> 
> Get bond_select_active_slave() out of if()s, you use it twice here.
> 
>> +                write_unlock_bh(&bond->curr_slave_lock);
>> +            } else if (!bond->primary_slave &&
> 
> Useless verification, if the name of a slave changed to our params.primary
> - then it means that bond->primary_slave was NULL, as it can only be
> not-null when we have a matching interface, and that would mean that we
> have two interfaces with the same name.
> 
>> +                   !strcmp(bond->params.primary,
>> +                      slave_dev->name)) {
>> +                pr_info("%s: Setting %s as primary slave.\n",
>> +                    bond->dev->name, slave_dev->name);
>> +                bond->primary_slave = slave;
>> +                write_lock_bh(&bond->curr_slave_lock);
>> +                bond_select_active_slave(bond);
>> +                write_unlock_bh(&bond->curr_slave_lock);
>> +            }
>> +        }
>>         break;
>>     case NETDEV_FEAT_CHANGE:
>>         bond_compute_features(bond);
>> -- 
>> 1.8.0
>>
>>
> 

fix in v2, thanks.

Regards
Ding 

> .
> 

^ permalink raw reply

* Re: [PATCH net-next] bonding: don't permit slaves to change their mtu
From: Ding Tianhong @ 2014-01-14  6:51 UTC (permalink / raw)
  To: Veaceslav Falico; +Cc: Jay Vosburgh, David S. Miller, Netdev
In-Reply-To: <20140114061556.GA7798@redhat.com>

On 2014/1/14 14:15, Veaceslav Falico wrote:
> On Tue, Jan 14, 2014 at 11:01:59AM +0800, Ding Tianhong wrote:
>> The commit 2315dc91a5059d7da9a8b9b9daf78d695c11383e
>> (net: make dev_set_mtu() honor notification return code)
>> will deal with the return value for NETDEV_CHANGEMTU notification,
>> and the slaves should not change their mtu, so add return value
>> to prevent doing it.
> 
> In another email you said you've tested the mtu changes and some of the
> bonds have packet loss when mtu is changed, and some of them don't.
> 
> Maybe it'd be good to understand which modes can tolerate the mtu change
> (if it can be tolerated at all/if it should really matter) and allow it for
> specific bond modes only/for any bond modes?
> 
Ok, need more analysis.


>>
>> Suggested-by: Veaceslav Falico <vfalico@redhat.com>
> 
> Don't add my name unless I specifically ask you to, please.
> 
> Thank you.
> 

Ok

>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>> ---
>> drivers/net/bonding/bond_main.c | 16 ++++------------
>> 1 file changed, 4 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>> index e06c445..af4e678 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -2846,19 +2846,11 @@ static int bond_slave_netdev_event(unsigned long event,
>>          */
>>         break;
>>     case NETDEV_CHANGEMTU:
>> -        /*
>> -         * TODO: Should slaves be allowed to
>> -         * independently alter their MTU?  For
>> -         * an active-backup bond, slaves need
>> -         * not be the same type of device, so
>> -         * MTUs may vary.  For other modes,
>> -         * slaves arguably should have the
>> -         * same MTUs. To do this, we'd need to
>> -         * take over the slave's change_mtu
>> -         * function for the duration of their
>> -         * servitude.
>> +        /* The master and slaves should have the
>> +         * the same mtu, so do't permit slaves
>> +         * to change their mtu independently.
>>          */
>> -        break;
>> +        return NOTIFY_BAD;
>>     case NETDEV_CHANGENAME:
>>         /*
>>          * TODO: handle changing the primary's name
>> -- 
>> 1.8.0
>>
>>
> -- 
> 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

* [PATCH net-next] tun/macvtap: limit the packets queued through rcvbuf
From: Jason Wang @ 2014-01-14  6:53 UTC (permalink / raw)
  To: davem, netdev, linux-kernel
  Cc: Jason Wang, Vlad Yasevich, Michael S. Tsirkin, John Fastabend,
	Stephen Hemminger, Herbert Xu

We used to limit the number of packets queued through tx_queue_length. This
has several issues:

- tx_queue_length is the control of qdisc queue length, simply reusing it
  to control the packets queued by device may cause confusion.
- After commit 6acf54f1cf0a6747bac9fea26f34cfc5a9029523 ("macvtap: Add
  support of packet capture on macvtap device."), an unexpected qdisc
  caused by non-zero tx_queue_length will lead qdisc lock contention for
  multiqueue deivce.
- What we really want is to limit the total amount of memory occupied not
  the number of packets.

So this patch tries to solve the above issues by using socket rcvbuf to
limit the packets could be queued for tun/macvtap. This was done by using
sock_queue_rcv_skb() instead of a direct call to skb_queue_tail(). Also two
new ioctl() were introduced for userspace to change the rcvbuf like what we
have done for sndbuf.

With this fix, we can safely change the tx_queue_len of macvtap to
zero. This will make multiqueue works without extra lock contention.

Cc: Vlad Yasevich <vyasevic@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: John Fastabend <john.r.fastabend@intel.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/net/macvtap.c       | 31 ++++++++++++++++++++---------
 drivers/net/tun.c           | 48 +++++++++++++++++++++++++++++++++------------
 include/uapi/linux/if_tun.h |  3 +++
 3 files changed, 60 insertions(+), 22 deletions(-)

diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index a2c3a89..c429c56 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -292,9 +292,6 @@ static rx_handler_result_t macvtap_handle_frame(struct sk_buff **pskb)
 	if (!q)
 		return RX_HANDLER_PASS;
 
-	if (skb_queue_len(&q->sk.sk_receive_queue) >= dev->tx_queue_len)
-		goto drop;
-
 	skb_push(skb, ETH_HLEN);
 
 	/* Apply the forward feature mask so that we perform segmentation
@@ -310,8 +307,10 @@ static rx_handler_result_t macvtap_handle_frame(struct sk_buff **pskb)
 			goto drop;
 
 		if (!segs) {
-			skb_queue_tail(&q->sk.sk_receive_queue, skb);
-			goto wake_up;
+			if (sock_queue_rcv_skb(&q->sk, skb))
+				goto drop;
+			else
+				goto wake_up;
 		}
 
 		kfree_skb(skb);
@@ -319,11 +318,17 @@ static rx_handler_result_t macvtap_handle_frame(struct sk_buff **pskb)
 			struct sk_buff *nskb = segs->next;
 
 			segs->next = NULL;
-			skb_queue_tail(&q->sk.sk_receive_queue, segs);
+			if (sock_queue_rcv_skb(&q->sk, segs)) {
+				skb = segs;
+				skb->next = nskb;
+				goto drop;
+			}
+
 			segs = nskb;
 		}
 	} else {
-		skb_queue_tail(&q->sk.sk_receive_queue, skb);
+		if (sock_queue_rcv_skb(&q->sk, skb))
+			goto drop;
 	}
 
 wake_up:
@@ -333,7 +338,7 @@ wake_up:
 drop:
 	/* Count errors/drops only here, thus don't care about args. */
 	macvlan_count_rx(vlan, 0, 0, 0);
-	kfree_skb(skb);
+	kfree_skb_list(skb);
 	return RX_HANDLER_CONSUMED;
 }
 
@@ -414,7 +419,7 @@ static void macvtap_dellink(struct net_device *dev,
 static void macvtap_setup(struct net_device *dev)
 {
 	macvlan_common_setup(dev);
-	dev->tx_queue_len = TUN_READQ_SIZE;
+	dev->tx_queue_len = 0;
 }
 
 static struct rtnl_link_ops macvtap_link_ops __read_mostly = {
@@ -469,6 +474,7 @@ static int macvtap_open(struct inode *inode, struct file *file)
 	sock_init_data(&q->sock, &q->sk);
 	q->sk.sk_write_space = macvtap_sock_write_space;
 	q->sk.sk_destruct = macvtap_sock_destruct;
+	q->sk.sk_rcvbuf = TUN_RCVBUF;
 	q->flags = IFF_VNET_HDR | IFF_NO_PI | IFF_TAP;
 	q->vnet_hdr_sz = sizeof(struct virtio_net_hdr);
 
@@ -1040,6 +1046,13 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
 		q->sk.sk_sndbuf = u;
 		return 0;
 
+	case TUNSETRCVBUF:
+		if (get_user(u, up))
+			return -EFAULT;
+
+		q->sk.sk_rcvbuf = u;
+		return 0;
+
 	case TUNGETVNETHDRSZ:
 		s = q->vnet_hdr_sz;
 		if (put_user(s, sp))
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 09f6662..7a08fa3 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -177,6 +177,7 @@ struct tun_struct {
 
 	int			vnet_hdr_sz;
 	int			sndbuf;
+	int			rcvbuf;
 	struct tap_filter	txflt;
 	struct sock_fprog	fprog;
 	/* protected by rtnl lock */
@@ -771,17 +772,6 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
 	if (!check_filter(&tun->txflt, skb))
 		goto drop;
 
-	if (tfile->socket.sk->sk_filter &&
-	    sk_filter(tfile->socket.sk, skb))
-		goto drop;
-
-	/* Limit the number of packets queued by dividing txq length with the
-	 * number of queues.
-	 */
-	if (skb_queue_len(&tfile->socket.sk->sk_receive_queue)
-			  >= dev->tx_queue_len / tun->numqueues)
-		goto drop;
-
 	if (unlikely(skb_orphan_frags(skb, GFP_ATOMIC)))
 		goto drop;
 
@@ -798,7 +788,8 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
 	nf_reset(skb);
 
 	/* Enqueue packet */
-	skb_queue_tail(&tfile->socket.sk->sk_receive_queue, skb);
+	if (sock_queue_rcv_skb(tfile->socket.sk, skb))
+		goto drop;
 
 	/* Notify and wake up reader process */
 	if (tfile->flags & TUN_FASYNC)
@@ -1668,6 +1659,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
 
 		tun->filter_attached = false;
 		tun->sndbuf = tfile->socket.sk->sk_sndbuf;
+		tun->rcvbuf = tfile->socket.sk->sk_rcvbuf;
 
 		spin_lock_init(&tun->lock);
 
@@ -1837,6 +1829,17 @@ static void tun_set_sndbuf(struct tun_struct *tun)
 	}
 }
 
+static void tun_set_rcvbuf(struct tun_struct *tun)
+{
+	struct tun_file *tfile;
+	int i;
+
+	for (i = 0; i < tun->numqueues; i++) {
+		tfile = rtnl_dereference(tun->tfiles[i]);
+		tfile->socket.sk->sk_sndbuf = tun->sndbuf;
+	}
+}
+
 static int tun_set_queue(struct file *file, struct ifreq *ifr)
 {
 	struct tun_file *tfile = file->private_data;
@@ -1878,7 +1881,7 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
 	struct ifreq ifr;
 	kuid_t owner;
 	kgid_t group;
-	int sndbuf;
+	int sndbuf, rcvbuf;
 	int vnet_hdr_sz;
 	unsigned int ifindex;
 	int ret;
@@ -2061,6 +2064,22 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
 		tun_set_sndbuf(tun);
 		break;
 
+	case TUNGETRCVBUF:
+		rcvbuf = tfile->socket.sk->sk_rcvbuf;
+		if (copy_to_user(argp, &rcvbuf, sizeof(rcvbuf)))
+			ret = -EFAULT;
+		break;
+
+	case TUNSETRCVBUF:
+		if (copy_from_user(&rcvbuf, argp, sizeof(rcvbuf))) {
+			ret = -EFAULT;
+			break;
+		}
+
+		tun->rcvbuf = rcvbuf;
+		tun_set_rcvbuf(tun);
+		break;
+
 	case TUNGETVNETHDRSZ:
 		vnet_hdr_sz = tun->vnet_hdr_sz;
 		if (copy_to_user(argp, &vnet_hdr_sz, sizeof(vnet_hdr_sz)))
@@ -2139,6 +2158,8 @@ static long tun_chr_compat_ioctl(struct file *file,
 	case TUNSETTXFILTER:
 	case TUNGETSNDBUF:
 	case TUNSETSNDBUF:
+	case TUNGETRCVBUF:
+	case TUNSETRCVBUF:
 	case SIOCGIFHWADDR:
 	case SIOCSIFHWADDR:
 		arg = (unsigned long)compat_ptr(arg);
@@ -2204,6 +2225,7 @@ static int tun_chr_open(struct inode *inode, struct file * file)
 
 	tfile->sk.sk_write_space = tun_sock_write_space;
 	tfile->sk.sk_sndbuf = INT_MAX;
+	tfile->sk.sk_rcvbuf = TUN_RCVBUF;
 
 	file->private_data = tfile;
 	set_bit(SOCK_EXTERNALLY_ALLOCATED, &tfile->socket.flags);
diff --git a/include/uapi/linux/if_tun.h b/include/uapi/linux/if_tun.h
index e9502dd..8e04657 100644
--- a/include/uapi/linux/if_tun.h
+++ b/include/uapi/linux/if_tun.h
@@ -22,6 +22,7 @@
 
 /* Read queue size */
 #define TUN_READQ_SIZE	500
+#define TUN_RCVBUF	(512 * PAGE_SIZE)
 
 /* TUN device flags */
 #define TUN_TUN_DEV 	0x0001	
@@ -58,6 +59,8 @@
 #define TUNSETQUEUE  _IOW('T', 217, int)
 #define TUNSETIFINDEX	_IOW('T', 218, unsigned int)
 #define TUNGETFILTER _IOR('T', 219, struct sock_fprog)
+#define TUNGETRCVBUF   _IOR('T', 220, int)
+#define TUNSETRCVBUF   _IOW('T', 221, int)
 
 /* TUNSETIFF ifr flags */
 #define IFF_TUN		0x0001
-- 
1.8.3.2

^ permalink raw reply related

* [PATCH 14/15] pktgen_dst_metrics[] can be static
From: Steffen Klassert @ 2014-01-14  6:49 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <1389682159-3260-1-git-send-email-steffen.klassert@secunet.com>

From: Fengguang Wu <fengguang.wu@intel.com>

CC: Fan Du <fan.du@windriver.com>
CC: Steffen Klassert <steffen.klassert@secunet.com>
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/core/pktgen.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 45ba476..a37ec53 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -2500,7 +2500,7 @@ static void mod_cur_headers(struct pktgen_dev *pkt_dev)
 
 
 #ifdef CONFIG_XFRM
-u32 pktgen_dst_metrics[RTAX_MAX + 1] = {
+static u32 pktgen_dst_metrics[RTAX_MAX + 1] = {
 
 	[RTAX_HOPLIMIT] = 0x5, /* Set a static hoplimit */
 };
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 15/15] {xfrm,pktgen} Fix compiling error when CONFIG_XFRM is not set
From: Steffen Klassert @ 2014-01-14  6:49 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <1389682159-3260-1-git-send-email-steffen.klassert@secunet.com>

From: Fan Du <fan.du@windriver.com>

0-DAY kernel build testing backend reported below error:
All error/warnings:

   net/core/pktgen.c: In function 'pktgen_if_write':
>> >> net/core/pktgen.c:1487:10: error: 'struct pktgen_dev' has no member named 'spi'
>> >> net/core/pktgen.c:1488:43: error: 'struct pktgen_dev' has no member named 'spi'

Fix this by encapuslating the code with CONFIG_XFRM.

Cc: Fengguang Wu <fengguang.wu@intel.com>
Signed-off-by: Fan Du <fan.du@windriver.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/core/pktgen.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index a37ec53..fa3e128 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -1482,7 +1482,7 @@ static ssize_t pktgen_if_write(struct file *file,
 		sprintf(pg_result, "OK: flows=%u", pkt_dev->cflows);
 		return count;
 	}
-
+#ifdef CONFIG_XFRM
 	if (!strcmp(name, "spi")) {
 		len = num_arg(&user_buffer[i], 10, &value);
 		if (len < 0)
@@ -1493,7 +1493,7 @@ static ssize_t pktgen_if_write(struct file *file,
 		sprintf(pg_result, "OK: spi=%u", pkt_dev->spi);
 		return count;
 	}
-
+#endif
 	if (!strcmp(name, "flowlen")) {
 		len = num_arg(&user_buffer[i], 10, &value);
 		if (len < 0)
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 13/15] {pktgen, xfrm} Document IPsec usage in pktgen.txt
From: Steffen Klassert @ 2014-01-14  6:49 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <1389682159-3260-1-git-send-email-steffen.klassert@secunet.com>

From: Fan Du <fan.du@windriver.com>

Update pktgen.txt for reference when using IPsec.

Signed-off-by: Fan Du <fan.du@windriver.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 Documentation/networking/pktgen.txt |   15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/Documentation/networking/pktgen.txt b/Documentation/networking/pktgen.txt
index 75e4fd7..5a61a240 100644
--- a/Documentation/networking/pktgen.txt
+++ b/Documentation/networking/pktgen.txt
@@ -108,7 +108,9 @@ Examples:
                               MPLS_RND, VID_RND, SVID_RND
                               QUEUE_MAP_RND # queue map random
                               QUEUE_MAP_CPU # queue map mirrors smp_processor_id()
+                              IPSEC # Make IPsec encapsulation for packet
 
+ pgset spi SPI_VALUE     Set specific SA used to transform packet.
 
  pgset "udp_src_min 9"   set UDP source port min, If < udp_src_max, then
                          cycle through the port range.
@@ -177,6 +179,18 @@ Note when adding devices to a specific CPU there good idea to also assign
 /proc/irq/XX/smp_affinity so the TX-interrupts gets bound to the same CPU.
 as this reduces cache bouncing when freeing skb's.
 
+Enable IPsec
+============
+Default IPsec transformation with ESP encapsulation plus Transport mode
+could be enabled by simply setting:
+
+pgset "flag IPSEC"
+pgset "flows 1"
+
+To avoid breaking existing testbed scripts for using AH type and tunnel mode,
+user could use "pgset spi SPI_VALUE" to specify which formal of transformation
+to employ.
+
 
 Current commands and configuration options
 ==========================================
@@ -225,6 +239,7 @@ flag
   UDPDST_RND
   MACSRC_RND
   MACDST_RND
+  IPSEC
 
 dst_min
 dst_max
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 10/15] {pktgen, xfrm} Construct skb dst for tunnel mode transformation
From: Steffen Klassert @ 2014-01-14  6:49 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <1389682159-3260-1-git-send-email-steffen.klassert@secunet.com>

From: Fan Du <fan.du@windriver.com>

IPsec tunnel mode encapuslation needs to set outter ip header
with right protocol/ttl/id value with regard to skb->dst->child.

Looking up a rt in a standard way is absolutely wrong for every
packet transmission. In a simple way, construct a dst by setting
neccessary information to make tunnel mode encapuslation working.

Signed-off-by: Fan Du <fan.du@windriver.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/core/pktgen.c |   28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 8bc4ddd..628f7c5 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -390,6 +390,8 @@ struct pktgen_dev {
 	__u8	ipsmode;		/* IPSEC mode (config) */
 	__u8	ipsproto;		/* IPSEC type (config) */
 	__u32	spi;
+	struct dst_entry dst;
+	struct dst_ops dstops;
 #endif
 	char result[512];
 };
@@ -2487,6 +2489,11 @@ static void mod_cur_headers(struct pktgen_dev *pkt_dev)
 
 
 #ifdef CONFIG_XFRM
+u32 pktgen_dst_metrics[RTAX_MAX + 1] = {
+
+	[RTAX_HOPLIMIT] = 0x5, /* Set a static hoplimit */
+};
+
 static int pktgen_output_ipsec(struct sk_buff *skb, struct pktgen_dev *pkt_dev)
 {
 	struct xfrm_state *x = pkt_dev->flows[pkt_dev->curfl].x;
@@ -2497,10 +2504,18 @@ static int pktgen_output_ipsec(struct sk_buff *skb, struct pktgen_dev *pkt_dev)
 		return 0;
 	/* XXX: we dont support tunnel mode for now until
 	 * we resolve the dst issue */
-	if (x->props.mode != XFRM_MODE_TRANSPORT)
+	if ((x->props.mode != XFRM_MODE_TRANSPORT) && (pkt_dev->spi == 0))
 		return 0;
 
+	/* But when user specify an valid SPI, transformation
+	 * supports both transport/tunnel mode + ESP/AH type.
+	 */
+	if ((x->props.mode == XFRM_MODE_TUNNEL) && (pkt_dev->spi != 0))
+		skb->_skb_refdst = (unsigned long)&pkt_dev->dst | SKB_DST_NOREF;
+
+	rcu_read_lock_bh();
 	err = x->outer_mode->output(x, skb);
+	rcu_read_unlock_bh();
 	if (err) {
 		XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTSTATEMODEERROR);
 		goto error;
@@ -3557,6 +3572,17 @@ static int pktgen_add_device(struct pktgen_thread *t, const char *ifname)
 #ifdef CONFIG_XFRM
 	pkt_dev->ipsmode = XFRM_MODE_TRANSPORT;
 	pkt_dev->ipsproto = IPPROTO_ESP;
+
+	/* xfrm tunnel mode needs additional dst to extract outter
+	 * ip header protocol/ttl/id field, here creat a phony one.
+	 * instead of looking for a valid rt, which definitely hurting
+	 * performance under such circumstance.
+	 */
+	pkt_dev->dstops.family = AF_INET;
+	pkt_dev->dst.dev = pkt_dev->odev;
+	dst_init_metrics(&pkt_dev->dst, pktgen_dst_metrics, false);
+	pkt_dev->dst.child = &pkt_dev->dst;
+	pkt_dev->dst.ops = &pkt_dev->dstops;
 #endif
 
 	return add_dev_to_thread(t, pkt_dev);
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 11/15] {pktgen, xfrm} Introduce xfrm_state_lookup_byspi for pktgen
From: Steffen Klassert @ 2014-01-14  6:49 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <1389682159-3260-1-git-send-email-steffen.klassert@secunet.com>

From: Fan Du <fan.du@windriver.com>

Introduce xfrm_state_lookup_byspi to find user specified by custom
from "pgset spi xxx". Using this scheme, any flow regardless its
saddr/daddr could be transform by SA specified with configurable
spi.

Signed-off-by: Fan Du <fan.du@windriver.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 include/net/xfrm.h    |    2 ++
 net/core/pktgen.c     |   22 +++++++++++++++-------
 net/xfrm/xfrm_state.c |   22 ++++++++++++++++++++++
 3 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index b7635ef..cd7c46f 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -1421,6 +1421,8 @@ struct xfrm_state *xfrm_stateonly_find(struct net *net, u32 mark,
 				       xfrm_address_t *saddr,
 				       unsigned short family,
 				       u8 mode, u8 proto, u32 reqid);
+struct xfrm_state *xfrm_state_lookup_byspi(struct net *net, __be32 spi,
+					      unsigned short family);
 int xfrm_state_check_expire(struct xfrm_state *x);
 void xfrm_state_insert(struct xfrm_state *x);
 int xfrm_state_add(struct xfrm_state *x);
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 628f7c5..b553c36 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -2247,13 +2247,21 @@ static void get_ipsec_sa(struct pktgen_dev *pkt_dev, int flow)
 	struct xfrm_state *x = pkt_dev->flows[flow].x;
 	struct pktgen_net *pn = net_generic(dev_net(pkt_dev->odev), pg_net_id);
 	if (!x) {
-		/*slow path: we dont already have xfrm_state*/
-		x = xfrm_stateonly_find(pn->net, DUMMY_MARK,
-					(xfrm_address_t *)&pkt_dev->cur_daddr,
-					(xfrm_address_t *)&pkt_dev->cur_saddr,
-					AF_INET,
-					pkt_dev->ipsmode,
-					pkt_dev->ipsproto, 0);
+
+		if (pkt_dev->spi) {
+			/* We need as quick as possible to find the right SA
+			 * Searching with minimum criteria to archieve this.
+			 */
+			x = xfrm_state_lookup_byspi(pn->net, htonl(pkt_dev->spi), AF_INET);
+		} else {
+			/* slow path: we dont already have xfrm_state */
+			x = xfrm_stateonly_find(pn->net, DUMMY_MARK,
+						(xfrm_address_t *)&pkt_dev->cur_daddr,
+						(xfrm_address_t *)&pkt_dev->cur_saddr,
+						AF_INET,
+						pkt_dev->ipsmode,
+						pkt_dev->ipsproto, 0);
+		}
 		if (x) {
 			pkt_dev->flows[flow].x = x;
 			set_pkt_overhead(pkt_dev);
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 3007440..6218148 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -915,6 +915,28 @@ xfrm_stateonly_find(struct net *net, u32 mark,
 }
 EXPORT_SYMBOL(xfrm_stateonly_find);
 
+struct xfrm_state *xfrm_state_lookup_byspi(struct net *net, __be32 spi,
+					      unsigned short family)
+{
+	struct xfrm_state *x;
+	struct xfrm_state_walk *w;
+
+	spin_lock_bh(&net->xfrm.xfrm_state_lock);
+	list_for_each_entry(w, &net->xfrm.state_all, all) {
+		x = container_of(w, struct xfrm_state, km);
+		if (x->props.family != family ||
+			x->id.spi != spi)
+			continue;
+
+		spin_unlock_bh(&net->xfrm.xfrm_state_lock);
+		xfrm_state_hold(x);
+		return x;
+	}
+	spin_unlock_bh(&net->xfrm.xfrm_state_lock);
+	return NULL;
+}
+EXPORT_SYMBOL(xfrm_state_lookup_byspi);
+
 static void __xfrm_state_insert(struct xfrm_state *x)
 {
 	struct net *net = xs_net(x);
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 12/15] {pktgen, xfrm} Show spi value properly when ipsec turned on
From: Steffen Klassert @ 2014-01-14  6:49 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <1389682159-3260-1-git-send-email-steffen.klassert@secunet.com>

From: Fan Du <fan.du@windriver.com>

If user run pktgen plus ipsec by using spi, show spi value
properly when cat /proc/net/pktgen/ethX

Signed-off-by: Fan Du <fan.du@windriver.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/core/pktgen.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index b553c36..45ba476 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -657,8 +657,11 @@ static int pktgen_if_show(struct seq_file *seq, void *v)
 	}
 
 #ifdef CONFIG_XFRM
-	if (pkt_dev->flags & F_IPSEC_ON)
+	if (pkt_dev->flags & F_IPSEC_ON) {
 		seq_printf(seq,  "IPSEC  ");
+		if (pkt_dev->spi)
+			seq_printf(seq, "spi:%u", pkt_dev->spi);
+	}
 #endif
 
 	if (pkt_dev->flags & F_MACSRC_RND)
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 08/15] {pktgen, xfrm} Correct xfrm_state_lock usage in xfrm_stateonly_find
From: Steffen Klassert @ 2014-01-14  6:49 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <1389682159-3260-1-git-send-email-steffen.klassert@secunet.com>

From: Fan Du <fan.du@windriver.com>

Acquiring xfrm_state_lock in process context is expected to turn BH off,
as this lock is also used in BH context, namely xfrm state timer handler.
Otherwise it surprises LOCKDEP with below messages.

[   81.422781] pktgen: Packet Generator for packet performance testing. Version: 2.74
[   81.725194]
[   81.725211] =========================================================
[   81.725212] [ INFO: possible irq lock inversion dependency detected ]
[   81.725215] 3.13.0-rc2+ #92 Not tainted
[   81.725216] ---------------------------------------------------------
[   81.725218] kpktgend_0/2780 just changed the state of lock:
[   81.725220]  (xfrm_state_lock){+.+...}, at: [<ffffffff816dd751>] xfrm_stateonly_find+0x41/0x1f0
[   81.725231] but this lock was taken by another, SOFTIRQ-safe lock in the past:
[   81.725232]  (&(&x->lock)->rlock){+.-...}
[   81.725232]
[   81.725232] and interrupts could create inverse lock ordering between them.
[   81.725232]
[   81.725235]
[   81.725235] other info that might help us debug this:
[   81.725237]  Possible interrupt unsafe locking scenario:
[   81.725237]
[   81.725238]        CPU0                    CPU1
[   81.725240]        ----                    ----
[   81.725241]   lock(xfrm_state_lock);
[   81.725243]                                local_irq_disable();
[   81.725244]                                lock(&(&x->lock)->rlock);
[   81.725246]                                lock(xfrm_state_lock);
[   81.725248]   <Interrupt>
[   81.725249]     lock(&(&x->lock)->rlock);
[   81.725251]
[   81.725251]  *** DEADLOCK ***
[   81.725251]
[   81.725254] no locks held by kpktgend_0/2780.
[   81.725255]
[   81.725255] the shortest dependencies between 2nd lock and 1st lock:
[   81.725269]  -> (&(&x->lock)->rlock){+.-...} ops: 8 {
[   81.725274]     HARDIRQ-ON-W at:
[   81.725276]                       [<ffffffff8109a64b>] __lock_acquire+0x65b/0x1d70
[   81.725282]                       [<ffffffff8109c3c7>] lock_acquire+0x97/0x130
[   81.725284]                       [<ffffffff81774af6>] _raw_spin_lock+0x36/0x70
[   81.725289]                       [<ffffffff816dc3a3>] xfrm_timer_handler+0x43/0x290
[   81.725292]                       [<ffffffff81059437>] __tasklet_hrtimer_trampoline+0x17/0x40
[   81.725300]                       [<ffffffff8105a1b7>] tasklet_hi_action+0xd7/0xf0
[   81.725303]                       [<ffffffff81059ac6>] __do_softirq+0xe6/0x2d0
[   81.725305]                       [<ffffffff8105a026>] irq_exit+0x96/0xc0
[   81.725308]                       [<ffffffff8177fd0a>] smp_apic_timer_interrupt+0x4a/0x60
[   81.725313]                       [<ffffffff8177e96f>] apic_timer_interrupt+0x6f/0x80
[   81.725316]                       [<ffffffff8100b7c6>] arch_cpu_idle+0x26/0x30
[   81.725329]                       [<ffffffff810ace28>] cpu_startup_entry+0x88/0x2b0
[   81.725333]                       [<ffffffff8102e5b0>] start_secondary+0x190/0x1f0
[   81.725338]     IN-SOFTIRQ-W at:
[   81.725340]                       [<ffffffff8109a61d>] __lock_acquire+0x62d/0x1d70
[   81.725342]                       [<ffffffff8109c3c7>] lock_acquire+0x97/0x130
[   81.725344]                       [<ffffffff81774af6>] _raw_spin_lock+0x36/0x70
[   81.725347]                       [<ffffffff816dc3a3>] xfrm_timer_handler+0x43/0x290
[   81.725349]                       [<ffffffff81059437>] __tasklet_hrtimer_trampoline+0x17/0x40
[   81.725352]                       [<ffffffff8105a1b7>] tasklet_hi_action+0xd7/0xf0
[   81.725355]                       [<ffffffff81059ac6>] __do_softirq+0xe6/0x2d0
[   81.725358]                       [<ffffffff8105a026>] irq_exit+0x96/0xc0
[   81.725360]                       [<ffffffff8177fd0a>] smp_apic_timer_interrupt+0x4a/0x60
[   81.725363]                       [<ffffffff8177e96f>] apic_timer_interrupt+0x6f/0x80
[   81.725365]                       [<ffffffff8100b7c6>] arch_cpu_idle+0x26/0x30
[   81.725368]                       [<ffffffff810ace28>] cpu_startup_entry+0x88/0x2b0
[   81.725370]                       [<ffffffff8102e5b0>] start_secondary+0x190/0x1f0
[   81.725373]     INITIAL USE at:
[   81.725375]                      [<ffffffff8109a31a>] __lock_acquire+0x32a/0x1d70
[   81.725385]                      [<ffffffff8109c3c7>] lock_acquire+0x97/0x130
[   81.725388]                      [<ffffffff81774af6>] _raw_spin_lock+0x36/0x70
[   81.725390]                      [<ffffffff816dc3a3>] xfrm_timer_handler+0x43/0x290
[   81.725394]                      [<ffffffff81059437>] __tasklet_hrtimer_trampoline+0x17/0x40
[   81.725398]                      [<ffffffff8105a1b7>] tasklet_hi_action+0xd7/0xf0
[   81.725401]                      [<ffffffff81059ac6>] __do_softirq+0xe6/0x2d0
[   81.725404]                      [<ffffffff8105a026>] irq_exit+0x96/0xc0
[   81.725407]                      [<ffffffff8177fd0a>] smp_apic_timer_interrupt+0x4a/0x60
[   81.725409]                      [<ffffffff8177e96f>] apic_timer_interrupt+0x6f/0x80
[   81.725412]                      [<ffffffff8100b7c6>] arch_cpu_idle+0x26/0x30
[   81.725415]                      [<ffffffff810ace28>] cpu_startup_entry+0x88/0x2b0
[   81.725417]                      [<ffffffff8102e5b0>] start_secondary+0x190/0x1f0
[   81.725420]   }
[   81.725421]   ... key      at: [<ffffffff8295b9c8>] __key.46349+0x0/0x8
[   81.725445]   ... acquired at:
[   81.725446]    [<ffffffff8109c3c7>] lock_acquire+0x97/0x130
[   81.725449]    [<ffffffff81774af6>] _raw_spin_lock+0x36/0x70
[   81.725452]    [<ffffffff816dc057>] __xfrm_state_delete+0x37/0x140
[   81.725454]    [<ffffffff816dc18c>] xfrm_state_delete+0x2c/0x50
[   81.725456]    [<ffffffff816dc277>] xfrm_state_flush+0xc7/0x1b0
[   81.725458]    [<ffffffffa005f6cc>] pfkey_flush+0x7c/0x100 [af_key]
[   81.725465]    [<ffffffffa005efb7>] pfkey_process+0x1c7/0x1f0 [af_key]
[   81.725468]    [<ffffffffa005f139>] pfkey_sendmsg+0x159/0x260 [af_key]
[   81.725471]    [<ffffffff8162c16f>] sock_sendmsg+0xaf/0xc0
[   81.725476]    [<ffffffff8162c99c>] SYSC_sendto+0xfc/0x130
[   81.725479]    [<ffffffff8162cf3e>] SyS_sendto+0xe/0x10
[   81.725482]    [<ffffffff8177dd12>] system_call_fastpath+0x16/0x1b
[   81.725484]
[   81.725486] -> (xfrm_state_lock){+.+...} ops: 11 {
[   81.725490]    HARDIRQ-ON-W at:
[   81.725493]                     [<ffffffff8109a64b>] __lock_acquire+0x65b/0x1d70
[   81.725504]                     [<ffffffff8109c3c7>] lock_acquire+0x97/0x130
[   81.725507]                     [<ffffffff81774e4b>] _raw_spin_lock_bh+0x3b/0x70
[   81.725510]                     [<ffffffff816dc1df>] xfrm_state_flush+0x2f/0x1b0
[   81.725513]                     [<ffffffffa005f6cc>] pfkey_flush+0x7c/0x100 [af_key]
[   81.725516]                     [<ffffffffa005efb7>] pfkey_process+0x1c7/0x1f0 [af_key]
[   81.725519]                     [<ffffffffa005f139>] pfkey_sendmsg+0x159/0x260 [af_key]
[   81.725522]                     [<ffffffff8162c16f>] sock_sendmsg+0xaf/0xc0
[   81.725525]                     [<ffffffff8162c99c>] SYSC_sendto+0xfc/0x130
[   81.725527]                     [<ffffffff8162cf3e>] SyS_sendto+0xe/0x10
[   81.725530]                     [<ffffffff8177dd12>] system_call_fastpath+0x16/0x1b
[   81.725533]    SOFTIRQ-ON-W at:
[   81.725534]                     [<ffffffff8109a67a>] __lock_acquire+0x68a/0x1d70
[   81.725537]                     [<ffffffff8109c3c7>] lock_acquire+0x97/0x130
[   81.725539]                     [<ffffffff81774af6>] _raw_spin_lock+0x36/0x70
[   81.725541]                     [<ffffffff816dd751>] xfrm_stateonly_find+0x41/0x1f0
[   81.725544]                     [<ffffffffa008af03>] mod_cur_headers+0x793/0x7f0 [pktgen]
[   81.725547]                     [<ffffffffa008bca2>] pktgen_thread_worker+0xd42/0x1880 [pktgen]
[   81.725550]                     [<ffffffff81078f84>] kthread+0xe4/0x100
[   81.725555]                     [<ffffffff8177dc6c>] ret_from_fork+0x7c/0xb0
[   81.725565]    INITIAL USE at:
[   81.725567]                    [<ffffffff8109a31a>] __lock_acquire+0x32a/0x1d70
[   81.725569]                    [<ffffffff8109c3c7>] lock_acquire+0x97/0x130
[   81.725572]                    [<ffffffff81774e4b>] _raw_spin_lock_bh+0x3b/0x70
[   81.725574]                    [<ffffffff816dc1df>] xfrm_state_flush+0x2f/0x1b0
[   81.725576]                    [<ffffffffa005f6cc>] pfkey_flush+0x7c/0x100 [af_key]
[   81.725580]                    [<ffffffffa005efb7>] pfkey_process+0x1c7/0x1f0 [af_key]
[   81.725583]                    [<ffffffffa005f139>] pfkey_sendmsg+0x159/0x260 [af_key]
[   81.725586]                    [<ffffffff8162c16f>] sock_sendmsg+0xaf/0xc0
[   81.725589]                    [<ffffffff8162c99c>] SYSC_sendto+0xfc/0x130
[   81.725594]                    [<ffffffff8162cf3e>] SyS_sendto+0xe/0x10
[   81.725597]                    [<ffffffff8177dd12>] system_call_fastpath+0x16/0x1b
[   81.725599]  }
[   81.725600]  ... key      at: [<ffffffff81cadef8>] xfrm_state_lock+0x18/0x50
[   81.725606]  ... acquired at:
[   81.725607]    [<ffffffff810995c0>] check_usage_backwards+0x110/0x150
[   81.725609]    [<ffffffff81099e96>] mark_lock+0x196/0x2f0
[   81.725611]    [<ffffffff8109a67a>] __lock_acquire+0x68a/0x1d70
[   81.725614]    [<ffffffff8109c3c7>] lock_acquire+0x97/0x130
[   81.725616]    [<ffffffff81774af6>] _raw_spin_lock+0x36/0x70
[   81.725627]    [<ffffffff816dd751>] xfrm_stateonly_find+0x41/0x1f0
[   81.725629]    [<ffffffffa008af03>] mod_cur_headers+0x793/0x7f0 [pktgen]
[   81.725632]    [<ffffffffa008bca2>] pktgen_thread_worker+0xd42/0x1880 [pktgen]
[   81.725635]    [<ffffffff81078f84>] kthread+0xe4/0x100
[   81.725637]    [<ffffffff8177dc6c>] ret_from_fork+0x7c/0xb0
[   81.725640]
[   81.725641]
[   81.725641] stack backtrace:
[   81.725645] CPU: 0 PID: 2780 Comm: kpktgend_0 Not tainted 3.13.0-rc2+ #92
[   81.725647] Hardware name: innotek GmbH VirtualBox, BIOS VirtualBox 12/01/2006
[   81.725649]  ffffffff82537b80 ffff880018199988 ffffffff8176af37 0000000000000007
[   81.725652]  ffff8800181999f0 ffff8800181999d8 ffffffff81099358 ffffffff82537b80
[   81.725655]  ffffffff81a32def ffff8800181999f4 0000000000000000 ffff880002cbeaa8
[   81.725659] Call Trace:
[   81.725664]  [<ffffffff8176af37>] dump_stack+0x46/0x58
[   81.725667]  [<ffffffff81099358>] print_irq_inversion_bug.part.42+0x1e8/0x1f0
[   81.725670]  [<ffffffff810995c0>] check_usage_backwards+0x110/0x150
[   81.725672]  [<ffffffff81099e96>] mark_lock+0x196/0x2f0
[   81.725675]  [<ffffffff810994b0>] ? check_usage_forwards+0x150/0x150
[   81.725685]  [<ffffffff8109a67a>] __lock_acquire+0x68a/0x1d70
[   81.725691]  [<ffffffff810899a5>] ? sched_clock_local+0x25/0x90
[   81.725694]  [<ffffffff81089b38>] ? sched_clock_cpu+0xa8/0x120
[   81.725697]  [<ffffffff8109a31a>] ? __lock_acquire+0x32a/0x1d70
[   81.725699]  [<ffffffff816dd751>] ? xfrm_stateonly_find+0x41/0x1f0
[   81.725702]  [<ffffffff8109c3c7>] lock_acquire+0x97/0x130
[   81.725704]  [<ffffffff816dd751>] ? xfrm_stateonly_find+0x41/0x1f0
[   81.725707]  [<ffffffff810899a5>] ? sched_clock_local+0x25/0x90
[   81.725710]  [<ffffffff81774af6>] _raw_spin_lock+0x36/0x70
[   81.725712]  [<ffffffff816dd751>] ? xfrm_stateonly_find+0x41/0x1f0
[   81.725715]  [<ffffffff810971ec>] ? lock_release_holdtime.part.26+0x1c/0x1a0
[   81.725717]  [<ffffffff816dd751>] xfrm_stateonly_find+0x41/0x1f0
[   81.725721]  [<ffffffffa008af03>] mod_cur_headers+0x793/0x7f0 [pktgen]
[   81.725724]  [<ffffffffa008bca2>] pktgen_thread_worker+0xd42/0x1880 [pktgen]
[   81.725727]  [<ffffffffa008ba71>] ? pktgen_thread_worker+0xb11/0x1880 [pktgen]
[   81.725729]  [<ffffffff8109cf9d>] ? trace_hardirqs_on+0xd/0x10
[   81.725733]  [<ffffffff81775410>] ? _raw_spin_unlock_irq+0x30/0x40
[   81.725745]  [<ffffffff8151faa0>] ? e1000_clean+0x9d0/0x9d0
[   81.725751]  [<ffffffff81094310>] ? __init_waitqueue_head+0x60/0x60
[   81.725753]  [<ffffffff81094310>] ? __init_waitqueue_head+0x60/0x60
[   81.725757]  [<ffffffffa008af60>] ? mod_cur_headers+0x7f0/0x7f0 [pktgen]
[   81.725759]  [<ffffffff81078f84>] kthread+0xe4/0x100
[   81.725762]  [<ffffffff81078ea0>] ? flush_kthread_worker+0x170/0x170
[   81.725765]  [<ffffffff8177dc6c>] ret_from_fork+0x7c/0xb0
[   81.725768]  [<ffffffff81078ea0>] ? flush_kthread_worker+0x170/0x170

Signed-off-by: Fan Du <fan.du@windriver.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_state.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 9e6a4d6..3007440 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -890,7 +890,7 @@ xfrm_stateonly_find(struct net *net, u32 mark,
 	unsigned int h;
 	struct xfrm_state *rx = NULL, *x = NULL;
 
-	spin_lock(&net->xfrm.xfrm_state_lock);
+	spin_lock_bh(&net->xfrm.xfrm_state_lock);
 	h = xfrm_dst_hash(net, daddr, saddr, reqid, family);
 	hlist_for_each_entry(x, net->xfrm.state_bydst+h, bydst) {
 		if (x->props.family == family &&
@@ -908,7 +908,7 @@ xfrm_stateonly_find(struct net *net, u32 mark,
 
 	if (rx)
 		xfrm_state_hold(rx);
-	spin_unlock(&net->xfrm.xfrm_state_lock);
+	spin_unlock_bh(&net->xfrm.xfrm_state_lock);
 
 
 	return rx;
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 09/15] {pktgen, xfrm} Using "pgset spi xxx" to spedifiy SA for a given flow
From: Steffen Klassert @ 2014-01-14  6:49 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <1389682159-3260-1-git-send-email-steffen.klassert@secunet.com>

From: Fan Du <fan.du@windriver.com>

User could set specific SPI value to arm pktgen flow with IPsec
transformation, instead of looking up SA by sadr/daddr. The reaseon
to do so is because current state lookup scheme is both slow and, most
important of all, in fact pktgen doesn't need to match any SA state
addresses information, all it needs is the SA transfromation shell to
do the encapuslation.

And this option also provide user an alternative to using pktgen
test existing SA without creating new ones.

Signed-off-by: Fan Du <fan.du@windriver.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/core/pktgen.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 156d57b..8bc4ddd 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -389,6 +389,7 @@ struct pktgen_dev {
 #ifdef CONFIG_XFRM
 	__u8	ipsmode;		/* IPSEC mode (config) */
 	__u8	ipsproto;		/* IPSEC type (config) */
+	__u32	spi;
 #endif
 	char result[512];
 };
@@ -1477,6 +1478,17 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 
+	if (!strcmp(name, "spi")) {
+		len = num_arg(&user_buffer[i], 10, &value);
+		if (len < 0)
+			return len;
+
+		i += len;
+		pkt_dev->spi = value;
+		sprintf(pg_result, "OK: spi=%u", pkt_dev->spi);
+		return count;
+	}
+
 	if (!strcmp(name, "flowlen")) {
 		len = num_arg(&user_buffer[i], 10, &value);
 		if (len < 0)
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 07/15] {pktgen, xfrm} Add statistics counting when transforming
From: Steffen Klassert @ 2014-01-14  6:49 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <1389682159-3260-1-git-send-email-steffen.klassert@secunet.com>

From: Fan Du <fan.du@windriver.com>

so /proc/net/xfrm_stat could give user clue about what's
wrong in this process.

Signed-off-by: Fan Du <fan.du@windriver.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/core/pktgen.c |   10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index b007586..156d57b 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -2479,6 +2479,7 @@ static int pktgen_output_ipsec(struct sk_buff *skb, struct pktgen_dev *pkt_dev)
 {
 	struct xfrm_state *x = pkt_dev->flows[pkt_dev->curfl].x;
 	int err = 0;
+	struct net *net = dev_net(pkt_dev->odev);
 
 	if (!x)
 		return 0;
@@ -2488,12 +2489,15 @@ static int pktgen_output_ipsec(struct sk_buff *skb, struct pktgen_dev *pkt_dev)
 		return 0;
 
 	err = x->outer_mode->output(x, skb);
-	if (err)
+	if (err) {
+		XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTSTATEMODEERROR);
 		goto error;
+	}
 	err = x->type->output(x, skb);
-	if (err)
+	if (err) {
+		XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTSTATEPROTOERROR);
 		goto error;
-
+	}
 	spin_lock_bh(&x->lock);
 	x->curlft.bytes += skb->len;
 	x->curlft.packets++;
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 05/15] xfrm: checkpatch erros with inline keyword position
From: Steffen Klassert @ 2014-01-14  6:49 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <1389682159-3260-1-git-send-email-steffen.klassert@secunet.com>

From: Weilong Chen <chenweilong@huawei.com>

Signed-off-by: Weilong Chen <chenweilong@huawei.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_policy.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index b5c315e..fe8942b 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -1641,7 +1641,7 @@ free_dst:
 	goto out;
 }
 
-static int inline
+static inline int
 xfrm_dst_alloc_copy(void **target, const void *src, int size)
 {
 	if (!*target) {
@@ -1653,7 +1653,7 @@ xfrm_dst_alloc_copy(void **target, const void *src, int size)
 	return 0;
 }
 
-static int inline
+static inline int
 xfrm_dst_update_parent(struct dst_entry *dst, const struct xfrm_selector *sel)
 {
 #ifdef CONFIG_XFRM_SUB_POLICY
@@ -1665,7 +1665,7 @@ xfrm_dst_update_parent(struct dst_entry *dst, const struct xfrm_selector *sel)
 #endif
 }
 
-static int inline
+static inline int
 xfrm_dst_update_origin(struct dst_entry *dst, const struct flowi *fl)
 {
 #ifdef CONFIG_XFRM_SUB_POLICY
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 03/15] xfrm: checkpatch erros with space prohibited
From: Steffen Klassert @ 2014-01-14  6:49 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <1389682159-3260-1-git-send-email-steffen.klassert@secunet.com>

From: Weilong Chen <chenweilong@huawei.com>

Fix checkpatch error "space prohibited xxx".

Signed-off-by: Weilong Chen <chenweilong@huawei.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_policy.c |    6 +++---
 net/xfrm/xfrm_user.c   |    2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index dc8bd1b..fbc72b4 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -1704,7 +1704,7 @@ static int xfrm_expand_policies(const struct flowi *fl, u16 family,
 				xfrm_pols_put(pols, *num_pols);
 				return PTR_ERR(pols[1]);
 			}
-			(*num_pols) ++;
+			(*num_pols)++;
 			(*num_xfrms) += pols[1]->xfrm_nr;
 		}
 	}
@@ -2378,7 +2378,7 @@ int __xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb,
 	pol->curlft.use_time = get_seconds();
 
 	pols[0] = pol;
-	npols ++;
+	npols++;
 #ifdef CONFIG_XFRM_SUB_POLICY
 	if (pols[0]->type != XFRM_POLICY_TYPE_MAIN) {
 		pols[1] = xfrm_policy_lookup_bytype(net, XFRM_POLICY_TYPE_MAIN,
@@ -2390,7 +2390,7 @@ int __xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb,
 				return 0;
 			}
 			pols[1]->curlft.use_time = get_seconds();
-			npols ++;
+			npols++;
 		}
 	}
 #endif
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index e50be42..3348566 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1824,7 +1824,7 @@ static int xfrm_new_ae(struct sk_buff *skb, struct nlmsghdr *nlh,
 	struct net *net = sock_net(skb->sk);
 	struct xfrm_state *x;
 	struct km_event c;
-	int err = - EINVAL;
+	int err = -EINVAL;
 	u32 mark = 0;
 	struct xfrm_mark m;
 	struct xfrm_aevent_id *p = nlmsg_data(nlh);
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 06/15] {pktgen, xfrm} Correct xfrm state lock usage when transforming
From: Steffen Klassert @ 2014-01-14  6:49 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <1389682159-3260-1-git-send-email-steffen.klassert@secunet.com>

From: Fan Du <fan.du@windriver.com>

xfrm_state lock protects its state, i.e., VALID/DEAD and statistics,
not the transforming procedure, as both mode/type output functions
are reentrant.

Another issue is state lock can be used in BH context when state timer
alarmed, after transformation in pktgen, update state statistics acquiring
state lock should disabled BH context for a moment. Otherwise LOCKDEP
critisize this:

[   62.354339] pktgen: Packet Generator for packet performance testing. Version: 2.74
[   62.655444]
[   62.655448] =================================
[   62.655451] [ INFO: inconsistent lock state ]
[   62.655455] 3.13.0-rc2+ #70 Not tainted
[   62.655457] ---------------------------------
[   62.655459] inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
[   62.655463] kpktgend_0/2764 [HC0[0]:SC0[0]:HE1:SE1] takes:
[   62.655466]  (&(&x->lock)->rlock){+.?...}, at: [<ffffffffa00886f6>] pktgen_thread_worker+0x1796/0x1860 [pktgen]
[   62.655479] {IN-SOFTIRQ-W} state was registered at:
[   62.655484]   [<ffffffff8109a61d>] __lock_acquire+0x62d/0x1d70
[   62.655492]   [<ffffffff8109c3c7>] lock_acquire+0x97/0x130
[   62.655498]   [<ffffffff81774af6>] _raw_spin_lock+0x36/0x70
[   62.655505]   [<ffffffff816dc3a3>] xfrm_timer_handler+0x43/0x290
[   62.655511]   [<ffffffff81059437>] __tasklet_hrtimer_trampoline+0x17/0x40
[   62.655519]   [<ffffffff8105a1b7>] tasklet_hi_action+0xd7/0xf0
[   62.655523]   [<ffffffff81059ac6>] __do_softirq+0xe6/0x2d0
[   62.655526]   [<ffffffff8105a026>] irq_exit+0x96/0xc0
[   62.655530]   [<ffffffff8177fd0a>] smp_apic_timer_interrupt+0x4a/0x60
[   62.655537]   [<ffffffff8177e96f>] apic_timer_interrupt+0x6f/0x80
[   62.655541]   [<ffffffff8100b7c6>] arch_cpu_idle+0x26/0x30
[   62.655547]   [<ffffffff810ace28>] cpu_startup_entry+0x88/0x2b0
[   62.655552]   [<ffffffff81761c3c>] rest_init+0xbc/0xd0
[   62.655557]   [<ffffffff81ea5e5e>] start_kernel+0x3c4/0x3d1
[   62.655583]   [<ffffffff81ea55a8>] x86_64_start_reservations+0x2a/0x2c
[   62.655588]   [<ffffffff81ea569f>] x86_64_start_kernel+0xf5/0xfc
[   62.655592] irq event stamp: 77
[   62.655594] hardirqs last  enabled at (77): [<ffffffff810ab7f2>] vprintk_emit+0x1b2/0x520
[   62.655597] hardirqs last disabled at (76): [<ffffffff810ab684>] vprintk_emit+0x44/0x520
[   62.655601] softirqs last  enabled at (22): [<ffffffff81059b57>] __do_softirq+0x177/0x2d0
[   62.655605] softirqs last disabled at (15): [<ffffffff8105a026>] irq_exit+0x96/0xc0
[   62.655609]
[   62.655609] other info that might help us debug this:
[   62.655613]  Possible unsafe locking scenario:
[   62.655613]
[   62.655616]        CPU0
[   62.655617]        ----
[   62.655618]   lock(&(&x->lock)->rlock);
[   62.655622]   <Interrupt>
[   62.655623]     lock(&(&x->lock)->rlock);
[   62.655626]
[   62.655626]  *** DEADLOCK ***
[   62.655626]
[   62.655629] no locks held by kpktgend_0/2764.
[   62.655631]
[   62.655631] stack backtrace:
[   62.655636] CPU: 0 PID: 2764 Comm: kpktgend_0 Not tainted 3.13.0-rc2+ #70
[   62.655638] Hardware name: innotek GmbH VirtualBox, BIOS VirtualBox 12/01/2006
[   62.655642]  ffffffff8216b7b0 ffff88001be43ab8 ffffffff8176af37 0000000000000007
[   62.655652]  ffff88001c8d4fc0 ffff88001be43b18 ffffffff81766d78 0000000000000000
[   62.655663]  ffff880000000001 ffff880000000001 ffffffff8101025f ffff88001be43b18
[   62.655671] Call Trace:
[   62.655680]  [<ffffffff8176af37>] dump_stack+0x46/0x58
[   62.655685]  [<ffffffff81766d78>] print_usage_bug+0x1f1/0x202
[   62.655691]  [<ffffffff8101025f>] ? save_stack_trace+0x2f/0x50
[   62.655696]  [<ffffffff81099f8c>] mark_lock+0x28c/0x2f0
[   62.655700]  [<ffffffff810994b0>] ? check_usage_forwards+0x150/0x150
[   62.655704]  [<ffffffff8109a67a>] __lock_acquire+0x68a/0x1d70
[   62.655712]  [<ffffffff81115b09>] ? irq_work_queue+0x69/0xb0
[   62.655717]  [<ffffffff810ab7f2>] ? vprintk_emit+0x1b2/0x520
[   62.655722]  [<ffffffff8109cec5>] ? trace_hardirqs_on_caller+0x105/0x1d0
[   62.655730]  [<ffffffffa00886f6>] ? pktgen_thread_worker+0x1796/0x1860 [pktgen]
[   62.655734]  [<ffffffff8109c3c7>] lock_acquire+0x97/0x130
[   62.655741]  [<ffffffffa00886f6>] ? pktgen_thread_worker+0x1796/0x1860 [pktgen]
[   62.655745]  [<ffffffff81774af6>] _raw_spin_lock+0x36/0x70
[   62.655752]  [<ffffffffa00886f6>] ? pktgen_thread_worker+0x1796/0x1860 [pktgen]
[   62.655758]  [<ffffffffa00886f6>] pktgen_thread_worker+0x1796/0x1860 [pktgen]
[   62.655766]  [<ffffffffa0087a79>] ? pktgen_thread_worker+0xb19/0x1860 [pktgen]
[   62.655771]  [<ffffffff8109cf9d>] ? trace_hardirqs_on+0xd/0x10
[   62.655777]  [<ffffffff81775410>] ? _raw_spin_unlock_irq+0x30/0x40
[   62.655785]  [<ffffffff8151faa0>] ? e1000_clean+0x9d0/0x9d0
[   62.655791]  [<ffffffff81094310>] ? __init_waitqueue_head+0x60/0x60
[   62.655795]  [<ffffffff81094310>] ? __init_waitqueue_head+0x60/0x60
[   62.655800]  [<ffffffffa0086f60>] ? mod_cur_headers+0x7f0/0x7f0 [pktgen]
[   62.655806]  [<ffffffff81078f84>] kthread+0xe4/0x100
[   62.655813]  [<ffffffff81078ea0>] ? flush_kthread_worker+0x170/0x170
[   62.655819]  [<ffffffff8177dc6c>] ret_from_fork+0x7c/0xb0
[   62.655824]  [<ffffffff81078ea0>] ? flush_kthread_worker+0x170/0x170

Signed-off-by: Fan Du <fan.du@windriver.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/core/pktgen.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index a797fff..b007586 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -2487,8 +2487,6 @@ static int pktgen_output_ipsec(struct sk_buff *skb, struct pktgen_dev *pkt_dev)
 	if (x->props.mode != XFRM_MODE_TRANSPORT)
 		return 0;
 
-	spin_lock(&x->lock);
-
 	err = x->outer_mode->output(x, skb);
 	if (err)
 		goto error;
@@ -2496,10 +2494,11 @@ static int pktgen_output_ipsec(struct sk_buff *skb, struct pktgen_dev *pkt_dev)
 	if (err)
 		goto error;
 
+	spin_lock_bh(&x->lock);
 	x->curlft.bytes += skb->len;
 	x->curlft.packets++;
+	spin_unlock_bh(&x->lock);
 error:
-	spin_unlock(&x->lock);
 	return err;
 }
 
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 01/15] xfrm: checkpatch errors with space
From: Steffen Klassert @ 2014-01-14  6:49 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <1389682159-3260-1-git-send-email-steffen.klassert@secunet.com>

From: Weilong Chen <chenweilong@huawei.com>

This patch cleanup some space errors.

Signed-off-by: Weilong Chen <chenweilong@huawei.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_policy.c |   10 +++++-----
 net/xfrm/xfrm_proc.c   |    2 +-
 net/xfrm/xfrm_state.c  |   10 +++++-----
 net/xfrm/xfrm_user.c   |    4 ++--
 4 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index a7487f3..8a2b9d8 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -1286,7 +1286,7 @@ xfrm_tmpl_resolve_one(struct xfrm_policy *policy, const struct flowi *fl,
 	xfrm_address_t *saddr = xfrm_flowi_saddr(fl, family);
 	xfrm_address_t tmp;
 
-	for (nx=0, i = 0; i < policy->xfrm_nr; i++) {
+	for (nx = 0, i = 0; i < policy->xfrm_nr; i++) {
 		struct xfrm_state *x;
 		xfrm_address_t *remote = daddr;
 		xfrm_address_t *local  = saddr;
@@ -1326,7 +1326,7 @@ xfrm_tmpl_resolve_one(struct xfrm_policy *policy, const struct flowi *fl,
 	return nx;
 
 fail:
-	for (nx--; nx>=0; nx--)
+	for (nx--; nx >= 0; nx--)
 		xfrm_state_put(xfrm[nx]);
 	return error;
 }
@@ -1363,7 +1363,7 @@ xfrm_tmpl_resolve(struct xfrm_policy **pols, int npols, const struct flowi *fl,
 	return cnx;
 
  fail:
-	for (cnx--; cnx>=0; cnx--)
+	for (cnx--; cnx >= 0; cnx--)
 		xfrm_state_put(tpp[cnx]);
 	return error;
 
@@ -2332,7 +2332,7 @@ int __xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb,
 	if (skb->sp) {
 		int i;
 
-		for (i=skb->sp->len-1; i>=0; i--) {
+		for (i = skb->sp->len-1; i >= 0; i--) {
 			struct xfrm_state *x = skb->sp->xvec[i];
 			if (!xfrm_selector_match(&x->sel, &fl, family)) {
 				XFRM_INC_STATS(net, LINUX_MIB_XFRMINSTATEMISMATCH);
@@ -2987,7 +2987,7 @@ static void xfrm_audit_common_policyinfo(struct xfrm_policy *xp,
 		audit_log_format(audit_buf, " sec_alg=%u sec_doi=%u sec_obj=%s",
 				 ctx->ctx_alg, ctx->ctx_doi, ctx->ctx_str);
 
-	switch(sel->family) {
+	switch (sel->family) {
 	case AF_INET:
 		audit_log_format(audit_buf, " src=%pI4", &sel->saddr.a4);
 		if (sel->prefixlen_s != 32)
diff --git a/net/xfrm/xfrm_proc.c b/net/xfrm/xfrm_proc.c
index 80cd1e5..fc5abd0 100644
--- a/net/xfrm/xfrm_proc.c
+++ b/net/xfrm/xfrm_proc.c
@@ -52,7 +52,7 @@ static int xfrm_statistics_seq_show(struct seq_file *seq, void *v)
 {
 	struct net *net = seq->private;
 	int i;
-	for (i=0; xfrm_mib_list[i].name; i++)
+	for (i = 0; xfrm_mib_list[i].name; i++)
 		seq_printf(seq, "%-24s\t%lu\n", xfrm_mib_list[i].name,
 			   snmp_fold_field((void __percpu **)
 					   net->mib.xfrm_statistics,
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index a62c25e..5fe79b4 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -448,7 +448,7 @@ static enum hrtimer_restart xfrm_timer_handler(struct hrtimer * me)
 	if (warn)
 		km_state_expired(x, 0, 0);
 resched:
-	if (next != LONG_MAX){
+	if (next != LONG_MAX) {
 		tasklet_hrtimer_start(&x->mtimer, ktime_set(next, 0), HRTIMER_MODE_REL);
 	}
 
@@ -1348,7 +1348,7 @@ int xfrm_state_check_expire(struct xfrm_state *x)
 	if (x->curlft.bytes >= x->lft.hard_byte_limit ||
 	    x->curlft.packets >= x->lft.hard_packet_limit) {
 		x->km.state = XFRM_STATE_EXPIRED;
-		tasklet_hrtimer_start(&x->mtimer, ktime_set(0,0), HRTIMER_MODE_REL);
+		tasklet_hrtimer_start(&x->mtimer, ktime_set(0, 0), HRTIMER_MODE_REL);
 		return -EINVAL;
 	}
 
@@ -1542,7 +1542,7 @@ int xfrm_alloc_spi(struct xfrm_state *x, u32 low, u32 high)
 		x->id.spi = minspi;
 	} else {
 		u32 spi = 0;
-		for (h=0; h<high-low+1; h++) {
+		for (h = 0; h < high-low+1; h++) {
 			spi = low + net_random()%(high-low+1);
 			x0 = xfrm_state_lookup(net, mark, &x->id.daddr, htonl(spi), x->id.proto, x->props.family);
 			if (x0 == NULL) {
@@ -2079,7 +2079,7 @@ static void xfrm_audit_helper_sainfo(struct xfrm_state *x,
 		audit_log_format(audit_buf, " sec_alg=%u sec_doi=%u sec_obj=%s",
 				 ctx->ctx_alg, ctx->ctx_doi, ctx->ctx_str);
 
-	switch(x->props.family) {
+	switch (x->props.family) {
 	case AF_INET:
 		audit_log_format(audit_buf, " src=%pI4 dst=%pI4",
 				 &x->props.saddr.a4, &x->id.daddr.a4);
@@ -2109,7 +2109,7 @@ static void xfrm_audit_helper_pktinfo(struct sk_buff *skb, u16 family,
 		iph6 = ipv6_hdr(skb);
 		audit_log_format(audit_buf,
 				 " src=%pI6 dst=%pI6 flowlbl=0x%x%02x%02x",
-				 &iph6->saddr,&iph6->daddr,
+				 &iph6->saddr, &iph6->daddr,
 				 iph6->flow_lbl[0] & 0x0f,
 				 iph6->flow_lbl[1],
 				 iph6->flow_lbl[2]);
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 97681a3..e50be42 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1731,11 +1731,11 @@ static int build_aevent(struct sk_buff *skb, struct xfrm_state *x, const struct
 		return -EMSGSIZE;
 
 	id = nlmsg_data(nlh);
-	memcpy(&id->sa_id.daddr, &x->id.daddr,sizeof(x->id.daddr));
+	memcpy(&id->sa_id.daddr, &x->id.daddr, sizeof(x->id.daddr));
 	id->sa_id.spi = x->id.spi;
 	id->sa_id.family = x->props.family;
 	id->sa_id.proto = x->id.proto;
-	memcpy(&id->saddr, &x->props.saddr,sizeof(x->props.saddr));
+	memcpy(&id->saddr, &x->props.saddr, sizeof(x->props.saddr));
 	id->reqid = x->props.reqid;
 	id->flags = c->data.aevent;
 
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 02/15] xfrm: checkpatch errors with foo * bar
From: Steffen Klassert @ 2014-01-14  6:49 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <1389682159-3260-1-git-send-email-steffen.klassert@secunet.com>

From: Weilong Chen <chenweilong@huawei.com>

This patch clean up some checkpatch errors like this:
ERROR: "foo * bar" should be "foo *bar"
ERROR: "(foo*)" should be "(foo *)"

Signed-off-by: Weilong Chen <chenweilong@huawei.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_input.c  |    6 +++---
 net/xfrm/xfrm_policy.c |   12 ++++++------
 net/xfrm/xfrm_state.c  |    8 ++++----
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c
index 8884399..6c7ac01 100644
--- a/net/xfrm/xfrm_input.c
+++ b/net/xfrm/xfrm_input.c
@@ -67,7 +67,7 @@ int xfrm_parse_spi(struct sk_buff *skb, u8 nexthdr, __be32 *spi, __be32 *seq)
 	case IPPROTO_COMP:
 		if (!pskb_may_pull(skb, sizeof(struct ip_comp_hdr)))
 			return -EINVAL;
-		*spi = htonl(ntohs(*(__be16*)(skb_transport_header(skb) + 2)));
+		*spi = htonl(ntohs(*(__be16 *)(skb_transport_header(skb) + 2)));
 		*seq = 0;
 		return 0;
 	default:
@@ -77,8 +77,8 @@ int xfrm_parse_spi(struct sk_buff *skb, u8 nexthdr, __be32 *spi, __be32 *seq)
 	if (!pskb_may_pull(skb, hlen))
 		return -EINVAL;
 
-	*spi = *(__be32*)(skb_transport_header(skb) + offset);
-	*seq = *(__be32*)(skb_transport_header(skb) + offset_seq);
+	*spi = *(__be32 *)(skb_transport_header(skb) + offset);
+	*seq = *(__be32 *)(skb_transport_header(skb) + offset_seq);
 	return 0;
 }
 
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 8a2b9d8..dc8bd1b 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -171,7 +171,7 @@ static inline unsigned long make_jiffies(long secs)
 
 static void xfrm_policy_timer(unsigned long data)
 {
-	struct xfrm_policy *xp = (struct xfrm_policy*)data;
+	struct xfrm_policy *xp = (struct xfrm_policy *)data;
 	unsigned long now = get_seconds();
 	long next = LONG_MAX;
 	int warn = 0;
@@ -1758,7 +1758,7 @@ xfrm_resolve_and_create_bundle(struct xfrm_policy **pols, int num_pols,
 	}
 
 	xdst->num_pols = num_pols;
-	memcpy(xdst->pols, pols, sizeof(struct xfrm_policy*) * num_pols);
+	memcpy(xdst->pols, pols, sizeof(struct xfrm_policy *) * num_pols);
 	xdst->policy_genid = atomic_read(&pols[0]->genid);
 
 	return xdst;
@@ -2027,7 +2027,7 @@ make_dummy_bundle:
 	}
 	xdst->num_pols = num_pols;
 	xdst->num_xfrms = num_xfrms;
-	memcpy(xdst->pols, pols, sizeof(struct xfrm_policy*) * num_pols);
+	memcpy(xdst->pols, pols, sizeof(struct xfrm_policy *) * num_pols);
 
 	dst_hold(&xdst->u.dst);
 	return &xdst->flo;
@@ -2136,7 +2136,7 @@ struct dst_entry *xfrm_lookup(struct net *net, struct dst_entry *dst_orig,
 
 		num_pols = xdst->num_pols;
 		num_xfrms = xdst->num_xfrms;
-		memcpy(pols, xdst->pols, sizeof(struct xfrm_policy*) * num_pols);
+		memcpy(pols, xdst->pols, sizeof(struct xfrm_policy *) * num_pols);
 		route = xdst->route;
 	}
 
@@ -3064,8 +3064,8 @@ static bool xfrm_migrate_selector_match(const struct xfrm_selector *sel_cmp,
 	return false;
 }
 
-static struct xfrm_policy * xfrm_migrate_policy_find(const struct xfrm_selector *sel,
-						     u8 dir, u8 type, struct net *net)
+static struct xfrm_policy *xfrm_migrate_policy_find(const struct xfrm_selector *sel,
+						    u8 dir, u8 type, struct net *net)
 {
 	struct xfrm_policy *pol, *ret = NULL;
 	struct hlist_head *chain;
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 5fe79b4..9e6a4d6 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -382,7 +382,7 @@ static inline unsigned long make_jiffies(long secs)
 		return secs*HZ;
 }
 
-static enum hrtimer_restart xfrm_timer_handler(struct hrtimer * me)
+static enum hrtimer_restart xfrm_timer_handler(struct hrtimer *me)
 {
 	struct tasklet_hrtimer *thr = container_of(me, struct tasklet_hrtimer, timer);
 	struct xfrm_state *x = container_of(thr, struct xfrm_state, mtimer);
@@ -1237,8 +1237,8 @@ struct xfrm_state *xfrm_migrate_state_find(struct xfrm_migrate *m, struct net *n
 }
 EXPORT_SYMBOL(xfrm_migrate_state_find);
 
-struct xfrm_state * xfrm_state_migrate(struct xfrm_state *x,
-				       struct xfrm_migrate *m)
+struct xfrm_state *xfrm_state_migrate(struct xfrm_state *x,
+				      struct xfrm_migrate *m)
 {
 	struct xfrm_state *xc;
 	int err;
@@ -1630,7 +1630,7 @@ EXPORT_SYMBOL(xfrm_state_walk_done);
 
 static void xfrm_replay_timer_handler(unsigned long data)
 {
-	struct xfrm_state *x = (struct xfrm_state*)data;
+	struct xfrm_state *x = (struct xfrm_state *)data;
 
 	spin_lock(&x->lock);
 
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 04/15] xfrm: fix checkpatch error
From: Steffen Klassert @ 2014-01-14  6:49 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <1389682159-3260-1-git-send-email-steffen.klassert@secunet.com>

From: Weilong Chen <chenweilong@huawei.com>

Fix that "else should follow close brace '}'".

Signed-off-by: Weilong Chen <chenweilong@huawei.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_policy.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index fbc72b4..b5c315e 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -1316,9 +1316,9 @@ xfrm_tmpl_resolve_one(struct xfrm_policy *policy, const struct flowi *fl,
 			error = (x->km.state == XFRM_STATE_ERROR ?
 				 -EINVAL : -EAGAIN);
 			xfrm_state_put(x);
-		}
-		else if (error == -ESRCH)
+		} else if (error == -ESRCH) {
 			error = -EAGAIN;
+		}
 
 		if (!tmpl->optional)
 			goto fail;
-- 
1.7.9.5

^ permalink raw reply related


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