Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next v2 3/4] sctp: convert sctp_peer_needs_update to boolean
From: Wang Weidong @ 2013-11-08  6:17 UTC (permalink / raw)
  To: vyasevich, nhorman; +Cc: davem, dingtianhong, linux-sctp, netdev
In-Reply-To: <1383891471-20892-1-git-send-email-wangweidong1@huawei.com>

sctp_peer_needs_update only return 0 or 1.

Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
---
 net/sctp/associola.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index 98acb47..8dab0bf 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -1349,7 +1349,7 @@ void sctp_assoc_sync_pmtu(struct sock *sk, struct sctp_association *asoc)
 }
 
 /* Should we send a SACK to update our peer? */
-static inline int sctp_peer_needs_update(struct sctp_association *asoc)
+static inline bool sctp_peer_needs_update(struct sctp_association *asoc)
 {
 	struct net *net = sock_net(asoc->base.sk);
 	switch (asoc->state) {
@@ -1361,12 +1361,12 @@ static inline int sctp_peer_needs_update(struct sctp_association *asoc)
 		    ((asoc->rwnd - asoc->a_rwnd) >= max_t(__u32,
 			   (asoc->base.sk->sk_rcvbuf >> net->sctp.rwnd_upd_shift),
 			   asoc->pathmtu)))
-			return 1;
+			return true;
 		break;
 	default:
 		break;
 	}
-	return 0;
+	return false;
 }
 
 /* Increase asoc's rwnd by len and send any window update SACK if needed. */
-- 
1.7.12

^ permalink raw reply related

* [PATCH net-next v2 0/4] sctp: do some clean ups
From: Wang Weidong @ 2013-11-08  6:17 UTC (permalink / raw)
  To: vyasevich, nhorman; +Cc: davem, dingtianhong, linux-sctp, netdev

This patch series include: remove the duplicate initialize,                                                                                                
make code simple, convert func to boolean, fix some typos.

v1 -> v2:
  - patch2: make the code more simplification, as suggested
    by Joe Perches.

Wang Weidong (4):
  sctp: remove the duplicate initialize
  sctp: remove the else path
  sctp: convert sctp_peer_needs_update to boolean
  sctp: fix some comments in associola.c

 net/sctp/associola.c | 65 ++++++++--------------------------------------------
 1 file changed, 10 insertions(+), 55 deletions(-)

-- 
1.7.12

^ permalink raw reply

* Re: [PATCH v2 net-next] inet: fix a UFO regression
From: Sridhar Samudrala @ 2013-11-08  6:41 UTC (permalink / raw)
  To: Alexei Starovoitov, Eric Dumazet
  Cc: David Miller, netdev, Hannes Frederic Sowa
In-Reply-To: <CAMEtUuyuHagy7j+fi8RnKjZNBnwYHhxfed508HyXLfHp-e634w@mail.gmail.com>

On 11/7/2013 9:44 PM, Alexei Starovoitov wrote:
> On Thu, Nov 7, 2013 at 6:32 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> From: Eric Dumazet <edumazet@google.com>
>>
>> While testing virtio_net and skb_segment() changes, Hannes reported
>> that UFO was sending wrong frames.
>>
>> It appears this was introduced by a recent commit :
>> 8c3a897bfab1 ("inet: restore gso for vxlan")
>>
>> The old condition to perform IP frag was :
>>
>> tunnel = !!skb->encapsulation;
>> ...
>>          if (!tunnel && proto == IPPROTO_UDP) {
>>
>> So the new one should be :
>>
>> udpfrag = !skb->encapsulation && proto == IPPROTO_UDP;
>> ...
>>          if (udpfrag) {
>>
>> Initialization of udpfrag must be done before call
>> to ops->callbacks.gso_segment(skb, features), as
>> skb_udp_tunnel_segment() clears skb->encapsulation
>>
>> (We want udpfrag to be true for UFO, false for VXLAN)
>>
>> With help from Alexei Starovoitov
>>
>> Reported-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>> Cc: Alexei Starovoitov <ast@plumgrid.com>
>> ---
> vxlan looks good through namespaces with and without gso
> and between physical machines via 10G nics

Does this fix also help vxlan performance between 2 VMs on 2 different 
physical m/cs across 10G NIC?
What is the throughput you are seeing via 10G nics?
With linux 3.12, i am only seeing around 2Gbps (iperf TCP_STREAM with 
16K messages)  between 2 VMs when using vxlan across a 10G nic.
Is this due to the overhead of software GSO and not doing GRO at the 
receiver?

Thanks
Sridhar
>
> Tested-by: Alexei Starovoitov <ast@plumgrid.com>
>
> Thanks Eric!
> --
> 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 net-next 0/5] sctp: fix and clean up
From: David Miller @ 2013-11-08  6:41 UTC (permalink / raw)
  To: wangweidong1; +Cc: vyasevich, nhorman, dingtianhong, linux-sctp, netdev
In-Reply-To: <1383879310-22792-1-git-send-email-wangweidong1@huawei.com>


Such patches are not appropriate to submit at this time.

Please wait until the current merge window is completed and
the net-next tree is openned back up.

Thank you.

^ permalink raw reply

* Re: [PATCH net-next v2 0/10] bonding: rebuild the lock use for bond monitor
From: David Miller @ 2013-11-08  6:45 UTC (permalink / raw)
  To: dingtianhong; +Cc: fubar, andy, nikolay, vfalico, netdev
In-Reply-To: <527C4768.3010909@huawei.com>


Such patches should not be submitted at this time, the merge window
has openned up and therefore the net-next tree is closed.

Please wait for the merge window to close and the net-next tree
to open back up before submitting these changes.

Thanks.

^ permalink raw reply

* Buyer.
From: Doku Michael @ 2013-11-08  7:04 UTC (permalink / raw)


Dear Buyer,

 I am Mr. Michael Doku, representative to my villagers in Ghana, we are
prepared to provide any quantities of 99.6% purity (23 karat+) alluvial
gold at any time of demand.

 Below is the information  of our Gold dust:
Quantity: Large Quantity available monthly
Quality: 23karat+ plus alluvial
Origin: Ghana, West Africa

Assay reports provided by Geological services.

 We also have a reasonable quantity of Rough Diamond for sale.  We seek big
and genuine buyers for serious and long time business relationship.

 Thanks for your utmost co-operation.
Best Regards.
Michael Doku.
Reply To:micdoku@yahoo.co.uk

^ permalink raw reply

* Re: [PATCH] wcn36xx: Fix logging macro with unnecessary semicolon
From: Kalle Valo @ 2013-11-08  7:06 UTC (permalink / raw)
  To: Joe Perches
  Cc: Eugene Krasnikov, Luis R. Rodriguez, John W. Linville, wcn36xx,
	linux-wireless, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	ath5k-devel-xDcbHBWguxEUs3QNXV6qNA, ath9k-devel,
	ath10k-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	wil6210-A+ZNKFmMK5xy9aJCnZT0Uw
In-Reply-To: <1383853221.9263.14.camel@joe-AO722>

Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org> writes:

>> have a
>> second round of convincing ath guys to change printing code. How does
>> that sound?
>
> Luis?  Kalle?  Other Qualcomm/Atheros folk?

I didn't quite get what you are asking from me. But for me most
important is that the current debugging facilities from user's point of
view don't change. I don't care if the code is in ath10k.ko or ath.ko,
we are talking about ~100 lines of code anyway.

> One of the nominal benefits of separating the ath_<level>
> macros by subsystem was perf/tracing.

Nominal?

-- 
Kalle Valo
--
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 v2 net-next] inet: fix a UFO regression
From: David Miller @ 2013-11-08  7:08 UTC (permalink / raw)
  To: eric.dumazet; +Cc: ast, netdev, hannes
In-Reply-To: <1383877926.9412.145.camel@edumazet-glaptop2.roam.corp.google.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 07 Nov 2013 18:32:06 -0800

