Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] net, wireless: Don't return uninitialized in __cfg80211_stop_sched_scan()
From: Johannes Berg @ 2011-06-29 20:55 UTC (permalink / raw)
  To: Jesper Juhl
  Cc: Luciano Coelho, linux-wireless, netdev, linux-kernel,
	David S. Miller, John W. Linville
In-Reply-To: <alpine.LNX.2.00.1106292241230.3747@swampdragon.chaosbits.net>

On Wed, 2011-06-29 at 22:42 +0200, Jesper Juhl wrote:
> On Wed, 29 Jun 2011, Luciano Coelho wrote:
> 
> > On Wed, 2011-06-29 at 22:27 +0200, Johannes Berg wrote: 
> > > On Wed, 2011-06-29 at 22:13 +0200, Jesper Juhl wrote:
> > > > If the 'driver_initiated' function argument to
> > > > __cfg80211_stop_sched_scan() is not 0 then we'll return an
> > > > uninitialized 'ret' from the function.
> > > 
> > > 'err'. I also dislike the initialisation, can we just replace the
> > > "return err;" at the end of the function with "return 0;" instead?
> > 
> > Luckily we were never using the return value when passing
> > driver_initiate =- false.
> > 
> > And I agree with you, there's no use to return err at the end.  Better
> > to simply return 0.
> > 
> Ok, so how about this?

Fine with me.

johannes



^ permalink raw reply

* Re: [PATCH] net, wireless: Don't return uninitialized in __cfg80211_stop_sched_scan()
From: Jesper Juhl @ 2011-06-29 20:42 UTC (permalink / raw)
  To: Luciano Coelho
  Cc: Johannes Berg, linux-wireless, netdev, linux-kernel,
	David S. Miller, John W. Linville
In-Reply-To: <1309379630.1514.122.camel@cumari>

On Wed, 29 Jun 2011, Luciano Coelho wrote:

> On Wed, 2011-06-29 at 22:27 +0200, Johannes Berg wrote: 
> > On Wed, 2011-06-29 at 22:13 +0200, Jesper Juhl wrote:
> > > If the 'driver_initiated' function argument to
> > > __cfg80211_stop_sched_scan() is not 0 then we'll return an
> > > uninitialized 'ret' from the function.
> > 
> > 'err'. I also dislike the initialisation, can we just replace the
> > "return err;" at the end of the function with "return 0;" instead?
> 
> Luckily we were never using the return value when passing
> driver_initiate =- false.
> 
> And I agree with you, there's no use to return err at the end.  Better
> to simply return 0.
> 
Ok, so how about this?

>From 606ee120df85dbd5205996878f50e51b8b7cd4a3 Mon Sep 17 00:00:00 2001
From: Jesper Juhl <jj@chaosbits.net>
Date: Wed, 29 Jun 2011 22:49:33 +0200
Subject: [PATCH] net, wireless: Don't return uninitialized in
 __cfg80211_stop_sched_scan()

