Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next-2.6 v2] net: ping: dont call udp_ioctl()
From: Vasiliy Kulikov @ 2011-05-16 12:48 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, solar, linux-kernel, netdev, peak, kees.cook,
	dan.j.rosenberg, eugene, nelhage, kuznet, pekkas, jmorris,
	yoshfuji, kaber
In-Reply-To: <1305530791.3120.217.camel@edumazet-laptop>

On Mon, May 16, 2011 at 09:26 +0200, Eric Dumazet wrote:
> Le dimanche 15 mai 2011 à 17:44 -0400, David Miller a écrit :
> 
> > Just get rid of ping_ioctl() entirely, as that is the effect of
> > this change since inet_ioctl() returns -ENOIOCTLCMD when
> > sk_prot->ioctl is NULL.
> > 
> > Also get rid of asm/ioctls.h since that will be no longer needed.
> 
> Sure, here is updated version, thanks.
> 
> [PATCH net-next-2.6 v2] net: ping: dont call udp_ioctl()
> 
> udp_ioctl() really handles UDP and UDPLite protocols.
> 
> 1) It can increment UDP_MIB_INERRORS in case first_packet_length() finds
> a frame with bad checksum.
> 
> 2) It has a dependency on sizeof(struct udphdr), not applicable to
> ICMP/PING
> 
> If ping sockets need to handle SIOCINQ/SIOCOUTQ ioctl, this should be
> done differently.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Acked-by: Vasiliy Kulikov <segoon@openwall.com>

Thanks,

-- 
Vasiliy Kulikov
http://www.openwall.com - bringing security into open computing environments

^ permalink raw reply

* Re: several packets in a single buffer in Rx
From: Emmanuel Grumbach @ 2011-05-16 12:59 UTC (permalink / raw)
  To: Michał Mirosław
  Cc: netdev, linux-wireless, Johannes Berg, Guy, Wey-Yi, guy.cohen
In-Reply-To: <BANLkTinb3kaK7_b=47AL+Mb6Zmt9twEM9g@mail.gmail.com>

2011/5/16 Michał Mirosław <mirqus@gmail.com>:
> 2011/5/16 Emmanuel Grumbach <egrumbach@gmail.com>:
>> I would like to be able to deliver the same page several times to the
>> stack without having the stack consume it before the last time I
>> deliver it.
>> Of course I would like to avoid cloning it.
>
> Just do get_page() on the page having another packet in it before
> passing skb up.
>

I can see the path:
__kfree_skb -> skb_release_all -> skb_release_data -> put_page
put_page will free the page iff the _count variable reaches 0. Of course,
_count is incremented by get_page.

I will give it try.

I understand that this will work regardless the order given to
alloc_pages right ?

Thanks

^ permalink raw reply

* Re: several packets in a single buffer in Rx
From: Michał Mirosław @ 2011-05-16 13:13 UTC (permalink / raw)
  To: Emmanuel Grumbach
  Cc: netdev, linux-wireless, Johannes Berg, Guy, Wey-Yi, guy.cohen
In-Reply-To: <BANLkTi=aBLPX7OMKi=sMhT81x-2f6Ci6sQ@mail.gmail.com>

W dniu 16 maja 2011 14:59 użytkownik Emmanuel Grumbach
<egrumbach@gmail.com> napisał:
> 2011/5/16 Michał Mirosław <mirqus@gmail.com>:
>> 2011/5/16 Emmanuel Grumbach <egrumbach@gmail.com>:
>>> I would like to be able to deliver the same page several times to the
>>> stack without having the stack consume it before the last time I
>>> deliver it.
>>> Of course I would like to avoid cloning it.
>>
>> Just do get_page() on the page having another packet in it before
>> passing skb up.
>>
>
> I can see the path:
> __kfree_skb -> skb_release_all -> skb_release_data -> put_page
> put_page will free the page iff the _count variable reaches 0. Of course,
> _count is incremented by get_page.
>
> I will give it try.
>
> I understand that this will work regardless the order given to
> alloc_pages right ?

Yes. Remember that if you put a lot of packets in a big-order page
then the memory will be freed only after all packets are freed.

Best Regards,
Michał Mirosław

^ permalink raw reply

* [PATCH] net: Change netdev_fix_features messages loglevel
From: Michał Mirosław @ 2011-05-16 13:17 UTC (permalink / raw)
  To: netdev; +Cc: Michael S. Tsirkin, Herbert Xu, Ben Hutchings

Those reduced to DEBUG can possibly be triggered by unprivileged processes
and are nothing exceptional. Illegal checksum combinations can only be
caused by driver bug, so promote those messages to WARN.

Since GSO without SG will now only cause DEBUG message from
netdev_fix_features(), remove the workaround from register_netdevice().

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---

This is for net tree, not net-next.

 net/core/dev.c |   22 ++++++++--------------
 1 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 9200944..b624fe4 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5186,27 +5186,27 @@ u32 netdev_fix_features(struct net_device *dev, u32 features)
 	/* Fix illegal checksum combinations */
 	if ((features & NETIF_F_HW_CSUM) &&
 	    (features & (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM))) {
-		netdev_info(dev, "mixed HW and IP checksum settings.\n");
+		netdev_warn(dev, "mixed HW and IP checksum settings.\n");
 		features &= ~(NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM);
 	}
 
 	if ((features & NETIF_F_NO_CSUM) &&
 	    (features & (NETIF_F_HW_CSUM|NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM))) {
-		netdev_info(dev, "mixed no checksumming and other settings.\n");
+		netdev_warn(dev, "mixed no checksumming and other settings.\n");
 		features &= ~(NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM|NETIF_F_HW_CSUM);
 	}
 
 	/* Fix illegal SG+CSUM combinations. */
 	if ((features & NETIF_F_SG) &&
 	    !(features & NETIF_F_ALL_CSUM)) {
-		netdev_info(dev,
-			    "Dropping NETIF_F_SG since no checksum feature.\n");
+		netdev_dbg(dev,
+			"Dropping NETIF_F_SG since no checksum feature.\n");
 		features &= ~NETIF_F_SG;
 	}
 
 	/* TSO requires that SG is present as well. */
 	if ((features & NETIF_F_ALL_TSO) && !(features & NETIF_F_SG)) {
-		netdev_info(dev, "Dropping TSO features since no SG feature.\n");
+		netdev_dbg(dev, "Dropping TSO features since no SG feature.\n");
 		features &= ~NETIF_F_ALL_TSO;
 	}
 
@@ -5216,7 +5216,7 @@ u32 netdev_fix_features(struct net_device *dev, u32 features)
 
 	/* Software GSO depends on SG. */
 	if ((features & NETIF_F_GSO) && !(features & NETIF_F_SG)) {
-		netdev_info(dev, "Dropping NETIF_F_GSO since no SG feature.\n");
+		netdev_dbg(dev, "Dropping NETIF_F_GSO since no SG feature.\n");
 		features &= ~NETIF_F_GSO;
 	}
 
