Netdev List
 help / color / mirror / Atom feed
* Re: linux-next: Tree for Jan 15 (netfilter: nft_reject)
From: Randy Dunlap @ 2014-01-15 23:39 UTC (permalink / raw)
  To: Stephen Rothwell, linux-next
  Cc: linux-kernel, netdev@vger.kernel.org, netfilter-devel
In-Reply-To: <20140115185545.497c14923814b6ecaa540d9f@canb.auug.org.au>

On 01/14/2014 11:55 PM, Stephen Rothwell wrote:
> Hi all,
> 
> This tree fails (more than usual) the powerpc allyesconfig build.
> 
> Changes since 20140114:
> 

on x86_64:
# CONFIG_IPV6 is not enabled.

warning: (NF_TABLES_INET) selects NF_TABLES_IPV6 which has unmet direct dependencies (NET && INET && IPV6 && NETFILTER && NF_TABLES)
...
ERROR: "ip6_dst_hoplimit" [net/netfilter/nft_reject.ko] undefined!
ERROR: "ip6_route_output" [net/netfilter/nft_reject.ko] undefined!
ERROR: "nf_ip6_checksum" [net/netfilter/nft_reject.ko] undefined!



-- 
~Randy

^ permalink raw reply

* Re: [RFC PATCH net-next] etherdevice: Use ether_addr_copy to copy an Ethernet address
From: David Miller @ 2014-01-15 23:39 UTC (permalink / raw)
  To: joe; +Cc: netdev, linux-arm-kernel, linux-arch
In-Reply-To: <1389741527.24849.68.camel@joe-AO722>

From: Joe Perches <joe@perches.com>
Date: Tue, 14 Jan 2014 15:18:47 -0800

> Some systems can use the normally known u16 alignment of
> Ethernet addresses to save some code/text bytes and cycles.
> 
> This does not change currently emitted code on x86 by gcc 4.8.
> 
> Signed-off-by: Joe Perches <joe@perches.com>

This looks fine, in fact I'll apply it.

^ permalink raw reply

* Re: [PATCH net-next 0/2] net: mvneta: simple cleanups
From: David Miller @ 2014-01-15 23:41 UTC (permalink / raw)
  To: arno; +Cc: w, thomas.petazzoni, netdev, linux-arm-kernel
In-Reply-To: <cover.1389742334.git.arno@natisbad.org>

From: Arnaud Ebalard <arno@natisbad.org>
Date: Wed, 15 Jan 2014 00:45:31 +0100

> 
> Those two patches are intended for net-next. They apply on top of
> performance improvements patches from Willy for mvneta driver.
> They provide some simple cleanups for unused variables, function
> params or return values.
> 
> Arnaud Ebalard (2):
>   net: mvneta: mvneta_tx_done_gbe() cleanups
>   net: mvneta: make mvneta_txq_done() return void

These patches do not apply to net-next, please respin.

Thanks.

^ permalink raw reply

* Re: [BUG] eth_type_trans() can access stale data
From: David Miller @ 2014-01-15 23:48 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1389743541.31367.279.camel@edumazet-glaptop2.roam.corp.google.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 14 Jan 2014 15:52:21 -0800

> @@ -194,7 +194,8 @@ __be16 eth_type_trans(struct sk_buff *skb, struct net_device *dev)
>  	 *      layer. We look for FFFF which isn't a used 802.2 SSAP/DSAP. This
>  	 *      won't work for fault tolerant netware but does for the rest.
>  	 */
> -	if (unlikely(skb->len >= 2 && *(unsigned short *)(skb->data) == 0xFFFF))
> +	if (unlikely(skb_headlen(skb) >= 2 &&
> +		     *(unsigned short *)(skb->data) == 0xFFFF))
>  		return htons(ETH_P_802_3);

I think using skb_header_pointer() will essentially have the same cost,
why not use it instead?

Thanks.

^ permalink raw reply

* [Patch net-next] net_sched: act: remove capab from struct tc_action_ops
From: Cong Wang @ 2014-01-15 23:49 UTC (permalink / raw)
  To: netdev; +Cc: Cong Wang, Jamal Hadi Salim, David S. Miller

It is not actually implemented.

Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 include/net/act_api.h   | 2 --
 net/sched/act_csum.c    | 1 -
 net/sched/act_gact.c    | 1 -
 net/sched/act_ipt.c     | 2 --
 net/sched/act_mirred.c  | 1 -
 net/sched/act_nat.c     | 1 -
 net/sched/act_pedit.c   | 1 -
 net/sched/act_police.c  | 1 -
 net/sched/act_simple.c  | 1 -
 net/sched/act_skbedit.c | 1 -
 10 files changed, 12 deletions(-)

diff --git a/include/net/act_api.h b/include/net/act_api.h
index e171387..8ed9746 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -81,13 +81,11 @@ struct tc_action {
 	struct list_head	list;
 };
 