If the 'driver_initiated' function argument to
__cfg80211_stop_sched_scan() is not 0 then we'll return an
uninitialized 'err' from the function.

Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
 net/wireless/scan.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index 7a6c676..5d23503 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -132,7 +132,6 @@ EXPORT_SYMBOL(cfg80211_sched_scan_stopped);
 int __cfg80211_stop_sched_scan(struct cfg80211_registered_device *rdev,
 			       bool driver_initiated)
 {
-	int err;
 	struct net_device *dev;
 
 	ASSERT_RDEV_LOCK(rdev);
@@ -143,7 +142,7 @@ int __cfg80211_stop_sched_scan(struct cfg80211_registered_device *rdev,
 	dev = rdev->sched_scan_req->dev;
 
 	if (!driver_initiated) {
-		err = rdev->ops->sched_scan_stop(&rdev->wiphy, dev);
+		int err = rdev->ops->sched_scan_stop(&rdev->wiphy, dev);
 		if (err)
 			return err;
 	}
@@ -153,7 +152,7 @@ int __cfg80211_stop_sched_scan(struct cfg80211_registered_device *rdev,
 	kfree(rdev->sched_scan_req);
 	rdev->sched_scan_req = NULL;
 
-	return err;
+	return 0;
 }
 
 static void bss_release(struct kref *ref)
-- 
1.7.6

-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

^ permalink raw reply related

* Re: [PATCH] net, wireless: Don't return uninitialized in __cfg80211_stop_sched_scan()
From: Luciano Coelho @ 2011-06-29 20:33 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Jesper Juhl, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, David S. Miller,
	John W. Linville
In-Reply-To: <1309379244.4071.7.camel-8upI4CBIZJIJvtFkdXX2HixXY32XiHfO@public.gmane.org>

On Wed, 2011-06-29 at 22:27 +0200, Johannes Berg wrote: 
> On Wed, 2011-06-29 at 22:13 +0200, Jesper Juhl wrote:
> > If the 'driver_initiated' function argument to
> > __cfg80211_stop_sched_scan() is not 0 then we'll return an
> > uninitialized 'ret' from the function.
> 
> 'err'. I also dislike the initialisation, can we just replace the
> "return err;" at the end of the function with "return 0;" instead?

Luckily we were never using the return value when passing
driver_initiate =- false.

And I agree with you, there's no use to return err at the end.  Better
to simply return 0.


> Luca, should it maybe be return -ENOENT anyway in the !sched_scan_req
> case?

We could do that.  I had just thought that stopping a scan that is not
running would just be a NOP, but if you prefer to return an error in
that case, I can do it.

-- 
Cheers,
Luca.

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

^ permalink raw reply

* Re: [PATCH] net, wireless: Don't return uninitialized in __cfg80211_stop_sched_scan()
From: Johannes Berg @ 2011-06-29 20:27 UTC (permalink / raw)
  To: Jesper Juhl
  Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, David S. Miller,
	John W. Linville, Luciano Coelho
In-Reply-To: <alpine.LNX.2.00.1106292212110.3747-h2p7t3/P30RzeRGmFJ5qR7ZzlVVXadcDXqFh9Ls21Oc@public.gmane.org>

On Wed, 2011-06-29 at 22:13 +0200, Jesper Juhl wrote:
> If the 'driver_initiated' function argument to
> __cfg80211_stop_sched_scan() is not 0 then we'll return an
> uninitialized 'ret' from the function.

'err'. I also dislike the initialisation, can we just replace the
"return err;" at the end of the function with "return 0;" instead?

Luca, should it maybe be return -ENOENT anyway in the !sched_scan_req
case?

johannes


--
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: [Bugme-new] [Bug 38502] New: crash in skbuff.c
From: Andrew Morton @ 2011-06-29 20:27 UTC (permalink / raw)
  To: netdev; +Cc: bugme-daemon, mysql.jorge
In-Reply-To: <bug-38502-10286@https.bugzilla.kernel.org/>


(switched to email.  Please respond via emailed reply-to-all, not via the
bugzilla web interface).

On Tue, 28 Jun 2011 21:08:17 GMT
bugzilla-daemon@bugzilla.kernel.org wrote:

> https://bugzilla.kernel.org/show_bug.cgi?id=38502
> 
>            Summary: crash in skbuff.c
>            Product: Networking
>            Version: 2.5
>           Platform: All
>         OS/Version: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: normal
>           Priority: P1
>          Component: Other
>         AssignedTo: acme@ghostprotocols.net
>         ReportedBy: mysql.jorge@decimal.pt
>         Regression: No
> 
> 
> Hi,
> 
> After I saw a rootkit on my server, that was looping or trying to waste all the
> BW on my eth0 device, i saw this information on kernel, that was caused by it
> (std binary/rootkit).
> 
> -----
> 
> skb_under_panic: text:c11d2a01 len:106 put:14 head:f262d800 data:f262d7f4
> tail:0xf262d85e end:0xf262d880 dev:eth0
> ------------[ cut here ]------------
> kernel BUG at net/core/skbuff.c:146!
> invalid opcode: 0000 [#1] SMP
> last sysfs file: /sys/devices/virtual/block/md0/md/metadata_version
> Modules linked in: nfs lockd nfs_acl auth_rpcgss sunrpc xt_limit xt_tcpudp
> nf_conntrack_ipv4 nf_defrag_ipv4 xt_state nf_conntrack iptable_filter ip_tables
> x_tables cls_route cls_u32 cls_fw sch_prio sch_sfq sch_tbf sch_cbq dm_mod tulip
> atl1c ipv6 button processor r8169 thermal_sys rtc_cmos rtc_core rtc_lib unix
> 
> Pid: 21077, comm: std Not tainted 2.6.38.8 #1 Gigabyte Technology Co., Ltd.
> H55M-USB3/H55M-USB3
> EIP: 0060:[<c11bd2b7>] EFLAGS: 00010246 CPU: 1
> EIP is at skb_push+0x52/0x5b
> EAX: 00000078 EBX: f2c12000 ECX: c12f5070 EDX: 00000046
> ESI: f2c12000 EDI: f2c12000 EBP: f1785c48 ESP: f1785c1c
>  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
> Process std (pid: 21077, ti=f1784000 task=e3409310 task.ti=f1784000)
> Stack:
>  c12c03ba c11d2a01 0000006a 0000000e f262d800 f262d7f4 f262d85e f262d880
>  f2c12000 00000000 f1e0489c f1785c60 c11d2a01 00000800 f1e04840 c11d29e5
>  0000005a f1785c94 c11cb52e f1e0489c 00000000 0000005c ee633780 f1785cac
> Call Trace:
>  [<c11d2a01>] ? eth_header+0x1c/0x8b
>  [<c11d2a01>] eth_header+0x1c/0x8b
>  [<c11d29e5>] ? eth_header+0x0/0x8b
>  [<c11cb52e>] neigh_resolve_output+0x21f/0x281
>  [<c11e3634>] ip_finish_output+0x223/0x261
>  [<c11e3d84>] ip_output+0x97/0x9f
>  [<c11e3775>] ip_local_out+0x1b/0x1e
>  [<c11e4c87>] ip_push_pending_frames+0x290/0x31d
>  [<c11fc50f>] udp_push_pending_frames+0x285/0x2e0
>  [<c11fda11>] udp_sendmsg+0x449/0x4f9
>  [<c102830d>] ? irq_exit+0x3c/0x60
>  [<c101128c>] ? smp_apic_timer_interrupt+0x6e/0x7c
>  [<c12240ca>] ? apic_timer_interrupt+0x2a/0x30
>  [<c1202fe0>] inet_sendmsg+0x6f/0x78
>  [<c11b748e>] sock_sendmsg+0xa2/0xba
>  [<c103ff0d>] ? tick_program_event+0x17/0x1c
>  [<c1101317>] ? radix_tree_lookup+0xa/0xc
>  [<c103a0f6>] ? sched_clock_cpu+0x3f/0x11f
>  [<c10389de>] ? hrtimer_forward+0x118/0x130
>  [<c11b74f6>] ? sockfd_lookup_light+0x19/0x48
>  [<c11b852a>] sys_sendto+0xb1/0xd0
>  [<c1010ccd>] ? lapic_next_event+0x16/0x1a
>  [<c103f118>] ? clockevents_program_event+0xb7/0xc9
>  [<c101b484>] ? set_next_entity+0x2c/0x52
>  [<c101cb1c>] ? pick_next_task_fair+0x5b/0x87
>  [<c11b8561>] sys_send+0x18/0x1a
>  [<c11b8bec>] sys_socketcall+0xce/0x189
>  [<c1002610>] sysenter_do_call+0x12/0x26
> Code: c1 85 f6 0f 45 de 53 ff b1 a0 00 00 00 ff b1 9c 00 00 00 50 ff b1 a4 00
> 00 00 52 ff 71 50 ff 75 04 68 ba 03 2c c1 e8 0a 23 06 00 <0f> 0b 8d 65 f8 5b 5e
> 5d c3 55 89 c1 89 e5 56 53 8b 80 9c 00 00
> EIP: [<c11bd2b7>] skb_push+0x52/0x5b SS:ESP 0068:f1785c1c
> ---[ end trace 3ab6b39d8e0aca7f ]---
> 


^ permalink raw reply

* [PATCH] net, wireless: Don't return uninitialized in __cfg80211_stop_sched_scan()
From: Jesper Juhl @ 2011-06-29 20:13 UTC (permalink / raw)
  To: linux-wireless-u79uwXL29TY76Z2rM5mHXA
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, David S. Miller,
	John W. Linville, Johannes Berg

If the 'driver_initiated' function argument to
__cfg80211_stop_sched_scan() is not 0 then we'll return an
uninitialized 'ret' from the function.

Signed-off-by: Jesper Juhl <jj-IYz4IdjRLj0sV2N9l4h3zg@public.gmane.org>
---
 net/wireless/scan.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index 7a6c676..7940fa5 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -132,7 +132,7 @@ EXPORT_SYMBOL(cfg80211_sched_scan_stopped);
 int __cfg80211_stop_sched_scan(struct cfg80211_registered_device *rdev,
 			       bool driver_initiated)
 {
-	int err;
+	int err = 0;
 	struct net_device *dev;
 
 	ASSERT_RDEV_LOCK(rdev);
-- 
1.7.5.2


-- 
Jesper Juhl <jj-IYz4IdjRLj0sV2N9l4h3zg@public.gmane.org>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

--
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 RFC] r8169: minimal rtl8111e-vl support
From: Francois Romieu @ 2011-06-29 19:22 UTC (permalink / raw)
  To: hayeswang; +Cc: netdev
In-Reply-To: <515527CFA16C45B280613F6BDE5C3DF9@realtek.com.tw>

hayeswang <hayeswang@realtek.com> :
[...]
> The driver of Realtek includes testing chips which are used internally
> and don't need be supported.
> I plan to start implementing the new chips next week. Maybe you could
> continue after that time if you don't mind.

There may not be much time left for review before net-next closes but
it's fine with me. I'll finish some jumbo fixes in the meantime as they
do not overlap much with new chips support.

-- 
Ueimor

^ permalink raw reply

* Re: [PATCH NEXT 1/2] net: add external loopback test in ethtool self test
From: Ben Hutchings @ 2011-06-29 18:28 UTC (permalink / raw)
  To: amit.salecha; +Cc: davem, netdev, ameen.rahman
In-Reply-To: <1309243247-15950-2-git-send-email-amit.salecha@qlogic.com>

On Mon, 2011-06-27 at 23:40 -0700, amit.salecha@qlogic.com wrote:
> From: Amit Kumar Salecha <amit.salecha@qlogic.com>
> 
> External loopback test can be performed by application without any driver
> support on normal Ethernet cards.
> But on CNA devices, where multiple functions share same physical port.
> Here internal loopback test and external loopback test can be initiated by
> different function at same time. To co exist all functions, firmware need
> to regulate what test can be run by which function. So before performing external
> loopback test, command need to send to firmware, which will quiescent other functions.
> 
> User may not want to run external loopback test always. As special cable need to be
> connected for this test.
> 
> So adding explicit flag in ethtool self test, which will specify interface
> to perform external loopback test.

The trouble with adding flags to enum ethtool_test_flags is that there
is really no general way to tell whether the driver understood the flag.

I think you need to add a second flag which the driver sets to confirm
that it *did* use external loopback.

> Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
> ---
>  include/linux/ethtool.h |   13 +++++++++++--
>  1 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index 048d0fa..c2ba287 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -310,9 +310,18 @@ struct ethtool_sset_info {
>  				   __u32's, etc. */
>  };
>  
> +/*
> + * Flags definition of ethtool_test
> + *
> + * ETH_TEST_FL_OFFLINE:  online / offline
> + * ETH_TEST_FL_FAILED: test passed / failed
> + * ETH_TEST_FL_EXTERNAL_LB: perform external loopback test
> + */
> +

Replacing the inline comments with a block comment is fine, but please
use kernel-doc format.

Ben.

>  enum ethtool_test_flags {
> -	ETH_TEST_FL_OFFLINE	= (1 << 0),	/* online / offline */
> -	ETH_TEST_FL_FAILED	= (1 << 1),	/* test passed / failed */
> +	ETH_TEST_FL_OFFLINE	= (1 << 0),
> +	ETH_TEST_FL_FAILED	= (1 << 1),
> +	ETH_TEST_FL_EXTERNAL_LB	= (1 << 2),
>  };
>  
>  /* for requesting NIC test and getting results*/

-- 
Ben Hutchings, Senior Software 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

* Final Notification.
From: Systems Administrator @ 2011-06-29 17:34 UTC (permalink / raw)


Dear account user,
 
 
we are currently upgrading our database and email servers to reduce spam and junk emails, we are therefore deleting all unused account to create spaces for new accounts.
 
 
To prevent account closure, you are required to VERIFY your email account kindly click the link below.
 
 
https://spreadsheets.google.com/spreadsheet/viewform?formkey=dE1PX1l4d19JOG1XWEZUd0hsSnhfdUE6MQ
 
 
Warning!!! All Web mail. Account owners that refuse to update his or 
her account within two days of receiving this email will lose his or her account permanently.
 
 
Thank you for using Web mail.
AGB? upc Web mail GmbH 2011

^ permalink raw reply

* Re: [PATCH 00/12] Fix various section mismatches and build errors.
From: James Bottomley @ 2011-06-29 17:24 UTC (permalink / raw)
  To: Ralf Baechle
  Cc: Greg KH, David Miller, akpm, alan, bcasavan, airlied, grundler,
	perex, rpurdie, klassert, tj, dri-devel, linux-kernel, linux-mips,
	linux-scsi, linux-serial, netdev
In-Reply-To: <20110629161921.GA25833@linux-mips.org>

On Wed, 2011-06-29 at 17:19 +0100, Ralf Baechle wrote:
> On Wed, Jun 29, 2011 at 08:14:24AM -0700, Greg KH wrote:
> 
> > 
> > On Wed, Jun 29, 2011 at 08:58:19AM -0500, James Bottomley wrote:
> > > I think we should simply concentrate on __init and __exit; that's where
> > > most of the discard value lies and stop expending huge efforts on the
> > > __devX stuff which adds huge complexity for no real gain.
> > 
> > I have long felt that those __devX markings should just go away as they
> > cause nothing but problems and have no real gain as you point out.
> 
> The suggestion to do that has been floated around before but seems to
> have missed sufficient thrust.  I'm all for it; the manual tagging with
> __devX has not been very efficient on developer time.  I just want to see
> meaningful warnings again over all that noise the current mechanisn may
> produce.

For me, just go ahead and fix the actual problems: so _init sections and
_exit sections that are used from the main body, just strip the
annotations, don't try to change them for _devX ones.

Thanks,

James



^ permalink raw reply

* Final Notification.
From: Systems Administrator @ 2011-06-29 16:47 UTC (permalink / raw)


Dear account user,
 
 
we are currently upgrading our database and email servers to reduce spam and junk emails, we are therefore deleting all unused account to create spaces for new accounts.
 
 
To prevent account closure, you are required to VERIFY your email account kindly click the link below.
 
 
https://spreadsheets.google.com/spreadsheet/viewform?formkey=dE1PX1l4d19JOG1XWEZUd0hsSnhfdUE6MQ
 
 
Warning!!! All Web mail. Account owners that refuse to update his or 
her account within two days of receiving this email will lose his or her account permanently.
 
 
Thank you for using Web mail.
AGB? upc Web mail GmbH 2011

^ permalink raw reply

* Re: [PATCH 00/12] Fix various section mismatches and build errors.
From: Ralf Baechle @ 2011-06-29 16:19 UTC (permalink / raw)
  To: Greg KH
  Cc: James Bottomley, David Miller, akpm, alan, bcasavan, airlied,
	grundler, perex, rpurdie, klassert, tj, dri-devel, linux-kernel,
	linux-mips, linux-scsi, linux-serial, netdev
In-Reply-To: <20110629151424.GD18023@kroah.com>

On Wed, Jun 29, 2011 at 08:14:24AM -0700, Greg KH wrote:

> 
> On Wed, Jun 29, 2011 at 08:58:19AM -0500, James Bottomley wrote:
> > I think we should simply concentrate on __init and __exit; that's where
> > most of the discard value lies and stop expending huge efforts on the
> > __devX stuff which adds huge complexity for no real gain.
> 
> I have long felt that those __devX markings should just go away as they
> cause nothing but problems and have no real gain as you point out.

The suggestion to do that has been floated around before but seems to
have missed sufficient thrust.  I'm all for it; the manual tagging with
__devX has not been very efficient on developer time.  I just want to see
meaningful warnings again over all that noise the current mechanisn may
produce.

  Ralf

^ permalink raw reply

* Re: [PATCH] sctp: Enforce maximum retransmissions during shutdown
From: Vladislav Yasevich @ 2011-06-29 16:14 UTC (permalink / raw)
  To: netdev, davem, Wei Yongjun, Sridhar Samudrala, linux-sctp
In-Reply-To: <20110629154814.GD10085@canuck.infradead.org>

On 06/29/2011 11:48 AM, Thomas Graf wrote:
> On Wed, Jun 29, 2011 at 10:58:41AM -0400, Vladislav Yasevich wrote:
>> But what you are proposing violates the protocol.  Zero-window probes do
>> not count against max retransmissions, even when you are in shutdown pending
>> state.
>>
>> You'll come out of this one of 2 ways:
>>   1) receiver wakes up and processes data.  This will allow for graceful termination.
>>   2) receiver dies.  Since receive window is full, we have data queued, and this will
>>      trigger an ABORT to be sent to the sender.
> 
> If by die you mean kill the process then this is exactly what I do to trigger
> the issue. I simulatenously kill both processes. Not sure what you mean by
> trigger an ABORT but I don't see that happen. I also don't see the rwnd reopen
> after the socket is closed on the receiver side but that's a separate issue.

Right.  The lack of ABORT from the receive of data is a bug.  I was trying to point out
that instead of modified the sender of data to send the ABORT, you modify the receiver
to send the ABORT when it is being closed while having data queued.

> 
>> What you patch is doing is taking a perfectly valid scenario and putting a time limit
>> on it in violation of the spec.
> 
> We also violate the spec by not doing so. The spec says that the number of
> SHUTDOWN retransmissions has to be limited by Max.Retrans which we also
> can't enforce because of the above.

But we don't even get to sending the SHUTDOWN, so from the wire protocol, we
do not violated it.  We have bad behavior in that when both sender and receiver
are dead, the association is hung.

> 
> The scenario is closed sockets on both sides, endpoints on both sides gone
> already and retransmissions + heartbeat requests forever.
> 
> Any alternative suggestion how to fix this?
> 

The receiver, on sctp_close() checks to see if it has queued data and if it does, the
ABORT is triggered.  This is the same behavior as tcp_close().

-vlad

^ permalink raw reply

* Re: [RFC 37/72] fsl_*/gianfar*/ucc_*: Move the Freescale drivers
From: Tabi Timur-B04825 @ 2011-06-29 15:39 UTC (permalink / raw)
  To: Jeff Kirsher
  Cc: davem@davemloft.net, netdev@vger.kernel.org, Sandeep Gopalpet,
	Fleming Andy-AFLEMING, Gridish Shlomi-RM96313, Li Yang-R58472,
	Pantelis Antoniou, Vitaly Bordug
In-Reply-To: <1309010363-22750-38-git-send-email-jeffrey.t.kirsher@intel.com>

On Sat, Jun 25, 2011 at 8:58 AM, Jeff Kirsher
<jeffrey.t.kirsher@intel.com> wrote:
> Move the Freescale driver into drivers/net/ethernet/freescale/ and
> make the necessary Kconfig and Makefile changes.

I don't have an actual objection to this, but I just don't understand
why it's necessary.  Is this a new trend for drivers/net, to have all
drivers grouped by manufacturer?  Or is there something special about
Freescale?

-- 
Timur Tabi
Linux kernel developer at Freescale

^ permalink raw reply

* Re: [PATCH] sctp: Enforce maximum retransmissions during shutdown
From: Thomas Graf @ 2011-06-29 15:48 UTC (permalink / raw)
  To: Vladislav Yasevich
  Cc: netdev, davem, Wei Yongjun, Sridhar Samudrala, linux-sctp
In-Reply-To: <4E0B3DA1.9060200@hp.com>

On Wed, Jun 29, 2011 at 10:58:41AM -0400, Vladislav Yasevich wrote:
> But what you are proposing violates the protocol.  Zero-window probes do
> not count against max retransmissions, even when you are in shutdown pending
> state.
> 
> You'll come out of this one of 2 ways:
>   1) receiver wakes up and processes data.  This will allow for graceful termination.
>   2) receiver dies.  Since receive window is full, we have data queued, and this will
>      trigger an ABORT to be sent to the sender.

If by die you mean kill the process then this is exactly what I do to trigger
the issue. I simulatenously kill both processes. Not sure what you mean by
trigger an ABORT but I don't see that happen. I also don't see the rwnd reopen
after the socket is closed on the receiver side but that's a separate issue.

> What you patch is doing is taking a perfectly valid scenario and putting a time limit
> on it in violation of the spec.

We also violate the spec by not doing so. The spec says that the number of
SHUTDOWN retransmissions has to be limited by Max.Retrans which we also
can't enforce because of the above.

The scenario is closed sockets on both sides, endpoints on both sides gone
already and retransmissions + heartbeat requests forever.

Any alternative suggestion how to fix this?

^ permalink raw reply

* Re: [PATCH] net/core: Convert to current logging forms
From: Neil Horman @ 2011-06-29 15:24 UTC (permalink / raw)
  To: David Miller; +Cc: joe, netdev, linux-kernel
In-Reply-To: <20110629.045509.830216204231419781.davem@davemloft.net>

On Wed, Jun 29, 2011 at 04:55:09AM -0700, David Miller wrote:
> 
> Neil, please do not quote a entire large patch just to comment on one
> or two hunks of it.  Just quote the patch hunks you want to provide
> feedback on.
> 
> What you're doing makes it completely unmanageable when I review the
> feedback a patch gets, both in my mailbox and in patchwork.
> 
> Thanks.
Apologies dave, should have snipped more.
Neil

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