@@ -5226,13 +5226,13 @@ u32 netdev_fix_features(struct net_device *dev, u32 features)
 		if (!((features & NETIF_F_GEN_CSUM) ||
 		    (features & (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM))
 			    == (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM))) {
-			netdev_info(dev,
+			netdev_dbg(dev,
 				"Dropping NETIF_F_UFO since no checksum offload features.\n");
 			features &= ~NETIF_F_UFO;
 		}
 
 		if (!(features & NETIF_F_SG)) {
-			netdev_info(dev,
+			netdev_dbg(dev,
 				"Dropping NETIF_F_UFO since no NETIF_F_SG feature.\n");
 			features &= ~NETIF_F_UFO;
 		}
@@ -5414,12 +5414,6 @@ int register_netdevice(struct net_device *dev)
 	dev->features |= NETIF_F_SOFT_FEATURES;
 	dev->wanted_features = dev->features & dev->hw_features;
 
-	/* Avoid warning from netdev_fix_features() for GSO without SG */
-	if (!(dev->wanted_features & NETIF_F_SG)) {
-		dev->wanted_features &= ~NETIF_F_GSO;
-		dev->features &= ~NETIF_F_GSO;
-	}
-
 	/* Enable GRO and NETIF_F_HIGHDMA for vlans by default,
 	 * vlan_dev_init() will do the dev->features check, so these features
 	 * are enabled only if supported by underlying device.


^ permalink raw reply related

* [PATCH] ethtool: ETHTOOL_SFEATURES: remove NETIF_F_COMPAT return
From: Michał Mirosław @ 2011-05-16 13:28 UTC (permalink / raw)
  To: netdev; +Cc: Ben Hutchings, David Miller
In-Reply-To: <20110516121339.GA1094@rere.qmqm.pl>

Remove NETIF_F_COMPAT since it's redundant and will be unused after
all drivers are converted to fix/set_features.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---

For net as we don't want to have ETHTOOL_F_COMPAT hit stable release.

 include/linux/ethtool.h |    5 -----
 net/core/ethtool.c      |    2 +-
 2 files changed, 1 insertions(+), 6 deletions(-)

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index dc80d82..5fb916c 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -625,9 +625,6 @@ struct ethtool_sfeatures {
  *      Probably there are other device-specific constraints on some features
  *      in the set. When %ETHTOOL_F_UNSUPPORTED is set, .valid is considered
  *      here as though ignored bits were cleared.
- *   %ETHTOOL_F_COMPAT - some or all changes requested were made by calling
- *      compatibility functions. Requested offload state cannot be properly
- *      managed by kernel.
  *
  * Meaning of bits in the masks are obtained by %ETHTOOL_GSSET_INFO (number of
  * bits in the arrays - always multiple of 32) and %ETHTOOL_GSTRINGS commands
@@ -637,12 +634,10 @@ struct ethtool_sfeatures {
 enum ethtool_sfeatures_retval_bits {
 	ETHTOOL_F_UNSUPPORTED__BIT,
 	ETHTOOL_F_WISH__BIT,
-	ETHTOOL_F_COMPAT__BIT,
 };
 
 #define ETHTOOL_F_UNSUPPORTED   (1 << ETHTOOL_F_UNSUPPORTED__BIT)
 #define ETHTOOL_F_WISH          (1 << ETHTOOL_F_WISH__BIT)
-#define ETHTOOL_F_COMPAT        (1 << ETHTOOL_F_COMPAT__BIT)
 
 #ifdef __KERNEL__
 
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 74ead9e..dc07569 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -308,7 +308,7 @@ static int ethtool_set_features(struct net_device *dev, void __user *useraddr)
 		return -EINVAL;
 
 	if (ethtool_set_features_compat(dev, features))
-		ret |= ETHTOOL_F_COMPAT;
+		ret |= ETHTOOL_F_WISH;
 
 	if (features[0].valid & ~dev->hw_features) {
 		features[0].valid &= dev->hw_features;


^ permalink raw reply related

* Re: several packets in a single buffer in Rx
From: Emmanuel Grumbach @ 2011-05-16 13:36 UTC (permalink / raw)
  To: Michał Mirosław
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA, Johannes Berg, Guy, Wey-Yi,
	guy.cohen-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <BANLkTi=xJbL-26Hzg0Pp5zKPUi35bV6VmA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

2011/5/16 Michał Mirosław <mirqus-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
> W dniu 16 maja 2011 14:59 użytkownik Emmanuel Grumbach
> <egrumbach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> napisał:
>> 2011/5/16 Michał Mirosław <mirqus-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
>>> 2011/5/16 Emmanuel Grumbach <egrumbach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
>>>> I would like to be able to deliver the same page several times to the
>>>> stack without having the stack consume it before the last time I
>>>> deliver it.
>>>> Of course I would like to avoid cloning it.
>>>
>>> Just do get_page() on the page having another packet in it before
>>> passing skb up.
>>>
>>
>> I can see the path:
>> __kfree_skb -> skb_release_all -> skb_release_data -> put_page
>> put_page will free the page iff the _count variable reaches 0. Of course,
>> _count is incremented by get_page.
>>
>> I will give it try.
>>
>> I understand that this will work regardless the order given to
>> alloc_pages right ?
>
> Yes. Remember that if you put a lot of packets in a big-order page
> then the memory will be freed only after all packets are freed.

Sure. Thanks for the help.

>
> Best Regards,
> Michał Mirosław
>
--
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] ethtool: ETHTOOL_SFEATURES: remove NETIF_F_COMPAT return
From: Ben Hutchings @ 2011-05-16 13:37 UTC (permalink / raw)
  To: Michał Mirosław; +Cc: netdev, David Miller
In-Reply-To: <20110516132807.1A89F13A6A@rere.qmqm.pl>

On Mon, 2011-05-16 at 15:28 +0200, Michał Mirosław wrote:
> Remove NETIF_F_COMPAT since it's redundant and will be unused after
> all drivers are converted to fix/set_features.
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---
> 
> For net as we don't want to have ETHTOOL_F_COMPAT hit stable release.
[...]

ETHTOOL_F_WISH means that the requested features could not all be
enabled, *but are remembered*.  ETHTOOL_F_COMPAT means they were not
remembered.

Ben.

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

* stateless nat *please* tell me how I'm supposed to use it
From: Erik Slagter @ 2011-05-16 13:55 UTC (permalink / raw)
  To: netdev

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

Hello devs,

Normally I wouldn't dare to ask my question here, but really there is
nothing to be found that explains how to do this.

For a high availability router-cluster I need to do static nat. The only
thing it needs to do is replace a dst ipv4 address or a src ipv4
address, depending on src/dst host ip. It does not need to keep any
state / connection tracking etc. Actually I'd prefer to not have it
tracked. Both routers need to be able to take over routing from the
other at any given moment and therefore cannot transfer state
information to the other one.

These are the approaches I've tried and/or considered:

- use iptables stateful NAT in combination with a NOTRACK target:
doesn't work, as soon as a packet is marked NOTRACK, it doesn't get
NATted at all
- use iptables stateful NAT anyway and hope for the best: this kind of
works, but I am not happy with it
- use stateless nat from "ip route": abandoned because "deprecated"
- use "mangle" from iptables: doesn't work because you can't "mangle"
the addresses (which is kind of stupid imho)
- use conntrackd: that will probably work, but it's way too complex for
something simple I want to do.
- finally: use tc action nat as seem to be recommended of late.

Attempt 1: using "tc filter ... action nat ..." syntax.

# tc filter add dev eth0 parent root protocol ip prio 10 u32 match u32 0
0 action nat ingress 1.2.3.4 4.5.6.7
RTNETLINK answers: Invalid argument

This suggests that the construct is recognised by tc but the kernel
doesn't (fully) understand this. That's weird because devs said earlier
it should work from somewhere 2.6.29 onwards. I've added some printk's
to the act_nat.c file and that learns that this code isn't called at
all, so probably something else (rtnetlink?) already bails out. So...
this doesn't work.

Attempt 2: using "tc action" syntax

This syntax is not described anywere other than in the help, so I'm just
try-and-error-ing.

# tc action add nat egress 1.2.3.4 4.5.6.7

Now this works, well, it doesn't throw errors. It doesn't do anything
either. I guess this action needs to be called from a filter, other than
incorporating it INTO the filter (which doesn't work). There is no
syntax (described) that allows a filter to call an action indirectly.

So my question is, how am I supposed to do something that simple as
replacing a few bytes in an ip header with the effect of a stateless nat.

Thanks very much.



[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5110 bytes --]

^ permalink raw reply

* Re: [PATCH] ethtool: ETHTOOL_SFEATURES: remove NETIF_F_COMPAT return
From: Michał Mirosław @ 2011-05-16 14:23 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: netdev, David Miller
In-Reply-To: <1305553066.19966.32.camel@localhost>

On Mon, May 16, 2011 at 02:37:46PM +0100, Ben Hutchings wrote:
> On Mon, 2011-05-16 at 15:28 +0200, Michał Mirosław wrote:
> > Remove NETIF_F_COMPAT since it's redundant and will be unused after
> > all drivers are converted to fix/set_features.
> > 
> > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > ---
> > 
> > For net as we don't want to have ETHTOOL_F_COMPAT hit stable release.
> [...]
> ETHTOOL_F_WISH means that the requested features could not all be
> enabled, *but are remembered*.  ETHTOOL_F_COMPAT means they were not
> remembered.

Hmm. So, lets just revert 39fc0ce5710c53bad14aaba1a789eec810c556f9
(net: Implement SFEATURES compatibility for not updated drivers).

Best Regards,
Michał Mirosław

^ permalink raw reply

* Re: [RFC] ethernet: avoid pre-assigned OUI values in random_ether_addr
From: Rick Jones @ 2011-05-16 14:29 UTC (permalink / raw)
  To: Bill Fink; +Cc: Stephen Hemminger, netdev
In-Reply-To: <20110514022825.bcd09e14.billfink@mindspring.com>

On Sat, 2011-05-14 at 02:28 -0400, Bill Fink wrote:
> On Fri, 13 May 2011, Rick Jones wrote:
> 
> > > Well, then by a quick glance at
> > > http://standards.ieee.org/develop/regauth/oui/oui.txt it looks like
> > > there are more to add... (some overlap with your list, I've not checked
> > > the entire thing, how the name conflicts should be resolved I've no
> > > idea)
> > > 
> > > raj@tardy:~$ grep ^02- oui.txt 
> > > 02-07-01   (hex)		RACAL-DATACOM
> > > 02-1C-7C   (hex)		PERQ SYSTEMS CORPORATION
> > > 02-60-86   (hex)		LOGIC REPLACEMENT TECH. LTD.
> > > 02-60-8C   (hex)		3COM CORPORATION
> > > 02-70-01   (hex)		RACAL-DATACOM
> > > 02-70-B0   (hex)		M/A-COM INC. COMPANIES
> > > 02-70-B3   (hex)		DATA RECALL LTD
> > > 02-9D-8E   (hex)		CARDIAC RECORDERS INC.
> > > 02-AA-3C   (hex)		OLIVETTI TELECOMM SPA (OLTECO)
> > > 02-BB-01   (hex)		OCTOTHORPE CORP.
> > > 02-C0-8C   (hex)		3COM CORPORATION
> > > 02-CF-1C   (hex)		COMMUNICATION MACHINERY CORP.
> > > 02-E6-D3   (hex)		NIXDORF COMPUTER CORPORATION
> > > 
> > > The ieee's list doesn't have any 52's though.
> > 
> > It does have some A's though:
> > ~$ grep ^.[26AE]- oui.txt 
> > [the aforementioned 02's]
> > AA-00-00   (hex)		DIGITAL EQUIPMENT CORPORATION
> > AA-00-01   (hex)		DIGITAL EQUIPMENT CORPORATION
> > AA-00-02   (hex)		DIGITAL EQUIPMENT CORPORATION
> > AA-00-03   (hex)		DIGITAL EQUIPMENT CORPORATION
> > AA-00-04   (hex)		DIGITAL EQUIPMENT CORPORATION
> > 
> > which if I've not botched my bits has the locally administered bit set.
> 
> The AA addresses are used by DECNET and related protocols.

Um, ok, I was figuring something along those lines - but regardless, in
the context of Stephen's RFC, they are pre-assigned OUIs with the
locally administered bit set and presumably as such ones that should not
be assigned by the random ethernet address generator.  Assuming that
checking for such things is indeed necessary in the first place.

rick jones


^ permalink raw reply

* [PATCH v3 0/2] Fix uevent race in register_netdevice()
From: Kalle Valo @ 2011-05-16 14:46 UTC (permalink / raw)
  To: netdev; +Cc: linux-wireless, linux-kernel

I'm trying to fix a race in register_netdevice(). The problem is that
there's a uevent to userspace before the netdevice is ready for use. The
problem is described here:

https://bugzilla.kernel.org/show_bug.cgi?id=15606

I have sent few different ways to fix this, but none of them have been
really usable. Now I came up with a way which changes the driver core
to make it possible send the uevent in a separate call. This is a clean
and safe way to fix the race. Downside is that two new functions are
added to the driver core interface.

Please comment.

---

Kalle Valo (2):
      driver core: add device_add_noevent() and device_uevent()
      net: postpone net device uevent to fix a race


 drivers/base/core.c    |   76 +++++++++++++++++++++++++++++++++++++-----------
 include/linux/device.h |    2 +
 net/core/dev.c         |    3 ++
 net/core/net-sysfs.c   |    2 +
 4 files changed, 65 insertions(+), 18 deletions(-)


^ permalink raw reply

* [PATCH v3 1/2] driver core: add device_add_noevent() and device_uevent()
From: Kalle Valo @ 2011-05-16 14:46 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20110516143913.13838.85357.stgit-bi+AKbBUZKagILUCTcTcHdKyNwTtLsGr@public.gmane.org>

From: Kalle Valo <kalle.valo-DlyHzToyqoxBDgjK7y7TUQ@public.gmane.org>

To make it possible to postpone uevent after the device is registered
create the two new functions: device_add_noevent() and device_uevent().
This is needed by register_netdevice() to fix a race where the netdevice
is not ready until the initialisation has finished.

Signed-off-by: Kalle Valo <kalle.valo-DlyHzToyqoxBDgjK7y7TUQ@public.gmane.org>
---
 drivers/base/core.c    |   76 +++++++++++++++++++++++++++++++++++++-----------
 include/linux/device.h |    2 +
 2 files changed, 61 insertions(+), 17 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 81b78ed..54208e0 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -872,22 +872,7 @@ int device_private_init(struct device *dev)
 	return 0;
 }
 
-/**
- * device_add - add device to device hierarchy.
- * @dev: device.
- *
- * This is part 2 of device_register(), though may be called
- * separately _iff_ device_initialize() has been called separately.
- *
- * This adds @dev to the kobject hierarchy via kobject_add(), adds it
- * to the global and sibling lists for the device, then
- * adds it to the other relevant subsystems of the driver model.
- *
- * NOTE: _Never_ directly free @dev after calling this function, even
- * if it returned an error! Always use put_device() to give up your
- * reference instead.
- */
-int device_add(struct device *dev)
+static int __device_add(struct device *dev, bool event)
 {
 	struct device *parent = NULL;
 	struct class_interface *class_intf;
@@ -974,7 +959,9 @@ int device_add(struct device *dev)
 		blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
 					     BUS_NOTIFY_ADD_DEVICE, dev);
 
-	kobject_uevent(&dev->kobj, KOBJ_ADD);
+	if (event)
+		kobject_uevent(&dev->kobj, KOBJ_ADD);
+
 	bus_probe_device(dev);
 	if (parent)
 		klist_add_tail(&dev->p->knode_parent,
@@ -1026,6 +1013,61 @@ name_error:
 }
 
 /**
+ * device_add - add device to device hierarchy.
+ * @dev: device.
+ *
+ * This is part 2 of device_register(), though may be called
+ * separately _iff_ device_initialize() has been called separately.
+ *
+ * This adds @dev to the kobject hierarchy via kobject_add(), adds it
+ * to the global and sibling lists for the device, then
+ * adds it to the other relevant subsystems of the driver model.
+ *
+ * NOTE: _Never_ directly free @dev after calling this function, even
+ * if it returned an error! Always use put_device() to give up your
+ * reference instead.
+ */
+int device_add(struct device *dev)
+{
+	return __device_add(dev, true);
+}
+
+/**
+ * device_add - add device to device hierarchy but omit uevent
+ * @dev: device.
+ *
+ * This is part 2 of device_register(), though may be called
+ * separately _iff_ device_initialize() has been called separately.
+ *
+ * This adds @dev to the kobject hierarchy via kobject_add(), adds it
+ * to the global and sibling lists for the device, then
+ * adds it to the other relevant subsystems of the driver model.
+ *
+ * This version differs from device_add() so that it does not emit uevent.
+ * Instead call device_uevent() separately.
+ *
+ * NOTE: _Never_ directly free @dev after calling this function, even
+ * if it returned an error! Always use put_device() to give up your
+ * reference instead.
+ */
+int device_add_noevent(struct device *dev)
+{
+	return __device_add(dev, false);
+}
+
+/**
+ * device_uevent - emit uevent for device addition
+ * @dev: device.
+ *
+ * This is part 2 of device_add_noevent(). It emits the signal which was
+ * not sent when device_add_noevent() was called.
+ */
+void device_uevent(struct device *dev)
+{
+	kobject_uevent(&dev->kobj, KOBJ_ADD);
+}
+
+/**
  * device_register - register a device with the system.
  * @dev: pointer to the device structure
  *
diff --git a/include/linux/device.h b/include/linux/device.h
index ab8dfc0..408d4f0 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -546,6 +546,8 @@ extern int __must_check device_register(struct device *dev);
 extern void device_unregister(struct device *dev);
 extern void device_initialize(struct device *dev);
 extern int __must_check device_add(struct device *dev);
+extern int __must_check device_add_noevent(struct device *dev);
+extern void device_uevent(struct device *dev);
 extern void device_del(struct device *dev);
 extern int device_for_each_child(struct device *dev, void *data,
 		     int (*fn)(struct device *dev, void *data));

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

* [PATCH v3 2/2] net: postpone net device uevent to fix a race
From: Kalle Valo @ 2011-05-16 14:46 UTC (permalink / raw)
  To: netdev; +Cc: linux-wireless, linux-kernel
In-Reply-To: <20110516143913.13838.85357.stgit@localhost6.localdomain6>

From: Kalle Valo <kalle.valo@atheros.com>

There's a race in register_netdevice() so that the udev event is sent before
the device is actually ready. This was visible with flimflam, chrome os
connection manager:

00:21:35 roska flimflamd[2598]: src/udev.c:add_net_device()
00:21:35 roska flimflamd[2598]: connman_inet_ifname: SIOCGIFNAME(index
   4): No such device

So the kobject is visible in udev before the device is ready.

The issue is reported here:

https://bugzilla.kernel.org/show_bug.cgi?id=15606

Fix this by postponing the uevent with device_add_noevent() and
calling device_uevent() once the device is really ready.

Thanks to Johannes Berg for the original idea.

Signed-off-by: Kalle Valo <kalle.valo@atheros.com>
---
 net/core/dev.c       |    3 +++
 net/core/net-sysfs.c |    2 +-
 2 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 856b6ee..80cfa23 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5454,6 +5454,9 @@ int register_netdevice(struct net_device *dev)
 		rollback_registered(dev);
 		dev->reg_state = NETREG_UNREGISTERED;
 	}
+
+	device_uevent(&(dev->dev));
+
 	/*
 	 *	Prevent userspace races by waiting until the network
 	 *	device is fully setup before sending notifications.
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 5ceb257..c10b1d5 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -1337,7 +1337,7 @@ int netdev_register_kobject(struct net_device *net)
 #endif
 #endif /* CONFIG_SYSFS */
 
-	error = device_add(dev);
+	error = device_add_noevent(dev);
 	if (error)
 		return error;
 


^ permalink raw reply related

* Re: [PATCH] ethtool: ETHTOOL_SFEATURES: remove NETIF_F_COMPAT return
From: Ben Hutchings @ 2011-05-16 14:53 UTC (permalink / raw)
  To: Michał Mirosław; +Cc: netdev, David Miller
In-Reply-To: <20110516142340.GA2980@rere.qmqm.pl>

On Mon, 2011-05-16 at 16:23 +0200, Michał Mirosław wrote:
> On Mon, May 16, 2011 at 02:37:46PM +0100, Ben Hutchings wrote:
> > On Mon, 2011-05-16 at 15:28 +0200, Michał Mirosław wrote:
> > > Remove NETIF_F_COMPAT since it's redundant and will be unused after
> > > all drivers are converted to fix/set_features.
> > > 
> > > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > > ---
> > > 
> > > For net as we don't want to have ETHTOOL_F_COMPAT hit stable release.
> > [...]
> > ETHTOOL_F_WISH means that the requested features could not all be
> > enabled, *but are remembered*.  ETHTOOL_F_COMPAT means they were not
> > remembered.
> 
> Hmm. So, lets just revert 39fc0ce5710c53bad14aaba1a789eec810c556f9
> (net: Implement SFEATURES compatibility for not updated drivers).

That's also problematic because it means we can't make any use of the
'available' masks from ETHTOOL_GFEATURES.

The patch I sent is actually tested with a modified ethtool.  The
fallback works.  I don't think you've tested whether any of your
proposals can actually practically be used by ethtool.

Ben.

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

* Re: [PATCH] ethtool: ETHTOOL_SFEATURES: remove NETIF_F_COMPAT return
From: Michał Mirosław @ 2011-05-16 15:01 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: Michał Mirosław, netdev, David Miller
In-Reply-To: <1305557597.2885.5.camel@bwh-desktop>

W dniu 16 maja 2011 16:53 użytkownik Ben Hutchings
<bhutchings@solarflare.com> napisał:
> On Mon, 2011-05-16 at 16:23 +0200, Michał Mirosław wrote:
>> On Mon, May 16, 2011 at 02:37:46PM +0100, Ben Hutchings wrote:
>> > On Mon, 2011-05-16 at 15:28 +0200, Michał Mirosław wrote:
>> > > Remove NETIF_F_COMPAT since it's redundant and will be unused after
>> > > all drivers are converted to fix/set_features.
>> > >
>> > > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
>> > > ---
>> > >
>> > > For net as we don't want to have ETHTOOL_F_COMPAT hit stable release.
>> > [...]
>> > ETHTOOL_F_WISH means that the requested features could not all be
>> > enabled, *but are remembered*.  ETHTOOL_F_COMPAT means they were not
>> > remembered.
>>
>> Hmm. So, lets just revert 39fc0ce5710c53bad14aaba1a789eec810c556f9
>> (net: Implement SFEATURES compatibility for not updated drivers).
> That's also problematic because it means we can't make any use of the
> 'available' masks from ETHTOOL_GFEATURES.
>
> The patch I sent is actually tested with a modified ethtool.  The
> fallback works.  I don't think you've tested whether any of your
> proposals can actually practically be used by ethtool.

You haven't posted the updates to ethtool to netdev. I just work blind here.

For now I use userspace version I sent along some of the first series
of the conversion patches. It actually makes it easier to use if the
compatibility handling stays in kernel (doesn't care about
ETHTOOL_F_COMPAT, BTW).

Best Regards,
Michał Mirosław

^ permalink raw reply

* RE: [RFC 2/3] RDMA/cma: Add support for netlink statistics export
From: Hefty, Sean @ 2011-05-16 15:08 UTC (permalink / raw)
  To: Or Gerlitz
  Cc: Roland Dreier, netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <4DD0DAB2.1080600-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

> Isn't the port space enough here? specifically, what qp type buys us
> over port space?

That assumes a 1:1 relationship between the port space and QP type, or more specifically, that the IB SID range is divided based on QP type.  This is outside of the architecture.  If we look at UC or XRC, it's not clear what port space those map to, but I believe it's desirable for the rdma_cm to support those.  Note that the AF_IB patch set ended up requiring that the user specify the qp type for these reasons.

- Sean
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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: [RFC] ethernet: avoid pre-assigned OUI values in random_ether_addr
From: Stephen Hemminger @ 2011-05-16 15:46 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Joe Perches, netdev
In-Reply-To: <1305493826.3120.174.camel@edumazet-laptop>

On Sun, 15 May 2011 23:10:26 +0200
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> Le dimanche 15 mai 2011 à 13:20 -0700, Joe Perches a écrit :
> > On Sun, 2011-05-15 at 21:46 +0200, Eric Dumazet wrote:
> > > Le vendredi 13 mai 2011 à 17:17 -0700, Stephen Hemminger a écrit :
> > > > There are some addresses in the assigned vendor block that don't obey
> > > > the locally assigned convention. These should be avoided by random_ether_addr
> > > > assignment.
> > > We call random_ether_addr() for some virtual devices, maybe we can add a
> > > __random_ether_addr() helper for them and not avoid these OUI ?
> > 
> > Unless it's speed critical, it's probably not worthwhile.
> > 
> 
> Speed was not my concern, but getting idea of why avoiding pre-assigned
> OUI was a concern for them, if they dont hit a real Ethernet domain.

My concern was that after some discussion with IEEE committee that many
virtual environments are using locally assigned addresses that get bridged
onto real networks.

That started me thinking that the current code should be more careful
to avoid potential conflicts. My opinion is that this not worth worrying
about because the likelihood of conflict with any one of these old
addresses is as about as the unlikely as two hosts choosing the same
value. But I wanted to raise the issue for explicit discussion and frame
it with what would be required to handle it.

-- 

^ permalink raw reply

* Re: kernel bug relating to networking
From: Daniel Baluta @ 2011-05-16 15:47 UTC (permalink / raw)
  To: James; +Cc: Kernel Mailing List, netdev
In-Reply-To: <4DD14424.8000003@lockie.ca>

On Mon, May 16, 2011 at 6:35 PM, James <bjlockie@lockie.ca> wrote:
> I originally posted to linux-net@vger.kernel.org but all that list

This should be netdev@vger.kernel.org.

> received in the last day was 3 Russian (spam?) messages.
> I'm hoping here will be less spam.
>
> I'm having a wireless problem that I think is kernel issue.
> kernel-2.6.36.4 is good but recent kernels are bad.
> CompatWireless (recent wireless code) works on kernel-2.6.36.4 so I
> don't think it is the wireless subsystem..
>
> I have a Network controller: Atheros Communications Inc. AR5008 Wireless
> Network Adapter (rev 01).
>
> "iwlist wlan0 scan | grep SSID" under kernel-2.6.36.4 returns  ~15 APs
> and associates to my AP.
> Recent kernels are bad and return ~0-3 APs and do not associate to my AP.
>
> I need help to debug this.
>
> I think the scans fail whenever dmesg shows this:
>> ath: Failed to stop TX DMA in 100 msec after killing last frame
>> ath: Failed to stop TX DMA!
> It seems to be 100% if I boot a kernel and that message is there, it'll
> be a bad kernel.
>
> I tried:
> $ git clone
> git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-2.6.37.y.git
> $ cd linux-2.6.37.y
> # git tag
> What tag is 2.6.36.4?
>
> v2.6.36
> v2.6.36-rc1
> v2.6.36-rc2
> v2.6.36-rc3
> v2.6.36-rc4
> v2.6.36-rc5
> v2.6.36-rc6
> v2.6.36-rc7
> v2.6.36-rc8
> v2.6.37
> v2.6.37-rc1
> v2.6.37-rc2
> v2.6.37-rc3
> v2.6.37-rc4
> v2.6.37-rc5
> v2.6.37-rc6
> v2.6.37-rc7
> v2.6.37-rc8
> v2.6.37.1
> v2.6.37.2
> v2.6.37.3
> v2.6.37.4
> v2.6.37.5
> v2.6.37.6
>
> Is the next step to do:
> $ git bisect start
> $ git bisect good v2.6.36.4
> $ git bisect bad v2.6.37.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

^ permalink raw reply

* Re: [PATCH] 2.6.38 ENC28J60 works with half-duplex DMA
From: David Miller @ 2011-05-16 15:48 UTC (permalink / raw)
  To: elpa.rizzo; +Cc: netdev
In-Reply-To: <BANLkTinG+nj6CmqRYsUuC7MDWaKVFssNMA@mail.gmail.com>


You've sent this patch 3 times, we've noticed and seen them all,
our silence doesn't mean it hasn't been received.

What new thing do you hope to accomplish on each subsequent
submission?

^ permalink raw reply

* Re: [PATCH net-next-2.6 v2] net: ping: dont call udp_ioctl()
From: David Miller @ 2011-05-16 15:50 UTC (permalink / raw)
  To: eric.dumazet
  Cc: solar, segoon, linux-kernel, netdev, peak, kees.cook,
	dan.j.rosenberg, eugene, nelhage, kuznet, pekkas, jmorris,
	yoshfuji, kaber
In-Reply-To: <1305530791.3120.217.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 16 May 2011 09:26:31 +0200

> Le dimanche 15 mai 2011 à 17:44 -0400, David Miller a écrit :
> 
>> Just get rid of ping_ioctl() entirely, as that is the effect of
>> this change since inet_ioctl() returns -ENOIOCTLCMD when
>> sk_prot->ioctl is NULL.
>> 
>> Also get rid of asm/ioctls.h since that will be no longer needed.
> 
> Sure, here is updated version, thanks.
> 
> [PATCH net-next-2.6 v2] net: ping: dont call udp_ioctl()
> 
> udp_ioctl() really handles UDP and UDPLite protocols.
> 
> 1) It can increment UDP_MIB_INERRORS in case first_packet_length() finds
> a frame with bad checksum.
> 
> 2) It has a dependency on sizeof(struct udphdr), not applicable to
> ICMP/PING
> 
> If ping sockets need to handle SIOCINQ/SIOCOUTQ ioctl, this should be
> done differently.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> CC: Vasiliy Kulikov <segoon@openwall.com>

