* Re: [PATCHv3 net-next-2.6 2/3] qlcnic: Take FW dump via ethtool
From: Anirban Chakraborty @ 2011-05-12 18:53 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev, David Miller
In-Reply-To: <1305221242.5214.36.camel@bwh-desktop>
On May 12, 2011, at 10:27 AM, Ben Hutchings wrote:
> On Wed, 2011-05-11 at 15:54 -0700, Anirban Chakraborty wrote:
>> Driver checks if the previous dump has been cleared before taking the dump.
>> It doesn't take the dump if it is not cleared.
>>
>> Changes from v2:
>> Added lock to protect dump data structures from being mangled while
>> dumping or setting them via ethtool.
>
> Unfortunately it still seems to be possible for the dump length to
> change between the ethtool core calling qlcnic_get_dump_flag() and
> qlcnic_get_dump_data().
dump length is serialized via the driver lock. dump length is a static entity for a given capture mask
and it can only be changed when there is a different capture mask set in the driver (via calling
set_dump() from ethtool core).
Actual dump size is determined during the initial steps of FW dump which takes the driver lock
to start with. So, I am not sure how the dump length could be changed between the calls to
get_dump_flag and get_dump_data from within the ethtool core without a call to set_dump()
in between.
>
> So I think qlcnic_get_dump_data() will need to double-check the length
> after taking the internal lock:
>
> [...]
>> +static int
>> +qlcnic_get_dump_data(struct net_device *netdev, struct ethtool_dump *dump,
>> + void *buffer)
>> +{
>> + int i, copy_sz;
>> + u32 *hdr_ptr, *data;
>> + struct qlcnic_adapter *adapter = netdev_priv(netdev);
>> + struct qlcnic_fw_dump *fw_dump = &adapter->ahw->fw_dump;
>> +
>> + if (qlcnic_api_lock(adapter))
>> + return -EIO;
> [...]
>
> if (dump->len < fw_dump->tmpl_hdr->size + fw_dump->size) {
> qlcnic_api_unlock(adapter);
> return -EINVAL;
> }
>
> I'm not sure about the error code... and I'm really not happy about the
> need to check lengths in both the ethtool core and the driver.
I can put the check in here but don't think it is required really.
>
> Can't you change the function that actually makes a dump to acquire the
> RTNL lock? (You'll need to do that *before* acquiring the driver's own
> lock.)
We can't do that because the driver lock is taken at much higher level where it does
some hardware specific things even before attempting to take FW dump.
Thanks.
-Anirban
^ permalink raw reply
* Re: [PATCHv3 net-next-2.6] ethtool: Added support for FW dump
From: Anirban Chakraborty @ 2011-05-12 18:53 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev, David Miller
In-Reply-To: <1305219899.5214.26.camel@bwh-desktop>
On May 12, 2011, at 10:04 AM, Ben Hutchings wrote:
> On Wed, 2011-05-11 at 15:54 -0700, Anirban Chakraborty wrote:
>> <snip>
>> +/**
>> + * struct ethtool_dump - used for retrieving, setting device dump
>> + * @cmd: Command number - %ETHTOOL_GET_DUMP_FLAG, %ETHTOOL_GET_DUMP_DATA, or
>> + * %ETHTOOL_SET_DUMP
>> + * @version: FW version of the dump, filled in by driver
>> + * @flag: driver dependent flag for dump setting, filled in by driver during
>> + * get and filled in by ethtool for set operation
>> + * @len: length of dump data, returned by driver for get operation
>
> This is also used as the length of the user buffer on entry to
> ETHTOOL_GET_DUMP_DATA.
Will add that.
>
>> + * @data: data collected for get dump data operation
>> + */
>> +struct ethtool_dump {
>> + __u32 cmd;
>> + __u32 version;
>> + __u32 flag;
>> + __u32 len;
>> + u8 data[0];
>> +};
>> +
>> /* for returning and changing feature sets */
>>
>> /**
>> @@ -853,6 +871,9 @@ bool ethtool_invalid_flags(struct net_device *dev, u32 data, u32 supported);
>> * @get_channels: Get number of channels.
>> * @set_channels: Set number of channels. Returns a negative error code or
>> * zero.
>> + * @get_dump_flag: Get dump flag indicating current dump settings of the device.
>
> This operation must also provide the dump length.
Thats correct. WIll add that bit of info.
>
>> <snip>
>> + memset(&tmp, 0, sizeof(tmp));
>> + tmp.cmd = dump.cmd;
>
> tmp.cmd should be ETHTOOL_GET_DUMP_FLAG.
Agreed.
>
>> + ret = ops->get_dump_flag(dev, &tmp);
>> + if (ret)
>> + return ret;
>> +
>> + len = (tmp.len > dump.len) ? dump.len : tmp.len;
>> +
>> + data = vzalloc(len);
>> + if (!data)
>> + return -ENOMEM;
> [...]
>
> The kernel buffer size must be tmp.len, otherwise the driver may overrun
> the buffer.
Agreed.
Thanks,
Anirban
^ permalink raw reply
* Re: [PATCH RESEND net-next] ethtool: bring back missing comma in netdev features strings
From: Mahesh Bandewar @ 2011-05-12 17:59 UTC (permalink / raw)
To: Michał Mirosław; +Cc: franco, netdev
In-Reply-To: <BANLkTikW=vURi7sAQ9ZJdgy=JDv+JacTQQ@mail.gmail.com>
On Thu, May 12, 2011 at 9:45 AM, Michał Mirosław <mirqus@gmail.com> wrote:
> 2011/5/12 <franco@lastsummer.de>:
>> The issue was introduced in commit eed2a12f1ed9aabf.
>>
>> Signed-off-by: Franco Fichtner <franco@lastsummer.de>
>> ---
>> net/core/ethtool.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
>> index b6f4058..b8c2b10 100644
>> --- a/net/core/ethtool.c
>> +++ b/net/core/ethtool.c
>> @@ -361,7 +361,7 @@ static const char
>> netdev_features_strings[ETHTOOL_DEV_FEATURE_WORDS * 32][ETH_GS
>> /* NETIF_F_NTUPLE */ "rx-ntuple-filter",
>> /* NETIF_F_RXHASH */ "rx-hashing",
>> /* NETIF_F_RXCSUM */ "rx-checksum",
>> - /* NETIF_F_NOCACHE_COPY */ "tx-nocache-copy"
>> + /* NETIF_F_NOCACHE_COPY */ "tx-nocache-copy",
>> /* NETIF_F_LOOPBACK */ "loopback",
>> };
>
> Acked-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
>
Acked-by: Mahesh Bandewar <maheshb@google.com>
^ permalink raw reply
* Re: [PATCH RESEND net-next] ethtool: bring back missing comma in netdev features strings
From: Ben Hutchings @ 2011-05-12 17:58 UTC (permalink / raw)
To: franco; +Cc: netdev, maheshb, mirqus
In-Reply-To: <20110512164204.EC8391220061@host64.kissl.de>
On Thu, 2011-05-12 at 18:42 +0200, franco@lastsummer.de wrote:
> The issue was introduced in commit eed2a12f1ed9aabf.
>
> Signed-off-by: Franco Fichtner <franco@lastsummer.de>
Acked-by: Ben Hutchings <bhutchings@solarflare.com>
> ---
> net/core/ethtool.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index b6f4058..b8c2b10 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -361,7 +361,7 @@ static const char
> netdev_features_strings[ETHTOOL_DEV_FEATURE_WORDS * 32][ETH_GS
> /* NETIF_F_NTUPLE */ "rx-ntuple-filter",
> /* NETIF_F_RXHASH */ "rx-hashing",
> /* NETIF_F_RXCSUM */ "rx-checksum",
> - /* NETIF_F_NOCACHE_COPY */ "tx-nocache-copy"
> + /* NETIF_F_NOCACHE_COPY */ "tx-nocache-copy",
> /* NETIF_F_LOOPBACK */ "loopback",
> };
>
--
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: future developments of usbnet
From: David Miller @ 2011-05-12 17:46 UTC (permalink / raw)
To: oliver-GvhC2dPhHPQdnm+yROfE0A
Cc: shemminger-ZtmgI6mnKB3QT0dZR+AlfA,
stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz,
tom.leiming-Re5JQEeQqe8AvxtiuMwx3w, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <201105120959.28473.oliver-GvhC2dPhHPQdnm+yROfE0A@public.gmane.org>
From: Oliver Neukum <oliver-GvhC2dPhHPQdnm+yROfE0A@public.gmane.org>
Date: Thu, 12 May 2011 09:59:28 +0200
> Our problem here is that USB doesn't work sanely without
> interrupts. We can stop IO regarding rx, but we cannot stop
> interrupts if we want to do rx.
Understood.
However, I was wondering if you could do this in software. When URBs
trigger, just queue that pending work up in a software queue when NAPI
is scheduled, then when you exit NAPI polling you check that queue to
see if there are interrupt events to process.
Similarly to how we handle software interrupts.
Anyways, just an idea.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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: AAARGH bisection is hard (Re: [2.6.39 regression] X locks up hard right after logging in)
From: Linus Torvalds @ 2011-05-12 17:37 UTC (permalink / raw)
To: Andrew Lutomirski; +Cc: linux-kernel, netdev, git
In-Reply-To: <BANLkTi=kb_m-CfrpnD8qQTVYLGaDdgy_tg@mail.gmail.com>
On Thu, May 12, 2011 at 10:15 AM, Andrew Lutomirski <luto@mit.edu> wrote:
>
> OK, this sucks. In the course of bisecting this, I've hit two other
> apparently unrelated bugs that prevent my from testing large numbers
> of kernels. Do I have two questions:
>
> 1. Anyone have any ideas from looking at the log?
Nope, that doesn't look very helpful.
> 2. The !&$#@ bisection is skipping all over the place. I've seen
> 2.6.37 versions and all manner of -rc's out of order.
That's the _point_ of bisection. It jumps around. You can start off
trying to pick points on my development tree, but I only have a
hundred merges or so. You're going to start delving into the actual
development versions very quickly. And if you don't do it early,
bisection is going to be much much slower, because it's not going to
pick half-way points.
So bisection works so well exactly because it picks points that are
far away from each other, and you should just totally ignore the
version number. It's meaningless. Looking at it just confuses you.
Don't do it.
Now, "pick stable points" would obviously be nice, but that is going
to have to be manual. You can certainly make some helper scripts, and
that's where that "--first-parent" thing comes in. So if you want to,
just use "git bisect reset" to the commit you want to test.
If you think it's networking, for example, and you've bisected into
there but aren't sure, do "gitk --bisect", find the point where I
merge, and pick that (and my parent), and "git bisect reset" those
points. That way you can verify that it's the networking merge (or
verify that it isn't).
Linus
^ permalink raw reply
* Re: [PATCHv3 net-next-2.6 2/3] qlcnic: Take FW dump via ethtool
From: Ben Hutchings @ 2011-05-12 17:27 UTC (permalink / raw)
To: Anirban Chakraborty; +Cc: netdev, David Miller
In-Reply-To: <1305154448-9687-4-git-send-email-anirban.chakraborty@qlogic.com>
On Wed, 2011-05-11 at 15:54 -0700, Anirban Chakraborty wrote:
> Driver checks if the previous dump has been cleared before taking the dump.
> It doesn't take the dump if it is not cleared.
>
> Changes from v2:
> Added lock to protect dump data structures from being mangled while
> dumping or setting them via ethtool.
Unfortunately it still seems to be possible for the dump length to
change between the ethtool core calling qlcnic_get_dump_flag() and
qlcnic_get_dump_data().
So I think qlcnic_get_dump_data() will need to double-check the length
after taking the internal lock:
[...]
> +static int
> +qlcnic_get_dump_data(struct net_device *netdev, struct ethtool_dump *dump,
> + void *buffer)
> +{
> + int i, copy_sz;
> + u32 *hdr_ptr, *data;
> + struct qlcnic_adapter *adapter = netdev_priv(netdev);
> + struct qlcnic_fw_dump *fw_dump = &adapter->ahw->fw_dump;
> +
> + if (qlcnic_api_lock(adapter))
> + return -EIO;
[...]
if (dump->len < fw_dump->tmpl_hdr->size + fw_dump->size) {
qlcnic_api_unlock(adapter);
return -EINVAL;
}
I'm not sure about the error code... and I'm really not happy about the
need to check lengths in both the ethtool core and the driver.
Can't you change the function that actually makes a dump to acquire the
RTNL lock? (You'll need to do that *before* acquiring the driver's own
lock.)
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
* AAARGH bisection is hard (Re: [2.6.39 regression] X locks up hard right after logging in)
From: Andrew Lutomirski @ 2011-05-12 17:15 UTC (permalink / raw)
To: linux-kernel; +Cc: netdev, git, Linus Torvalds
On Thu, May 12, 2011 at 9:31 AM, Andrew Lutomirski <luto@mit.edu> wrote:
> I just installed 9f381a6 (-linus from yesterday) on my Sandy Bridge
> desktop, and it locks up hard within a few seconds of logging in.
> netconsole says:
>
> [ 506.629723] block group 24725422080 has an wrong amount of free space
> [ 506.629723] block group 24725422080 has an wrong amount of free space
> [ 506.808501] fuse init (API version 7.16)
> [ 506.819996] SELinux: initialized (dev fuse, type fuse), uses genfs_contexts
> [ 506.829847] SELinux: initialized (dev fusectl, type fusectl), uses
> genfs_contexts
> [ 506.808501] fuse init (API version 7.16)
> [ 506.819996] SELinux: initialized (dev fuse, type fuse), uses genfs_contexts
> [ 506.829847] SELinux: initialized (dev fusectl, type fusectl), uses
> genfs_contexts
>
> If it's any help, the system is locked so hard that the reset button
> doesn't work. It's an Intel DQ67SW board, which apparently doesn't
> have the most reliable reset button in the world :)
>
> 2.6.38.{4,5,6} are all rock-solid on this box.
>
> I've started bisecting, but I don't expect to finish today. I need to
> do some work other than kernel hacking...
OK, this sucks. In the course of bisecting this, I've hit two other
apparently unrelated bugs that prevent my from testing large numbers
of kernels. Do I have two questions:
1. Anyone have any ideas from looking at the log?
It looks like most of what's left is network code, so cc netdev.
2. The !&$#@ bisection is skipping all over the place. I've seen
2.6.37 versions and all manner of -rc's out of order. Linus, and
other people who like pontificating about git bisection: is there any
way to get the bisection to follow Linus' tree? I think that if
bisect could be persuaded to consider only changes that are reached by
following only the *first* merge parent all the way from the bad
revision to the good revision, then the bisection would build versions
that were at least good enough for Linus to pull and might have fewer
bisection-killing bugs.
(This isn't a new idea [1], and git rev-list --bisect --first-parent
isn't so bad except that it doesn't bisect.)
Here's the log.
$ git bisect log
# bad: [9f381a61f58bb6487c93ce2233bb9992f8ea9211] Merge
git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-2.6
# good: [521cb40b0c44418a4fd36dc633f575813d59a43d] Linux 2.6.38
git bisect start 'HEAD' 'v2.6.38'
# skip: [6899608533410557e6698cb9d4ff6df553916e98] Merge branch
'for-linus' of git://codeaurora.org/quic/kernel/davidb/linux-msm
# ******* This revision didn't build due to PSTORE.
# ******* Fixed config for the rest but no point in retrying...
git bisect skip 6899608533410557e6698cb9d4ff6df553916e98
# bad: [d3e458d78167102cc961237cfceef6fffc80c0b3] Merge branch
'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-2.6
git bisect bad d3e458d78167102cc961237cfceef6fffc80c0b3
# good: [6445ced8670f37cfc2c5e24a9de9b413dbfc788d] Merge branch
'staging-next' of
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging-2.6
git bisect good 6445ced8670f37cfc2c5e24a9de9b413dbfc788d
# bad: [40c7f2112ce18fa5eb6dc209c50dd0f046790191] Merge branch
'drm-core-next' of
git://git.kernel.org/pub/scm/linux/kernel/git/airlied/drm-2.6
git bisect bad 40c7f2112ce18fa5eb6dc209c50dd0f046790191
# bad: [23b41168fc942a4a041325a04ecc1bd17d031a3e] netdevice: make
initial group visible to userspace
git bisect bad 23b41168fc942a4a041325a04ecc1bd17d031a3e
# bad: [c0c84ef5c130f8871adbdaac2ba824b9195cb6d9] Merge branch
'master' of git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-next-2.6
git bisect bad c0c84ef5c130f8871adbdaac2ba824b9195cb6d9
# skip: [3ad97fbcc233a295f2ccc2c6bdeb32323e360a5e] mac80211: remove
unneeded check
# ******* This revision hangs at edd=off
git bisect skip 3ad97fbcc233a295f2ccc2c6bdeb32323e360a5e
# skip: [5bec3e5ade813ee4bdbab03af1bb6f85859272ea] ath9k: fix tx queue
index confusion in debugfs code
# ******* This revision hangs at edd=off
git bisect skip 5bec3e5ade813ee4bdbab03af1bb6f85859272ea
# skip: [c210de8f88215db31cf3529c9763fc3124d6e09d] ath5k: Fix fast
channel switching
# ******* This revision hangs at edd=off
git bisect skip c210de8f88215db31cf3529c9763fc3124d6e09d
# ******* For added fun, 479600777bb588724d044815415f7d708d06644b gets
stuck in systemd initialization.
--Andy
^ permalink raw reply
* Re: [PATCH net-next 1/2 RESEND v3] net: use NETIF_F_ALL_TSO for vlan features
From: Dimitris Michailidis @ 2011-05-12 17:05 UTC (permalink / raw)
To: Shan Wei; +Cc: David Miller, netdev, eilong, leedom
In-Reply-To: <4DCBAE3B.3070201@cn.fujitsu.com>
On 05/12/2011 02:54 AM, Shan Wei wrote:
> Dimitris Michailidis wrote, at 2011年05月12日 00:35:
>> On 05/10/2011 11:24 PM, Shan Wei wrote:
>>> As Dimitris Michailidis suggested, use NETIF_F_ALL_TSO for vlan_features,
>>> which is a mask, but not hw_features.
>>>
>>> Signed-off-by: Shan Wei <shanwei@cn.fujitsu.com>
>> While these changes aren't wrong I don't see a good reason to make them. I am also curious why you're changing only these three drivers.
>
> A personal reason to quickly know what device supports ALL TSO,
> not to search one by one. :;)
But vlan_features doesn't indicate by itself what the device supports, and it
is because of this that you can use NETIF_F_ALL_TSO in vlan_features without
much future breakage risk. There's still the potential that in the future some
additional TSO variant will be added that some of the drivers you're changing
support, but not over VLANs, and then they'll need to undo you change.
If you want to know which drivers support all TSOs you really need to look at
their features and hw_features. One could set NETIF_F_ALL_TSO in vlan_features
of a driver that doesn't support any form of TSO. The way you're checking
you'd be misled into thinking that driver supports all TSO.
^ permalink raw reply
* Re: [PATCHv3 net-next-2.6] ethtool: Added support for FW dump
From: Ben Hutchings @ 2011-05-12 17:04 UTC (permalink / raw)
To: Anirban Chakraborty; +Cc: netdev, David Miller
In-Reply-To: <1305154448-9687-2-git-send-email-anirban.chakraborty@qlogic.com>
On Wed, 2011-05-11 at 15:54 -0700, Anirban Chakraborty wrote:
> Added code to take FW dump via ethtool. Dump level can be controlled via setting the
> dump flag. A get function is provided to query the current setting of the dump flag.
> Dump data is obtained from the driver via a separate get function.
>
> Changes from v2:
> Provided separate commands for get flag and data.
> Check for minimum of the two buffer length obtained via ethtool and driver and
> use that for dump buffer
> Pass up the driver return error codes up to the caller.
> Added kernel doc comments.
>
> Signed-off-by: Anirban Chakraborty <anirban.chakraborty@qlogic.com>
> ---
> include/linux/ethtool.h | 28 +++++++++++++++
> net/core/ethtool.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 116 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index bd0b50b..5ac7e18 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -601,6 +601,24 @@ struct ethtool_flash {
> char data[ETHTOOL_FLASH_MAX_FILENAME];
> };
>
> +/**
> + * struct ethtool_dump - used for retrieving, setting device dump
> + * @cmd: Command number - %ETHTOOL_GET_DUMP_FLAG, %ETHTOOL_GET_DUMP_DATA, or
> + * %ETHTOOL_SET_DUMP
> + * @version: FW version of the dump, filled in by driver
> + * @flag: driver dependent flag for dump setting, filled in by driver during
> + * get and filled in by ethtool for set operation
> + * @len: length of dump data, returned by driver for get operation
This is also used as the length of the user buffer on entry to
ETHTOOL_GET_DUMP_DATA.
> + * @data: data collected for get dump data operation
> + */
> +struct ethtool_dump {
> + __u32 cmd;
> + __u32 version;
> + __u32 flag;
> + __u32 len;
> + u8 data[0];
> +};
> +
> /* for returning and changing feature sets */
>
> /**
> @@ -853,6 +871,9 @@ bool ethtool_invalid_flags(struct net_device *dev, u32 data, u32 supported);
> * @get_channels: Get number of channels.
> * @set_channels: Set number of channels. Returns a negative error code or
> * zero.
> + * @get_dump_flag: Get dump flag indicating current dump settings of the device.
This operation must also provide the dump length.
> + * @get_dump_data: Get dump data.
> + * @set_dump: Set dump specific flags to the device.
> *
> * All operations are optional (i.e. the function pointer may be set
> * to %NULL) and callers must take this into account. Callers must
> @@ -927,6 +948,10 @@ struct ethtool_ops {
> const struct ethtool_rxfh_indir *);
> void (*get_channels)(struct net_device *, struct ethtool_channels *);
> int (*set_channels)(struct net_device *, struct ethtool_channels *);
> + int (*get_dump_flag)(struct net_device *, struct ethtool_dump *);
> + int (*get_dump_data)(struct net_device *,
> + struct ethtool_dump *, void *);
> + int (*set_dump)(struct net_device *, struct ethtool_dump *);
>
> };
> #endif /* __KERNEL__ */
[...]
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
[...]
> +static int ethtool_get_dump_data(struct net_device *dev,
> + void __user *useraddr)
> +{
> + int ret;
> + __u32 len;
> + struct ethtool_dump dump, tmp;
> + const struct ethtool_ops *ops = dev->ethtool_ops;
> + void *data = NULL;
> +
> + if (!dev->ethtool_ops->get_dump_data ||
> + !dev->ethtool_ops->get_dump_flag)
> + return -EOPNOTSUPP;
> +
> + if (copy_from_user(&dump, useraddr, sizeof(dump)))
> + return -EFAULT;
> +
> + memset(&tmp, 0, sizeof(tmp));
> + tmp.cmd = dump.cmd;
tmp.cmd should be ETHTOOL_GET_DUMP_FLAG.
> + ret = ops->get_dump_flag(dev, &tmp);
> + if (ret)
> + return ret;
> +
> + len = (tmp.len > dump.len) ? dump.len : tmp.len;
> +
> + data = vzalloc(len);
> + if (!data)
> + return -ENOMEM;
[...]
The kernel buffer size must be tmp.len, otherwise the driver may overrun
the buffer.
'len' is the minimum of the two buffer sizes and should only be used as
the size copied to user space (which you have done).
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: [RFC PATCH v2] net: fold dev_disable_lro() into netdev_fix_features()
From: Stephen Hemminger @ 2011-05-12 16:57 UTC (permalink / raw)
To: Ben Hutchings
Cc: Michał Mirosław, netdev, David S. Miller,
Alexey Kuznetsov, Pekka Savola (ipv6), James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, Eric Dumazet, Tom Herbert,
bridge
In-Reply-To: <1305217747.5214.17.camel@bwh-desktop>
On Thu, 12 May 2011 17:29:07 +0100
Ben Hutchings <bhutchings@solarflare.com> wrote:
> > dev->priv_flags |= IFF_BRIDGE_PORT;
> >
> > - dev_disable_lro(dev);
> > -
> > list_add_rcu(&p->list, &br->port_list);
> >
> > - netdev_update_features(br->dev);
> > + netdev_change_features(dev);
> >
> > spin_lock_bh(&br->lock);
> > changed_addr = br_stp_recalculate_bridge_id(br);
>
> Why netdev_change_features() here? I thought that was primarily for use
> when vlan_features may have been changed.
Setting IFF_BRIDGE_PORT in priv_flags causes change_features
to disable LRO.
--
^ permalink raw reply
* Re: [PATCH RESEND net-next] ethtool: bring back missing comma in netdev features strings
From: Michał Mirosław @ 2011-05-12 16:45 UTC (permalink / raw)
To: franco; +Cc: netdev, maheshb
In-Reply-To: <20110512164204.EC8391220061@host64.kissl.de>
2011/5/12 <franco@lastsummer.de>:
> The issue was introduced in commit eed2a12f1ed9aabf.
>
> Signed-off-by: Franco Fichtner <franco@lastsummer.de>
> ---
> net/core/ethtool.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index b6f4058..b8c2b10 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -361,7 +361,7 @@ static const char
> netdev_features_strings[ETHTOOL_DEV_FEATURE_WORDS * 32][ETH_GS
> /* NETIF_F_NTUPLE */ "rx-ntuple-filter",
> /* NETIF_F_RXHASH */ "rx-hashing",
> /* NETIF_F_RXCSUM */ "rx-checksum",
> - /* NETIF_F_NOCACHE_COPY */ "tx-nocache-copy"
> + /* NETIF_F_NOCACHE_COPY */ "tx-nocache-copy",
> /* NETIF_F_LOOPBACK */ "loopback",
> };
Acked-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
^ permalink raw reply
* [PATCH RESEND net-next] ethtool: bring back missing comma in netdev features strings
From: franco @ 2011-05-12 16:42 UTC (permalink / raw)
To: netdev; +Cc: maheshb, mirqus, franco
The issue was introduced in commit eed2a12f1ed9aabf.
Signed-off-by: Franco Fichtner <franco@lastsummer.de>
---
net/core/ethtool.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index b6f4058..b8c2b10 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -361,7 +361,7 @@ static const char
netdev_features_strings[ETHTOOL_DEV_FEATURE_WORDS * 32][ETH_GS
/* NETIF_F_NTUPLE */ "rx-ntuple-filter",
/* NETIF_F_RXHASH */ "rx-hashing",
/* NETIF_F_RXCSUM */ "rx-checksum",
- /* NETIF_F_NOCACHE_COPY */ "tx-nocache-copy"
+ /* NETIF_F_NOCACHE_COPY */ "tx-nocache-copy",
/* NETIF_F_LOOPBACK */ "loopback",
};
--
1.7.3.2.493.g0b0cd
^ permalink raw reply related
* [RFC PATCH v3] net: fold dev_disable_lro() into netdev_fix_features()
From: Michał Mirosław @ 2011-05-12 16:37 UTC (permalink / raw)
To: netdev
Cc: netdev, David S. Miller, Stephen Hemminger, Alexey Kuznetsov,
Pekka Savola (ipv6), James Morris, Hideaki YOSHIFUJI,
Patrick McHardy, Eric Dumazet, Tom Herbert, bridge
In-Reply-To: <20110512160640.2A0B713A6B@rere.qmqm.pl>
This implements checks for forwarding mode in netdev_fix_features() using
dev->priv_flags bits. As a side effect, after device is no longer
forwarding it gets LRO back. This also means that user is not allowed to
enable LRO after device is put to forwarding mode.
This patch depends on removal of discrete offload setting ethtool ops.
v2: remove protocol-specific checks from core code
v3: add comment and missing negation
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
include/linux/if.h | 5 +++++
include/linux/netdevice.h | 1 -
net/bridge/br_if.c | 9 ++++++---
net/core/dev.c | 25 +++++--------------------
net/ipv4/devinet.c | 30 +++++++++++++++++++-----------
net/ipv6/addrconf.c | 17 +++++++++++++----
6 files changed, 48 insertions(+), 39 deletions(-)
diff --git a/include/linux/if.h b/include/linux/if.h
index 3bc63e6..1aef6a7 100644
--- a/include/linux/if.h
+++ b/include/linux/if.h
@@ -76,6 +76,11 @@
#define IFF_BRIDGE_PORT 0x4000 /* device used as bridge port */
#define IFF_OVS_DATAPATH 0x8000 /* device used as Open vSwitch
* datapath port */
+#define IFF_FORWARDING_IPV4 0x10000 /* IPv4 router port */
+#define IFF_FORWARDING_IPV6 0x20000 /* IPv6 router port */
+
+#define IFF_LRO_FORBIDDEN (IFF_BRIDGE_PORT | \
+ IFF_FORWARDING_IPV4 | IFF_FORWARDING_IPV6)
#define IF_GET_IFACE 0x0001 /* for querying only */
#define IF_GET_PROTO 0x0002
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 41972b9..f698730 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1628,7 +1628,6 @@ extern struct net_device *__dev_get_by_name(struct net *net, const char *name);
extern int dev_alloc_name(struct net_device *dev, const char *name);
extern int dev_open(struct net_device *dev);
extern int dev_close(struct net_device *dev);
-extern void dev_disable_lro(struct net_device *dev);
extern int dev_queue_xmit(struct sk_buff *skb);
extern int register_netdevice(struct net_device *dev);
extern void unregister_netdevice_queue(struct net_device *dev,
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 5dbdfdf..31ba100 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -158,6 +158,8 @@ static void del_nbp(struct net_bridge_port *p)
br_netpoll_disable(p);
call_rcu(&p->rcu, destroy_nbp_rcu);
+
+ netdev_update_features(dev);
}
/* called with RTNL */
@@ -368,11 +370,12 @@ int br_add_if(struct net_bridge *br, struct net_device *dev)
dev->priv_flags |= IFF_BRIDGE_PORT;
- dev_disable_lro(dev);
-
list_add_rcu(&p->list, &br->port_list);
- netdev_update_features(br->dev);
+ /* possibly disable LRO and force recalculation of bridge dev's
+ * features through NETDEV_FEAT_CHANGE notifier call.
+ */
+ netdev_change_features(dev);
spin_lock_bh(&br->lock);
changed_addr = br_stp_recalculate_bridge_id(br);
diff --git a/net/core/dev.c b/net/core/dev.c
index 491b8eb..7c7df7c 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1296,26 +1296,6 @@ int dev_close(struct net_device *dev)
EXPORT_SYMBOL(dev_close);
-/**
- * dev_disable_lro - disable Large Receive Offload on a device
- * @dev: device
- *
- * Disable Large Receive Offload (LRO) on a net device. Must be
- * called under RTNL. This is needed if received packets may be
- * forwarded to another interface.
- */
-void dev_disable_lro(struct net_device *dev)
-{
- dev->wanted_features &= ~NETIF_F_LRO;
- netdev_update_features(dev);
-
- if (unlikely(dev->features & NETIF_F_LRO))
- netdev_WARN(dev, "failed to disable LRO!\n");
-
-}
-EXPORT_SYMBOL(dev_disable_lro);
-
-
static int dev_boot_phase = 1;
/**
@@ -5241,6 +5221,11 @@ u32 netdev_fix_features(struct net_device *dev, u32 features)
}
}
+ if (features & NETIF_F_LRO && dev->priv_flags & IFF_LRO_FORBIDDEN) {
+ netdev_info(dev, "Disabling LRO for forwarding interface.\n");
+ features &= ~NETIF_F_LRO;
+ }
+
return features;
}
EXPORT_SYMBOL(netdev_fix_features);
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 0d4a184..9552c68 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -221,6 +221,7 @@ void in_dev_finish_destroy(struct in_device *idev)
printk(KERN_DEBUG "in_dev_finish_destroy: %p=%s\n",
idev, dev ? dev->name : "NIL");
#endif
+ dev->priv_flags &= ~IFF_FORWARDING_IPV4;
dev_put(dev);
if (!idev->dead)
pr_err("Freeing alive in_device %p\n", idev);
@@ -229,6 +230,16 @@ void in_dev_finish_destroy(struct in_device *idev)
}
EXPORT_SYMBOL(in_dev_finish_destroy);
+static void mark_ipv4_forwarding(struct net_device *dev, bool on)
+{
+ if (on)
+ dev->priv_flags |= IFF_FORWARDING_IPV4;
+ else
+ dev->priv_flags &= ~IFF_FORWARDING_IPV4;
+
+ netdev_update_features(dev);
+}
+
static struct in_device *inetdev_init(struct net_device *dev)
{
struct in_device *in_dev;
@@ -245,8 +256,7 @@ static struct in_device *inetdev_init(struct net_device *dev)
in_dev->arp_parms = neigh_parms_alloc(dev, &arp_tbl);
if (!in_dev->arp_parms)
goto out_kfree;
- if (IPV4_DEVCONF(in_dev->cnf, FORWARDING))
- dev_disable_lro(dev);
+ mark_ipv4_forwarding(dev, IPV4_DEVCONF(in_dev->cnf, FORWARDING));
/* Reference in_dev->dev */
dev_hold(dev);
/* Account for reference dev->ip_ptr (below) */
@@ -1475,14 +1485,12 @@ static void inet_forward_change(struct net *net)
IPV4_DEVCONF_DFLT(net, FORWARDING) = on;
for_each_netdev(net, dev) {
- struct in_device *in_dev;
- if (on)
- dev_disable_lro(dev);
- rcu_read_lock();
- in_dev = __in_dev_get_rcu(dev);
- if (in_dev)
+ struct in_device *in_dev = __in_dev_get_rtnl(dev);
+
+ if (in_dev) {
IN_DEV_CONF_SET(in_dev, FORWARDING, on);
- rcu_read_unlock();
+ mark_ipv4_forwarding(in_dev->dev, on);
+ }
}
}
@@ -1527,11 +1535,11 @@ static int devinet_sysctl_forward(ctl_table *ctl, int write,
}
if (valp == &IPV4_DEVCONF_ALL(net, FORWARDING)) {
inet_forward_change(net);
- } else if (*valp) {
+ } else {
struct ipv4_devconf *cnf = ctl->extra1;
struct in_device *idev =
container_of(cnf, struct in_device, cnf);
- dev_disable_lro(idev->dev);
+ mark_ipv4_forwarding(idev->dev, *valp);
}
rtnl_unlock();
rt_cache_flush(net, 0);
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index f2f9b2e..7e0e8fa 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -333,6 +333,7 @@ void in6_dev_finish_destroy(struct inet6_dev *idev)
#ifdef NET_REFCNT_DEBUG
printk(KERN_DEBUG "in6_dev_finish_destroy: %s\n", dev ? dev->name : "NIL");
#endif
+ dev->priv_flags &= ~IFF_FORWARDING_IPV6;
dev_put(dev);
if (!idev->dead) {
pr_warning("Freeing alive inet6 device %p\n", idev);
@@ -344,6 +345,16 @@ void in6_dev_finish_destroy(struct inet6_dev *idev)
EXPORT_SYMBOL(in6_dev_finish_destroy);
+static void mark_ipv6_forwarding(struct net_device *dev, bool on)
+{
+ if (on)
+ dev->priv_flags |= IFF_FORWARDING_IPV6;
+ else
+ dev->priv_flags &= ~IFF_FORWARDING_IPV6;
+
+ netdev_update_features(dev);
+}
+
static struct inet6_dev * ipv6_add_dev(struct net_device *dev)
{
struct inet6_dev *ndev;
@@ -370,8 +381,7 @@ static struct inet6_dev * ipv6_add_dev(struct net_device *dev)
kfree(ndev);
return NULL;
}
- if (ndev->cnf.forwarding)
- dev_disable_lro(dev);
+ mark_ipv6_forwarding(dev, ndev->cnf.forwarding);
/* We refer to the device */
dev_hold(dev);
@@ -469,8 +479,7 @@ static void dev_forward_change(struct inet6_dev *idev)
if (!idev)
return;
dev = idev->dev;
- if (idev->cnf.forwarding)
- dev_disable_lro(dev);
+ mark_ipv6_forwarding(dev, idev->cnf.forwarding);
if (dev && (dev->flags & IFF_MULTICAST)) {
if (idev->cnf.forwarding)
ipv6_dev_mc_inc(dev, &in6addr_linklocal_allrouters);
--
1.7.2.5
^ permalink raw reply related
* Re: [RFC PATCH v2] net: fold dev_disable_lro() into netdev_fix_features()
From: Michał Mirosław @ 2011-05-12 16:35 UTC (permalink / raw)
To: Ben Hutchings
Cc: netdev, David S. Miller, Stephen Hemminger, Alexey Kuznetsov,
Pekka Savola (ipv6), James Morris, Hideaki YOSHIFUJI,
Patrick McHardy, Eric Dumazet, Tom Herbert, bridge
In-Reply-To: <1305217747.5214.17.camel@bwh-desktop>
On Thu, May 12, 2011 at 05:29:07PM +0100, Ben Hutchings wrote:
> On Thu, 2011-05-12 at 18:06 +0200, Michał Mirosław wrote:
> > This implements checks for forwarding mode in netdev_fix_features() using
> > dev->priv_flags bits. As a side effect, after device is no longer
> > forwarding it gets LRO back. This also means that user is not allowed to
> > enable LRO after device is put to forwarding mode.
> >
> > This patch depends on removal of discrete offload setting ethtool ops.
>
> This is nice, but:
>
> [...]
> > --- a/net/bridge/br_if.c
> > +++ b/net/bridge/br_if.c
> > @@ -158,6 +158,8 @@ static void del_nbp(struct net_bridge_port *p)
> > br_netpoll_disable(p);
> >
> > call_rcu(&p->rcu, destroy_nbp_rcu);
> > +
> > + netdev_update_features(dev);
> > }
> >
> > /* called with RTNL */
> > @@ -368,11 +370,9 @@ int br_add_if(struct net_bridge *br, struct net_device *dev)
> >
> > dev->priv_flags |= IFF_BRIDGE_PORT;
> >
> > - dev_disable_lro(dev);
> > -
> > list_add_rcu(&p->list, &br->port_list);
> >
> > - netdev_update_features(br->dev);
> > + netdev_change_features(dev);
> >
> > spin_lock_bh(&br->lock);
> > changed_addr = br_stp_recalculate_bridge_id(br);
> Why netdev_change_features() here? I thought that was primarily for use
> when vlan_features may have been changed.
Because this recalculates port's features and then cascades to bridge's
device. I could have written:
netdev_update_features(dev);
netdev_update_features(br->dev);
But that might cause redundant recalculations for br->dev (first through
notifier calls). I'll add a comment here.
>
> [...]
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> [...]
> > @@ -5241,6 +5221,11 @@ u32 netdev_fix_features(struct net_device *dev, u32 features)
> > }
> > }
> >
> > + if (features & NETIF_F_LRO && dev->priv_flags & IFF_LRO_FORBIDDEN) {
> > + netdev_info(dev, "Disabling LRO for forwarding interface.\n");
> > + features &= NETIF_F_LRO;
> [...]
>
> Mising '~'.
Fixed.
Best Regards,
Michał Mirosław
^ permalink raw reply
* Re: [RFC PATCH v2] net: fold dev_disable_lro() into netdev_fix_features()
From: Ben Hutchings @ 2011-05-12 16:29 UTC (permalink / raw)
To: Michał Mirosław
Cc: netdev, David S. Miller, Stephen Hemminger, Alexey Kuznetsov,
Pekka Savola (ipv6), James Morris, Hideaki YOSHIFUJI,
Patrick McHardy, Eric Dumazet, Tom Herbert, bridge
In-Reply-To: <20110512160640.2A0B713A6B@rere.qmqm.pl>
On Thu, 2011-05-12 at 18:06 +0200, Michał Mirosław wrote:
> This implements checks for forwarding mode in netdev_fix_features() using
> dev->priv_flags bits. As a side effect, after device is no longer
> forwarding it gets LRO back. This also means that user is not allowed to
> enable LRO after device is put to forwarding mode.
>
> This patch depends on removal of discrete offload setting ethtool ops.
This is nice, but:
[...]
> --- a/net/bridge/br_if.c
> +++ b/net/bridge/br_if.c
> @@ -158,6 +158,8 @@ static void del_nbp(struct net_bridge_port *p)
> br_netpoll_disable(p);
>
> call_rcu(&p->rcu, destroy_nbp_rcu);
> +
> + netdev_update_features(dev);
> }
>
> /* called with RTNL */
> @@ -368,11 +370,9 @@ int br_add_if(struct net_bridge *br, struct net_device *dev)
>
> dev->priv_flags |= IFF_BRIDGE_PORT;
>
> - dev_disable_lro(dev);
> -
> list_add_rcu(&p->list, &br->port_list);
>
> - netdev_update_features(br->dev);
> + netdev_change_features(dev);
>
> spin_lock_bh(&br->lock);
> changed_addr = br_stp_recalculate_bridge_id(br);
Why netdev_change_features() here? I thought that was primarily for use
when vlan_features may have been changed.
[...]
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
[...]
> @@ -5241,6 +5221,11 @@ u32 netdev_fix_features(struct net_device *dev, u32 features)
> }
> }
>
> + if (features & NETIF_F_LRO && dev->priv_flags & IFF_LRO_FORBIDDEN) {
> + netdev_info(dev, "Disabling LRO for forwarding interface.\n");
> + features &= NETIF_F_LRO;
[...]
Mising '~'.
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] bonding,llc: Fix structure sizeof incompatibility for some PDUs
From: Joe Perches @ 2011-05-12 16:16 UTC (permalink / raw)
To: Vitalii Demianets
Cc: Jay Vosburgh, Ben Hutchings, Andy Gospodarek,
Arnaldo Carvalho de Melo, netdev, bonding-devel
In-Reply-To: <201105121910.13090.vitas@nppfactor.kiev.ua>
On Thu, 2011-05-12 at 19:10 +0300, Vitalii Demianets wrote:
> On Thursday 12 May 2011 18:52:34 you wrote:
> [...]
> > What arch needs __packed for
> > struct mac_addr {
> > unsigned char addr[6];
> > };
> > ?
> Currently I'm working with AT91SAM9260 which is an ARM CPU, and without
> __packed I get sizeof(struct mac_addr) == 8.
Thanks.
^ permalink raw reply
* Re: [PATCH] bonding,llc: Fix structure sizeof incompatibility for some PDUs
From: Vitalii Demianets @ 2011-05-12 16:10 UTC (permalink / raw)
To: Joe Perches
Cc: Ben Hutchings, Jay Vosburgh, Andy Gospodarek,
Arnaldo Carvalho de Melo, netdev, bonding-devel
In-Reply-To: <1305215554.6124.35.camel@Joe-Laptop>
On Thursday 12 May 2011 18:52:34 you wrote:
[...]
> What arch needs __packed for
>
> struct mac_addr {
> unsigned char addr[6];
> };
>
> ?
Currently I'm working with AT91SAM9260 which is an ARM CPU, and without
__packed I get sizeof(struct mac_addr) == 8.
^ permalink raw reply
* [PATCH net-2.6] sfc: Always map MCDI shared memory as uncacheable
From: Ben Hutchings @ 2011-05-12 16:09 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <1305216428.5214.13.camel@bwh-desktop>
We enabled write-combining for memory-mapped registers in commit
65f0b417dee94f779ce9b77102b7d73c93723b39, but inhibited it for the
MCDI shared memory where this is not supported. However,
write-combining mappings also allow read-reordering, which may also
be a problem.
I found that when an SFC9000-family controller is connected to an
Intel 3000 chipset, and write-combining is enabled, the controller
stops responding to PCIe read requests during driver initialisation
while the driver is polling for completion of an MCDI command. This
results in an NMI and system hang. Adding read memory barriers
between all reads to the shared memory area appears to reduce but not
eliminate the probability of this.
We have not yet established whether this is a bug in our BIU or in the
PCIe bridge. For now, work around by mapping the shared memory area
separately.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/sfc/mcdi.c | 49 ++++++++++++++++++++++++++++------------------
drivers/net/sfc/nic.h | 2 +
drivers/net/sfc/siena.c | 25 ++++++++++++++++++++---
3 files changed, 53 insertions(+), 23 deletions(-)
diff --git a/drivers/net/sfc/mcdi.c b/drivers/net/sfc/mcdi.c
index d984790..3dd45ed 100644
--- a/drivers/net/sfc/mcdi.c
+++ b/drivers/net/sfc/mcdi.c
@@ -50,6 +50,20 @@ static inline struct efx_mcdi_iface *efx_mcdi(struct efx_nic *efx)
return &nic_data->mcdi;
}
+static inline void
+efx_mcdi_readd(struct efx_nic *efx, efx_dword_t *value, unsigned reg)
+{
+ struct siena_nic_data *nic_data = efx->nic_data;
+ value->u32[0] = (__force __le32)__raw_readl(nic_data->mcdi_smem + reg);
+}
+
+static inline void
+efx_mcdi_writed(struct efx_nic *efx, const efx_dword_t *value, unsigned reg)
+{
+ struct siena_nic_data *nic_data = efx->nic_data;
+ __raw_writel((__force u32)value->u32[0], nic_data->mcdi_smem + reg);
+}
+
void efx_mcdi_init(struct efx_nic *efx)
{
struct efx_mcdi_iface *mcdi;
@@ -70,8 +84,8 @@ static void efx_mcdi_copyin(struct efx_nic *efx, unsigned cmd,
const u8 *inbuf, size_t inlen)
{
struct efx_mcdi_iface *mcdi = efx_mcdi(efx);
- unsigned pdu = FR_CZ_MC_TREG_SMEM + MCDI_PDU(efx);
- unsigned doorbell = FR_CZ_MC_TREG_SMEM + MCDI_DOORBELL(efx);
+ unsigned pdu = MCDI_PDU(efx);
+ unsigned doorbell = MCDI_DOORBELL(efx);
unsigned int i;
efx_dword_t hdr;
u32 xflags, seqno;
@@ -92,30 +106,28 @@ static void efx_mcdi_copyin(struct efx_nic *efx, unsigned cmd,
MCDI_HEADER_SEQ, seqno,
MCDI_HEADER_XFLAGS, xflags);
- efx_writed(efx, &hdr, pdu);
+ efx_mcdi_writed(efx, &hdr, pdu);
- for (i = 0; i < inlen; i += 4) {
- _efx_writed(efx, *((__le32 *)(inbuf + i)), pdu + 4 + i);
- /* use wmb() within loop to inhibit write combining */
- wmb();
- }
+ for (i = 0; i < inlen; i += 4)
+ efx_mcdi_writed(efx, (const efx_dword_t *)(inbuf + i),
+ pdu + 4 + i);
/* ring the doorbell with a distinctive value */
- _efx_writed(efx, (__force __le32) 0x45789abc, doorbell);
- wmb();
+ EFX_POPULATE_DWORD_1(hdr, EFX_DWORD_0, 0x45789abc);
+ efx_mcdi_writed(efx, &hdr, doorbell);
}
static void efx_mcdi_copyout(struct efx_nic *efx, u8 *outbuf, size_t outlen)
{
struct efx_mcdi_iface *mcdi = efx_mcdi(efx);
- unsigned int pdu = FR_CZ_MC_TREG_SMEM + MCDI_PDU(efx);
+ unsigned int pdu = MCDI_PDU(efx);
int i;
BUG_ON(atomic_read(&mcdi->state) == MCDI_STATE_QUIESCENT);
BUG_ON(outlen & 3 || outlen >= 0x100);
for (i = 0; i < outlen; i += 4)
- *((__le32 *)(outbuf + i)) = _efx_readd(efx, pdu + 4 + i);
+ efx_mcdi_readd(efx, (efx_dword_t *)(outbuf + i), pdu + 4 + i);
}
static int efx_mcdi_poll(struct efx_nic *efx)
@@ -123,7 +135,7 @@ static int efx_mcdi_poll(struct efx_nic *efx)
struct efx_mcdi_iface *mcdi = efx_mcdi(efx);
unsigned int time, finish;
unsigned int respseq, respcmd, error;
- unsigned int pdu = FR_CZ_MC_TREG_SMEM + MCDI_PDU(efx);
+ unsigned int pdu = MCDI_PDU(efx);
unsigned int rc, spins;
efx_dword_t reg;
@@ -149,8 +161,7 @@ static int efx_mcdi_poll(struct efx_nic *efx)
time = get_seconds();
- rmb();
- efx_readd(efx, ®, pdu);
+ efx_mcdi_readd(efx, ®, pdu);
/* All 1's indicates that shared memory is in reset (and is
* not a valid header). Wait for it to come out reset before
@@ -177,7 +188,7 @@ static int efx_mcdi_poll(struct efx_nic *efx)
respseq, mcdi->seqno);
rc = EIO;
} else if (error) {
- efx_readd(efx, ®, pdu + 4);
+ efx_mcdi_readd(efx, ®, pdu + 4);
switch (EFX_DWORD_FIELD(reg, EFX_DWORD_0)) {
#define TRANSLATE_ERROR(name) \
case MC_CMD_ERR_ ## name: \
@@ -211,21 +222,21 @@ out:
/* Test and clear MC-rebooted flag for this port/function */
int efx_mcdi_poll_reboot(struct efx_nic *efx)
{
- unsigned int addr = FR_CZ_MC_TREG_SMEM + MCDI_REBOOT_FLAG(efx);
+ unsigned int addr = MCDI_REBOOT_FLAG(efx);
efx_dword_t reg;
uint32_t value;
if (efx_nic_rev(efx) < EFX_REV_SIENA_A0)
return false;
- efx_readd(efx, ®, addr);
+ efx_mcdi_readd(efx, ®, addr);
value = EFX_DWORD_FIELD(reg, EFX_DWORD_0);
if (value == 0)
return 0;
EFX_ZERO_DWORD(reg);
- efx_writed(efx, ®, addr);
+ efx_mcdi_writed(efx, ®, addr);
if (value == MC_STATUS_DWORD_ASSERT)
return -EINTR;
diff --git a/drivers/net/sfc/nic.h b/drivers/net/sfc/nic.h
index a42db6e..d91701a 100644
--- a/drivers/net/sfc/nic.h
+++ b/drivers/net/sfc/nic.h
@@ -143,10 +143,12 @@ static inline struct falcon_board *falcon_board(struct efx_nic *efx)
/**
* struct siena_nic_data - Siena NIC state
* @mcdi: Management-Controller-to-Driver Interface
+ * @mcdi_smem: MCDI shared memory mapping. The mapping is always uncacheable.
* @wol_filter_id: Wake-on-LAN packet filter id
*/
struct siena_nic_data {
struct efx_mcdi_iface mcdi;
+ void __iomem *mcdi_smem;
int wol_filter_id;
};
diff --git a/drivers/net/sfc/siena.c b/drivers/net/sfc/siena.c
index e4dd898..837869b 100644
--- a/drivers/net/sfc/siena.c
+++ b/drivers/net/sfc/siena.c
@@ -220,12 +220,26 @@ static int siena_probe_nic(struct efx_nic *efx)
efx_reado(efx, ®, FR_AZ_CS_DEBUG);
efx->net_dev->dev_id = EFX_OWORD_FIELD(reg, FRF_CZ_CS_PORT_NUM) - 1;
+ /* Initialise MCDI */
+ nic_data->mcdi_smem = ioremap_nocache(efx->membase_phys +
+ FR_CZ_MC_TREG_SMEM,
+ FR_CZ_MC_TREG_SMEM_STEP *
+ FR_CZ_MC_TREG_SMEM_ROWS);
+ if (!nic_data->mcdi_smem) {
+ netif_err(efx, probe, efx->net_dev,
+ "could not map MCDI at %llx+%x\n",
+ (unsigned long long)efx->membase_phys +
+ FR_CZ_MC_TREG_SMEM,
+ FR_CZ_MC_TREG_SMEM_STEP * FR_CZ_MC_TREG_SMEM_ROWS);
+ rc = -ENOMEM;
+ goto fail1;
+ }
efx_mcdi_init(efx);
/* Recover from a failed assertion before probing */
rc = efx_mcdi_handle_assertion(efx);
if (rc)
- goto fail1;
+ goto fail2;
/* Let the BMC know that the driver is now in charge of link and
* filter settings. We must do this before we reset the NIC */
@@ -280,6 +294,7 @@ fail4:
fail3:
efx_mcdi_drv_attach(efx, false, NULL);
fail2:
+ iounmap(nic_data->mcdi_smem);
fail1:
kfree(efx->nic_data);
return rc;
@@ -359,6 +374,8 @@ static int siena_init_nic(struct efx_nic *efx)
static void siena_remove_nic(struct efx_nic *efx)
{
+ struct siena_nic_data *nic_data = efx->nic_data;
+
efx_nic_free_buffer(efx, &efx->irq_status);
siena_reset_hw(efx, RESET_TYPE_ALL);
@@ -368,7 +385,8 @@ static void siena_remove_nic(struct efx_nic *efx)
efx_mcdi_drv_attach(efx, false, NULL);
/* Tear down the private nic state */
- kfree(efx->nic_data);
+ iounmap(nic_data->mcdi_smem);
+ kfree(nic_data);
efx->nic_data = NULL;
}
@@ -606,8 +624,7 @@ struct efx_nic_type siena_a0_nic_type = {
.default_mac_ops = &efx_mcdi_mac_operations,
.revision = EFX_REV_SIENA_A0,
- .mem_map_size = (FR_CZ_MC_TREG_SMEM +
- FR_CZ_MC_TREG_SMEM_STEP * FR_CZ_MC_TREG_SMEM_ROWS),
+ .mem_map_size = FR_CZ_MC_TREG_SMEM, /* MC_TREG_SMEM mapped separately */
.txd_ptr_tbl_base = FR_BZ_TX_DESC_PTR_TBL,
.rxd_ptr_tbl_base = FR_BZ_RX_DESC_PTR_TBL,
.buf_tbl_base = FR_BZ_BUF_FULL_TBL,
--
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
* pull request: sfc-2.6 2011-05-12
From: Ben Hutchings @ 2011-05-12 16:07 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
The following changes since commit ff538818f4a82c4cf02d2d6bd6ac5c7360b9d41d:
sysctl: net: call unregister_net_sysctl_table where needed (2011-05-02 16:12:14 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/bwh/sfc-2.6.git sfc-2.6.39
Ben Hutchings (1):
sfc: Always map MCDI shared memory as uncacheable
Sorry for sending this so late. I was really hoping to get to the
bottom of this issue and find a simple fix.
Ben.
drivers/net/sfc/mcdi.c | 49 ++++++++++++++++++++++++++++------------------
drivers/net/sfc/nic.h | 2 +
drivers/net/sfc/siena.c | 25 ++++++++++++++++++++---
3 files changed, 53 insertions(+), 23 deletions(-)
--
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
* [RFC PATCH v2] net: fold dev_disable_lro() into netdev_fix_features()
From: Michał Mirosław @ 2011-05-12 16:06 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Stephen Hemminger, Alexey Kuznetsov,
Pekka Savola (ipv6), James Morris, Hideaki YOSHIFUJI,
Patrick McHardy, Eric Dumazet, Tom Herbert, Ben Hutchings, bridge
In-Reply-To: <20110507114803.0D80A13A6B@rere.qmqm.pl>
This implements checks for forwarding mode in netdev_fix_features() using
dev->priv_flags bits. As a side effect, after device is no longer
forwarding it gets LRO back. This also means that user is not allowed to
enable LRO after device is put to forwarding mode.
This patch depends on removal of discrete offload setting ethtool ops.
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
v2: remove protocol-specific checks from core code
include/linux/if.h | 5 +++++
include/linux/netdevice.h | 1 -
net/bridge/br_if.c | 6 +++---
net/core/dev.c | 25 +++++--------------------
net/ipv4/devinet.c | 30 +++++++++++++++++++-----------
net/ipv6/addrconf.c | 17 +++++++++++++----
6 files changed, 45 insertions(+), 39 deletions(-)
diff --git a/include/linux/if.h b/include/linux/if.h
index 3bc63e6..1aef6a7 100644
--- a/include/linux/if.h
+++ b/include/linux/if.h
@@ -76,6 +76,11 @@
#define IFF_BRIDGE_PORT 0x4000 /* device used as bridge port */
#define IFF_OVS_DATAPATH 0x8000 /* device used as Open vSwitch
* datapath port */
+#define IFF_FORWARDING_IPV4 0x10000 /* IPv4 router port */
+#define IFF_FORWARDING_IPV6 0x20000 /* IPv6 router port */
+
+#define IFF_LRO_FORBIDDEN (IFF_BRIDGE_PORT | \
+ IFF_FORWARDING_IPV4 | IFF_FORWARDING_IPV6)
#define IF_GET_IFACE 0x0001 /* for querying only */
#define IF_GET_PROTO 0x0002
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 41972b9..f698730 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1628,7 +1628,6 @@ extern struct net_device *__dev_get_by_name(struct net *net, const char *name);
extern int dev_alloc_name(struct net_device *dev, const char *name);
extern int dev_open(struct net_device *dev);
extern int dev_close(struct net_device *dev);
-extern void dev_disable_lro(struct net_device *dev);
extern int dev_queue_xmit(struct sk_buff *skb);
extern int register_netdevice(struct net_device *dev);
extern void unregister_netdevice_queue(struct net_device *dev,
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 5dbdfdf..568a9dd 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -158,6 +158,8 @@ static void del_nbp(struct net_bridge_port *p)
br_netpoll_disable(p);
call_rcu(&p->rcu, destroy_nbp_rcu);
+
+ netdev_update_features(dev);
}
/* called with RTNL */
@@ -368,11 +370,9 @@ int br_add_if(struct net_bridge *br, struct net_device *dev)
dev->priv_flags |= IFF_BRIDGE_PORT;
- dev_disable_lro(dev);
-
list_add_rcu(&p->list, &br->port_list);
- netdev_update_features(br->dev);
+ netdev_change_features(dev);
spin_lock_bh(&br->lock);
changed_addr = br_stp_recalculate_bridge_id(br);
diff --git a/net/core/dev.c b/net/core/dev.c
index 491b8eb..b70b6c2 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1296,26 +1296,6 @@ int dev_close(struct net_device *dev)
EXPORT_SYMBOL(dev_close);
-/**
- * dev_disable_lro - disable Large Receive Offload on a device
- * @dev: device
- *
- * Disable Large Receive Offload (LRO) on a net device. Must be
- * called under RTNL. This is needed if received packets may be
- * forwarded to another interface.
- */
-void dev_disable_lro(struct net_device *dev)
-{
- dev->wanted_features &= ~NETIF_F_LRO;
- netdev_update_features(dev);
-
- if (unlikely(dev->features & NETIF_F_LRO))
- netdev_WARN(dev, "failed to disable LRO!\n");
-
-}
-EXPORT_SYMBOL(dev_disable_lro);
-
-
static int dev_boot_phase = 1;
/**
@@ -5241,6 +5221,11 @@ u32 netdev_fix_features(struct net_device *dev, u32 features)
}
}
+ if (features & NETIF_F_LRO && dev->priv_flags & IFF_LRO_FORBIDDEN) {
+ netdev_info(dev, "Disabling LRO for forwarding interface.\n");
+ features &= NETIF_F_LRO;
+ }
+
return features;
}
EXPORT_SYMBOL(netdev_fix_features);
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 0d4a184..9552c68 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -221,6 +221,7 @@ void in_dev_finish_destroy(struct in_device *idev)
printk(KERN_DEBUG "in_dev_finish_destroy: %p=%s\n",
idev, dev ? dev->name : "NIL");
#endif
+ dev->priv_flags &= ~IFF_FORWARDING_IPV4;
dev_put(dev);
if (!idev->dead)
pr_err("Freeing alive in_device %p\n", idev);
@@ -229,6 +230,16 @@ void in_dev_finish_destroy(struct in_device *idev)
}
EXPORT_SYMBOL(in_dev_finish_destroy);
+static void mark_ipv4_forwarding(struct net_device *dev, bool on)
+{
+ if (on)
+ dev->priv_flags |= IFF_FORWARDING_IPV4;
+ else
+ dev->priv_flags &= ~IFF_FORWARDING_IPV4;
+
+ netdev_update_features(dev);
+}
+
static struct in_device *inetdev_init(struct net_device *dev)
{
struct in_device *in_dev;
@@ -245,8 +256,7 @@ static struct in_device *inetdev_init(struct net_device *dev)
in_dev->arp_parms = neigh_parms_alloc(dev, &arp_tbl);
if (!in_dev->arp_parms)
goto out_kfree;
- if (IPV4_DEVCONF(in_dev->cnf, FORWARDING))
- dev_disable_lro(dev);
+ mark_ipv4_forwarding(dev, IPV4_DEVCONF(in_dev->cnf, FORWARDING));
/* Reference in_dev->dev */
dev_hold(dev);
/* Account for reference dev->ip_ptr (below) */
@@ -1475,14 +1485,12 @@ static void inet_forward_change(struct net *net)
IPV4_DEVCONF_DFLT(net, FORWARDING) = on;
for_each_netdev(net, dev) {
- struct in_device *in_dev;
- if (on)
- dev_disable_lro(dev);
- rcu_read_lock();
- in_dev = __in_dev_get_rcu(dev);
- if (in_dev)
+ struct in_device *in_dev = __in_dev_get_rtnl(dev);
+
+ if (in_dev) {
IN_DEV_CONF_SET(in_dev, FORWARDING, on);
- rcu_read_unlock();
+ mark_ipv4_forwarding(in_dev->dev, on);
+ }
}
}
@@ -1527,11 +1535,11 @@ static int devinet_sysctl_forward(ctl_table *ctl, int write,
}
if (valp == &IPV4_DEVCONF_ALL(net, FORWARDING)) {
inet_forward_change(net);
- } else if (*valp) {
+ } else {
struct ipv4_devconf *cnf = ctl->extra1;
struct in_device *idev =
container_of(cnf, struct in_device, cnf);
- dev_disable_lro(idev->dev);
+ mark_ipv4_forwarding(idev->dev, *valp);
}
rtnl_unlock();
rt_cache_flush(net, 0);
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index f2f9b2e..7e0e8fa 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -333,6 +333,7 @@ void in6_dev_finish_destroy(struct inet6_dev *idev)
#ifdef NET_REFCNT_DEBUG
printk(KERN_DEBUG "in6_dev_finish_destroy: %s\n", dev ? dev->name : "NIL");
#endif
+ dev->priv_flags &= ~IFF_FORWARDING_IPV6;
dev_put(dev);
if (!idev->dead) {
pr_warning("Freeing alive inet6 device %p\n", idev);
@@ -344,6 +345,16 @@ void in6_dev_finish_destroy(struct inet6_dev *idev)
EXPORT_SYMBOL(in6_dev_finish_destroy);
+static void mark_ipv6_forwarding(struct net_device *dev, bool on)
+{
+ if (on)
+ dev->priv_flags |= IFF_FORWARDING_IPV6;
+ else
+ dev->priv_flags &= ~IFF_FORWARDING_IPV6;
+
+ netdev_update_features(dev);
+}
+
static struct inet6_dev * ipv6_add_dev(struct net_device *dev)
{
struct inet6_dev *ndev;
@@ -370,8 +381,7 @@ static struct inet6_dev * ipv6_add_dev(struct net_device *dev)
kfree(ndev);
return NULL;
}
- if (ndev->cnf.forwarding)
- dev_disable_lro(dev);
+ mark_ipv6_forwarding(dev, ndev->cnf.forwarding);
/* We refer to the device */
dev_hold(dev);
@@ -469,8 +479,7 @@ static void dev_forward_change(struct inet6_dev *idev)
if (!idev)
return;
dev = idev->dev;
- if (idev->cnf.forwarding)
- dev_disable_lro(dev);
+ mark_ipv6_forwarding(dev, idev->cnf.forwarding);
if (dev && (dev->flags & IFF_MULTICAST)) {
if (idev->cnf.forwarding)
ipv6_dev_mc_inc(dev, &in6addr_linklocal_allrouters);
--
1.7.2.5
^ permalink raw reply related
* Re: [RFC PATCH] net: fold dev_disable_lro() into netdev_fix_features()
From: Michał Mirosław @ 2011-05-12 16:06 UTC (permalink / raw)
To: David Miller
Cc: netdev, shemminger, kuznet, pekkas, jmorris, yoshfuji, kaber,
eric.dumazet, therbert, bhutchings, bridge
In-Reply-To: <20110509.120811.246541047.davem@davemloft.net>
On Mon, May 09, 2011 at 12:08:11PM -0700, David Miller wrote:
> From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> Date: Sat, 7 May 2011 13:48:02 +0200 (CEST)
> > This moves checks that device is forwarding from bridge, IPv4 and IPv6
> > code into netdev_fix_features(). As a side effect, after device is no longer
> > forwarding it gets LRO back. This also means that user is not allowed to
> > enable LRO after device is put to forwarding mode.
> We need to keep the check in the protocols because we don't want to
> be testing protocol specific device state in generic code like
> net/core/dev.c
I modified it to use priv_flags that mirror the protocol-internal state.
Patch follows.
Best Regards,
Michał Mirosław
^ permalink raw reply
* Re: [PATCH] bonding,llc: Fix structure sizeof incompatibility for some PDUs
From: Eric Dumazet @ 2011-05-12 16:02 UTC (permalink / raw)
To: Joe Perches
Cc: Vitalii Demianets, Ben Hutchings, Jay Vosburgh, Andy Gospodarek,
Arnaldo Carvalho de Melo, netdev, bonding-devel
In-Reply-To: <1305215554.6124.35.camel@Joe-Laptop>
Le jeudi 12 mai 2011 à 08:52 -0700, Joe Perches a écrit :
> On Thu, 2011-05-12 at 18:45 +0300, Vitalii Demianets wrote:
> > On Thursday 12 May 2011 17:33:36 Ben Hutchings wrote:
> > [...]
> > > The '__packed' macro is preferred.
> > Also, I'm slightly in doubt if I should split the patch in 2 parts, one for
> > bonding an one for LLC?
>
> What arch needs __packed for
>
> struct mac_addr {
> unsigned char addr[6];
> };
>
> ?
Believe it or not, they do exist. (ARM ...)
See "struct ethhdr" definition
Check commit 7cd636fe9ce5de0051c112 for another argument.
^ permalink raw reply
* Re: [PATCH] iproute2: use IFLA_TXQLEN when it is available
From: Stephen Hemminger @ 2011-05-12 15:57 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Changli Gao, netdev, kuznet
In-Reply-To: <1305175255.20318.4.camel@edumazet-laptop>
I just applied Eric's patch
--
^ permalink raw reply
* Re: [PATCH] bonding,llc: Fix structure sizeof incompatibility for some PDUs
From: Joe Perches @ 2011-05-12 15:52 UTC (permalink / raw)
To: Vitalii Demianets
Cc: Ben Hutchings, Jay Vosburgh, Andy Gospodarek,
Arnaldo Carvalho de Melo, netdev, bonding-devel
In-Reply-To: <201105121845.49090.vitas@nppfactor.kiev.ua>
On Thu, 2011-05-12 at 18:45 +0300, Vitalii Demianets wrote:
> On Thursday 12 May 2011 17:33:36 Ben Hutchings wrote:
> [...]
> > The '__packed' macro is preferred.
> Also, I'm slightly in doubt if I should split the patch in 2 parts, one for
> bonding an one for LLC?
What arch needs __packed for
struct mac_addr {
unsigned char addr[6];
};
?
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox