* Re: ip_vs_ftp causing ip_vs oops on module load.
From: Hans Schillstrom @ 2011-05-19 8:52 UTC (permalink / raw)
To: Simon Horman
Cc: Julian Anastasov, Dave Jones, netdev@vger.kernel.org,
Wensong Zhang, Hans Schillstrom
In-Reply-To: <20110519081706.GE3922@verge.net.au>
On Thursday 19 May 2011 10:17:07 Simon Horman wrote:
> On Thu, May 19, 2011 at 09:58:55AM +0200, Hans Schillstrom wrote:
> > On Thursday 19 May 2011 08:33:55 Julian Anastasov wrote:
> > >
> > > Hello,
> > >
> > [snip]
> > >
> > > One unregister_netdevice_notifier(&ip_vs_dst_notifier);
> > > is missing in ip_vs_control_cleanup for sure.
> > >
> > Oops,
> > Should I prepare a patch for that one ?
>
> Could you test the one I posted?
> (Or send another one if I got it wrong? :-)
>
Tested,
it works fine :-)
--
Regards
Hans Schillstrom <hans.schillstrom@ericsson.com>
^ permalink raw reply
* Re: ip_vs_ftp causing ip_vs oops on module load.
From: Julian Anastasov @ 2011-05-19 8:55 UTC (permalink / raw)
To: Hans Schillstrom
Cc: Simon Horman, Dave Jones, netdev@vger.kernel.org, Wensong Zhang
In-Reply-To: <201105190952.49006.hans.schillstrom@ericsson.com>
Hello,
On Thu, 19 May 2011, Hans Schillstrom wrote:
> I can reproduce the source of the problem,
> use multiple netns and then unload the ftp module...
> i.e. same list head used in multiple netns
>
> This brings up a question:
> - How should ftp be handled in a netns ?
> You might want to have it in one netns but not in another,
> this requires changes to ipvsadm
>
> A way of doing it could be a disable switch like --noftp [port,port]
> i.e. do not break old apps.
>
> Any other ideas ?
>
> This patch solves the root problem, I'm not sure if this is the way to go
> or if we should split the ip_vs_app struct ?
This patch is a fast fix but may be it is too late for it,
after 2.6.39 is out. It seems we overlooked the apps when
migrating to netns. I think, the apps do not need to be pernet.
If one day application needs pernet context we can add such
fields in the ipvs structure.
While the protocols have controls that manipulate pernet
timeouts, the apps do not have such controls about app->timeouts.
May be we can remove app->timeouts to avoid confusion because
it was never implemented in user space. May be instead of this
fix we should restore the global ip_vs_app_list and all things in
ip_vs_app.c and ip_vs_ftp.c as before the netns changes?
> If it's the way to go I can send it as a proper formated patch ...
> (after some testing)
>
> diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
> index 4fff432..481f856 100644
> --- a/include/net/ip_vs.h
> +++ b/include/net/ip_vs.h
> @@ -797,7 +797,8 @@ struct netns_ipvs {
> struct list_head rs_table[IP_VS_RTAB_SIZE];
> /* ip_vs_app */
> struct list_head app_list;
> -
> + /* ip_vs_ftp */
> + struct ip_vs_app *ftp_app;
> /* ip_vs_proto */
> #define IP_VS_PROTO_TAB_SIZE 32 /* must be power of 2 */
> struct ip_vs_proto_data *proto_data_table[IP_VS_PROTO_TAB_SIZE];
> diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
> index 6b5dd6d..17afb09 100644
> --- a/net/netfilter/ipvs/ip_vs_ftp.c
> +++ b/net/netfilter/ipvs/ip_vs_ftp.c
> @@ -411,25 +411,36 @@ static struct ip_vs_app ip_vs_ftp = {
> static int __net_init __ip_vs_ftp_init(struct net *net)
> {
> int i, ret;
> - struct ip_vs_app *app = &ip_vs_ftp;
> + struct ip_vs_app *app;
> + struct netns_ipvs *ipvs = net_ipvs(net);
> +
> + app = kmemdup(&ip_vs_ftp, sizeof(struct ip_vs_app), GFP_KERNEL);
> + if (!app)
> + return -ENOMEM;
> + INIT_LIST_HEAD(&app->a_list);
> + INIT_LIST_HEAD(&app->incs_list);
> + INIT_LIST_HEAD(&app->p_list);
> + ipvs->ftp_app = app;
>
> ret = register_ip_vs_app(net, app);
> if (ret)
> - return ret;
> + goto err_exit;
>
> for (i=0; i<IP_VS_APP_MAX_PORTS; i++) {
> if (!ports[i])
> continue;
> ret = register_ip_vs_app_inc(net, app, app->protocol, ports[i]);
> if (ret)
> - break;
> + goto err_unreg;
> pr_info("%s: loaded support on port[%d] = %d\n",
> app->name, i, ports[i]);
> }
> + return 0;
>
> - if (ret)
> - unregister_ip_vs_app(net, app);
> -
> +err_unreg:
> + unregister_ip_vs_app(net, app);
> +err_exit:
> + kfree(ipvs->ftp_app);
> return ret;
> }
> /*
> @@ -437,9 +448,7 @@ static int __net_init __ip_vs_ftp_init(struct net *net)
> */
> static void __ip_vs_ftp_exit(struct net *net)
> {
> - struct ip_vs_app *app = &ip_vs_ftp;
> -
> - unregister_ip_vs_app(net, app);
> + unregister_ip_vs_app(net, net_ipvs(net)->ftp_app);
> }
>
> static struct pernet_operations ip_vs_ftp_ops = {
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* Re: net: add seq_before/seq_after functions
From: Sven Eckelmann @ 2011-05-19 8:54 UTC (permalink / raw)
To: Antonio Quartulli; +Cc: linux-kernel, netdev, davem, Paul Mackerras, linux-ppp
In-Reply-To: <1305722319-8315-1-git-send-email-ordex@autistici.org>
[-- Attachment #1: Type: Text/Plain, Size: 4595 bytes --]
On Wednesday 18 May 2011 14:38:39 Antonio Quartulli wrote:
> Introduce two operations to handle comparison between packet sequence
> numbers taking into account overflow/wraparound. Batman-adv uses
> these functions already to check for successor packet even in case of
> overflow.
Thanks for your efforts to bring that to the kernel. But when you prepare a
patch then you have to add a signoff. And also David S. Miller is the
maintainer for this header - it would be interesting to ask him first when we
want to change that file.
> ---
> I added this two functions in net.h because I didn't really know where
> best placement is. I saw several modules that redefine their own functions
> for the same purpose.
>
> include/linux/net.h | 17 +++++++++++++++++
> 1 files changed, 17 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/net.h b/include/linux/net.h
> index 94de83c..c7bc9bf 100644
> --- a/include/linux/net.h
> +++ b/include/linux/net.h
> @@ -295,4 +295,21 @@ extern struct ratelimit_state net_ratelimit_state;
> #endif
>
> #endif /* __KERNEL__ */
> +
> +/* Returns the smallest signed integer in two's complement with the sizeof
> x */ +#define smallest_signed_int(x) (1u << (7u + 8u * (sizeof(x) - 1u)))
> +
> +/* Checks if a sequence number x is a predecessor/successor of y.
> + * they handle overflows/underflows and can correctly check for a
> + * predecessor/successor unless the variable sequence number has grown by
> + * more then 2**(bitwidth(x)-1)-1.
> + * This means that for a uint8_t with the maximum value 255, it would
> think: + * - when adding nothing - it is neither a predecessor nor a
> successor + * - before adding more than 127 to the starting value - it is
> a predecessor, + * - when adding 128 - it is neither a predecessor nor a
> successor, + * - after adding more than 127 to the starting value - it is
> a successor */ +#define seq_before(x, y) ({typeof(x) _dummy = (x - y); \
> + _dummy > smallest_signed_int(_dummy); })
> +#define seq_after(x, y) seq_before(y, x)
> +
> #endif /* _LINUX_NET_H */
I suggested yesterday (probably too late) that it would be good to check the
type of both parameters (similar to the min and max functions in
include/linux/kernel.h
#define seq_before(x, y) ({typeof(x) _d1 = (x); \
typeof(y) _d2 = (y); \
(void) (&_d1 == &_d2); \
typeof(x) _dummy = (_d1 - _d2); \
_dummy > smallest_signed_int(_dummy); })
And your seq_before/after conflicts with the one defined in ppp_generic.c
drivers/net/ppp_generic.c:232:0: warning: "seq_before" redefined [enabled by
default]
include/linux/net.h:312:0: note: this is the location of the previous
definition
drivers/net/ppp_generic.c:233:0: warning: "seq_after" redefined [enabled by
default]
include/linux/net.h:314:0: note: this is the location of the previous
definition
The definition there is only for u32 - thus you would have to remove it and
check that it always gives the same result:
#define seq_before(a, b) ((s32)((a) - (b)) < 0)
#define seq_after(a, b) ((s32)((a) - (b)) > 0)
But I would say that they have a different definition of seq_before. Changing
that behaviour for batman-adv would not be that problematic, but maybe for
ppp.
A defintion which should fulfil the requirements for ppp could be:
#define seq_after(x, y) ({typeof(x) _d1 = (x); \
typeof(y) _d2 = (y); \
(void) (&_d1 == &_d2); \
typeof(x) _dummy = (_d2 - _d1); \
_dummy > smallest_signed_int(_dummy); })
#define seq_before(x, y) ({typeof(x) _d1 = (x); \
typeof(y) _d2 = (y); \
(void) (&_d1 == &_d2); \
typeof(x) _dummy = (_d1 - _d2); \
_dummy >= smallest_signed_int(_dummy); })
Of course the comment above the seq_before/seq_after would be wrong.
/* Checks if a sequence number x is a predecessor/successor of y.
* they handle overflows/underflows and can correctly check for a
* predecessor/successor unless the variable sequence number has grown by
* more then 2**(bitwidth(x)-1).
* This means that for a uint8_t with the maximum value 255, it would think:
* - when adding nothing - it is neither a predecessor nor a successor
* - before adding more than 128 to the starting value - it is a predecessor,
* - after adding more than 127 to the starting value - it is a successor */
I think there could be more candidates which would like to use this abstract
functionality. Maybe some one else on linux-kernel or netdev has a suggestion.
Kind regards,
Sven
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: net: add seq_before/seq_after functions
From: David Miller @ 2011-05-19 9:08 UTC (permalink / raw)
To: sven; +Cc: ordex, linux-kernel, netdev, paulus, linux-ppp
In-Reply-To: <201105191054.34912.sven@narfation.org>
From: Sven Eckelmann <sven@narfation.org>
Date: Thu, 19 May 2011 10:54:32 +0200
> On Wednesday 18 May 2011 14:38:39 Antonio Quartulli wrote:
>> Introduce two operations to handle comparison between packet sequence
>> numbers taking into account overflow/wraparound. Batman-adv uses
>> these functions already to check for successor packet even in case of
>> overflow.
>
> Thanks for your efforts to bring that to the kernel. But when you prepare a
> patch then you have to add a signoff. And also David S. Miller is the
> maintainer for this header - it would be interesting to ask him first when we
> want to change that file.
Well it makes no sense to add these interfaces until we see an
upstream submission of code which will actually use it.
Also I'm skeptical that such generic sounding interfaces make
sense when it appears to me that these are protocol specific
sequence number tests so probably belong in whatever protocol
is upcoming which will use these interfaces.
Again, this is why we want to see the code that's going to use
these new routines before we can seriously consider adding them
at all.
^ permalink raw reply
* Re: [PATCH] ethtool: ETHTOOL_SFEATURES: remove NETIF_F_COMPAT return
From: Michał Mirosław @ 2011-05-19 9:18 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev, David Miller
In-Reply-To: <1305745379.2929.9.camel@bwh-desktop>
On Wed, May 18, 2011 at 08:02:59PM +0100, Ben Hutchings wrote:
> On Mon, 2011-05-16 at 23:09 +0100, Ben Hutchings wrote:
> > On Mon, 2011-05-16 at 23:50 +0200, Michał Mirosław wrote:
> > > On Mon, May 16, 2011 at 10:08:59PM +0100, Ben Hutchings wrote:
> > > > I've explained before that I do not want to add new options to do
> > > > (mostly) the same thing. Users should have not have to use a different
> > > > command depending on the kernel version.
> > > We can avoid new option by checking feature-strings for unrecognised
> > > arguments to -K. This way, we will have the old options which work
> > > regardless of kernel version ('tx', 'rx', 'sg', etc.) and new options
> > > which need recent kernel anyway (separated 'tx-checksum-*', 'loopback',
> > > others coming in for 2.6.40).
> > This is just too subtle a distinction. It will mostly confuse users.
> Sorry, I think I misunderstood you here. I agree that new feature names
> that do not correspond exactly to existing keywords should be supported
> as keywords after the -K option. I think those that do (e.g.
> "tx-udp-fragmentation" vs "ufo") should not be, as adding a
> kernel-version-dependent *alias* would be confusing.
The alias can be marked as such in the documentation. Shouldn't it be
that hard for a user to read the manpage to know what the new options
are for when he sees them. I don't like the idea of translating strings,
either, because if e.g. ufo becomes split in the feature to ufo4+ufo6
or new checksum offloads are implemented, it will break.
> I also want users to benefit from your improvements (as I explained
> above) even when they use the old names, if they are using a new kernel
> version. That is why I want ethtool to try using ETHTOOL_SFEATURES
> first, and why the fallback in the kernel is problematic.
Which benefits do you want to have? If checking what other features
changed with selected one, it's easily done by rereading the state -
possibly with GFEATURES.
I'll cook another PoC patch over those I sent to show the idea.
Best Regards,
Michał Mirosław
^ permalink raw reply
* Re: net: add seq_before/seq_after functions
From: Sven Eckelmann @ 2011-05-19 9:21 UTC (permalink / raw)
To: David Miller; +Cc: ordex, linux-kernel, netdev, paulus, linux-ppp
In-Reply-To: <20110519.050824.838971689287732822.davem@davemloft.net>
[-- Attachment #1: Type: Text/Plain, Size: 1736 bytes --]
On Thursday 19 May 2011 11:08:24 David Miller wrote:
> From: Sven Eckelmann <sven@narfation.org>
> Date: Thu, 19 May 2011 10:54:32 +0200
>
> > On Wednesday 18 May 2011 14:38:39 Antonio Quartulli wrote:
> >> Introduce two operations to handle comparison between packet sequence
> >> numbers taking into account overflow/wraparound. Batman-adv uses
> >> these functions already to check for successor packet even in case of
> >> overflow.
> >
> > Thanks for your efforts to bring that to the kernel. But when you prepare
> > a patch then you have to add a signoff. And also David S. Miller is the
> > maintainer for this header - it would be interesting to ask him first
> > when we want to change that file.
>
> Well it makes no sense to add these interfaces until we see an
> upstream submission of code which will actually use it.
>
> Also I'm skeptical that such generic sounding interfaces make
> sense when it appears to me that these are protocol specific
> sequence number tests so probably belong in whatever protocol
> is upcoming which will use these interfaces.
>
> Again, this is why we want to see the code that's going to use
> these new routines before we can seriously consider adding them
> at all.
This is currently used by vis.c in net/batman-adv and could also be used by
ppp-generic.c (with my changes of course). And it is planned to be used by
transtable.c in net/batman-adv. The idea was to propose this to linux-
kernel/netdev before we move it to a place were only batman-adv can use it
(the current situation is that vis.c in batman-adv can only use it).
It is ok that you say that it should be batman-adv specific - we only wanted
to ask first.
Thanks,
Sven
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] ethtool: ETHTOOL_SFEATURES: remove NETIF_F_COMPAT return
From: Michał Mirosław @ 2011-05-19 10:03 UTC (permalink / raw)
To: David Miller; +Cc: bhutchings, netdev
In-Reply-To: <20110516.140958.625993829749556424.davem@davemloft.net>
On Mon, May 16, 2011 at 02:09:58PM -0400, David Miller wrote:
> You guys really need to sort this out properly.
> Please resubmit whatever final solution is agreed upon.
I noticed that v2.6.39 was tagged today. We should definitely remove
NETIF_F_COMPAT so it won't bite us in the future. The other patch that
fixes ethtool_ops->set_flags compatibility is a bugfix, so it should go
in - if we decide that the SFEATURES compatibility should be removed
it won't matter.
Ben, do you agree?
Best Regards,
Michał Mirosław
^ permalink raw reply
* Re: ip_vs_ftp causing ip_vs oops on module load.
From: Hans Schillstrom @ 2011-05-19 10:19 UTC (permalink / raw)
To: Julian Anastasov, lvs-devel@vger.kernel.org
Cc: Simon Horman, Dave Jones, netdev@vger.kernel.org, Wensong Zhang
In-Reply-To: <alpine.LFD.2.00.1105191139360.2171@ja.ssi.bg>
On Thursday 19 May 2011 10:55:07 Julian Anastasov wrote:
>
> Hello,
>
> On Thu, 19 May 2011, Hans Schillstrom wrote:
>
> > I can reproduce the source of the problem,
> > use multiple netns and then unload the ftp module...
> > i.e. same list head used in multiple netns
> >
> > This brings up a question:
> > - How should ftp be handled in a netns ?
> > You might want to have it in one netns but not in another,
> > this requires changes to ipvsadm
> >
> > A way of doing it could be a disable switch like --noftp [port,port]
> > i.e. do not break old apps.
> >
> > Any other ideas ?
> >
> > This patch solves the root problem, I'm not sure if this is the way to go
> > or if we should split the ip_vs_app struct ?
>
> This patch is a fast fix but may be it is too late for it,
> after 2.6.39 is out. It seems we overlooked the apps when
> migrating to netns. I think, the apps do not need to be pernet.
> If one day application needs pernet context we can add such
> fields in the ipvs structure.
If we talk about the long term solution,
applications that affects other netns should have their own data.
ex. like the ftp, ports should be per netns
ipvsadm --appftp [port list]
>
> While the protocols have controls that manipulate pernet
> timeouts, the apps do not have such controls about app->timeouts.
> May be we can remove app->timeouts to avoid confusion because
> it was never implemented in user space. May be instead of this
> fix we should restore the global ip_vs_app_list and all things in
> ip_vs_app.c and ip_vs_ftp.c as before the netns changes?
Doesn't matter, just keep the patch as simple as possible
while we discuss the long term solution.
>
> > If it's the way to go I can send it as a proper formated patch ...
> > (after some testing)
> >
> > diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
> > index 4fff432..481f856 100644
> > --- a/include/net/ip_vs.h
> > +++ b/include/net/ip_vs.h
> > @@ -797,7 +797,8 @@ struct netns_ipvs {
> > struct list_head rs_table[IP_VS_RTAB_SIZE];
> > /* ip_vs_app */
> > struct list_head app_list;
> > -
> > + /* ip_vs_ftp */
> > + struct ip_vs_app *ftp_app;
> > /* ip_vs_proto */
> > #define IP_VS_PROTO_TAB_SIZE 32 /* must be power of 2 */
> > struct ip_vs_proto_data *proto_data_table[IP_VS_PROTO_TAB_SIZE];
> > diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
> > index 6b5dd6d..17afb09 100644
> > --- a/net/netfilter/ipvs/ip_vs_ftp.c
> > +++ b/net/netfilter/ipvs/ip_vs_ftp.c
> > @@ -411,25 +411,36 @@ static struct ip_vs_app ip_vs_ftp = {
> > static int __net_init __ip_vs_ftp_init(struct net *net)
> > {
> > int i, ret;
> > - struct ip_vs_app *app = &ip_vs_ftp;
> > + struct ip_vs_app *app;
> > + struct netns_ipvs *ipvs = net_ipvs(net);
> > +
> > + app = kmemdup(&ip_vs_ftp, sizeof(struct ip_vs_app), GFP_KERNEL);
> > + if (!app)
> > + return -ENOMEM;
> > + INIT_LIST_HEAD(&app->a_list);
> > + INIT_LIST_HEAD(&app->incs_list);
> > + INIT_LIST_HEAD(&app->p_list);
> > + ipvs->ftp_app = app;
> >
> > ret = register_ip_vs_app(net, app);
> > if (ret)
> > - return ret;
> > + goto err_exit;
> >
> > for (i=0; i<IP_VS_APP_MAX_PORTS; i++) {
> > if (!ports[i])
> > continue;
> > ret = register_ip_vs_app_inc(net, app, app->protocol, ports[i]);
> > if (ret)
> > - break;
> > + goto err_unreg;
> > pr_info("%s: loaded support on port[%d] = %d\n",
> > app->name, i, ports[i]);
> > }
> > + return 0;
> >
> > - if (ret)
> > - unregister_ip_vs_app(net, app);
> > -
> > +err_unreg:
> > + unregister_ip_vs_app(net, app);
> > +err_exit:
> > + kfree(ipvs->ftp_app);
> > return ret;
> > }
> > /*
> > @@ -437,9 +448,7 @@ static int __net_init __ip_vs_ftp_init(struct net *net)
> > */
> > static void __ip_vs_ftp_exit(struct net *net)
> > {
> > - struct ip_vs_app *app = &ip_vs_ftp;
> > -
> > - unregister_ip_vs_app(net, app);
> > + unregister_ip_vs_app(net, net_ipvs(net)->ftp_app);
> > }
> >
> > static struct pernet_operations ip_vs_ftp_ops = {
>
> Regards
>
> --
> Julian Anastasov <ja@ssi.bg>
>
--
Regards
Hans Schillstrom <hans.schillstrom@ericsson.com>
^ permalink raw reply
* [Patch] bridge: call NETDEV_ENSLAVE notifiers when adding a slave
From: Amerigo Wang @ 2011-05-19 10:24 UTC (permalink / raw)
To: linux-kernel
Cc: akpm, WANG Cong, Neil Horman, Jay Vosburgh, Stephen Hemminger,
David S. Miller, netdev, bridge
In-Reply-To: <1305794393-20775-1-git-send-email-amwang@redhat.com>
In the previous patch I added NETDEV_ENSLAVE, now
we can notify netconsole when adding a device to a bridge too.
By the way, s/netdev_bonding_change/call_netdevice_notifiers/ in
bond_main.c, since this is not bonding specific.
Signed-off-by: WANG Cong <amwang@redhat.com>
Cc: Neil Horman <nhorman@redhat.com>
---
drivers/net/bonding/bond_main.c | 2 +-
net/bridge/br_if.c | 2 ++
2 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index b9c70c5..765fdcf 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1640,7 +1640,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
}
}
- netdev_bonding_change(slave_dev, NETDEV_ENSLAVE);
+ call_netdevice_notifiers(NETDEV_ENSLAVE, slave_dev);
/* If this is the first slave, then we need to set the master's hardware
* address to be the same as the slave's. */
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 5dbdfdf..b44fae5 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -338,6 +338,8 @@ int br_add_if(struct net_bridge *br, struct net_device *dev)
if (IS_ERR(p))
return PTR_ERR(p);
+ call_netdevice_notifiers(NETDEV_ENSLAVE, dev);
+
err = dev_set_promiscuity(dev, 1);
if (err)
goto put_back;
^ permalink raw reply related
* Re: [Patch net-next-2.6] netpoll: disable netpoll when enslave a device
From: Neil Horman @ 2011-05-19 11:03 UTC (permalink / raw)
To: Cong Wang
Cc: Neil Horman, linux-kernel, Jay Vosburgh, Andy Gospodarek,
David S. Miller, Alexey Dobriyan, Ferenc Wagner, Andrew Morton,
Paul E. McKenney, Josh Triplett, Ian Campbell, netdev
In-Reply-To: <4DD4A6F9.4010002@redhat.com>
On Thu, May 19, 2011 at 01:13:29PM +0800, Cong Wang wrote:
> 于 2011年05月18日 18:56, Neil Horman 写道:
> >On Wed, May 18, 2011 at 06:00:35PM +0800, Amerigo Wang wrote:
> ...
> >>- case NETDEV_GOING_DOWN:
> >> case NETDEV_BONDING_DESLAVE:
> >>+ case NETDEV_ENSLAVE:
> >> nt->enabled = 0;
> >> stopped = true;
> >> break;
> >This wasn't introduced by this patch, but looking at it made me realize that
> >nt->enabled, if it passes through this code path, doesn't properly track weather
> >or not netpoll_setup has been called on this interface. If you look at
> >drop_netconsole_target, you'll see we only call netpoll_cleanup_target if
> >nt->enabled is set. We should probably change the nt->enabled check there, and
> >in store_enabled to be if (nt->np.dev), like we do in the NETDEV_UNREGISTER case
> >in netconsole_netdev_event.
>
> Yeah, also note that we can change ->enabled via configfs too.
> I guess we probably need to fix this in another patch...
>
Yeah, or you can roll it into this one, I think this is the only location that
needs fixing.
>
> >>+#define NETDEV_ENSLAVE 0x0014
> >>
> >Nit:
> >Shouldn't this be NETDEV_BONDING_ENSLAVE, to keep it in line with
> >NETDEV_BONDING_DESLAVE above?
>
> Actually that is my first thought, but I plan to use this in bridge
> case too, because using netconsole on a device underlying a bridge
> makes little sense too. Thus, I prefer NETDEV_ENSLAVE to
> NETDEV_BONDING_ENSLAVE.
>
That seems reasonable, but if its going to be more generic, could you change
NETDEV_BONDING_DESLAVE to NETDEV_DESLAVE?
> >
> >> #define SYS_DOWN 0x0001 /* Notify of system down */
> >> #define SYS_RESTART SYS_DOWN
> >>
> >
> >
> >Other than those two points, this looks good to me
>
> Thanks for review.
Thank you!
^ permalink raw reply
* [PATCH net-next-2.6] ipv6: reduce per device ICMP mib sizes
From: Eric Dumazet @ 2011-05-19 11:14 UTC (permalink / raw)
To: Denys Fedoryshchenko, David Miller; +Cc: netdev
In-Reply-To: <1305788113.3019.19.camel@edumazet-laptop>
Le jeudi 19 mai 2011 à 08:55 +0200, Eric Dumazet a écrit :
> Looking at snmp6_alloc_dev(), we allocate three mib per device :
>
> ipstats_mib (30 * sizeof(u64) * number_of_possible_cpus)
> icmpv6_mib (4 * sizeof(long) * number_of_possible_cpus)
> icmpv6msg_mib (26 * sizeof(long))
>
Oops, I forgot that mibs were doubled (one set for USER, one set for BH)
And :
#define __ICMP6MSG_MIB_MAX 512
So icmpv6msg_mib is really 512*sizeof(long)*number_of_possible_cpus*2
32 kbytes per device on a 8cpu machine, 32bit kernel.
Plus all other mibs... yes thats way too big for a seldom used stuff.
Here is patch I cooked and tested on my machine :
[PATCH net-next-2.6] ipv6: reduce per device ICMP mib sizes.
ipv6 has per device ICMP SNMP counters, taking too much space because
they use percpu storage.
needed size per device is :
(512+4)*sizeof(long)*number_of_possible_cpus*2
On a 32bit kernel, 16 possible cpus, this wastes more than 64kbytes of
memory per ipv6 enabled network device, taken in vmalloc pool.
Since ICMP messages are rare, just use shared counters (atomic_long_t)
Per network space ICMP counters are still using percpu memory, we might
also convert them to shared counters in a future patch.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
CC: Denys Fedoryshchenko <denys@visp.net.lb>
---
include/net/if_inet6.h | 4 +--
include/net/ipv6.h | 19 +++++++++++++-----
include/net/snmp.h | 14 +++++++++++++
net/ipv6/addrconf.c | 24 +++++++++++------------
net/ipv6/proc.c | 40 +++++++++++++++++++++++++--------------
5 files changed, 68 insertions(+), 33 deletions(-)
diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h
index 0c603fe..11cf373 100644
--- a/include/net/if_inet6.h
+++ b/include/net/if_inet6.h
@@ -154,8 +154,8 @@ struct ifacaddr6 {
struct ipv6_devstat {
struct proc_dir_entry *proc_dir_entry;
DEFINE_SNMP_STAT(struct ipstats_mib, ipv6);
- DEFINE_SNMP_STAT(struct icmpv6_mib, icmpv6);
- DEFINE_SNMP_STAT(struct icmpv6msg_mib, icmpv6msg);
+ DEFINE_SNMP_STAT_ATOMIC(struct icmpv6_mib_device, icmpv6dev);
+ DEFINE_SNMP_STAT_ATOMIC(struct icmpv6msg_mib_device, icmpv6msgdev);
};
struct inet6_dev {
diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index e1c60b4..c033ed0 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -123,6 +123,15 @@ extern struct ctl_path net_ipv6_ctl_path[];
SNMP_INC_STATS##modifier((net)->mib.statname##_statistics, (field));\
})
+/* per device counters are atomic_long_t */
+#define _DEVINCATOMIC(net, statname, modifier, idev, field) \
+({ \
+ struct inet6_dev *_idev = (idev); \
+ if (likely(_idev != NULL)) \
+ SNMP_INC_STATS_ATOMIC_LONG((_idev)->stats.statname##dev, (field)); \
+ SNMP_INC_STATS##modifier((net)->mib.statname##_statistics, (field));\
+})
+
#define _DEVADD(net, statname, modifier, idev, field, val) \
({ \
struct inet6_dev *_idev = (idev); \
@@ -154,16 +163,16 @@ extern struct ctl_path net_ipv6_ctl_path[];
#define IP6_UPD_PO_STATS_BH(net, idev,field,val) \
_DEVUPD(net, ipv6, 64_BH, idev, field, val)
#define ICMP6_INC_STATS(net, idev, field) \
- _DEVINC(net, icmpv6, , idev, field)
+ _DEVINCATOMIC(net, icmpv6, , idev, field)
#define ICMP6_INC_STATS_BH(net, idev, field) \
- _DEVINC(net, icmpv6, _BH, idev, field)
+ _DEVINCATOMIC(net, icmpv6, _BH, idev, field)
#define ICMP6MSGOUT_INC_STATS(net, idev, field) \
- _DEVINC(net, icmpv6msg, , idev, field +256)
+ _DEVINCATOMIC(net, icmpv6msg, , idev, field +256)
#define ICMP6MSGOUT_INC_STATS_BH(net, idev, field) \
- _DEVINC(net, icmpv6msg, _BH, idev, field +256)
+ _DEVINCATOMIC(net, icmpv6msg, _BH, idev, field +256)
#define ICMP6MSGIN_INC_STATS_BH(net, idev, field) \
- _DEVINC(net, icmpv6msg, _BH, idev, field)
+ _DEVINCATOMIC(net, icmpv6msg, _BH, idev, field)
struct ip6_ra_chain {
struct ip6_ra_chain *next;
diff --git a/include/net/snmp.h b/include/net/snmp.h
index 27461d6..479083a 100644
--- a/include/net/snmp.h
+++ b/include/net/snmp.h
@@ -72,14 +72,24 @@ struct icmpmsg_mib {
/* ICMP6 (IPv6-ICMP) */
#define ICMP6_MIB_MAX __ICMP6_MIB_MAX
+/* per network ns counters */
struct icmpv6_mib {
unsigned long mibs[ICMP6_MIB_MAX];
};
+/* per device counters, (shared on all cpus) */
+struct icmpv6_mib_device {
+ atomic_long_t mibs[ICMP6_MIB_MAX];
+};
#define ICMP6MSG_MIB_MAX __ICMP6MSG_MIB_MAX
+/* per network ns counters */
struct icmpv6msg_mib {
unsigned long mibs[ICMP6MSG_MIB_MAX];
};
+/* per device counters, (shared on all cpus) */
+struct icmpv6msg_mib_device {
+ atomic_long_t mibs[ICMP6MSG_MIB_MAX];
+};
/* TCP */
@@ -114,6 +124,8 @@ struct linux_xfrm_mib {
*/
#define DEFINE_SNMP_STAT(type, name) \
__typeof__(type) __percpu *name[2]
+#define DEFINE_SNMP_STAT_ATOMIC(type, name) \
+ __typeof__(type) *name
#define DECLARE_SNMP_STAT(type, name) \
extern __typeof__(type) __percpu *name[2]
@@ -124,6 +136,8 @@ struct linux_xfrm_mib {
__this_cpu_inc(mib[0]->mibs[field])
#define SNMP_INC_STATS_USER(mib, field) \
this_cpu_inc(mib[1]->mibs[field])
+#define SNMP_INC_STATS_ATOMIC_LONG(mib, field) \
+ atomic_long_inc(&mib->mibs[field])
#define SNMP_INC_STATS(mib, field) \
this_cpu_inc(mib[!in_softirq()]->mibs[field])
#define SNMP_DEC_STATS(mib, field) \
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index f2f9b2e..3cfbbf3 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -289,19 +289,19 @@ static int snmp6_alloc_dev(struct inet6_dev *idev)
sizeof(struct ipstats_mib),
__alignof__(struct ipstats_mib)) < 0)
goto err_ip;
- if (snmp_mib_init((void __percpu **)idev->stats.icmpv6,
- sizeof(struct icmpv6_mib),
- __alignof__(struct icmpv6_mib)) < 0)
+ idev->stats.icmpv6dev = kzalloc(sizeof(struct icmpv6_mib_device),
+ GFP_KERNEL);
+ if (!idev->stats.icmpv6dev)
goto err_icmp;
- if (snmp_mib_init((void __percpu **)idev->stats.icmpv6msg,
- sizeof(struct icmpv6msg_mib),
- __alignof__(struct icmpv6msg_mib)) < 0)
+ idev->stats.icmpv6msgdev = kzalloc(sizeof(struct icmpv6msg_mib_device),
+ GFP_KERNEL);
+ if (!idev->stats.icmpv6msgdev)
goto err_icmpmsg;
return 0;
err_icmpmsg:
- snmp_mib_free((void __percpu **)idev->stats.icmpv6);
+ kfree(idev->stats.icmpv6dev);
err_icmp:
snmp_mib_free((void __percpu **)idev->stats.ipv6);
err_ip:
@@ -310,8 +310,8 @@ err_ip:
static void snmp6_free_dev(struct inet6_dev *idev)
{
- snmp_mib_free((void __percpu **)idev->stats.icmpv6msg);
- snmp_mib_free((void __percpu **)idev->stats.icmpv6);
+ kfree(idev->stats.icmpv6msgdev);
+ kfree(idev->stats.icmpv6dev);
snmp_mib_free((void __percpu **)idev->stats.ipv6);
}
@@ -3838,7 +3838,7 @@ static inline size_t inet6_if_nlmsg_size(void)
+ nla_total_size(inet6_ifla6_size()); /* IFLA_PROTINFO */
}
-static inline void __snmp6_fill_stats(u64 *stats, void __percpu **mib,
+static inline void __snmp6_fill_statsdev(u64 *stats, atomic_long_t *mib,
int items, int bytes)
{
int i;
@@ -3848,7 +3848,7 @@ static inline void __snmp6_fill_stats(u64 *stats, void __percpu **mib,
/* Use put_unaligned() because stats may not be aligned for u64. */
put_unaligned(items, &stats[0]);
for (i = 1; i < items; i++)
- put_unaligned(snmp_fold_field(mib, i), &stats[i]);
+ put_unaligned(atomic_long_read(&mib[i]), &stats[i]);
memset(&stats[items], 0, pad);
}
@@ -3877,7 +3877,7 @@ static void snmp6_fill_stats(u64 *stats, struct inet6_dev *idev, int attrtype,
IPSTATS_MIB_MAX, bytes, offsetof(struct ipstats_mib, syncp));
break;
case IFLA_INET6_ICMP6STATS:
- __snmp6_fill_stats(stats, (void __percpu **)idev->stats.icmpv6, ICMP6_MIB_MAX, bytes);
+ __snmp6_fill_statsdev(stats, idev->stats.icmpv6dev->mibs, ICMP6_MIB_MAX, bytes);
break;
}
}
diff --git a/net/ipv6/proc.c b/net/ipv6/proc.c
index 24b3558..18ff5df 100644
--- a/net/ipv6/proc.c
+++ b/net/ipv6/proc.c
@@ -141,7 +141,11 @@ static const struct snmp_mib snmp6_udplite6_list[] = {
SNMP_MIB_SENTINEL
};
-static void snmp6_seq_show_icmpv6msg(struct seq_file *seq, void __percpu **mib)
+/* can be called either with percpu mib (pcpumib != NULL),
+ * or shared one (smib != NULL)
+ */
+static void snmp6_seq_show_icmpv6msg(struct seq_file *seq, void __percpu **pcpumib,
+ atomic_long_t *smib)
{
char name[32];
int i;
@@ -158,14 +162,14 @@ static void snmp6_seq_show_icmpv6msg(struct seq_file *seq, void __percpu **mib)
snprintf(name, sizeof(name), "Icmp6%s%s",
i & 0x100 ? "Out" : "In", p);
seq_printf(seq, "%-32s\t%lu\n", name,
- snmp_fold_field(mib, i));
+ pcpumib ? snmp_fold_field(pcpumib, i) : atomic_long_read(smib + i));
}
/* print by number (nonzero only) - ICMPMsgStat format */
for (i = 0; i < ICMP6MSG_MIB_MAX; i++) {
unsigned long val;
- val = snmp_fold_field(mib, i);
+ val = pcpumib ? snmp_fold_field(pcpumib, i) : atomic_long_read(smib + i);
if (!val)
continue;
snprintf(name, sizeof(name), "Icmp6%sType%u",
@@ -174,14 +178,22 @@ static void snmp6_seq_show_icmpv6msg(struct seq_file *seq, void __percpu **mib)
}
}
-static void snmp6_seq_show_item(struct seq_file *seq, void __percpu **mib,
+/* can be called either with percpu mib (pcpumib != NULL),
+ * or shared one (smib != NULL)
+ */
+static void snmp6_seq_show_item(struct seq_file *seq, void __percpu **pcpumib,
+ atomic_long_t *smib,
const struct snmp_mib *itemlist)
{
int i;
+ unsigned long val;
- for (i = 0; itemlist[i].name; i++)
- seq_printf(seq, "%-32s\t%lu\n", itemlist[i].name,
- snmp_fold_field(mib, itemlist[i].entry));
+ for (i = 0; itemlist[i].name; i++) {
+ val = pcpumib ?
+ snmp_fold_field(pcpumib, itemlist[i].entry) :
+ atomic_long_read(smib + itemlist[i].entry);
+ seq_printf(seq, "%-32s\t%lu\n", itemlist[i].name, val);
+ }
}
static void snmp6_seq_show_item64(struct seq_file *seq, void __percpu **mib,
@@ -201,13 +213,13 @@ static int snmp6_seq_show(struct seq_file *seq, void *v)
snmp6_seq_show_item64(seq, (void __percpu **)net->mib.ipv6_statistics,
snmp6_ipstats_list, offsetof(struct ipstats_mib, syncp));
snmp6_seq_show_item(seq, (void __percpu **)net->mib.icmpv6_statistics,
- snmp6_icmp6_list);
+ NULL, snmp6_icmp6_list);
snmp6_seq_show_icmpv6msg(seq,
- (void __percpu **)net->mib.icmpv6msg_statistics);
+ (void __percpu **)net->mib.icmpv6msg_statistics, NULL);
snmp6_seq_show_item(seq, (void __percpu **)net->mib.udp_stats_in6,
- snmp6_udp6_list);
+ NULL, snmp6_udp6_list);
snmp6_seq_show_item(seq, (void __percpu **)net->mib.udplite_stats_in6,
- snmp6_udplite6_list);
+ NULL, snmp6_udplite6_list);
return 0;
}
@@ -229,11 +241,11 @@ static int snmp6_dev_seq_show(struct seq_file *seq, void *v)
struct inet6_dev *idev = (struct inet6_dev *)seq->private;
seq_printf(seq, "%-32s\t%u\n", "ifIndex", idev->dev->ifindex);
- snmp6_seq_show_item(seq, (void __percpu **)idev->stats.ipv6,
+ snmp6_seq_show_item(seq, (void __percpu **)idev->stats.ipv6, NULL,
snmp6_ipstats_list);
- snmp6_seq_show_item(seq, (void __percpu **)idev->stats.icmpv6,
+ snmp6_seq_show_item(seq, NULL, idev->stats.icmpv6dev->mibs,
snmp6_icmp6_list);
- snmp6_seq_show_icmpv6msg(seq, (void __percpu **)idev->stats.icmpv6msg);
+ snmp6_seq_show_icmpv6msg(seq, NULL, idev->stats.icmpv6msgdev->mibs);
return 0;
}
^ permalink raw reply related
* Re: [PATCH net-next-2.6] ipv6: reduce per device ICMP mib sizes
From: Denys Fedoryshchenko @ 2011-05-19 11:26 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <1305803663.3028.39.camel@edumazet-laptop>
On Thu, 19 May 2011 13:14:23 +0200, Eric Dumazet wrote:
> Le jeudi 19 mai 2011 à 08:55 +0200, Eric Dumazet a écrit :
>
>> Looking at snmp6_alloc_dev(), we allocate three mib per device :
>>
>> ipstats_mib (30 * sizeof(u64) * number_of_possible_cpus)
>> icmpv6_mib (4 * sizeof(long) * number_of_possible_cpus)
>> icmpv6msg_mib (26 * sizeof(long))
>>
>
> Oops, I forgot that mibs were doubled (one set for USER, one set for
> BH)
>
> And :
> #define __ICMP6MSG_MIB_MAX 512
>
> So icmpv6msg_mib is really 512*sizeof(long)*number_of_possible_cpus*2
>
> 32 kbytes per device on a 8cpu machine, 32bit kernel.
>
> Plus all other mibs... yes thats way too big for a seldom used stuff.
>
> Here is patch I cooked and tested on my machine :
>
> [PATCH net-next-2.6] ipv6: reduce per device ICMP mib sizes.
I'll test it tonight, thanks a lot :-)
I guess it will help also for people with lot of interfaces
(virtualisation?), not only ppp.
^ permalink raw reply
* Re: [V2 Patch net-next-2.6] netpoll: disable netpoll when enslave a device
From: Andy Gospodarek @ 2011-05-19 11:31 UTC (permalink / raw)
To: Amerigo Wang
Cc: linux-kernel, akpm, Neil Horman, Jay Vosburgh, David S. Miller,
Ian Campbell, Paul E. McKenney, Josh Triplett, netdev
In-Reply-To: <1305794393-20775-1-git-send-email-amwang@redhat.com>
On Thu, May 19, 2011 at 04:39:53PM +0800, Amerigo Wang wrote:
[...]
> diff --git a/include/linux/notifier.h b/include/linux/notifier.h
> index 621dfa1..3d82867 100644
> --- a/include/linux/notifier.h
> +++ b/include/linux/notifier.h
> @@ -211,6 +211,7 @@ static inline int notifier_to_errno(int ret)
> #define NETDEV_UNREGISTER_BATCH 0x0011
> #define NETDEV_BONDING_DESLAVE 0x0012
> #define NETDEV_NOTIFY_PEERS 0x0013
> +#define NETDEV_ENSLAVE 0x0014
>
> #define SYS_DOWN 0x0001 /* Notify of system down */
> #define SYS_RESTART SYS_DOWN
Neil just noted the same concern I had -- the asymmetry between
NETDEV_ENSLAVE and NETDEV_BONDING_DESLAVE bothers me a bit. I also
don't really like the followup patch that uses 'ENSLAVE' in the bridging
code when we typically use that language for bonding only.
What about changing NETDEV_BONDING_DESLAVE to NETDEV_RELEASE and create
NETDEV_JOIN instead of NETDEV_ENSLAVE? I would prefer that or something
else that might use more generic language that could be applied to all
for stacked interfaces.
^ permalink raw reply
* (unknown)
From: Stella @ 2011-05-19 11:44 UTC (permalink / raw)
Please help me contact my lawyer. I got this
letter from Mrs. Helen saying you should contact
her lawyer for help, so that you can be her
beneficiary.
*************************************
Lawyer Contact Address
Barr. Morgan Owen
Email: bar.morganowen@yahoo.co.uk
*************************************
^ permalink raw reply
* (unknown)
From: Stella @ 2011-05-19 11:46 UTC (permalink / raw)
Please help me contact my lawyer. I got this
letter from Mrs. Helen saying you should contact
her lawyer for help, so that you can be her
beneficiary.
*************************************
Lawyer Contact Address
Barr. Morgan Owen
Email: bar.morganowen@yahoo.co.uk
*************************************
^ permalink raw reply
* (unknown)
From: Stella @ 2011-05-19 11:46 UTC (permalink / raw)
Please help me contact my lawyer. I got this
letter from Mrs. Helen saying you should contact
her lawyer for help, so that you can be her
beneficiary.
*************************************
Lawyer Contact Address
Barr. Morgan Owen
Email: bar.morganowen@yahoo.co.uk
*************************************
^ permalink raw reply
* (unknown)
From: Stella @ 2011-05-19 11:46 UTC (permalink / raw)
Please help me contact my lawyer. I got this
letter from Mrs. Helen saying you should contact
her lawyer for help, so that you can be her
beneficiary.
*************************************
Lawyer Contact Address
Barr. Morgan Owen
Email: bar.morganowen@yahoo.co.uk
*************************************
^ permalink raw reply
* [GIT PULL nf-2.6] IPVS
From: Simon Horman @ 2011-05-19 12:32 UTC (permalink / raw)
To: lvs-devel, netdev, netfilter-devel, netfilter
Cc: Wensong Zhang, Julian Anastasov, Patrick McHardy,
Hans Schillstrom, Dave Jones, Pablo Neira Ayuso
Hi Pablo, Hi Patrick,
please consider pulling
git://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs-2.6.git master
to get the following fix for IPVS.
I have based my tree on net-2.6 as it contains all of the IPVS patches -
currently some are in Patrick's tree and one of them is in Pablo's tree but
all of them are in Dave's tree.
Please feel free to apply the patch manually if that suits you better.
Simon Horman (1):
IPVS: Free resources on module removal
net/netfilter/ipvs/ip_vs_ctl.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
^ permalink raw reply
* [PATCH] IPVS: Free resources on module removal
From: Simon Horman @ 2011-05-19 12:32 UTC (permalink / raw)
To: lvs-devel, netdev, netfilter-devel, netfilter
Cc: Wensong Zhang, Julian Anastasov, Patrick McHardy,
Hans Schillstrom, Dave Jones, Pablo Neira Ayuso, Simon Horman
In-Reply-To: <1305808377-12255-1-git-send-email-horms@verge.net.au>
Reported-by: Dave Jones <davej@redhat.com>
Acked-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Hans Schillstrom <hans.schillstrom@ericsson.com>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
net/netfilter/ipvs/ip_vs_ctl.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 37890f2..9b9039b 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -3774,6 +3774,7 @@ err_sock:
void ip_vs_control_cleanup(void)
{
EnterFunction(2);
+ unregister_netdevice_notifier(&ip_vs_dst_notifier);
ip_vs_genl_unregister();
nf_unregister_sockopt(&ip_vs_sockopts);
LeaveFunction(2);
--
1.7.4.4
^ permalink raw reply related
* Re: [RFC/PATCH 00/13] wl12xx re-factor
From: Luciano Coelho @ 2011-05-19 12:49 UTC (permalink / raw)
To: Felipe Balbi; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <1305321990-22041-1-git-send-email-balbi@ti.com>
On Sat, 2011-05-14 at 00:26 +0300, Felipe Balbi wrote:
> Hi Luca,
Hi Felipe,
> this is the re-factor I was talking to you
> about. Please have a look and give your
> comments.
>
> It probably won't work as is, I compile
> tested only, but it shows the idea.
This looks very good! I think we should do something like this to avoid
the code that is duplicated in the bus modules.
But, as I already mentioned briefly on IRC, there is a problem with the
way you changed the platform data structure, because it will break
compat-wireless. The actual memory and data that is used by the
platform data is in the board components and not part of the wireless
subsystem. With compat-wireless, we need to make sure that new stuff
works with older kernels. In your patches you modify the platform data
structure, so when we run an old kernel with new compat-wireless, things
will break.
We already found a similar bug due to a previous change in the platform
data structure, so I don't want this to happen again. So for now, I'll
keep these patches aside, but as soon as we find a good solution, I'll
definitely use your ideas here (or ask you to rebase :P).
I'll probably apply some of the patches that are not related to the
platform data change. I'll respond to those specific patches
separately.
--
Cheers,
Luca.
^ permalink raw reply
* Re: [V2 Patch net-next-2.6] netpoll: disable netpoll when enslave a device
From: Neil Horman @ 2011-05-19 13:25 UTC (permalink / raw)
To: Andy Gospodarek
Cc: Amerigo Wang, linux-kernel, akpm, Jay Vosburgh, David S. Miller,
Ian Campbell, Paul E. McKenney, Josh Triplett, netdev
In-Reply-To: <20110519113127.GE21309@gospo.rdu.redhat.com>
On Thu, May 19, 2011 at 07:31:27AM -0400, Andy Gospodarek wrote:
> On Thu, May 19, 2011 at 04:39:53PM +0800, Amerigo Wang wrote:
> [...]
> > diff --git a/include/linux/notifier.h b/include/linux/notifier.h
> > index 621dfa1..3d82867 100644
> > --- a/include/linux/notifier.h
> > +++ b/include/linux/notifier.h
> > @@ -211,6 +211,7 @@ static inline int notifier_to_errno(int ret)
> > #define NETDEV_UNREGISTER_BATCH 0x0011
> > #define NETDEV_BONDING_DESLAVE 0x0012
> > #define NETDEV_NOTIFY_PEERS 0x0013
> > +#define NETDEV_ENSLAVE 0x0014
> >
> > #define SYS_DOWN 0x0001 /* Notify of system down */
> > #define SYS_RESTART SYS_DOWN
>
> Neil just noted the same concern I had -- the asymmetry between
> NETDEV_ENSLAVE and NETDEV_BONDING_DESLAVE bothers me a bit. I also
> don't really like the followup patch that uses 'ENSLAVE' in the bridging
> code when we typically use that language for bonding only.
>
> What about changing NETDEV_BONDING_DESLAVE to NETDEV_RELEASE and create
> NETDEV_JOIN instead of NETDEV_ENSLAVE? I would prefer that or something
> else that might use more generic language that could be applied to all
> for stacked interfaces.
JOIN and RELEASE (or perhaps LEAVE) sounds good to me.
Neil
^ permalink raw reply
* [RFC PATCH v3 ethtool] ethtool: implement [GS]FEATURES calls
From: Michał Mirosław @ 2011-05-19 13:25 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev, David Miller
In-Reply-To: <20110519091833.GA24484@rere.qmqm.pl>
This is all-in-one PoC patch for [GS]FEATURES support and checking of
all feature changes when altering some.
Example result:
icybox:~# ./ethtool -K ge0 tx_checksum-ipv6 on
feature group tx is enabled (expected: disabled)
feature group sg is enabled (expected: disabled)
feature group gso is enabled (expected: disabled)
feature tx-scatter-gather is enabled (expected: disabled, saved: enabled)
feature tx-generic-segmentation is enabled (expected: disabled, saved: enabled)
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
ethtool.c | 583 +++++++++++++++++++++++++++++++++++++++++++++++++++----------
1 files changed, 494 insertions(+), 89 deletions(-)
diff --git a/ethtool.c b/ethtool.c
index 34fe107..40456bb 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -33,6 +33,7 @@
#include <limits.h>
#include <ctype.h>
+#include <unistd.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>
@@ -83,6 +84,8 @@ static int do_gcoalesce(int fd, struct ifreq *ifr);
static int do_scoalesce(int fd, struct ifreq *ifr);
static int do_goffload(int fd, struct ifreq *ifr);
static int do_soffload(int fd, struct ifreq *ifr);
+static void parse_sfeatures_args(int argc, char **argp, int argi);
+static int do_gfeatures(int fd, struct ifreq *ifr);
static int do_gstats(int fd, struct ifreq *ifr);
static int rxflow_str_to_type(const char *str);
static int parse_rxfhashopts(char *optstr, u32 *data);
@@ -196,7 +199,8 @@ static struct option {
" [ txvlan on|off ]\n"
" [ ntuple on|off ]\n"
" [ rxhash on|off ]\n"
- },
+ " [ feature-name on|off [...] ]\n"
+ " see --show-offload output for feature-name strings\n" },
{ "-i", "--driver", MODE_GDRV, "Show driver information" },
{ "-d", "--register-dump", MODE_GREGS, "Do a register dump",
" [ raw on|off ]\n"
@@ -296,7 +300,6 @@ 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;
@@ -306,6 +309,9 @@ 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 int n_feature_strings;
+static const char **feature_strings;
+static struct ethtool_sfeatures *features_req;
static struct ethtool_pauseparam epause;
static int gpause_changed = 0;
@@ -851,10 +857,7 @@ static void parse_cmdline(int argc, char **argp)
break;
}
if (mode == MODE_SOFFLOAD) {
- parse_generic_cmdline(argc, argp, i,
- &goffload_changed,
- cmdline_offload,
- ARRAY_SIZE(cmdline_offload));
+ parse_sfeatures_args(argc, argp, i);
i = argc;
break;
}
@@ -1788,9 +1791,15 @@ 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)
+struct offload_state {
+ int rx, tx, sg, tso, ufo, gso, gro, lro, rxvlan, txvlan, ntuple, rxhash;
+};
+
+const char *const old_feature_names[] = {
+ "rx", "tx", "sg", "tso", "ufo", "gso", "gro", "lro", "rxvlan", "txvlan", "ntuple", "rxhash"
+};
+
+static int dump_offload(const struct offload_state *offload)
{
fprintf(stdout,
"rx-checksumming: %s\n"
@@ -1805,18 +1814,18 @@ static int dump_offload(int rx, int tx, int sg, int tso, int ufo, int gso,
"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");
+ offload->rx ? "on" : "off",
+ offload->tx ? "on" : "off",
+ offload->sg ? "on" : "off",
+ offload->tso ? "on" : "off",
+ offload->ufo ? "on" : "off",
+ offload->gso ? "on" : "off",
+ offload->gro ? "on" : "off",
+ offload->lro ? "on" : "off",
+ offload->rxvlan ? "on" : "off",
+ offload->txvlan ? "on" : "off",
+ offload->ntuple ? "on" : "off",
+ offload->rxhash ? "on" : "off");
return 0;
}
@@ -1867,21 +1876,33 @@ static int dump_rxfhash(int fhash, u64 val)
return 0;
}
-static int doit(void)
-{
- struct ifreq ifr;
- int fd;
+static int control_fd = -1;
+static int get_control_socket(struct ifreq *ifr)
+{
/* Setup our control structures. */
- memset(&ifr, 0, sizeof(ifr));
- strcpy(ifr.ifr_name, devname);
+ memset(ifr, 0, sizeof(*ifr));
+ strcpy(ifr->ifr_name, devname);
+
+ if (control_fd >= 0)
+ return control_fd;
/* Open control socket. */
- fd = socket(AF_INET, SOCK_DGRAM, 0);
- if (fd < 0) {
+ control_fd = socket(AF_INET, SOCK_DGRAM, 0);
+ if (control_fd < 0)
perror("Cannot get control socket");
+
+ return control_fd;
+}
+
+static int doit(void)
+{
+ struct ifreq ifr;
+ int fd;
+
+ fd = get_control_socket(&ifr);
+ if (fd < 0)
return 70;
- }
/* all of these are expected to populate ifr->ifr_data as needed */
if (mode == MODE_GDRV) {
@@ -2139,14 +2160,13 @@ static int do_scoalesce(int fd, struct ifreq *ifr)
return 0;
}
-static int do_goffload(int fd, struct ifreq *ifr)
+static int send_goffloads(int fd, struct ifreq *ifr,
+ struct offload_state *offload)
{
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;
- fprintf(stdout, "Offload parameters for %s:\n", devname);
+ memset(offload, 0, sizeof(*offload));
eval.cmd = ETHTOOL_GRXCSUM;
ifr->ifr_data = (caddr_t)&eval;
@@ -2154,7 +2174,7 @@ static int do_goffload(int fd, struct ifreq *ifr)
if (err)
perror("Cannot get device rx csum settings");
else {
- rx = eval.data;
+ offload->rx = eval.data;
allfail = 0;
}
@@ -2164,7 +2184,7 @@ static int do_goffload(int fd, struct ifreq *ifr)
if (err)
perror("Cannot get device tx csum settings");
else {
- tx = eval.data;
+ offload->tx = eval.data;
allfail = 0;
}
@@ -2174,7 +2194,7 @@ static int do_goffload(int fd, struct ifreq *ifr)
if (err)
perror("Cannot get device scatter-gather settings");
else {
- sg = eval.data;
+ offload->sg = eval.data;
allfail = 0;
}
@@ -2184,7 +2204,7 @@ static int do_goffload(int fd, struct ifreq *ifr)
if (err)
perror("Cannot get device tcp segmentation offload settings");
else {
- tso = eval.data;
+ offload->tso = eval.data;
allfail = 0;
}
@@ -2194,7 +2214,7 @@ static int do_goffload(int fd, struct ifreq *ifr)
if (err)
perror("Cannot get device udp large send offload settings");
else {
- ufo = eval.data;
+ offload->ufo = eval.data;
allfail = 0;
}
@@ -2204,7 +2224,7 @@ static int do_goffload(int fd, struct ifreq *ifr)
if (err)
perror("Cannot get device generic segmentation offload settings");
else {
- gso = eval.data;
+ offload->gso = eval.data;
allfail = 0;
}
@@ -2214,11 +2234,11 @@ 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;
+ offload->lro = (eval.data & ETH_FLAG_LRO) != 0;
+ offload->rxvlan = (eval.data & ETH_FLAG_RXVLAN) != 0;
+ offload->txvlan = (eval.data & ETH_FLAG_TXVLAN) != 0;
+ offload->ntuple = (eval.data & ETH_FLAG_NTUPLE) != 0;
+ offload->rxhash = (eval.data & ETH_FLAG_RXHASH) != 0;
allfail = 0;
}
@@ -2228,130 +2248,515 @@ static int do_goffload(int fd, struct ifreq *ifr)
if (err)
perror("Cannot get device GRO settings");
else {
- gro = eval.data;
+ offload->gro = eval.data;
allfail = 0;
}
+ return -allfail;
+}
+
+static int do_goffload(int fd, struct ifreq *ifr)
+{
+ struct offload_state offload;
+ int err, allfail;
+
+ allfail = send_goffloads(fd, ifr, &offload);
+
+ if (!allfail) {
+ fprintf(stdout, "Offload parameters for %s:\n", devname);
+
+ dump_offload(&offload);
+ }
+
+ err = do_gfeatures(fd, ifr);
+ if (!err)
+ allfail = 0;
+
if (allfail) {
fprintf(stdout, "no offload info available\n");
return 83;
}
- return dump_offload(rx, tx, sg, tso, ufo, gso, gro, lro, rxvlan, txvlan,
- ntuple, rxhash);
+ return 0;
}
-static int do_soffload(int fd, struct ifreq *ifr)
+static int send_soffloads(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) {
+ if (err)
perror("Cannot set device rx csum settings");
- return 84;
- }
+ else
+ changed = 1;
}
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) {
+ if (err)
perror("Cannot set device tx csum settings");
- return 85;
- }
+ else
+ changed = 1;
}
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) {
+ if (err)
perror("Cannot set device scatter-gather settings");
- return 86;
- }
+ else
+ changed = 1;
}
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) {
+ if (err)
perror("Cannot set device tcp segmentation offload settings");
- return 88;
- }
+ else
+ changed = 1;
}
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) {
+ if (err)
perror("Cannot set device udp large send offload settings");
- return 89;
- }
+ else
+ changed = 1;
}
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) {
+ if (err)
perror("Cannot set device generic segmentation offload settings");
- return 90;
- }
+ else
+ changed = 1;
}
if (off_flags_mask) {
- changed = 1;
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 = ((eval.data & ~off_flags_mask) |
- off_flags_wanted);
-
- err = ioctl(fd, SIOCETHTOOL, ifr);
- if (err) {
- perror("Cannot set device flag settings");
- return 92;
+ } else {
+ eval.cmd = ETHTOOL_SFLAGS;
+ eval.data = ((eval.data & ~off_flags_mask) |
+ off_flags_wanted);
+
+ err = ioctl(fd, SIOCETHTOOL, ifr);
+ if (err)
+ perror("Cannot set device flag settings");
+ else
+ changed = 1;
}
}
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) {
+ if (err)
perror("Cannot set device GRO settings");
- return 93;
+ else
+ changed = 1;
+ }
+
+ return changed;
+}
+
+static int get_feature_strings(int fd, struct ifreq *ifr,
+ struct ethtool_gstrings **strs)
+{
+ struct ethtool_sset_info *sset_info;
+ struct ethtool_gstrings *strings;
+ int sz_str, n_strings, err;
+
+ sset_info = malloc(sizeof(struct ethtool_sset_info) + sizeof(u32));
+ sset_info->cmd = ETHTOOL_GSSET_INFO;
+ sset_info->sset_mask = (1ULL << ETH_SS_FEATURES);
+ ifr->ifr_data = (caddr_t)sset_info;
+ err = send_ioctl(fd, ifr);
+
+ n_strings = sset_info->data[0];
+ free(sset_info);
+
+ if ((err < 0) ||
+ (!(sset_info->sset_mask & (1ULL << ETH_SS_FEATURES))) ||
+ (n_strings == 0)) {
+ return -100;
+ }
+
+ sz_str = n_strings * ETH_GSTRING_LEN;
+ strings = calloc(1, sz_str + sizeof(struct ethtool_gstrings));
+ if (!strings) {
+ fprintf(stderr, "no memory available\n");
+ exit(95);
+ }
+
+ strings->cmd = ETHTOOL_GSTRINGS;
+ strings->string_set = ETH_SS_FEATURES;
+ strings->len = n_strings;
+ ifr->ifr_data = (caddr_t) strings;
+ err = send_ioctl(fd, ifr);
+ if (err < 0) {
+ perror("Cannot get feature strings information");
+ free(strings);
+ exit(96);
+ }
+
+ *strs = strings;
+ return n_strings;
+}
+
+static int init_feature_strings(void)
+{
+ struct ethtool_gstrings *strings;
+ struct ifreq ifr;
+ int fd, i, n;
+
+ if (feature_strings)
+ return n_feature_strings;
+
+ fd = get_control_socket(&ifr);
+ if (fd < 0)
+ exit(100);
+
+ n = get_feature_strings(fd, &ifr, &strings);
+
+ if (n < 0)
+ return n;
+
+ n_feature_strings = n;
+ feature_strings = calloc(n, sizeof(*feature_strings));
+ if (!feature_strings) {
+ fprintf(stderr, "no memory available for string table [size=%d]\n", n);
+ exit(95);
+ }
+
+ for (i = 0; i < n; ++i) {
+ if (!strings->data[i*ETH_GSTRING_LEN])
+ continue;
+
+ feature_strings[i] = strndup(
+ (const char *)&strings->data[i * ETH_GSTRING_LEN],
+ ETH_GSTRING_LEN);
+
+ if (!feature_strings[i]) {
+ fprintf(stderr, "no memory available for a string\n");
+ exit(95);
}
}
+ free(strings);
+ return n;
+}
+
+static void parse_sfeatures_args(int argc, char **argp, int argi)
+{
+ struct cmdline_info *cmdline_desc, *cp;
+ int sz_features, i;
+ int changed = 0;
+
+ if (init_feature_strings() < 0) {
+ /* ETHTOOL_GFEATURES unavailable */
+ parse_generic_cmdline(argc, argp, argi, &changed,
+ cmdline_offload, ARRAY_SIZE(cmdline_offload));
+ return;
+ }
+
+ sz_features = sizeof(*features_req->features) * ((n_feature_strings + 31) / 32);
+
+ cp = cmdline_desc = calloc(n_feature_strings + ARRAY_SIZE(cmdline_offload),
+ sizeof(*cmdline_desc));
+ memcpy(cp, cmdline_offload, sizeof(cmdline_offload));
+ cp += ARRAY_SIZE(cmdline_offload);
+
+ features_req = calloc(1, sizeof(*features_req) + sz_features);
+ if (!cmdline_desc || !features_req) {
+ fprintf(stderr, "no memory available\n");
+ exit(95);
+ }
+
+ features_req->size = (n_feature_strings + 31) / 32;
+
+ for (i = 0; i < n_feature_strings; ++i) {
+ if (!feature_strings[i])
+ continue;
+
+ cp->name = feature_strings[i];
+ cp->type = CMDL_FLAG;
+ cp->flag_val = 1 << (i % 32);
+ cp->wanted_val = &features_req->features[i / 32].requested;
+ cp->seen_val = &features_req->features[i / 32].valid;
+ ++cp;
+ }
+
+ parse_generic_cmdline(argc, argp, argi, &changed,
+ cmdline_desc, cp - cmdline_desc);
+
+ free(cmdline_desc);
+
+ if (!changed) {
+ free(features_req);
+ features_req = NULL;
+ }
+}
+
+static int send_gfeatures(int fd, struct ifreq *ifr,
+ struct ethtool_gfeatures **features_p)
+{
+ struct ethtool_gfeatures *features;
+ int err, sz_features;
+
+ sz_features = sizeof(*features->features) * ((n_feature_strings + 31) / 32);
+ features = calloc(1, sz_features + sizeof(*features));
+ if (!features) {
+ fprintf(stderr, "no memory available\n");
+ return 95;
+ }
+
+ features->cmd = ETHTOOL_GFEATURES;
+ features->size = (n_feature_strings + 31) / 32;
+ ifr->ifr_data = (caddr_t) features;
+ err = send_ioctl(fd, ifr);
+
+ if (err < 0) {
+ perror("Cannot get feature status");
+ free(features);
+ return 97;
+ }
+
+ *features_p = features;
+ return 0;
+}
+
+static int do_gfeatures(int fd, struct ifreq *ifr)
+{
+ struct ethtool_gfeatures *features;
+ int err, i;
+
+ err = init_feature_strings();
+ if (err < 0)
+ return -err;
+
+ err = send_gfeatures(fd, ifr, &features);
+ if (err)
+ return err;
+
+ fprintf(stdout, "\nFull offload state: (feature-name: active,wanted,changable)\n");
+ for (i = 0; i < n_feature_strings; i++) {
+ if (!feature_strings[i])
+ continue; /* empty */
+#define P_FLAG(f) \
+ (features->features[i / 32].f & (1 << (i % 32))) ? "yes" : " no"
+#define PA_FLAG(f) \
+ (features->features[i / 32].available & (1 << (i % 32))) ? P_FLAG(f) : "---"
+#define PN_FLAG(f) \
+ (features->features[i / 32].never_changed & (1 << (i % 32))) ? "---" : P_FLAG(f)
+ fprintf(stdout, " %-*.*s %s,%s,%s\n",
+ ETH_GSTRING_LEN, ETH_GSTRING_LEN, feature_strings[i],
+ P_FLAG(active), PA_FLAG(requested), PN_FLAG(available));
+#undef P_FLAG
+#undef PA_FLAG
+#undef PN_FLAG
+ }
+ free(features);
+
+ return 0;
+}
+
+static void print_gfeatures_diff(
+ const struct ethtool_get_features_block *expected,
+ const struct ethtool_get_features_block *set,
+ const char **strings, int n_strings)
+{
+ int i;
+
+ if (n_strings > 32)
+ n_strings = 32;
+
+ for (i = 0; i < n_strings; ++i) {
+ u32 mask = 1 << i;
+
+ if (!strings[i])
+ continue;
+
+ if (!((expected->active ^ set->active) & mask))
+ continue;
+
+ fprintf(stderr, "feature %.*s is %s (expected: %s, saved: %s)\n",
+ ETH_GSTRING_LEN, strings[i],
+ set->active & mask ? "enabled" : "disabled",
+ expected->active & mask ? "enabled" : "disabled",
+ !(set->available & mask) ? "not user-changeable" :
+ set->requested & mask ? "enabled" : "disabled"
+ );
+ }
+}
+
+static int get_offload_state(int fd, struct ifreq *ifr,
+ struct ethtool_gfeatures **features,
+ struct offload_state *offload)
+{
+ int err, allfail;
+
+ allfail = send_goffloads(fd, ifr, offload);
+
+ err = init_feature_strings();
+ if (err < 0)
+ return allfail ? err : 0;
+
+ err = send_gfeatures(fd, ifr, features);
+ if (err)
+ perror("Cannot read features");
+
+ return allfail ? -err : 0;
+}
+
+static int send_sfeatures(int fd, struct ifreq *ifr)
+{
+ int err;
+
+ features_req->cmd = ETHTOOL_SFEATURES;
+ ifr->ifr_data = (caddr_t) features_req;
+ err = send_ioctl(fd, ifr);
+ if (err < 0) {
+ perror("Cannot change features");
+ return 97;
+ }
+
+ return 0;
+}
+
+static void compare_offload_state(struct offload_state *offload0,
+ struct offload_state *offload1)
+{
+ int *expected = (int *)offload0, *new = (int *)offload1;
+ int i;
+
+ if (off_csum_rx_wanted >= 0)
+ offload0->rx = off_csum_rx_wanted;
+
+ if (off_csum_tx_wanted >= 0)
+ offload0->tx = off_csum_tx_wanted;
+
+ if (off_sg_wanted >= 0)
+ offload0->sg = off_sg_wanted;
+
+ if (off_tso_wanted >= 0)
+ offload0->tso = off_tso_wanted;
+
+ if (off_ufo_wanted >= 0)
+ offload0->ufo = off_ufo_wanted;
+
+ if (off_gso_wanted >= 0)
+ offload0->gso = off_gso_wanted;
+
+ if (off_gro_wanted >= 0)
+ offload0->gro = off_gro_wanted;
+
+ if (off_flags_mask & ETH_FLAG_LRO)
+ offload0->lro = !!(off_flags_wanted & ETH_FLAG_LRO);
+
+ if (off_flags_mask & ETH_FLAG_RXVLAN)
+ offload0->rxvlan = !!(off_flags_wanted & ETH_FLAG_RXVLAN);
+
+ if (off_flags_mask & ETH_FLAG_TXVLAN)
+ offload0->txvlan = !!(off_flags_wanted & ETH_FLAG_TXVLAN);
+
+ if (off_flags_mask & ETH_FLAG_NTUPLE)
+ offload0->ntuple = !!(off_flags_wanted & ETH_FLAG_NTUPLE);
+
+ if (off_flags_mask & ETH_FLAG_RXHASH)
+ offload0->rxhash = !!(off_flags_wanted & ETH_FLAG_RXHASH);
+
+ for (i = 0; i < ARRAY_SIZE(old_feature_names); i++) {
+ if (expected[i] == new[i])
+ continue;
+
+ fprintf(stderr, "feature group %s is %s (expected: %s)\n",
+ old_feature_names[i],
+ new[i] ? "enabled" : "disabled",
+ expected[i] ? "enabled" : "disabled"
+ );
+ }
+}
+
+static void compare_features(struct ethtool_gfeatures *features0,
+ struct ethtool_gfeatures *features1)
+{
+ int i;
+
+ /* make features0 .active what we expect to be set */
+ i = (n_feature_strings + 31) / 32;
+ while (i--) {
+ features0->features[i].active &= ~features_req->features[i].valid;
+ features0->features[i].active |=
+ features_req->features[i].requested &
+ features_req->features[i].valid;
+ }
+
+ for (i = 0; i < n_feature_strings; i += 32)
+ print_gfeatures_diff(&features0->features[i / 32],
+ &features1->features[i / 32],
+ feature_strings + i,
+ n_feature_strings - i);
+}
+
+static int do_soffload(int fd, struct ifreq *ifr)
+{
+ struct ethtool_gfeatures *features_old, *features_new;
+ struct offload_state offload_old, offload_new;
+ int err, changed;
+
+ err = get_offload_state(fd, ifr, &features_old, &offload_old);
+ if (err)
+ return -err;
+
+ changed = send_soffloads(fd, ifr);
+
+ if (features_req) {
+ err = send_sfeatures(fd, ifr);
+ if (!err)
+ changed = 1;
+ }
+
if (!changed) {
fprintf(stdout, "no offload settings changed\n");
+ return err;
+ }
+
+ err = get_offload_state(fd, ifr, &features_new, &offload_new);
+ if (err) {
+ perror("can't verify offload setting");
+ return 101;
+ }
+ if ((!features_old) ^ (!features_new)) {
+ fprintf(stderr, "can't compare features (one GFEATURES failed)\n");
+ features_old = NULL;
}
+ compare_offload_state(&offload_old, &offload_new);
+ if (features_old)
+ compare_features(features_old, features_new);
+
return 0;
}
^ permalink raw reply related
* Re: [PATCH v2] CDC NCM: release interfaces fix in unbind()
From: Oliver Neukum @ 2011-05-19 13:40 UTC (permalink / raw)
To: Alexey ORISHKO
Cc: Alan Stern, gregkh-l3A5Bk7waGM@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org,
linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <2AC7D4AD8BA1C640B4C60C61C8E520153E3ACE0774-8ZTw5gFVCTjVH5byLeRTJxkTb7+GphCuwzqs5ZKRSiY@public.gmane.org>
Am Mittwoch, 18. Mai 2011, 19:11:37 schrieb Alexey ORISHKO:
> I wonder, if Greg or Oliver could provide any comments why master interface
> is not claimed in modem/ether drivers, since they are working with the
> code for quite a while.
Probe is called for the master interface. You claim only _additional_
interfaces. The problem is in disconnect(). You may be called in any order.
HTH
Oliver
--
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
* [PATCH net-next-2.6] netfilter: add more values to enum ip_conntrack_info
From: Eric Dumazet @ 2011-05-19 13:44 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Netfilter Development Mailinglist, netdev, David Miller
Following error is raised (and other similar ones) :
net/ipv4/netfilter/nf_nat_standalone.c: In function ‘nf_nat_fn’:
net/ipv4/netfilter/nf_nat_standalone.c:119:2: warning: case value ‘4’
not in enumerated type ‘enum ip_conntrack_info’
gcc barfs on adding two enum values and getting a not enumerated
result :
case IP_CT_RELATED+IP_CT_IS_REPLY:
Add missing enum values
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
CC: David Miller <davem@davemloft.net>
---
include/linux/netfilter/nf_conntrack_common.h | 3 +++
net/ipv4/netfilter/ipt_CLUSTERIP.c | 6 +++---
net/ipv4/netfilter/ipt_MASQUERADE.c | 2 +-
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c | 2 +-
net/ipv4/netfilter/nf_nat_core.c | 2 +-
net/ipv4/netfilter/nf_nat_rule.c | 2 +-
net/ipv4/netfilter/nf_nat_standalone.c | 4 ++--
net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c | 2 +-
net/netfilter/nf_conntrack_core.c | 4 ++--
net/netfilter/nf_conntrack_ftp.c | 2 +-
net/netfilter/nf_conntrack_h323_main.c | 10 ++++------
net/netfilter/nf_conntrack_irc.c | 3 +--
net/netfilter/nf_conntrack_pptp.c | 3 +--
net/netfilter/nf_conntrack_sane.c | 2 +-
net/netfilter/nf_conntrack_sip.c | 2 +-
net/netfilter/xt_socket.c | 4 ++--
16 files changed, 26 insertions(+), 27 deletions(-)
diff --git a/include/linux/netfilter/nf_conntrack_common.h b/include/linux/netfilter/nf_conntrack_common.h
index 50cdc25..0d3dd66 100644
--- a/include/linux/netfilter/nf_conntrack_common.h
+++ b/include/linux/netfilter/nf_conntrack_common.h
@@ -18,6 +18,9 @@ enum ip_conntrack_info {
/* >= this indicates reply direction */
IP_CT_IS_REPLY,
+ IP_CT_ESTABLISHED_REPLY = IP_CT_ESTABLISHED + IP_CT_IS_REPLY,
+ IP_CT_RELATED_REPLY = IP_CT_RELATED + IP_CT_IS_REPLY,
+ IP_CT_NEW_REPLY = IP_CT_NEW + IP_CT_IS_REPLY,
/* Number of distinct IP_CT types (no NEW in reply dirn). */
IP_CT_NUMBER = IP_CT_IS_REPLY * 2 - 1
};
diff --git a/net/ipv4/netfilter/ipt_CLUSTERIP.c b/net/ipv4/netfilter/ipt_CLUSTERIP.c
index d609ac3..5c9e97c 100644
--- a/net/ipv4/netfilter/ipt_CLUSTERIP.c
+++ b/net/ipv4/netfilter/ipt_CLUSTERIP.c
@@ -307,7 +307,7 @@ clusterip_tg(struct sk_buff *skb, const struct xt_action_param *par)
* error messages (RELATED) and information requests (see below) */
if (ip_hdr(skb)->protocol == IPPROTO_ICMP &&
(ctinfo == IP_CT_RELATED ||
- ctinfo == IP_CT_RELATED + IP_CT_IS_REPLY))
+ ctinfo == IP_CT_RELATED_REPLY))
return XT_CONTINUE;
/* ip_conntrack_icmp guarantees us that we only have ICMP_ECHO,
@@ -321,12 +321,12 @@ clusterip_tg(struct sk_buff *skb, const struct xt_action_param *par)
ct->mark = hash;
break;
case IP_CT_RELATED:
- case IP_CT_RELATED+IP_CT_IS_REPLY:
+ case IP_CT_RELATED_REPLY:
/* FIXME: we don't handle expectations at the
* moment. they can arrive on a different node than
* the master connection (e.g. FTP passive mode) */
case IP_CT_ESTABLISHED:
- case IP_CT_ESTABLISHED+IP_CT_IS_REPLY:
+ case IP_CT_ESTABLISHED_REPLY:
break;
default:
break;
diff --git a/net/ipv4/netfilter/ipt_MASQUERADE.c b/net/ipv4/netfilter/ipt_MASQUERADE.c
index d2ed9dc..9931152 100644
--- a/net/ipv4/netfilter/ipt_MASQUERADE.c
+++ b/net/ipv4/netfilter/ipt_MASQUERADE.c
@@ -60,7 +60,7 @@ masquerade_tg(struct sk_buff *skb, const struct xt_action_param *par)
nat = nfct_nat(ct);
NF_CT_ASSERT(ct && (ctinfo == IP_CT_NEW || ctinfo == IP_CT_RELATED ||
- ctinfo == IP_CT_RELATED + IP_CT_IS_REPLY));
+ ctinfo == IP_CT_RELATED_REPLY));
/* Source address is 0.0.0.0 - locally generated packet that is
* probably not supposed to be masqueraded.
diff --git a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
index 5a03c02..db10075 100644
--- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
+++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
@@ -101,7 +101,7 @@ static unsigned int ipv4_confirm(unsigned int hooknum,
/* This is where we call the helper: as the packet goes out. */
ct = nf_ct_get(skb, &ctinfo);
- if (!ct || ctinfo == IP_CT_RELATED + IP_CT_IS_REPLY)
+ if (!ct || ctinfo == IP_CT_RELATED_REPLY)
goto out;
help = nfct_help(ct);
diff --git a/net/ipv4/netfilter/nf_nat_core.c b/net/ipv4/netfilter/nf_nat_core.c
index 9c71b27..3346de5 100644
--- a/net/ipv4/netfilter/nf_nat_core.c
+++ b/net/ipv4/netfilter/nf_nat_core.c
@@ -433,7 +433,7 @@ int nf_nat_icmp_reply_translation(struct nf_conn *ct,
/* Must be RELATED */
NF_CT_ASSERT(skb->nfctinfo == IP_CT_RELATED ||
- skb->nfctinfo == IP_CT_RELATED+IP_CT_IS_REPLY);
+ skb->nfctinfo == IP_CT_RELATED_REPLY);
/* Redirects on non-null nats must be dropped, else they'll
start talking to each other without our translation, and be
diff --git a/net/ipv4/netfilter/nf_nat_rule.c b/net/ipv4/netfilter/nf_nat_rule.c
index 21c3042..733c9ab 100644
--- a/net/ipv4/netfilter/nf_nat_rule.c
+++ b/net/ipv4/netfilter/nf_nat_rule.c
@@ -53,7 +53,7 @@ ipt_snat_target(struct sk_buff *skb, const struct xt_action_param *par)
/* Connection must be valid and new. */
NF_CT_ASSERT(ct && (ctinfo == IP_CT_NEW || ctinfo == IP_CT_RELATED ||
- ctinfo == IP_CT_RELATED + IP_CT_IS_REPLY));
+ ctinfo == IP_CT_RELATED_REPLY));
NF_CT_ASSERT(par->out != NULL);
return nf_nat_setup_info(ct, &mr->range[0], IP_NAT_MANIP_SRC);
diff --git a/net/ipv4/netfilter/nf_nat_standalone.c b/net/ipv4/netfilter/nf_nat_standalone.c
index 7317bdf..483b76d 100644
--- a/net/ipv4/netfilter/nf_nat_standalone.c
+++ b/net/ipv4/netfilter/nf_nat_standalone.c
@@ -116,7 +116,7 @@ nf_nat_fn(unsigned int hooknum,
switch (ctinfo) {
case IP_CT_RELATED:
- case IP_CT_RELATED+IP_CT_IS_REPLY:
+ case IP_CT_RELATED_REPLY:
if (ip_hdr(skb)->protocol == IPPROTO_ICMP) {
if (!nf_nat_icmp_reply_translation(ct, ctinfo,
hooknum, skb))
@@ -144,7 +144,7 @@ nf_nat_fn(unsigned int hooknum,
default:
/* ESTABLISHED */
NF_CT_ASSERT(ctinfo == IP_CT_ESTABLISHED ||
- ctinfo == (IP_CT_ESTABLISHED+IP_CT_IS_REPLY));
+ ctinfo == IP_CT_ESTABLISHED_REPLY);
}
return nf_nat_packet(ct, ctinfo, hooknum, skb);
diff --git a/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c b/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c
index c8af58b..4111050 100644
--- a/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c
+++ b/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c
@@ -160,7 +160,7 @@ static unsigned int ipv6_confirm(unsigned int hooknum,
/* This is where we call the helper: as the packet goes out. */
ct = nf_ct_get(skb, &ctinfo);
- if (!ct || ctinfo == IP_CT_RELATED + IP_CT_IS_REPLY)
+ if (!ct || ctinfo == IP_CT_RELATED_REPLY)
goto out;
help = nfct_help(ct);
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 2e1c11f..0bd5689 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -850,7 +850,7 @@ resolve_normal_ct(struct net *net, struct nf_conn *tmpl,
/* It exists; we have (non-exclusive) reference. */
if (NF_CT_DIRECTION(h) == IP_CT_DIR_REPLY) {
- *ctinfo = IP_CT_ESTABLISHED + IP_CT_IS_REPLY;
+ *ctinfo = IP_CT_ESTABLISHED_REPLY;
/* Please set reply bit if this packet OK */
*set_reply = 1;
} else {
@@ -1143,7 +1143,7 @@ static void nf_conntrack_attach(struct sk_buff *nskb, struct sk_buff *skb)
/* This ICMP is in reverse direction to the packet which caused it */
ct = nf_ct_get(skb, &ctinfo);
if (CTINFO2DIR(ctinfo) == IP_CT_DIR_ORIGINAL)
- ctinfo = IP_CT_RELATED + IP_CT_IS_REPLY;
+ ctinfo = IP_CT_RELATED_REPLY;
else
ctinfo = IP_CT_RELATED;
diff --git a/net/netfilter/nf_conntrack_ftp.c b/net/netfilter/nf_conntrack_ftp.c
index e17cb7c..6f5801e 100644
--- a/net/netfilter/nf_conntrack_ftp.c
+++ b/net/netfilter/nf_conntrack_ftp.c
@@ -368,7 +368,7 @@ static int help(struct sk_buff *skb,
/* Until there's been traffic both ways, don't look in packets. */
if (ctinfo != IP_CT_ESTABLISHED &&
- ctinfo != IP_CT_ESTABLISHED + IP_CT_IS_REPLY) {
+ ctinfo != IP_CT_ESTABLISHED_REPLY) {
pr_debug("ftp: Conntrackinfo = %u\n", ctinfo);
return NF_ACCEPT;
}
diff --git a/net/netfilter/nf_conntrack_h323_main.c b/net/netfilter/nf_conntrack_h323_main.c
index 18b2ce5..f03c2d4 100644
--- a/net/netfilter/nf_conntrack_h323_main.c
+++ b/net/netfilter/nf_conntrack_h323_main.c
@@ -571,10 +571,9 @@ static int h245_help(struct sk_buff *skb, unsigned int protoff,
int ret;
/* Until there's been traffic both ways, don't look in packets. */
- if (ctinfo != IP_CT_ESTABLISHED &&
- ctinfo != IP_CT_ESTABLISHED + IP_CT_IS_REPLY) {
+ if (ctinfo != IP_CT_ESTABLISHED && ctinfo != IP_CT_ESTABLISHED_REPLY)
return NF_ACCEPT;
- }
+
pr_debug("nf_ct_h245: skblen = %u\n", skb->len);
spin_lock_bh(&nf_h323_lock);
@@ -1125,10 +1124,9 @@ static int q931_help(struct sk_buff *skb, unsigned int protoff,
int ret;
/* Until there's been traffic both ways, don't look in packets. */
- if (ctinfo != IP_CT_ESTABLISHED &&
- ctinfo != IP_CT_ESTABLISHED + IP_CT_IS_REPLY) {
+ if (ctinfo != IP_CT_ESTABLISHED && ctinfo != IP_CT_ESTABLISHED_REPLY)
return NF_ACCEPT;
- }
+
pr_debug("nf_ct_q931: skblen = %u\n", skb->len);
spin_lock_bh(&nf_h323_lock);
diff --git a/net/netfilter/nf_conntrack_irc.c b/net/netfilter/nf_conntrack_irc.c
index b394aa3..4f9390b 100644
--- a/net/netfilter/nf_conntrack_irc.c
+++ b/net/netfilter/nf_conntrack_irc.c
@@ -125,8 +125,7 @@ static int help(struct sk_buff *skb, unsigned int protoff,
return NF_ACCEPT;
/* Until there's been traffic both ways, don't look in packets. */
- if (ctinfo != IP_CT_ESTABLISHED &&
- ctinfo != IP_CT_ESTABLISHED + IP_CT_IS_REPLY)
+ if (ctinfo != IP_CT_ESTABLISHED && ctinfo != IP_CT_ESTABLISHED_REPLY)
return NF_ACCEPT;
/* Not a full tcp header? */
diff --git a/net/netfilter/nf_conntrack_pptp.c b/net/netfilter/nf_conntrack_pptp.c
index 0889448..2fd4565 100644
--- a/net/netfilter/nf_conntrack_pptp.c
+++ b/net/netfilter/nf_conntrack_pptp.c
@@ -519,8 +519,7 @@ conntrack_pptp_help(struct sk_buff *skb, unsigned int protoff,
u_int16_t msg;
/* don't do any tracking before tcp handshake complete */
- if (ctinfo != IP_CT_ESTABLISHED &&
- ctinfo != IP_CT_ESTABLISHED + IP_CT_IS_REPLY)
+ if (ctinfo != IP_CT_ESTABLISHED && ctinfo != IP_CT_ESTABLISHED_REPLY)
return NF_ACCEPT;
nexthdr_off = protoff;
diff --git a/net/netfilter/nf_conntrack_sane.c b/net/netfilter/nf_conntrack_sane.c
index d9e2773..8501823 100644
--- a/net/netfilter/nf_conntrack_sane.c
+++ b/net/netfilter/nf_conntrack_sane.c
@@ -78,7 +78,7 @@ static int help(struct sk_buff *skb,
ct_sane_info = &nfct_help(ct)->help.ct_sane_info;
/* Until there's been traffic both ways, don't look in packets. */
if (ctinfo != IP_CT_ESTABLISHED &&
- ctinfo != IP_CT_ESTABLISHED+IP_CT_IS_REPLY)
+ ctinfo != IP_CT_ESTABLISHED_REPLY)
return NF_ACCEPT;
/* Not a full tcp header? */
diff --git a/net/netfilter/nf_conntrack_sip.c b/net/netfilter/nf_conntrack_sip.c
index cb5a285..93faf6a 100644
--- a/net/netfilter/nf_conntrack_sip.c
+++ b/net/netfilter/nf_conntrack_sip.c
@@ -1423,7 +1423,7 @@ static int sip_help_tcp(struct sk_buff *skb, unsigned int protoff,
typeof(nf_nat_sip_seq_adjust_hook) nf_nat_sip_seq_adjust;
if (ctinfo != IP_CT_ESTABLISHED &&
- ctinfo != IP_CT_ESTABLISHED + IP_CT_IS_REPLY)
+ ctinfo != IP_CT_ESTABLISHED_REPLY)
return NF_ACCEPT;
/* No Data ? */
diff --git a/net/netfilter/xt_socket.c b/net/netfilter/xt_socket.c
index 9cc4635..fe39f7e 100644
--- a/net/netfilter/xt_socket.c
+++ b/net/netfilter/xt_socket.c
@@ -143,9 +143,9 @@ socket_match(const struct sk_buff *skb, struct xt_action_param *par,
ct = nf_ct_get(skb, &ctinfo);
if (ct && !nf_ct_is_untracked(ct) &&
((iph->protocol != IPPROTO_ICMP &&
- ctinfo == IP_CT_IS_REPLY + IP_CT_ESTABLISHED) ||
+ ctinfo == IP_CT_ESTABLISHED_REPLY) ||
(iph->protocol == IPPROTO_ICMP &&
- ctinfo == IP_CT_IS_REPLY + IP_CT_RELATED)) &&
+ ctinfo == IP_CT_RELATED_REPLY)) &&
(ct->status & IPS_SRC_NAT_DONE)) {
daddr = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.u3.ip;
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [RFC/PATCH 00/13] wl12xx re-factor
From: Felipe Balbi @ 2011-05-19 14:06 UTC (permalink / raw)
To: Luciano Coelho; +Cc: Felipe Balbi, linux-wireless, netdev, linux-kernel
In-Reply-To: <1305809345.12586.1579.camel@cumari>
[-- Attachment #1: Type: text/plain, Size: 1589 bytes --]
Hi,
On Thu, May 19, 2011 at 03:49:05PM +0300, Luciano Coelho wrote:
> > this is the re-factor I was talking to you
> > about. Please have a look and give your
> > comments.
> >
> > It probably won't work as is, I compile
> > tested only, but it shows the idea.
>
> This looks very good! I think we should do something like this to avoid
> the code that is duplicated in the bus modules.
>
> But, as I already mentioned briefly on IRC, there is a problem with the
> way you changed the platform data structure, because it will break
> compat-wireless. The actual memory and data that is used by the
> platform data is in the board components and not part of the wireless
> subsystem. With compat-wireless, we need to make sure that new stuff
> works with older kernels. In your patches you modify the platform data
> structure, so when we run an old kernel with new compat-wireless, things
> will break.
>
> We already found a similar bug due to a previous change in the platform
> data structure, so I don't want this to happen again. So for now, I'll
> keep these patches aside, but as soon as we find a good solution, I'll
> definitely use your ideas here (or ask you to rebase :P).
>
> I'll probably apply some of the patches that are not related to the
> platform data change. I'll respond to those specific patches
> separately.
ok good. I have stated my POV WRT compatibility layers for in-tree
drivers on a separate thread. In summary, I think those shouldn't exist
at all :-)
Just let me know if you need anything ;-)
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ 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