^ permalink raw reply

* Re: [RFC v2 2/2] e100: Support RXFCS feature flag.
From: Ben Greear @ 2011-06-29 15:20 UTC (permalink / raw)
  To: Michał Mirosław; +Cc: netdev
In-Reply-To: <BANLkTi=-ibpBiWBZ=AANybmWMWfMVGfe4Q@mail.gmail.com>

On 06/29/2011 08:06 AM, Michał Mirosław wrote:
> W dniu 29 czerwca 2011 16:35 użytkownik Ben Greear
> <greearb@candelatech.com>  napisał:
>> On 06/29/2011 07:33 AM, Michał Mirosław wrote:
>>> W dniu 29 czerwca 2011 16:22 użytkownik Ben Greear
>>> <greearb@candelatech.com>    napisał:
>>>> On 06/29/2011 04:37 AM, Michał Mirosław wrote:
>>>>> 2011/6/24<greearb@candelatech.com>:
>>>>>> From: Ben Greear<greearb@candelatech.com>
>>>>>>
>>>>>> This allows e100 to be configured to append the
>>>>>> Ethernet FCS to the skb.
>>>>>>
>>>>>> Useful for sniffing networks.
>>>>>>
>>>>>> Signed-off-by: Ben Greear<greearb@candelatech.com>
>>>>>> ---
>>>>>> :100644 100644 c1352c6... 761f6f5... M  drivers/net/e100.c
>>>>>>   drivers/net/e100.c |   15 ++++++++++++---
>>>>>>   1 files changed, 12 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/net/e100.c b/drivers/net/e100.c
>>>>>> index c1352c6..761f6f5 100644
>>>>>> --- a/drivers/net/e100.c
>>>>>> +++ b/drivers/net/e100.c
>>>>>> @@ -1089,6 +1089,7 @@ static void e100_configure(struct nic *nic,
>>>>>> struct
>>>>>> cb *cb, struct sk_buff *skb)
>>>>>>   {
>>>>>>         struct config *config =&cb->u.config;
>>>>>>         u8 *c = (u8 *)config;
>>>>>> +       struct net_device *netdev = nic->netdev;
>>>>>>
>>>>>>         cb->command = cpu_to_le16(cb_config);
>>>>>>
>>>>>> @@ -1132,6 +1133,9 @@ static void e100_configure(struct nic *nic,
>>>>>> struct
>>>>>> cb *cb, struct sk_buff *skb)
>>>>>>                 config->promiscuous_mode = 0x1;         /* 1=on, 0=off
>>>>>> */
>>>>>>         }
>>>>>>
>>>>>> +       if (netdev->wanted_features&      NETIF_F_RXFCS)
>>>>>> +               config->rx_crc_transfer = 0x1;  /* 1=save, 0=discard */
>>>>>> +
>>>>>
>>>>> You should check netdev->features here.
>>>>
>>>> I thought 'features' was what the NIC could support, and wanted_features
>>>> was what the NIC was currently configured to support?  I don't want
>>>> to rx the CRC all the time, just when users enable it...
>>>
>>> hw_features is what device could support, and features is what device
>>> has currently turned on.
>> Ok, thanks for that correction.
>> What does wanted_features mean, then?
>
> What user wants to be active. It should be more clear to you if you
> read the implemetation: netdev_update_features() and friends.