> From: Eric Dumazet <edumazet@google.com>
> 
> While testing virtio_net and skb_segment() changes, Hannes reported
> that UFO was sending wrong frames.
> 
> It appears this was introduced by a recent commit :
> 8c3a897bfab1 ("inet: restore gso for vxlan")
> 
> The old condition to perform IP frag was :
> 
> tunnel = !!skb->encapsulation;
> ...
>         if (!tunnel && proto == IPPROTO_UDP) {
> 
> So the new one should be :
> 
> udpfrag = !skb->encapsulation && proto == IPPROTO_UDP;
> ...
>         if (udpfrag) {
> 
> Initialization of udpfrag must be done before call
> to ops->callbacks.gso_segment(skb, features), as
> skb_udp_tunnel_segment() clears skb->encapsulation
> 
> (We want udpfrag to be true for UFO, false for VXLAN)
> 
> With help from Alexei Starovoitov
> 
> Reported-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied, thanks a lot Eric.

^ permalink raw reply

* [PATCH] svcrpc: remove the unnecessary evaluation
From: Weng Meiling @ 2013-11-08  7:23 UTC (permalink / raw)
  To: linux-nfs-u79uwXL29TY76Z2rM5mHXA
  Cc: Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
	bfields-uC3wQj2KruNg9hUCZPvPmw, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Li Zefan
In-Reply-To: <52786417.6090707-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>


From: Weng Meiling <wengmeiling.weng-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>

Signed-off-by: Weng Meiling <wengmeiling.weng-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 net/sunrpc/svc.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index b974571..e7fbe36 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -1104,8 +1104,6 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
 	rqstp->rq_vers = vers = svc_getnl(argv);	/* version number */
 	rqstp->rq_proc = proc = svc_getnl(argv);	/* procedure number */

-	progp = serv->sv_program;
-
 	for (progp = serv->sv_program; progp; progp = progp->pg_next)
 		if (prog == progp->pg_prog)
 			break;
-- 
1.8.3.1




--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" 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 related

* Re: [PATCH net-next v2] net_sched: tbf: support of 64bit rates
From: David Miller @ 2013-11-08  7:38 UTC (permalink / raw)
  To: eric.dumazet; +Cc: yangyingliang, netdev, jhs, sergei.shtylyov
In-Reply-To: <1383878060.9412.146.camel@edumazet-glaptop2.roam.corp.google.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 07 Nov 2013 18:34:20 -0800

> On Fri, 2013-11-08 at 10:23 +0800, Yang Yingliang wrote:
>> With psched_ratecfg_precompute(), tbf can deal with 64bit rates.
>> Add two new attributes so that tc can use them to break the 32bit
>> limit.
>> 
>> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
>> Suggested-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>> ---
>>  include/uapi/linux/pkt_sched.h |  2 ++
>>  net/sched/sch_tbf.c            | 22 ++++++++++++++++++----
>>  2 files changed, 20 insertions(+), 4 deletions(-)
> 
> Acked-by: Eric Dumazet <edumazet@google.com>

Applied, thanks.

^ permalink raw reply

* Re: [patch net-next 1/2] ip6_output: fragment outgoing reassembled skb properly
From: Jiri Pirko @ 2013-11-08  7:52 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, pablo, netfilter-devel, yoshfuji, kadlec, kaber, mleitner,
	kuznet, jmorris, wensong, horms, ja, edumazet, pshelar, jasowang,
	alexander.h.duyck, fw
In-Reply-To: <20131107.185453.1341640661253119352.davem@davemloft.net>

Fri, Nov 08, 2013 at 12:54:53AM CET, davem@davemloft.net wrote:
>From: Jiri Pirko <jiri@resnulli.us>
>Date: Wed,  6 Nov 2013 17:52:19 +0100
>
>> If reassembled packet would fit into outdev MTU, it is not fragmented
>> according the original frag size and it is send as single big packet.
>> 
>> The second case is if skb is gso. In that case fragmentation does not happen
>> according to the original frag size.
>> 
>> This patch fixes these.
>> 
>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> ...
>
>>  	if ((skb->len > ip6_skb_dst_mtu(skb) && !skb_is_gso(skb)) ||
>> -	    dst_allfrag(skb_dst(skb)))
>> +	    dst_allfrag(skb_dst(skb)) ||
>> +	    (IP6CB(skb)->frag_max_size && skb->len > IP6CB(skb)->frag_max_size))
>>  		return ip6_fragment(skb, ip6_finish_output2);
>
>Jiri are you sure that you don't need to take GSO into account in the
>new part you are adding to the test?


For gso skb, we need co cap outgoing fragments by the original frag size
as well. So I believe that this code is correct for that case as well.

>--
>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] net: make ndev->irq signed for error handling
From: Dan Carpenter @ 2013-11-08  8:41 UTC (permalink / raw)
  To: Mugunthan V N; +Cc: David S. Miller, netdev, linux-kernel, kernel-janitors
In-Reply-To: <527BC0CF.70003@ti.com>

