Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] net: sky2: convert to hw_features
From: Stephen Hemminger @ 2011-04-11 18:09 UTC (permalink / raw)
  To: Michał Mirosław; +Cc: netdev
In-Reply-To: <20110411175823.GA31338@rere.qmqm.pl>

On Mon, 11 Apr 2011 19:58:23 +0200
Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:

> On Mon, Apr 11, 2011 at 07:58:29AM -0700, Stephen Hemminger wrote:
> > On Mon, 11 Apr 2011 02:51:00 +0200
> > Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
> > 
> > > On Sun, Apr 10, 2011 at 11:53:02AM -0700, Stephen Hemminger wrote:
> > > > On Sun, 10 Apr 2011 15:13:21 +0200 (CEST)
> > > > Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
> > > > > Caveats:
> > > > >  - driver modifies vlan_features on HW VLAN TX changes
> > > > >  - broken RX checksum will be reenabled on features change
> > > > To be more precise. This is acceptable if and only if all cases
> > > > where features are disabled in response to MTU and chip versions
> > > > are exactly the same. We don't want to let some user stumble upon
> > > > cases where hardware features don't work in their configuration.
> > > I was referring to the unlikely case detected by sky2_rx_checksum().
> > > Before this conversion, user could reenable the feature using ethtool.
> > > The change is that now, in this case, it's reenabled also when other
> > > features are changed (i.e. whenever netdev_update_features() gets called).
> > 
> > Ok.
> > 
> > It does expose a pre-existing issue. If this logic trips (and I have
> > gotten reports of it happening), then the GRO will not get disabled.
> > Probably need to mask of GRO as well, since GRO depends on RXCSUM.
> 
> This is not a problem, since the code checks skb->ip_summed for protocols
> that implement GRO. See tcp4_gro_receive() for example. This might be
> a slight hit in performance, though.

The performance hit is expected, just thought that GRO depended on rx checksum.

^ permalink raw reply

* Re: [RFC net-next] qlge: use ethtool set_phys_id
From: Ron Mercer @ 2011-04-11 18:02 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David Miller, Linux Driver, netdev@vger.kernel.org
In-Reply-To: <20110406164750.11cfeef3@nehalam>

On Wed, Apr 06, 2011 at 04:47:50PM -0700, Stephen Hemminger wrote:
> This is a stab at replacing old ethtool phys_id with set_phys_id
> on the Qlogic 10Gb driver. Compile tested only.
> 
> Not sure if set_led_cfg will flash continuously, or needs
> to be replaced by ETHTOOL_ID_ON/ETHTOOL_ID_OFF
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
>

Stephen,

This worked fine as is.
Thanks,
Ron Mercer

Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>

^ permalink raw reply

* RE: [PATCH] net: ksz884x: convert to hw_features
From: Ha, Tristram @ 2011-04-11 18:21 UTC (permalink / raw)
  To: Michał Mirosław; +Cc: netdev, David S. Miller
In-Reply-To: <20110410131321.BE47D13909@rere.qmqm.pl>

Michał Mirosław wrote:
> This also fixes possible race when changing receive checksum state and removes
> IPV6_CSUM_GEN_HACK as it's always set. 
> 
> BTW, The claim about fake IPV6 checksum looks dubious. If that were true, then there's a problem
> in networking core and should be fixed there and not in random drivers. 
> 
> BTW#2, there's no MAINTAINERS entry for this driver. I assume this driver is supported by Micrel?
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---

Micrel KSZ884X PCI Ethernet Controller does not support generating checksums on IPv6 packets.  But if the driver advertises IPv6 checksum generation support and uses the skb_copy_and_csum_dev function to consolidate the socket buffer, the network performance is actually better than the normal method.  Common sense seems to dictate that using the normal method is faster than the scatter/gather then consolidation method,  but actually this method is faster.  Indeed that is another network driver in the kernel that advertises IPv4 checksum generation support but does not do anything special in hardware and just uses the skb_copy_and_csum_dev function.  If this method is no longer faster or workable, then KSZ884X driver should not advertise IPv6 checksum generation.

^ permalink raw reply

* Re: extending feature word.
From: Mahesh Bandewar @ 2011-04-11 18:45 UTC (permalink / raw)
  To: Michał Mirosław; +Cc: linux-netdev, Ben Hutchings, David Miller
In-Reply-To: <20110410101954.GA23753@rere.qmqm.pl>

On Sun, Apr 10, 2011 at 3:19 AM, Michał Mirosław
<mirq-linux@rere.qmqm.pl> wrote:
> On Fri, Apr 08, 2011 at 11:17:41AM -0700, Mahesh Bandewar wrote:
>> On Fri, Apr 8, 2011 at 3:05 AM, Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
>> > On Fri, Apr 01, 2011 at 07:07:05PM -0700, Mahesh Bandewar wrote:
>> >> Thanks for your comments on my loop-back patch. I was looking at the
>> >> code today from the perspective of extending various "features" for
>> >> word to an array of words and as Michael has pointed out, it's a huge
>> >> change. So I'm thinking on the following lines
>> >> (include/linux/netdevice.h)
>> >>
>> >> +#define DEV_FEATURE_WORDS      2
>> >> +#define LEGACY_FEATURE_WORD    0
>> >>        /* currently active device features */
>> >> -       u32                     features;
>> >> +       u32                     features[DEV_FEATURE_WORDS];
>> >>        /* user-changeable features */
>> >> -       u32                     hw_features;
>> >> +       u32                     hw_features[DEV_FEATURE_WORDS];
>> >>        /* user-requested features */
>> >> -       u32                     wanted_features;
>> >> +       u32                     wanted_features[DEV_FEATURE_WORDS];
>> >>        /* VLAN feature mask */
>> >> -       u32                     vlan_features;
>> >> +       u32                     vlan_features[DEV_FEATURE_WORDS];
>> >
>> > Hmm. There might be no point in making features field an array.
>> > This gives us nothing really. Maybe just add features_2 or similar?
>> > If we ever get to the point there need to be more than two words for
>> > features we can think of some abstraction layer then.
>> >
>> That is right! making it an array doesn't really buy us anything
>> unless there is a uniform way of managing all the bits spread across
>> multiple words inside that array. This was the reason why I have
>> changed that array into a bitmap in the patch that I have posted
>> earlier. This way the upper limit (currently only 32 bits) will be
>> removed and we'll have a long term solution. There will be little bit
>> of work involved but 'doing-things-right' has cost associated.
>
> I really don't like the bitmap idea. It multiplies the amount of code
> needed to manipulate multiple bits at once --- and that's a common
> thing for drivers to do. Almost every driver that needs ndo_fix_features
> will clear sets --- checkumming set, TSO set, all TX offloads set, ...
>
Should the added code be of any concern? If that is happening in the
control-path and does not affect the data-path as such; those added
instructions is a cost of added flexibility to we got through bitmap.
If performance is not at risk then that shouldn't be a problem.

> As a first step just add another set of words:
>
> union {
>        struct {
>                u32 features;
>                u32 features_2;
>        } /* anonymous struct */;
>        u32 features_array[2];
> } /* anonymous union */;
>
> This allows to change drivers after core supporting code gets implemented.
>
I agree that this will be an easier path as far as the extension of
features is concerned. Though this still does not simplify the
management of bits that are spanning across multiple words? Also
atomicity of those operations?

Thanks,
--mahesh..

^ permalink raw reply

* [RFC] iproute2: Fix meta match u32 with 0xffffffff
From: Stephen Hemminger @ 2011-04-11 18:52 UTC (permalink / raw)
  To: Thomas Graf; +Cc: netdev

The value 0xffffffff is a valid mask and bstrtoul() would return
ULONG_MAX which was the error value. Resolve the problem by separating
return value and error indication.

---
 tc/em_cmp.c   |   12 ++++--------
 tc/em_meta.c  |    9 +++------
 tc/em_nbyte.c |    6 ++----
 tc/em_u32.c   |   19 +++++++++----------
 tc/m_ematch.c |   17 +++++++++++------
 tc/m_ematch.h |    2 +-
 6 files changed, 30 insertions(+), 35 deletions(-)