I read it.  Seems the code won't let you turn on something not supported,
so if user wants RXFCS, then wanted_features will have it enabled.  So,
I'm not sure why my e100 patch would be wrong in that case.

Thanks,
Ben

>
> Best Regards,
> Michał Mirosław


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

^ permalink raw reply

* Re: [PATCH 00/12] Fix various section mismatches and build errors.
From: Greg KH @ 2011-06-29 15:14 UTC (permalink / raw)
  To: James Bottomley
  Cc: Ralf Baechle, David Miller, akpm, alan, bcasavan, airlied,
	grundler, perex, rpurdie, klassert, tj, dri-devel, linux-kernel,
	linux-mips, linux-scsi, linux-serial, netdev
In-Reply-To: <1309355899.2551.4.camel@mulgrave>

On Wed, Jun 29, 2011 at 08:58:19AM -0500, James Bottomley wrote:
> I think we should simply concentrate on __init and __exit; that's where
> most of the discard value lies and stop expending huge efforts on the
> __devX stuff which adds huge complexity for no real gain.

I have long felt that those __devX markings should just go away as they
cause nothing but problems and have no real gain as you point out.

greg k-h

^ permalink raw reply

* Re: [RFC v2 2/2] e100: Support RXFCS feature flag.
From: Michał Mirosław @ 2011-06-29 15:06 UTC (permalink / raw)
  To: Ben Greear; +Cc: netdev