-#define TCA_CAP_NONE 0
 struct tc_action_ops {
 	struct list_head head;
 	struct tcf_hashinfo *hinfo;
 	char    kind[IFNAMSIZ];
 	__u32   type; /* TBD to match kind */
-	__u32 	capab;  /* capabilities includes 4 bit version */
 	struct module		*owner;
 	int     (*act)(struct sk_buff *, const struct tc_action *, struct tcf_result *);
 	int     (*dump)(struct sk_buff *, struct tc_action *, int, int);
diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c
index ee28e1c..9d5c1d3 100644
--- a/net/sched/act_csum.c
+++ b/net/sched/act_csum.c
@@ -572,7 +572,6 @@ static struct tc_action_ops act_csum_ops = {
 	.kind		= "csum",
 	.hinfo		= &csum_hash_info,
 	.type		= TCA_ACT_CSUM,
-	.capab		= TCA_CAP_NONE,
 	.owner		= THIS_MODULE,
 	.act		= tcf_csum,
 	.dump		= tcf_csum_dump,
diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
index 3133307..72c49de 100644
--- a/net/sched/act_gact.c
+++ b/net/sched/act_gact.c
@@ -194,7 +194,6 @@ static struct tc_action_ops act_gact_ops = {
 	.kind		=	"gact",
 	.hinfo		=	&gact_hash_info,
 	.type		=	TCA_ACT_GACT,
-	.capab		=	TCA_CAP_NONE,
 	.owner		=	THIS_MODULE,
 	.act		=	tcf_gact,
 	.dump		=	tcf_gact_dump,
diff --git a/net/sched/act_ipt.c b/net/sched/act_ipt.c
index bc9f498..67d701e 100644
--- a/net/sched/act_ipt.c
+++ b/net/sched/act_ipt.c
@@ -287,7 +287,6 @@ static struct tc_action_ops act_ipt_ops = {
 	.kind		=	"ipt",
 	.hinfo		=	&ipt_hash_info,
 	.type		=	TCA_ACT_IPT,
-	.capab		=	TCA_CAP_NONE,
 	.owner		=	THIS_MODULE,
 	.act		=	tcf_ipt,
 	.dump		=	tcf_ipt_dump,
@@ -299,7 +298,6 @@ static struct tc_action_ops act_xt_ops = {
 	.kind		=	"xt",
 	.hinfo		=	&ipt_hash_info,
 	.type		=	TCA_ACT_XT,
-	.capab		=	TCA_CAP_NONE,
 	.owner		=	THIS_MODULE,
 	.act		=	tcf_ipt,
 	.dump		=	tcf_ipt_dump,
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 5d05b57..376234e 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -257,7 +257,6 @@ static struct tc_action_ops act_mirred_ops = {
 	.kind		=	"mirred",
 	.hinfo		=	&mirred_hash_info,
 	.type		=	TCA_ACT_MIRRED,
-	.capab		=	TCA_CAP_NONE,
 	.owner		=	THIS_MODULE,
 	.act		=	tcf_mirred,
 	.dump		=	tcf_mirred_dump,
diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c
index a49fa23..46e1aa3 100644
--- a/net/sched/act_nat.c
+++ b/net/sched/act_nat.c
@@ -296,7 +296,6 @@ static struct tc_action_ops act_nat_ops = {
 	.kind		=	"nat",
 	.hinfo		=	&nat_hash_info,
 	.type		=	TCA_ACT_NAT,
-	.capab		=	TCA_CAP_NONE,
 	.owner		=	THIS_MODULE,
 	.act		=	tcf_nat,
 	.dump		=	tcf_nat_dump,
diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index f361e4e..109265d 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -233,7 +233,6 @@ static struct tc_action_ops act_pedit_ops = {
 	.kind		=	"pedit",
 	.hinfo		=	&pedit_hash_info,
 	.type		=	TCA_ACT_PEDIT,
-	.capab		=	TCA_CAP_NONE,
 	.owner		=	THIS_MODULE,
 	.act		=	tcf_pedit,
 	.dump		=	tcf_pedit_dump,
diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index a719fdf..7fd0993 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -385,7 +385,6 @@ static struct tc_action_ops act_police_ops = {
 	.kind		=	"police",
 	.hinfo		=	&police_hash_info,
 	.type		=	TCA_ID_POLICE,
-	.capab		=	TCA_CAP_NONE,
 	.owner		=	THIS_MODULE,
 	.act		=	tcf_act_police,
 	.dump		=	tcf_act_police_dump,
diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c
index f7d5406..92236da 100644
--- a/net/sched/act_simple.c
+++ b/net/sched/act_simple.c
@@ -190,7 +190,6 @@ static struct tc_action_ops act_simp_ops = {
 	.kind		=	"simple",
 	.hinfo		=	&simp_hash_info,
 	.type		=	TCA_ACT_SIMP,
-	.capab		=	TCA_CAP_NONE,
 	.owner		=	THIS_MODULE,
 	.act		=	tcf_simp,
 	.dump		=	tcf_simp_dump,
diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index 74af461..c36b520 100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -189,7 +189,6 @@ static struct tc_action_ops act_skbedit_ops = {
 	.kind		=	"skbedit",
 	.hinfo		=	&skbedit_hash_info,
 	.type		=	TCA_ACT_SKBEDIT,
-	.capab		=	TCA_CAP_NONE,
 	.owner		=	THIS_MODULE,
 	.act		=	tcf_skbedit,
 	.dump		=	tcf_skbedit_dump,
-- 
1.8.3.1

^ permalink raw reply related

* Re: throughput problems with realtek
From: Francois Romieu @ 2014-01-15 23:51 UTC (permalink / raw)
  To: Dmitry Kasatkin; +Cc: Rick Jones, nic_swsd, netdev, l.moiseichuk
In-Reply-To: <CACE9dm92Wu=XwCChXJjjsz=FEWAgJW-1iLRw5-d4c5ZTCYVe3w@mail.gmail.com>

Dmitry Kasatkin <dmitry.kasatkin@gmail.com> :
[...]
> I do not see any link speed changes... it stays the same...
> The same problem is visible on absolutely different computers.

No netdev watchdog message either ?

A serving 8168c does not seem to fluctuate (3.12.5, Intel 82578 client).
My hardware is a bit old though. Please send XID message from the r8169
driver. It should be seen in dmesg.

Thanks.

-- 
Ueimor

^ permalink raw reply

* Re: [PATCH net-next 00/11] be2net: patch set
From: David Miller @ 2014-01-15 23:52 UTC (permalink / raw)
  To: sathya.perla; +Cc: netdev
In-Reply-To: <1389772421-24925-1-git-send-email-sathya.perla@emulex.com>

From: Sathya Perla <sathya.perla@emulex.com>
Date: Wed, 15 Jan 2014 13:23:30 +0530

> The following patch set is best suited for net-next as it
> contains code-cleanup, support for newer versions of FW cmds and
> a few minor fixes. Please apply. Thanks!

Series applied, thanks.

^ permalink raw reply

* Re: [PATCH net-next v3] ipv6: move IPV6_TCLASS_SHIFT into ipv6.h and define a helper
From: David Miller @ 2014-01-15 23:54 UTC (permalink / raw)
  To: roy.qing.li; +Cc: netdev
In-Reply-To: <1389776610-19703-1-git-send-email-roy.qing.li@gmail.com>

From: roy.qing.li@gmail.com
Date: Wed, 15 Jan 2014 17:03:30 +0800

> From: Li RongQing <roy.qing.li@gmail.com>
> 
> Two places defined IPV6_TCLASS_SHIFT, so we should move it into ipv6.h,
> and use this macro as possible. And define ip6_tclass helper to return
> tclass
> 
> Signed-off-by: Li RongQing <roy.qing.li@gmail.com>

Applied, thank you.

^ permalink raw reply

* Re: [PATCH net-next v4 1/9] xen-netback: Introduce TX grant map definitions
From: Wei Liu @ 2014-01-16  0:00 UTC (permalink / raw)
  To: Zoltan Kiss
  Cc: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
	jonathan.davies
In-Reply-To: <1389731995-9887-2-git-send-email-zoltan.kiss@citrix.com>

There is a stray blank line change in xenvif_tx_create_gop. (I removed
that part too early and didn't bother to paste it back...)

On Tue, Jan 14, 2014 at 08:39:47PM +0000, Zoltan Kiss wrote:
[...]
> +void xenvif_idx_unmap(struct xenvif *vif, u16 pending_idx)
> +{
> +	int ret;
> +	struct gnttab_unmap_grant_ref tx_unmap_op;
> +
> +	if (vif->grant_tx_handle[pending_idx] == NETBACK_INVALID_HANDLE) {
> +		netdev_err(vif->dev,
> +			   "Trying to unmap invalid handle! pending_idx: %x\n",
> +			   pending_idx);
> +		return;
> +	}
> +	gnttab_set_unmap_op(&tx_unmap_op,
> +			    idx_to_kaddr(vif, pending_idx),
> +			    GNTMAP_host_map,
> +			    vif->grant_tx_handle[pending_idx]);
> +	ret = gnttab_unmap_refs(&tx_unmap_op,
> +				&vif->mmap_pages[pending_idx],
> +				1);
> +
> +	ret = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref,
> +					&tx_unmap_op,
> +					1);

As you said in your other email, this should be removed. :-)

> +	BUG_ON(ret);
> +	vif->grant_tx_handle[pending_idx] = NETBACK_INVALID_HANDLE;
> +}
>  
>  static void make_tx_response(struct xenvif *vif,
>  			     struct xen_netif_tx_request *txp,
> @@ -1738,6 +1879,14 @@ static inline int tx_work_todo(struct xenvif *vif)
>  	return 0;
>  }
>  
> +static inline bool tx_dealloc_work_todo(struct xenvif *vif)
> +{
> +	if (vif->dealloc_cons != vif->dealloc_prod)
> +		return true;
> +
> +	return false;

This can be simplified as
  return vif->dealloc_cons != vif->dealloc_prod;

Wei.

^ permalink raw reply

* Re: [PATCH net-next v4 2/9] xen-netback: Change TX path from grant copy to mapping
From: Wei Liu @ 2014-01-16  0:01 UTC (permalink / raw)
  To: Zoltan Kiss
  Cc: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
	jonathan.davies
In-Reply-To: <1389731995-9887-3-git-send-email-zoltan.kiss@citrix.com>

On Tue, Jan 14, 2014 at 08:39:48PM +0000, Zoltan Kiss wrote:
> This patch changes the grant copy on the TX patch to grant mapping
> 
> v2:
> - delete branch for handling fragmented packets fit PKT_PROT_LEN sized first
>   request
> - mark the effect of using ballooned pages in a comment
> - place setting of skb_shinfo(skb)->tx_flags |= SKBTX_DEV_ZEROCOPY right
>   before netif_receive_skb, and mark the importance of it
> - grab dealloc_lock before __napi_complete to avoid contention with the
>   callback's napi_schedule
> - handle fragmented packets where first request < PKT_PROT_LEN
> - fix up error path when checksum_setup failed
> - check before teardown for pending grants, and start complain if they are
>   there after 10 second
> 
> v3:
> - delete a surplus checking from tx_action
> - remove stray line
> - squash xenvif_idx_unmap changes into the first patch
> - init spinlocks
> - call map hypercall directly instead of gnttab_map_refs()
> - fix unmapping timeout in xenvif_free()
> 
> v4:
> - fix indentations and comments
> - handle errors of set_phys_to_machine

There's no call to set_phys_to_machine in this patch. Did I miss
something?

> - go back to gnttab_map_refs instead of direct hypercall. Now we rely on the
>   modified API
> 
> Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
> ---
>  drivers/net/xen-netback/interface.c |   60 +++++++-
>  drivers/net/xen-netback/netback.c   |  256 ++++++++++++++---------------------
>  2 files changed, 159 insertions(+), 157 deletions(-)
> 
> diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
> index a7855b3..1e0bf71 100644
> --- a/drivers/net/xen-netback/interface.c
> +++ b/drivers/net/xen-netback/interface.c
> @@ -123,7 +123,9 @@ static int xenvif_start_xmit(struct sk_buff *skb, struct net_device *dev)
>  	BUG_ON(skb->dev != dev);
>  
>  	/* Drop the packet if vif is not ready */
> -	if (vif->task == NULL || !xenvif_schedulable(vif))
> +	if (vif->task == NULL ||
> +	    vif->dealloc_task == NULL ||
> +	    !xenvif_schedulable(vif))
>  		goto drop;
>  
>  	/* At best we'll need one slot for the header and one for each
> @@ -345,8 +347,26 @@ struct xenvif *xenvif_alloc(struct device *parent, domid_t domid,

At the beginning of the function there's BUG_ON checks for vif->task. I
would suggest you do the same for vif->dealloc_task, just to be
consistent.

>  	vif->pending_prod = MAX_PENDING_REQS;
>  	for (i = 0; i < MAX_PENDING_REQS; i++)
>  		vif->pending_ring[i] = i;
> -	for (i = 0; i < MAX_PENDING_REQS; i++)
> -		vif->mmap_pages[i] = NULL;
> +	spin_lock_init(&vif->dealloc_lock);
> +	spin_lock_init(&vif->response_lock);
> +	/* If ballooning is disabled, this will consume real memory, so you
> +	 * better enable it. The long term solution would be to use just a
> +	 * bunch of valid page descriptors, without dependency on ballooning
> +	 */
> +	err = alloc_xenballooned_pages(MAX_PENDING_REQS,
> +				       vif->mmap_pages,
> +				       false);
> +	if (err) {
> +		netdev_err(dev, "Could not reserve mmap_pages\n");
> +		return NULL;
> +	}
> +	for (i = 0; i < MAX_PENDING_REQS; i++) {
> +		vif->pending_tx_info[i].callback_struct = (struct ubuf_info)
> +			{ .callback = xenvif_zerocopy_callback,
> +			  .ctx = NULL,
> +			  .desc = i };
> +		vif->grant_tx_handle[i] = NETBACK_INVALID_HANDLE;
> +	}
>  
>  	/*
>  	 * Initialise a dummy MAC address. We choose the numerically
> @@ -390,6 +410,7 @@ int xenvif_connect(struct xenvif *vif, unsigned long tx_ring_ref,
>  		goto err;
>  
>  	init_waitqueue_head(&vif->wq);
> +	init_waitqueue_head(&vif->dealloc_wq);
>  
>  	if (tx_evtchn == rx_evtchn) {
>  		/* feature-split-event-channels == 0 */
> @@ -431,6 +452,16 @@ int xenvif_connect(struct xenvif *vif, unsigned long tx_ring_ref,
>  		goto err_rx_unbind;
>  	}
>  
> +	vif->dealloc_task = kthread_create(xenvif_dealloc_kthread,
> +					   (void *)vif,
> +					   "%s-dealloc",
> +					   vif->dev->name);
> +	if (IS_ERR(vif->dealloc_task)) {
> +		pr_warn("Could not allocate kthread for %s\n", vif->dev->name);
> +		err = PTR_ERR(vif->dealloc_task);
> +		goto err_rx_unbind;
> +	}
> +
>  	vif->task = task;

Please move this line before the above hunk. Don't separate it from
corresponding kthread_create.

Last but not least, though I've looked at this patch for several rounds
and and the basic logic looks correct to me, I would like it to go
through XenRT tests if possible -- eye inspection is error-prone to such
complicated change. (If I'm not mistaken you once told me you've done
regression tests already. That would be neat!)

Wei.

^ permalink raw reply

* Re: [PATCH net-next v4 3/9] xen-netback: Remove old TX grant copy definitons and fix indentations
From: Wei Liu @ 2014-01-16  0:02 UTC (permalink / raw)
  To: Zoltan Kiss
  Cc: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
	jonathan.davies
In-Reply-To: <1389731995-9887-4-git-send-email-zoltan.kiss@citrix.com>

On Tue, Jan 14, 2014 at 08:39:49PM +0000, Zoltan Kiss wrote:
> These became obsolate with grant mapping. I've left intentionally the
               ^ obsolete
> indentations in this way, to improve readability of previous patches.
> 

^ permalink raw reply

* Re: [PATCH net-next v4 6/9] xen-netback: Handle guests with too many frags
From: Wei Liu @ 2014-01-16  0:03 UTC (permalink / raw)
  To: Zoltan Kiss
  Cc: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
	jonathan.davies
In-Reply-To: <1389731995-9887-7-git-send-email-zoltan.kiss@citrix.com>

On Tue, Jan 14, 2014 at 08:39:52PM +0000, Zoltan Kiss wrote:
[...]
>  	/* Skip first skb fragment if it is on same page as header fragment. */
> @@ -832,6 +851,29 @@ static struct gnttab_map_grant_ref *xenvif_get_requests(struct xenvif *vif,
>  
>  	BUG_ON(shinfo->nr_frags > MAX_SKB_FRAGS);
>  
> +	if (frag_overflow) {
> +		struct sk_buff *nskb = xenvif_alloc_skb(0);
> +		if (unlikely(nskb == NULL)) {
> +			netdev_err(vif->dev,
> +				   "Can't allocate the frag_list skb.\n");

This, and other occurences of netdev_* logs need to be rate limit.
Otherwise you risk flooding kernel log when system is under memory
pressure.

> +			return NULL;
> +		}
> +
> +		shinfo = skb_shinfo(nskb);
> +		frags = shinfo->frags;
> +
> +		for (shinfo->nr_frags = 0; shinfo->nr_frags < frag_overflow;
> +		     shinfo->nr_frags++, txp++, gop++) {
> +			index = pending_index(vif->pending_cons++);
> +			pending_idx = vif->pending_ring[index];
> +			xenvif_tx_create_gop(vif, pending_idx, txp, gop);
> +			frag_set_pending_idx(&frags[shinfo->nr_frags],
> +					     pending_idx);
> +		}
> +
> +		skb_shinfo(skb)->frag_list = nskb;
> +	}
> +
>  	return gop;
>  }
>  
[...]
> @@ -1537,6 +1613,32 @@ static int xenvif_tx_submit(struct xenvif *vif)
>  				  pending_idx :
>  				  INVALID_PENDING_IDX);
>  
> +		if (skb_shinfo(skb)->frag_list) {
> +			nskb = skb_shinfo(skb)->frag_list;
> +			xenvif_fill_frags(vif, nskb, INVALID_PENDING_IDX);
> +			skb->len += nskb->len;
> +			skb->data_len += nskb->len;
> +			skb->truesize += nskb->truesize;
> +			skb_shinfo(skb)->tx_flags |= SKBTX_DEV_ZEROCOPY;
> +			skb_shinfo(nskb)->tx_flags |= SKBTX_DEV_ZEROCOPY;
> +			vif->tx_zerocopy_sent += 2;
> +			nskb = skb;
> +
> +			skb = skb_copy_expand(skb,
> +					      0,
> +					      0,
> +					      GFP_ATOMIC | __GFP_NOWARN);
> +			if (!skb) {
> +				netdev_dbg(vif->dev,
> +					   "Can't consolidate skb with too many fragments\n");

Rate limit.

> +				if (skb_shinfo(nskb)->destructor_arg)
> +					skb_shinfo(nskb)->tx_flags |=
> +						SKBTX_DEV_ZEROCOPY;

Why is this needed? nskb is the saved pointer to original skb, which has
already had SKBTX_DEV_ZEROCOPY in tx_flags. Did I miss something?

Wei.

> +				kfree_skb(nskb);
> +				continue;
> +			}
> +			skb_shinfo(skb)->destructor_arg = NULL;
> +		}
>  		if (skb_is_nonlinear(skb) && skb_headlen(skb) < PKT_PROT_LEN) {
>  			int target = min_t(int, skb->len, PKT_PROT_LEN);
>  			__pskb_pull_tail(skb, target - skb_headlen(skb));
> @@ -1590,6 +1692,9 @@ static int xenvif_tx_submit(struct xenvif *vif)
>  		}
>  
>  		netif_receive_skb(skb);
> +
> +		if (nskb)
> +			kfree_skb(nskb);
>  	}
>  
>  	return work_done;

^ permalink raw reply

* Re: [PATCH net-next v4 8/9] xen-netback: Timeout packets in RX path
From: Wei Liu @ 2014-01-16  0:03 UTC (permalink / raw)
  To: Zoltan Kiss
  Cc: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
	jonathan.davies
In-Reply-To: <1389731995-9887-9-git-send-email-zoltan.kiss@citrix.com>

On Tue, Jan 14, 2014 at 08:39:54PM +0000, Zoltan Kiss wrote:
[...]
> diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
> index 109c29f..d1cd8ce 100644
> --- a/drivers/net/xen-netback/common.h
> +++ b/drivers/net/xen-netback/common.h
> @@ -129,6 +129,9 @@ struct xenvif {
>  	struct xen_netif_rx_back_ring rx;
>  	struct sk_buff_head rx_queue;
>  	RING_IDX rx_last_skb_slots;

Hmm... You seemed to mix your other patch with this series. :-)

> +	bool rx_queue_purge;
> +
> +	struct timer_list wake_queue;
>  
>  	/* This array is allocated seperately as it is large */
>  	struct gnttab_copy *grant_copy_op;
> @@ -225,4 +228,7 @@ void xenvif_idx_unmap(struct xenvif *vif, u16 pending_idx);
>  
>  extern bool separate_tx_rx_irq;
>  
[...]
> @@ -559,7 +579,7 @@ void xenvif_free(struct xenvif *vif)
>  		if (vif->grant_tx_handle[i] != NETBACK_INVALID_HANDLE) {
>  			unmap_timeout++;
>  			schedule_timeout(msecs_to_jiffies(1000));
> -			if (unmap_timeout > 9 &&
> +			if (unmap_timeout > ((rx_drain_timeout_msecs/1000) * DIV_ROUND_UP(XENVIF_QUEUE_LENGTH, (XEN_NETIF_RX_RING_SIZE / MAX_SKB_FRAGS))) &&

This line is really too long. And what's the rationale behind this long
expression?

Wei.

^ permalink raw reply

* Re: [RFC PATCH net-next] etherdevice: Use ether_addr_copy to copy an Ethernet address
From: Joe Perches @ 2014-01-16  0:07 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-arm-kernel, linux-arch
In-Reply-To: <20140115.153951.743060257410154565.davem@davemloft.net>

On Wed, 2014-01-15 at 15:39 -0800, David Miller wrote:
> From: Joe Perches <joe@perches.com>
> Date: Tue, 14 Jan 2014 15:18:47 -0800
> 
> > Some systems can use the normally known u16 alignment of
> > Ethernet addresses to save some code/text bytes and cycles.
> > 
> > This does not change currently emitted code on x86 by gcc 4.8.
> > 
> > Signed-off-by: Joe Perches <joe@perches.com>
> 
> This looks fine, in fact I'll apply it.

OK, good.

There are a couple thousand memcpy(foo, bar, ETH_ALEN)
in the kernel tree that could be converted.

Some will have a small performance improvement for
arm and powerpc.

If you want the ones for net-next net/ now (but not
for batman-adv, that maybe could use a new function like
ether_addr_copy_unaligned) here's a changestat.

Otherwise, I'll wait for the next cycle.

(done via coccinelle)

 net/8021q/vlan.c                          |  2 +-
 net/8021q/vlan_dev.c                      |  6 ++--
 net/appletalk/aarp.c                      | 10 +++---
 net/atm/lec.c                             |  9 ++---
 net/atm/mpc.c                             |  2 +-
 net/bluetooth/bnep/core.c                 | 19 +++++-----
 net/bluetooth/bnep/netdev.c               | 14 ++++----
 net/bridge/br_device.c                    |  4 +--
 net/bridge/br_fdb.c                       |  4 +--
 net/bridge/br_multicast.c                 |  4 +--
 net/bridge/br_netfilter.c                 |  2 +-
 net/bridge/br_stp_if.c                    | 10 +++---
 net/bridge/netfilter/ebt_among.c          |  2 +-
 net/bridge/netfilter/ebt_dnat.c           |  2 +-
 net/bridge/netfilter/ebt_redirect.c       |  6 ++--
 net/bridge/netfilter/ebt_snat.c           |  2 +-
 net/caif/caif_usb.c                       |  4 +--
 net/core/netpoll.c                        |  4 +--
 net/core/pktgen.c                         |  8 ++---
 net/decnet/dn_dev.c                       |  2 +-
 net/decnet/dn_neigh.c                     |  6 ++--
 net/decnet/dn_route.c                     |  6 ++--
 net/dsa/slave.c                           |  2 +-
 net/ethernet/eth.c                        | 18 +++++-----
 net/hsr/hsr_device.c                      |  8 ++---
 net/hsr/hsr_framereg.c                    | 21 +++++------
 net/hsr/hsr_main.c                        |  4 +--
 net/ipv4/netfilter/ipt_CLUSTERIP.c        |  2 +-
 net/mac80211/agg-rx.c                     | 10 +++---
 net/mac80211/agg-tx.c                     | 18 +++++-----
 net/mac80211/cfg.c                        | 34 +++++++++---------
 net/mac80211/debugfs_netdev.c             | 12 +++----
 net/mac80211/ht.c                         | 16 ++++-----
 net/mac80211/ibss.c                       | 14 ++++----
 net/mac80211/iface.c                      | 27 +++++++-------
 net/mac80211/mesh.c                       | 30 ++++++++--------
 net/mac80211/mesh_hwmp.c                  | 32 ++++++++---------
 net/mac80211/mesh_pathtbl.c               | 20 +++++------
 net/mac80211/mesh_plink.c                 |  6 ++--
 net/mac80211/mesh_ps.c                    |  2 +-
 net/mac80211/mlme.c                       | 32 ++++++++---------
 net/mac80211/rx.c                         | 14 ++++----
 net/mac80211/spectmgmt.c                  |  6 ++--
 net/mac80211/sta_info.c                   |  8 ++---
 net/mac80211/tx.c                         | 60 +++++++++++++++----------------
 net/mac80211/util.c                       | 22 ++++++------
 net/mac80211/wpa.c                        |  4 +--
 net/netfilter/ipset/ip_set_bitmap_ipmac.c |  6 ++--
 net/openvswitch/actions.c                 |  4 +--
 net/openvswitch/flow.c                    | 16 ++++-----
 net/openvswitch/flow_netlink.c            | 14 ++++----
 net/tipc/eth_media.c                      |  2 +-
 net/wireless/core.c                       |  2 +-
 net/wireless/ibss.c                       | 11 +++---
 net/wireless/lib80211_crypt_ccmp.c        |  4 +--
 net/wireless/lib80211_crypt_tkip.c        | 18 +++++-----
 net/wireless/mlme.c                       |  2 +-
 net/wireless/nl80211.c                    |  6 ++--
 net/wireless/scan.c                       |  6 ++--
 net/wireless/sme.c                        | 18 +++++-----
 net/wireless/util.c                       | 42 +++++++++++-----------
 net/wireless/wext-compat.c                |  8 ++---
 net/wireless/wext-sme.c                   |  5 +--
 net/wireless/wext-spy.c                   |  8 ++---
 64 files changed, 365 insertions(+), 357 deletions(-)

^ permalink raw reply

* Re: [RFC] sysfs_rename_link() and its usage
From: Veaceslav Falico @ 2014-01-16  0:11 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: Tejun Heo, Greg KH, linux-kernel, netdev
In-Reply-To: <87fvoo25gj.fsf@xmission.com>

On Wed, Jan 15, 2014 at 03:25:16PM -0800, Eric W. Biederman wrote:
>Tejun Heo <tj@kernel.org> writes:
>
>> Hey, Veaceslav, Eric.

Hi Tejun, Eric,

>>
>> On Tue, Jan 14, 2014 at 05:35:23PM -0800, Eric W. Biederman wrote:
>>> >>> >>This works like a charm. However, if I want to use (obviously, with the
>>> >>> >>symlink present):
>>> >>> >>
>>> >>> >>sysfs_rename_link(&(a->dev.kobj), &(b->dev.kobj), oldname, newname);
>>> >>> >
>>> >>> >You forgot the namespace option to this call, what kernel version are
>>> >>> >you using here?
>>> >>>
>>> >>> It's git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next ,
>>> >>> 3.13-rc6 with some networking patches on top of it.
>>
>> Does this work on 3.12?  How about Greg's driver-core-next?  Do you
>> have a minimal test case that I can use to reproduce the issue?

Sorry for the latency in responses, I'll update once I'll manage to test it
on those.

...snip...
>> Veaceslav, please confirm whether the issue is reproducible w/ v3.12.
>
>Anyway since a symlink living in a different namespace from it's target
>is just nonsense this (only compile tested) patch should fix the issue,
>and make sysfs_rename_link usable for people without a masters degree in
>sysfs again.

It's still there :-(. I've used your patch and added my small addition[1] to
test the sysfs_rename_link() (on top of net-next, 3.13-rc7), however the
issue is still there:

[   79.038340] net bond0: renaming to bondbla
[   79.038380] ------------[ cut here ]------------
[   79.038411] WARNING: CPU: 1 PID: 5318 at fs/sysfs/dir.c:618
sysfs_find_dirent+0x84/0x110()
[   79.038449] sysfs: ns invalid in 'bridge0' for 'lower_bond0'
...snip...
[   79.038877]  [<ffffffff810ae826>] warn_slowpath_fmt+0x46/0x50
[   79.038903]  [<ffffffff812ba890>] ? sysfs_get_dirent_ns+0x30/0x80
[   79.038930]  [<ffffffff812b97c4>] sysfs_find_dirent+0x84/0x110
[   79.038957]  [<ffffffff812ba89e>] sysfs_get_dirent_ns+0x3e/0x80
[   79.038983]  [<ffffffff812baf87>] sysfs_rename_link+0x57/0xe0
[   79.039030]  [<ffffffff81689e72>] netdev_adjacent_rename_links+0xa2/0x160

The current scheme (sysfs_remove_link() + sysfs_add_link()) works perfectly
well without any namespaces. I'll dig into it once I have some spare time,
it's not at all critical.

[1]: the patch (I've included your patch too, just in case):

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 67b180d..0c9377a 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1825,9 +1825,8 @@ int device_rename(struct device *dev, const char *new_name)
  	}
  
  	if (dev->class) {
-		error = sysfs_rename_link_ns(&dev->class->p->subsys.kobj,
-					     kobj, old_device_name,
-					     new_name, kobject_namespace(kobj));
+		error = sysfs_rename_link(&dev->class->p->subsys.kobj,
+					  kobj, old_device_name, new_name);
  		if (error)
  			goto out;
  	}
diff --git a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
index 3ae3f1b..651444a 100644
--- a/fs/sysfs/symlink.c
+++ b/fs/sysfs/symlink.c
@@ -194,12 +194,10 @@ EXPORT_SYMBOL_GPL(sysfs_remove_link);
   *	@targ:	object we're pointing to.
   *	@old:	previous name of the symlink.
   *	@new:	new name of the symlink.
- *	@new_ns: new namespace of the symlink.
- *
   *	A helper function for the common rename symlink idiom.
   */
-int sysfs_rename_link_ns(struct kobject *kobj, struct kobject *targ,
-			 const char *old, const char *new, const void *new_ns)
+int sysfs_rename_link(struct kobject *kobj, struct kobject *targ,
+		      const char *old, const char *new)
  {
  	struct sysfs_dirent *parent_sd, *sd = NULL;
  	const void *old_ns = NULL;
@@ -224,13 +222,13 @@ int sysfs_rename_link_ns(struct kobject *kobj, struct kobject *targ,
  	if (sd->s_symlink.target_sd->s_dir.kobj != targ)
  		goto out;
  
-	result = sysfs_rename(sd, parent_sd, new, new_ns);
+	result = sysfs_rename(sd, parent_sd, new, kobject_namespace(targ));
  
  out:
  	sysfs_put(sd);
  	return result;
  }
-EXPORT_SYMBOL_GPL(sysfs_rename_link_ns);
+EXPORT_SYMBOL_GPL(sysfs_rename_link);
  
  static int sysfs_get_target_path(struct sysfs_dirent *parent_sd,
  				 struct sysfs_dirent *target_sd, char *path)
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index 6695040..093d992 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -213,9 +213,8 @@ int __must_check sysfs_create_link_nowarn(struct kobject *kobj,
  					  const char *name);
  void sysfs_remove_link(struct kobject *kobj, const char *name);
  
-int sysfs_rename_link_ns(struct kobject *kobj, struct kobject *target,
-			 const char *old_name, const char *new_name,
-			 const void *new_ns);
+int sysfs_rename_link(struct kobject *kobj, struct kobject *target,
+		      const char *old_name, const char *new_name);
  
  void sysfs_delete_link(struct kobject *dir, struct kobject *targ,
  			const char *name);
@@ -341,9 +340,8 @@ static inline void sysfs_remove_link(struct kobject *kobj, const char *name)
  {
  }
  
-static inline int sysfs_rename_link_ns(struct kobject *k, struct kobject *t,
-				       const char *old_name,
-				       const char *new_name, const void *ns)
+static inline int sysfs_rename_link(struct kobject *k, struct kobject *t,
+				    const char *old_name, const char *new_name)
  {
  	return 0;
  }
@@ -455,12 +453,6 @@ static inline void sysfs_remove_file(struct kobject *kobj,
  	return sysfs_remove_file_ns(kobj, attr, NULL);
  }
  
-static inline int sysfs_rename_link(struct kobject *kobj, struct kobject *target,
-				    const char *old_name, const char *new_name)
-{
-	return sysfs_rename_link_ns(kobj, target, old_name, new_name, NULL);
-}
-
  static inline struct sysfs_dirent *
  sysfs_get_dirent(struct sysfs_dirent *parent_sd, const unsigned char *name)
  {
diff --git a/net/core/dev.c b/net/core/dev.c
index 9957557..5d24d8e 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5005,19 +5005,20 @@ EXPORT_SYMBOL(netdev_upper_dev_unlink);
  void netdev_adjacent_rename_links(struct net_device *dev, char *oldname)
  {
  	struct netdev_adjacent *iter;
+	char old_linkname[IFNAMSIZ+7], new_linkname[IFNAMSIZ+7];
  
  	list_for_each_entry(iter, &dev->adj_list.upper, list) {
-		netdev_adjacent_sysfs_del(iter->dev, oldname,
-					  &iter->dev->adj_list.lower);
-		netdev_adjacent_sysfs_add(iter->dev, dev,
-					  &iter->dev->adj_list.lower);
+		sprintf(old_linkname, "lower_%s", oldname);
+		sprintf(new_linkname, "lower_%s", dev->name);
+		sysfs_rename_link(&(iter->dev->dev.kobj), &(dev->dev.kobj),
+				  old_linkname, new_linkname);
  	}
  
  	list_for_each_entry(iter, &dev->adj_list.lower, list) {
-		netdev_adjacent_sysfs_del(iter->dev, oldname,
-					  &iter->dev->adj_list.upper);
-		netdev_adjacent_sysfs_add(iter->dev, dev,
-					  &iter->dev->adj_list.upper);
+		sprintf(old_linkname, "upper_%s", oldname);
+		sprintf(new_linkname, "upper_%s", dev->name);
+		sysfs_rename_link(&(iter->dev->dev.kobj), &(dev->dev.kobj),
+				  old_linkname, new_linkname);
  	}
  }
  

^ permalink raw reply related

* Re: [PATCH v5 net-next 2/4] sh_eth: Add support for r7s72100
From: Simon Horman @ 2014-01-16  0:14 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: David S. Miller, netdev, linux-sh, linux-arm-kernel, Magnus Damm
In-Reply-To: <52D70B03.9040506@cogentembedded.com>

On Thu, Jan 16, 2014 at 01:26:11AM +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 01/15/2014 09:12 AM, Simon Horman wrote:
> 
> >This is a fast ethernet controller.
> 
>    I have to say it's not exact enough patch description: R7S72100
> is not Ethernet controller itself, it's a SoC containing the
> Ethernet controller.

I will update the text to the following:

The r7s72100 SoC includes a fast ethernet controller.

> 
> >Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> 
> >---
> >v5
> >* As suggested by Sergei Shtylyov
> >   - Do not use sh_eth_chip_reset_r8a7740 as it accesses non-existent
> >     RMII registers. Instead use sh_eth_chip_reset.
> >   - Do not use sh_eth_set_rate_gether as it accesses non-existent registers.
> >   - Do not use reserved LCHNG bit of ECSR
> >   - Do not use reserved LCHNGIP bit of ECSIPR
> >   - Document that R8A779x also needs a 16 bit shift of the RFS bits
> >   - Do not document that the R7S72100 has GECMR, it does not
> 
>   The above change list was moved from v2 section and doesn't match
> the real changes done in v5. ;-)

I'll fix that up so that when its automatically discarded by git
it can rest in peace in /dev/null.

> 
> >v4
> >* As requested by David Miller
> >   - Use a boolean for the return value of sh_eth_is_rz_fast_ether()
> >   - Correct coding style in sh_eth_get_stats()
> 
> >v3
> >* No change
> 
> >v2
> >* As suggested by Magnus Damm and Sergei Shtylyov
> >   - r7s72100 ethernet is not gigabit so do not refer to it as such
> 
> >* As suggested by Magnus Damm
> >   - As RZ specific register layout rather than using the gigabit layout
> >     which includes registers that do not exist on this chip.
> >---
> >  drivers/net/ethernet/renesas/sh_eth.c | 126 ++++++++++++++++++++++++++++++++--
> >  drivers/net/ethernet/renesas/sh_eth.h |   3 +-
> >  2 files changed, 121 insertions(+), 8 deletions(-)
> 
> >diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> >index 4f5cfad..a7a0555 100644
> >--- a/drivers/net/ethernet/renesas/sh_eth.c
> >+++ b/drivers/net/ethernet/renesas/sh_eth.c
> >@@ -190,6 +190,64 @@ static const u16 sh_eth_offset_fast_rcar[SH_ETH_MAX_REGISTER_OFFSET] = {
> >  	[TRIMD]		= 0x027c,
> >  };
> >
> >+static const u16 sh_eth_offset_fast_rz[SH_ETH_MAX_REGISTER_OFFSET] = {
> 
>    Shouldn't this map precede R-Car one since this SoC is newer the
> same way you've reordered *enum* values, etc.? Sorry for not
> noticing in the previous review...

I will move it, thanks.

> [...]
> >+	[ARSTR]		= 0x0000,
> >+	[TSU_CTRST]	= 0x0004,
> >+	[TSU_VTAG0]	= 0x0058,
> >+	[TSU_ADSBSY]	= 0x0060,
> >+	[TSU_TEN]	= 0x0064,
> >+	[TXNLCR0]	= 0x0080,
> >+	[TXALCR0]	= 0x0084,
> >+	[RXNLCR0]	= 0x0088,
> >+	[RXALCR0]	= 0x008C,
> 
>    Well, the above counter register subgroup stands out from the
> TSU_* registers in the Gigabit mapping, not sure if we should follow
> that. These registers are not currently used anyway...

I will add a blank line.

> 
> >+	[TSU_ADRH0]	= 0x0100,
> >+	[TSU_ADRL0]	= 0x0104,
> >+	[TSU_ADRH31]	= 0x01f8,
> >+	[TSU_ADRL31]	= 0x01fc,
> >+};
> >+
> >  static const u16 sh_eth_offset_fast_sh4[SH_ETH_MAX_REGISTER_OFFSET] = {
> >  	[ECMR]		= 0x0100,
> >  	[RFLR]		= 0x0108,
> >@@ -318,6 +376,14 @@ static bool sh_eth_is_gether(struct sh_eth_private *mdp)
> >  		return false;
> >  }
> >
> >+static bool sh_eth_is_rz_fast_ether(struct sh_eth_private *mdp)
> >+{
> >+	if (mdp->reg_offset == sh_eth_offset_fast_rz)
> >+		return true;
> >+	else
> >+		return false;
> 
>    Perhaps you should compress the above functions to one-liners as
> Joe has suggested. Or I/you could do it in a separate patch...

Will do, Joe's suggestion is a good one.

> 
> >+}
> >+
> >  static void sh_eth_select_mii(struct net_device *ndev)
> >  {
> >  	u32 value = 0x0;
> [...]
> >@@ -1309,9 +1409,9 @@ static int sh_eth_rx(struct net_device *ndev, u32 intr_status, int *quota)
> >
> >  		/* In case of almost all GETHER/ETHERs, the Receive Frame State
> >  		 * (RFS) bits in the Receive Descriptor 0 are from bit 9 to
> >-		 * bit 0. However, in case of the R8A7740's GETHER, the RFS
> >-		 * bits are from bit 25 to bit 16. So, the driver needs right
> >-		 * shifting by 16.
> >+		 * bit 0. However, in case of the R8A7740, R8A779x and
> 
>    Small nit: comma needed before "and" as far as I know English grammar.

To be honest I don't think it is. But I'll add one for you.

> 
> >+		 * R7S72100 the RFS bits are from bit 25 to bit 16. So, the
> >+		 * driver needs right shifting by 16.
> >  		 */
> >  		if (mdp->cd->shift_rd0)
> >  			desc_status >>= 16;
> 
>    Other than that, this looks fine now, you can add my:
> 
> Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

Thanks.

^ permalink raw reply

* Re: [PATCH net-next 2/2] reciprocal_divide: correction/update of the algorithm
From: Ben Hutchings @ 2014-01-16  0:17 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: davem, netdev, linux-kernel, Hannes Frederic Sowa, Eric Dumazet,
	Austin S Hemmelgarn, Jesse Gross, Jamal Hadi Salim,
	Stephen Hemminger, Matt Mackall, Pekka Enberg, Christoph Lameter,
	Andy Gospodarek, Veaceslav Falico, Jay Vosburgh, Jakub Zawadzki
In-Reply-To: <1389828228-30312-3-git-send-email-dborkman@redhat.com>

On Thu, 2014-01-16 at 00:23 +0100, Daniel Borkmann wrote:
[...]
> --- a/include/linux/reciprocal_div.h
> +++ b/include/linux/reciprocal_div.h
[...]
> + * RECIPROCAL_VALUE_TO_ZERO can be used to express an element, which
> + * used as the argument to reciprocal_divide always yields zero.
>   */
[...]
> +#define RECIPROCAL_VALUE_RESULT_TO_ZERO ((struct reciprocal_value){.sh1 = 32})
[...]
> +static inline u32 reciprocal_divide(u32 a, struct reciprocal_value R)
>  {
> -	return (u32)(((u64)A * R) >> 32);
> +	u32 t = (u32)(((u64)a * R.m) >> 32);
> +	return (t + ((a - t) >> R.sh1)) >> R.sh2;
[...]

(a - t) has type u32.
So (a - t) >> 32 has undefined behaviour.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* Re: [PATCH v2 00/16] wl1251 patches from linux-n900 tree
From: Pavel Machek @ 2014-01-16  0:21 UTC (permalink / raw)
  To: John W. Linville
  Cc: Pali Rohár, Luciano Coelho, linux-wireless, netdev,
	linux-kernel, freemangordon, aaro.koskinen, sre, joni.lapilainen,
	Johannes Berg, Felipe Contreras
In-Reply-To: <20140106200050.GG10885@tuxdriver.com>

On Mon 2014-01-06 15:00:50, John W. Linville wrote:
> On Tue, Dec 31, 2013 at 10:47:29AM +0100, Pali Rohár wrote:
> > On Sunday 08 December 2013 10:24:58 Pali Rohár wrote:
> > > Hello, I'm sending wl1251 patches from linux-n900 tree [1] for
> > > comments. More patches come from David's monitor & packet
> > > injection work. Patches are tested with 3.12 rc5 kernel on
> > > Nokia N900.
> > > 
> > > Second version contains new patch for fixing NULL pointer
> > > dereference which sometimes cause kernel panic and fixes code
> > > suggested by Pavel Machek.
>  
> > So far, there are no new comments for patches 01, 03-13 and 16. 
> > Are there any other problems with that patches or can be accepted 
> > for upstream?
>  
> They are probably fine to merge now.  Of course, I was still deleting
> wl12xx patches when they were originally posted, so I'll need someone
> to resend the patches to me...

Ping? Did they get merged? Did you receive the patches?

Regards,
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply

* Re: [PATCH net-next RFC] virtio-net: drop rq->max and rq->num
From: Rusty Russell @ 2014-01-15 23:55 UTC (permalink / raw)
  To: Jason Wang, mst, virtualization, netdev, linux-kernel; +Cc: Jason Wang
In-Reply-To: <87mwixx670.fsf@rustcorp.com.au>

Rusty Russell <rusty@rustcorp.com.au> writes:
> Jason Wang <jasowang@redhat.com> writes:
>> It looks like there's no need for those two fields:
>>
>> - Unless there's a failure for the first refill try, rq->max should be always
>>   equal to the vring size.
>> - rq->num is only used to determine the condition that we need to do the refill,
>>   we could check vq->num_free instead.
>> - rq->num was required to be increased or decreased explicitly after each
>>   get/put which results a bad API.
>>
>> So this patch removes them both to make the code simpler.
>
> Nice.  These fields date from when the vq struct was opaque.
>
> Applied,
> Rusty.

Oops, this doesn't require any core virtio changes, so it's for DaveM:

Acked-by: Rusty Russell <rusty@rustcorp.com.au>

Thanks,
Rusty.

>> Cc: Rusty Russell <rusty@rustcorp.com.au>
>> Cc: Michael S. Tsirkin <mst@redhat.com>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>>  drivers/net/virtio_net.c | 16 +++-------------
>>  1 file changed, 3 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index c51a988..4e1bce3 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -72,9 +72,6 @@ struct receive_queue {
>>  
>>  	struct napi_struct napi;
>>  
>> -	/* Number of input buffers, and max we've ever had. */
>> -	unsigned int num, max;
>> -
>>  	/* Chain pages by the private ptr. */
>>  	struct page *pages;
>>  
>> @@ -360,7 +357,6 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
>>  		}
>>  
>>  		page = virt_to_head_page(buf);
>> -		--rq->num;
>>  
>>  		num_skb_frags = skb_shinfo(curr_skb)->nr_frags;
>>  		if (unlikely(num_skb_frags == MAX_SKB_FRAGS)) {
>> @@ -406,7 +402,6 @@ err_skb:
>>  		}
>>  		page = virt_to_head_page(buf);
>>  		put_page(page);
>> -		--rq->num;
>>  	}
>>  err_buf:
>>  	dev->stats.rx_dropped++;
>> @@ -628,10 +623,7 @@ static bool try_fill_recv(struct receive_queue *rq, gfp_t gfp)
>>  		oom = err == -ENOMEM;
>>  		if (err)
>>  			break;
>> -		++rq->num;
>>  	} while (rq->vq->num_free);
>> -	if (unlikely(rq->num > rq->max))
>> -		rq->max = rq->num;
>>  	if (unlikely(!virtqueue_kick(rq->vq)))
>>  		return false;
>>  	return !oom;
>> @@ -699,11 +691,10 @@ again:
>>  	while (received < budget &&
>>  	       (buf = virtqueue_get_buf(rq->vq, &len)) != NULL) {
>>  		receive_buf(rq, buf, len);
>> -		--rq->num;
>>  		received++;
>>  	}
>>  
>> -	if (rq->num < rq->max / 2) {
>> +	if (rq->vq->num_free > virtqueue_get_vring_size(rq->vq) / 2) {
>>  		if (!try_fill_recv(rq, GFP_ATOMIC))
>>  			schedule_delayed_work(&vi->refill, 0);
>>  	}
>> @@ -1398,9 +1389,7 @@ static void free_unused_bufs(struct virtnet_info *vi)
>>  				give_pages(&vi->rq[i], buf);
>>  			else
>>  				dev_kfree_skb(buf);
>> -			--vi->rq[i].num;
>>  		}
>> -		BUG_ON(vi->rq[i].num != 0);
>>  	}
>>  }
>>  
>> @@ -1671,7 +1660,8 @@ static int virtnet_probe(struct virtio_device *vdev)
>>  		try_fill_recv(&vi->rq[i], GFP_KERNEL);
>>  
>>  		/* If we didn't even get one input buffer, we're useless. */
>> -		if (vi->rq[i].num == 0) {
>> +		if (vi->rq[i].vq->num_free ==
>> +		    virtqueue_get_vring_size(vi->rq[i].vq)) {
>>  			free_unused_bufs(vi);
>>  			err = -ENOMEM;
>>  			goto free_recv_bufs;
>> -- 
>> 1.8.3.2
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply

* Re: [PATCH net-next 1/2] random32: add prandom_u32_lt_N and convert "misuses" of reciprocal_divide
From: Joe Perches @ 2014-01-16  0:29 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: davem, netdev, linux-kernel, Jakub Zawadzki, Eric Dumazet,
	Hannes Frederic Sowa
In-Reply-To: <1389828228-30312-2-git-send-email-dborkman@redhat.com>

On Thu, 2014-01-16 at 00:23 +0100, Daniel Borkmann wrote:
> Many functions have open coded a function that return a random
> number in range [0,N-1]. Also, only because we have a function
> that is named reciprocal_divide(), it has not much to do with
> the pupose where it is being used when a previous reciprocal_value()
> has not been obtained.

prandom_u32_lt_N?

I do not like the camelcase name and thought the
prandom_u32_max was better.

How about using

u32 prandom_u32_max(u32 max)
{
	return (u32)(((u64)prandom_u32() * max) >> 32);
}

u32 prandom_u32_range(u32 a, u32 b)
{
	if (b < a)
		swap(a, b);

	return a + (u32)(((u64)prandom_u32() * (b - a)) >> 32);
}

^ permalink raw reply

* Re: [RFC PATCH net-next] etherdevice: Use ether_addr_copy to copy an Ethernet address
From: David Miller @ 2014-01-16  0:45 UTC (permalink / raw)
  To: joe; +Cc: netdev, linux-arm-kernel, linux-arch
In-Reply-To: <1389830878.14001.39.camel@joe-AO722>

From: Joe Perches <joe@perches.com>
Date: Wed, 15 Jan 2014 16:07:58 -0800

> If you want the ones for net-next net/ now (but not
> for batman-adv, that maybe could use a new function like
> ether_addr_copy_unaligned) here's a changestat.
> 
> Otherwise, I'll wait for the next cycle.

This looks fine, why don't you toss it my way over the weekend as I
still have some backlog to process at the moment?

Thanks.

^ permalink raw reply

* Re: [PATCH net-next 2/2] reciprocal_divide: correction/update of the algorithm
From: Hannes Frederic Sowa @ 2014-01-16  0:46 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Daniel Borkmann, davem, netdev, linux-kernel, Eric Dumazet,
	Austin S Hemmelgarn, Jesse Gross, Jamal Hadi Salim,
	Stephen Hemminger, Matt Mackall, Pekka Enberg, Christoph Lameter,
	Andy Gospodarek, Veaceslav Falico, Jay Vosburgh, Jakub Zawadzki
In-Reply-To: <1389831470.11912.40.camel@bwh-desktop.uk.level5networks.com>

On Thu, Jan 16, 2014 at 12:17:50AM +0000, Ben Hutchings wrote:
> On Thu, 2014-01-16 at 00:23 +0100, Daniel Borkmann wrote:
> [...]
> > --- a/include/linux/reciprocal_div.h
> > +++ b/include/linux/reciprocal_div.h
> [...]
> > + * RECIPROCAL_VALUE_TO_ZERO can be used to express an element, which
> > + * used as the argument to reciprocal_divide always yields zero.
> >   */
> [...]
> > +#define RECIPROCAL_VALUE_RESULT_TO_ZERO ((struct reciprocal_value){.sh1 = 32})
> [...]
> > +static inline u32 reciprocal_divide(u32 a, struct reciprocal_value R)
> >  {
> > -	return (u32)(((u64)A * R) >> 32);
> > +	u32 t = (u32)(((u64)a * R.m) >> 32);
> > +	return (t + ((a - t) >> R.sh1)) >> R.sh2;
> [...]
> 
> (a - t) has type u32.
> So (a - t) >> 32 has undefined behaviour.

Good catch, totally overlooked that.

Thanks,

  Hannes

^ permalink raw reply

* Re: [PATCH net-next RFC] virtio-net: drop rq->max and rq->num
From: David Miller @ 2014-01-16  0:46 UTC (permalink / raw)
  To: rusty; +Cc: netdev, virtualization, linux-kernel, mst
In-Reply-To: <87zjmwvlzl.fsf@rustcorp.com.au>

From: Rusty Russell <rusty@rustcorp.com.au>
Date: Thu, 16 Jan 2014 10:25:26 +1030

> Rusty Russell <rusty@rustcorp.com.au> writes:
>> Jason Wang <jasowang@redhat.com> writes:
>>> It looks like there's no need for those two fields:
>>>
>>> - Unless there's a failure for the first refill try, rq->max should be always
>>>   equal to the vring size.
>>> - rq->num is only used to determine the condition that we need to do the refill,
>>>   we could check vq->num_free instead.
>>> - rq->num was required to be increased or decreased explicitly after each
>>>   get/put which results a bad API.
>>>
>>> So this patch removes them both to make the code simpler.
>>
>> Nice.  These fields date from when the vq struct was opaque.
>>
>> Applied,
>> Rusty.
> 
> Oops, this doesn't require any core virtio changes, so it's for DaveM:
> 
> Acked-by: Rusty Russell <rusty@rustcorp.com.au>

Jason please repost this with Rusty's ACK, thanks.

^ permalink raw reply

* Re: [PATCH net-next] sctp: create helper function to enable|disable sackdelay
From: David Miller @ 2014-01-16  0:48 UTC (permalink / raw)
  To: wangweidong1; +Cc: vyasevich, nhorman, linux-sctp, netdev
In-Reply-To: <52D653B1.4040708@huawei.com>

From: Wang Weidong <wangweidong1@huawei.com>
Date: Wed, 15 Jan 2014 17:24:01 +0800

> add sctp_spp_sackdelay_{enable|disable} helper function for
> avoiding code duplication. 
> 
> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>

Applied.

^ permalink raw reply

* Re: [PATCH net] bnx2x: Don't release PCI bars on shutdown
From: David Miller @ 2014-01-16  0:49 UTC (permalink / raw)
  To: yuvalmin; +Cc: netdev, ariele
In-Reply-To: <1389780330-6927-1-git-send-email-yuvalmin@broadcom.com>

From: Yuval Mintz <yuvalmin@broadcom.com>
Date: Wed, 15 Jan 2014 12:05:30 +0200

> The bnx2x driver in its pci shutdown() callback releases its pci bars (in the
> same manner it does during its pci remove() callback).
> During a system reboot while VFs are enabled, its possible for the VF's remove
> to be called (as a result of pci_disable_sriov()) after its shutdown callback
> has already finished running; This will cause a paging request fault as the VF
> tries to access the pci bar which it has previously released, crashing the
> system.
> 
> This patch further differentiates the shutdown and remove callbacks, preventing the
> pci release procedures from being called during shutdown.
> 
> Signed-off-by: Yuval Mintz <yuvalmin@broadcom.com>
> Signed-off-by: Ariel Elior <ariele@broadcom.com>

Applied, thanks.

^ permalink raw reply


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