Applied, thanks.

^ permalink raw reply

* [RFC PATCH ethtool 1/3] ethtool: Regularise offload feature settings
From: Ben Hutchings @ 2011-05-16 15:57 UTC (permalink / raw)
  To: netdev; +Cc: linux-net-drivers, Michał Mirosław, netdev,
	David Miller
In-Reply-To: <BANLkTi=Q_RrGgw0H=OeNeJ4JkkCgyYckFA@mail.gmail.com>

This is partly preparation for use of the new net device features API,
but is useful in its own right.

Replace repetitive code for getting/setting offload flags with data-
driven loops.

This changes error messages to use the same long names for offload
flags as in dump_offload(), and changes various exit codes to 1.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 ethtool-util.h |   24 ++++
 ethtool.c      |  321 ++++++++++++++++++-------------------------------------
 2 files changed, 129 insertions(+), 216 deletions(-)

diff --git a/ethtool-util.h b/ethtool-util.h
index 79be7f2..10b8c00 100644
--- a/ethtool-util.h
+++ b/ethtool-util.h
@@ -67,6 +67,30 @@ static inline u64 cpu_to_be64(u64 value)
 
 #define	RX_CLS_LOC_UNSPEC	0xffffffffUL
 
+#ifndef NETIF_F_SG
+#define NETIF_F_SG		(1 << 0)
+#define NETIF_F_IP_CSUM		(1 << 1)
+#define NETIF_F_NO_CSUM		(1 << 2)
+#define NETIF_F_HW_CSUM		(1 << 3)
+#define NETIF_F_IPV6_CSUM	(1 << 4)
+#define NETIF_F_HW_VLAN_TX	(1 << 7)
+#define NETIF_F_HW_VLAN_RX	(1 << 8)
+#define NETIF_F_HW_VLAN_FILTER	(1 << 9)
+#define NETIF_F_GSO		(1 << 11)
+#define NETIF_F_GRO		(1 << 14)
+#define NETIF_F_LRO		(1 << 15)
+#define NETIF_F_TSO		(1 << 16)
+#define NETIF_F_UFO		(1 << 17)
+#define NETIF_F_TSO_ECN		(1 << 19)
+#define NETIF_F_TSO6		(1 << 20)
+#define NETIF_F_NTUPLE		(1 << 27)
+#define NETIF_F_RXHASH		(1 << 28)
+#define NETIF_F_RXCSUM		(1 << 29)
+#define NETIF_F_ALL_CSUM	(NETIF_F_NO_CSUM | NETIF_F_HW_CSUM | \
+				 NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM)
+#define NETIF_F_ALL_TSO 	(NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_TSO_ECN)
+#endif
+
 /* National Semiconductor DP83815, DP83816 */
 int natsemi_dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs *regs);
 int natsemi_dump_eeprom(struct ethtool_drvinfo *info,
diff --git a/ethtool.c b/ethtool.c
index 34fe107..419f349 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -297,15 +297,8 @@ static void show_usage(void)
 static char *devname = NULL;
 
 static int goffload_changed = 0;
-static int off_csum_rx_wanted = -1;
-static int off_csum_tx_wanted = -1;
-static int off_sg_wanted = -1;
-static int off_tso_wanted = -1;
-static int off_ufo_wanted = -1;
-static int off_gso_wanted = -1;
-static u32 off_flags_wanted = 0;
-static u32 off_flags_mask = 0;
-static int off_gro_wanted = -1;
+static u32 off_features_wanted = 0;
+static u32 off_features_mask = 0;
 
 static struct ethtool_pauseparam epause;
 static int gpause_changed = 0;
@@ -446,23 +439,30 @@ static struct cmdline_info cmdline_seeprom[] = {
 };
 
 static struct cmdline_info cmdline_offload[] = {
-	{ "rx", CMDL_BOOL, &off_csum_rx_wanted, NULL },
-	{ "tx", CMDL_BOOL, &off_csum_tx_wanted, NULL },
-	{ "sg", CMDL_BOOL, &off_sg_wanted, NULL },
-	{ "tso", CMDL_BOOL, &off_tso_wanted, NULL },
-	{ "ufo", CMDL_BOOL, &off_ufo_wanted, NULL },
-	{ "gso", CMDL_BOOL, &off_gso_wanted, NULL },
-	{ "lro", CMDL_FLAG, &off_flags_wanted, NULL,
-	  ETH_FLAG_LRO, &off_flags_mask },
-	{ "gro", CMDL_BOOL, &off_gro_wanted, NULL },
-	{ "rxvlan", CMDL_FLAG, &off_flags_wanted, NULL,
-	  ETH_FLAG_RXVLAN, &off_flags_mask },
-	{ "txvlan", CMDL_FLAG, &off_flags_wanted, NULL,
-	  ETH_FLAG_TXVLAN, &off_flags_mask },
-	{ "ntuple", CMDL_FLAG, &off_flags_wanted, NULL,
-	  ETH_FLAG_NTUPLE, &off_flags_mask },
-	{ "rxhash", CMDL_FLAG, &off_flags_wanted, NULL,
-	  ETH_FLAG_RXHASH, &off_flags_mask },
+	{ "rx", CMDL_FLAG, &off_features_wanted, NULL,
+	  NETIF_F_RXCSUM, &off_features_mask },
+	{ "tx", CMDL_FLAG, &off_features_wanted, NULL,
+	  NETIF_F_ALL_CSUM, &off_features_mask },
+	{ "sg", CMDL_FLAG, &off_features_wanted, NULL,
+	  NETIF_F_SG, &off_features_mask },
+	{ "tso", CMDL_FLAG, &off_features_wanted, NULL,
+	  NETIF_F_ALL_TSO, &off_features_mask },
+	{ "ufo", CMDL_FLAG, &off_features_wanted, NULL,
+	  NETIF_F_UFO, &off_features_mask },
+	{ "gso", CMDL_FLAG, &off_features_wanted, NULL,
+	  NETIF_F_GSO, &off_features_mask },
+	{ "lro", CMDL_FLAG, &off_features_wanted, NULL,
+	  NETIF_F_LRO, &off_features_mask },
+	{ "gro", CMDL_FLAG, &off_features_wanted, NULL,
+	  NETIF_F_GRO, &off_features_mask },
+	{ "rxvlan", CMDL_FLAG, &off_features_wanted, NULL,
+	  NETIF_F_HW_VLAN_TX, &off_features_mask },
+	{ "txvlan", CMDL_FLAG, &off_features_wanted, NULL,
+	  NETIF_F_HW_VLAN_TX, &off_features_mask },
+	{ "ntuple", CMDL_FLAG, &off_features_wanted, NULL,
+	  NETIF_F_NTUPLE, &off_features_mask },
+	{ "rxhash", CMDL_FLAG, &off_features_wanted, NULL,
+	  NETIF_F_RXHASH, &off_features_mask },
 };
 
 static struct cmdline_info cmdline_pause[] = {
@@ -1788,35 +1788,36 @@ static int dump_coalesce(void)
 	return 0;
 }
 
-static int dump_offload(int rx, int tx, int sg, int tso, int ufo, int gso,
-			int gro, int lro, int rxvlan, int txvlan, int ntuple,
-			int rxhash)
+static const struct {
+	const char *long_name;
+	u32 cmd;
+	u32 value;
+} off_feature_def[] = {
+	{ "rx-checksumming",		  ETHTOOL_GRXCSUM, NETIF_F_RXCSUM },
+	{ "tx-checksumming",		  ETHTOOL_GTXCSUM, NETIF_F_ALL_CSUM },
+	{ "scatter-gather",		  ETHTOOL_GSG,	   NETIF_F_SG },
+	{ "tcp-segmentation-offload",	  ETHTOOL_GTSO,	   NETIF_F_ALL_TSO },
+	{ "udp-fragmentation-offload",	  ETHTOOL_GUFO,	   NETIF_F_UFO },
+	{ "generic-segmentation-offload", ETHTOOL_GGSO,	   NETIF_F_GSO },
+	{ "generic-receive-offload",	  ETHTOOL_GGRO,	   NETIF_F_GRO },
+	{ "large-receive-offload",	  0,		   NETIF_F_LRO },
+	{ "rx-vlan-offload",		  0,		   NETIF_F_HW_VLAN_RX },
+	{ "tx-vlan-offload",		  0,		   NETIF_F_HW_VLAN_TX },
+	{ "ntuple-filters",		  0,		   NETIF_F_NTUPLE },
+	{ "receive-hashing",		  0,		   NETIF_F_RXHASH },
+};
+
+static int dump_offload(u32 active)
 {
-	fprintf(stdout,
-		"rx-checksumming: %s\n"
-		"tx-checksumming: %s\n"
-		"scatter-gather: %s\n"
-		"tcp-segmentation-offload: %s\n"
-		"udp-fragmentation-offload: %s\n"
-		"generic-segmentation-offload: %s\n"
-		"generic-receive-offload: %s\n"
-		"large-receive-offload: %s\n"
-		"rx-vlan-offload: %s\n"
-		"tx-vlan-offload: %s\n"
-		"ntuple-filters: %s\n"
-		"receive-hashing: %s\n",
-		rx ? "on" : "off",
-		tx ? "on" : "off",
-		sg ? "on" : "off",
-		tso ? "on" : "off",
-		ufo ? "on" : "off",
-		gso ? "on" : "off",
-		gro ? "on" : "off",
-		lro ? "on" : "off",
-		rxvlan ? "on" : "off",
-		txvlan ? "on" : "off",
-		ntuple ? "on" : "off",
-		rxhash ? "on" : "off");
+	u32 value;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(off_feature_def); i++) {
+		value = off_feature_def[i].value;
+		printf("%s: %s\n",
+		       off_feature_def[i].long_name,
+		       (active & value) ? "on" : "off");
+	}
 
 	return 0;
 }
