Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 2/2] gue: Call remcsum_adjust
From: Tom Herbert @ 2014-11-25 19:21 UTC (permalink / raw)
  To: davem, netdev
In-Reply-To: <1416943280-28473-1-git-send-email-therbert@google.com>

Change remote checksum offload to call remcsum_adjust. This also
eliminates the optimization to skip an IP header as part of the
adjustment (really does not seem to be much of a win).

Signed-off-by: Tom Herbert <therbert@google.com>
---
 net/ipv4/fou.c | 84 ++++++++++++----------------------------------------------
 1 file changed, 17 insertions(+), 67 deletions(-)

diff --git a/net/ipv4/fou.c b/net/ipv4/fou.c
index 3dfe982..b986298 100644
--- a/net/ipv4/fou.c
+++ b/net/ipv4/fou.c
@@ -64,15 +64,13 @@ static int fou_udp_recv(struct sock *sk, struct sk_buff *skb)
 }
 
 static struct guehdr *gue_remcsum(struct sk_buff *skb, struct guehdr *guehdr,
-				  void *data, int hdrlen, u8 ipproto)
+				  void *data, size_t hdrlen, u8 ipproto)
 {
 	__be16 *pd = data;
-	u16 start = ntohs(pd[0]);
-	u16 offset = ntohs(pd[1]);
-	u16 poffset = 0;
-	u16 plen;
-	__wsum csum, delta;
-	__sum16 *psum;
+	size_t start = ntohs(pd[0]);
+	size_t offset = ntohs(pd[1]);
+	size_t plen = hdrlen + max_t(size_t, offset + sizeof(u16), start);
+	__wsum delta;
 
 	if (skb->remcsum_offload) {
 		/* Already processed in GRO path */
@@ -80,35 +78,15 @@ static struct guehdr *gue_remcsum(struct sk_buff *skb, struct guehdr *guehdr,
 		return guehdr;
 	}
 
-	if (start > skb->len - hdrlen ||
-	    offset > skb->len - hdrlen - sizeof(u16))
-		return NULL;
-
-	if (unlikely(skb->ip_summed != CHECKSUM_COMPLETE))
-		__skb_checksum_complete(skb);
-
-	plen = hdrlen + offset + sizeof(u16);
 	if (!pskb_may_pull(skb, plen))
 		return NULL;
 	guehdr = (struct guehdr *)&udp_hdr(skb)[1];
 
-	if (ipproto == IPPROTO_IP && sizeof(struct iphdr) < plen) {
-		struct iphdr *ip = (struct iphdr *)(skb->data + hdrlen);
-
-		/* If next header happens to be IP we can skip that for the
-		 * checksum calculation since the IP header checksum is zero
-		 * if correct.
-		 */
-		poffset = ip->ihl * 4;
-	}
-
-	csum = csum_sub(skb->csum, skb_checksum(skb, poffset + hdrlen,
-						start - poffset - hdrlen, 0));
+	if (unlikely(skb->ip_summed != CHECKSUM_COMPLETE))
+		__skb_checksum_complete(skb);
 
-	/* Set derived checksum in packet */
-	psum = (__sum16 *)(skb->data + hdrlen + offset);
-	delta = csum_sub(csum_fold(csum), *psum);
-	*psum = csum_fold(csum);
+	delta = remcsum_adjust((void *)guehdr + hdrlen,
+			       skb->csum, start, offset);
 
 	/* Adjust skb->csum since we changed the packet */
 	skb->csum = csum_add(skb->csum, delta);
@@ -158,9 +136,6 @@ static int gue_udp_recv(struct sock *sk, struct sk_buff *skb)
 
 	ip_hdr(skb)->tot_len = htons(ntohs(ip_hdr(skb)->tot_len) - len);
 
-	/* Pull UDP header now, skb->data points to guehdr */
-	__skb_pull(skb, sizeof(struct udphdr));
-
 	/* Pull csum through the guehdr now . This can be used if
 	 * there is a remote checksum offload.
 	 */
@@ -188,7 +163,7 @@ static int gue_udp_recv(struct sock *sk, struct sk_buff *skb)
 	if (unlikely(guehdr->control))
 		return gue_control_message(skb, guehdr);
 
-	__skb_pull(skb, hdrlen);
+	__skb_pull(skb, sizeof(struct udphdr) + hdrlen);
 	skb_reset_transport_header(skb);
 
 	return -guehdr->proto_ctype;
@@ -248,24 +223,17 @@ static struct guehdr *gue_gro_remcsum(struct sk_buff *skb, unsigned int off,
 				      size_t hdrlen, u8 ipproto)
 {
 	__be16 *pd = data;
-	u16 start = ntohs(pd[0]);
-	u16 offset = ntohs(pd[1]);
-	u16 poffset = 0;
-	u16 plen;
-	void *ptr;
-	__wsum csum, delta;
-	__sum16 *psum;
+	size_t start = ntohs(pd[0]);
+	size_t offset = ntohs(pd[1]);
+	size_t plen = hdrlen + max_t(size_t, offset + sizeof(u16), start);
+	__wsum delta;
 
 	if (skb->remcsum_offload)
 		return guehdr;
 
-	if (start > skb_gro_len(skb) - hdrlen ||
-	    offset > skb_gro_len(skb) - hdrlen - sizeof(u16) ||
-	    !NAPI_GRO_CB(skb)->csum_valid || skb->remcsum_offload)
+	if (!NAPI_GRO_CB(skb)->csum_valid)
 		return NULL;
 
-	plen = hdrlen + offset + sizeof(u16);
-
 	/* Pull checksum that will be written */
 	if (skb_gro_header_hard(skb, off + plen)) {
 		guehdr = skb_gro_header_slow(skb, off + plen, off);
@@ -273,26 +241,8 @@ static struct guehdr *gue_gro_remcsum(struct sk_buff *skb, unsigned int off,
 			return NULL;
 	}
 
-	ptr = (void *)guehdr + hdrlen;
-
-	if (ipproto == IPPROTO_IP &&
-	    (hdrlen + sizeof(struct iphdr) < plen)) {
-		struct iphdr *ip = (struct iphdr *)(ptr + hdrlen);
-
-		/* If next header happens to be IP we can skip
-		 * that for the checksum calculation since the
-		 * IP header checksum is zero if correct.
-		 */
-		poffset = ip->ihl * 4;
-	}
-
-	csum = csum_sub(NAPI_GRO_CB(skb)->csum,
-			csum_partial(ptr + poffset, start - poffset, 0));
-
-	/* Set derived checksum in packet */
-	psum = (__sum16 *)(ptr + offset);
-	delta = csum_sub(csum_fold(csum), *psum);
-	*psum = csum_fold(csum);
+	delta = remcsum_adjust((void *)guehdr + hdrlen,
+			       NAPI_GRO_CB(skb)->csum, start, offset);
 
 	/* Adjust skb->csum since we changed the packet */
 	skb->csum = csum_add(skb->csum, delta);
-- 
2.1.0.rc2.206.gedb03e5

^ permalink raw reply related

* Re: [PATCH v4 0/7] kernel tinification: optionally compile out splice family of syscalls (splice, vmsplice, tee and sendfile)
From: David Miller @ 2014-11-25 19:27 UTC (permalink / raw)
  To: ebiederm-aS9lmoZGLiVWk0Htik3J/w
  Cc: josh-iaAMLnmF4UmaiuxdJuQwMA, rdunlap-wEGCiKHe2LqWVfeAwA7xHQ,
	pieter-qeJ+1H9vRZbz+pZb47iToQ,
	alexander.h.duyck-ral2JQCrhuEAvxtiuMwx3w,
	viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn, ast-uqk4Ao+rVK5Wk0Htik3J/w,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	beber-2YnHqweIUXrk1uMJSBkQmQ,
	catalina.mocanu-Re5JQEeQqe8AvxtiuMwx3w,
	dborkman-H+wXaHxf7aLQT0dZR+AlfA, edumazet-hpIqsD4AKlfQT0dZR+AlfA,
	fabf-AgBVmzD5pcezQB+pC5nmwQ,
	fuse-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	geert-Td1EMuHUCqxL1ZNQvxDV9g, hughd-hpIqsD4AKlfQT0dZR+AlfA,
	iulia.manda21-Re5JQEeQqe8AvxtiuMwx3w, JBeulich-IBi9RG/b67k,
	bfields-uC3wQj2KruNg9hUCZPvPmw, jlayton-vpEMnDpepFuMZCB2o+C8xQ,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA, mcgrof-IBi9RG/b67k,
	mattst88-Re5JQEeQqe8AvxtiuMwx3w, mgorman-l3A5Bk7waGM,
	mst-H+wXaHxf7aLQT0dZR+AlfA, miklos-sUDqSbJrdHQHWmgEVkV9KA,
	netdev-u79uwXL29TY76Z2rM5mHXA, oleg-H+wXaHxf7aLQT0dZR+AlfA,
	Paul.Durrant-Sxgqhf6Nn4DQT0dZR+AlfA,
	paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
	pefoley2-lY0TAiDIAFlBDgjK7y7TUQ, tgraf-G/eBtMaohhA,
	therbert-hpIqsD4AKlfQT0dZR+AlfA,
	trond.myklebust-7I+n7zu2hftEKMMhf/gKZA,
	willemb-hpIqsD4AKlfQT0dZR+AlfA,
	xiaoguangrong-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, zhe
In-Reply-To: <87egsr9jkz.fsf-JOvCrm2gF+uungPnsOpG7nhyD016LWXt@public.gmane.org>

From: ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org (Eric W. Biederman)
Date: Tue, 25 Nov 2014 13:16:44 -0600

> David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> writes:
> 
>> From: josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org
>> Date: Tue, 25 Nov 2014 10:53:10 -0800
>>
>>> It's not a "slippery slope"; it's been our standard practice for ages.
>>
>> We've never put an entire class of generic system calls behind
>> a config option.
> 
> CONFIG_SYSVIPC has been in the kernel as long as I can remember.
> 
> I seem to remember a plan to remove that code once userspace had
> finished migrating to more unixy interfaces to ipc.  But in 20 years
> that migration does does not seem to have finished, or even look
> like it ever will.
> 
> But if we started a slippery slope it was long long ago.

Fair enough.

Would be amusing if these tiny systems have it enabled.

^ permalink raw reply

* Re: [PATCH v2 01/17] new helper: skb_copy_and_csum_datagram_msg()
From: David Miller @ 2014-11-25 19:28 UTC (permalink / raw)
  To: viro; +Cc: netdev, linux-kernel
In-Reply-To: <1416924151-28698-1-git-send-email-viro@ZenIV.linux.org.uk>


Al, this series looks fine to me, do you have a tree I can pull
it from?

^ permalink raw reply

* Re: [PATCH] tg3: fix ring init when there are more TX than RX channels
From: David Miller @ 2014-11-25 19:33 UTC (permalink / raw)
  To: cascardo; +Cc: netdev, prashant, mchan
In-Reply-To: <1416932471-8744-1-git-send-email-cascardo@linux.vnet.ibm.com>

From: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
Date: Tue, 25 Nov 2014 14:21:11 -0200

> @@ -8563,7 +8563,8 @@ static int tg3_init_rings(struct tg3 *tp)
>  		if (tnapi->rx_rcb)
>  			memset(tnapi->rx_rcb, 0, TG3_RX_RCB_RING_BYTES(tp));
>  
> -		if (tg3_rx_prodring_alloc(tp, &tnapi->prodring)) {
> +		if (tnapi->prodring.rx_std &&
> +		    tg3_rx_prodring_alloc(tp, &tnapi->prodring)) {
>  			tg3_free_rings(tp);
>  			return -ENOMEM;

Maybe a better test is "i < tp->rxq_cnt"?  This is what is used in
tg3_mem_rx_acquire() to determine if tg3_rx_prodring_init() happens.

^ permalink raw reply

* Re: [PATCH v6 5/5] patch to allow arm cross-compile
From: Joe Perches @ 2014-11-25 19:35 UTC (permalink / raw)
  To: Alexander Duyck, Russell King
  Cc: linux-arch, netdev, linux-kernel, mathieu.desnoyers, peterz, benh,
	heiko.carstens, mingo, mikey, linux, donald.c.skidmore,
	matthew.vick, geert, jeffrey.t.kirsher, romieu, paulmck, nic_swsd,
	arnd, will.deacon, michael, tony.luck, torvalds, oleg,
	schwidefsky, fweisbec, davem
In-Reply-To: <20141125183322.1486.90766.stgit@ahduyck-server>

(adding Russell King)

On Tue, 2014-11-25 at 10:33 -0800, Alexander Duyck wrote:
> ---
>  arch/arm/kernel/asm-offsets.c |    4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c
> index 2d2d608..6f8e79c 100644
> --- a/arch/arm/kernel/asm-offsets.c
> +++ b/arch/arm/kernel/asm-offsets.c
> @@ -49,10 +49,6 @@
>  #error Your compiler is too buggy; it is known to miscompile kernels.
>  #error    Known good compilers: 3.3, 4.x
>  #endif
> -#if GCC_VERSION >= 40800 && GCC_VERSION < 40803
> -#error Your compiler is too buggy; it is known to miscompile kernels
> -#error and result in filesystem corruption and oopses.
> -#endif
>  #endif

^ permalink raw reply

* Re: [PATCH net-next] gre: Set inner mac header in gro complete
From: David Miller @ 2014-11-25 19:35 UTC (permalink / raw)
  To: eric.dumazet; +Cc: therbert, linux, netdev
In-Reply-To: <1416938241.29427.37.camel@edumazet-glaptop2.roam.corp.google.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 25 Nov 2014 09:57:21 -0800

> On Tue, 2014-11-25 at 09:30 -0800, Tom Herbert wrote:
>> Set the inner mac header to point to the GRE payload when
>> doing GRO. This is needed if we proceed to send the packet
>> through GRE GSO which now uses the inner mac header instead
>> of inner network header to determine the length of encapsulation
>> headers.
>> 
>> Reported-by: Wolfgang Walter <linux@stwm.de>
>> Signed-off-by: Tom Herbert <therbert@google.com>
>> ---
> 
> Please Tom, add in your bug fixes something to ease backports.
> 
> Fixes: 14051f0452a2 ("gre: Use inner mac length when computing tunnel length")

Also this is extremely far reaching consequences.

The bug is in Linus's tree, so net-next is not a suitable target,
'net' is.

Also, the commit in question has been backported into -stable, so
I would need to queue it up there as well.

Please respin this against 'net', and add the Fixes: tag, thanks.

^ permalink raw reply

* Re: [PATCHv10 ovs 00/15] Revalidate flows with unique identifiers.
From: Joe Stringer @ 2014-11-25 19:45 UTC (permalink / raw)
  To: dev@openvswitch.org; +Cc: Linux Netdev List
In-Reply-To: <1415906275-3172-1-git-send-email-joestringer@nicira.com>

On 13 November 2014 at 11:17, Joe Stringer <joestringer@nicira.com> wrote:
>
> This series modifies the dpif interface for flow commands to use 128-bit unique
> identifiers as an alternative to the netlink-formatted flow key, and caches the
> mask/actions in the udpif_key. This significantly reduces the cost of
> assembling messages between revalidators and the datapath, improving
> revalidation performance by 40% or more. In a test environment of many
> short-lived flows constantly being set up in the datapath, this increases the
> number of flows that can be maintained in the linux datapath from around
> 130-140K up to 190-200K. For the userspace datapath, this decreases the time
> spent revalidating 160K flows from 250ms to 150ms.
>
> The core of the changes sits in the handler and revalidator code. Handlers take
> responsibility for creating udpif_key cache entries which now include a copy of
> the flow mask and actions. Revalidators request datapaths to dump flows using
> only the unique identifier and stats, rather than the full set of
> netlink-formatted flow key, mask and actions.
>
> In cases where full revalidation is required, revalidators will use the
> udpif_key cache of the key/mask/acts to validate the flow. The dpif will
> detect datapath support for the unique identifer "UFID" feature, and omit flow
> keys from netlink transactions if it is supported. For backwards compatibility,
> flow keys will always be serialised if UFID support is not detected in the
> datapath.
>
> Patches 1,2,3,15 are unreviewed. Patch 12 needs further review.
>
> This series is also made available here to assist review:
> https://github.com/joestringer/openvswitch/tree/submit/ufid_v10
>

<snip>

>
> Joe Stringer (15):
>   tests: Add command to purge revalidators of flows.
>   ovs-bugtool: Log more detail for dumped flows.
>   datapath: Add 'is_mask' to ovs_nla_put_flow().
>   revalidator: Use 'cmap' for storing ukeys.
>   revalidator: Protect ukeys with a mutex.
>   udpif: Separate udpif_key maps from revalidators.
>   upcall: Rename dump_op -> ukey_op.
>   upcall: Create ukeys in handler threads.
>   upcall: Revalidate using cache of mask, actions.
>   hash: Add 128-bit murmurhash.
>   dpif: Generate flow_hash for revalidators in dpif.
>   datapath: Add support for unique flow identifiers.
>   dpif: Index flows using unique identifiers.
>   dpif: Minimize memory copy for revalidation.
>   dpctl: Add support for using UFID to add/del flows.


<snip>

I'm currently addressing feedback for the datapath patches, which also
involves minor changes to patch #13.

I've already pushed patch #1 as it provided an unrelated benefit.  As
this series is fairly close, my current plan is to push patches 4-11
to master this afternoon; these are long-running unchanged patches. I
will then resend patches 2,13,14,15 and the openvswitch.h changes for
final review here. I'll rebase the datapath changes against net-next
and go through review there.

^ permalink raw reply

* Re: [PATCH 1/2 3.18] rtlwifi: rtl8821ae: Fix 5G detection problem
From: Larry Finger @ 2014-11-25 19:50 UTC (permalink / raw)
  To: John W. Linville; +Cc: linux-wireless, netdev, Valerio Passini
In-Reply-To: <20141125184625.GC15476@tuxdriver.com>

On 11/25/2014 12:46 PM, John W. Linville wrote:
> On Tue, Nov 25, 2014 at 10:32:06AM -0600, Larry Finger wrote:
>> The changes associated with moving this driver from staging to the regular
>> tree missed one section setting the allowable rates for the 5GHz band.
>>
>> This patch is needed to fix the regression reported in Bug #88811
>> (https://bugzilla.kernel.org/show_bug.cgi?id=88811).
>>
>> Reported-by: Valerio Passini <valerio.passini@unicam.it>
>> Tested-by: Valerio Passini <valerio.passini@unicam.it>
>> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
>> Cc: Valerio Passini <valerio.passini@unicam.it>
>> ---
>>   drivers/net/wireless/rtlwifi/rtl8821ae/hw.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/wireless/rtlwifi/rtl8821ae/hw.c b/drivers/net/wireless/rtlwifi/rtl8821ae/hw.c
>> index 310d316..18f34f7 100644
>> --- a/drivers/net/wireless/rtlwifi/rtl8821ae/hw.c
>> +++ b/drivers/net/wireless/rtlwifi/rtl8821ae/hw.c
>> @@ -3672,8 +3672,9 @@ static void rtl8821ae_update_hal_rate_mask(struct ieee80211_hw *hw,
>>   		mac->opmode == NL80211_IFTYPE_ADHOC)
>>   		macid = sta->aid + 1;
>>   	if (wirelessmode == WIRELESS_MODE_N_5G ||
>> -	    wirelessmode == WIRELESS_MODE_AC_5G)
>> -		ratr_bitmap = sta->supp_rates[NL80211_BAND_5GHZ];
>> +	    wirelessmode == WIRELESS_MODE_AC_5G ||
>> +	    wirelessmode == WIRELESS_MODE_A)
>> +		ratr_bitmap = (sta->supp_rates[NL80211_BAND_5GHZ])<<4;
>
> The parenthesis seem superfluous.  How about this line instead?
>
> +		ratr_bitmap = sta->supp_rates[NL80211_BAND_5GHZ] << 4;
>
>>   	else
>>   		ratr_bitmap = sta->supp_rates[NL80211_BAND_2GHZ];

Good idea. V2 is on its way.

Larry

^ permalink raw reply

* Re: [PATCH rfc 1/4] net-timestamp: pull headers for SOCK_STREAM
From: Willem de Bruijn @ 2014-11-25 19:52 UTC (permalink / raw)
  To: David Miller; +Cc: Network Development, Andy Lutomirski, Richard Cochran
In-Reply-To: <20141125.134240.861582393401487675.davem@davemloft.net>

On Tue, Nov 25, 2014 at 1:42 PM, David Miller <davem@davemloft.net> wrote:
> From: Willem de Bruijn <willemb@google.com>
> Date: Tue, 25 Nov 2014 12:58:03 -0500
>
>> From: Willem de Bruijn <willemb@google.com>
>>
>> When returning timestamped packets on the error queue, only return
>> the data that the application initially sent: not the protocol
>> headers.
>>
>> This changes the ABI. The TCP interface is new enough that it should
>> be safe to do so. The UDP interface could be changed analogously with
>>
>> +  else if (sk->sk_protocol == IPPROTO_UDP)
>> +    skb_pull(skb, skb_transport_offset(skb) + sizeof(struct udphdr));
>>
>> Tested with Documentation/networking/timestamping/txtimestamp -l 60 -x
>>
>> Signed-off-by: Willem de Bruijn <willemb@google.com>
>
> What's the harm in exposing the headers?  Either it's harmful, and
> therefore doing so for UDP is bad too, or it's harmless and

Headers may expose information not available otherwise. I don't
immediately see critical problems, but that does not mean that they
might not lurk there.

We so far avoid exposing the sequence number, though keeping it hidden
is more about third parties. More in general, unprivileged processes
may start requesting timestamps only to learn tcp state that they
should either get from tcpinfo or cannot currently get at all, likely
for good reason. A far-fetched example is identifying admin iptables
tos mangling rules by reading the tos bits at the driver layer. At least
on my machine, iptables -L is privileged.

> we should probably leave it alone to not risk breaking anyone.

That's fair. I sent it for rfc first for that reason. I won't resubmit
unless more serious concerns are raised.

^ permalink raw reply

* Re: [PATCH 1/2 3.18] rtlwifi: rtl8821ae: Fix 5G detection problem
From: John W. Linville @ 2014-11-25 19:53 UTC (permalink / raw)
  To: Larry Finger
  Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, Valerio Passini
In-Reply-To: <5474DD88.7060002-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>

On Tue, Nov 25, 2014 at 01:50:32PM -0600, Larry Finger wrote:
> On 11/25/2014 12:46 PM, John W. Linville wrote:
> >On Tue, Nov 25, 2014 at 10:32:06AM -0600, Larry Finger wrote:
> >>The changes associated with moving this driver from staging to the regular
> >>tree missed one section setting the allowable rates for the 5GHz band.
> >>
> >>This patch is needed to fix the regression reported in Bug #88811
> >>(https://bugzilla.kernel.org/show_bug.cgi?id=88811).
> >>
> >>Reported-by: Valerio Passini <valerio.passini-vbqOIlEVYMdeoWH0uzbU5w@public.gmane.org>
> >>Tested-by: Valerio Passini <valerio.passini-vbqOIlEVYMdeoWH0uzbU5w@public.gmane.org>
> >>Signed-off-by: Larry Finger <Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
> >>Cc: Valerio Passini <valerio.passini-vbqOIlEVYMdeoWH0uzbU5w@public.gmane.org>
> >>---
> >>  drivers/net/wireless/rtlwifi/rtl8821ae/hw.c | 5 +++--
> >>  1 file changed, 3 insertions(+), 2 deletions(-)
> >>
> >>diff --git a/drivers/net/wireless/rtlwifi/rtl8821ae/hw.c b/drivers/net/wireless/rtlwifi/rtl8821ae/hw.c
> >>index 310d316..18f34f7 100644
> >>--- a/drivers/net/wireless/rtlwifi/rtl8821ae/hw.c
> >>+++ b/drivers/net/wireless/rtlwifi/rtl8821ae/hw.c
> >>@@ -3672,8 +3672,9 @@ static void rtl8821ae_update_hal_rate_mask(struct ieee80211_hw *hw,
> >>  		mac->opmode == NL80211_IFTYPE_ADHOC)
> >>  		macid = sta->aid + 1;
> >>  	if (wirelessmode == WIRELESS_MODE_N_5G ||
> >>-	    wirelessmode == WIRELESS_MODE_AC_5G)
> >>-		ratr_bitmap = sta->supp_rates[NL80211_BAND_5GHZ];
> >>+	    wirelessmode == WIRELESS_MODE_AC_5G ||
> >>+	    wirelessmode == WIRELESS_MODE_A)
> >>+		ratr_bitmap = (sta->supp_rates[NL80211_BAND_5GHZ])<<4;
> >
> >The parenthesis seem superfluous.  How about this line instead?
> >
> >+		ratr_bitmap = sta->supp_rates[NL80211_BAND_5GHZ] << 4;
> >
> >>  	else
> >>  		ratr_bitmap = sta->supp_rates[NL80211_BAND_2GHZ];
> 
> Good idea. V2 is on its way.

No need -- I merged it with my version of the line. :-)

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org			might be all we have.  Be ready.
--
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 rfc 3/4] net-timestamp: tcp sockets return v6 errors on v6 sockets
From: Willem de Bruijn @ 2014-11-25 19:53 UTC (permalink / raw)
  To: David Miller; +Cc: Network Development, Andy Lutomirski, Richard Cochran
In-Reply-To: <20141125.134152.1265765328256853180.davem@davemloft.net>

On Tue, Nov 25, 2014 at 1:41 PM, David Miller <davem@davemloft.net> wrote:
> From: Willem de Bruijn <willemb@google.com>
> Date: Tue, 25 Nov 2014 12:58:05 -0500
>
>> From: Willem de Bruijn <willemb@google.com>
>>
>> TCP timestamping introduced MSG_ERRQUEUE handling for TCP sockets.
>> It always passed errorqueue requests onto ip_recv_error, but the
>> same tcp_recvmsg code may also be called for IPv6 sockets. In that
>> case, pass to ipv6_recv_error.
>>
>> Tested by asking for PKTINFO with
>>
>>   Documentation/networking/timestamping/txtimestamp -I
>>
>> Before this change, IPv6 sockets would return AF_INET/IP_PKTINFO
>> after the change, these sockets return AF_INET6/IPV6_PKTINFO
>>
>> Signed-off-by: Willem de Bruijn <willemb@google.com>
>
> This looks like a bug fix to me, and is therefore probably 'net'
> material.

Okay. I'll submit it separately to net.

^ permalink raw reply

* Re: [PATCH rfc 1/4] net-timestamp: pull headers for SOCK_STREAM
From: David Miller @ 2014-11-25 19:54 UTC (permalink / raw)
  To: willemb; +Cc: netdev, luto, richardcochran
In-Reply-To: <CA+FuTSdT9=Hi7K3OnMxVRiK8PmS4=DvVqLumWx74OEz3okx23A@mail.gmail.com>

From: Willem de Bruijn <willemb@google.com>
Date: Tue, 25 Nov 2014 14:52:00 -0500

> On Tue, Nov 25, 2014 at 1:42 PM, David Miller <davem@davemloft.net> wrote:
>> From: Willem de Bruijn <willemb@google.com>
>> Date: Tue, 25 Nov 2014 12:58:03 -0500
>>
>> What's the harm in exposing the headers?  Either it's harmful, and
>> therefore doing so for UDP is bad too, or it's harmless and
> 
> Headers may expose information not available otherwise. I don't
> immediately see critical problems, but that does not mean that they
> might not lurk there.
> 
> We so far avoid exposing the sequence number, though keeping it hidden
> is more about third parties. More in general, unprivileged processes
> may start requesting timestamps only to learn tcp state that they
> should either get from tcpinfo or cannot currently get at all, likely
> for good reason. A far-fetched example is identifying admin iptables
> tos mangling rules by reading the tos bits at the driver layer. At least
> on my machine, iptables -L is privileged.
> 
>> we should probably leave it alone to not risk breaking anyone.
> 
> That's fair. I sent it for rfc first for that reason. I won't resubmit
> unless more serious concerns are raised.

I just worry about the potential breakage.

Your concerns are valid... I honestly don't know what we should do here.
Both choices have merit.

^ permalink raw reply

* Re: [PATCH] tg3: fix ring init when there are more TX than RX channels
From: cascardo @ 2014-11-25 19:57 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, prashant, mchan
In-Reply-To: <20141125.143352.669361329120151757.davem@davemloft.net>

On Tue, Nov 25, 2014 at 02:33:52PM -0500, David Miller wrote:
> From: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
> Date: Tue, 25 Nov 2014 14:21:11 -0200
> 
> > @@ -8563,7 +8563,8 @@ static int tg3_init_rings(struct tg3 *tp)
> >  		if (tnapi->rx_rcb)
> >  			memset(tnapi->rx_rcb, 0, TG3_RX_RCB_RING_BYTES(tp));
> >  
> > -		if (tg3_rx_prodring_alloc(tp, &tnapi->prodring)) {
> > +		if (tnapi->prodring.rx_std &&
> > +		    tg3_rx_prodring_alloc(tp, &tnapi->prodring)) {
> >  			tg3_free_rings(tp);
> >  			return -ENOMEM;
> 
> Maybe a better test is "i < tp->rxq_cnt"?  This is what is used in
> tg3_mem_rx_acquire() to determine if tg3_rx_prodring_init() happens.
> 

Well, what we have in net-next/master does:

	for (i = 0; i < limit; i++) {
		struct tg3_napi *tnapi = &tp->napi[i];

		if (tg3_rx_prodring_init(tp, &tnapi->prodring))
			goto err_out;

where limit is:

	limit = tp->rxq_cnt;

	if (tg3_flag(tp, ENABLE_RSS))
		limit++;

So, I thought that, instead of rewriting that same logic in
tg3_init_rings, that we should better just check that the rx_std has
been allocated, which is going to happen in tg3_rx_prodring_init.

So, the alternative would be:

 
diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index dbb41c19..51d45aa 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -8540,7 +8540,11 @@ static void tg3_free_rings(struct tg3 *tp)
  */
 static int tg3_init_rings(struct tg3 *tp)
 {
-	int i;
+	int i, limit;
+
+	limit = tp->rxq_cnt;
+	if (tg3_flag(tp, ENABLE_RSS))
+		limit++;
 
 	/* Free up all the SKBs. */
 	tg3_free_rings(tp);
@@ -8563,7 +8567,8 @@ static int tg3_init_rings(struct tg3 *tp)
 		if (tnapi->rx_rcb)
 			memset(tnapi->rx_rcb, 0, TG3_RX_RCB_RING_BYTES(tp));
 
-		if (tg3_rx_prodring_alloc(tp, &tnapi->prodring)) {
+		if (i < limit &&
+		    tg3_rx_prodring_alloc(tp, &tnapi->prodring)) {
 			tg3_free_rings(tp);
 			return -ENOMEM;
 		}

^ permalink raw reply related

* Re: [PATCH net] Revert "netfilter: conntrack: fix race in __nf_conntrack_confirm against get_next_corpse"
From: David Miller @ 2014-11-25 20:00 UTC (permalink / raw)
  To: pablo; +Cc: netfilter-devel, netdev, brouer
In-Reply-To: <1416941687-25471-1-git-send-email-pablo@netfilter.org>

From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Tue, 25 Nov 2014 19:54:47 +0100

> This reverts commit 5195c14c8b27cc0b18220ddbf0e5ad3328a04187.
> 
> If the conntrack clashes with an existing one, it is left out of
> the unconfirmed list, thus, crashing when dropping the packet and
> releasing the conntrack since golden rule is that conntracks are
> always placed in any of the existing lists for traceability reasons.
> 
> Reported-by: Daniel Borkmann <dborkman@redhat.com>
> Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=88841
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
> Hi David,
> 
> Could you manually apply this to your net tree? We have a better
> candidate fix to replace this broken patch that I will pass to you
> once it gets sufficient testing.

Done, thanks Pablo.

^ permalink raw reply

* Re: [PATCH v4 0/7] kernel tinification: optionally compile out splice family of syscalls (splice, vmsplice, tee and sendfile)
From: Eric W. Biederman @ 2014-11-25 20:01 UTC (permalink / raw)
  To: David Miller
  Cc: josh-iaAMLnmF4UmaiuxdJuQwMA, rdunlap-wEGCiKHe2LqWVfeAwA7xHQ,
	pieter-qeJ+1H9vRZbz+pZb47iToQ,
	alexander.h.duyck-ral2JQCrhuEAvxtiuMwx3w,
	viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn, ast-uqk4Ao+rVK5Wk0Htik3J/w,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	beber-2YnHqweIUXrk1uMJSBkQmQ,
	catalina.mocanu-Re5JQEeQqe8AvxtiuMwx3w,
	dborkman-H+wXaHxf7aLQT0dZR+AlfA, edumazet-hpIqsD4AKlfQT0dZR+AlfA,
	fabf-AgBVmzD5pcezQB+pC5nmwQ,
	fuse-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	geert-Td1EMuHUCqxL1ZNQvxDV9g, hughd-hpIqsD4AKlfQT0dZR+AlfA,
	iulia.manda21-Re5JQEeQqe8AvxtiuMwx3w, JBeulich-IBi9RG/b67k,
	bfields-uC3wQj2KruNg9hUCZPvPmw, jlayton-vpEMnDpepFuMZCB2o+C8xQ,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA, mcgrof-IBi9RG/b67k,
	mattst88-Re5JQEeQqe8AvxtiuMwx3w, mgorman-l3A5Bk7waGM,
	mst-H+wXaHxf7aLQT0dZR+AlfA, miklos-sUDqSbJrdHQHWmgEVkV9KA,
	netdev-u79uwXL29TY76Z2rM5mHXA, oleg-H+wXaHxf7aLQT0dZR+AlfA,
	Paul.Durrant-Sxgqhf6Nn4DQT0dZR+AlfA,
	paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
	pefoley2-lY0TAiDIAFlBDgjK7y7TUQ, tgraf-G/eBtMaohhA,
	therbert-hpIqsD4AKlfQT0dZR+AlfA,
	trond.myklebust-7I+n7zu2hftEKMMhf/gKZA,
	willemb-hpIqsD4AKlfQT0dZR+AlfA,
	xiaoguangrong-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, zhe
In-Reply-To: <20141125.142741.1620673255148724338.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>

David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> writes:

> From: ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org (Eric W. Biederman)
> Date: Tue, 25 Nov 2014 13:16:44 -0600
>
>> David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> writes:
>> 
>>> From: josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org
>>> Date: Tue, 25 Nov 2014 10:53:10 -0800
>>>
>>>> It's not a "slippery slope"; it's been our standard practice for ages.
>>>
>>> We've never put an entire class of generic system calls behind
>>> a config option.
>> 
>> CONFIG_SYSVIPC has been in the kernel as long as I can remember.
>> 
>> I seem to remember a plan to remove that code once userspace had
>> finished migrating to more unixy interfaces to ipc.  But in 20 years
>> that migration does does not seem to have finished, or even look
>> like it ever will.
>> 
>> But if we started a slippery slope it was long long ago.
>
> Fair enough.
>
> Would be amusing if these tiny systems have it enabled.

It would.

In practice when I was playing in that space I had a hard time
justifying CONFIG_NET and CONFIG_INET.  Despite writing a network
bootloader to use with kexec.

Eric
--
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

* [PATCH 1/2 V2 3.18] rtlwifi: rtl8821ae: Fix 5G detection problem
From: Larry Finger @ 2014-11-25 20:02 UTC (permalink / raw)
  To: linville-2XuSBdqkA4R54TAoqtyWWQ
  Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA, Larry Finger,
	netdev-u79uwXL29TY76Z2rM5mHXA, Valerio Passini

The changes associated with moving this driver from staging to the regular
tree missed one section setting the allowable rates for the 5GHz band.

This patch is needed to fix the regression reported in Bug #88811
(https://bugzilla.kernel.org/show_bug.cgi?id=88811).

Reported-by: Valerio Passini <valerio.passini-vbqOIlEVYMdeoWH0uzbU5w@public.gmane.org>
Tested-by: Valerio Passini <valerio.passini-vbqOIlEVYMdeoWH0uzbU5w@public.gmane.org>
Signed-off-by: Larry Finger <Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
Cc: Valerio Passini <valerio.passini-vbqOIlEVYMdeoWH0uzbU5w@public.gmane.org>
---

V2 - removed extra parentheses as suggested by John Linville.

---
 drivers/net/wireless/rtlwifi/rtl8821ae/hw.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/rtlwifi/rtl8821ae/hw.c b/drivers/net/wireless/rtlwifi/rtl8821ae/hw.c
index 310d316..18f34f7 100644
--- a/drivers/net/wireless/rtlwifi/rtl8821ae/hw.c
+++ b/drivers/net/wireless/rtlwifi/rtl8821ae/hw.c
@@ -3672,8 +3672,9 @@ static void rtl8821ae_update_hal_rate_mask(struct ieee80211_hw *hw,
 		mac->opmode == NL80211_IFTYPE_ADHOC)
 		macid = sta->aid + 1;
 	if (wirelessmode == WIRELESS_MODE_N_5G ||
-	    wirelessmode == WIRELESS_MODE_AC_5G)
-		ratr_bitmap = sta->supp_rates[NL80211_BAND_5GHZ];
+	    wirelessmode == WIRELESS_MODE_AC_5G ||
+	    wirelessmode == WIRELESS_MODE_A)
+		ratr_bitmap = sta->supp_rates[NL80211_BAND_5GHZ]<<4;
 	else
 		ratr_bitmap = sta->supp_rates[NL80211_BAND_2GHZ];
 
-- 
2.1.2

--
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 related

* Re: [PATCH 3/3] sh_eth: Fix dma mapping issue
From: Sergei Shtylyov @ 2014-11-25 20:07 UTC (permalink / raw)
  To: Simon Horman
  Cc: Yoshihiro Kaneko, netdev, David S. Miller, Magnus Damm, linux-sh
In-Reply-To: <20141117040935.GA10017@verge.net.au>

Hello.

On 11/17/2014 07:09 AM, Simon Horman wrote:

>>> From: Mitsuhiro Kimura <mitsuhiro.kimura.kc@renesas.com>

>>> When CONFIG_DMA_API_DEBUG=y, many DMA error messages reports.
>>> In order to use DMA debug, This patch fix following issues.

>>> Issue 1:
>>> If dma_mapping_error function is not called appropriately after
>>> DMA mapping, DMA debug will report error message when DMA unmap
>>> function is called.

>>> Issue 2:
>>> If skb_reserve function is called after DMA mapping, the relationship
>>> between mapping addr and mapping size will be broken.
>>> In this case, DMA debug will report error messages when DMA sync
>>> function and DMA unmap function are called.

>>> Issue 3:
>>> If the size of frame data is less than ETH_ZLEN, the size is resized
>>> to ETH_ZLEN after DMA map function is called.
>>> In the TX skb freeing function, dma unmap function is called with that
>>> resized value. So, unmap size error will reported.

>>> Issue 4:
>>> In the rx function, DMA map function is called without DMA unmap function
>>> is called for RX skb reallocating.
>>> It will case the DMA debug error that number of debug entry is full and
>>> DMA debug logic is stopped.

>>     The rule of thumb is "fix one issue per patch". Please split accordingly.

>>> Signed-off-by: Mitsuhiro Kimura <mitsuhiro.kimura.kc@renesas.com>
>>> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>

>>     Thanks for beating me to it. Fixing these issues has been on my agenda
>> for a long time... :-)

> as this patch is somewhat involved and as you have pointed out needs a bit
> of work I'm wondering if you could take it over.

    Perhaps I could... but I'm busy with other stuff... not sure when can I 
get to it.

[...]

WBR, Sergei


^ permalink raw reply

* Re: [PATCH 1/2 V2 3.18] rtlwifi: rtl8821ae: Fix 5G detection problem
From: Sergei Shtylyov @ 2014-11-25 20:10 UTC (permalink / raw)
  To: Larry Finger, linville; +Cc: linux-wireless, netdev, Valerio Passini
In-Reply-To: <1416945727-10743-1-git-send-email-Larry.Finger@lwfinger.net>

Hello.

On 11/25/2014 11:02 PM, Larry Finger wrote:

> The changes associated with moving this driver from staging to the regular
> tree missed one section setting the allowable rates for the 5GHz band.

> This patch is needed to fix the regression reported in Bug #88811
> (https://bugzilla.kernel.org/show_bug.cgi?id=88811).

> Reported-by: Valerio Passini <valerio.passini@unicam.it>
> Tested-by: Valerio Passini <valerio.passini@unicam.it>
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> Cc: Valerio Passini <valerio.passini@unicam.it>
> ---

> V2 - removed extra parentheses as suggested by John Linville.

    He's also suggested spaces around << in his example

> ---
>   drivers/net/wireless/rtlwifi/rtl8821ae/hw.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/rtlwifi/rtl8821ae/hw.c b/drivers/net/wireless/rtlwifi/rtl8821ae/hw.c
> index 310d316..18f34f7 100644
> --- a/drivers/net/wireless/rtlwifi/rtl8821ae/hw.c
> +++ b/drivers/net/wireless/rtlwifi/rtl8821ae/hw.c
> @@ -3672,8 +3672,9 @@ static void rtl8821ae_update_hal_rate_mask(struct ieee80211_hw *hw,
>   		mac->opmode == NL80211_IFTYPE_ADHOC)
>   		macid = sta->aid + 1;
>   	if (wirelessmode == WIRELESS_MODE_N_5G ||
> -	    wirelessmode == WIRELESS_MODE_AC_5G)
> -		ratr_bitmap = sta->supp_rates[NL80211_BAND_5GHZ];
> +	    wirelessmode == WIRELESS_MODE_AC_5G ||
> +	    wirelessmode == WIRELESS_MODE_A)
> +		ratr_bitmap = sta->supp_rates[NL80211_BAND_5GHZ]<<4;

    The kernel style generally assumes spaces around operators, and it would 
only be consistent with the other code in the vicinity if you added them...

WBR, Sergei

^ permalink raw reply

* Re: [PATCH v4 0/7] kernel tinification: optionally compile out splice family of syscalls (splice, vmsplice, tee and sendfile)
From: Pieter Smith @ 2014-11-25 20:11 UTC (permalink / raw)
  To: David Miller
  Cc: josh-iaAMLnmF4UmaiuxdJuQwMA, rdunlap-wEGCiKHe2LqWVfeAwA7xHQ,
	alexander.h.duyck-ral2JQCrhuEAvxtiuMwx3w,
	viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn, ast-uqk4Ao+rVK5Wk0Htik3J/w,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	beber-2YnHqweIUXrk1uMJSBkQmQ,
	catalina.mocanu-Re5JQEeQqe8AvxtiuMwx3w,
	dborkman-H+wXaHxf7aLQT0dZR+AlfA, edumazet-hpIqsD4AKlfQT0dZR+AlfA,
	ebiederm-aS9lmoZGLiVWk0Htik3J/w, fabf-AgBVmzD5pcezQB+pC5nmwQ,
	fuse-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	geert-Td1EMuHUCqxL1ZNQvxDV9g, hughd-hpIqsD4AKlfQT0dZR+AlfA,
	iulia.manda21-Re5JQEeQqe8AvxtiuMwx3w, JBeulich-IBi9RG/b67k,
	bfields-uC3wQj2KruNg9hUCZPvPmw, jlayton-vpEMnDpepFuMZCB2o+C8xQ,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA, mcgrof-IBi9RG/b67k,
	mattst88-Re5JQEeQqe8AvxtiuMwx3w, mgorman-l3A5Bk7waGM,
	mst-H+wXaHxf7aLQT0dZR+AlfA, miklos-sUDqSbJrdHQHWmgEVkV9KA,
	netdev-u79uwXL29TY76Z2rM5mHXA, oleg-H+wXaHxf7aLQT0dZR+AlfA,
	Paul.Durrant-Sxgqhf6Nn4DQT0dZR+AlfA,
	paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
	pefoley2-lY0TAiDIAFlBDgjK7y7TUQ, tgraf-G/eBtMaohhA,
	therbert-hpIqsD4AKlfQT0dZR+AlfA,
	trond.myklebust-7I+n7zu2hftEKMMhf/gKZA,
	willemb-hpIqsD4AKlfQT0dZR+AlfA,
	xiaoguangrong-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8
In-Reply-To: <20141125.140441.401150380839514113.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>

On Tue, Nov 25, 2014 at 02:04:41PM -0500, David Miller wrote:
> From: josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org
> Date: Tue, 25 Nov 2014 10:53:10 -0800
> 
> > It's not a "slippery slope"; it's been our standard practice for ages.
> 
> We've never put an entire class of generic system calls behind
> a config option.

I would have loved to make them optional individually, but they all are
semantic variations of the same thing: Moving data between fd's without that
data passing through userspace. It therefore isn't surprising that these
syscalls share an underlying entanglement of code (which is where the bulk of
the space saving is to be had).

What a tiny product developer should be asking himself, is: "Do I really need
to efficiently move data between file descriptors?". If the answer no, he can
disable CONFIG_SYSCALL_SPLICE to squeeze an extra 8KB out of his kernel.

^ permalink raw reply

* Re: [PATCH] tg3: fix ring init when there are more TX than RX channels
From: David Miller @ 2014-11-25 20:19 UTC (permalink / raw)
  To: cascardo; +Cc: netdev, prashant, mchan
In-Reply-To: <20141125195721.GB3933@oc0812247204.br.ibm.com>

From: cascardo@linux.vnet.ibm.com
Date: Tue, 25 Nov 2014 17:57:21 -0200

> On Tue, Nov 25, 2014 at 02:33:52PM -0500, David Miller wrote:
>> From: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
>> Date: Tue, 25 Nov 2014 14:21:11 -0200
>> 
>> > @@ -8563,7 +8563,8 @@ static int tg3_init_rings(struct tg3 *tp)
>> >  		if (tnapi->rx_rcb)
>> >  			memset(tnapi->rx_rcb, 0, TG3_RX_RCB_RING_BYTES(tp));
>> >  
>> > -		if (tg3_rx_prodring_alloc(tp, &tnapi->prodring)) {
>> > +		if (tnapi->prodring.rx_std &&
>> > +		    tg3_rx_prodring_alloc(tp, &tnapi->prodring)) {
>> >  			tg3_free_rings(tp);
>> >  			return -ENOMEM;
>> 
>> Maybe a better test is "i < tp->rxq_cnt"?  This is what is used in
>> tg3_mem_rx_acquire() to determine if tg3_rx_prodring_init() happens.
>> 
> 
> Well, what we have in net-next/master does:
> 
> 	for (i = 0; i < limit; i++) {
> 		struct tg3_napi *tnapi = &tp->napi[i];
> 
> 		if (tg3_rx_prodring_init(tp, &tnapi->prodring))
> 			goto err_out;
> 
> where limit is:
> 
> 	limit = tp->rxq_cnt;
> 
> 	if (tg3_flag(tp, ENABLE_RSS))
> 		limit++;
> 
> So, I thought that, instead of rewriting that same logic in
> tg3_init_rings, that we should better just check that the rx_std has
> been allocated, which is going to happen in tg3_rx_prodring_init.
> 
> So, the alternative would be:

Ok, come to think of it, your original test is fine and I've applied
that patch and queued it up for -stable.

Thanks.

^ permalink raw reply

* Re: [PATCH v6 5/5] patch to allow arm cross-compile
From: Alexander Duyck @ 2014-11-25 20:29 UTC (permalink / raw)
  To: Joe Perches, Russell King
  Cc: linux-arch, netdev, linux-kernel, mathieu.desnoyers, peterz, benh,
	heiko.carstens, mingo, mikey, donald.c.skidmore, matthew.vick,
	geert, jeffrey.t.kirsher, romieu, paulmck, nic_swsd, arnd,
	will.deacon, michael, tony.luck, torvalds, oleg, schwidefsky,
	fweisbec, davem
In-Reply-To: <1416944122.8358.10.camel@perches.com>


On 11/25/2014 11:35 AM, Joe Perches wrote:
> (adding Russell King)
>
> On Tue, 2014-11-25 at 10:33 -0800, Alexander Duyck wrote:
>> ---
>>   arch/arm/kernel/asm-offsets.c |    4 ----
>>   1 file changed, 4 deletions(-)
>>
>> diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c
>> index 2d2d608..6f8e79c 100644
>> --- a/arch/arm/kernel/asm-offsets.c
>> +++ b/arch/arm/kernel/asm-offsets.c
>> @@ -49,10 +49,6 @@
>>   #error Your compiler is too buggy; it is known to miscompile kernels.
>>   #error    Known good compilers: 3.3, 4.x
>>   #endif
>> -#if GCC_VERSION >= 40800 && GCC_VERSION < 40803
>> -#error Your compiler is too buggy; it is known to miscompile kernels
>> -#error and result in filesystem corruption and oopses.
>> -#endif
>>   #endif

Sorry, that was for my testing only.  I will resubmit with that patch 
dropped.

Thanks,

Alex

^ permalink raw reply

* [PATCH 1/2 V3 3.18] rtlwifi: rtl8821ae: Fix 5G detection problem
From: Larry Finger @ 2014-11-25 20:28 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, Larry Finger, netdev, Valerio Passini

The changes associated with moving this driver from staging to the regular
tree missed one section setting the allowable rates for the 5GHz band.

This patch is needed to fix the regression reported in Bug #88811
(https://bugzilla.kernel.org/show_bug.cgi?id=88811).

Reported-by: Valerio Passini <valerio.passini@unicam.it>
Tested-by: Valerio Passini <valerio.passini@unicam.it>
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
Cc: Valerio Passini <valerio.passini@unicam.it>
---

V2 - removed extra parentheses as suggested by John Linville.
V3 - Add spaces around the << operator
---
 drivers/net/wireless/rtlwifi/rtl8821ae/hw.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/rtlwifi/rtl8821ae/hw.c b/drivers/net/wireless/rtlwifi/rtl8821ae/hw.c
index 310d316..18f34f7 100644
--- a/drivers/net/wireless/rtlwifi/rtl8821ae/hw.c
+++ b/drivers/net/wireless/rtlwifi/rtl8821ae/hw.c
@@ -3672,8 +3672,9 @@ static void rtl8821ae_update_hal_rate_mask(struct ieee80211_hw *hw,
 		mac->opmode == NL80211_IFTYPE_ADHOC)
 		macid = sta->aid + 1;
 	if (wirelessmode == WIRELESS_MODE_N_5G ||
-	    wirelessmode == WIRELESS_MODE_AC_5G)
-		ratr_bitmap = sta->supp_rates[NL80211_BAND_5GHZ];
+	    wirelessmode == WIRELESS_MODE_AC_5G ||
+	    wirelessmode == WIRELESS_MODE_A)
+		ratr_bitmap = sta->supp_rates[NL80211_BAND_5GHZ] << 4;
 	else
 		ratr_bitmap = sta->supp_rates[NL80211_BAND_2GHZ];
 
-- 
2.1.2

^ permalink raw reply related

* [PATCH v7 0/4] arch: Add lightweight memory barriers for coherent memory access
From: Alexander Duyck @ 2014-11-25 20:35 UTC (permalink / raw)
  To: linux-arch, netdev, linux-kernel
  Cc: mathieu.desnoyers, peterz, benh, heiko.carstens, mingo, mikey,
	linux, donald.c.skidmore, matthew.vick, geert, jeffrey.t.kirsher,
	romieu, paulmck, nic_swsd, arnd, will.deacon, michael, tony.luck,
	torvalds, oleg, schwidefsky, fweisbec, davem

These patches introduce two new primitives for synchronizing cache coherent
memory writes and reads.  These two new primitives are:

	dma_rmb()
	dma_wmb()

The first patch cleans up some unnecessary overhead related to the
definition of read_barrier_depends, smp_read_barrier_depends, and comments
related to the barrier.

The second patch adds the primitives for the applicable architectures and
asm-generic.

The third patch adds the barriers to r8169 which turns out to be a good
example of where the new barriers might be useful as they have full
rmb()/wmb() barriers ordering accesses to the descriptors and the DescOwn
bit.

The fourth patch adds support for coherent_rmb() to the Intel fm10k, igb,
and ixgbe drivers.  Testing with the ixgbe driver has shown a processing
time reduction of at least 7ns per 64B frame on a Core i7-4930K.

This patch series is essentially the v7 for:
v4-6:	Add lightweight memory barriers for coherent memory access
v3:	Add lightweight memory barriers fast_rmb() and fast_wmb()
v2:	Introduce load_acquire() and store_release()
v1:	Introduce read_acquire()

The key changes in this patch series versus the earlier patches are:
v7:
	- Dropped test/debug patch that was accidentally slipped in
v6:
	- Replaced "memory based device I/O" with "consistent memory" in
	  docs
	- Added reference to DMA-API.txt to explain consistent memory
v5:
	- Renamed barriers dma_rmb and dma_wmb
	- Undid smp_wmb changes in x86 and PowerPC
	- Defined smp_rmb as __lwsync for SMP case on PowerPC
v4:
	- Renamed barriers coherent_rmb and coherent_wmb
	- Added smp_lwsync for use in smp_load_acquire/smp_store_release
v3:
	- Moved away from acquire()/store() and instead focused on barriers
	- Added cleanup of read_barrier_depends
	- Added change in r8169 to fix cur_tx/DescOwn ordering
	- Simplified changes to just replacing/moving barriers in r8169
	- Added update to documentation with code example
v2:
	- Renamed read_acquire() to be consistent with smp_load_acquire()
	- Changed barrier used to be consistent with smp_load_acquire()
	- Updated PowerPC code to use __lwsync based on IBM article
	- Added store_release() as this is a viable use case for drivers
	- Added r8169 patch which is able to fully use primitives
	- Added fm10k/igb/ixgbe patch which is able to test performance

---

Alexander Duyck (4):
      arch: Cleanup read_barrier_depends() and comments
      arch: Add lightweight memory barriers dma_rmb() and dma_wmb()
      r8169: Use dma_rmb() and dma_wmb() for DescOwn checks
      fm10k/igb/ixgbe: Use dma_rmb on Rx descriptor reads


 Documentation/memory-barriers.txt             |   42 +++++++++++++++
 arch/alpha/include/asm/barrier.h              |   51 ++++++++++++++++++
 arch/arm/include/asm/barrier.h                |    4 +
 arch/arm64/include/asm/barrier.h              |    3 +
 arch/blackfin/include/asm/barrier.h           |   51 ++++++++++++++++++
 arch/ia64/include/asm/barrier.h               |   25 ++++-----
 arch/metag/include/asm/barrier.h              |   19 ++++---
 arch/mips/include/asm/barrier.h               |   61 ++--------------------
 arch/powerpc/include/asm/barrier.h            |   19 ++++---
 arch/s390/include/asm/barrier.h               |    7 ++-
 arch/sparc/include/asm/barrier_64.h           |    7 ++-
 arch/x86/include/asm/barrier.h                |   70 ++++---------------------
 arch/x86/um/asm/barrier.h                     |   20 ++++---
 drivers/net/ethernet/intel/fm10k/fm10k_main.c |    6 +-
 drivers/net/ethernet/intel/igb/igb_main.c     |    6 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    9 +--
 drivers/net/ethernet/realtek/r8169.c          |   29 ++++++++--
 include/asm-generic/barrier.h                 |    8 +++
 18 files changed, 258 insertions(+), 179 deletions(-)

--

^ permalink raw reply

* [PATCH v7 3/4] r8169: Use dma_rmb() and dma_wmb() for DescOwn checks
From: Alexander Duyck @ 2014-11-25 20:35 UTC (permalink / raw)
  To: linux-arch, netdev, linux-kernel
  Cc: mathieu.desnoyers, peterz, benh, heiko.carstens, mingo, mikey,
	linux, donald.c.skidmore, matthew.vick, geert, jeffrey.t.kirsher,
	romieu, paulmck, nic_swsd, arnd, will.deacon, michael, tony.luck,
	torvalds, oleg, schwidefsky, fweisbec, davem
In-Reply-To: <20141125203310.8240.27370.stgit@ahduyck-server>

The r8169 use a pair of wmb() calls when setting up the descriptor rings.
The first is to synchronize the descriptor data with the descriptor status,
and the second is to synchronize the descriptor status with the use of the
MMIO doorbell to notify the device that descriptors are ready.  This can
come at a heavy price on some systems, and is not really necessary on
systems such as x86 as a simple barrier() would suffice to order store/store
accesses.  As such we can replace the first memory barrier with
dma_wmb() to reduce the cost for these accesses.

In addition the r8169 uses a rmb() to prevent compiler optimization in the
cleanup paths, however by moving the barrier down a few lines and replacing
it with a dma_rmb() we should be able to use it to guarantee
descriptor accesses do not occur until the device has updated the DescOwn
bit from its end.

One last change I made is to move the update of cur_tx in the xmit path to
after the wmb.  This way we can guarantee the device and all CPUs should
see the DescOwn update before they see the cur_tx value update.

Cc: Realtek linux nic maintainers <nic_swsd@realtek.com>
Cc: Francois Romieu <romieu@fr.zoreil.com>
Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
 drivers/net/ethernet/realtek/r8169.c |   29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index cf154f7..39e9796 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -6601,6 +6601,9 @@ static inline void rtl8169_mark_to_asic(struct RxDesc *desc, u32 rx_buf_sz)
 {
 	u32 eor = le32_to_cpu(desc->opts1) & RingEnd;
 
+	/* Force memory writes to complete before releasing descriptor */
+	dma_wmb();
+
 	desc->opts1 = cpu_to_le32(DescOwn | eor | rx_buf_sz);
 }
 
@@ -6608,7 +6611,6 @@ static inline void rtl8169_map_to_asic(struct RxDesc *desc, dma_addr_t mapping,
 				       u32 rx_buf_sz)
 {
 	desc->addr = cpu_to_le64(mapping);
-	wmb();
 	rtl8169_mark_to_asic(desc, rx_buf_sz);
 }
 
@@ -7077,16 +7079,18 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
 
 	skb_tx_timestamp(skb);
 
-	wmb();
+	/* Force memory writes to complete before releasing descriptor */
+	dma_wmb();
 
 	/* Anti gcc 2.95.3 bugware (sic) */
 	status = opts[0] | len | (RingEnd * !((entry + 1) % NUM_TX_DESC));
 	txd->opts1 = cpu_to_le32(status);
 
-	tp->cur_tx += frags + 1;
-
+	/* Force all memory writes to complete before notifying device */
 	wmb();
 
+	tp->cur_tx += frags + 1;
+
 	RTL_W8(TxPoll, NPQ);
 
 	mmiowb();
@@ -7185,11 +7189,16 @@ static void rtl_tx(struct net_device *dev, struct rtl8169_private *tp)
 		struct ring_info *tx_skb = tp->tx_skb + entry;
 		u32 status;
 
-		rmb();
 		status = le32_to_cpu(tp->TxDescArray[entry].opts1);
 		if (status & DescOwn)
 			break;
 
+		/* This barrier is needed to keep us from reading
+		 * any other fields out of the Tx descriptor until
+		 * we know the status of DescOwn
+		 */
+		dma_rmb();
+
 		rtl8169_unmap_tx_skb(&tp->pci_dev->dev, tx_skb,
 				     tp->TxDescArray + entry);
 		if (status & LastFrag) {
@@ -7284,11 +7293,16 @@ static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp, u32 budget
 		struct RxDesc *desc = tp->RxDescArray + entry;
 		u32 status;
 
-		rmb();
 		status = le32_to_cpu(desc->opts1) & tp->opts1_mask;
-
 		if (status & DescOwn)
 			break;
+
+		/* This barrier is needed to keep us from reading
+		 * any other fields out of the Rx descriptor until
+		 * we know the status of DescOwn
+		 */
+		dma_rmb();
+
 		if (unlikely(status & RxRES)) {
 			netif_info(tp, rx_err, dev, "Rx ERROR. status = %08x\n",
 				   status);
@@ -7350,7 +7364,6 @@ process_pkt:
 		}
 release_descriptor:
 		desc->opts2 = 0;
-		wmb();
 		rtl8169_mark_to_asic(desc, rx_buf_sz);
 	}
 

^ permalink raw reply related

* [PATCH v7 4/4] fm10k/igb/ixgbe: Use dma_rmb on Rx descriptor reads
From: Alexander Duyck @ 2014-11-25 20:35 UTC (permalink / raw)
  To: linux-arch, netdev, linux-kernel
  Cc: mathieu.desnoyers, peterz, benh, heiko.carstens, mingo, mikey,
	linux, donald.c.skidmore, matthew.vick, geert, jeffrey.t.kirsher,
	romieu, paulmck, nic_swsd, arnd, will.deacon, michael, tony.luck,
	torvalds, oleg, schwidefsky, fweisbec, davem
In-Reply-To: <20141125203310.8240.27370.stgit@ahduyck-server>

This change makes it so that dma_rmb is used when reading the Rx
descriptor.  The advantage of dma_rmb is that it allows for a much
lower cost barrier on x86, powerpc, arm, and arm64 architectures than a
traditional memory barrier when dealing with reads that only have to
synchronize to coherent memory.

In addition I have updated the code so that it just checks to see if any
bits have been set instead of just the DD bit since the DD bit will always
be set as a part of a descriptor write-back so we just need to check for a
non-zero value being present at that memory location rather than just
checking for any specific bit.  This allows the code itself to appear much
cleaner and allows the compiler more room to optimize.

Cc: Matthew Vick <matthew.vick@intel.com>
Cc: Don Skidmore <donald.c.skidmore@intel.com>
Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_main.c |    6 +++---
 drivers/net/ethernet/intel/igb/igb_main.c     |    6 +++---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    9 ++++-----
 3 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_main.c b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
index 73457ed..b348178 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_main.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
@@ -620,14 +620,14 @@ static bool fm10k_clean_rx_irq(struct fm10k_q_vector *q_vector,
 
 		rx_desc = FM10K_RX_DESC(rx_ring, rx_ring->next_to_clean);
 
-		if (!fm10k_test_staterr(rx_desc, FM10K_RXD_STATUS_DD))
+		if (!rx_desc->d.staterr)
 			break;
 
 		/* This memory barrier is needed to keep us from reading
 		 * any other fields out of the rx_desc until we know the
-		 * RXD_STATUS_DD bit is set
+		 * descriptor has been written back
 		 */
-		rmb();
+		dma_rmb();
 
 		/* retrieve a buffer from the ring */
 		skb = fm10k_fetch_rx_buffer(rx_ring, rx_desc, skb);
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 3c02216..ecf38cf 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -6922,14 +6922,14 @@ static bool igb_clean_rx_irq(struct igb_q_vector *q_vector, const int budget)
 
 		rx_desc = IGB_RX_DESC(rx_ring, rx_ring->next_to_clean);
 
-		if (!igb_test_staterr(rx_desc, E1000_RXD_STAT_DD))
+		if (!rx_desc->wb.upper.status_error)
 			break;
 
 		/* This memory barrier is needed to keep us from reading
 		 * any other fields out of the rx_desc until we know the
-		 * RXD_STAT_DD bit is set
+		 * descriptor has been written back
 		 */
-		rmb();
+		dma_rmb();
 
 		/* retrieve a buffer from the ring */
 		skb = igb_fetch_rx_buffer(rx_ring, rx_desc, skb);
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index c19af9f..5bc9650 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -2006,15 +2006,14 @@ static int ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
 
 		rx_desc = IXGBE_RX_DESC(rx_ring, rx_ring->next_to_clean);
 
-		if (!ixgbe_test_staterr(rx_desc, IXGBE_RXD_STAT_DD))
+		if (!rx_desc->wb.upper.status_error)
 			break;
 
-		/*
-		 * This memory barrier is needed to keep us from reading
+		/* This memory barrier is needed to keep us from reading
 		 * any other fields out of the rx_desc until we know the
-		 * RXD_STAT_DD bit is set
+		 * descriptor has been written back
 		 */
-		rmb();
+		dma_rmb();
 
 		/* retrieve a buffer from the ring */
 		skb = ixgbe_fetch_rx_buffer(rx_ring, rx_desc);

^ 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