On Thu, Nov 07, 2013 at 10:03:19PM +0530, Mugunthan V N wrote:
> On Thursday 07 November 2013 05:52 PM, Dan Carpenter wrote:
> > On Thu, Nov 07, 2013 at 05:44:38PM +0530, Mugunthan V N wrote:
> >> On Thursday 07 November 2013 01:18 PM, Dan Carpenter wrote:
> >>> There is a bug in cpsw_probe() where we do:
> >>>
> >>> 	ndev->irq = platform_get_irq(pdev, 0);
> >>> 	if (ndev->irq < 0) {
> >>>
> >>> The problem is that "ndev->irq" is unsigned so the error handling
> >>> doesn't work.  I have changed it to a regular int.
> >>>
> >>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> >> ndev->irq is never used any where in the driver, I think its better to
> >> remove this part of code from probe. If every one is ok, I can send a
> >> patch to remove the code.
> > It seems like cpsw_ndo_poll_controller() uses it.
> >
> >
> 
> That can be changed to pass one of the interrupt numbers from priv as
> the irq number is not used in interrupt service routine.

I don't know anything about this driver, so I'm fine with your idea.

regards,
dan carpenter

^ permalink raw reply

* [net-next PATCH 0/2] ixgbe: macvlan fixes
From: John Fastabend @ 2013-11-08  8:50 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: netdev, davem, nhorman

Two additional fixes for ixgbe l2 forwarding acceleration. One
was reported by kbuild build bot and the second after hardening
my test scripts.

Typically I would send ixgbe fixes through JeffK's tree but I
know a few people who are looking at the feature in their setup's 
so wanted to be sure the patches were visible.

For reference the ixgbe accelerated macvlan patch is here:

commit 2a47fa45d4dfbc54659d28de311a1f764b296a3c
Author: John Fastabend <john.r.fastabend@intel.com>
Date:   Wed Nov 6 09:54:52 2013 -0800

    ixgbe: enable l2 forwarding acceleration for macvlans

Thanks,
John

---

John Fastabend (2):
      ixgbe: fix build err, num_rx_queues is only available with CONFIG_RPS
      ixgbe: deleting dfwd stations out of order can cause null ptr deref


 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

-- 
Signature

^ permalink raw reply

* [net-next PATCH 1/2] ixgbe: fix build err, num_rx_queues is only available with CONFIG_RPS
From: John Fastabend @ 2013-11-08  8:50 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: netdev, davem, nhorman
In-Reply-To: <20131108084654.6935.36501.stgit@nitbit.x32>

In the recent support for layer 2 hardware acceleration, I added a
few references to real_num_rx_queues and num_rx_queues which are
only available with CONFIG_RPS.

The fix is first to remove unnecessary references to num_rx_queues.
Because the hardware offload case is limited to cases where RX queues
and TX queues are equal we only need a single check. Then wrap the
single case in an ifdef.

The patch that introduce this is here,

commit a6cc0cfa72e0b6d9f2c8fd858aacc32313c4f272
Author: John Fastabend <john.r.fastabend@intel.com>
Date:   Wed Nov 6 09:54:46 2013 -0800

    net: Add layer 2 hardware acceleration operations for macvlan devices

Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 607275d..2e17c30 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -4164,7 +4164,7 @@ static void ixgbe_add_mac_filter(struct ixgbe_adapter *adapter,
 static void ixgbe_fwd_psrtype(struct ixgbe_fwd_adapter *vadapter)
 {
 	struct ixgbe_adapter *adapter = vadapter->real_adapter;
-	int rss_i = vadapter->netdev->real_num_rx_queues;
+	int rss_i = adapter->num_rx_queues_per_pool;
 	struct ixgbe_hw *hw = &adapter->hw;
 	u16 pool = vadapter->pool;
 	u32 psrtype = IXGBE_PSRTYPE_TCPHDR |
@@ -4315,8 +4315,6 @@ static int ixgbe_fwd_ring_up(struct net_device *vdev,
 	if (err)
 		goto fwd_queue_err;
 
-	queues = min_t(unsigned int,
-		       adapter->num_rx_queues_per_pool, vdev->num_rx_queues);
 	err = netif_set_real_num_rx_queues(vdev, queues);
 	if (err)
 		goto fwd_queue_err;
@@ -7540,9 +7538,15 @@ static void *ixgbe_fwd_add(struct net_device *pdev, struct net_device *vdev)
 	struct ixgbe_adapter *adapter = netdev_priv(pdev);
 	int pool, err;
 
+#ifdef CONFIG_RPS
+	if (vdev->num_rx_queues != vdev->num_tx_queues) {
+		netdev_info(pdev, "%s: Only supports a single queue count for TX and RX\n",
+			    vdev->name);
+		return ERR_PTR(-EINVAL);
+	}
+#endif
 	/* Check for hardware restriction on number of rx/tx queues */
-	if (vdev->num_rx_queues != vdev->num_tx_queues ||
-	    vdev->num_tx_queues > IXGBE_MAX_L2A_QUEUES ||
+	if (vdev->num_tx_queues > IXGBE_MAX_L2A_QUEUES ||
 	    vdev->num_tx_queues == IXGBE_BAD_L2A_QUEUE) {
 		netdev_info(pdev,
 			    "%s: Supports RX/TX Queue counts 1,2, and 4\n",
@@ -7566,7 +7570,7 @@ static void *ixgbe_fwd_add(struct net_device *pdev, struct net_device *vdev)
 	/* Enable VMDq flag so device will be set in VM mode */
 	adapter->flags |= IXGBE_FLAG_VMDQ_ENABLED | IXGBE_FLAG_SRIOV_ENABLED;
 	adapter->ring_feature[RING_F_VMDQ].limit = adapter->num_rx_pools;
-	adapter->ring_feature[RING_F_RSS].limit = vdev->num_rx_queues;
+	adapter->ring_feature[RING_F_RSS].limit = vdev->num_tx_queues;
 
 	/* Force reinit of ring allocation with VMDQ enabled */
 	err = ixgbe_setup_tc(pdev, netdev_get_num_tc(pdev));

^ permalink raw reply related

* [net-next PATCH 2/2] ixgbe: deleting dfwd stations out of order can cause null ptr deref
From: John Fastabend @ 2013-11-08  8:51 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: netdev, davem, nhorman
In-Reply-To: <20131108084654.6935.36501.stgit@nitbit.x32>

The number of stations in use is kept in the num_rx_pools counter
in the ixgbe_adapter structure. This is in turn used by the queue
allocation scheme to determine how many queues are needed to support
the number of pools in use with the current feature set.

This works as long as the pools are added and destroyed in order
because (num_rx_pools * queues_per_pool) is equal to the last
queue in use by a pool. But as soon as you delete a pool out of
order this is no longer the case. So the above multiplication
allocates to few queues and a pool may reference a ring that has
not been allocated/initialized.

To resolve use the bit mask of in use pools to determine the final
pool being used and allocate enough queues so that we don't
inadvertently remove its queues.

# ip link add link eth2 \
	numtxqueues 4 numrxqueues 4 txqueuelen 50 type macvlan
# ip link set dev macvlan0 up
# ip link add link eth2 \
	numtxqueues 4 numrxqueues 4 txqueuelen 50 type macvlan
# ip link set dev macvlan1 up
# for i in {0..100}; do
  ip link set dev macvlan0 down; ip link set dev macvlan0 up;
  done;

Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 2e17c30..ec1bf3e 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -7536,6 +7536,7 @@ static void *ixgbe_fwd_add(struct net_device *pdev, struct net_device *vdev)
 {
 	struct ixgbe_fwd_adapter *fwd_adapter = NULL;
 	struct ixgbe_adapter *adapter = netdev_priv(pdev);
+	unsigned int limit;
 	int pool, err;
 
 #ifdef CONFIG_RPS
@@ -7566,10 +7567,11 @@ static void *ixgbe_fwd_add(struct net_device *pdev, struct net_device *vdev)
 	pool = find_first_zero_bit(&adapter->fwd_bitmask, 32);
 	adapter->num_rx_pools++;
 	set_bit(pool, &adapter->fwd_bitmask);
+	limit = find_last_bit(&adapter->fwd_bitmask, 32);
 
 	/* Enable VMDq flag so device will be set in VM mode */
 	adapter->flags |= IXGBE_FLAG_VMDQ_ENABLED | IXGBE_FLAG_SRIOV_ENABLED;
-	adapter->ring_feature[RING_F_VMDQ].limit = adapter->num_rx_pools;
+	adapter->ring_feature[RING_F_VMDQ].limit = limit + 1;
 	adapter->ring_feature[RING_F_RSS].limit = vdev->num_tx_queues;
 
 	/* Force reinit of ring allocation with VMDQ enabled */
@@ -7597,11 +7599,13 @@ static void ixgbe_fwd_del(struct net_device *pdev, void *priv)
 {
 	struct ixgbe_fwd_adapter *fwd_adapter = priv;
 	struct ixgbe_adapter *adapter = fwd_adapter->real_adapter;
+	unsigned int limit;
 
 	clear_bit(fwd_adapter->pool, &adapter->fwd_bitmask);
 	adapter->num_rx_pools--;
 
-	adapter->ring_feature[RING_F_VMDQ].limit = adapter->num_rx_pools;
+	limit = find_last_bit(&adapter->fwd_bitmask, 32);
+	adapter->ring_feature[RING_F_VMDQ].limit = limit + 1;
 	ixgbe_fwd_ring_down(fwd_adapter->netdev, fwd_adapter);
 	ixgbe_setup_tc(pdev, netdev_get_num_tc(pdev));
 	netdev_dbg(pdev, "pool %i:%i queues %i:%i VSI bitmask %lx\n",

^ permalink raw reply related

* [PATCH 0/2 v3] Open vSwitch zerocopy upcall
From: Thomas Graf @ 2013-11-08  9:15 UTC (permalink / raw)
  To: jesse-l0M0P4e3n4LQT0dZR+AlfA, davem-fT/PcQaiUtIeIZ0/mPfg9Q
  Cc: dev-yBygre7rU0TnMu66kgdUjQ, netdev-u79uwXL29TY76Z2rM5mHXA,
	eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w

Respin of the zerocopy patches for the openvswitch upcall.

V3: - Removed unneeded alignment of nlmsg_len after padding 
V2: - Added skb_zerocopy_headlen() to calculate headroom of destination
      buffer. This also takes care of the from->head_frag issue.
    - Attribute alignment for frag_list case
    - API documentation
    - performance data for CHECKSUM_PARTIAL tx case


Thomas Graf (2):
  net: Export skb_zerocopy() to zerocopy from one skb to another
  openvswitch: Use skb_zerocopy() for upcall

 include/linux/skbuff.h               |  3 ++
 net/core/skbuff.c                    | 85 ++++++++++++++++++++++++++++++++++++
 net/netfilter/nfnetlink_queue_core.c | 59 ++-----------------------
 net/openvswitch/datapath.c           | 52 +++++++++++++++++++---
 4 files changed, 137 insertions(+), 62 deletions(-)

-- 
1.8.3.1

^ permalink raw reply

* [PATCH 1/2 net-next] net: Export skb_zerocopy() to zerocopy from one skb to another
From: Thomas Graf @ 2013-11-08  9:15 UTC (permalink / raw)
  To: jesse-l0M0P4e3n4LQT0dZR+AlfA, davem-fT/PcQaiUtIeIZ0/mPfg9Q
  Cc: dev-yBygre7rU0TnMu66kgdUjQ, netdev-u79uwXL29TY76Z2rM5mHXA,
	eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <cover.1383901577.git.tgraf-G/eBtMaohhA@public.gmane.org>

Make the skb zerocopy logic written for nfnetlink queue available for
use by other modules.

Signed-off-by: Thomas Graf <tgraf-G/eBtMaohhA@public.gmane.org>
---
 include/linux/skbuff.h               |  3 ++
 net/core/skbuff.c                    | 85 ++++++++++++++++++++++++++++++++++++
 net/netfilter/nfnetlink_queue_core.c | 59 ++-----------------------
 3 files changed, 92 insertions(+), 55 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 2e153b6..bf26616 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2371,6 +2371,9 @@ int skb_splice_bits(struct sk_buff *skb, unsigned int offset,
 		    struct pipe_inode_info *pipe, unsigned int len,
 		    unsigned int flags);
 void skb_copy_and_csum_dev(const struct sk_buff *skb, u8 *to);
+extern unsigned int    skb_zerocopy_headlen(const struct sk_buff *from);
+extern void	       skb_zerocopy(struct sk_buff *to, const struct sk_buff *from,
+				    int len, int hlen);
 void skb_split(struct sk_buff *skb, struct sk_buff *skb1, const u32 len);
 int skb_shift(struct sk_buff *tgt, struct sk_buff *skb, int shiftlen);
 void skb_scrub_packet(struct sk_buff *skb, bool xnet);
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 3735fad..ab00e0b 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2102,6 +2102,91 @@ __wsum skb_copy_and_csum_bits(const struct sk_buff *skb, int offset,
 }
 EXPORT_SYMBOL(skb_copy_and_csum_bits);
 
+ /**
+ *	skb_zerocopy_headlen - Calculate headroom needed for skb_zerocopy()
+ *	@from: source buffer
+ *
+ *	Calculates the amount of linear headroom needed in the 'to' skb passed
+ *	into skb_zerocopy().
+ */
+unsigned int
+skb_zerocopy_headlen(const struct sk_buff *from)
+{
+	unsigned int hlen = 0;
+
+	if (!from->head_frag ||
+	    skb_headlen(from) < L1_CACHE_BYTES ||
+	    skb_shinfo(from)->nr_frags >= MAX_SKB_FRAGS)
+		hlen = skb_headlen(from);
+
+	if (skb_has_frag_list(from))
+		hlen = from->len;
+
+	return hlen;
+}
+EXPORT_SYMBOL_GPL(skb_zerocopy_headlen);
+
+/**
+ *	skb_zerocopy - Zero copy skb to skb
+ *	@to: destination buffer
+ *	@source: source buffer
+ *	@len: number of bytes to copy from source buffer
+ *	@hlen: size of linear headroom in destination buffer
+ *
+ *	Copies up to `len` bytes from `from` to `to` by creating references
+ *	to the frags in the source buffer.
+ *
+ *	The `hlen` as calculated by skb_zerocopy_headlen() specifies the
+ *	headroom in the `to` buffer.
+ */
+void
+skb_zerocopy(struct sk_buff *to, const struct sk_buff *from, int len, int hlen)
+{
+	int i, j = 0;
+	int plen = 0; /* length of skb->head fragment */
+	struct page *page;
+	unsigned int offset;
+
+	BUG_ON(!from->head_frag && !hlen);
+
+	/* dont bother with small payloads */
+	if (len <= skb_tailroom(to)) {
+		skb_copy_bits(from, 0, skb_put(to, len), len);
+		return;
+	}
+
+	if (hlen) {
+		skb_copy_bits(from, 0, skb_put(to, hlen), hlen);
+		len -= hlen;
+	} else {
+		plen = min_t(int, skb_headlen(from), len);
+		if (plen) {
+			page = virt_to_head_page(from->head);
+			offset = from->data - (unsigned char *)page_address(page);
+			__skb_fill_page_desc(to, 0, page, offset, plen);
+			get_page(page);
+			j = 1;
+			len -= plen;
+		}
+	}
+
+	to->truesize += len + plen;
+	to->len += len + plen;
+	to->data_len += len + plen;
+
+	for (i = 0; i < skb_shinfo(from)->nr_frags; i++) {
+		if (!len)
+			break;
+		skb_shinfo(to)->frags[j] = skb_shinfo(from)->frags[i];
+		skb_shinfo(to)->frags[j].size = min_t(int, skb_shinfo(to)->frags[j].size, len);
+		len -= skb_shinfo(to)->frags[j].size;
+		skb_frag_ref(to, j);
+		j++;
+	}
+	skb_shinfo(to)->nr_frags = j;
+}
+EXPORT_SYMBOL_GPL(skb_zerocopy);
+
 void skb_copy_and_csum_dev(const struct sk_buff *skb, u8 *to)
 {
 	__wsum csum;
diff --git a/net/netfilter/nfnetlink_queue_core.c b/net/netfilter/nfnetlink_queue_core.c
index 21258cf..615ee12 100644
--- a/net/netfilter/nfnetlink_queue_core.c
+++ b/net/netfilter/nfnetlink_queue_core.c
@@ -235,51 +235,6 @@ nfqnl_flush(struct nfqnl_instance *queue, nfqnl_cmpfn cmpfn, unsigned long data)
 	spin_unlock_bh(&queue->lock);
 }
 
-static void
-nfqnl_zcopy(struct sk_buff *to, const struct sk_buff *from, int len, int hlen)
-{
-	int i, j = 0;
-	int plen = 0; /* length of skb->head fragment */
-	struct page *page;
-	unsigned int offset;
-
-	/* dont bother with small payloads */
-	if (len <= skb_tailroom(to)) {
-		skb_copy_bits(from, 0, skb_put(to, len), len);
-		return;
-	}
-
-	if (hlen) {
-		skb_copy_bits(from, 0, skb_put(to, hlen), hlen);
-		len -= hlen;
-	} else {
-		plen = min_t(int, skb_headlen(from), len);
-		if (plen) {
-			page = virt_to_head_page(from->head);
-			offset = from->data - (unsigned char *)page_address(page);
-			__skb_fill_page_desc(to, 0, page, offset, plen);
-			get_page(page);
-			j = 1;
-			len -= plen;
-		}
-	}
-
-	to->truesize += len + plen;
-	to->len += len + plen;
-	to->data_len += len + plen;
-
-	for (i = 0; i < skb_shinfo(from)->nr_frags; i++) {
-		if (!len)
-			break;
-		skb_shinfo(to)->frags[j] = skb_shinfo(from)->frags[i];
-		skb_shinfo(to)->frags[j].size = min_t(int, skb_shinfo(to)->frags[j].size, len);
-		len -= skb_shinfo(to)->frags[j].size;
-		skb_frag_ref(to, j);
-		j++;
-	}
-	skb_shinfo(to)->nr_frags = j;
-}
-
 static int
 nfqnl_put_packet_info(struct sk_buff *nlskb, struct sk_buff *packet,
 		      bool csum_verify)
@@ -304,7 +259,7 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
 {
 	size_t size;
 	size_t data_len = 0, cap_len = 0;
-	int hlen = 0;
+	unsigned int hlen = 0;
 	struct sk_buff *skb;
 	struct nlattr *nla;
 	struct nfqnl_msg_packet_hdr *pmsg;
@@ -356,14 +311,8 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
 		if (data_len > entskb->len)
 			data_len = entskb->len;
 
-		if (!entskb->head_frag ||
-		    skb_headlen(entskb) < L1_CACHE_BYTES ||
-		    skb_shinfo(entskb)->nr_frags >= MAX_SKB_FRAGS)
-			hlen = skb_headlen(entskb);
-
-		if (skb_has_frag_list(entskb))
-			hlen = entskb->len;
-		hlen = min_t(int, data_len, hlen);
+		hlen = skb_zerocopy_headlen(entskb);
+		hlen = min_t(unsigned int, hlen, data_len);
 		size += sizeof(struct nlattr) + hlen;
 		cap_len = entskb->len;
 		break;
@@ -504,7 +453,7 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
 		nla->nla_type = NFQA_PAYLOAD;
 		nla->nla_len = nla_attr_size(data_len);
 
-		nfqnl_zcopy(skb, entskb, data_len, hlen);
+		skb_zerocopy(skb, entskb, data_len, hlen);
 	}
 
 	nlh->nlmsg_len = skb->len;
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH 2/2 net-next] openvswitch: Use skb_zerocopy() for upcall
From: Thomas Graf @ 2013-11-08  9:15 UTC (permalink / raw)
  To: jesse, davem; +Cc: dev, netdev, eric.dumazet
In-Reply-To: <cover.1383901577.git.tgraf@suug.ch>

Use of skb_zerocopy() avoids the expensive call to memcpy() when
copying the packet data into the Netlink skb. Completes checksum
through skb_checksum_help() if needed.

Netlink messaged must be properly padded and aligned to meet
sanity checks of the user space counterpart.

Cost of memcpy is significantly reduced from:
+   7.48%       vhost-8471  [k] memcpy
+   5.57%     ovs-vswitchd  [k] memcpy
+   2.81%       vhost-8471  [k] csum_partial_copy_generic

to:
+   5.72%     ovs-vswitchd  [k] memcpy
+   3.32%       vhost-5153  [k] memcpy
+   0.68%       vhost-5153  [k] skb_zerocopy

(megaflows disabled)

Signed-off-by: Thomas Graf <tgraf@suug.ch>
---
 net/openvswitch/datapath.c | 52 +++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 45 insertions(+), 7 deletions(-)

diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 1408adc..3f170e3 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -381,10 +381,11 @@ static size_t key_attr_size(void)
 }
 
 static size_t upcall_msg_size(const struct sk_buff *skb,
-			      const struct nlattr *userdata)
+			      const struct nlattr *userdata,
+			      unsigned int hdrlen)
 {
 	size_t size = NLMSG_ALIGN(sizeof(struct ovs_header))
-		+ nla_total_size(skb->len) /* OVS_PACKET_ATTR_PACKET */
+		+ nla_total_size(hdrlen) /* OVS_PACKET_ATTR_PACKET */
 		+ nla_total_size(key_attr_size()); /* OVS_PACKET_ATTR_KEY */
 
 	/* OVS_PACKET_ATTR_USERDATA */
@@ -402,6 +403,7 @@ static int queue_userspace_packet(struct net *net, int dp_ifindex,
 	struct sk_buff *nskb = NULL;
 	struct sk_buff *user_skb; /* to be queued to userspace */
 	struct nlattr *nla;
+	unsigned int plen, hlen;
 	int err;
 
 	if (vlan_tx_tag_present(skb)) {
@@ -422,7 +424,13 @@ static int queue_userspace_packet(struct net *net, int dp_ifindex,
 		goto out;
 	}
 
-	user_skb = genlmsg_new(upcall_msg_size(skb, upcall_info->userdata), GFP_ATOMIC);
+	/* Complete checksum if needed */
+	if (skb->ip_summed == CHECKSUM_PARTIAL &&
+	    (err = skb_checksum_help(skb)))
+		goto out;
+
+	hlen = skb_zerocopy_headlen(skb);
+	user_skb = genlmsg_new(upcall_msg_size(skb, upcall_info->userdata, hlen), GFP_ATOMIC);
 	if (!user_skb) {
 		err = -ENOMEM;
 		goto out;
@@ -441,13 +449,43 @@ static int queue_userspace_packet(struct net *net, int dp_ifindex,
 			  nla_len(upcall_info->userdata),
 			  nla_data(upcall_info->userdata));
 
-	nla = __nla_reserve(user_skb, OVS_PACKET_ATTR_PACKET, skb->len);
+	/* Only reserve room for attribute header, packet data is added
+	 * in skb_zerocopy() */
+	if (!(nla = nla_reserve(user_skb, OVS_PACKET_ATTR_PACKET, 0)))
+		goto out;
+	nla->nla_len = nla_attr_size(skb->len);
 
-	skb_copy_and_csum_dev(skb, nla_data(nla));
+	skb_zerocopy(user_skb, skb, skb->len, hlen);
 
-	genlmsg_end(user_skb, upcall);
-	err = genlmsg_unicast(net, user_skb, upcall_info->portid);
+	/* OVS user space expects the size of the message to be aligned to
+	 * NLA_ALIGNTO. Aligning nlmsg_len is not enough, the actual bytes
+	 * read must match nlmsg_len.
+	 */
+	plen = NLA_ALIGN(user_skb->len) - user_skb->len;
+	if (plen > 0) {
+		int nr_frags = skb_shinfo(user_skb)->nr_frags;
+
+		if (nr_frags) {
+			skb_frag_t *frag;
+
+			frag = &skb_shinfo(user_skb)->frags[nr_frags -1];
+			skb_frag_size_add(frag, plen);
+			BUG_ON(frag->size > PAGE_SIZE);
+
+			user_skb->truesize += plen;
+			user_skb->len += plen;
+			user_skb->data_len += plen;
+		} else {
+			/* The linear headroom is aligned to NLMSG_ALIGN by
+			 * genlmsg_new(), room must be available.
+			 */
+			memset(skb_put(user_skb, plen), 0, plen);
+		}
+	}
 
+	((struct nlmsghdr *) user_skb->data)->nlmsg_len = user_skb->len;
+
+	err = genlmsg_unicast(net, user_skb, upcall_info->portid);
 out:
 	kfree_skb(nskb);
 	return err;
-- 
1.8.3.1

^ permalink raw reply related

* Re: [PATCH v3] can: add Renesas R-Car CAN driver
From: Marc Kleine-Budde @ 2013-11-08  9:17 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: netdev, wg, linux-can, linux-sh, vksavl
In-Reply-To: <527C28DE.7070402@cogentembedded.com>

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

On 11/08/2013 12:57 AM, Sergei Shtylyov wrote:
>>> Index: linux-can-next/drivers/net/can/rcar_can.c
>>> ===================================================================
>>> --- /dev/null
>>> +++ linux-can-next/drivers/net/can/rcar_can.c
>>> @@ -0,0 +1,893 @@
>>> +/*
>>> + * Renesas R-Car CAN device driver
>>> + *
>>> + * Copyright (C) 2013 Cogent Embedded, Inc. <source@cogentembedded.com>
>>> + * Copyright (C) 2013 Renesas Solutions Corp.
>>> + *
>>> + * This program is free software; you can redistribute  it and/or
>>> modify it
>>> + * under  the terms of  the GNU General  Public License as published
>>> by the
>>> + * Free Software Foundation;  either version 2 of the  License, or
>>> (at your
>>> + * option) any later version.
>>> + */
>>> +
>>> +#include <linux/module.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/types.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/errno.h>
>>> +#include <linux/netdevice.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/can/led.h>
>>> +#include <linux/can/dev.h>
>>> +#include <linux/clk.h>
>>> +#include <linux/can/platform/rcar_can.h>
>>> +
>>> +#define DRV_NAME    "rcar_can"
>>> +
>>> +/* Mailbox configuration:
>>> + * mailbox 0 - not used
>>> + * mailbox 1-31 - Rx
>>> + * mailbox 32-63 - Tx
>>> + * no FIFO mailboxes
>>> + */
>>> +#define N_MBX        64
>>> +#define FIRST_TX_MB    32
>>> +#define N_TX_MB        (N_MBX - FIRST_TX_MB)
>>> +#define RX_MBX_MASK    0xFFFFFFFE
> 
>> Please use a common prefix for all defines.
> 
>    OK, done now. Could you however explain why the file-local #define's
> should be prefixed? It's not quite obvious to me...

It's about readability and maintainability. If you don't know the
driver, but all driver local defines and functions have a common prefix,
it's much easier to read if you are not the author of the driver IMHO.

> 
> [...]
>>> +static netdev_tx_t rcar_can_start_xmit(struct sk_buff *skb,
>>> +                       struct net_device *ndev)
>>> +{
>>> +    struct rcar_can_priv *priv = netdev_priv(ndev);
>>> +    struct can_frame *cf = (struct can_frame *)skb->data;
>>> +    u32 data, mier1, mbxno, i;
>>> +    unsigned long flags;
>>> +    u8 mctl = 0;
>>> +
>>> +    if (can_dropped_invalid_skb(ndev, skb))
>>> +        return NETDEV_TX_OK;
>>> +
>>> +    spin_lock_irqsave(&priv->mier_lock, flags);
>>> +    mier1 = readl(&priv->regs->mier1);
>>> +    if (mier1) {
>>> +        i = __builtin_clz(mier1);
>>> +        mbxno = i ? N_MBX - i : FIRST_TX_MB;
>>> +    } else {
>>> +        mbxno = FIRST_TX_MB;
>>> +    }
> 
>> Can you explain how the hardware arbitration works, and you do you
>> guarantee the CAN frames are send by the hardware in the same order you
>> put them into the hardware.
> 
>    Tx mailbox with the smallest mailbox number has the highest priority.
> The other possible Tx mailbox selection rule (not used by the driver
> now) is ID priority transmit mode (as defined in the ISO 11898-1 specs).
> The algorithm used guarantees the mailboxes are filled sequentially.

I see. You are using mier1 to track the used mailboxes....correct?

> +	if (unlikely(mier1 == 0xffffffff))
> +		netif_stop_queue(ndev);

Then you have a race condition in your tx-complete handler
rcar_can_tx_done(), as you call netif_wake_queue() unconditionally. If
mier1 == 0xffffffff you must wait until _all_ mailboxes are transmitted
until you are allowed to reenable the mailboxes. Have a look at the
at91_can driver, it's using a similar scheme. The lowest mailbox is
transmitted first, but there are three additional bits that indicate the
priority.

> I've used 'canfdtest' as suggested by Wolfgang Grandegger to verify, see
> the log below:
> 
> root@am335x-evm:~# ./canfdtest -v -g can0
> interface = can0, family = 29, type = 3, proto = 1
> ...............................................................................C
> 
> Test messages sent and received: 483203
> Exiting...
> root@am335x-evm:~#
> 
> [...]
>>> +static int rcar_can_rx_poll(struct napi_struct *napi, int quota)
>>> +{
>>> +    struct rcar_can_priv *priv = container_of(napi,
>>> +                          struct rcar_can_priv, napi);
>>> +    int num_pkts = 0;
>>> +
>>> +    /* Find mailbox */
>>> +    while (num_pkts < quota) {
>>> +        u8 mctl, mbx;
>>> +
>>> +        mbx = readb(&priv->regs->mssr);
> 
>> How does the RX work? Is it a hardware FIFO?
> 
>    In short, the MSSR register provides the smallest Rx mailbox number
> that is looked up in the Rx search mode. We read MSSR until no search
> results can be obtained, so it is some sort of FIFO.

This looks racy....

>    And there is separate FIFO operation mode: some mailboxes can be
> configured as FIFO and serviced by special registers but this operation
> mode is not supported by the driver.

if you hardware supports a real FIFO then I strongly suggest to make use
of it.

> [...]
>>> +static int rcar_can_probe(struct platform_device *pdev)
>>> +{
>>> +    struct rcar_can_platform_data *pdata;
>>> +    struct rcar_can_priv *priv;
>>> +    struct net_device *ndev;
>>> +    struct resource *mem;
>>> +    void __iomem *addr;
>>> +    int err = -ENODEV;
>>> +    int irq;
>>> +
>>> +    pdata = dev_get_platdata(&pdev->dev);
>>> +    if (!pdata) {
>>> +        dev_err(&pdev->dev, "No platform data provided!\n");
>>> +        goto fail;
>>> +    }
>>> +
>>> +    irq = platform_get_irq(pdev, 0);
>>> +    if (!irq) {
>>> +        dev_err(&pdev->dev, "No IRQ resource\n");
>>> +        goto fail;
>>> +    }
>>> +
>>> +    mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> +    addr = devm_ioremap_resource(&pdev->dev, mem);
>>> +    if (IS_ERR(addr)) {
>>> +        err = PTR_ERR(addr);
>>> +        goto fail;
>>> +    }
>>> +
>>> +    ndev = alloc_candev(sizeof(struct rcar_can_priv), N_MBX -
>>> FIRST_TX_MB);
>>> +    if (!ndev) {
>>> +        dev_err(&pdev->dev, "alloc_candev failed\n");
>>> +        err = -ENOMEM;
>>> +        goto fail;
>>> +    }
>>> +
>>> +    priv = netdev_priv(ndev);
>>> +
>>> +    priv->clk = devm_clk_get(&pdev->dev, NULL);
>>> +    if (IS_ERR(priv->clk)) {
>>> +        err = PTR_ERR(priv->clk);
>>> +        dev_err(&pdev->dev, "cannot get clock: %d\n", err);
>>> +        goto fail_clk;
>>> +    }
>>> +
>>> +    ndev->netdev_ops = &rcar_can_netdev_ops;
>>> +    ndev->irq = irq;
>>> +    ndev->flags |= IFF_ECHO;
>>> +    priv->ndev = ndev;
>>> +    priv->regs = addr;
>>> +    priv->clock_select = pdata->clock_select;
>>> +    priv->can.clock.freq = clk_get_rate(priv->clk);
>>> +    priv->can.bittiming_const = &rcar_can_bittiming_const;
>>> +    priv->can.do_set_bittiming = rcar_can_set_bittiming;
> 
>> Please call this function directly during the open() function.
> 
>    OK, done, and the method installation was removed. However, I'm not
> sure why you requested this as many drivers are still using the method.

The callback was there from the beginning, but then we figured out that
we don't need it in the driver, but no one has cleaned up the drivers
yet. So don't use it in new drivers. I know it's not documented anywhere :(

regards,
Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

^ permalink raw reply

* Re: [patch] net: make ndev->irq signed for error handling
From: Mugunthan V N @ 2013-11-08  9:18 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: David S. Miller, netdev, linux-kernel, kernel-janitors
In-Reply-To: <20131108084136.GY26669@mwanda>

On Friday 08 November 2013 02:11 PM, Dan Carpenter wrote:
> On Thu, Nov 07, 2013 at 10:03:19PM +0530, Mugunthan V N wrote:
>> On Thursday 07 November 2013 05:52 PM, Dan Carpenter wrote:
>>> On Thu, Nov 07, 2013 at 05:44:38PM +0530, Mugunthan V N wrote:
>>>> On Thursday 07 November 2013 01:18 PM, Dan Carpenter wrote:
>>>>> There is a bug in cpsw_probe() where we do:
>>>>>
>>>>> 	ndev->irq = platform_get_irq(pdev, 0);
>>>>> 	if (ndev->irq < 0) {
>>>>>
>>>>> The problem is that "ndev->irq" is unsigned so the error handling
>>>>> doesn't work.  I have changed it to a regular int.
>>>>>
>>>>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>>>> ndev->irq is never used any where in the driver, I think its better to
>>>> remove this part of code from probe. If every one is ok, I can send a
>>>> patch to remove the code.
>>> It seems like cpsw_ndo_poll_controller() uses it.
>>>
>>>
>> That can be changed to pass one of the interrupt numbers from priv as
>> the irq number is not used in interrupt service routine.
> I don't know anything about this driver, so I'm fine with your idea.
>
Anyhow David has applied your patch, I will send a driver cleanup patch
seperately.

Regards
Mugunthan V N

^ permalink raw reply

* Re: ipv6: a question about ECMP
From: Nicolas Dichtel @ 2013-11-08 10:09 UTC (permalink / raw)
  To: sowmini varadhan, Duan Jiong, David Miller, netdev,
	Hannes Frederic Sowa
In-Reply-To: <CACP96tRh345x4D1BqpOK2eWfoeVbqMnu-+jGfF7eyFC1dbQoug@mail.gmail.com>

Le 07/11/2013 19:32, sowmini varadhan a écrit :
> On Thu, Nov 7, 2013 at 7:16 AM, Hannes Frederic Sowa
> <hannes@stressinduktion.org> wrote:
>> Hi Duan!
>>
>> On Thu, Nov 07, 2013 at 06:33:20PM +0800, Duan Jiong wrote:
>>>    After reading the ip6_pol_route(), i have a question about ECMP. Why we call
>>> the rt6_multipath_select() after calling rt6_select()?
>>>    In my opinion, the route returned by rt6_select() has a highest score, but the route
>>> returned by rt6_multipath_select() may has a lower score than the former, because the
>>> ECMP don't take the route preference into consideration. That means that the kernel will
>>> choose a less-desirable route.
>>
>> ECMP routes only differ in the gateway the specify, so I doubt there will be
>> any change in the score they woud receive. rt6_multipath_select does merly
>> make sure we don't select the same route again and again.
>
>   rt6_multipath_select() -> rt6_socre_route() seems to require that the
> interface *must* matchi, which is consistent with your assertion above that
> "ECMP routes differ in gw only".
In fact, ECMP routes have the same metric/weight and destination but not the
same next hop (ie gw + oif).

>
> But for IPv6, the gw addr is a a link-local, which is only required to be
> unique on the link. Thus, e.g.,  you can have fe80::1 as the gw on both eth0 and
> eth1.
Yes, oif can be different.
Note that gw can also be a global address.

>
> What is the assumption around "cost" for ECMP here- are we assuming some
> form of link bundling (Section 6 of rfc 2991) here? or is the "multiple parallel
> links" case handled somewhere else, that I am missing?
rt6_score_route() is called to check requested oif (see 52bd4c0c1551 "ipv6: fix 
ecmp lookup when oif is specified").

Regards,
Nicolas

>
> --Sowmini
>
>>
>> Please note, the rt6_info's siblings fields were added for the solely purpose
>> of ECMP and the insertion only updates the siblings list if the above criteria
>> did hold. They make sure the routes lookup up do differ on each lookup, so it
>> does actually do multipath and does not depend on the order the routes where
>> inserted.
>>
>> Hope that helps,
>>
>>    Hannes
>>
>> --
>> 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 linux-2.6.32.y] sctp: unbalanced rcu lock in ip_queue_xmit()
From: Nicolas Dichtel @ 2013-11-08 10:13 UTC (permalink / raw)
  To: netdev, davem; +Cc: eric.dumazet, ben, w, stable, Nicolas Dichtel

The bug was introduced by commit b8710128e201 ("inet: add RCU protection to
inet->opt") (it's a backport of upstream commit f6d8bd051c39).

In SCTP case, packet is already routed, hence we jump to the label
'packet_routed', but without rcu_read_lock(). After this label,
rcu_read_unlock() is called unconditionally.

Spotted-by: Guo Fengtian <fengtian.guo@6wind.com>
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 net/ipv4/ip_output.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 7dde039374e4..2cd69e3497ad 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -320,13 +320,13 @@ int ip_queue_xmit(struct sk_buff *skb, int ipfragok)
 	/* Skip all of this if the packet is already routed,
 	 * f.e. by something like SCTP.
 	 */
+	rcu_read_lock();
 	rt = skb_rtable(skb);
 	if (rt != NULL)
 		goto packet_routed;
 
 	/* Make sure we can route this packet. */
 	rt = (struct rtable *)__sk_dst_check(sk, 0);
-	rcu_read_lock();
 	inet_opt = rcu_dereference(inet->inet_opt);
 	if (rt == NULL) {
 		__be32 daddr;
-- 
1.8.4.1

^ permalink raw reply related

* [PATCH 0/2 v4] Open vSwitch zerocopy upcall
From: Thomas Graf @ 2013-11-08 10:47 UTC (permalink / raw)
  To: jesse, davem; +Cc: dev, netdev, eric.dumazet

Respin of the zerocopy patches for the openvswitch upcall.

V4: - Daniel Borkmann pointed out that the style in skbuff.h has changed
      in net-next, adapted to no longer using extern in headers.
V3: - Removed unneeded alignment of nlmsg_len after padding 
V2: - Added skb_zerocopy_headlen() to calculate headroom of destination
      buffer. This also takes care of the from->head_frag issue.
    - Attribute alignment for frag_list case
    - API documentation
    - performance data for CHECKSUM_PARTIAL tx case

Thomas Graf (2):
  net: Export skb_zerocopy() to zerocopy from one skb to another
  openvswitch: Use skb_zerocopy() for upcall

 include/linux/skbuff.h               |  3 ++
 net/core/skbuff.c                    | 85 ++++++++++++++++++++++++++++++++++++
 net/netfilter/nfnetlink_queue_core.c | 59 ++-----------------------
 net/openvswitch/datapath.c           | 52 +++++++++++++++++++---
 4 files changed, 137 insertions(+), 62 deletions(-)

-- 
1.8.3.1

^ permalink raw reply

* [PATCH 2/2 net-next] openvswitch: Use skb_zerocopy() for upcall
From: Thomas Graf @ 2013-11-08 10:47 UTC (permalink / raw)
  To: jesse, davem; +Cc: dev, netdev, eric.dumazet
In-Reply-To: <cover.1383906944.git.tgraf@suug.ch>

Use of skb_zerocopy() avoids the expensive call to memcpy() when
copying the packet data into the Netlink skb. Completes checksum
through skb_checksum_help() if needed.

Netlink messaged must be properly padded and aligned to meet
sanity checks of the user space counterpart.

Cost of memcpy is significantly reduced from:
+   7.48%       vhost-8471  [k] memcpy
+   5.57%     ovs-vswitchd  [k] memcpy
+   2.81%       vhost-8471  [k] csum_partial_copy_generic

to:
+   5.72%     ovs-vswitchd  [k] memcpy
+   3.32%       vhost-5153  [k] memcpy
+   0.68%       vhost-5153  [k] skb_zerocopy

(megaflows disabled)

Signed-off-by: Thomas Graf <tgraf@suug.ch>
Cc: Jesse Gross <jesse@nicira.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
---
 net/openvswitch/datapath.c | 52 +++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 45 insertions(+), 7 deletions(-)

diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 1408adc..3f170e3 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -381,10 +381,11 @@ static size_t key_attr_size(void)
 }
 
 static size_t upcall_msg_size(const struct sk_buff *skb,
-			      const struct nlattr *userdata)
+			      const struct nlattr *userdata,
+			      unsigned int hdrlen)
 {
 	size_t size = NLMSG_ALIGN(sizeof(struct ovs_header))
-		+ nla_total_size(skb->len) /* OVS_PACKET_ATTR_PACKET */
+		+ nla_total_size(hdrlen) /* OVS_PACKET_ATTR_PACKET */
 		+ nla_total_size(key_attr_size()); /* OVS_PACKET_ATTR_KEY */
 
 	/* OVS_PACKET_ATTR_USERDATA */
@@ -402,6 +403,7 @@ static int queue_userspace_packet(struct net *net, int dp_ifindex,
 	struct sk_buff *nskb = NULL;
 	struct sk_buff *user_skb; /* to be queued to userspace */
 	struct nlattr *nla;
+	unsigned int plen, hlen;
 	int err;
 
 	if (vlan_tx_tag_present(skb)) {
@@ -422,7 +424,13 @@ static int queue_userspace_packet(struct net *net, int dp_ifindex,
 		goto out;
 	}
 
-	user_skb = genlmsg_new(upcall_msg_size(skb, upcall_info->userdata), GFP_ATOMIC);
+	/* Complete checksum if needed */
+	if (skb->ip_summed == CHECKSUM_PARTIAL &&
+	    (err = skb_checksum_help(skb)))
+		goto out;
+
+	hlen = skb_zerocopy_headlen(skb);
+	user_skb = genlmsg_new(upcall_msg_size(skb, upcall_info->userdata, hlen), GFP_ATOMIC);
 	if (!user_skb) {
 		err = -ENOMEM;
 		goto out;
@@ -441,13 +449,43 @@ static int queue_userspace_packet(struct net *net, int dp_ifindex,
 			  nla_len(upcall_info->userdata),
 			  nla_data(upcall_info->userdata));
 
-	nla = __nla_reserve(user_skb, OVS_PACKET_ATTR_PACKET, skb->len);
+	/* Only reserve room for attribute header, packet data is added
+	 * in skb_zerocopy() */
+	if (!(nla = nla_reserve(user_skb, OVS_PACKET_ATTR_PACKET, 0)))
+		goto out;
+	nla->nla_len = nla_attr_size(skb->len);
 
-	skb_copy_and_csum_dev(skb, nla_data(nla));
+	skb_zerocopy(user_skb, skb, skb->len, hlen);
 
-	genlmsg_end(user_skb, upcall);
-	err = genlmsg_unicast(net, user_skb, upcall_info->portid);
+	/* OVS user space expects the size of the message to be aligned to
+	 * NLA_ALIGNTO. Aligning nlmsg_len is not enough, the actual bytes
+	 * read must match nlmsg_len.
+	 */
+	plen = NLA_ALIGN(user_skb->len) - user_skb->len;
+	if (plen > 0) {
+		int nr_frags = skb_shinfo(user_skb)->nr_frags;
+
+		if (nr_frags) {
+			skb_frag_t *frag;
+
+			frag = &skb_shinfo(user_skb)->frags[nr_frags -1];
+			skb_frag_size_add(frag, plen);
+			BUG_ON(frag->size > PAGE_SIZE);
+
+			user_skb->truesize += plen;
+			user_skb->len += plen;
+			user_skb->data_len += plen;
+		} else {
+			/* The linear headroom is aligned to NLMSG_ALIGN by
+			 * genlmsg_new(), room must be available.
+			 */
+			memset(skb_put(user_skb, plen), 0, plen);
+		}
+	}
 
+	((struct nlmsghdr *) user_skb->data)->nlmsg_len = user_skb->len;
+
+	err = genlmsg_unicast(net, user_skb, upcall_info->portid);
 out:
 	kfree_skb(nskb);
 	return err;
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH 1/2 net-next] net: Export skb_zerocopy() to zerocopy from one skb to another
From: Thomas Graf @ 2013-11-08 10:47 UTC (permalink / raw)
  To: jesse, davem; +Cc: dev, netdev, eric.dumazet
In-Reply-To: <cover.1383906944.git.tgraf@suug.ch>

Make the skb zerocopy logic written for nfnetlink queue available for
use by other modules.

Signed-off-by: Thomas Graf <tgraf@suug.ch>
---
 include/linux/skbuff.h               |  3 ++
 net/core/skbuff.c                    | 85 ++++++++++++++++++++++++++++++++++++
 net/netfilter/nfnetlink_queue_core.c | 59 ++-----------------------
 3 files changed, 92 insertions(+), 55 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 036ec7d..9e6a293 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2372,6 +2372,9 @@ int skb_splice_bits(struct sk_buff *skb, unsigned int offset,
 		    struct pipe_inode_info *pipe, unsigned int len,
 		    unsigned int flags);
 void skb_copy_and_csum_dev(const struct sk_buff *skb, u8 *to);
+unsigned int skb_zerocopy_headlen(const struct sk_buff *from);
+void skb_zerocopy(struct sk_buff *to, const struct sk_buff *from,
+		  int len, int hlen);
 void skb_split(struct sk_buff *skb, struct sk_buff *skb1, const u32 len);
 int skb_shift(struct sk_buff *tgt, struct sk_buff *skb, int shiftlen);
 void skb_scrub_packet(struct sk_buff *skb, bool xnet);
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 8c5197f..c5cefbe 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2125,6 +2125,91 @@ __wsum skb_copy_and_csum_bits(const struct sk_buff *skb, int offset,
 }
 EXPORT_SYMBOL(skb_copy_and_csum_bits);
 
+ /**
+ *	skb_zerocopy_headlen - Calculate headroom needed for skb_zerocopy()
+ *	@from: source buffer
+ *
+ *	Calculates the amount of linear headroom needed in the 'to' skb passed
+ *	into skb_zerocopy().
+ */
+unsigned int
+skb_zerocopy_headlen(const struct sk_buff *from)
+{
+	unsigned int hlen = 0;
+
+	if (!from->head_frag ||
+	    skb_headlen(from) < L1_CACHE_BYTES ||
+	    skb_shinfo(from)->nr_frags >= MAX_SKB_FRAGS)
+		hlen = skb_headlen(from);
+
+	if (skb_has_frag_list(from))
+		hlen = from->len;
+
+	return hlen;
+}
+EXPORT_SYMBOL_GPL(skb_zerocopy_headlen);
+
+/**
+ *	skb_zerocopy - Zero copy skb to skb
+ *	@to: destination buffer
+ *	@source: source buffer
+ *	@len: number of bytes to copy from source buffer
+ *	@hlen: size of linear headroom in destination buffer
+ *
+ *	Copies up to `len` bytes from `from` to `to` by creating references
+ *	to the frags in the source buffer.
+ *
+ *	The `hlen` as calculated by skb_zerocopy_headlen() specifies the
+ *	headroom in the `to` buffer.
+ */
+void
+skb_zerocopy(struct sk_buff *to, const struct sk_buff *from, int len, int hlen)
+{
+	int i, j = 0;
+	int plen = 0; /* length of skb->head fragment */
+	struct page *page;
+	unsigned int offset;
+
+	BUG_ON(!from->head_frag && !hlen);
+
+	/* dont bother with small payloads */
+	if (len <= skb_tailroom(to)) {
+		skb_copy_bits(from, 0, skb_put(to, len), len);
+		return;
+	}
+
+	if (hlen) {
+		skb_copy_bits(from, 0, skb_put(to, hlen), hlen);
+		len -= hlen;
+	} else {
+		plen = min_t(int, skb_headlen(from), len);
+		if (plen) {
+			page = virt_to_head_page(from->head);
+			offset = from->data - (unsigned char *)page_address(page);
+			__skb_fill_page_desc(to, 0, page, offset, plen);
+			get_page(page);
+			j = 1;
+			len -= plen;
+		}
+	}
+
+	to->truesize += len + plen;
+	to->len += len + plen;
+	to->data_len += len + plen;
+
+	for (i = 0; i < skb_shinfo(from)->nr_frags; i++) {
+		if (!len)
+			break;
+		skb_shinfo(to)->frags[j] = skb_shinfo(from)->frags[i];
+		skb_shinfo(to)->frags[j].size = min_t(int, skb_shinfo(to)->frags[j].size, len);
+		len -= skb_shinfo(to)->frags[j].size;
+		skb_frag_ref(to, j);
+		j++;
+	}
+	skb_shinfo(to)->nr_frags = j;
+}
+EXPORT_SYMBOL_GPL(skb_zerocopy);
+
 void skb_copy_and_csum_dev(const struct sk_buff *skb, u8 *to)
 {
 	__wsum csum;
diff --git a/net/netfilter/nfnetlink_queue_core.c b/net/netfilter/nfnetlink_queue_core.c
index 21258cf..615ee12 100644
--- a/net/netfilter/nfnetlink_queue_core.c
+++ b/net/netfilter/nfnetlink_queue_core.c
@@ -235,51 +235,6 @@ nfqnl_flush(struct nfqnl_instance *queue, nfqnl_cmpfn cmpfn, unsigned long data)
 	spin_unlock_bh(&queue->lock);
 }
 
-static void
-nfqnl_zcopy(struct sk_buff *to, const struct sk_buff *from, int len, int hlen)
-{
-	int i, j = 0;
-	int plen = 0; /* length of skb->head fragment */
-	struct page *page;
-	unsigned int offset;
-
-	/* dont bother with small payloads */
-	if (len <= skb_tailroom(to)) {
-		skb_copy_bits(from, 0, skb_put(to, len), len);
-		return;
-	}
-
-	if (hlen) {
-		skb_copy_bits(from, 0, skb_put(to, hlen), hlen);
-		len -= hlen;
-	} else {
-		plen = min_t(int, skb_headlen(from), len);
-		if (plen) {
-			page = virt_to_head_page(from->head);
-			offset = from->data - (unsigned char *)page_address(page);
-			__skb_fill_page_desc(to, 0, page, offset, plen);
-			get_page(page);
-			j = 1;
-			len -= plen;
-		}
-	}
-
-	to->truesize += len + plen;
-	to->len += len + plen;
-	to->data_len += len + plen;
-
-	for (i = 0; i < skb_shinfo(from)->nr_frags; i++) {
-		if (!len)
-			break;
-		skb_shinfo(to)->frags[j] = skb_shinfo(from)->frags[i];
-		skb_shinfo(to)->frags[j].size = min_t(int, skb_shinfo(to)->frags[j].size, len);
-		len -= skb_shinfo(to)->frags[j].size;
-		skb_frag_ref(to, j);
-		j++;
-	}
-	skb_shinfo(to)->nr_frags = j;
-}
-
 static int
 nfqnl_put_packet_info(struct sk_buff *nlskb, struct sk_buff *packet,
 		      bool csum_verify)
@@ -304,7 +259,7 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
 {
 	size_t size;
 	size_t data_len = 0, cap_len = 0;
-	int hlen = 0;
+	unsigned int hlen = 0;
 	struct sk_buff *skb;
 	struct nlattr *nla;
 	struct nfqnl_msg_packet_hdr *pmsg;
@@ -356,14 +311,8 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
 		if (data_len > entskb->len)
 			data_len = entskb->len;
 
-		if (!entskb->head_frag ||
-		    skb_headlen(entskb) < L1_CACHE_BYTES ||
-		    skb_shinfo(entskb)->nr_frags >= MAX_SKB_FRAGS)
-			hlen = skb_headlen(entskb);
-
-		if (skb_has_frag_list(entskb))
-			hlen = entskb->len;
-		hlen = min_t(int, data_len, hlen);
+		hlen = skb_zerocopy_headlen(entskb);
+		hlen = min_t(unsigned int, hlen, data_len);
 		size += sizeof(struct nlattr) + hlen;
 		cap_len = entskb->len;
 		break;
@@ -504,7 +453,7 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
 		nla->nla_type = NFQA_PAYLOAD;
 		nla->nla_len = nla_attr_size(data_len);
 
-		nfqnl_zcopy(skb, entskb, data_len, hlen);
+		skb_zerocopy(skb, entskb, data_len, hlen);
 	}
 
 	nlh->nlmsg_len = skb->len;
-- 
1.8.3.1

^ 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