diff --git a/tc/em_cmp.c b/tc/em_cmp.c
index 6addce0..af3e591 100644
--- a/tc/em_cmp.c
+++ b/tc/em_cmp.c
@@ -69,8 +69,7 @@ static int cmp_parse_eopt(struct nlmsghdr *n, struct tcf_ematch_hdr *hdr,
 				return PARSE_ERR(a, "cmp: missing argument");
 			a = bstr_next(a);
 
-			offset = bstrtoul(a);
-			if (offset == ULONG_MAX)
+			if (bstrtoul(a, &offset) < 0)
 				return PARSE_ERR(a, "cmp: invalid offset, " \
 				    "must be numeric");
 
@@ -82,8 +81,7 @@ static int cmp_parse_eopt(struct nlmsghdr *n, struct tcf_ematch_hdr *hdr,
 
 			layer = parse_layer(a);
 			if (layer == INT_MAX) {
-				layer = bstrtoul(a);
-				if (layer == ULONG_MAX)
+				if (bstrtoul(a, &layer) < 0)
 					return PARSE_ERR(a, "cmp: invalid " \
 					    "layer");
 			}
@@ -96,8 +94,7 @@ static int cmp_parse_eopt(struct nlmsghdr *n, struct tcf_ematch_hdr *hdr,
 				return PARSE_ERR(a, "cmp: missing argument");
 			a = bstr_next(a);
 
-			mask = bstrtoul(a);
-			if (mask == ULONG_MAX)
+			if (bstrtoul(a, &mask) < 0)
 				return PARSE_ERR(a, "cmp: invalid mask");
 		} else if (!bstrcmp(a, "trans")) {
 			cmp.flags |= TCF_EM_CMP_TRANS;
@@ -115,8 +112,7 @@ static int cmp_parse_eopt(struct nlmsghdr *n, struct tcf_ematch_hdr *hdr,
 				return PARSE_ERR(a, "cmp: missing argument");
 			a = bstr_next(a);
 
-			value = bstrtoul(a);
-			if (value == ULONG_MAX)
+			if (bstrtoul(a, &value) < 0)
 				return PARSE_ERR(a, "cmp: invalid value");
 
 			value_present = 1;
diff --git a/tc/em_meta.c b/tc/em_meta.c
index 033e29f..276223a 100644
--- a/tc/em_meta.c
+++ b/tc/em_meta.c
@@ -260,8 +260,7 @@ parse_object(struct bstr *args, struct bstr *arg, struct tcf_meta_val *obj,
 		return bstr_next(arg);
 	}
 
-	num = bstrtoul(arg);
-	if (num != ULONG_MAX) {
+	if (bstrtoul(arg, &num) < 0) {
 		obj->kind = TCF_META_TYPE_INT << 12;
 		obj->kind |= TCF_META_ID_VALUE;
 		*dst = (unsigned long) num;
@@ -318,8 +317,7 @@ compatible:
 			}
 			a = bstr_next(a);
 
-			shift = bstrtoul(a);
-			if (shift == ULONG_MAX) {
+			if (bstrtoul(a, &shift) < 0) {
 				PARSE_ERR(a, "meta: invalid shift, must " \
 				    "be numeric");
 				return PARSE_FAILURE;
@@ -336,8 +334,7 @@ compatible:
 			}
 			a = bstr_next(a);
 
-			mask = bstrtoul(a);
-			if (mask == ULONG_MAX) {
+			if (bstrtoul(a, &mask) < 0) {
 				PARSE_ERR(a, "meta: invalid mask, must be " \
 				    "numeric");
 				return PARSE_FAILURE;
diff --git a/tc/em_nbyte.c b/tc/em_nbyte.c
index 87f3e9d..9a52ffc 100644
--- a/tc/em_nbyte.c
+++ b/tc/em_nbyte.c
@@ -63,8 +63,7 @@ static int nbyte_parse_eopt(struct nlmsghdr *n, struct tcf_ematch_hdr *hdr,
 				return PARSE_ERR(a, "nbyte: missing argument");
 			a = bstr_next(a);
 
-			offset = bstrtoul(a);
-			if (offset == ULONG_MAX)
+			if (bstrtoul(a, &offset) < 0)
 				return PARSE_ERR(a, "nbyte: invalid offset, " \
 				    "must be numeric");
 
@@ -76,8 +75,7 @@ static int nbyte_parse_eopt(struct nlmsghdr *n, struct tcf_ematch_hdr *hdr,
 
 			layer = parse_layer(a);
 			if (layer == INT_MAX) {
-				layer = bstrtoul(a);
-				if (layer == ULONG_MAX)
+				if (bstrtoul(a, &layer) < 0)
 					return PARSE_ERR(a, "nbyte: invalid " \
 					    "layer");
 			}
diff --git a/tc/em_u32.c b/tc/em_u32.c
index 21ed70f..88b5fa1 100644
--- a/tc/em_u32.c
+++ b/tc/em_u32.c
@@ -33,6 +33,7 @@ static void u32_print_usage(FILE *fd)
 	    "Example: u32(u16 0x1122 0xffff at nexthdr+4)\n");
 }
 
+
 static int u32_parse_eopt(struct nlmsghdr *n, struct tcf_ematch_hdr *hdr,
 			  struct bstr *args)
 {
@@ -62,16 +63,14 @@ static int u32_parse_eopt(struct nlmsghdr *n, struct tcf_ematch_hdr *hdr,
 	if (a == NULL)
 		return PARSE_ERR(a, "u32: missing key");
 
-	key = bstrtoul(a);
-	if (key == ULONG_MAX)
+	if (bstrtoul(a, &key) < 0)
 		return PARSE_ERR(a, "u32: invalid key, must be numeric");
 
 	a = bstr_next(a);
 	if (a == NULL)
 		return PARSE_ERR(a, "u32: missing mask");
 
-	mask = bstrtoul(a);
-	if (mask == ULONG_MAX)
+	if (bstrtoul(a, &mask) < 0)
 		return PARSE_ERR(a, "u32: invalid mask, must be numeric");
 
 	a = bstr_next(a);
@@ -92,12 +91,12 @@ static int u32_parse_eopt(struct nlmsghdr *n, struct tcf_ematch_hdr *hdr,
 		a = bstr_next(a);
 		if (a == NULL)
 			return PARSE_ERR(a, "u32: missing offset");
-		offset = bstrtoul(a);
-	} else
-		offset = bstrtoul(a);
-
-	if (offset == ULONG_MAX)
-		return PARSE_ERR(a, "u32: invalid offset");
+		if (bstrtoul(a, &offset) < 0)
+			return PARSE_ERR(a, "u32: invalid offset");
+	} else {
+		if (bstrtoul(a, &offset) < 0)
+			return PARSE_ERR(a, "u32: invalid offset");
+	}
 
 	if (a->next)
 		return PARSE_ERR(a->next, "u32: unexpected trailer");
diff --git a/tc/m_ematch.c b/tc/m_ematch.c
index 4c3acf8..92600ad 100644
--- a/tc/m_ematch.c
+++ b/tc/m_ematch.c
@@ -510,20 +510,25 @@ struct bstr * bstr_alloc(const char *text)
 	return b;
 }
 
-unsigned long bstrtoul(const struct bstr *b)
+int bstrtoul(const struct bstr *b, unsigned long *lp)
 {
 	char *inv = NULL;
-	unsigned long l;
 	char buf[b->len+1];
 
+	if (b->len == 0)
+		return -EINVAL;
+
 	memcpy(buf, b->data, b->len);
 	buf[b->len] = '\0';
 
-	l = strtoul(buf, &inv, 0);
-	if (l == ULONG_MAX || inv == buf)
-		return ULONG_MAX;
+	*lp = strtoul(buf, &inv, 0);
+	if (inv == buf)
+		return -EINVAL;
+
+	if (*lp == ULONG_MAX || errno == ERANGE)
+		return -ERANGE;
 
-	return l;
+	return 0;
 }
 
 void bstr_print(FILE *fd, const struct bstr *b, int ascii)
diff --git a/tc/m_ematch.h b/tc/m_ematch.h
index 5036e9b..e676290 100644
--- a/tc/m_ematch.h
+++ b/tc/m_ematch.h
@@ -49,7 +49,7 @@ static inline struct bstr *bstr_next(struct bstr *b)
 	return b->next;
 }
 
-extern unsigned long bstrtoul(const struct bstr *b);
+extern int bstrtoul(const struct bstr *b, unsigned long *lp);
 extern void bstr_print(FILE *fd, const struct bstr *b, int ascii);
 
 
-- 
1.7.1


^ permalink raw reply related

* Re: [PATCH RESEND] uts: Set default hostname to "localhost", rather than "(none)"
From: Josh Triplett @ 2011-04-11 18:53 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Stephen Hemminger, David Miller, netdev, Serge E. Hallyn,
	Andrew Morton, linux-kernel
In-Reply-To: <BANLkTinVGTOAsxG1CiJO1mSGRxBSNP5P4Q@mail.gmail.com>

On Mon, Apr 11, 2011 at 08:12:40AM -0700, Linus Torvalds wrote:
> On Mon, Apr 11, 2011 at 8:02 AM, Stephen Hemminger
> <shemminger@vyatta.com> wrote:
> > It makes sense but this behavior has existed so long in Linux
> > that some distro might actually be depending on it.

I checked fairly thoroughly for evidence of userspace relying on
"(none)", and found no instances of that.  Userspace typically just says
"I haven't set it yet, so nobody has set it yet", and unconditionally
sets the hostname.  dhclient doesn't rely on "(none)", distro networking
scripts don't rely on "(none)", the hostname tool doesn't rely on
"(none)", and nothing else I've found looks for "(none)".

That said, I'll certainly acknowledge that some random userspace bits
somewhere could have relied on this behavior.  Would a transitional
period help at all here?  Alternatively, see below.

> Yes. Also, quite frankly, I do _not_ think that "localhost" is in any
> way a more correct hostname.
> 
> I'd rather have an obviously invalid hostname for a machine that
> hasn't been set up correctly than one that might work by random
> chance.

Choosing "localhost" for systems that don't configure a hostname at
least makes it fairly likely that you can resolve the system hostname.
So, "localhost" will have a tendency to Just Work, but seeing a hostname
of "localhost" will still obviously remind a user that they ought to set
the hostname to something more sensible on a network-connected box.  On
a non-networked box, localhost makes perfect sense, and the box need not
configure anything else.  Similarly, this default would also make sense
for various types of virtual systems.

Also, "(none)" doesn't necessarily represent an "invalid" hostname.
In general, tools seem to assume the validity of the system hostname,
and use it wherever the hostname appears.  (For that matter, as some
recent security vulnerabilities have demonstrated, "valid hostname"
doesn't necessarily mean "alphanumeric"; both DNS and DHCP will happily
hand out a hostname with crazy characters in it, including '(' and ')'.)

I wrote this patch with the same motivation as the option to mount
devtmpfs on /dev automatically: it makes systems behave more or less
sanely in the absence of explicit setup, even though more explicit setup
may produce a more optimal configuration (just as devtmpfs makes all
devices root:root in the absence of a better configuration).  It would
often make sense for embedded or virtual systems.

In general, I'd rather see the kernel provide a configuration that will
work in at least some cases and do no worse in others (requiring
explicit configuration), rather than a configuration which will not work
in any cases and *always* requires explicit userspace configuration.

If the above does not provide a sufficiently good argument for changing
the default (perhaps with a transition period added), would you consider
accepting a patch which added a CONFIG_DEFAULT_UTS_HOSTNAME, defaulting
to "(none)"?  Then systems could choose to use that configuration option
if they know their userspace will work with it.

- Josh Triplett

^ permalink raw reply

* Re: extending feature word.
From: Stephen Hemminger @ 2011-04-11 18:54 UTC (permalink / raw)
  To: Mahesh Bandewar
  Cc: Michał Mirosław, linux-netdev, Ben Hutchings,
	David Miller
In-Reply-To: <BANLkTimFzTGKmhfRxzEUt_LwrnhAaetU5Q@mail.gmail.com>

On Mon, 11 Apr 2011 11:45:05 -0700
Mahesh Bandewar <maheshb@google.com> wrote:

> >> That is right! making it an array doesn't really buy us anything
> >> unless there is a uniform way of managing all the bits spread across
> >> multiple words inside that array. This was the reason why I have
> >> changed that array into a bitmap in the patch that I have posted
> >> earlier. This way the upper limit (currently only 32 bits) will be
> >> removed and we'll have a long term solution. There will be little bit
> >> of work involved but 'doing-things-right' has cost associated.  
> >
> > I really don't like the bitmap idea. It multiplies the amount of code
> > needed to manipulate multiple bits at once --- and that's a common
> > thing for drivers to do. Almost every driver that needs ndo_fix_features
> > will clear sets --- checkumming set, TSO set, all TX offloads set, ...
> >  
> Should the added code be of any concern? If that is happening in the
> control-path and does not affect the data-path as such; those added
> instructions is a cost of added flexibility to we got through bitmap.
> If performance is not at risk then that shouldn't be a problem.

Just to be dense... What is wrong with just using u64?

-- 

^ permalink raw reply

* Re: [PATCH v2] net: r8169: convert to hw_features
From: François Romieu @ 2011-04-11 18:47 UTC (permalink / raw)
  To: Michał Mirosław; +Cc: netdev, David Dillow, Hayes Wang
In-Reply-To: <20110411133029.GA12119@rere.qmqm.pl>

On Mon, Apr 11, 2011 at 03:30:29PM +0200, Michał Mirosław wrote:
> On Sun, Apr 10, 2011 at 05:45:08PM +0200, François Romieu wrote:
[...]
> > The description should include :
> > - Rx VLAN tag stripping is now enabled by default
> 
> Actually, it was enabled by default before. NETIF_F_HW_VLAN_TX_RX was
> #defined to be NETIF_F_HW_VLAN_TX+NETIF_F_HW_VLAN_RX.

It was enabled in dev->features but it was necessary to configure at
least one VLAN before the hardware CPlusCmd register was instructed
to strip Rx vlan tag (let aside 8110SCd or tagged Tx packets which
where sent as such).

I am not sure it will be noticed.

[...]
> Yes. hw_features signifies what can be toggled by user, but does not
> imply state of features not present there.

Thanks for the clarification.

On a different topic, David was right. The large send feature needs
more fixing. I should have a first tested patch for wednesday.

Hayes, I have a 8168c manual at hand. Do all 8168 have the same Tx
descriptors layout ?

-- 
Ueimor

^ permalink raw reply

* Re: [PATCH] bonding:set save_load to 0 when initializing
From: Jay Vosburgh @ 2011-04-11 18:56 UTC (permalink / raw)
  To: Weiping Pan(潘卫平); +Cc: andy, netdev, linux-kernel
In-Reply-To: <1302270019-11698-1-git-send-email-panweiping3@gmail.com>


>It is unnecessary to set save_load to 1 here,
>as the tx_hashtbl is just kzalloced.
>
>Signed-off-by: Weiping Pan(潘卫平) <panweiping3@gmail.com>

	I don't think this will cause any behavioral change, but I'll
grant is more correct in intent (there's no load information to save at
this point).

Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>

>---
> drivers/net/bonding/bond_alb.c |    2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
>index 9bc5de3..ab69e5a 100644
>--- a/drivers/net/bonding/bond_alb.c
>+++ b/drivers/net/bonding/bond_alb.c
>@@ -176,7 +176,7 @@ static int tlb_initialize(struct bonding *bond)
> 	bond_info->tx_hashtbl = new_hashtbl;
>
> 	for (i = 0; i < TLB_HASH_TABLE_SIZE; i++) {
>-		tlb_init_table_entry(&bond_info->tx_hashtbl[i], 1);
>+		tlb_init_table_entry(&bond_info->tx_hashtbl[i], 0);
> 	}
>
> 	_unlock_tx_hashtbl(bond);
>-- 
>1.7.4
>

^ permalink raw reply

* Re: [PATCH 0/2] bonding:delete two unused variables
From: Jay Vosburgh @ 2011-04-11 19:02 UTC (permalink / raw)
  To: Weiping Pan(潘卫平); +Cc: andy, netdev, linux-kernel
In-Reply-To: <cover.1302509480.git.panweiping3@gmail.com>


>I found that variable alb_timer and rlb_interval_counter in struct
>alb_bond_info are not used by other codes any more, so delete them.
>
>Weiping Pan(潘卫平) (2):
>  bonding:delete unused alb_timer
>  bonding:delete unused rlb_interval_counter

	For both patches.

Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>


> drivers/net/bonding/bond_alb.h |    2 --
> 1 files changed, 0 insertions(+), 2 deletions(-)
>
>-- 
>1.7.4
>
>--
>To unsubscribe from this list: send the line "unsubscribe netdev" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2] net: r8169: convert to hw_features
From: Michał Mirosław @ 2011-04-11 19:15 UTC (permalink / raw)
  To: François Romieu; +Cc: netdev, David Dillow, Hayes Wang
In-Reply-To: <20110411184739.GA17331@electric-eye.fr.zoreil.com>

On Mon, Apr 11, 2011 at 08:47:39PM +0200, François Romieu wrote:
> On Mon, Apr 11, 2011 at 03:30:29PM +0200, Michał Mirosław wrote:
> > On Sun, Apr 10, 2011 at 05:45:08PM +0200, François Romieu wrote:
> [...]
> > > The description should include :
> > > - Rx VLAN tag stripping is now enabled by default
> > Actually, it was enabled by default before. NETIF_F_HW_VLAN_TX_RX was
> > #defined to be NETIF_F_HW_VLAN_TX+NETIF_F_HW_VLAN_RX.
> It was enabled in dev->features but it was necessary to configure at
> least one VLAN before the hardware CPlusCmd register was instructed
> to strip Rx vlan tag (let aside 8110SCd or tagged Tx packets which
> where sent as such).

Are you sure? rtl8169_vlan_mode() that configured this before was
called from rtl8169_init_one(). The change is that this logic was
merged with enabling RX csum offload in rtl8169_set_features().

Best Regards,
Michał Mirosław

^ permalink raw reply

* Re: extending feature word.
From: Mahesh Bandewar @ 2011-04-11 19:16 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Michał Mirosław, linux-netdev, Ben Hutchings,
	David Miller
In-Reply-To: <20110411115459.13ac3c73@nehalam>

On Mon, Apr 11, 2011 at 11:54 AM, Stephen Hemminger
<shemminger@vyatta.com> wrote:
> On Mon, 11 Apr 2011 11:45:05 -0700
> Mahesh Bandewar <maheshb@google.com> wrote:
>
>> >> That is right! making it an array doesn't really buy us anything
>> >> unless there is a uniform way of managing all the bits spread across
>> >> multiple words inside that array. This was the reason why I have
>> >> changed that array into a bitmap in the patch that I have posted
>> >> earlier. This way the upper limit (currently only 32 bits) will be
>> >> removed and we'll have a long term solution. There will be little bit
>> >> of work involved but 'doing-things-right' has cost associated.
>> >
>> > I really don't like the bitmap idea. It multiplies the amount of code
>> > needed to manipulate multiple bits at once --- and that's a common
>> > thing for drivers to do. Almost every driver that needs ndo_fix_features
>> > will clear sets --- checkumming set, TSO set, all TX offloads set, ...
>> >
>> Should the added code be of any concern? If that is happening in the
>> control-path and does not affect the data-path as such; those added
>> instructions is a cost of added flexibility to we got through bitmap.
>> If performance is not at risk then that shouldn't be a problem.
>
> Just to be dense... What is wrong with just using u64?
>
I have already suggested that in this thread. With this theoretically
you removing one limit and imposing another and that's why I said it
would be a mid-term solution. But again by the time all 64 bits are
gone (got used), we may have u128 available.

> --
>

^ permalink raw reply

* Re: extending feature word.
From: Michał Mirosław @ 2011-04-11 19:19 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Mahesh Bandewar, linux-netdev, Ben Hutchings, David Miller
In-Reply-To: <20110411115459.13ac3c73@nehalam>

On Mon, Apr 11, 2011 at 11:54:59AM -0700, Stephen Hemminger wrote:
> On Mon, 11 Apr 2011 11:45:05 -0700
> Mahesh Bandewar <maheshb@google.com> wrote:
> > >> That is right! making it an array doesn't really buy us anything
> > >> unless there is a uniform way of managing all the bits spread across
> > >> multiple words inside that array. This was the reason why I have
> > >> changed that array into a bitmap in the patch that I have posted
> > >> earlier. This way the upper limit (currently only 32 bits) will be
> > >> removed and we'll have a long term solution. There will be little bit
> > >> of work involved but 'doing-things-right' has cost associated.  
> > > I really don't like the bitmap idea. It multiplies the amount of code
> > > needed to manipulate multiple bits at once --- and that's a common
> > > thing for drivers to do. Almost every driver that needs ndo_fix_features
> > > will clear sets --- checkumming set, TSO set, all TX offloads set, ...
> > Should the added code be of any concern? If that is happening in the
> > control-path and does not affect the data-path as such; those added
> > instructions is a cost of added flexibility to we got through bitmap.
> > If performance is not at risk then that shouldn't be a problem.
> Just to be dense... What is wrong with just using u64?

Hmm. Looks like this is so simple that nobody thought of it seriously. ;)

This of course needs a bit of glue code in G/SFEATURES handling, but most
of the change would be s/u32/u64/ in apropriate places.

Best Regards,
Michał Mirosław

^ permalink raw reply

* arp_solicit and source ip selection
From: Oskar Berggren @ 2011-04-11 19:33 UTC (permalink / raw)
  To: netdev

Hi,

arp_solicit() tries to use the source IP-address of the IP-packet that
caused an ARP resolution to be necessary, when sending an ARP request.
If the IP-packet is not generated locally, that its source IP cannot
be reused in the ARP packet, and instead inet_select_addr() is called
for the relevant network interface.

Any reason why arp_solicit() could not, or should not, use the routing
subsystem to determine source IP address for the ARP request? That
would enable it to take account of any "src" directives specified in
the routing tables.

/Oskar

^ permalink raw reply

* pull request: wireless-2.6 2011-04-11
From: John W. Linville @ 2011-04-11 19:34 UTC (permalink / raw)
  To: davem; +Cc: linux-wireless, netdev, linux-kernel

Dave,

Here is another batch intended for 2.6.39.  Included is an mwl8k fix
to avoid freeing an irq that we never requested, an ath common code
fix to add a regulatory domain mapping code, an ath9k fix to avoid some
crashes on MIPS boards and fix some radio stability issues on AR9285, a
p54 fix to avoid a device lock-up caused by an uninitialized variable,
an iwlagn fix to correct for an incorrect antenna configuration value
in EEPROM on iwl5300 hardware, and an Kconfig fix for iwlegacy to
keep old kernel build configurations still building iwl3945 and/or
iwl4965 after the iwlagn/iwlegacy split.

Please let me know if there are problems!

John

---

The following changes since commit 88edaa415966af965bb7eb7056d8b58145462c8e:

  net: Add support for SMSC LAN9530, LAN9730 and LAN89530 (2011-04-10 18:59:27 -0700)

are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-2.6.git master

Brian Cavagnolo (1):
      mwl8k: do not free unrequested irq

Christian Lamparter (1):
      ath: add missing regdomain pair 0x5c mapping

Felix Fietkau (1):
      ath9k: fix missing ath9k_ps_wakeup/ath9k_ps_restore calls

Jason Conti (1):
      p54: Initialize extra_len in p54_tx_80211

Johannes Berg (1):
      iwlagn: override 5300 EEPROM # of chains

John W. Linville (1):
      iwlegacy: make iwl3945 and iwl4965 select IWLWIFI_LEGACY

 drivers/net/wireless/ath/ath9k/main.c   |   12 ++++++++++--
 drivers/net/wireless/ath/regd_common.h  |    1 +
 drivers/net/wireless/iwlegacy/Kconfig   |    9 +++++----
 drivers/net/wireless/iwlwifi/iwl-5000.c |    3 +++
 drivers/net/wireless/mwl8k.c            |    9 ++++++++-
 drivers/net/wireless/p54/txrx.c         |    2 +-
 6 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index dddb85d..17d04ff 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1376,7 +1376,6 @@ static void ath9k_calculate_summary_state(struct ieee80211_hw *hw,
 
 	ath9k_calculate_iter_data(hw, vif, &iter_data);
 
-	ath9k_ps_wakeup(sc);
 	/* Set BSSID mask. */
 	memcpy(common->bssidmask, iter_data.mask, ETH_ALEN);
 	ath_hw_setbssidmask(common);
@@ -1411,7 +1410,6 @@ static void ath9k_calculate_summary_state(struct ieee80211_hw *hw,
 	}
 
 	ath9k_hw_set_interrupts(ah, ah->imask);
-	ath9k_ps_restore(sc);
 
 	/* Set up ANI */
 	if ((iter_data.naps + iter_data.nadhocs) > 0) {
@@ -1457,6 +1455,7 @@ static int ath9k_add_interface(struct ieee80211_hw *hw,
 	struct ath_vif *avp = (void *)vif->drv_priv;
 	int ret = 0;
 
+	ath9k_ps_wakeup(sc);
 	mutex_lock(&sc->mutex);
 
 	switch (vif->type) {
@@ -1503,6 +1502,7 @@ static int ath9k_add_interface(struct ieee80211_hw *hw,
 	ath9k_do_vif_add_setup(hw, vif);
 out:
 	mutex_unlock(&sc->mutex);
+	ath9k_ps_restore(sc);
 	return ret;
 }
 
@@ -1517,6 +1517,7 @@ static int ath9k_change_interface(struct ieee80211_hw *hw,
 
 	ath_dbg(common, ATH_DBG_CONFIG, "Change Interface\n");
 	mutex_lock(&sc->mutex);
+	ath9k_ps_wakeup(sc);
 
 	/* See if new interface type is valid. */
 	if ((new_type == NL80211_IFTYPE_ADHOC) &&
@@ -1546,6 +1547,7 @@ static int ath9k_change_interface(struct ieee80211_hw *hw,
 
 	ath9k_do_vif_add_setup(hw, vif);
 out:
+	ath9k_ps_restore(sc);
 	mutex_unlock(&sc->mutex);
 	return ret;
 }
@@ -1558,6 +1560,7 @@ static void ath9k_remove_interface(struct ieee80211_hw *hw,
 
 	ath_dbg(common, ATH_DBG_CONFIG, "Detach Interface\n");
 
+	ath9k_ps_wakeup(sc);
 	mutex_lock(&sc->mutex);
 
 	sc->nvifs--;
@@ -1569,6 +1572,7 @@ static void ath9k_remove_interface(struct ieee80211_hw *hw,
 	ath9k_calculate_summary_state(hw, NULL);
 
 	mutex_unlock(&sc->mutex);
+	ath9k_ps_restore(sc);
 }
 
 static void ath9k_enable_ps(struct ath_softc *sc)
@@ -1809,6 +1813,7 @@ static int ath9k_conf_tx(struct ieee80211_hw *hw, u16 queue,
 
 	txq = sc->tx.txq_map[queue];
 
+	ath9k_ps_wakeup(sc);
 	mutex_lock(&sc->mutex);
 
 	memset(&qi, 0, sizeof(struct ath9k_tx_queue_info));
@@ -1832,6 +1837,7 @@ static int ath9k_conf_tx(struct ieee80211_hw *hw, u16 queue,
 			ath_beaconq_config(sc);
 
 	mutex_unlock(&sc->mutex);
+	ath9k_ps_restore(sc);
 
 	return ret;
 }
@@ -1894,6 +1900,7 @@ static void ath9k_bss_info_changed(struct ieee80211_hw *hw,
 	int slottime;
 	int error;
 
+	ath9k_ps_wakeup(sc);
 	mutex_lock(&sc->mutex);
 
 	if (changed & BSS_CHANGED_BSSID) {
@@ -1994,6 +2001,7 @@ static void ath9k_bss_info_changed(struct ieee80211_hw *hw,
 	}
 
 	mutex_unlock(&sc->mutex);
+	ath9k_ps_restore(sc);
 }
 
 static u64 ath9k_get_tsf(struct ieee80211_hw *hw)
diff --git a/drivers/net/wireless/ath/regd_common.h b/drivers/net/wireless/ath/regd_common.h
index 248c670..5c2cfe6 100644
--- a/drivers/net/wireless/ath/regd_common.h
+++ b/drivers/net/wireless/ath/regd_common.h
@@ -195,6 +195,7 @@ static struct reg_dmn_pair_mapping regDomainPairs[] = {
 	{APL9_WORLD, CTL_ETSI, CTL_ETSI},
 
 	{APL3_FCCA, CTL_FCC, CTL_FCC},
+	{APL7_FCCA, CTL_FCC, CTL_FCC},
 	{APL1_ETSIC, CTL_FCC, CTL_ETSI},
 	{APL2_ETSIC, CTL_FCC, CTL_ETSI},
 	{APL2_APLD, CTL_FCC, NO_CTL},
diff --git a/drivers/net/wireless/iwlegacy/Kconfig b/drivers/net/wireless/iwlegacy/Kconfig
index 2a45dd4..aef65cd 100644
--- a/drivers/net/wireless/iwlegacy/Kconfig
+++ b/drivers/net/wireless/iwlegacy/Kconfig
@@ -1,6 +1,5 @@
 config IWLWIFI_LEGACY
-	tristate "Intel Wireless Wifi legacy devices"
-	depends on PCI && MAC80211
+	tristate
 	select FW_LOADER
 	select NEW_LEDS
 	select LEDS_CLASS
@@ -65,7 +64,8 @@ endmenu
 
 config IWL4965
 	tristate "Intel Wireless WiFi 4965AGN (iwl4965)"
-	depends on IWLWIFI_LEGACY
+	depends on PCI && MAC80211
+	select IWLWIFI_LEGACY
 	---help---
 	  This option enables support for
 
@@ -92,7 +92,8 @@ config IWL4965
 
 config IWL3945
 	tristate "Intel PRO/Wireless 3945ABG/BG Network Connection (iwl3945)"
-	depends on IWLWIFI_LEGACY
+	depends on PCI && MAC80211
+	select IWLWIFI_LEGACY
 	---help---
 	  Select to build the driver supporting the:
 
diff --git a/drivers/net/wireless/iwlwifi/iwl-5000.c b/drivers/net/wireless/iwlwifi/iwl-5000.c
index 3ea31b6..22e045b 100644
--- a/drivers/net/wireless/iwlwifi/iwl-5000.c
+++ b/drivers/net/wireless/iwlwifi/iwl-5000.c
@@ -530,6 +530,9 @@ static struct iwl_ht_params iwl5000_ht_params = {
 struct iwl_cfg iwl5300_agn_cfg = {
 	.name = "Intel(R) Ultimate N WiFi Link 5300 AGN",
 	IWL_DEVICE_5000,
+	/* at least EEPROM 0x11A has wrong info */
+	.valid_tx_ant = ANT_ABC,	/* .cfg overwrite */
+	.valid_rx_ant = ANT_ABC,	/* .cfg overwrite */
 	.ht_params = &iwl5000_ht_params,
 };
 
diff --git a/drivers/net/wireless/mwl8k.c b/drivers/net/wireless/mwl8k.c
index 3695227..c1ceb4b 100644
--- a/drivers/net/wireless/mwl8k.c
+++ b/drivers/net/wireless/mwl8k.c
@@ -137,6 +137,7 @@ struct mwl8k_tx_queue {
 struct mwl8k_priv {
 	struct ieee80211_hw *hw;
 	struct pci_dev *pdev;
+	int irq;
 
 	struct mwl8k_device_info *device_info;
 
@@ -3761,9 +3762,11 @@ static int mwl8k_start(struct ieee80211_hw *hw)
 	rc = request_irq(priv->pdev->irq, mwl8k_interrupt,
 			 IRQF_SHARED, MWL8K_NAME, hw);
 	if (rc) {
+		priv->irq = -1;
 		wiphy_err(hw->wiphy, "failed to register IRQ handler\n");
 		return -EIO;
 	}
+	priv->irq = priv->pdev->irq;
 
 	/* Enable TX reclaim and RX tasklets.  */
 	tasklet_enable(&priv->poll_tx_task);
@@ -3800,6 +3803,7 @@ static int mwl8k_start(struct ieee80211_hw *hw)
 	if (rc) {
 		iowrite32(0, priv->regs + MWL8K_HIU_A2H_INTERRUPT_MASK);
 		free_irq(priv->pdev->irq, hw);
+		priv->irq = -1;
 		tasklet_disable(&priv->poll_tx_task);
 		tasklet_disable(&priv->poll_rx_task);
 	}
@@ -3818,7 +3822,10 @@ static void mwl8k_stop(struct ieee80211_hw *hw)
 
 	/* Disable interrupts */
 	iowrite32(0, priv->regs + MWL8K_HIU_A2H_INTERRUPT_MASK);
-	free_irq(priv->pdev->irq, hw);
+	if (priv->irq != -1) {
+		free_irq(priv->pdev->irq, hw);
+		priv->irq = -1;
+	}
 
 	/* Stop finalize join worker */
 	cancel_work_sync(&priv->finalize_join_worker);
diff --git a/drivers/net/wireless/p54/txrx.c b/drivers/net/wireless/p54/txrx.c
index 7834c26..042842e 100644
--- a/drivers/net/wireless/p54/txrx.c
+++ b/drivers/net/wireless/p54/txrx.c
@@ -703,7 +703,7 @@ void p54_tx_80211(struct ieee80211_hw *dev, struct sk_buff *skb)
 	struct p54_tx_info *p54info;
 	struct p54_hdr *hdr;
 	struct p54_tx_data *txhdr;
-	unsigned int padding, len, extra_len;
+	unsigned int padding, len, extra_len = 0;
 	int i, j, ridx;
 	u16 hdr_flags = 0, aid = 0;
 	u8 rate, queue = 0, crypt_offset = 0;
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply related

* Re: [PATCH v2] net: r8169: convert to hw_features
From: François Romieu @ 2011-04-11 19:30 UTC (permalink / raw)
  To: Michał Mirosław; +Cc: netdev, David Dillow, Hayes Wang
In-Reply-To: <20110411191513.GB31338@rere.qmqm.pl>

On Mon, Apr 11, 2011 at 09:15:13PM +0200, Michał Mirosław wrote:
[...]
> Are you sure?

Sure, especially when I'm wrong.

I did not document the change of behavior when the driver was
converted to the new vlan model and I forgot what I did. Bad. :o(

-- 
Ueimor

^ permalink raw reply

* igmp_max_memberships and CONFIG_IP_MULTICAST
From: Joakim Tjernlund @ 2011-04-11 19:34 UTC (permalink / raw)
  To: netdev


Why is igmp_max_memberships hidden behind CONFIG_IP_MULTICAST?

I am running Quagga(OSPF) without CONFIG_IP_MULTICAST and ran into
this limit. Now I need to increase it but then I find it is hidden behind
CONFIG_IP_MULTICAST. Is there a reason it is hidden?

 Jocke


^ permalink raw reply

* Re: extending feature word.
From: Stephen Hemminger @ 2011-04-11 19:49 UTC (permalink / raw)
  To: Michał Mirosław
  Cc: Mahesh Bandewar, linux-netdev, Ben Hutchings, David Miller
In-Reply-To: <20110411191947.GC31338@rere.qmqm.pl>

On Mon, 11 Apr 2011 21:19:47 +0200
Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:

> On Mon, Apr 11, 2011 at 11:54:59AM -0700, Stephen Hemminger wrote:
> > On Mon, 11 Apr 2011 11:45:05 -0700
> > Mahesh Bandewar <maheshb@google.com> wrote:
> > > >> That is right! making it an array doesn't really buy us anything
> > > >> unless there is a uniform way of managing all the bits spread across
> > > >> multiple words inside that array. This was the reason why I have
> > > >> changed that array into a bitmap in the patch that I have posted
> > > >> earlier. This way the upper limit (currently only 32 bits) will be
> > > >> removed and we'll have a long term solution. There will be little bit
> > > >> of work involved but 'doing-things-right' has cost associated.  
> > > > I really don't like the bitmap idea. It multiplies the amount of code
> > > > needed to manipulate multiple bits at once --- and that's a common
> > > > thing for drivers to do. Almost every driver that needs ndo_fix_features
> > > > will clear sets --- checkumming set, TSO set, all TX offloads set, ...
> > > Should the added code be of any concern? If that is happening in the
> > > control-path and does not affect the data-path as such; those added
> > > instructions is a cost of added flexibility to we got through bitmap.
> > > If performance is not at risk then that shouldn't be a problem.
> > Just to be dense... What is wrong with just using u64?
> 
> Hmm. Looks like this is so simple that nobody thought of it seriously. ;)
> 
> This of course needs a bit of glue code in G/SFEATURES handling, but most
> of the change would be s/u32/u64/ in apropriate places.

I am a strong proponent of not building stuff until it is needed.
  http://www.extremeprogramming.org/rules/early.html
By the time 64 bits are exhausted the existing model of network device
may have changed significantly anyway.
  

^ permalink raw reply

* [PATCH v3] net: bnx2x: convert to hw_features
From: Michał Mirosław @ 2011-04-11 19:54 UTC (permalink / raw)
  To: netdev; +Cc: Vladislav Zolotarov, Eilon Greenstein
In-Reply-To: <1302531021.21065.79.camel@lb-tlvb-vladz>

Since ndo_fix_features callback is postponing features change when
bp->recovery_state != BNX2X_RECOVERY_DONE, netdev_update_features()
has to be called again when this condition changes. Previously,
ethtool_ops->set_flags callback returned -EBUSY in that case
(it's not possible in the new model).

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
v3: - include NETIF_F_LRO in hw_features
    - don't call netdev_update_features() if bnx2x_nic_load() failed
v2: - comment in ndo_fix_features callback

 drivers/net/bnx2x/bnx2x_cmn.c     |   52 ++++++++++++++++++--
 drivers/net/bnx2x/bnx2x_cmn.h     |    3 +
 drivers/net/bnx2x/bnx2x_ethtool.c |   95 -------------------------------------
 drivers/net/bnx2x/bnx2x_main.c    |   35 ++++++++------
 4 files changed, 70 insertions(+), 115 deletions(-)

diff --git a/drivers/net/bnx2x/bnx2x_cmn.c b/drivers/net/bnx2x/bnx2x_cmn.c
index e83ac6d..9691b67 100644
--- a/drivers/net/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/bnx2x/bnx2x_cmn.c
@@ -2443,11 +2443,21 @@ alloc_err:
 
 }
 
+static int bnx2x_reload_if_running(struct net_device *dev)
+{
+	struct bnx2x *bp = netdev_priv(dev);
+
+	if (unlikely(!netif_running(dev)))
+		return 0;
+
+	bnx2x_nic_unload(bp, UNLOAD_NORMAL);
+	return bnx2x_nic_load(bp, LOAD_NORMAL);
+}
+
 /* called with rtnl_lock */
 int bnx2x_change_mtu(struct net_device *dev, int new_mtu)
 {
 	struct bnx2x *bp = netdev_priv(dev);
-	int rc = 0;
 
 	if (bp->recovery_state != BNX2X_RECOVERY_DONE) {
 		printk(KERN_ERR "Handling parity error recovery. Try again later\n");
@@ -2464,12 +2474,44 @@ int bnx2x_change_mtu(struct net_device *dev, int new_mtu)
 	 */
 	dev->mtu = new_mtu;
 
-	if (netif_running(dev)) {
-		bnx2x_nic_unload(bp, UNLOAD_NORMAL);
-		rc = bnx2x_nic_load(bp, LOAD_NORMAL);
+	return bnx2x_reload_if_running(dev);
+}
+
+u32 bnx2x_fix_features(struct net_device *dev, u32 features)
+{
+	struct bnx2x *bp = netdev_priv(dev);
+
+	if (bp->recovery_state != BNX2X_RECOVERY_DONE) {
+		netdev_err(dev, "Handling parity error recovery. Try again later\n");
+
+		/* Don't allow bnx2x_set_features() to be called now. */
+		return dev->features;
+	}
+
+	/* TPA requires Rx CSUM offloading */
+	if (!(features & NETIF_F_RXCSUM) || bp->disable_tpa)
+		features &= ~NETIF_F_LRO;
+
+	return features;
+}
+
+int bnx2x_set_features(struct net_device *dev, u32 features)
+{
+	struct bnx2x *bp = netdev_priv(dev);
+	u32 flags = bp->flags;
+
+	if (features & NETIF_F_LRO)
+		flags |= TPA_ENABLE_FLAG;
+	else
+		flags &= ~TPA_ENABLE_FLAG;
+
+	if (flags ^ bp->flags) {
+		bp->flags = flags;
+
+		return bnx2x_reload_if_running(dev);
 	}
 
-	return rc;
+	return 0;
 }
 
 void bnx2x_tx_timeout(struct net_device *dev)
diff --git a/drivers/net/bnx2x/bnx2x_cmn.h b/drivers/net/bnx2x/bnx2x_cmn.h
index 775fef0..1cdab69 100644
--- a/drivers/net/bnx2x/bnx2x_cmn.h
+++ b/drivers/net/bnx2x/bnx2x_cmn.h
@@ -431,6 +431,9 @@ void bnx2x_free_mem_bp(struct bnx2x *bp);
  */
 int bnx2x_change_mtu(struct net_device *dev, int new_mtu);
 
+u32 bnx2x_fix_features(struct net_device *dev, u32 features);
+int bnx2x_set_features(struct net_device *dev, u32 features);
+
 /**
  * tx timeout netdev callback
  *
diff --git a/drivers/net/bnx2x/bnx2x_ethtool.c b/drivers/net/bnx2x/bnx2x_ethtool.c
index 1479994..ad7d91e 100644
--- a/drivers/net/bnx2x/bnx2x_ethtool.c
+++ b/drivers/net/bnx2x/bnx2x_ethtool.c
@@ -1299,91 +1299,6 @@ static int bnx2x_set_pauseparam(struct net_device *dev,
 	return 0;
 }
 
-static int bnx2x_set_flags(struct net_device *dev, u32 data)
-{
-	struct bnx2x *bp = netdev_priv(dev);
-	int changed = 0;
-	int rc = 0;
-
-	if (bp->recovery_state != BNX2X_RECOVERY_DONE) {
-		printk(KERN_ERR "Handling parity error recovery. Try again later\n");
-		return -EAGAIN;
-	}
-
-	if (!(data & ETH_FLAG_RXVLAN))
-		return -EINVAL;
-
-	if ((data & ETH_FLAG_LRO) && bp->rx_csum && bp->disable_tpa)
-		return -EINVAL;
-
-	rc = ethtool_op_set_flags(dev, data, ETH_FLAG_LRO | ETH_FLAG_RXVLAN |
-					ETH_FLAG_TXVLAN | ETH_FLAG_RXHASH);
-	if (rc)
-		return rc;
-
-	/* TPA requires Rx CSUM offloading */
-	if ((data & ETH_FLAG_LRO) && bp->rx_csum) {
-		if (!(bp->flags & TPA_ENABLE_FLAG)) {
-			bp->flags |= TPA_ENABLE_FLAG;
-			changed = 1;
-		}
-	} else if (bp->flags & TPA_ENABLE_FLAG) {
-		dev->features &= ~NETIF_F_LRO;
-		bp->flags &= ~TPA_ENABLE_FLAG;
-		changed = 1;
-	}
-
-	if (changed && netif_running(dev)) {
-		bnx2x_nic_unload(bp, UNLOAD_NORMAL);
-		rc = bnx2x_nic_load(bp, LOAD_NORMAL);
-	}
-
-	return rc;
-}
-
-static u32 bnx2x_get_rx_csum(struct net_device *dev)
-{
-	struct bnx2x *bp = netdev_priv(dev);
-
-	return bp->rx_csum;
-}
-
-static int bnx2x_set_rx_csum(struct net_device *dev, u32 data)
-{
-	struct bnx2x *bp = netdev_priv(dev);
-	int rc = 0;
-
-	if (bp->recovery_state != BNX2X_RECOVERY_DONE) {
-		printk(KERN_ERR "Handling parity error recovery. Try again later\n");
-		return -EAGAIN;
-	}
-
-	bp->rx_csum = data;
-
-	/* Disable TPA, when Rx CSUM is disabled. Otherwise all
-	   TPA'ed packets will be discarded due to wrong TCP CSUM */
-	if (!data) {
-		u32 flags = ethtool_op_get_flags(dev);
-
-		rc = bnx2x_set_flags(dev, (flags & ~ETH_FLAG_LRO));
-	}
-
-	return rc;
-}
-
-static int bnx2x_set_tso(struct net_device *dev, u32 data)
-{
-	if (data) {
-		dev->features |= (NETIF_F_TSO | NETIF_F_TSO_ECN);
-		dev->features |= NETIF_F_TSO6;
-	} else {
-		dev->features &= ~(NETIF_F_TSO | NETIF_F_TSO_ECN);
-		dev->features &= ~NETIF_F_TSO6;
-	}
-
-	return 0;
-}
-
 static const struct {
 	char string[ETH_GSTRING_LEN];
 } bnx2x_tests_str_arr[BNX2X_NUM_TESTS] = {
@@ -2207,16 +2122,6 @@ static const struct ethtool_ops bnx2x_ethtool_ops = {
 	.set_ringparam		= bnx2x_set_ringparam,
 	.get_pauseparam		= bnx2x_get_pauseparam,
 	.set_pauseparam		= bnx2x_set_pauseparam,
-	.get_rx_csum		= bnx2x_get_rx_csum,
-	.set_rx_csum		= bnx2x_set_rx_csum,
-	.get_tx_csum		= ethtool_op_get_tx_csum,
-	.set_tx_csum		= ethtool_op_set_tx_hw_csum,
-	.set_flags		= bnx2x_set_flags,
-	.get_flags		= ethtool_op_get_flags,
-	.get_sg			= ethtool_op_get_sg,
-	.set_sg			= ethtool_op_set_sg,
-	.get_tso		= ethtool_op_get_tso,
-	.set_tso		= bnx2x_set_tso,
 	.self_test		= bnx2x_self_test,
 	.get_sset_count		= bnx2x_get_sset_count,
 	.get_strings		= bnx2x_get_strings,
diff --git a/drivers/net/bnx2x/bnx2x_main.c b/drivers/net/bnx2x/bnx2x_main.c
index f3cf889..5a60269 100644
--- a/drivers/net/bnx2x/bnx2x_main.c
+++ b/drivers/net/bnx2x/bnx2x_main.c
@@ -7728,6 +7728,7 @@ static void bnx2x_parity_recover(struct bnx2x *bp)
 						return;
 					}
 
+					netdev_update_features(bp->dev);
 					return;
 				}
 			} else { /* non-leader */
@@ -7755,10 +7756,12 @@ static void bnx2x_parity_recover(struct bnx2x *bp)
 					  * the "process kill". It's an exit
 					  * point for a non-leader.
 					  */
-					bnx2x_nic_load(bp, LOAD_NORMAL);
+					int rc = bnx2x_nic_load(bp, LOAD_NORMAL);
 					bp->recovery_state =
 						BNX2X_RECOVERY_DONE;
 					smp_wmb();
+					if (!rc)
+						netdev_update_features(bp->dev);
 					return;
 				}
 			}
@@ -8904,8 +8907,6 @@ static int __devinit bnx2x_init_bp(struct bnx2x *bp)
 	bp->multi_mode = multi_mode;
 	bp->int_mode = int_mode;
 
-	bp->dev->features |= NETIF_F_GRO;
-
 	/* Set TPA flags */
 	if (disable_tpa) {
 		bp->flags &= ~TPA_ENABLE_FLAG;
@@ -8954,6 +8955,7 @@ static int __devinit bnx2x_init_bp(struct bnx2x *bp)
 static int bnx2x_open(struct net_device *dev)
 {
 	struct bnx2x *bp = netdev_priv(dev);
+	int rc;
 
 	netif_carrier_off(dev);
 
@@ -8993,7 +8995,10 @@ static int bnx2x_open(struct net_device *dev)
 
 	bp->recovery_state = BNX2X_RECOVERY_DONE;
 
-	return bnx2x_nic_load(bp, LOAD_OPEN);
+	rc = bnx2x_nic_load(bp, LOAD_OPEN);
+	if (!rc)
+		netdev_update_features(bp->dev);
+	return rc;
 }
 
 /* called with rtnl_lock */
@@ -9304,6 +9309,8 @@ static const struct net_device_ops bnx2x_netdev_ops = {
 	.ndo_validate_addr	= eth_validate_addr,
 	.ndo_do_ioctl		= bnx2x_ioctl,
 	.ndo_change_mtu		= bnx2x_change_mtu,
+	.ndo_fix_features	= bnx2x_fix_features,
+	.ndo_set_features	= bnx2x_set_features,
 	.ndo_tx_timeout		= bnx2x_tx_timeout,
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	.ndo_poll_controller	= poll_bnx2x,
@@ -9430,20 +9437,18 @@ static int __devinit bnx2x_init_dev(struct pci_dev *pdev,
 
 	dev->netdev_ops = &bnx2x_netdev_ops;
 	bnx2x_set_ethtool_ops(dev);
-	dev->features |= NETIF_F_SG;
-	dev->features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
+
 	if (bp->flags & USING_DAC_FLAG)
 		dev->features |= NETIF_F_HIGHDMA;
-	dev->features |= (NETIF_F_TSO | NETIF_F_TSO_ECN);
-	dev->features |= NETIF_F_TSO6;
-	dev->features |= (NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX);
 
-	dev->vlan_features |= NETIF_F_SG;
-	dev->vlan_features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
-	if (bp->flags & USING_DAC_FLAG)
-		dev->vlan_features |= NETIF_F_HIGHDMA;
-	dev->vlan_features |= (NETIF_F_TSO | NETIF_F_TSO_ECN);
-	dev->vlan_features |= NETIF_F_TSO6;
+	dev->hw_features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
+		NETIF_F_TSO | NETIF_F_TSO_ECN | NETIF_F_TSO6 |
+		NETIF_F_LRO | NETIF_F_HW_VLAN_TX;
+
+	dev->features |= dev->hw_features | NETIF_F_HW_VLAN_RX;
+
+	dev->vlan_features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
+		NETIF_F_TSO | NETIF_F_TSO_ECN | NETIF_F_TSO6 | NETIF_F_HIGHDMA;
 
 #ifdef BCM_DCBNL
 	dev->dcbnl_ops = &bnx2x_dcbnl_ops;
-- 
1.7.2.5


^ permalink raw reply related

* Re: pull request: wireless-2.6 2011-04-11
From: David Miller @ 2011-04-11 19:56 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20110411193443.GD2511@tuxdriver.com>

From: "John W. Linville" <linville@tuxdriver.com>
Date: Mon, 11 Apr 2011 15:34:43 -0400

> Here is another batch intended for 2.6.39.  Included is an mwl8k fix
> to avoid freeing an irq that we never requested, an ath common code
> fix to add a regulatory domain mapping code, an ath9k fix to avoid some
> crashes on MIPS boards and fix some radio stability issues on AR9285, a
> p54 fix to avoid a device lock-up caused by an uninitialized variable,
> an iwlagn fix to correct for an incorrect antenna configuration value
> in EEPROM on iwl5300 hardware, and an Kconfig fix for iwlegacy to
> keep old kernel build configurations still building iwl3945 and/or
> iwl4965 after the iwlagn/iwlegacy split.
> 
> Please let me know if there are problems!

Pulled, thanks a lot John.

^ permalink raw reply

* Re: [PATCH 1/3] stmmac: fixed dma lib build when turn-on the debug option
From: David Miller @ 2011-04-11 19:56 UTC (permalink / raw)
  To: peppe.cavallaro; +Cc: netdev
In-Reply-To: <1302513406-3758-1-git-send-email-peppe.cavallaro@st.com>

From: Giuseppe CAVALLARO <peppe.cavallaro@st.com>
Date: Mon, 11 Apr 2011 11:16:44 +0200

> This patch fixes a compilation error when build the
> dwmac_lib with the DEBUG option enabled.
> 
> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>

Applied.

^ permalink raw reply

* Re: [PATCH 2/3] stmmac: fix open funct when exit on error
From: David Miller @ 2011-04-11 19:56 UTC (permalink / raw)
  To: peppe.cavallaro; +Cc: netdev, shiraz.hashim
In-Reply-To: <1302513406-3758-2-git-send-email-peppe.cavallaro@st.com>

From: Giuseppe CAVALLARO <peppe.cavallaro@st.com>
Date: Mon, 11 Apr 2011 11:16:45 +0200

> This patch reviews the open function and fixes some
> errors when exit with an error state.
> It also moves the request_irq after core is initialized
> when interrupts are properly masked.
> 
> Signed-off-by: Shiraz Hashim <shiraz.hashim@st.com>
> Hacked-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>

Applied.

^ permalink raw reply

* Re: [PATCH 3/3] stmmac: fix Transmit Underflow error
From: David Miller @ 2011-04-11 19:56 UTC (permalink / raw)
  To: peppe.cavallaro; +Cc: netdev
In-Reply-To: <1302513406-3758-3-git-send-email-peppe.cavallaro@st.com>

From: Giuseppe CAVALLARO <peppe.cavallaro@st.com>
Date: Mon, 11 Apr 2011 11:16:46 +0200

> On some old MAC chips without COE sometime the
> Transmit Underflow error is issued.
> 
> The driver aborted all the transmission process
> and initialized it from scratch.
> This breaks the network activity as raised by Nachiketa
> on a SPEAr board.
> 
> The patch is to fix this rare underflow event.
> The driver will only clear the interrupt and the Tx
> DMA will go out the Suspend state as soon as the
> descriptor is fetched again.
> The driver will continue to bump-up the DMA FIFO threshold
> that, indeed, helped somebody to prevent this kind of error
> in the past as well.
> 
> Reported-by: Nachiketa Prachanda <nprachanda@ncomputing.com>
> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>

Applied.

^ permalink raw reply

* Re: [PATCHv2] caif: Add CAIF HSI Link Layer
From: David Miller @ 2011-04-11 20:08 UTC (permalink / raw)
  To: sjur.brandeland; +Cc: netdev, linus.walleij, sjurbren, daniel.martensson
In-Reply-To: <1302552320-11549-1-git-send-email-sjur.brandeland@stericsson.com>

From: sjur.brandeland@stericsson.com
Date: Mon, 11 Apr 2011 22:05:20 +0200

> I've made cfhsi_tx_frm lock protected (by moving some spin_unlock)
> and added a WARN_ON(!spin_is_locked(...)) to be safe.

spin_is_locked() always returns false on uniprocessor builds.

^ permalink raw reply

* r8169 doesn't report link state correctly.
From: Ben Greear @ 2011-04-11 20:09 UTC (permalink / raw)
  To: netdev

I notice that in kernel 2.6.38-wl, the realtek 8169 NIC doesn't
report link down when in fact there is no cable connected.  Instead,
it shows 10Mbps half-duplex.  I have no idea if this previously worked
or not.

[root@lec2010-ath9k-1 lanforge]# ethtool -i eth1
driver: r8169
version: 2.3LK-NAPI
firmware-version:
bus-info: 0000:02:00.0
[root@lec2010-ath9k-1 lanforge]# ethtool eth1
Settings for eth1:
	Supported ports: [ TP MII ]
	Supported link modes:   10baseT/Half 10baseT/Full
	                        100baseT/Half 100baseT/Full
	                        1000baseT/Half 1000baseT/Full
	Supports auto-negotiation: Yes
	Advertised link modes:  10baseT/Half 10baseT/Full
	                        100baseT/Half 100baseT/Full
	                        1000baseT/Half 1000baseT/Full
	Advertised pause frame use: No
	Advertised auto-negotiation: Yes
	Speed: 10Mb/s
	Duplex: Half
	Port: MII
	PHYAD: 0
	Transceiver: internal
	Auto-negotiation: on
	Supports Wake-on: pumbg
	Wake-on: g
	Current message level: 0x00000033 (51)
			       drv probe ifdown ifup
	Link detected: yes

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