@@ -2139,73 +2140,38 @@ static int do_scoalesce(int fd, struct ifreq *ifr)
 	return 0;
 }
 
+/* the following list of flags are the same as their associated
+ * NETIF_F_xxx values in include/linux/netdevice.h
+ */
+static const u32 flags_dup_features =
+	(ETH_FLAG_LRO | ETH_FLAG_RXVLAN | ETH_FLAG_TXVLAN | ETH_FLAG_NTUPLE |
+	 ETH_FLAG_RXHASH);
+
 static int do_goffload(int fd, struct ifreq *ifr)
 {
 	struct ethtool_value eval;
-	int err, allfail = 1, rx = 0, tx = 0, sg = 0;
-	int tso = 0, ufo = 0, gso = 0, gro = 0, lro = 0, rxvlan = 0, txvlan = 0,
-	    ntuple = 0, rxhash = 0;
+	int err, allfail = 1;
+	u32 features = 0, value;
+	int i;
 
 	fprintf(stdout, "Offload parameters for %s:\n", devname);
 
-	eval.cmd = ETHTOOL_GRXCSUM;
-	ifr->ifr_data = (caddr_t)&eval;
-	err = send_ioctl(fd, ifr);
-	if (err)
-		perror("Cannot get device rx csum settings");
-	else {
-		rx = eval.data;
-		allfail = 0;
-	}
-
-	eval.cmd = ETHTOOL_GTXCSUM;
-	ifr->ifr_data = (caddr_t)&eval;
-	err = send_ioctl(fd, ifr);
-	if (err)
-		perror("Cannot get device tx csum settings");
-	else {
-		tx = eval.data;
-		allfail = 0;
-	}
-
-	eval.cmd = ETHTOOL_GSG;
-	ifr->ifr_data = (caddr_t)&eval;
-	err = send_ioctl(fd, ifr);
-	if (err)
-		perror("Cannot get device scatter-gather settings");
-	else {
-		sg = eval.data;
-		allfail = 0;
-	}
-
-	eval.cmd = ETHTOOL_GTSO;
-	ifr->ifr_data = (caddr_t)&eval;
-	err = send_ioctl(fd, ifr);
-	if (err)
-		perror("Cannot get device tcp segmentation offload settings");
-	else {
-		tso = eval.data;
-		allfail = 0;
-	}
-
-	eval.cmd = ETHTOOL_GUFO;
-	ifr->ifr_data = (caddr_t)&eval;
-	err = ioctl(fd, SIOCETHTOOL, ifr);
-	if (err)
-		perror("Cannot get device udp large send offload settings");
-	else {
-		ufo = eval.data;
-		allfail = 0;
-	}
-
-	eval.cmd = ETHTOOL_GGSO;
-	ifr->ifr_data = (caddr_t)&eval;
-	err = ioctl(fd, SIOCETHTOOL, ifr);
-	if (err)
-		perror("Cannot get device generic segmentation offload settings");
-	else {
-		gso = eval.data;
-		allfail = 0;
+	for (i = 0; i < ARRAY_SIZE(off_feature_def); i++) {
+		value = off_feature_def[i].value;
+		if (!off_feature_def[i].cmd)
+			continue;
+		eval.cmd = off_feature_def[i].cmd;
+		ifr->ifr_data = (caddr_t)&eval;
+		err = send_ioctl(fd, ifr);
+		if (err) {
+			fprintf(stderr,
+				"Cannot get device %s settings: %m\n",
+				off_feature_def[i].long_name);
+		} else {
+			if (eval.data)
+				features |= value;
+			allfail = 0;
+		}
 	}
 
 	eval.cmd = ETHTOOL_GFLAGS;
@@ -2214,21 +2180,7 @@ static int do_goffload(int fd, struct ifreq *ifr)
 	if (err) {
 		perror("Cannot get device flags");
 	} else {
-		lro = (eval.data & ETH_FLAG_LRO) != 0;
-		rxvlan = (eval.data & ETH_FLAG_RXVLAN) != 0;
-		txvlan = (eval.data & ETH_FLAG_TXVLAN) != 0;
-		ntuple = (eval.data & ETH_FLAG_NTUPLE) != 0;
-		rxhash = (eval.data & ETH_FLAG_RXHASH) != 0;
-		allfail = 0;
-	}
-
-	eval.cmd = ETHTOOL_GGRO;
-	ifr->ifr_data = (caddr_t)&eval;
-	err = ioctl(fd, SIOCETHTOOL, ifr);
-	if (err)
-		perror("Cannot get device GRO settings");
-	else {
-		gro = eval.data;
+		features |= eval.data & flags_dup_features;
 		allfail = 0;
 	}
 
@@ -2237,86 +2189,33 @@ static int do_goffload(int fd, struct ifreq *ifr)
 		return 83;
 	}
 
-	return dump_offload(rx, tx, sg, tso, ufo, gso, gro, lro, rxvlan, txvlan,
-			    ntuple, rxhash);
+	return dump_offload(features);
 }
 
 static int do_soffload(int fd, struct ifreq *ifr)
 {
 	struct ethtool_value eval;
-	int err, changed = 0;
-
-	if (off_csum_rx_wanted >= 0) {
-		changed = 1;
-		eval.cmd = ETHTOOL_SRXCSUM;
-		eval.data = (off_csum_rx_wanted == 1);
-		ifr->ifr_data = (caddr_t)&eval;
-		err = send_ioctl(fd, ifr);
-		if (err) {
-			perror("Cannot set device rx csum settings");
-			return 84;
-		}
-	}
-
-	if (off_csum_tx_wanted >= 0) {
-		changed = 1;
-		eval.cmd = ETHTOOL_STXCSUM;
-		eval.data = (off_csum_tx_wanted == 1);
-		ifr->ifr_data = (caddr_t)&eval;
-		err = send_ioctl(fd, ifr);
-		if (err) {
-			perror("Cannot set device tx csum settings");
-			return 85;
-		}
-	}
-
-	if (off_sg_wanted >= 0) {
-		changed = 1;
-		eval.cmd = ETHTOOL_SSG;
-		eval.data = (off_sg_wanted == 1);
-		ifr->ifr_data = (caddr_t)&eval;
-		err = send_ioctl(fd, ifr);
-		if (err) {
-			perror("Cannot set device scatter-gather settings");
-			return 86;
-		}
-	}
+	int err;
+	int i;
 
-	if (off_tso_wanted >= 0) {
-		changed = 1;
-		eval.cmd = ETHTOOL_STSO;
-		eval.data = (off_tso_wanted == 1);
-		ifr->ifr_data = (caddr_t)&eval;
-		err = send_ioctl(fd, ifr);
-		if (err) {
-			perror("Cannot set device tcp segmentation offload settings");
-			return 88;
-		}
-	}
-	if (off_ufo_wanted >= 0) {
-		changed = 1;
-		eval.cmd = ETHTOOL_SUFO;
-		eval.data = (off_ufo_wanted == 1);
-		ifr->ifr_data = (caddr_t)&eval;
-		err = ioctl(fd, SIOCETHTOOL, ifr);
-		if (err) {
-			perror("Cannot set device udp large send offload settings");
-			return 89;
-		}
-	}
-	if (off_gso_wanted >= 0) {
-		changed = 1;
-		eval.cmd = ETHTOOL_SGSO;
-		eval.data = (off_gso_wanted == 1);
-		ifr->ifr_data = (caddr_t)&eval;
-		err = ioctl(fd, SIOCETHTOOL, ifr);
-		if (err) {
-			perror("Cannot set device generic segmentation offload settings");
-			return 90;
+	for (i = 0; i < ARRAY_SIZE(off_feature_def); i++) {
+		if (!off_feature_def[i].cmd)
+			continue;
+		if (off_features_mask & off_feature_def[i].value) {
+			eval.cmd = off_feature_def[i].cmd + 1;
+			eval.data = !!(off_features_wanted &
+				       off_feature_def[i].value);
+			ifr->ifr_data = (caddr_t)&eval;
+			err = send_ioctl(fd, ifr);
+			if (err) {
+				fprintf(stderr,
+					"Cannot set device %s settings: %m\n",
+					off_feature_def[i].long_name);
+				return 1;
+			}
 		}
 	}
-	if (off_flags_mask) {
-		changed = 1;
+	if (off_features_mask & flags_dup_features) {
 		eval.cmd = ETHTOOL_GFLAGS;
 		eval.data = 0;
 		ifr->ifr_data = (caddr_t)&eval;
@@ -2327,8 +2226,9 @@ static int do_soffload(int fd, struct ifreq *ifr)
 		}
 
 		eval.cmd = ETHTOOL_SFLAGS;
-		eval.data = ((eval.data & ~off_flags_mask) |
-			     off_flags_wanted);
+		eval.data &= ~(off_features_mask & flags_dup_features);
+		eval.data |= (off_features_wanted &
+			      flags_dup_features);
 
 		err = ioctl(fd, SIOCETHTOOL, ifr);
 		if (err) {
@@ -2336,19 +2236,8 @@ static int do_soffload(int fd, struct ifreq *ifr)
 			return 92;
 		}
 	}
-	if (off_gro_wanted >= 0) {
-		changed = 1;
-		eval.cmd = ETHTOOL_SGRO;
-		eval.data = (off_gro_wanted == 1);
-		ifr->ifr_data = (caddr_t)&eval;
-		err = ioctl(fd, SIOCETHTOOL, ifr);
-		if (err) {
-			perror("Cannot set device GRO settings");
-			return 93;
-		}
-	}
 
-	if (!changed) {
+	if (off_features_mask == 0) {
 		fprintf(stdout, "no offload settings changed\n");
 	}
 
-- 
1.7.4



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

* [RFC PATCH ethtool 2/3] ethtool: Report any consequential offload feature changes
From: Ben Hutchings @ 2011-05-16 15:57 UTC (permalink / raw)
  To: netdev; +Cc: linux-net-drivers, Michał Mirosław, netdev,
	David Miller
In-Reply-To: <BANLkTi=Q_RrGgw0H=OeNeJ4JkkCgyYckFA@mail.gmail.com>

When an offload feature is enabled or disabled, this can change the
state of other features that depend on it.  Report any such changes.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 ethtool.c |   47 +++++++++++++++++++++++++++++++++++++++--------
 1 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/ethtool.c b/ethtool.c
index 419f349..5eeca64 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -1807,13 +1807,15 @@ static const struct {
 	{ "receive-hashing",		  0,		   NETIF_F_RXHASH },
 };
 
-static int dump_offload(u32 active)
+static int dump_offload(u32 active, u32 mask)
 {
 	u32 value;
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(off_feature_def); i++) {
 		value = off_feature_def[i].value;
+		if (!(mask & value))
+			continue;
 		printf("%s: %s\n",
 		       off_feature_def[i].long_name,
 		       (active & value) ? "on" : "off");
@@ -2147,14 +2149,14 @@ static const u32 flags_dup_features =
 	(ETH_FLAG_LRO | ETH_FLAG_RXVLAN | ETH_FLAG_TXVLAN | ETH_FLAG_NTUPLE |
 	 ETH_FLAG_RXHASH);
 
-static int do_goffload(int fd, struct ifreq *ifr)
+static int get_offload(int fd, struct ifreq *ifr, u32 *features)
 {
 	struct ethtool_value eval;
 	int err, allfail = 1;
-	u32 features = 0, value;
+	u32 value;
 	int i;
 
-	fprintf(stdout, "Offload parameters for %s:\n", devname);
+	*features = 0; 
 
 	for (i = 0; i < ARRAY_SIZE(off_feature_def); i++) {
 		value = off_feature_def[i].value;
@@ -2169,7 +2171,7 @@ static int do_goffload(int fd, struct ifreq *ifr)
 				off_feature_def[i].long_name);
 		} else {
 			if (eval.data)
-				features |= value;
+				*features |= value;
 			allfail = 0;
 		}
 	}
@@ -2180,24 +2182,39 @@ static int do_goffload(int fd, struct ifreq *ifr)
 	if (err) {
 		perror("Cannot get device flags");
 	} else {
-		features |= eval.data & flags_dup_features;
+		*features |= eval.data & flags_dup_features;
 		allfail = 0;
 	}
 
-	if (allfail) {
+	return allfail;
+}
+
+static int do_goffload(int fd, struct ifreq *ifr)
+{
+	u32 features;
+
+	fprintf(stdout, "Offload parameters for %s:\n", devname);
+
+	if (get_offload(fd, ifr, &features)) {
 		fprintf(stdout, "no offload info available\n");
 		return 83;
 	}
 
-	return dump_offload(features);
+	return dump_offload(features, ~(u32)0);
 }
 
 static int do_soffload(int fd, struct ifreq *ifr)
 {
+	u32 old_features, new_features, diff;
 	struct ethtool_value eval;
 	int err;
 	int i;
 
+	if (get_offload(fd, ifr, &old_features)) {
+		fprintf(stderr, "no offload info available\n");
+		return 1;
+	}
+
 	for (i = 0; i < ARRAY_SIZE(off_feature_def); i++) {
 		if (!off_feature_def[i].cmd)
 			continue;
@@ -2239,6 +2256,20 @@ static int do_soffload(int fd, struct ifreq *ifr)
 
 	if (off_features_mask == 0) {
 		fprintf(stdout, "no offload settings changed\n");
+		return 0;
+	}
+
+	/* Were any additional changes made automatically? */
+	if (get_offload(fd, ifr, &new_features)) {
+		fprintf(stderr, "no offload info available\n");
+		return 1;
+	}
+	diff = ((old_features & ~off_features_mask) |
+		(off_features_wanted & off_features_mask)) ^
+		new_features;
+	if (diff) {
+		printf("Additional changes:\n");
+		dump_offload(new_features, diff);
 	}
 
 	return 0;
-- 
1.7.4



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

* [RFC PATCH ethtool 3/3] ethtool: Use ETHTOOL_{G,S}FEATURES where available
From: Ben Hutchings @ 2011-05-16 15:58 UTC (permalink / raw)
  To: netdev; +Cc: linux-net-drivers, Michał Mirosław, netdev,
	David Miller
In-Reply-To: <BANLkTi=Q_RrGgw0H=OeNeJ4JkkCgyYckFA@mail.gmail.com>

Also use the new structures in any case.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 ethtool.c |  294 +++++++++++++++++++++++++++++++++++++++++--------------------
 1 files changed, 199 insertions(+), 95 deletions(-)

diff --git a/ethtool.c b/ethtool.c
index 5eeca64..db1873c 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -297,8 +297,7 @@ static void show_usage(void)
 static char *devname = NULL;
 
 static int goffload_changed = 0;
-static u32 off_features_wanted = 0;
-static u32 off_features_mask = 0;
+struct ethtool_set_features_block off_features;
 
 static struct ethtool_pauseparam epause;
 static int gpause_changed = 0;
@@ -439,30 +438,30 @@ static struct cmdline_info cmdline_seeprom[] = {
 };
 
 static struct cmdline_info cmdline_offload[] = {
-	{ "rx", CMDL_FLAG, &off_features_wanted, NULL,
-	  NETIF_F_RXCSUM, &off_features_mask },
-	{ "tx", CMDL_FLAG, &off_features_wanted, NULL,
-	  NETIF_F_ALL_CSUM, &off_features_mask },
-	{ "sg", CMDL_FLAG, &off_features_wanted, NULL,
-	  NETIF_F_SG, &off_features_mask },
-	{ "tso", CMDL_FLAG, &off_features_wanted, NULL,
-	  NETIF_F_ALL_TSO, &off_features_mask },
-	{ "ufo", CMDL_FLAG, &off_features_wanted, NULL,
-	  NETIF_F_UFO, &off_features_mask },
-	{ "gso", CMDL_FLAG, &off_features_wanted, NULL,
-	  NETIF_F_GSO, &off_features_mask },
-	{ "lro", CMDL_FLAG, &off_features_wanted, NULL,
-	  NETIF_F_LRO, &off_features_mask },
-	{ "gro", CMDL_FLAG, &off_features_wanted, NULL,
-	  NETIF_F_GRO, &off_features_mask },
-	{ "rxvlan", CMDL_FLAG, &off_features_wanted, NULL,
-	  NETIF_F_HW_VLAN_TX, &off_features_mask },
-	{ "txvlan", CMDL_FLAG, &off_features_wanted, NULL,
-	  NETIF_F_HW_VLAN_TX, &off_features_mask },
-	{ "ntuple", CMDL_FLAG, &off_features_wanted, NULL,
-	  NETIF_F_NTUPLE, &off_features_mask },
-	{ "rxhash", CMDL_FLAG, &off_features_wanted, NULL,
-	  NETIF_F_RXHASH, &off_features_mask },
+	{ "rx", CMDL_FLAG, &off_features.requested, NULL,
+	  NETIF_F_RXCSUM, &off_features.valid },
+	{ "tx", CMDL_FLAG, &off_features.requested, NULL,
+	  NETIF_F_ALL_CSUM, &off_features.valid },
+	{ "sg", CMDL_FLAG, &off_features.requested, NULL,
+	  NETIF_F_SG, &off_features.valid },
+	{ "tso", CMDL_FLAG, &off_features.requested, NULL,
+	  NETIF_F_ALL_TSO, &off_features.valid },
+	{ "ufo", CMDL_FLAG, &off_features.requested, NULL,
+	  NETIF_F_UFO, &off_features.valid },
+	{ "gso", CMDL_FLAG, &off_features.requested, NULL,
+	  NETIF_F_GSO, &off_features.valid },
+	{ "lro", CMDL_FLAG, &off_features.requested, NULL,
+	  NETIF_F_LRO, &off_features.valid },
+	{ "gro", CMDL_FLAG, &off_features.requested, NULL,
+	  NETIF_F_GRO, &off_features.valid },
+	{ "rxvlan", CMDL_FLAG, &off_features.requested, NULL,
+	  NETIF_F_HW_VLAN_TX, &off_features.valid },
+	{ "txvlan", CMDL_FLAG, &off_features.requested, NULL,
+	  NETIF_F_HW_VLAN_TX, &off_features.valid },
+	{ "ntuple", CMDL_FLAG, &off_features.requested, NULL,
+	  NETIF_F_NTUPLE, &off_features.valid },
+	{ "rxhash", CMDL_FLAG, &off_features.requested, NULL,
+	  NETIF_F_RXHASH, &off_features.valid },
 };
 
 static struct cmdline_info cmdline_pause[] = {
@@ -1807,7 +1806,8 @@ static const struct {
 	{ "receive-hashing",		  0,		   NETIF_F_RXHASH },
 };
 
-static int dump_offload(u32 active, u32 mask)
+static int
+dump_offload(const struct ethtool_get_features_block *features, u32 mask)
 {
 	u32 value;
 	int i;
@@ -1816,9 +1816,12 @@ static int dump_offload(u32 active, u32 mask)
 		value = off_feature_def[i].value;
 		if (!(mask & value))
 			continue;
-		printf("%s: %s\n",
+		printf("%s: %s%s%s\n",
 		       off_feature_def[i].long_name,
-		       (active & value) ? "on" : "off");
+		       (features->active & value) ? "on" : "off",
+		       (features->requested & ~features->active & value) ?
+		       " [requested on]" : "",
+		       !(features->available & value) ? " [unchangeable]" : "");
 	}
 
 	return 0;
@@ -2149,41 +2152,65 @@ static const u32 flags_dup_features =
 	(ETH_FLAG_LRO | ETH_FLAG_RXVLAN | ETH_FLAG_TXVLAN | ETH_FLAG_NTUPLE |
 	 ETH_FLAG_RXHASH);
 
-static int get_offload(int fd, struct ifreq *ifr, u32 *features)
+static int get_offload(int fd, struct ifreq *ifr,
+		       struct ethtool_get_features_block *pfeatures)
 {
+	struct {
+		struct ethtool_gfeatures cmd;
+		struct ethtool_get_features_block data[1];
+	} features;
 	struct ethtool_value eval;
 	int err, allfail = 1;
 	u32 value;
 	int i;
 
-	*features = 0; 
+	features.cmd.cmd = ETHTOOL_GFEATURES;
+	features.cmd.size = ARRAY_SIZE(features.data);
+	ifr->ifr_data = (caddr_t)&features;
+	err = ioctl(fd, SIOCETHTOOL, ifr);
+	if (err == 0) {
+		allfail = 0;
+		pfeatures[0] = features.data[0];
+	} else if (errno != EOPNOTSUPP && errno != EPERM) {
+		perror("Cannot get device offload settings");
+	} else {
+		memset(pfeatures, 0, sizeof(*pfeatures));
 
-	for (i = 0; i < ARRAY_SIZE(off_feature_def); i++) {
-		value = off_feature_def[i].value;
-		if (!off_feature_def[i].cmd)
-			continue;
-		eval.cmd = off_feature_def[i].cmd;
+		for (i = 0; i < ARRAY_SIZE(off_feature_def); i++) {
+			value = off_feature_def[i].value;
+
+			/* Assume that anything we can get is changeable */
+			pfeatures[0].available |= value;
+
+			if (!off_feature_def[i].cmd)
+				continue;
+
+			eval.cmd = off_feature_def[i].cmd;
+			ifr->ifr_data = (caddr_t)&eval;
+			err = send_ioctl(fd, ifr);
+			if (err) {
+				fprintf(stderr,
+					"Cannot get device %s settings: %m\n",
+					off_feature_def[i].long_name);
+			} else {
+				if (eval.data)
+					pfeatures[0].active |= value;
+				allfail = 0;
+			}
+		}
+
+		eval.cmd = ETHTOOL_GFLAGS;
 		ifr->ifr_data = (caddr_t)&eval;
-		err = send_ioctl(fd, ifr);
+		err = ioctl(fd, SIOCETHTOOL, ifr);
 		if (err) {
-			fprintf(stderr,
-				"Cannot get device %s settings: %m\n",
-				off_feature_def[i].long_name);
+			perror("Cannot get device flags");
 		} else {
-			if (eval.data)
-				*features |= value;
+			pfeatures[0].active |=
+				eval.data & flags_dup_features;
 			allfail = 0;
 		}
-	}
 
-	eval.cmd = ETHTOOL_GFLAGS;
-	ifr->ifr_data = (caddr_t)&eval;
-	err = ioctl(fd, SIOCETHTOOL, ifr);
-	if (err) {
-		perror("Cannot get device flags");
-	} else {
-		*features |= eval.data & flags_dup_features;
-		allfail = 0;
+		pfeatures[0].requested = pfeatures[0].active;
 	}
 
 	return allfail;
@@ -2191,7 +2218,7 @@ static int get_offload(int fd, struct ifreq *ifr, u32 *features)
 
 static int do_goffload(int fd, struct ifreq *ifr)
 {
-	u32 features;
+	struct ethtool_get_features_block features;
 
 	fprintf(stdout, "Offload parameters for %s:\n", devname);
 
@@ -2200,79 +2227,156 @@ static int do_goffload(int fd, struct ifreq *ifr)
 		return 83;
 	}
 
-	return dump_offload(features, ~(u32)0);
+	return dump_offload(&features, ~(u32)0);
 }
 
 static int do_soffload(int fd, struct ifreq *ifr)
 {
-	u32 old_features, new_features, diff;
-	struct ethtool_value eval;
+	struct ethtool_get_features_block old_features, new_features;
+	struct {
+		struct ethtool_sfeatures cmd;
+		struct ethtool_set_features_block data[1];
+	} set_features;
+	int failed;
+	u32 diff;
 	int err;
 	int i;
 
+	if (off_features.valid == 0) {
+		fprintf(stdout, "no offload settings changed\n");
+		return 0;
+	}
+
 	if (get_offload(fd, ifr, &old_features)) {
 		fprintf(stderr, "no offload info available\n");
 		return 1;
 	}
 
+	set_features.cmd.cmd = ETHTOOL_SFEATURES;
+	set_features.cmd.size = ARRAY_SIZE(set_features.data);
+	set_features.data[0] = off_features;
+
 	for (i = 0; i < ARRAY_SIZE(off_feature_def); i++) {
-		if (!off_feature_def[i].cmd)
+		u32 value = off_feature_def[i].value;
+
+		if (!(off_features.valid & value))
 			continue;
-		if (off_features_mask & off_feature_def[i].value) {
-			eval.cmd = off_feature_def[i].cmd + 1;
-			eval.data = !!(off_features_wanted &
-				       off_feature_def[i].value);
-			ifr->ifr_data = (caddr_t)&eval;
-			err = send_ioctl(fd, ifr);
-			if (err) {
-				fprintf(stderr,
-					"Cannot set device %s settings: %m\n",
-					off_feature_def[i].long_name);
-				return 1;
-			}
+		if (!(old_features.available & value)) {
+			/* None of these features can be changed */
+			fprintf(stderr,
+				"Cannot set device %s settings: "
+				"Operation not supported\n",
+				off_feature_def[i].long_name);
+			return 1;
 		}
+		/* At least some of these features can be
+		 * enabled; mask out any that cannot
+		 */
+		set_features.data[0].valid &=
+			~(value & ~old_features.available);
 	}
-	if (off_features_mask & flags_dup_features) {
-		eval.cmd = ETHTOOL_GFLAGS;
-		eval.data = 0;
-		ifr->ifr_data = (caddr_t)&eval;
-		err = ioctl(fd, SIOCETHTOOL, ifr);
-		if (err) {
-			perror("Cannot get device flag settings");
-			return 91;
+
+	ifr->ifr_data = (caddr_t)&set_features;
+	err = ioctl(fd, SIOCETHTOOL, ifr);
+	if (err >= 0) {
+		/* We'll break down the warnings/errors below */
+	} else if (errno != EOPNOTSUPP && errno != EPERM) {
+		perror("Cannot set device offload settings");
+		return 1;
+	} else {
+		struct ethtool_value eval;
+
+		for (i = 0; i < ARRAY_SIZE(off_feature_def); i++) {
+			if (!off_feature_def[i].cmd)
+				continue;
+			if (off_features.valid & off_feature_def[i].value) {
+				eval.cmd = off_feature_def[i].cmd + 1;
+				eval.data = !!(off_features.requested &
+					       off_feature_def[i].value);
+				ifr->ifr_data = (caddr_t)&eval;
+				err = send_ioctl(fd, ifr);
+				if (err) {
+					fprintf(stderr,
+						"Cannot set device %s settings: %m\n",
+						off_feature_def[i].long_name);
+					return 1;
+				}
+			}
 		}
+		if (off_features.valid & flags_dup_features) {
+			eval.cmd = ETHTOOL_GFLAGS;
+			eval.data = 0;
+			ifr->ifr_data = (caddr_t)&eval;
+			err = ioctl(fd, SIOCETHTOOL, ifr);
+			if (err) {
+				perror("Cannot get device flag settings");
+				return 91;
+			}
 
-		eval.cmd = ETHTOOL_SFLAGS;
-		eval.data &= ~(off_features_mask & flags_dup_features);
-		eval.data |= (off_features_wanted &
-			      flags_dup_features);
+			eval.cmd = ETHTOOL_SFLAGS;
+			eval.data &= ~(off_features.valid & flags_dup_features);
+			eval.data |= (off_features.requested &
+				      flags_dup_features);
 
-		err = ioctl(fd, SIOCETHTOOL, ifr);
-		if (err) {
-			perror("Cannot set device flag settings");
-			return 92;
+			err = ioctl(fd, SIOCETHTOOL, ifr);
+			if (err) {
+				perror("Cannot set device flag settings");
+				return 92;
+			}
 		}
 	}
 
-	if (off_features_mask == 0) {
-		fprintf(stdout, "no offload settings changed\n");
-		return 0;
-	}
-
-	/* Were any additional changes made automatically? */
 	if (get_offload(fd, ifr, &new_features)) {
 		fprintf(stderr, "no offload info available\n");
 		return 1;
 	}
-	diff = ((old_features & ~off_features_mask) |
-		(off_features_wanted & off_features_mask)) ^
-		new_features;
+
+	failed = 0;
+
+	/* Report specific failures */
+	for (i = 0; i < ARRAY_SIZE(off_feature_def); i++) {
+		if (!(off_feature_def[i].value & off_features.valid))
+			continue;
+
+		/* For offloads where we have one name for multiple
+		 * feature flags, squash them into a boolean.
+		 */
+		if (!(off_features.requested & off_feature_def[i].value) != 
+		    !(new_features.active & off_feature_def[i].value)) {
+			int did1 = 0;
+
+			fprintf(stderr,
+				"Cannot set device %s settings",
+				off_feature_def[i].long_name);
+			if (err & ETHTOOL_F_WISH) {
+				fprintf(stderr,
+					": Feature depends on other settings");
+				did1++;
+			}
+			/* Report any remaining warning flags (including
+			 * ETHTOOL_F_UNSUPPORTED, which should not be set
+			 * since we checked for unsupported flags above).
+			 */
+			if (err & ~ETHTOOL_F_WISH) {
+				fprintf(stderr, "%s Warning flags %#x",
+					did1 ? ";" : ":",
+					err & ~ETHTOOL_F_WISH);
+			}
+			fprintf(stderr, "\n");
+
+			failed = 1;
+		}
+	}
+
+	/* Were any additional changes made automatically? */
+	diff = (new_features.active ^ old_features.active) &
+		    ~off_features.valid;
 	if (diff) {
 		printf("Additional changes:\n");
-		dump_offload(new_features, diff);
+		dump_offload(&new_features, diff);
 	}
 
-	return 0;
+	return failed;
 }
 
 static int do_gset(int fd, struct ifreq *ifr)
-- 
1.7.4


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

* [PATCH net-2.6] sfc: Fix oops in register dump after mapping change
From: Ben Hutchings @ 2011-05-16 16:13 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-net-drivers

Commit 747df2258b1b9a2e25929ef496262c339c380009 ('sfc: Always map MCDI
shared memory as uncacheable') introduced a separate mapping for the
MCDI shared memory (MC_TREG_SMEM).  This means we can no longer easily
include it in the register dump.  Since it is not particularly useful
in debugging, substitute a recognisable dummy value.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
'ethtool -d' on a Siena controller will now result in an 'oops'.  This
is not a huge problem because I suspect I'm the only one actually
actually using this feature.  The patch is against net-2.6 because the
above commit has not yet been merged into net-next-2.6, but I don't
particularly mind if this misses 2.6.39.

Ben.

 drivers/net/sfc/nic.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/net/sfc/nic.c b/drivers/net/sfc/nic.c
index 10f1cb7..9b29a8d 100644
--- a/drivers/net/sfc/nic.c
+++ b/drivers/net/sfc/nic.c
@@ -1937,6 +1937,13 @@ void efx_nic_get_regs(struct efx_nic *efx, void *buf)
 
 		size = min_t(size_t, table->step, 16);
 
+		if (table->offset >= efx->type->mem_map_size) {
+			/* No longer mapped; return dummy data */
+			memcpy(buf, "\xde\xc0\xad\xde", 4);
+			buf += table->rows * size;
+			continue;
+		}
+
 		for (i = 0; i < table->rows; i++) {
 			switch (table->step) {
 			case 4: /* 32-bit register or SRAM */
-- 
1.7.4


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

* Re: [ethtool PATCH 0/4] v6 Add support for network flow classifier
From: Alexander Duyck @ 2011-05-16 16:32 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: davem@davemloft.net, Kirsher, Jeffrey T, dm@chelsio.com,
	netdev@vger.kernel.org
In-Reply-To: <1305249292.14174.13.camel@bwh-desktop>

On 5/12/2011 6:14 PM, Ben Hutchings wrote:
> Thanks a lot for persevering with this, Alexander.
>
> I've applied this series with minor changes to the last patch.  I
> abbreviated the version history in the commit message, and I deleted a
> blank line in the manual page additions which made the indentation wrong
> for the -U option.  I also recorded the author as you, since you've
> largely rewritten it!  The commit message credits Santwona Behera.
>
> You managed to uncover a bug in the sfc driver, which is that
> set_rx_ntuple can return a positive value.  Previously ethtool accepted
> any non-negative return value as successful, so I made a separate commit
> to restore that behaviour.  Of course I'll fix the driver too.
>
> Ben.
>

Thanks for getting this all applied.  Our validation team is now testing 
the ixgbe patches with your version of ethtool and if I hear about any 
other bugs I might have introduced I will let you know.

Alex

^ 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