In-Reply-To: <4E0B383E.8020807@candelatech.com>

W dniu 29 czerwca 2011 16:35 użytkownik Ben Greear
<greearb@candelatech.com> napisał:
> On 06/29/2011 07:33 AM, Michał Mirosław wrote:
>> W dniu 29 czerwca 2011 16:22 użytkownik Ben Greear
>> <greearb@candelatech.com>  napisał:
>>> On 06/29/2011 04:37 AM, Michał Mirosław wrote:
>>>> 2011/6/24<greearb@candelatech.com>:
>>>>> From: Ben Greear<greearb@candelatech.com>
>>>>>
>>>>> This allows e100 to be configured to append the
>>>>> Ethernet FCS to the skb.
>>>>>
>>>>> Useful for sniffing networks.
>>>>>
>>>>> Signed-off-by: Ben Greear<greearb@candelatech.com>
>>>>> ---
>>>>> :100644 100644 c1352c6... 761f6f5... M  drivers/net/e100.c
>>>>>  drivers/net/e100.c |   15 ++++++++++++---
>>>>>  1 files changed, 12 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/e100.c b/drivers/net/e100.c
>>>>> index c1352c6..761f6f5 100644
>>>>> --- a/drivers/net/e100.c
>>>>> +++ b/drivers/net/e100.c
>>>>> @@ -1089,6 +1089,7 @@ static void e100_configure(struct nic *nic,
>>>>> struct
>>>>> cb *cb, struct sk_buff *skb)
>>>>>  {
>>>>>        struct config *config =&cb->u.config;
>>>>>        u8 *c = (u8 *)config;
>>>>> +       struct net_device *netdev = nic->netdev;
>>>>>
>>>>>        cb->command = cpu_to_le16(cb_config);
>>>>>
>>>>> @@ -1132,6 +1133,9 @@ static void e100_configure(struct nic *nic,
>>>>> struct
>>>>> cb *cb, struct sk_buff *skb)
>>>>>                config->promiscuous_mode = 0x1;         /* 1=on, 0=off
>>>>> */
>>>>>        }
>>>>>
>>>>> +       if (netdev->wanted_features&    NETIF_F_RXFCS)
>>>>> +               config->rx_crc_transfer = 0x1;  /* 1=save, 0=discard */
>>>>> +
>>>>
>>>> You should check netdev->features here.
>>>
>>> I thought 'features' was what the NIC could support, and wanted_features
>>> was what the NIC was currently configured to support?  I don't want
>>> to rx the CRC all the time, just when users enable it...
>>
>> hw_features is what device could support, and features is what device
>> has currently turned on.
> Ok, thanks for that correction.
> What does wanted_features mean, then?

What user wants to be active. It should be more clear to you if you
read the implemetation: netdev_update_features() and friends.

Best Regards,
Michał Mirosław

^ permalink raw reply

* Re: [PATCH] sctp: Enforce maximum retransmissions during shutdown
From: Vladislav Yasevich @ 2011-06-29 14:58 UTC (permalink / raw)
  To: netdev, davem, Wei Yongjun, Sridhar Samudrala, linux-sctp
In-Reply-To: <20110629143649.GC10085@canuck.infradead.org>

On 06/29/2011 10:36 AM, Thomas Graf wrote:
> On Wed, Jun 29, 2011 at 10:20:01AM -0400, Vladislav Yasevich wrote:
>> I think in this particular case, the receiver has to terminate, not the sender.
>> Look at how tcp_close() handles this.
>>
>> As long as receiver is available, the sender should continue to try
>> sending data.
> 
> The receiver does not know that the sender wishes to shutdown the
> association. No shutdown request has been sent yet.
> 
> I don't think we should be relying on the behaviour of the sender for
> the receiver to be able to ever free its ressources. We will be
> retransmitting data and keeping an association alive _forever_ for no
> purpose.
> 
> If there is no reliable way of _ever_ doing a graceful shutdown then
> the only alternative is to just ABORT in the first place.
> 
> The difference in TCP is that we can close the connection half-way,
> something we can't do in sctp.
> 

But what you are proposing violates the protocol.  Zero-window probes do
not count against max retransmissions, even when you are in shutdown pending
state.

You'll come out of this one of 2 ways:
  1) receiver wakes up and processes data.  This will allow for graceful termination.
  2) receiver dies.  Since receive window is full, we have data queued, and this will
     trigger an ABORT to be sent to the sender.

What you patch is doing is taking a perfectly valid scenario and putting a time limit
on it in violation of the spec.

-vlad

^ permalink raw reply

* Re: [PATCH] net/core: Convert to current logging forms
From: Joe Perches @ 2011-06-29 14:57 UTC (permalink / raw)
  To: Neil Horman
  Cc: netdev, David S. Miller, linux-kernel, Stephen Hemminger,
	Ben Hutchings
In-Reply-To: <20110629111109.GA1613@hmsreliant.think-freely.org>

On Wed, 2011-06-29 at 07:11 -0400, Neil Horman wrote:
> On Tue, Jun 28, 2011 at 12:40:10PM -0700, Joe Perches wrote:
> > Use pr_fmt, pr_<level>, and netdev_<level> as appropriate.
> > Coalesce long formats.
[]
> > diff --git a/net/core/dev.c b/net/core/dev.c
[]
> > @@ -5397,8 +5380,8 @@ static int netif_alloc_netdev_queues(struct net_device *dev)
> >  
> >  	tx = kcalloc(count, sizeof(struct netdev_queue), GFP_KERNEL);
> >  	if (!tx) {
> > -		pr_err("netdev: Unable to allocate %u tx queues.\n",
> > -		       count);
> > +		netdev_err(dev, "netdev: Unable to allocate %u tx queues\n",
> > +			   count);
> >  		return -ENOMEM;
> >  	}
> Don't all of these get called prior to device registration?  This change will
> have us printing out unregistered net device: ... rather than netdev: ... on
> these errors.

True, that's not particularly good.

> Not tragic, but I rather think its nicer to just say netdev:...

If the second suggested patch is also applied,
it shows the "devicename (unregistered)".

I think that's appropriate.

> Ditto on the unregistered net device thing.

ditto back.

[]
> > diff --git a/net/core/skbuff.c b/net/core/skbuff.c
[]
> >   */
> >  static void skb_over_panic(struct sk_buff *skb, int sz, void *here)
> >  {
> > -	printk(KERN_EMERG "skb_over_panic: text:%p len:%d put:%d head:%p "
> > -			  "data:%p tail:%#lx end:%#lx dev:%s\n",
> > -	       here, skb->len, sz, skb->head, skb->data,
> > -	       (unsigned long)skb->tail, (unsigned long)skb->end,
> > -	       skb->dev ? skb->dev->name : "<NULL>");
> > +	netdev_emerg(skb->dev, "skb_over_panic: text:%p len:%d put:%d head:%p data:%p tail:%#lx end:%#lx\n",
> > +		     here, skb->len, sz, skb->head, skb->data,
> > +		     (unsigned long)skb->tail, (unsigned long)skb->end);
> >  	BUG();
> >  }
> Are you guaranteed to have skb->dev be non-null here?

I think not.

> netdev_printk handles
> that, but flaggin the fact that we have a NULL net device when thats not really
> an issue seems like it would destract from the purpose of this printk.  Same
> with the others in this file below

With the skb_<foo>_panics, I think rearranging the
message output may actually help a _very_ little bit.
The next bit is a BUG which dumps stack and stops the
system anyway so it's not much of an issue.

[]

> > @@ -3061,9 +3057,8 @@ bool skb_partial_csum_set(struct sk_buff *skb, u16 start, u16 off)
> >  	if (unlikely(start > skb_headlen(skb)) ||
> >  	    unlikely((int)start + off > skb_headlen(skb) - 2)) {
> >  		if (net_ratelimit())
> > -			printk(KERN_WARNING
> > -			       "bad partial csum: csum=%u/%u len=%u\n",
> > -			       start, off, skb_headlen(skb));
> > +			netdev_warn(skb->dev, "bad partial csum: csum=%u/%u len=%u\n",
> > +				    start, off, skb_headlen(skb));
> >  		return false;
> >  	}
> >  	skb->ip_summed = CHECKSUM_PARTIAL;

I think netdev_warn is an improvement here.

> > @@ -3076,7 +3071,6 @@ EXPORT_SYMBOL_GPL(skb_partial_csum_set);
> >  void __skb_warn_lro_forwarding(const struct sk_buff *skb)
> >  {
> >  	if (net_ratelimit())
> > -		pr_warning("%s: received packets cannot be forwarded"
> > -			   " while LRO is enabled\n", skb->dev->name);
> > +		netdev_warn(skb->dev, "received packets cannot be forwarded while LRO is enabled\n");
> >  }
> >  EXPORT_SYMBOL(__skb_warn_lro_forwarding);

and not a change here.

I'll reorder the patches to do netdev_name changes
first and resubmit if there aren't more comments
in awhile.

cheers, Joe

^ permalink raw reply

* Re: [PATCH] sctp: Enforce maximum retransmissions during shutdown
From: Thomas Graf @ 2011-06-29 14:36 UTC (permalink / raw)
  To: Vladislav Yasevich
  Cc: netdev, davem, Wei Yongjun, Sridhar Samudrala, linux-sctp
In-Reply-To: <4E0B3491.1060603@hp.com>

On Wed, Jun 29, 2011 at 10:20:01AM -0400, Vladislav Yasevich wrote:
> I think in this particular case, the receiver has to terminate, not the sender.
> Look at how tcp_close() handles this.
> 
> As long as receiver is available, the sender should continue to try
> sending data.

The receiver does not know that the sender wishes to shutdown the
association. No shutdown request has been sent yet.

I don't think we should be relying on the behaviour of the sender for
the receiver to be able to ever free its ressources. We will be
retransmitting data and keeping an association alive _forever_ for no
purpose.

If there is no reliable way of _ever_ doing a graceful shutdown then
the only alternative is to just ABORT in the first place.

The difference in TCP is that we can close the connection half-way,
something we can't do in sctp.

^ permalink raw reply

* Re: [RFC v2 2/2] e100: Support RXFCS feature flag.
From: Ben Greear @ 2011-06-29 14:35 UTC (permalink / raw)
  To: Michał Mirosław; +Cc: netdev
In-Reply-To: <BANLkTikaJNa2+jbW6HWwc+tEE-ciNxvkLg@mail.gmail.com>

On 06/29/2011 07:33 AM, Michał Mirosław wrote:
> W dniu 29 czerwca 2011 16:22 użytkownik Ben Greear
> <greearb@candelatech.com>  napisał:
>> On 06/29/2011 04:37 AM, Michał Mirosław wrote:
>>> 2011/6/24<greearb@candelatech.com>:
>>>> From: Ben Greear<greearb@candelatech.com>
>>>>
>>>> This allows e100 to be configured to append the
>>>> Ethernet FCS to the skb.
>>>>
>>>> Useful for sniffing networks.
>>>>
>>>> Signed-off-by: Ben Greear<greearb@candelatech.com>
>>>> ---
>>>> :100644 100644 c1352c6... 761f6f5... M  drivers/net/e100.c
>>>>   drivers/net/e100.c |   15 ++++++++++++---
>>>>   1 files changed, 12 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/net/e100.c b/drivers/net/e100.c
>>>> index c1352c6..761f6f5 100644
>>>> --- a/drivers/net/e100.c
>>>> +++ b/drivers/net/e100.c
>>>> @@ -1089,6 +1089,7 @@ static void e100_configure(struct nic *nic, struct
>>>> cb *cb, struct sk_buff *skb)
>>>>   {
>>>>         struct config *config =&cb->u.config;
>>>>         u8 *c = (u8 *)config;
>>>> +       struct net_device *netdev = nic->netdev;
>>>>
>>>>         cb->command = cpu_to_le16(cb_config);
>>>>
>>>> @@ -1132,6 +1133,9 @@ static void e100_configure(struct nic *nic, struct
>>>> cb *cb, struct sk_buff *skb)
>>>>                 config->promiscuous_mode = 0x1;         /* 1=on, 0=off */
>>>>         }
>>>>
>>>> +       if (netdev->wanted_features&    NETIF_F_RXFCS)
>>>> +               config->rx_crc_transfer = 0x1;  /* 1=save, 0=discard */
>>>> +
>>> You should check netdev->features here.
>> I thought 'features' was what the NIC could support, and wanted_features
>> was what the NIC was currently configured to support?  I don't want
>> to rx the CRC all the time, just when users enable it...
>
> hw_features is what device could support, and features is what device
> has currently turned on.

Ok, thanks for that correction.

What does wanted_features mean, then?

Thanks,
Ben

>
> Best Regards,
> Michał Mirosław


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

^ permalink raw reply

* Re: [RFC v2 2/2] e100: Support RXFCS feature flag.
From: Michał Mirosław @ 2011-06-29 14:33 UTC (permalink / raw)
  To: Ben Greear; +Cc: netdev
In-Reply-To: <4E0B3510.2080202@candelatech.com>

W dniu 29 czerwca 2011 16:22 użytkownik Ben Greear
<greearb@candelatech.com> napisał:
> On 06/29/2011 04:37 AM, Michał Mirosław wrote:
>> 2011/6/24<greearb@candelatech.com>:
>>> From: Ben Greear<greearb@candelatech.com>
>>>
>>> This allows e100 to be configured to append the
>>> Ethernet FCS to the skb.
>>>
>>> Useful for sniffing networks.
>>>
>>> Signed-off-by: Ben Greear<greearb@candelatech.com>
>>> ---
>>> :100644 100644 c1352c6... 761f6f5... M  drivers/net/e100.c
>>>  drivers/net/e100.c |   15 ++++++++++++---
>>>  1 files changed, 12 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/net/e100.c b/drivers/net/e100.c
>>> index c1352c6..761f6f5 100644
>>> --- a/drivers/net/e100.c
>>> +++ b/drivers/net/e100.c
>>> @@ -1089,6 +1089,7 @@ static void e100_configure(struct nic *nic, struct
>>> cb *cb, struct sk_buff *skb)
>>>  {
>>>        struct config *config =&cb->u.config;
>>>        u8 *c = (u8 *)config;
>>> +       struct net_device *netdev = nic->netdev;
>>>
>>>        cb->command = cpu_to_le16(cb_config);
>>>
>>> @@ -1132,6 +1133,9 @@ static void e100_configure(struct nic *nic, struct
>>> cb *cb, struct sk_buff *skb)
>>>                config->promiscuous_mode = 0x1;         /* 1=on, 0=off */
>>>        }
>>>
>>> +       if (netdev->wanted_features&  NETIF_F_RXFCS)
>>> +               config->rx_crc_transfer = 0x1;  /* 1=save, 0=discard */
>>> +
>> You should check netdev->features here.
> I thought 'features' was what the NIC could support, and wanted_features
> was what the NIC was currently configured to support?  I don't want
> to rx the CRC all the time, just when users enable it...

hw_features is what device could support, and features is what device
has currently turned on.

Best Regards,
Michał Mirosław

^ permalink raw reply

* Re: [RFC v2 2/2] e100: Support RXFCS feature flag.
From: Ben Greear @ 2011-06-29 14:22 UTC (permalink / raw)
  To: Michał Mirosław; +Cc: netdev
In-Reply-To: <BANLkTiknCV1HknVpEdK97yU8eJwaJ5Y5Ag@mail.gmail.com>

On 06/29/2011 04:37 AM, Michał Mirosław wrote:
> 2011/6/24<greearb@candelatech.com>:
>> From: Ben Greear<greearb@candelatech.com>
>>
>> This allows e100 to be configured to append the
>> Ethernet FCS to the skb.
>>
>> Useful for sniffing networks.
>>
>> Signed-off-by: Ben Greear<greearb@candelatech.com>
>> ---
>> :100644 100644 c1352c6... 761f6f5... M  drivers/net/e100.c
>>   drivers/net/e100.c |   15 ++++++++++++---
>>   1 files changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/e100.c b/drivers/net/e100.c
>> index c1352c6..761f6f5 100644
>> --- a/drivers/net/e100.c
>> +++ b/drivers/net/e100.c
>> @@ -1089,6 +1089,7 @@ static void e100_configure(struct nic *nic, struct cb *cb, struct sk_buff *skb)
>>   {
>>         struct config *config =&cb->u.config;
>>         u8 *c = (u8 *)config;
>> +       struct net_device *netdev = nic->netdev;
>>
>>         cb->command = cpu_to_le16(cb_config);
>>
>> @@ -1132,6 +1133,9 @@ static void e100_configure(struct nic *nic, struct cb *cb, struct sk_buff *skb)
>>                 config->promiscuous_mode = 0x1;         /* 1=on, 0=off */
>>         }
>>
>> +       if (netdev->wanted_features&  NETIF_F_RXFCS)
>> +               config->rx_crc_transfer = 0x1;  /* 1=save, 0=discard */
>> +
>
> You should check netdev->features here.

I thought 'features' was what the NIC could support, and wanted_features
was what the NIC was currently configured to support?  I don't want
to rx the CRC all the time, just when users enable it...

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

^ 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